test: add integration tests for Anvil migrations #11002

arthurgousset · 2024-05-21T14:27:40Z

Description

adds test to assert that all core contracts on the devchain have a registered address in the Registry.
adds test to assert that all core contracts on the devchain have the correct runtime bytecode, excluding those that depend on linked libraries.

Note

We cannot currently test contracts that depend on linked libraries because vm.getDeployedCode() cannot fetch bytecode of contracts that depend on linked libraries. I created a follow-up issue with context on why this can't be done in this PR:

Include tests that depend on linked libraries in Foundry devchain integration test #11016

Other changes

adds a constant in constants.sol that defines the list of contracts that are expected to be in Registry.sol. Refactors the migration and integration test scripts to import that list instead of defining the same list twice.
deletes integration_tests.sh, and refactors the integration test script (run_integration_tests_in_anvil.sh) to include commands from integration_tests.sh instead.
downgrades the foundry version to nightly-f625d0fa7c51e65b4bf1e8f7931cd1c6e2e285e9 (2024-04-02). This is the last foundry version that doesn't have a regression that affects the integration tests.
reverts how ECDSAHelper.sol is deployed to comply with the downgraded foundry version nightly-f625d0fa7c51e65b4bf1e8f7931cd1c6e2e285e9. This reverts a change made in Deprecate random.sol #10997.

Note

For context
1. nightly-fbad377ab26a432e48444cf324feee1195a30960 (2024-06-04) has the regression ❌
2. nightly-0fc39763d46b8ffab5fa4eaeb2f65ae078fa07de (2024-06-03) has the regression ❌
3. nightly-5ac78a9cd4b94dc53d1fe5e0f42372b28b5a7559 (2024-06-02) has the regression ❌
4. nightly-f479e945c6be78bb902df12f9d683c3bb55e3fb0 (2024-06-01) has the regression ❌
5. nightly-cafc2606a2187a42b236df4aa65f4e8cdfcea970 (2024-05-02) has the regression ❌
6. nightly-f625d0fa7c51e65b4bf1e8f7931cd1c6e2e285e9 (2024-04-02) doesn't have the regression ✅

Tested

Yes, these are all tests. The tests run during CI when migrations are generated and separately.

Related issues

Fixes Add integration tests for Anvil migrations #10999

Backwards compatibility

Yes, only adds additional tests.

Documentation

Documentation in docstrings in code.

arthurgousset · 2024-05-21T14:29:13Z

MVP test: Assert that existing tests that are known to pass also pass when run against the devchain.

Run Anvil with migrations (in another terminal)

./migrations_sol/create_and_migrate_anvil_devchain.sh

Run existing tests against local Anvil instance with --rpc-url flag. Existing tests can be found in the CI workflow protocol_tests.yml

forge test \ 
-vvv \ 
--match-path "test-sol/common/*" \
--rpc-url=http://127.0.0.1:8546

# ... test logs
Ran 84 test suites: 409 tests passed, 0 failed, 0 skipped (409 total tests)

forge test \
-vvv \
--block-gas-limit 50000000 \
--match-path "test-sol/governance/network/*" \
--rpc-url=http://127.0.0.1:8546

Failing tests:
Encountered 1 failing test in test-sol/governance/network/BlockchainParameters.t.sol:BlockchainParametersTest_initialize
[FAIL. Reason: log != expected log] test_Emits_UptimeLookbackWindowSet() (gas: 124192)

Encountered 1 failing test in test-sol/governance/network/BlockchainParameters.t.sol:BlockchainParametersTest_setUptimeLookbackWindow
[FAIL. Reason: log != expected log] test_Emits_UptimeLookbackWindowSet() (gas: 72044)

Encountered a total of 2 failing tests, 413 tests succeeded

This is done in cd987f3

arthurgousset · 2024-05-21T14:30:01Z

Ideas for invariants in the integration tests:

assert that the core number or list of contracts are deployed
assert that they are initialized correctly
assert that they are registered in the Registry

Check if there are integration tests in the Ganache devchain setup

packages/protocol/migrations_sol/integration_tests.sh

This is a test run, to see how `setUp()` and contract definitions work. 1. run devchain ```sh ./migrations_sol/create_and_migrate_anvil_devchain.sh ``` 2. run test against devchain ```sh forge test \ --match-path test-sol/integration/Integration.t.sol \ --match-contract GoldTokenTest_General \ -vvv \ --fork-url http://127.0.0.1:8546 ``` Tests pass Output: ```sh [⠰] Compiling... No files changed, compilation skipped Running 3 tests for test-sol/integration/Integration.t.sol:GoldTokenTest_General [PASS] test_decimals() (gas: 10837) Logs: GoldToken address is: 0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448 [PASS] test_name() (gas: 12537) Logs: GoldToken address is: 0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448 [PASS] test_symbol() (gas: 12579) Logs: GoldToken address is: 0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448 Test result: ok. 3 passed; 0 failed; 0 skipped; finished in 135.40ms Ran 1 test suites: 3 tests passed, 0 failed, 0 skipped (3 total tests) ```

