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

Contract supporting migration staging path #14

Merged
merged 40 commits into from
Feb 5, 2024
Merged

Conversation

sisyphusSmiling
Copy link
Contributor

@sisyphusSmiling sisyphusSmiling commented Jan 25, 2024

Closes: #13

The basic interface to stage a contract is the same as deploying a contract - name + code. See the stage_contract & unstage_contract transactions.

To stage a contract, the developer first saves a Host resource in their account which they pass as a reference along with the contract name and code they wish to stage. The Host reference simply serves as proof of authority that the caller has access to the contract-hosting account, which in the simplest case would be the signer of the staging transaction, though conceivably this could be delegated to some other account via Capability - possibly helpful for some multisig contract hosts.

Within the MigrationContractStaging contract account, code is saved on a contract-basis as a ContractUpdate struct within a Capsule resource and stored at a the derived path. The Capsule simply serves as a dedicated repository for staged contract code.

Included in the contact are methods for querying staging status and retrieval of staged code. This enables platforms like Flowview, Flowdiver, ContractBrowser, etc. to display the staging status of contracts on any given account.

To support ongoing staging progress across the network, the single StagingStatusUpdated event is emitted any time a contract is staged (status == "stage"), staged code is replaced (status == "replace"), or a contract is unstaged (status == "unstage").

@sisyphusSmiling sisyphusSmiling requested a review from a team as a code owner January 25, 2024 00:25
@sisyphusSmiling sisyphusSmiling marked this pull request as draft January 25, 2024 00:26
@sisyphusSmiling sisyphusSmiling changed the title Migration staging Contract supporting migration staging path Jan 25, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jan 25, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (dd81300) 74.30% compared to head (9551380) 82.22%.

Files Patch % Lines
contracts/MigrationContractStaging.cdc 96.29% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #14      +/-   ##
==========================================
+ Coverage   74.30%   82.22%   +7.91%     
==========================================
  Files           1        2       +1     
  Lines         144      225      +81     
==========================================
+ Hits          107      185      +78     
- Misses         37       40       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Show resolved Hide resolved
/// Event emitted when a contract's code is staged
/// status == true - insert | staged == false - replace | staged == nil - remove
/// NOTE: Does not guarantee that the contract code is valid Cadence
access(all) event StagingStatusUpdated(

Choose a reason for hiding this comment

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

🧡

/// Stages the contract code for the given contract name at the host address. If the contract is already staged,
/// the code will be replaced.
///
access(all) fun stageContract(host: &Host, name: String, code: String) {
Copy link

Choose a reason for hiding this comment

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

Should this guard against using a host that is not saved? Or is it not possible to get a ref to a host with it not beeing saved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, the Host must be saved as host.address() panics if owner == nil

contracts/MigrationContractStaging.cdc Show resolved Hide resolved
address: Address,
codeHash: [UInt8],
contract: String,
status: Bool?
Copy link

Choose a reason for hiding this comment

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

personally i think a string that just uses the words "insert", "replace", "remove" is better here then this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Updated to action field with values `"stage", "replace", and "unstage"

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice!

Mostly reviewed it from a Cadence code perspective, only reviewed the logic a little bit

contracts/MigrationContractStaging.cdc Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
}

execute {
self.admin.setStagingCutoff(atHeight: cutoff)
Copy link
Member

Choose a reason for hiding this comment

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

See above

Suggested change
self.admin.setStagingCutoff(atHeight: cutoff)
self.admin.setStagingCutoff(at: cutoff)

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Looks good!

Maybe just include the code as-is in the event, there isn't really a need to turn it into a byte array

contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
contracts/MigrationContractStaging.cdc Outdated Show resolved Hide resolved
@sisyphusSmiling sisyphusSmiling requested a review from a team February 5, 2024 19:05
@sisyphusSmiling sisyphusSmiling merged commit a7fffa0 into main Feb 5, 2024
1 check passed
@sisyphusSmiling sisyphusSmiling deleted the migration-staging branch February 5, 2024 20:28
@sisyphusSmiling sisyphusSmiling restored the migration-staging branch February 8, 2024 23:43
@sisyphusSmiling sisyphusSmiling deleted the migration-staging branch February 8, 2024 23:43
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.

Add contract supporting contract staging under contract migration plan
6 participants