Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci-matrix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
2 changes: 1 addition & 1 deletion multinode_integration_tests/tests/verify_bill_payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
178 changes: 101 additions & 77 deletions node/src/accountant/mod.rs

Large diffs are not rendered by default.

9 changes: 6 additions & 3 deletions node/src/accountant/scanners/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ impl Scanners {
}

pub fn acknowledge_scan_error(&mut self, error: &ScanError, logger: &Logger) {
debug!(logger, "Acknowledging a scan that couldn't finish");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you could add more details to this log, using the contents of the error.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Insufficient logging detail in scan error handler (Bugbot Rules)

The debug log in acknowledge_scan_error logs a generic message without including error details. The PR reviewer explicitly requested adding more details using the contents of the error parameter, which contains scan_type, response_skeleton_opt, and msg fields that would aid debugging. The commit adds the log but doesn't incorporate the error's contents as requested.

Fix in Cursor Fix in Web

match error.scan_type {
DetailedScanType::NewPayables | DetailedScanType::RetryPayables => {
self.payable.mark_as_ended(logger)
Expand Down Expand Up @@ -794,11 +795,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!(
Expand Down Expand Up @@ -1199,7 +1200,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"
),
])
}

Expand Down
4 changes: 4 additions & 0 deletions node/src/accountant/scanners/payable_scanner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
7 changes: 2 additions & 5 deletions node/src/accountant/scanners/payable_scanner/start_scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -157,11 +157,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!(
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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(),
)
Expand Down Expand Up @@ -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() {
Expand Down
Loading
Loading