arthurgousset · 2024-05-28T16:24:51Z

As of ee24877, I'm able to run end-to-end tests against a local devchain:

MVP test

This is a test run, to see how setUp() and contract definitions work using GoldToken.sol as an example.

run devchain locally (in a separate terminal)

./migrations_sol/create_and_migrate_anvil_devchain.sh

run MVP test against local devchain

forge test \
--match-path test-sol/integration/Integration.t.sol \
--match-contract GoldTokenTest_General \
-vvv \
--fork-url http://127.0.0.1:8546/

The tests pass

Output:

[⠰] Compiling...
No files changed, compilation skipped

Running 3 tests for test-sol/integration/Integration.t.sol:GoldTokenTest_General
[PASS] test_decimals() (gas: 10837)
Logs:
  GoldToken address is: 0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448

[PASS] test_name() (gas: 12537)
Logs:
  GoldToken address is: 0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448

[PASS] test_symbol() (gas: 12579)
Logs:
  GoldToken address is: 0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448

Test result: ok. 3 passed; 0 failed; 0 skipped; finished in 135.40ms

Ran 1 test suites: 3 tests passed, 0 failed, 0 skipped (3 total tests)

This is to allow me to use the `.code` property on deployed smart contracts, which is only supported on Solidity 0.8 and above.

In Solidity versions 0.7.0 and later, constructors are implicitly internal, meaning they can only be called within the contract itself or derived contracts, making the visibility keyword redundant.

The state mutability of a function can be restricted to view if the function does not modify the state of the blockchain. A view function promises not to alter the state, which allows the Ethereum Virtual Machine (EVM) to optimize how it handles the function. The function does not modify any state; it only reads from the registry and logs some information.

arthurgousset · 2024-05-30T15:12:22Z

As of 714bc75, I'm asserting that a list of contract names is registered in Registry.sol on the devchain.

[PASS] test_shouldHaveAddressInRegistry() (gas: 200293)
Logs:
  Accounts address in Registry is:  0x8f24d37a4697e49AECd08d2b197E6968D2f007d3
  BlockchainParameters address in Registry is:  0xFEB14C5787fd63a417612bF8Bc2232e169A7034F
  DoubleSigningSlasher address in Registry is:  0xA920a6fF4249f9A4CDdE3D67ff6275EdA6A1D89f
  DowntimeSlasher address in Registry is:  0xAADB35CAD2f922180106F50BF7ead66C5AD0f101
  Election address in Registry is:  0x5E386A280030a076528aD192BA94D7e31090F461
  EpochRewards address in Registry is:  0xfff723D9B8f466Cd9e11BF4aaca171550aCF18Fc
  Escrow address in Registry is:  0xd773882801F417427aE5C7A032296D93Fcf11DA9
  FederatedAttestations address in Registry is:  0x93f2E9307e3003a0a10A5171478CE18796aA2333
  FeeCurrencyWhitelist address in Registry is:  0x99f389e8a9903aF72ba481e8f000e8E229e529dA
  FeeCurrencyDirectory address in Registry is:  0x42Fe5a2A61ed9705eb2F08a04A58CEB606D22f6a
  Freezer address in Registry is:  0x91dfD4C1B1262fad0f75A38D955B42b4bC586Bc1
  FeeHandler address in Registry is:  0x04dE09026097347E2D540b6fbD9dC4aae3Ab0A90
  GoldToken address in Registry is:  0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448
  Governance address in Registry is:  0xcF636ab1f15d3FB684A82cB1c69977DAfDFc6a0d
  GovernanceSlasher address in Registry is:  0xE41a630372DE5B890a81B148a982b464Aa3B3625
  LockedGold address in Registry is:  0xc7F0E00681356896c06d5c810F0333ab30fBB8D1
  OdisPayments address in Registry is:  0xf7EfD92E61aB144483738B5F0f9cd46E6E9190BC
  Random address in Registry is:  0x8982140ccFb38d7Dc439f953b37829f019A3e6e5
  Registry address in Registry is:  0x000000000000000000000000000000000000ce10
  SortedOracles address in Registry is:  0xa2204011717369e044106e3bC93599E02538d65b
  UniswapFeeHandlerSeller address in Registry is:  0x27D282cBE154FD738de3242F80005cc2ca183981
  MentoFeeHandlerSeller address in Registry is:  0x677e4E735a36A7Ed935D424fCCe57a33831BF0Dc
  Validators address in Registry is:  0xA6aaDC309AA8E134d4a150eb4b58254801353FDc

