forge-std icon indicating copy to clipboard operation
forge-std copied to clipboard

refactor: add an abstract `Script` contract

Open mds1 opened this issue 3 years ago • 1 comments

With solidity scripting and deploys on the way (https://github.com/foundry-rs/foundry/pull/1208), a a series of new cheatcodes for sending transactions will be added to the Vm interface. To access these, you'd currently need to have your scripts inherit from Test which is a bit awkward because (1) this is not a test file, and (2) even if you just renamed Test, you still inherit lots of cheatcodes and forge-std methods which can be dangerous for live transactions, and therefore are not desirable to have present in a script.

As a result, I think we should refactor to have abstract contract Script and abstract contract Test is Script, this way scripts can just inherit from Script, and tests can continue to inherit from Test with no breaking changes.

This abstract Script contract will contain:

  • A minimal Vm interface with just the new broadcast cheatcodes, as well as "view" cheatcodes that don't change state. I may be missing some, but I think this includes: load, addr, getNonce, records, accesses, getCode, and label.
  • The stdMath library.
  • The stdStorage library, but with the checked_write methods disabled.

Libraries don't get inherited by default, so we may need to split the last one into stdStorageRead and stdStorage is stdStorageRead (assuming libraries can inherit? I'm not actually sure). Alternatively perhaps we make it into a contract and use a storage var to conditionally disable checked_write based on that, though this would be a breaking change.

mds1 avatar May 23 '22 18:05 mds1

Interfaces and libraries cannot inherit in Solidity version 0.6.0. This would have allowed us to never duplicate code.

To address checked_write without breaking changes, we'd need to:

  • copy-paste stdStorage and delete checked_write
  • leave it as-is (does not seem to cause any txs but need to experiment more)

I'll get back to this.

ZeroEkkusu avatar May 30 '22 13:05 ZeroEkkusu