feat(forge): Solidity Scripting #1208

brockelmore · 2022-04-05T16:53:27Z

Motivation

We should allow for scripting in solidity. This can be used for deployment as well as generic onchain state mutating interaction. Closes #402

Solution

Create broadcast, startBroadcast, and stopBroadcast cheatcodes. This tells the cheatcode handler to construct a transaction out of a particular call.

This will be the backbone of forge deploy as well, but likely will have tighter semantics for easier usage (like expecting deploy contracts to be in a deploy folder and have particular naming conventions, etc).

Current impl is based on run command and currently has significant code duplication, but will diverge significantly later so it is its own file.

You can run forge script ./testdata/cheats/Broadcast.t.sol --tc BroadcastTest --sig "deploy()" -vvv to see it generate the transactions. Testing this feature will become significantly easier once forge node is merged.

Checklist

Implement Cheatcodes
Construct basic TypedTransactions
Detect if chain supports EIP-1559, convert Legacy to 1559 txs
- edit: we are currently converting all transactions to 1559 in this MVP. Can add chain_id checking easily
Support multiple signing keys
- Clap doesn't seem to like Vec<Wallet>
Signing transactions
Transaction lifecycle management
- We need to make sure that if transaction nonce for the EOA changes out from under us, we abort the whole batch of transactions until we rerun (otherwise contract addresses may have changed, causing potentially future inputs/to addresses to be incorrect)
- edit: Maybe we check that the calldata of previous nonce for non-zeroeth transaction == expected call data
Caching of executed/to-be-executed transactions
Ensure created contract addresses match expected (i.e. simulated nonce to actual nonce)
- If it doesn't, all transactions must be resimmed otherwise see above (tx lifecycle)
- edit: we use strict nonces for now, which guarantees this for us

gakonst

pumped for this progress, highlights the need to clean up the linking/run code.

.gitmodules

evm/src/executor/inspector/cheatcodes/mod.rs

brockelmore · 2022-04-05T23:05:11Z

Some notes as I go:

Supporting multiple signers that do deployment is difficult and i lean toward not allowing it in first released version. Take for example a contract like:

contract Test is DSTest {
    function t(uint256 a) public returns (uint256) {
        uint256 b = 0;
        for (uint256 i; i < a; i++) {
            b += F.t2();
        }
        emit log_string("here");
        return b;
    }
}

library F {
    function t2() public view returns (uint256) {
        return 1;
    }
}

contract BroadcastTest is DSTest {
    Cheats constant cheats = Cheats(HEVM_ADDRESS);

    function deploy() public {
        // this will generate a `create` transaction
        cheats.broadcast(address(0x1337));
        Test test = new Test();

        // this wont generate tx to sign
        uint256 b = test.t(4);

        // this will
        cheats.broadcast(address(0x1338));
        test.t(b);     
    }

    function deployOther() public {
        cheats.broadcast(address(0x1338));
        Test test = new Test();

        cheats.broadcast(address(0x1339));
        test.t(0);     
    }

    function deployPanics() public {
        cheats.broadcast(address(0x1337));
        Test test = new Test();

        cheats.broadcast(address(0x1338));
        Test test2 = new Test();

        cheats.broadcast(address(0x1338));
        test.t(0);     
    }
}

While we can get smart about deduping libraries, and that helps, the crux of the matter is which address is suppose to deploy said libraries? 0x1337 or 0x1338? This changes how we would link the entirety of BroadcastTest contract (and its dependency Test). First address would make sense. Or we force the user to tell us the library deployer address. With multiple contracts it doesn't get much easier and we will have to expand the linking & setup logic.

For first iteration, unless we are sure we are rock solid, I suggest we hold off allowing multiple deployers. This doesnt stop us from allow multiple senders, to be clear. And the solution is to split a script into two functions and call the command twice.

Currently, we relink contracts based on the first deployer deploying the contract libraries, then rerun the dry-run with the re-linked contracts to confirm it still works.

The code currently works for generating transactions, i.e. for deploy function above:

forge script ./cheats/Broadcast.t.sol --tc BroadcastTest --sig "deploy()" -vvvv

prints & writes to out/Broadcast.t.sol/scripted_transactions/deploy().json:

