[Rolling upgrade] Create and deploy contract for synchronizing versions

[j1010001]

In order to synchronise EN version switchover, as described in a design document height to version map is to be stored as part of service account, and thus available to every node in an uniform manner.

It should be designed with https://app.zenhub.com/workspace/o/onflow/flow-go/issues/3047

This includes first actually defining and designing the best approach to the problem, and then implementing it.

Requirements

• We have to release code changes to ENs in between sporks which requires us to have cleanly coordinated code upgrades when a given block height is encountered.
• By having a smart contract maintaining a mapping between height to version we can tightly control when the switchover to a given code version is to happen
• Needs to check input to ensure that a version is not proposed for the immediate next 1000 blocks, eg a cool down period
• Should be part of the service account so that signers can be gathered for approving the change (will need to confirm the specifics)

[sisyphusSmiling]

Posting questions here to close comms loop on a convo I began with @m4ksio

A few clarifying questions:

1. The contract will be managed by the Flow Service account yes? I'm assuming we'll want administration of this versioning contract to be handled by some unique resource that can later be handed off to some governance contract. Does that sound right?
2. Do we want the version to live centrally in the contract or be accessible within a resource owned by the Execution Nodes? My sense is a central mapping would be best, that way ENs could simply check the contract rather than need their resources updated if they go down for some period of time
3. Is it safe to assume that all versions will be named in accordance with semver? (e.g. 2.1.0) I saw mentioned in a doc that version names may differ in ways that could make comparisons difficult

and his responses:

