Skip to content

Conversation

@pistomat
Copy link
Contributor

@pistomat pistomat commented Jun 10, 2025

Motivation

I love the new anvil_dealERC20 endpoint! So why not add an endpoint to override allowances? I present you the brand new anvil_setERC20Allowance endpoint! One should not exist without the other imho, it is very useful to have both.

Solution

Basically copied the code from anvil_dealERC20 and modified it to use the allowance(owner,spender,val) function of ERC20.

Note: I also fixed my mistake of adding tenderly_setErc20Balance alias, but Tenderly has different order of parameters, so it would be very confusing.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

pistomat added 2 commits June 10, 2025 11:13
Unfortunately anvil has the parameter order (owner, token) while tenderly has (token, owner), so they are not compatible. Fixing my previous mistake here.
@grandizzy
Copy link
Collaborator

hey @pistomat mind to resolve conflicts in order to approve and run CI? thank you

@pistomat
Copy link
Contributor Author

@grandizzy done

@mattsse
Copy link
Member

mattsse commented Jun 10, 2025

has different order of parameters, so it would be very confusing.

is this otherwise the same impl, if so we should consider changing the order

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

there possibly a way to unify this search behaviour

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.

makes sense, thank you. I think we could reduce amount of code by having a helper fn to accept calldata and override and reuse in both deal / allowance calls?

@zerosnacks
Copy link
Member

agreed on the above, other than that implementation lgtm 👍 , supportive of having this endpoint

@mattsse
Copy link
Member

mattsse commented Jun 10, 2025

yeah, but def easier to do this in a followup

@mattsse mattsse merged commit 1836d5e into foundry-rs:master Jun 10, 2025
22 checks passed
@github-project-automation github-project-automation bot moved this to Done in Foundry Jun 10, 2025
mattsse added a commit that referenced this pull request Jun 10, 2025
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]>
mattsse added a commit that referenced this pull request Jun 10, 2025
* refactor: unify ERC20 storage slot discovery logic

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]>

* chore: add clippy back

---------

Co-authored-by: Claude <[email protected]>
@jenpaff jenpaff added this to the v1.3.0 milestone Jul 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants