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

Introduce forked upgrade testing #13323

Merged
merged 105 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
105 commits
Select commit Hold shift + click to select a range
c506e77
feat: Basic upgrade test scaffolding
maurelian Dec 3, 2024
125d95b
Add Superchain Registry
maurelian Dec 9, 2024
bcb20b6
Fork from Registry
maurelian Dec 9, 2024
ca936fd
feat: allowCheatcodes on setPreinstalls in L2Genesis
maurelian Dec 9, 2024
378dd46
feat: Refactory and clean up registry reads
maurelian Dec 9, 2024
f69d32c
feat: cleanup Upgrade.s.sol
maurelian Dec 9, 2024
c43469c
fix: Move strings into function body
maurelian Dec 9, 2024
dd0d84c
feat: Some cleanup
maurelian Dec 9, 2024
23a2ab4
feat: write optimismPortal2
maurelian Dec 9, 2024
abe3dfd
feat: Automatically run anvil on 8546
maurelian Dec 9, 2024
d50fa07
feat: Add upgrade testing to Ci
maurelian Dec 9, 2024
5d5d37b
fix: use parameter to set UPGRADE_TEST env var
maurelian Dec 9, 2024
5b2b58f
feat: more cleanup
maurelian Dec 9, 2024
38840ff
fix: PR feedback
maurelian Dec 10, 2024
824c6a0
tweak ci
maurelian Dec 10, 2024
17b8d82
fix: justfile bug
maurelian Dec 10, 2024
226749d
fix: Upgrade syntax errors
maurelian Dec 10, 2024
37e1d28
add export MAINNET_RPC_URL to ci
maurelian Dec 10, 2024
22b0a83
debug address in use
maurelian Dec 10, 2024
30c2115
feat: Allow cheatcodes for FFIInterface
maurelian Dec 10, 2024
d6c3fd3
feat: use EIP1967Helper
maurelian Dec 10, 2024
7c34f17
feat: improve comments in Setup.sol
maurelian Dec 10, 2024
f024a65
feat: add support for interop and optimized test setups
maurelian Dec 10, 2024
a1c0bda
feat: Use ETH_RPC_URL as env var name
maurelian Dec 10, 2024
ee2ab79
test fixes for forking
maurelian Dec 10, 2024
60945f5
feat: anvil-fork-stop does not error
maurelian Dec 11, 2024
329256a
feat: Do not mess with EVM env on a fork test
maurelian Dec 11, 2024
84d74b6
feat: Take anvil out of the loop
maurelian Dec 11, 2024
624edb2
feat: Don't warp if forked
maurelian Dec 11, 2024
8ce9290
fix: isForkTest not forkBlock
maurelian Dec 11, 2024
528213a
lint
maurelian Dec 11, 2024
b2b9b3c
feat: enforce forking from sepolia or mainnet
maurelian Dec 12, 2024
5adaac7
fix(ctb): test fixes for a forked env
maurelian Dec 11, 2024
bd04ce1
Revert "feat: Take anvil out of the loop"
maurelian Dec 12, 2024
85571bf
feat: remove anvil-fork-stop
maurelian Dec 12, 2024
b9e927a
fix: unsilence anvil
maurelian Dec 12, 2024
8082370
fix snapshot and semgrep
maurelian Dec 12, 2024
e36b4bc
temp: slightly faster just-upgrade loop
maurelian Dec 12, 2024
2a77bcb
feat: pin fork block
maurelian Dec 12, 2024
ad9eb46
feat: change upgrade_test to test_upgrade in config.yml for consistency
maurelian Dec 12, 2024
6a1aebe
feat: cache forked state
maurelian Dec 12, 2024
7d80c64
speculative: skip l2 work
maurelian Dec 12, 2024
d6f8959
feat: skip benchmark tests if isFork
maurelian Dec 12, 2024
ec7c2ab
feat: skip deploying L2 tokens when forking
maurelian Dec 12, 2024
8aaf397
feat: add skipIfForkTest
maurelian Dec 12, 2024
4f9e200
feat: limit upgrade testing to L1 contracts
maurelian Dec 12, 2024
7f9797d
feat: move skipping logic into Setup.sol
maurelian Dec 13, 2024
d369129
feat: Simplify ci config for upgrade tests
maurelian Dec 13, 2024
a12152d
gas snapshot
maurelian Dec 13, 2024
d3b8e57
delete ignored deploy artifact
maurelian Dec 13, 2024
80f0d22
remove unused added vm call
maurelian Dec 13, 2024
53afbeb
feat: remove modifications to skipped test paths
maurelian Dec 13, 2024
d2880b4
feat: improve block number handling
maurelian Dec 13, 2024
d0ef77a
feat: Document adding a new contract
maurelian Dec 13, 2024
8e9ac15
fix: ci config syntax error
maurelian Dec 13, 2024
ae464b5
better justfile comments
maurelian Dec 13, 2024
09ac602
feat: skip some tests and log messages when you do
maurelian Dec 13, 2024
e19cbe1
feat: skip SystemConfig tests with incompatible iface
maurelian Dec 13, 2024
a666b84
feat: add returnIfForkTest and fix a bunch of tests
maurelian Dec 13, 2024
bff735e
feat: more test fixes and tweaks
maurelian Dec 13, 2024
5ed7691
gas snapshot
maurelian Dec 13, 2024
9cf9e08
feat: Add test-upgrade-rerun to justfile and ci config
maurelian Dec 13, 2024
90701ac
feat: Make PINNED_BLOCK_NUMBER be not a magic number
maurelian Dec 13, 2024
3d0745b
fix: long standing bug unrelated to this work
maurelian Dec 13, 2024
73499b1
feat: PDG: cleaner fork test handling
maurelian Dec 13, 2024
f92872a
feat: remove unused import
maurelian Dec 16, 2024
a32d437
fix incorrect vm.roll/warp skipping logic
maurelian Dec 16, 2024
fad55f8
fix: OptimismPortal2 tests
maurelian Dec 16, 2024
5acba58
fix gas snapshot
maurelian Dec 16, 2024
12aebfc
fix: restore test_list to contracts-bedrock-tests job
maurelian Dec 17, 2024
98a5b2b
feat(ci): Split up the test-upgrade CI job
maurelian Dec 17, 2024
841f29b
fix(ci): remove erroneous param reference
maurelian Dec 17, 2024
4affd8a
feat: Improve caching of forked state
maurelian Dec 17, 2024
16d1cb6
feat: use config syntax for inserting rpc url
maurelian Dec 17, 2024
f9e4701
debug: cache key
maurelian Dec 17, 2024
14dba07
feat: Fix cache key checksum
maurelian Dec 17, 2024
56fd1c9
Apply suggestions from code review
maurelian Dec 18, 2024
1907561
clarify comment about setup.setup forking
maurelian Dec 18, 2024
cab6856
feat: remove diff meant for upstack
maurelian Dec 18, 2024
f25169e
fix lint
maurelian Dec 18, 2024
b02c91c
feat: make isForkTest private and add a getter
maurelian Dec 19, 2024
3886b05
debugging with annotations
maurelian Dec 13, 2024
d2a348f
feat: fix test_findLatestGames_static_succeeds
maurelian Dec 16, 2024
cb44fa6
fix: OptimismPortal2 test_initialize_succeeds
maurelian Dec 16, 2024
cc9f9d5
feat: fix returnIfForkTest
maurelian Dec 16, 2024
9b73e76
fix: Portal tests for forking
maurelian Dec 16, 2024
bc31ab3
fix: DGF test for forking
maurelian Dec 16, 2024
3b51f53
fix: Skip CGT tests on L1StandardBridge
maurelian Dec 16, 2024
3965bc6
fix: Skip CGT tests on L1CrossDomainMessenger
maurelian Dec 16, 2024
4b88da3
fix DGF testFuzz_create_succeeds
maurelian Dec 16, 2024
f992c5a
fix: DGF testFuzz_create_sameUUID_reverts
maurelian Dec 16, 2024
3f53f3a
feat: remove debugging annotations
maurelian Dec 16, 2024
dde971b
fix: remove left over console
maurelian Dec 16, 2024
a0b9059
fix: PermissionedDisputeGame tests
maurelian Dec 16, 2024
b15f210
fix: remove unused import
maurelian Dec 16, 2024
13330c3
fix: test_donateETH_succeeds for fork testing
maurelian Dec 17, 2024
5bc0837
fix: isForkTest getter call
maurelian Dec 19, 2024
c9203b0
Merge pull request #13424 from ethereum-optimism/upgrade-against-regi…
maurelian Dec 19, 2024
b0ce211
feat: Temporarily skip UnexpectedRootClaim failures on forked upgrade…
maurelian Dec 16, 2024
feeae04
Revert "feat: remove diff meant for upstack"
maurelian Dec 18, 2024
a8306c2
feat: clearer env var name for NO_MATCH_CONTRACTS
maurelian Dec 18, 2024
4f6dd0d
fix gas snapshot
maurelian Dec 19, 2024
9f44651
fix test_constructor_succeeds tests after disableInit rebase
maurelian Dec 19, 2024
98c86a8
lint
maurelian Dec 19, 2024
27263ed
Merge pull request #13428 from ethereum-optimism/upgrade-against-regi…
maurelian Dec 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 64 additions & 1 deletion .circleci/config.yml
maurelian marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,67 @@ jobs:
- "/root/.cache/go-build"
- notify-failures-on-develop