packages/protocol/test-sol/integration/Integration.t.sol

…ecode

packages/protocol/migrations_sol/integration_tests.sh

The goal is to use that in both the migration script (`Migration.s.sol`) and the integration test (`Integration.t.sol`).

….sol`

Mistakenly deleted earlier

arthurgousset · 2024-05-31T12:03:42Z

As of 130523e:

I refactored the migration and integration test scripts, so that both import a list of contract names from constants.sol ✅
I refactored the integration test script run_integration_tests_in_anvil.sh and deleted integration_tests.sh ✅

The only thing stopping this PR from being ready for review is the failing bytecode test.

Also adds docstring for better readability.

… test

Because I'm downgrading to a previous foundry version, I have to undo the change to this line made in this PR: #10997

I always forget to run `yarn prettify` in the top-level directory

arthurgousset · 2024-06-04T10:40:10Z

As of dcb09e1, the PR is all green and ready for review ✅

To explain what flags and arguments are used.

packages/protocol/test-sol/integration/Integration.t.sol

packages/protocol/test-sol/utils/ECDSAHelper.sol

Using `yarn prettify` in the top-level directory

packages/protocol/test-sol/integration/Integration.t.sol

.github/workflows/protocol_tests.yml

Adds function in `Integration.t.sol` directly to avoid 0.5.x version problems.

…r function Because the script is now on 0.8, we can use the `.code` property to fetch the runtime bytecode of a smart contract deployed on the devchain. Previously, we needed to load the bytecode from storage with inline assembly.

Functionally this should be the same, but the configs and flags are more readable.

test(protocol/migrations_sol): adds existing unit tests

cd987f3

arthurgousset force-pushed the arthurgousset/chore/anvil-integration-tests branch from a12fb19 to cd987f3 Compare May 21, 2024 14:35

arthurgousset commented May 21, 2024

View reviewed changes

packages/protocol/migrations_sol/integration_tests.sh Outdated Show resolved Hide resolved

Merge branch 'master' into arthurgousset/chore/anvil-integration-tests

601ad7c

soloseng mentioned this pull request May 24, 2024

Deactivate BlochainParameters Contract on L2 #11008

Merged

arthurgousset added 2 commits May 28, 2024 12:47

Merge branch 'master' into arthurgousset/chore/anvil-integration-tests

3ae51c4

arthurgousset added 7 commits May 29, 2024 18:42

test(test-sol/integration): adds loop to assert contract is in Registry

714bc75

chore(test-sol/integration): adds TODO comment about config file

8651f31

test(test-sol/integration): upgrades to solidity 0.8

eec3863

This is to allow me to use the `.code` property on deployed smart contracts, which is only supported on Solidity 0.8 and above.

Merge branch 'master' into arthurgousset/chore/anvil-integration-tests

710059f

nit(test-sol/integration): updates constructor visibility

c4f55d5

In Solidity versions 0.7.0 and later, constructors are implicitly internal, meaning they can only be called within the contract itself or derived contracts, making the visibility keyword redundant.

test(test-sol/integration): adds MVP assertion that bytecode matches

d99a746

arthurgousset commented May 30, 2024

View reviewed changes

packages/protocol/test-sol/integration/Integration.t.sol Outdated Show resolved Hide resolved

fix(test-sol/integration): use implementation address and deployedByt…

4df1e7c

…ecode

arthurgousset commented May 30, 2024

View reviewed changes

packages/protocol/migrations_sol/integration_tests.sh Outdated Show resolved Hide resolved

arthurgousset added 5 commits May 31, 2024 10:56

refactor(test-sol): adds registry array in constants.sol

dc6a6c5

The goal is to use that in both the migration script (`Migration.s.sol`) and the integration test (`Integration.t.sol`).

refactor(protocol/migration_sol): uses registry array from `constants…

d6389c8

