diff --git a/src/constants/evm_transaction.rs b/src/constants/evm_transaction.rs index 03fa54aa6..6a949099c 100644 --- a/src/constants/evm_transaction.rs +++ b/src/constants/evm_transaction.rs @@ -96,3 +96,29 @@ pub fn get_evm_status_check_initial_delay() -> Duration { pub fn get_evm_min_age_for_hash_recovery() -> Duration { Duration::minutes(EVM_MIN_AGE_FOR_HASH_RECOVERY_MINUTES) } + +/// Error message patterns indicating a transaction was already submitted or its nonce consumed. +/// Shared between the retry layer (`is_non_retriable_transaction_rpc_message`) and +/// the domain layer (`is_already_submitted_error`). +/// +/// Each entry is a lowercased substring to match against the RPC error message. +pub const ALREADY_SUBMITTED_PATTERNS: &[&str] = &[ + "nonce too low", + "nonce is too low", + "already known", + "replacement transaction underpriced", + "same hash was already imported", +]; + +/// Checks if a lowercased message matches "known transaction" without matching +/// "unknown transaction" (substring false positive). +pub fn matches_known_transaction(msg_lower: &str) -> bool { + if let Some(pos) = msg_lower.find("known transaction") { + // Reject if preceded by "un" (i.e. "unknown transaction") + if msg_lower[..pos].ends_with("un") { + return false; + } + return true; + } + false +} diff --git a/src/domain/transaction/evm/evm_transaction.rs b/src/domain/transaction/evm/evm_transaction.rs index f2a32bc9e..c7318a39e 100644 --- a/src/domain/transaction/evm/evm_transaction.rs +++ b/src/domain/transaction/evm/evm_transaction.rs @@ -11,7 +11,10 @@ use std::sync::Arc; use tracing::{debug, error, info, warn}; use crate::{ - constants::{DEFAULT_EVM_GAS_LIMIT_ESTIMATION, GAS_LIMIT_BUFFER_MULTIPLIER}, + constants::{ + matches_known_transaction, ALREADY_SUBMITTED_PATTERNS, DEFAULT_EVM_GAS_LIMIT_ESTIMATION, + GAS_LIMIT_BUFFER_MULTIPLIER, + }, domain::{ evm::is_noop, transaction::{ @@ -145,11 +148,15 @@ where /// Checks if a provider error indicates the transaction was already submitted to the blockchain. /// This handles cases where the transaction was submitted by another instance or in a previous retry. + /// + /// Uses the shared `ALREADY_SUBMITTED_PATTERNS` from constants, consistent with + /// `is_non_retriable_transaction_rpc_message` in `services::provider`. fn is_already_submitted_error(error: &impl std::fmt::Display) -> bool { let error_msg = error.to_string().to_lowercase(); - error_msg.contains("already known") - || error_msg.contains("nonce too low") - || error_msg.contains("replacement transaction underpriced") + ALREADY_SUBMITTED_PATTERNS + .iter() + .any(|p| error_msg.contains(p)) + || matches_known_transaction(&error_msg) } /// Helper method to schedule a transaction status check job. @@ -2774,6 +2781,22 @@ mod tests { &"Error: nonce too low" )); + // Test "nonce is too low" variants (some providers use this wording) + assert!(DefaultEvmTransaction::is_already_submitted_error( + &"nonce is too low" + )); + assert!(DefaultEvmTransaction::is_already_submitted_error( + &"Error: nonce is too low" + )); + + // Test "known transaction" variants (Besu) + assert!(DefaultEvmTransaction::is_already_submitted_error( + &"known transaction" + )); + assert!(DefaultEvmTransaction::is_already_submitted_error( + &"Known Transaction" + )); + // Test "replacement transaction underpriced" variants assert!(DefaultEvmTransaction::is_already_submitted_error( &"replacement transaction underpriced" @@ -2782,6 +2805,11 @@ mod tests { &"Replacement Transaction Underpriced" )); + // Test "same hash was already imported" (OpenEthereum) + assert!(DefaultEvmTransaction::is_already_submitted_error( + &"same hash was already imported" + )); + // Test non-matching errors assert!(!DefaultEvmTransaction::is_already_submitted_error( &"insufficient funds" @@ -2795,6 +2823,10 @@ mod tests { assert!(!DefaultEvmTransaction::is_already_submitted_error( &"timeout" )); + // "unknown transaction" must NOT match "known transaction" + assert!(!DefaultEvmTransaction::is_already_submitted_error( + &"Unknown transaction status" + )); } /// Test submit_transaction with "already known" error in Sent status diff --git a/src/services/provider/mod.rs b/src/services/provider/mod.rs index 35108691f..4cd68cebb 100644 --- a/src/services/provider/mod.rs +++ b/src/services/provider/mod.rs @@ -7,6 +7,7 @@ use tracing::debug; use crate::config::ServerConfig; use crate::constants::{ + matches_known_transaction, ALREADY_SUBMITTED_PATTERNS, DEFAULT_HTTP_CLIENT_CONNECT_TIMEOUT_SECONDS, DEFAULT_HTTP_CLIENT_HTTP2_KEEP_ALIVE_INTERVAL_SECONDS, DEFAULT_HTTP_CLIENT_HTTP2_KEEP_ALIVE_TIMEOUT_SECONDS, @@ -441,6 +442,20 @@ pub fn should_mark_provider_failed(error: &ProviderError) -> bool { } } +/// Returns true if the RPC error message indicates a transaction-level error +/// that should not be retried — the RPC is working correctly, but rejecting +/// the transaction itself. +/// +/// Uses the shared `ALREADY_SUBMITTED_PATTERNS` from constants, consistent with +/// `is_already_submitted_error` in `domain::transaction::evm::evm_transaction`. +fn is_non_retriable_transaction_rpc_message(message: &str) -> bool { + let msg_lower = message.to_lowercase(); + ALREADY_SUBMITTED_PATTERNS + .iter() + .any(|p| msg_lower.contains(p)) + || matches_known_transaction(&msg_lower) +} + // Errors that are retriable pub fn is_retriable_error(error: &ProviderError) -> bool { match error { @@ -470,14 +485,16 @@ pub fn is_retriable_error(error: &ProviderError) -> bool { } // JSON-RPC error codes (EIP-1474) - ProviderError::RpcErrorCode { code, .. } => { + ProviderError::RpcErrorCode { code, message } => { match code { - // -32002: Resource unavailable (temporary state) - -32002 => true, + // -32002: Resource unavailable — retriable unless the message indicates a + // transaction-level rejection (some providers wrap nonce/tx errors here) + -32002 => !is_non_retriable_transaction_rpc_message(message), // -32005: Limit exceeded / rate limited -32005 => true, - // -32603: Internal error (may be temporary) - -32603 => true, + // -32603: Internal error — retriable unless the message indicates a + // transaction-level rejection (some providers wrap nonce/tx errors here) + -32603 => !is_non_retriable_transaction_rpc_message(message), // -32000: Invalid input -32000 => false, // -32001: Resource not found @@ -1320,4 +1337,85 @@ mod tests { ); } } + + #[test] + fn test_is_non_retriable_transaction_rpc_message() { + // Positive cases: these messages should be recognized as non-retriable + assert!(is_non_retriable_transaction_rpc_message("nonce too low")); + assert!(is_non_retriable_transaction_rpc_message("Nonce Too Low")); + assert!(is_non_retriable_transaction_rpc_message("nonce is too low")); + assert!(is_non_retriable_transaction_rpc_message("already known")); + assert!(is_non_retriable_transaction_rpc_message( + "known transaction" + )); + assert!(is_non_retriable_transaction_rpc_message( + "Known Transaction" + )); + assert!(is_non_retriable_transaction_rpc_message( + "replacement transaction underpriced" + )); + assert!(is_non_retriable_transaction_rpc_message( + "same hash was already imported" + )); + assert!(is_non_retriable_transaction_rpc_message( + "Transaction nonce too low" + )); + + // Negative cases: generic/unrelated messages should not match + assert!(!is_non_retriable_transaction_rpc_message("Internal error")); + assert!(!is_non_retriable_transaction_rpc_message("server busy")); + assert!(!is_non_retriable_transaction_rpc_message("")); + // "unknown transaction" must NOT match "known transaction" + assert!(!is_non_retriable_transaction_rpc_message( + "Unknown transaction status" + )); + } + + #[test] + fn test_is_retriable_error_rpc_tx_errors_not_retriable() { + // Transaction-level messages that should NOT be retriable regardless of code + let non_retriable_messages = vec![ + "Transaction nonce too low", + "nonce too low", + "nonce is too low", + "already known", + "known transaction", + "replacement transaction underpriced", + "same hash was already imported", + ]; + + // Messages that should remain retriable (generic/unrelated) + let retriable_messages = vec![ + "Internal error", + "", + // "unknown transaction" must NOT false-positive on "known transaction" + "Unknown transaction status", + "Resource unavailable", + ]; + + // Both -32603 and -32002 should behave the same way for tx-level messages + for code in [-32603, -32002] { + for message in &non_retriable_messages { + let error = ProviderError::RpcErrorCode { + code, + message: message.to_string(), + }; + assert!( + !is_retriable_error(&error), + "{code} with message {message:?} should NOT be retriable" + ); + } + + for message in &retriable_messages { + let error = ProviderError::RpcErrorCode { + code, + message: message.to_string(), + }; + assert!( + is_retriable_error(&error), + "{code} with message {message:?} should be retriable" + ); + } + } + } } diff --git a/src/services/provider/retry.rs b/src/services/provider/retry.rs index 45116921b..414ba50a7 100644 --- a/src/services/provider/retry.rs +++ b/src/services/provider/retry.rs @@ -1948,4 +1948,76 @@ mod tests { selected.len() ); } + + #[tokio::test] + #[serial] + async fn test_retry_rpc_call_non_retriable_rpc_error_no_failover() { + use crate::services::provider::{ + is_retriable_error, should_mark_provider_failed, ProviderError, + }; + + RpcHealthStore::instance().clear_all(); + + let url1 = "http://localhost:9977"; + let url2 = "http://localhost:9976"; + let configs = vec![ + RpcConfig::new(url1.to_string()), + RpcConfig::new(url2.to_string()), + ]; + let selector = RpcSelector::new(configs, 1, 60, 60).expect("Failed to create selector"); + + let attempts = Arc::new(AtomicU8::new(0)); + let attempts_clone = attempts.clone(); + + let provider_initializer = + |_url: &str| -> Result { Ok("mock_provider".to_string()) }; + + // Simulate the exact error a Monad-style provider returns: + // JSON-RPC -32603 with "nonce too low" in the message + let operation = move |_provider: String| { + let attempts = attempts_clone.clone(); + async move { + attempts.fetch_add(1, AtomicOrdering::SeqCst); + Err::(ProviderError::RpcErrorCode { + code: -32603, + message: "nonce too low".to_string(), + }) + } + }; + + let config = RetryConfig::new(3, 1, 0, 0); + + let result = retry_rpc_call( + &selector, + "test_operation", + is_retriable_error, + should_mark_provider_failed, + provider_initializer, + operation, + Some(config), + ) + .await; + + assert!(result.is_err(), "Expected Err result but got: {result:?}"); + assert_eq!( + attempts.load(AtomicOrdering::SeqCst), + 1, + "Operation should be called exactly once (no retries)" + ); + + // Verify neither provider was marked as failed in the health store + let health = RpcHealthStore::instance(); + let meta1 = health.get_metadata(url1); + let meta2 = health.get_metadata(url2); + assert!( + meta1.failure_timestamps.is_empty(), + "Provider 1 should have 0 failures, got: {}", + meta1.failure_timestamps.len() + ); + assert!( + meta2.failure_timestamps.is_empty(), + "Provider 2 should have 0 failures, got: {}", + meta2.failure_timestamps.len() + ); + } }