1. Good question, but I frankly... don't know. Is this how those things are usually done?
2. Yes, central place is best, since it will be identical for everyone
3. Yes, semver is fine. EN can have extra additions like v0.2.0-whatever but contract just needs a comparison so keeping `something like v0.2.0 is enough

I would imagine we want the ability to update versions in some admin resource in the interest of composability. E.g. the versioning process could conceivably be utilized by a future governance contract where the version is updated on consensus

Regarding version representation, thinking I'll represent version by some struct and introduce conditions between version increments that are in line with semver as mentioned above.

Current status: Prototyping in progress based on preliminary design docs

[sisyphusSmiling]

So far, I have a contract interface that I'd like to post here for visibility and feedback. Fundamentally, the approach is to separate state of the contract from the task of administration.

All ENs would know the contract address, and only need to concern themselves with maintaining the correct version and updating in time for a version rollover. At least for the time being, version updates could be handled by the Flow Service account or some other administrative account with checks around cooldown block height thresholds to provide ENs with sufficient notice of a coming update. Related events are emitted on updates to the height:table mapping.

Here's the interface:

/// This contract is used to defined block height and software version boundaries
/// for the software run by Execution Nodes.
pub contract ENVersionBeacon {

    pub enum ENVersionUpdateAction: UInt8 {
        pub case added
        pub case deleted
        pub case amended
    }
    
    /// Struct representing Execution Node software version
    /// along with helper functions
    pub struct ENVersion {
        /// Compnents defining a semantic version
        pub let major: UInt8
        pub let minor: UInt8
        pub let patch: UInt8
        
        /// Oldest compatitible version with this version
        ///
        /// Defining this can help with logic around updating before a target block is reached
        /// Such behavior may be desireable in the event breaking changes are released
        /// between versions
        pub let oldestCompatibleVersion: ENVersion

        init(major: UInt8, minor: UInt8, patch: UInt8, oldestCompatibleVersion: ENVersion)
        
        /// Returns version in Semver format (e.g. v<major>.<minor>.<patch>)
        /// as a String
        pub fun toString(): String

        /* Custom Comparators */

        /// Returns true if ENVersion is greater than
        /// passed ENVersion and false otherwise
        pub fun greaterThan(_ other: ENVersion): Bool

        /// Returns true if ENVersion is greater than or
        /// equal to passed ENVersion and false otherwise
        pub fun greaterThanOrEqualTo(_ other: ENVersion): Bool

        /// Returns true if ENVersion is less than
        /// passed ENVersion and false otherwise
        pub fun lessThan(_ other: ENVersion): Bool

        /// Returns true if ENVersion is less than or
        /// equal to passed ENVersion and false otherwise
        pub fun lessThanOrEqualTo(_ other: ENVersion): Bool

        /// Returns true if ENVersion is equal to passed
        /// ENVersion and false otherwise
        pub fun equalTo(_ other: ENVersion): Bool
    }

    /// Event emitted any time a new version is added to the version table
    pub event ENVersionTableUpdated(height: UInt64, version: ENVersion, action: ENVersionUpdateAction)

    /// Event emitted any time the version cooldown period is updated
    pub event ENVersionCooldownPeriodUpdated(newVersionCooldownPeriod: UInt64)

    /// Canonical storage path for ENVersionKeeper
    pub let ENVersionAdminStoragePath: StoragePath

    /// Table dictating minimum version - {blockHeight: ENVersion}
    access(contract) let versionTable: {UInt64: ENVersion}

    /// Number of blocks between height at updating and target height for version rollover
    access(contract) let versionCooldownPeriod: UInt64

    /// Admin interface that manages the Execution Node versioning
    /// maintained in this contract
    pub resource interface ENVersionAdmin {
        
        /// Update the minimum version to take effect at the given block height
        pub fun addMinimumVersion(targetblockHeight: UInt64, version: ENVersion) {
            pre {
                targetblockHeight > getCurrentBlock().height + ENVersionAdmin.versionCooldownPeriod: "Target block height occurs too soon to update EN version"
            }
            emit EMVersionTableUpdated(height: targetblockHeight, version: version, action: ENVersionAction.added)
        }

        /// Deletes the entry in versionTable at the passed block height
        /// Could be helpful in event of rollback
        pub fun deleteBlockTargetVersionMapping(blockHeight: UInt64, version: ENVersion) {
            pre {
                targetblockHeight > getCurrentBlock().height + ENVersionAdmin.versionCooldownPeriod: "Target block height occurs too soon to update EN version"
            }
            emit EMVersionTableUpdated(height: targetblockHeight, version: version, action: ENVersionAction.deleted)
        }

        pub fun updateVersionCooldownPeriod(cooldownInBlocks: UInt64) {
            emit ENVersionCooldownPeriodUpdated(newVersionCooldownPeriod: cooldownInBlocks)
        }
    }

    /// Returns the current cooldown period within which ENs
    /// can be assured version will not change
    pub fun getVersionCooldownPeriod(): UInt64

    /// Function that returns that minimum current version based on the current
    /// block height and version table mapping
    pub fun getCurrentMinimumENVersion(): ENVersion

    /// Returns a copy of the full historical versionTable
    pub fun getVersionTable(): {ENVersion: UInt64}

    /// Checks whether given version was compatible at the given block height
    pub fun isCompatibleENVersion(blockHeight: UInt64, version: ENVersion): Bool

    /// Returns 
    pub fun getNextVersion

    init() {
        self.versionTable = {}
        self.ENVersionAdminStoragePath = /storage/ENVersionAdmin

        self.account.save(<-create ENVersionAdmin(), to: self.ENVersionAdminStoragePath)
    }
}

[joshuahannan]

Looks like a good start! One piece of feedback I have is that you don't need to use EN as an abbreviation, you can just use ExecutionNode. Better to be clear with your naming in smart contracts.

@m4ksio @j1010001 What do you think about this proposal?

[satyamakgec]

@sisyphusSmiling Overall looks good to me.

Some nitpick / Feedback -

• Why do we need oldestCompatibleVersion as a part of ENVersion?
• I am not sure of this function naming i.e addMinimumVersion ?
• May want to avoid the use of enums as it can be a backburner during the upgrade of the contract (If we want to have an upgradeable contract).

[sisyphusSmiling]

Thank you both for the feedback. A couple clarifying points.

• Noted, I'll opt for more verbose naming for more clarity
• 'oldestCompatibleVersion' is included to address concerns brought up in a design doc. That is, how does a node operator know if it's okay for them to upgrade before the target block? Additionally, since execution outputs are deterministic and can be different between versions, maintaining record of oldest compatible version for each version seemed helpful from at least a historical perspective.
• Noted, I'll consider more explicit naming conventions for functions
• Re: enums - that's interesting. Should be easy enough to avoid use of enums

[janezpodhostnik]

The ENVersion struct is really just a semver struct. It might be better just naming the type SemVer
Might also be nice to add the other parts of semver: <valid semver> ::= <version core> "-" <pre-release> "+" <build> the pre-release part has been frequently used in the past.

Looks good otherwise.

[bluesign]

Few comments & questions

• Do we need semver functionality on chain ? ( version comparison etc )

• Scope seems like EN, but wouldn't be better to make this as a Network Version struct, with individual Role versions ?

• Is this cooldown related to deadline to upgrade ? ( Sorry design docs are not accessible for me ) Something like validBefore and validAfter can be useful maybe for versions.

• Are there any plans to make this Protocol Level requirement ? Nodes sending their version in messages, other node's rejecting depending on their version etc.

• oldestCompatibleVersion is a bit confusing

That is, how does a node operator know if it's okay for them to upgrade before the target block?
Is there a case early upgrade is problematic?

• Linking version to off-chain, changelog, spec etc can be useful for old versions.

• This feels like there can be a case for skipping a version, then upgrade to the next one. If it is the case I think this can be little tricky.

[sisyphusSmiling]

Re: @janezpodhostnik is there documentation I can refer to regarding version naming conventions, particularly relating to pre-releases?

Re: @bluesign

• That is a good question. I do think we need a standardized representation of semver, so a struct makes sense. The comparisons could be done off-chain, though I don't know if there's harm done by including comparators so long as we all agree on naming conventions. The comparators support some of the contract methods, namely isCompatibleENVersion() which would allow an EN to check that their version is current.
• I'll look into that. Semver certainly has broader applications than just execution nodes, so it does make sense to broaden the scope of application. I have had the thought that we could attach some metadata to a version resource that might include things like download URL and compiled binary hash for verifiability, but that's beyond the scope of this issue.
• Yes, that is what I meant by cooldown. Thanks, I'll reconsider the verbiage I used.
• From what I've seen, I don't believe so. Anyone more informed than myself can feel free to jump in and clarify if I'm wrong.
• Noted, I'll reconsider
• Agreed, though I think the emphasis for this task is delivery timeframe. As mentioned, I agree that there's a case for adding version metadata that could lead to interesting distributed CI/CD type functionality (just an idea atm). I could add in a releaseDetailsURL that would serve the purpose for the time being if that would be helpful historically and/or to the nodes in real-time
• I think what you're saying is that if an EN sees that their current version is compatible with the new target version, they may simply stay put instead of upgrading.

[sisyphusSmiling]

Posting an update on progress here:

I've finished working on the contract and scripts and have been working on JavaScript tests as I'm not familiar with Go. Due to some dependency issues in the flow-js-testing library which seem to be rooted in changes to flow-cadut, I haven't made as much progress as I would've hoped.

Planning on having these issues sorted and a PR up on Monday or Tuesday of next week

[sisyphusSmiling]

Made some progress a debug call yesterday to get beyond the blocking issue in flow-js-testing and am working to get testing done today. Will submit a PR as soon as that's done!

[sisyphusSmiling]

PR #310 was merged and should resolve this issue.