-
Notifications
You must be signed in to change notification settings - Fork 142
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
Initial spec for upgradability #64
Conversation
Your Render PR Server URL is https://nomicon-pr-64.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-bqtcomhj38f7norl6jbg. |
Your Render PR Server at https://nomicon-pr-64.onrender.com is now live! View it on your dashboard at https://dashboard.render.com/static/srv-bqtcomhj38f7norl6jbg. |
@abacabadabacaba suggested from Protocol Research call:
I generally, like the (1) & (3). Overall I don't fully agree with (4): even though we expect agreement in what next version will be, if there are any kind of conflicting change that people want to introduce - this mechanism will be the final way to decide. Obviously that with hash of spec reported by binary doesn't mean binary follows that spec or that there are no issues or that there are no issues in the spec. It's more to be able to handle forks in decision making. Let's review a simple example, there are group of validators who want to increase the minimum gas price, forked the code and pushed a new version. At the same time core dev team pushes new update of protocol with some bells & whistles. |
Linking for context: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should specify that the protocol version X must be backward compatible with at least protocol version X-1.
My opinion:
It would be very irresponsible for the validators to do this without any testing and coordination with the rest of the participants. Imagine what happens if a group of companies running some of the Internet core infrastructure decide to change something in TCP/IP protocol. They fork the software their infrastructure is running and roll out a new version before telling anyone else. Would it work? I don't think so. In our case, upgrading the consensus would require agreement of a supermajority of the validators, and this includes an agreement to not do any conflicting upgrade in parallel, which means that such situation with two upgrades happening at the same time shouldn't be possible. |
Interesting. It changes my initial intuition from the "voting for some specific update" to the "voting for (some) update".. Which actually makes sense to me from that perspective (especially b/c there should be only one canonical chain and since validators are not voting for some specific binary(es)). |
We can add it to telemetry. Forcing it as part of the protocol doesn't make a lot of sense. |
I don't think it's really a problem. In practice I don't see upgrades happening in two consecutive epochs, but it also makes sense to be more cautious. |
Yeah, that's fine.
Well, that was the point of using this as a final coordination mechanism. Social consensus is great and should be used as much as possible, but you also want to make sure to have final stops. TCP/IP is a bad example, because it's been 10+ years as they have been trying to update. We want to update every month the slowest.
This is already in telemetry - https://explorer.betanet.near.org/nodes/validators
The condition is 80% of validators indicated a switch, epoch after next will activate it. So the rest of validators have epoch+some to switch. We can add a bit more timing, but I also don't want to give infinite amount of time here, because in case of security updates etc - we want this to be timely. |
We can discuss this separately but I feel like for critical vulnerabilities I am not entirely sure that using this kind of voting is the best idea. |
Quick recap from research meeting:
|
Your Render PR Server at https://nomicon-pr-64.onrender.com is now live! View it on your dashboard at https://dashboard.render.com/static/srv-bqtcomhj38f7norl6jbg. |
A deploy for your Render PR Server at https://nomicon-pr-64.onrender.com just failed. View details on your dashboard at https://dashboard.render.com/static/srv-bqtcomhj38f7norl6jbg. |
A deploy for your Render PR Server at https://nomicon-pr-64.onrender.com just failed. View details on your dashboard at https://dashboard.render.com/static/srv-bqtcomhj38f7norl6jbg. |
Want to follow up and close the thread on upgradability spec. Specifically, I went back and forth, and even though @abacabadabacaba idea of using proposals to indicate next version makes sense - it’s more complicated to implement, more complicated for validators and doesn’t really change that much the behavior. It’s worth noting with more chunk producers (more shards) and hidden validators there indeed can be problem that if only block producers upgraded there is still large % of stake didn’t update yet. Given that, I kept the version for indicating about upcoming transition in block header, but now it aggregates total stake voting for the version and it checks K epochs before the current one to give everyone time. See details here: https://github.com/nearprotocol/NEPs/blob/3748234118ea504453b4eabb7eeee17cd978e9ba/specs/ChainSpec/Upgradability.md#consensus Additionally, I added example of how we can handle quite a few updates to data structures like this: https://github.com/nearprotocol/NEPs/blob/3748234118ea504453b4eabb7eeee17cd978e9ba/specs/ChainSpec/Upgradability.md#versioned-data-structures We will need to work more on this as we actually get real examples of protocol changes. |
A deploy for your Render PR Server at https://nomicon-pr-64.onrender.com just failed. View details on your dashboard at https://dashboard.render.com/static/srv-bqtcomhj38f7norl6jbg. |
Co-authored-by: Bowen Wang <bowenwang1996@users.noreply.github.com>
A deploy for your Render PR Server at https://nomicon-pr-64.onrender.com just failed. View details on your dashboard at https://dashboard.render.com/static/srv-bqtcomhj38f7norl6jbg. |
@ilblackdragon given the data structure you proposed enum VersionedBlockHeader {
BlockHeaderV1(BlockHeaderV1),
/// Current version, where `BlockHeader` is used internally for all operations.
BlockHeaderV2(BlockHeader),
} when a node with the new version enters the network, how do the other nodes (with old version) process the header produced by the new version? It seems that there has to be some logic to produce the block in the old version with the new version number. |
Node running new binary will keep producing old block headers until the version transitions to the new one. |
struct BlockHeaderInnerRest { | ||
... | ||
/// Latest version that current producing node binary is running on. | ||
version: ProtocolVersion, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just double checking for my understanding. This field represents the highest version number the node producing the block could support, but not the version of the block header itself, correct? The actual version of the block header would be determined based on the enum variant it deserializes to.
If that is the case I wonder if a different name should be used. Say proposed_version
or something like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct, I used latest_protocol_version
in implementation here - near/nearcore#2701
So there is a small alternation that makes it close to what @abacabadabacaba proposed, while allowing block producer to automatically do this. Instead of adding a field to the While by default Proposals from Note my self: make sure our proposals code validation checks invalid for proposals. |
@ilblackdragon when do chunk producers add it? Also this is not future-proof. What do we do when we have hidden validators? |
@bowenwang1996 Chunk producers add it when it's their turn to produce chunk and it's the first time they produce a chunk since their binary version changed. Hidden validators don't participate in either voting scheme, so the latest proposal is strictly more inclusive than original one (as it includes chunk producers on top of only block producers in the original proposal). |
Currently they are the same. If you are talking about future possibilities, you are assuming that every block producer is a chunk producer. |
This is outcome of discussions:
And in short provides smooth upgrading that is defined by % of validators switching to new version and reaching consensus within an epoch on that.