[
  {
    "type": "0x00",
    "from": "0x0000000000000000000000000000000000001337",
    "data": "0x6059610038600b82828239805160001a607314602b57634e487b7160e01b600052600060045260246000fd5b30600052607381538281f3fe730000000000000000000000000000000000000000301460806040526004361060335760003560e01c8063baf2f868146038575b600080fd5b600160405190815260200160405180910390f3fea164736f6c634300080d000a"
  },
  {
    "type": "0x00",
    "from": "0x0000000000000000000000000000000000001337",
    "value": "0x0",
    "data": "0x60806040526000805460ff1916600117905534801561001d57600080fd5b5061020e8061002d6000396000f3fe608060405234801561001057600080fd5b50600436106100415760003560e01c8063afe29f7114610046578063ba414fa61461006c578063fa7626d41461008e575b600080fd5b610059610054366004610188565b61009b565b6040519081526020015b60405180910390f35b60005461007e90610100900460ff1681565b6040519015158152602001610063565b60005461007e9060ff1681565b600080805b838110156101335773910cbd665263306807e5ace0351e4358dc6164d863baf2f8686040518163ffffffff1660e01b8152600401602060405180830381865af41580156100f1573d6000803e3d6000fd5b505050506040513d601f19601f8201168201806040525081019061011591906101a1565b61011f90836101d0565b91508061012b816101e8565b9150506100a0565b507f0b2e13ff20ac7b474198655583edf70dedd2c1dc980e329c4fbb2fc0748b796b60405161017a906020808252600490820152636865726560e01b604082015260600190565b60405180910390a192915050565b60006020828403121561019a57600080fd5b5035919050565b6000602082840312156101b357600080fd5b5051919050565b634e487b7160e01b600052601160045260246000fd5b600082198211156101e3576101e36101ba565b500190565b6000600182016101fa576101fa6101ba565b506001019056fea164736f6c634300080d000a"
  },
  {
    "type": "0x00",
    "from": "0x0000000000000000000000000000000000001338",
    "to": "0x1885d5b3c1563cd9237e7d8e6cddbada4a353b4e",
    "value": "0x0",
    "data": "0xafe29f710000000000000000000000000000000000000000000000000000000000000004"
  }
]

and the corresponding trace:

First transaction is deploying the F library, second deploys the Test contract, and finally a call to Test.t. Nonce management hasnt been tackled yet either.

gakonst · 2022-04-05T23:50:54Z

Supporting multiple signers that do deployment is difficult and i lean toward not allowing it in first released version.

ACK - this makes sense. Let's get it over the line with 1 signer (which is what all deployment frameworks do, for the most part), and we can revisit later if ppl really want it.

First transaction is deploying the F library, second deploys the Test contract, and finally a call to Test.t. Nonce management hasnt been tackled yet either.

You probably want to leave nonces unfilled and let the "driver" script fill them in?

brockelmore · 2022-04-06T00:02:34Z

You probably want to leave nonces unfilled and let the "driver" script fill them in?

For deployments we likely need to have a nonce expectation at the very least. This is because we may have future transactions that depend on the address created

brockelmore · 2022-04-15T23:35:22Z

We now can send transactions on behalf of users:

forge script ./cheats/Broadcast.t.sol -i 1 --tc BroadcastTest --sig "deployOther()" --fork-url http://localhost:8545 --execute

-i 1 means the user wants to input a single private key via secure command prompt. --execute tells the command that the user wishes for the transactions to be sent onchain.

Order of operations:

No special linking done, just execute the script with the evm_opts.sender as the linker, display the trace if requested
Relink the contracts if needed, if this codepath is activated, there can only be one address that does contract creation. Otherwise, you can have as many deployers as you want
Reexecute the full script with the relinked contracts, display trace if requested
Grab the transactions that were generated by step 3, printing the raw json and writing transaction to a file
Simulate just the generated transactions, give a user an estimate of total gas used
if the user added --execute, connect/parse wallet config, and send onchain

continuing from the above screenshot & json, the program continues as such:

sambacha · 2022-04-16T17:08:07Z

Can a failover rpc endpoint be specified in the event of some threshold of latency is experienced so as to not prematurely end the deployment?

eg, given some stall-out time in ms switch rpc endpoint from A to B, refetch chainId, getBalance, etc (relevant rpc commands) then resume?

