From d8b01fcc7848a994d6e4a35af5f0b08da43b4f7e Mon Sep 17 00:00:00 2001 From: Felix Leupold Date: Fri, 10 May 2024 16:52:09 +0200 Subject: [PATCH] Remove boundary encoding for quotes (#2704) # Description I noticed that we are also using boundary encoding when generating quotes. This should also be done in the domain # Changes - [x] Expose methods to encode approvals and protocol provided liquidity - [x] Replicate boundary quote encoding logic in domain - [x] Add new error type in case quote encoding fails ## How to test Existing unit tests. --- crates/driver/src/boundary/mod.rs | 1 - crates/driver/src/boundary/quote.rs | 74 ------------------- .../domain/competition/solution/encoding.rs | 4 +- crates/driver/src/domain/eth/allowance.rs | 8 ++ crates/driver/src/domain/quote.rs | 73 +++++++++++++++++- crates/driver/src/infra/api/error.rs | 1 + crates/driver/src/infra/observe/mod.rs | 1 + 7 files changed, 83 insertions(+), 79 deletions(-) delete mode 100644 crates/driver/src/boundary/quote.rs diff --git a/crates/driver/src/boundary/mod.rs b/crates/driver/src/boundary/mod.rs index 8994b09428..db03539d4f 100644 --- a/crates/driver/src/boundary/mod.rs +++ b/crates/driver/src/boundary/mod.rs @@ -23,7 +23,6 @@ //! Software (2014) pub mod liquidity; -pub mod quote; pub mod settlement; // The [`anyhow::Error`] type is re-exported because the legacy code mostly diff --git a/crates/driver/src/boundary/quote.rs b/crates/driver/src/boundary/quote.rs deleted file mode 100644 index 40b7578396..0000000000 --- a/crates/driver/src/boundary/quote.rs +++ /dev/null @@ -1,74 +0,0 @@ -use { - crate::{ - boundary, - domain::{competition::solution, eth}, - infra::blockchain::Ethereum, - }, - shared::{external_prices::ExternalPrices, http_solver::model::InternalizationStrategy}, - solver::{interactions::Erc20ApproveInteraction, liquidity::slippage::SlippageCalculator}, - std::sync::Arc, -}; - -const DEFAULT_QUOTE_SLIPPAGE_BPS: u32 = 100; // 1% - -pub fn encode_interactions( - eth: &Ethereum, - interactions: &[solution::Interaction], -) -> Result, boundary::Error> { - let slippage_calculator = SlippageCalculator::from_bps(DEFAULT_QUOTE_SLIPPAGE_BPS, None); - let external_prices = ExternalPrices::new(eth.contracts().weth().address(), Default::default()) - .expect("empty external prices is valid"); - let slippage_context = slippage_calculator.context(&external_prices); - - let mut settlement = solver::settlement::Settlement::new(Default::default()); - - for interaction in interactions { - let allowances = interaction.allowances(); - for allowance in allowances { - // When encoding approvals for quotes, reset the allowance instead - // of just setting it. This is required as some tokens only allow - // you to approve a non-0 value if the allowance was 0 to begin - // with, such as Tether USD. - // - // Alternatively, we could check existing allowances and only encode - // the approvals if needed, but this would only result in small gas - // optimizations which is mostly inconsequential for quotes and not - // worth the performance hit. - settlement - .encoder - .append_to_execution_plan(Arc::new(Erc20ApproveInteraction { - token: eth.contract_at(allowance.0.token.into()), - spender: allowance.0.spender.into(), - amount: eth::U256::zero(), - })); - settlement - .encoder - .append_to_execution_plan(Arc::new(Erc20ApproveInteraction { - token: eth.contract_at(allowance.0.token.into()), - spender: allowance.0.spender.into(), - amount: eth::U256::max_value(), - })); - } - - let boundary_interaction = boundary::settlement::to_boundary_interaction( - &slippage_context, - eth.contracts().settlement().address().into(), - interaction, - )?; - settlement - .encoder - .append_to_execution_plan_internalizable(Arc::new(boundary_interaction), false); - } - - let encoded_settlement = settlement.encode(InternalizationStrategy::EncodeAllInteractions); - Ok(encoded_settlement - .interactions - .into_iter() - .flatten() - .map(|(target, value, call_data)| eth::Interaction { - target: target.into(), - value: value.into(), - call_data: call_data.0.into(), - }) - .collect()) -} diff --git a/crates/driver/src/domain/competition/solution/encoding.rs b/crates/driver/src/domain/competition/solution/encoding.rs index 2d89f745f1..c27255a45b 100644 --- a/crates/driver/src/domain/competition/solution/encoding.rs +++ b/crates/driver/src/domain/competition/solution/encoding.rs @@ -230,7 +230,7 @@ pub fn tx( }) } -fn liquidity_interaction( +pub fn liquidity_interaction( liquidity: &Liquidity, slippage: &slippage::Parameters, settlement: &contracts::GPv2Settlement, @@ -261,7 +261,7 @@ fn liquidity_interaction( .ok_or(Error::InvalidInteractionExecution(liquidity.clone())) } -fn approve(allowance: &Allowance) -> eth::Interaction { +pub fn approve(allowance: &Allowance) -> eth::Interaction { let mut amount = [0u8; 32]; let selector = hex_literal::hex!("095ea7b3"); allowance.amount.to_big_endian(&mut amount); diff --git a/crates/driver/src/domain/eth/allowance.rs b/crates/driver/src/domain/eth/allowance.rs index 51447e930a..58761cb01f 100644 --- a/crates/driver/src/domain/eth/allowance.rs +++ b/crates/driver/src/domain/eth/allowance.rs @@ -60,4 +60,12 @@ impl Approval { ..self.0 }) } + + /// Revoke the approval, i.e. set the approved amount to [`U256::zero`]. + pub fn revoke(self) -> Self { + Self(Allowance { + amount: U256::zero(), + ..self.0 + }) + } } diff --git a/crates/driver/src/domain/quote.rs b/crates/driver/src/domain/quote.rs index 4569e88d9a..cfe72b7815 100644 --- a/crates/driver/src/domain/quote.rs +++ b/crates/driver/src/domain/quote.rs @@ -1,5 +1,5 @@ use { - super::competition::auction, + super::competition::{auction, solution}, crate::{ boundary, domain::{ @@ -58,7 +58,14 @@ impl Quote { Ok(Self { amount: eth::U256::from_big_rational(&amount)?, - interactions: boundary::quote::encode_interactions(eth, solution.interactions())?, + interactions: solution + .interactions() + .iter() + .map(|i| encode::interaction(i, eth.contracts().settlement())) + .collect::, _>>()? + .into_iter() + .flatten() + .collect(), solver: solution.solver().address(), gas: solution.gas(), tx_origin: *solution.solver().quote_tx_origin(), @@ -258,6 +265,66 @@ impl Tokens { } } +mod encode { + use { + crate::domain::{ + competition::solution, + eth::{ + self, + allowance::{Approval, Required}, + }, + }, + num::rational::Ratio, + }; + + const DEFAULT_QUOTE_SLIPPAGE_BPS: u32 = 100; + + pub(super) fn interaction( + interaction: &solution::Interaction, + settlement: &contracts::GPv2Settlement, + ) -> Result, solution::encoding::Error> { + let slippage = solution::slippage::Parameters { + relative: Ratio::new_raw(DEFAULT_QUOTE_SLIPPAGE_BPS.into(), 10_000.into()), + max: None, + min: None, + prices: Default::default(), + }; + + let encoded = match interaction { + solution::Interaction::Custom(interaction) => eth::Interaction { + value: interaction.value, + target: interaction.target.0.into(), + call_data: interaction.call_data.clone(), + }, + solution::Interaction::Liquidity(liquidity) => { + solution::encoding::liquidity_interaction(liquidity, &slippage, settlement)? + } + }; + + Ok(interaction + .allowances() + .iter() + .flat_map(|Required(allowance)| { + let approval = Approval(*allowance); + // When encoding approvals for quotes, reset the allowance instead + // of just setting it. This is required as some tokens only allow + // you to approve a non-0 value if the allowance was 0 to begin + // with, such as Tether USD. + // + // Alternatively, we could check existing allowances and only encode + // the approvals if needed, but this would only result in small gas + // optimizations which is mostly inconsequential for quotes and not + // worth the performance hit. + vec![ + solution::encoding::approve(&approval.revoke().0), + solution::encoding::approve(&approval.max().0), + ] + }) + .chain(std::iter::once(encoded)) + .collect()) + } +} + #[derive(Debug, thiserror::Error)] pub enum Error { /// This can happen e.g. if there's no available liquidity for the tokens @@ -273,6 +340,8 @@ pub enum Error { Solver(#[from] solver::Error), #[error("boundary error: {0:?}")] Boundary(#[from] boundary::Error), + #[error("encoding error: {0:?}")] + Encoding(#[from] solution::encoding::Error), } #[derive(Debug, thiserror::Error)] diff --git a/crates/driver/src/infra/api/error.rs b/crates/driver/src/infra/api/error.rs index ccc62d8f7e..ef204550c1 100644 --- a/crates/driver/src/infra/api/error.rs +++ b/crates/driver/src/infra/api/error.rs @@ -70,6 +70,7 @@ impl From for (hyper::StatusCode, axum::Json) { quote::Error::Solver(_) => Kind::SolverFailed, quote::Error::Blockchain(_) => Kind::Unknown, quote::Error::Boundary(_) => Kind::Unknown, + quote::Error::Encoding(_) => Kind::Unknown, }; error.into() } diff --git a/crates/driver/src/infra/observe/mod.rs b/crates/driver/src/infra/observe/mod.rs index 5adf7136ff..9b8dba3f62 100644 --- a/crates/driver/src/infra/observe/mod.rs +++ b/crates/driver/src/infra/observe/mod.rs @@ -271,6 +271,7 @@ pub fn quoted(solver: &solver::Name, order: "e::Order, result: &Result "SolverDtoError", quote::Error::Boundary(_) => "Unknown", + quote::Error::Encoding(_) => "Encoding", }, ]) .inc();