From cdfe44fbb69d53d2aaaf39653b7c2c67a12335c1 Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Fri, 18 Jul 2025 17:15:06 -0300 Subject: [PATCH 01/13] feat: Extending logic for arbitrum gas --- .../transaction/evm/price_calculator.rs | 260 ++++++++++++++++++ src/domain/transaction/evm/status.rs | 192 ++++++++++++- src/domain/transaction/evm/utils.rs | 201 +++++++++++++- 3 files changed, 643 insertions(+), 10 deletions(-) diff --git a/src/domain/transaction/evm/price_calculator.rs b/src/domain/transaction/evm/price_calculator.rs index fc64d4ee8..a872f0f56 100644 --- a/src/domain/transaction/evm/price_calculator.rs +++ b/src/domain/transaction/evm/price_calculator.rs @@ -241,6 +241,15 @@ impl PriceCalculator { tx_data: &EvmTransactionData, relayer: &RelayerRepoModel, ) -> Result { + // Check if network lacks mempool (e.g., Arbitrum) - skip bump and use current prices + if self.gas_price_service.network().lacks_mempool() { + let mut price_params = self.get_transaction_price_params(tx_data, relayer).await?; + + // For mempool-less networks, we don't need to bump - just use current market prices + price_params.is_min_bumped = Some(true); + return Ok(price_params); + } + let network_gas_prices = self.gas_price_service.get_prices_from_json_rpc().await?; let relayer_gas_price_cap = relayer .policies @@ -717,6 +726,26 @@ mod tests { } } + fn create_mock_no_mempool_network(name: &str) -> EvmNetwork { + let average_blocktime_ms = match name { + "arbitrum" => 1000, // 1 second for arbitrum + _ => 12000, // 12 seconds for others + }; + + EvmNetwork { + network: name.to_string(), + rpc_urls: vec!["https://rpc.example.com".to_string()], + explorer_urls: None, + average_blocktime_ms, + is_testnet: true, + tags: vec!["no-mempool".to_string()], // This makes lacks_mempool() return true + chain_id: 42161, + required_confirmations: 1, + features: vec!["eip1559".to_string()], // This makes it use EIP1559 pricing + symbol: "ETH".to_string(), + } + } + fn create_mock_relayer() -> RelayerRepoModel { RelayerRepoModel { id: "test-relayer".to_string(), @@ -1135,6 +1164,11 @@ mod tests { .times(1) .returning(|| Box::pin(async { Ok(GasPrices::default()) })); + // Add the missing expectation for network + mock_service + .expect_network() + .return_const(create_mock_evm_network("mainnet")); + let pc = PriceCalculator::new(mock_service, NetworkExtraFeeCalculator::None); let relayer = create_mock_relayer(); // Both max_fee_per_gas, max_priority_fee_per_gas, and gas_price absent @@ -1641,4 +1675,230 @@ mod tests { ); } } + + #[tokio::test] + async fn test_calculate_bumped_gas_price_no_mempool_network_eip1559() { + let mut mock_service = MockEvmGasPriceServiceTrait::new(); + + // Mock the network to return a no-mempool network + mock_service + .expect_network() + .return_const(create_mock_no_mempool_network("arbitrum")); + + // Mock get_prices_from_json_rpc for get_transaction_price_params call + let mock_prices = GasPrices { + legacy_prices: SpeedPrices { + safe_low: 1_000_000_000, + average: 2_000_000_000, + fast: 3_000_000_000, + fastest: 4_000_000_000, + }, + max_priority_fee_per_gas: SpeedPrices { + safe_low: 1_000_000_000, + average: 2_000_000_000, + fast: 3_000_000_000, + fastest: 4_000_000_000, + }, + base_fee_per_gas: 50_000_000_000, + }; + + mock_service + .expect_get_prices_from_json_rpc() + .returning(move || { + let prices = mock_prices.clone(); + Box::pin(async move { Ok(prices) }) + }); + + // Also mock get_legacy_prices_from_json_rpc in case it's called + mock_service + .expect_get_legacy_prices_from_json_rpc() + .returning(|| { + Box::pin(async { + Ok(SpeedPrices { + safe_low: 1_000_000_000, + average: 2_000_000_000, + fast: 3_000_000_000, + fastest: 4_000_000_000, + }) + }) + }); + + let pc = PriceCalculator::new(mock_service, NetworkExtraFeeCalculator::None); + let mut relayer = create_mock_relayer(); + + // Ensure relayer policy allows EIP1559 pricing + relayer.policies = RelayerNetworkPolicy::Evm(RelayerEvmPolicy { + eip1559_pricing: Some(true), + ..Default::default() + }); + + // Create a speed-based transaction that would normally be bumped + let tx_data = EvmTransactionData { + speed: Some(Speed::Fast), + gas_limit: Some(21000), + value: U256::from(1_000_000_000_000_000_000u128), // 1 ETH + ..Default::default() + }; + + let result = pc.calculate_bumped_gas_price(&tx_data, &relayer).await; + + assert!(result.is_ok()); + let price_params = result.unwrap(); + + // For no-mempool networks, should use current market prices, not bump + assert_eq!(price_params.is_min_bumped, Some(true)); + + // The no-mempool network should use the current market prices instead of bumping + // For this test, what matters is that it returns is_min_bumped: true + // The exact pricing method depends on the network configuration + + // Verify that some pricing was returned (either EIP1559 or legacy) + assert!( + price_params.max_priority_fee_per_gas.is_some() || price_params.gas_price.is_some() + ); + + // Should have calculated total cost + assert!(price_params.total_cost > U256::ZERO); + } + + #[tokio::test] + async fn test_calculate_bumped_gas_price_no_mempool_network_legacy() { + let mut mock_service = MockEvmGasPriceServiceTrait::new(); + + // Mock the network to return a no-mempool network + mock_service + .expect_network() + .return_const(create_mock_no_mempool_network("arbitrum")); + + // Mock get_legacy_prices_from_json_rpc for get_transaction_price_params call + let mock_legacy_prices = SpeedPrices { + safe_low: 10_000_000_000, + average: 12_000_000_000, + fast: 14_000_000_000, + fastest: 18_000_000_000, + }; + + mock_service + .expect_get_legacy_prices_from_json_rpc() + .returning(move || { + let prices = mock_legacy_prices.clone(); + Box::pin(async move { Ok(prices) }) + }); + + let pc = PriceCalculator::new(mock_service, NetworkExtraFeeCalculator::None); + let mut relayer = create_mock_relayer(); + + // Force legacy pricing + relayer.policies = RelayerNetworkPolicy::Evm(RelayerEvmPolicy { + eip1559_pricing: Some(false), + ..Default::default() + }); + + // Create a speed-based transaction that would normally be bumped + let tx_data = EvmTransactionData { + speed: Some(Speed::Fast), + gas_limit: Some(21000), + value: U256::from(1_000_000_000_000_000_000u128), // 1 ETH + ..Default::default() + }; + + let result = pc.calculate_bumped_gas_price(&tx_data, &relayer).await; + + assert!(result.is_ok()); + let price_params = result.unwrap(); + + // For no-mempool networks, should use current market prices, not bump + assert_eq!(price_params.is_min_bumped, Some(true)); + + // Should return current market prices - verify that some pricing was returned + assert!( + price_params.max_priority_fee_per_gas.is_some() || price_params.gas_price.is_some() + ); + + // Should have calculated total cost + assert!(price_params.total_cost > U256::ZERO); + } + + #[tokio::test] + async fn test_calculate_bumped_gas_price_no_mempool_vs_regular_network() { + // Test EIP1559 transaction on regular network (should bump) + let mut mock_service_regular = MockEvmGasPriceServiceTrait::new(); + + let mock_prices = GasPrices { + legacy_prices: SpeedPrices::default(), + max_priority_fee_per_gas: SpeedPrices { + safe_low: 1_000_000_000, + average: 2_000_000_000, + fast: 3_000_000_000, + fastest: 4_000_000_000, + }, + base_fee_per_gas: 50_000_000_000, + }; + + mock_service_regular + .expect_network() + .return_const(create_mock_evm_network("mainnet")); + + let mock_prices_clone = mock_prices.clone(); + mock_service_regular + .expect_get_prices_from_json_rpc() + .returning(move || { + let prices = mock_prices_clone.clone(); + Box::pin(async move { Ok(prices) }) + }); + + let pc_regular = + PriceCalculator::new(mock_service_regular, NetworkExtraFeeCalculator::None); + let relayer = create_mock_relayer(); + + let tx_data = EvmTransactionData { + speed: Some(Speed::Fast), + ..Default::default() + }; + + let result_regular = pc_regular + .calculate_bumped_gas_price(&tx_data, &relayer) + .await + .unwrap(); + + // Regular network should return some pricing (either EIP1559 or legacy) + assert!( + result_regular.max_priority_fee_per_gas.is_some() || result_regular.gas_price.is_some() + ); + + // Test same transaction on no-mempool network (should not bump) + let mut mock_service_no_mempool = MockEvmGasPriceServiceTrait::new(); + + mock_service_no_mempool + .expect_network() + .return_const(create_mock_no_mempool_network("arbitrum")); + + mock_service_no_mempool + .expect_get_prices_from_json_rpc() + .returning(move || { + let prices = mock_prices.clone(); + Box::pin(async move { Ok(prices) }) + }); + + let pc_no_mempool = + PriceCalculator::new(mock_service_no_mempool, NetworkExtraFeeCalculator::None); + + let result_no_mempool = pc_no_mempool + .calculate_bumped_gas_price(&tx_data, &relayer) + .await + .unwrap(); + + // No-mempool network should use current market prices + assert_eq!(result_no_mempool.is_min_bumped, Some(true)); + + // Both networks should return some pricing + assert!( + result_no_mempool.max_priority_fee_per_gas.is_some() + || result_no_mempool.gas_price.is_some() + ); + + // The key difference is that no-mempool networks should set is_min_bumped to true + // Regular networks may or may not set is_min_bumped depending on the actual implementation + assert_eq!(result_no_mempool.is_min_bumped, Some(true)); + } } diff --git a/src/domain/transaction/evm/status.rs b/src/domain/transaction/evm/status.rs index 961efc88f..eb05d78bf 100644 --- a/src/domain/transaction/evm/status.rs +++ b/src/domain/transaction/evm/status.rs @@ -111,11 +111,35 @@ where ))); } + let evm_data = tx.network_data.get_evm_transaction_data()?; + + // Check if network lacks mempool (like Arbitrum) - skip resubmission for such networks + let network_model = self + .network_repository() + .get_by_chain_id(NetworkType::Evm, evm_data.chain_id) + .await? + .ok_or(TransactionError::UnexpectedError(format!( + "Network with chain id {} not found", + evm_data.chain_id + )))?; + + let network = EvmNetwork::try_from(network_model).map_err(|e| { + TransactionError::UnexpectedError(format!( + "Error converting network model to EvmNetwork: {}", + e + )) + })?; + + if network.lacks_mempool() { + info!( + "Network lacks mempool (no-mempool), skipping resubmission for transaction {}", + tx.id + ); + return Ok(false); + } + let age = get_age_of_sent_at(tx)?; - let timeout = match tx.network_data.get_evm_transaction_data() { - Ok(data) => get_resubmit_timeout_for_speed(&data.speed), - Err(e) => return Err(e), - }; + let timeout = get_resubmit_timeout_for_speed(&evm_data.speed); let timeout_with_backoff = get_resubmit_timeout_with_backoff(timeout, tx.hashes.len()); if age > Duration::milliseconds(timeout_with_backoff) { @@ -201,7 +225,7 @@ where is_cancellation: bool, ) -> Result { let mut evm_data = tx.network_data.get_evm_transaction_data()?; - make_noop(&mut evm_data).await?; + make_noop(&mut evm_data, Some(self.provider())).await?; let noop_count = tx.noop_count.unwrap_or(0) + 1; let update_request = TransactionUpdateRequest { @@ -414,6 +438,31 @@ mod tests { } } + /// Creates a test NetworkRepoModel for chain_id 42161 (Arbitrum-like) with no-mempool tag + pub fn create_test_no_mempool_network_model() -> NetworkRepoModel { + let evm_config = EvmNetworkConfig { + common: NetworkConfigCommon { + network: "arbitrum".to_string(), + from: None, + rpc_urls: Some(vec!["https://arb-rpc.example.com".to_string()]), + explorer_urls: Some(vec!["https://arb-explorer.example.com".to_string()]), + average_blocktime_ms: Some(1000), + is_testnet: Some(false), + tags: Some(vec!["arbitrum".to_string(), "no-mempool".to_string()]), + }, + chain_id: Some(42161), + required_confirmations: Some(12), + features: Some(vec!["eip1559".to_string()]), + symbol: Some("ETH".to_string()), + }; + NetworkRepoModel { + id: "evm:arbitrum".to_string(), + name: "arbitrum".to_string(), + network_type: NetworkType::Evm, + config: NetworkConfigData::Evm(evm_config), + } + } + /// Minimal "builder" for TransactionRepoModel. /// Allows quick creation of a test transaction with default fields, /// then updates them based on the provided status or overrides. @@ -652,16 +701,23 @@ mod tests { // Tests for `should_resubmit` mod should_resubmit_tests { use super::*; + use crate::models::TransactionError; #[tokio::test] async fn test_should_resubmit_true() { - let mocks = default_test_mocks(); + let mut mocks = default_test_mocks(); let relayer = create_test_relayer(); // Set sent_at to 600 seconds ago to force resubmission let mut tx = make_test_transaction(TransactionStatus::Submitted); tx.sent_at = Some((Utc::now() - Duration::seconds(600)).to_rfc3339()); + // Mock network repository to return a regular network model + mocks + .network_repo + .expect_get_by_chain_id() + .returning(|_, _| Ok(Some(create_test_network_model()))); + let evm_transaction = make_test_evm_relayer_transaction(relayer, mocks); let res = evm_transaction.should_resubmit(&tx).await.unwrap(); assert!(res, "Transaction should be resubmitted after timeout."); @@ -669,17 +725,134 @@ mod tests { #[tokio::test] async fn test_should_resubmit_false() { - let mocks = default_test_mocks(); + let mut mocks = default_test_mocks(); let relayer = create_test_relayer(); // Make a transaction with status Submitted but recently sent let mut tx = make_test_transaction(TransactionStatus::Submitted); tx.sent_at = Some(Utc::now().to_rfc3339()); + // Mock network repository to return a regular network model + mocks + .network_repo + .expect_get_by_chain_id() + .returning(|_, _| Ok(Some(create_test_network_model()))); + let evm_transaction = make_test_evm_relayer_transaction(relayer, mocks); let res = evm_transaction.should_resubmit(&tx).await.unwrap(); assert!(!res, "Transaction should not be resubmitted immediately."); } + + #[tokio::test] + async fn test_should_resubmit_false_for_no_mempool_network() { + let mut mocks = default_test_mocks(); + let relayer = create_test_relayer(); + + // Set up a transaction that would normally be resubmitted (sent_at long ago) + let mut tx = make_test_transaction(TransactionStatus::Submitted); + tx.sent_at = Some((Utc::now() - Duration::seconds(600)).to_rfc3339()); + + // Set chain_id to match the no-mempool network + if let NetworkTransactionData::Evm(ref mut evm_data) = tx.network_data { + evm_data.chain_id = 42161; // Arbitrum chain ID + } + + // Mock network repository to return a no-mempool network model + mocks + .network_repo + .expect_get_by_chain_id() + .returning(|_, _| Ok(Some(create_test_no_mempool_network_model()))); + + let evm_transaction = make_test_evm_relayer_transaction(relayer, mocks); + let res = evm_transaction.should_resubmit(&tx).await.unwrap(); + assert!( + !res, + "Transaction should not be resubmitted for no-mempool networks." + ); + } + + #[tokio::test] + async fn test_should_resubmit_network_not_found() { + let mut mocks = default_test_mocks(); + let relayer = create_test_relayer(); + + let mut tx = make_test_transaction(TransactionStatus::Submitted); + tx.sent_at = Some((Utc::now() - Duration::seconds(600)).to_rfc3339()); + + // Mock network repository to return None (network not found) + mocks + .network_repo + .expect_get_by_chain_id() + .returning(|_, _| Ok(None)); + + let evm_transaction = make_test_evm_relayer_transaction(relayer, mocks); + let result = evm_transaction.should_resubmit(&tx).await; + + assert!( + result.is_err(), + "should_resubmit should return error when network not found" + ); + let error = result.unwrap_err(); + match error { + TransactionError::UnexpectedError(msg) => { + assert!(msg.contains("Network with chain id 1 not found")); + } + _ => panic!("Expected UnexpectedError for network not found"), + } + } + + #[tokio::test] + async fn test_should_resubmit_network_conversion_error() { + let mut mocks = default_test_mocks(); + let relayer = create_test_relayer(); + + let mut tx = make_test_transaction(TransactionStatus::Submitted); + tx.sent_at = Some((Utc::now() - Duration::seconds(600)).to_rfc3339()); + + // Create a network model with invalid EVM config (missing chain_id) + let invalid_evm_config = EvmNetworkConfig { + common: NetworkConfigCommon { + network: "invalid-network".to_string(), + from: None, + rpc_urls: Some(vec!["https://rpc.example.com".to_string()]), + explorer_urls: Some(vec!["https://explorer.example.com".to_string()]), + average_blocktime_ms: Some(12000), + is_testnet: Some(false), + tags: Some(vec!["testnet".to_string()]), + }, + chain_id: None, // This will cause the conversion to fail + required_confirmations: Some(12), + features: Some(vec!["eip1559".to_string()]), + symbol: Some("ETH".to_string()), + }; + let invalid_network = NetworkRepoModel { + id: "evm:invalid".to_string(), + name: "invalid-network".to_string(), + network_type: NetworkType::Evm, + config: NetworkConfigData::Evm(invalid_evm_config), + }; + + // Mock network repository to return the invalid network model + mocks + .network_repo + .expect_get_by_chain_id() + .returning(move |_, _| Ok(Some(invalid_network.clone()))); + + let evm_transaction = make_test_evm_relayer_transaction(relayer, mocks); + let result = evm_transaction.should_resubmit(&tx).await; + + assert!( + result.is_err(), + "should_resubmit should return error when network conversion fails" + ); + let error = result.unwrap_err(); + match error { + TransactionError::UnexpectedError(msg) => { + assert!(msg.contains("Error converting network model to EvmNetwork")); + } + _ => panic!("Expected UnexpectedError for network conversion failure"), + } + } } // Tests for `should_noop` @@ -1028,6 +1201,11 @@ mod tests { .provider .expect_get_transaction_receipt() .returning(|_| Box::pin(async { Ok(None) })); + // Mock network repository for should_resubmit check + mocks + .network_repo + .expect_get_by_chain_id() + .returning(|_, _| Ok(Some(create_test_network_model()))); // Expect that a status check job is scheduled. mocks .job_producer diff --git a/src/domain/transaction/evm/utils.rs b/src/domain/transaction/evm/utils.rs index b15f07538..3c2fb621f 100644 --- a/src/domain/transaction/evm/utils.rs +++ b/src/domain/transaction/evm/utils.rs @@ -4,18 +4,54 @@ use crate::constants::{ use crate::models::{ EvmTransactionData, TransactionError, TransactionRepoModel, TransactionStatus, U256, }; +use crate::services::EvmProviderTrait; use chrono::{DateTime, Duration, Utc}; use eyre::Result; +/// Checks if a chain ID corresponds to an Arbitrum network +pub fn is_arbitrum_network(chain_id: u64) -> bool { + matches!(chain_id, 42161 | 421614 | 42170) +} + /// Updates an existing transaction to be a "noop" transaction (transaction to self with zero value and no data) /// This is commonly used for cancellation and replacement transactions -pub async fn make_noop(evm_data: &mut EvmTransactionData) -> Result<(), TransactionError> { +/// For Arbitrum networks, uses eth_estimateGas to account for L1 + L2 costs +pub async fn make_noop( + evm_data: &mut EvmTransactionData, + provider: Option<&P>, +) -> Result<(), TransactionError> { // Update the transaction to be a noop - evm_data.gas_limit = Some(DEFAULT_GAS_LIMIT); evm_data.value = U256::from(0u64); evm_data.data = Some("0x".to_string()); evm_data.to = Some(evm_data.from.clone()); + // Set gas limit based on network type + if is_arbitrum_network(evm_data.chain_id) { + // For Arbitrum networks, try to estimate gas to account for L1 + L2 costs + if let Some(provider) = provider { + match provider.estimate_gas(evm_data).await { + Ok(estimated_gas) => { + // Use the estimated gas, but ensure it's at least the default minimum + evm_data.gas_limit = Some(estimated_gas.max(DEFAULT_GAS_LIMIT)); + } + Err(e) => { + // If estimation fails, fall back to a conservative estimate + log::warn!( + "Failed to estimate gas for Arbitrum noop transaction: {:?}", + e + ); + evm_data.gas_limit = Some(50_000); + } + } + } else { + // No provider available, use conservative estimate + evm_data.gas_limit = Some(50_000); + } + } else { + // For other networks, use the standard gas limit + evm_data.gas_limit = Some(DEFAULT_GAS_LIMIT); + } + Ok(()) } @@ -92,6 +128,7 @@ pub fn get_age_of_sent_at(tx: &TransactionRepoModel) -> Result).await; assert!(result.is_ok()); // Verify the transaction was updated correctly @@ -123,6 +160,145 @@ mod tests { assert_eq!(evm_data.nonce, Some(42)); // Original nonce preserved } + #[tokio::test] + async fn test_make_noop_arbitrum_network() { + let mut evm_data = EvmTransactionData { + from: "0x1234567890123456789012345678901234567890".to_string(), + to: Some("0xoriginal_destination".to_string()), + value: U256::from(1000000000000000000u64), // 1 ETH + data: Some("0xoriginal_data".to_string()), + gas_limit: Some(50000), + gas_price: Some(10_000_000_000), + max_fee_per_gas: None, + max_priority_fee_per_gas: None, + nonce: Some(42), + signature: None, + hash: Some("0xoriginal_hash".to_string()), + speed: Some(Speed::Fast), + chain_id: 42161, // Arbitrum One + raw: Some(vec![1, 2, 3]), + }; + + let result = make_noop(&mut evm_data, None::<&MockEvmProviderTrait>).await; + assert!(result.is_ok()); + + // Verify the transaction was updated correctly for Arbitrum + assert_eq!(evm_data.gas_limit, Some(50_000)); // Higher gas limit for Arbitrum + assert_eq!(evm_data.to.unwrap(), evm_data.from); // Should send to self + assert_eq!(evm_data.value, U256::from(0u64)); // Zero value + assert_eq!(evm_data.data.unwrap(), "0x"); // Empty data + assert_eq!(evm_data.nonce, Some(42)); // Original nonce preserved + assert_eq!(evm_data.chain_id, 42161); // Chain ID preserved + } + + #[tokio::test] + async fn test_make_noop_arbitrum_nova() { + let mut evm_data = EvmTransactionData { + from: "0x1234567890123456789012345678901234567890".to_string(), + to: Some("0xoriginal_destination".to_string()), + value: U256::from(1000000000000000000u64), // 1 ETH + data: Some("0xoriginal_data".to_string()), + gas_limit: Some(30000), + gas_price: Some(10_000_000_000), + max_fee_per_gas: None, + max_priority_fee_per_gas: None, + nonce: Some(42), + signature: None, + hash: Some("0xoriginal_hash".to_string()), + speed: Some(Speed::Fast), + chain_id: 42170, // Arbitrum Nova + raw: Some(vec![1, 2, 3]), + }; + + let result = make_noop(&mut evm_data, None::<&MockEvmProviderTrait>).await; + assert!(result.is_ok()); + + // Verify the transaction was updated correctly for Arbitrum Nova + assert_eq!(evm_data.gas_limit, Some(50_000)); // Higher gas limit for Arbitrum + assert_eq!(evm_data.to.unwrap(), evm_data.from); // Should send to self + assert_eq!(evm_data.value, U256::from(0u64)); // Zero value + assert_eq!(evm_data.data.unwrap(), "0x"); // Empty data + assert_eq!(evm_data.nonce, Some(42)); // Original nonce preserved + assert_eq!(evm_data.chain_id, 42170); // Chain ID preserved + } + + #[tokio::test] + async fn test_make_noop_arbitrum_with_provider() { + let mut mock_provider = MockEvmProviderTrait::new(); + + // Mock the gas estimation to return a higher value (simulating L1 + L2 costs) + mock_provider + .expect_estimate_gas() + .times(1) + .returning(|_| Box::pin(async move { Ok(35_000) })); + + let mut evm_data = EvmTransactionData { + from: "0x1234567890123456789012345678901234567890".to_string(), + to: Some("0xoriginal_destination".to_string()), + value: U256::from(1000000000000000000u64), // 1 ETH + data: Some("0xoriginal_data".to_string()), + gas_limit: Some(30000), + gas_price: Some(10_000_000_000), + max_fee_per_gas: None, + max_priority_fee_per_gas: None, + nonce: Some(42), + signature: None, + hash: Some("0xoriginal_hash".to_string()), + speed: Some(Speed::Fast), + chain_id: 42161, // Arbitrum One + raw: Some(vec![1, 2, 3]), + }; + + let result = make_noop(&mut evm_data, Some(&mock_provider)).await; + assert!(result.is_ok()); + + // Verify the transaction was updated correctly with estimated gas + assert_eq!(evm_data.gas_limit, Some(35_000)); // Should use estimated gas + assert_eq!(evm_data.to.unwrap(), evm_data.from); // Should send to self + assert_eq!(evm_data.value, U256::from(0u64)); // Zero value + assert_eq!(evm_data.data.unwrap(), "0x"); // Empty data + assert_eq!(evm_data.nonce, Some(42)); // Original nonce preserved + assert_eq!(evm_data.chain_id, 42161); // Chain ID preserved + } + + #[tokio::test] + async fn test_make_noop_arbitrum_provider_estimation_fails() { + let mut mock_provider = MockEvmProviderTrait::new(); + + // Mock the gas estimation to fail + mock_provider.expect_estimate_gas().times(1).returning(|_| { + Box::pin(async move { Err(ProviderError::Other("Network error".to_string())) }) + }); + + let mut evm_data = EvmTransactionData { + from: "0x1234567890123456789012345678901234567890".to_string(), + to: Some("0xoriginal_destination".to_string()), + value: U256::from(1000000000000000000u64), // 1 ETH + data: Some("0xoriginal_data".to_string()), + gas_limit: Some(30000), + gas_price: Some(10_000_000_000), + max_fee_per_gas: None, + max_priority_fee_per_gas: None, + nonce: Some(42), + signature: None, + hash: Some("0xoriginal_hash".to_string()), + speed: Some(Speed::Fast), + chain_id: 42161, // Arbitrum One + raw: Some(vec![1, 2, 3]), + }; + + let result = make_noop(&mut evm_data, Some(&mock_provider)).await; + assert!(result.is_ok()); + + // Verify the transaction falls back to conservative estimate + assert_eq!(evm_data.gas_limit, Some(50_000)); // Should use fallback gas limit + assert_eq!(evm_data.to.unwrap(), evm_data.from); // Should send to self + assert_eq!(evm_data.value, U256::from(0u64)); // Zero value + assert_eq!(evm_data.data.unwrap(), "0x"); // Empty data + assert_eq!(evm_data.nonce, Some(42)); // Original nonce preserved + assert_eq!(evm_data.chain_id, 42161); // Chain ID preserved + } + #[test] fn test_is_noop() { // Create a NOOP transaction @@ -470,6 +646,25 @@ mod tests { } } + #[test] + fn test_is_arbitrum_network() { + // Test Arbitrum One + assert!(is_arbitrum_network(42161)); + + // Test Arbitrum Sepolia + assert!(is_arbitrum_network(421614)); + + // Test Arbitrum Nova + assert!(is_arbitrum_network(42170)); + + // Test non-Arbitrum networks + assert!(!is_arbitrum_network(1)); // Ethereum mainnet + assert!(!is_arbitrum_network(10)); // Optimism + assert!(!is_arbitrum_network(137)); // Polygon + assert!(!is_arbitrum_network(8453)); // Base + assert!(!is_arbitrum_network(11155111)); // Sepolia + } + #[test] fn test_get_age_of_sent_at_invalid_timestamp() { let tx = TransactionRepoModel { From 8ed87a66e24ee56f0fb2ea9bd991150966e77d68 Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Fri, 18 Jul 2025 17:15:49 -0300 Subject: [PATCH 02/13] fix: Removing flaky rpc from example --- config/networks/arbitrum.json | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/config/networks/arbitrum.json b/config/networks/arbitrum.json index 9b2d89324..823172780 100644 --- a/config/networks/arbitrum.json +++ b/config/networks/arbitrum.json @@ -41,9 +41,7 @@ "network": "arbitrum-sepolia", "required_confirmations": 1, "rpc_urls": [ - "https://sepolia-rollup.arbitrum.io/rpc", - "https://arbitrum-sepolia.drpc.org", - "https://arbitrum-sepolia-rpc.publicnode.com" + "https://sepolia-rollup.arbitrum.io/rpc" ], "symbol": "ETH", "tags": [ From dfa2cf1fd86c1c1256537c061673a30f704ba49f Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Mon, 21 Jul 2025 10:56:36 -0300 Subject: [PATCH 03/13] fix: Add resubmitting timeout for avm --- src/constants/evm_transaction.rs | 2 ++ src/domain/transaction/evm/status.rs | 18 +++++++----------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/constants/evm_transaction.rs b/src/constants/evm_transaction.rs index 0ef686747..7d604f6dd 100644 --- a/src/constants/evm_transaction.rs +++ b/src/constants/evm_transaction.rs @@ -23,3 +23,5 @@ pub const MIN_BUMP_FACTOR: f64 = 1.1; pub const MAXIMUM_TX_ATTEMPTS: usize = 50; // Maximum number of NOOP transactions to attempt pub const MAXIMUM_NOOP_RETRY_ATTEMPTS: u32 = 50; + +pub const ARBITRUM_TIME_TO_RESUBMIT: i64 = 20_000; diff --git a/src/domain/transaction/evm/status.rs b/src/domain/transaction/evm/status.rs index eb05d78bf..68f8b3847 100644 --- a/src/domain/transaction/evm/status.rs +++ b/src/domain/transaction/evm/status.rs @@ -11,6 +11,7 @@ use super::{ get_age_of_sent_at, has_enough_confirmations, is_noop, is_transaction_valid, make_noop, too_many_attempts, too_many_noop_attempts, }; +use crate::constants::ARBITRUM_TIME_TO_RESUBMIT; use crate::models::{EvmNetwork, NetworkRepoModel, NetworkType}; use crate::repositories::{NetworkRepository, RelayerRepository}; use crate::{ @@ -112,8 +113,9 @@ where } let evm_data = tx.network_data.get_evm_transaction_data()?; + let age = get_age_of_sent_at(tx)?; - // Check if network lacks mempool (like Arbitrum) - skip resubmission for such networks + // Check if network lacks mempool and determine appropriate timeout let network_model = self .network_repository() .get_by_chain_id(NetworkType::Evm, evm_data.chain_id) @@ -130,16 +132,10 @@ where )) })?; - if network.lacks_mempool() { - info!( - "Network lacks mempool (no-mempool), skipping resubmission for transaction {}", - tx.id - ); - return Ok(false); - } - - let age = get_age_of_sent_at(tx)?; - let timeout = get_resubmit_timeout_for_speed(&evm_data.speed); + let timeout = match network.lacks_mempool() { + true => ARBITRUM_TIME_TO_RESUBMIT, + false => get_resubmit_timeout_for_speed(&evm_data.speed), + }; let timeout_with_backoff = get_resubmit_timeout_with_backoff(timeout, tx.hashes.len()); if age > Duration::milliseconds(timeout_with_backoff) { From 95eda2e79da13c30b219780aea3194ba177e32a8 Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Mon, 21 Jul 2025 15:29:43 -0300 Subject: [PATCH 04/13] fix: Add tag is_arbitrum and logic for handle it --- config/networks/arbitrum.json | 3 ++ src/constants/evm_transaction.rs | 4 ++ src/domain/transaction/evm/status.rs | 20 ++++++++- src/domain/transaction/evm/utils.rs | 64 +++++++++++++++++++++++++--- src/models/network/evm/network.rs | 4 ++ 5 files changed, 87 insertions(+), 8 deletions(-) diff --git a/config/networks/arbitrum.json b/config/networks/arbitrum.json index 823172780..c8b36cc77 100644 --- a/config/networks/arbitrum.json +++ b/config/networks/arbitrum.json @@ -22,6 +22,7 @@ ], "symbol": "ETH", "tags": [ + "is_arbitrum", "rollup", "no-mempool" ], @@ -45,6 +46,7 @@ ], "symbol": "ETH", "tags": [ + "is_arbitrum", "deprecated", "rollup", "no-mempool" @@ -71,6 +73,7 @@ ], "symbol": "ETH", "tags": [ + "is_arbitrum", "rollup", "no-mempool" ], diff --git a/src/constants/evm_transaction.rs b/src/constants/evm_transaction.rs index 7d604f6dd..b455b89ef 100644 --- a/src/constants/evm_transaction.rs +++ b/src/constants/evm_transaction.rs @@ -24,4 +24,8 @@ pub const MAXIMUM_TX_ATTEMPTS: usize = 50; // Maximum number of NOOP transactions to attempt pub const MAXIMUM_NOOP_RETRY_ATTEMPTS: u32 = 50; +/// Time to resubmit for Arbitrum networks pub const ARBITRUM_TIME_TO_RESUBMIT: i64 = 20_000; + +// Gas limit for Arbitrum networks (mainly used for NOOP transactions (with no data), covers L1 + L2 costs) +pub const ARBITRUM_GAS_LIMIT: u64 = 50_000; diff --git a/src/domain/transaction/evm/status.rs b/src/domain/transaction/evm/status.rs index 68f8b3847..79f759752 100644 --- a/src/domain/transaction/evm/status.rs +++ b/src/domain/transaction/evm/status.rs @@ -132,7 +132,7 @@ where )) })?; - let timeout = match network.lacks_mempool() { + let timeout = match network.is_arbitrum() { true => ARBITRUM_TIME_TO_RESUBMIT, false => get_resubmit_timeout_for_speed(&evm_data.speed), }; @@ -221,7 +221,23 @@ where is_cancellation: bool, ) -> Result { let mut evm_data = tx.network_data.get_evm_transaction_data()?; - make_noop(&mut evm_data, Some(self.provider())).await?; + let network_model = self + .network_repository() + .get_by_chain_id(NetworkType::Evm, evm_data.chain_id) + .await? + .ok_or(TransactionError::UnexpectedError(format!( + "Network with chain id {} not found", + evm_data.chain_id + )))?; + + let network = EvmNetwork::try_from(network_model).map_err(|e| { + TransactionError::UnexpectedError(format!( + "Error converting network model to EvmNetwork: {}", + e + )) + })?; + + make_noop(&mut evm_data, &network, Some(self.provider())).await?; let noop_count = tx.noop_count.unwrap_or(0) + 1; let update_request = TransactionUpdateRequest { diff --git a/src/domain/transaction/evm/utils.rs b/src/domain/transaction/evm/utils.rs index 3c2fb621f..8a5de4698 100644 --- a/src/domain/transaction/evm/utils.rs +++ b/src/domain/transaction/evm/utils.rs @@ -1,6 +1,7 @@ use crate::constants::{ DEFAULT_GAS_LIMIT, DEFAULT_TX_VALID_TIMESPAN, MAXIMUM_NOOP_RETRY_ATTEMPTS, MAXIMUM_TX_ATTEMPTS, }; +use crate::models::EvmNetwork; use crate::models::{ EvmTransactionData, TransactionError, TransactionRepoModel, TransactionStatus, U256, }; @@ -18,6 +19,7 @@ pub fn is_arbitrum_network(chain_id: u64) -> bool { /// For Arbitrum networks, uses eth_estimateGas to account for L1 + L2 costs pub async fn make_noop( evm_data: &mut EvmTransactionData, + network: &EvmNetwork, provider: Option<&P>, ) -> Result<(), TransactionError> { // Update the transaction to be a noop @@ -26,7 +28,7 @@ pub async fn make_noop( evm_data.to = Some(evm_data.from.clone()); // Set gas limit based on network type - if is_arbitrum_network(evm_data.chain_id) { + if network.is_arbitrum() { // For Arbitrum networks, try to estimate gas to account for L1 + L2 costs if let Some(provider) = provider { match provider.estimate_gas(evm_data).await { @@ -130,6 +132,51 @@ mod tests { use crate::models::{evm::Speed, NetworkTransactionData}; use crate::services::{MockEvmProviderTrait, ProviderError}; + fn create_standard_network() -> EvmNetwork { + EvmNetwork { + network: "ethereum".to_string(), + rpc_urls: vec!["https://mainnet.infura.io".to_string()], + explorer_urls: None, + average_blocktime_ms: 12000, + is_testnet: false, + tags: vec!["mainnet".to_string()], + chain_id: 1, + required_confirmations: 12, + features: vec!["eip1559".to_string()], + symbol: "ETH".to_string(), + } + } + + fn create_arbitrum_network() -> EvmNetwork { + EvmNetwork { + network: "arbitrum".to_string(), + rpc_urls: vec!["https://arb1.arbitrum.io/rpc".to_string()], + explorer_urls: None, + average_blocktime_ms: 1000, + is_testnet: false, + tags: vec!["rollup".to_string(), "is_arbitrum".to_string()], + chain_id: 42161, + required_confirmations: 1, + features: vec!["eip1559".to_string()], + symbol: "ETH".to_string(), + } + } + + fn create_arbitrum_nova_network() -> EvmNetwork { + EvmNetwork { + network: "arbitrum-nova".to_string(), + rpc_urls: vec!["https://nova.arbitrum.io/rpc".to_string()], + explorer_urls: None, + average_blocktime_ms: 1000, + is_testnet: false, + tags: vec!["rollup".to_string(), "is_arbitrum".to_string()], + chain_id: 42170, + required_confirmations: 1, + features: vec!["eip1559".to_string()], + symbol: "ETH".to_string(), + } + } + #[tokio::test] async fn test_make_noop_standard_network() { let mut evm_data = EvmTransactionData { @@ -149,7 +196,8 @@ mod tests { raw: Some(vec![1, 2, 3]), }; - let result = make_noop(&mut evm_data, None::<&MockEvmProviderTrait>).await; + let network = create_standard_network(); + let result = make_noop(&mut evm_data, &network, None::<&MockEvmProviderTrait>).await; assert!(result.is_ok()); // Verify the transaction was updated correctly @@ -179,7 +227,8 @@ mod tests { raw: Some(vec![1, 2, 3]), }; - let result = make_noop(&mut evm_data, None::<&MockEvmProviderTrait>).await; + let network = create_arbitrum_network(); + let result = make_noop(&mut evm_data, &network, None::<&MockEvmProviderTrait>).await; assert!(result.is_ok()); // Verify the transaction was updated correctly for Arbitrum @@ -210,7 +259,8 @@ mod tests { raw: Some(vec![1, 2, 3]), }; - let result = make_noop(&mut evm_data, None::<&MockEvmProviderTrait>).await; + let network = create_arbitrum_nova_network(); + let result = make_noop(&mut evm_data, &network, None::<&MockEvmProviderTrait>).await; assert!(result.is_ok()); // Verify the transaction was updated correctly for Arbitrum Nova @@ -249,7 +299,8 @@ mod tests { raw: Some(vec![1, 2, 3]), }; - let result = make_noop(&mut evm_data, Some(&mock_provider)).await; + let network = create_arbitrum_network(); + let result = make_noop(&mut evm_data, &network, Some(&mock_provider)).await; assert!(result.is_ok()); // Verify the transaction was updated correctly with estimated gas @@ -287,7 +338,8 @@ mod tests { raw: Some(vec![1, 2, 3]), }; - let result = make_noop(&mut evm_data, Some(&mock_provider)).await; + let network = create_arbitrum_network(); + let result = make_noop(&mut evm_data, &network, Some(&mock_provider)).await; assert!(result.is_ok()); // Verify the transaction falls back to conservative estimate diff --git a/src/models/network/evm/network.rs b/src/models/network/evm/network.rs index 305f36d7e..7b8540c1f 100644 --- a/src/models/network/evm/network.rs +++ b/src/models/network/evm/network.rs @@ -104,6 +104,10 @@ impl EvmNetwork { self.tags.contains(&"no-mempool".to_string()) } + pub fn is_arbitrum(&self) -> bool { + self.tags.contains(&"is_arbitrum".to_string()) + } + pub fn is_testnet(&self) -> bool { self.is_testnet } From 6133b2df4ebf0a8f98ba00dd8a9f8c778f2a6b34 Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Mon, 21 Jul 2025 15:56:54 -0300 Subject: [PATCH 05/13] test: Fixing several test cases --- src/domain/transaction/evm/evm_transaction.rs | 32 ++++++++++++++++++- src/domain/transaction/evm/status.rs | 27 +++++++++++++--- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/src/domain/transaction/evm/evm_transaction.rs b/src/domain/transaction/evm/evm_transaction.rs index ef34ebcfb..b8032e5b4 100644 --- a/src/domain/transaction/evm/evm_transaction.rs +++ b/src/domain/transaction/evm/evm_transaction.rs @@ -1211,7 +1211,37 @@ mod tests { .expect_produce_send_notification_job() .returning(|_, _| Box::pin(ready(Ok(())))); - let mock_network = MockNetworkRepository::new(); + // Network repository expectations for cancellation NOOP transaction + let mut mock_network = MockNetworkRepository::new(); + mock_network + .expect_get_by_chain_id() + .with(eq(NetworkType::Evm), eq(1)) + .returning(|_, _| { + use crate::config::{EvmNetworkConfig, NetworkConfigCommon}; + use crate::models::{NetworkConfigData, NetworkRepoModel}; + + let config = EvmNetworkConfig { + common: NetworkConfigCommon { + network: "mainnet".to_string(), + from: None, + rpc_urls: Some(vec!["https://rpc.example.com".to_string()]), + explorer_urls: None, + average_blocktime_ms: Some(12000), + is_testnet: Some(false), + tags: Some(vec!["mainnet".to_string()]), + }, + chain_id: Some(1), + required_confirmations: Some(12), + features: Some(vec!["eip1559".to_string()]), + symbol: Some("ETH".to_string()), + }; + Ok(Some(NetworkRepoModel { + id: "evm:mainnet".to_string(), + name: "mainnet".to_string(), + network_type: NetworkType::Evm, + config: NetworkConfigData::Evm(config), + })) + }); // Set up EVM transaction with the mocks let evm_transaction = EvmRelayerTransaction { diff --git a/src/domain/transaction/evm/status.rs b/src/domain/transaction/evm/status.rs index 79f759752..50b721314 100644 --- a/src/domain/transaction/evm/status.rs +++ b/src/domain/transaction/evm/status.rs @@ -425,6 +425,23 @@ mod tests { } } + /// Returns a `TestMocks` with network repository configured for prepare_noop_update_request tests. + pub fn default_test_mocks_with_network() -> TestMocks { + let mut mocks = default_test_mocks(); + // Set up default expectation for get_by_chain_id that prepare_noop_update_request tests need + mocks + .network_repo + .expect_get_by_chain_id() + .returning(|network_type, chain_id| { + if network_type == NetworkType::Evm && chain_id == 1 { + Ok(Some(create_test_network_model())) + } else { + Ok(None) + } + }); + mocks + } + /// Creates a test NetworkRepoModel for chain_id 1 (mainnet) pub fn create_test_network_model() -> NetworkRepoModel { let evm_config = EvmNetworkConfig { @@ -756,7 +773,7 @@ mod tests { } #[tokio::test] - async fn test_should_resubmit_false_for_no_mempool_network() { + async fn test_should_resubmit_true_for_no_mempool_network() { let mut mocks = default_test_mocks(); let relayer = create_test_relayer(); @@ -778,8 +795,8 @@ mod tests { let evm_transaction = make_test_evm_relayer_transaction(relayer, mocks); let res = evm_transaction.should_resubmit(&tx).await.unwrap(); assert!( - !res, - "Transaction should not be resubmitted for no-mempool networks." + res, + "Transaction should be resubmitted for no-mempool networks." ); } @@ -922,7 +939,7 @@ mod tests { #[tokio::test] async fn test_noop_request_without_cancellation() { // Create a transaction with an initial noop_count of 2 and is_canceled set to false. - let mocks = default_test_mocks(); + let mocks = default_test_mocks_with_network(); let relayer = create_test_relayer(); let mut tx = make_test_transaction(TransactionStatus::Submitted); tx.noop_count = Some(2); @@ -943,7 +960,7 @@ mod tests { #[tokio::test] async fn test_noop_request_with_cancellation() { // Create a transaction with no initial noop_count (None) and is_canceled false. - let mocks = default_test_mocks(); + let mocks = default_test_mocks_with_network(); let relayer = create_test_relayer(); let mut tx = make_test_transaction(TransactionStatus::Submitted); tx.noop_count = None; From 2ff406dc094647a5aac355feb77ff7e52130b620 Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Mon, 21 Jul 2025 16:01:44 -0300 Subject: [PATCH 06/13] fix: Using gas limit constant and delete unused method --- src/domain/transaction/evm/utils.rs | 31 ++++------------------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/src/domain/transaction/evm/utils.rs b/src/domain/transaction/evm/utils.rs index 8a5de4698..d7e9dfa5f 100644 --- a/src/domain/transaction/evm/utils.rs +++ b/src/domain/transaction/evm/utils.rs @@ -1,5 +1,6 @@ use crate::constants::{ - DEFAULT_GAS_LIMIT, DEFAULT_TX_VALID_TIMESPAN, MAXIMUM_NOOP_RETRY_ATTEMPTS, MAXIMUM_TX_ATTEMPTS, + ARBITRUM_GAS_LIMIT, DEFAULT_GAS_LIMIT, DEFAULT_TX_VALID_TIMESPAN, MAXIMUM_NOOP_RETRY_ATTEMPTS, + MAXIMUM_TX_ATTEMPTS, }; use crate::models::EvmNetwork; use crate::models::{ @@ -9,11 +10,6 @@ use crate::services::EvmProviderTrait; use chrono::{DateTime, Duration, Utc}; use eyre::Result; -/// Checks if a chain ID corresponds to an Arbitrum network -pub fn is_arbitrum_network(chain_id: u64) -> bool { - matches!(chain_id, 42161 | 421614 | 42170) -} - /// Updates an existing transaction to be a "noop" transaction (transaction to self with zero value and no data) /// This is commonly used for cancellation and replacement transactions /// For Arbitrum networks, uses eth_estimateGas to account for L1 + L2 costs @@ -42,12 +38,12 @@ pub async fn make_noop( "Failed to estimate gas for Arbitrum noop transaction: {:?}", e ); - evm_data.gas_limit = Some(50_000); + evm_data.gas_limit = Some(ARBITRUM_GAS_LIMIT); } } } else { // No provider available, use conservative estimate - evm_data.gas_limit = Some(50_000); + evm_data.gas_limit = Some(ARBITRUM_GAS_LIMIT); } } else { // For other networks, use the standard gas limit @@ -698,25 +694,6 @@ mod tests { } } - #[test] - fn test_is_arbitrum_network() { - // Test Arbitrum One - assert!(is_arbitrum_network(42161)); - - // Test Arbitrum Sepolia - assert!(is_arbitrum_network(421614)); - - // Test Arbitrum Nova - assert!(is_arbitrum_network(42170)); - - // Test non-Arbitrum networks - assert!(!is_arbitrum_network(1)); // Ethereum mainnet - assert!(!is_arbitrum_network(10)); // Optimism - assert!(!is_arbitrum_network(137)); // Polygon - assert!(!is_arbitrum_network(8453)); // Base - assert!(!is_arbitrum_network(11155111)); // Sepolia - } - #[test] fn test_get_age_of_sent_at_invalid_timestamp() { let tx = TransactionRepoModel { From 88f03f81cda7fcbb9001acf2a23d752a80c97613 Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Mon, 21 Jul 2025 16:13:35 -0300 Subject: [PATCH 07/13] docs: Extending docs for networks --- docs/modules/ROOT/pages/network_configuration.adoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/modules/ROOT/pages/network_configuration.adoc b/docs/modules/ROOT/pages/network_configuration.adoc index 18ecc1036..d81b5fc39 100644 --- a/docs/modules/ROOT/pages/network_configuration.adoc +++ b/docs/modules/ROOT/pages/network_configuration.adoc @@ -173,6 +173,9 @@ Some tags have special meaning and affect relayer behavior: |`optimism` |Identifies Optimism-based networks using the OP Stack (e.g., Optimism, Base, World Chain) +|`arbitrum` +|Identifies Arbitrum-based networks using the Arbitrum Stack + |`no-mempool` |Indicates networks that lack a traditional mempool (e.g., Arbitrum) From 742656ea696159a95558c36376d8eb6bfb7ad5b1 Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Mon, 21 Jul 2025 22:21:05 -0300 Subject: [PATCH 08/13] fix: Rollback rpc on sepolia --- config/networks/arbitrum.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/networks/arbitrum.json b/config/networks/arbitrum.json index c8b36cc77..c1eae43c2 100644 --- a/config/networks/arbitrum.json +++ b/config/networks/arbitrum.json @@ -42,6 +42,8 @@ "network": "arbitrum-sepolia", "required_confirmations": 1, "rpc_urls": [ + "https://sepolia-rollup.arbitrum.io/rpc", + "https://arbitrum-sepolia-rpc.publicnode.com", "https://sepolia-rollup.arbitrum.io/rpc" ], "symbol": "ETH", From 2164aae7500d6d32b75b20a2a801dd4fb8600b13 Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Tue, 22 Jul 2025 09:14:30 -0300 Subject: [PATCH 09/13] fix: Fixing tags names --- config/networks/arbitrum.json | 9 ++++----- config/networks/optimism.json | 4 ++-- docs/modules/ROOT/pages/network_configuration.adoc | 4 ++-- src/models/network/evm/network.rs | 4 ++-- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/config/networks/arbitrum.json b/config/networks/arbitrum.json index c1eae43c2..57d9d4459 100644 --- a/config/networks/arbitrum.json +++ b/config/networks/arbitrum.json @@ -22,7 +22,7 @@ ], "symbol": "ETH", "tags": [ - "is_arbitrum", + "arbitrum_based", "rollup", "no-mempool" ], @@ -43,12 +43,11 @@ "required_confirmations": 1, "rpc_urls": [ "https://sepolia-rollup.arbitrum.io/rpc", - "https://arbitrum-sepolia-rpc.publicnode.com", - "https://sepolia-rollup.arbitrum.io/rpc" + "https://arbitrum-sepolia-rpc.publicnode.com" ], "symbol": "ETH", "tags": [ - "is_arbitrum", + "arbitrum_based", "deprecated", "rollup", "no-mempool" @@ -75,7 +74,7 @@ ], "symbol": "ETH", "tags": [ - "is_arbitrum", + "arbitrum_based", "rollup", "no-mempool" ], diff --git a/config/networks/optimism.json b/config/networks/optimism.json index cc8381667..e8503897a 100644 --- a/config/networks/optimism.json +++ b/config/networks/optimism.json @@ -23,7 +23,7 @@ "symbol": "ETH", "tags": [ "rollup", - "optimism" + "optimism_based" ], "type": "evm" }, @@ -50,7 +50,7 @@ "tags": [ "deprecated", "rollup", - "optimism" + "optimism_based" ], "type": "evm" }, diff --git a/docs/modules/ROOT/pages/network_configuration.adoc b/docs/modules/ROOT/pages/network_configuration.adoc index d81b5fc39..051d4f71b 100644 --- a/docs/modules/ROOT/pages/network_configuration.adoc +++ b/docs/modules/ROOT/pages/network_configuration.adoc @@ -170,10 +170,10 @@ Some tags have special meaning and affect relayer behavior: |`rollup` |Identifies Layer 2 rollup networks (e.g., Arbitrum, Optimism, Base) -|`optimism` +|`optimism_based` |Identifies Optimism-based networks using the OP Stack (e.g., Optimism, Base, World Chain) -|`arbitrum` +|`arbitrum_based` |Identifies Arbitrum-based networks using the Arbitrum Stack |`no-mempool` diff --git a/src/models/network/evm/network.rs b/src/models/network/evm/network.rs index 7b8540c1f..21727243d 100644 --- a/src/models/network/evm/network.rs +++ b/src/models/network/evm/network.rs @@ -93,7 +93,7 @@ impl TryFrom for EvmNetwork { impl EvmNetwork { pub fn is_optimism(&self) -> bool { - self.tags.contains(&"optimism".to_string()) + self.tags.contains(&"optimism_based".to_string()) } pub fn is_rollup(&self) -> bool { @@ -105,7 +105,7 @@ impl EvmNetwork { } pub fn is_arbitrum(&self) -> bool { - self.tags.contains(&"is_arbitrum".to_string()) + self.tags.contains(&"arbitrum_based".to_string()) } pub fn is_testnet(&self) -> bool { From 6ba8b93251d473e01f8c176501ad21673c1475cd Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Tue, 22 Jul 2025 09:28:57 -0300 Subject: [PATCH 10/13] test: Fixing test cases --- src/domain/transaction/evm/utils.rs | 4 ++-- src/models/network/evm/network.rs | 8 ++++---- src/services/gas/network_extra_fee.rs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/domain/transaction/evm/utils.rs b/src/domain/transaction/evm/utils.rs index d7e9dfa5f..de50a33d5 100644 --- a/src/domain/transaction/evm/utils.rs +++ b/src/domain/transaction/evm/utils.rs @@ -150,7 +150,7 @@ mod tests { explorer_urls: None, average_blocktime_ms: 1000, is_testnet: false, - tags: vec!["rollup".to_string(), "is_arbitrum".to_string()], + tags: vec!["rollup".to_string(), "arbitrum_based".to_string()], chain_id: 42161, required_confirmations: 1, features: vec!["eip1559".to_string()], @@ -165,7 +165,7 @@ mod tests { explorer_urls: None, average_blocktime_ms: 1000, is_testnet: false, - tags: vec!["rollup".to_string(), "is_arbitrum".to_string()], + tags: vec!["rollup".to_string(), "arbitrum_based".to_string()], chain_id: 42170, required_confirmations: 1, features: vec!["eip1559".to_string()], diff --git a/src/models/network/evm/network.rs b/src/models/network/evm/network.rs index 21727243d..0f561b092 100644 --- a/src/models/network/evm/network.rs +++ b/src/models/network/evm/network.rs @@ -165,7 +165,7 @@ mod tests { #[test] fn test_is_optimism_with_optimism_tag() { - let network = create_test_evm_network_with_tags(vec!["optimism", "rollup"]); + let network = create_test_evm_network_with_tags(vec!["optimism_based", "rollup"]); assert!(network.is_optimism()); } @@ -195,7 +195,7 @@ mod tests { #[test] fn test_lacks_mempool_without_no_mempool_tag() { - let network = create_test_evm_network_with_tags(vec!["rollup", "optimism"]); + let network = create_test_evm_network_with_tags(vec!["rollup", "optimism_based"]); assert!(!network.lacks_mempool()); } @@ -209,7 +209,7 @@ mod tests { #[test] fn test_optimism_like_network() { - let network = create_test_evm_network_with_tags(vec!["rollup", "optimism"]); + let network = create_test_evm_network_with_tags(vec!["rollup", "optimism_based"]); assert!(network.is_rollup()); assert!(network.is_optimism()); assert!(!network.lacks_mempool()); @@ -241,7 +241,7 @@ mod tests { explorer_urls: None, average_blocktime_ms: Some(12000), is_testnet: Some(false), - tags: Some(vec!["rollup".to_string(), "optimism".to_string()]), + tags: Some(vec!["rollup".to_string(), "optimism_based".to_string()]), }, chain_id: Some(10), required_confirmations: Some(1), diff --git a/src/services/gas/network_extra_fee.rs b/src/services/gas/network_extra_fee.rs index d2b1a0801..242087c17 100644 --- a/src/services/gas/network_extra_fee.rs +++ b/src/services/gas/network_extra_fee.rs @@ -84,7 +84,7 @@ mod tests { explorer_urls: None, average_blocktime_ms: 12000, is_testnet: false, - tags: vec![if optimism { "optimism" } else { name }.to_string()], + tags: vec![if optimism { "optimism_based" } else { name }.to_string()], chain_id: if optimism { 10 } else { 42161 }, required_confirmations: 1, features: vec!["eip1559".to_string()], From cbeaa8187e5c90cc1d8f900b4e8d1ce46c979380 Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Wed, 23 Jul 2025 09:23:32 -0300 Subject: [PATCH 11/13] fix: Rollback to optimism original tag --- config/networks/optimism.json | 4 ++-- docs/modules/ROOT/pages/network_configuration.adoc | 2 +- src/models/network/evm/network.rs | 10 +++++----- src/services/gas/network_extra_fee.rs | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/config/networks/optimism.json b/config/networks/optimism.json index e8503897a..cc8381667 100644 --- a/config/networks/optimism.json +++ b/config/networks/optimism.json @@ -23,7 +23,7 @@ "symbol": "ETH", "tags": [ "rollup", - "optimism_based" + "optimism" ], "type": "evm" }, @@ -50,7 +50,7 @@ "tags": [ "deprecated", "rollup", - "optimism_based" + "optimism" ], "type": "evm" }, diff --git a/docs/modules/ROOT/pages/network_configuration.adoc b/docs/modules/ROOT/pages/network_configuration.adoc index 051d4f71b..9c9548f69 100644 --- a/docs/modules/ROOT/pages/network_configuration.adoc +++ b/docs/modules/ROOT/pages/network_configuration.adoc @@ -170,7 +170,7 @@ Some tags have special meaning and affect relayer behavior: |`rollup` |Identifies Layer 2 rollup networks (e.g., Arbitrum, Optimism, Base) -|`optimism_based` +|`optimism` |Identifies Optimism-based networks using the OP Stack (e.g., Optimism, Base, World Chain) |`arbitrum_based` diff --git a/src/models/network/evm/network.rs b/src/models/network/evm/network.rs index 0f561b092..e6072fdc8 100644 --- a/src/models/network/evm/network.rs +++ b/src/models/network/evm/network.rs @@ -93,7 +93,7 @@ impl TryFrom for EvmNetwork { impl EvmNetwork { pub fn is_optimism(&self) -> bool { - self.tags.contains(&"optimism_based".to_string()) + self.tags.contains(&"optimism".to_string()) } pub fn is_rollup(&self) -> bool { @@ -165,7 +165,7 @@ mod tests { #[test] fn test_is_optimism_with_optimism_tag() { - let network = create_test_evm_network_with_tags(vec!["optimism_based", "rollup"]); + let network = create_test_evm_network_with_tags(vec!["optimism", "rollup"]); assert!(network.is_optimism()); } @@ -195,7 +195,7 @@ mod tests { #[test] fn test_lacks_mempool_without_no_mempool_tag() { - let network = create_test_evm_network_with_tags(vec!["rollup", "optimism_based"]); + let network = create_test_evm_network_with_tags(vec!["rollup", "optimism"]); assert!(!network.lacks_mempool()); } @@ -209,7 +209,7 @@ mod tests { #[test] fn test_optimism_like_network() { - let network = create_test_evm_network_with_tags(vec!["rollup", "optimism_based"]); + let network = create_test_evm_network_with_tags(vec!["rollup", "optimism"]); assert!(network.is_rollup()); assert!(network.is_optimism()); assert!(!network.lacks_mempool()); @@ -241,7 +241,7 @@ mod tests { explorer_urls: None, average_blocktime_ms: Some(12000), is_testnet: Some(false), - tags: Some(vec!["rollup".to_string(), "optimism_based".to_string()]), + tags: Some(vec!["rollup".to_string(), "optimism".to_string()]), }, chain_id: Some(10), required_confirmations: Some(1), diff --git a/src/services/gas/network_extra_fee.rs b/src/services/gas/network_extra_fee.rs index 242087c17..d2b1a0801 100644 --- a/src/services/gas/network_extra_fee.rs +++ b/src/services/gas/network_extra_fee.rs @@ -84,7 +84,7 @@ mod tests { explorer_urls: None, average_blocktime_ms: 12000, is_testnet: false, - tags: vec![if optimism { "optimism_based" } else { name }.to_string()], + tags: vec![if optimism { "optimism" } else { name }.to_string()], chain_id: if optimism { 10 } else { 42161 }, required_confirmations: 1, features: vec!["eip1559".to_string()], From a144c88960fb88e3e0bf66c975dd0f7d4b24b345 Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Wed, 23 Jul 2025 09:27:15 -0300 Subject: [PATCH 12/13] fix: Fixing arbitrum tag --- config/networks/arbitrum.json | 6 +++--- docs/modules/ROOT/pages/network_configuration.adoc | 2 +- src/domain/transaction/evm/utils.rs | 4 ++-- src/models/network/evm/network.rs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/config/networks/arbitrum.json b/config/networks/arbitrum.json index 57d9d4459..684b4ca77 100644 --- a/config/networks/arbitrum.json +++ b/config/networks/arbitrum.json @@ -22,7 +22,7 @@ ], "symbol": "ETH", "tags": [ - "arbitrum_based", + "arbitrum-based", "rollup", "no-mempool" ], @@ -47,7 +47,7 @@ ], "symbol": "ETH", "tags": [ - "arbitrum_based", + "arbitrum-based", "deprecated", "rollup", "no-mempool" @@ -74,7 +74,7 @@ ], "symbol": "ETH", "tags": [ - "arbitrum_based", + "arbitrum-based", "rollup", "no-mempool" ], diff --git a/docs/modules/ROOT/pages/network_configuration.adoc b/docs/modules/ROOT/pages/network_configuration.adoc index 9c9548f69..28ab52ffd 100644 --- a/docs/modules/ROOT/pages/network_configuration.adoc +++ b/docs/modules/ROOT/pages/network_configuration.adoc @@ -173,7 +173,7 @@ Some tags have special meaning and affect relayer behavior: |`optimism` |Identifies Optimism-based networks using the OP Stack (e.g., Optimism, Base, World Chain) -|`arbitrum_based` +|`arbitrum-based` |Identifies Arbitrum-based networks using the Arbitrum Stack |`no-mempool` diff --git a/src/domain/transaction/evm/utils.rs b/src/domain/transaction/evm/utils.rs index de50a33d5..d87240f53 100644 --- a/src/domain/transaction/evm/utils.rs +++ b/src/domain/transaction/evm/utils.rs @@ -150,7 +150,7 @@ mod tests { explorer_urls: None, average_blocktime_ms: 1000, is_testnet: false, - tags: vec!["rollup".to_string(), "arbitrum_based".to_string()], + tags: vec!["rollup".to_string(), "arbitrum-based".to_string()], chain_id: 42161, required_confirmations: 1, features: vec!["eip1559".to_string()], @@ -165,7 +165,7 @@ mod tests { explorer_urls: None, average_blocktime_ms: 1000, is_testnet: false, - tags: vec!["rollup".to_string(), "arbitrum_based".to_string()], + tags: vec!["rollup".to_string(), "arbitrum-based".to_string()], chain_id: 42170, required_confirmations: 1, features: vec!["eip1559".to_string()], diff --git a/src/models/network/evm/network.rs b/src/models/network/evm/network.rs index e6072fdc8..fa4ed4f73 100644 --- a/src/models/network/evm/network.rs +++ b/src/models/network/evm/network.rs @@ -105,7 +105,7 @@ impl EvmNetwork { } pub fn is_arbitrum(&self) -> bool { - self.tags.contains(&"arbitrum_based".to_string()) + self.tags.contains(&"arbitrum-based".to_string()) } pub fn is_testnet(&self) -> bool { From 018740f5cc6f564c7cecea88f169a645d21173d7 Mon Sep 17 00:00:00 2001 From: Nicolas Molina Date: Thu, 24 Jul 2025 09:35:32 -0300 Subject: [PATCH 13/13] fix: Fixing timeout for arbitrum --- .../transaction/evm/price_calculator.rs | 96 ++++++++++++++++++- src/domain/transaction/evm/status.rs | 6 +- 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/src/domain/transaction/evm/price_calculator.rs b/src/domain/transaction/evm/price_calculator.rs index a872f0f56..996261003 100644 --- a/src/domain/transaction/evm/price_calculator.rs +++ b/src/domain/transaction/evm/price_calculator.rs @@ -242,7 +242,9 @@ impl PriceCalculator { relayer: &RelayerRepoModel, ) -> Result { // Check if network lacks mempool (e.g., Arbitrum) - skip bump and use current prices - if self.gas_price_service.network().lacks_mempool() { + if self.gas_price_service.network().lacks_mempool() + || self.gas_price_service.network().is_arbitrum() + { let mut price_params = self.get_transaction_price_params(tx_data, relayer).await?; // For mempool-less networks, we don't need to bump - just use current market prices @@ -1901,4 +1903,96 @@ mod tests { // Regular networks may or may not set is_min_bumped depending on the actual implementation assert_eq!(result_no_mempool.is_min_bumped, Some(true)); } + + #[tokio::test] + async fn test_calculate_bumped_gas_price_arbitrum_network() { + let mut mock_service = MockEvmGasPriceServiceTrait::new(); + + // Create a network that returns true for is_arbitrum() but not lacks_mempool() + let arbitrum_network = EvmNetwork { + network: "arbitrum-one".to_string(), + rpc_urls: vec!["https://arb1.arbitrum.io/rpc".to_string()], + explorer_urls: None, + average_blocktime_ms: 1000, // 1 second for arbitrum + is_testnet: false, + tags: vec!["arbitrum-based".to_string()], // This makes is_arbitrum() return true + chain_id: 42161, + required_confirmations: 1, + features: vec!["eip1559".to_string()], + symbol: "ETH".to_string(), + }; + + // Mock the network to return our arbitrum network + mock_service.expect_network().return_const(arbitrum_network); + + // Mock get_prices_from_json_rpc for get_transaction_price_params call + let mock_prices = GasPrices { + legacy_prices: SpeedPrices { + safe_low: 100_000_000, // 0.1 Gwei (typical for Arbitrum) + average: 200_000_000, + fast: 300_000_000, + fastest: 400_000_000, + }, + max_priority_fee_per_gas: SpeedPrices { + safe_low: 10_000_000, // 0.01 Gwei + average: 20_000_000, + fast: 30_000_000, + fastest: 40_000_000, + }, + base_fee_per_gas: 100_000_000, // 0.1 Gwei + }; + + mock_service + .expect_get_prices_from_json_rpc() + .returning(move || { + let prices = mock_prices.clone(); + Box::pin(async move { Ok(prices) }) + }); + + let pc = PriceCalculator::new(mock_service, NetworkExtraFeeCalculator::None); + let relayer = create_mock_relayer(); + + // Create a speed-based transaction that would normally be bumped on regular networks + let tx_data = EvmTransactionData { + speed: Some(Speed::Fast), + gas_limit: Some(21000), + value: U256::from(1_000_000_000_000_000_000u128), // 1 ETH + ..Default::default() + }; + + let result = pc.calculate_bumped_gas_price(&tx_data, &relayer).await; + + assert!(result.is_ok()); + let price_params = result.unwrap(); + + // For Arbitrum networks (is_arbitrum() == true), should skip bump and use current market prices + assert_eq!( + price_params.is_min_bumped, + Some(true), + "Arbitrum networks should skip bumping and use current market prices" + ); + + // Should return pricing based on current market conditions, not bumped prices + assert!( + price_params.max_priority_fee_per_gas.is_some() || price_params.gas_price.is_some(), + "Should return some form of pricing" + ); + + // Should have calculated total cost + assert!( + price_params.total_cost > U256::ZERO, + "Should have non-zero total cost" + ); + + // Verify that the prices returned are based on current market (Speed::Fast) + // and not bumped versions of existing transaction prices + if let Some(priority_fee) = price_params.max_priority_fee_per_gas { + // For Speed::Fast, should be around 30_000_000 (0.03 Gwei) based on our mock + assert!( + priority_fee <= 50_000_000, // Should be reasonable for current market, not a bump + "Priority fee should be based on current market, not bumped: {}", + priority_fee + ); + } + } } diff --git a/src/domain/transaction/evm/status.rs b/src/domain/transaction/evm/status.rs index 50b721314..e6d2e8076 100644 --- a/src/domain/transaction/evm/status.rs +++ b/src/domain/transaction/evm/status.rs @@ -137,7 +137,11 @@ where false => get_resubmit_timeout_for_speed(&evm_data.speed), }; - let timeout_with_backoff = get_resubmit_timeout_with_backoff(timeout, tx.hashes.len()); + let timeout_with_backoff = match network.is_arbitrum() { + true => timeout, // Use base timeout without backoff for Arbitrum + false => get_resubmit_timeout_with_backoff(timeout, tx.hashes.len()), + }; + if age > Duration::milliseconds(timeout_with_backoff) { info!("Transaction has been pending for too long, resubmitting"); return Ok(true);