Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(chain): Upgradability functionality #2701

Merged
merged 31 commits into from
Jun 8, 2020
Merged

Conversation

ilblackdragon
Copy link
Member

@ilblackdragon ilblackdragon commented May 21, 2020

Implemented next functionality:

To simplify reviewing, specific places:

Test plan

  1. All CI tests pass

Future plan

  1. Make backward_compatible.py test as mandatory starting now and they pass.
  2. New upgradable.py nightly test passes, after first release of this to beta.
  3. Remove state_migration.py test, as it's superseded by upgradable.py

@gitpod-io
Copy link

gitpod-io bot commented May 21, 2020

@gitpod-io
Copy link

gitpod-io bot commented May 23, 2020

@ilblackdragon
Copy link
Member Author

I've started with trying to make this backward compatible, but mid way realized that because previous version doesn't have upgradability voting on consensus level - even if it's backward compatible there is no consensus way to understand when supermajority should move to next version.

Hence, this update will still require a hard fork.

@ilblackdragon ilblackdragon marked this pull request as ready for review May 29, 2020 22:53
@ilblackdragon ilblackdragon requested a review from olonho as a code owner June 3, 2020 17:23
@gitpod-io
Copy link

gitpod-io bot commented Jun 3, 2020

chain/epoch_manager/src/lib.rs Outdated Show resolved Hide resolved
core/primitives/src/block.rs Show resolved Hide resolved
neard/src/lib.rs Outdated Show resolved Hide resolved
neard/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@bowenwang1996 bowenwang1996 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the Cargo.lock change intended?

@ilblackdragon
Copy link
Member Author

@bowenwang1996 reset Cargo.lock back to master.

@ilblackdragon
Copy link
Member Author

@bowenwang1996
Copy link
Collaborator

@ilblackdragon I cannot open that link.

@ilblackdragon
Copy link
Member Author

@bowenwang1996 Opens fine for me - just wait 30-40 seconds.

@SkidanovAlex
Copy link
Collaborator

It is unlikely that we will change the Block or BlockHeader, changes will likely go via ShardChunkHeader and BlockHeaderInner* (and couple other structs).

What is the plan once they change? If we change ShardChunkHeader at the moment when the current Block was BlockV5, is the idea to create a new struct ShardChunkHeaderV5 that will be included in BlockV5, and have ShardChunkHeader have the new structure? It will be pretty ugly in the code, given that ShardChunkHeader on itself is not versioned, so it's not an enum we can match in the places that receive ShardChunkHeader.

@ilblackdragon
Copy link
Member Author

The versioning is done on the level of protocol messages, e.g. what is sent around in the network in PeerMessage (https://github.com/nearprotocol/nearcore/blob/4a6fbd718c856b5d37b848df1b124eff9703d83f/chain/network/src/types.rs#L401). ShardChunkHeader is not part of it.

Copy link
Contributor

@MaksymZavershynskyi MaksymZavershynskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for genesis tools.

@ilblackdragon ilblackdragon merged commit b30864b into master Jun 8, 2020
@ilblackdragon ilblackdragon deleted the upgradability branch June 8, 2020 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgradable BlockHeader Nightly test: upgradability Migrate state and configs by default
7 participants