….sol`

fix(test-sol/integration): adds back Test.sol import

ab7bab9

Mistakenly deleted earlier

refactor(migrations_sol): simplifies script that runs integration tests

e372b8a

chore(test-sol/integration): commits WIP on failing bytecode test

130523e

arthurgousset added 3 commits May 31, 2024 15:41

test(test-sol): adds helper to remove metadata from bytecode

1e61942

Also adds docstring for better readability.

chore(test-sol/integration): excludes failing contracts from bytecode…

e55c973

… test

chor(test-sol/integration): adds code comment for readability

14bd4cf

arthurgousset added 3 commits June 4, 2024 11:00

fix(workflows/protocol_tests): excludes integration tests correctly

2f9f837

chore(test-sol/utils): reverts deployCodeTo edit in ECDSAHelper

30de987

Because I'm downgrading to a previous foundry version, I have to undo the change to this line made in this PR: #10997

style(protocol): linting

dcb09e1

I always forget to run `yarn prettify` in the top-level directory

docs(workflows/protocol_tests): adds better code comment

c27a53e

To explain what flags and arguments are used.

pahor167 reviewed Jun 4, 2024

View reviewed changes

packages/protocol/test-sol/integration/Integration.t.sol Outdated Show resolved Hide resolved

pahor167 reviewed Jun 4, 2024

View reviewed changes

packages/protocol/test-sol/integration/Integration.t.sol Outdated Show resolved Hide resolved

pahor167 reviewed Jun 4, 2024

View reviewed changes

packages/protocol/test-sol/integration/Integration.t.sol Outdated Show resolved Hide resolved

pahor167 reviewed Jun 4, 2024

View reviewed changes

packages/protocol/test-sol/utils/ECDSAHelper.sol Show resolved Hide resolved

arthurgousset added 4 commits June 4, 2024 15:34

refactor(test-sol/integration): rename variables for better readability

b9ce129

refactor(test-sol/integration): take constants out of for-loop

7a740cb

style(protocol): linting

7201463

Using `yarn prettify` in the top-level directory

fix(test-sol/integration): add contract name back into for-loop

19faa9b

arthurgousset force-pushed the arthurgousset/chore/anvil-integration-tests branch from 8248fdf to 19faa9b Compare June 4, 2024 15:03

nit(test-sol/integration): rename variable for better readability

d31c336

soloseng reviewed Jun 4, 2024

View reviewed changes

packages/protocol/test-sol/integration/Integration.t.sol Show resolved Hide resolved

soloseng reviewed Jun 4, 2024

View reviewed changes

.github/workflows/protocol_tests.yml Outdated Show resolved Hide resolved

arthurgousset added 4 commits June 4, 2024 19:30

refactor(migrations_sol/constants): creates new constants.sol

a7f4905

refactor(migrations_sol&test-sol): removes Utils import

75a4306

Adds function in `Integration.t.sol` directly to avoid 0.5.x version problems.

test(test-sol/integration): bumps solidity version to >=0.8.7 <0.8.20

1e6b230

pahor167 approved these changes Jun 5, 2024

View reviewed changes

refactor(foundry): simplifies configs to exclude tests

255f467

Functionally this should be the same, but the configs and flags are more readable.

arthurgousset mentioned this pull request Jun 5, 2024

Include tests that depend on linked libraries in Foundry devchain integration test #11016

Open

arthurgousset merged commit 2908c52 into master Jun 5, 2024
22 checks passed

arthurgousset deleted the arthurgousset/chore/anvil-integration-tests branch June 5, 2024 11:41

arthurgousset changed the title ~~Integration tests for Anvil migrations~~ Add integration tests for Anvil migrations Jun 5, 2024

arthurgousset mentioned this pull request Jun 5, 2024

chore: downgrade foundry version to match across workflows #11017

Merged

arthurgousset changed the title ~~Add integration tests for Anvil migrations~~ test: add integration tests for Anvil migrations Jun 5, 2024

arthurgousset self-assigned this Jun 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add integration tests for Anvil migrations #11002

test: add integration tests for Anvil migrations #11002

arthurgousset commented May 21, 2024 •

edited

arthurgousset commented May 21, 2024 •

edited

arthurgousset commented May 21, 2024 •

edited

arthurgousset commented May 28, 2024 •

edited

arthurgousset commented May 30, 2024

arthurgousset commented May 31, 2024

arthurgousset commented Jun 4, 2024

test: add integration tests for Anvil migrations #11002

test: add integration tests for Anvil migrations #11002

Conversation

arthurgousset commented May 21, 2024 • edited

Description

Other changes

Tested

Related issues

Backwards compatibility

Documentation

arthurgousset commented May 21, 2024 • edited

arthurgousset commented May 21, 2024 • edited

arthurgousset commented May 28, 2024 • edited

MVP test

arthurgousset commented May 30, 2024

arthurgousset commented May 31, 2024

arthurgousset commented Jun 4, 2024

arthurgousset commented May 21, 2024 •

edited

arthurgousset commented May 21, 2024 •

edited

arthurgousset commented May 21, 2024 •

edited

arthurgousset commented May 28, 2024 •

edited