From 3a69bbf6035df123a076f1f91011300e1c672527 Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Wed, 29 Jan 2025 17:29:36 +0000 Subject: [PATCH 01/12] feat: Remove generalisation on user approvals manager This is a too early over generalisation. We need to encode the permit and the signature all together with the other method input variables. This way the return type will be specific to permit2 (instead Vec) anyway. For simplicity, decided to remove the generalisation and keep things simple --- don't change below this line --- ENG-4081 Took 2 hours 23 minutes Took 9 seconds Took 1 minute --- src/encoding/evm/approvals/permit2.rs | 117 ++++++++++--------------- src/encoding/mod.rs | 1 - src/encoding/router_encoder.rs | 3 +- src/encoding/user_approvals_manager.rs | 17 ---- 4 files changed, 46 insertions(+), 92 deletions(-) delete mode 100644 src/encoding/user_approvals_manager.rs diff --git a/src/encoding/evm/approvals/permit2.rs b/src/encoding/evm/approvals/permit2.rs index 392e0d2..ac673ca 100644 --- a/src/encoding/evm/approvals/permit2.rs +++ b/src/encoding/evm/approvals/permit2.rs @@ -7,9 +7,10 @@ use alloy::{ signers::{local::PrivateKeySigner, SignerSync}, transports::BoxTransport, }; -use alloy_primitives::{ChainId, U256}; +use alloy_primitives::{ChainId, Signature, U256}; use alloy_sol_types::{eip712_domain, sol, SolStruct, SolValue}; use chrono::Utc; +use num_bigint::BigUint; use tokio::runtime::Runtime; use tycho_core::Bytes; @@ -19,7 +20,6 @@ use crate::encoding::{ approvals::protocol_approvals_manager::get_client, utils::{biguint_to_u256, bytes_to_address, encode_input}, }, - user_approvals_manager::{Approval, UserApprovalsManager}, }; /// Struct for managing Permit2 operations, including encoding approvals and fetching allowance @@ -107,59 +107,48 @@ impl Permit2 { ))), } } -} -impl UserApprovalsManager for Permit2 { - /// Encodes multiple approvals into ABI-encoded data and signs them. - fn encode_approvals(&self, approvals: Vec) -> Result>, EncodingError> { + /// Creates permit single and signature + pub fn get_permit( + &self, + spender: &Bytes, + owner: &Bytes, + token: &Bytes, + amount: &BigUint, + ) -> Result<(PermitSingle, Signature), EncodingError> { let current_time = Utc::now() .naive_utc() .and_utc() .timestamp() as u64; - let mut encoded_approvals = Vec::new(); + let (_, _, nonce) = self.get_existing_allowance(owner, spender, token)?; + let expiration = U48::from(current_time + PERMIT_EXPIRATION); + let sig_deadline = U256::from(current_time + PERMIT_SIG_EXPIRATION); + let amount = U160::from(biguint_to_u256(amount)); - for approval in approvals { - let (_, _, nonce) = - self.get_existing_allowance(&approval.owner, &approval.spender, &approval.token)?; - let expiration = U48::from(current_time + PERMIT_EXPIRATION); - let sig_deadline = U256::from(current_time + PERMIT_SIG_EXPIRATION); - let amount = U160::from(biguint_to_u256(&approval.amount)); + let details = PermitDetails { token: bytes_to_address(token)?, amount, expiration, nonce }; - let details = PermitDetails { - token: bytes_to_address(&approval.token)?, - amount, - expiration, - nonce, - }; + let permit_single = PermitSingle { + details, + spender: bytes_to_address(spender)?, + sigDeadline: sig_deadline, + }; - let permit_single = PermitSingle { - details, - spender: bytes_to_address(&approval.spender)?, - sigDeadline: sig_deadline, - }; - - let domain = eip712_domain! { - name: "Permit2", - chain_id: self.chain_id, - verifying_contract: self.address, - }; - let hash = permit_single.eip712_signing_hash(&domain); - let signature = self - .signer - .sign_hash_sync(&hash) - .map_err(|e| { - EncodingError::FatalError(format!( - "Failed to sign permit2 approval with error: {}", - e - )) - })?; - let encoded = - (bytes_to_address(&approval.owner)?, permit_single, signature.as_bytes().to_vec()) - .abi_encode(); - encoded_approvals.push(encoded); - } - - Ok(encoded_approvals) + let domain = eip712_domain! { + name: "Permit2", + chain_id: self.chain_id, + verifying_contract: self.address, + }; + let hash = permit_single.eip712_signing_hash(&domain); + let signature = self + .signer + .sign_hash_sync(&hash) + .map_err(|e| { + EncodingError::FatalError(format!( + "Failed to sign permit2 approval with error: {}", + e + )) + })?; + Ok((permit_single, signature)) } } @@ -222,7 +211,7 @@ mod tests { } #[test] - fn test_encode_approvals() { + fn test_get_permit() { // Set up a mock private key for signing let private_key = B256::from_str("4c0883a69102937d6231471b5dbb6204fe512961708279feb1be6ae5538da033") @@ -234,21 +223,10 @@ mod tests { let spender = Bytes::from_str("0xba12222222228d8ba445958a75a0704d566bf2c8").unwrap(); let token = Bytes::from_str("0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48").unwrap(); let amount = BigUint::from(1000u64); - let approvals = - vec![Approval { owner, spender, token: token.clone(), amount: amount.clone() }]; - let encoded_approvals = permit2 - .encode_approvals(approvals) + let (permit, _) = permit2 + .get_permit(&spender, &owner, &token, &amount) .unwrap(); - assert_eq!(encoded_approvals.len(), 1, "Expected 1 encoded approval"); - - let encoded = &encoded_approvals[0]; - - // Remove prefix and owner (first 64 bytes) and signature (last 65 bytes) - let permit_single_encoded = &encoded[64..encoded.len() - 65]; - - let decoded_permit_single = PermitSingle::abi_decode(permit_single_encoded, false) - .expect("Failed to decode PermitSingle"); let expected_details = PermitDetails { token: bytes_to_address(&token).unwrap(), @@ -263,7 +241,7 @@ mod tests { }; assert_eq!( - decoded_permit_single, expected_permit_single, + permit, expected_permit_single, "Decoded PermitSingle does not match expected values" ); } @@ -309,18 +287,13 @@ mod tests { assert!(receipt.status(), "Approve transaction failed"); let spender = Bytes::from_str("0xba12222222228d8ba445958a75a0704d566bf2c8").unwrap(); - let approvals = vec![Approval { - owner: anvil_account.clone(), - spender: spender.clone(), - token: token.clone(), - amount: amount.clone(), - }]; - let encoded_approvals = permit2 - .encode_approvals(approvals) + let (permit, signature) = permit2 + .get_permit(&spender, &anvil_account, &token, &amount) .unwrap(); - - let encoded = &encoded_approvals[0]; + let encoded = + (bytes_to_address(&anvil_account).unwrap(), permit, signature.as_bytes().to_vec()) + .abi_encode(); let function_signature = "permit(address,((address,uint160,uint48,uint48),address,uint256),bytes)"; diff --git a/src/encoding/mod.rs b/src/encoding/mod.rs index 495a30a..ef2afaa 100644 --- a/src/encoding/mod.rs +++ b/src/encoding/mod.rs @@ -5,4 +5,3 @@ mod models; mod router_encoder; mod strategy_encoder; mod swap_encoder; -mod user_approvals_manager; diff --git a/src/encoding/router_encoder.rs b/src/encoding/router_encoder.rs index be6d1d5..8136af6 100644 --- a/src/encoding/router_encoder.rs +++ b/src/encoding/router_encoder.rs @@ -2,11 +2,10 @@ use crate::encoding::{ errors::EncodingError, models::{Solution, Transaction}, strategy_encoder::StrategySelector, - user_approvals_manager::UserApprovalsManager, }; #[allow(dead_code)] -pub trait RouterEncoder { +pub trait RouterEncoder { fn encode_router_calldata( &self, solutions: Vec, diff --git a/src/encoding/user_approvals_manager.rs b/src/encoding/user_approvals_manager.rs deleted file mode 100644 index 773e048..0000000 --- a/src/encoding/user_approvals_manager.rs +++ /dev/null @@ -1,17 +0,0 @@ -use num_bigint::BigUint; -use tycho_core::Bytes; - -use crate::encoding::errors::EncodingError; - -#[allow(dead_code)] -pub struct Approval { - pub spender: Bytes, - pub owner: Bytes, - pub token: Bytes, - pub amount: BigUint, -} - -pub trait UserApprovalsManager { - #[allow(dead_code)] - fn encode_approvals(&self, approvals: Vec) -> Result>, EncodingError>; -} From feb91cc639aaf9e5056662158f2cbbbb61f9021e Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Thu, 30 Jan 2025 11:22:30 +0000 Subject: [PATCH 02/12] feat: Implement SplitSwapStrategyEncoder The strategy works as follows: - Manage approvals needed - Compute min amount (if check amount is any): - if slippage is defined, apply slippage on the expected amount and take the min value between that and the check amount - if not, it's just the check amount - Iterate through the swaps - call the corresponding swap encoder to encode the swap - add swap header (tokens indexes and split) - ple encode the swaps - Add extra inputs (amounts, token addresses, min amount, (un)wrap, number of tokens and receiver) Misc: - Move executor address and selector encoding inside the SwapEncoder - Add default executor_selector to SwapEncoder - Pass router address inside the SplitSwapStrategyEncoder - Move Permit2 inside the SplitSwapStrategyEncoder. It is a responsibility and a specificity of the strategy to need permit2 approvals --- don't change below this line --- ENG-4081 Took 1 hour 21 minutes --- src/encoding/evm/approvals/permit2.rs | 6 +- src/encoding/evm/router_encoder.rs | 58 ++-- src/encoding/evm/strategy_encoder/encoder.rs | 257 +++++++++++++++++- src/encoding/evm/strategy_encoder/selector.rs | 20 +- src/encoding/evm/swap_encoder/encoders.rs | 30 +- src/encoding/evm/utils.rs | 13 +- src/encoding/models.rs | 10 +- src/encoding/router_encoder.rs | 1 - src/encoding/strategy_encoder.rs | 17 +- src/encoding/swap_encoder.rs | 8 + 10 files changed, 355 insertions(+), 65 deletions(-) diff --git a/src/encoding/evm/approvals/permit2.rs b/src/encoding/evm/approvals/permit2.rs index ac673ca..c9b687c 100644 --- a/src/encoding/evm/approvals/permit2.rs +++ b/src/encoding/evm/approvals/permit2.rs @@ -1,13 +1,14 @@ use std::{str::FromStr, sync::Arc}; use alloy::{ - primitives::{aliases::U48, Address, Bytes as AlloyBytes, TxKind, U160}, + primitives::{aliases::U48, Address, Bytes as AlloyBytes, ChainId, TxKind, U160, U256}, providers::{Provider, RootProvider}, rpc::types::{TransactionInput, TransactionRequest}, signers::{local::PrivateKeySigner, SignerSync}, transports::BoxTransport, }; -use alloy_primitives::{ChainId, Signature, U256}; +#[allow(deprecated)] +use alloy_primitives::Signature; use alloy_sol_types::{eip712_domain, sol, SolStruct, SolValue}; use chrono::Utc; use num_bigint::BigUint; @@ -108,6 +109,7 @@ impl Permit2 { } } /// Creates permit single and signature + #[allow(deprecated)] pub fn get_permit( &self, spender: &Bytes, diff --git a/src/encoding/evm/router_encoder.rs b/src/encoding/evm/router_encoder.rs index 53ab437..a3c74d7 100644 --- a/src/encoding/evm/router_encoder.rs +++ b/src/encoding/evm/router_encoder.rs @@ -1,5 +1,7 @@ use std::str::FromStr; +use alloy::signers::local::PrivateKeySigner; +use alloy_primitives::ChainId; use num_bigint::BigUint; use tycho_core::Bytes; @@ -9,37 +11,48 @@ use crate::encoding::{ models::{NativeAction, Solution, Transaction}, router_encoder::RouterEncoder, strategy_encoder::StrategySelector, - user_approvals_manager::{Approval, UserApprovalsManager}, }; #[allow(dead_code)] -pub struct EVMRouterEncoder { +pub struct EVMRouterEncoder { strategy_selector: S, - approvals_manager: A, + signer: Option, + chain_id: ChainId, router_address: String, } #[allow(dead_code)] -impl EVMRouterEncoder { - pub fn new(strategy_selector: S, approvals_manager: A, router_address: String) -> Self { - EVMRouterEncoder { strategy_selector, approvals_manager, router_address } +impl EVMRouterEncoder { + pub fn new( + strategy_selector: S, + router_address: String, + signer: Option, + chain_id: ChainId, + ) -> Result { + Ok(EVMRouterEncoder { strategy_selector, signer, chain_id, router_address }) } } -impl RouterEncoder for EVMRouterEncoder { +impl RouterEncoder for EVMRouterEncoder { fn encode_router_calldata( &self, solutions: Vec, ) -> Result, EncodingError> { - let _approvals_calldata = self.handle_approvals(&solutions)?; // TODO: where should we append this? let mut transactions: Vec = Vec::new(); for solution in solutions.iter() { let exact_out = solution.exact_out; let straight_to_pool = solution.straight_to_pool; - - let strategy = self - .strategy_selector - .select_strategy(solution); - let method_calldata = strategy.encode_strategy((*solution).clone())?; + let router_address = solution + .router_address + .clone() + .unwrap_or(Bytes::from_str(&self.router_address).map_err(|_| { + EncodingError::FatalError("Invalid router address".to_string()) + })?); + let strategy = self.strategy_selector.select_strategy( + solution, + self.signer.clone(), + self.chain_id, + )?; + let method_calldata = strategy.encode_strategy(solution.clone(), router_address)?; let contract_interaction = if straight_to_pool { method_calldata @@ -56,23 +69,4 @@ impl RouterEncoder for EVMRo } Ok(transactions) } - - fn handle_approvals(&self, solutions: &[Solution]) -> Result>, EncodingError> { - let mut approvals = Vec::new(); - for solution in solutions.iter() { - approvals.push(Approval { - token: solution.given_token.clone(), - spender: solution - .router_address - .clone() - .unwrap_or(Bytes::from_str(&self.router_address).map_err(|_| { - EncodingError::FatalError("Invalid router address".to_string()) - })?), - amount: solution.given_amount.clone(), - owner: solution.sender.clone(), - }); - } - self.approvals_manager - .encode_approvals(approvals) - } } diff --git a/src/encoding/evm/strategy_encoder/encoder.rs b/src/encoding/evm/strategy_encoder/encoder.rs index 7068608..4a2d17a 100644 --- a/src/encoding/evm/strategy_encoder/encoder.rs +++ b/src/encoding/evm/strategy_encoder/encoder.rs @@ -1,35 +1,150 @@ -use alloy_primitives::Address; +use std::cmp::min; + +use alloy::signers::local::PrivateKeySigner; +use alloy_primitives::{aliases::U24, map::HashSet, ChainId, U256, U8}; use alloy_sol_types::SolValue; +use num_bigint::BigUint; +use tycho_core::Bytes; use crate::encoding::{ errors::EncodingError, - evm::swap_encoder::SWAP_ENCODER_REGISTRY, - models::{EncodingContext, Solution}, + evm::{ + approvals::permit2::Permit2, + swap_encoder::SWAP_ENCODER_REGISTRY, + utils::{biguint_to_u256, bytes_to_address, percentage_to_uint24, ple_encode}, + }, + models::{EncodingContext, NativeAction, Solution}, strategy_encoder::StrategyEncoder, }; #[allow(dead_code)] pub trait EVMStrategyEncoder: StrategyEncoder { - fn encode_protocol_header( + fn encode_swap_header( &self, + token_in: U8, + token_out: U8, + split: U24, protocol_data: Vec, - executor_address: Address, - // Token indices, split, and token inclusion are only used for split swaps - token_in: u16, - token_out: u16, - split: u16, // not sure what should be the type of this :/ ) -> Vec { - let args = (executor_address, token_in, token_out, split, protocol_data); - args.abi_encode() + let mut encoded = Vec::new(); + encoded.push(token_in.to_be_bytes_vec()[0]); + encoded.push(token_out.to_be_bytes_vec()[0]); + encoded.extend_from_slice(&split.to_be_bytes_vec()); + encoded.extend(protocol_data); + encoded } } -pub struct SplitSwapStrategyEncoder {} +pub struct SplitSwapStrategyEncoder { + permit2: Permit2, +} + +impl SplitSwapStrategyEncoder { + pub fn new(signer: PrivateKeySigner, chain_id: ChainId) -> Result { + Ok(Self { permit2: Permit2::new(signer, chain_id)? }) + } +} impl EVMStrategyEncoder for SplitSwapStrategyEncoder {} impl StrategyEncoder for SplitSwapStrategyEncoder { - fn encode_strategy(&self, _solution: Solution) -> Result, EncodingError> { - todo!() + fn encode_strategy( + &self, + solution: Solution, + router_address: Bytes, + ) -> Result, EncodingError> { + let (permit, signature) = self.permit2.get_permit( + &router_address, + &solution.sender, + &solution.given_token, + &solution.given_amount, + )?; + let min_amount_out = if solution.check_amount.is_some() { + let mut min_amount_out = solution.check_amount.clone().unwrap(); + if solution.slippage.is_some() { + let one_hundred = BigUint::from(100u32); + let slippage_percent = BigUint::from((solution.slippage.unwrap() * 100.0) as u32); + let multiplier = &one_hundred - slippage_percent; + let expected_amount_with_slippage = + (&solution.expected_amount * multiplier) / one_hundred; + min_amount_out = min(min_amount_out, expected_amount_with_slippage); + } + min_amount_out + } else { + BigUint::ZERO + }; + + let tokens: Vec = solution + .swaps + .iter() + .flat_map(|swap| vec![swap.token_in.clone(), swap.token_out.clone()]) + .collect::>() + .into_iter() + .collect(); + + let mut swaps = vec![]; + for swap in solution.swaps.iter() { + let registry = SWAP_ENCODER_REGISTRY.read().unwrap(); + let swap_encoder = registry + .get_encoder(&swap.component.protocol_system) + .expect("Swap encoder not found"); + + let encoding_context = EncodingContext { + receiver: router_address.clone(), + exact_out: solution.exact_out, + router_address: router_address.clone(), + }; + let protocol_data = swap_encoder.encode_swap(swap.clone(), encoding_context)?; + let swap_data = self.encode_swap_header( + U8::from( + tokens + .iter() + .position(|t| *t == swap.token_in) + .ok_or_else(|| { + EncodingError::InvalidInput( + "Token in not found in tokens array".to_string(), + ) + })?, + ), + U8::from( + tokens + .iter() + .position(|t| *t == swap.token_out) + .ok_or_else(|| { + EncodingError::InvalidInput( + "Token out not found in tokens array".to_string(), + ) + })?, + ), + percentage_to_uint24(swap.split), + protocol_data, + ); + swaps.push(swap_data); + } + + let encoded_swaps = ple_encode(swaps); + let (mut unwrap, mut wrap) = (false, false); + if solution.native_action.is_some() { + match solution.native_action.unwrap() { + NativeAction::Wrap => wrap = true, + NativeAction::Unwrap => unwrap = true, + } + } + let method_calldata = ( + biguint_to_u256(&solution.given_amount), + bytes_to_address(&solution.given_token)?, + bytes_to_address(&solution.checked_token)?, + biguint_to_u256(&min_amount_out), + wrap, + unwrap, + U256::from(tokens.len()), + bytes_to_address(&solution.receiver)?, + permit, + signature.as_bytes().to_vec(), + encoded_swaps, + ) + .abi_encode(); + Ok(method_calldata) } + fn selector(&self, _exact_out: bool) -> &str { "swap(uint256, address, uint256, bytes[])" } @@ -40,7 +155,11 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { pub struct StraightToPoolStrategyEncoder {} impl EVMStrategyEncoder for StraightToPoolStrategyEncoder {} impl StrategyEncoder for StraightToPoolStrategyEncoder { - fn encode_strategy(&self, solution: Solution) -> Result, EncodingError> { + fn encode_strategy( + &self, + solution: Solution, + _router_address: Bytes, + ) -> Result, EncodingError> { if solution.router_address.is_none() { return Err(EncodingError::InvalidInput( "Router address is required for straight to pool solutions".to_string(), @@ -75,3 +194,111 @@ impl StrategyEncoder for StraightToPoolStrategyEncoder { unimplemented!(); } } + +#[cfg(test)] +mod tests { + use std::str::FromStr; + + use alloy::hex::encode; + use alloy_primitives::B256; + use tycho_core::dto::ProtocolComponent; + + use super::*; + use crate::encoding::models::Swap; + + #[test] + fn test_split_swap_strategy_encoder() { + // Set up a mock private key for signing + let private_key = + B256::from_str("4c0883a69102937d6231471b5dbb6204fe512961708279feb1be6ae5538da033") + .expect("Invalid private key"); + let signer = PrivateKeySigner::from_bytes(&private_key).expect("Failed to create signer"); + + let weth = Bytes::from_str("0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2").unwrap(); + let dai = Bytes::from_str("0x6b175474e89094c44da98b954eedeac495271d0f").unwrap(); + + let swap = Swap { + component: ProtocolComponent { + id: "0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640".to_string(), + protocol_system: "uniswap_v2".to_string(), + ..Default::default() + }, + token_in: weth.clone(), + token_out: dai.clone(), + split: 0f64, + }; + + let encoder = SplitSwapStrategyEncoder::new(signer, 1).unwrap(); + let solution = Solution { + exact_out: false, + given_token: weth, + given_amount: BigUint::from_str("1_000000000000000000").unwrap(), + checked_token: dai, + expected_amount: BigUint::from_str("3_000_000000000000000000").unwrap(), + check_amount: None, + sender: Bytes::from_str("0x2c6A3cd97c6283b95Ac8C5A4459eBB0d5Fd404F4").unwrap(), + receiver: Bytes::from_str("0x2c6A3cd97c6283b95Ac8C5A4459eBB0d5Fd404F4").unwrap(), + swaps: vec![swap], + ..Default::default() + }; + let router_address = Bytes::from_str("0x2c6A3cd97c6283b95Ac8C5A4459eBB0d5Fd404F4").unwrap(); + + let calldata = encoder + .encode_strategy(solution, router_address) + .unwrap(); + + let expected_input = String::from(concat!( + "0000000000000000000000000000000000000000000000000000000000000020", // offset + "0000000000000000000000000000000000000000000000000de0b6b3a7640000", // amount out + "000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", // token in + "0000000000000000000000006b175474e89094c44da98b954eedeac495271d0f", // token out + "0000000000000000000000000000000000000000000000000000000000000000", // min amount out + "0000000000000000000000000000000000000000000000000000000000000000", // wrap + "0000000000000000000000000000000000000000000000000000000000000000", // unwrap + "0000000000000000000000000000000000000000000000000000000000000002", // tokens length + "0000000000000000000000002c6a3cd97c6283b95ac8c5a4459ebb0d5fd404f4", // receiver + )); + // after this there is the permit and because of the deadlines (that depend on block time) + // it's hard to assert + // "000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", // token in + // "0000000000000000000000000000000000000000000000000de0b6b3a7640000", // amount in + // "0000000000000000000000000000000000000000000000000000000067c205fe", // expiration + // "0000000000000000000000000000000000000000000000000000000000000000", // nonce + // "0000000000000000000000002c6a3cd97c6283b95ac8c5a4459ebb0d5fd404f4", // spender + // "00000000000000000000000000000000000000000000000000000000679a8006", // deadline + // offsets??? + // "0000000000000000000000000000000000000000000000000000000000000200", + // "0000000000000000000000000000000000000000000000000000000000000280", + // "0000000000000000000000000000000000000000000000000000000000000041", + // signature + // "fc5bac4e27cd5d71c85d232d8c6b31ea924d2e0031091ff9a39579d9e49c214328ea34876961d9200af691373c71a174e166793d02241c76adb93c5f87fe0f381c", + + let expected_swaps = String::from(concat!( + // ple encode adds aaalll of this :/ is it correct? + "0000000000000000000000000000000000000000000000000000000000000000", + "0000000000000000000000000000000000000000000000000000000000120000", + "0000000000000000000000000000000000000000000000000000000000020000", + "0000000000000000000000000000000000000000000000000000000000060000", + "000000000000000000000000000000000000000000000000000000000005b000", + "0000000000000000000000000000000000000000000000000000000000080000", + "0000000000000000000000000000000000000000000000000000000000000000", + "000000000000000000000000000000000000000000000000000000000005b", + // Swap header + "00", // token in index + "01", // token out index + "000000", // split + // Swap data + "5c2f5a71f67c01775180adc06909288b4c329308", // executor address + "bd0625ab", // selector + "c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", // token in + "88e6a0c2ddd26feeb64f039a2c41296fcb3f5640", // component id + "2c6a3cd97c6283b95ac8c5a4459ebb0d5fd404f4", // receiver + "00", // zero2one + "00", // exact out + "0000000000", // padding + )); + let hex_calldata = encode(&calldata); + assert_eq!(hex_calldata[..576], expected_input); + assert_eq!(hex_calldata[1283..], expected_swaps); + } +} diff --git a/src/encoding/evm/strategy_encoder/selector.rs b/src/encoding/evm/strategy_encoder/selector.rs index 37e397c..7301552 100644 --- a/src/encoding/evm/strategy_encoder/selector.rs +++ b/src/encoding/evm/strategy_encoder/selector.rs @@ -1,4 +1,8 @@ +use alloy::signers::local::PrivateKeySigner; +use alloy_primitives::ChainId; + use crate::encoding::{ + errors::EncodingError, evm::strategy_encoder::encoder::{SplitSwapStrategyEncoder, StraightToPoolStrategyEncoder}, models::Solution, strategy_encoder::{StrategyEncoder, StrategySelector}, @@ -7,11 +11,21 @@ use crate::encoding::{ pub struct EVMStrategySelector; impl StrategySelector for EVMStrategySelector { - fn select_strategy(&self, solution: &Solution) -> Box { + fn select_strategy( + &self, + solution: &Solution, + signer: Option, + chain_id: ChainId, + ) -> Result, EncodingError> { if solution.straight_to_pool { - Box::new(StraightToPoolStrategyEncoder {}) + Ok(Box::new(StraightToPoolStrategyEncoder {})) } else { - Box::new(SplitSwapStrategyEncoder {}) + let signer = signer.ok_or_else(|| { + EncodingError::FatalError( + "Signer is required for SplitSwapStrategyEncoder".to_string(), + ) + })?; + Ok(Box::new(SplitSwapStrategyEncoder::new(signer, chain_id).unwrap())) } } } diff --git a/src/encoding/evm/swap_encoder/encoders.rs b/src/encoding/evm/swap_encoder/encoders.rs index de13cfc..b0ae977 100644 --- a/src/encoding/evm/swap_encoder/encoders.rs +++ b/src/encoding/evm/swap_encoder/encoders.rs @@ -42,6 +42,9 @@ impl SwapEncoder for UniswapV2SwapEncoder { // Token in address is always needed to perform a manual transfer from the router, // since no optimizations are performed that send from one pool to the next let args = ( + Address::from_str(self.executor_address()) + .map_err(|_| EncodingError::FatalError("Invalid executor address".to_string()))?, + self.executor_selector(), token_in_address, component_id, bytes_to_address(&encoding_context.receiver)?, @@ -107,6 +110,9 @@ impl SwapEncoder for UniswapV3SwapEncoder { })?; let args = ( + Address::from_str(self.executor_address()) + .map_err(|_| EncodingError::FatalError("Invalid executor address".to_string()))?, + self.executor_selector(), token_in_address, token_out_address, pool_fee_u24, @@ -155,6 +161,9 @@ impl SwapEncoder for BalancerV2SwapEncoder { .map_err(|_| EncodingError::FatalError("Invalid component ID".to_string()))?; let args = ( + Address::from_str(self.executor_address()) + .map_err(|_| EncodingError::FatalError("Invalid executor address".to_string()))?, + self.executor_selector(), bytes_to_address(&swap.token_in)?, bytes_to_address(&swap.token_out)?, component_id, @@ -196,7 +205,8 @@ mod tests { exact_out: false, router_address: Bytes::zero(20), }; - let encoder = UniswapV2SwapEncoder::new(String::from("0x")); + let encoder = + UniswapV2SwapEncoder::new(String::from("0x543778987b293C7E8Cf0722BB2e935ba6f4068D4")); let encoded_swap = encoder .encode_swap(swap, encoding_context) .unwrap(); @@ -204,6 +214,10 @@ mod tests { assert_eq!( hex_swap, String::from(concat!( + // executor address + "543778987b293c7e8cf0722bb2e935ba6f4068d4", + // executor selector + "bd0625ab", // in token "c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", // component id @@ -239,7 +253,8 @@ mod tests { exact_out: false, router_address: Bytes::zero(20), }; - let encoder = UniswapV3SwapEncoder::new(String::from("0x")); + let encoder = + UniswapV3SwapEncoder::new(String::from("0x543778987b293C7E8Cf0722BB2e935ba6f4068D4")); let encoded_swap = encoder .encode_swap(swap, encoding_context) .unwrap(); @@ -247,6 +262,10 @@ mod tests { assert_eq!( hex_swap, String::from(concat!( + // executor address + "543778987b293c7e8cf0722bb2e935ba6f4068d4", + // executor selector + "bd0625ab", // in token "c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", // out token @@ -283,7 +302,8 @@ mod tests { exact_out: false, router_address: Bytes::zero(20), }; - let encoder = BalancerV2SwapEncoder::new(String::from("0x")); + let encoder = + BalancerV2SwapEncoder::new(String::from("0x543778987b293C7E8Cf0722BB2e935ba6f4068D4")); let encoded_swap = encoder .encode_swap(swap, encoding_context) .unwrap(); @@ -292,6 +312,10 @@ mod tests { assert_eq!( hex_swap, String::from(concat!( + // executor address + "543778987b293c7e8cf0722bb2e935ba6f4068d4", + // executor selector + "bd0625ab", // token in "c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", // token out diff --git a/src/encoding/evm/utils.rs b/src/encoding/evm/utils.rs index c1b7d56..0afe763 100644 --- a/src/encoding/evm/utils.rs +++ b/src/encoding/evm/utils.rs @@ -1,4 +1,4 @@ -use alloy_primitives::{Address, Keccak256, U256}; +use alloy_primitives::{aliases::U24, Address, Keccak256, U256}; use alloy_sol_types::SolValue; use num_bigint::BigUint; use tycho_core::Bytes; @@ -13,7 +13,7 @@ pub fn bytes_to_address(address: &Bytes) -> Result { if address.len() == 20 { Ok(Address::from_slice(address)) } else { - Err(EncodingError::InvalidInput(format!("Invalid ERC20 token address: {:?}", address))) + Err(EncodingError::InvalidInput(format!("Invalid address: {:?}", address))) } } @@ -56,3 +56,12 @@ pub fn encode_input(selector: &str, mut encoded_args: Vec) -> Vec { call_data.extend(encoded_args); call_data } + +/// Converts a percentage to a `U24` value. The percentage is a `f64` value between 0 and 100. +/// MAX_UINT24 corresponds to 100%. +pub fn percentage_to_uint24(percentage: f64) -> U24 { + const MAX_UINT24: u32 = 16_777_215; // 2^24 - 1 + + let scaled = (percentage / 100.0) * (MAX_UINT24 as f64); + U24::from(scaled.round()) +} diff --git a/src/encoding/models.rs b/src/encoding/models.rs index 4e2cbbd..0bd14bd 100644 --- a/src/encoding/models.rs +++ b/src/encoding/models.rs @@ -1,7 +1,7 @@ use num_bigint::BigUint; use tycho_core::{dto::ProtocolComponent, Bytes}; -#[derive(Clone)] +#[derive(Clone, Default, Debug)] #[allow(dead_code)] pub struct Solution { /// True if the solution is an exact output solution. @@ -11,7 +11,7 @@ pub struct Solution { /// Amount of the given token. pub given_amount: BigUint, /// The token being bought (exact in) or sold (exact out). - checked_token: Bytes, + pub checked_token: Bytes, /// Expected amount of the bought token (exact in) or sold token (exact out). pub expected_amount: BigUint, /// Minimum amount to be checked for the solution to be valid. @@ -35,14 +35,14 @@ pub struct Solution { pub native_action: Option, } -#[derive(Clone, PartialEq)] +#[derive(Clone, PartialEq, Debug)] #[allow(dead_code)] pub enum NativeAction { Wrap, Unwrap, } -#[derive(Clone)] +#[derive(Clone, Debug)] #[allow(dead_code)] pub struct Swap { /// Protocol component from tycho indexer @@ -51,7 +51,7 @@ pub struct Swap { pub token_in: Bytes, /// Token being output from the pool. pub token_out: Bytes, - /// Fraction of the amount to be swapped in this operation. + /// Percentage of the amount to be swapped in this operation. pub split: f64, } diff --git a/src/encoding/router_encoder.rs b/src/encoding/router_encoder.rs index 8136af6..e3af9bb 100644 --- a/src/encoding/router_encoder.rs +++ b/src/encoding/router_encoder.rs @@ -10,5 +10,4 @@ pub trait RouterEncoder { &self, solutions: Vec, ) -> Result, EncodingError>; - fn handle_approvals(&self, solutions: &[Solution]) -> Result>, EncodingError>; } diff --git a/src/encoding/strategy_encoder.rs b/src/encoding/strategy_encoder.rs index 2b4e348..1204e6b 100644 --- a/src/encoding/strategy_encoder.rs +++ b/src/encoding/strategy_encoder.rs @@ -1,12 +1,25 @@ +use alloy::signers::local::PrivateKeySigner; +use alloy_primitives::ChainId; +use tycho_core::Bytes; + use crate::encoding::{errors::EncodingError, models::Solution}; #[allow(dead_code)] pub trait StrategyEncoder { - fn encode_strategy(&self, to_encode: Solution) -> Result, EncodingError>; + fn encode_strategy( + &self, + to_encode: Solution, + router_address: Bytes, + ) -> Result, EncodingError>; fn selector(&self, exact_out: bool) -> &str; } pub trait StrategySelector { #[allow(dead_code)] - fn select_strategy(&self, solution: &Solution) -> Box; + fn select_strategy( + &self, + solution: &Solution, + signer: Option, + chain_id: ChainId, + ) -> Result, EncodingError>; } diff --git a/src/encoding/swap_encoder.rs b/src/encoding/swap_encoder.rs index 1a1fc0c..65e94c3 100644 --- a/src/encoding/swap_encoder.rs +++ b/src/encoding/swap_encoder.rs @@ -1,3 +1,6 @@ +use alloy_primitives::FixedBytes; +use tycho_core::keccak256; + use crate::encoding::{ errors::EncodingError, models::{EncodingContext, Swap}, @@ -14,4 +17,9 @@ pub trait SwapEncoder: Sync + Send { encoding_context: EncodingContext, ) -> Result, EncodingError>; fn executor_address(&self) -> &str; + + fn executor_selector(&self) -> FixedBytes<4> { + let hash = keccak256("swap(uint256,bytes)".as_bytes()); + FixedBytes::<4>::from([hash[0], hash[1], hash[2], hash[3]]) + } } From 7a8872cc415cf99f0122dcc92e87b6e09932d465 Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Thu, 30 Jan 2025 13:17:23 +0000 Subject: [PATCH 03/12] fix: Don't leak evm specific code to interfaces(PrivateKeySigner, Chain) Pass the signer's private key as a string around and only create a PrivateKeySigner inside Permit2 (where it's needed) Use tycho-core Chain instead of Alloy ChainID. Create a temp method to convert between them Create a EVMSwapEncoder that implements the executor_selector (that is evm specific) --- don't change below this line --- ENG-4081 Took 1 hour 7 minutes --- src/encoding/evm/approvals/permit2.rs | 35 +++++++++++-------- src/encoding/evm/router_encoder.rs | 16 ++++----- src/encoding/evm/strategy_encoder/encoder.rs | 26 +++++++------- src/encoding/evm/strategy_encoder/selector.rs | 11 +++--- src/encoding/evm/swap_encoder/encoders.rs | 14 +++++++- src/encoding/evm/utils.rs | 9 ++++- src/encoding/strategy_encoder.rs | 8 ++--- src/encoding/swap_encoder.rs | 8 ----- 8 files changed, 69 insertions(+), 58 deletions(-) diff --git a/src/encoding/evm/approvals/permit2.rs b/src/encoding/evm/approvals/permit2.rs index c9b687c..900d4cb 100644 --- a/src/encoding/evm/approvals/permit2.rs +++ b/src/encoding/evm/approvals/permit2.rs @@ -9,17 +9,18 @@ use alloy::{ }; #[allow(deprecated)] use alloy_primitives::Signature; +use alloy_primitives::B256; use alloy_sol_types::{eip712_domain, sol, SolStruct, SolValue}; use chrono::Utc; use num_bigint::BigUint; use tokio::runtime::Runtime; -use tycho_core::Bytes; +use tycho_core::{models::Chain, Bytes}; use crate::encoding::{ errors::EncodingError, evm::{ approvals::protocol_approvals_manager::get_client, - utils::{biguint_to_u256, bytes_to_address, encode_input}, + utils::{biguint_to_u256, bytes_to_address, encode_input, to_chain_id}, }, }; @@ -60,10 +61,17 @@ sol! { #[allow(dead_code)] impl Permit2 { - pub fn new(signer: PrivateKeySigner, chain_id: ChainId) -> Result { + pub fn new(signer_pk: String, chain: Chain) -> Result { + let chain_id = to_chain_id(chain)?; let runtime = Runtime::new() .map_err(|_| EncodingError::FatalError("Failed to create runtime".to_string()))?; let client = runtime.block_on(get_client())?; + let pk = B256::from_str(&signer_pk).map_err(|_| { + EncodingError::FatalError("Failed to convert signer private key to B256".to_string()) + })?; + let signer = PrivateKeySigner::from_bytes(&pk).map_err(|_| { + EncodingError::FatalError("Failed to create signer from private key".to_string()) + })?; Ok(Self { address: Address::from_str("0x000000000022D473030F116dDEE9F6B43aC78BA3") .map_err(|_| EncodingError::FatalError("Permit2 address not valid".to_string()))?, @@ -158,7 +166,7 @@ impl Permit2 { mod tests { use std::str::FromStr; - use alloy_primitives::{Uint, B256}; + use alloy_primitives::Uint; use num_bigint::BigUint; use super::*; @@ -196,8 +204,9 @@ mod tests { #[test] fn test_get_existing_allowance() { - let signer = PrivateKeySigner::random(); - let manager = Permit2::new(signer, 1).unwrap(); + let signer_pk = + "4c0883a69102937d6231471b5dbb6204fe512961708279feb1be6ae5538da033".to_string(); + let manager = Permit2::new(signer_pk, Chain::Ethereum).unwrap(); let token = Bytes::from_str("0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48").unwrap(); let owner = Bytes::from_str("0x2c6a3cd97c6283b95ac8c5a4459ebb0d5fd404f4").unwrap(); @@ -216,10 +225,8 @@ mod tests { fn test_get_permit() { // Set up a mock private key for signing let private_key = - B256::from_str("4c0883a69102937d6231471b5dbb6204fe512961708279feb1be6ae5538da033") - .expect("Invalid private key"); - let signer = PrivateKeySigner::from_bytes(&private_key).expect("Failed to create signer"); - let permit2 = Permit2::new(signer, 1).expect("Failed to create Permit2"); + "4c0883a69102937d6231471b5dbb6204fe512961708279feb1be6ae5538da033".to_string(); + let permit2 = Permit2::new(private_key, Chain::Ethereum).expect("Failed to create Permit2"); let owner = Bytes::from_str("0x2c6a3cd97c6283b95ac8c5a4459ebb0d5fd404f4").unwrap(); let spender = Bytes::from_str("0xba12222222228d8ba445958a75a0704d566bf2c8").unwrap(); @@ -257,12 +264,10 @@ mod tests { fn test_permit() { let anvil_account = Bytes::from_str("0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266").unwrap(); let anvil_private_key = - B256::from_str("0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80") - .unwrap(); + "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80".to_string(); - let signer = - PrivateKeySigner::from_bytes(&anvil_private_key).expect("Failed to create signer"); - let permit2 = Permit2::new(signer, 1).expect("Failed to create Permit2"); + let permit2 = + Permit2::new(anvil_private_key, Chain::Ethereum).expect("Failed to create Permit2"); let token = Bytes::from_str("0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48").unwrap(); let amount = BigUint::from(1000u64); diff --git a/src/encoding/evm/router_encoder.rs b/src/encoding/evm/router_encoder.rs index a3c74d7..e1f2f08 100644 --- a/src/encoding/evm/router_encoder.rs +++ b/src/encoding/evm/router_encoder.rs @@ -1,9 +1,7 @@ use std::str::FromStr; -use alloy::signers::local::PrivateKeySigner; -use alloy_primitives::ChainId; use num_bigint::BigUint; -use tycho_core::Bytes; +use tycho_core::{models::Chain, Bytes}; use crate::encoding::{ errors::EncodingError, @@ -16,8 +14,8 @@ use crate::encoding::{ #[allow(dead_code)] pub struct EVMRouterEncoder { strategy_selector: S, - signer: Option, - chain_id: ChainId, + signer: Option, + chain: Chain, router_address: String, } @@ -26,10 +24,10 @@ impl EVMRouterEncoder { pub fn new( strategy_selector: S, router_address: String, - signer: Option, - chain_id: ChainId, + signer: Option, + chain: Chain, ) -> Result { - Ok(EVMRouterEncoder { strategy_selector, signer, chain_id, router_address }) + Ok(EVMRouterEncoder { strategy_selector, signer, chain, router_address }) } } impl RouterEncoder for EVMRouterEncoder { @@ -50,7 +48,7 @@ impl RouterEncoder for EVMRouterEncoder { let strategy = self.strategy_selector.select_strategy( solution, self.signer.clone(), - self.chain_id, + self.chain, )?; let method_calldata = strategy.encode_strategy(solution.clone(), router_address)?; diff --git a/src/encoding/evm/strategy_encoder/encoder.rs b/src/encoding/evm/strategy_encoder/encoder.rs index 4a2d17a..dd5ba6f 100644 --- a/src/encoding/evm/strategy_encoder/encoder.rs +++ b/src/encoding/evm/strategy_encoder/encoder.rs @@ -1,10 +1,9 @@ use std::cmp::min; -use alloy::signers::local::PrivateKeySigner; -use alloy_primitives::{aliases::U24, map::HashSet, ChainId, U256, U8}; +use alloy_primitives::{aliases::U24, map::HashSet, U256, U8}; use alloy_sol_types::SolValue; use num_bigint::BigUint; -use tycho_core::Bytes; +use tycho_core::{models::Chain, Bytes}; use crate::encoding::{ errors::EncodingError, @@ -40,8 +39,8 @@ pub struct SplitSwapStrategyEncoder { } impl SplitSwapStrategyEncoder { - pub fn new(signer: PrivateKeySigner, chain_id: ChainId) -> Result { - Ok(Self { permit2: Permit2::new(signer, chain_id)? }) + pub fn new(signer_pk: String, chain: Chain) -> Result { + Ok(Self { permit2: Permit2::new(signer_pk, chain)? }) } } impl EVMStrategyEncoder for SplitSwapStrategyEncoder {} @@ -72,7 +71,7 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { BigUint::ZERO }; - let tokens: Vec = solution + let mut tokens: Vec = solution .swaps .iter() .flat_map(|swap| vec![swap.token_in.clone(), swap.token_out.clone()]) @@ -80,6 +79,10 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { .into_iter() .collect(); + // this is only to make the test deterministic (same index for the same token for different + // runs) + tokens.sort(); + let mut swaps = vec![]; for swap in solution.swaps.iter() { let registry = SWAP_ENCODER_REGISTRY.read().unwrap(); @@ -200,7 +203,6 @@ mod tests { use std::str::FromStr; use alloy::hex::encode; - use alloy_primitives::B256; use tycho_core::dto::ProtocolComponent; use super::*; @@ -210,9 +212,7 @@ mod tests { fn test_split_swap_strategy_encoder() { // Set up a mock private key for signing let private_key = - B256::from_str("4c0883a69102937d6231471b5dbb6204fe512961708279feb1be6ae5538da033") - .expect("Invalid private key"); - let signer = PrivateKeySigner::from_bytes(&private_key).expect("Failed to create signer"); + "4c0883a69102937d6231471b5dbb6204fe512961708279feb1be6ae5538da033".to_string(); let weth = Bytes::from_str("0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2").unwrap(); let dai = Bytes::from_str("0x6b175474e89094c44da98b954eedeac495271d0f").unwrap(); @@ -228,7 +228,7 @@ mod tests { split: 0f64, }; - let encoder = SplitSwapStrategyEncoder::new(signer, 1).unwrap(); + let encoder = SplitSwapStrategyEncoder::new(private_key, Chain::Ethereum).unwrap(); let solution = Solution { exact_out: false, given_token: weth, @@ -284,8 +284,8 @@ mod tests { "0000000000000000000000000000000000000000000000000000000000000000", "000000000000000000000000000000000000000000000000000000000005b", // Swap header - "00", // token in index - "01", // token out index + "01", // token in index + "00", // token out index "000000", // split // Swap data "5c2f5a71f67c01775180adc06909288b4c329308", // executor address diff --git a/src/encoding/evm/strategy_encoder/selector.rs b/src/encoding/evm/strategy_encoder/selector.rs index 7301552..b8e675c 100644 --- a/src/encoding/evm/strategy_encoder/selector.rs +++ b/src/encoding/evm/strategy_encoder/selector.rs @@ -1,5 +1,4 @@ -use alloy::signers::local::PrivateKeySigner; -use alloy_primitives::ChainId; +use tycho_core::models::Chain; use crate::encoding::{ errors::EncodingError, @@ -14,18 +13,18 @@ impl StrategySelector for EVMStrategySelector { fn select_strategy( &self, solution: &Solution, - signer: Option, - chain_id: ChainId, + signer: Option, + chain: Chain, ) -> Result, EncodingError> { if solution.straight_to_pool { Ok(Box::new(StraightToPoolStrategyEncoder {})) } else { - let signer = signer.ok_or_else(|| { + let signer_pk = signer.ok_or_else(|| { EncodingError::FatalError( "Signer is required for SplitSwapStrategyEncoder".to_string(), ) })?; - Ok(Box::new(SplitSwapStrategyEncoder::new(signer, chain_id).unwrap())) + Ok(Box::new(SplitSwapStrategyEncoder::new(signer_pk, chain).unwrap())) } } } diff --git a/src/encoding/evm/swap_encoder/encoders.rs b/src/encoding/evm/swap_encoder/encoders.rs index b0ae977..e950f56 100644 --- a/src/encoding/evm/swap_encoder/encoders.rs +++ b/src/encoding/evm/swap_encoder/encoders.rs @@ -1,7 +1,8 @@ use std::str::FromStr; -use alloy_primitives::{Address, Bytes as AlloyBytes}; +use alloy_primitives::{Address, Bytes as AlloyBytes, FixedBytes}; use alloy_sol_types::SolValue; +use tycho_core::keccak256; use crate::encoding::{ errors::EncodingError, @@ -12,10 +13,18 @@ use crate::encoding::{ swap_encoder::SwapEncoder, }; +pub trait EVMSwapEncoder: SwapEncoder { + fn executor_selector(&self) -> FixedBytes<4> { + let hash = keccak256("swap(uint256,bytes)".as_bytes()); + FixedBytes::<4>::from([hash[0], hash[1], hash[2], hash[3]]) + } +} + pub struct UniswapV2SwapEncoder { executor_address: String, } +impl EVMSwapEncoder for UniswapV2SwapEncoder {} impl UniswapV2SwapEncoder { fn get_zero_to_one(sell_token_address: Address, buy_token_address: Address) -> bool { sell_token_address < buy_token_address @@ -63,6 +72,7 @@ impl SwapEncoder for UniswapV2SwapEncoder { pub struct UniswapV3SwapEncoder { executor_address: String, } +impl EVMSwapEncoder for UniswapV3SwapEncoder {} impl UniswapV3SwapEncoder { fn get_zero_to_one(sell_token_address: Address, buy_token_address: Address) -> bool { @@ -135,6 +145,8 @@ pub struct BalancerV2SwapEncoder { vault_address: String, } +impl EVMSwapEncoder for BalancerV2SwapEncoder {} + impl SwapEncoder for BalancerV2SwapEncoder { fn new(executor_address: String) -> Self { Self { diff --git a/src/encoding/evm/utils.rs b/src/encoding/evm/utils.rs index 0afe763..3925d44 100644 --- a/src/encoding/evm/utils.rs +++ b/src/encoding/evm/utils.rs @@ -1,7 +1,7 @@ use alloy_primitives::{aliases::U24, Address, Keccak256, U256}; use alloy_sol_types::SolValue; use num_bigint::BigUint; -use tycho_core::Bytes; +use tycho_core::{models::Chain, Bytes}; use crate::encoding::errors::EncodingError; @@ -65,3 +65,10 @@ pub fn percentage_to_uint24(percentage: f64) -> U24 { let scaled = (percentage / 100.0) * (MAX_UINT24 as f64); U24::from(scaled.round()) } + +pub fn to_chain_id(chain: Chain) -> Result { + match chain { + Chain::Ethereum => Ok(1), + _ => Err(EncodingError::FatalError("Unsupported chain".to_string())), + } +} diff --git a/src/encoding/strategy_encoder.rs b/src/encoding/strategy_encoder.rs index 1204e6b..8d37fe1 100644 --- a/src/encoding/strategy_encoder.rs +++ b/src/encoding/strategy_encoder.rs @@ -1,6 +1,4 @@ -use alloy::signers::local::PrivateKeySigner; -use alloy_primitives::ChainId; -use tycho_core::Bytes; +use tycho_core::{models::Chain, Bytes}; use crate::encoding::{errors::EncodingError, models::Solution}; @@ -19,7 +17,7 @@ pub trait StrategySelector { fn select_strategy( &self, solution: &Solution, - signer: Option, - chain_id: ChainId, + signer_pk: Option, + chain_id: Chain, ) -> Result, EncodingError>; } diff --git a/src/encoding/swap_encoder.rs b/src/encoding/swap_encoder.rs index 65e94c3..1a1fc0c 100644 --- a/src/encoding/swap_encoder.rs +++ b/src/encoding/swap_encoder.rs @@ -1,6 +1,3 @@ -use alloy_primitives::FixedBytes; -use tycho_core::keccak256; - use crate::encoding::{ errors::EncodingError, models::{EncodingContext, Swap}, @@ -17,9 +14,4 @@ pub trait SwapEncoder: Sync + Send { encoding_context: EncodingContext, ) -> Result, EncodingError>; fn executor_address(&self) -> &str; - - fn executor_selector(&self) -> FixedBytes<4> { - let hash = keccak256("swap(uint256,bytes)".as_bytes()); - FixedBytes::<4>::from([hash[0], hash[1], hash[2], hash[3]]) - } } From 0e70e827a02bb21adc47829fa60ba47a805269fe Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Thu, 30 Jan 2025 13:19:09 +0000 Subject: [PATCH 04/12] chore: Rename encoder to strategy/swap_encoder This was confusing me when I had all of them open at the same time. Better to be extra explicit --- don't change below this line --- ENG-4081 Took 2 minutes --- src/encoding/evm/strategy_encoder/mod.rs | 4 ++-- .../evm/strategy_encoder/{encoder.rs => strategy_encoders.rs} | 0 .../strategy_encoder/{selector.rs => strategy_selector.rs} | 4 +++- src/encoding/evm/swap_encoder/builder.rs | 2 +- src/encoding/evm/swap_encoder/mod.rs | 2 +- .../evm/swap_encoder/{encoders.rs => swap_encoders.rs} | 0 6 files changed, 7 insertions(+), 5 deletions(-) rename src/encoding/evm/strategy_encoder/{encoder.rs => strategy_encoders.rs} (100%) rename src/encoding/evm/strategy_encoder/{selector.rs => strategy_selector.rs} (87%) rename src/encoding/evm/swap_encoder/{encoders.rs => swap_encoders.rs} (100%) diff --git a/src/encoding/evm/strategy_encoder/mod.rs b/src/encoding/evm/strategy_encoder/mod.rs index 9d36e12..44c9689 100644 --- a/src/encoding/evm/strategy_encoder/mod.rs +++ b/src/encoding/evm/strategy_encoder/mod.rs @@ -1,2 +1,2 @@ -mod encoder; -mod selector; +mod strategy_encoders; +mod strategy_selector; diff --git a/src/encoding/evm/strategy_encoder/encoder.rs b/src/encoding/evm/strategy_encoder/strategy_encoders.rs similarity index 100% rename from src/encoding/evm/strategy_encoder/encoder.rs rename to src/encoding/evm/strategy_encoder/strategy_encoders.rs diff --git a/src/encoding/evm/strategy_encoder/selector.rs b/src/encoding/evm/strategy_encoder/strategy_selector.rs similarity index 87% rename from src/encoding/evm/strategy_encoder/selector.rs rename to src/encoding/evm/strategy_encoder/strategy_selector.rs index b8e675c..2451711 100644 --- a/src/encoding/evm/strategy_encoder/selector.rs +++ b/src/encoding/evm/strategy_encoder/strategy_selector.rs @@ -2,7 +2,9 @@ use tycho_core::models::Chain; use crate::encoding::{ errors::EncodingError, - evm::strategy_encoder::encoder::{SplitSwapStrategyEncoder, StraightToPoolStrategyEncoder}, + evm::strategy_encoder::strategy_encoders::{ + SplitSwapStrategyEncoder, StraightToPoolStrategyEncoder, + }, models::Solution, strategy_encoder::{StrategyEncoder, StrategySelector}, }; diff --git a/src/encoding/evm/swap_encoder/builder.rs b/src/encoding/evm/swap_encoder/builder.rs index 141370e..cd5525b 100644 --- a/src/encoding/evm/swap_encoder/builder.rs +++ b/src/encoding/evm/swap_encoder/builder.rs @@ -1,5 +1,5 @@ use crate::encoding::{ - evm::swap_encoder::encoders::{BalancerV2SwapEncoder, UniswapV2SwapEncoder}, + evm::swap_encoder::swap_encoders::{BalancerV2SwapEncoder, UniswapV2SwapEncoder}, swap_encoder::SwapEncoder, }; diff --git a/src/encoding/evm/swap_encoder/mod.rs b/src/encoding/evm/swap_encoder/mod.rs index ebdb2d5..1148d2d 100644 --- a/src/encoding/evm/swap_encoder/mod.rs +++ b/src/encoding/evm/swap_encoder/mod.rs @@ -1,6 +1,6 @@ mod builder; -mod encoders; mod registry; +mod swap_encoders; use std::sync::RwLock; diff --git a/src/encoding/evm/swap_encoder/encoders.rs b/src/encoding/evm/swap_encoder/swap_encoders.rs similarity index 100% rename from src/encoding/evm/swap_encoder/encoders.rs rename to src/encoding/evm/swap_encoder/swap_encoders.rs From 089e7d2e0f7fcd60c591ff0d1e5e56a7d7ae93dd Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Thu, 30 Jan 2025 15:01:53 +0000 Subject: [PATCH 05/12] feat: Add ChainId model Remove to_chain_id helper method --- don't change below this line --- ENG-4081 Took 28 minutes --- src/encoding/evm/approvals/permit2.rs | 9 +++++---- src/encoding/evm/mod.rs | 1 + src/encoding/evm/models.rs | 20 ++++++++++++++++++++ src/encoding/evm/utils.rs | 9 +-------- 4 files changed, 27 insertions(+), 12 deletions(-) create mode 100644 src/encoding/evm/models.rs diff --git a/src/encoding/evm/approvals/permit2.rs b/src/encoding/evm/approvals/permit2.rs index 900d4cb..aca2c0c 100644 --- a/src/encoding/evm/approvals/permit2.rs +++ b/src/encoding/evm/approvals/permit2.rs @@ -1,7 +1,7 @@ use std::{str::FromStr, sync::Arc}; use alloy::{ - primitives::{aliases::U48, Address, Bytes as AlloyBytes, ChainId, TxKind, U160, U256}, + primitives::{aliases::U48, Address, Bytes as AlloyBytes, TxKind, U160, U256}, providers::{Provider, RootProvider}, rpc::types::{TransactionInput, TransactionRequest}, signers::{local::PrivateKeySigner, SignerSync}, @@ -20,7 +20,8 @@ use crate::encoding::{ errors::EncodingError, evm::{ approvals::protocol_approvals_manager::get_client, - utils::{biguint_to_u256, bytes_to_address, encode_input, to_chain_id}, + models::ChainId, + utils::{biguint_to_u256, bytes_to_address, encode_input}, }, }; @@ -62,7 +63,7 @@ sol! { #[allow(dead_code)] impl Permit2 { pub fn new(signer_pk: String, chain: Chain) -> Result { - let chain_id = to_chain_id(chain)?; + let chain_id = ChainId::from(chain); let runtime = Runtime::new() .map_err(|_| EncodingError::FatalError("Failed to create runtime".to_string()))?; let client = runtime.block_on(get_client())?; @@ -145,7 +146,7 @@ impl Permit2 { let domain = eip712_domain! { name: "Permit2", - chain_id: self.chain_id, + chain_id: self.chain_id.id(), verifying_contract: self.address, }; let hash = permit_single.eip712_signing_hash(&domain); diff --git a/src/encoding/evm/mod.rs b/src/encoding/evm/mod.rs index d2abfd8..031dbc4 100644 --- a/src/encoding/evm/mod.rs +++ b/src/encoding/evm/mod.rs @@ -1,4 +1,5 @@ pub mod approvals; +mod models; mod router_encoder; mod strategy_encoder; mod swap_encoder; diff --git a/src/encoding/evm/models.rs b/src/encoding/evm/models.rs new file mode 100644 index 0000000..68121bf --- /dev/null +++ b/src/encoding/evm/models.rs @@ -0,0 +1,20 @@ +use tycho_core::models::Chain; + +pub struct ChainId(u64); + +impl ChainId { + pub fn id(&self) -> u64 { + self.0 + } +} + +impl From for ChainId { + fn from(chain: Chain) -> Self { + match chain { + Chain::Ethereum => ChainId(1), + Chain::ZkSync => ChainId(324), + Chain::Arbitrum => ChainId(42161), + Chain::Starknet => ChainId(0), + } + } +} diff --git a/src/encoding/evm/utils.rs b/src/encoding/evm/utils.rs index 3925d44..0afe763 100644 --- a/src/encoding/evm/utils.rs +++ b/src/encoding/evm/utils.rs @@ -1,7 +1,7 @@ use alloy_primitives::{aliases::U24, Address, Keccak256, U256}; use alloy_sol_types::SolValue; use num_bigint::BigUint; -use tycho_core::{models::Chain, Bytes}; +use tycho_core::Bytes; use crate::encoding::errors::EncodingError; @@ -65,10 +65,3 @@ pub fn percentage_to_uint24(percentage: f64) -> U24 { let scaled = (percentage / 100.0) * (MAX_UINT24 as f64); U24::from(scaled.round()) } - -pub fn to_chain_id(chain: Chain) -> Result { - match chain { - Chain::Ethereum => Ok(1), - _ => Err(EncodingError::FatalError("Unsupported chain".to_string())), - } -} From 6e8d2ede595ecafe42cd1025f67ed4ab0083360d Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Thu, 30 Jan 2025 15:19:09 +0000 Subject: [PATCH 06/12] feat: Simplify router encoder Don't make selector() a member of the StrategyEncoder trait. It is needed only for certain strategies. The strategy should manage it itself. --- don't change below this line --- ENG-4081 Took 17 minutes --- src/encoding/evm/router_encoder.rs | 17 +++++++------- .../evm/strategy_encoder/strategy_encoders.rs | 23 +++++++++---------- src/encoding/strategy_encoder.rs | 1 - 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/encoding/evm/router_encoder.rs b/src/encoding/evm/router_encoder.rs index e1f2f08..e953a67 100644 --- a/src/encoding/evm/router_encoder.rs +++ b/src/encoding/evm/router_encoder.rs @@ -5,7 +5,6 @@ use tycho_core::{models::Chain, Bytes}; use crate::encoding::{ errors::EncodingError, - evm::utils::encode_input, models::{NativeAction, Solution, Transaction}, router_encoder::RouterEncoder, strategy_encoder::StrategySelector, @@ -37,8 +36,12 @@ impl RouterEncoder for EVMRouterEncoder { ) -> Result, EncodingError> { let mut transactions: Vec = Vec::new(); for solution in solutions.iter() { - let exact_out = solution.exact_out; - let straight_to_pool = solution.straight_to_pool; + if solution.exact_out { + return Err(EncodingError::FatalError( + "Currently only exact input solutions are supported".to_string(), + )); + } + let router_address = solution .router_address .clone() @@ -50,13 +53,9 @@ impl RouterEncoder for EVMRouterEncoder { self.signer.clone(), self.chain, )?; - let method_calldata = strategy.encode_strategy(solution.clone(), router_address)?; - let contract_interaction = if straight_to_pool { - method_calldata - } else { - encode_input(strategy.selector(exact_out), method_calldata) - }; + let contract_interaction = + strategy.encode_strategy(solution.clone(), router_address)?; let value = if solution.native_action.clone().unwrap() == NativeAction::Wrap { solution.given_amount.clone() diff --git a/src/encoding/evm/strategy_encoder/strategy_encoders.rs b/src/encoding/evm/strategy_encoder/strategy_encoders.rs index dd5ba6f..3e0b73b 100644 --- a/src/encoding/evm/strategy_encoder/strategy_encoders.rs +++ b/src/encoding/evm/strategy_encoder/strategy_encoders.rs @@ -10,7 +10,9 @@ use crate::encoding::{ evm::{ approvals::permit2::Permit2, swap_encoder::SWAP_ENCODER_REGISTRY, - utils::{biguint_to_u256, bytes_to_address, percentage_to_uint24, ple_encode}, + utils::{ + biguint_to_u256, bytes_to_address, encode_input, percentage_to_uint24, ple_encode, + }, }, models::{EncodingContext, NativeAction, Solution}, strategy_encoder::StrategyEncoder, @@ -36,11 +38,13 @@ pub trait EVMStrategyEncoder: StrategyEncoder { pub struct SplitSwapStrategyEncoder { permit2: Permit2, + selector: String, } impl SplitSwapStrategyEncoder { pub fn new(signer_pk: String, chain: Chain) -> Result { - Ok(Self { permit2: Permit2::new(signer_pk, chain)? }) + let selector = "swap(uint256, address, address, uint256, bool, bool, uint256, address, ((address,uint160,uint48,uint48),address,uint256),bytes, bytes)".to_string(); + Ok(Self { permit2: Permit2::new(signer_pk, chain)?, selector }) } } impl EVMStrategyEncoder for SplitSwapStrategyEncoder {} @@ -145,11 +149,9 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { encoded_swaps, ) .abi_encode(); - Ok(method_calldata) - } - fn selector(&self, _exact_out: bool) -> &str { - "swap(uint256, address, uint256, bytes[])" + let contract_interaction = encode_input(&self.selector, method_calldata); + Ok(contract_interaction) } } @@ -193,9 +195,6 @@ impl StrategyEncoder for StraightToPoolStrategyEncoder { // TODO: here we need to pass also the address of the executor to be used Ok(protocol_data) } - fn selector(&self, _exact_out: bool) -> &str { - unimplemented!(); - } } #[cfg(test)] @@ -248,7 +247,7 @@ mod tests { .unwrap(); let expected_input = String::from(concat!( - "0000000000000000000000000000000000000000000000000000000000000020", // offset + "e73e3baa", // selector "0000000000000000000000000000000000000000000000000de0b6b3a7640000", // amount out "000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", // token in "0000000000000000000000006b175474e89094c44da98b954eedeac495271d0f", // token out @@ -298,7 +297,7 @@ mod tests { "0000000000", // padding )); let hex_calldata = encode(&calldata); - assert_eq!(hex_calldata[..576], expected_input); - assert_eq!(hex_calldata[1283..], expected_swaps); + assert_eq!(hex_calldata[..520], expected_input); + assert_eq!(hex_calldata[1227..], expected_swaps); } } diff --git a/src/encoding/strategy_encoder.rs b/src/encoding/strategy_encoder.rs index 8d37fe1..295816b 100644 --- a/src/encoding/strategy_encoder.rs +++ b/src/encoding/strategy_encoder.rs @@ -9,7 +9,6 @@ pub trait StrategyEncoder { to_encode: Solution, router_address: Bytes, ) -> Result, EncodingError>; - fn selector(&self, exact_out: bool) -> &str; } pub trait StrategySelector { From 82e671df395477e800afc6b6bc94d5a07d78ec04 Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Thu, 30 Jan 2025 15:39:07 +0000 Subject: [PATCH 07/12] fix: Use abi_encode_packed in ple_encode() --- don't change below this line --- ENG-4081 Took 17 minutes --- .../evm/strategy_encoder/strategy_encoders.rs | 14 ++++---------- src/encoding/evm/utils.rs | 2 +- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/encoding/evm/strategy_encoder/strategy_encoders.rs b/src/encoding/evm/strategy_encoder/strategy_encoders.rs index 3e0b73b..c3d7a1a 100644 --- a/src/encoding/evm/strategy_encoder/strategy_encoders.rs +++ b/src/encoding/evm/strategy_encoder/strategy_encoders.rs @@ -265,7 +265,7 @@ mod tests { // "0000000000000000000000000000000000000000000000000000000000000000", // nonce // "0000000000000000000000002c6a3cd97c6283b95ac8c5a4459ebb0d5fd404f4", // spender // "00000000000000000000000000000000000000000000000000000000679a8006", // deadline - // offsets??? + // offsets // "0000000000000000000000000000000000000000000000000000000000000200", // "0000000000000000000000000000000000000000000000000000000000000280", // "0000000000000000000000000000000000000000000000000000000000000041", @@ -273,15 +273,9 @@ mod tests { // "fc5bac4e27cd5d71c85d232d8c6b31ea924d2e0031091ff9a39579d9e49c214328ea34876961d9200af691373c71a174e166793d02241c76adb93c5f87fe0f381c", let expected_swaps = String::from(concat!( - // ple encode adds aaalll of this :/ is it correct? + // ple encoded swaps "0000000000000000000000000000000000000000000000000000000000000000", - "0000000000000000000000000000000000000000000000000000000000120000", - "0000000000000000000000000000000000000000000000000000000000020000", - "0000000000000000000000000000000000000000000000000000000000060000", - "000000000000000000000000000000000000000000000000000000000005b000", - "0000000000000000000000000000000000000000000000000000000000080000", - "0000000000000000000000000000000000000000000000000000000000000000", - "000000000000000000000000000000000000000000000000000000000005b", + "000000000000000000000000000000000000000000000000000000000005d005b", // Swap header "01", // token in index "00", // token out index @@ -294,7 +288,7 @@ mod tests { "2c6a3cd97c6283b95ac8c5a4459ebb0d5fd404f4", // receiver "00", // zero2one "00", // exact out - "0000000000", // padding + "000000", // padding )); let hex_calldata = encode(&calldata); assert_eq!(hex_calldata[..520], expected_input); diff --git a/src/encoding/evm/utils.rs b/src/encoding/evm/utils.rs index 0afe763..2111958 100644 --- a/src/encoding/evm/utils.rs +++ b/src/encoding/evm/utils.rs @@ -29,7 +29,7 @@ pub fn ple_encode(action_data_array: Vec>) -> Vec { for action_data in action_data_array { let args = (encoded_action_data, action_data.len() as u16, action_data); - encoded_action_data = args.abi_encode(); + encoded_action_data = args.abi_encode_packed(); } encoded_action_data From 5b86ae4ac579831b51723641699a672632b5f596 Mon Sep 17 00:00:00 2001 From: TAMARA LIPOWSKI Date: Thu, 30 Jan 2025 12:45:36 -0500 Subject: [PATCH 08/12] docs: (split strategy test) better calldata comments ...for clarity and debugging purposes. --- .../evm/strategy_encoder/strategy_encoders.rs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/encoding/evm/strategy_encoder/strategy_encoders.rs b/src/encoding/evm/strategy_encoder/strategy_encoders.rs index c3d7a1a..f28a768 100644 --- a/src/encoding/evm/strategy_encoder/strategy_encoders.rs +++ b/src/encoding/evm/strategy_encoder/strategy_encoders.rs @@ -192,6 +192,7 @@ impl StrategyEncoder for StraightToPoolStrategyEncoder { router_address, }; let protocol_data = swap_encoder.encode_swap(swap.clone(), encoding_context)?; + // TODO: here we need to pass also the address of the executor to be used Ok(protocol_data) } @@ -247,7 +248,7 @@ mod tests { .unwrap(); let expected_input = String::from(concat!( - "e73e3baa", // selector + "e73e3baa", "0000000000000000000000000000000000000000000000000de0b6b3a7640000", // amount out "000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", // token in "0000000000000000000000006b175474e89094c44da98b954eedeac495271d0f", // token out @@ -265,17 +266,22 @@ mod tests { // "0000000000000000000000000000000000000000000000000000000000000000", // nonce // "0000000000000000000000002c6a3cd97c6283b95ac8c5a4459ebb0d5fd404f4", // spender // "00000000000000000000000000000000000000000000000000000000679a8006", // deadline - // offsets + // offset of signature (from start of call data to beginning of length indication) // "0000000000000000000000000000000000000000000000000000000000000200", + // offset of ple encoded swaps (from start of call data to beginning of length indication) // "0000000000000000000000000000000000000000000000000000000000000280", + // length of signature without padding // "0000000000000000000000000000000000000000000000000000000000000041", - // signature - // "fc5bac4e27cd5d71c85d232d8c6b31ea924d2e0031091ff9a39579d9e49c214328ea34876961d9200af691373c71a174e166793d02241c76adb93c5f87fe0f381c", + // signature + padding + // "a031b63a01ef5d25975663e5d6c420ef498e3a5968b593cdf846c6729a788186", + // "1ddaf79c51453cd501d321ee541d13593e3a266be44103eefdf6e76a032d2870", + // "1b00000000000000000000000000000000000000000000000000000000000000" let expected_swaps = String::from(concat!( + // length of ple encoded swaps without padding + "000000000000000000000000000000000000000000000000000000000000005d", // ple encoded swaps - "0000000000000000000000000000000000000000000000000000000000000000", - "000000000000000000000000000000000000000000000000000000000005d005b", + "005b", // Swap header "01", // token in index "00", // token out index @@ -291,7 +297,8 @@ mod tests { "000000", // padding )); let hex_calldata = encode(&calldata); + assert_eq!(hex_calldata[..520], expected_input); - assert_eq!(hex_calldata[1227..], expected_swaps); + assert_eq!(hex_calldata[1288..], expected_swaps); } } From a28b54888e08fe23ed20ef8b0a385f094bca3c28 Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Thu, 30 Jan 2025 19:38:22 +0000 Subject: [PATCH 09/12] fix: Post merge's fixes Because of the renaming, git couldn't identify the new files and handle the conflicts gracefully. Copied implementation for ExecutorStrategyEncoder from main Rollbacked on decision to encode the executor address and selector inside the SwapEncoders. This is only necessary for certain strategies. So it should be done at the strategy level --- don't change below this line --- ENG-4081 Took 35 minutes --- .../evm/strategy_encoder/strategy_encoders.rs | 102 +++++++++++++++--- .../evm/strategy_encoder/strategy_selector.rs | 8 +- .../evm/swap_encoder/swap_encoders.rs | 53 +++------ src/encoding/strategy_encoder.rs | 7 +- src/encoding/swap_encoder.rs | 1 + 5 files changed, 113 insertions(+), 58 deletions(-) diff --git a/src/encoding/evm/strategy_encoder/strategy_encoders.rs b/src/encoding/evm/strategy_encoder/strategy_encoders.rs index f28a768..2d776f4 100644 --- a/src/encoding/evm/strategy_encoder/strategy_encoders.rs +++ b/src/encoding/evm/strategy_encoder/strategy_encoders.rs @@ -1,9 +1,9 @@ -use std::cmp::min; +use std::{cmp::min, str::FromStr}; -use alloy_primitives::{aliases::U24, map::HashSet, U256, U8}; +use alloy_primitives::{aliases::U24, map::HashSet, FixedBytes, U256, U8}; use alloy_sol_types::SolValue; use num_bigint::BigUint; -use tycho_core::{models::Chain, Bytes}; +use tycho_core::{keccak256, models::Chain, Bytes}; use crate::encoding::{ errors::EncodingError, @@ -25,15 +25,23 @@ pub trait EVMStrategyEncoder: StrategyEncoder { token_in: U8, token_out: U8, split: U24, + executor_address: Bytes, + executor_selector: FixedBytes<4>, protocol_data: Vec, ) -> Vec { let mut encoded = Vec::new(); encoded.push(token_in.to_be_bytes_vec()[0]); encoded.push(token_out.to_be_bytes_vec()[0]); encoded.extend_from_slice(&split.to_be_bytes_vec()); + encoded.extend(executor_address.to_vec()); + encoded.extend(executor_selector); encoded.extend(protocol_data); encoded } + fn encode_executor_selector(&self, selector: &str) -> FixedBytes<4> { + let hash = keccak256(selector.as_bytes()); + FixedBytes::<4>::from([hash[0], hash[1], hash[2], hash[3]]) + } } pub struct SplitSwapStrategyEncoder { @@ -53,7 +61,7 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { &self, solution: Solution, router_address: Bytes, - ) -> Result, EncodingError> { + ) -> Result<(Vec, Bytes), EncodingError> { let (permit, signature) = self.permit2.get_permit( &router_address, &solution.sender, @@ -122,6 +130,10 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { })?, ), percentage_to_uint24(swap.split), + Bytes::from_str(swap_encoder.executor_address()).map_err(|_| { + EncodingError::FatalError("Invalid executor address".to_string()) + })?, + self.encode_executor_selector(swap_encoder.executor_selector()), protocol_data, ); swaps.push(swap_data); @@ -151,20 +163,20 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { .abi_encode(); let contract_interaction = encode_input(&self.selector, method_calldata); - Ok(contract_interaction) + Ok((contract_interaction, router_address)) } } /// This strategy encoder is used for solutions that are sent directly to the pool. /// Only 1 solution with 1 swap is supported. -pub struct StraightToPoolStrategyEncoder {} -impl EVMStrategyEncoder for StraightToPoolStrategyEncoder {} -impl StrategyEncoder for StraightToPoolStrategyEncoder { +pub struct ExecutorStrategyEncoder {} +impl EVMStrategyEncoder for ExecutorStrategyEncoder {} +impl StrategyEncoder for ExecutorStrategyEncoder { fn encode_strategy( &self, solution: Solution, _router_address: Bytes, - ) -> Result, EncodingError> { + ) -> Result<(Vec, Bytes), EncodingError> { if solution.router_address.is_none() { return Err(EncodingError::InvalidInput( "Router address is required for straight to pool solutions".to_string(), @@ -193,8 +205,9 @@ impl StrategyEncoder for StraightToPoolStrategyEncoder { }; let protocol_data = swap_encoder.encode_swap(swap.clone(), encoding_context)?; - // TODO: here we need to pass also the address of the executor to be used - Ok(protocol_data) + let executor_address = Bytes::from_str(swap_encoder.executor_address()) + .map_err(|_| EncodingError::FatalError("Invalid executor address".to_string()))?; + Ok((protocol_data, executor_address)) } } @@ -203,11 +216,70 @@ mod tests { use std::str::FromStr; use alloy::hex::encode; - use tycho_core::dto::ProtocolComponent; + use num_bigint::BigUint; + use tycho_core::{dto::ProtocolComponent, Bytes}; use super::*; use crate::encoding::models::Swap; + #[test] + fn test_executor_strategy_encode() { + let encoder = ExecutorStrategyEncoder {}; + + let token_in = Bytes::from("0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2"); + let token_out = Bytes::from("0x6b175474e89094c44da98b954eedeac495271d0f"); + + let swap = Swap { + component: ProtocolComponent { + id: "0xA478c2975Ab1Ea89e8196811F51A7B7Ade33eB11".to_string(), + protocol_system: "uniswap_v2".to_string(), + ..Default::default() + }, + token_in: token_in.clone(), + token_out: token_out.clone(), + split: 0f64, + }; + + let solution = Solution { + exact_out: false, + given_token: token_in, + given_amount: BigUint::from(1000000000000000000u64), + expected_amount: BigUint::from(1000000000000000000u64), + checked_token: token_out, + check_amount: None, + sender: Bytes::from_str("0x0000000000000000000000000000000000000000").unwrap(), + // The receiver was generated with `makeAddr("bob") using forge` + receiver: Bytes::from_str("0x1d96f2f6bef1202e4ce1ff6dad0c2cb002861d3e").unwrap(), + swaps: vec![swap], + direct_execution: true, + router_address: Some(Bytes::zero(20)), + slippage: None, + native_action: None, + }; + + let (protocol_data, executor_address) = encoder + .encode_strategy(solution, Bytes::zero(20)) + .unwrap(); + let hex_protocol_data = encode(&protocol_data); + assert_eq!( + executor_address, + Bytes::from_str("0x5c2f5a71f67c01775180adc06909288b4c329308").unwrap() + ); + assert_eq!( + hex_protocol_data, + String::from(concat!( + // in token + "c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", + // component id + "a478c2975ab1ea89e8196811f51a7b7ade33eb11", + // receiver + "1d96f2f6bef1202e4ce1ff6dad0c2cb002861d3e", + // zero for one + "00", + )) + ); + } + #[test] fn test_split_swap_strategy_encoder() { // Set up a mock private key for signing @@ -243,7 +315,7 @@ mod tests { }; let router_address = Bytes::from_str("0x2c6A3cd97c6283b95Ac8C5A4459eBB0d5Fd404F4").unwrap(); - let calldata = encoder + let (calldata, _) = encoder .encode_strategy(solution, router_address) .unwrap(); @@ -279,9 +351,9 @@ mod tests { let expected_swaps = String::from(concat!( // length of ple encoded swaps without padding - "000000000000000000000000000000000000000000000000000000000000005d", + "000000000000000000000000000000000000000000000000000000000000005c", // ple encoded swaps - "005b", + "005a", // Swap header "01", // token in index "00", // token out index diff --git a/src/encoding/evm/strategy_encoder/strategy_selector.rs b/src/encoding/evm/strategy_encoder/strategy_selector.rs index 2451711..b45cebf 100644 --- a/src/encoding/evm/strategy_encoder/strategy_selector.rs +++ b/src/encoding/evm/strategy_encoder/strategy_selector.rs @@ -2,9 +2,7 @@ use tycho_core::models::Chain; use crate::encoding::{ errors::EncodingError, - evm::strategy_encoder::strategy_encoders::{ - SplitSwapStrategyEncoder, StraightToPoolStrategyEncoder, - }, + evm::strategy_encoder::strategy_encoders::{ExecutorStrategyEncoder, SplitSwapStrategyEncoder}, models::Solution, strategy_encoder::{StrategyEncoder, StrategySelector}, }; @@ -18,8 +16,8 @@ impl StrategySelector for EVMStrategySelector { signer: Option, chain: Chain, ) -> Result, EncodingError> { - if solution.straight_to_pool { - Ok(Box::new(StraightToPoolStrategyEncoder {})) + if solution.direct_execution { + Ok(Box::new(ExecutorStrategyEncoder {})) } else { let signer_pk = signer.ok_or_else(|| { EncodingError::FatalError( diff --git a/src/encoding/evm/swap_encoder/swap_encoders.rs b/src/encoding/evm/swap_encoder/swap_encoders.rs index 661b309..6c0a894 100644 --- a/src/encoding/evm/swap_encoder/swap_encoders.rs +++ b/src/encoding/evm/swap_encoder/swap_encoders.rs @@ -1,8 +1,7 @@ use std::str::FromStr; -use alloy_primitives::{Address, Bytes as AlloyBytes, FixedBytes}; +use alloy_primitives::{Address, Bytes as AlloyBytes}; use alloy_sol_types::SolValue; -use tycho_core::keccak256; use crate::encoding::{ errors::EncodingError, @@ -13,18 +12,11 @@ use crate::encoding::{ swap_encoder::SwapEncoder, }; -pub trait EVMSwapEncoder: SwapEncoder { - fn executor_selector(&self) -> FixedBytes<4> { - let hash = keccak256("swap(uint256,bytes)".as_bytes()); - FixedBytes::<4>::from([hash[0], hash[1], hash[2], hash[3]]) - } -} - pub struct UniswapV2SwapEncoder { executor_address: String, + executor_selector: String, } -impl EVMSwapEncoder for UniswapV2SwapEncoder {} impl UniswapV2SwapEncoder { fn get_zero_to_one(sell_token_address: Address, buy_token_address: Address) -> bool { sell_token_address < buy_token_address @@ -33,7 +25,7 @@ impl UniswapV2SwapEncoder { impl SwapEncoder for UniswapV2SwapEncoder { fn new(executor_address: String) -> Self { - Self { executor_address } + Self { executor_address, executor_selector: "swap(uint256,bytes)".to_string() } } fn encode_swap( @@ -51,9 +43,6 @@ impl SwapEncoder for UniswapV2SwapEncoder { // Token in address is always needed to perform a manual transfer from the router, // since no optimizations are performed that send from one pool to the next let args = ( - Address::from_str(self.executor_address()) - .map_err(|_| EncodingError::FatalError("Invalid executor address".to_string()))?, - self.executor_selector(), token_in_address, component_id, bytes_to_address(&encoding_context.receiver)?, @@ -66,12 +55,16 @@ impl SwapEncoder for UniswapV2SwapEncoder { fn executor_address(&self) -> &str { &self.executor_address } + + fn executor_selector(&self) -> &str { + &self.executor_selector + } } pub struct UniswapV3SwapEncoder { executor_address: String, + executor_selector: String, } -impl EVMSwapEncoder for UniswapV3SwapEncoder {} impl UniswapV3SwapEncoder { fn get_zero_to_one(sell_token_address: Address, buy_token_address: Address) -> bool { @@ -81,7 +74,7 @@ impl UniswapV3SwapEncoder { impl SwapEncoder for UniswapV3SwapEncoder { fn new(executor_address: String) -> Self { - Self { executor_address } + Self { executor_address, executor_selector: "swap(uint256,bytes)".to_string() } } fn encode_swap( @@ -119,9 +112,6 @@ impl SwapEncoder for UniswapV3SwapEncoder { })?; let args = ( - Address::from_str(self.executor_address()) - .map_err(|_| EncodingError::FatalError("Invalid executor address".to_string()))?, - self.executor_selector(), token_in_address, token_out_address, pool_fee_u24, @@ -136,19 +126,22 @@ impl SwapEncoder for UniswapV3SwapEncoder { fn executor_address(&self) -> &str { &self.executor_address } + fn executor_selector(&self) -> &str { + &self.executor_selector + } } pub struct BalancerV2SwapEncoder { executor_address: String, + executor_selector: String, vault_address: String, } -impl EVMSwapEncoder for BalancerV2SwapEncoder {} - impl SwapEncoder for BalancerV2SwapEncoder { fn new(executor_address: String) -> Self { Self { executor_address, + executor_selector: "swap(uint256,bytes)".to_string(), vault_address: "0xba12222222228d8ba445958a75a0704d566bf2c8".to_string(), } } @@ -171,9 +164,6 @@ impl SwapEncoder for BalancerV2SwapEncoder { .map_err(|_| EncodingError::FatalError("Invalid component ID".to_string()))?; let args = ( - Address::from_str(self.executor_address()) - .map_err(|_| EncodingError::FatalError("Invalid executor address".to_string()))?, - self.executor_selector(), bytes_to_address(&swap.token_in)?, bytes_to_address(&swap.token_out)?, component_id, @@ -187,6 +177,9 @@ impl SwapEncoder for BalancerV2SwapEncoder { fn executor_address(&self) -> &str { &self.executor_address } + fn executor_selector(&self) -> &str { + &self.executor_selector + } } #[cfg(test)] @@ -224,10 +217,6 @@ mod tests { assert_eq!( hex_swap, String::from(concat!( - // executor address - "543778987b293c7e8cf0722bb2e935ba6f4068d4", - // executor selector - "bd0625ab", // in token "c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", // component id @@ -270,10 +259,6 @@ mod tests { assert_eq!( hex_swap, String::from(concat!( - // executor address - "543778987b293c7e8cf0722bb2e935ba6f4068d4", - // executor selector - "bd0625ab", // in token "c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", // out token @@ -318,10 +303,6 @@ mod tests { assert_eq!( hex_swap, String::from(concat!( - // executor address - "543778987b293c7e8cf0722bb2e935ba6f4068d4", - // executor selector - "bd0625ab", // token in "c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", // token out diff --git a/src/encoding/strategy_encoder.rs b/src/encoding/strategy_encoder.rs index dcdcac0..99dfb36 100644 --- a/src/encoding/strategy_encoder.rs +++ b/src/encoding/strategy_encoder.rs @@ -4,8 +4,11 @@ use crate::encoding::{errors::EncodingError, models::Solution}; #[allow(dead_code)] pub trait StrategyEncoder { - fn encode_strategy(&self, to_encode: Solution,router_address: Bytes,) -> Result<(Vec, Bytes), EncodingError>; - fn selector(&self, exact_out: bool) -> &str; + fn encode_strategy( + &self, + to_encode: Solution, + router_address: Bytes, + ) -> Result<(Vec, Bytes), EncodingError>; } pub trait StrategySelector { diff --git a/src/encoding/swap_encoder.rs b/src/encoding/swap_encoder.rs index 1a1fc0c..14f49f2 100644 --- a/src/encoding/swap_encoder.rs +++ b/src/encoding/swap_encoder.rs @@ -14,4 +14,5 @@ pub trait SwapEncoder: Sync + Send { encoding_context: EncodingContext, ) -> Result, EncodingError>; fn executor_address(&self) -> &str; + fn executor_selector(&self) -> &str; } From 575c5bea5ec9c07f3bc729d140ad14a2a779b184 Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Thu, 30 Jan 2025 19:44:57 +0000 Subject: [PATCH 10/12] fix: Use max instead of min to get the min_amount_out between slippaged amount and checked amount Save router's address as Bytes and not String --- don't change below this line --- ENG-4081 Took 7 minutes --- src/encoding/evm/router_encoder.rs | 10 +++++----- src/encoding/evm/strategy_encoder/strategy_encoders.rs | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/encoding/evm/router_encoder.rs b/src/encoding/evm/router_encoder.rs index e870460..7a95612 100644 --- a/src/encoding/evm/router_encoder.rs +++ b/src/encoding/evm/router_encoder.rs @@ -15,7 +15,7 @@ pub struct EVMRouterEncoder { strategy_selector: S, signer: Option, chain: Chain, - router_address: String, + router_address: Bytes, } #[allow(dead_code)] @@ -26,6 +26,8 @@ impl EVMRouterEncoder { signer: Option, chain: Chain, ) -> Result { + let router_address = Bytes::from_str(&router_address) + .map_err(|_| EncodingError::FatalError("Invalid router address".to_string()))?; Ok(EVMRouterEncoder { strategy_selector, signer, chain, router_address }) } } @@ -45,16 +47,14 @@ impl RouterEncoder for EVMRouterEncoder { let router_address = solution .router_address .clone() - .unwrap_or(Bytes::from_str(&self.router_address).map_err(|_| { - EncodingError::FatalError("Invalid router address".to_string()) - })?); + .unwrap_or(self.router_address.clone()); let strategy = self.strategy_selector.select_strategy( solution, self.signer.clone(), self.chain, )?; - let (contract_interaction,target_address) = + let (contract_interaction, target_address) = strategy.encode_strategy(solution.clone(), router_address)?; let value = if solution.native_action.clone().unwrap() == NativeAction::Wrap { diff --git a/src/encoding/evm/strategy_encoder/strategy_encoders.rs b/src/encoding/evm/strategy_encoder/strategy_encoders.rs index 2d776f4..cf840e9 100644 --- a/src/encoding/evm/strategy_encoder/strategy_encoders.rs +++ b/src/encoding/evm/strategy_encoder/strategy_encoders.rs @@ -1,4 +1,4 @@ -use std::{cmp::min, str::FromStr}; +use std::{cmp::max, str::FromStr}; use alloy_primitives::{aliases::U24, map::HashSet, FixedBytes, U256, U8}; use alloy_sol_types::SolValue; @@ -76,7 +76,7 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { let multiplier = &one_hundred - slippage_percent; let expected_amount_with_slippage = (&solution.expected_amount * multiplier) / one_hundred; - min_amount_out = min(min_amount_out, expected_amount_with_slippage); + min_amount_out = max(min_amount_out, expected_amount_with_slippage); } min_amount_out } else { @@ -115,7 +115,7 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { .position(|t| *t == swap.token_in) .ok_or_else(|| { EncodingError::InvalidInput( - "Token in not found in tokens array".to_string(), + "In token not found in tokens array".to_string(), ) })?, ), @@ -125,7 +125,7 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { .position(|t| *t == swap.token_out) .ok_or_else(|| { EncodingError::InvalidInput( - "Token out not found in tokens array".to_string(), + "Out token not found in tokens array".to_string(), ) })?, ), From 01d101acb514e8d97cca12b5a94cd4b5d7ec42ba Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Thu, 30 Jan 2025 19:47:36 +0000 Subject: [PATCH 11/12] chore: Move ple_encode to EVMStrategyEncoder --- don't change below this line --- ENG-4081 Took 3 minutes --- .../evm/strategy_encoder/strategy_encoders.rs | 17 +++++++++++++---- src/encoding/evm/utils.rs | 13 ------------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/encoding/evm/strategy_encoder/strategy_encoders.rs b/src/encoding/evm/strategy_encoder/strategy_encoders.rs index cf840e9..51d840b 100644 --- a/src/encoding/evm/strategy_encoder/strategy_encoders.rs +++ b/src/encoding/evm/strategy_encoder/strategy_encoders.rs @@ -10,9 +10,7 @@ use crate::encoding::{ evm::{ approvals::permit2::Permit2, swap_encoder::SWAP_ENCODER_REGISTRY, - utils::{ - biguint_to_u256, bytes_to_address, encode_input, percentage_to_uint24, ple_encode, - }, + utils::{biguint_to_u256, bytes_to_address, encode_input, percentage_to_uint24}, }, models::{EncodingContext, NativeAction, Solution}, strategy_encoder::StrategyEncoder, @@ -42,6 +40,17 @@ pub trait EVMStrategyEncoder: StrategyEncoder { let hash = keccak256(selector.as_bytes()); FixedBytes::<4>::from([hash[0], hash[1], hash[2], hash[3]]) } + + fn ple_encode(&self, action_data_array: Vec>) -> Vec { + let mut encoded_action_data: Vec = Vec::new(); + + for action_data in action_data_array { + let args = (encoded_action_data, action_data.len() as u16, action_data); + encoded_action_data = args.abi_encode_packed(); + } + + encoded_action_data + } } pub struct SplitSwapStrategyEncoder { @@ -139,7 +148,7 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { swaps.push(swap_data); } - let encoded_swaps = ple_encode(swaps); + let encoded_swaps = self.ple_encode(swaps); let (mut unwrap, mut wrap) = (false, false); if solution.native_action.is_some() { match solution.native_action.unwrap() { diff --git a/src/encoding/evm/utils.rs b/src/encoding/evm/utils.rs index 2111958..a6b4275 100644 --- a/src/encoding/evm/utils.rs +++ b/src/encoding/evm/utils.rs @@ -1,5 +1,4 @@ use alloy_primitives::{aliases::U24, Address, Keccak256, U256}; -use alloy_sol_types::SolValue; use num_bigint::BigUint; use tycho_core::Bytes; @@ -23,18 +22,6 @@ pub fn biguint_to_u256(value: &BigUint) -> U256 { U256::from_be_slice(&bytes) } -#[allow(dead_code)] -pub fn ple_encode(action_data_array: Vec>) -> Vec { - let mut encoded_action_data: Vec = Vec::new(); - - for action_data in action_data_array { - let args = (encoded_action_data, action_data.len() as u16, action_data); - encoded_action_data = args.abi_encode_packed(); - } - - encoded_action_data -} - #[allow(dead_code)] pub fn encode_input(selector: &str, mut encoded_args: Vec) -> Vec { let mut hasher = Keccak256::new(); From 5f3d4406bdfed1d20df5614e74efeb7fcd5cffc1 Mon Sep 17 00:00:00 2001 From: TAMARA LIPOWSKI Date: Thu, 30 Jan 2025 17:19:54 -0500 Subject: [PATCH 12/12] fix: replace all unwraps with proper error handling - Or more elegant assignments in place of the `.is_some()` and `.unwrap()` method. --- src/encoding/evm/router_encoder.rs | 8 ++-- .../evm/strategy_encoder/strategy_encoders.rs | 42 +++++++++++-------- .../evm/strategy_encoder/strategy_selector.rs | 2 +- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/encoding/evm/router_encoder.rs b/src/encoding/evm/router_encoder.rs index 7a95612..896f707 100644 --- a/src/encoding/evm/router_encoder.rs +++ b/src/encoding/evm/router_encoder.rs @@ -57,11 +57,11 @@ impl RouterEncoder for EVMRouterEncoder { let (contract_interaction, target_address) = strategy.encode_strategy(solution.clone(), router_address)?; - let value = if solution.native_action.clone().unwrap() == NativeAction::Wrap { - solution.given_amount.clone() - } else { - BigUint::ZERO + let value = match solution.native_action.as_ref() { + Some(NativeAction::Wrap) => solution.given_amount.clone(), + _ => BigUint::ZERO, }; + transactions.push(Transaction { value, data: contract_interaction, diff --git a/src/encoding/evm/strategy_encoder/strategy_encoders.rs b/src/encoding/evm/strategy_encoder/strategy_encoders.rs index 51d840b..3f2c6d1 100644 --- a/src/encoding/evm/strategy_encoder/strategy_encoders.rs +++ b/src/encoding/evm/strategy_encoder/strategy_encoders.rs @@ -77,20 +77,17 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { &solution.given_token, &solution.given_amount, )?; - let min_amount_out = if solution.check_amount.is_some() { - let mut min_amount_out = solution.check_amount.clone().unwrap(); - if solution.slippage.is_some() { + let mut min_amount_out = BigUint::ZERO; + if let Some(user_specified_min_amount) = solution.check_amount { + if let Some(slippage) = solution.slippage { let one_hundred = BigUint::from(100u32); - let slippage_percent = BigUint::from((solution.slippage.unwrap() * 100.0) as u32); + let slippage_percent = BigUint::from((slippage * 100.0) as u32); let multiplier = &one_hundred - slippage_percent; let expected_amount_with_slippage = (&solution.expected_amount * multiplier) / one_hundred; - min_amount_out = max(min_amount_out, expected_amount_with_slippage); + min_amount_out = max(user_specified_min_amount, expected_amount_with_slippage); } - min_amount_out - } else { - BigUint::ZERO - }; + } let mut tokens: Vec = solution .swaps @@ -106,7 +103,13 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { let mut swaps = vec![]; for swap in solution.swaps.iter() { - let registry = SWAP_ENCODER_REGISTRY.read().unwrap(); + let registry = SWAP_ENCODER_REGISTRY + .read() + .map_err(|_| { + EncodingError::FatalError( + "Failed to read the swap encoder registry".to_string(), + ) + })?; let swap_encoder = registry .get_encoder(&swap.component.protocol_system) .expect("Swap encoder not found"); @@ -150,8 +153,8 @@ impl StrategyEncoder for SplitSwapStrategyEncoder { let encoded_swaps = self.ple_encode(swaps); let (mut unwrap, mut wrap) = (false, false); - if solution.native_action.is_some() { - match solution.native_action.unwrap() { + if let Some(action) = solution.native_action { + match action { NativeAction::Wrap => wrap = true, NativeAction::Unwrap => unwrap = true, } @@ -186,12 +189,16 @@ impl StrategyEncoder for ExecutorStrategyEncoder { solution: Solution, _router_address: Bytes, ) -> Result<(Vec, Bytes), EncodingError> { - if solution.router_address.is_none() { - return Err(EncodingError::InvalidInput( + let router_address = solution.router_address.ok_or_else(|| { + EncodingError::InvalidInput( "Router address is required for straight to pool solutions".to_string(), - )); - } - let swap = solution.swaps.first().unwrap(); + ) + })?; + + let swap = solution + .swaps + .first() + .ok_or_else(|| EncodingError::InvalidInput("No swaps found in solution".to_string()))?; let registry = SWAP_ENCODER_REGISTRY .read() .map_err(|_| { @@ -205,7 +212,6 @@ impl StrategyEncoder for ExecutorStrategyEncoder { swap.component.protocol_system )) })?; - let router_address = solution.router_address.unwrap(); let encoding_context = EncodingContext { receiver: solution.receiver, diff --git a/src/encoding/evm/strategy_encoder/strategy_selector.rs b/src/encoding/evm/strategy_encoder/strategy_selector.rs index b45cebf..3ba9de3 100644 --- a/src/encoding/evm/strategy_encoder/strategy_selector.rs +++ b/src/encoding/evm/strategy_encoder/strategy_selector.rs @@ -24,7 +24,7 @@ impl StrategySelector for EVMStrategySelector { "Signer is required for SplitSwapStrategyEncoder".to_string(), ) })?; - Ok(Box::new(SplitSwapStrategyEncoder::new(signer_pk, chain).unwrap())) + Ok(Box::new(SplitSwapStrategyEncoder::new(signer_pk, chain)?)) } } }