Skip to content

Commit

Permalink
GH-606: Apply PR feedback changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@masqrauder
masqrauder committed Jun 16, 2024
1 parent 82f39d6 commit 9226df7
Show file tree
Hide file tree
Showing 8 changed files with 279 additions and 174 deletions.
32 changes: 18 additions & 14 deletions node/src/accountant/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub struct ReceivedPayments {
// a problem? Do we want to correct the timestamp? Discuss.
pub timestamp: SystemTime,
pub payments: Vec<BlockchainTransaction>,
pub new_start_block: u64,
pub new_start_block: Option<u64>,
pub response_skeleton_opt: Option<ResponseSkeleton>,
}

Expand Down Expand Up @@ -1013,6 +1013,7 @@ mod tests {
use crate::blockchain::test_utils::{make_tx_hash, BlockchainInterfaceMock};
use crate::database::rusqlite_wrappers::TransactionSafeWrapper;
use crate::database::test_utils::transaction_wrapper_mock::TransactionInnerWrapperMockBuilder;
use crate::db_config::config_dao::ConfigDaoRecord;
use crate::db_config::mocks::ConfigDaoMock;
use crate::match_every_type_id;
use crate::sub_lib::accountant::{
Expand Down Expand Up @@ -1266,7 +1267,11 @@ mod tests {
config.suppress_initial_scans = true;
let subject = AccountantBuilder::default()
.bootstrapper_config(config)
.config_dao(ConfigDaoMock::new().set_result(Ok(())))
.config_dao(
ConfigDaoMock::new()
.get_result(Ok(ConfigDaoRecord::new("start_block", None, false)))
.set_result(Ok(())),
)
.build();
let (ui_gateway, _, ui_gateway_recording_arc) = make_recorder();
let subject_addr = subject.start();
Expand All @@ -1276,7 +1281,7 @@ mod tests {
let received_payments = ReceivedPayments {
timestamp: SystemTime::now(),
payments: vec![],
new_start_block: 1234567,
new_start_block: Some(1234567),
response_skeleton_opt: Some(ResponseSkeleton {
client_id: 1234,
context_id: 4321,
Expand Down Expand Up @@ -1876,7 +1881,8 @@ mod tests {
) {
let more_money_received_params_arc = Arc::new(Mutex::new(vec![]));
let commit_params_arc = Arc::new(Mutex::new(vec![]));
let set_by_guest_transaction_params_arc = Arc::new(Mutex::new(vec![]));
let set_params_arc = Arc::new(Mutex::new(vec![]));
let get_params_arc = Arc::new(Mutex::new(vec![]));
let now = SystemTime::now();
let earning_wallet = make_wallet("earner3000");
let expected_receivable_1 = BlockchainTransaction {
Expand All @@ -1899,8 +1905,10 @@ mod tests {
.more_money_received_params(&more_money_received_params_arc)
.more_money_received_result(wrapped_transaction);
let config_dao = ConfigDaoMock::new()
.set_by_guest_transaction_params(&set_by_guest_transaction_params_arc)
.set_by_guest_transaction_result(Ok(()));
.get_params(&get_params_arc)
.get_result(Ok(ConfigDaoRecord::new("start_block", None, false)))
.set_params(&set_params_arc)
.set_result(Ok(()));
let accountant = AccountantBuilder::default()
.bootstrapper_config(bc_from_earning_wallet(earning_wallet.clone()))
.receivable_daos(vec![ForReceivableScanner(receivable_dao)])
Expand All @@ -1913,7 +1921,7 @@ mod tests {
.try_send(ReceivedPayments {
timestamp: now,
payments: vec![expected_receivable_1.clone(), expected_receivable_2.clone()],
new_start_block: 123456789,
new_start_block: Some(123456789u64),
response_skeleton_opt: None,
})
.expect("unexpected actix error");
Expand All @@ -1927,14 +1935,10 @@ mod tests {
);
let commit_params = commit_params_arc.lock().unwrap();
assert_eq!(*commit_params, vec![()]);
let set_by_guest_transaction_params = set_by_guest_transaction_params_arc.lock().unwrap();
let set_params = set_params_arc.lock().unwrap();
assert_eq!(
*set_by_guest_transaction_params,
vec![(
transaction_id,
"start_block".to_string(),
Some("123456789".to_string())
)]
*set_params,
vec![("start_block".to_string(), Some("123456789".to_string()))]
)
}

Expand Down
121 changes: 70 additions & 51 deletions node/src/accountant/scanners/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -859,15 +859,23 @@ impl Scanner<RetrieveTransactions, ReceivedPayments> for ReceivableScanner {
"No newly received payments were detected during the scanning process."
);

match self
.persistent_configuration
.set_start_block(Some(msg.new_start_block))
{
Ok(()) => debug!(logger, "Start block updated to {}", msg.new_start_block),
Err(e) => panic!(
"Attempt to set new start block to {} failed due to: {:?}",
msg.new_start_block, e
),
if let Some(new_start_block) = msg.new_start_block {
let current_start_block = match self.persistent_configuration.start_block() {
Ok(Some(current_start_block)) => current_start_block,
_ => 0u64,
};
if new_start_block > current_start_block {
match self
.persistent_configuration
.set_start_block(msg.new_start_block)
{
Ok(()) => debug!(logger, "Start block updated to {}", &new_start_block),
Err(e) => panic!(
"Attempt to set new start block to {} failed due to: {:?}",
&new_start_block, e
),
}
}
}
} else {
self.handle_new_received_payments(&msg, logger)
Expand Down Expand Up @@ -907,28 +915,34 @@ impl ReceivableScanner {
}

fn handle_new_received_payments(&mut self, msg: &ReceivedPayments, logger: &Logger) {
let mut txn = self
let txn = self
.receivable_dao
.as_mut()
.more_money_received(msg.timestamp, &msg.payments);

let new_start_block = msg.new_start_block;
match self
.persistent_configuration
.set_start_block_from_txn(new_start_block, &mut txn)
{
Ok(()) => (),
Err(e) => panic!(
"Attempt to set new start block to {} failed due to: {:?}",
new_start_block, e
),
}

match txn.commit() {
Ok(_) => {
debug!(logger, "Updated start block to: {}", new_start_block)
if let Some(new_start_block) = msg.new_start_block {
let current_start_block = match self.persistent_configuration.start_block() {
Ok(Some(start_block)) => start_block,
_ => 0u64,
};
if new_start_block > current_start_block {
match self
.persistent_configuration
.set_start_block(msg.new_start_block)
{
Ok(()) => (),
Err(e) => panic!(
"Attempt to set new start block to {} failed due to: {:?}",
new_start_block, e
),
}
match txn.commit() {
Ok(_) => {
debug!(logger, "Updated start block to: {}", new_start_block)
}
Err(e) => panic!("Commit of received transactions failed: {:?}", e),
}
}
Err(e) => panic!("Commit of received transactions failed: {:?}", e),
}

let total_newly_paid_receivable = msg
Expand Down Expand Up @@ -1623,7 +1637,7 @@ mod tests {
.iter()
.map(|ppayable| ppayable.hash)
.collect::<HashSet<H256>>();
// Not in an ascending order
// Not in ascending order
let rowids_and_hashes_from_fingerprints = vec![(hash_1, 3), (hash_3, 5), (hash_2, 6)]
.iter()
.map(|(hash, _id)| *hash)
Expand Down Expand Up @@ -1875,7 +1889,7 @@ mod tests {
00000000000000000000000000000000000000000315 failed due to RecordDeletion(\"Gosh, I overslept \
without an alarm set\")");
let log_handler = TestLogHandler::new();
// There is a possible situation when we stumble over missing fingerprints and so we log it.
// There is a possible situation when we stumble over missing fingerprints, so we log it.
// Here we don't and so any ERROR log shouldn't turn up
log_handler.exists_no_log_containing(&format!("ERROR: {}", test_name))
}
Expand Down Expand Up @@ -2075,9 +2089,7 @@ mod tests {
};
let payable_thresholds_gauge = PayableThresholdsGaugeMock::default()
.is_innocent_age_params(&is_innocent_age_params_arc)
.is_innocent_age_result(
debt_age_s <= custom_payment_thresholds.maturity_threshold_sec as u64,
)
.is_innocent_age_result(debt_age_s <= custom_payment_thresholds.maturity_threshold_sec)
.is_innocent_balance_params(&is_innocent_balance_params_arc)
.is_innocent_balance_result(
balance <= gwei_to_wei(custom_payment_thresholds.permanent_debt_allowed_gwei),
Expand All @@ -2098,7 +2110,7 @@ mod tests {
assert_eq!(debt_age_returned_innocent, debt_age_s);
assert_eq!(
curve_derived_time,
custom_payment_thresholds.maturity_threshold_sec as u64
custom_payment_thresholds.maturity_threshold_sec
);
let is_innocent_balance_params = is_innocent_balance_params_arc.lock().unwrap();
assert_eq!(
Expand Down Expand Up @@ -3043,8 +3055,9 @@ mod tests {
init_test_logging();
let test_name = "receivable_scanner_aborts_scan_if_no_payments_were_supplied";
let set_start_block_params_arc = Arc::new(Mutex::new(vec![]));
let new_start_block = 4321;
let new_start_block = Some(4321);
let persistent_config = PersistentConfigurationMock::new()
.start_block_result(Ok(None))
.set_start_block_params(&set_start_block_params_arc)
.set_start_block_result(Ok(()));
let mut subject = ReceivableScannerBuilder::new()
Expand Down Expand Up @@ -3074,16 +3087,21 @@ mod tests {
init_test_logging();
let test_name = "no_transactions_received_but_start_block_setting_fails";
let now = SystemTime::now();
let persistent_config = PersistentConfigurationMock::new().set_start_block_result(Err(
PersistentConfigError::UninterpretableValue("Illiterate database manager".to_string()),
));
let set_start_block_params_arc = Arc::new(Mutex::new(vec![]));
let new_start_block = Some(6709u64);
let persistent_config = PersistentConfigurationMock::new()
.start_block_result(Ok(None))
.set_start_block_params(&set_start_block_params_arc)
.set_start_block_result(Err(PersistentConfigError::UninterpretableValue(
"Illiterate database manager".to_string(),
)));
let mut subject = ReceivableScannerBuilder::new()
.persistent_configuration(persistent_config)
.build();
let msg = ReceivedPayments {
timestamp: now,
payments: vec![],
new_start_block: 6709,
new_start_block,
response_skeleton_opt: None,
};
// Not necessary, rather for preciseness
Expand All @@ -3107,8 +3125,9 @@ mod tests {
.set_arbitrary_id_stamp(transaction_id);
let transaction = TransactionSafeWrapper::new_with_builder(txn_inner_builder);
let persistent_config = PersistentConfigurationMock::new()
.set_start_block_from_txn_params(&set_start_block_from_txn_params_arc)
.set_start_block_from_txn_result(Ok(()));
.start_block_result(Ok(None))
.set_start_block_params(&set_start_block_from_txn_params_arc)
.set_start_block_result(Ok(()));
let receivable_dao = ReceivableDaoMock::new()
.more_money_received_params(&more_money_received_params_arc)
.more_money_received_result(transaction);
Expand All @@ -3134,7 +3153,7 @@ mod tests {
let msg = ReceivedPayments {
timestamp: now,
payments: receivables.clone(),
new_start_block: 7890123,
new_start_block: Some(7890123),
response_skeleton_opt: None,
};
subject.mark_as_started(SystemTime::now());
Expand All @@ -3151,10 +3170,7 @@ mod tests {
let more_money_received_params = more_money_received_params_arc.lock().unwrap();
assert_eq!(*more_money_received_params, vec![(now, receivables)]);
let set_by_guest_transaction_params = set_start_block_from_txn_params_arc.lock().unwrap();
assert_eq!(
*set_by_guest_transaction_params,
vec![(7890123, transaction_id)]
);
assert_eq!(*set_by_guest_transaction_params, vec![Some(7890123u64)]);
let commit_params = commit_params_arc.lock().unwrap();
assert_eq!(*commit_params, vec![()]);
TestLogHandler::new().exists_log_matching(
Expand All @@ -3171,9 +3187,11 @@ mod tests {
let now = SystemTime::now();
let txn_inner_builder = TransactionInnerWrapperMockBuilder::default();
let transaction = TransactionSafeWrapper::new_with_builder(txn_inner_builder);
let persistent_config = PersistentConfigurationMock::new().set_start_block_from_txn_result(
Err(PersistentConfigError::DatabaseError("Fatigue".to_string())),
);
let persistent_config = PersistentConfigurationMock::new()
.start_block_result(Ok(None))
.set_start_block_result(Err(PersistentConfigError::DatabaseError(
"Fatigue".to_string(),
)));
let receivable_dao = ReceivableDaoMock::new().more_money_received_result(transaction);
let mut subject = ReceivableScannerBuilder::new()
.receivable_dao(receivable_dao)
Expand All @@ -3187,7 +3205,7 @@ mod tests {
let msg = ReceivedPayments {
timestamp: now,
payments: receivables,
new_start_block: 7890123,
new_start_block: Some(7890123),
response_skeleton_opt: None,
};
// Not necessary, rather for preciseness
Expand Down Expand Up @@ -3215,8 +3233,9 @@ mod tests {
let txn_inner_builder =
TransactionInnerWrapperMockBuilder::default().commit_result(commit_err);
let transaction = TransactionSafeWrapper::new_with_builder(txn_inner_builder);
let persistent_config =
PersistentConfigurationMock::new().set_start_block_from_txn_result(Ok(()));
let persistent_config = PersistentConfigurationMock::new()
.start_block_result(Ok(None))
.set_start_block_result(Ok(()));
let receivable_dao = ReceivableDaoMock::new().more_money_received_result(transaction);
let mut subject = ReceivableScannerBuilder::new()
.receivable_dao(receivable_dao)
Expand All @@ -3230,7 +3249,7 @@ mod tests {
let msg = ReceivedPayments {
timestamp: now,
payments: receivables,
new_start_block: 7890123,
new_start_block: Some(7890123),
response_skeleton_opt: None,
};
// Not necessary, rather for preciseness
Expand Down

0 comments on commit 9226df7

Please sign in to comment.