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

Simulation ignores gas limits #560

Closed
schmanu opened this issue Sep 16, 2022 · 0 comments
Closed

Simulation ignores gas limits #560

schmanu opened this issue Sep 16, 2022 · 0 comments
Assignees

Comments

@schmanu
Copy link
Member

schmanu commented Sep 16, 2022

Bug description

If a transaction is fully signed or only the current owner's signature is missing we do not need and should not overwrite the safe's threshold and pass the manual selected or estimated gas limit.

Reason: The number of signatures matters for the transactions gas limit. If the tx will only check 1 signature instead of n>1 signatures it can occur that a successfully simulated tx reverts when being executed.

Steps to reproduce

  1. Create a new ERC20 transfer
  2. Go to review step
  3. change the gas limit to 21000 (minimum)
  4. click simulate
  5. Observe simulation success
  6. try to execute with that gas limit
  7. Observe error: Your transaction was unsuccessful. [ethjs-query] while formatting outputs from RPC '{"value":{"code":-32000,"message":"intrinsic gas too low"}}'

Expected result

The simulation should fail with "out of gas"

Obtained result

The simulation always uses 30 million gas as limit and always succeeds.

@schmanu schmanu self-assigned this Sep 16, 2022
schmanu added a commit that referenced this issue Sep 16, 2022
- add prevalidated signatures if more signatures are needed and the owner's sig is missing
- pass gasLimit to txSimulation if estimated / manually set
- only overwrite threshold if necessary (threshold won't be reached otherwise)

closes issue #559 & issue #560
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

No branches or pull requests

2 participants