GH-665: Introduce new states in FailedPayableDao (#666)

* GH-605: add the steps to solve this card

* GH-605: change the recheck to status

* GH-605: all tests pass in failed_payable_dao.rs

* GH-605: few more changes

* GH-605: add instructions inside test retry_payable_scanner_can_initiate_a_scan

* GH-605: introduce mocks for FailedPayableDAO

* GH-665: add another variant for the reason: General

* GH-665: add the string conversion for General

* GH-665: review changes

Author: utkarshg6

node/src/accountant/db_access_objects/failed_payable_dao.rs

@@ -6,7 +6,7 @@ use rusqlite::{Connection, OpenFlags};
 use crate::accountant::db_access_objects::sent_payable_dao::{ Tx};
 use crate::accountant::db_access_objects::utils::{current_unix_timestamp, TxHash};
 use web3::types::{Address};
-use crate::accountant::db_access_objects::failed_payable_dao::{FailedTx, FailureReason};
+use crate::accountant::db_access_objects::failed_payable_dao::{FailedTx, FailureReason, FailureStatus};
 use crate::blockchain::blockchain_interface::blockchain_interface_web3::lower_level_interface_web3::TransactionBlock;
 use crate::database::db_initializer::{DbInitializationConfig, DbInitializer, DbInitializerReal, DATABASE_FILE};
 use crate::database::rusqlite_wrappers::ConnectionWrapperReal;
@@ -69,7 +69,7 @@ pub struct FailedTxBuilder {
     gas_price_wei_opt: Option<u128>,
     nonce_opt: Option<u64>,
     reason_opt: Option<FailureReason>,
-    rechecked_opt: Option<bool>,
+    status_opt: Option<FailureStatus>,
 }
 
 impl FailedTxBuilder {
@@ -97,8 +97,8 @@ impl FailedTxBuilder {
         self
     }
 
-    pub fn rechecked(mut self, rechecked: bool) -> Self {
-        self.rechecked_opt = Some(rechecked);
+    pub fn status(mut self, failure_status: FailureStatus) -> Self {
+        self.status_opt = Some(failure_status);
         self
     }
 
@@ -113,7 +113,9 @@ impl FailedTxBuilder {
             reason: self
                 .reason_opt
                 .unwrap_or_else(|| FailureReason::PendingTooLong),
-            rechecked: self.rechecked_opt.unwrap_or_else(|| false),
+            status: self
+                .status_opt
+                .unwrap_or_else(|| FailureStatus::RetryRequired),
         }
     }
 }

node/src/accountant/db_access_objects/test_utils.rs

@@ -520,6 +520,9 @@ impl StartableScanner<ScanForRetryPayables, QualifiedPayablesMessage> for Payabl
         _logger: &Logger,
     ) -> Result<QualifiedPayablesMessage, StartScanError> {
         todo!("Complete me under GH-605")
+        // 1. Find the failed payables
+        // 2. Look into the payable DAO to update the amount
+        // 3. Prepare UnpricedQualifiedPayables
     }
 }
 
