-
Notifications
You must be signed in to change notification settings - Fork 168
Pre-cache BUY_ETH_ADDRESS as non-vault #4390
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ use { | |
| dashmap::DashSet, | ||
| ethrpc::{AlloyProvider, alloy::errors::ContractErrorExt}, | ||
| futures::{FutureExt, future::BoxFuture}, | ||
| model::order::BUY_ETH_ADDRESS, | ||
| num::{BigInt, BigRational, ToPrimitive}, | ||
| number::conversions::u256_to_big_rational, | ||
| std::time::{Duration, Instant}, | ||
|
|
@@ -37,7 +38,9 @@ impl Eip4626 { | |
| Self { | ||
| inner, | ||
| provider, | ||
| non_vault_tokens: DashSet::new(), | ||
| // BUY_ETH_ADDRESS is not ERC-20, but it is a valid estimation address | ||
| // so we need to make sure it bypasses the EIP-4626 estimator | ||
| non_vault_tokens: DashSet::from_iter([BUY_ETH_ADDRESS]), | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -224,6 +227,8 @@ mod tests { | |
| use { | ||
| super::*, | ||
| crate::{HEALTHY_PRICE_ESTIMATION_TIME, native::MockNativePriceEstimating}, | ||
| alloy::providers::mock::Asserter, | ||
| std::borrow::Cow, | ||
| }; | ||
|
|
||
| #[test] | ||
|
|
@@ -253,6 +258,41 @@ mod tests { | |
| assert!((rate - 2.0).abs() < 1e-9, "rate={rate}"); | ||
| } | ||
|
|
||
| /// Tests two (related) things: | ||
| /// * Cached tokens bypass the EIP-4626 provider calls — i.e. calling | ||
| /// decimals, assets, etc | ||
| /// * 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() { | ||
| let mut inner = MockNativePriceEstimating::new(); | ||
| let token = BUY_ETH_ADDRESS; | ||
| let expected_price = 1.5; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll argue that if it returns 1.5 it's definitely not by chance 😛 |
||
| inner | ||
| .expect_estimate_native_price() | ||
| .withf(move |t, _| *t == token) | ||
| .returning(move |_, _| Box::pin(async move { Ok(expected_price) })); | ||
|
|
||
| let asserter = Asserter::new(); | ||
| asserter.push_failure_msg(Cow::from("calls are not being bypassed")); | ||
| let web3 = ethrpc::Web3::with_asserter(asserter); | ||
|
|
||
| let estimator = Eip4626::new(Box::new(inner), web3.provider); | ||
|
|
||
| let result = estimator | ||
| .estimate(token, HEALTHY_PRICE_ESTIMATION_TIME) | ||
| .await; | ||
| assert_eq!(result.unwrap(), expected_price); | ||
|
|
||
| let result = estimator | ||
| .estimate(Address::random(), HEALTHY_PRICE_ESTIMATION_TIME) | ||
| .await; | ||
| assert!( | ||
| matches!(result, Err(PriceEstimationError::EstimatorInternal(_))), | ||
| "{result:?}" | ||
| ); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn non_vault_tokens_delegate_to_inner() { | ||
| let mut inner = MockNativePriceEstimating::new(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the second
ethmeanseth_callbut, reads ambiguously next toBUY_ETH.imobuy_eth_address_bypasses_rpc_callscould be cleaner.