diff --git a/config/networks/arbitrum.json b/config/networks/arbitrum.json index 9b2d89324..684b4ca77 100644 --- a/config/networks/arbitrum.json +++ b/config/networks/arbitrum.json @@ -22,6 +22,7 @@ ], "symbol": "ETH", "tags": [ + "arbitrum-based", "rollup", "no-mempool" ], @@ -42,11 +43,11 @@ "required_confirmations": 1, "rpc_urls": [ "https://sepolia-rollup.arbitrum.io/rpc", - "https://arbitrum-sepolia.drpc.org", "https://arbitrum-sepolia-rpc.publicnode.com" ], "symbol": "ETH", "tags": [ + "arbitrum-based", "deprecated", "rollup", "no-mempool" @@ -73,6 +74,7 @@ ], "symbol": "ETH", "tags": [ + "arbitrum-based", "rollup", "no-mempool" ], diff --git a/docs/modules/ROOT/pages/network_configuration.adoc b/docs/modules/ROOT/pages/network_configuration.adoc index 18ecc1036..28ab52ffd 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-based` +|Identifies Arbitrum-based networks using the Arbitrum Stack + |`no-mempool` |Indicates networks that lack a traditional mempool (e.g., Arbitrum) diff --git a/src/constants/evm_transaction.rs b/src/constants/evm_transaction.rs index 0ef686747..b455b89ef 100644 --- a/src/constants/evm_transaction.rs +++ b/src/constants/evm_transaction.rs @@ -23,3 +23,9 @@ 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; + +/// 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/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/price_calculator.rs b/src/domain/transaction/evm/price_calculator.rs index fc64d4ee8..996261003 100644 --- a/src/domain/transaction/evm/price_calculator.rs +++ b/src/domain/transaction/evm/price_calculator.rs @@ -241,6 +241,17 @@ 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() + || 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 + 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 +728,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 +1166,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 +1677,322 @@ 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)); + } + + #[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 961efc88f..e6d2e8076 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::{ @@ -111,13 +112,36 @@ where ))); } + let evm_data = tx.network_data.get_evm_transaction_data()?; 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), + + // 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) + .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 + )) + })?; + + let timeout = match network.is_arbitrum() { + true => ARBITRUM_TIME_TO_RESUBMIT, + false => get_resubmit_timeout_for_speed(&evm_data.speed), + }; + + 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()), }; - let timeout_with_backoff = 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); @@ -201,7 +225,23 @@ where is_cancellation: bool, ) -> Result { let mut evm_data = tx.network_data.get_evm_transaction_data()?; - make_noop(&mut evm_data).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 { @@ -389,6 +429,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 { @@ -414,6 +471,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 +734,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 +758,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_true_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 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` @@ -737,7 +943,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); @@ -758,7 +964,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; @@ -1028,6 +1234,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..d87240f53 100644 --- a/src/domain/transaction/evm/utils.rs +++ b/src/domain/transaction/evm/utils.rs @@ -1,21 +1,55 @@ 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::{ EvmTransactionData, TransactionError, TransactionRepoModel, TransactionStatus, U256, }; +use crate::services::EvmProviderTrait; use chrono::{DateTime, Duration, Utc}; use eyre::Result; /// 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, + network: &EvmNetwork, + 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 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 { + 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(ARBITRUM_GAS_LIMIT); + } + } + } else { + // No provider available, use conservative estimate + evm_data.gas_limit = Some(ARBITRUM_GAS_LIMIT); + } + } else { + // For other networks, use the standard gas limit + evm_data.gas_limit = Some(DEFAULT_GAS_LIMIT); + } + Ok(()) } @@ -92,6 +126,52 @@ pub fn get_age_of_sent_at(tx: &TransactionRepoModel) -> Result 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(), "arbitrum-based".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(), "arbitrum-based".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() { @@ -112,7 +192,8 @@ mod tests { raw: Some(vec![1, 2, 3]), }; - let result = make_noop(&mut evm_data).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 @@ -123,6 +204,149 @@ 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 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 + 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 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 + 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 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 + 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 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 + 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 diff --git a/src/models/network/evm/network.rs b/src/models/network/evm/network.rs index 305f36d7e..fa4ed4f73 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(&"arbitrum-based".to_string()) + } + pub fn is_testnet(&self) -> bool { self.is_testnet }