From 9226df70efe2634f1f50b3baf593d86be1c6c7e2 Mon Sep 17 00:00:00 2001 From: masqrauder <60554948+masqrauder@users.noreply.github.com> Date: Sat, 15 Jun 2024 20:26:20 -0400 Subject: [PATCH] GH-606: Apply PR feedback changes --- node/src/accountant/mod.rs | 32 ++-- node/src/accountant/scanners/mod.rs | 121 +++++++------ node/src/blockchain/blockchain_bridge.rs | 161 ++++++++++++++---- .../blockchain_interface_web3/mod.rs | 66 +++++-- .../data_structures/mod.rs | 6 +- node/src/database/config_dumper.rs | 8 +- .../src/db_config/persistent_configuration.rs | 47 +---- .../persistent_configuration_mock.rs | 12 -- 8 files changed, 279 insertions(+), 174 deletions(-) diff --git a/node/src/accountant/mod.rs b/node/src/accountant/mod.rs index bd444ef8f..827ad2114 100644 --- a/node/src/accountant/mod.rs +++ b/node/src/accountant/mod.rs @@ -123,7 +123,7 @@ pub struct ReceivedPayments { // a problem? Do we want to correct the timestamp? Discuss. pub timestamp: SystemTime, pub payments: Vec, - pub new_start_block: u64, + pub new_start_block: Option, pub response_skeleton_opt: Option, } @@ -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::{ @@ -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(); @@ -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, @@ -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 { @@ -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)]) @@ -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"); @@ -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()))] ) } diff --git a/node/src/accountant/scanners/mod.rs b/node/src/accountant/scanners/mod.rs index 61d607753..30ae79e90 100644 --- a/node/src/accountant/scanners/mod.rs +++ b/node/src/accountant/scanners/mod.rs @@ -859,15 +859,23 @@ impl Scanner 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) @@ -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 @@ -1623,7 +1637,7 @@ mod tests { .iter() .map(|ppayable| ppayable.hash) .collect::>(); - // 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) @@ -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)) } @@ -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), @@ -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!( @@ -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() @@ -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 @@ -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); @@ -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()); @@ -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( @@ -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) @@ -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 @@ -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) @@ -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 diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index c9afc97a7..80910a8ec 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -351,40 +351,43 @@ impl BlockchainBridge { .retrieve_transactions(start_block, end_block, &msg.recipient); match retrieved_transactions { Ok(transactions) => { - debug!( - self.logger, - "Write new start block: {}", transactions.new_start_block - ); - if let Err(e) = self - .persistent_config - .set_start_block(Some(transactions.new_start_block)) - { - panic! ("Cannot set start block {} in database; payments to you may not be processed: {:?}", transactions.new_start_block, e) - }; - if transactions.transactions.is_empty() { - debug!(self.logger, "No new receivable detected"); + if let BlockNumber::Number(new_start_block_number) = transactions.new_start_block { + debug!( + self.logger, + "Write new start block: {}", + new_start_block_number.as_u64() + ); + if let Err(e) = self + .persistent_config + .set_start_block(Some(new_start_block_number.as_u64())) + { + panic! ("Cannot set start block {} in database; payments to you may not be processed: {:?}", new_start_block_number.as_u64(), e) + }; + if transactions.transactions.is_empty() { + debug!(self.logger, "No new receivable detected"); + } + self.received_payments_subs_opt + .as_ref() + .expect("Accountant is unbound") + .try_send(ReceivedPayments { + timestamp: SystemTime::now(), + payments: transactions.transactions, + new_start_block: Some(new_start_block_number.as_u64()), + response_skeleton_opt: msg.response_skeleton_opt, + }) + .expect("Accountant is dead."); } - self.received_payments_subs_opt - .as_ref() - .expect("Accountant is unbound") - .try_send(ReceivedPayments { - timestamp: SystemTime::now(), - payments: transactions.transactions, - new_start_block: transactions.new_start_block, - response_skeleton_opt: msg.response_skeleton_opt, - }) - .expect("Accountant is dead."); Ok(()) } Err(e) => { if let Some(max_block_count) = self.extract_max_block_count(e.clone()) { - debug!(self.logger, "Writing max_block_count({})", max_block_count); + debug!(self.logger, "Writing max_block_count({})", &max_block_count); self.persistent_config .set_max_block_count(Some(max_block_count)) .map_or_else( |_| { - warning!(self.logger, "{} update max_block_count to {}. Scheduling next scan with that limit.", e, max_block_count); - Err(format!("{} updated max_block_count to {}. Scheduling next scan with that limit.", e, max_block_count)) + warning!(self.logger, "{} update max_block_count to {}. Scheduling next scan with that limit.", e, &max_block_count); + Err(format!("{} updated max_block_count to {}. Scheduling next scan with that limit.", e, &max_block_count)) }, |e| { warning!(self.logger, "Writing max_block_count failed: {:?}", e); @@ -1418,7 +1421,7 @@ mod tests { let amount = 42; let amount2 = 55; let expected_transactions = RetrievedBlockchainTransactions { - new_start_block: 8675309u64, + new_start_block: BlockNumber::Number(8675309u64.into()), transactions: vec![ BlockchainTransaction { block_number: 7, @@ -1490,7 +1493,7 @@ mod tests { &ReceivedPayments { timestamp: received_payments.timestamp, payments: expected_transactions.transactions, - new_start_block: 8675309u64, + new_start_block: Some(8675309u64), response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321 @@ -1512,7 +1515,7 @@ mod tests { let amount = 42; let amount2 = 55; let expected_transactions = RetrievedBlockchainTransactions { - new_start_block: 8675309u64, + new_start_block: BlockNumber::Number(8675309u64.into()), transactions: vec![ BlockchainTransaction { block_number: 8675308u64, @@ -1581,7 +1584,97 @@ mod tests { &ReceivedPayments { timestamp: received_payments.timestamp, payments: expected_transactions.transactions, - new_start_block: 8675309u64, + new_start_block: Some(8675309u64), + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: 4321 + }), + } + ); + } + + #[test] + fn handle_retrieve_transactions_with_latest_for_start_and_end_block_is_supported() { + let retrieve_transactions_params_arc = Arc::new(Mutex::new(vec![])); + let earning_wallet = make_wallet("somewallet"); + let amount = 42; + let amount2 = 55; + let expected_transactions = RetrievedBlockchainTransactions { + new_start_block: BlockNumber::Number(98765u64.into()), + transactions: vec![ + BlockchainTransaction { + block_number: 77, + from: earning_wallet.clone(), + wei_amount: amount, + }, + BlockchainTransaction { + block_number: 99, + from: earning_wallet.clone(), + wei_amount: amount2, + }, + ], + }; + + let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); + let system = System::new( + "handle_retrieve_transactions_with_latest_for_start_and_end_block_is_supported", + ); + let (accountant, _, accountant_recording_arc) = make_recorder(); + let persistent_config = PersistentConfigurationMock::new() + .max_block_count_result(Ok(None)) + .start_block_result(Ok(None)) + .set_start_block_params(&set_start_block_params_arc) + .set_start_block_result(Ok(())); + let latest_block_number = LatestBlockNumber::Err(BlockchainError::QueryFailed( + "Failed to read from block chain service".to_string(), + )); + let lower_interface = + LowBlockchainIntMock::default().get_block_number_result(latest_block_number); + let blockchain_interface = BlockchainInterfaceMock::default() + .retrieve_transactions_params(&retrieve_transactions_params_arc) + .retrieve_transactions_result(Ok(expected_transactions.clone())) + .lower_interface_results(Box::new(lower_interface)); + let subject = BlockchainBridge::new( + Box::new(blockchain_interface), + Box::new(persistent_config), + false, + Some(make_wallet("consuming")), + ); + let addr = subject.start(); + let subject_subs = BlockchainBridge::make_subs_from(&addr); + let peer_actors = peer_actors_builder().accountant(accountant).build(); + send_bind_message!(subject_subs, peer_actors); + let retrieve_transactions = RetrieveTransactions { + recipient: earning_wallet.clone(), + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: 4321, + }), + }; + let before = SystemTime::now(); + + let _ = addr.try_send(retrieve_transactions).unwrap(); + + System::current().stop(); + system.run(); + let after = SystemTime::now(); + let set_start_block_params = set_start_block_params_arc.lock().unwrap(); + assert_eq!(*set_start_block_params, vec![Some(98765u64)]); + let retrieve_transactions_params = retrieve_transactions_params_arc.lock().unwrap(); + assert_eq!( + *retrieve_transactions_params, + vec![(BlockNumber::Latest, BlockNumber::Latest, earning_wallet)] + ); + let accountant_received_payment = accountant_recording_arc.lock().unwrap(); + assert_eq!(accountant_received_payment.len(), 1); + let received_payments = accountant_received_payment.get_record::(0); + check_timestamp(before, received_payments.timestamp, after); + assert_eq!( + received_payments, + &ReceivedPayments { + timestamp: received_payments.timestamp, + payments: expected_transactions.transactions, + new_start_block: Some(98765), response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321 @@ -1601,7 +1694,7 @@ mod tests { let amount = 42; let amount2 = 55; let expected_transactions = RetrievedBlockchainTransactions { - new_start_block: 9876, + new_start_block: BlockNumber::Number(9876.into()), transactions: vec![ BlockchainTransaction { block_number: 7, @@ -1671,7 +1764,7 @@ mod tests { &ReceivedPayments { timestamp: received_payments.timestamp, payments: expected_transactions.transactions, - new_start_block: 9876, + new_start_block: Some(9876), response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321 @@ -1687,7 +1780,7 @@ mod tests { LowBlockchainIntMock::default().get_block_number_result(Ok(0u64.into())); let blockchain_interface_mock = BlockchainInterfaceMock::default() .retrieve_transactions_result(Ok(RetrievedBlockchainTransactions { - new_start_block: 7, + new_start_block: BlockNumber::Number(7.into()), transactions: vec![], })) .lower_interface_results(Box::new(lower_interface)); @@ -1735,7 +1828,7 @@ mod tests { &ReceivedPayments { timestamp: received_payments.timestamp, payments: vec![], - new_start_block: 7, + new_start_block: Some(7), response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321 @@ -1784,7 +1877,7 @@ mod tests { LowBlockchainIntMock::default().get_block_number_result(Ok(0u64.into())); let blockchain_interface = BlockchainInterfaceMock::default() .retrieve_transactions_result(Ok(RetrievedBlockchainTransactions { - new_start_block: 1234, + new_start_block: BlockNumber::Number(1234.into()), transactions: vec![BlockchainTransaction { block_number: 1000, from: make_wallet("somewallet"), diff --git a/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs b/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs index 09d54ad89..768b260cb 100644 --- a/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs +++ b/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs @@ -119,12 +119,12 @@ where .build(); let fallback_start_block_number = match end_block { - BlockNumber::Number(eb) => eb.as_u64(), + BlockNumber::Number(eb) => Some(eb.as_u64()), _ => { if let BlockNumber::Number(start_block_number) = start_block { - start_block_number.as_u64() + 1u64 + Some(start_block_number.as_u64() + 1u64) } else { - panic!("start_block of Latest, Earliest, and Pending are not supported"); + None } } }; @@ -137,12 +137,12 @@ where let response_block_number = match block_request.wait() { Ok(block_nbr) => { debug!(logger, "Latest block number: {}", block_nbr.as_u64()); - block_nbr.as_u64() + Some(block_nbr.as_u64()) } Err(_) => { debug!( logger, - "Using fallback block number: {}", fallback_start_block_number + "Using fallback block number: {:?}", fallback_start_block_number ); fallback_start_block_number } @@ -183,11 +183,14 @@ where ); debug!( logger, - "Discovered transaction max block nbr: {}", + "Discovered transaction max block nbr: {:?}", transaction_max_block_number ); Ok(RetrievedBlockchainTransactions { - new_start_block: 1u64 + transaction_max_block_number, + new_start_block: transaction_max_block_number + .map_or(BlockNumber::Latest, |nsb| { + BlockNumber::Number((1u64 + nsb).into()) + }), transactions, }) } @@ -601,15 +604,18 @@ where fn find_largest_transaction_block_number( &self, - response_block_number: u64, + response_block_number: Option, transactions: &[BlockchainTransaction], - ) -> u64 { + ) -> Option { if transactions.is_empty() { response_block_number } else { transactions .iter() - .fold(response_block_number, |a, b| a.max(b.block_number)) + .fold(response_block_number.unwrap_or(0u64), |a, b| { + a.max(b.block_number) + }) + .into() } } } @@ -830,7 +836,7 @@ mod tests { assert_eq!( result, RetrievedBlockchainTransactions { - new_start_block: 0x4be664, + new_start_block: BlockNumber::Number(0x4be664.into()), transactions: vec![ BlockchainTransaction { block_number: 0x4be663, @@ -892,7 +898,7 @@ mod tests { assert_eq!( result, RetrievedBlockchainTransactions { - new_start_block: 1 + end_block_nbr, + new_start_block: BlockNumber::Number((1 + end_block_nbr).into()), transactions: vec![] } ); @@ -1001,7 +1007,7 @@ mod tests { assert_eq!( result, Ok(RetrievedBlockchainTransactions { - new_start_block: 1 + end_block_nbr, + new_start_block: BlockNumber::Number((1 + end_block_nbr).into()), transactions: vec![] }) ); @@ -1043,7 +1049,37 @@ mod tests { assert_eq!( result, Ok(RetrievedBlockchainTransactions { - new_start_block: 1 + expected_fallback_start_block, + new_start_block: BlockNumber::Number((1 + expected_fallback_start_block).into()), + transactions: vec![] + }) + ); + } + + #[test] + fn blockchain_interface_retrieve_transactions_start_and_end_blocks_can_be_latest() { + let port = find_free_port(); + let _test_server = TestServer::start (port, vec![ + br#"[{"jsonrpc":"2.0","id":1,"result":"error"},{"jsonrpc":"2.0","id":2,"result":[{"address":"0xcd6c588e005032dd882cd43bf53a32129be81302","blockHash":"0x1a24b9169cbaec3f6effa1f600b70c7ab9e8e86db44062b49132a4415d26732a","data":"0x0000000000000000000000000000000000000000000000000010000000000000","logIndex":"0x0","removed":false,"topics":["0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef","0x0000000000000000000000003f69f9efd4f2592fd70be8c32ecd9dce71c472fc","0x000000000000000000000000adc1853c7859369639eb414b6342b36288fe6092"],"transactionHash":"0x955cec6ac4f832911ab894ce16aa22c3003f46deff3f7165b32700d2f5ff0681","transactionIndex":"0x0"}]}]"#.to_vec() + ]); + let (event_loop_handle, transport) = Http::with_max_parallel( + &format!("http://{}:{}", &Ipv4Addr::LOCALHOST, port), + REQUESTS_IN_PARALLEL, + ) + .unwrap(); + let chain = TEST_DEFAULT_CHAIN; + let subject = BlockchainInterfaceWeb3::new(transport, event_loop_handle, chain); + + let result = subject.retrieve_transactions( + BlockNumber::Latest, + BlockNumber::Latest, + &make_wallet("earning-wallet"), + ); + + let expected_new_start_block = BlockNumber::Latest; + assert_eq!( + result, + Ok(RetrievedBlockchainTransactions { + new_start_block: expected_new_start_block, transactions: vec![] }) ); @@ -1218,7 +1254,7 @@ mod tests { //exercising also the layer of web3 functions, but the transport layer is mocked init_test_logging(); let send_batch_params_arc = Arc::new(Mutex::new(vec![])); - //we compute the hashes ourselves during the batch preparation and so we don't care about + //we compute the hashes ourselves during the batch preparation, and so we don't care about //the same ones coming back with the response; we use the returned OKs as indicators of success only. //Any eventual rpc errors brought back are processed as well... let expected_batch_responses = vec![ diff --git a/node/src/blockchain/blockchain_interface/data_structures/mod.rs b/node/src/blockchain/blockchain_interface/data_structures/mod.rs index d1d785aae..d8b86d5d9 100644 --- a/node/src/blockchain/blockchain_interface/data_structures/mod.rs +++ b/node/src/blockchain/blockchain_interface/data_structures/mod.rs @@ -3,7 +3,7 @@ pub mod errors; use crate::accountant::db_access_objects::pending_payable_dao::PendingPayable; use crate::sub_lib::wallet::Wallet; -use web3::types::H256; +use web3::types::{BlockNumber, H256}; use web3::Error; #[derive(Clone, Debug, Eq, PartialEq)] @@ -13,9 +13,9 @@ pub struct BlockchainTransaction { pub wei_amount: u128, } -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq)] pub struct RetrievedBlockchainTransactions { - pub new_start_block: u64, + pub new_start_block: BlockNumber, pub transactions: Vec, } diff --git a/node/src/database/config_dumper.rs b/node/src/database/config_dumper.rs index e5874cd51..3e8991e64 100644 --- a/node/src/database/config_dumper.rs +++ b/node/src/database/config_dumper.rs @@ -669,12 +669,10 @@ mod tests { fn assert_null(key: &str, map: &Map) { assert!(map.contains_key(key)); - match map + let value = map .get(key) - .unwrap_or_else(|| panic!("record for {} is missing", key)) - { - value => assert!(value.is_null()), - } + .unwrap_or_else(|| panic!("record for {} is missing", key)); + assert!(value.is_null()) } fn assert_encrypted_value( diff --git a/node/src/db_config/persistent_configuration.rs b/node/src/db_config/persistent_configuration.rs index f710bc3f4..68a764f94 100644 --- a/node/src/db_config/persistent_configuration.rs +++ b/node/src/db_config/persistent_configuration.rs @@ -3,7 +3,7 @@ use crate::arbitrary_id_stamp_in_trait; use crate::blockchain::bip32::Bip32EncryptionKeyProvider; use crate::blockchain::bip39::{Bip39, Bip39Error}; -use crate::database::rusqlite_wrappers::{ConnectionWrapper, TransactionSafeWrapper}; +use crate::database::rusqlite_wrappers::ConnectionWrapper; use crate::db_config::config_dao::{ConfigDao, ConfigDaoError, ConfigDaoReal, ConfigDaoRecord}; use crate::db_config::secure_config_layer::{SecureConfigLayer, SecureConfigLayerError}; use crate::db_config::typed_config_layer::{ @@ -135,11 +135,6 @@ pub trait PersistentConfiguration { fn set_start_block(&mut self, value: Option) -> Result<(), PersistentConfigError>; fn max_block_count(&self) -> Result, PersistentConfigError>; fn set_max_block_count(&mut self, value: Option) -> Result<(), PersistentConfigError>; - fn set_start_block_from_txn( - &mut self, - value: u64, - transaction: &mut TransactionSafeWrapper, - ) -> Result<(), PersistentConfigError>; fn set_wallet_info( &mut self, consuming_wallet_private_key: &str, @@ -422,14 +417,6 @@ impl PersistentConfiguration for PersistentConfigurationReal { Ok(self.dao.set("max_block_count", encode_u64(value)?)?) } - fn set_start_block_from_txn( - &mut self, - value: u64, - transaction: &mut TransactionSafeWrapper, - ) -> Result<(), PersistentConfigError> { - self.simple_set_method_from_provided_txn("start_block", value, transaction) - } - fn set_wallet_info( &mut self, consuming_wallet_private_key: &str, @@ -565,17 +552,6 @@ impl PersistentConfigurationReal { Ok(self.dao.set(parameter_name, Some(value.to_string()))?) } - fn simple_set_method_from_provided_txn( - &mut self, - parameter_name: &str, - value: T, - txn: &mut TransactionSafeWrapper, - ) -> Result<(), PersistentConfigError> { - Ok(self - .dao - .set_by_guest_transaction(txn, parameter_name, Some(value.to_string()))?) - } - fn combined_params_get_method<'a, T, C>( &'a self, values_parser: C, @@ -605,12 +581,10 @@ mod tests { use crate::database::db_initializer::{ DbInitializationConfig, DbInitializer, DbInitializerReal, }; - use crate::database::test_utils::transaction_wrapper_mock::TransactionInnerWrapperMockBuilder; use crate::db_config::config_dao::ConfigDaoRecord; use crate::db_config::mocks::ConfigDaoMock; use crate::db_config::secure_config_layer::EXAMPLE_ENCRYPTED; use crate::test_utils::main_cryptde; - use crate::test_utils::unshared_test_utils::arbitrary_id_stamp::ArbitraryIdStamp; use bip39::{Language, MnemonicType}; use lazy_static::lazy_static; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; @@ -1510,7 +1484,7 @@ mod tests { } #[test] - fn set_start_block_success() { + fn set_start_block_success_with_some() { let set_params_arc = Arc::new(Mutex::new(vec![])); let config_dao = Box::new( ConfigDaoMock::new() @@ -1530,27 +1504,20 @@ mod tests { } #[test] - fn set_start_block_from_txn_success() { + fn set_start_block_success_with_none() { let set_params_arc = Arc::new(Mutex::new(vec![])); let config_dao = Box::new( ConfigDaoMock::new() - .set_by_guest_transaction_params(&set_params_arc) - .set_by_guest_transaction_result(Ok(())), + .set_params(&set_params_arc) + .set_result(Ok(())), ); - let txn_id = ArbitraryIdStamp::new(); - let txn_inner_builder = - TransactionInnerWrapperMockBuilder::default().set_arbitrary_id_stamp(txn_id); - let mut txn = TransactionSafeWrapper::new_with_builder(txn_inner_builder); let mut subject = PersistentConfigurationReal::new(config_dao); - let result = subject.set_start_block_from_txn(1234, &mut txn); + let result = subject.set_start_block(None); assert_eq!(result, Ok(())); let set_params = set_params_arc.lock().unwrap(); - assert_eq!( - *set_params, - vec![(txn_id, "start_block".to_string(), Some("1234".to_string()))] - ) + assert_eq!(*set_params, vec![("start_block".to_string(), None)]) } #[test] diff --git a/node/src/test_utils/persistent_configuration_mock.rs b/node/src/test_utils/persistent_configuration_mock.rs index ef1b85163..3638f6fde 100644 --- a/node/src/test_utils/persistent_configuration_mock.rs +++ b/node/src/test_utils/persistent_configuration_mock.rs @@ -2,7 +2,6 @@ #![cfg(test)] -use crate::database::rusqlite_wrappers::TransactionSafeWrapper; use crate::db_config::persistent_configuration::{PersistentConfigError, PersistentConfiguration}; use crate::sub_lib::accountant::{PaymentThresholds, ScanIntervals}; use crate::sub_lib::neighborhood::{Hops, NodeDescriptor, RatePack}; @@ -250,17 +249,6 @@ impl PersistentConfiguration for PersistentConfigurationMock { Self::result_from(&self.set_max_block_count_results) } - fn set_start_block_from_txn( - &mut self, - value: u64, - transaction: &mut TransactionSafeWrapper, - ) -> Result<(), PersistentConfigError> { - self.set_start_block_from_txn_params - .lock() - .unwrap() - .push((value, transaction.arbitrary_id_stamp())); - Self::result_from(&self.set_start_block_from_txn_results) - } fn set_wallet_info( &mut self, consuming_wallet_private_key: &str,