-
Notifications
You must be signed in to change notification settings - Fork 49
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
Execution Node Versioning Contract for Rolling Upgrades #310
Execution Node Versioning Contract for Rolling Upgrades #310
Conversation
…d interface. Needs testing
…ance methods in ENVersionBeacon contract
… boundaries + transaction changes
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.
Great work! I had no idea this contract was going to be so complex. I only reviewed the cadence code, so I'll trust you and the other reviewers to ensure that there is thorough unit tests.
transactions/executionNodeVersionBeacon/scripts/get_current_execution_node_version.cdc
Outdated
Show resolved
Hide resolved
...sactions/executionNodeVersionBeacon/scripts/get_current_execution_node_version_as_string.cdc
Outdated
Show resolved
Hide resolved
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.
Changes by @satyamakgec look good to me based on my understanding of the contract.
I can review that integration point, namely |
Sorry I messed up with the CI with a commit for a couple of typos 🤦 it seems that it's needed to run some go command to update the go tests like in this commit e7c3e1d please @satyamakgec do that before merging! Anyway due to branch protection rules we need @j1010001 approval for merging (Jan please if you are approving this right now fix the go tests, sorry about that!!) |
This PR addresses #308
Since this issue is high priority, I'm opening up this PR while I work on completing the test suite to get eyes on the contract's core functionality as well as supporting transactions & scripts.
On that note, you'll notice that the tests are written in JavaScript. That's because I'm not familiar with Go and wanted to prioritize getting the test done & tested.
Semantic Version Representation
With regards to representing versions,
Semver
is a struct containing attributes mapping to semantic versioning standards. The struct also contains some custom comparators & atoString()
method to make things easier at the script layer.In the design doc referenced in #308, there was the question of signifying to ENs that updates earlier than the defined block height are allowed. To address this, I included
isBackwardsCompatible: Bool
. The idea is that if aupcomingVersion.isBackwardsCompatible == true
, preemptive updates should be permissible. In a previous iteration, I thought of addingoldestCompatibleVersion: Semver
, but that lead to recursive construction and it'd be versions all the way down.Contract
Purpose
The ExecutionNodeVersionBeacon contract is meant to address the need to coordinate software versions across a number of execution nodes (EN). I wanted to make it as easy as possible to expose the current labeled version requirement & the next upcoming version boundary.
Usage by Execution Nodes
ENs can call
getCurrentExecutionNodeVersion()
to get the version they should be running at the moment in time the method is called.Similarly,
getNextVersionBoundaryPair()
will return an array containing:[blockHeightBoundary: UInt64, version: Semver]
if there is an upcoming version boundary defined. This tells them when they need to update and to what version.Alternatively, callers can simply get the
versionTable
viagetVersionTable()
and do comparison or verification themselves.Getters for other values including
versionUpdateBuffer
andversionUpdateBufferVariance
are present as well.Contract Administration
Administration of the contract is handled via the
ExecutionNodeVersionAdmin
resource. Key functionality can be seen below:Of course, adding to & deleting a version boundary from the table are pretty straight forward. Conditions around both of these actions protect the key expectation ENs have about version updates - that they will have at least
versionUpdateBuffer
number of blocks to react to a version update. Additionally, if this window is to change, it will only do so by a factor defined byversionUpdateBufferVariance
.The buffer and buffer variances can also be changed by the Admin, but the buffer must not bring the next block boundary within range the current block height. With this conditional, an edge case arose where the buffer is defined arbitrarily large and could not be reduced if an upcoming block boundary was defined. To prevent this, I added the buffer variance value so that it could at least be reduced by some factor.
Other Details
Note that
archivedBlockBoundaries
andupcomingBlockBoundaries
are included to maintain ordered arrays of historical and upcoming block boundaries. This made discovering upcoming version boundaries easier as well as determining compatibility of previous versions inisCompatibleVersion()
The method
archiveOldBlockBoundaries()
maintains these arrays based on the block height at the time it's called.