GH-598: second QA fix - first review

.github/workflows/ci-matrix.yml

@@ -14,7 +14,7 @@ jobs:
       matrix:
         target:
           - { name: linux, os: ubuntu-22.04 }
-          - { name: macos, os: macos-13 }
+          - { name: macos, os: macos-latest }
           - { name: windows, os: windows-2022 }
 
     name: Build node on ${{ matrix.target.os }}

masq_lib/src/constants.rs

@@ -83,7 +83,8 @@ pub const VALUE_EXCEEDS_ALLOWED_LIMIT: u64 = ACCOUNTANT_PREFIX | 3;
 pub const MASQ_TOTAL_SUPPLY: u64 = 37_500_000;
 
 pub const DEFAULT_GAS_PRICE: u64 = 1; //TODO ?? Really
-pub const DEFAULT_GAS_PRICE_MARGIN: u64 = 30;
+pub const DEFAULT_GAS_PRICE_RETRY_PERCENTAGE: u64 = 30;
+pub const DEFAULT_GAS_PRICE_RETRY_CONSTANT: u128 = 5_000;
 pub const DEFAULT_MAX_BLOCK_COUNT: u64 = 100_000;
 
 //chains
@@ -142,7 +143,8 @@ mod tests {
         assert_eq!(CURRENT_LOGFILE_NAME, "MASQNode_rCURRENT.log");
         assert_eq!(MASQ_PROMPT, "masq> ");
         assert_eq!(DEFAULT_GAS_PRICE, 1);
-        assert_eq!(DEFAULT_GAS_PRICE_MARGIN, 30);
+        assert_eq!(DEFAULT_GAS_PRICE_RETRY_PERCENTAGE, 30);
+        assert_eq!(DEFAULT_GAS_PRICE_RETRY_CONSTANT, 5_000);
         assert_eq!(WALLET_ADDRESS_LENGTH, 42);
         assert_eq!(MASQ_TOTAL_SUPPLY, 37_500_000);
         assert_eq!(WEIS_IN_GWEI, 1_000_000_000);

multinode_integration_tests/tests/verify_bill_payment.rs

@@ -465,7 +465,7 @@ fn verify_pending_payables() {
     }
     MASQNodeUtils::assert_node_wrote_log_containing(
         real_consuming_node.name(),
-        "Found 3 pending payables and 0 unfinalized failures to process",
+        "Found 3 pending payables and 0 suspected failures to process",
         Duration::from_secs(5),
     );
     MASQNodeUtils::assert_node_wrote_log_containing(

node/src/accountant/scanners/mod.rs

@@ -122,7 +122,10 @@ impl Scanners {
             });
         }
 
