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
Evm safe upgrades #1272
Merged
Merged
Evm safe upgrades #1272
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for tubular-dango-1656b2 canceled.
|
kcsongor
force-pushed
the
evm-safe-upgrades
branch
from
June 14, 2022 19:12
e6032f3
to
9a514f3
Compare
✅ Deploy Preview for roaring-moonbeam-aa0ca5 canceled.
|
kcsongor
force-pushed
the
evm-safe-upgrades
branch
18 times, most recently
from
June 14, 2022 22:46
3de6568
to
b0ec15b
Compare
properly catches broken commit: b0ec15b https://github.com/certusone/wormhole/runs/6890087832?check_suite_focus=true |
kcsongor
force-pushed
the
evm-safe-upgrades
branch
2 times, most recently
from
June 14, 2022 23:13
d13c952
to
fb10b34
Compare
kcsongor
force-pushed
the
evm-safe-upgrades
branch
3 times, most recently
from
June 25, 2022 14:16
f96426f
to
bb6fab4
Compare
* Add command to update evm storage slot * Add command to ecrecover * Add command to print contract addresses * Add command to print RPC urls * Add command to query on-chain state of EVM contracts * Print digest in parse VAA command
kcsongor
force-pushed
the
evm-safe-upgrades
branch
from
June 25, 2022 14:20
bb6fab4
to
db095fd
Compare
kcsongor
requested review from
evan-gray,
bruce-riley,
calebrate,
claudijd and
chase-45
June 25, 2022 14:24
chase-45
approved these changes
Jun 27, 2022
jumpsiegel
approved these changes
Jun 27, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements a new test for the EVM contracts that ensures that new changes do not clobber the on-chain storage or break upgradeability. At a high level, it achieves this by simulating an upgrade of each ethereum mainnet contract (core, token bridge, nft bridge) twice. Twice, to ensure that after the upgrade, the contract is still upgradeable.
To implement this test, we need several components in place. We use
anvil
(fromfoundry
) to run a fork of mainnet.ganache
is not sufficient for this, because we need an RPC method to upgrade arbitrary storage slots (hardhat_setStorageAt
, which is supported byhardhat
andanvil
). This is needed because the mainnet contracts only accept VAAs signed by the 19 guardians, and naturally we can't obtain these signatures in a test setting. Therefore, the first thing the test does is overrides the guardian set to the devnet guardian set using that RPC method. (See 5362e65#diff-fa1b92083deb8655306f5f9d48d04ff921732a7ae054c62987d471eacf77e9abR243; this requires a bunch of code for handling storage slots. Tried to make that code as reusable as possible.)Once the guardian set is swapped out, the upgrade simulation can proceed. In order to tell whether the storage has been messed up, we need a way to query all state parameters to compare before and after the upgrade. The
query_contract_evm
function (5362e65#diff-fa1b92083deb8655306f5f9d48d04ff921732a7ae054c62987d471eacf77e9abR12) does this by querying all public getters of a contract. This function is exposed as part of the CLI client, for example the mainnet ethereum token bridge state can be queried by runningworm evm info -c ethereum -m TokenBridge -n mainnet
which yieldsThe actual testing script is implemented in ba42642 as a thin orchestration tool that runs anvil and uses the command line CLI to query the before state, do an upgrade, then query the after state, and diff the two. The easiest way to test it is by running
make test-upgrade
, which is exactly what the new CI action does.The test scripts just build the contracts in the repo and upgrade to those. The test simulation script (https://github.com/certusone/wormhole/pull/1272/files#diff-78989f247e5f17e336f7c4c9a9d6ca68a84dbbd7e8b5d699a55ebcadfd3b2a8aR7-R19) can also be used to test upgrading to a contract address already uploaded to mainnet. This mode can be used as an additional safety check when doing actual mainnet upgrades, as the contract address can be verified to work before governance even takes place. I plan to write up more detailed instructions on that the next time we do an upgrade.
This change is