Skip to content
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

If safeTxGas is 0 the tx should be reverted on failure #274

Closed
rmeissner opened this issue Mar 7, 2021 · 2 comments · Fixed by #283
Closed

If safeTxGas is 0 the tx should be reverted on failure #274

rmeissner opened this issue Mar 7, 2021 · 2 comments · Fixed by #283

Comments

@rmeissner
Copy link
Member

rmeissner commented Mar 7, 2021

Description

We do not revert if a Safe transaction fails to be able to invalidate the nonce. This is especially important if we use a relay service. In this case it is also important that a proper safeTxGas is set so that the relayer is required to sent enough gas.

In cases where no safeTxGas is sent (aka it is 0) the relayer functionality of the Safe contracts should therefore not be used. If we assume that the transaction is not executed by a relayer (e.g. an owner) we should revert on failure.

Considerations

To ensure that this is only applied when no relayer is used the gasPrice should also be required to be 0

@Uxio0
Copy link
Member

Uxio0 commented Mar 23, 2021

  • On one hand, if we force safeTxGas to be always present estimations when sending the transaction should be very accurate
  • On the other hand, it will take a little more gas to submit a cancellation transaction, but I think that should be alright
  • We will break compatibility for people already using our services, but we will do either way changing how the hash is calculated, so I think this could be a good idea

@gatleas17
Copy link

help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants