crytic / medusa

Parallelized, coverage-guided, mutational Solidity smart contract fuzzing, powered by go-ethereum
https://www.trailofbits.com/
GNU Affero General Public License v3.0
290 stars 34 forks source link

Allow Echidna & Medusa to share the same corpus #234

Open aviggiano opened 12 months ago

aviggiano commented 12 months ago

Describe the desired feature

This is a feature request to standardize the corpus between https://github.com/crytic/echidna and https://github.com/crytic/medusa, so that it is easier to replay transactions from one tool on the other.

Ref: https://github.com/crytic/echidna/issues/1123

karmacoma-eth commented 9 months ago

I think there is an opportunity to standardize on a simple common format. It would open up some interesting possibilities:

I'd like to suggest a format inspired by https://github.com/ethereum/tests: https://karmacoma.notion.site/Common-Corpus-Format-1d273a31ae4b43a4bc9f4df1e96fa900?pvs=4

I think I'd be pretty happy to work with this format but I'd love to hear what you think.

aviggiano commented 9 months ago

I believe the whole industry would benefit from this standardization. CC @gakonst @mds1

grandizzy commented 4 months ago

@karmacoma-eth wanted to make sure I properly understand your proposed format - for a scenario like:

target contract ```Solidity contract TargetContract { function doSomething(uint256 x) external {} } ```
test handler contract: modifies env, calls target, then modifies again env ```Solidity contract TestHandler is Test { TargetContract target = new TargetContract(); function work(uint256 x) external { vm.roll(block.number + 1); target.doSomething(x); vm.warp(block.number + 1); } } ```

where the test execution would look something like

  [30977] TestHandler::work(3613300754934649 [3.613e15])
    ├─ [0] VM::roll(19812633)
    │   └─ ← [Return] 
    ├─ [189] TargetContract::doSomething(3613300754934649)
    │   └─ ← [Stop] 
    ├─ [0] VM::warp(19812634 [1.981e7])
    │   └─ ← [Return] 
    └─ ← [Stop] 

Should the call sequence include only the children txes as in

sequence ```json [ { "to":"0x7109709ECfa91a80626fF3989D68f67F5b1DD12D", "data":"0x1f7b4f3000000000000000000000000000000000000000000000000000000000012e5119", "_info":"vm.roll(19812633)" }, { "sender":"0x5615deb798bb3e4dfa0139dfa1b3d433cc23b72f", "to":"0x104fbc016f4bb334d775a19e8a6510109ac63e00", "data":"0xa6b206bf000000000000000000000000000000000000000000000000000cd64723f6eb79", "_info":"TargetContract::doSomething(3613300754934649)" }, { "to":"0x7109709ECfa91a80626fF3989D68f67F5b1DD12D", "data":"0xe5d6bf0200000000000000000000000000000000000000000000000000000000012e511a", "_info":"vm.warp(19812634)" } ] ```

or should also include the parent as in

sequence ```json { "sender":"0xeaa1ffc3f1012ad923577141ed582ab41d006934", "to":"0x5615deb798bb3e4dfa0139dfa1b3d433cc23b72f", "data":"0x5858d161000000000000000000000000000000000000000000000000000cd64723f6eb79", "_info":"TestHandler::work(3613300754934649)", "calls":[ { "to":"0x7109709ECfa91a80626fF3989D68f67F5b1DD12D", "data":"0x1f7b4f3000000000000000000000000000000000000000000000000000000000012e5119", "_info":"vm.roll(19812633)" }, { "sender":"0x5615deb798bb3e4dfa0139dfa1b3d433cc23b72f", "to":"0x104fbc016f4bb334d775a19e8a6510109ac63e00", "data":"0xa6b206bf000000000000000000000000000000000000000000000000000cd64723f6eb79", "_info":"TargetContract::doSomething(3613300754934649)" }, { "to":"0x7109709ECfa91a80626fF3989D68f67F5b1DD12D", "data":"0xe5d6bf0200000000000000000000000000000000000000000000000000000000012e511a", "_info":"vm.warp(19812634)" } ] } ```

(I assume 1st option?) thanks!

karmacoma-eth commented 4 months ago

@grandizzy there is a 3rd option (include only the top level call):

calls = [
  {
     "sender":"0xeaa1ffc3f1012ad923577141ed582ab41d006934",
     "to":"0x5615deb798bb3e4dfa0139dfa1b3d433cc23b72f",
     "data":"0x5858d161000000000000000000000000000000000000000000000000000cd64723f6eb79",
     "_info":"TestHandler::work(3613300754934649)"
  }
]

I think this option is more correct, the idea is that we should only represent calls made by the driver to the test code. In this case, the driver has invoked TestHandler::work(3613300754934649), and the subcalls happened internally in TestHandler. If we do include the subcalls in the trace, it should only be informational.

In other words, the only thing necessary to replay the same result should be for the test driver to run the same setUp and then invoke TestHandler::work(3613300754934649) again

grandizzy commented 4 months ago

thank you, makes sense, will update support to use this format

0xalpharush commented 4 months ago

We are still working on a draft of a standard that includes necessary info e.g. the ABI of TestHandler::work and will share with Foundry and Halmos soon for feedback.