contracts-bedrock-tests-upgrade:
maurelian marked this conversation as resolved.
Show resolved Hide resolved
machine: true
resource_class: ethereum-optimism/latitude-1
steps:
- checkout
- attach_workspace: { at: "." }
- install-contracts-dependencies
- check-changed:
patterns: contracts-bedrock,op-node
- run:
name: Print dependencies
command: just dep-status
working_directory: packages/contracts-bedrock
- run:
name: Print forge version
command: forge --version
working_directory: packages/contracts-bedrock
- run:
name: Pull artifacts
command: bash scripts/ops/pull-artifacts.sh
working_directory: packages/contracts-bedrock
- run:
name: Write pinned block number for cache key
command: |
just print-pinned-block-number > ./pinnedBlockNumber.txt
cat pinnedBlockNumber.txt
pwd
working_directory: packages/contracts-bedrock
- restore_cache:
name: Restore forked state
key: forked-state-contracts-bedrock-tests-upgrade-{{ checksum "packages/contracts-bedrock/pinnedBlockNumber.txt" }}
- run:
name: Run tests
command: just test-upgrade
environment:
FOUNDRY_PROFILE: ci
ETH_RPC_URL: https://ci-mainnet-l1-archive.optimism.io
working_directory: packages/contracts-bedrock
no_output_timeout: 15m
- run:
name: Print failed test traces
command: just test-upgrade-rerun
environment:
FOUNDRY_PROFILE: ci
ETH_RPC_URL: https://ci-mainnet-l1-archive.optimism.io
working_directory: packages/contracts-bedrock
when: on_fail
- save_cache:
name: Save Go build cache
key: golang-build-cache-contracts-bedrock-tests-{{ checksum "go.sum" }}
paths:
- "/root/.cache/go-build"
- save_cache:
name: Save forked state
key: forked-state-contracts-bedrock-tests-upgrade-{{ checksum "packages/contracts-bedrock/pinnedBlockNumber.txt" }}
when: always
paths:
- "/root/.foundry/cache"
- notify-failures-on-develop

