search

safe-global / safe-smart-account

Safe allows secure management of blockchain assets.
https://safe.global
GNU Lesser General Public License v3.0
1.88k stars 927 forks source link

PoC for Safe simulateTransaction function #789

Closed mshakeg closed 1 month ago

mshakeg commented 4 months ago

This PR introduces a new function, simulateTransaction, to the Safe contract, allowing users to simulate transactions without requiring the threshold of owner signatures. The simulation helps estimate gas usage and verify if a transaction would succeed without actually executing it, thus providing a reliable method for pre-execution checks.

Fixes #788

github-actions[bot] commented 4 months ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

mshakeg commented 4 months ago

I have read the CLA Document and I hereby sign the CLA

mshakeg commented 4 months ago

recheck

mmv08 commented 4 months ago

Hey, thank you for the contribution. this can be an external contract used through https://github.com/safe-global/safe-smart-account/blob/8f80a8372d193be121dcdb52e869a258824e5c0f/contracts/common/StorageAccessible.sol#L42. We're quite short on the bytecode size, so I'm almost sure we will not include this in the core contract.

mshakeg commented 4 months ago

@mmv08 thanks, so I took a look at linked StorageAccessible. simulateAndRevert section of code and what from I can tell it should work something like:

  1. say I've got a safeTx that I want to simulate on safe where the safeTx is to, value, data and operation
  2. now I can simulate the above safeTx but I don't have any owner signatures
  3. so I call safe.simulateAndRevert(targetContract, calldataPayload)
    • targetContract: this will be the SimulateTxAccessor address
    • calldataPayload: this will be the encoded bytes of SimulateTxAccessor.simulate(to, value, data, operation)
  4. now success can be extracted from the revert message

Is this understanding correct? And if so, does the ts sdk expose a function to do this simulation? Also, since it reverts eth_estimateGas wouldn't return the gasUsed for the simulation safeTx, so that seems to be a deficiency with this method.

mshakeg commented 4 months ago

@mmv08 I added the gasUsed to the revert data. It seems to work fine for most txs, however the simulations for txs that transfer the native asset all fail:

npx hardhat test test/core/Safe.Execution.spec.ts
mmv08 commented 4 months ago

@mmv08 thanks, so I took a look at linked StorageAccessible. simulateAndRevert section of code and what from I can tell it should work something like:

  1. say I've got a safeTx that I want to simulate on safe where the safeTx is to, value, data and operation
  2. now I can simulate the above safeTx but I don't have any owner signatures

  3. so I call safe.simulateAndRevert(targetContract, calldataPayload)

    • targetContract: this will be the SimulateTxAccessor address
    • calldataPayload: this will be the encoded bytes of SimulateTxAccessor.simulate(to, value, data, operation)
  4. now success can be extracted from the revert message

Is this understanding correct? And if so, does the ts sdk expose a function to do this simulation? Also, since it reverts eth_estimateGas wouldn't return the gasUsed for the simulation safeTx, so that seems to be a deficiency with this method.

You can make a library that contains a function similar to your original simulateTransaction method, have this function return whatever you want and then use the return data from there. There are no code changes required in the safe core contract to achieve the functionality you're suggesting

mshakeg commented 4 months ago

@mmv08 agreed, no changes are required to core, but does safe-global have a repo with periphery contracts? seems like this functionality would be quite essential to gas estimation and should be included in the sdk(and so would require leveraging a safe deployed contract).