From 29c1ba044ecc4c8ce83c9c608fc7ad144b532bdc Mon Sep 17 00:00:00 2001 From: masqrauder <60554948+masqrauder@users.noreply.github.com> Date: Sun, 19 Jan 2025 16:43:51 -0500 Subject: [PATCH 1/3] GH-552: Eliminate extra block increment --- node/src/blockchain/blockchain_bridge.rs | 205 +++++++++++++++++- .../blockchain_interface_web3/mod.rs | 197 +++++++++++++++-- 2 files changed, 386 insertions(+), 16 deletions(-) diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index 67d018e27..a0c665b72 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -1290,7 +1290,7 @@ mod tests { let amount = 42; let amount2 = 55; let expected_transactions = RetrievedBlockchainTransactions { - new_start_block: BlockNumber::Number(8675309u64.into()), + new_start_block: BlockNumber::Number(100_006u64.into()), transactions: vec![ BlockchainTransaction { block_number: 7, @@ -1356,7 +1356,7 @@ mod tests { &ReceivedPayments { timestamp: received_payments.timestamp, payments: expected_transactions.transactions, - new_start_block: 8675309u64, + new_start_block: 100_006u64, response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321 @@ -1703,6 +1703,207 @@ mod tests { let _ = subject.handle_retrieve_transactions(retrieve_transactions); } + #[test] + fn handle_retrieve_transactions_repeated_calling_does_not_skip_blocks() { + let retrieve_transactions_params_arc = Arc::new(Mutex::new(vec![])); + let system = + System::new("handle_retrieve_transactions_repeated_calling_does_not_skip_blocks"); + let (accountant, _, accountant_recording_arc) = make_recorder(); + let earning_wallet = make_wallet("earningwallet"); + let amount1 = 42; + let amount2 = 55; + let amount3 = 75; + let amount4 = 99; + let initial_start_block: u64 = 0x45f2f3; // 4_584_179 + let new_start_block_1 = initial_start_block + 1 + DEFAULT_MAX_BLOCK_COUNT; + let expected_transactions1 = RetrievedBlockchainTransactions { + new_start_block: BlockNumber::Number(new_start_block_1.into()), + transactions: vec![BlockchainTransaction { + block_number: initial_start_block + 0x14382, + from: earning_wallet.clone(), + wei_amount: amount1, + }], + }; + let new_start_block_2 = initial_start_block + 2 + (2 * DEFAULT_MAX_BLOCK_COUNT); + let expected_transactions2 = RetrievedBlockchainTransactions { + new_start_block: BlockNumber::Number(new_start_block_2.into()), + transactions: vec![BlockchainTransaction { + block_number: initial_start_block + 0x308b3, + from: earning_wallet.clone(), + wei_amount: amount2, + }], + }; + let new_start_block_3 = initial_start_block + 3 + (3 * DEFAULT_MAX_BLOCK_COUNT); + let expected_transactions3 = RetrievedBlockchainTransactions { + new_start_block: BlockNumber::Number(new_start_block_3.into()), + transactions: vec![BlockchainTransaction { + block_number: initial_start_block + 0x34248, + from: earning_wallet.clone(), + wei_amount: amount3, + }], + }; + let new_start_block_4 = 0x4be667; + let expected_transactions4 = RetrievedBlockchainTransactions { + new_start_block: BlockNumber::Number(new_start_block_4.into()), + transactions: vec![BlockchainTransaction { + block_number: initial_start_block + 0x49b7e, + from: earning_wallet.clone(), + wei_amount: amount4, + }], + }; + let lower_interface = LowBlockchainIntMock::default() + .get_block_number_result(Ok(0x4be663.into())) + .get_block_number_result(Ok(0x4be664.into())) + .get_block_number_result(Ok(0x4be665.into())) + .get_block_number_result(Ok(new_start_block_4.into())); + let blockchain_interface = BlockchainInterfaceMock::default() + .lower_interface_results(Box::new(lower_interface)) + .retrieve_transactions_params(&retrieve_transactions_params_arc) + .retrieve_transactions_result(Ok(expected_transactions1.clone())) + .retrieve_transactions_result(Ok(expected_transactions2.clone())) + .retrieve_transactions_result(Ok(expected_transactions3.clone())) + .retrieve_transactions_result(Ok(expected_transactions4.clone())); + let persistent_config = PersistentConfigurationMock::new() + .max_block_count_result(Ok(Some(DEFAULT_MAX_BLOCK_COUNT))) + .start_block_result(Ok(Some(initial_start_block))) + .start_block_result(Ok(Some(new_start_block_1))) + .start_block_result(Ok(Some(new_start_block_2))) + .start_block_result(Ok(Some(new_start_block_3))) + .start_block_result(Ok(Some(new_start_block_4))); + let subject = BlockchainBridge::new( + Box::new(blockchain_interface), + Box::new(persistent_config), + false, + ); + 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_transactions1 = RetrieveTransactions { + recipient: earning_wallet.clone(), + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: 1, + }), + }; + let retrieve_transactions2 = RetrieveTransactions { + recipient: earning_wallet.clone(), + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: 2, + }), + }; + let retrieve_transactions3 = RetrieveTransactions { + recipient: earning_wallet.clone(), + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: 3, + }), + }; + let retrieve_transactions4 = RetrieveTransactions { + recipient: earning_wallet.clone(), + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: 4, + }), + }; + let before = SystemTime::now(); + + let _ = addr.try_send(retrieve_transactions1).unwrap(); + let _ = addr.try_send(retrieve_transactions2).unwrap(); + let _ = addr.try_send(retrieve_transactions3).unwrap(); + let _ = addr.try_send(retrieve_transactions4).unwrap(); + + System::current().stop(); + system.run(); + let after = SystemTime::now(); + + let retrieve_transactions_params = retrieve_transactions_params_arc.lock().unwrap(); + assert_eq!( + *retrieve_transactions_params, + vec![ + ( + BlockNumber::Number(initial_start_block.into()), + BlockNumber::Number((initial_start_block + DEFAULT_MAX_BLOCK_COUNT).into()), + earning_wallet.clone() + ), + ( + BlockNumber::Number(new_start_block_1.into()), + BlockNumber::Number((new_start_block_1 + DEFAULT_MAX_BLOCK_COUNT).into()), + earning_wallet.clone() + ), + ( + BlockNumber::Number(new_start_block_2.into()), + BlockNumber::Number((new_start_block_2 + DEFAULT_MAX_BLOCK_COUNT).into()), + earning_wallet.clone() + ), + ( + BlockNumber::Number(new_start_block_3.into()), + BlockNumber::Number(new_start_block_4.into()), + earning_wallet.clone() + ), + ] + ); + let accountant_received_payment = accountant_recording_arc.lock().unwrap(); + assert_eq!(accountant_received_payment.len(), 4); + let received_payments1 = accountant_received_payment.get_record::(0); + check_timestamp(before, received_payments1.timestamp, after); + assert_eq!( + received_payments1, + &ReceivedPayments { + timestamp: received_payments1.timestamp, + payments: expected_transactions1.transactions, + new_start_block: new_start_block_1, + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: 1 + }), + } + ); + let received_payments2 = accountant_received_payment.get_record::(1); + check_timestamp(before, received_payments2.timestamp, after); + assert_eq!( + received_payments2, + &ReceivedPayments { + timestamp: received_payments2.timestamp, + payments: expected_transactions2.transactions, + new_start_block: new_start_block_2, + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: 2 + }), + } + ); + let received_payments3 = accountant_received_payment.get_record::(2); + check_timestamp(before, received_payments3.timestamp, after); + assert_eq!( + received_payments3, + &ReceivedPayments { + timestamp: received_payments3.timestamp, + payments: expected_transactions3.transactions, + new_start_block: new_start_block_3, + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: 3 + }), + } + ); + let received_payments4 = accountant_received_payment.get_record::(3); + check_timestamp(before, received_payments4.timestamp, after); + assert_eq!( + received_payments4, + &ReceivedPayments { + timestamp: received_payments4.timestamp, + payments: expected_transactions4.transactions, + new_start_block: new_start_block_4, + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: 4 + }), + } + ); + } + fn success_handler( _bcb: &mut BlockchainBridge, _msg: RetrieveTransactions, 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 a4425a82f..40603b9d6 100644 --- a/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs +++ b/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs @@ -118,11 +118,11 @@ where ) .build(); - let fallback_start_block_number = match end_block { + let end_block_number_opt = match end_block { BlockNumber::Number(eb) => Some(eb.as_u64()), _ => { if let BlockNumber::Number(start_block_number) = start_block { - Some(start_block_number.as_u64() + 1u64) + Some(start_block_number.as_u64()) } else { None } @@ -142,9 +142,9 @@ where Err(_) => { debug!( logger, - "Using fallback block number: {:?}", fallback_start_block_number + "Using fallback block number: {:?}", end_block_number_opt ); - fallback_start_block_number + end_block_number_opt } }; @@ -178,7 +178,7 @@ where // was not successful. let transaction_max_block_number = self .find_largest_transaction_block_number( - response_block_number_opt, + response_block_number_opt.min(end_block_number_opt), &transactions, ); debug!( @@ -671,6 +671,7 @@ mod tests { use masq_lib::utils::find_free_port; use serde_derive::Deserialize; use serde_json::{json, Value}; + use std::default::Default; use std::net::Ipv4Addr; use crate::accountant::db_access_objects::payable_dao::PayableAccount; @@ -683,6 +684,7 @@ mod tests { BlockchainTransaction, RpcPayablesFailure, }; use indoc::indoc; + use masq_lib::constants::DEFAULT_MAX_BLOCK_COUNT; use sodiumoxide::hex; use std::str::FromStr; use std::sync::{Arc, Mutex}; @@ -1020,7 +1022,7 @@ mod tests { } #[test] - fn blockchain_interface_non_clandestine_retrieve_transactions_uses_block_number_latest_as_fallback_start_block_plus_one( + fn blockchain_interface_non_clandestine_retrieve_transactions_uses_block_number_latest_as_start_block_plus_one( ) { let port = find_free_port(); let _test_server = TestServer::start (port, vec![ @@ -1035,23 +1037,18 @@ mod tests { let subject = BlockchainInterfaceWeb3::new(transport, event_loop_handle, chain); let start_block = BlockNumber::Number(42u64.into()); + let expected_new_start_block = BlockNumber::Number(43u64.into()); + let result = subject.retrieve_transactions( start_block, BlockNumber::Latest, &Wallet::from_str("0x3f69f9efd4f2592fd70be8c32ecd9dce71c472fc").unwrap(), ); - let expected_fallback_start_block = - if let BlockNumber::Number(start_block_nbr) = start_block { - start_block_nbr.as_u64() + 1u64 - } else { - panic!("start_block of Latest, Earliest, and Pending are not supported!") - }; - assert_eq!( result, Ok(RetrievedBlockchainTransactions { - new_start_block: BlockNumber::Number((1 + expected_fallback_start_block).into()), + new_start_block: expected_new_start_block, transactions: vec![] }) ); @@ -2311,4 +2308,176 @@ mod tests { TRANSFER_METHOD_ID, ); } + + #[test] + fn retrieve_transactions_ensures_no_skipped_blocks() { + let to = "0x3f69f9efd4f2592fd70be8c32ecd9dce71c472fc"; + let port = find_free_port(); + #[rustfmt::skip] + let _test_server = TestServer::start (port, vec![ + br#"[{"jsonrpc":"2.0","id":1,"result":"0x40bbf8"},{ + "jsonrpc":"2.0", + "id":2, + "result":[ + { + "address":"0xcd6c588e005032dd882cd43bf53a32129be81302", + "blockHash":"0x1a24b9169cbaec3f6effa1f600b70c7ab9e8e86db44062b49132a4415d26732a", + "blockNumber":"0x14382", + "data":"0x0000000000000000000000000000000000000000000000000010000000000000", + "logIndex":"0x0", + "removed":false, + "topics":[ + "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", + "0x0000000000000000000000003ab28ecedea6cdb6feed398e93ae8c7b316b1182", + "0x000000000000000000000000adc1853c7859369639eb414b6342b36288fe6092" + ], + "transactionHash":"0x955cec6ac4f832911ab894ce16aa22c3003f46deff3f7165b32700d2f5ff0681", + "transactionIndex":"0x0" + } + ]}]"#.to_vec(), + br#"[{"jsonrpc":"2.0","id":1,"result":"0x40bbf9"},{ + "jsonrpc":"2.0", + "id":2, + "result":[ + { + "address":"0xcd6c588e005032dd882cd43bf53a32129be81302", + "blockHash":"0x1a24b9169cbaec3f6effa1f600b70c7ab9e8e86db44062b49132a4415d26732b", + "blockNumber":"0x308b3", + "data":"0x0000000000000000000000000000000000000000000000000010000000000000", + "logIndex":"0x0", + "removed":false, + "topics":[ + "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", + "0x0000000000000000000000003ab28ecedea6cdb6feed398e93ae8c7b316b1182", + "0x000000000000000000000000adc1853c7859369639eb414b6342b36288fe6092" + ], + "transactionHash":"0x955cec6ac4f832911ab894ce16aa22c3003f46deff3f7165b32700d2f5ff0680", + "transactionIndex":"0x0" + } + ]}]"#.to_vec(), + br#"[{"jsonrpc":"2.0","id":1,"result":"0x40bbfa"},{ + "jsonrpc":"2.0", + "id":2, + "result":[ + { + "address":"0xcd6c588e005032dd882cd43bf53a32129be81302", + "blockHash":"0x1a24b9169cbaec3f6effa1f600b70c7ab9e8e86db44062b49132a4415d26732a", + "blockNumber":"0x34248", + "data":"0x0000000000000000000000000000000000000000000000000010000000000000", + "logIndex":"0x0", + "removed":false, + "topics":[ + "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", + "0x0000000000000000000000003ab28ecedea6cdb6feed398e93ae8c7b316b1182", + "0x000000000000000000000000adc1853c7859369639eb414b6342b36288fe6092" + ], + "transactionHash":"0x955cec6ac4f832911ab894ce16aa22c3003f46deff3f7165b32700d2f5ff0681", + "transactionIndex":"0x0" + } + ]}]"#.to_vec(), + br#"[{"jsonrpc":"2.0","id":1,"result":"0x40bbfb"},{ + "jsonrpc":"2.0", + "id":2, + "result":[ + { + "address":"0xcd6c588e005032dd882cd43bf53a32129be81302", + "blockHash":"0x1a24b9169cbaec3f6effa1f600b70c7ab9e8e86db44062b49132a4415d26732a", + "blockNumber":"0x49b7e", + "data":"0x0000000000000000000000000000000000000000000000000010000000000000", + "logIndex":"0x0", + "removed":false, + "topics":[ + "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", + "0x0000000000000000000000003ab28ecedea6cdb6feed398e93ae8c7b316b1182", + "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 _end_block_nbr = 0x4be662u64; + let wallet = Wallet::new(to); + init_test_logging(); + + // First call to retrieve_transactions + let end_block_1 = BlockNumber::Number((1u64 + DEFAULT_MAX_BLOCK_COUNT).into()); + let result1 = subject + .retrieve_transactions(BlockNumber::Number(1u64.into()), end_block_1, &wallet) + .unwrap(); + + // Second call to retrieve_transactions with non-overlapping but continuous block range + let new_start_block_1 = if let BlockNumber::Number(value) = result1.new_start_block { + value.as_u64() + } else { + panic!("Unexpected block number type") + }; + + let end_block_2 = BlockNumber::Number((new_start_block_1 + DEFAULT_MAX_BLOCK_COUNT).into()); + let result2 = subject + .retrieve_transactions(result1.new_start_block, end_block_2, &wallet) + .unwrap(); + + let new_start_block_2 = if let BlockNumber::Number(value) = result2.new_start_block { + value.as_u64() + } else { + panic!("Unexpected block number type") + }; + let end_block_3 = BlockNumber::Number((new_start_block_2 + DEFAULT_MAX_BLOCK_COUNT).into()); + let result3 = subject + .retrieve_transactions(result2.new_start_block, end_block_3, &wallet) + .unwrap(); + + let new_start_block_3 = if let BlockNumber::Number(value) = result3.new_start_block { + value.as_u64() + } else { + panic!("Unexpected block number type") + }; + + let end_block_4 = BlockNumber::Number((new_start_block_3 + DEFAULT_MAX_BLOCK_COUNT).into()); + let result4 = subject + .retrieve_transactions(result3.new_start_block, end_block_4, &wallet) + .unwrap(); + + let new_start_block_4 = if let BlockNumber::Number(value) = result4.new_start_block { + value.as_u64() + } else { + panic!("Unexpected block number type") + }; + + assert_eq!(new_start_block_1, 2 + DEFAULT_MAX_BLOCK_COUNT); + assert_eq!( + new_start_block_2, + 1 + new_start_block_1 + DEFAULT_MAX_BLOCK_COUNT + ); + assert_eq!( + new_start_block_3, + 1 + new_start_block_2 + DEFAULT_MAX_BLOCK_COUNT + ); + assert_eq!( + new_start_block_4, + 1 + new_start_block_3 + DEFAULT_MAX_BLOCK_COUNT + ); + + let test_log_handler = TestLogHandler::new(); + test_log_handler.exists_log_containing( + "Retrieving transactions from start block: Number(1) to end block: Number(100001) for: 0x3f69f9efd4f2592fd70be8c32ecd9dce71c472fc chain_id: 3 contract: 0x384dec25e03f94931767ce4c3556168468ba24c3", + ); + test_log_handler.exists_log_containing( + "Retrieving transactions from start block: Number(100002) to end block: Number(200002) for: 0x3f69f9efd4f2592fd70be8c32ecd9dce71c472fc chain_id: 3 contract: 0x384dec25e03f94931767ce4c3556168468ba24c3", + ); + test_log_handler.exists_log_containing( + "Retrieving transactions from start block: Number(200003) to end block: Number(300003) for: 0x3f69f9efd4f2592fd70be8c32ecd9dce71c472fc chain_id: 3 contract: 0x384dec25e03f94931767ce4c3556168468ba24c3", + ); + test_log_handler.exists_log_containing( + "Retrieving transactions from start block: Number(300004) to end block: Number(400004) for: 0x3f69f9efd4f2592fd70be8c32ecd9dce71c472fc chain_id: 3 contract: 0x384dec25e03f94931767ce4c3556168468ba24c3", + ); + } } From eed3e898e02adf3adac580054657df8d6a56083f Mon Sep 17 00:00:00 2001 From: Bert Date: Thu, 25 Sep 2025 14:31:48 +0200 Subject: [PATCH 2/3] GH-552: answering my own review --- node/src/blockchain/blockchain_bridge.rs | 188 ++++++------------ .../blockchain_interface_web3/mod.rs | 74 +++---- 2 files changed, 90 insertions(+), 172 deletions(-) diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index a0c665b72..c745919b4 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -1710,47 +1710,34 @@ mod tests { System::new("handle_retrieve_transactions_repeated_calling_does_not_skip_blocks"); let (accountant, _, accountant_recording_arc) = make_recorder(); let earning_wallet = make_wallet("earningwallet"); - let amount1 = 42; - let amount2 = 55; - let amount3 = 75; - let amount4 = 99; + let amount_1 = 42; + let amount_2 = 55; + let amount_3 = 75; + let amount_4 = 99; let initial_start_block: u64 = 0x45f2f3; // 4_584_179 let new_start_block_1 = initial_start_block + 1 + DEFAULT_MAX_BLOCK_COUNT; - let expected_transactions1 = RetrievedBlockchainTransactions { - new_start_block: BlockNumber::Number(new_start_block_1.into()), - transactions: vec![BlockchainTransaction { - block_number: initial_start_block + 0x14382, - from: earning_wallet.clone(), - wei_amount: amount1, - }], - }; let new_start_block_2 = initial_start_block + 2 + (2 * DEFAULT_MAX_BLOCK_COUNT); - let expected_transactions2 = RetrievedBlockchainTransactions { - new_start_block: BlockNumber::Number(new_start_block_2.into()), - transactions: vec![BlockchainTransaction { - block_number: initial_start_block + 0x308b3, - from: earning_wallet.clone(), - wei_amount: amount2, - }], - }; let new_start_block_3 = initial_start_block + 3 + (3 * DEFAULT_MAX_BLOCK_COUNT); - let expected_transactions3 = RetrievedBlockchainTransactions { - new_start_block: BlockNumber::Number(new_start_block_3.into()), - transactions: vec![BlockchainTransaction { - block_number: initial_start_block + 0x34248, - from: earning_wallet.clone(), - wei_amount: amount3, - }], - }; let new_start_block_4 = 0x4be667; - let expected_transactions4 = RetrievedBlockchainTransactions { - new_start_block: BlockNumber::Number(new_start_block_4.into()), - transactions: vec![BlockchainTransaction { - block_number: initial_start_block + 0x49b7e, - from: earning_wallet.clone(), - wei_amount: amount4, - }], - }; + let make_expected_transactions = + |new_start_block: u64, initial_start_block_increment: u64, wei_amount: u128| { + RetrievedBlockchainTransactions { + new_start_block: BlockNumber::Number(new_start_block.into()), + transactions: vec![BlockchainTransaction { + block_number: initial_start_block + initial_start_block_increment, + from: earning_wallet.clone(), + wei_amount, + }], + } + }; + let expected_transactions_1 = + make_expected_transactions(new_start_block_1, 0x14382, amount_1); + let expected_transactions_2 = + make_expected_transactions(new_start_block_2, 0x308b3, amount_2); + let expected_transactions_3 = + make_expected_transactions(new_start_block_3, 0x34248, amount_3); + let expected_transactions_4 = + make_expected_transactions(new_start_block_4, 0x49b7e, amount_4); let lower_interface = LowBlockchainIntMock::default() .get_block_number_result(Ok(0x4be663.into())) .get_block_number_result(Ok(0x4be664.into())) @@ -1759,10 +1746,10 @@ mod tests { let blockchain_interface = BlockchainInterfaceMock::default() .lower_interface_results(Box::new(lower_interface)) .retrieve_transactions_params(&retrieve_transactions_params_arc) - .retrieve_transactions_result(Ok(expected_transactions1.clone())) - .retrieve_transactions_result(Ok(expected_transactions2.clone())) - .retrieve_transactions_result(Ok(expected_transactions3.clone())) - .retrieve_transactions_result(Ok(expected_transactions4.clone())); + .retrieve_transactions_result(Ok(expected_transactions_1.clone())) + .retrieve_transactions_result(Ok(expected_transactions_2.clone())) + .retrieve_transactions_result(Ok(expected_transactions_3.clone())) + .retrieve_transactions_result(Ok(expected_transactions_4.clone())); let persistent_config = PersistentConfigurationMock::new() .max_block_count_result(Ok(Some(DEFAULT_MAX_BLOCK_COUNT))) .start_block_result(Ok(Some(initial_start_block))) @@ -1779,45 +1766,30 @@ mod tests { 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_transactions1 = RetrieveTransactions { - recipient: earning_wallet.clone(), - response_skeleton_opt: Some(ResponseSkeleton { - client_id: 1234, - context_id: 1, - }), - }; - let retrieve_transactions2 = RetrieveTransactions { - recipient: earning_wallet.clone(), - response_skeleton_opt: Some(ResponseSkeleton { - client_id: 1234, - context_id: 2, - }), - }; - let retrieve_transactions3 = RetrieveTransactions { - recipient: earning_wallet.clone(), - response_skeleton_opt: Some(ResponseSkeleton { - client_id: 1234, - context_id: 3, - }), - }; - let retrieve_transactions4 = RetrieveTransactions { + let make_retrieve_transactions_msg = |context_id: u64| RetrieveTransactions { recipient: earning_wallet.clone(), response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, - context_id: 4, + context_id, }), }; + let requests = (1..=4).map(|context_id|{ + RetrieveTransactions { + recipient: earning_wallet.clone(), + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id, + }) + }}).collect_vec(); let before = SystemTime::now(); - let _ = addr.try_send(retrieve_transactions1).unwrap(); - let _ = addr.try_send(retrieve_transactions2).unwrap(); - let _ = addr.try_send(retrieve_transactions3).unwrap(); - let _ = addr.try_send(retrieve_transactions4).unwrap(); + requests.into_iter().for_each(|request|{ + addr.try_send(request).unwrap(); + }); System::current().stop(); system.run(); let after = SystemTime::now(); - let retrieve_transactions_params = retrieve_transactions_params_arc.lock().unwrap(); assert_eq!( *retrieve_transactions_params, @@ -1845,63 +1817,31 @@ mod tests { ] ); let accountant_received_payment = accountant_recording_arc.lock().unwrap(); + vec![ + (expected_transactions_1, new_start_block_1), + (expected_transactions_2, new_start_block_2), + (expected_transactions_3, new_start_block_3), + (expected_transactions_4, new_start_block_4), + ] + .into_iter() + .enumerate() + .for_each(|(idx, (expected_transactions, new_start_block))| { + let received_payment = accountant_received_payment.get_record::(idx); + check_timestamp(before, received_payment.timestamp, after); + assert_eq!( + received_payment, + &ReceivedPayments { + timestamp: received_payment.timestamp, + payments: expected_transactions.transactions, + new_start_block, + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: idx as u64 + 1 + }), + } + ); + }); assert_eq!(accountant_received_payment.len(), 4); - let received_payments1 = accountant_received_payment.get_record::(0); - check_timestamp(before, received_payments1.timestamp, after); - assert_eq!( - received_payments1, - &ReceivedPayments { - timestamp: received_payments1.timestamp, - payments: expected_transactions1.transactions, - new_start_block: new_start_block_1, - response_skeleton_opt: Some(ResponseSkeleton { - client_id: 1234, - context_id: 1 - }), - } - ); - let received_payments2 = accountant_received_payment.get_record::(1); - check_timestamp(before, received_payments2.timestamp, after); - assert_eq!( - received_payments2, - &ReceivedPayments { - timestamp: received_payments2.timestamp, - payments: expected_transactions2.transactions, - new_start_block: new_start_block_2, - response_skeleton_opt: Some(ResponseSkeleton { - client_id: 1234, - context_id: 2 - }), - } - ); - let received_payments3 = accountant_received_payment.get_record::(2); - check_timestamp(before, received_payments3.timestamp, after); - assert_eq!( - received_payments3, - &ReceivedPayments { - timestamp: received_payments3.timestamp, - payments: expected_transactions3.transactions, - new_start_block: new_start_block_3, - response_skeleton_opt: Some(ResponseSkeleton { - client_id: 1234, - context_id: 3 - }), - } - ); - let received_payments4 = accountant_received_payment.get_record::(3); - check_timestamp(before, received_payments4.timestamp, after); - assert_eq!( - received_payments4, - &ReceivedPayments { - timestamp: received_payments4.timestamp, - payments: expected_transactions4.transactions, - new_start_block: new_start_block_4, - response_skeleton_opt: Some(ResponseSkeleton { - client_id: 1234, - context_id: 4 - }), - } - ); } fn success_handler( 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 40603b9d6..c034d6f03 100644 --- a/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs +++ b/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs @@ -2406,65 +2406,43 @@ mod tests { let _end_block_nbr = 0x4be662u64; let wallet = Wallet::new(to); init_test_logging(); - + let test_retrieve_transactions = |last_end_block: u64, newly_computed_end_block: BlockNumber, expected_new_start_block: u64| -> u64{ + let result = subject + .retrieve_transactions(BlockNumber::Number(last_end_block.into()), newly_computed_end_block, &wallet) + .unwrap(); + let new_start_block = if let BlockNumber::Number(value) = result.new_start_block { + value.as_u64() + } else { + panic!("Unexpected block number type") + }; + assert_eq!(new_start_block, expected_new_start_block); + new_start_block + }; // First call to retrieve_transactions let end_block_1 = BlockNumber::Number((1u64 + DEFAULT_MAX_BLOCK_COUNT).into()); - let result1 = subject - .retrieve_transactions(BlockNumber::Number(1u64.into()), end_block_1, &wallet) - .unwrap(); + let last_end_block_1 = 1; + let expected_new_start_block_1 = 1 + 1 + DEFAULT_MAX_BLOCK_COUNT; - // Second call to retrieve_transactions with non-overlapping but continuous block range - let new_start_block_1 = if let BlockNumber::Number(value) = result1.new_start_block { - value.as_u64() - } else { - panic!("Unexpected block number type") - }; + let new_start_block_1 = test_retrieve_transactions(last_end_block_1, end_block_1, expected_new_start_block_1); + // Second call to retrieve_transactions with non-overlapping but continuous block range let end_block_2 = BlockNumber::Number((new_start_block_1 + DEFAULT_MAX_BLOCK_COUNT).into()); - let result2 = subject - .retrieve_transactions(result1.new_start_block, end_block_2, &wallet) - .unwrap(); + let last_end_block_2 = new_start_block_1; + let expected_new_start_block_2 =1 + new_start_block_1 + DEFAULT_MAX_BLOCK_COUNT; + + let new_start_block_2 = test_retrieve_transactions(last_end_block_2, end_block_2, expected_new_start_block_2); - let new_start_block_2 = if let BlockNumber::Number(value) = result2.new_start_block { - value.as_u64() - } else { - panic!("Unexpected block number type") - }; let end_block_3 = BlockNumber::Number((new_start_block_2 + DEFAULT_MAX_BLOCK_COUNT).into()); - let result3 = subject - .retrieve_transactions(result2.new_start_block, end_block_3, &wallet) - .unwrap(); + let last_end_block_3 = new_start_block_2; + let expected_new_start_block_3 = 1 + new_start_block_2 + DEFAULT_MAX_BLOCK_COUNT; - let new_start_block_3 = if let BlockNumber::Number(value) = result3.new_start_block { - value.as_u64() - } else { - panic!("Unexpected block number type") - }; + let new_start_block_3 = test_retrieve_transactions(last_end_block_3, end_block_3, expected_new_start_block_3); let end_block_4 = BlockNumber::Number((new_start_block_3 + DEFAULT_MAX_BLOCK_COUNT).into()); - let result4 = subject - .retrieve_transactions(result3.new_start_block, end_block_4, &wallet) - .unwrap(); - - let new_start_block_4 = if let BlockNumber::Number(value) = result4.new_start_block { - value.as_u64() - } else { - panic!("Unexpected block number type") - }; + let last_end_block_4 = new_start_block_3; + let expected_new_start_block_4 = 1 + new_start_block_3 + DEFAULT_MAX_BLOCK_COUNT; - assert_eq!(new_start_block_1, 2 + DEFAULT_MAX_BLOCK_COUNT); - assert_eq!( - new_start_block_2, - 1 + new_start_block_1 + DEFAULT_MAX_BLOCK_COUNT - ); - assert_eq!( - new_start_block_3, - 1 + new_start_block_2 + DEFAULT_MAX_BLOCK_COUNT - ); - assert_eq!( - new_start_block_4, - 1 + new_start_block_3 + DEFAULT_MAX_BLOCK_COUNT - ); + test_retrieve_transactions(last_end_block_4, end_block_4, expected_new_start_block_4); let test_log_handler = TestLogHandler::new(); test_log_handler.exists_log_containing( From 0658afadd1f6a600910c34877a7a43913978318f Mon Sep 17 00:00:00 2001 From: Bert Date: Thu, 25 Sep 2025 14:34:00 +0200 Subject: [PATCH 3/3] GH-552: formatting --- node/src/blockchain/blockchain_bridge.rs | 11 +++++----- .../blockchain_interface_web3/mod.rs | 22 ++++++++++++++----- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index c745919b4..e7659ca2e 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -1773,17 +1773,18 @@ mod tests { context_id, }), }; - let requests = (1..=4).map(|context_id|{ - RetrieveTransactions { + let requests = (1..=4) + .map(|context_id| RetrieveTransactions { recipient: earning_wallet.clone(), response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id, - }) - }}).collect_vec(); + }), + }) + .collect_vec(); let before = SystemTime::now(); - requests.into_iter().for_each(|request|{ + requests.into_iter().for_each(|request| { addr.try_send(request).unwrap(); }); 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 c034d6f03..212338d72 100644 --- a/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs +++ b/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs @@ -2406,9 +2406,16 @@ mod tests { let _end_block_nbr = 0x4be662u64; let wallet = Wallet::new(to); init_test_logging(); - let test_retrieve_transactions = |last_end_block: u64, newly_computed_end_block: BlockNumber, expected_new_start_block: u64| -> u64{ + let test_retrieve_transactions = |last_end_block: u64, + newly_computed_end_block: BlockNumber, + expected_new_start_block: u64| + -> u64 { let result = subject - .retrieve_transactions(BlockNumber::Number(last_end_block.into()), newly_computed_end_block, &wallet) + .retrieve_transactions( + BlockNumber::Number(last_end_block.into()), + newly_computed_end_block, + &wallet, + ) .unwrap(); let new_start_block = if let BlockNumber::Number(value) = result.new_start_block { value.as_u64() @@ -2423,20 +2430,23 @@ mod tests { let last_end_block_1 = 1; let expected_new_start_block_1 = 1 + 1 + DEFAULT_MAX_BLOCK_COUNT; - let new_start_block_1 = test_retrieve_transactions(last_end_block_1, end_block_1, expected_new_start_block_1); + let new_start_block_1 = + test_retrieve_transactions(last_end_block_1, end_block_1, expected_new_start_block_1); // Second call to retrieve_transactions with non-overlapping but continuous block range let end_block_2 = BlockNumber::Number((new_start_block_1 + DEFAULT_MAX_BLOCK_COUNT).into()); let last_end_block_2 = new_start_block_1; - let expected_new_start_block_2 =1 + new_start_block_1 + DEFAULT_MAX_BLOCK_COUNT; + let expected_new_start_block_2 = 1 + new_start_block_1 + DEFAULT_MAX_BLOCK_COUNT; - let new_start_block_2 = test_retrieve_transactions(last_end_block_2, end_block_2, expected_new_start_block_2); + let new_start_block_2 = + test_retrieve_transactions(last_end_block_2, end_block_2, expected_new_start_block_2); let end_block_3 = BlockNumber::Number((new_start_block_2 + DEFAULT_MAX_BLOCK_COUNT).into()); let last_end_block_3 = new_start_block_2; let expected_new_start_block_3 = 1 + new_start_block_2 + DEFAULT_MAX_BLOCK_COUNT; - let new_start_block_3 = test_retrieve_transactions(last_end_block_3, end_block_3, expected_new_start_block_3); + let new_start_block_3 = + test_retrieve_transactions(last_end_block_3, end_block_3, expected_new_start_block_3); let end_block_4 = BlockNumber::Number((new_start_block_3 + DEFAULT_MAX_BLOCK_COUNT).into()); let last_end_block_4 = new_start_block_3;