Skip to content

refactor: unify ERC20 storage slot discovery logic #10749

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

Merged
merged 2 commits into from
Jun 10, 2025

Conversation

mattsse
Copy link
Member

@mattsse mattsse commented Jun 10, 2025

Summary

Extracts duplicated access list handling code from anvil_deal_erc20 and anvil_set_erc20_allowance into a shared find_erc20_storage_slot helper function.

This is a follow-up cleanup to #10746 which introduced anvil_set_erc20_allowance.

Changes

  • Add comprehensive documentation explaining the ERC20 storage slot discovery process
  • Extract helper function find_erc20_storage_slot that encapsulates the slot-finding logic
  • Reduce code duplication by ~80 lines (both functions had nearly identical for-loops)
  • Improve maintainability - changes to slot discovery logic only need to be made in one place
  • Enhance readability - both functions are now much clearer in their intent

How It Works

The helper function performs "storage slot discovery" for ERC20 tokens by:

  1. Access List Generation: Creates an access list by simulating the ERC20 function call
  2. Slot Testing: For each accessed storage slot, temporarily overrides it with the expected value
  3. Value Verification: If the function returns the same value we just wrote, we found the right slot
  4. Slot Identification: Returns the storage slot that corresponds to the ERC20 data

This approach works with any ERC20 token regardless of its internal storage layout, avoiding the need to reverse-engineer Solidity mapping storage calculations.

Before/After

Before (duplicated in both functions):

// ~45 lines of identical access list iteration logic
for item in access_list.0 {
    if item.address \!= token_address {
        continue;
    };
    for slot in &item.storage_keys {
        // ... test slot logic ...
    }
}

After:

// Simple, clear intent
let slot = self.find_erc20_storage_slot(token_address, calldata, expected_value).await?;
self.anvil_set_storage_at(token_address, U256::from_be_bytes(slot.0), value).await?;

<!-- Original prompt that led to this refactoring:
Analyze anvil_deal_erc20 and anvil_set_erc20_allowance functions in crates/anvil/src/eth/api.rs in particular the code for acceslist handling (the for loop) which is mostly duplicated; try to unify this behaviour with a helper function for finding the slot
-->

🤖 Generated with Claude Code

Extracts duplicated access list handling code from `anvil_deal_erc20`
and `anvil_set_erc20_allowance` into a shared `find_erc20_storage_slot`
helper function.

Changes:
- Add comprehensive documentation explaining the slot discovery process
- Reduce code duplication by ~80 lines
- Improve maintainability and consistency between functions
- Fix unfulfilled clippy expectation in cast/tx.rs

This is a follow-up cleanup to #10746 which introduced `anvil_set_erc20_allowance`.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

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

🔥 yyikes

Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@mattsse mattsse added the C-anvil Command: anvil label Jun 10, 2025
@mattsse mattsse enabled auto-merge (squash) June 10, 2025 14:32
@mattsse mattsse merged commit 3aac249 into master Jun 10, 2025
22 checks passed
@mattsse mattsse deleted the refactor/unify-erc20-slot-discovery branch June 10, 2025 14:34
@github-project-automation github-project-automation bot moved this to Done in Foundry Jun 10, 2025
@pistomat
Copy link
Contributor

Thanks for implementing the refactor, you guys are too quick with your response times, you got ahead of me :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-anvil Command: anvil
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants