From ed7ca6915e4e76365c3357cd86523ebd4400e6f4 Mon Sep 17 00:00:00 2001 From: masqrauder <60554948+masqrauder@users.noreply.github.com> Date: Sun, 14 Jan 2024 23:14:41 -0500 Subject: [PATCH 01/10] GH-606: Initialize start_block to none to use latest block --- masq/src/commands/configuration_command.rs | 13 ++- .../src/commands/set_configuration_command.rs | 15 ++- masq_lib/src/constants.rs | 2 +- masq_lib/src/messages.rs | 2 +- node/src/blockchain/blockchain_bridge.rs | 105 ++++++++++++++---- node/src/database/config_dumper.rs | 25 ++--- node/src/database/db_initializer.rs | 20 +--- .../src/database/db_migrations/db_migrator.rs | 2 + .../migrations/migration_9_to_10.rs | 71 ++++++++++++ .../database/db_migrations/migrations/mod.rs | 1 + node/src/db_config/config_dao.rs | 11 +- .../src/db_config/persistent_configuration.rs | 34 ++---- node/src/node_configurator/configurator.rs | 78 ++++++++++--- .../src/test_utils/database_version_0_sql.txt | 4 +- .../persistent_configuration_mock.rs | 12 +- 15 files changed, 277 insertions(+), 118 deletions(-) create mode 100644 node/src/database/db_migrations/migrations/migration_9_to_10.rs diff --git a/masq/src/commands/configuration_command.rs b/masq/src/commands/configuration_command.rs index f9e72d5d4..62d2b4650 100644 --- a/masq/src/commands/configuration_command.rs +++ b/masq/src/commands/configuration_command.rs @@ -131,7 +131,10 @@ impl ConfigurationCommand { dump_parameter_line( stream, "Start block:", - &configuration.start_block.to_string(), + &configuration + .start_block_opt + .map(|m| m.separate_with_commas()) + .unwrap_or_else(|| "[Latest]".to_string()), ); Self::dump_value_list(stream, "Past neighbors:", &configuration.past_neighbors); let payment_thresholds = Self::preprocess_combined_parameters({ @@ -333,7 +336,7 @@ mod tests { exit_byte_rate: 129000000, exit_service_rate: 160000000, }, - start_block: 3456, + start_block_opt: None, scan_intervals: UiScanIntervals { pending_payable_sec: 150500, payable_sec: 155000, @@ -378,7 +381,7 @@ mod tests { |Max block count: [Unlimited]\n\ |Neighborhood mode: standard\n\ |Port mapping protocol: PCP\n\ -|Start block: 3456\n\ +|Start block: [Latest]\n\ |Past neighbors: neighbor 1\n\ | neighbor 2\n\ |Payment thresholds: \n\ @@ -433,7 +436,7 @@ mod tests { exit_byte_rate: 20, exit_service_rate: 30, }, - start_block: 3456, + start_block_opt: Some(1234567890u64), scan_intervals: UiScanIntervals { pending_payable_sec: 1000, payable_sec: 1000, @@ -476,7 +479,7 @@ mod tests { |Max block count: 100,000\n\ |Neighborhood mode: zero-hop\n\ |Port mapping protocol: PCP\n\ -|Start block: 3456\n\ +|Start block: 1,234,567,890\n\ |Past neighbors: [?]\n\ |Payment thresholds: \n\ | Debt threshold: 2,500 gwei\n\ diff --git a/masq/src/commands/set_configuration_command.rs b/masq/src/commands/set_configuration_command.rs index 99d979f07..e22101fd1 100644 --- a/masq/src/commands/set_configuration_command.rs +++ b/masq/src/commands/set_configuration_command.rs @@ -35,9 +35,13 @@ impl SetConfigurationCommand { } fn validate_start_block(start_block: String) -> Result<(), String> { - match start_block.parse::() { - Ok(_) => Ok(()), - _ => Err(start_block), + if "none".eq_ignore_ascii_case(&start_block) { + Ok(()) + } else { + match start_block.parse::() { + Ok(_) => Ok(()), + _ => Err(start_block), + } } } @@ -59,7 +63,7 @@ impl Command for SetConfigurationCommand { const SET_CONFIGURATION_ABOUT: &str = "Sets Node configuration parameters being enabled for this operation when the Node is running."; const START_BLOCK_HELP: &str = - "Ordinal number of the Ethereum block where scanning for transactions will start."; + "Ordinal number of the Ethereum block where scanning for transactions will start. Use 'none' for Latest block."; pub fn set_configurationify<'a>(shared_schema_arg: Arg<'a, 'a>) -> Arg<'a, 'a> { shared_schema_arg.takes_value(true).min_values(1) @@ -103,7 +107,7 @@ mod tests { ); assert_eq!( START_BLOCK_HELP, - "Ordinal number of the Ethereum block where scanning for transactions will start." + "Ordinal number of the Ethereum block where scanning for transactions will start. Use 'none' for Latest block." ); } @@ -126,6 +130,7 @@ mod tests { fn validate_start_block_works() { assert!(validate_start_block("abc".to_string()).is_err()); assert!(validate_start_block("1566".to_string()).is_ok()); + assert!(validate_start_block("none".to_string()).is_ok()); } #[test] diff --git a/masq_lib/src/constants.rs b/masq_lib/src/constants.rs index 9cfdc90c6..ee93fd31a 100644 --- a/masq_lib/src/constants.rs +++ b/masq_lib/src/constants.rs @@ -5,7 +5,7 @@ use crate::data_version::DataVersion; use const_format::concatcp; pub const DEFAULT_CHAIN: Chain = Chain::PolyMainnet; -pub const CURRENT_SCHEMA_VERSION: usize = 9; +pub const CURRENT_SCHEMA_VERSION: usize = 10; pub const HIGHEST_RANDOM_CLANDESTINE_PORT: u16 = 9999; pub const HTTP_PORT: u16 = 80; diff --git a/masq_lib/src/messages.rs b/masq_lib/src/messages.rs index 59522171e..45842e419 100644 --- a/masq_lib/src/messages.rs +++ b/masq_lib/src/messages.rs @@ -492,7 +492,7 @@ pub struct UiConfigurationResponse { #[serde(rename = "portMappingProtocol")] pub port_mapping_protocol_opt: Option, #[serde(rename = "startBlock")] - pub start_block: u64, + pub start_block_opt: Option, #[serde(rename = "consumingWalletPrivateKeyOpt")] pub consuming_wallet_private_key_opt: Option, // This item is calculated from the private key, not stored in the database, so that diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index 0bb34fbfd..76de3911b 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -281,8 +281,9 @@ impl BlockchainBridge { fn handle_retrieve_transactions(&mut self, msg: RetrieveTransactions) -> Result<(), String> { let start_block_nbr = match self.persistent_config.start_block() { - Ok (sb) => sb, - Err (e) => panic! ("Cannot retrieve start block from database; payments to you may not be processed: {:?}", e) + Ok(Some(sb)) => sb, + Ok(None) => u64::MAX, + Err(e) => panic!("Cannot retrieve start block from database; payments to you may not be processed: {:?}", e) }; let max_block_count = match self.persistent_config.max_block_count() { Ok(Some(mbc)) => mbc, @@ -294,30 +295,46 @@ impl BlockchainBridge { .get_block_number() { Ok(eb) => { - if u64::MAX == max_block_count { + if u64::MAX == max_block_count || u64::MAX == start_block_nbr { BlockNumber::Number(eb) } else { BlockNumber::Number(eb.as_u64().min(start_block_nbr + max_block_count).into()) } } Err(e) => { - info!( - self.logger, - "Using 'latest' block number instead of a literal number. {:?}", e - ); if max_block_count == u64::MAX { + info!( + self.logger, + "Using 'latest' block number instead of a literal number. {:?}", e + ); + BlockNumber::Latest + } else if u64::MAX == start_block_nbr { BlockNumber::Latest } else { BlockNumber::Number((start_block_nbr + max_block_count).into()) } } }; - let start_block = BlockNumber::Number(start_block_nbr.into()); + let start_block = if u64::MAX == start_block_nbr { + end_block + } else { + BlockNumber::Number(start_block_nbr.into()) + }; let retrieved_transactions = self.blockchain_interface .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"); } @@ -1005,7 +1022,7 @@ mod tests { .lower_interface_results(Box::new(lower_interface)); let persistent_config = PersistentConfigurationMock::new() .max_block_count_result(Ok(Some(100_000))) - .start_block_result(Ok(5)); // no set_start_block_result: set_start_block() must not be called + .start_block_result(Ok(Some(5))); // no set_start_block_result: set_start_block() must not be called let mut subject = BlockchainBridge::new( Box::new(blockchain_interface), Box::new(persistent_config), @@ -1287,17 +1304,20 @@ mod tests { }, ], }; - let lower_interface = - LowBlockchainIntMock::default().get_block_number_result(LatestBlockNumber::Err( - BlockchainError::QueryFailed("Failed to read the latest block number".to_string()), - )); + let lower_interface = LowBlockchainIntMock::default().get_block_number_result( + LatestBlockNumber::Err(BlockchainError::QueryFailed( + "\"Failed to read the latest block number\"".to_string(), + )), + ); let blockchain_interface_mock = BlockchainInterfaceMock::default() .retrieve_transactions_params(&retrieve_transactions_params_arc) .retrieve_transactions_result(Ok(expected_transactions.clone())) .lower_interface_results(Box::new(lower_interface)); let persistent_config = PersistentConfigurationMock::new() - .max_block_count_result(Ok(Some(10000u64))) - .start_block_result(Ok(6)); + .max_block_count_result(Ok(None)) + .start_block_result(Ok(Some(6))) + .set_start_block_params(&set_start_block_params_arc) + .set_start_block_result(Ok(())); let subject = BlockchainBridge::new( Box::new(blockchain_interface_mock), Box::new(persistent_config), @@ -1321,12 +1341,14 @@ mod tests { 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(8675309u64)]); let retrieve_transactions_params = retrieve_transactions_params_arc.lock().unwrap(); assert_eq!( *retrieve_transactions_params, vec![( BlockNumber::Number(6u64.into()), - BlockNumber::Number(10006u64.into()), + BlockNumber::Latest, earning_wallet )] ); @@ -1346,7 +1368,9 @@ mod tests { }), } ); - TestLogHandler::new().exists_log_containing("INFO: BlockchainBridge: Using 'latest' block number instead of a literal number. QueryFailed(\"Failed to read the latest block number\")"); + TestLogHandler::new().exists_log_containing( + "INFO: BlockchainBridge: Using 'latest' block number instead of a literal number.", + ); } #[test] @@ -1382,7 +1406,9 @@ mod tests { .lower_interface_results(Box::new(lower_interface)); let persistent_config = PersistentConfigurationMock::new() .max_block_count_result(Ok(Some(10000u64))) - .start_block_result(Ok(6)); + .start_block_result(Ok(Some(6))) + .set_start_block_params(&set_start_block_params_arc) + .set_start_block_result(Ok(())); let subject = BlockchainBridge::new( Box::new(blockchain_interface_mock), Box::new(persistent_config), @@ -1406,6 +1432,8 @@ mod tests { 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(1234u64)]); let retrieve_transactions_params = retrieve_transactions_params_arc.lock().unwrap(); assert_eq!( *retrieve_transactions_params, @@ -1446,7 +1474,9 @@ mod tests { .lower_interface_results(Box::new(lower_interface)); let persistent_config = PersistentConfigurationMock::new() .max_block_count_result(Ok(Some(10000u64))) - .start_block_result(Ok(6)); + .start_block_result(Ok(Some(6))) + .set_start_block_params(&set_start_block_params_arc) + .set_start_block_result(Ok(())); let (accountant, _, accountant_recording_arc) = make_recorder(); let system = System::new( "processing_of_received_payments_continues_even_if_no_payments_are_detected", @@ -1474,6 +1504,8 @@ mod tests { 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(7)]); let accountant_received_payment = accountant_recording_arc.lock().unwrap(); let received_payments = accountant_received_payment.get_record::(0); check_timestamp(before, received_payments.timestamp, after); @@ -1517,6 +1549,41 @@ mod tests { let _ = subject.handle_retrieve_transactions(retrieve_transactions); } + #[test] + #[should_panic( + expected = "Cannot set start block 1234 in database; payments to you may not be processed: TransactionError" + )] + fn handle_retrieve_transactions_panics_if_start_block_cannot_be_written() { + let persistent_config = PersistentConfigurationMock::new() + .start_block_result(Ok(Some(1234))) + .max_block_count_result(Ok(Some(10000u64))) + .set_start_block_result(Err(PersistentConfigError::TransactionError)); + let lower_interface = + LowBlockchainIntMock::default().get_block_number_result(Ok(0u64.into())); + let blockchain_interface = BlockchainInterfaceMock::default() + .retrieve_transactions_result(Ok(RetrievedBlockchainTransactions { + new_start_block: 1234, + transactions: vec![BlockchainTransaction { + block_number: 1000, + from: make_wallet("somewallet"), + wei_amount: 2345, + }], + })) + .lower_interface_results(Box::new(lower_interface)); + let mut subject = BlockchainBridge::new( + Box::new(blockchain_interface), + Box::new(persistent_config), + false, + None, //not needed in this test + ); + let retrieve_transactions = RetrieveTransactions { + recipient: make_wallet("somewallet"), + response_skeleton_opt: None, + }; + + let _ = subject.handle_retrieve_transactions(retrieve_transactions); + } + fn success_handler( _bcb: &mut BlockchainBridge, _msg: RetrieveTransactions, diff --git a/node/src/database/config_dumper.rs b/node/src/database/config_dumper.rs index 78f23ade7..c0f7aa61c 100644 --- a/node/src/database/config_dumper.rs +++ b/node/src/database/config_dumper.rs @@ -353,11 +353,8 @@ mod tests { ); assert_value("neighborhoodMode", "zero-hop", &map); assert_value("schemaVersion", &CURRENT_SCHEMA_VERSION.to_string(), &map); - assert_value( - "startBlock", - &Chain::PolyMainnet.rec().contract_creation_block.to_string(), - &map, - ); + assert!(map.contains_key("startBlock")); + assert_none("startBlock", &map); assert_value( "exampleEncrypted", &dao.get("example_encrypted").unwrap().value_opt.unwrap(), @@ -503,11 +500,8 @@ mod tests { assert_value("pastNeighbors", "masq://polygon-mainnet:QUJDREVGR0hJSktMTU5PUFFSU1RVVldYWVowMTIzNDU@1.2.3.4:1234,masq://polygon-mainnet:QkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWjAxMjM0NTY@2.3.4.5:2345", &map); assert_value("neighborhoodMode", "consume-only", &map); assert_value("schemaVersion", &CURRENT_SCHEMA_VERSION.to_string(), &map); - assert_value( - "startBlock", - &Chain::PolyMainnet.rec().contract_creation_block.to_string(), - &map, - ); + assert!(map.contains_key("startBlock")); + assert_none("startBlock", &map); let expected_ee_entry = dao.get("example_encrypted").unwrap().value_opt.unwrap(); let expected_ee_decrypted = Bip39::decrypt_bytes(&expected_ee_entry, "password").unwrap(); let expected_ee_string = encode_bytes(Some(expected_ee_decrypted)).unwrap().unwrap(); @@ -620,11 +614,8 @@ mod tests { ); assert_value("neighborhoodMode", "standard", &map); assert_value("schemaVersion", &CURRENT_SCHEMA_VERSION.to_string(), &map); - assert_value( - "startBlock", - &Chain::PolyMainnet.rec().contract_creation_block.to_string(), - &map, - ); + assert!(map.contains_key("startBlock")); + assert_none("startBlock", &map); assert_value( "exampleEncrypted", &dao.get("example_encrypted").unwrap().value_opt.unwrap(), @@ -679,6 +670,10 @@ mod tests { assert_eq!(actual_value, expected_value); } + fn assert_none(key: &str, map: &Map) { + assert!(!map.get(key).is_none()); + } + fn assert_encrypted_value( key: &str, expected_value: &str, diff --git a/node/src/database/db_initializer.rs b/node/src/database/db_initializer.rs index 8bfb9c1eb..3619cc1b4 100644 --- a/node/src/database/db_initializer.rs +++ b/node/src/database/db_initializer.rs @@ -205,13 +205,7 @@ impl DbInitializerReal { Self::set_config_value( conn, "start_block", - Some( - &external_params - .chain - .rec() - .contract_creation_block - .to_string(), - ), + None, false, &format!( "{} start block", @@ -658,7 +652,7 @@ mod tests { #[test] fn constants_have_correct_values() { assert_eq!(DATABASE_FILE, "node-data.db"); - assert_eq!(CURRENT_SCHEMA_VERSION, 9); + assert_eq!(CURRENT_SCHEMA_VERSION, 10); } #[test] @@ -967,15 +961,7 @@ mod tests { Some(&CURRENT_SCHEMA_VERSION.to_string()), false, ); - verify( - &mut config_vec, - "start_block", - Some(&format!( - "{}", - &TEST_DEFAULT_CHAIN.rec().contract_creation_block.to_string() - )), - false, - ); + verify(&mut config_vec, "start_block", None, false); assert_eq!(config_vec, vec![]); } diff --git a/node/src/database/db_migrations/db_migrator.rs b/node/src/database/db_migrations/db_migrator.rs index 746af3e26..7d1ec4f8c 100644 --- a/node/src/database/db_migrations/db_migrator.rs +++ b/node/src/database/db_migrations/db_migrator.rs @@ -10,6 +10,7 @@ use crate::database::db_migrations::migrations::migration_5_to_6::Migrate_5_to_6 use crate::database::db_migrations::migrations::migration_6_to_7::Migrate_6_to_7; use crate::database::db_migrations::migrations::migration_7_to_8::Migrate_7_to_8; use crate::database::db_migrations::migrations::migration_8_to_9::Migrate_8_to_9; +use crate::database::db_migrations::migrations::migration_9_to_10::Migrate_9_to_10; use crate::database::db_migrations::migrator_utils::{ DBMigDeclarator, DBMigrationUtilities, DBMigrationUtilitiesReal, DBMigratorInnerConfiguration, }; @@ -78,6 +79,7 @@ impl DbMigratorReal { &Migrate_6_to_7, &Migrate_7_to_8, &Migrate_8_to_9, + &Migrate_9_to_10, ] } diff --git a/node/src/database/db_migrations/migrations/migration_9_to_10.rs b/node/src/database/db_migrations/migrations/migration_9_to_10.rs new file mode 100644 index 000000000..de761a3a5 --- /dev/null +++ b/node/src/database/db_migrations/migrations/migration_9_to_10.rs @@ -0,0 +1,71 @@ +use crate::database::db_migrations::db_migrator::DatabaseMigration; +use crate::database::db_migrations::migrator_utils::DBMigDeclarator; + +#[allow(non_camel_case_types)] +pub struct Migrate_9_to_10; + +impl DatabaseMigration for Migrate_9_to_10 { + fn migrate<'a>( + &self, + declaration_utils: Box, + ) -> rusqlite::Result<()> { + declaration_utils.execute_upon_transaction(&[ + &"INSERT INTO config (name, value, encrypted) VALUES ('start_block', null, 0) ON CONFLICT DO NOTHING", + ]) + } + + fn old_version(&self) -> usize { + 9 + } +} + +#[cfg(test)] +mod tests { + use crate::database::db_initializer::{ + DbInitializationConfig, DbInitializer, DbInitializerReal, DATABASE_FILE, + }; + use crate::test_utils::database_utils::{ + bring_db_0_back_to_life_and_return_connection, make_external_data, retrieve_config_row, + }; + use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler}; + use masq_lib::test_utils::utils::ensure_node_home_directory_exists; + use std::fs::create_dir_all; + + #[test] + fn migration_from_9_to_10_is_properly_set() { + init_test_logging(); + let dir_path = ensure_node_home_directory_exists( + "db_migrations", + "migration_from_9_to_10_is_properly_set", + ); + create_dir_all(&dir_path).unwrap(); + let db_path = dir_path.join(DATABASE_FILE); + let _ = bring_db_0_back_to_life_and_return_connection(&db_path); + let subject = DbInitializerReal::default(); + + let result = subject.initialize_to_version( + &dir_path, + 10, + DbInitializationConfig::create_or_migrate(make_external_data()), + ); + let connection = result.unwrap(); + let (mp_value, mp_encrypted) = retrieve_config_row(connection.as_ref(), "start_block"); + let (cs_value, cs_encrypted) = retrieve_config_row(connection.as_ref(), "schema_version"); + assert_eq!(mp_value, None); + assert_eq!(mp_encrypted, false); + assert_eq!(cs_value, Some("10".to_string())); + assert_eq!(cs_encrypted, false); + TestLogHandler::new().assert_logs_contain_in_order(vec![ + "DbMigrator: Database successfully migrated from version 0 to 1", + "DbMigrator: Database successfully migrated from version 1 to 2", + "DbMigrator: Database successfully migrated from version 2 to 3", + "DbMigrator: Database successfully migrated from version 3 to 4", + "DbMigrator: Database successfully migrated from version 4 to 5", + "DbMigrator: Database successfully migrated from version 5 to 6", + "DbMigrator: Database successfully migrated from version 6 to 7", + "DbMigrator: Database successfully migrated from version 7 to 8", + "DbMigrator: Database successfully migrated from version 8 to 9", + "DbMigrator: Database successfully migrated from version 9 to 10", + ]); + } +} diff --git a/node/src/database/db_migrations/migrations/mod.rs b/node/src/database/db_migrations/migrations/mod.rs index 68b10ca9b..bcdb14176 100644 --- a/node/src/database/db_migrations/migrations/mod.rs +++ b/node/src/database/db_migrations/migrations/mod.rs @@ -9,3 +9,4 @@ pub mod migration_5_to_6; pub mod migration_6_to_7; pub mod migration_7_to_8; pub mod migration_8_to_9; +pub mod migration_9_to_10; diff --git a/node/src/db_config/config_dao.rs b/node/src/db_config/config_dao.rs index 759440c42..36798dd05 100644 --- a/node/src/db_config/config_dao.rs +++ b/node/src/db_config/config_dao.rs @@ -180,7 +180,7 @@ mod tests { use crate::database::db_initializer::{DbInitializer, DbInitializerReal}; use crate::database::test_utils::ConnectionWrapperMock; use crate::test_utils::assert_contains; - use masq_lib::constants::{CURRENT_SCHEMA_VERSION, ETH_ROPSTEN_CONTRACT_CREATION_BLOCK}; + use masq_lib::constants::CURRENT_SCHEMA_VERSION; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; use rusqlite::Connection; use std::path::Path; @@ -201,14 +201,7 @@ mod tests { false, ), ); - assert_contains( - &result, - &ConfigDaoRecord::new( - "start_block", - Some(Ð_ROPSTEN_CONTRACT_CREATION_BLOCK.to_string()), - false, - ), - ); + assert_contains(&result, &ConfigDaoRecord::new("start_block", None, false)); assert_contains( &result, &ConfigDaoRecord::new("consuming_wallet_private_key", None, true), diff --git a/node/src/db_config/persistent_configuration.rs b/node/src/db_config/persistent_configuration.rs index da3fa1583..f710bc3f4 100644 --- a/node/src/db_config/persistent_configuration.rs +++ b/node/src/db_config/persistent_configuration.rs @@ -131,8 +131,8 @@ pub trait PersistentConfiguration { node_descriptors_opt: Option>, db_password: &str, ) -> Result<(), PersistentConfigError>; - fn start_block(&self) -> Result; - fn set_start_block(&mut self, value: u64) -> Result<(), PersistentConfigError>; + fn start_block(&self) -> Result, PersistentConfigError>; + 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( @@ -406,12 +406,12 @@ impl PersistentConfiguration for PersistentConfigurationReal { )?) } - fn start_block(&self) -> Result { - self.simple_get_method(decode_u64, "start_block") + fn start_block(&self) -> Result, PersistentConfigError> { + Ok(decode_u64(self.get("start_block")?)?) } - fn set_start_block(&mut self, value: u64) -> Result<(), PersistentConfigError> { - self.simple_set_method("start_block", value) + fn set_start_block(&mut self, value: Option) -> Result<(), PersistentConfigError> { + Ok(self.dao.set("start_block", encode_u64(value)?)?) } fn max_block_count(&self) -> Result, PersistentConfigError> { @@ -576,17 +576,6 @@ impl PersistentConfigurationReal { .set_by_guest_transaction(txn, parameter_name, Some(value.to_string()))?) } - fn simple_get_method( - &self, - decoder: fn(Option) -> Result, TypedConfigLayerError>, - parameter: &str, - ) -> Result { - match decoder(self.get(parameter)?)? { - None => Self::missing_value_panic(parameter), - Some(value) => Ok(value), - } - } - fn combined_params_get_method<'a, T, C>( &'a self, values_parser: C, @@ -1503,12 +1492,11 @@ mod tests { let start_block = subject.start_block().unwrap(); - assert_eq!(start_block, 6); + assert_eq!(start_block, Some(6)); } #[test] - #[should_panic(expected = "ever-supplied value missing: start_block; database is corrupt!")] - fn start_block_does_not_tolerate_optional_output() { + fn start_block_can_be_none() { let config_dao = Box::new(ConfigDaoMock::new().get_result(Ok(ConfigDaoRecord::new( "start_block", None, @@ -1516,7 +1504,9 @@ mod tests { )))); let subject = PersistentConfigurationReal::new(config_dao); - let _ = subject.start_block(); + let start_block = subject.start_block(); + + assert_eq!(start_block, Ok(None)); } #[test] @@ -1529,7 +1519,7 @@ mod tests { ); let mut subject = PersistentConfigurationReal::new(config_dao); - let result = subject.set_start_block(1234); + let result = subject.set_start_block(Some(1234)); assert_eq!(result, Ok(())); let set_params = set_params_arc.lock().unwrap(); diff --git a/node/src/node_configurator/configurator.rs b/node/src/node_configurator/configurator.rs index 30f0eed57..6f6bef43a 100644 --- a/node/src/node_configurator/configurator.rs +++ b/node/src/node_configurator/configurator.rs @@ -551,7 +551,8 @@ impl Configurator { persistent_config.earning_wallet_address(), "earningWalletAddressOpt", )?; - let start_block = Self::value_required(persistent_config.start_block(), "startBlock")?; + let start_block_opt = + Self::value_not_required(persistent_config.start_block(), "startBlock")?; let max_block_count_opt = match persistent_config.max_block_count() { Ok(value) => value, Err(e) => panic!( @@ -649,7 +650,7 @@ impl Configurator { exit_byte_rate, exit_service_rate, }, - start_block, + start_block_opt, scan_intervals: UiScanIntervals { pending_payable_sec, payable_sec, @@ -786,11 +787,15 @@ impl Configurator { } fn set_start_block(&mut self, string_number: String) -> Result<(), (u64, String)> { - let block_number = match string_number.parse::() { - Ok(num) => num, - Err(e) => return Err((NON_PARSABLE_VALUE, format!("start block: {:?}", e))), + let block_number_opt = if "none".eq_ignore_ascii_case(&string_number) { + None + } else { + match string_number.parse::() { + Ok(num) => Some(num), + Err(e) => return Err((NON_PARSABLE_VALUE, format!("start block: {:?}", e))), + } }; - match self.persistent_config.set_start_block(block_number) { + match self.persistent_config.set_start_block(block_number_opt) { Ok(_) => Ok(()), Err(e) => Err((CONFIGURATOR_WRITE_ERROR, format!("start block: {:?}", e))), } @@ -2118,7 +2123,49 @@ mod tests { let (_, context_id) = UiSetConfigurationResponse::fmb(response.body.clone()).unwrap(); assert_eq!(context_id, 4444); let check_start_block_params = set_start_block_params_arc.lock().unwrap(); - assert_eq!(*check_start_block_params, vec![166666]); + assert_eq!(*check_start_block_params, vec![Some(166666)]); + TestLogHandler::new().exists_log_containing(&format!( + "DEBUG: {}: A request from UI received: {:?} from context id: {}", + test_name, msg, context_id + )); + } + + #[test] + fn handle_set_configuration_accepts_none_to_unset_start_block() { + init_test_logging(); + let test_name = "handle_set_configuration_accepts_none_to_unset_start_block"; + let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); + let (ui_gateway, _, ui_gateway_recording_arc) = make_recorder(); + let persistent_config = PersistentConfigurationMock::new() + .set_start_block_params(&set_start_block_params_arc) + .set_start_block_result(Ok(())); + let mut subject = make_subject(Some(persistent_config)); + subject.logger = Logger::new(test_name); + let subject_addr = subject.start(); + let peer_actors = peer_actors_builder().ui_gateway(ui_gateway).build(); + subject_addr.try_send(BindMessage { peer_actors }).unwrap(); + let msg = UiSetConfigurationRequest { + name: "start-block".to_string(), + value: "none".to_string(), + }; + let context_id = 4444; + + subject_addr + .try_send(NodeFromUiMessage { + client_id: 1234, + body: msg.clone().tmb(context_id), + }) + .unwrap(); + + let system = System::new("test"); + System::current().stop(); + system.run(); + let ui_gateway_recording = ui_gateway_recording_arc.lock().unwrap(); + let response = ui_gateway_recording.get_record::(0); + let (_, context_id) = UiSetConfigurationResponse::fmb(response.body.clone()).unwrap(); + assert_eq!(context_id, 4444); + let check_start_block_params = set_start_block_params_arc.lock().unwrap(); + assert_eq!(*check_start_block_params, vec![None]); TestLogHandler::new().exists_log_containing(&format!( "DEBUG: {}: A request from UI received: {:?} from context id: {}", test_name, msg, context_id @@ -2498,7 +2545,7 @@ mod tests { .neighborhood_mode_result(Ok(NeighborhoodModeLight::Standard)) .past_neighbors_result(Ok(Some(vec![node_descriptor.clone()]))) .earning_wallet_address_result(Ok(Some(earning_wallet_address.clone()))) - .start_block_result(Ok(3456)); + .start_block_result(Ok(Some(3456))); let persistent_config = payment_thresholds_scan_intervals_rate_pack(persistent_config); let mut subject = make_subject(Some(persistent_config)); @@ -2541,7 +2588,7 @@ mod tests { exit_byte_rate: 10, exit_service_rate: 13 }, - start_block: 3456, + start_block_opt: Some(3456), scan_intervals: UiScanIntervals { pending_payable_sec: 122, payable_sec: 125, @@ -2629,8 +2676,7 @@ mod tests { .past_neighbors_params(&past_neighbors_params_arc) .past_neighbors_result(Ok(Some(vec![node_descriptor.clone()]))) .earning_wallet_address_result(Ok(Some(earning_wallet_address.clone()))) - .start_block_result(Ok(3456)) - .start_block_result(Ok(3456)); + .start_block_result(Ok(Some(3456))); let persistent_config = payment_thresholds_scan_intervals_rate_pack(persistent_config); let mut subject = make_subject(Some(persistent_config)); @@ -2673,7 +2719,7 @@ mod tests { exit_byte_rate: 10, exit_service_rate: 13 }, - start_block: 3456, + start_block_opt: Some(3456), scan_intervals: UiScanIntervals { pending_payable_sec: 122, payable_sec: 125, @@ -2700,7 +2746,7 @@ mod tests { .chain_name_result("ropsten".to_string()) .gas_price_result(Ok(2345)) .earning_wallet_address_result(Ok(None)) - .start_block_result(Ok(3456)) + .start_block_result(Ok(Some(3456))) .max_block_count_result(Ok(None)) .neighborhood_mode_result(Ok(NeighborhoodModeLight::ZeroHop)) .mapping_protocol_result(Ok(None)) @@ -2766,7 +2812,7 @@ mod tests { exit_byte_rate: 0, exit_service_rate: 0 }, - start_block: 3456, + start_block_opt: Some(3456), scan_intervals: UiScanIntervals { pending_payable_sec: 0, payable_sec: 0, @@ -2789,7 +2835,7 @@ mod tests { .chain_name_result("ropsten".to_string()) .gas_price_result(Ok(2345)) .earning_wallet_address_result(Ok(Some("4a5e43b54c6C56Ebf7".to_string()))) - .start_block_result(Ok(3456)) + .start_block_result(Ok(Some(3456))) .max_block_count_result(Err(PersistentConfigError::DatabaseError( "Corruption".to_string(), ))); @@ -2839,7 +2885,7 @@ mod tests { .earning_wallet_address_result(Ok(Some( "0x0123456789012345678901234567890123456789".to_string(), ))) - .start_block_result(Ok(3456)) + .start_block_result(Ok(Some(3456))) .max_block_count_result(Ok(Some(100000))) .neighborhood_mode_result(Ok(NeighborhoodModeLight::ConsumeOnly)) .mapping_protocol_result(Ok(Some(AutomapProtocol::Igdp))) diff --git a/node/src/test_utils/database_version_0_sql.txt b/node/src/test_utils/database_version_0_sql.txt index cacdbb40d..c2763125a 100644 --- a/node/src/test_utils/database_version_0_sql.txt +++ b/node/src/test_utils/database_version_0_sql.txt @@ -7,7 +7,7 @@ insert into config (name, value, encrypted) values ('consuming_wallet_public_key insert into config (name, value, encrypted) values ('earning_wallet_address', null, 0) insert into config (name, value, encrypted) values ('schema_version', '0', 0) insert into config (name, value, encrypted) values ('seed', null, 0) -insert into config (name, value, encrypted) values ('start_block', 8688171, 0) +insert into config (name, value, encrypted) values ('start_block', null, 0) insert into config (name, value, encrypted) values ('gas_price', '1', 0) insert into config (name, value, encrypted) values ('past_neighbors', null, 1) create table payable (wallet_address text primary key, balance integer not null, last_paid_timestamp integer not null, pending_payment_transaction text null) @@ -15,4 +15,4 @@ create unique index idx_payable_wallet_address on payable (wallet_address) create table receivable (wallet_address text primary key, balance integer not null, last_received_timestamp integer not null) create unique index idx_receivable_wallet_address on receivable (wallet_address) create table banned ( wallet_address text primary key ) -create unique index idx_banned_wallet_address on banned (wallet_address) \ No newline at end of file +create unique index idx_banned_wallet_address on banned (wallet_address) diff --git a/node/src/test_utils/persistent_configuration_mock.rs b/node/src/test_utils/persistent_configuration_mock.rs index 7b7ace61d..ef1b85163 100644 --- a/node/src/test_utils/persistent_configuration_mock.rs +++ b/node/src/test_utils/persistent_configuration_mock.rs @@ -58,8 +58,8 @@ pub struct PersistentConfigurationMock { set_past_neighbors_params: Arc>, String)>>>, set_past_neighbors_results: RefCell>>, start_block_params: Arc>>, - start_block_results: RefCell>>, - set_start_block_params: Arc>>, + start_block_results: RefCell, PersistentConfigError>>>, + set_start_block_params: Arc>>>, set_start_block_results: RefCell>>, max_block_count_params: Arc>>, max_block_count_results: RefCell, PersistentConfigError>>>, @@ -230,12 +230,12 @@ impl PersistentConfiguration for PersistentConfigurationMock { self.set_past_neighbors_results.borrow_mut().remove(0) } - fn start_block(&self) -> Result { + fn start_block(&self) -> Result, PersistentConfigError> { self.start_block_params.lock().unwrap().push(()); Self::result_from(&self.start_block_results) } - fn set_start_block(&mut self, value: u64) -> Result<(), PersistentConfigError> { + fn set_start_block(&mut self, value: Option) -> Result<(), PersistentConfigError> { self.set_start_block_params.lock().unwrap().push(value); Self::result_from(&self.set_start_block_results) } @@ -546,12 +546,12 @@ impl PersistentConfigurationMock { self } - pub fn start_block_result(self, result: Result) -> Self { + pub fn start_block_result(self, result: Result, PersistentConfigError>) -> Self { self.start_block_results.borrow_mut().push(result); self } - pub fn set_start_block_params(mut self, params: &Arc>>) -> Self { + pub fn set_start_block_params(mut self, params: &Arc>>>) -> Self { self.set_start_block_params = params.clone(); self } From c61be525ea09d256361d6e554ff11eb8a2dde6df Mon Sep 17 00:00:00 2001 From: masqrauder <60554948+masqrauder@users.noreply.github.com> Date: Fri, 15 Mar 2024 22:18:20 -0400 Subject: [PATCH 02/10] GH-606: Apply PR feedback changes --- .../src/commands/set_configuration_command.rs | 7 +- masq_lib/src/constants.rs | 2 +- node/src/accountant/scanners/mod.rs | 6 +- node/src/blockchain/blockchain_bridge.rs | 5 +- node/src/database/db_initializer.rs | 2 +- .../src/database/db_migrations/db_migrator.rs | 2 - .../migrations/migration_9_to_10.rs | 71 ------------------- .../database/db_migrations/migrations/mod.rs | 1 - node/src/test_utils/database_utils.rs | 23 +++++- .../src/test_utils/database_version_0_sql.txt | 4 +- .../src/test_utils/database_version_9_sql.txt | 18 +++++ 11 files changed, 55 insertions(+), 86 deletions(-) delete mode 100644 node/src/database/db_migrations/migrations/migration_9_to_10.rs create mode 100644 node/src/test_utils/database_version_9_sql.txt diff --git a/masq/src/commands/set_configuration_command.rs b/masq/src/commands/set_configuration_command.rs index e22101fd1..8cf0eebc7 100644 --- a/masq/src/commands/set_configuration_command.rs +++ b/masq/src/commands/set_configuration_command.rs @@ -35,7 +35,7 @@ impl SetConfigurationCommand { } fn validate_start_block(start_block: String) -> Result<(), String> { - if "none".eq_ignore_ascii_case(&start_block) { + if "latest".eq_ignore_ascii_case(&start_block) || "none".eq_ignore_ascii_case(&start_block) { Ok(()) } else { match start_block.parse::() { @@ -63,7 +63,7 @@ impl Command for SetConfigurationCommand { const SET_CONFIGURATION_ABOUT: &str = "Sets Node configuration parameters being enabled for this operation when the Node is running."; const START_BLOCK_HELP: &str = - "Ordinal number of the Ethereum block where scanning for transactions will start. Use 'none' for Latest block."; + "Ordinal number of the Ethereum block where scanning for transactions will start. Use 'latest' or 'none' for Latest block."; pub fn set_configurationify<'a>(shared_schema_arg: Arg<'a, 'a>) -> Arg<'a, 'a> { shared_schema_arg.takes_value(true).min_values(1) @@ -107,7 +107,7 @@ mod tests { ); assert_eq!( START_BLOCK_HELP, - "Ordinal number of the Ethereum block where scanning for transactions will start. Use 'none' for Latest block." + "Ordinal number of the Ethereum block where scanning for transactions will start. Use 'latest' or 'none' for Latest block." ); } @@ -130,6 +130,7 @@ mod tests { fn validate_start_block_works() { assert!(validate_start_block("abc".to_string()).is_err()); assert!(validate_start_block("1566".to_string()).is_ok()); + assert!(validate_start_block("latest".to_string()).is_ok()); assert!(validate_start_block("none".to_string()).is_ok()); } diff --git a/masq_lib/src/constants.rs b/masq_lib/src/constants.rs index ee93fd31a..9cfdc90c6 100644 --- a/masq_lib/src/constants.rs +++ b/masq_lib/src/constants.rs @@ -5,7 +5,7 @@ use crate::data_version::DataVersion; use const_format::concatcp; pub const DEFAULT_CHAIN: Chain = Chain::PolyMainnet; -pub const CURRENT_SCHEMA_VERSION: usize = 10; +pub const CURRENT_SCHEMA_VERSION: usize = 9; pub const HIGHEST_RANDOM_CLANDESTINE_PORT: u16 = 9999; pub const HTTP_PORT: u16 = 80; diff --git a/node/src/accountant/scanners/mod.rs b/node/src/accountant/scanners/mod.rs index f8bc4b163..07695164e 100644 --- a/node/src/accountant/scanners/mod.rs +++ b/node/src/accountant/scanners/mod.rs @@ -862,7 +862,7 @@ impl Scanner for ReceivableScanner { match self .persistent_configuration - .set_start_block(msg.new_start_block) + .set_start_block(Some(msg.new_start_block)) { Ok(()) => debug!(logger, "Start block updated to {}", msg.new_start_block), Err(e) => panic!( @@ -1243,7 +1243,7 @@ mod tests { assert_eq!(receivable_scanner.common.initiated_at_opt.is_some(), false); receivable_scanner .persistent_configuration - .set_start_block(136890) + .set_start_block(Some(136890)) .unwrap(); let set_params = set_params_arc.lock().unwrap(); assert_eq!( @@ -3086,7 +3086,7 @@ mod tests { assert_eq!(message_opt, None); let set_start_block_params = set_start_block_params_arc.lock().unwrap(); - assert_eq!(*set_start_block_params, vec![4321]); + assert_eq!(*set_start_block_params, vec![Some(4321)]); TestLogHandler::new().exists_log_containing(&format!( "INFO: {test_name}: No newly received payments were detected during the scanning process." )); diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index 76de3911b..8935322b3 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -1313,6 +1313,7 @@ mod tests { .retrieve_transactions_params(&retrieve_transactions_params_arc) .retrieve_transactions_result(Ok(expected_transactions.clone())) .lower_interface_results(Box::new(lower_interface)); + let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); let persistent_config = PersistentConfigurationMock::new() .max_block_count_result(Ok(None)) .start_block_result(Ok(Some(6))) @@ -1376,6 +1377,7 @@ mod tests { #[test] fn handle_retrieve_transactions_sends_received_payments_back_to_accountant() { let retrieve_transactions_params_arc = Arc::new(Mutex::new(vec![])); + let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); let system = System::new("handle_retrieve_transactions_sends_received_payments_back_to_accountant"); let (accountant, _, accountant_recording_arc) = make_recorder(); @@ -1433,7 +1435,7 @@ mod tests { 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(1234u64)]); + assert_eq!(*set_start_block_params, vec![Some(9876u64)]); let retrieve_transactions_params = retrieve_transactions_params_arc.lock().unwrap(); assert_eq!( *retrieve_transactions_params, @@ -1472,6 +1474,7 @@ mod tests { transactions: vec![], })) .lower_interface_results(Box::new(lower_interface)); + let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); let persistent_config = PersistentConfigurationMock::new() .max_block_count_result(Ok(Some(10000u64))) .start_block_result(Ok(Some(6))) diff --git a/node/src/database/db_initializer.rs b/node/src/database/db_initializer.rs index 3619cc1b4..bcb9a3a0a 100644 --- a/node/src/database/db_initializer.rs +++ b/node/src/database/db_initializer.rs @@ -652,7 +652,7 @@ mod tests { #[test] fn constants_have_correct_values() { assert_eq!(DATABASE_FILE, "node-data.db"); - assert_eq!(CURRENT_SCHEMA_VERSION, 10); + assert_eq!(CURRENT_SCHEMA_VERSION, 9); } #[test] diff --git a/node/src/database/db_migrations/db_migrator.rs b/node/src/database/db_migrations/db_migrator.rs index 7d1ec4f8c..746af3e26 100644 --- a/node/src/database/db_migrations/db_migrator.rs +++ b/node/src/database/db_migrations/db_migrator.rs @@ -10,7 +10,6 @@ use crate::database::db_migrations::migrations::migration_5_to_6::Migrate_5_to_6 use crate::database::db_migrations::migrations::migration_6_to_7::Migrate_6_to_7; use crate::database::db_migrations::migrations::migration_7_to_8::Migrate_7_to_8; use crate::database::db_migrations::migrations::migration_8_to_9::Migrate_8_to_9; -use crate::database::db_migrations::migrations::migration_9_to_10::Migrate_9_to_10; use crate::database::db_migrations::migrator_utils::{ DBMigDeclarator, DBMigrationUtilities, DBMigrationUtilitiesReal, DBMigratorInnerConfiguration, }; @@ -79,7 +78,6 @@ impl DbMigratorReal { &Migrate_6_to_7, &Migrate_7_to_8, &Migrate_8_to_9, - &Migrate_9_to_10, ] } diff --git a/node/src/database/db_migrations/migrations/migration_9_to_10.rs b/node/src/database/db_migrations/migrations/migration_9_to_10.rs deleted file mode 100644 index de761a3a5..000000000 --- a/node/src/database/db_migrations/migrations/migration_9_to_10.rs +++ /dev/null @@ -1,71 +0,0 @@ -use crate::database::db_migrations::db_migrator::DatabaseMigration; -use crate::database::db_migrations::migrator_utils::DBMigDeclarator; - -#[allow(non_camel_case_types)] -pub struct Migrate_9_to_10; - -impl DatabaseMigration for Migrate_9_to_10 { - fn migrate<'a>( - &self, - declaration_utils: Box, - ) -> rusqlite::Result<()> { - declaration_utils.execute_upon_transaction(&[ - &"INSERT INTO config (name, value, encrypted) VALUES ('start_block', null, 0) ON CONFLICT DO NOTHING", - ]) - } - - fn old_version(&self) -> usize { - 9 - } -} - -#[cfg(test)] -mod tests { - use crate::database::db_initializer::{ - DbInitializationConfig, DbInitializer, DbInitializerReal, DATABASE_FILE, - }; - use crate::test_utils::database_utils::{ - bring_db_0_back_to_life_and_return_connection, make_external_data, retrieve_config_row, - }; - use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler}; - use masq_lib::test_utils::utils::ensure_node_home_directory_exists; - use std::fs::create_dir_all; - - #[test] - fn migration_from_9_to_10_is_properly_set() { - init_test_logging(); - let dir_path = ensure_node_home_directory_exists( - "db_migrations", - "migration_from_9_to_10_is_properly_set", - ); - create_dir_all(&dir_path).unwrap(); - let db_path = dir_path.join(DATABASE_FILE); - let _ = bring_db_0_back_to_life_and_return_connection(&db_path); - let subject = DbInitializerReal::default(); - - let result = subject.initialize_to_version( - &dir_path, - 10, - DbInitializationConfig::create_or_migrate(make_external_data()), - ); - let connection = result.unwrap(); - let (mp_value, mp_encrypted) = retrieve_config_row(connection.as_ref(), "start_block"); - let (cs_value, cs_encrypted) = retrieve_config_row(connection.as_ref(), "schema_version"); - assert_eq!(mp_value, None); - assert_eq!(mp_encrypted, false); - assert_eq!(cs_value, Some("10".to_string())); - assert_eq!(cs_encrypted, false); - TestLogHandler::new().assert_logs_contain_in_order(vec![ - "DbMigrator: Database successfully migrated from version 0 to 1", - "DbMigrator: Database successfully migrated from version 1 to 2", - "DbMigrator: Database successfully migrated from version 2 to 3", - "DbMigrator: Database successfully migrated from version 3 to 4", - "DbMigrator: Database successfully migrated from version 4 to 5", - "DbMigrator: Database successfully migrated from version 5 to 6", - "DbMigrator: Database successfully migrated from version 6 to 7", - "DbMigrator: Database successfully migrated from version 7 to 8", - "DbMigrator: Database successfully migrated from version 8 to 9", - "DbMigrator: Database successfully migrated from version 9 to 10", - ]); - } -} diff --git a/node/src/database/db_migrations/migrations/mod.rs b/node/src/database/db_migrations/migrations/mod.rs index bcdb14176..68b10ca9b 100644 --- a/node/src/database/db_migrations/migrations/mod.rs +++ b/node/src/database/db_migrations/migrations/mod.rs @@ -9,4 +9,3 @@ pub mod migration_5_to_6; pub mod migration_6_to_7; pub mod migration_7_to_8; pub mod migration_8_to_9; -pub mod migration_9_to_10; diff --git a/node/src/test_utils/database_utils.rs b/node/src/test_utils/database_utils.rs index 2005166c0..9c483ee51 100644 --- a/node/src/test_utils/database_utils.rs +++ b/node/src/test_utils/database_utils.rs @@ -40,6 +40,27 @@ pub fn bring_db_0_back_to_life_and_return_connection(db_path: &Path) -> Connecti conn } +pub fn bring_db_9_back_to_life_and_return_connection(db_path: &Path) -> Connection { + match remove_file(db_path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => (), + Err(e) => panic!("Unexpected but serious error: {}", e), + _ => (), + } + let file_path = current_dir() + .unwrap() + .join("src") + .join("test_utils") + .join("database_version_9_sql.txt"); + let mut file = File::open(file_path).unwrap(); + let mut buffer = String::new(); + file.read_to_string(&mut buffer).unwrap(); + let conn = Connection::open(&db_path).unwrap(); + buffer.lines().for_each(|stm| { + conn.execute(stm, []).unwrap(); + }); + conn +} + #[derive(Default)] pub struct DbMigratorMock { logger: Option, @@ -196,7 +217,7 @@ fn contains_particular_list_of_key_words( found += 1 } }); - assert_eq!(found,1, "We found {} occurrences of the searched line in the tested sql although only a one is considered correct", found) + assert_eq!(found, 1, "We found {} occurrences of the searched line in the tested sql although only a one is considered correct", found) } fn prepare_expected_vectors_of_words_including_sorting( diff --git a/node/src/test_utils/database_version_0_sql.txt b/node/src/test_utils/database_version_0_sql.txt index c2763125a..cacdbb40d 100644 --- a/node/src/test_utils/database_version_0_sql.txt +++ b/node/src/test_utils/database_version_0_sql.txt @@ -7,7 +7,7 @@ insert into config (name, value, encrypted) values ('consuming_wallet_public_key insert into config (name, value, encrypted) values ('earning_wallet_address', null, 0) insert into config (name, value, encrypted) values ('schema_version', '0', 0) insert into config (name, value, encrypted) values ('seed', null, 0) -insert into config (name, value, encrypted) values ('start_block', null, 0) +insert into config (name, value, encrypted) values ('start_block', 8688171, 0) insert into config (name, value, encrypted) values ('gas_price', '1', 0) insert into config (name, value, encrypted) values ('past_neighbors', null, 1) create table payable (wallet_address text primary key, balance integer not null, last_paid_timestamp integer not null, pending_payment_transaction text null) @@ -15,4 +15,4 @@ create unique index idx_payable_wallet_address on payable (wallet_address) create table receivable (wallet_address text primary key, balance integer not null, last_received_timestamp integer not null) create unique index idx_receivable_wallet_address on receivable (wallet_address) create table banned ( wallet_address text primary key ) -create unique index idx_banned_wallet_address on banned (wallet_address) +create unique index idx_banned_wallet_address on banned (wallet_address) \ No newline at end of file diff --git a/node/src/test_utils/database_version_9_sql.txt b/node/src/test_utils/database_version_9_sql.txt new file mode 100644 index 000000000..7bb54be14 --- /dev/null +++ b/node/src/test_utils/database_version_9_sql.txt @@ -0,0 +1,18 @@ +create table config (name text not null, value text, encrypted integer not null) +create unique index idx_config_name on config (name) +insert into config (name, value, encrypted) values ('example_encrypted', null, 1) +insert into config (name, value, encrypted) values ('clandestine_port', '2897', 0) +insert into config (name, value, encrypted) values ('consuming_wallet_derivation_path', null, 0) +insert into config (name, value, encrypted) values ('consuming_wallet_public_key', null, 0) +insert into config (name, value, encrypted) values ('earning_wallet_address', null, 0) +insert into config (name, value, encrypted) values ('schema_version', '10', 0) +insert into config (name, value, encrypted) values ('seed', null, 0) +insert into config (name, value, encrypted) values ('start_block', null, 0) +insert into config (name, value, encrypted) values ('gas_price', '1', 0) +insert into config (name, value, encrypted) values ('past_neighbors', null, 1) +create table payable (wallet_address text primary key, balance integer not null, last_paid_timestamp integer not null, pending_payment_transaction text null) +create unique index idx_payable_wallet_address on payable (wallet_address) +create table receivable (wallet_address text primary key, balance integer not null, last_received_timestamp integer not null) +create unique index idx_receivable_wallet_address on receivable (wallet_address) +create table banned ( wallet_address text primary key ) +create unique index idx_banned_wallet_address on banned (wallet_address) From aa54ff09586fbb3d6e6d3467f348a970e8ec1868 Mon Sep 17 00:00:00 2001 From: masqrauder <60554948+masqrauder@users.noreply.github.com> Date: Sun, 21 Apr 2024 15:00:03 -0400 Subject: [PATCH 03/10] GH-606: Apply PR feedback changes --- .../src/commands/set_configuration_command.rs | 31 ++++- node/src/blockchain/blockchain_bridge.rs | 119 ++++++++++++++++-- node/src/database/config_dumper.rs | 19 +-- node/src/node_configurator/configurator.rs | 8 -- node/src/test_utils/database_utils.rs | 21 ---- .../src/test_utils/database_version_9_sql.txt | 18 --- 6 files changed, 143 insertions(+), 73 deletions(-) delete mode 100644 node/src/test_utils/database_version_9_sql.txt diff --git a/masq/src/commands/set_configuration_command.rs b/masq/src/commands/set_configuration_command.rs index 8cf0eebc7..6f822f1f3 100644 --- a/masq/src/commands/set_configuration_command.rs +++ b/masq/src/commands/set_configuration_command.rs @@ -7,6 +7,7 @@ use masq_lib::shared_schema::gas_price_arg; use masq_lib::shared_schema::min_hops_arg; use masq_lib::short_writeln; use masq_lib::utils::ExpectValue; +use std::num::IntErrorKind; #[derive(Debug, PartialEq, Eq)] pub struct SetConfigurationCommand { @@ -40,7 +41,11 @@ fn validate_start_block(start_block: String) -> Result<(), String> { } else { match start_block.parse::() { Ok(_) => Ok(()), - _ => Err(start_block), + Err(e) if e.kind() == &IntErrorKind::PosOverflow => Err( + format!("Unable to parse '{}' into a starting block number or provide 'none' or 'latest' for the latest block number: digits exceed {}.", + start_block, u64::MAX), + ), + Err(e) => Err(format!("Unable to parse '{}' into a starting block number or provide 'none' or 'latest' for the latest block number: {}.", start_block, e)) } } } @@ -126,12 +131,28 @@ mod tests { assert!(result.contains("cannot be used with one or more of the other specified arguments")); } + #[test] + fn validate_start_block_catches_invalid_values() { + assert_eq!(validate_start_block("abc".to_string()), Err("Unable to parse 'abc' into a starting block number or provide 'none' or 'latest' for the latest block number: invalid digit found in string.".to_string())); + assert_eq!(validate_start_block("918446744073709551615".to_string()), Err("Unable to parse '918446744073709551615' into a starting block number or provide 'none' or 'latest' for the latest block number: digits exceed 18446744073709551615.".to_string())); + assert_eq!(validate_start_block("123,456,789".to_string()), Err("Unable to parse '123,456,789' into a starting block number or provide 'none' or 'latest' for the latest block number: invalid digit found in string.".to_string())); + assert_eq!(validate_start_block("123'456'789".to_string()), Err("Unable to parse '123'456'789' into a starting block number or provide 'none' or 'latest' for the latest block number: invalid digit found in string.".to_string())); + } #[test] fn validate_start_block_works() { - assert!(validate_start_block("abc".to_string()).is_err()); - assert!(validate_start_block("1566".to_string()).is_ok()); - assert!(validate_start_block("latest".to_string()).is_ok()); - assert!(validate_start_block("none".to_string()).is_ok()); + assert_eq!( + validate_start_block("18446744073709551615".to_string()), + Ok(()) + ); + assert_eq!(validate_start_block("1566".to_string()), Ok(())); + assert_eq!(validate_start_block("none".to_string()), Ok(())); + assert_eq!(validate_start_block("None".to_string()), Ok(())); + assert_eq!(validate_start_block("NONE".to_string()), Ok(())); + assert_eq!(validate_start_block("nOnE".to_string()), Ok(())); + assert_eq!(validate_start_block("latest".to_string()), Ok(())); + assert_eq!(validate_start_block("LATEST".to_string()), Ok(())); + assert_eq!(validate_start_block("LaTeST".to_string()), Ok(())); + assert_eq!(validate_start_block("lATEst".to_string()), Ok(())); } #[test] diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index 8935322b3..47ef63c02 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -289,33 +289,39 @@ impl BlockchainBridge { Ok(Some(mbc)) => mbc, _ => u64::MAX, }; + let use_unlimited_block_count_range = u64::MAX == max_block_count; + let use_latest_block = u64::MAX == start_block_nbr; let end_block = match self .blockchain_interface .lower_interface() .get_block_number() { Ok(eb) => { - if u64::MAX == max_block_count || u64::MAX == start_block_nbr { + if use_unlimited_block_count_range || use_latest_block { BlockNumber::Number(eb) } else { BlockNumber::Number(eb.as_u64().min(start_block_nbr + max_block_count).into()) } } Err(e) => { - if max_block_count == u64::MAX { - info!( + if use_unlimited_block_count_range || use_latest_block { + debug!( self.logger, "Using 'latest' block number instead of a literal number. {:?}", e ); BlockNumber::Latest - } else if u64::MAX == start_block_nbr { - BlockNumber::Latest } else { + debug!( + self.logger, + "Using '{}' ending block number. {:?}", + start_block_nbr + max_block_count, + e + ); BlockNumber::Number((start_block_nbr + max_block_count).into()) } } }; - let start_block = if u64::MAX == start_block_nbr { + let start_block = if use_latest_block { end_block } else { BlockNumber::Number(start_block_nbr.into()) @@ -1304,11 +1310,10 @@ mod tests { }, ], }; - let lower_interface = LowBlockchainIntMock::default().get_block_number_result( - LatestBlockNumber::Err(BlockchainError::QueryFailed( - "\"Failed to read the latest block number\"".to_string(), - )), - ); + let lower_interface = + LowBlockchainIntMock::default().get_block_number_result(LatestBlockNumber::Err( + BlockchainError::QueryFailed("Failed to read the latest block number".to_string()), + )); let blockchain_interface_mock = BlockchainInterfaceMock::default() .retrieve_transactions_params(&retrieve_transactions_params_arc) .retrieve_transactions_result(Ok(expected_transactions.clone())) @@ -1369,8 +1374,96 @@ mod tests { }), } ); - TestLogHandler::new().exists_log_containing( - "INFO: BlockchainBridge: Using 'latest' block number instead of a literal number.", + TestLogHandler::new().exists_log_containing("DEBUG: BlockchainBridge: Using 'latest' block number instead of a literal number. QueryFailed(\"Failed to read the latest block number\")"); + } + + #[test] + fn handle_retrieve_transactions_when_start_block_number_starts_undefined_in_a_brand_new_database( + ) { + let retrieve_transactions_params_arc = Arc::new(Mutex::new(vec![])); + let system = System::new( + "handle_retrieve_transactions_when_start_block_number_starts_undefined_in_a_brand_new_database", + ); + let (accountant, _, accountant_recording_arc) = make_recorder(); + let earning_wallet = make_wallet("somewallet"); + let amount = 42; + let amount2 = 55; + let expected_transactions = RetrievedBlockchainTransactions { + new_start_block: 8675309u64, + transactions: vec![ + BlockchainTransaction { + block_number: 8675308u64, + from: earning_wallet.clone(), + wei_amount: amount, + }, + BlockchainTransaction { + block_number: 8675309u64, + from: earning_wallet.clone(), + wei_amount: amount2, + }, + ], + }; + let lower_interface = LowBlockchainIntMock::default().get_block_number_result( + LatestBlockNumber::Err(BlockchainError::QueryFailed( + "\"Failed to read the latest block number\"".to_string(), + )), + ); + let blockchain_interface_mock = BlockchainInterfaceMock::default() + .retrieve_transactions_params(&retrieve_transactions_params_arc) + .retrieve_transactions_result(Ok(expected_transactions.clone())) + .lower_interface_results(Box::new(lower_interface)); + let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); + 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 subject = BlockchainBridge::new( + Box::new(blockchain_interface_mock), + 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(8675309u64)]); + 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: 8675309u64, + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: 4321 + }), + } ); } diff --git a/node/src/database/config_dumper.rs b/node/src/database/config_dumper.rs index c0f7aa61c..e5874cd51 100644 --- a/node/src/database/config_dumper.rs +++ b/node/src/database/config_dumper.rs @@ -353,8 +353,7 @@ mod tests { ); assert_value("neighborhoodMode", "zero-hop", &map); assert_value("schemaVersion", &CURRENT_SCHEMA_VERSION.to_string(), &map); - assert!(map.contains_key("startBlock")); - assert_none("startBlock", &map); + assert_null("startBlock", &map); assert_value( "exampleEncrypted", &dao.get("example_encrypted").unwrap().value_opt.unwrap(), @@ -500,8 +499,7 @@ mod tests { assert_value("pastNeighbors", "masq://polygon-mainnet:QUJDREVGR0hJSktMTU5PUFFSU1RVVldYWVowMTIzNDU@1.2.3.4:1234,masq://polygon-mainnet:QkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWjAxMjM0NTY@2.3.4.5:2345", &map); assert_value("neighborhoodMode", "consume-only", &map); assert_value("schemaVersion", &CURRENT_SCHEMA_VERSION.to_string(), &map); - assert!(map.contains_key("startBlock")); - assert_none("startBlock", &map); + assert_null("startBlock", &map); let expected_ee_entry = dao.get("example_encrypted").unwrap().value_opt.unwrap(); let expected_ee_decrypted = Bip39::decrypt_bytes(&expected_ee_entry, "password").unwrap(); let expected_ee_string = encode_bytes(Some(expected_ee_decrypted)).unwrap().unwrap(); @@ -614,8 +612,7 @@ mod tests { ); assert_value("neighborhoodMode", "standard", &map); assert_value("schemaVersion", &CURRENT_SCHEMA_VERSION.to_string(), &map); - assert!(map.contains_key("startBlock")); - assert_none("startBlock", &map); + assert_null("startBlock", &map); assert_value( "exampleEncrypted", &dao.get("example_encrypted").unwrap().value_opt.unwrap(), @@ -670,8 +667,14 @@ mod tests { assert_eq!(actual_value, expected_value); } - fn assert_none(key: &str, map: &Map) { - assert!(!map.get(key).is_none()); + fn assert_null(key: &str, map: &Map) { + assert!(map.contains_key(key)); + match map + .get(key) + .unwrap_or_else(|| panic!("record for {} is missing", key)) + { + value => assert!(value.is_null()), + } } fn assert_encrypted_value( diff --git a/node/src/node_configurator/configurator.rs b/node/src/node_configurator/configurator.rs index 6f6bef43a..f37d1bbd5 100644 --- a/node/src/node_configurator/configurator.rs +++ b/node/src/node_configurator/configurator.rs @@ -2124,10 +2124,6 @@ mod tests { assert_eq!(context_id, 4444); let check_start_block_params = set_start_block_params_arc.lock().unwrap(); assert_eq!(*check_start_block_params, vec![Some(166666)]); - TestLogHandler::new().exists_log_containing(&format!( - "DEBUG: {}: A request from UI received: {:?} from context id: {}", - test_name, msg, context_id - )); } #[test] @@ -2166,10 +2162,6 @@ mod tests { assert_eq!(context_id, 4444); let check_start_block_params = set_start_block_params_arc.lock().unwrap(); assert_eq!(*check_start_block_params, vec![None]); - TestLogHandler::new().exists_log_containing(&format!( - "DEBUG: {}: A request from UI received: {:?} from context id: {}", - test_name, msg, context_id - )); } #[test] diff --git a/node/src/test_utils/database_utils.rs b/node/src/test_utils/database_utils.rs index 9c483ee51..02ba441a4 100644 --- a/node/src/test_utils/database_utils.rs +++ b/node/src/test_utils/database_utils.rs @@ -40,27 +40,6 @@ pub fn bring_db_0_back_to_life_and_return_connection(db_path: &Path) -> Connecti conn } -pub fn bring_db_9_back_to_life_and_return_connection(db_path: &Path) -> Connection { - match remove_file(db_path) { - Err(e) if e.kind() == std::io::ErrorKind::NotFound => (), - Err(e) => panic!("Unexpected but serious error: {}", e), - _ => (), - } - let file_path = current_dir() - .unwrap() - .join("src") - .join("test_utils") - .join("database_version_9_sql.txt"); - let mut file = File::open(file_path).unwrap(); - let mut buffer = String::new(); - file.read_to_string(&mut buffer).unwrap(); - let conn = Connection::open(&db_path).unwrap(); - buffer.lines().for_each(|stm| { - conn.execute(stm, []).unwrap(); - }); - conn -} - #[derive(Default)] pub struct DbMigratorMock { logger: Option, diff --git a/node/src/test_utils/database_version_9_sql.txt b/node/src/test_utils/database_version_9_sql.txt deleted file mode 100644 index 7bb54be14..000000000 --- a/node/src/test_utils/database_version_9_sql.txt +++ /dev/null @@ -1,18 +0,0 @@ -create table config (name text not null, value text, encrypted integer not null) -create unique index idx_config_name on config (name) -insert into config (name, value, encrypted) values ('example_encrypted', null, 1) -insert into config (name, value, encrypted) values ('clandestine_port', '2897', 0) -insert into config (name, value, encrypted) values ('consuming_wallet_derivation_path', null, 0) -insert into config (name, value, encrypted) values ('consuming_wallet_public_key', null, 0) -insert into config (name, value, encrypted) values ('earning_wallet_address', null, 0) -insert into config (name, value, encrypted) values ('schema_version', '10', 0) -insert into config (name, value, encrypted) values ('seed', null, 0) -insert into config (name, value, encrypted) values ('start_block', null, 0) -insert into config (name, value, encrypted) values ('gas_price', '1', 0) -insert into config (name, value, encrypted) values ('past_neighbors', null, 1) -create table payable (wallet_address text primary key, balance integer not null, last_paid_timestamp integer not null, pending_payment_transaction text null) -create unique index idx_payable_wallet_address on payable (wallet_address) -create table receivable (wallet_address text primary key, balance integer not null, last_received_timestamp integer not null) -create unique index idx_receivable_wallet_address on receivable (wallet_address) -create table banned ( wallet_address text primary key ) -create unique index idx_banned_wallet_address on banned (wallet_address) From 2a0e285b054096fb39130ebfd39ff64222b42cb8 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 04/10] GH-606: Apply PR feedback changes --- node/src/accountant/mod.rs | 16 +- node/src/accountant/scanners/mod.rs | 112 +++++++----- 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 | 35 +++- .../persistent_configuration_mock.rs | 6 +- 8 files changed, 293 insertions(+), 117 deletions(-) diff --git a/node/src/accountant/mod.rs b/node/src/accountant/mod.rs index cd7381622..14479f607 100644 --- a/node/src/accountant/mod.rs +++ b/node/src/accountant/mod.rs @@ -124,7 +124,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, } @@ -1056,6 +1056,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::{ @@ -1373,7 +1374,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(); @@ -1383,7 +1388,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, @@ -1991,6 +1996,7 @@ mod tests { ) { let more_money_received_params_arc = Arc::new(Mutex::new(vec![])); let commit_params_arc = Arc::new(Mutex::new(vec![])); + let get_params_arc = Arc::new(Mutex::new(vec![])); let set_by_guest_transaction_params_arc = Arc::new(Mutex::new(vec![])); let now = SystemTime::now(); let earning_wallet = make_wallet("earner3000"); @@ -2014,6 +2020,8 @@ mod tests { .more_money_received_params(&more_money_received_params_arc) .more_money_received_result(wrapped_transaction); let config_dao = ConfigDaoMock::new() + .get_params(&get_params_arc) + .get_result(Ok(ConfigDaoRecord::new("start_block", None, false))) .set_by_guest_transaction_params(&set_by_guest_transaction_params_arc) .set_by_guest_transaction_result(Ok(())); let accountant = AccountantBuilder::default() @@ -2028,7 +2036,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"); diff --git a/node/src/accountant/scanners/mod.rs b/node/src/accountant/scanners/mod.rs index 07695164e..97e3710fe 100644 --- a/node/src/accountant/scanners/mod.rs +++ b/node/src/accountant/scanners/mod.rs @@ -860,15 +860,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) @@ -911,23 +919,29 @@ impl ReceivableScanner { .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_from_txn(msg.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) + } + Err(e) => panic!("Commit of received transactions failed: {:?}", e), + } } - Err(e) => panic!("Commit of received transactions failed: {:?}", e), } let total_newly_paid_receivable = msg @@ -1631,7 +1645,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) @@ -1883,7 +1897,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)) } @@ -2083,9 +2097,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), @@ -2106,7 +2118,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!( @@ -3068,8 +3080,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() @@ -3099,16 +3112,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 @@ -3132,6 +3150,7 @@ mod tests { .set_arbitrary_id_stamp(transaction_id); let transaction = TransactionSafeWrapper::new_with_builder(txn_inner_builder); let persistent_config = PersistentConfigurationMock::new() + .start_block_result(Ok(None)) .set_start_block_from_txn_params(&set_start_block_from_txn_params_arc) .set_start_block_from_txn_result(Ok(())); let receivable_dao = ReceivableDaoMock::new() @@ -3159,7 +3178,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()); @@ -3178,7 +3197,7 @@ mod tests { 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)] + vec![(Some(7890123u64), transaction_id)] ); let commit_params = commit_params_arc.lock().unwrap(); assert_eq!(*commit_params, vec![()]); @@ -3196,9 +3215,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_from_txn_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) @@ -3212,7 +3233,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 @@ -3240,8 +3261,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_from_txn_result(Ok(())); let receivable_dao = ReceivableDaoMock::new().more_money_received_result(transaction); let mut subject = ReceivableScannerBuilder::new() .receivable_dao(receivable_dao) @@ -3255,7 +3277,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 47ef63c02..9fe3fea53 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -331,40 +331,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); @@ -1296,7 +1299,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, @@ -1367,7 +1370,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 @@ -1389,7 +1392,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, @@ -1458,7 +1461,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 @@ -1478,7 +1571,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, @@ -1547,7 +1640,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 @@ -1563,7 +1656,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)); @@ -1610,7 +1703,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 @@ -1658,7 +1751,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 b9bfa37bf..f1a9c7b5d 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, }) } @@ -603,15 +606,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() } } } @@ -833,7 +839,7 @@ mod tests { assert_eq!( result, RetrievedBlockchainTransactions { - new_start_block: 0x4be664, + new_start_block: BlockNumber::Number(0x4be664.into()), transactions: vec![ BlockchainTransaction { block_number: 0x4be663, @@ -895,7 +901,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![] } ); @@ -1004,7 +1010,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![] }) ); @@ -1046,7 +1052,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![] }) ); @@ -1221,7 +1257,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..92bf81311 100644 --- a/node/src/db_config/persistent_configuration.rs +++ b/node/src/db_config/persistent_configuration.rs @@ -137,7 +137,7 @@ pub trait PersistentConfiguration { fn set_max_block_count(&mut self, value: Option) -> Result<(), PersistentConfigError>; fn set_start_block_from_txn( &mut self, - value: u64, + value: Option, transaction: &mut TransactionSafeWrapper, ) -> Result<(), PersistentConfigError>; fn set_wallet_info( @@ -424,7 +424,7 @@ impl PersistentConfiguration for PersistentConfigurationReal { fn set_start_block_from_txn( &mut self, - value: u64, + value: Option, transaction: &mut TransactionSafeWrapper, ) -> Result<(), PersistentConfigError> { self.simple_set_method_from_provided_txn("start_block", value, transaction) @@ -568,12 +568,14 @@ impl PersistentConfigurationReal { fn simple_set_method_from_provided_txn( &mut self, parameter_name: &str, - value: T, + value: Option, txn: &mut TransactionSafeWrapper, ) -> Result<(), PersistentConfigError> { - Ok(self - .dao - .set_by_guest_transaction(txn, parameter_name, Some(value.to_string()))?) + Ok(self.dao.set_by_guest_transaction( + txn, + parameter_name, + value.map(|v| v.to_string()).or(None), + )?) } fn combined_params_get_method<'a, T, C>( @@ -1510,7 +1512,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() @@ -1543,7 +1545,7 @@ mod tests { 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_from_txn(Some(1234), &mut txn); assert_eq!(result, Ok(())); let set_params = set_params_arc.lock().unwrap(); @@ -1553,6 +1555,23 @@ mod tests { ) } + #[test] + fn set_start_block_success_with_none() { + let set_params_arc = Arc::new(Mutex::new(vec![])); + let config_dao = Box::new( + ConfigDaoMock::new() + .set_params(&set_params_arc) + .set_result(Ok(())), + ); + let mut subject = PersistentConfigurationReal::new(config_dao); + + let result = subject.set_start_block(None); + + assert_eq!(result, Ok(())); + let set_params = set_params_arc.lock().unwrap(); + assert_eq!(*set_params, vec![("start_block".to_string(), None)]) + } + #[test] fn gas_price() { let config_dao = Box::new(ConfigDaoMock::new().get_result(Ok(ConfigDaoRecord::new( diff --git a/node/src/test_utils/persistent_configuration_mock.rs b/node/src/test_utils/persistent_configuration_mock.rs index ef1b85163..d50613392 100644 --- a/node/src/test_utils/persistent_configuration_mock.rs +++ b/node/src/test_utils/persistent_configuration_mock.rs @@ -65,7 +65,7 @@ pub struct PersistentConfigurationMock { max_block_count_results: RefCell, PersistentConfigError>>>, set_max_block_count_params: Arc>>>, set_max_block_count_results: RefCell>>, - set_start_block_from_txn_params: Arc>>, + set_start_block_from_txn_params: Arc, ArbitraryIdStamp)>>>, set_start_block_from_txn_results: RefCell>>, payment_thresholds_results: RefCell>>, set_payment_thresholds_params: Arc>>, @@ -252,7 +252,7 @@ impl PersistentConfiguration for PersistentConfigurationMock { fn set_start_block_from_txn( &mut self, - value: u64, + value: Option, transaction: &mut TransactionSafeWrapper, ) -> Result<(), PersistentConfigError> { self.set_start_block_from_txn_params @@ -586,7 +586,7 @@ impl PersistentConfigurationMock { pub fn set_start_block_from_txn_params( mut self, - params: &Arc>>, + params: &Arc, ArbitraryIdStamp)>>>, ) -> Self { self.set_start_block_from_txn_params = params.clone(); self From c96503582fc2e4d26c476f739ef6aa0318d85188 Mon Sep 17 00:00:00 2001 From: masqrauder <60554948+masqrauder@users.noreply.github.com> Date: Sun, 23 Jun 2024 16:17:58 -0400 Subject: [PATCH 05/10] GH-606: Apply PR review 4 feedback changes --- node/src/accountant/scanners/mod.rs | 4 ++++ .../blockchain_interface_web3/mod.rs | 9 ++++----- node/src/database/config_dumper.rs | 6 +++++- node/src/db_config/persistent_configuration.rs | 8 +++----- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/node/src/accountant/scanners/mod.rs b/node/src/accountant/scanners/mod.rs index 97e3710fe..cf826ed1a 100644 --- a/node/src/accountant/scanners/mod.rs +++ b/node/src/accountant/scanners/mod.rs @@ -876,6 +876,8 @@ impl Scanner for ReceivableScanner { &new_start_block, e ), } + } else { + warning!(logger, "The new_start_block ({}) is less than the current_start_block ({}). This is not a problem but by checking we avoid rescanning the same blocks again later.", &new_start_block, ¤t_start_block); } } } else { @@ -941,6 +943,8 @@ impl ReceivableScanner { } Err(e) => panic!("Commit of received transactions failed: {:?}", e), } + } else { + warning!(logger, "The new_start_block ({}) is less than the current_start_block ({}). This is not a problem but by checking we avoid rescanning the same blocks again later.", &new_start_block, ¤t_start_block); } } 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 f1a9c7b5d..0edbbdf68 100644 --- a/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs +++ b/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs @@ -134,7 +134,7 @@ where let logger = self.logger.clone(); match self.web3_batch.transport().submit_batch().wait() { Ok(_) => { - let response_block_number = match block_request.wait() { + let response_block_number_opt = match block_request.wait() { Ok(block_nbr) => { debug!(logger, "Latest block number: {}", block_nbr.as_u64()); Some(block_nbr.as_u64()) @@ -178,7 +178,7 @@ where // was not successful. let transaction_max_block_number = self .find_largest_transaction_block_number( - response_block_number, + response_block_number_opt, &transactions, ); debug!( @@ -188,9 +188,8 @@ where ); Ok(RetrievedBlockchainTransactions { new_start_block: transaction_max_block_number - .map_or(BlockNumber::Latest, |nsb| { - BlockNumber::Number((1u64 + nsb).into()) - }), + .map(|nsb| BlockNumber::Number((1u64 + nsb).into())) + .unwrap_or(BlockNumber::Latest), transactions, }) } diff --git a/node/src/database/config_dumper.rs b/node/src/database/config_dumper.rs index 3e8991e64..17e24899e 100644 --- a/node/src/database/config_dumper.rs +++ b/node/src/database/config_dumper.rs @@ -672,7 +672,11 @@ mod tests { let value = map .get(key) .unwrap_or_else(|| panic!("record for {} is missing", key)); - assert!(value.is_null()) + assert!( + value.is_null(), + "Expecting {} to be null, but it wasn't", + value + ) } fn assert_encrypted_value( diff --git a/node/src/db_config/persistent_configuration.rs b/node/src/db_config/persistent_configuration.rs index 92bf81311..e3ad060e5 100644 --- a/node/src/db_config/persistent_configuration.rs +++ b/node/src/db_config/persistent_configuration.rs @@ -571,11 +571,9 @@ impl PersistentConfigurationReal { value: Option, txn: &mut TransactionSafeWrapper, ) -> Result<(), PersistentConfigError> { - Ok(self.dao.set_by_guest_transaction( - txn, - parameter_name, - value.map(|v| v.to_string()).or(None), - )?) + Ok(self + .dao + .set_by_guest_transaction(txn, parameter_name, value.map(|v| v.to_string()))?) } fn combined_params_get_method<'a, T, C>( From f7938dedd926ea75d49bf3150f923a1c98964b74 Mon Sep 17 00:00:00 2001 From: masqrauder <60554948+masqrauder@users.noreply.github.com> Date: Wed, 10 Jul 2024 21:16:23 -0400 Subject: [PATCH 06/10] GH-606: Squashing commits - Save start_block_nbr if no msg but send as non-Option - Always commit - Reduce logging levels and simplify - Follow the Option naming pattern --- .../docker/blockchain/Dockerfile | 4 +- .../docker/blockchain/entrypoint.sh | 7 +- multinode_integration_tests/src/main.rs | 16 +-- .../src/mock_blockchain_client_server.rs | 14 +- .../tests/verify_bill_payment.rs | 13 +- node/src/accountant/mod.rs | 7 +- node/src/accountant/scanners/mod.rs | 84 +++++------- node/src/blockchain/blockchain_bridge.rs | 128 +++++++----------- .../src/db_config/persistent_configuration.rs | 34 ++--- 9 files changed, 136 insertions(+), 171 deletions(-) diff --git a/multinode_integration_tests/docker/blockchain/Dockerfile b/multinode_integration_tests/docker/blockchain/Dockerfile index 027eb7a27..7ff65ea16 100644 --- a/multinode_integration_tests/docker/blockchain/Dockerfile +++ b/multinode_integration_tests/docker/blockchain/Dockerfile @@ -1,8 +1,8 @@ # Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. -FROM trufflesuite/ganache-cli:v6.7.0 +FROM trufflesuite/ganache-cli:v6.12.2 ADD ./entrypoint.sh /app/ EXPOSE 18545 -ENTRYPOINT /app/entrypoint.sh +ENTRYPOINT ["/app/entrypoint.sh"] diff --git a/multinode_integration_tests/docker/blockchain/entrypoint.sh b/multinode_integration_tests/docker/blockchain/entrypoint.sh index f9d6cc220..c184cbb50 100755 --- a/multinode_integration_tests/docker/blockchain/entrypoint.sh +++ b/multinode_integration_tests/docker/blockchain/entrypoint.sh @@ -1,3 +1,8 @@ #!/bin/sh -node /app/ganache-core.docker.cli.js -p 18545 --networkId 2 --verbose --mnemonic "timber cage wide hawk phone shaft pattern movie army dizzy hen tackle lamp absent write kind term toddler sphere ripple idle dragon curious hold" +node /app/ganache-core.docker.cli.js \ + -h 0.0.0.0 \ + -p 18545 \ + --networkId 2 \ + --verbose \ + --mnemonic "timber cage wide hawk phone shaft pattern movie army dizzy hen tackle lamp absent write kind term toddler sphere ripple idle dragon curious hold" diff --git a/multinode_integration_tests/src/main.rs b/multinode_integration_tests/src/main.rs index d78421672..8f705fed9 100644 --- a/multinode_integration_tests/src/main.rs +++ b/multinode_integration_tests/src/main.rs @@ -1,11 +1,10 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. -use self::sub_lib::utils::indicates_dead_stream; use masq_lib::command::{Command, StdStreams}; use masq_lib::constants::{HIGHEST_USABLE_PORT, LOWEST_USABLE_INSECURE_PORT}; -use node_lib::sub_lib; use node_lib::sub_lib::framer::Framer; use node_lib::sub_lib::node_addr::NodeAddr; +use node_lib::sub_lib::utils::indicates_dead_stream; use node_lib::test_utils::data_hunk::DataHunk; use node_lib::test_utils::data_hunk_framer::DataHunkFramer; use std::borrow::BorrowMut; @@ -14,10 +13,9 @@ use std::env; use std::io; use std::io::Read; use std::io::Write; -use std::net::Shutdown; -use std::net::SocketAddr; use std::net::TcpListener; use std::net::TcpStream; +use std::net::{Shutdown, SocketAddr}; use std::process; use std::str::FromStr; use std::sync::{Arc, Mutex, MutexGuard}; @@ -223,10 +221,10 @@ impl MockNode { } fn usage(stderr: &mut dyn Write) -> u8 { - writeln! (stderr, "Usage: MockNode ://... where is the address MockNode is running on and is between {} and {}", - LOWEST_USABLE_INSECURE_PORT, - HIGHEST_USABLE_PORT, - ).unwrap (); + writeln!(stderr, "Usage: MockNode ://... where is the address MockNode is running on and is between {} and {}", + LOWEST_USABLE_INSECURE_PORT, + HIGHEST_USABLE_PORT, + ).unwrap(); 1 } @@ -369,7 +367,7 @@ mod tests { assert_eq!(result, 1); let stderr = holder.stderr; - assert_eq! (stderr.get_string (), String::from ("Usage: MockNode ://... where is the address MockNode is running on and is between 1025 and 65535\n\n")); + assert_eq!(stderr.get_string(), String::from("Usage: MockNode ://... where is the address MockNode is running on and is between 1025 and 65535\n\n")); } #[test] diff --git a/multinode_integration_tests/src/mock_blockchain_client_server.rs b/multinode_integration_tests/src/mock_blockchain_client_server.rs index 24031b2cc..a40543808 100644 --- a/multinode_integration_tests/src/mock_blockchain_client_server.rs +++ b/multinode_integration_tests/src/mock_blockchain_client_server.rs @@ -241,8 +241,10 @@ impl MockBlockchainClientServer { let mut requests = requests_arc.lock().unwrap(); requests.push(body); } - let response = responses.remove(0); - Self::send_body(conn_state, response); + if !responses.is_empty() { + let response = responses.remove(0); + Self::send_body(conn_state, response); + } let _ = notifier_tx.send(()); // receiver doesn't exist if test didn't set it up } None => (), @@ -437,7 +439,7 @@ mod tests { .response("Thank you and good night", 40) .start(); let mut client = connect(port); - client.write (b"POST /biddle HTTP/1.1\r\nContent-Length: 5\r\n\r\nfirstPOST /biddle HTTP/1.1\r\nContent-Length: 6\r\n\r\nsecond").unwrap(); + client.write(b"POST /biddle HTTP/1.1\r\nContent-Length: 5\r\n\r\nfirstPOST /biddle HTTP/1.1\r\nContent-Length: 6\r\n\r\nsecond").unwrap(); let (_, body) = receive_response(&mut client); assert_eq!( @@ -567,7 +569,7 @@ mod tests { assert_eq!(notified.try_recv().is_err(), true); let requests = subject.requests(); - assert_eq! (requests, vec! [ + assert_eq!(requests, vec![ "POST /biddle HTTP/1.1\r\nContent-Type: application-json\r\nContent-Length: 82\r\n\r\n{\"jsonrpc\": \"2.0\", \"method\": \"first\", \"params\": [\"biddle\", \"de\", \"bee\"], \"id\": 40}".to_string(), "POST /biddle HTTP/1.1\r\nContent-Type: application-json\r\nContent-Length: 48\r\n\r\n{\"jsonrpc\": \"2.0\", \"method\": \"second\", \"id\": 42}".to_string(), "POST /biddle HTTP/1.1\r\nContent-Type: application-json\r\nContent-Length: 47\r\n\r\n{\"jsonrpc\": \"2.0\", \"method\": \"third\", \"id\": 42}".to_string(), @@ -600,7 +602,7 @@ mod tests { r#"{"jsonrpc": "2.0", "result": {"name":"Billy","age":15}, "id": 42}"# ); let requests = subject.requests(); - assert_eq! (requests, vec! [ + assert_eq!(requests, vec![ "POST / HTTP/1.1\r\ncontent-type: application/json\r\nuser-agent: web3.rs\r\nhost: 172.18.0.1:32768\r\ncontent-length: 308\r\n\r\n{\"jsonrpc\":\"2.0\",\"method\":\"eth_getLogs\",\"params\":[{\"address\":\"0x59882e4a8f5d24643d4dda422922a870f1b3e664\",\"fromBlock\":\"0x3e8\",\"toBlock\":\"latest\",\"topics\":[\"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",null,\"0x00000000000000000000000027d9a2ac83b493f88ce9b4532edcf74e95b9788d\"]}],\"id\":0}".to_string() ]) } @@ -704,6 +706,6 @@ mod tests { body.len(), body ) - .into_bytes() + .into_bytes() } } diff --git a/multinode_integration_tests/tests/verify_bill_payment.rs b/multinode_integration_tests/tests/verify_bill_payment.rs index 5e9b50347..1240deb58 100644 --- a/multinode_integration_tests/tests/verify_bill_payment.rs +++ b/multinode_integration_tests/tests/verify_bill_payment.rs @@ -38,10 +38,7 @@ use web3::Web3; #[test] fn verify_bill_payment() { - let mut cluster = match MASQNodeCluster::start() { - Ok(cluster) => cluster, - Err(e) => panic!("{}", e), - }; + let mut cluster = MASQNodeCluster::start().unwrap(); let blockchain_server = BlockchainServer { name: "ganache-cli", }; @@ -64,7 +61,7 @@ fn verify_bill_payment() { assert_balances( &contract_owner_wallet, &blockchain_interface, - "99998043204000000000", + "99998381140000000000", "472000000000000000000000000", ); let payment_thresholds = PaymentThresholds { @@ -189,7 +186,7 @@ fn verify_bill_payment() { assert_balances( &contract_owner_wallet, &blockchain_interface, - "99998043204000000000", + "99998381140000000000", "472000000000000000000000000", ); @@ -235,7 +232,7 @@ fn verify_bill_payment() { assert_balances( &contract_owner_wallet, &blockchain_interface, - "99997886466000000000", + "99998223682000000000", "471999999700000000000000000", ); @@ -330,7 +327,7 @@ fn assert_balances( assert_eq!( format!("{}", eth_balance), String::from(expected_eth_balance), - "Actual EthBalance {} doesn't much with expected {}", + "Actual EthBalance {} doesn't match with expected {}", eth_balance, expected_eth_balance ); diff --git a/node/src/accountant/mod.rs b/node/src/accountant/mod.rs index 14479f607..e76b15a0d 100644 --- a/node/src/accountant/mod.rs +++ b/node/src/accountant/mod.rs @@ -124,7 +124,7 @@ pub struct ReceivedPayments { // a problem? Do we want to correct the timestamp? Discuss. pub timestamp: SystemTime, pub payments: Vec, - pub new_start_block: Option, + pub new_start_block: u64, pub response_skeleton_opt: Option, } @@ -1388,7 +1388,7 @@ mod tests { let received_payments = ReceivedPayments { timestamp: SystemTime::now(), payments: vec![], - new_start_block: Some(1234567), + new_start_block: 1234567, response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321, @@ -2036,7 +2036,7 @@ mod tests { .try_send(ReceivedPayments { timestamp: now, payments: vec![expected_receivable_1.clone(), expected_receivable_2.clone()], - new_start_block: Some(123456789u64), + new_start_block: 123456789u64, response_skeleton_opt: None, }) .expect("unexpected actix error"); @@ -4782,6 +4782,7 @@ mod tests { let factory = Accountant::dao_factory(data_dir); factory.make(); }; + assert_on_initialization_with_panic_on_migration(&data_dir, &act); } } diff --git a/node/src/accountant/scanners/mod.rs b/node/src/accountant/scanners/mod.rs index cf826ed1a..fc5f5ce91 100644 --- a/node/src/accountant/scanners/mod.rs +++ b/node/src/accountant/scanners/mod.rs @@ -860,25 +860,15 @@ impl Scanner for ReceivableScanner { "No newly received payments were detected during the scanning process." ); - 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 { - warning!(logger, "The new_start_block ({}) is less than the current_start_block ({}). This is not a problem but by checking we avoid rescanning the same blocks again later.", &new_start_block, ¤t_start_block); - } + 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 + ), } } else { self.handle_new_received_payments(&msg, logger) @@ -921,31 +911,23 @@ impl ReceivableScanner { .as_mut() .more_money_received(msg.timestamp, &msg.payments); - 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_from_txn(msg.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) - } - Err(e) => panic!("Commit of received transactions failed: {:?}", e), - } - } else { - warning!(logger, "The new_start_block ({}) is less than the current_start_block ({}). This is not a problem but by checking we avoid rescanning the same blocks again later.", &new_start_block, ¤t_start_block); + let new_start_block = msg.new_start_block; + match self + .persistent_configuration + .set_start_block_from_txn(Some(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) } + Err(e) => panic!("Commit of received transactions failed: {:?}", e), } let total_newly_paid_receivable = msg @@ -2101,7 +2083,9 @@ 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) + .is_innocent_age_result( + debt_age_s <= custom_payment_thresholds.maturity_threshold_sec as u64, + ) .is_innocent_balance_params(&is_innocent_balance_params_arc) .is_innocent_balance_result( balance <= gwei_to_wei(custom_payment_thresholds.permanent_debt_allowed_gwei), @@ -2122,7 +2106,7 @@ mod tests { assert_eq!(debt_age_returned_innocent, debt_age_s); assert_eq!( curve_derived_time, - custom_payment_thresholds.maturity_threshold_sec + custom_payment_thresholds.maturity_threshold_sec as u64 ); let is_innocent_balance_params = is_innocent_balance_params_arc.lock().unwrap(); assert_eq!( @@ -3084,7 +3068,7 @@ 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 = Some(4321); + let new_start_block = 4321; let persistent_config = PersistentConfigurationMock::new() .start_block_result(Ok(None)) .set_start_block_params(&set_start_block_params_arc) @@ -3117,7 +3101,7 @@ mod tests { let test_name = "no_transactions_received_but_start_block_setting_fails"; let now = SystemTime::now(); let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); - let new_start_block = Some(6709u64); + let new_start_block = 6709u64; let persistent_config = PersistentConfigurationMock::new() .start_block_result(Ok(None)) .set_start_block_params(&set_start_block_params_arc) @@ -3182,7 +3166,7 @@ mod tests { let msg = ReceivedPayments { timestamp: now, payments: receivables.clone(), - new_start_block: Some(7890123), + new_start_block: 7890123, response_skeleton_opt: None, }; subject.mark_as_started(SystemTime::now()); @@ -3237,7 +3221,7 @@ mod tests { let msg = ReceivedPayments { timestamp: now, payments: receivables, - new_start_block: Some(7890123), + new_start_block: 7890123, response_skeleton_opt: None, }; // Not necessary, rather for preciseness @@ -3281,7 +3265,7 @@ mod tests { let msg = ReceivedPayments { timestamp: now, payments: receivables, - new_start_block: Some(7890123), + new_start_block: 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 9fe3fea53..e657193a5 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -332,30 +332,31 @@ impl BlockchainBridge { match retrieved_transactions { Ok(transactions) => { 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"); + 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) + }; + } else { + self.received_payments_subs_opt + .as_ref() + .expect("Accountant is unbound") + .try_send(ReceivedPayments { + timestamp: SystemTime::now(), + payments: transactions.transactions, + new_start_block: 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: Some(new_start_block_number.as_u64()), - response_skeleton_opt: msg.response_skeleton_opt, - }) - .expect("Accountant is dead."); } Ok(()) } @@ -1321,12 +1322,9 @@ mod tests { .retrieve_transactions_params(&retrieve_transactions_params_arc) .retrieve_transactions_result(Ok(expected_transactions.clone())) .lower_interface_results(Box::new(lower_interface)); - let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); let persistent_config = PersistentConfigurationMock::new() .max_block_count_result(Ok(None)) - .start_block_result(Ok(Some(6))) - .set_start_block_params(&set_start_block_params_arc) - .set_start_block_result(Ok(())); + .start_block_result(Ok(Some(6))); let subject = BlockchainBridge::new( Box::new(blockchain_interface_mock), Box::new(persistent_config), @@ -1350,8 +1348,6 @@ mod tests { 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(8675309u64)]); let retrieve_transactions_params = retrieve_transactions_params_arc.lock().unwrap(); assert_eq!( *retrieve_transactions_params, @@ -1370,7 +1366,7 @@ mod tests { &ReceivedPayments { timestamp: received_payments.timestamp, payments: expected_transactions.transactions, - new_start_block: Some(8675309u64), + new_start_block: 8675309u64, response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321 @@ -1415,17 +1411,13 @@ mod tests { .retrieve_transactions_params(&retrieve_transactions_params_arc) .retrieve_transactions_result(Ok(expected_transactions.clone())) .lower_interface_results(Box::new(lower_interface)); - let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); 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(())); + .start_block_result(Ok(None)); let subject = BlockchainBridge::new( Box::new(blockchain_interface_mock), Box::new(persistent_config), false, - Some(make_wallet("consuming")), ); let addr = subject.start(); let subject_subs = BlockchainBridge::make_subs_from(&addr); @@ -1445,8 +1437,6 @@ mod tests { 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(8675309u64)]); let retrieve_transactions_params = retrieve_transactions_params_arc.lock().unwrap(); assert_eq!( *retrieve_transactions_params, @@ -1461,7 +1451,7 @@ mod tests { &ReceivedPayments { timestamp: received_payments.timestamp, payments: expected_transactions.transactions, - new_start_block: Some(8675309u64), + new_start_block: 8675309u64, response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321 @@ -1492,16 +1482,13 @@ mod tests { ], }; - 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(())); + .start_block_result(Ok(None)); let latest_block_number = LatestBlockNumber::Err(BlockchainError::QueryFailed( "Failed to read from block chain service".to_string(), )); @@ -1515,7 +1502,6 @@ mod tests { 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); @@ -1535,8 +1521,6 @@ mod tests { 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, @@ -1551,7 +1535,7 @@ mod tests { &ReceivedPayments { timestamp: received_payments.timestamp, payments: expected_transactions.transactions, - new_start_block: Some(98765), + new_start_block: 98765, response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321 @@ -1563,7 +1547,6 @@ mod tests { #[test] fn handle_retrieve_transactions_sends_received_payments_back_to_accountant() { let retrieve_transactions_params_arc = Arc::new(Mutex::new(vec![])); - let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); let system = System::new("handle_retrieve_transactions_sends_received_payments_back_to_accountant"); let (accountant, _, accountant_recording_arc) = make_recorder(); @@ -1594,9 +1577,7 @@ mod tests { .lower_interface_results(Box::new(lower_interface)); let persistent_config = PersistentConfigurationMock::new() .max_block_count_result(Ok(Some(10000u64))) - .start_block_result(Ok(Some(6))) - .set_start_block_params(&set_start_block_params_arc) - .set_start_block_result(Ok(())); + .start_block_result(Ok(Some(6))); let subject = BlockchainBridge::new( Box::new(blockchain_interface_mock), Box::new(persistent_config), @@ -1620,8 +1601,6 @@ mod tests { 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(9876u64)]); let retrieve_transactions_params = retrieve_transactions_params_arc.lock().unwrap(); assert_eq!( *retrieve_transactions_params, @@ -1640,7 +1619,7 @@ mod tests { &ReceivedPayments { timestamp: received_payments.timestamp, payments: expected_transactions.transactions, - new_start_block: Some(9876), + new_start_block: 9876, response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321 @@ -1666,7 +1645,7 @@ mod tests { .start_block_result(Ok(Some(6))) .set_start_block_params(&set_start_block_params_arc) .set_start_block_result(Ok(())); - let (accountant, _, accountant_recording_arc) = make_recorder(); + let (accountant, _, _) = make_recorder(); let system = System::new( "processing_of_received_payments_continues_even_if_no_payments_are_detected", ); @@ -1686,31 +1665,33 @@ mod tests { context_id: 4321, }), }; - let before = SystemTime::now(); + // let before = SystemTime::now(); let _ = addr.try_send(retrieve_transactions).unwrap(); System::current().stop(); system.run(); - let after = SystemTime::now(); + // let after = SystemTime::now(); let set_start_block_params = set_start_block_params_arc.lock().unwrap(); assert_eq!(*set_start_block_params, vec![Some(7)]); - let accountant_received_payment = accountant_recording_arc.lock().unwrap(); - 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: vec![], - new_start_block: Some(7), - response_skeleton_opt: Some(ResponseSkeleton { - client_id: 1234, - context_id: 4321 - }), - } - ); - TestLogHandler::new() + // let accountant_received_payment = accountant_recording_arc.lock().unwrap(); + // 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: vec![], + // new_start_block: 7, + // response_skeleton_opt: Some(ResponseSkeleton { + // client_id: 1234, + // context_id: 4321 + // }), + // } + // ); + let test_log_handler = TestLogHandler::new(); + test_log_handler.exists_log_containing("DEBUG: BlockchainBridge: Write new start block: 7"); + test_log_handler .exists_log_containing("DEBUG: BlockchainBridge: No new receivable detected"); } @@ -1752,18 +1733,13 @@ mod tests { let blockchain_interface = BlockchainInterfaceMock::default() .retrieve_transactions_result(Ok(RetrievedBlockchainTransactions { new_start_block: BlockNumber::Number(1234.into()), - transactions: vec![BlockchainTransaction { - block_number: 1000, - from: make_wallet("somewallet"), - wei_amount: 2345, - }], + transactions: vec![], })) .lower_interface_results(Box::new(lower_interface)); let mut subject = BlockchainBridge::new( Box::new(blockchain_interface), Box::new(persistent_config), false, - None, //not needed in this test ); let retrieve_transactions = RetrieveTransactions { recipient: make_wallet("somewallet"), diff --git a/node/src/db_config/persistent_configuration.rs b/node/src/db_config/persistent_configuration.rs index e3ad060e5..532048a34 100644 --- a/node/src/db_config/persistent_configuration.rs +++ b/node/src/db_config/persistent_configuration.rs @@ -113,7 +113,7 @@ pub trait PersistentConfiguration { fn mapping_protocol(&self) -> Result, PersistentConfigError>; fn set_mapping_protocol( &mut self, - value: Option, + value_opt: Option, ) -> Result<(), PersistentConfigError>; fn min_hops(&self) -> Result; fn set_min_hops(&mut self, value: Hops) -> Result<(), PersistentConfigError>; @@ -132,12 +132,12 @@ pub trait PersistentConfiguration { db_password: &str, ) -> Result<(), PersistentConfigError>; fn start_block(&self) -> Result, PersistentConfigError>; - fn set_start_block(&mut self, value: Option) -> Result<(), PersistentConfigError>; + fn set_start_block(&mut self, value_opt: Option) -> Result<(), PersistentConfigError>; fn max_block_count(&self) -> Result, PersistentConfigError>; - fn set_max_block_count(&mut self, value: Option) -> Result<(), PersistentConfigError>; + fn set_max_block_count(&mut self, value_opt: Option) -> Result<(), PersistentConfigError>; fn set_start_block_from_txn( &mut self, - value: Option, + value_opt: Option, transaction: &mut TransactionSafeWrapper, ) -> Result<(), PersistentConfigError>; fn set_wallet_info( @@ -335,9 +335,9 @@ impl PersistentConfiguration for PersistentConfigurationReal { fn set_mapping_protocol( &mut self, - value: Option, + value_opt: Option, ) -> Result<(), PersistentConfigError> { - Ok(self.dao.set("mapping_protocol", value.map(to_string))?) + Ok(self.dao.set("mapping_protocol", value_opt.map(to_string))?) } fn min_hops(&self) -> Result { @@ -410,24 +410,24 @@ impl PersistentConfiguration for PersistentConfigurationReal { Ok(decode_u64(self.get("start_block")?)?) } - fn set_start_block(&mut self, value: Option) -> Result<(), PersistentConfigError> { - Ok(self.dao.set("start_block", encode_u64(value)?)?) + fn set_start_block(&mut self, value_opt: Option) -> Result<(), PersistentConfigError> { + Ok(self.dao.set("start_block", encode_u64(value_opt)?)?) } fn max_block_count(&self) -> Result, PersistentConfigError> { Ok(decode_u64(self.get("max_block_count")?)?) } - fn set_max_block_count(&mut self, value: Option) -> Result<(), PersistentConfigError> { - Ok(self.dao.set("max_block_count", encode_u64(value)?)?) + fn set_max_block_count(&mut self, value_opt: Option) -> Result<(), PersistentConfigError> { + Ok(self.dao.set("max_block_count", encode_u64(value_opt)?)?) } fn set_start_block_from_txn( &mut self, - value: Option, + value_opt: Option, transaction: &mut TransactionSafeWrapper, ) -> Result<(), PersistentConfigError> { - self.simple_set_method_from_provided_txn("start_block", value, transaction) + self.simple_set_method_from_provided_txn("start_block", value_opt, transaction) } fn set_wallet_info( @@ -568,12 +568,14 @@ impl PersistentConfigurationReal { fn simple_set_method_from_provided_txn( &mut self, parameter_name: &str, - value: Option, + value_opt: Option, txn: &mut TransactionSafeWrapper, ) -> Result<(), PersistentConfigError> { - Ok(self - .dao - .set_by_guest_transaction(txn, parameter_name, value.map(|v| v.to_string()))?) + Ok(self.dao.set_by_guest_transaction( + txn, + parameter_name, + value_opt.map(|v| v.to_string()), + )?) } fn combined_params_get_method<'a, T, C>( From 89e71716f2b89bb1d1542480d6530ce6a80f80f4 Mon Sep 17 00:00:00 2001 From: masqrauder <60554948+masqrauder@users.noreply.github.com> Date: Wed, 9 Oct 2024 23:50:59 -0400 Subject: [PATCH 07/10] GH-600: set_start_block only called in accountant/scanners/mod.rs --- node/src/blockchain/blockchain_bridge.rs | 109 +++++++---------------- 1 file changed, 30 insertions(+), 79 deletions(-) diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index e657193a5..eadfc40d3 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -334,29 +334,17 @@ impl BlockchainBridge { if let BlockNumber::Number(new_start_block_number) = transactions.new_start_block { if transactions.transactions.is_empty() { debug!(self.logger, "No new receivable detected"); - 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) - }; - } else { - self.received_payments_subs_opt - .as_ref() - .expect("Accountant is unbound") - .try_send(ReceivedPayments { - timestamp: SystemTime::now(), - payments: transactions.transactions, - new_start_block: 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: new_start_block_number.as_u64(), + response_skeleton_opt: msg.response_skeleton_opt, + }) + .expect("Accountant is dead."); } Ok(()) } @@ -1639,13 +1627,10 @@ mod tests { transactions: vec![], })) .lower_interface_results(Box::new(lower_interface)); - let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); let persistent_config = PersistentConfigurationMock::new() .max_block_count_result(Ok(Some(10000u64))) - .start_block_result(Ok(Some(6))) - .set_start_block_params(&set_start_block_params_arc) - .set_start_block_result(Ok(())); - let (accountant, _, _) = make_recorder(); + .start_block_result(Ok(Some(6))); + let (accountant, _, accountant_recording_arc) = make_recorder(); let system = System::new( "processing_of_received_payments_continues_even_if_no_payments_are_detected", ); @@ -1665,33 +1650,29 @@ mod tests { context_id: 4321, }), }; - // let before = SystemTime::now(); + 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(7)]); - // let accountant_received_payment = accountant_recording_arc.lock().unwrap(); - // 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: vec![], - // new_start_block: 7, - // response_skeleton_opt: Some(ResponseSkeleton { - // client_id: 1234, - // context_id: 4321 - // }), - // } - // ); - let test_log_handler = TestLogHandler::new(); - test_log_handler.exists_log_containing("DEBUG: BlockchainBridge: Write new start block: 7"); - test_log_handler + let after = SystemTime::now(); + let accountant_received_payment = accountant_recording_arc.lock().unwrap(); + 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: vec![], + new_start_block: 7, + response_skeleton_opt: Some(ResponseSkeleton { + client_id: 1234, + context_id: 4321 + }), + } + ); + TestLogHandler::new() .exists_log_containing("DEBUG: BlockchainBridge: No new receivable detected"); } @@ -1719,36 +1700,6 @@ mod tests { let _ = subject.handle_retrieve_transactions(retrieve_transactions); } - #[test] - #[should_panic( - expected = "Cannot set start block 1234 in database; payments to you may not be processed: TransactionError" - )] - fn handle_retrieve_transactions_panics_if_start_block_cannot_be_written() { - let persistent_config = PersistentConfigurationMock::new() - .start_block_result(Ok(Some(1234))) - .max_block_count_result(Ok(Some(10000u64))) - .set_start_block_result(Err(PersistentConfigError::TransactionError)); - let lower_interface = - LowBlockchainIntMock::default().get_block_number_result(Ok(0u64.into())); - let blockchain_interface = BlockchainInterfaceMock::default() - .retrieve_transactions_result(Ok(RetrievedBlockchainTransactions { - new_start_block: BlockNumber::Number(1234.into()), - transactions: vec![], - })) - .lower_interface_results(Box::new(lower_interface)); - let mut subject = BlockchainBridge::new( - Box::new(blockchain_interface), - Box::new(persistent_config), - false, - ); - let retrieve_transactions = RetrieveTransactions { - recipient: make_wallet("somewallet"), - response_skeleton_opt: None, - }; - - let _ = subject.handle_retrieve_transactions(retrieve_transactions); - } - fn success_handler( _bcb: &mut BlockchainBridge, _msg: RetrieveTransactions, From 382f0de3bd558a97ef9f9ea902a8801df05b9bea Mon Sep 17 00:00:00 2001 From: masqrauder <60554948+masqrauder@users.noreply.github.com> Date: Sun, 20 Oct 2024 16:07:49 -0400 Subject: [PATCH 08/10] GH-606: PR Feedback - parameterize a test --- node/Cargo.lock | 14 +++++++++++ node/Cargo.toml | 29 +++++++++++----------- node/src/node_configurator/configurator.rs | 12 ++++++--- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/node/Cargo.lock b/node/Cargo.lock index 63e2b2029..e013a9f2d 100644 --- a/node/Cargo.lock +++ b/node/Cargo.lock @@ -2132,6 +2132,7 @@ dependencies = [ "regex", "rlp", "rpassword", + "rstest", "rusqlite", "rustc-hex", "secp256k1", @@ -3046,6 +3047,19 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "rstest" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c77c86a545c460cffcb2e5f558392e2e6a1edcc9d463cf092bf4ec3c7990641d" +dependencies = [ + "cfg-if 1.0.0", + "proc-macro2 1.0.59", + "quote 1.0.28", + "rustc_version 0.3.3", + "syn 1.0.85", +] + [[package]] name = "rusqlite" version = "0.28.0" diff --git a/node/Cargo.toml b/node/Cargo.toml index 7d01fd728..04a19e32f 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -11,20 +11,20 @@ members = ["../multinode_integration_tests", "../masq_lib", "../masq"] [dependencies] actix = "0.7.9" -automap = { path = "../automap"} +automap = { path = "../automap" } backtrace = "0.3.57" base64 = "0.13.0" bytes = "0.4.12" -time = {version = "0.3.11", features = [ "macros" ]} +time = { version = "0.3.11", features = ["macros"] } clap = "2.33.3" crossbeam-channel = "0.5.1" dirs = "4.0.0" ethabi = "12.0.0" -ethsign = {version = "0.7.3", default-features = false, features = ["pure-rust"]} +ethsign = { version = "0.7.3", default-features = false, features = ["pure-rust"] } ethsign-crypto = "0.2.1" ethereum-types = "0.9.0" fdlimit = "0.2.1" -flexi_logger = { version = "0.15.12", features = [ "ziplogs" ] } +flexi_logger = { version = "0.15.12", features = ["ziplogs"] } futures = "0.1.31" heck = "0.3.3" http = "0.2.5" @@ -34,15 +34,15 @@ lazy_static = "1.4.0" libc = "0.2.107" libsecp256k1 = "0.7.0" log = "0.4.14" -masq_lib = { path = "../masq_lib"} +masq_lib = { path = "../masq_lib" } paste = "1.0.6" pretty-hex = "0.2.1" -primitive-types = {version = "0.5.0", default-features = false, features = ["default", "rlp", "serde"]} -rand = {version = "0.8.4", features = ["getrandom", "small_rng"]} +primitive-types = { version = "0.5.0", default-features = false, features = ["default", "rlp", "serde"] } +rand = { version = "0.8.4", features = ["getrandom", "small_rng"] } regex = "1.5.4" rlp = "0.4.6" rpassword = "5.0.1" -rusqlite = {version = "0.28.0", features = ["bundled","functions"]} +rusqlite = { version = "0.28.0", features = ["bundled", "functions"] } rustc-hex = "2.1.0" serde = "1.0.136" serde_derive = "1.0.136" @@ -61,9 +61,9 @@ trust-dns = "0.17.0" trust-dns-resolver = "0.12.0" unindent = "0.1.7" variant_count = "1.1.0" -web3 = {version = "0.11.0", default-features = false, features = ["http", "tls"]} -websocket = {version = "0.26.2", default-features = false, features = ["async", "sync"]} -secp256k1secrets = {package = "secp256k1", version = "0.17.2"} +web3 = { version = "0.11.0", default-features = false, features = ["http", "tls"] } +websocket = { version = "0.26.2", default-features = false, features = ["async", "sync"] } +secp256k1secrets = { package = "secp256k1", version = "0.17.2" } uuid = "0.7.4" [target.'cfg(target_os = "macos")'.dependencies] @@ -72,7 +72,7 @@ core-foundation = "0.7.0" [target.'cfg(not(target_os = "windows"))'.dependencies] nix = "0.23.0" -openssl = {version = "0.10.38", features = ["vendored"]} +openssl = { version = "0.10.38", features = ["vendored"] } [target.'cfg(target_os = "windows")'.dependencies] winreg = "0.10.1" @@ -81,7 +81,8 @@ ipconfig = "0.2.2" [dev-dependencies] base58 = "0.2.0" jsonrpc-core = "14.0.0" -native-tls = {version = "0.2.8", features = ["vendored"]} +native-tls = { version = "0.2.8", features = ["vendored"] } +rstest = "0.9.0" simple-server = "0.4.0" serial_test_derive = "0.5.1" serial_test = "0.5.1" @@ -103,4 +104,4 @@ path = "src/lib.rs" expose_test_privates = [] #[profile.release] -#opt-level = 0 \ No newline at end of file +#opt-level = 0 diff --git a/node/src/node_configurator/configurator.rs b/node/src/node_configurator/configurator.rs index f37d1bbd5..3f79d04ab 100644 --- a/node/src/node_configurator/configurator.rs +++ b/node/src/node_configurator/configurator.rs @@ -914,6 +914,7 @@ mod tests { use masq_lib::constants::MISSING_DATA; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; use masq_lib::utils::{derivation_path, AutomapProtocol, NeighborhoodModeLight}; + use rstest::rstest; use rustc_hex::FromHex; use tiny_hderive::bip32::ExtendedPrivKey; @@ -2126,8 +2127,13 @@ mod tests { assert_eq!(*check_start_block_params, vec![Some(166666)]); } - #[test] - fn handle_set_configuration_accepts_none_to_unset_start_block() { + #[rstest] + #[case("none")] + #[case("None")] + #[case("nOnE")] + #[case("NoNe")] + #[case("NONE")] + fn handle_set_configuration_accepts_none_to_unset_start_block(#[case] cfgValue: &str) { init_test_logging(); let test_name = "handle_set_configuration_accepts_none_to_unset_start_block"; let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); @@ -2142,7 +2148,7 @@ mod tests { subject_addr.try_send(BindMessage { peer_actors }).unwrap(); let msg = UiSetConfigurationRequest { name: "start-block".to_string(), - value: "none".to_string(), + value: cfgValue.to_string(), }; let context_id = 4444; From 7c9b69d55666d55cd14f8c677299c8f981e7a75b Mon Sep 17 00:00:00 2001 From: masqrauder <60554948+masqrauder@users.noreply.github.com> Date: Mon, 21 Oct 2024 22:17:04 -0400 Subject: [PATCH 09/10] GH-606: Address PR feedback --- .../blockchain_interface/blockchain_interface_web3/mod.rs | 8 +++++--- node/src/node_configurator/configurator.rs | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) 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 0edbbdf68..a4425a82f 100644 --- a/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs +++ b/node/src/blockchain/blockchain_interface/blockchain_interface_web3/mod.rs @@ -1060,9 +1060,11 @@ mod tests { #[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 contains_error_causing_to_pick_fallback_value = 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"}]}]"#; + let _test_server = TestServer::start( + port, + vec![contains_error_causing_to_pick_fallback_value.to_vec()], + ); let (event_loop_handle, transport) = Http::with_max_parallel( &format!("http://{}:{}", &Ipv4Addr::LOCALHOST, port), REQUESTS_IN_PARALLEL, diff --git a/node/src/node_configurator/configurator.rs b/node/src/node_configurator/configurator.rs index 3f79d04ab..8fa24d1eb 100644 --- a/node/src/node_configurator/configurator.rs +++ b/node/src/node_configurator/configurator.rs @@ -2133,7 +2133,7 @@ mod tests { #[case("nOnE")] #[case("NoNe")] #[case("NONE")] - fn handle_set_configuration_accepts_none_to_unset_start_block(#[case] cfgValue: &str) { + fn handle_set_configuration_accepts_none_to_unset_start_block(#[case] cfg_value: &str) { init_test_logging(); let test_name = "handle_set_configuration_accepts_none_to_unset_start_block"; let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); @@ -2148,7 +2148,7 @@ mod tests { subject_addr.try_send(BindMessage { peer_actors }).unwrap(); let msg = UiSetConfigurationRequest { name: "start-block".to_string(), - value: cfgValue.to_string(), + value: cfg_value.to_string(), }; let context_id = 4444; From a9c7fafa1775c9d3395f50c1277b0807aaf9b0cf Mon Sep 17 00:00:00 2001 From: masqrauder <60554948+masqrauder@users.noreply.github.com> Date: Mon, 28 Oct 2024 21:41:29 -0400 Subject: [PATCH 10/10] GH-606: Implement parameterized test without crate macro --- node/Cargo.lock | 14 ----------- node/Cargo.toml | 29 +++++++++++----------- node/src/node_configurator/configurator.rs | 23 +++++++++-------- 3 files changed, 27 insertions(+), 39 deletions(-) diff --git a/node/Cargo.lock b/node/Cargo.lock index e013a9f2d..63e2b2029 100644 --- a/node/Cargo.lock +++ b/node/Cargo.lock @@ -2132,7 +2132,6 @@ dependencies = [ "regex", "rlp", "rpassword", - "rstest", "rusqlite", "rustc-hex", "secp256k1", @@ -3047,19 +3046,6 @@ dependencies = [ "winapi 0.3.9", ] -[[package]] -name = "rstest" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c77c86a545c460cffcb2e5f558392e2e6a1edcc9d463cf092bf4ec3c7990641d" -dependencies = [ - "cfg-if 1.0.0", - "proc-macro2 1.0.59", - "quote 1.0.28", - "rustc_version 0.3.3", - "syn 1.0.85", -] - [[package]] name = "rusqlite" version = "0.28.0" diff --git a/node/Cargo.toml b/node/Cargo.toml index 04a19e32f..7d01fd728 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -11,20 +11,20 @@ members = ["../multinode_integration_tests", "../masq_lib", "../masq"] [dependencies] actix = "0.7.9" -automap = { path = "../automap" } +automap = { path = "../automap"} backtrace = "0.3.57" base64 = "0.13.0" bytes = "0.4.12" -time = { version = "0.3.11", features = ["macros"] } +time = {version = "0.3.11", features = [ "macros" ]} clap = "2.33.3" crossbeam-channel = "0.5.1" dirs = "4.0.0" ethabi = "12.0.0" -ethsign = { version = "0.7.3", default-features = false, features = ["pure-rust"] } +ethsign = {version = "0.7.3", default-features = false, features = ["pure-rust"]} ethsign-crypto = "0.2.1" ethereum-types = "0.9.0" fdlimit = "0.2.1" -flexi_logger = { version = "0.15.12", features = ["ziplogs"] } +flexi_logger = { version = "0.15.12", features = [ "ziplogs" ] } futures = "0.1.31" heck = "0.3.3" http = "0.2.5" @@ -34,15 +34,15 @@ lazy_static = "1.4.0" libc = "0.2.107" libsecp256k1 = "0.7.0" log = "0.4.14" -masq_lib = { path = "../masq_lib" } +masq_lib = { path = "../masq_lib"} paste = "1.0.6" pretty-hex = "0.2.1" -primitive-types = { version = "0.5.0", default-features = false, features = ["default", "rlp", "serde"] } -rand = { version = "0.8.4", features = ["getrandom", "small_rng"] } +primitive-types = {version = "0.5.0", default-features = false, features = ["default", "rlp", "serde"]} +rand = {version = "0.8.4", features = ["getrandom", "small_rng"]} regex = "1.5.4" rlp = "0.4.6" rpassword = "5.0.1" -rusqlite = { version = "0.28.0", features = ["bundled", "functions"] } +rusqlite = {version = "0.28.0", features = ["bundled","functions"]} rustc-hex = "2.1.0" serde = "1.0.136" serde_derive = "1.0.136" @@ -61,9 +61,9 @@ trust-dns = "0.17.0" trust-dns-resolver = "0.12.0" unindent = "0.1.7" variant_count = "1.1.0" -web3 = { version = "0.11.0", default-features = false, features = ["http", "tls"] } -websocket = { version = "0.26.2", default-features = false, features = ["async", "sync"] } -secp256k1secrets = { package = "secp256k1", version = "0.17.2" } +web3 = {version = "0.11.0", default-features = false, features = ["http", "tls"]} +websocket = {version = "0.26.2", default-features = false, features = ["async", "sync"]} +secp256k1secrets = {package = "secp256k1", version = "0.17.2"} uuid = "0.7.4" [target.'cfg(target_os = "macos")'.dependencies] @@ -72,7 +72,7 @@ core-foundation = "0.7.0" [target.'cfg(not(target_os = "windows"))'.dependencies] nix = "0.23.0" -openssl = { version = "0.10.38", features = ["vendored"] } +openssl = {version = "0.10.38", features = ["vendored"]} [target.'cfg(target_os = "windows")'.dependencies] winreg = "0.10.1" @@ -81,8 +81,7 @@ ipconfig = "0.2.2" [dev-dependencies] base58 = "0.2.0" jsonrpc-core = "14.0.0" -native-tls = { version = "0.2.8", features = ["vendored"] } -rstest = "0.9.0" +native-tls = {version = "0.2.8", features = ["vendored"]} simple-server = "0.4.0" serial_test_derive = "0.5.1" serial_test = "0.5.1" @@ -104,4 +103,4 @@ path = "src/lib.rs" expose_test_privates = [] #[profile.release] -#opt-level = 0 +#opt-level = 0 \ No newline at end of file diff --git a/node/src/node_configurator/configurator.rs b/node/src/node_configurator/configurator.rs index 8fa24d1eb..19b0b958a 100644 --- a/node/src/node_configurator/configurator.rs +++ b/node/src/node_configurator/configurator.rs @@ -914,7 +914,6 @@ mod tests { use masq_lib::constants::MISSING_DATA; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; use masq_lib::utils::{derivation_path, AutomapProtocol, NeighborhoodModeLight}; - use rstest::rstest; use rustc_hex::FromHex; use tiny_hderive::bip32::ExtendedPrivKey; @@ -2127,22 +2126,26 @@ mod tests { assert_eq!(*check_start_block_params, vec![Some(166666)]); } - #[rstest] - #[case("none")] - #[case("None")] - #[case("nOnE")] - #[case("NoNe")] - #[case("NONE")] - fn handle_set_configuration_accepts_none_to_unset_start_block(#[case] cfg_value: &str) { + #[test] + fn handle_none_cases() { + vec!["none", "None", "nOnE", "NoNe", "NONE"] + .iter() + .for_each(|value| handle_set_configuration_accepts_none_to_unset_start_block(value)); + } + + fn handle_set_configuration_accepts_none_to_unset_start_block(cfg_value: &str) { init_test_logging(); - let test_name = "handle_set_configuration_accepts_none_to_unset_start_block"; + let test_name = format!( + "handle_set_configuration_accepts_{}_to_unset_start_block", + &cfg_value + ); let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); let (ui_gateway, _, ui_gateway_recording_arc) = make_recorder(); let persistent_config = PersistentConfigurationMock::new() .set_start_block_params(&set_start_block_params_arc) .set_start_block_result(Ok(())); let mut subject = make_subject(Some(persistent_config)); - subject.logger = Logger::new(test_name); + subject.logger = Logger::new(test_name.as_str()); let subject_addr = subject.start(); let peer_actors = peer_actors_builder().ui_gateway(ui_gateway).build(); subject_addr.try_send(BindMessage { peer_actors }).unwrap();