safe-global / safe-smart-account

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

Expose revert message when internal transaction fails and `safeTxGas`/`gasPrice` are 0 #715

Open froggiedev opened 9 months ago

froggiedev commented 9 months ago

Context / issue

As per #274, (explained here) will revert if the internal Safe transaction fails and both safeTxGas and gasPrice are set to 0.

In this case, it would be very helpful to expose the actual revert message of the failing transaction, so it can be debugged. Currently, the contract just reverts with GS013, which is not in fact helpful in this case.

Proposed solution

Expose the revert message using the following code, or similar:

success = execute(to, value, data, operation, gasPrice == 0 ? (gasleft() - 2500) : safeTxGas);

if (!success) {
    assembly {
           returndatacopy(0, 0, returndatasize())
           revert(0, returndatasize())
     }
 }

Alternatives

Use a different method to return the original revert string.

Additional context

Returning GS013 in the case where the internal transaction fails leads to a wild goose change that eventually leads users here. Once a user gets here, it is nearly impossible to find the actual reason for the transaction failing without a long game of guess-and-check.