Skip to content

Commit

Permalink
Verify quotes from zero address (#3150)
Browse files Browse the repository at this point in the history
# Description
@nlordell's recent PR stack enabled the use case where a real trader
(wallet is connected in the frontend) does not have the required
balances by faking balances.
But with the new ability to fake balances for many tokens we can even go
one step further.

# Changes
If we know how to fake balances for the given sell token AND no wallet
is connected (i.e. `verification.from.is_zero()`) we simply generate a
random address and use that as the trader with fake balances.
Using a non-zero address is important because many token transfer
functions don't like sending tokens to or from the zero address.

This should prevent the annoying UX where you sometime see a really good
(unverified) price when your wallet is not connected and end up seeing
worse (but verified) prices after you connect your wallet.

## How to test
added an e2e test which produces a verified quote with `from:
H160::zero()`
  • Loading branch information
MartinquaXD authored Dec 6, 2024
1 parent d58ffd7 commit 78a1b81
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 50 deletions.
18 changes: 18 additions & 0 deletions crates/e2e/tests/e2e/quote_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,4 +409,22 @@ async fn verified_quote_with_simulated_balance(web3: Web3) {
.await
.unwrap();
assert!(response.verified);

// with balance overrides we can even verify quotes for the 0 address
// which is used when no wallet is connected in the frontend
let response = services
.submit_quote(&OrderQuoteRequest {
from: H160::zero(),
sell_token: weth.address(),
buy_token: token.address(),
side: OrderQuoteSide::Sell {
sell_amount: SellAmount::BeforeFee {
value: to_wei(1).try_into().unwrap(),
},
},
..Default::default()
})
.await
.unwrap();
assert!(response.verified);
}
110 changes: 60 additions & 50 deletions crates/shared/src/price_estimation/trade_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use {
},
ethcontract::{tokens::Tokenize, Bytes, H160, U256},
ethrpc::{block_stream::CurrentBlockWatcher, extensions::StateOverride, Web3},
maplit::hashmap,
model::{
order::{OrderData, OrderKind, BUY_ETH_ADDRESS},
signature::{Signature, SigningScheme},
Expand Down Expand Up @@ -102,17 +101,20 @@ impl TradeVerifier {
async fn verify_inner(
&self,
query: &PriceQuery,
verification: &Verification,
mut verification: Verification,
trade: &TradeKind,
out_amount: &U256,
) -> Result<Estimate, Error> {
if verification.from.is_zero() {
// Don't waste time on common simulations which will always fail.
return Err(anyhow::anyhow!("trader is zero address").into());
}

let start = std::time::Instant::now();

// this may change the `verification` parameter (to make more
// quotes verifiable) so we do it as the first thing to ensure
// that all the following code uses the updated value
let overrides = self
.prepare_state_overrides(&mut verification, query, trade)
.await
.map_err(Error::SimulationFailed)?;

// Use `tx_origin` if response indicates that a special address is needed for
// the simulation to pass. Otherwise just use the solver address.
let solver = trade.tx_origin().unwrap_or(trade.solver());
Expand All @@ -132,7 +134,7 @@ impl TradeVerifier {

let settlement = encode_settlement(
query,
verification,
&verification,
trade,
&tokens,
&clearing_prices,
Expand All @@ -141,7 +143,7 @@ impl TradeVerifier {
&self.domain_separator,
)?;

let settlement = add_balance_queries(settlement, query, verification, &solver);
let settlement = add_balance_queries(settlement, query, &verification, &solver);

let settlement = self
.settlement
Expand Down Expand Up @@ -191,11 +193,6 @@ impl TradeVerifier {
..Default::default()
};

let overrides = self
.prepare_state_overrides(verification, query, trade)
.await
.map_err(Error::SimulationFailed)?;

let output = self
.simulator
.simulate(call, overrides, Some(block.number))
Expand Down Expand Up @@ -287,17 +284,60 @@ impl TradeVerifier {
/// trade.
async fn prepare_state_overrides(
&self,
verification: &Verification,
verification: &mut Verification,
query: &PriceQuery,
trade: &TradeKind,
) -> Result<HashMap<H160, StateOverride>> {
let mut overrides = HashMap::with_capacity(6);

// Provide mocked balances if possible to the spardose to allow it to
// give some balances to the trader in order to verify trades even for
// owners without balances. Note that we use a separate account for
// funding to not interfere with the settlement process. This allows the
// simulation to conditionally transfer the balance only when it is
// safe to mock the trade pre-conditions on behalf of the user and to
// not alter solver balances which may be used during settlement. We use
// a similar strategy for determining whether or not to set approvals on
// behalf of the trader.
if let Some(solver_balance_override) = self
.balance_overrides
.state_override(BalanceOverrideRequest {
token: query.sell_token,
holder: Self::SPARDOSE,
amount: match query.kind {
OrderKind::Sell => query.in_amount.get(),
OrderKind::Buy => trade.out_amount(
&query.buy_token,
&query.sell_token,
&query.in_amount.get(),
&query.kind,
)?,
},
})
.await
{
tracing::trace!(?solver_balance_override, "solver balance override enabled");
overrides.insert(query.sell_token, solver_balance_override);

if verification.from.is_zero() {
verification.from = H160::random();
tracing::debug!(
trader = ?verification.from,
"use random trader address with fake balances"
);
}
} else if verification.from.is_zero() {
anyhow::bail!("trader is zero address and balances can not be faked");
}

// Set up mocked trader.
let mut overrides = hashmap! {
verification.from => StateOverride {
overrides.insert(
verification.from,
StateOverride {
code: Some(deployed_bytecode!(Trader)),
..Default::default()
},
};
);

// If the trader is a smart contract we also need to store its implementation
// to proxy into it during the simulation.
Expand Down Expand Up @@ -328,36 +368,6 @@ impl TradeVerifier {
},
);

// Provide mocked balances if possible to the spardose to allow it to
// give some balances to the trader in order to verify trades even for
// owners without balances. Note that we use a separate account for
// funding to not interfere with the settlement process. This allows the
// simulation to conditionally transfer the balance only when it is
// safe to mock the trade pre-conditions on behalf of the user and to
// not alter solver balances which may be used during settlement. We use
// a similar strategy for determining whether or not to set approvals on
// behalf of the trader.
if let Some(solver_balance_override) = self
.balance_overrides
.state_override(BalanceOverrideRequest {
token: query.sell_token,
holder: Self::SPARDOSE,
amount: match query.kind {
OrderKind::Sell => query.in_amount.get(),
OrderKind::Buy => trade.out_amount(
&query.buy_token,
&query.sell_token,
&query.in_amount.get(),
&query.kind,
)?,
},
})
.await
{
tracing::trace!(?solver_balance_override, "solver balance override enabled");
overrides.insert(query.sell_token, solver_balance_override);
}

// Set up mocked solver.
let mut solver_override = StateOverride {
code: Some(deployed_bytecode!(Solver)),
Expand Down Expand Up @@ -408,7 +418,7 @@ impl TradeVerifying for TradeVerifier {
)
.context("failed to compute trade out amount")?;
match self
.verify_inner(query, verification, &trade, &out_amount)
.verify_inner(query, verification.clone(), &trade, &out_amount)
.await
{
Ok(verified) => Ok(verified),
Expand Down Expand Up @@ -790,7 +800,7 @@ enum Error {

#[cfg(test)]
mod tests {
use {super::*, std::str::FromStr};
use {super::*, maplit::hashmap, std::str::FromStr};

#[test]
fn discards_inaccurate_quotes() {
Expand Down

0 comments on commit 78a1b81

Please sign in to comment.