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

Transactions to upgrade epoch contracts and move epoch admin resources to the service account #243

Merged
merged 4 commits into from Jul 5, 2023

Conversation

joshuahannan
Copy link
Member

@joshuahannan joshuahannan commented May 9, 2023

Based on this post and this PR, we are planning on moving all the epoch admin resources from the staking account to the service account so they are all in one place.

We are also introducing delegator staking minimums in the same upgrade.

This adds the transactions to upgrade the smart contracts and move the resources, and the README with instructions.

The transaction to move resource will need to be signed with the old method because it requires a multisig from the staking and service account.

@joshuahannan
Copy link
Member Author

@janezpodhostnik Do we need to include moving the other heartbeat resource in this transaction? That is already stored in the service account, right?

@vishalchangrani I might need some help filling out the instructions for the multisig with the old method

@janezpodhostnik
Copy link
Contributor

@joshuahannan on mainnet the NodeVersion Heartbeat is already/still on the service account

Comment on lines 17 to 19
// The service account already stores a staking admin capability, so load that first
serviceAccount.load<Capability>(from: FlowIDTableStaking.StakingAdminStoragePath)
?? panic("Could not load staking admin capability from the service account")
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this part only check something? It doesn't set anything

Copy link
Member Author

Choose a reason for hiding this comment

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

On mainnet, there is a capability stored in the staking admin storage path, so since we want to store the actual admin there, we need to remove and the delete the existing capability first

@janezpodhostnik
Copy link
Contributor

Forgot to mention that this should not be done before the corresponding FVM upgrade: onflow/flow-go#4298

@joshuahannan joshuahannan changed the title Transaction to move epoch admin resources to the service account Transactions to upgrade epoch contracts and move epoch admin resources to the service account Jun 22, 2023
@joshuahannan
Copy link
Member Author

@janezpodhostnik has that FVM upgrade been enabled on canary, testnet, and mainnet yet? Not sure where to check? I updated the transactions here to include the contract upgrades too and we'd like to upgrade starting next week if possible.

@janezpodhostnik
Copy link
Contributor

Yes, the change to the system transaction to look for the heartbeat resources both on the system account and the epoch account has been deployed everywhere. The resource can now be moved.

@joshuahannan
Copy link
Member Author

I've significantly changed the plans since the last time y'all reviewed it to include the upgrades for all the epoch contracts in addition to moving the resources. The upgrades are going to require five separate transactions, the last of which is a multisig I just did all the upgrades on canary and everything seems to be going well, so I'll be running through these steps to upgrade on testnet on wednesday. I would appreciate another review on these to make sure everything looks good though!

@vishalchangrani or @janezpodhostnik can you take a look at the README and especially the multisig instructions to make sure everything looks good? The transaction requires multiple authorizers and it requires that the staking account is the first authorizer in the prepare block. I'm not 100% sure if I made the CLI command properly to ensure that is the case. Thank you!

@joshuahannan
Copy link
Member Author

I ran everything on testnet as it is here and all the upgrades worked, so all that is left is mainnet next wednesday. Let me know if there are any other changes y'all want!

@vishalchangrani
Copy link
Collaborator

I have created another PR against this branch with updates to the readme.
@joshuahannan can you take a look.
#243

@vishalchangrani vishalchangrani merged commit 9d822c7 into main Jul 5, 2023
bors bot added a commit to onflow/flow-go that referenced this pull request Aug 8, 2023
4554: Remove dual authorizers for system transaction r=janezpodhostnik a=janezpodhostnik

closes: #4291

PR #4298 added another authorizer to the system transaction so that the Heartbeat resources could be moved from the epoch account to the service account. Which was done here: onflow/service-account#243

Now that that is done, the epoch account does not need to participate in the service transaction anymore.

Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com>
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.

None yet

4 participants