Maybe not for the first releasee just thought i would mention it, this looks great 👍💯🔥

cli/src/cmd/forge/run.rs

gakonst · 2022-04-18T17:06:36Z

cli/src/cmd/forge/mod.rs

@@ -48,6 +48,7 @@ pub mod inspect;
 pub mod install;
 pub mod remappings;
 pub mod run;
+pub mod script;


i cannot comment on teh file, but the .gitmodules above is probably unneeded

gakonst · 2022-04-18T17:07:39Z

testdata/foundry.toml

@@ -0,0 +1,45 @@
+[default]


is this needed?

the file itself? no. i am actually a bit annoyed by the structure of testdata directory personally. I think it should be a standard forge style structure. interested in your thoughts @onbjerg

gakonst · 2022-04-18T17:09:11Z

utils/src/lib.rs

    // we dont use mainnet state for evm_opts.sender so this will always be 1
    // I am leaving this here so that in the future if this needs to change,
    // its easy to find.


probably need to update comment

gakonst · 2022-04-18T17:10:58Z

utils/src/lib.rs

+            // NOTE: We add 1 to the nonce, because if we reached this codepath, this contract has a
+            // dependency. At the very least, that one contract will be deployed *prior*
+            // to this one, but wont appear in our deployment vector. It probably should
+            // here, but it doesn't.


what do you think can go wrong here wrt dependencies?

I tested this manually pretty extensively and basically we are doing what we do now, but in a slightly better way, where before there was a footgun when doing the linking. I didn't understand why I needed nonce to be 1 (i.e. in the link function) before, but now i do - its because if there is a dependency, fundamentally there is going to be some root contract that has no dependency that will be first. As far as what can go wrong, im not entirely sure, as I said, this is basically what we currently do just in a less dumb way

gakonst · 2022-04-18T17:15:25Z

cli/src/cmd/forge/script.rs

@@ -0,0 +1,932 @@
+use crate::{


everything looks good - need to spend some more time testing it / reviewing this file, but overall very exciting. might be time we got a "nice" abstraction for linking, because it seems to complicate things a lot in this case?

Rjected

Cool stuff, tested contract deployment - dry run seems to work but will re-test with once nonce/lifecycle management is finished

Rjected · 2022-04-24T03:40:44Z

cli/src/cmd/forge/script.rs

+        let mut nonce = if let Some(ref fork_url) = evm_opts.fork_url {
+            foundry_utils::next_nonce(evm_opts.sender, fork_url, None)?
+        } else {
+            U256::zero()
+        };


Ran into an issue when testing on goerli - evm_opts.sender was the default 0x00a329c0648769a73afac7f9381e08fb43dbea72, leading to a nonce of 43 since this is the blank seed phrase address, I guess it has transactions on goerli and other networks.
new_sender was never set, so the nonce remained at 43 and every transaction would get stuck in the mempool, crashing when passing --execute.
I'm guessing the default sender will not be used to determine the nonce when tx lifecycle management is added, just commenting in case ppl have issues with nonces or are testing on live networks

brockelmore · 2022-04-24T14:14:01Z

thanks for testing :) @Rjected

joshieDo · 2022-05-24T10:28:39Z

@theforager thanks for the feedback! highly appreciate it

Seems to me that both item 2 and item 3 are from a transaction being stuck on the pool after the initial --broadcast failed. During item 2, the transaction was still there. During item 3 it had been confirmed already, but we had no way of knowing that.

Made a few changes to try and make --resume a bit more robust. It now tries to store the TxHash when failing any broadcast, and uses it to try and retrieve a receipt on a later --resume run.

gakonst

alright - some nits but I think we're good to merge and we can follow up separately

gakonst · 2022-05-29T01:10:18Z

cli/src/cmd/forge/script/broadcast.rs

+        let local_wallets = self.wallets.find_all(chain, required_addresses)?;
+        if local_wallets.is_empty() {
+            eyre::bail!("Error accessing local wallet when trying to send onchain transaction, did you set a private key, mnemonic or keystore?")
+        }


Let's follow up with Ledger/Trezor support here @joshieDo in a separate PR?

cli/src/cmd/forge/script/build.rs

gakonst · 2022-05-29T01:16:31Z