Skip to content

fix: safe (or other sca) gas estimates#980

Merged
laurgk merged 13 commits intomainfrom
fix/safe-gas-estimates
Apr 28, 2025
Merged

fix: safe (or other sca) gas estimates#980
laurgk merged 13 commits intomainfrom
fix/safe-gas-estimates

Conversation

@TateB
Copy link
Copy Markdown
Member

@TateB TateB commented Apr 2, 2025

most (if not all) SCA implementations use more than the gas allocated for a simple .transfer(). this means that when interacting with a contract that utilises .transfer(), that function will fail with an out of gas error by default. to avoid this, we can generate an access list of storage slots accessed during the transaction, which then pays for the cold gas of any storage slot at the start of the transaction. with cold gas paid for, during the execution of .transfer(), only the warm gas is paid, and (generally) this means it stays below the gas allowed for that operation.

access lists were already being generated for safe transactions in the transaction dialog, but this PR expands that to any transaction (e.g. for non-safe SCAs), and adds support for them to be generated when doing register/renew gas estimates.

changes:

  • estimating gas in the transaction dialog now generates an access list for all transactions, even if not connected via safe. this allows for compatibility with any SCA.
  • when estimating gas with state overrides, a .transfer access list is now generated from running this yul code. this just executes a transfer to the target SCA address (the address in the gist is just a placeholder), meaning the required access list for a .transfer() can be derived.

fixes fet-1800, fet-1799, fet-1771, fet-1772

@TateB TateB requested a review from sugh01 as a code owner April 2, 2025 17:46
@storywithoutend storywithoutend requested a review from laurgk as a code owner April 28, 2025 05:30
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@laurgk laurgk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested - good to go.

@laurgk laurgk merged commit 106bfc7 into main Apr 28, 2025
39 checks passed
@laurgk laurgk deleted the fix/safe-gas-estimates branch April 28, 2025 10:38
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 this pull request may close these issues.

5 participants