contracts-bedrock-checks:
machine: true
resource_class: ethereum-optimism/latitude-1
Expand Down Expand Up @@ -1195,7 +1256,7 @@ workflows:
- contracts-bedrock-tests:
# Test everything except PreimageOracle.t.sol since it's slow.
name: contracts-bedrock-tests
test_list: find test -name "*.t.sol" -not -name "PreimageOracle.t.sol"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I introduced a bug here which caused ONLY the preimage oracle tests to run.

test_list: find test -name "PreimageOracle.t.sol"
- contracts-bedrock-tests:
# PreimageOracle test is slow, run it separately to unblock CI.
name: contracts-bedrock-tests-preimage-oracle
Expand All @@ -1214,6 +1275,8 @@ workflows:
test_flags: --report lcov
test_timeout: 1h
test_profile: cicoverage
- contracts-bedrock-tests-upgrade:
name: contracts-bedrock-tests-upgrade
- contracts-bedrock-checks:
requires:
- contracts-bedrock-build
Expand Down
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,6 @@
[submodule "packages/contracts-bedrock/lib/solady-v0.0.245"]
path = packages/contracts-bedrock/lib/solady-v0.0.245
url = https://github.com/vectorized/solady
[submodule "packages/contracts-bedrock/lib/superchain-registry"]
path = packages/contracts-bedrock/lib/superchain-registry
url = https://github.com/ethereum-optimism/superchain-registry
1 change: 1 addition & 0 deletions packages/contracts-bedrock/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ deployments/kontrol.json
deployments/kontrol.jsonReversed
deployments/kontrol-fp.json
deployments/kontrol-fp.jsonReversed
deployments/1-deploy.json

