Open Anish-Agnihotri opened 2 years ago
Can you expand on cheats.preview
/cheats.batch
?
Can you expand on cheats.preview/cheats.batch?
I think cheats.preview()
is unneeded given we have traces?
cheats.batch()
would probably convert all transactions into a gnosis safe execTransaction
? might need a better name
Thanks for bump—yes, cheats.batch()
is just generating a multi-call transaction.
Is there a standard for that we can use? @gakonst mentioned Gnosis Safe, but I wonder if cheats.batch
would be general enough. Alternatively, this should(?) be solveable using utility libraries (perhaps we can add it in forge-std) by encoding the transactions into e.g. a Gnosis Safe transaction and broadcasting that
Is there interest in supporting this feature? I think it's a natural extension to scripting and would help teams avoid manual Gnosis Transaction Builder inputs or relying on another tech stack (e.g. python with ape-safe) for setting up MS transactions. forge script
currently is good for deployment and setup, but breaks down if you change over the admin address to a MS contract (which you can't send txns from with vm.broadcast()
).
An initial version could be similar to the first iteration of CREATE2, hardcode the MultiSend contract from Gnosis and use their format for encoding transactions. The user would provide their safe address and a API call could be sent to the Gnosis Transaction Service to initiate the transaction.
API could mirror broadcasting with something like vm.startBatch()
and vm.stopBatch()
. The CLI could reuse the --broadcast
flag and call the script in the same manner as existing ones.
I looked through the codebase a bit to see where it could naturally fit in, but I don't have a strong grasp of the existing scripting architecture. I am willing to work on this, but am not very experienced with Rust so I would need some pointers.
Yep I think that's valuable. Motivation makes sense.
Can you give a description of how you imagine the API working and then how a list of transactions would get encoded internally into a gnosis safe TX?
+1 on this, would really improve the developers' experience of interacting with Safe, without going out of Solidity code, which is a quite typical use case.
Regarding the API, maybe following the approach of other tx sender types like --ledger
. So something like --safe-multisig
and setting --sender
as the specific Safe instance.
Here's a potential UX: First, we add logic to scripting so that it can take the gas estimates and split all broadcasted txs into chunks of 30M (or whatever the gas limit is per-chain). This has two use cases: (1) batching multi-sig txs into a single tx, and (2) batching flashbots txs into a single bundle.
For (2), we use that logic when the flashbots RPC (or --flashbots
?) is passed, and automatically split the script broadcast into as few bundles as possible
For (1), if you pass --sender <address>
, use the RPC to check if that's a known contract wallet (gnosis safe being the main one we should support first, maybe expanding to argent and DSProxy later). If so, bundle the txs into as few txs as possible. Then for each, either:
For dry runs, you might not have an RPC provided which means you can't infer that it's a multisig tx that needs bundling, so perhaps there's also a --multisig <walletType>
to force this instead of relying on inference
An alternative to passing the safe address to --sender
or another CLI param could be to include it in the forge-std
API, e.g. vm.startBatch(safeAddress)
. My thoughts on the Solidity API were to be able to create different batches in one function with blocks of vm.startBatch()
and vm.stopBatch()
bounding them. @mds1 makes a good point about the transaction gas limit which should be checked for each batch.
A rough example would be:
function swap(uint256 amountIn) public {
address safe = vm.envAddress("MS_ADDRESS");
// Get amount out for swap
uint256 minAmountOut = router.getAmountOut(path, amountIn);
// Start batch
vm.startBatch(safe);
// Approve the router for the swap token
token1.approve(router, amountIn);
// Execute swap
uint256 amountReceived = router.swap(...);
// Stake tokens received in a contract
token2.approve(staking);
staking.stake(amountReceived);
vm.stopBatch();
}
The MultiSend contract expects the transaction batch to be encoded as follows:
/// @dev Sends multiple transactions and reverts all if one fails.
/// @param transactions Encoded transactions. Each transaction is encoded as a packed bytes of
/// operation as a uint8 with 0 for a call or 1 for a delegatecall (=> 1 byte),
/// to as a address (=> 20 bytes),
/// value as a uint256 (=> 32 bytes),
/// data length as a uint256 (=> 32 bytes),
/// data as bytes.
/// see abi.encodePacked for more information on packed encoding
Gnosis has implemented logic to batch transactions in this format in ethers-multisend, which should provide a good starting point.
The Safe Transaction Service allows Safe owners and their delegates to initiate transactions, which will appear in Safe app for signers to execute. In this context, --broadcast
or another flag could be used to designate whether the batch is sent to the service for execution.
Something important to have is the possibility of creating the Safe tx with DELEGATE_CALL (1) operation
type when communicating with the Safe tx Service (params here). Basically to indicate that the tx, once signed, will be executed by the Safe contract via DELEGATECALL, as an alternative to batch CALL.
We use this pattern for clarify-sake of what is executed: doing 1 single tx calling 1 single function in a contract, via DELEGATECALL.
@eboadom what are all the ways that you configure the safe tx beyond the operation
?
Currently, we are not doing this programmatically, basically because we can achieve something similar with a "push pattern" of permissions to a smart contract, instead of DELEGATECALL.
And the reason for this is to not really have many dependencies outside of the Solidity setup (Foundry).
So in terms of which params to send to the API, I think pretty default in our case, just the operation
I've been thinking about a more minimalist way to implement this. The current Solidity API with vm.startBroadcast(address)
can support these features if some additional options are added to the CLI.
Taking a step back, I believe there are two separate features here.
If you do both of those, then you could batch broadcasted transactions and submit to the the service.
For 1, a CLI flag --batch
could be added that encodes the VecDeque<TypedTransaction>
of broadcastable transactions into a single MultiSend transaction per the required format above. One place to insert this logic could be cmd/forge/script/broadcast.rs::ScriptArgs::handle_broadcastable_transactions()
. These have the benefit of behaving like single transactions vs. RPC batch transactions.
In regards to @eboadom's comments on the Operation type (CALL vs. DELEGATECALL), ape-safe assumes all the batched transactions are CALLs and then makes the single txn from the Safe contract to the MultiSend contract a DELEGATECALL (https://github.com/banteg/ape-safe/blob/66ab8fefbb0edd4a892e18b26a92fedc4f65ed84/ape_safe.py#L117). If more nuanced control is needed, a cheatcode could be added to make individual batched calls into delegate calls (e.g. vm.delegatecall()
).
For 2, I'm less sure of a good UX. One option could be to update the --broadcast
flag to a different variable type and condition how the transaction is sent based on the provided parameter, e.g. --broadcast safe_transaction
. A default value could be used to keep the existing behavior the same (submitting the transactions to the provided blockchain network).
Regardless of the CLI flag used, a new function in broadcast.rs
could send the transaction to a default or provided endpoint for the safe transaction service. Additional branching logic at the bottom of handle_broadcastable_transactions
can handle whether the txns are broadcast to the network or another service.
Looking for feedback on this framing and implementation approach.
I see your point. Having a "mode" for broadcast
makes sense, as sometimes I may want to execute the script from a Safe and sometimes from an EOA.
So this might be simpler than we thought:
convert to a single safe tx
converting to a single safe TX is likely undesirable. This is a power user feature, and we should have a relatively powerful interface to it.
We want to switch back and forth within the context of a script. E.g. broadcast all unpermissioned actions, and queue any permissioned actions for execution in a later batch.
We also don't want to assume that only 1 batch will be produced in the script, so we need a batch identifier.
the right api is probably more like:
vm.sendViaSafe(address safe)
- decorator for submitting a single txnvm.newSafe(address safe) returns (uint identifier)
- should check that the address is a safe, we have a signer for an owner of that safevm.inSafe(identifier)
decorator above a transactionvm.startSafe(identifer)
for a region of codevm.stopSafe(identifier)
Scripts that produce safe batches should sign and submit to the API, and print a link to the relevant safe UI after execution
This might sound super scary, but it may be worth rewriting the script feature in the coming months (cc @joshieDo FYI/Not a Request) if we're thinking about how to support use cases like that. Need to noodle on it. Might be a nice X-mas refactoring project for me...
@prestwich If non-permissioned and permissioned actions are interleaved during script execution, but actual execution is not interleaved and happens a completely different times, it will be impossible to test the script. That seems like a source of hard to understand errors.
At least in my case, the following would be enough and I'm guessing quicker to be ready for use:
--contract-sender
CLI option.
--broadcast
flag submits a tx package to the --sender
safe directly.Delegatecalls can be manually initiated from the script.
If non-permissioned and permissioned actions are interleaved during script execution, but actual execution is not interleaved and happens a completely different times, it will be impossible to test the script.
This really depends on what is being done, and users sophisticated enough to design a system that does that should be allowed to manage their own rough edges. It's not an average use case, so we should allow it but without guard rails
@Oighty @gakonst Currently going through deployments using Gnosis Safe and stumbled upon this issue. Still trying to figure out the best way to glue everything together, and where to jump from forge scripts into typescript and the Gnosis Safe Transactions API.
Anything new since this thread has been updated? I see https://github.com/foundry-rs/foundry/pull/4878 was added, but looks like it requires building the typed data manually in scripts as opposed to just broadcasting? Let me know if I am misunderstanding.
Another observation regarding the above APIs. If it will be possible to have multiple batches I suggest having each batch have an id
, similar to snapshots. So
uint256 batchId = vm.startBatch();
// do some stuff
vm.stopBatch(batchId); // sanity check that we are starting/stopping the correct batch
// customize how batch is sent
address safe;
uint256 GNOSIS_SAFE; // some constant for the batch "method" or "mode"
vm.broadcastBatch(batchId, safe, GNOSIS_SAFE);
Some more thoughts:
sendViaSafe
hardcodes too much and pollutes the namespace. I could see e.g. falling back to a Ledger for certain deployments with the same script, so the method can be switched from GNOSIS_SAFE to something else depending on the chainId or something else.forge
doesn't already have the method available for some new multi-sig standard, it can be extended outside of forge.Anything new since this thread has been updated? I see #4878 was added, but looks like it requires building the typed data manually in scripts as opposed to just broadcasting? Let me know if I am misunderstanding.
I don't believe a built-in solution is currently being worked on.
I think having the batch "method" be runtime customizable is important - I think having tons of explicit cheats like sendViaSafe hardcodes too much and pollutes the namespace. I could see e.g. falling back to a Ledger for certain deployments with the same script, so the method can be switched from GNOSIS_SAFE to something else depending on the chainId or something else.
I generally agree with this, especially in the context of ERC-4337 and future contract wallet designs. I think it might make sense to have the routing of transactions be handled based on wallet config. More specifically, be able to "send" transactions to RPCs (e.g. traditional mempool) or alternative destinations (e.g. Safe Transaction Service or AA Alt Mempools). The options here depend on the wallet type. Therefore, it may make sense to add a Wallet
variation for contract wallets and configure the txn destination as part of the wallet. GnosisSafe + Safe Transaction Service could be one wallet + destination config. A signer would need to be configured for a contract wallet and adds some complexity.
Once you have the above, transactions batches could be defined in a script via something like the API you referenced. The type of "batch" is dependent on the destination and what it supports. Some chains support sending batches of txns to RPCs. STS supports MultiSend type batches.
Hi guys, just stumbled upon this need and wanted to offer my 2c on how I resolved it in case it is helpful to anyone else running into this use case.
The motivation was to generate and test a gnosis safe bundle for an existing multisig on mainnet. We could then submit the bundle using cast
, or the Gnosis Safe Transaction Builder UI. For simulation, it required manipulation of the threshold
and owners
storage slots forcefully set threshold to 1 and set a known address as an owner. The following snippet achieves the bundle generation and simulation entirely in Foundry tests:
Gnosis.sol
pragma solidity =0.8.15;
import "forge-std/Test.sol";
import {MULTISIG} from "src/scripts/Config.sol";
struct GnosisTransaction {
address to;
uint256 value;
bytes data;
}
interface IGnosisSafe {
function getThreshold() external view returns (uint256);
function isOwner(address owner) external view returns (bool);
function getOwners() external view returns (address[] memory);
function execTransaction(
address to,
uint256 value,
bytes memory data,
uint8 operation,
uint256 safeTxGas,
uint256 baseGas,
uint256 gasPrice,
address gasToken,
address refundReceiver,
bytes memory signatures
) external payable returns (bool success);
function encodeTransactionData(
address to,
uint256 value,
bytes memory data,
uint8 operation,
uint256 safeTxGas,
uint256 baseGas,
uint256 gasPrice,
address gasToken,
address refundReceiver,
uint256 _nonce
) external view returns (bytes memory);
function nonce() external view returns (uint256);
}
interface IMultiSendCallOnly {
/// Each transaction is encoded as a packed bytes of
/// operation has to be uint8(0) in this version (=> 1 byte),
/// to as a address (=> 20 bytes),
/// value as a uint256 (=> 32 bytes),
/// data length as a uint256 (=> 32 bytes),
/// data as bytes.
function multiSend(bytes memory transactions) external payable;
}
contract GnosisTest is Test {
using stdStorage for StdStorage;
IGnosisSafe safe = IGnosisSafe(MULTISIG);
IMultiSendCallOnly multiSendCallOnly =
IMultiSendCallOnly(0x40A2aCCbd92BCA938b02010E17A5b8929b49130D);
function enableSimulation() public {
address newOwner = vm.addr(0xB0B);
vm.store(MULTISIG, bytes32(uint256(4)), bytes32(uint256(1))); // slot for threshold is 4
assertEq(safe.getThreshold(), 1);
address[] memory owners = safe.getOwners();
bytes32 ownerData = vm.load(
MULTISIG,
keccak256(abi.encode(owners[0], 2)) // slot for owners is 2
);
// zero out previous owner
vm.store(MULTISIG, keccak256(abi.encode(owners[0], 2)), bytes32(0));
// swap in new owner
vm.store(MULTISIG, keccak256(abi.encode(newOwner, 2)), ownerData);
assertEq(safe.isOwner(newOwner), true);
}
function getSignature(
address to,
uint256 value,
bytes memory data,
uint8 operation,
uint256 safeTxGas,
uint256 baseGas,
uint256 gasPrice,
address gasToken,
address refundReceiver,
uint256 nonce
) public returns (bytes memory) {
bytes memory txHashData = safe.encodeTransactionData(
to,
value,
data,
operation,
safeTxGas,
baseGas,
gasPrice,
gasToken,
refundReceiver,
nonce
);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(0xB0B, keccak256(txHashData));
bytes memory signature = abi.encodePacked(r, s, v);
return signature;
}
function executeBatch(GnosisTransaction[] memory batch) public {
bytes memory data = getBatchExecutionData(batch);
executeData(address(multiSendCallOnly), 1, data);
}
function getBatchExecutionData(
GnosisTransaction[] memory batch
) public returns (bytes memory) {
bytes memory transactions = new bytes(0);
for (uint256 i = 0; i < batch.length; i++) {
transactions = abi.encodePacked(
transactions,
uint8(0),
batch[i].to,
batch[i].value,
batch[i].data.length,
batch[i].data
);
}
// calldata for calling multiSend with transactions
bytes memory data = abi.encodeWithSelector(
multiSendCallOnly.multiSend.selector,
transactions
);
return data;
}
function executeData(
address to,
uint8 operation,
bytes memory data
) public {
uint256 value = 0;
uint256 safeTxGas = 0;
uint256 baseGas = 0;
uint256 gasPrice = 0;
address gasToken = address(0);
address refundReceiver = address(0);
uint256 nonce = safe.nonce();
bytes memory signature = getSignature(
to,
value,
data,
operation,
safeTxGas,
baseGas,
gasPrice,
gasToken,
refundReceiver,
nonce
);
vm.prank(vm.addr(0xB0B));
safe.execTransaction(
to,
value,
data,
operation,
safeTxGas,
baseGas,
gasPrice,
gasToken,
refundReceiver,
signature
);
}
}
BatchTest.t.sol
pragma solidity =0.8.15;
import "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
import {GnosisTest, GnosisTransaction} from "./Gnosis.sol";
contract BatchTest is GnosisTest {
function testForkUpgrade() public {
vm.createSelectFork(vm.envString("MAINNET_RPC"));
enableSimulation();
executeBatch(createTestBatch());
}
function createTestBatch() public returns (GnosisTransaction[] memory) {
GnosisTransaction[] memory batch = new GnosisTransaction[](2);
address guyToApprove = address(0xdeadbabe);
address token = 0xF17A3fE536F8F7847F1385ec1bC967b2Ca9caE8D;
// Sample transaction 1
bytes4 approveFunctionSignature = bytes4(
keccak256("approve(address,uint256)")
);
uint256 wad1 = 100;
bytes memory approveData1 = abi.encodeWithSelector(
approveFunctionSignature,
guyToApprove,
wad1
);
batch[0] = GnosisTransaction({to: token, value: 0, data: approveData1});
// Sample transaction 2
uint256 wad2 = 200;
bytes memory approveData2 = abi.encodeWithSelector(
approveFunctionSignature,
guyToApprove,
wad2
);
batch[1] = GnosisTransaction({to: token, value: 0, data: approveData2});
return batch;
}
}
@junhohong yep, that's the approach I ended up pursuing in lieu of native support for this. With #4878 , you can send payloads to be signed from the Solidity script via FFI. @ind-igo and I built out a library to do this called forge-safe. It also interacts with the safe-transaction-service API via Surl. I have a minor update to push that allows you to get values back from simulated calls and use them in the scripts.
Just a heads up, I recently submitted a PR to do some refactoring of the https://github.com/ind-igo/forge-safe library to utilize the new improvements to wallet UX and vm.sign
keystore compatibility which should remove any need to rely on environment variables.
Haven't tested it with hardware wallets though, not sure if vm.sign
can correctly identify a --sender
with a ledger/trezor provided via options.
Curious if the new vm.broadcastRawTransaction
could be useful in this context: https://github.com/foundry-rs/foundry/pull/4931
Component
Forge
Describe the feature you would like
Description:
My favorite tool from the Brownie/Vyper ecosystem is ape-safe. It lets you:
Now that Foundry is adding support for a local node and deploy scripts, it would be useful to have similar functionality out of the box.
Example:
I'm not sure what this looks like practically yet, but it might just be an executable Solidity script (transcribing the ape-safe Python example):
Potential use-cases:
Some potential applications:
Additional context
No response