From ff8fb3a0c47db34bca45768afbcdeeed7bd5df37 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Thu, 3 Nov 2022 11:27:42 -0400 Subject: [PATCH 01/19] ctb: Add echidna tests --- .gitignore | 3 + .../contracts/L1/OptimismPortal.sol | 27 ++++ .../fuzz_tests/FuzzAddressAliasing.sol | 31 ++++ .../contracts/fuzz_tests/FuzzBurn.sol | 57 ++++++++ .../contracts/fuzz_tests/FuzzEncoding.sol | 57 ++++++++ .../contracts/fuzz_tests/FuzzHashing.sol | 102 +++++++++++++ .../fuzz_tests/FuzzOptimismPortal.sol | 109 ++++++++++++++ .../FuzzOptimismPortalWithdrawals.sol | 138 ++++++++++++++++++ .../fuzz_tests/FuzzResourceMetering.sol | 96 ++++++++++++ 9 files changed, 620 insertions(+) create mode 100644 packages/contracts-bedrock/contracts/fuzz_tests/FuzzAddressAliasing.sol create mode 100644 packages/contracts-bedrock/contracts/fuzz_tests/FuzzBurn.sol create mode 100644 packages/contracts-bedrock/contracts/fuzz_tests/FuzzEncoding.sol create mode 100644 packages/contracts-bedrock/contracts/fuzz_tests/FuzzHashing.sol create mode 100644 packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol create mode 100644 packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortalWithdrawals.sol create mode 100644 packages/contracts-bedrock/contracts/fuzz_tests/FuzzResourceMetering.sol diff --git a/.gitignore b/.gitignore index 8bfae506506ef..c2745217cdf2c 100644 --- a/.gitignore +++ b/.gitignore @@ -56,3 +56,6 @@ op-exporter __pycache__ + +# Ignore echidna artifacts +crytic-export diff --git a/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol b/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol index 3239729dea46e..fddad7f19d00b 100644 --- a/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol +++ b/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol @@ -367,8 +367,35 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { // Emit a TransactionDeposited event so that the rollup node can derive a deposit // transaction for this deposit. emit TransactionDeposited(from, _to, DEPOSIT_VERSION, opaqueData); + + // Call our post-deposit test hook + depositTransactionTestInternal(from, _to, DEPOSIT_VERSION, msg.value, _value, _gasLimit, _isCreation, _data); } + + /** + * @notice A helper function which can be overridden to capture properties of a deposit. + * + * @param from The address registered for the deposit (aliased if a contract). + * @param to Target address on L2. + * @param version Version of the encoded deposit data. + * @param mintValue ETH value to be minted. + * @param sendValue ETH value to be transferred. + * @param gasLimit Minimum L2 gas limit (can be greater than or equal to this value). + * @param isCreation Whether or not the transaction is a contract creation. + * @param data Data to trigger the recipient with. + */ + function depositTransactionTestInternal( + address from, + address to, + uint256 version, + uint256 mintValue, + uint256 sendValue, + uint64 gasLimit, + bool isCreation, + bytes memory data + ) virtual internal {} + /** * @notice Determine if the finalization period has elapsed with respect to the * passed timestamp. diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzAddressAliasing.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzAddressAliasing.sol new file mode 100644 index 0000000000000..113063dab087a --- /dev/null +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzAddressAliasing.sol @@ -0,0 +1,31 @@ +import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol"; + +contract FuzzAddressAliasing { + bool failedRoundtrip; + + /** + * @notice Takes an address to be aliased with AddressAliasHelper and then unaliased + * and updates the test contract's state indicating if the round trip encoding + * failed. + */ + function testRoundTrip(address addr) public { + // Alias our address + address aliasedAddr = AddressAliasHelper.applyL1ToL2Alias(addr); + + // Unalias our address + address undoneAliasAddr = AddressAliasHelper.undoL1ToL2Alias(aliasedAddr); + + // If our round trip aliasing did not return the original result, set our state. + if (addr != undoneAliasAddr) { + failedRoundtrip = true; + } + } + + /** + * @notice Verifies that testRoundTrip(...) did not ever fail. + */ + function echidna_round_trip_aliasing() public view returns(bool) { + // ASSERTION: The round trip aliasing done in testRoundTrip(...) should never fail. + return !failedRoundtrip; + } +} \ No newline at end of file diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzBurn.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzBurn.sol new file mode 100644 index 0000000000000..29dfdd395bab1 --- /dev/null +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzBurn.sol @@ -0,0 +1,57 @@ +import { Burn } from "../libraries/Burn.sol"; + +contract FuzzBurn { + bool failedEthBurn; + bool failedGasBurn; + + /** + * @notice Takes an integer amount of eth to burn through the Burn library and + * updates the contract state if an incorrect amount of eth moved from the contract + */ + function testBurn(uint256 _value) public { + // cache the contract's eth balance + uint256 preBurnBalance = address(this).balance; + + // execute a burn of _value eth + // (may way to add guardrails to this value rather than a truly unbounded uint256) + Burn.eth(_value); + + // check that exactly _value eth was transfered from the contract + if (address(this).balance != preBurnBalance - _value) { + failedEthBurn = true; + } + } + + /** + * @notice Takes an integer amount of gas to burn through the Burn library and + * updates the contract state if at least that amount of gas was not burned + * by the library + */ + function testGas(uint256 _value) public { + // cache the contract's current remaining gas + uint256 preBurnGas = gasleft(); + + // execute the gas burn + Burn.gas(_value); + + // cache the remaining gas post burn + uint256 postBurnGas = gasleft(); + + // check that at least _value gas was burnt + if (postBurnGas > preBurnGas - _value) { + failedGasBurn; + } + } + + function echidna_burn_eth() public view returns(bool) { + // ASSERTION: The amount burned should always match the amount passed exactly + return !failedEthBurn; + } + + function echidna_burn_gas() public view returns (bool) { + // ASSERTION: The amount of gas burned should be strictly greater than the + // the amount passed as _value (minimum _value + whatever minor overhead to + // the value after the call) + return !failedGasBurn; + } +} \ No newline at end of file diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzEncoding.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzEncoding.sol new file mode 100644 index 0000000000000..67872d6767294 --- /dev/null +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzEncoding.sol @@ -0,0 +1,57 @@ +import { Encoding } from "../libraries/Encoding.sol"; + +contract FuzzEncoding { + bool failedRoundtripAToB; + bool failedRoundtripBToA; + + /** + * @notice Takes a pair of integers to be encoded into a versioned nonce with the + * Encoding library and then decoded and updates the test contract's state + * indicating if the round trip encoding failed. + */ + function testRoundTripAToB(uint240 _nonce, uint16 _version) public { + // encode the address + uint256 encodedVersionedNonce = Encoding.encodeVersionedNonce(_nonce, _version); + + // Unalias our address + uint240 decodedNonce; + uint16 decodedVersion; + + (decodedNonce, decodedVersion) = Encoding.decodeVersionedNonce(encodedVersionedNonce); + + // If our round trip encoding did not return the original result, set our state. + if ((decodedNonce != _nonce) || (decodedVersion != _version)) { + failedRoundtripAToB = true; + } + } + + /** + * @notice Takes an integer representing a packed version and nonce and attempts + * to decode them using the Encoding library before re-encoding and updates + * the test contract's state indicating if the round trip encoding failed. + */ + function testRoundTripBToA(uint256 _versionedNonce) public { + + // Unalias our address + uint240 decodedNonce; + uint16 decodedVersion; + + (decodedNonce, decodedVersion) = Encoding.decodeVersionedNonce(_versionedNonce); + + // encode the address + uint256 encodedVersionedNonce = Encoding.encodeVersionedNonce(decodedNonce, decodedVersion); + + // If our round trip encoding did not return the original result, set our state. + if (encodedVersionedNonce != _versionedNonce) { + failedRoundtripBToA = true; + } + } + + /** + * @notice Verifies that testRoundTrip(...) did not ever fail. + */ + function echidna_round_trip_encoding() public view returns(bool) { + // ASSERTION: The round trip encoding done in testRoundTripAToB(...)/BToA(...) should never fail. + return !failedRoundtripAToB && !failedRoundtripBToA; + } +} \ No newline at end of file diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzHashing.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzHashing.sol new file mode 100644 index 0000000000000..f2669c4f909fd --- /dev/null +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzHashing.sol @@ -0,0 +1,102 @@ +import { Hashing } from "../libraries/Hashing.sol"; +import { Encoding } from "../libraries/Encoding.sol"; + +contract FuzzHashing { + bool failedCrossDomainHashHighVersion; + bool failedCrossDomainHashV0; + bool failedCrossDomainHashV1; + + /** + * @notice Takes the necessary parameters to perform a cross domain hash with a randomly + * generated version. Only schema versions 0 and 1 are supported and all others should revert. + */ + function testHashCrossDomainMessageHighVersion( + uint16 _version, + uint240 _nonce, + address _sender, + address _target, + uint256 _value, + uint256 _gasLimit, + bytes memory _data + ) public { + // generate the versioned nonce + uint256 encodedNonce = Encoding.encodeVersionedNonce(_nonce, _version); + + // hash the cross domain message. we don't need to store the result since the function validates + // and should revert if an invalid version (>1) is encoded + Hashing.hashCrossDomainMessage(encodedNonce, _sender, _target, _value, _gasLimit, _data); + + // check that execution never makes it this far for an invalid version + if (_version > 1) { + failedCrossDomainHashHighVersion = true; + } + } + + /** + * @notice Takes the necessary parameters to perform a cross domain hash using the v0 schema + * and compares the output of a call to the unversioned function to the v0 function directly + */ + function testHashCrossDomainMessageV0( + uint240 _nonce, + address _sender, + address _target, + uint256 _value, + uint256 _gasLimit, + bytes memory _data + ) public { + // generate the versioned nonce with the version set to 0 + uint256 encodedNonce = Encoding.encodeVersionedNonce(_nonce, 0); + + // hash the cross domain message using the unversioned and versioned functions for comparison + bytes32 sampleHash1 = Hashing.hashCrossDomainMessage(encodedNonce, _sender, _target, _value, _gasLimit, _data); + bytes32 sampleHash2 = Hashing.hashCrossDomainMessageV0(_target, _sender, _data, encodedNonce); + + // check that the output of both functions matches + if (sampleHash1 != sampleHash2) { + failedCrossDomainHashV0 = true; + } + } + + /** + * @notice Takes the necessary parameters to perform a cross domain hash using the v1 schema + * and compares the output of a call to the unversioned function to the v1 function directly + */ + function testHashCrossDomainMessageV1( + uint240 _nonce, + address _sender, + address _target, + uint256 _value, + uint256 _gasLimit, + bytes memory _data + ) public { + // generate the versioned nonce with the version set to 1 + uint256 encodedNonce = Encoding.encodeVersionedNonce(_nonce, 1); + + // hash the cross domain message using the unversioned and versioned functions for comparison + bytes32 sampleHash1 = Hashing.hashCrossDomainMessage(encodedNonce, _sender, _target, _value, _gasLimit, _data); + bytes32 sampleHash2 = Hashing.hashCrossDomainMessageV1(encodedNonce, _sender, _target, _value, _gasLimit, _data); + + // check that the output of both functions matches + if (sampleHash1 != sampleHash2) { + failedCrossDomainHashV1 = true; + } + } + + + function echidna_hash_xdomain_msg_high_version() public view returns(bool) { + // ASSERTION: A call to hashCrossDomainMessage will never succeed for a version > 1 + return !failedCrossDomainHashHighVersion; + } + + function echidna_hash_xdomain_msg_0() public view returns (bool) { + // ASSERTION: A call to hashCrossDomainMessage and hashCrossDomainMessageV0 + // should always match when the version passed is 0 + return !failedCrossDomainHashV0; + } + + function echidna_hash_xdomain_msg_1() public view returns (bool) { + // ASSERTION: A call to hashCrossDomainMessage and hashCrossDomainMessageV1 + // should always match when the version passed is 1 + return !failedCrossDomainHashV1; + } +} \ No newline at end of file diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol new file mode 100644 index 0000000000000..8b090b9473c58 --- /dev/null +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol @@ -0,0 +1,109 @@ +import { OptimismPortal } from "../L1/OptimismPortal.sol"; +import { L2OutputOracle } from "../L1/L2OutputOracle.sol"; +import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol"; + +contract FuzzOptimismPortal is OptimismPortal { + uint reinitializedCount; + bool failedDepositCreationNonZeroAddr; + bool failedAliasingContractFromAddr; + bool failedNoAliasingFromEOA; + bool failedMintedLessThanTaken; + + + constructor() OptimismPortal(L2OutputOracle(address(0)), 10) { + // Note: The base constructor will call initialize() once here. + } + + /** + * @notice This method calls upon OptimismPortal.initialize() to ensure + * no additional initializations are possible. + */ + function initializeEx() public { + super.initialize(); + reinitializedCount++; + } + + /** + * @notice This method calls upon OptimismPortal.depositTransaction() to ensure + * a deposit with _isCreation set to true never succeeds with a _to address that + * has a non-zero value. + */ + function depositTransactionIsCreation(address _to, uint256 _value, uint64 _gasLimit, bytes memory _data) public { + // Deposit with our given fuzz parameters and _isCreation set to true + depositTransaction(_to, _value, _gasLimit, true, _data); + + // If we did not revert and our _to address is not zero, flag a failure. + if (_to != address(0x0)) + { + failedDepositCreationNonZeroAddr = true; + } + } + + /** + * @notice This method calls upon OptimismPortal.depositTransaction() from a + * contract address (itself, it performs an external call) to ensure contract + * aliasing is tested by depositTransactionTestInternal. + */ + function depositTransactionFromContract(address _to, uint256 _value, uint64 _gasLimit, bool _isCreation, bytes memory _data) public payable { + // We perform an external call to our own address to trigger the conditions for a deposit by a contract address. + // This is because when we perform an external call, the receiving function will see msg.sender as the caller's address. + // Because we provide a function to ensure a call from a contract address, we'll be sure the fuzzer tested + // this case. + OptimismPortal(payable(this)).depositTransaction{value: msg.value}(_to, _value, _gasLimit, _isCreation, _data); + } + + /** + * @notice This override is called at the end of OptimismPortal.depositTransaction() + * so that we can sanity check all of the input and omitted data. + */ + function depositTransactionTestInternal( + address from, + address to, + uint256 version, + uint256 mintValue, + uint256 sendValue, + uint64 gasLimit, + bool isCreation, + bytes memory data + ) override internal { + // Check if the caller is a contract and confirm our address aliasing properties + if (msg.sender != tx.origin) { + // If the caller is a contract, we expect the address to be aliased. + if(AddressAliasHelper.undoL1ToL2Alias(from) != msg.sender) { + failedAliasingContractFromAddr = true; + } + } else { + // If the caller is an EOA address, we expect the address not to be aliased. + if (from != msg.sender) { + failedNoAliasingFromEOA = true; + } + } + + // If our mint value exceeds the amount paid, we failed a test. + if (mintValue > msg.value) { + failedMintedLessThanTaken = true; + } + } + + function echidna_never_initialize_twice() public view returns (bool) { + return reinitializedCount == 0; + } + + function echidna_never_nonzero_to_creation_deposit() public view returns (bool) { + return !failedDepositCreationNonZeroAddr; + } + + function echidna_alias_from_contract_deposit() public view returns (bool) { + return !failedAliasingContractFromAddr; + } + + function echidna_no_alias_from_EOA_deposit() public view returns (bool) { + return !failedNoAliasingFromEOA; + } + + function echidna_mint_less_than_taken() public view returns (bool) { + return !failedMintedLessThanTaken; + } + + +} \ No newline at end of file diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortalWithdrawals.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortalWithdrawals.sol new file mode 100644 index 0000000000000..b2de5f69f641a --- /dev/null +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortalWithdrawals.sol @@ -0,0 +1,138 @@ +import { OptimismPortal } from "../L1/OptimismPortal.sol"; +import { L2OutputOracle } from "../L1/L2OutputOracle.sol"; +import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol"; +import { Types } from "../libraries/Types.sol"; + +contract EchidnaL2OutputOracle is L2OutputOracle { + constructor( + uint256 _submissionInterval, + bytes32 _genesisL2Output, + uint256 _historicalTotalBlocks, + uint256 _startingBlockNumber, + uint256 _startingTimestamp, + uint256 _l2BlockTime + ) L2OutputOracle( + _submissionInterval, + _genesisL2Output, + _historicalTotalBlocks, + _startingBlockNumber, + _startingTimestamp, + _l2BlockTime, + address(0xAbBa), + address(0xACDC) + ) { + // standard initialization migrates both proposer and owner from the + // deployer to new addresses and prevents them from being the same + // address. however, we need the Fuzz[...] contract, the deployer in + // this case, to be able to propose to the oracle, so we extend it + // just to be able to force the proposer role back on to the testing + // contract (we don't care what addresses are passed to the constructor + // so we borrow from CommonTest.t.sol again + proposer = msg.sender; + } +} + +contract FuzzOptimismPortalWithdrawals{ + // since portal.finalizedWithdrawals is declared `external`, we can't use helper + // functions to check state before/after withdrawals, so we split these tests + // into a separate contract that only interacts externally + OptimismPortal portal; + L2OutputOracle oracle; + + Types.WithdrawalTransaction cachedTx; + uint256 cachedL2BlockNumber; + Types.OutputRootProof cachedOutputRootProof; + bytes cachedWithdrawalProof; + + uint256 offset; + + bool failedFinalizeEarly; + bool failedMismatchedOutputRoot; + + constructor() { + // seeding with the values from CommonTest.t.sol + oracle = new EchidnaL2OutputOracle(1800, keccak256(abi.encode(0)), 199, 200, 1000, 2); + portal = new OptimismPortal(oracle, 7 days); + } + + /** + * @notice Submits a proposal to the L2OutputOracle. Currently completely unstructured + */ + function proposeL2Output( + bytes32 _outputRoot, + bytes32 _l2BlockNumber, + bytes32 _l1Blockhash, + uint256 _l1BlockNumber + ) public { + // TODO: provide more structure for this data + oracle.proposeL2Output(_outputRoot, oracle.nextBlockNumber(), _l1Blockhash, _l1BlockNumber); + } + + /** + * @notice Generates a withdrawal transaction and (TODO) performs the necessary state changes + * to ensure it will validate + */ + function finalizeValidWithdrawal( + Types.WithdrawalTransaction calldata _tx, + uint256 _l2BlockNumber, + Types.OutputRootProof calldata _outputRootProof, + bytes calldata _withdrawalProof + ) public { + // TODO: craft a valid call to oracle.proposeL2Output for this generated withdrawal + + portal.finalizeWithdrawalTransaction(_tx, _l2BlockNumber, _outputRootProof, _withdrawalProof); + + // if we haven't reverted, we have a valid withdrawal transaction. + // save it so we can test for replay protection + cachedTx = _tx; + cachedL2BlockNumber = _l2BlockNumber; + cachedOutputRootProof = _outputRootProof; + cachedWithdrawalProof = _withdrawalProof; + } + + /** + * @notice Generates a withdrawal transaction and (TODO) performs the necessary state changes + * such that it is an otherwise valid transaction, but the finalization period has not + * yet concluded. + */ + function finalizeEarlyWithdrawal( + Types.WithdrawalTransaction calldata _tx, + uint256 _l2BlockNumber, + Types.OutputRootProof calldata _outputRootProof, + bytes calldata _withdrawalProof + ) public { + // TODO: craft a valid call to oracle.proposeL2Output, still within the finalization period + + portal.finalizeWithdrawalTransaction(_tx, _l2BlockNumber, _outputRootProof, _withdrawalProof); + + // Execution should have reverted prior to this since we're still within the finalization window + failedFinalizeEarly = true; + } + + /** + * @notice Helper function so we can access some randomness in the echidna_* tests + */ + function setOffset(uint256 _offset) public { + offset = _offset; + } + + function echidna_never_finalize_early() public view returns (bool) { + return !failedFinalizeEarly; + } + + function echidna_never_finalize_twice() public returns (bool) { + if (cachedL2BlockNumber != 0) { + portal.finalizeWithdrawalTransaction(cachedTx, cachedL2BlockNumber, cachedOutputRootProof, cachedWithdrawalProof); + return false; + } + return true; + } + + function echidna_never_finalize_no_oracle_data() public returns (bool) { + if (cachedL2BlockNumber != 0) { + portal.finalizeWithdrawalTransaction(cachedTx, (oracle.nextBlockNumber() + (offset % 5000)), cachedOutputRootProof, cachedWithdrawalProof); + return false; + } + return true; + } +} \ No newline at end of file diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzResourceMetering.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzResourceMetering.sol new file mode 100644 index 0000000000000..0c8e83bb166c2 --- /dev/null +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzResourceMetering.sol @@ -0,0 +1,96 @@ +import { ResourceMetering } from "../L1/ResourceMetering.sol"; + +contract FuzzResourceMetering is ResourceMetering { + bool failedMaxGasPerBlock; + bool failedRaiseBasefee; + bool failedLowerBasefee; + bool failedNeverBelowMinBasefee; + bool failedMaxRaiseBasefeePerBlock; + bool failedMaxLowerBasefeePerBlock; + + constructor() { + initialize(); + } + + function initialize() public initializer { + __ResourceMetering_init(); + } + + /** + * @notice Takes the necessary parameters to allow us to burn arbitrary amounts of gas to test the + * underlying resource metering/gas market logic + */ + function testBurn(uint256 _gasToBurn, bool _raiseBaseFee) public { + uint256 cachedPrevBaseFee = uint256(params.prevBaseFee); + uint256 cachedPrevBoughtGas = uint256(params.prevBoughtGas); + uint256 cachedPrevBlockNum = uint256(params.prevBlockNum); + uint256 maxBasefeeChange = cachedPrevBaseFee / uint256(BASE_FEE_MAX_CHANGE_DENOMINATOR); + + // check that the last block's base fee hadn't dropped below the minimum + if (cachedPrevBaseFee < uint256(MINIMUM_BASE_FEE)) { + failedNeverBelowMinBasefee = true; + } + // check that the last block didn't consume more than the max amount of gas + if (cachedPrevBoughtGas > uint256(MAX_RESOURCE_LIMIT)) { + failedMaxGasPerBlock = true; + } + // if the last block used more than the target amount of gas (and there were no + // empty blocks in between), ensure this block's basefee increased, but not by + // more than the max amount per block + if ((cachedPrevBoughtGas > uint256(TARGET_RESOURCE_LIMIT)) && (uint256(params.prevBlockNum) - cachedPrevBlockNum == 1)) { + failedRaiseBasefee = failedRaiseBasefee || (params.prevBaseFee <= cachedPrevBaseFee); + failedMaxRaiseBasefeePerBlock = failedMaxRaiseBasefeePerBlock || ((uint256(params.prevBaseFee) - cachedPrevBaseFee) < maxBasefeeChange); + } + // if the last blocked used less than the target amount of gas (and currently, + // that there were no empty blocks in between), ensure this block's basefee decreased, + // but not by more than the max amount + if ((cachedPrevBoughtGas < uint256(TARGET_RESOURCE_LIMIT)) || (uint256(params.prevBlockNum) - cachedPrevBlockNum > 1)) { + failedLowerBasefee = failedLowerBasefee || (uint256(params.prevBaseFee) >= cachedPrevBaseFee); + // TODO: account for empty blocks + if (params.prevBlockNum - cachedPrevBaseFee == 1) { + failedMaxLowerBasefeePerBlock = failedMaxLowerBasefeePerBlock || ((cachedPrevBaseFee - uint256(params.prevBaseFee)) < maxBasefeeChange); + } + } + + // force the gasToBurn into the correct range based on whether we intend to + // raise or lower the basefee after this block, respectively + uint256 gasToBurn; + if (_raiseBaseFee) { + gasToBurn = (_gasToBurn % (uint256(MAX_RESOURCE_LIMIT) - uint256(TARGET_RESOURCE_LIMIT))) + + uint256(TARGET_RESOURCE_LIMIT); + } + else { + gasToBurn = _gasToBurn % uint256(TARGET_RESOURCE_LIMIT); + } + + _burnInternal(uint64(gasToBurn)); + } + + function _burnInternal(uint64 _gasToBurn) private metered(_gasToBurn) { + + } + + function echidna_high_usage_raise_basefee() public view returns(bool) { + return !failedRaiseBasefee; + } + + function echidna_low_usage_lower_basefee() public view returns(bool) { + return !failedLowerBasefee; + } + + function echidna_never_below_min_basefee() public view returns(bool) { + return !failedNeverBelowMinBasefee; + } + + function echidna_never_above_max_gas_limit() public view returns(bool) { + return !failedMaxGasPerBlock; + } + + function echidna_never_exceed_max_increase() public view returns(bool) { + return !failedMaxRaiseBasefeePerBlock; + } + + function echidna_never_exceed_max_decrease() public view returns(bool) { + return !failedMaxLowerBasefeePerBlock; + } +} \ No newline at end of file From 3c0ad5eabafbf87ec5e505af055d76d6e147b94e Mon Sep 17 00:00:00 2001 From: Maurelian Date: Thu, 3 Nov 2022 20:25:40 -0400 Subject: [PATCH 02/19] ctb: Undo changes to portal for echidna tests --- .../contracts/L1/OptimismPortal.sol | 27 ------------------- .../fuzz_tests/FuzzOptimismPortal.sol | 5 +++- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol b/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol index fddad7f19d00b..3239729dea46e 100644 --- a/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol +++ b/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol @@ -367,35 +367,8 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { // Emit a TransactionDeposited event so that the rollup node can derive a deposit // transaction for this deposit. emit TransactionDeposited(from, _to, DEPOSIT_VERSION, opaqueData); - - // Call our post-deposit test hook - depositTransactionTestInternal(from, _to, DEPOSIT_VERSION, msg.value, _value, _gasLimit, _isCreation, _data); } - - /** - * @notice A helper function which can be overridden to capture properties of a deposit. - * - * @param from The address registered for the deposit (aliased if a contract). - * @param to Target address on L2. - * @param version Version of the encoded deposit data. - * @param mintValue ETH value to be minted. - * @param sendValue ETH value to be transferred. - * @param gasLimit Minimum L2 gas limit (can be greater than or equal to this value). - * @param isCreation Whether or not the transaction is a contract creation. - * @param data Data to trigger the recipient with. - */ - function depositTransactionTestInternal( - address from, - address to, - uint256 version, - uint256 mintValue, - uint256 sendValue, - uint64 gasLimit, - bool isCreation, - bytes memory data - ) virtual internal {} - /** * @notice Determine if the finalization period has elapsed with respect to the * passed timestamp. diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol index 8b090b9473c58..33096689a9d47 100644 --- a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol @@ -55,6 +55,9 @@ contract FuzzOptimismPortal is OptimismPortal { /** * @notice This override is called at the end of OptimismPortal.depositTransaction() * so that we can sanity check all of the input and omitted data. + * + * Note: This is currently disabled (by setting the visibility to internal), as it required + * modifying the target contracts. We keep it here for posterity. */ function depositTransactionTestInternal( address from, @@ -106,4 +109,4 @@ contract FuzzOptimismPortal is OptimismPortal { } -} \ No newline at end of file +} From 9e30656c19fb7dfa1f45571a58746cd32c19db6b Mon Sep 17 00:00:00 2001 From: Maurelian Date: Thu, 3 Nov 2022 23:39:15 -0400 Subject: [PATCH 03/19] ctb: Fixes to echidna fuzz contracts ctb: Format echidna fuzz contracts --- .../fuzz_tests/FuzzAddressAliasing.sol | 6 +- .../contracts/fuzz_tests/FuzzBurn.sol | 8 ++- .../contracts/fuzz_tests/FuzzEncoding.sol | 12 ++-- .../contracts/fuzz_tests/FuzzHashing.sol | 53 +++++++++++---- .../fuzz_tests/FuzzOptimismPortal.sol | 58 ++++++++++------ .../FuzzOptimismPortalWithdrawals.sol | 67 +++++++++++++------ .../fuzz_tests/FuzzResourceMetering.sol | 54 +++++++++------ 7 files changed, 173 insertions(+), 85 deletions(-) diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzAddressAliasing.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzAddressAliasing.sol index 113063dab087a..280a4734cd240 100644 --- a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzAddressAliasing.sol +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzAddressAliasing.sol @@ -1,3 +1,5 @@ +pragma solidity 0.8.15; + import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol"; contract FuzzAddressAliasing { @@ -24,8 +26,8 @@ contract FuzzAddressAliasing { /** * @notice Verifies that testRoundTrip(...) did not ever fail. */ - function echidna_round_trip_aliasing() public view returns(bool) { + function echidna_round_trip_aliasing() public view returns (bool) { // ASSERTION: The round trip aliasing done in testRoundTrip(...) should never fail. return !failedRoundtrip; } -} \ No newline at end of file +} diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzBurn.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzBurn.sol index 29dfdd395bab1..83cc642f75f1b 100644 --- a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzBurn.sol +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzBurn.sol @@ -1,3 +1,5 @@ +pragma solidity 0.8.15; + import { Burn } from "../libraries/Burn.sol"; contract FuzzBurn { @@ -27,7 +29,7 @@ contract FuzzBurn { * updates the contract state if at least that amount of gas was not burned * by the library */ - function testGas(uint256 _value) public { + function testGas(uint256 _value) public view { // cache the contract's current remaining gas uint256 preBurnGas = gasleft(); @@ -43,7 +45,7 @@ contract FuzzBurn { } } - function echidna_burn_eth() public view returns(bool) { + function echidna_burn_eth() public view returns (bool) { // ASSERTION: The amount burned should always match the amount passed exactly return !failedEthBurn; } @@ -54,4 +56,4 @@ contract FuzzBurn { // the value after the call) return !failedGasBurn; } -} \ No newline at end of file +} diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzEncoding.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzEncoding.sol index 67872d6767294..b53ab51812521 100644 --- a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzEncoding.sol +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzEncoding.sol @@ -1,3 +1,5 @@ +pragma solidity 0.8.15; + import { Encoding } from "../libraries/Encoding.sol"; contract FuzzEncoding { @@ -9,7 +11,7 @@ contract FuzzEncoding { * Encoding library and then decoded and updates the test contract's state * indicating if the round trip encoding failed. */ - function testRoundTripAToB(uint240 _nonce, uint16 _version) public { + function testRoundTripAToB(uint240 _nonce, uint16 _version) public { // encode the address uint256 encodedVersionedNonce = Encoding.encodeVersionedNonce(_nonce, _version); @@ -31,7 +33,6 @@ contract FuzzEncoding { * the test contract's state indicating if the round trip encoding failed. */ function testRoundTripBToA(uint256 _versionedNonce) public { - // Unalias our address uint240 decodedNonce; uint16 decodedVersion; @@ -50,8 +51,9 @@ contract FuzzEncoding { /** * @notice Verifies that testRoundTrip(...) did not ever fail. */ - function echidna_round_trip_encoding() public view returns(bool) { - // ASSERTION: The round trip encoding done in testRoundTripAToB(...)/BToA(...) should never fail. + function echidna_round_trip_encoding() public view returns (bool) { + // ASSERTION: The round trip encoding done in testRoundTripAToB(...)/BToA(...) should never + // fail. return !failedRoundtripAToB && !failedRoundtripBToA; } -} \ No newline at end of file +} diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzHashing.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzHashing.sol index f2669c4f909fd..fbc4bb31ab01e 100644 --- a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzHashing.sol +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzHashing.sol @@ -1,3 +1,5 @@ +pragma solidity 0.8.15; + import { Hashing } from "../libraries/Hashing.sol"; import { Encoding } from "../libraries/Encoding.sol"; @@ -22,8 +24,8 @@ contract FuzzHashing { // generate the versioned nonce uint256 encodedNonce = Encoding.encodeVersionedNonce(_nonce, _version); - // hash the cross domain message. we don't need to store the result since the function validates - // and should revert if an invalid version (>1) is encoded + // hash the cross domain message. we don't need to store the result since the function + // validates and should revert if an invalid version (>1) is encoded Hashing.hashCrossDomainMessage(encodedNonce, _sender, _target, _value, _gasLimit, _data); // check that execution never makes it this far for an invalid version @@ -47,9 +49,22 @@ contract FuzzHashing { // generate the versioned nonce with the version set to 0 uint256 encodedNonce = Encoding.encodeVersionedNonce(_nonce, 0); - // hash the cross domain message using the unversioned and versioned functions for comparison - bytes32 sampleHash1 = Hashing.hashCrossDomainMessage(encodedNonce, _sender, _target, _value, _gasLimit, _data); - bytes32 sampleHash2 = Hashing.hashCrossDomainMessageV0(_target, _sender, _data, encodedNonce); + // hash the cross domain message using the unversioned and versioned functions for + // comparison + bytes32 sampleHash1 = Hashing.hashCrossDomainMessage( + encodedNonce, + _sender, + _target, + _value, + _gasLimit, + _data + ); + bytes32 sampleHash2 = Hashing.hashCrossDomainMessageV0( + _target, + _sender, + _data, + encodedNonce + ); // check that the output of both functions matches if (sampleHash1 != sampleHash2) { @@ -57,7 +72,7 @@ contract FuzzHashing { } } - /** + /** * @notice Takes the necessary parameters to perform a cross domain hash using the v1 schema * and compares the output of a call to the unversioned function to the v1 function directly */ @@ -72,9 +87,24 @@ contract FuzzHashing { // generate the versioned nonce with the version set to 1 uint256 encodedNonce = Encoding.encodeVersionedNonce(_nonce, 1); - // hash the cross domain message using the unversioned and versioned functions for comparison - bytes32 sampleHash1 = Hashing.hashCrossDomainMessage(encodedNonce, _sender, _target, _value, _gasLimit, _data); - bytes32 sampleHash2 = Hashing.hashCrossDomainMessageV1(encodedNonce, _sender, _target, _value, _gasLimit, _data); + // hash the cross domain message using the unversioned and versioned functions for + // comparison + bytes32 sampleHash1 = Hashing.hashCrossDomainMessage( + encodedNonce, + _sender, + _target, + _value, + _gasLimit, + _data + ); + bytes32 sampleHash2 = Hashing.hashCrossDomainMessageV1( + encodedNonce, + _sender, + _target, + _value, + _gasLimit, + _data + ); // check that the output of both functions matches if (sampleHash1 != sampleHash2) { @@ -82,8 +112,7 @@ contract FuzzHashing { } } - - function echidna_hash_xdomain_msg_high_version() public view returns(bool) { + function echidna_hash_xdomain_msg_high_version() public view returns (bool) { // ASSERTION: A call to hashCrossDomainMessage will never succeed for a version > 1 return !failedCrossDomainHashHighVersion; } @@ -99,4 +128,4 @@ contract FuzzHashing { // should always match when the version passed is 1 return !failedCrossDomainHashV1; } -} \ No newline at end of file +} diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol index 33096689a9d47..cec188cc08b1d 100644 --- a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol @@ -1,15 +1,16 @@ +pragma solidity 0.8.15; + import { OptimismPortal } from "../L1/OptimismPortal.sol"; import { L2OutputOracle } from "../L1/L2OutputOracle.sol"; import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol"; contract FuzzOptimismPortal is OptimismPortal { - uint reinitializedCount; + uint256 reinitializedCount; bool failedDepositCreationNonZeroAddr; bool failedAliasingContractFromAddr; bool failedNoAliasingFromEOA; bool failedMintedLessThanTaken; - constructor() OptimismPortal(L2OutputOracle(address(0)), 10) { // Note: The base constructor will call initialize() once here. } @@ -28,13 +29,17 @@ contract FuzzOptimismPortal is OptimismPortal { * a deposit with _isCreation set to true never succeeds with a _to address that * has a non-zero value. */ - function depositTransactionIsCreation(address _to, uint256 _value, uint64 _gasLimit, bytes memory _data) public { + function depositTransactionIsCreation( + address _to, + uint256 _value, + uint64 _gasLimit, + bytes memory _data + ) public { // Deposit with our given fuzz parameters and _isCreation set to true depositTransaction(_to, _value, _gasLimit, true, _data); // If we did not revert and our _to address is not zero, flag a failure. - if (_to != address(0x0)) - { + if (_to != address(0x0)) { failedDepositCreationNonZeroAddr = true; } } @@ -44,12 +49,25 @@ contract FuzzOptimismPortal is OptimismPortal { * contract address (itself, it performs an external call) to ensure contract * aliasing is tested by depositTransactionTestInternal. */ - function depositTransactionFromContract(address _to, uint256 _value, uint64 _gasLimit, bool _isCreation, bytes memory _data) public payable { - // We perform an external call to our own address to trigger the conditions for a deposit by a contract address. - // This is because when we perform an external call, the receiving function will see msg.sender as the caller's address. - // Because we provide a function to ensure a call from a contract address, we'll be sure the fuzzer tested - // this case. - OptimismPortal(payable(this)).depositTransaction{value: msg.value}(_to, _value, _gasLimit, _isCreation, _data); + function depositTransactionFromContract( + address _to, + uint256 _value, + uint64 _gasLimit, + bool _isCreation, + bytes memory _data + ) public payable { + // We perform an external call to our own address to trigger the conditions for a deposit + // by a contract address. This is because when we perform an external call, the receiving + // function will see msg.sender as the caller's address. + // Because we provide a function to ensure a call from a contract address, we'll be sure the + // fuzzer tested this case. + OptimismPortal(payable(this)).depositTransaction{ value: msg.value }( + _to, + _value, + _gasLimit, + _isCreation, + _data + ); } /** @@ -61,18 +79,18 @@ contract FuzzOptimismPortal is OptimismPortal { */ function depositTransactionTestInternal( address from, - address to, - uint256 version, + address, + uint256, uint256 mintValue, - uint256 sendValue, - uint64 gasLimit, - bool isCreation, - bytes memory data - ) override internal { + uint256, + uint64, + bool, + bytes memory + ) internal { // Check if the caller is a contract and confirm our address aliasing properties if (msg.sender != tx.origin) { // If the caller is a contract, we expect the address to be aliased. - if(AddressAliasHelper.undoL1ToL2Alias(from) != msg.sender) { + if (AddressAliasHelper.undoL1ToL2Alias(from) != msg.sender) { failedAliasingContractFromAddr = true; } } else { @@ -107,6 +125,4 @@ contract FuzzOptimismPortal is OptimismPortal { function echidna_mint_less_than_taken() public view returns (bool) { return !failedMintedLessThanTaken; } - - } diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortalWithdrawals.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortalWithdrawals.sol index b2de5f69f641a..9060ce77bc405 100644 --- a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortalWithdrawals.sol +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortalWithdrawals.sol @@ -1,3 +1,5 @@ +pragma solidity 0.8.15; + import { OptimismPortal } from "../L1/OptimismPortal.sol"; import { L2OutputOracle } from "../L1/L2OutputOracle.sol"; import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol"; @@ -11,16 +13,18 @@ contract EchidnaL2OutputOracle is L2OutputOracle { uint256 _startingBlockNumber, uint256 _startingTimestamp, uint256 _l2BlockTime - ) L2OutputOracle( - _submissionInterval, - _genesisL2Output, - _historicalTotalBlocks, - _startingBlockNumber, - _startingTimestamp, - _l2BlockTime, - address(0xAbBa), - address(0xACDC) - ) { + ) + L2OutputOracle( + _submissionInterval, + _genesisL2Output, + _historicalTotalBlocks, + _startingBlockNumber, + _startingTimestamp, + _l2BlockTime, + address(0xAbBa), + address(0xACDC) + ) + { // standard initialization migrates both proposer and owner from the // deployer to new addresses and prevents them from being the same // address. however, we need the Fuzz[...] contract, the deployer in @@ -32,7 +36,7 @@ contract EchidnaL2OutputOracle is L2OutputOracle { } } -contract FuzzOptimismPortalWithdrawals{ +contract FuzzOptimismPortalWithdrawals { // since portal.finalizedWithdrawals is declared `external`, we can't use helper // functions to check state before/after withdrawals, so we split these tests // into a separate contract that only interacts externally @@ -42,7 +46,7 @@ contract FuzzOptimismPortalWithdrawals{ Types.WithdrawalTransaction cachedTx; uint256 cachedL2BlockNumber; Types.OutputRootProof cachedOutputRootProof; - bytes cachedWithdrawalProof; + bytes[] cachedWithdrawalProof; uint256 offset; @@ -60,7 +64,7 @@ contract FuzzOptimismPortalWithdrawals{ */ function proposeL2Output( bytes32 _outputRoot, - bytes32 _l2BlockNumber, + bytes32, bytes32 _l1Blockhash, uint256 _l1BlockNumber ) public { @@ -76,11 +80,16 @@ contract FuzzOptimismPortalWithdrawals{ Types.WithdrawalTransaction calldata _tx, uint256 _l2BlockNumber, Types.OutputRootProof calldata _outputRootProof, - bytes calldata _withdrawalProof + bytes[] memory _withdrawalProof ) public { // TODO: craft a valid call to oracle.proposeL2Output for this generated withdrawal - portal.finalizeWithdrawalTransaction(_tx, _l2BlockNumber, _outputRootProof, _withdrawalProof); + portal.finalizeWithdrawalTransaction( + _tx, + _l2BlockNumber, + _outputRootProof, + _withdrawalProof + ); // if we haven't reverted, we have a valid withdrawal transaction. // save it so we can test for replay protection @@ -99,13 +108,19 @@ contract FuzzOptimismPortalWithdrawals{ Types.WithdrawalTransaction calldata _tx, uint256 _l2BlockNumber, Types.OutputRootProof calldata _outputRootProof, - bytes calldata _withdrawalProof + bytes[] calldata _withdrawalProof ) public { // TODO: craft a valid call to oracle.proposeL2Output, still within the finalization period - portal.finalizeWithdrawalTransaction(_tx, _l2BlockNumber, _outputRootProof, _withdrawalProof); + portal.finalizeWithdrawalTransaction( + _tx, + _l2BlockNumber, + _outputRootProof, + _withdrawalProof + ); - // Execution should have reverted prior to this since we're still within the finalization window + // Execution should have reverted prior to this since we're still within the finalization + // window failedFinalizeEarly = true; } @@ -122,7 +137,12 @@ contract FuzzOptimismPortalWithdrawals{ function echidna_never_finalize_twice() public returns (bool) { if (cachedL2BlockNumber != 0) { - portal.finalizeWithdrawalTransaction(cachedTx, cachedL2BlockNumber, cachedOutputRootProof, cachedWithdrawalProof); + portal.finalizeWithdrawalTransaction( + cachedTx, + cachedL2BlockNumber, + cachedOutputRootProof, + cachedWithdrawalProof + ); return false; } return true; @@ -130,9 +150,14 @@ contract FuzzOptimismPortalWithdrawals{ function echidna_never_finalize_no_oracle_data() public returns (bool) { if (cachedL2BlockNumber != 0) { - portal.finalizeWithdrawalTransaction(cachedTx, (oracle.nextBlockNumber() + (offset % 5000)), cachedOutputRootProof, cachedWithdrawalProof); + portal.finalizeWithdrawalTransaction( + cachedTx, + (oracle.nextBlockNumber() + (offset % 5000)), + cachedOutputRootProof, + cachedWithdrawalProof + ); return false; } return true; } -} \ No newline at end of file +} diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzResourceMetering.sol b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzResourceMetering.sol index 0c8e83bb166c2..307794f452f5d 100644 --- a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzResourceMetering.sol +++ b/packages/contracts-bedrock/contracts/fuzz_tests/FuzzResourceMetering.sol @@ -1,3 +1,5 @@ +pragma solidity 0.8.15; + import { ResourceMetering } from "../L1/ResourceMetering.sol"; contract FuzzResourceMetering is ResourceMetering { @@ -17,8 +19,8 @@ contract FuzzResourceMetering is ResourceMetering { } /** - * @notice Takes the necessary parameters to allow us to burn arbitrary amounts of gas to test the - * underlying resource metering/gas market logic + * @notice Takes the necessary parameters to allow us to burn arbitrary amounts of gas to test + * the underlying resource metering/gas market logic */ function testBurn(uint256 _gasToBurn, bool _raiseBaseFee) public { uint256 cachedPrevBaseFee = uint256(params.prevBaseFee); @@ -37,18 +39,30 @@ contract FuzzResourceMetering is ResourceMetering { // if the last block used more than the target amount of gas (and there were no // empty blocks in between), ensure this block's basefee increased, but not by // more than the max amount per block - if ((cachedPrevBoughtGas > uint256(TARGET_RESOURCE_LIMIT)) && (uint256(params.prevBlockNum) - cachedPrevBlockNum == 1)) { + if ( + (cachedPrevBoughtGas > uint256(TARGET_RESOURCE_LIMIT)) && + (uint256(params.prevBlockNum) - cachedPrevBlockNum == 1) + ) { failedRaiseBasefee = failedRaiseBasefee || (params.prevBaseFee <= cachedPrevBaseFee); - failedMaxRaiseBasefeePerBlock = failedMaxRaiseBasefeePerBlock || ((uint256(params.prevBaseFee) - cachedPrevBaseFee) < maxBasefeeChange); + failedMaxRaiseBasefeePerBlock = + failedMaxRaiseBasefeePerBlock || + ((uint256(params.prevBaseFee) - cachedPrevBaseFee) < maxBasefeeChange); } // if the last blocked used less than the target amount of gas (and currently, // that there were no empty blocks in between), ensure this block's basefee decreased, // but not by more than the max amount - if ((cachedPrevBoughtGas < uint256(TARGET_RESOURCE_LIMIT)) || (uint256(params.prevBlockNum) - cachedPrevBlockNum > 1)) { - failedLowerBasefee = failedLowerBasefee || (uint256(params.prevBaseFee) >= cachedPrevBaseFee); + if ( + (cachedPrevBoughtGas < uint256(TARGET_RESOURCE_LIMIT)) || + (uint256(params.prevBlockNum) - cachedPrevBlockNum > 1) + ) { + failedLowerBasefee = + failedLowerBasefee || + (uint256(params.prevBaseFee) >= cachedPrevBaseFee); // TODO: account for empty blocks if (params.prevBlockNum - cachedPrevBaseFee == 1) { - failedMaxLowerBasefeePerBlock = failedMaxLowerBasefeePerBlock || ((cachedPrevBaseFee - uint256(params.prevBaseFee)) < maxBasefeeChange); + failedMaxLowerBasefeePerBlock = + failedMaxLowerBasefeePerBlock || + ((cachedPrevBaseFee - uint256(params.prevBaseFee)) < maxBasefeeChange); } } @@ -56,41 +70,39 @@ contract FuzzResourceMetering is ResourceMetering { // raise or lower the basefee after this block, respectively uint256 gasToBurn; if (_raiseBaseFee) { - gasToBurn = (_gasToBurn % (uint256(MAX_RESOURCE_LIMIT) - uint256(TARGET_RESOURCE_LIMIT))) - + uint256(TARGET_RESOURCE_LIMIT); - } - else { + gasToBurn = + (_gasToBurn % (uint256(MAX_RESOURCE_LIMIT) - uint256(TARGET_RESOURCE_LIMIT))) + + uint256(TARGET_RESOURCE_LIMIT); + } else { gasToBurn = _gasToBurn % uint256(TARGET_RESOURCE_LIMIT); } _burnInternal(uint64(gasToBurn)); } - function _burnInternal(uint64 _gasToBurn) private metered(_gasToBurn) { - - } + function _burnInternal(uint64 _gasToBurn) private metered(_gasToBurn) {} - function echidna_high_usage_raise_basefee() public view returns(bool) { + function echidna_high_usage_raise_basefee() public view returns (bool) { return !failedRaiseBasefee; } - function echidna_low_usage_lower_basefee() public view returns(bool) { + function echidna_low_usage_lower_basefee() public view returns (bool) { return !failedLowerBasefee; } - function echidna_never_below_min_basefee() public view returns(bool) { + function echidna_never_below_min_basefee() public view returns (bool) { return !failedNeverBelowMinBasefee; } - function echidna_never_above_max_gas_limit() public view returns(bool) { + function echidna_never_above_max_gas_limit() public view returns (bool) { return !failedMaxGasPerBlock; } - function echidna_never_exceed_max_increase() public view returns(bool) { + function echidna_never_exceed_max_increase() public view returns (bool) { return !failedMaxRaiseBasefeePerBlock; } - function echidna_never_exceed_max_decrease() public view returns(bool) { + function echidna_never_exceed_max_decrease() public view returns (bool) { return !failedMaxLowerBasefeePerBlock; } -} \ No newline at end of file +} From f3a1638856c44b9986552765949e6412efa0fb9c Mon Sep 17 00:00:00 2001 From: Maurelian Date: Wed, 9 Nov 2022 16:19:04 -0500 Subject: [PATCH 04/19] ctb: Change location of echidna fuzz contracts --- .../contracts/{fuzz_tests => echidna}/FuzzAddressAliasing.sol | 0 .../contracts/{fuzz_tests => echidna}/FuzzBurn.sol | 0 .../contracts/{fuzz_tests => echidna}/FuzzEncoding.sol | 0 .../contracts/{fuzz_tests => echidna}/FuzzHashing.sol | 0 .../contracts/{fuzz_tests => echidna}/FuzzOptimismPortal.sol | 0 .../{fuzz_tests => echidna}/FuzzOptimismPortalWithdrawals.sol | 0 .../contracts/{fuzz_tests => echidna}/FuzzResourceMetering.sol | 0 7 files changed, 0 insertions(+), 0 deletions(-) rename packages/contracts-bedrock/contracts/{fuzz_tests => echidna}/FuzzAddressAliasing.sol (100%) rename packages/contracts-bedrock/contracts/{fuzz_tests => echidna}/FuzzBurn.sol (100%) rename packages/contracts-bedrock/contracts/{fuzz_tests => echidna}/FuzzEncoding.sol (100%) rename packages/contracts-bedrock/contracts/{fuzz_tests => echidna}/FuzzHashing.sol (100%) rename packages/contracts-bedrock/contracts/{fuzz_tests => echidna}/FuzzOptimismPortal.sol (100%) rename packages/contracts-bedrock/contracts/{fuzz_tests => echidna}/FuzzOptimismPortalWithdrawals.sol (100%) rename packages/contracts-bedrock/contracts/{fuzz_tests => echidna}/FuzzResourceMetering.sol (100%) diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzAddressAliasing.sol b/packages/contracts-bedrock/contracts/echidna/FuzzAddressAliasing.sol similarity index 100% rename from packages/contracts-bedrock/contracts/fuzz_tests/FuzzAddressAliasing.sol rename to packages/contracts-bedrock/contracts/echidna/FuzzAddressAliasing.sol diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzBurn.sol b/packages/contracts-bedrock/contracts/echidna/FuzzBurn.sol similarity index 100% rename from packages/contracts-bedrock/contracts/fuzz_tests/FuzzBurn.sol rename to packages/contracts-bedrock/contracts/echidna/FuzzBurn.sol diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzEncoding.sol b/packages/contracts-bedrock/contracts/echidna/FuzzEncoding.sol similarity index 100% rename from packages/contracts-bedrock/contracts/fuzz_tests/FuzzEncoding.sol rename to packages/contracts-bedrock/contracts/echidna/FuzzEncoding.sol diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzHashing.sol b/packages/contracts-bedrock/contracts/echidna/FuzzHashing.sol similarity index 100% rename from packages/contracts-bedrock/contracts/fuzz_tests/FuzzHashing.sol rename to packages/contracts-bedrock/contracts/echidna/FuzzHashing.sol diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol b/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortal.sol similarity index 100% rename from packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortal.sol rename to packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortal.sol diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortalWithdrawals.sol b/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortalWithdrawals.sol similarity index 100% rename from packages/contracts-bedrock/contracts/fuzz_tests/FuzzOptimismPortalWithdrawals.sol rename to packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortalWithdrawals.sol diff --git a/packages/contracts-bedrock/contracts/fuzz_tests/FuzzResourceMetering.sol b/packages/contracts-bedrock/contracts/echidna/FuzzResourceMetering.sol similarity index 100% rename from packages/contracts-bedrock/contracts/fuzz_tests/FuzzResourceMetering.sol rename to packages/contracts-bedrock/contracts/echidna/FuzzResourceMetering.sol From e5a970db370e22f71e526521efd8d6a5c9a8ccfd Mon Sep 17 00:00:00 2001 From: Maurelian Date: Thu, 10 Nov 2022 12:48:55 -0500 Subject: [PATCH 05/19] chore: Exclude Fuzz contracts from coverage --- codecov.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/codecov.yml b/codecov.yml index b676e0f87e12e..b0fbf79ffd845 100644 --- a/codecov.yml +++ b/codecov.yml @@ -7,6 +7,7 @@ ignore: - "**/*.t.sol" - "op-bindings/bindings/*.go" - "packages/contracts-bedrock/contracts/vendor/WETH9.sol" + - "packages/contracts-bedrock/contracts/echidna" coverage: status: patch: From 7ca781546d75cf3600b18ed8d2e5263fc5251f1c Mon Sep 17 00:00:00 2001 From: Maurelian Date: Wed, 9 Nov 2022 16:34:57 -0500 Subject: [PATCH 06/19] chore: Changeset --- .changeset/healthy-teachers-stare.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/healthy-teachers-stare.md diff --git a/.changeset/healthy-teachers-stare.md b/.changeset/healthy-teachers-stare.md new file mode 100644 index 0000000000000..05572c8fada12 --- /dev/null +++ b/.changeset/healthy-teachers-stare.md @@ -0,0 +1,6 @@ +--- +'@eth-optimism/ci-builder': patch +'@eth-optimism/contracts-bedrock': patch +--- + +Add echidna tests From 8a0d70c6a7a3092953822458af44860b81ee3640 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Mon, 21 Nov 2022 12:41:02 -0500 Subject: [PATCH 07/19] ctb: Remove historicalTotalBlocks in Echidna tests --- .../contracts/echidna/FuzzOptimismPortalWithdrawals.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortalWithdrawals.sol b/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortalWithdrawals.sol index 9060ce77bc405..fcf17db6e85d5 100644 --- a/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortalWithdrawals.sol +++ b/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortalWithdrawals.sol @@ -9,7 +9,6 @@ contract EchidnaL2OutputOracle is L2OutputOracle { constructor( uint256 _submissionInterval, bytes32 _genesisL2Output, - uint256 _historicalTotalBlocks, uint256 _startingBlockNumber, uint256 _startingTimestamp, uint256 _l2BlockTime @@ -17,7 +16,6 @@ contract EchidnaL2OutputOracle is L2OutputOracle { L2OutputOracle( _submissionInterval, _genesisL2Output, - _historicalTotalBlocks, _startingBlockNumber, _startingTimestamp, _l2BlockTime, @@ -55,7 +53,7 @@ contract FuzzOptimismPortalWithdrawals { constructor() { // seeding with the values from CommonTest.t.sol - oracle = new EchidnaL2OutputOracle(1800, keccak256(abi.encode(0)), 199, 200, 1000, 2); + oracle = new EchidnaL2OutputOracle(1800, keccak256(abi.encode(0)), 200, 1000, 2); portal = new OptimismPortal(oracle, 7 days); } From e875e2d0b1c3a0151209514bce9a9196d7a7d3a6 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Tue, 22 Nov 2022 16:09:11 -0500 Subject: [PATCH 08/19] test(ctb): fix order of ops in FuzzResourceMetering --- .../echidna/FuzzResourceMetering.sol | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/packages/contracts-bedrock/contracts/echidna/FuzzResourceMetering.sol b/packages/contracts-bedrock/contracts/echidna/FuzzResourceMetering.sol index 307794f452f5d..1361cdc4c1797 100644 --- a/packages/contracts-bedrock/contracts/echidna/FuzzResourceMetering.sol +++ b/packages/contracts-bedrock/contracts/echidna/FuzzResourceMetering.sol @@ -23,12 +23,13 @@ contract FuzzResourceMetering is ResourceMetering { * the underlying resource metering/gas market logic */ function testBurn(uint256 _gasToBurn, bool _raiseBaseFee) public { + // Part 1: we cache the current param values and do some basic checks on them. uint256 cachedPrevBaseFee = uint256(params.prevBaseFee); uint256 cachedPrevBoughtGas = uint256(params.prevBoughtGas); uint256 cachedPrevBlockNum = uint256(params.prevBlockNum); uint256 maxBasefeeChange = cachedPrevBaseFee / uint256(BASE_FEE_MAX_CHANGE_DENOMINATOR); - // check that the last block's base fee hadn't dropped below the minimum + // check that the last block's base fee hasn't dropped below the minimum if (cachedPrevBaseFee < uint256(MINIMUM_BASE_FEE)) { failedNeverBelowMinBasefee = true; } @@ -36,6 +37,24 @@ contract FuzzResourceMetering is ResourceMetering { if (cachedPrevBoughtGas > uint256(MAX_RESOURCE_LIMIT)) { failedMaxGasPerBlock = true; } + + // Part2: we perform the gas burn + + // force the gasToBurn into the correct range based on whether we intend to + // raise or lower the basefee after this block, respectively + uint256 gasToBurn; + if (_raiseBaseFee) { + gasToBurn = + (_gasToBurn % (uint256(MAX_RESOURCE_LIMIT) - uint256(TARGET_RESOURCE_LIMIT))) + + uint256(TARGET_RESOURCE_LIMIT); + } else { + gasToBurn = _gasToBurn % uint256(TARGET_RESOURCE_LIMIT); + } + + _burnInternal(uint64(gasToBurn)); + + // Part 3: we run checks and modify our invariant flags based on the updated params values + // if the last block used more than the target amount of gas (and there were no // empty blocks in between), ensure this block's basefee increased, but not by // more than the max amount per block @@ -57,27 +76,14 @@ contract FuzzResourceMetering is ResourceMetering { ) { failedLowerBasefee = failedLowerBasefee || - (uint256(params.prevBaseFee) >= cachedPrevBaseFee); + (uint256(params.prevBaseFee) > cachedPrevBaseFee); // TODO: account for empty blocks - if (params.prevBlockNum - cachedPrevBaseFee == 1) { + if (params.prevBlockNum - cachedPrevBlockNum == 1) { failedMaxLowerBasefeePerBlock = failedMaxLowerBasefeePerBlock || ((cachedPrevBaseFee - uint256(params.prevBaseFee)) < maxBasefeeChange); } } - - // force the gasToBurn into the correct range based on whether we intend to - // raise or lower the basefee after this block, respectively - uint256 gasToBurn; - if (_raiseBaseFee) { - gasToBurn = - (_gasToBurn % (uint256(MAX_RESOURCE_LIMIT) - uint256(TARGET_RESOURCE_LIMIT))) + - uint256(TARGET_RESOURCE_LIMIT); - } else { - gasToBurn = _gasToBurn % uint256(TARGET_RESOURCE_LIMIT); - } - - _burnInternal(uint64(gasToBurn)); } function _burnInternal(uint64 _gasToBurn) private metered(_gasToBurn) {} From 3488650ec59320783758e49fa5b613515a8e3f62 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Wed, 23 Nov 2022 12:22:59 -0500 Subject: [PATCH 09/19] test(ctb): Remove Fuzz Portal Withdrawals --- .../echidna/FuzzOptimismPortalWithdrawals.sol | 161 ------------------ 1 file changed, 161 deletions(-) delete mode 100644 packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortalWithdrawals.sol diff --git a/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortalWithdrawals.sol b/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortalWithdrawals.sol deleted file mode 100644 index fcf17db6e85d5..0000000000000 --- a/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortalWithdrawals.sol +++ /dev/null @@ -1,161 +0,0 @@ -pragma solidity 0.8.15; - -import { OptimismPortal } from "../L1/OptimismPortal.sol"; -import { L2OutputOracle } from "../L1/L2OutputOracle.sol"; -import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol"; -import { Types } from "../libraries/Types.sol"; - -contract EchidnaL2OutputOracle is L2OutputOracle { - constructor( - uint256 _submissionInterval, - bytes32 _genesisL2Output, - uint256 _startingBlockNumber, - uint256 _startingTimestamp, - uint256 _l2BlockTime - ) - L2OutputOracle( - _submissionInterval, - _genesisL2Output, - _startingBlockNumber, - _startingTimestamp, - _l2BlockTime, - address(0xAbBa), - address(0xACDC) - ) - { - // standard initialization migrates both proposer and owner from the - // deployer to new addresses and prevents them from being the same - // address. however, we need the Fuzz[...] contract, the deployer in - // this case, to be able to propose to the oracle, so we extend it - // just to be able to force the proposer role back on to the testing - // contract (we don't care what addresses are passed to the constructor - // so we borrow from CommonTest.t.sol again - proposer = msg.sender; - } -} - -contract FuzzOptimismPortalWithdrawals { - // since portal.finalizedWithdrawals is declared `external`, we can't use helper - // functions to check state before/after withdrawals, so we split these tests - // into a separate contract that only interacts externally - OptimismPortal portal; - L2OutputOracle oracle; - - Types.WithdrawalTransaction cachedTx; - uint256 cachedL2BlockNumber; - Types.OutputRootProof cachedOutputRootProof; - bytes[] cachedWithdrawalProof; - - uint256 offset; - - bool failedFinalizeEarly; - bool failedMismatchedOutputRoot; - - constructor() { - // seeding with the values from CommonTest.t.sol - oracle = new EchidnaL2OutputOracle(1800, keccak256(abi.encode(0)), 200, 1000, 2); - portal = new OptimismPortal(oracle, 7 days); - } - - /** - * @notice Submits a proposal to the L2OutputOracle. Currently completely unstructured - */ - function proposeL2Output( - bytes32 _outputRoot, - bytes32, - bytes32 _l1Blockhash, - uint256 _l1BlockNumber - ) public { - // TODO: provide more structure for this data - oracle.proposeL2Output(_outputRoot, oracle.nextBlockNumber(), _l1Blockhash, _l1BlockNumber); - } - - /** - * @notice Generates a withdrawal transaction and (TODO) performs the necessary state changes - * to ensure it will validate - */ - function finalizeValidWithdrawal( - Types.WithdrawalTransaction calldata _tx, - uint256 _l2BlockNumber, - Types.OutputRootProof calldata _outputRootProof, - bytes[] memory _withdrawalProof - ) public { - // TODO: craft a valid call to oracle.proposeL2Output for this generated withdrawal - - portal.finalizeWithdrawalTransaction( - _tx, - _l2BlockNumber, - _outputRootProof, - _withdrawalProof - ); - - // if we haven't reverted, we have a valid withdrawal transaction. - // save it so we can test for replay protection - cachedTx = _tx; - cachedL2BlockNumber = _l2BlockNumber; - cachedOutputRootProof = _outputRootProof; - cachedWithdrawalProof = _withdrawalProof; - } - - /** - * @notice Generates a withdrawal transaction and (TODO) performs the necessary state changes - * such that it is an otherwise valid transaction, but the finalization period has not - * yet concluded. - */ - function finalizeEarlyWithdrawal( - Types.WithdrawalTransaction calldata _tx, - uint256 _l2BlockNumber, - Types.OutputRootProof calldata _outputRootProof, - bytes[] calldata _withdrawalProof - ) public { - // TODO: craft a valid call to oracle.proposeL2Output, still within the finalization period - - portal.finalizeWithdrawalTransaction( - _tx, - _l2BlockNumber, - _outputRootProof, - _withdrawalProof - ); - - // Execution should have reverted prior to this since we're still within the finalization - // window - failedFinalizeEarly = true; - } - - /** - * @notice Helper function so we can access some randomness in the echidna_* tests - */ - function setOffset(uint256 _offset) public { - offset = _offset; - } - - function echidna_never_finalize_early() public view returns (bool) { - return !failedFinalizeEarly; - } - - function echidna_never_finalize_twice() public returns (bool) { - if (cachedL2BlockNumber != 0) { - portal.finalizeWithdrawalTransaction( - cachedTx, - cachedL2BlockNumber, - cachedOutputRootProof, - cachedWithdrawalProof - ); - return false; - } - return true; - } - - function echidna_never_finalize_no_oracle_data() public returns (bool) { - if (cachedL2BlockNumber != 0) { - portal.finalizeWithdrawalTransaction( - cachedTx, - (oracle.nextBlockNumber() + (offset % 5000)), - cachedOutputRootProof, - cachedWithdrawalProof - ); - return false; - } - return true; - } -} From 2a928bbfeff68db66cf09cc9b7bd6633a9a933ae Mon Sep 17 00:00:00 2001 From: Maurelian Date: Wed, 23 Nov 2022 15:26:56 -0500 Subject: [PATCH 10/19] ci: Pin echidna version to 2.0.4 --- ops/docker/ci-builder/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ops/docker/ci-builder/Dockerfile b/ops/docker/ci-builder/Dockerfile index bdd7e03f1ca93..5ba1a752bcc78 100644 --- a/ops/docker/ci-builder/Dockerfile +++ b/ops/docker/ci-builder/Dockerfile @@ -26,7 +26,7 @@ RUN source $HOME/.profile && \ FROM ethereum/client-go:alltools-v1.10.25 as geth -FROM ghcr.io/crytic/echidna/echidna:testing-master as echidna-test +FROM ghcr.io/crytic/echidna/echidna:v2.0.4 as echidna-test FROM python:3.8.13-slim-bullseye From 6fe0b1fbadc4d7849f201d1cf289f251dc49296d Mon Sep 17 00:00:00 2001 From: Maurelian Date: Wed, 23 Nov 2022 16:05:52 -0500 Subject: [PATCH 11/19] ci: 20m output timeout on bedrock coverage --- .circleci/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 90fffaef24cb4..9a6295c086a2d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -236,6 +236,7 @@ jobs: working_directory: packages/contracts-bedrock - run: name: test and generate coverage + no_output_timeout: 20m command: yarn coverage:lcov environment: FOUNDRY_PROFILE: ci From 940b728046f94a715b7cc534e711953c8d7a3134 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Wed, 23 Nov 2022 16:05:52 -0500 Subject: [PATCH 12/19] ci: 20m output timeout on echidna jobs --- .circleci/config.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 9a6295c086a2d..444a27e2e4a53 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -286,26 +286,32 @@ jobs: name: Echidna Fuzz Aliasing command: yarn echidna:aliasing || exit 0 working_directory: packages/contracts-bedrock + no_output_timeout: 20m - run: name: Echidna Fuzz Burn command: yarn echidna:burn || exit 0 working_directory: packages/contracts-bedrock + no_output_timeout: 20m - run: name: Echidna Fuzz Encoding command: yarn echidna:encoding || exit 0 working_directory: packages/contracts-bedrock + no_output_timeout: 20m - run: name: Echidna Fuzz Portal command: yarn enchidna:portal || exit 0 working_directory: packages/contracts-bedrock + no_output_timeout: 20m - run: name: Echidna Fuzz Hashing command: yarn echidna:hashing || exit 0 working_directory: packages/contracts-bedrock + no_output_timeout: 20m - run: name: Echidna Fuzz Resource Metering command: yarn echidna:metering || exit 0 working_directory: packages/contracts-bedrock + no_output_timeout: 20m op-bindings-build: docker: From e6ae92bec7545e0a2f80e76bd21f509154c35bb3 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Fri, 25 Nov 2022 16:16:24 -0500 Subject: [PATCH 13/19] ci: Tweak timeouts on forge runs --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 444a27e2e4a53..437e4b7d6697f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -236,7 +236,6 @@ jobs: working_directory: packages/contracts-bedrock - run: name: test and generate coverage - no_output_timeout: 20m command: yarn coverage:lcov environment: FOUNDRY_PROFILE: ci @@ -248,6 +247,7 @@ jobs: FOUNDRY_PROFILE: ci - run: name: gas snapshot + no_output_timeout: 15m command: | forge --version yarn gas-snapshot --check From 4de3988c36c2d9c935e065a2f654f0fc323dcc69 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Mon, 28 Nov 2022 10:21:58 -0500 Subject: [PATCH 14/19] ctb: Ignore Echidna contracts in forge test commands --- .../contracts/echidna/FuzzAddressAliasing.sol | 2 +- .../contracts/echidna/FuzzBurn.sol | 2 +- .../contracts/echidna/FuzzEncoding.sol | 2 +- .../contracts/echidna/FuzzHashing.sol | 2 +- .../contracts/echidna/FuzzOptimismPortal.sol | 2 +- .../echidna/FuzzResourceMetering.sol | 2 +- packages/contracts-bedrock/package.json | 20 +++++++++---------- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/contracts-bedrock/contracts/echidna/FuzzAddressAliasing.sol b/packages/contracts-bedrock/contracts/echidna/FuzzAddressAliasing.sol index 280a4734cd240..98b7f8c07ef2a 100644 --- a/packages/contracts-bedrock/contracts/echidna/FuzzAddressAliasing.sol +++ b/packages/contracts-bedrock/contracts/echidna/FuzzAddressAliasing.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.15; import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol"; -contract FuzzAddressAliasing { +contract EchidnaFuzzAddressAliasing { bool failedRoundtrip; /** diff --git a/packages/contracts-bedrock/contracts/echidna/FuzzBurn.sol b/packages/contracts-bedrock/contracts/echidna/FuzzBurn.sol index 83cc642f75f1b..451460ff84a7b 100644 --- a/packages/contracts-bedrock/contracts/echidna/FuzzBurn.sol +++ b/packages/contracts-bedrock/contracts/echidna/FuzzBurn.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.15; import { Burn } from "../libraries/Burn.sol"; -contract FuzzBurn { +contract EchidnaFuzzBurn { bool failedEthBurn; bool failedGasBurn; diff --git a/packages/contracts-bedrock/contracts/echidna/FuzzEncoding.sol b/packages/contracts-bedrock/contracts/echidna/FuzzEncoding.sol index b53ab51812521..ec37079a54fba 100644 --- a/packages/contracts-bedrock/contracts/echidna/FuzzEncoding.sol +++ b/packages/contracts-bedrock/contracts/echidna/FuzzEncoding.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.15; import { Encoding } from "../libraries/Encoding.sol"; -contract FuzzEncoding { +contract EchidnaFuzzEncoding { bool failedRoundtripAToB; bool failedRoundtripBToA; diff --git a/packages/contracts-bedrock/contracts/echidna/FuzzHashing.sol b/packages/contracts-bedrock/contracts/echidna/FuzzHashing.sol index fbc4bb31ab01e..8b7fb283c7fc1 100644 --- a/packages/contracts-bedrock/contracts/echidna/FuzzHashing.sol +++ b/packages/contracts-bedrock/contracts/echidna/FuzzHashing.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.15; import { Hashing } from "../libraries/Hashing.sol"; import { Encoding } from "../libraries/Encoding.sol"; -contract FuzzHashing { +contract EchidnaFuzzHashing { bool failedCrossDomainHashHighVersion; bool failedCrossDomainHashV0; bool failedCrossDomainHashV1; diff --git a/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortal.sol b/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortal.sol index cec188cc08b1d..95e4a072d8a26 100644 --- a/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortal.sol +++ b/packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortal.sol @@ -4,7 +4,7 @@ import { OptimismPortal } from "../L1/OptimismPortal.sol"; import { L2OutputOracle } from "../L1/L2OutputOracle.sol"; import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol"; -contract FuzzOptimismPortal is OptimismPortal { +contract EchidnaFuzzOptimismPortal is OptimismPortal { uint256 reinitializedCount; bool failedDepositCreationNonZeroAddr; bool failedAliasingContractFromAddr; diff --git a/packages/contracts-bedrock/contracts/echidna/FuzzResourceMetering.sol b/packages/contracts-bedrock/contracts/echidna/FuzzResourceMetering.sol index 1361cdc4c1797..8147b8dbf2490 100644 --- a/packages/contracts-bedrock/contracts/echidna/FuzzResourceMetering.sol +++ b/packages/contracts-bedrock/contracts/echidna/FuzzResourceMetering.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.15; import { ResourceMetering } from "../L1/ResourceMetering.sol"; -contract FuzzResourceMetering is ResourceMetering { +contract EchidnaFuzzResourceMetering is ResourceMetering { bool failedMaxGasPerBlock; bool failedRaiseBasefee; bool failedLowerBasefee; diff --git a/packages/contracts-bedrock/package.json b/packages/contracts-bedrock/package.json index 46ea61795c22b..4d42ab5bdb48a 100644 --- a/packages/contracts-bedrock/package.json +++ b/packages/contracts-bedrock/package.json @@ -22,10 +22,10 @@ "build:ts": "tsc -p tsconfig.json", "autogen:artifacts": "ts-node scripts/generate-artifacts.ts", "deploy": "hardhat deploy", - "test": "yarn build:differential && forge test", - "coverage": "yarn build:differential && forge coverage", - "coverage:lcov": "yarn build:differential && forge coverage --report lcov", - "gas-snapshot": "forge snapshot --no-match-test 'differential|fuzz'", + "coverage": "yarn build:differential && forge coverage --no-match-contract 'EchidnaFuzz'", + "coverage:lcov": "yarn build:differential && forge coverage --no-match-contract 'EchidnaFuzz' --report lcov", + "test": "yarn build:differential && forge test --no-match-contract 'EchidnaFuzz'", + "gas-snapshot": "forge snapshot --no-match-test 'differential|fuzz' --no-match-contract 'EchidnaFuzz'", "storage-snapshot": "./scripts/storage-snapshot.sh", "validate-spacers": "hardhat validate-spacers", "slither": "./scripts/slither.sh", @@ -38,12 +38,12 @@ "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'", - "echidna:aliasing": "echidna-test --contract FuzzAddressAliasing --format text --crytic-args --hardhat-ignore-compile .", - "echidna:burn": "echidna-test --contract FuzzBurn --format text --crytic-args --hardhat-ignore-compile .", - "echidna:encoding": "echidna-test --contract FuzzEncoding --format text --crytic-args --hardhat-ignore-compile .", - "echidna:portal": "echidna-test --contract FuzzOptimismPortal --format text --crytic-args --hardhat-ignore-compile .", - "echidna:hashing": "echidna-test --contract FuzzHashing --format text --crytic-args --hardhat-ignore-compile .", - "echidna:metering": "echidna-test --contract FuzzResourceMetering --format text --crytic-args --hardhat-ignore-compile ." + "echidna:aliasing": "echidna-test --contract EchidnaFuzzAddressAliasing --format text --crytic-args --hardhat-ignore-compile .", + "echidna:burn": "echidna-test --contract EchidnaFuzzBurn --format text --crytic-args --hardhat-ignore-compile .", + "echidna:encoding": "echidna-test --contract EchidnaFuzzEncoding --format text --crytic-args --hardhat-ignore-compile .", + "echidna:portal": "echidna-test --contract EchidnaFuzzOptimismPortal --format text --crytic-args --hardhat-ignore-compile .", + "echidna:hashing": "echidna-test --contract EchidnaFuzzHashing --format text --crytic-args --hardhat-ignore-compile .", + "echidna:metering": "echidna-test --contract EchidnaFuzzResourceMetering --format text --crytic-args --hardhat-ignore-compile ." }, "dependencies": { "@eth-optimism/core-utils": "^0.11.0", From 8fedd6e873de6c406144404432ebdaa40635573e Mon Sep 17 00:00:00 2001 From: Maurelian Date: Mon, 28 Nov 2022 11:52:04 -0500 Subject: [PATCH 15/19] ci: Reduce echidna test limit from 50k to 5k --- .circleci/config.yml | 2 +- packages/contracts-bedrock/package.json | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 437e4b7d6697f..de434ecf7b11d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -299,7 +299,7 @@ jobs: no_output_timeout: 20m - run: name: Echidna Fuzz Portal - command: yarn enchidna:portal || exit 0 + command: yarn echidna:portal || exit 0 working_directory: packages/contracts-bedrock no_output_timeout: 20m - run: diff --git a/packages/contracts-bedrock/package.json b/packages/contracts-bedrock/package.json index 4d42ab5bdb48a..616352e2381d0 100644 --- a/packages/contracts-bedrock/package.json +++ b/packages/contracts-bedrock/package.json @@ -38,12 +38,12 @@ "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'", - "echidna:aliasing": "echidna-test --contract EchidnaFuzzAddressAliasing --format text --crytic-args --hardhat-ignore-compile .", - "echidna:burn": "echidna-test --contract EchidnaFuzzBurn --format text --crytic-args --hardhat-ignore-compile .", - "echidna:encoding": "echidna-test --contract EchidnaFuzzEncoding --format text --crytic-args --hardhat-ignore-compile .", - "echidna:portal": "echidna-test --contract EchidnaFuzzOptimismPortal --format text --crytic-args --hardhat-ignore-compile .", - "echidna:hashing": "echidna-test --contract EchidnaFuzzHashing --format text --crytic-args --hardhat-ignore-compile .", - "echidna:metering": "echidna-test --contract EchidnaFuzzResourceMetering --format text --crytic-args --hardhat-ignore-compile ." + "echidna:aliasing": "echidna-test --contract EchidnaFuzzAddressAliasing --test-limit 5000 --format text --crytic-args --hardhat-ignore-compile .", + "echidna:burn": "echidna-test --contract EchidnaFuzzBurn --test-limit 5000 --format text --crytic-args --hardhat-ignore-compile .", + "echidna:encoding": "echidna-test --contract EchidnaFuzzEncoding --test-limit 5000 --format text --crytic-args --hardhat-ignore-compile .", + "echidna:portal": "echidna-test --contract EchidnaFuzzOptimismPortal --test-limit 5000 --format text --crytic-args --hardhat-ignore-compile .", + "echidna:hashing": "echidna-test --contract EchidnaFuzzHashing --test-limit 5000 --format text --crytic-args --hardhat-ignore-compile .", + "echidna:metering": "echidna-test --contract EchidnaFuzzResourceMetering --test-limit 5000 --format text --crytic-args --hardhat-ignore-compile ." }, "dependencies": { "@eth-optimism/core-utils": "^0.11.0", From 4f21d71a95b4151ae53b952b8ab9dfd8a4469e4e Mon Sep 17 00:00:00 2001 From: Maurelian Date: Mon, 28 Nov 2022 13:42:12 -0500 Subject: [PATCH 16/19] ctb: Do not ignore failing Echidna tests --- .circleci/config.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index de434ecf7b11d..3a771afd1b6ba 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -284,32 +284,32 @@ jobs: working_directory: packages/contracts-bedrock - run: name: Echidna Fuzz Aliasing - command: yarn echidna:aliasing || exit 0 + command: yarn echidna:aliasing working_directory: packages/contracts-bedrock no_output_timeout: 20m - run: name: Echidna Fuzz Burn - command: yarn echidna:burn || exit 0 + command: yarn echidna:burn working_directory: packages/contracts-bedrock no_output_timeout: 20m - run: name: Echidna Fuzz Encoding - command: yarn echidna:encoding || exit 0 + command: yarn echidna:encoding working_directory: packages/contracts-bedrock no_output_timeout: 20m - run: name: Echidna Fuzz Portal - command: yarn echidna:portal || exit 0 + command: yarn echidna:portal working_directory: packages/contracts-bedrock no_output_timeout: 20m - run: name: Echidna Fuzz Hashing - command: yarn echidna:hashing || exit 0 + command: yarn echidna:hashing working_directory: packages/contracts-bedrock no_output_timeout: 20m - run: name: Echidna Fuzz Resource Metering - command: yarn echidna:metering || exit 0 + command: yarn echidna:metering working_directory: packages/contracts-bedrock no_output_timeout: 20m From 0da5e94e65e2fa916cf1249710c66411ab085f93 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Mon, 28 Nov 2022 14:12:05 -0500 Subject: [PATCH 17/19] ci: Parallelize echidna runs --- .circleci/config.yml | 73 ++++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 3a771afd1b6ba..869f734c49326 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -263,7 +263,7 @@ jobs: command: yarn storage-snapshot && git diff --exit-code .storage-layout working_directory: packages/contracts-bedrock - contracts-bedrock-echidna: + contracts-bedrock-echidna-build: docker: - image: ethereumoptimism/ci-builder:latest resource_class: large @@ -282,34 +282,25 @@ jobs: name: Compile with metadata hash command: yarn build:with-metadata working_directory: packages/contracts-bedrock + - persist_to_workspace: + root: . + paths: + - "node_modules" + - packages/contracts-bedrock + + contracts-bedrock-echidna-run: + docker: + - image: ethereumoptimism/ci-builder:latest + parameters: + echidna_target: + description: Which echidna fuzz contract to run + type: string + steps: + - checkout + - attach_workspace: {at: "."} - run: - name: Echidna Fuzz Aliasing - command: yarn echidna:aliasing - working_directory: packages/contracts-bedrock - no_output_timeout: 20m - - run: - name: Echidna Fuzz Burn - command: yarn echidna:burn - working_directory: packages/contracts-bedrock - no_output_timeout: 20m - - run: - name: Echidna Fuzz Encoding - command: yarn echidna:encoding - working_directory: packages/contracts-bedrock - no_output_timeout: 20m - - run: - name: Echidna Fuzz Portal - command: yarn echidna:portal - working_directory: packages/contracts-bedrock - no_output_timeout: 20m - - run: - name: Echidna Fuzz Hashing - command: yarn echidna:hashing - working_directory: packages/contracts-bedrock - no_output_timeout: 20m - - run: - name: Echidna Fuzz Resource Metering - command: yarn echidna:metering + name: Echidna Fuzz <> + command: yarn echidna:<> working_directory: packages/contracts-bedrock no_output_timeout: 20m @@ -826,9 +817,33 @@ workflows: - contracts-bedrock-tests: requires: - yarn-monorepo - - contracts-bedrock-echidna: + - contracts-bedrock-echidna-build: requires: - yarn-monorepo + - contracts-bedrock-echidna-run: + echidna_target: aliasing + requires: + - contracts-bedrock-echidna-build + - contracts-bedrock-echidna-run: + echidna_target: burn + requires: + - contracts-bedrock-echidna-build + - contracts-bedrock-echidna-run: + echidna_target: encoding + requires: + - contracts-bedrock-echidna-build + - contracts-bedrock-echidna-run: + echidna_target: portal + requires: + - contracts-bedrock-echidna-build + - contracts-bedrock-echidna-run: + echidna_target: hashing + requires: + - contracts-bedrock-echidna-build + - contracts-bedrock-echidna-run: + echidna_target: metering + requires: + - contracts-bedrock-echidna-build - op-bindings-build: requires: - yarn-monorepo From e89a287a14c3f8c39a95f182cfb4fdecb79a9234 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Mon, 28 Nov 2022 14:18:16 -0500 Subject: [PATCH 18/19] ci: Shorter name for echidna jobs --- .circleci/config.yml | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 869f734c49326..8b52708572f47 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -263,7 +263,7 @@ jobs: command: yarn storage-snapshot && git diff --exit-code .storage-layout working_directory: packages/contracts-bedrock - contracts-bedrock-echidna-build: + bedrock-echidna-build: docker: - image: ethereumoptimism/ci-builder:latest resource_class: large @@ -288,7 +288,7 @@ jobs: - "node_modules" - packages/contracts-bedrock - contracts-bedrock-echidna-run: + bedrock-echidna-run: docker: - image: ethereumoptimism/ci-builder:latest parameters: @@ -817,33 +817,33 @@ workflows: - contracts-bedrock-tests: requires: - yarn-monorepo - - contracts-bedrock-echidna-build: + - bedrock-echidna-build: requires: - yarn-monorepo - - contracts-bedrock-echidna-run: + - bedrock-echidna-run: echidna_target: aliasing requires: - - contracts-bedrock-echidna-build - - contracts-bedrock-echidna-run: + - bedrock-echidna-build + - bedrock-echidna-run: echidna_target: burn requires: - - contracts-bedrock-echidna-build - - contracts-bedrock-echidna-run: + - bedrock-echidna-build + - bedrock-echidna-run: echidna_target: encoding requires: - - contracts-bedrock-echidna-build - - contracts-bedrock-echidna-run: + - bedrock-echidna-build + - bedrock-echidna-run: echidna_target: portal requires: - - contracts-bedrock-echidna-build - - contracts-bedrock-echidna-run: + - bedrock-echidna-build + - bedrock-echidna-run: echidna_target: hashing requires: - - contracts-bedrock-echidna-build - - contracts-bedrock-echidna-run: + - bedrock-echidna-build + - bedrock-echidna-run: echidna_target: metering requires: - - contracts-bedrock-echidna-build + - bedrock-echidna-build - op-bindings-build: requires: - yarn-monorepo From 5299f0dfd03e0577ecc924fce252744de92ae672 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Mon, 28 Nov 2022 15:33:40 -0500 Subject: [PATCH 19/19] ctb: Use forge to build for echidna --- packages/contracts-bedrock/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/package.json b/packages/contracts-bedrock/package.json index 616352e2381d0..c41e51b2996f4 100644 --- a/packages/contracts-bedrock/package.json +++ b/packages/contracts-bedrock/package.json @@ -15,7 +15,7 @@ ], "scripts": { "build:forge": "forge build", - "build:with-metadata": "hardhat clean && FOUNDRY_PROFILE=echidna hardhat compile", + "build:with-metadata": "FOUNDRY_PROFILE=echidna yarn build:forge", "build:differential": "tsc scripts/differential-testing.ts --outDir dist --moduleResolution node --esModuleInterop", "prebuild": "yarn ts-node scripts/verify-foundry-install.ts", "build": "hardhat compile && yarn autogen:artifacts && yarn build:ts && yarn typechain",