-
Notifications
You must be signed in to change notification settings - Fork 361
Fix wrong estimation when only executing and selected safeTxGas is 0 #2013
Fix wrong estimation when only executing and selected safeTxGas is 0 #2013
Conversation
|
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
| console.info(`Estimating transaction success for execution & approval...`) | ||
| let gasEstimation | ||
| // If safeTxGas === 0 we still have to estimate the gas limit to execute the transaction so we need to get an estimation | ||
| if (approvalAndExecution || safeTxGas === 0) { |
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.
why is || safeTxGas === 0 needed?
shouldn't this enter the ìfno matter the safeTxGas value?
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.
nevermind, I debug it and I understand it now.
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.
Verified
Both issues
https://app.zenhub.com/workspaces/safe-multisig---web-5ce554debb310a35b2f8b6f8/issues/gnosis/safe-react/1975 and
https://app.zenhub.com/workspaces/safe-multisig---web-5ce554debb310a35b2f8b6f8/issues/gnosis/safe-react/1970 are fixed
if the transaction has safeTxGas=0 ( created as contract interaction tx or as cancellation tx via mobile) and signed on mobile before execution it's possible to execute such tx without any issues with gas estimation.
|
Verified |
Closes #1970 and #1975
We were trusting safeTxGas would be enough or we would should a "transaction would fail" message when all signatures are gathered and only execution is pending.
But safeTxGas 0 means that the operation can use up to all gasLimit if necessary to execute the operation.
In this case we have to assume this is a correct safeTxGas but estimate the value in order to set a correct gasLimit so transaction will be correctly executed