-        Self::start_correct_payable_scanner::<ScanForNewPayables>(
+        <(dyn MultistageDualPayableScanner) as StartableScanner<
+            ScanForNewPayables,
+            InitialTemplatesMessage,
+        >>::start_scan(
             &mut *self.payable,
             wallet,
             timestamp,
@@ -150,7 +153,10 @@ impl Scanners {
             )
         }
 
-        Self::start_correct_payable_scanner::<ScanForRetryPayables>(
+        <(dyn MultistageDualPayableScanner) as StartableScanner<
+            ScanForRetryPayables,
+            InitialTemplatesMessage,
+        >>::start_scan(
             &mut *self.payable,
             wallet,
             timestamp,
@@ -168,10 +174,14 @@ impl Scanners {
         automatic_scans_enabled: bool,
     ) -> Result<RequestTransactionReceipts, StartScanError> {
         let triggered_manually = response_skeleton_opt.is_some();
-        self.check_general_conditions_for_pending_payable_scan(
-            triggered_manually,
-            automatic_scans_enabled,
-        )?;
+        if triggered_manually && automatic_scans_enabled {
+            return Err(StartScanError::ManualTriggerError(
+                ManulTriggerError::AutomaticScanConflict,
+            ));
+        }
+
+        self.check_pending_payable_existence(triggered_manually)?;
+
         match (
             self.pending_payable.scan_started_at(),
             self.payable.scan_started_at(),
@@ -256,6 +266,7 @@ impl Scanners {
     }
 
     pub fn acknowledge_scan_error(&mut self, error: &ScanError, logger: &Logger) {
+        debug!(logger, "Acknowledging a scan that couldn't finish");
         match error.scan_type {
             DetailedScanType::NewPayables | DetailedScanType::RetryPayables => {
                 self.payable.mark_as_ended(logger)
@@ -298,41 +309,14 @@ impl Scanners {
         self.initial_pending_payable_scan = false
     }
 
-    // This is a helper function reducing a boilerplate of complex trait resolving where
-    // the compiler requires to specify which trigger message distinguishes the scan to run.
-    // The payable scanner offers two modes through doubled implementations of StartableScanner
-    // which uses the trigger message type as the only distinction between them.
-    fn start_correct_payable_scanner<'a, TriggerMessage>(
-        scanner: &'a mut (dyn MultistageDualPayableScanner + 'a),
-        wallet: &Wallet,
-        timestamp: SystemTime,
-        response_skeleton_opt: Option<ResponseSkeleton>,
-        logger: &Logger,
-    ) -> Result<InitialTemplatesMessage, StartScanError>
-    where
-        TriggerMessage: Message,
-        (dyn MultistageDualPayableScanner + 'a):
-            StartableScanner<TriggerMessage, InitialTemplatesMessage>,
-    {
-        <(dyn MultistageDualPayableScanner + 'a) as StartableScanner<
-            TriggerMessage,
-            InitialTemplatesMessage,
-        >>::start_scan(scanner, wallet, timestamp, response_skeleton_opt, logger)
-    }
-
-    fn check_general_conditions_for_pending_payable_scan(
+    fn check_pending_payable_existence(
         &mut self,
         triggered_manually: bool,
-        automatic_scans_enabled: bool,
     ) -> Result<(), StartScanError> {
-        if triggered_manually && automatic_scans_enabled {
-            return Err(StartScanError::ManualTriggerError(
-                ManulTriggerError::AutomaticScanConflict,
-            ));
-        }
         if self.initial_pending_payable_scan {
             return Ok(());
         }
+
         if triggered_manually && !self.aware_of_unresolved_pending_payable {
             return Err(StartScanError::ManualTriggerError(
                 ManulTriggerError::UnnecessaryRequest {
@@ -794,11 +778,11 @@ mod tests {
             false
         );
         let dumped_records = pending_payable_scanner
-            .yet_unproven_failed_payables
+            .suspected_failed_payables
             .dump_cache();
         assert!(
             dumped_records.is_empty(),
-            "There should be no yet unproven failures but found {:?}.",
+            "There should be no suspected failures but found {:?}.",
             dumped_records
         );
         assert_eq!(
@@ -1199,7 +1183,9 @@ mod tests {
         );
         TestLogHandler::new().assert_logs_match_in_order(vec![
             &format!("INFO: {test_name}: Scanning for pending payable"),
-            &format!("DEBUG: {test_name}: Found 1 pending payables and 1 unfinalized failures to process"),
+            &format!(
+                "DEBUG: {test_name}: Found 1 pending payables and 1 suspected failures to process"
+            ),
         ])
     }
 
@@ -1340,8 +1326,8 @@ mod tests {
 
     #[test]
     #[should_panic(
-        expected = "internal error: entered unreachable code: Automatic pending payable \
-    scan should never start if there are no pending payables to process."
+        expected = "internal error: entered unreachable code: Automatic pending payable scan should \
+        never start if there are no pending payables to process."
     )]
     fn pending_payable_scanner_bumps_into_zero_pending_payable_awareness_in_the_automatic_mode() {
         let consuming_wallet = make_paying_wallet(b"consuming");
@@ -1360,11 +1346,12 @@ mod tests {
     }
 
     #[test]
-    fn check_general_conditions_for_pending_payable_scan_if_it_is_initial_pending_payable_scan() {
+    fn check_pending_payable_existence_for_initial_pending_payable_scan_and_zero_awareness() {
         let mut subject = make_dull_subject();
+        subject.aware_of_unresolved_pending_payable = false;
         subject.initial_pending_payable_scan = true;
 
-        let result = subject.check_general_conditions_for_pending_payable_scan(false, true);
+        let result = subject.check_pending_payable_existence(false);
 
         assert_eq!(result, Ok(()));
         assert_eq!(subject.initial_pending_payable_scan, true);

node/src/accountant/scanners/payable_scanner/mod.rs

@@ -251,6 +251,10 @@ impl PayableScanner {
             self.insert_records_in_sent_payables(&batch_results.sent_txs);
         }
         if failed > 0 {
+            debug!(
+                logger,
+                "Recording failed txs: {:?}", batch_results.failed_txs
+            );
             self.insert_records_in_failed_payables(&batch_results.failed_txs);
         }
     }

node/src/accountant/scanners/payable_scanner/start_scan.rs

@@ -66,7 +66,7 @@ impl StartableScanner<ScanForRetryPayables, InitialTemplatesMessage> for Payable
         info!(logger, "Scanning for retry payables");
         let failed_txs = self.get_txs_to_retry();
         let amount_from_payables = self.find_amount_from_payables(&failed_txs);
-        let retry_tx_templates = RetryTxTemplates::new(&failed_txs, &amount_from_payables);
+        let retry_tx_templates = RetryTxTemplates::new(&failed_txs, &amount_from_payables, logger);
 
         Ok(InitialTemplatesMessage {
             initial_templates: Either::Right(retry_tx_templates),
@@ -89,7 +89,7 @@ mod tests {
     use crate::accountant::scanners::payable_scanner::tx_templates::initial::retry::{
         RetryTxTemplate, RetryTxTemplates,
     };
-    use crate::accountant::scanners::Scanners;
+    use crate::accountant::scanners::payable_scanner::MultistageDualPayableScanner;
     use crate::accountant::test_utils::{
         make_payable_account, FailedPayableDaoMock, PayableDaoMock,
     };
@@ -144,7 +144,10 @@ mod tests {
             .payable_dao(payable_dao)
             .build();
 
-        let result = Scanners::start_correct_payable_scanner::<ScanForRetryPayables>(
+        let result = <(dyn MultistageDualPayableScanner) as StartableScanner<
+            ScanForRetryPayables,
+            InitialTemplatesMessage,
+        >>::start_scan(
             &mut subject,
             &consuming_wallet,
             timestamp,
@@ -157,11 +160,8 @@ mod tests {
         let retrieve_payables_params = retrieve_payables_params_arc.lock().unwrap();
         let expected_tx_templates = {
             let mut tx_template_1 = RetryTxTemplate::from(&failed_tx_1);
-            tx_template_1.base.amount_in_wei =
-                tx_template_1.base.amount_in_wei + payable_account_1.balance_wei;
-
+            tx_template_1.base.amount_in_wei = payable_account_1.balance_wei;
             let tx_template_2 = RetryTxTemplate::from(&failed_tx_2);
-
             RetryTxTemplates(vec![tx_template_1, tx_template_2])
         };
         assert_eq!(

node/src/accountant/scanners/payable_scanner/tx_templates/initial/retry.rs

@@ -1,6 +1,7 @@
 // Copyright (c) 2025, MASQ (https://masq.ai) and/or its affiliates. All rights reserved.
 use crate::accountant::db_access_objects::failed_payable_dao::FailedTx;
 use crate::accountant::scanners::payable_scanner::tx_templates::BaseTxTemplate;
+use masq_lib::logger::Logger;
 use std::collections::{BTreeSet, HashMap};
 use std::ops::{Deref, DerefMut};
 use web3::types::Address;
@@ -13,11 +14,25 @@ pub struct RetryTxTemplate {
 }
 
 impl RetryTxTemplate {
-    pub fn new(failed_tx: &FailedTx, payable_scan_amount_opt: Option<u128>) -> Self {
+    pub fn new(
+        failed_tx: &FailedTx,
+        updated_payable_balance_opt: Option<u128>,
+        logger: &Logger,
+    ) -> Self {
         let mut retry_template = RetryTxTemplate::from(failed_tx);
 
-        if let Some(payable_scan_amount) = payable_scan_amount_opt {
-            retry_template.base.amount_in_wei += payable_scan_amount;
+        debug!(logger, "Tx to retry {:?}", failed_tx);
+
+        if let Some(updated_payable_balance) = updated_payable_balance_opt {
+            debug!(
+                logger,
+                "Updating the pay for {:?} from former {} to latest accounted balance {} of minor",
+                failed_tx.receiver_address,
+                failed_tx.amount_minor,
+                updated_payable_balance
+            );
+
+            retry_template.base.amount_in_wei = updated_payable_balance;
         }
 
         retry_template
@@ -44,6 +59,7 @@ impl RetryTxTemplates {
     pub fn new(
         txs_to_retry: &BTreeSet<FailedTx>,
         amounts_from_payables: &HashMap<Address, u128>,
+        logger: &Logger,
     ) -> Self {
         Self(
             txs_to_retry
@@ -52,7 +68,7 @@ impl RetryTxTemplates {
                     let payable_scan_amount_opt = amounts_from_payables
                         .get(&tx_to_retry.receiver_address)
                         .copied();
-                    RetryTxTemplate::new(tx_to_retry, payable_scan_amount_opt)
+                    RetryTxTemplate::new(tx_to_retry, payable_scan_amount_opt, logger)
                 })
                 .collect(),
         )
@@ -98,6 +114,49 @@ mod tests {
     };
     use crate::accountant::scanners::payable_scanner::tx_templates::BaseTxTemplate;
     use crate::blockchain::test_utils::{make_address, make_tx_hash};
+    use masq_lib::logger::Logger;
+
+    #[test]
+    fn retry_tx_template_constructor_works() {
+        let receiver_address = make_address(42);
+        let amount_in_wei = 1_000_000;
+        let gas_price = 20_000_000_000;
+        let nonce = 123;
+        let tx_hash = make_tx_hash(789);
+        let failed_tx = FailedTx {
+            hash: tx_hash,
+            receiver_address,
+            amount_minor: amount_in_wei,
+            gas_price_minor: gas_price,
+            nonce,
+            timestamp: 1234567,
+            reason: FailureReason::PendingTooLong,
+            status: FailureStatus::RetryRequired,
+        };
+        let logger = Logger::new("test");
+        let fetched_balance_from_payable_table_opt_1 = None;
+        let fetched_balance_from_payable_table_opt_2 = Some(1_234_567);
+
+        let result_1 = RetryTxTemplate::new(
+            &failed_tx,
+            fetched_balance_from_payable_table_opt_1,
+            &logger,
+        );
+        let result_2 = RetryTxTemplate::new(
+            &failed_tx,
+            fetched_balance_from_payable_table_opt_2,
+            &logger,
+        );
+
+        let assert = |result: RetryTxTemplate, expected_amount_in_wei: u128| {
+            assert_eq!(result.base.receiver_address, receiver_address);
+            assert_eq!(result.base.amount_in_wei, expected_amount_in_wei);
+            assert_eq!(result.prev_gas_price_wei, gas_price);
+            assert_eq!(result.prev_nonce, nonce);
+        };
+        assert(result_1, amount_in_wei);
+        assert(result_2, fetched_balance_from_payable_table_opt_2.unwrap());
+    }
 
     #[test]
     fn retry_tx_template_can_be_created_from_failed_tx() {

node/src/accountant/scanners/payable_scanner/tx_templates/priced/new.rs

@@ -4,7 +4,7 @@ use crate::accountant::scanners::payable_scanner::tx_templates::initial::new::{
     NewTxTemplate, NewTxTemplates,
 };
 use crate::accountant::scanners::payable_scanner::tx_templates::BaseTxTemplate;
-use crate::blockchain::blockchain_bridge::increase_gas_price_by_margin;
+use crate::blockchain::blockchain_bridge::increase_by_percentage;
 use masq_lib::logger::Logger;
 use std::ops::Deref;
 use thousands::Separable;
@@ -63,7 +63,7 @@ impl PricedNewTxTemplates {
         ceil: u128,
         logger: &Logger,
     ) -> Self {
-        let computed_gas_price_wei = increase_gas_price_by_margin(latest_gas_price_wei);
+        let computed_gas_price_wei = increase_by_percentage(latest_gas_price_wei);
 
         let safe_gas_price_wei = if computed_gas_price_wei > ceil {
             warning!(