# Devnet config which changes with each 'make devnet-up'
deploy-config/devnetL1.json
Expand Down
1 change: 1 addition & 0 deletions packages/contracts-bedrock/foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ fs_permissions = [
{ access='read-write', path='./.testdata/' },
{ access='read', path='./kout-deployment' },
{ access='read', path='./test/fixtures' },
{ access='read', path='./lib/superchain-registry/superchain/configs/' },
]

# 5159 error code is selfdestruct error code
Expand Down
39 changes: 39 additions & 0 deletions packages/contracts-bedrock/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,45 @@ test *ARGS: build-go-ffi
test-dev *ARGS: build-go-ffi
FOUNDRY_PROFILE=lite forge test {{ARGS}}

# Default block number for the forked upgrade path.
export pinnedBlockNumber := env_var_or_default('FORK_BLOCK_NUMBER', '21387830')
print-pinned-block-number:
echo $pinnedBlockNumber

# Runs upgrade path variant of contract tests.
# Env Vars:
# - ETH_RPC_URL must be set to a production (Sepolia or Mainnet) RPC URL.
# - FORK_BLOCK_NUMBER can be set in the env, or else will fallback to the default block number.
# Reusing the default block number greatly speeds up the test execution time by caching the
# rpc call responses in ~/.foundry/cache/rpc. The default block will need to be updated
# when the L1 chain is upgraded.
maurelian marked this conversation as resolved.
Show resolved Hide resolved
# TODO(opcm upgrades): unskip the "NMC" tests which fail on the forked upgrade path with "UnexpectedRootClaim" errors.
test-upgrade *ARGS: build-go-ffi
#!/bin/bash
echo "Running upgrade tests at block $pinnedBlockNumber"
export FORK_BLOCK_NUMBER=$pinnedBlockNumber
export NO_MATCH_CONTRACTS="OptimismPortal2WithMockERC20_Test|OptimismPortal2_FinalizeWithdrawal_Test|AnchorStateRegistry_Initialize_Test|AnchorStateRegistry_TryUpdateAnchorState_Test|FaultDisputeGame_Test|FaultDispute_1v1_Actors_Test"
FORK_RPC_URL=$ETH_RPC_URL \
UPGRADE_TEST=true \
forge test --match-path "test/{L1,dispute}/**" \
--no-match-contract "$NO_MATCH_CONTRACTS" \
{{ARGS}}

test-upgrade-rerun *ARGS: build-go-ffi
just test-upgrade {{ARGS}} --rerun -vvv

# Starts a local anvil node with a mainnet fork and sends it to the background
# Requires ETH_RPC_URL to be set to a production (Sepolia or Mainnet) RPC URL.
anvil-fork:
maurelian marked this conversation as resolved.
Show resolved Hide resolved
anvil --fork-url $ETH_RPC_URL

# Use anvil-fork in a separate terminal before running this command.
# Helpful for debugging.
test-upgrade-against-anvil *ARGS: build-go-ffi
FORK_RPC_URL=http://127.0.0.1:8545 \
UPGRADE_TEST=true \
forge test {{ARGS}}

