search

dapphub / dapptools

Dapp, Seth, Hevm, and more
https://dapp.tools
2.1k stars 325 forks source link

Test function that takes a struct argument breaks fuzzer/symbolic executor #820

Open transmissions11 opened 3 years ago

transmissions11 commented 3 years ago

I have a small test contract like this:

pragma solidity 0.7.6;
pragma abicoder v2;

import {DSTestPlus} from "solmate/src/tests/utils/DSTestPlus.sol";

import {MockCrossDomainMessenger} from "nova/contracts/mocks/MockCrossDomainMessenger.sol";

import {L1_NovaExecutionManager} from "nova/contracts/L1_NovaExecutionManager.sol";

contract ExecutionManagerTest is DSTestPlus {
    L1_NovaExecutionManager executionManager;
    MockCrossDomainMessenger mockCrossDomainMessenger;

    function setUp() public {
        mockCrossDomainMessenger = new MockCrossDomainMessenger();
        executionManager = new L1_NovaExecutionManager(DEAD_ADDRESS, mockCrossDomainMessenger);
    }

    function proveUpdatingGasConfig(L1_NovaExecutionManager.GasConfig calldata newGasConfig) public {}
}

https://github.com/Rari-Capital/nova-invariants/blob/proveUpdatingGasConfig-demo/src/ExecutionManager.t.sol

L1_NovaExecutionManager.GasConfig is a struct like so:

struct GasConfig {
        uint32 calldataByteGasEstimate;
        uint96 missingGasEstimate;
        uint96 strategyCallGasBuffer;
        uint32 execCompletedMessageGasLimit;
}

https://github.com/Rari-Capital/nova/blob/3252ceea5e50d3c29be69861b77908a5b4f28815/contracts/L1_NovaExecutionManager.sol#L90-L103

Running dapp test yields:

❯ dapp test -m proveUpdatingGasConfig
+ dapp clean
+ rm -rf out
Running 1 tests for src/ExecutionManager.t.sol:ExecutionManagerTest
[FAIL] proveUpdatingGasConfig(tuple)

Failure: proveUpdatingGasConfig(tuple)

  Counterexample:

    result:   Revert
    calldata: proveUpdatingGasConfig((0, 0, 0, 0))

    src/ExecutionManager.t.sol:ExecutionManagerTest
     ├╴constructor
     ├╴setUp()
     │  ├╴create MockCrossDomainMessenger@0xCe71065D4017F316EC606Fe4422e11eB2c47c246 (src/ExecutionManager.t.sol:16)
     │  │  └╴← 1244 bytes of code
     │  └╴create L1_NovaExecutionManager@0x185a4dc360CE69bDCceE33b3784B0282f7961aea (src/ExecutionManager.t.sol:17)
     │     ├╴OwnerUpdated() (lib/nova/contracts/L1_NovaExecutionManager.sol:60)
     │     ├╴create L1_NovaApprovalEscrow@0x9cC6334F1A7Bc20c9Dde91Db536E194865Af0067 (lib/nova/contracts/L1_NovaExecutionManager.sol:16)
     │     │  └╴← 878 bytes of code
     │     └╴← 8066 bytes of code
     └╴proveUpdatingGasConfig(tuple)

Even when switching the test from prove to test it still unexpectedly reverts:

❯ dapp test -m testUpdatingGasConfig
        + dapp clean
+ rm -rf out
Running 1 tests for src/ExecutionManager.t.sol:ExecutionManagerTest
*** Failed! Falsified (after 1 test):  
0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004b7ba4a4
[FAIL] testUpdatingGasConfig(tuple). Counterexample: ((0, 1, 0, 1266394276))
Run:
 dapp test --replay '("testUpdatingGasConfig(tuple)","0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004b7ba4a4")'
to test this case again, or 
 dapp debug --replay '("testUpdatingGasConfig(tuple)","0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004b7ba4a4")'
to debug it.

Failure: 
  src/ExecutionManager.t.sol:ExecutionManagerTest
   ├╴constructor
   ├╴setUp()
   │  ├╴create MockCrossDomainMessenger@0xCe71065D4017F316EC606Fe4422e11eB2c47c246 (src/ExecutionManager.t.sol:16)
   │  │  └╴← 1244 bytes of code
   │  └╴create L1_NovaExecutionManager@0x185a4dc360CE69bDCceE33b3784B0282f7961aea (src/ExecutionManager.t.sol:17)
   │     ├╴OwnerUpdated() (lib/nova/contracts/L1_NovaExecutionManager.sol:60)
   │     ├╴create L1_NovaApprovalEscrow@0x9cC6334F1A7Bc20c9Dde91Db536E194865Af0067 (lib/nova/contracts/L1_NovaExecutionManager.sol:16)
   │     │  └╴← 878 bytes of code
   │     └╴← 8066 bytes of code
   └╴testUpdatingGasConfig(tuple)
      └╴error Revert 0x (src/ExecutionManager.t.sol:10)

Does hevm not support generating args for functions that take structs?

MrChico commented 3 years ago

hmm, no it should work, but I guess something might be wrong with the encoder

cruzdanilo commented 3 years ago

i'm having the same issue. any workaround?

cruzdanilo commented 3 years ago

the issue seems to be about method signature lookup. look at this trace of a successful call to a function that takes a struct as an argument (BreedingColor::[unknown method] on setUp):

$ dapp test -m testColorRequest
Running 1 tests for test/dapptools/BreedingColor.t.sol:BreedingColorTest
[BAIL] testColorRequest()

Failure: testColorRequest
  test/dapptools/BreedingColor.t.sol:BreedingColorTest
   ├╴constructor
   ├╴setUp()
   │  ├╴create MockLink@0xCe71065D4017F316EC606Fe4422e11eB2c47c246 (test/dapptools/BreedingColor.t.sol:17)
   │  │  └╴← 2977 bytes of code
   │  ├╴create BreedingColor@0x185a4dc360CE69bDCceE33b3784B0282f7961aea (test/dapptools/BreedingColor.t.sol:18)
   │  │  ├╴RoleGranted() (node_modules/@openzeppelin/contracts/access/AccessControl.sol:200)
   │  │  ├╴RoleGranted() (node_modules/@openzeppelin/contracts/access/AccessControl.sol:200)
   │  │  ├╴RoleGranted() (node_modules/@openzeppelin/contracts/access/AccessControl.sol:200)
   │  │  └╴← 16256 bytes of code
   │  ├╴call BreedingColor::[unknown method](0x592f4d8700000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000) (test/dapptools/BreedingColor.t.sol:24)
   │  │  └╴← 0x
   │  └╴call MockLink::mint(address,uint256)(BreedingColor@0x185a4dc360CE69bDCceE33b3784B0282f7961aea, 666) (test/dapptools/BreedingColor.t.sol:25)
   │     ├╴Transfer(666) (node_modules/@openzeppelin/contracts/token/ERC20/ERC20.sol:258)
   │     └╴← 0x
   └╴testColorRequest()
      ├╴call BreedingColor::requestBreeding(uint256,uint256,uint256)(0, 0, 0) (test/dapptools/BreedingColor.t.sol:41)
      │  └╴error Revert 0x (contracts/normal_deployment/BreedingColor.sol:146)
      └╴error Revert 0x (test/dapptools/BreedingColor.t.sol:41)