@@ -1737,6 +1740,17 @@ mod tests {
 
     #[test]
     fn retry_payable_scanner_can_initiate_a_scan() {
+        //
+        // Setup Part:
+        // DAOs: PayableDao, FailedPayableDao
+        // Fetch data from FailedPayableDao (inject it into Payable Scanner -- allow the change in production code).
+        // Scanners constructor will require to create it with the Factory -- try it
+        // Configure it such that it returns at least 2 failed tx
+        // Once I get those 2 records, I should get hold of those identifiers used in the Payable DAO
+        // Update the new balance for those transactions
+        // Modify Payable DAO and add another method, that will return just the corresponding payments
+        // The account which I get from the PayableDAO can go straight to the QualifiedPayableBeforePriceSelection
+
         todo!("this must be set up under GH-605");
         // TODO make sure the QualifiedPayableRawPack will express the difference from
         // the NewPayable scanner: The QualifiedPayablesBeforeGasPriceSelection needs to carry

node/src/accountant/scanners/mod.rs

@@ -3,6 +3,10 @@
 #![cfg(test)]
 
 use crate::accountant::db_access_objects::banned_dao::{BannedDao, BannedDaoFactory};
+use crate::accountant::db_access_objects::failed_payable_dao::{
+    FailedPayableDao, FailedPayableDaoError, FailedPayableDaoFactory, FailedTx,
+    FailureRetrieveCondition, FailureStatus,
+};
 use crate::accountant::db_access_objects::payable_dao::{
     PayableAccount, PayableDao, PayableDaoError, PayableDaoFactory,
 };
@@ -13,7 +17,7 @@ use crate::accountant::db_access_objects::receivable_dao::{
     ReceivableAccount, ReceivableDao, ReceivableDaoError, ReceivableDaoFactory,
 };
 use crate::accountant::db_access_objects::utils::{
-    from_unix_timestamp, to_unix_timestamp, CustomQuery,
+    from_unix_timestamp, to_unix_timestamp, CustomQuery, TxHash, TxIdentifiers,
 };
 use crate::accountant::payment_adjuster::{Adjustment, AnalysisError, PaymentAdjuster};
 use crate::accountant::scanners::payable_scanner_extension::msgs::{
@@ -45,6 +49,7 @@ use masq_lib::logger::Logger;
 use rusqlite::{Connection, OpenFlags, Row};
 use std::any::type_name;
 use std::cell::RefCell;
+use std::collections::{HashMap, HashSet};
 use std::fmt::Debug;
 use std::path::Path;
 use std::rc::Rc;
@@ -1077,6 +1082,158 @@ impl PendingPayableDaoFactoryMock {
     }
 }
 
+#[derive(Default)]
+pub struct FailedPayableDaoMock {
+    get_tx_identifiers_params: Arc<Mutex<Vec<HashSet<TxHash>>>>,
+    get_tx_identifiers_results: RefCell<Vec<TxIdentifiers>>,
+    insert_new_records_params: Arc<Mutex<Vec<Vec<FailedTx>>>>,
+    insert_new_records_results: RefCell<Vec<Result<(), FailedPayableDaoError>>>,
+    retrieve_txs_params: Arc<Mutex<Vec<Option<FailureRetrieveCondition>>>>,
+    retrieve_txs_results: RefCell<Vec<Vec<FailedTx>>>,
+    update_statuses_params: Arc<Mutex<Vec<HashMap<TxHash, FailureStatus>>>>,
+    update_statuses_results: RefCell<Vec<Result<(), FailedPayableDaoError>>>,
+    delete_records_params: Arc<Mutex<Vec<HashSet<TxHash>>>>,
+    delete_records_results: RefCell<Vec<Result<(), FailedPayableDaoError>>>,
+}
+
+impl FailedPayableDao for FailedPayableDaoMock {
+    fn get_tx_identifiers(&self, hashes: &HashSet<TxHash>) -> TxIdentifiers {
+        self.get_tx_identifiers_params
+            .lock()
+            .unwrap()
+            .push(hashes.clone());
+        self.get_tx_identifiers_results.borrow_mut().remove(0)
+    }
+
+    fn insert_new_records(&self, txs: &[FailedTx]) -> Result<(), FailedPayableDaoError> {
+        self.insert_new_records_params
+            .lock()
+            .unwrap()
+            .push(txs.to_vec());
+        self.insert_new_records_results.borrow_mut().remove(0)
+    }
+
+    fn retrieve_txs(&self, condition: Option<FailureRetrieveCondition>) -> Vec<FailedTx> {
+        self.retrieve_txs_params.lock().unwrap().push(condition);
+        self.retrieve_txs_results.borrow_mut().remove(0)
+    }
+
+    fn update_statuses(
+        &self,
+        status_updates: HashMap<TxHash, FailureStatus>,
+    ) -> Result<(), FailedPayableDaoError> {
+        self.update_statuses_params
+            .lock()
+            .unwrap()
+            .push(status_updates);
+        self.update_statuses_results.borrow_mut().remove(0)
+    }
+
+    fn delete_records(&self, hashes: &HashSet<TxHash>) -> Result<(), FailedPayableDaoError> {
+        self.delete_records_params
+            .lock()
+            .unwrap()
+            .push(hashes.clone());
+        self.delete_records_results.borrow_mut().remove(0)
+    }
+}
+
+impl FailedPayableDaoMock {
+    pub fn new() -> Self {
+        Self::default()
+    }
+
+    pub fn get_tx_identifiers_params(mut self, params: &Arc<Mutex<Vec<HashSet<TxHash>>>>) -> Self {
+        self.get_tx_identifiers_params = params.clone();
+        self
+    }
+
+    pub fn get_tx_identifiers_result(self, result: TxIdentifiers) -> Self {
+        self.get_tx_identifiers_results.borrow_mut().push(result);
+        self
+    }
+
+    pub fn insert_new_records_params(mut self, params: &Arc<Mutex<Vec<Vec<FailedTx>>>>) -> Self {
+        self.insert_new_records_params = params.clone();
+        self
+    }
+
+    pub fn insert_new_records_result(self, result: Result<(), FailedPayableDaoError>) -> Self {
+        self.insert_new_records_results.borrow_mut().push(result);
+        self
+    }
+
+    pub fn retrieve_txs_params(
+        mut self,
+        params: &Arc<Mutex<Vec<Option<FailureRetrieveCondition>>>>,
+    ) -> Self {
+        self.retrieve_txs_params = params.clone();
+        self
+    }
+
+    pub fn retrieve_txs_result(self, result: Vec<FailedTx>) -> Self {
+        self.retrieve_txs_results.borrow_mut().push(result);
+        self
+    }
+
+    pub fn update_statuses_params(
+        mut self,
+        params: &Arc<Mutex<Vec<HashMap<TxHash, FailureStatus>>>>,
+    ) -> Self {
+        self.update_statuses_params = params.clone();
+        self
+    }
+
+    pub fn update_statuses_result(self, result: Result<(), FailedPayableDaoError>) -> Self {
+        self.update_statuses_results.borrow_mut().push(result);
+        self
+    }
+
+    pub fn delete_records_params(mut self, params: &Arc<Mutex<Vec<HashSet<TxHash>>>>) -> Self {
+        self.delete_records_params = params.clone();
+        self
+    }
+
+    pub fn delete_records_result(self, result: Result<(), FailedPayableDaoError>) -> Self {
+        self.delete_records_results.borrow_mut().push(result);
+        self
+    }
+}
+
+pub struct FailedPayableDaoFactoryMock {
+    make_params: Arc<Mutex<Vec<()>>>,
+    make_results: RefCell<Vec<Box<dyn FailedPayableDao>>>,
+}
+
+impl FailedPayableDaoFactory for FailedPayableDaoFactoryMock {
+    fn make(&self) -> Box<dyn FailedPayableDao> {
+        if self.make_results.borrow().len() == 0 {
+            panic!("FailedPayableDao Missing.")
+        };
+        self.make_params.lock().unwrap().push(());
+        self.make_results.borrow_mut().remove(0)
+    }
+}
+
+impl FailedPayableDaoFactoryMock {
+    pub fn new() -> Self {
+        Self {
+            make_params: Arc::new(Mutex::new(vec![])),
+            make_results: RefCell::new(vec![]),
+        }
+    }
+
+    pub fn make_params(mut self, params: &Arc<Mutex<Vec<()>>>) -> Self {
+        self.make_params = params.clone();
+        self
+    }
+
+    pub fn make_result(self, result: FailedPayableDaoMock) -> Self {
+        self.make_results.borrow_mut().push(Box::new(result));
+        self
+    }
+}
+
 pub struct PayableScannerBuilder {
     payable_dao: PayableDaoMock,
     pending_payable_dao: PendingPayableDaoMock,

node/src/accountant/test_utils.rs

@@ -299,7 +299,7 @@ impl DbInitializerReal {
                 gas_price_wei_low_b integer not null,
                 nonce integer not null,
                 reason text not null,
-                rechecked integer not null
+                status text not null
             )",
             [],
         )
@@ -846,7 +846,7 @@ mod tests {
                         gas_price_wei_low_b,
                         nonce,
                         reason,
-                        rechecked
+                        status
                  FROM failed_payable",
             )
             .unwrap();

node/src/database/db_initializer.rs

@@ -34,7 +34,7 @@ impl DatabaseMigration for Migrate_10_to_11 {
                 gas_price_wei_low_b integer not null,
                 nonce integer not null,
                 reason text not null,
-                rechecked integer not null
+                status text not null
             )";
 
         declaration_utils.execute_upon_transaction(&[

node/src/database/db_migrations/migrations/migration_10_to_11.rs

@@ -37,7 +37,7 @@ pub const SQL_ATTRIBUTES_FOR_CREATING_FAILED_PAYABLE: &[&[&str]] = &[
     &["gas_price_wei_low_b", "integer", "not", "null"],
     &["nonce", "integer", "not", "null"],
     &["reason", "text", "not", "null"],
-    &["rechecked", "integer", "not", "null"],
+    &["status", "text", "not", "null"],
 ];
 
 #[derive(Debug, Default)]

node/src/database/test_utils/mod.rs

@@ -529,7 +529,6 @@ mod tests {
     use masq_lib::test_utils::environment_guard::EnvironmentGuard;
     use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler};
     use masq_lib::test_utils::utils::TEST_DEFAULT_CHAIN;
-    use std::fmt::format;
     use std::net::SocketAddr;
     use std::str::FromStr;
     use std::time::SystemTime;