# Runs standard contract tests with rerun flag.
test-rerun: build-go-ffi
forge test --rerun -vvv
Expand Down
1 change: 1 addition & 0 deletions packages/contracts-bedrock/lib/superchain-registry
Submodule superchain-registry added at b9383c
24 changes: 12 additions & 12 deletions packages/contracts-bedrock/snapshots/.gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
GasBenchMark_L1BlockInterop_DepositsComplete:test_depositsComplete_benchmark() (gas: 7567)
GasBenchMark_L1BlockInterop_DepositsComplete_Warm:test_depositsComplete_benchmark() (gas: 5567)
GasBenchMark_L1BlockInterop_SetValuesInterop:test_setL1BlockValuesInterop_benchmark() (gas: 175677)
GasBenchMark_L1BlockInterop_SetValuesInterop_Warm:test_setL1BlockValuesInterop_benchmark() (gas: 5099)
GasBenchMark_L1Block_SetValuesEcotone:test_setL1BlockValuesEcotone_benchmark() (gas: 158531)
GasBenchMark_L1Block_SetValuesEcotone_Warm:test_setL1BlockValuesEcotone_benchmark() (gas: 7597)
GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369280)
GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967420)
GasBenchMark_L1BlockInterop_DepositsComplete:test_depositsComplete_benchmark() (gas: 7589)
GasBenchMark_L1BlockInterop_DepositsComplete_Warm:test_depositsComplete_benchmark() (gas: 5589)
GasBenchMark_L1BlockInterop_SetValuesInterop:test_setL1BlockValuesInterop_benchmark() (gas: 175722)
GasBenchMark_L1BlockInterop_SetValuesInterop_Warm:test_setL1BlockValuesInterop_benchmark() (gas: 5144)
GasBenchMark_L1Block_SetValuesEcotone:test_setL1BlockValuesEcotone_benchmark() (gas: 158553)
GasBenchMark_L1Block_SetValuesEcotone_Warm:test_setL1BlockValuesEcotone_benchmark() (gas: 7619)
GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369235)
GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967442)
GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564398)
GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076613)
GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4076568)
GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 467098)
GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3512802)
GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72664)
GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 72619)
GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 92973)
GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 68422)
GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 69031)
GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 155610)
GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 69008)
GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 155565)
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ contract L1CrossDomainMessenger_Test is CommonTest {
assertEq(address(impl.superchainConfig()), address(0));
assertEq(address(impl.PORTAL()), address(0));
assertEq(address(impl.portal()), address(0));

// The constructor now uses _disableInitializers, whereas OP Mainnet has the other messenger in storage
returnIfForkTest("L1CrossDomainMessenger_Test: impl storage differs on forked network");
assertEq(address(impl.OTHER_MESSENGER()), address(0));
assertEq(address(impl.otherMessenger()), address(0));
}
Expand Down Expand Up @@ -129,7 +132,7 @@ contract L1CrossDomainMessenger_Test is CommonTest {

// Try to relay a v2 message.
vm.prank(address(optimismPortal));
l2CrossDomainMessenger.relayMessage(
l1CrossDomainMessenger.relayMessage(
Encoding.encodeVersionedNonce({ _nonce: 0, _version: 2 }), // nonce
sender,
target,
Expand Down Expand Up @@ -745,6 +748,9 @@ contract L1CrossDomainMessenger_Test is CommonTest {

/// @dev Tests that the sendMessage reverts when call value is non-zero with custom gas token.
function test_sendMessage_customGasTokenWithValue_reverts() external {
// TODO(opcm upgrades): remove skip once upgrade path is implemented
skipIfForkTest("L1CrossDomainMessenger_Test: gas paying token functionality DNE on op mainnet");

// Mock the gasPayingToken function to return a custom gas token
vm.mockCall(
address(systemConfig), abi.encodeCall(systemConfig.gasPayingToken, ()), abi.encode(address(1), uint8(2))
Expand All @@ -756,6 +762,9 @@ contract L1CrossDomainMessenger_Test is CommonTest {

/// @dev Tests that the relayMessage succeeds with a custom gas token when the call value is zero.
function test_relayMessage_customGasTokenAndNoValue_succeeds() external {
// TODO(opcm upgrades): remove skip once upgrade path is implemented
skipIfForkTest("L1CrossDomainMessenger_Test: gas paying token functionality DNE on op mainnet");

// Mock the gasPayingToken function to return a custom gas token
vm.mockCall(
address(systemConfig), abi.encodeCall(systemConfig.gasPayingToken, ()), abi.encode(address(1), uint8(2))
Expand Down
5 changes: 4 additions & 1 deletion packages/contracts-bedrock/test/L1/L1ERC721Bridge.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,12 @@ contract L1ERC721Bridge_Test is CommonTest {
IL1ERC721Bridge impl = IL1ERC721Bridge(deploy.mustGetAddress("L1ERC721Bridge"));
assertEq(address(impl.MESSENGER()), address(0));
assertEq(address(impl.messenger()), address(0));
assertEq(address(impl.superchainConfig()), address(0));

// The constructor now uses _disableInitializers, whereas OP Mainnet has the other bridge in storage
returnIfForkTest("L1ERC721Bridge_Test: impl storage differs on forked network");
assertEq(address(impl.OTHER_BRIDGE()), address(0));
assertEq(address(impl.otherBridge()), address(0));
assertEq(address(impl.superchainConfig()), address(0));
}

/// @dev Tests that the proxy is initialized with the correct values.
Expand Down
Loading