medusa icon indicating copy to clipboard operation
medusa copied to clipboard

Allow Echidna & Medusa to share the same corpus

Open aviggiano opened this issue 2 years ago • 6 comments

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

aviggiano avatar Sep 19 '23 17:09 aviggiano

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

  • start in one tool, continue in the other as @aviggiano suggested
  • write new tools that generate a starting corpus or extend an existing corpus (like Optik)
  • write new tools that can process / analyze / compare corpuses across runs of a fuzzer and across fuzzers

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.

0xkarmacoma avatar Dec 20 '23 23:12 0xkarmacoma

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

aviggiano avatar Dec 22 '23 08:12 aviggiano

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

target contract
contract TargetContract {
  function doSomething(uint256 x) external {}
}
test handler contract: modifies env, calls target, then modifies again env
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
[
   {
      "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
{
   "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!

grandizzy avatar May 08 '24 07:05 grandizzy

@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

0xkarmacoma avatar May 13 '24 20:05 0xkarmacoma

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

grandizzy avatar May 14 '24 08:05 grandizzy

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.

0xalpharush avatar May 14 '24 12:05 0xalpharush