From 028c189d468a0c7e16d7d291f44cd27cb46e8ac6 Mon Sep 17 00:00:00 2001 From: teddy Date: Fri, 1 Nov 2024 10:56:55 -0300 Subject: [PATCH] test: medusa testing campaign (#4) --- .github/workflows/medusa.yml | 29 + .../contracts/L1/winddown/BalanceClaimer.sol | 2 +- .../contracts/test/BondManager.t.sol | 2 +- .../contracts/test/L1StandardBridge.t.sol | 2 +- .../contracts/test/SafeCall.t.sol | 6 +- .../contracts/test/invariants/PROPERTIES.md | 52 + .../invariants/balance-claimer/FuzzTest.t.sol | 8 + .../balance-claimer/generate-random-tree.sh | 54 + .../handlers/guided/BalanceClaimer.t.sol | 26 + .../handlers/unguided/BalanceClaimer.t.sol | 51 + .../properties/BalanceClaimer.t.sol | 23 + .../setup/BalanceClaimer.t.sol | 66 ++ .../balance-claimer/setup/ClaimList.t.sol | 1025 +++++++++++++++++ .../balance-claimer/setup/Claims.t.sol | 91 ++ .../balance-claimer/setup/Tokens.t.sol | 30 + packages/contracts-bedrock/foundry.toml | 5 + packages/contracts-bedrock/medusa.json | 89 ++ packages/contracts-bedrock/package.json | 3 +- yarn.lock | 8 +- 19 files changed, 1561 insertions(+), 11 deletions(-) create mode 100644 .github/workflows/medusa.yml create mode 100644 packages/contracts-bedrock/contracts/test/invariants/PROPERTIES.md create mode 100644 packages/contracts-bedrock/contracts/test/invariants/balance-claimer/FuzzTest.t.sol create mode 100755 packages/contracts-bedrock/contracts/test/invariants/balance-claimer/generate-random-tree.sh create mode 100644 packages/contracts-bedrock/contracts/test/invariants/balance-claimer/handlers/guided/BalanceClaimer.t.sol create mode 100644 packages/contracts-bedrock/contracts/test/invariants/balance-claimer/handlers/unguided/BalanceClaimer.t.sol create mode 100644 packages/contracts-bedrock/contracts/test/invariants/balance-claimer/properties/BalanceClaimer.t.sol create mode 100644 packages/contracts-bedrock/contracts/test/invariants/balance-claimer/setup/BalanceClaimer.t.sol create mode 100644 packages/contracts-bedrock/contracts/test/invariants/balance-claimer/setup/ClaimList.t.sol create mode 100644 packages/contracts-bedrock/contracts/test/invariants/balance-claimer/setup/Claims.t.sol create mode 100644 packages/contracts-bedrock/contracts/test/invariants/balance-claimer/setup/Tokens.t.sol create mode 100644 packages/contracts-bedrock/medusa.json diff --git a/.github/workflows/medusa.yml b/.github/workflows/medusa.yml new file mode 100644 index 000000000000..f4676c7ba487 --- /dev/null +++ b/.github/workflows/medusa.yml @@ -0,0 +1,29 @@ +name: CI + +on: [push] + +jobs: + medusa-tests: + name: Medusa Test + runs-on: ubuntu-latest + container: ghcr.io/defi-wonderland/eth-security-toolbox-ci:dev + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + submodules: recursive + + - name: Setup Node.js 16.x + uses: actions/setup-node@master + with: + node-version: 16.x + cache: yarn + + - name: Install dependencies + working-directory: ./packages/contracts-bedrock + run: yarn --frozen-lockfile --network-concurrency 1 + + - name: Run Medusa + working-directory: ./packages/contracts-bedrock + run: medusa fuzz --test-limit 200000 diff --git a/packages/contracts-bedrock/contracts/L1/winddown/BalanceClaimer.sol b/packages/contracts-bedrock/contracts/L1/winddown/BalanceClaimer.sol index 8fb3f34aa7bb..b4b208f13ad6 100644 --- a/packages/contracts-bedrock/contracts/L1/winddown/BalanceClaimer.sol +++ b/packages/contracts-bedrock/contracts/L1/winddown/BalanceClaimer.sol @@ -74,4 +74,4 @@ contract BalanceClaimer is Semver, IBalanceClaimer { _canClaimTokens = MerkleProof.verify(_proof, ROOT, _leaf); } -} \ No newline at end of file +} diff --git a/packages/contracts-bedrock/contracts/test/BondManager.t.sol b/packages/contracts-bedrock/contracts/test/BondManager.t.sol index 8cae49c28fa7..994cbbe4d842 100644 --- a/packages/contracts-bedrock/contracts/test/BondManager.t.sol +++ b/packages/contracts-bedrock/contracts/test/BondManager.t.sol @@ -325,7 +325,7 @@ contract BondManager_Test is Test { unchecked { vm.assume(block.timestamp + minClaimHold > minClaimHold); } - assumeNoPrecompiles(owner); + assumeNotPrecompile(owner); // Post the bond vm.deal(address(this), amount); diff --git a/packages/contracts-bedrock/contracts/test/L1StandardBridge.t.sol b/packages/contracts-bedrock/contracts/test/L1StandardBridge.t.sol index e042ea5a9c0b..10011c08265d 100644 --- a/packages/contracts-bedrock/contracts/test/L1StandardBridge.t.sol +++ b/packages/contracts-bedrock/contracts/test/L1StandardBridge.t.sol @@ -742,7 +742,7 @@ contract L1StandardBridge_WithdrawErc20Balance_Test is Bridge_Initializer { { uint8 _claimArraySize; for (uint256 _i; _i < _fuzzBalances.length; _i++) { - assumeNoPrecompiles(_fuzzBalances[_i].token); + assumeNotPrecompile(_fuzzBalances[_i].token); if (_fuzzBalances[_i].balance > 0) { _claimArraySize++; vm.mockCall( diff --git a/packages/contracts-bedrock/contracts/test/SafeCall.t.sol b/packages/contracts-bedrock/contracts/test/SafeCall.t.sol index f5bb61b8fb27..c644697d11c5 100644 --- a/packages/contracts-bedrock/contracts/test/SafeCall.t.sol +++ b/packages/contracts-bedrock/contracts/test/SafeCall.t.sol @@ -14,7 +14,7 @@ contract SafeCall_Test is CommonTest { vm.assume(from.balance == 0); vm.assume(to.balance == 0); // no precompiles (mainnet) - assumeNoPrecompiles(to, 1); + assumeNotPrecompile(to, 1); // don't call the vm vm.assume(to != address(vm)); vm.assume(from != address(vm)); @@ -54,7 +54,7 @@ contract SafeCall_Test is CommonTest { vm.assume(from.balance == 0); vm.assume(to.balance == 0); // no precompiles (mainnet) - assumeNoPrecompiles(to, 1); + assumeNotPrecompile(to, 1); // don't call the vm vm.assume(to != address(vm)); vm.assume(from != address(vm)); @@ -94,7 +94,7 @@ contract SafeCall_Test is CommonTest { vm.assume(from.balance == 0); vm.assume(to.balance == 0); // no precompiles (mainnet) - assumeNoPrecompiles(to, 1); + assumeNotPrecompile(to, 1); // don't call the vm vm.assume(to != address(vm)); vm.assume(from != address(vm)); diff --git a/packages/contracts-bedrock/contracts/test/invariants/PROPERTIES.md b/packages/contracts-bedrock/contracts/test/invariants/PROPERTIES.md new file mode 100644 index 000000000000..acce7e3ffbe3 --- /dev/null +++ b/packages/contracts-bedrock/contracts/test/invariants/PROPERTIES.md @@ -0,0 +1,52 @@ +# Scope + +- OptimismPortal's withdrawEthBalance function: contracts/L1/OptimismPortal.sol:504 +- L1StandardBridge's withdrawErc20Balance function: contracts/L1/L1StandardBridge.sol:272 +- BalanceClaimer contract: contracts/L1/winddown/BalanceClaimer.sol + +# Properties + +| Id | Properties | Type | Checked | +| --- | --------------------------------------------------- | ------------ | --- | +| 1 | a valid claim should be redeemable once | State transition | [x] | +| 2 | a valid claim should not be redeemable more than once | State transition | [x] | +| 3 | a user should be set as claimed when they process a claim | State transition | [x] | +| 4 | an invalid claim should not be redeemable | State transition | [x] | +| 5 | for each token, token.balanceOf(L1StandardBridge) == initialBalance - sum of claims | High-level | [x] | +| 6 | OptimismPortal.balance == initialBalance - sum of claims | High-level | [x] | + + +## testing methodology +The fact that the state root is not writeable in the lifetime of the contract is cool from a design standpoint, but that also means the root has to be generated, and the valid claims chosen, before it makes sense to call any other handler. + +As a first approach, we've meta-programmed a solidity source file with a hard-coded set of valid claims, which can be refreshed by calling `./generate-random-tree.sh`. +This is not ideal, as all fuzzing runs are going to run on the same merkle tree instead of letting the fuzzer explore new ones. Some alternatives are described below: + +### mutate the state root +Idea for this is to initialize the BalanceClaimer in the campaign constructor with either + +- [ ] an empty state root (for ...purity? ie allowing the fuzzer choose the inputs with the greatest variability) +- [ ] pre-filled state root (to cover code faster) and set of valid claims, with the downside of calls creating + +and have handlers to _add_ valid claims to the set, overwriting the state root + +This has the downside of being dissimilar to the actual production usage in a very crucial way, but also the invariant we would be breaking (the state root not changing) can be easily enforced by the compiler (ie: make the field immutable), and the upside of exploring a lot of possible trees in a simpler way + +### use a modifier to ensure the first call of the sequence initializes a state root +this would involve +- [ ] not creating the balanceClaimer in the constructor +- [ ] have a modifier (and an extra param of fuzzed input in every handler/property check) which will be used to initialize the state root on the first call +- [ ] have all handlers afterwards only process claims (valid or not, obviously) and not create new ones + +This has the upside of being identical to the production setup, but would yield uglier code and potentially have worse pseudorandom input since we would be having all the state as fields of structs in arrays + +# nice to haves +- [ ] use tokens' actual bytecode in the fuzzing campaign +- [ ] use a full uint256 for the range of the amounts in merkle tree + - [ ] use bigger number in script + - [ ] handle fails caused by insufficient balances +- [ ] create a larger share of claims with incomplete list of tokens or zero eth +- [ ] handlers for withdraw{Erc20,Eth}Balance methods + - [ ] guided + - [ ] unguided + diff --git a/packages/contracts-bedrock/contracts/test/invariants/balance-claimer/FuzzTest.t.sol b/packages/contracts-bedrock/contracts/test/invariants/balance-claimer/FuzzTest.t.sol new file mode 100644 index 000000000000..1295b58f1170 --- /dev/null +++ b/packages/contracts-bedrock/contracts/test/invariants/balance-claimer/FuzzTest.t.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.8.15; + +import {BalanceClaimerGuidedHandlers} from "./handlers/guided/BalanceClaimer.t.sol"; +import {BalanceClaimerUnguidedHandlers} from "./handlers/unguided/BalanceClaimer.t.sol"; +import {BalanceClaimerProperties} from "./properties/BalanceClaimer.t.sol"; + +contract FuzzTest is BalanceClaimerGuidedHandlers, BalanceClaimerUnguidedHandlers, BalanceClaimerProperties {} diff --git a/packages/contracts-bedrock/contracts/test/invariants/balance-claimer/generate-random-tree.sh b/packages/contracts-bedrock/contracts/test/invariants/balance-claimer/generate-random-tree.sh new file mode 100755 index 000000000000..fd0b1530f878 --- /dev/null +++ b/packages/contracts-bedrock/contracts/test/invariants/balance-claimer/generate-random-tree.sh @@ -0,0 +1,54 @@ +#!/bin/sh + +claims_file=./setup/ClaimList.t.sol +claims_amount=100 +users_amount=100 + +random_int() { + # 2^ 64 - 2 , max range for shuf, == 18e18, not ideal. + echo "$(shuf -n 1 -i 0-18446744073709551614)" +} + +cat - > "$claims_file" <> "$claims_file" + +done + + +cat - >> "$claims_file" < bool) internal ghost_claimed; + mapping(bytes32 => bool) internal ghost_claimInTree; + // only used as dynamic array + address[] private _tokens; + // only used as dynamic array + uint256[] private _amounts; + + constructor() { + for (uint256 i = 0; i < randomClaims.length; i++) { + ClaimEntry memory rawClaim = randomClaims[i]; + if (rawClaim.daiAmount > 0) { + _tokens.push(address(supportedTokens[0])); + _amounts.push(rawClaim.daiAmount); + } + if (rawClaim.gtcAmount > 0) { + _tokens.push(address(supportedTokens[1])); + _amounts.push(rawClaim.gtcAmount); + } + if (rawClaim.usdtAmount > 0) { + _tokens.push(address(supportedTokens[2])); + _amounts.push(rawClaim.usdtAmount); + } + if (rawClaim.usdcAmount > 0) { + _tokens.push(address(supportedTokens[3])); + _amounts.push(rawClaim.usdcAmount); + } + Claim memory claim = Claim({ + user: rawClaim.recipient, + ethAmount: rawClaim.ethAmount, + tokens: _tokens, + tokenAmounts: _amounts + }); + delete _amounts; + delete _tokens; + ghost_validClaims.push(claim); + leaves.push(_hashClaim(claim)); + } + tree = generateMerkleTree(leaves); + } + + function _hashClaim(Claim memory claim) internal pure returns (bytes32) { + IErc20BalanceWithdrawer.Erc20BalanceClaim[] memory erc20Claims = + new IErc20BalanceWithdrawer.Erc20BalanceClaim[](claim.tokens.length); + for (uint256 i = 0; i < claim.tokens.length; i++) { + erc20Claims[i].token = claim.tokens[i]; + erc20Claims[i].balance = claim.tokenAmounts[i]; + } + return keccak256(bytes.concat(keccak256(abi.encode(claim.user, claim.ethAmount, erc20Claims)))); + } + + function _hashClaim(address user, uint256 ethAmount, IErc20BalanceWithdrawer.Erc20BalanceClaim[] memory erc20Claims) + internal + pure + returns (bytes32) + { + return keccak256(bytes.concat(keccak256(abi.encode(user, ethAmount, erc20Claims)))); + } + + function _claimToErc20ClaimArray(Claim memory claim) + internal + pure + returns (IErc20BalanceWithdrawer.Erc20BalanceClaim[] memory) + { + IErc20BalanceWithdrawer.Erc20BalanceClaim[] memory erc20Claims = + new IErc20BalanceWithdrawer.Erc20BalanceClaim[](claim.tokens.length); + for (uint256 i = 0; i < claim.tokens.length; i++) { + erc20Claims[i].token = claim.tokens[i]; + erc20Claims[i].balance = claim.tokenAmounts[i]; + } + return erc20Claims; + } +} diff --git a/packages/contracts-bedrock/contracts/test/invariants/balance-claimer/setup/Tokens.t.sol b/packages/contracts-bedrock/contracts/test/invariants/balance-claimer/setup/Tokens.t.sol new file mode 100644 index 000000000000..ef591636b8d7 --- /dev/null +++ b/packages/contracts-bedrock/contracts/test/invariants/balance-claimer/setup/Tokens.t.sol @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.8.15; + +import {MockERC20} from "forge-std/mocks/MockERC20.sol"; +import {IERC20} from "forge-std/interfaces/IERC20.sol"; + +contract FuzzERC20 is MockERC20 { + function mint(address _to, uint256 _amount) public { + _mint(_to, _amount); + } +} + +contract Tokens { + uint8 internal constant TOKENS = 4; + uint256 internal constant INITIAL_BALANCE = 100000e18; + IERC20[] internal supportedTokens; + + mapping(address => uint256) internal ghost_claimedTokens; + uint256 internal ghost_claimedEther; + + constructor() { + for (uint256 i = 0; i < TOKENS; i++) { + // TODO: use bytecode from production tokens + FuzzERC20 token = new FuzzERC20(); + // TODO: use 6 decimals for usdt + token.initialize("name", "symbol", 18); + supportedTokens.push(token); + } + } +} diff --git a/packages/contracts-bedrock/foundry.toml b/packages/contracts-bedrock/foundry.toml index 18e9f3361ed8..3b04c12b8cd4 100644 --- a/packages/contracts-bedrock/foundry.toml +++ b/packages/contracts-bedrock/foundry.toml @@ -24,6 +24,11 @@ fs_permissions = [ { 'access'='read-write', 'path'='./.resource-metering.csv' }, ] +[profile.medusa] +src='contracts/test/invariants/balance-claimer' +test='contracts/test/invariants/balance-claimer' +script='contracts/test/invariants/balance-claimer' + [profile.ci] fuzz_runs = 512 diff --git a/packages/contracts-bedrock/medusa.json b/packages/contracts-bedrock/medusa.json new file mode 100644 index 000000000000..ede7245d9619 --- /dev/null +++ b/packages/contracts-bedrock/medusa.json @@ -0,0 +1,89 @@ +{ + "fuzzing": { + "workers": 10, + "workerResetLimit": 50, + "timeout": 0, + "testLimit": 0, + "shrinkLimit": 5000, + "callSequenceLength": 100, + "corpusDirectory": "corpus", + "coverageEnabled": true, + "coverageFormats": [ + "html", + "lcov" + ], + "targetContracts": ["FuzzTest"], + "predeployedContracts": {}, + "targetContractsBalances": [], + "constructorArgs": {}, + "deployerAddress": "0x30000", + "senderAddresses": [ + "0x10000", + "0x20000", + "0x30000" + ], + "blockNumberDelayMax": 60480, + "blockTimestampDelayMax": 604800, + "blockGasLimit": 125000000, + "transactionGasLimit": 12500000, + "testing": { + "stopOnFailedTest": false, + "stopOnFailedContractMatching": false, + "stopOnNoTests": true, + "testAllContracts": false, + "traceAll": false, + "assertionTesting": { + "enabled": true, + "testViewMethods": true, + "panicCodeConfig": { + "failOnCompilerInsertedPanic": false, + "failOnAssertion": true, + "failOnArithmeticUnderflow": false, + "failOnDivideByZero": false, + "failOnEnumTypeConversionOutOfBounds": false, + "failOnIncorrectStorageAccess": false, + "failOnPopEmptyArray": false, + "failOnOutOfBoundsArrayAccess": false, + "failOnAllocateTooMuchMemory": false, + "failOnCallUninitializedVariable": false + } + }, + "propertyTesting": { + "enabled": false, + "testPrefixes": [ + "property_" + ] + }, + "optimizationTesting": { + "enabled": false, + "testPrefixes": [ + "optimize_" + ] + }, + "targetFunctionSignatures": [], + "excludeFunctionSignatures": [] + }, + "chainConfig": { + "codeSizeCheckDisabled": true, + "cheatCodes": { + "cheatCodesEnabled": true, + "enableFFI": false + }, + "skipAccountChecks": true + } + }, + "compilation": { + "platform": "crytic-compile", + "platformConfig": { + "target": ".", + "solcVersion": "", + "exportDirectory": "", + "args": ["--foundry-compile-all", "--foundry-out-directory", "artifacts"] + } + }, + "logging": { + "level": "info", + "logDirectory": "", + "noColor": false + } +} diff --git a/packages/contracts-bedrock/package.json b/packages/contracts-bedrock/package.json index 743c74d50a89..a81ac2fea050 100644 --- a/packages/contracts-bedrock/package.json +++ b/packages/contracts-bedrock/package.json @@ -46,6 +46,7 @@ "lint:fix": "yarn lint:contracts:fix && yarn lint:ts:fix", "lint": "yarn lint:fix && yarn lint:check", "typechain": "typechain --target ethers-v5 --out-dir dist/types --glob 'artifacts/!(build-info)/**/+([a-zA-Z0-9_]).json'", + "medusa": "FOUNDRY_PROFILE=medusa medusa fuzz", "echidna:aliasing": "echidna-test --contract EchidnaFuzzAddressAliasing --config ./echidna.yaml .", "echidna:burn:gas": "echidna-test --contract EchidnaFuzzBurnGas --config ./echidna.yaml .", "echidna:burn:eth": "echidna-test --contract EchidnaFuzzBurnEth --config ./echidna.yaml .", @@ -81,7 +82,7 @@ "dotenv": "^16.0.0", "ds-test": "https://github.com/dapphub/ds-test.git#9310e879db8ba3ea6d5c6489a579118fd264a3f5", "ethereum-waffle": "^3.0.0", - "forge-std": "https://github.com/foundry-rs/forge-std.git#46264e9788017fc74f9f58b7efa0bc6e1df6d410", + "forge-std": "https://github.com/foundry-rs/forge-std.git#v1.9.4", "glob": "^7.1.6", "hardhat": "^2.9.6", "hardhat-deploy": "^0.11.4", diff --git a/yarn.lock b/yarn.lock index cc9d66aa0cc5..8c31a3d61a57 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11524,14 +11524,14 @@ forever-agent@~0.6.1: resolved "https://registry.yarnpkg.com/forever-agent/-/forever-agent-0.6.1.tgz#fbc71f0c41adeb37f96c577ad1ed42d8fdacca91" integrity sha1-+8cfDEGt6zf5bFd60e1C2P2sypE= -"forge-std@https://github.com/foundry-rs/forge-std.git#46264e9788017fc74f9f58b7efa0bc6e1df6d410": - version "1.5.2" - resolved "https://github.com/foundry-rs/forge-std.git#46264e9788017fc74f9f58b7efa0bc6e1df6d410" - "forge-std@https://github.com/foundry-rs/forge-std.git#53331f4cb2e313466f72440f3e73af048c454d02": version "1.2.0" resolved "https://github.com/foundry-rs/forge-std.git#53331f4cb2e313466f72440f3e73af048c454d02" +"forge-std@https://github.com/foundry-rs/forge-std.git#v1.9.4": + version "1.9.4" + resolved "https://github.com/foundry-rs/forge-std.git#1eea5bae12ae557d589f9f0f0edae2faa47cb262" + form-data@^2.2.0: version "2.5.1" resolved "https://registry.yarnpkg.com/form-data/-/form-data-2.5.1.tgz#f2cbec57b5e59e23716e128fe44d4e5dd23895f4"