Pre-cache BUY_ETH_ADDRESS as non-vault#4390
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request configures the EIP-4626 price estimator to include the native ETH address in its non-vault tokens cache by default, preventing unnecessary contract calls. It also adds unit and E2E tests to verify this behavior and updates test dependencies. I have no feedback to provide.
AryanGodara
left a comment
AryanGodara
left a comment
There was a problem hiding this comment.
LGTM
one nit from the under development claude skill 👀
Metric drift (very minor) — the pre-seeded entry isn't reflected in eip4626_non_vault_cache_size until the first runtime classification calls metrics::non_vault_cache_size(...). The gauge will be off by one in steady state until any other non-vault is discovered. Negligible operationally.
Not sure if this is even a problem, when I went through it manually. Still pasting here, just in case
| /// * That the BUY_ETH_ADDRESS is cached by default (and the previous | ||
| /// applies to it) | ||
| #[tokio::test] | ||
| async fn buy_eth_address_bypasses_eth_calls() { |
There was a problem hiding this comment.
nit: the second eth means eth_call but, reads ambiguously next to BUY_ETH. imo buy_eth_address_bypasses_rpc_calls could be cleaner.
| async fn buy_eth_address_bypasses_eth_calls() { | ||
| let mut inner = MockNativePriceEstimating::new(); | ||
| let token = BUY_ETH_ADDRESS; | ||
| let expected_price = 1.5; |
There was a problem hiding this comment.
Nit: the native price of ETH is by definition 1 so it's strange to use 1.5 here. Doesn't change the correctness of the test, though.
There was a problem hiding this comment.
I'll argue that if it returns 1.5 it's definitely not by chance 😛
3 participants
Description
Pre-caches BUY_ETH_ADDRESS as a non-vault token at the EIP-4626 estimator level.
Given that BUY_ETH_ADDRESS does not have code deployed to it, this makes it a "valid-enough" token to be ignored by EIP-4626 when enabled and passed on to the rest of the estimation pipeline.
Changes
How to test
Regular tests