Fix GH-683 critical issues from code review

node/src/accountant/db_access_objects/failed_payable_dao.rs

@@ -4,7 +4,7 @@ use crate::accountant::db_access_objects::utils::{
 };
 use crate::accountant::db_big_integer::big_int_divider::BigIntDivider;
 use crate::accountant::{checked_conversion, comma_joined_stringifiable};
-use crate::blockchain::errors::AppRpcError;
+use crate::blockchain::errors::validation_status::PreviousAttempts;
 use crate::database::rusqlite_wrappers::ConnectionWrapper;
 use itertools::Itertools;
 use masq_lib::utils::ExpectValue;
@@ -25,7 +25,7 @@ pub enum FailedPayableDaoError {
 
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub enum FailureReason {
-    Submission(AppRpcError),
+    Submission(PreviousAttempts),
     Reverted,
     PendingTooLong,
 }
@@ -75,7 +75,7 @@ impl FromStr for FailureStatus {
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub enum ValidationStatus {
     Waiting,
-    Reattempting { attempt: usize, error: AppRpcError },
+    Reattempting(PreviousAttempts),
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -381,16 +381,21 @@ mod tests {
         make_read_only_db_connection, FailedTxBuilder,
     };
     use crate::accountant::db_access_objects::utils::current_unix_timestamp;
-    use crate::blockchain::errors::{AppRpcError, LocalError, RemoteError};
-    use crate::blockchain::test_utils::make_tx_hash;
+    use crate::blockchain::errors::blockchain_db_error::app_rpc_web3_error_kind::AppRpcWeb3ErrorKind;
+    use crate::blockchain::errors::validation_status::{
+        PreviousAttempts, ValidationFailureClockReal,
+    };
+    use crate::blockchain::test_utils::{make_tx_hash, ValidationFailureClockMock};
     use crate::database::db_initializer::{
         DbInitializationConfig, DbInitializer, DbInitializerReal,
     };
     use crate::database::test_utils::ConnectionWrapperMock;
     use masq_lib::test_utils::utils::ensure_node_home_directory_exists;
     use rusqlite::Connection;
     use std::collections::{HashMap, HashSet};
+    use std::ops::Add;
     use std::str::FromStr;
+    use std::time::{Duration, SystemTime};
 
     #[test]
     fn insert_new_records_works() {
@@ -582,13 +587,14 @@ mod tests {
 
     #[test]
     fn failure_reason_from_str_works() {
+        let validation_failure_clock = ValidationFailureClockMock::default();
         // Submission error
         assert_eq!(
-            FailureReason::from_str(r#"{"Submission":{"Local":{"Decoder":"Test decoder error"}}}"#)
-                .unwrap(),
-            FailureReason::Submission(AppRpcError::Local(LocalError::Decoder(
-                "Test decoder error".to_string()
-            )))
+            FailureReason::from_str(r#"{"Submission":{"Local":{"Decoder"}}}"#).unwrap(),
+            FailureReason::Submission(PreviousAttempts::new(
+                Box::new(AppRpcWeb3ErrorKind::Decoder),
+                &validation_failure_clock
+            ))
         );
 
         // Reverted
@@ -620,6 +626,11 @@ mod tests {
 
     #[test]
     fn failure_status_from_str_works() {
+        let validation_failure_clock = ValidationFailureClockMock::default().now_result(
+            SystemTime::UNIX_EPOCH
+                .add(Duration::from_secs(1755080031))
+                .add(Duration::from_nanos(612180914)),
+        );
         assert_eq!(
             FailureStatus::from_str("\"RetryRequired\"").unwrap(),
             FailureStatus::RetryRequired
@@ -631,8 +642,8 @@ mod tests {
         );
 
         assert_eq!(
-            FailureStatus::from_str(r#"{"RecheckRequired":{"Reattempting":{"attempt":2,"error":{"Remote":"Unreachable"}}}}"#).unwrap(),
-            FailureStatus::RecheckRequired(ValidationStatus::Reattempting { attempt: 2, error: AppRpcError::Remote(RemoteError::Unreachable) })
+            FailureStatus::from_str(r#"{"RecheckRequired":{"Reattempting":{"ServerUnreachable":{"firstSeen":{"secs_since_epoch":1755080031,"nanos_since_epoch":612180914},"attempts":1}}}}"#).unwrap(),
+            FailureStatus::RecheckRequired(ValidationStatus::Reattempting( PreviousAttempts::new(Box::new(AppRpcWeb3ErrorKind::ServerUnreachable), &validation_failure_clock)))
         );
 
         assert_eq!(
@@ -713,10 +724,12 @@ mod tests {
         let tx3 = FailedTxBuilder::default()
             .hash(make_tx_hash(3))
             .reason(PendingTooLong)
-            .status(RecheckRequired(ValidationStatus::Reattempting {
-                attempt: 1,
-                error: AppRpcError::Remote(RemoteError::Unreachable),
-            }))
+            .status(RecheckRequired(ValidationStatus::Reattempting(
+                PreviousAttempts::new(
+                    Box::new(AppRpcWeb3ErrorKind::ServerUnreachable),
+                    &ValidationFailureClockReal::default(),
+                ),
+            )))
             .build();
         let tx4 = FailedTxBuilder::default()
             .hash(make_tx_hash(4))
@@ -768,10 +781,10 @@ mod tests {
             (tx1.hash, Concluded),
             (
                 tx2.hash,
-                RecheckRequired(ValidationStatus::Reattempting {
-                    attempt: 1,
-                    error: AppRpcError::Remote(RemoteError::Unreachable),
-                }),
+                RecheckRequired(ValidationStatus::Reattempting(PreviousAttempts::new(
+                    Box::new(AppRpcWeb3ErrorKind::ServerUnreachable),
+                    &ValidationFailureClockReal::default(),
+                ))),
             ),
             (tx3.hash, Concluded),
         ]);
@@ -785,10 +798,10 @@ mod tests {
         assert_eq!(tx2.status, RecheckRequired(ValidationStatus::Waiting));
         assert_eq!(
             updated_txs[1].status,
-            RecheckRequired(ValidationStatus::Reattempting {
-                attempt: 1,
-                error: AppRpcError::Remote(RemoteError::Unreachable)
-            })
+            RecheckRequired(ValidationStatus::Reattempting(PreviousAttempts::new(
+                Box::new(AppRpcWeb3ErrorKind::ServerUnreachable),
+                &ValidationFailureClockReal::default()
+            )))
         );
         assert_eq!(tx3.status, RetryRequired);
         assert_eq!(updated_txs[2].status, Concluded);

node/src/accountant/db_access_objects/sent_payable_dao.rs

@@ -424,8 +424,10 @@ impl SentPayableDao for SentPayableDaoReal<'_> {
 #[cfg(test)]
 mod tests {
     use std::collections::{HashMap, HashSet};
+    use std::ops::Add;
     use std::str::FromStr;
     use std::sync::{Arc, Mutex};
+    use std::time::{Duration, UNIX_EPOCH};
     use crate::accountant::db_access_objects::sent_payable_dao::{Detection, RetrieveCondition, SentPayableDao, SentPayableDaoError, SentPayableDaoReal, TxConfirmation, TxStatus};
     use crate::database::db_initializer::{
         DbInitializationConfig, DbInitializer, DbInitializerReal,
@@ -439,8 +441,9 @@ mod tests {
     use crate::accountant::db_access_objects::sent_payable_dao::SentPayableDaoError::{EmptyInput, PartialExecution};
     use crate::accountant::db_access_objects::test_utils::{make_read_only_db_connection, TxBuilder};
     use crate::blockchain::blockchain_interface::blockchain_interface_web3::lower_level_interface_web3::{TransactionBlock};
-    use crate::blockchain::errors::{AppRpcError, RemoteError};
-    use crate::blockchain::test_utils::{make_block_hash, make_tx_hash};
+    use crate::blockchain::errors::blockchain_db_error::app_rpc_web3_error_kind::AppRpcWeb3ErrorKind;
+    use crate::blockchain::errors::validation_status::{PreviousAttempts, ValidationFailureClockReal};
+    use crate::blockchain::test_utils::{make_block_hash, make_tx_hash, ValidationFailureClockMock};
 
     #[test]
     fn insert_new_records_works() {
@@ -452,10 +455,16 @@ mod tests {
         let tx1 = TxBuilder::default().hash(make_tx_hash(1)).build();
         let tx2 = TxBuilder::default()
             .hash(make_tx_hash(2))
-            .status(TxStatus::Pending(ValidationStatus::Reattempting {
-                attempt: 2,
-                error: AppRpcError::Remote(RemoteError::Unreachable),
-            }))
+            .status(TxStatus::Pending(ValidationStatus::Reattempting(
+                PreviousAttempts::new(
+                    Box::new(AppRpcWeb3ErrorKind::ServerUnreachable),
+                    &ValidationFailureClockReal::default(),
+                )
+                .add_attempt(
+                    Box::new(AppRpcWeb3ErrorKind::ServerUnreachable),
+                    &ValidationFailureClockReal::default(),
+                ),
+            )))
             .build();
         let subject = SentPayableDaoReal::new(wrapped_conn);
         let txs = vec![tx1, tx2];
@@ -682,10 +691,12 @@ mod tests {
             .build();
         let tx2 = TxBuilder::default()
             .hash(make_tx_hash(2))
-            .status(TxStatus::Pending(ValidationStatus::Reattempting {
-                attempt: 1,
-                error: AppRpcError::Remote(RemoteError::Unreachable),
-            }))
+            .status(TxStatus::Pending(ValidationStatus::Reattempting(
+                PreviousAttempts::new(
+                    Box::new(AppRpcWeb3ErrorKind::ServerUnreachable),
+                    &ValidationFailureClockReal::default(),
+                ),
+            )))
             .build();
         let tx3 = TxBuilder::default()
             .hash(make_tx_hash(3))
@@ -1169,14 +1180,16 @@ mod tests {
 
     #[test]
     fn tx_status_from_str_works() {
+        let validation_failure_clock = ValidationFailureClockMock::default()
+            .now_result(UNIX_EPOCH.add(Duration::from_secs(12456)));
         assert_eq!(
             TxStatus::from_str(r#"{"Pending":"Waiting"}"#).unwrap(),
             TxStatus::Pending(ValidationStatus::Waiting)
         );
 
         assert_eq!(
-            TxStatus::from_str(r#"{"Pending":{"Reattempting":{"attempt":3,"error":{"Remote":{"InvalidResponse":"bluh"}}}}}"#).unwrap(),
-            TxStatus::Pending(ValidationStatus::Reattempting { attempt: 3, error: AppRpcError::Remote(RemoteError::InvalidResponse("bluh".to_string())) })
+            TxStatus::from_str(r#"{"Pending":{"Reattempting":{"InvalidResponse":{"firstSeen":{"secs_since_epoch":12456,"nanos_since_epoch":0},"attempts":1}}}}"#).unwrap(),
+            TxStatus::Pending(ValidationStatus::Reattempting(PreviousAttempts::new(Box::new(AppRpcWeb3ErrorKind::InvalidResponse), &validation_failure_clock)))
         );
 
         assert_eq!(

node/src/accountant/mod.rs

@@ -2387,7 +2387,7 @@ mod tests {
                 payable_scanner_mock,
             )));
         subject.ui_message_sub_opt = Some(ui_gateway_addr.recipient());
-        // It must be populated because no errors are tolerated at the RetryPayableScanner
+        // It must be populated because no app_rpc_web3_error_kind are tolerated at the RetryPayableScanner
         // if automatic scans are on
         let response_skeleton_opt = Some(ResponseSkeleton {
             client_id: 789,
@@ -4020,7 +4020,7 @@ mod tests {
         // the first message. Now we reset the state by ending the first scan by a failure and see
         // that the third scan request is going to be accepted willingly again.
         addr.try_send(SentPayables {
-            payment_procedure_result: Err(PayableTransactionError::Signing("bluh".to_string())),
+            payment_procedure_result: Err(PayableTransactionError::Signing("blah".to_string())),
             response_skeleton_opt: Some(ResponseSkeleton {
                 client_id: 1122,
                 context_id: 7788,

node/src/accountant/scanners/mod.rs

@@ -9,19 +9,18 @@ pub mod test_utils;
 
 use crate::accountant::db_access_objects::payable_dao::{PayableAccount, PayableDao};
 use crate::accountant::db_access_objects::pending_payable_dao::{PendingPayable, PendingPayableDao};
-use crate::accountant::db_access_objects::receivable_dao::ReceivableDao;
 use crate::accountant::payment_adjuster::{PaymentAdjuster, PaymentAdjusterReal};
 use crate::accountant::scanners::scanners_utils::payable_scanner_utils::PayableTransactingErrorEnum::{
     LocallyCausedError, RemotelyCausedErrors,
 };
 use crate::accountant::scanners::scanners_utils::payable_scanner_utils::{debugging_summary_after_error_separation, err_msg_for_failure_with_expected_but_missing_fingerprints, investigate_debt_extremes, mark_pending_payable_fatal_error, payables_debug_summary, separate_errors, separate_rowids_and_hashes, OperationOutcome, PayableScanResult, PayableThresholdsGauge, PayableThresholdsGaugeReal, PayableTransactingErrorEnum, PendingPayableMetadata};
-use crate::accountant::{PendingPayableId, ScanError, ScanForPendingPayables, ScanForRetryPayables};
+use crate::accountant::{ScanError, ScanForPendingPayables, ScanForRetryPayables};
 use crate::accountant::{
     comma_joined_stringifiable, gwei_to_wei, ReceivedPayments,
     ReportTransactionReceipts, RequestTransactionReceipts, ResponseSkeleton, ScanForNewPayables,
     ScanForReceivables, SentPayables,
 };
-use crate::blockchain::blockchain_bridge::{BlockMarker, PendingPayableFingerprint, RetrieveTransactions};
+use crate::blockchain::blockchain_bridge::{RetrieveTransactions};
 use crate::sub_lib::accountant::{
     DaoFactories, FinancialStatistics, PaymentThresholds,
 };
@@ -48,7 +47,6 @@ use crate::accountant::scanners::payable_scanner_extension::msgs::{BlockchainAge
 use crate::accountant::scanners::pending_payable_scanner::PendingPayableScanner;
 use crate::accountant::scanners::pending_payable_scanner::utils::PendingPayableScanResult;
 use crate::accountant::scanners::receivable_scanner::ReceivableScanner;
-use crate::blockchain::blockchain_interface::blockchain_interface_web3::lower_level_interface_web3::{TxStatus};
 use crate::blockchain::blockchain_interface::data_structures::errors::PayableTransactionError;
 use crate::db_config::persistent_configuration::{PersistentConfigurationReal};
 
@@ -976,10 +974,10 @@ mod tests {
     };
     use crate::accountant::db_access_objects::utils::{from_unix_timestamp, to_unix_timestamp};
     use crate::accountant::scanners::payable_scanner_extension::msgs::{QualifiedPayablesBeforeGasPriceSelection, QualifiedPayablesMessage, UnpricedQualifiedPayables};
-    use crate::accountant::scanners::scanners_utils::payable_scanner_utils::{OperationOutcome, PayableScanResult, PendingPayableMetadata};
+    use crate::accountant::scanners::scanners_utils::payable_scanner_utils::{OperationOutcome, PayableScanResult};
     use crate::accountant::scanners::{Scanner, StartScanError, StartableScanner, PayableScanner, PendingPayableScanner, ReceivableScanner, ScannerCommon, Scanners, ManulTriggerError};
     use crate::accountant::test_utils::{make_custom_payment_thresholds, make_payable_account, make_qualified_and_unqualified_payables, make_pending_payable_fingerprint, make_receivable_account, BannedDaoFactoryMock, BannedDaoMock, ConfigDaoFactoryMock, PayableDaoFactoryMock, PayableDaoMock, PayableScannerBuilder, PayableThresholdsGaugeMock, PendingPayableDaoFactoryMock, PendingPayableDaoMock, PendingPayableScannerBuilder, ReceivableDaoFactoryMock, ReceivableDaoMock, ReceivableScannerBuilder};
-    use crate::accountant::{gwei_to_wei, PendingPayableId, ReceivedPayments, ReportTransactionReceipts, RequestTransactionReceipts, ScanError, ScanForRetryPayables, SentPayables, DEFAULT_PENDING_TOO_LONG_SEC};
+    use crate::accountant::{gwei_to_wei, ReceivedPayments, ReportTransactionReceipts, RequestTransactionReceipts, ScanError, ScanForRetryPayables, SentPayables};
     use crate::blockchain::blockchain_bridge::{BlockMarker, PendingPayableFingerprint, RetrieveTransactions};
     use crate::blockchain::blockchain_interface::data_structures::errors::PayableTransactionError;
     use crate::blockchain::blockchain_interface::data_structures::{
@@ -1004,13 +1002,11 @@ mod tests {
     use regex::{Regex};
     use rusqlite::{ffi, ErrorCode};
     use std::cell::RefCell;
-    use std::collections::HashSet;
-    use std::ops::Sub;
     use std::panic::{catch_unwind, AssertUnwindSafe};
     use std::rc::Rc;
     use std::sync::{Arc, Mutex};
     use std::time::{Duration, SystemTime};
-    use web3::types::{TransactionReceipt, H256};
+    use web3::types::{H256};
     use web3::Error;
     use masq_lib::messages::ScanType;
     use masq_lib::ui_gateway::NodeToUiMessage;
@@ -3099,7 +3095,7 @@ mod tests {
             ScanError {
                 scan_type,
                 response_skeleton_opt: None,
-                msg: "bluh".to_string(),
+                msg: "blah".to_string(),
             }
         }
 

node/src/accountant/scanners/scanners_utils.rs

@@ -342,7 +342,7 @@ mod tests {
     use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler};
     use std::time::SystemTime;
     use crate::accountant::db_access_objects::pending_payable_dao::PendingPayable;
-    use crate::blockchain::blockchain_interface::data_structures::errors::{BlockchainError, PayableTransactionError};
+    use crate::blockchain::blockchain_interface::data_structures::errors::{BlockchainInterfaceError, PayableTransactionError};
     use crate::blockchain::blockchain_interface::data_structures::{ProcessedPayableFallible, RpcPayableFailure};
 
     #[test]
@@ -645,11 +645,11 @@ mod tests {
     #[test]
     fn count_total_errors_says_unknown_number_for_early_local_errors() {
         let early_local_errors = [
-            PayableTransactionError::TransactionID(BlockchainError::QueryFailed(
+            PayableTransactionError::TransactionID(BlockchainInterfaceError::QueryFailed(
                 "blah".to_string(),
             )),
             PayableTransactionError::MissingConsumingWallet,
-            PayableTransactionError::GasPriceQueryFailed(BlockchainError::QueryFailed(
+            PayableTransactionError::GasPriceQueryFailed(BlockchainInterfaceError::QueryFailed(
                 "ouch".to_string(),
             )),
             PayableTransactionError::UnusableWallet("fooo".to_string()),