-
Notifications
You must be signed in to change notification settings - Fork 925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PoC for Safe simulateTransaction function #789
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
recheck |
Hey, thank you for the contribution. this can be an external contract used through
|
@mmv08 thanks, so I took a look at linked
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. |
@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 |
You can make a library that contains a function similar to your original |
let success := delegatecall(gas(), targetContract, add(calldataPayload, 0x20), mload(calldataPayload), 0, 0) | ||
let gasAfter := gas() | ||
let gasUsed := sub(gasBefore, gasAfter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be returned by the targetContract
. You can check the SimulateTxAccessor
contract for an example. You can basically combine its approach with your original contract for detailed simulations
@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). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simulation
let success := delegatecall(gas(), targetContract, add(calldataPayload, 0x20), mload(calldataPayload), 0, 0) | ||
let gasAfter := gas() | ||
let gasUsed := sub(gasBefore, gasAfter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is redundant since SimulateTxAccessor.simulate
returndata
includes the safeTxGas
:
(uint256 estimate, bool success, bytes memory returnData)
This PR introduces a new function,
simulateTransaction
, to theSafe
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