From bd56d3f3214f8514b1d682b83755043f5fcab792 Mon Sep 17 00:00:00 2001 From: Bert <65427484+bertllll@users.noreply.github.com> Date: Sat, 27 Jan 2024 16:58:48 +0700 Subject: [PATCH] GH-747: Txn received from unknown wallets + start block set on the same db txn as where we account inward payments (#350) * GH-554: rough infrastructure for PaymentAdjuster; almost get first round test running * GH-672: simplifying GH-554, let's go on without the Neighborhood involved for the time being * GH-672: PaymentAdjusterMock and the supertrait with ScannerEtensions implemented for a try * GH-672-with-trait-impl-and-generics: perhaps going in the right direction * GH-672-best-methodology-found: implementation works * GH-672: first version of adjustment implemented * GH-672: finished implementing the main test; not refactored yet * GH-672: payment adjuster refactored; tuning logging; but wanna master features that are missing here * GH-672: some logging integrated * GH-672: fine debug log * GH-672: interim commit * GH-672: interim commit * GH-672: another package of big changes (partly-private msgs; payment adjuster) * GH-672: decent debug log for adjusted payables * GH-672: broadening the adjustment capabilities with gas; rough framework * GH-672: savepoint; before experimenting with early gas_limit computation * GH-672: continuing in implementation of the check for gas availability. * GH-672: some renaming and moving files * GH-672: almost completed but there is a cenceptual error; maybe extra tests for BlockchainInterfaceClandestine should be made first * GH-672: last details; adding tests for untested code from BlockchainInterfaceClandestine * GH-672: almost done; some refactoring ahead * GH-672: heading to auto-review * GH-672: auto review findings * GH-672: refactored scan scheduling code * GH-672: fixing issues from the review; save point before the shift of responsibility of the estimated gas limit from BchI to Chain * GH-672: before connecting the early fetched gas price and the actual gas price used for generating hashed transactions * GH-672: logic redesigned + renaming * GH-672: I believe the first review is answered by now * GH-672: almost all the old code updated; will have to implement the transaction fee calculators * GH-672: before serious changes with PayablePaymentsAgent starts * GH-672-with-agent: saving when getting into troubles with testing * GH-672-with-agent: going in a nice direction; interim commit * GH-672-with-agent: approaching the fixture...this might work well; last vexing situations unhandled meaning there are like 8 failing tests at the moment * GH-672-with-agent: could be called roughly complete; will go with refactoring and knocking off some todos left behind me * GH-672-with-agent: interim commit * GH-672-with-agent: hopefully the last changes before a review * GH-672-with-agent: almost all what I found in my auto-review * GH-672-with-agent: refactored the structure of PayablePaymentsSetupMsg * GH-672-with-agent: formatting * GH-672-wa-no-test-hack: recorder can also record and stop the system from PartalEq missing Actor messages; two of four tests repaired * GH-672-wa-no-test-hack: hurrah, managed to get out of the trap with an agent in Actor msgs even without having to implement Debug, PartialEq and Clone a trait object...this is definitely a lighter version * GH-672-wa-no-test-hack: little modifications in comments * GH-672: first batch of fixed items from the review 3 * GH-672: another batch of fixes * GH-672: refactoring some complicated test concepts for actor_system_factory and adding one test for blockchain_bridge of that kind * GH-672: more refactoring and little changes * GH-672: fixing stuff araound blockchain_interface web3 and its initialization * GH-672: savepoint * GH-672: renaming * GH-672: almost all things done; AgentDigest fully in * GH-672: finished last few missing places * GH-672: reworking comments and found a little mistake * GH-672: match stop condition turned into easier one to use * GH-672: protected payables implemented * GH-672: until now the protected payables were implemented badly; improved design * GH-672: compiling, but many failing tests * GH-672: still fixing tests, but a lot of them passing now * GH-672: halting hard with thoughts about implementation of the build_blockcahin_agent_method * GH-672: blockchain interface onward and backwards move; messy rearrangement; wanna make a savepoint before trying cargo fix * GH-672: all tests good; saving before cargo fix * GH-672: roughly ready code; let's review now * GH-672: renaming in accountant folders * GH-672: reverting some renaming in favor of another branch taking care of this * GH-672: interim commit * GH-672: little fixes and adding more in to some null methods with todos * GH-672: polishing; savepoint before moving get_transaction_receipt to plain_rpc * GH-672: improved details * GH-747: mostly complete, one test still left to be done and some ideas about refactoring arose * GH-747: even more tests needed * GH-747: last todo knocked out * GH-747: db connection options for big_int_db_processor simplified * GH-747: better design for 'other_params()' * GH-747: alias types for easier reading * GH-672: mostly done; just a few details left * GH-672: abondoning enhancement of blockchain interface initializer...leaving as it is * GH-672: unnoticable change for the augly low level tests in actor_system_factory * GH-672: blockchain_bridge_shared_test_simplified * GH-672: ready for review 5 * Revert "GH-747: db connection options for big_int_db_processor simplified" This reverts commit e987841e3f600a5ef4d06e08b33d27382bae268e. * GH-747: prepared for new big redesign of the test to watch the transaction roll back at panic for a hard error * GH-747: most of this is over me * GH-747: savepoint before trying to mess with lifetimes hard * GH-747: savepoint before trying to mess with lifetimes hard * GH-747: preparing for mocking BigIntProcessor * GH-747: a lot stabilized...refactoring next?? * GH-747: found the right hierarchy in the code (redesigned to be cleaner and robust)... will have to fix a tougher test and figure out what to do with a manual Debug impl * GH-747: renaming * GH-747: hard work on creating a reliable test tool; yet more of the undone left * GH-747: I'm gonna try to make the test tool more of how an unbiased viewer would expect to see it * GH-747: tools enhanced and some little debrits cleaned up * GH-747: small corrections in text * GH-747-experiment-start-block: before trying to do 'impossible' * GH-672: interim commit * GH-672: finished review 5 * GH-672: review 6 answered * GH-672: fixes in multinode tests * GH-672: little cosmetics * GH-747-experiment-start-block * GH-747-experiment-start-block: interim commit * GH-747-experiment-start-block: since now prepared for all sorts of testing; let's roll up our sleeves * GH-747-experiment-start-block: new procedure for setting start block works first time * GH-757-experiment-start-block: turned out a good idea... completed * GH-747: a dull sketch * GH-747: for both situations, no and some transactions found, start block setting implemented * GH-747: last error handling missing - but added in * GH-747: some small changes * GH-747: working on a more convenient wrapper around the rusqlite transaction; also rephrased a big comment * GH-747: first stage of enhancing the trasnaction wrapper seems done * GH-747: better interface for wrapped transaction * GH-747: interim commit * GH-747: transaction wrapper redesigned successfuly * GH-747: didn't include these accidently * GH-747: builder implemented; interface now doesn't allow misuse of the inner tx wrapper * GH-747: few more comments knocked off * GH-747: transaction wrapper test version polishing * GH-747: new little safety feature implemented * GH-747: small improvements, still working the review * GH-747: and again small impovements, still working the review * GH-747: interim commit * GH-747: more code neatened in big_int_db_processor * GH-747: revamp in receivable (at least I think) * GH-747: revamp of bulky comments * GH-747: interim commit before a big revamp * GH-747: I lost interst in the previous thing but will do files replacement now; save point * GH-747: deleted files mistakely merged in * GH-747: finished before trying master merge * GH-747: finalizing before the review; writing better comments etc * GH-747: more text revamped * GH-747: first bunch of fixes after the 2nd review * GH-747: second big bunch * GH-747: some new minor changes * GH-747: all comments addressed * GH-747: clippy * GH-747: review 2 reaction: revamping text * GH-747: review 2 reaction: another attempt, smaller * bumping version 0.8.0 --------- Co-authored-by: Bert --- automap/Cargo.lock | 4 +- automap/Cargo.toml | 2 +- dns_utility/Cargo.lock | 4 +- dns_utility/Cargo.toml | 2 +- masq/Cargo.toml | 2 +- masq/src/commands/change_password_command.rs | 4 +- masq/src/commands/check_password_command.rs | 4 +- masq/src/commands/configuration_command.rs | 4 +- .../src/commands/connection_status_command.rs | 4 +- masq/src/commands/generate_wallets_command.rs | 4 +- masq/src/commands/recover_wallets_command.rs | 4 +- .../src/commands/set_configuration_command.rs | 4 +- masq/src/commands/setup_command.rs | 4 +- masq/src/commands/wallet_addresses_command.rs | 4 +- masq/src/communications/broadcast_handler.rs | 6 +- masq_lib/Cargo.toml | 2 +- masq_lib/src/utils.rs | 25 +- multinode_integration_tests/Cargo.toml | 2 +- multinode_integration_tests/src/utils.rs | 2 +- node/Cargo.lock | 10 +- node/Cargo.toml | 2 +- .../db_access_objects/banned_dao.rs | 2 +- .../db_access_objects/payable_dao.rs | 144 +-- .../db_access_objects/pending_payable_dao.rs | 6 +- .../db_access_objects/receivable_dao.rs | 734 +++++++++--- .../src/accountant/db_access_objects/utils.rs | 4 +- .../db_big_integer/big_int_db_processor.rs | 1060 ++++++++++------- node/src/accountant/db_big_integer/mod.rs | 3 +- .../accountant/db_big_integer/test_utils.rs | 81 +- node/src/accountant/mod.rs | 70 +- node/src/accountant/payment_adjuster.rs | 4 +- .../payable_scanner/agent_null.rs | 2 +- .../payable_scanner/blockchain_agent.rs | 2 +- node/src/accountant/scanners/mod.rs | 336 ++++-- .../src/accountant/scanners/scanners_utils.rs | 5 +- node/src/accountant/test_utils.rs | 76 +- node/src/actor_system_factory.rs | 2 + node/src/blockchain/blockchain_bridge.rs | 74 +- .../blockchain_interface_null/mod.rs | 2 +- .../blockchain/blockchain_interface/mod.rs | 2 +- .../blockchain_interface/test_utils.rs | 1 - node/src/blockchain/test_utils.rs | 2 +- node/src/bootstrapper.rs | 1 + node/src/daemon/daemon_initializer.rs | 2 +- node/src/daemon/setup_reporter.rs | 2 +- node/src/database/config_dumper.rs | 4 +- node/src/database/connection_wrapper.rs | 29 - node/src/database/db_initializer.rs | 133 +-- .../src/database/db_migrations/db_migrator.rs | 37 +- .../migrations/migration_4_to_5.rs | 2 +- .../migrations/migration_6_to_7.rs | 2 +- .../database/db_migrations/migrator_utils.rs | 39 +- node/src/database/db_migrations/test_utils.rs | 4 +- node/src/database/mod.rs | 3 +- node/src/database/rusqlite_wrappers.rs | 185 +++ node/src/database/test_utils/mod.rs | 116 ++ .../test_utils/transaction_wrapper_mock.rs | 328 +++++ node/src/db_config/config_dao.rs | 150 ++- node/src/db_config/config_dao_null.rs | 50 + node/src/db_config/mocks.rs | 38 +- .../src/db_config/persistent_configuration.rs | 54 +- node/src/run_modes_factories.rs | 6 +- node/src/server_initializer.rs | 2 +- node/src/sub_lib/accountant.rs | 6 +- node/src/sub_lib/utils.rs | 8 +- node/src/test_utils/actor_system_factory.rs | 2 +- node/src/test_utils/database_utils.rs | 2 +- .../persistent_configuration_mock.rs | 32 + node/tests/utils.rs | 2 +- port_exposer/Cargo.lock | 2 +- port_exposer/Cargo.toml | 2 +- 71 files changed, 2775 insertions(+), 1179 deletions(-) delete mode 100644 node/src/database/connection_wrapper.rs create mode 100644 node/src/database/rusqlite_wrappers.rs create mode 100644 node/src/database/test_utils/mod.rs create mode 100644 node/src/database/test_utils/transaction_wrapper_mock.rs diff --git a/automap/Cargo.lock b/automap/Cargo.lock index dfd2a58cb..c970dc200 100644 --- a/automap/Cargo.lock +++ b/automap/Cargo.lock @@ -137,7 +137,7 @@ checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" [[package]] name = "automap" -version = "0.7.3" +version = "0.8.0" dependencies = [ "crossbeam-channel 0.5.8", "flexi_logger", @@ -1051,7 +1051,7 @@ dependencies = [ [[package]] name = "masq_lib" -version = "0.7.3" +version = "0.8.0" dependencies = [ "actix", "clap", diff --git a/automap/Cargo.toml b/automap/Cargo.toml index 25fd22993..c89c47ced 100644 --- a/automap/Cargo.toml +++ b/automap/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "automap" -version = "0.7.3" +version = "0.8.0" authors = ["Dan Wiebe ", "MASQ"] license = "GPL-3.0-only" description = "Library full of code to make routers map ports through firewalls" diff --git a/dns_utility/Cargo.lock b/dns_utility/Cargo.lock index 04c8bf983..52a3ec6b3 100644 --- a/dns_utility/Cargo.lock +++ b/dns_utility/Cargo.lock @@ -430,7 +430,7 @@ dependencies = [ [[package]] name = "dns_utility" -version = "0.7.3" +version = "0.8.0" dependencies = [ "core-foundation", "ipconfig 0.2.2", @@ -854,7 +854,7 @@ dependencies = [ [[package]] name = "masq_lib" -version = "0.7.3" +version = "0.8.0" dependencies = [ "actix", "clap", diff --git a/dns_utility/Cargo.toml b/dns_utility/Cargo.toml index 05ca8fb15..f8ce5620f 100644 --- a/dns_utility/Cargo.toml +++ b/dns_utility/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "dns_utility" -version = "0.7.3" +version = "0.8.0" license = "GPL-3.0-only" authors = ["Dan Wiebe ", "MASQ"] copyright = "Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved." diff --git a/masq/Cargo.toml b/masq/Cargo.toml index f2300bbd4..87a20ad67 100644 --- a/masq/Cargo.toml +++ b/masq/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "masq" -version = "0.7.3" +version = "0.8.0" authors = ["Dan Wiebe ", "MASQ"] license = "GPL-3.0-only" description = "Reference implementation of user interface for MASQ Node" diff --git a/masq/src/commands/change_password_command.rs b/masq/src/commands/change_password_command.rs index 9dcc71075..2498ea900 100644 --- a/masq/src/commands/change_password_command.rs +++ b/masq/src/commands/change_password_command.rs @@ -9,7 +9,7 @@ use clap::{App, Arg, SubCommand}; use masq_lib::messages::{ UiChangePasswordRequest, UiChangePasswordResponse, UiNewPasswordBroadcast, }; -use masq_lib::{as_any_in_trait_impl, short_writeln}; +use masq_lib::{as_any_ref_in_trait_impl, short_writeln}; use std::io::Write; #[derive(Debug, PartialEq, Eq)] @@ -79,7 +79,7 @@ impl Command for ChangePasswordCommand { Ok(()) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } pub fn change_password_subcommand() -> App<'static, 'static> { diff --git a/masq/src/commands/check_password_command.rs b/masq/src/commands/check_password_command.rs index ee7081e7d..4fb78ac71 100644 --- a/masq/src/commands/check_password_command.rs +++ b/masq/src/commands/check_password_command.rs @@ -5,7 +5,7 @@ use crate::commands::commands_common::{ transaction, Command, CommandError, STANDARD_COMMAND_TIMEOUT_MILLIS, }; use clap::{App, Arg, SubCommand}; -use masq_lib::as_any_in_trait_impl; +use masq_lib::as_any_ref_in_trait_impl; use masq_lib::messages::{UiCheckPasswordRequest, UiCheckPasswordResponse}; use masq_lib::short_writeln; use masq_lib::utils::to_string; @@ -51,7 +51,7 @@ impl Command for CheckPasswordCommand { Ok(()) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } impl CheckPasswordCommand { diff --git a/masq/src/commands/configuration_command.rs b/masq/src/commands/configuration_command.rs index 766eedc6d..8701f1617 100644 --- a/masq/src/commands/configuration_command.rs +++ b/masq/src/commands/configuration_command.rs @@ -6,7 +6,7 @@ use crate::commands::commands_common::{ dump_parameter_line, transaction, Command, CommandError, STANDARD_COMMAND_TIMEOUT_MILLIS, }; use clap::{App, Arg, SubCommand}; -use masq_lib::as_any_in_trait_impl; +use masq_lib::as_any_ref_in_trait_impl; use masq_lib::constants::NODE_NOT_RUNNING_ERROR; use masq_lib::messages::{UiConfigurationRequest, UiConfigurationResponse}; use masq_lib::short_writeln; @@ -64,7 +64,7 @@ impl Command for ConfigurationCommand { } } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } impl ConfigurationCommand { diff --git a/masq/src/commands/connection_status_command.rs b/masq/src/commands/connection_status_command.rs index fc96c6716..52d769647 100644 --- a/masq/src/commands/connection_status_command.rs +++ b/masq/src/commands/connection_status_command.rs @@ -6,7 +6,7 @@ use crate::commands::commands_common::{ transaction, Command, CommandError, STANDARD_COMMAND_TIMEOUT_MILLIS, }; use clap::{App, SubCommand}; -use masq_lib::as_any_in_trait_impl; +use masq_lib::as_any_ref_in_trait_impl; use masq_lib::constants::NODE_NOT_RUNNING_ERROR; use masq_lib::messages::{ UiConnectionStage, UiConnectionStatusRequest, UiConnectionStatusResponse, @@ -62,7 +62,7 @@ impl Command for ConnectionStatusCommand { } } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } impl ConnectionStatusCommand { diff --git a/masq/src/commands/generate_wallets_command.rs b/masq/src/commands/generate_wallets_command.rs index 6bfd8b359..ff0ff799a 100644 --- a/masq/src/commands/generate_wallets_command.rs +++ b/masq/src/commands/generate_wallets_command.rs @@ -6,7 +6,7 @@ use crate::commands::commands_common::{ }; use clap::{App, Arg, SubCommand}; use lazy_static::lazy_static; -use masq_lib::as_any_in_trait_impl; +use masq_lib::as_any_ref_in_trait_impl; use masq_lib::messages::{UiGenerateSeedSpec, UiGenerateWalletsRequest, UiGenerateWalletsResponse}; use masq_lib::short_writeln; use masq_lib::utils::DEFAULT_EARNING_DERIVATION_PATH; @@ -165,7 +165,7 @@ impl Command for GenerateWalletsCommand { Ok(()) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } pub fn generate_wallets_subcommand() -> App<'static, 'static> { diff --git a/masq/src/commands/recover_wallets_command.rs b/masq/src/commands/recover_wallets_command.rs index b7f3e6bc1..91993233b 100644 --- a/masq/src/commands/recover_wallets_command.rs +++ b/masq/src/commands/recover_wallets_command.rs @@ -6,7 +6,7 @@ use crate::commands::commands_common::{ }; use clap::{App, Arg, ArgGroup, SubCommand}; use itertools::{Either, Itertools}; -use masq_lib::as_any_in_trait_impl; +use masq_lib::as_any_ref_in_trait_impl; use masq_lib::messages::{UiRecoverSeedSpec, UiRecoverWalletsRequest, UiRecoverWalletsResponse}; use masq_lib::short_writeln; use masq_lib::utils::to_string; @@ -120,7 +120,7 @@ impl Command for RecoverWalletsCommand { Ok(()) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } const RECOVER_WALLETS_ABOUT: &str = diff --git a/masq/src/commands/set_configuration_command.rs b/masq/src/commands/set_configuration_command.rs index 72fabc498..99d979f07 100644 --- a/masq/src/commands/set_configuration_command.rs +++ b/masq/src/commands/set_configuration_command.rs @@ -1,7 +1,7 @@ use crate::command_context::CommandContext; use crate::commands::commands_common::{transaction, Command, CommandError}; use clap::{App, Arg, ArgGroup, SubCommand}; -use masq_lib::as_any_in_trait_impl; +use masq_lib::as_any_ref_in_trait_impl; use masq_lib::messages::{UiSetConfigurationRequest, UiSetConfigurationResponse}; use masq_lib::shared_schema::gas_price_arg; use masq_lib::shared_schema::min_hops_arg; @@ -53,7 +53,7 @@ impl Command for SetConfigurationCommand { Ok(()) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } const SET_CONFIGURATION_ABOUT: &str = diff --git a/masq/src/commands/setup_command.rs b/masq/src/commands/setup_command.rs index baec35e1b..92f605247 100644 --- a/masq/src/commands/setup_command.rs +++ b/masq/src/commands/setup_command.rs @@ -4,7 +4,7 @@ use crate::command_context::CommandContext; use crate::commands::commands_common::{transaction, Command, CommandError}; use crate::terminal::terminal_interface::TerminalWrapper; use clap::{value_t, App, SubCommand}; -use masq_lib::as_any_in_trait_impl; +use masq_lib::as_any_ref_in_trait_impl; use masq_lib::constants::SETUP_ERROR; use masq_lib::messages::{ UiSetupBroadcast, UiSetupInner, UiSetupRequest, UiSetupRequestValue, UiSetupResponse, @@ -51,7 +51,7 @@ impl Command for SetupCommand { Err(e) => Err(e), } } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } impl SetupCommand { diff --git a/masq/src/commands/wallet_addresses_command.rs b/masq/src/commands/wallet_addresses_command.rs index edc2e2a0f..0150f1244 100644 --- a/masq/src/commands/wallet_addresses_command.rs +++ b/masq/src/commands/wallet_addresses_command.rs @@ -6,7 +6,7 @@ use crate::commands::commands_common::{ }; use clap::{App, Arg, SubCommand}; use masq_lib::messages::{UiWalletAddressesRequest, UiWalletAddressesResponse}; -use masq_lib::{as_any_in_trait_impl, short_writeln}; +use masq_lib::{as_any_ref_in_trait_impl, short_writeln}; #[derive(Debug, PartialEq, Eq)] pub struct WalletAddressesCommand { @@ -66,7 +66,7 @@ impl Command for WalletAddressesCommand { ); Ok(()) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } #[cfg(test)] diff --git a/masq/src/communications/broadcast_handler.rs b/masq/src/communications/broadcast_handler.rs index e4903ef17..1b8ee4a28 100644 --- a/masq/src/communications/broadcast_handler.rs +++ b/masq/src/communications/broadcast_handler.rs @@ -11,7 +11,7 @@ use masq_lib::messages::{ }; use masq_lib::ui_gateway::MessageBody; use masq_lib::utils::ExpectValue; -use masq_lib::{as_any_in_trait, as_any_in_trait_impl, short_writeln}; +use masq_lib::{as_any_ref_in_trait, as_any_ref_in_trait_impl, short_writeln}; use std::fmt::Debug; use std::io::Write; use std::thread; @@ -20,7 +20,7 @@ use crate::notifications::connection_change_notification::ConnectionChangeNotifi pub trait BroadcastHandle: Send { fn send(&self, message_body: MessageBody); - as_any_in_trait!(); + as_any_ref_in_trait!(); } pub struct BroadcastHandleInactive; @@ -28,7 +28,7 @@ pub struct BroadcastHandleInactive; impl BroadcastHandle for BroadcastHandleInactive { //simply dropped (unless we find a better use for such a message) fn send(&self, _message_body: MessageBody) {} - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } pub struct BroadcastHandleGeneric { diff --git a/masq_lib/Cargo.toml b/masq_lib/Cargo.toml index 60fa87c79..f87de81a1 100644 --- a/masq_lib/Cargo.toml +++ b/masq_lib/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "masq_lib" -version = "0.7.3" +version = "0.8.0" authors = ["Dan Wiebe ", "MASQ"] license = "GPL-3.0-only" description = "Code common to Node and masq; also, temporarily, to dns_utility" diff --git a/masq_lib/src/utils.rs b/masq_lib/src/utils.rs index 24dc6d320..1083af2b9 100644 --- a/masq_lib/src/utils.rs +++ b/masq_lib/src/utils.rs @@ -412,7 +412,7 @@ macro_rules! intentionally_blank { } #[macro_export] -macro_rules! as_any_in_trait { +macro_rules! as_any_ref_in_trait { () => { #[cfg(test)] fn as_any(&self) -> &dyn std::any::Any { @@ -423,7 +423,7 @@ macro_rules! as_any_in_trait { } #[macro_export] -macro_rules! as_any_in_trait_impl { +macro_rules! as_any_ref_in_trait_impl { () => { #[cfg(test)] fn as_any(&self) -> &dyn std::any::Any { @@ -432,6 +432,27 @@ macro_rules! as_any_in_trait_impl { }; } +#[macro_export] +macro_rules! as_any_mut_in_trait { + () => { + #[cfg(test)] + fn as_any_mut(&mut self) -> &mut dyn std::any::Any { + use masq_lib::intentionally_blank; + intentionally_blank!() + } + }; +} + +#[macro_export] +macro_rules! as_any_mut_in_trait_impl { + () => { + #[cfg(test)] + fn as_any_mut(&mut self) -> &mut dyn std::any::Any { + self + } + }; +} + #[macro_export] macro_rules! test_only_use { ($($use_clause: item),+) => { diff --git a/multinode_integration_tests/Cargo.toml b/multinode_integration_tests/Cargo.toml index cc6b9424a..edf7a03f4 100644 --- a/multinode_integration_tests/Cargo.toml +++ b/multinode_integration_tests/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "multinode_integration_tests" -version = "0.7.3" +version = "0.8.0" authors = ["Dan Wiebe ", "MASQ"] license = "GPL-3.0-only" description = "" diff --git a/multinode_integration_tests/src/utils.rs b/multinode_integration_tests/src/utils.rs index 47f452881..12c63f1ec 100644 --- a/multinode_integration_tests/src/utils.rs +++ b/multinode_integration_tests/src/utils.rs @@ -7,10 +7,10 @@ use masq_lib::test_utils::utils::TEST_DEFAULT_MULTINODE_CHAIN; use masq_lib::utils::NeighborhoodModeLight; use node_lib::accountant::db_access_objects::payable_dao::{PayableDao, PayableDaoReal}; use node_lib::accountant::db_access_objects::receivable_dao::{ReceivableDao, ReceivableDaoReal}; -use node_lib::database::connection_wrapper::ConnectionWrapper; use node_lib::database::db_initializer::{ DbInitializationConfig, DbInitializer, DbInitializerReal, ExternalData, }; +use node_lib::database::rusqlite_wrappers::ConnectionWrapper; use node_lib::db_config::config_dao::{ConfigDao, ConfigDaoReal}; use node_lib::neighborhood::node_record::NodeRecordInner_0v1; use node_lib::neighborhood::AccessibleGossipRecord; diff --git a/node/Cargo.lock b/node/Cargo.lock index 67aed1891..04806d093 100644 --- a/node/Cargo.lock +++ b/node/Cargo.lock @@ -182,7 +182,7 @@ checksum = "cdb031dd78e28731d87d56cc8ffef4a8f36ca26c38fe2de700543e627f8a464a" [[package]] name = "automap" -version = "0.7.3" +version = "0.8.0" dependencies = [ "crossbeam-channel 0.5.1", "flexi_logger 0.17.1", @@ -1802,7 +1802,7 @@ dependencies = [ [[package]] name = "masq" -version = "0.7.3" +version = "0.8.0" dependencies = [ "atty", "clap", @@ -1822,7 +1822,7 @@ dependencies = [ [[package]] name = "masq_lib" -version = "0.7.3" +version = "0.8.0" dependencies = [ "actix", "clap", @@ -1999,7 +1999,7 @@ dependencies = [ [[package]] name = "multinode_integration_tests" -version = "0.7.3" +version = "0.8.0" dependencies = [ "base64 0.13.0", "crossbeam-channel 0.5.1", @@ -2092,7 +2092,7 @@ dependencies = [ [[package]] name = "node" -version = "0.7.3" +version = "0.8.0" dependencies = [ "actix", "automap", diff --git a/node/Cargo.toml b/node/Cargo.toml index 3635fc1de..4ae89971d 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "node" -version = "0.7.3" +version = "0.8.0" license = "GPL-3.0-only" authors = ["Dan Wiebe ", "MASQ"] description = "MASQ Node is the foundation of MASQ Network, an open-source network that allows anyone to allocate spare computing resources to make the internet a free and fair place for the entire world." diff --git a/node/src/accountant/db_access_objects/banned_dao.rs b/node/src/accountant/db_access_objects/banned_dao.rs index 24b3ced81..4528d3a80 100644 --- a/node/src/accountant/db_access_objects/banned_dao.rs +++ b/node/src/accountant/db_access_objects/banned_dao.rs @@ -1,7 +1,7 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. use crate::accountant::db_access_objects::utils::{DaoFactoryReal, VigilantRusqliteFlatten}; -use crate::database::connection_wrapper::ConnectionWrapper; +use crate::database::rusqlite_wrappers::ConnectionWrapper; use crate::sub_lib::wallet::Wallet; use lazy_static::lazy_static; use rusqlite::{Error, ErrorCode, ToSql}; diff --git a/node/src/accountant/db_access_objects/payable_dao.rs b/node/src/accountant/db_access_objects/payable_dao.rs index 16f72d5d4..88897281b 100644 --- a/node/src/accountant/db_access_objects/payable_dao.rs +++ b/node/src/accountant/db_access_objects/payable_dao.rs @@ -1,14 +1,9 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. -use crate::accountant::db_big_integer::big_int_db_processor::KnownKeyVariants::{ +use crate::accountant::db_big_integer::big_int_db_processor::KeyVariants::{ PendingPayableRowid, WalletAddress, }; -use crate::accountant::db_big_integer::big_int_db_processor::WeiChange::{ - Addition, Subtraction, -}; -use crate::accountant::db_big_integer::big_int_db_processor::{ - BigIntDbProcessor, BigIntSqlConfig, Param, SQLParamsBuilder, TableNameDAO, -}; +use crate::accountant::db_big_integer::big_int_db_processor::{BigIntDbProcessor, BigIntDbProcessorReal, BigIntSqlConfig, DisplayableRusqliteParamPair, ParamByUse, SQLParamsBuilder, TableNameDAO, WeiChange, WeiChangeDirection}; use crate::accountant::db_big_integer::big_int_divider::BigIntDivider; use crate::accountant::db_access_objects::utils; use crate::accountant::db_access_objects::utils::{ @@ -20,11 +15,10 @@ use crate::accountant::db_access_objects::payable_dao::mark_pending_payable_asso }; use crate::accountant::{checked_conversion, sign_conversion, PendingPayableId}; use crate::blockchain::blockchain_bridge::PendingPayableFingerprint; -use crate::database::connection_wrapper::ConnectionWrapper; +use crate::database::rusqlite_wrappers::ConnectionWrapper; use crate::sub_lib::wallet::Wallet; #[cfg(test)] use ethereum_types::{BigEndianHash, U256}; -use itertools::Either::Left; use masq_lib::utils::ExpectValue; #[cfg(test)] use rusqlite::OptionalExtension; @@ -32,6 +26,7 @@ use rusqlite::{Error, Row}; use std::fmt::Debug; use std::str::FromStr; use std::time::SystemTime; +use itertools::Either; use web3::types::H256; #[derive(Debug, PartialEq, Eq)] @@ -89,7 +84,7 @@ impl PayableDaoFactory for DaoFactoryReal { #[derive(Debug)] pub struct PayableDaoReal { conn: Box, - big_int_db_processor: BigIntDbProcessor, + big_int_db_processor: BigIntDbProcessorReal, } impl PayableDao for PayableDaoReal { @@ -102,24 +97,28 @@ impl PayableDao for PayableDaoReal { let main_sql = "insert into payable (wallet_address, balance_high_b, balance_low_b, last_paid_timestamp, pending_payable_rowid) \ values (:wallet, :balance_high_b, :balance_low_b, :last_paid_timestamp, null) on conflict (wallet_address) do update set \ balance_high_b = balance_high_b + :balance_high_b, balance_low_b = balance_low_b + :balance_low_b where wallet_address = :wallet"; - let overflow_update_clause = "update payable set \ + let update_clause_with_compensated_overflow = "update payable set \ balance_high_b = :balance_high_b, balance_low_b = :balance_low_b where wallet_address = :wallet"; - Ok(self.big_int_db_processor.execute( - Left(self.conn.as_ref()), - BigIntSqlConfig::new( - main_sql, - overflow_update_clause, - SQLParamsBuilder::default() - .key(WalletAddress(wallet)) - .wei_change(Addition("balance", amount)) - .other_params(vec![Param::new( - (":last_paid_timestamp", &to_time_t(timestamp)), - false, - )]) - .build(), - ), - )?) + let last_paid_timestamp = to_time_t(timestamp); + let params = SQLParamsBuilder::default() + .key(WalletAddress(wallet)) + .wei_change(WeiChange::new( + "balance", + amount, + WeiChangeDirection::Addition, + )) + .other_params(vec![ParamByUse::BeforeOverflowOnly( + DisplayableRusqliteParamPair::new(":last_paid_timestamp", &last_paid_timestamp), + )]) + .build(); + + self.big_int_db_processor.execute( + Either::Left(self.conn.as_ref()), + BigIntSqlConfig::new(main_sql, update_clause_with_compensated_overflow, params), + )?; + + Ok(()) } fn mark_pending_payables_rowids( @@ -149,24 +148,29 @@ impl PayableDao for PayableDaoReal { &self, confirmed_payables: &[PendingPayableFingerprint], ) -> Result<(), PayableDaoError> { - confirmed_payables.iter().try_for_each(|fgp| { + confirmed_payables.iter().try_for_each(|pending_payable_fingerprint| { let main_sql = "update payable set \ balance_high_b = balance_high_b + :balance_high_b, balance_low_b = balance_low_b + :balance_low_b, \ last_paid_timestamp = :last_paid, pending_payable_rowid = null where pending_payable_rowid = :rowid"; - let overflow_update_clause = "update payable set \ + let update_clause_with_compensated_overflow = "update payable set \ balance_high_b = :balance_high_b, balance_low_b = :balance_low_b, last_paid_timestamp = :last_paid, \ pending_payable_rowid = null where pending_payable_rowid = :rowid"; - let conn = Left(self.conn.as_ref()); - Ok(self.big_int_db_processor.execute(conn, BigIntSqlConfig::new( + let i64_rowid = checked_conversion::(pending_payable_fingerprint.rowid); + let last_paid = to_time_t(pending_payable_fingerprint.timestamp); + let params = SQLParamsBuilder::default() + .key( PendingPayableRowid(&i64_rowid)) + .wei_change(WeiChange::new( "balance", pending_payable_fingerprint.amount, WeiChangeDirection::Subtraction)) + .other_params(vec![ParamByUse::BeforeAndAfterOverflow(DisplayableRusqliteParamPair::new(":last_paid", &last_paid))]) + .build(); + + self.big_int_db_processor.execute(Either::Left(self.conn.as_ref()), BigIntSqlConfig::new( main_sql, - overflow_update_clause, - SQLParamsBuilder::default() - .key( PendingPayableRowid(&checked_conversion::(fgp.rowid))) - .wei_change(Subtraction("balance",fgp.amount)) - .other_params(vec![Param::new((":last_paid", &to_time_t(fgp.timestamp)),true)]) - .build()))?) + update_clause_with_compensated_overflow, + params))?; + + Ok(()) }) } @@ -303,7 +307,7 @@ impl PayableDaoReal { pub fn new(conn: Box) -> PayableDaoReal { PayableDaoReal { conn, - big_int_db_processor: BigIntDbProcessor::default(), + big_int_db_processor: BigIntDbProcessorReal::default(), } } @@ -391,7 +395,7 @@ mod mark_pending_payable_associated_functions { use crate::accountant::db_access_objects::utils::{ update_rows_and_return_valid_count, VigilantRusqliteFlatten, }; - use crate::database::connection_wrapper::ConnectionWrapper; + use crate::database::rusqlite_wrappers::ConnectionWrapper; use crate::sub_lib::wallet::Wallet; use itertools::Itertools; use rusqlite::Row; @@ -540,13 +544,9 @@ mod tests { use crate::accountant::db_access_objects::utils::{from_time_t, now_time_t, to_time_t}; use crate::accountant::gwei_to_wei; use crate::accountant::db_access_objects::payable_dao::mark_pending_payable_associated_functions::explanatory_extension; - use crate::accountant::test_utils::{ - assert_account_creation_fn_fails_on_finding_wrong_columns_and_value_types, - make_pending_payable_fingerprint, - }; + use crate::accountant::test_utils::{assert_account_creation_fn_fails_on_finding_wrong_columns_and_value_types, make_pending_payable_fingerprint, trick_rusqlite_with_read_only_conn}; use crate::blockchain::test_utils::make_tx_hash; - use crate::database::connection_wrapper::ConnectionWrapperReal; - use crate::database::db_initializer::test_utils::ConnectionWrapperMock; + use crate::database::rusqlite_wrappers::ConnectionWrapperReal; use crate::database::db_initializer::{ DbInitializationConfig, DbInitializer, DbInitializerReal, DATABASE_FILE, }; @@ -554,9 +554,10 @@ mod tests { use masq_lib::messages::TopRecordsOrdering::{Age, Balance}; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; use rusqlite::{Connection, OpenFlags}; - use rusqlite::{Connection as RusqliteConnection, ToSql}; + use rusqlite::{ToSql}; use std::path::Path; use std::str::FromStr; + use crate::database::test_utils::ConnectionWrapperMock; #[test] fn more_money_payable_works_for_new_address() { @@ -661,10 +662,10 @@ mod tests { #[should_panic( expected = "Overflow detected with 340282366920938463463374607431768211455: cannot be converted from u128 to i128" )] - fn more_money_payable_works_for_overflow() { + fn more_money_payable_works_for_128_bits_value_overflow() { let home_dir = ensure_node_home_directory_exists( "payable_dao", - "more_money_payable_works_for_overflow", + "more_money_payable_works_for_128_bits_value_overflow", ); let wallet = make_wallet("booga"); let subject = PayableDaoReal::new( @@ -676,6 +677,26 @@ mod tests { let _ = subject.more_money_payable(SystemTime::now(), &wallet, u128::MAX); } + #[test] + fn more_money_payable_handles_error() { + let home_dir = + ensure_node_home_directory_exists("payable_dao", "more_money_payable_handles_error"); + let wallet = make_wallet("booga"); + let conn = payable_read_only_conn(&home_dir); + let wrapped_conn = ConnectionWrapperReal::new(conn); + let subject = PayableDaoReal::new(Box::new(wrapped_conn)); + + let result = subject.more_money_payable(SystemTime::now(), &wallet, 123456); + + assert_eq!( + result, + Err(PayableDaoError::RusqliteError("Error from invalid upsert command for payable table \ + and change of 123456 wei to 'wallet_address = 0x000000000000000000000000000000626f6f6761' \ + with error 'attempt to write a readonly database'".to_string()) + ) + ) + } + #[test] fn mark_pending_payables_marks_pending_transactions_for_new_addresses() { //the extra unchanged record checks the safety of right count of changed rows; @@ -841,7 +862,7 @@ mod tests { ); let wallet = make_wallet("booga"); let rowid = 656; - let conn = trick_rusqlite_with_read_only_conn(&home_dir); + let conn = payable_read_only_conn(&home_dir); let conn_wrapped = ConnectionWrapperReal::new(conn); let subject = PayableDaoReal::new(Box::new(conn_wrapped)); @@ -1037,7 +1058,7 @@ mod tests { "payable_dao", "transaction_confirmed_works_for_generic_sql_error", ); - let conn = trick_rusqlite_with_read_only_conn(&home_dir); + let conn = payable_read_only_conn(&home_dir); let conn_wrapped = Box::new(ConnectionWrapperReal::new(conn)); let mut pending_payable_fingerprint = make_pending_payable_fingerprint(); let hash = make_tx_hash(12345); @@ -1128,27 +1149,6 @@ mod tests { assert_eq!(account_2_opt, None); } - fn trick_rusqlite_with_read_only_conn(path: &Path) -> Connection { - let db_path = path.join("experiment.db"); - let conn = RusqliteConnection::open_with_flags(&db_path, OpenFlags::default()).unwrap(); - conn.prepare( - " - create table payable ( - wallet_address text primary key, - balance_high_b integer not null, - balance_low_b integer not null, - last_paid_timestamp integer not null, - pending_payable_rowid integer null)", - ) - .unwrap() - .execute([]) - .unwrap(); - conn.close().unwrap(); - let conn = RusqliteConnection::open_with_flags(&db_path, OpenFlags::SQLITE_OPEN_READ_ONLY) - .unwrap(); - conn - } - #[test] fn non_pending_payables_should_return_an_empty_vec_when_the_database_is_empty() { let home_dir = ensure_node_home_directory_exists( @@ -1664,6 +1664,10 @@ mod tests { assert_eq!(PayableDaoReal::table_name(), "payable") } + fn payable_read_only_conn(path: &Path) -> Connection { + trick_rusqlite_with_read_only_conn(path, DbInitializerReal::create_payable_table) + } + fn custom_query_test_body_for_payable(test_name: &str, main_setup_fn: F) -> PayableDaoReal where F: Fn(&dyn ConnectionWrapper, InsertPayableHelperFn), diff --git a/node/src/accountant/db_access_objects/pending_payable_dao.rs b/node/src/accountant/db_access_objects/pending_payable_dao.rs index a918c7175..767410bc0 100644 --- a/node/src/accountant/db_access_objects/pending_payable_dao.rs +++ b/node/src/accountant/db_access_objects/pending_payable_dao.rs @@ -6,7 +6,7 @@ use crate::accountant::db_access_objects::utils::{ use crate::accountant::db_big_integer::big_int_divider::BigIntDivider; use crate::accountant::{checked_conversion, comma_joined_stringifiable}; use crate::blockchain::blockchain_bridge::PendingPayableFingerprint; -use crate::database::connection_wrapper::ConnectionWrapper; +use crate::database::rusqlite_wrappers::ConnectionWrapper; use crate::sub_lib::wallet::Wallet; use itertools::Itertools; use masq_lib::utils::ExpectValue; @@ -264,11 +264,11 @@ mod tests { use crate::accountant::db_big_integer::big_int_divider::BigIntDivider; use crate::blockchain::blockchain_bridge::PendingPayableFingerprint; use crate::blockchain::test_utils::make_tx_hash; - use crate::database::connection_wrapper::ConnectionWrapperReal; - use crate::database::db_initializer::test_utils::ConnectionWrapperMock; use crate::database::db_initializer::{ DbInitializationConfig, DbInitializer, DbInitializerReal, DATABASE_FILE, }; + use crate::database::rusqlite_wrappers::ConnectionWrapperReal; + use crate::database::test_utils::ConnectionWrapperMock; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; use rusqlite::{Connection, OpenFlags}; use std::str::FromStr; diff --git a/node/src/accountant/db_access_objects/receivable_dao.rs b/node/src/accountant/db_access_objects/receivable_dao.rs index 14fe4b4e9..9b71a3939 100644 --- a/node/src/accountant/db_access_objects/receivable_dao.rs +++ b/node/src/accountant/db_access_objects/receivable_dao.rs @@ -7,25 +7,24 @@ use crate::accountant::db_access_objects::utils::{ sum_i128_values_from_table, to_time_t, AssemblerFeeder, CustomQuery, DaoFactoryReal, RangeStmConfig, ThresholdUtils, TopStmConfig, VigilantRusqliteFlatten, }; -use crate::accountant::db_big_integer::big_int_db_processor::KnownKeyVariants::WalletAddress; -use crate::accountant::db_big_integer::big_int_db_processor::WeiChange::{Addition, Subtraction}; +use crate::accountant::db_big_integer::big_int_db_processor::KeyVariants::WalletAddress; use crate::accountant::db_big_integer::big_int_db_processor::{ - BigIntDbProcessor, BigIntSqlConfig, Param, SQLParamsBuilder, TableNameDAO, + BigIntDatabaseError, BigIntDbProcessor, BigIntDbProcessorReal, BigIntSqlConfig, + DisplayableRusqliteParamPair, ParamByUse, SQLParamsBuilder, TableNameDAO, WeiChange, + WeiChangeDirection, }; use crate::accountant::db_big_integer::big_int_divider::BigIntDivider; use crate::accountant::gwei_to_wei; use crate::blockchain::blockchain_interface::data_structures::BlockchainTransaction; -use crate::database::connection_wrapper::ConnectionWrapper; use crate::database::db_initializer::{connection_or_panic, DbInitializerReal}; -use crate::db_config::persistent_configuration::PersistentConfigError; +use crate::database::rusqlite_wrappers::{ConnectionWrapper, TransactionSafeWrapper}; use crate::sub_lib::accountant::PaymentThresholds; use crate::sub_lib::wallet::Wallet; use indoc::indoc; use itertools::Either; -use itertools::Either::Left; use masq_lib::constants::WEIS_IN_GWEI; use masq_lib::logger::Logger; -use masq_lib::utils::{plus, ExpectValue}; +use masq_lib::utils::ExpectValue; use rusqlite::OptionalExtension; use rusqlite::Row; use rusqlite::{named_params, Error}; @@ -38,12 +37,6 @@ pub enum ReceivableDaoError { RusqliteError(String), } -impl From for ReceivableDaoError { - fn from(input: PersistentConfigError) -> Self { - ReceivableDaoError::ConfigurationError(format!("{:?}", input)) - } -} - impl From for ReceivableDaoError { fn from(input: Error) -> Self { RusqliteError(format!("{:?}", input)) @@ -57,7 +50,7 @@ pub struct ReceivableAccount { pub last_received_timestamp: SystemTime, } -pub trait ReceivableDao: Send { +pub trait ReceivableDao { fn more_money_receivable( &self, now: SystemTime, @@ -65,7 +58,11 @@ pub trait ReceivableDao: Send { amount: u128, ) -> Result<(), ReceivableDaoError>; - fn more_money_received(&mut self, now: SystemTime, transactions: Vec); + fn more_money_received( + &mut self, + now: SystemTime, + transactions: &[BlockchainTransaction], + ) -> TransactionSafeWrapper; fn new_delinquencies( &self, @@ -79,10 +76,10 @@ pub trait ReceivableDao: Send { fn total(&self) -> i128; - //test-only-like method but because of share with multi-node tests #[cfg(test)] is disallowed + // Test-only method but because of shares with multi-node tests #[cfg(test)] cannot be applied fn account_status(&self, wallet: &Wallet) -> Option; - as_any_in_trait!(); + as_any_ref_in_trait!(); } pub trait ReceivableDaoFactory { @@ -106,7 +103,7 @@ impl ReceivableDaoFactory for DaoFactoryReal { #[derive(Debug)] pub struct ReceivableDaoReal { conn: Box, - big_int_db_processor: BigIntDbProcessor, + big_int_db_processor: Box>, logger: Logger, } @@ -118,31 +115,76 @@ impl ReceivableDao for ReceivableDaoReal { amount: u128, ) -> Result<(), ReceivableDaoError> { let main_sql = "insert into receivable (wallet_address, balance_high_b, balance_low_b, last_received_timestamp) values \ - (:wallet, :balance_high_b, :balance_low_b, :last_received) on conflict (wallet_address) do update set \ + (:wallet, :balance_high_b, :balance_low_b, :last_received_timestamp) on conflict (wallet_address) do update set \ balance_high_b = balance_high_b + :balance_high_b, balance_low_b = balance_low_b + :balance_low_b"; - let overflow_update_clause = "update receivable set balance_high_b = :balance_high_b, balance_low_b = :balance_low_b \ + let update_clause_with_compensated_overflow = "update receivable set balance_high_b = :balance_high_b, balance_low_b = :balance_low_b \ where wallet_address = :wallet"; - Ok(self.big_int_db_processor.execute( - Left(self.conn.as_ref()), - BigIntSqlConfig::new( - main_sql, - overflow_update_clause, - SQLParamsBuilder::default() - .key(WalletAddress(wallet)) - .wei_change(Addition("balance", amount)) - .other_params(vec![Param::new( - (":last_received", &to_time_t(timestamp)), - false, - )]) - .build(), - ), - )?) + let last_received_timestamp = to_time_t(timestamp); + let params = SQLParamsBuilder::default() + .key(WalletAddress(wallet)) + .wei_change(WeiChange::new( + "balance", + amount, + WeiChangeDirection::Addition, + )) + .other_params(vec![ParamByUse::BeforeOverflowOnly( + DisplayableRusqliteParamPair::new( + ":last_received_timestamp", + &last_received_timestamp, + ), + )]) + .build(); + + self.big_int_db_processor.execute( + Either::Left(self.conn.as_ref()), + BigIntSqlConfig::new(main_sql, update_clause_with_compensated_overflow, params), + )?; + + Ok(()) } - fn more_money_received(&mut self, timestamp: SystemTime, payments: Vec) { - self.try_multi_insert_payment(timestamp, &payments) - .unwrap_or_else(|e| self.more_money_received_pretty_error_log(&payments, e)) + fn more_money_received( + &mut self, + timestamp: SystemTime, + received_payments: &[BlockchainTransaction], + ) -> TransactionSafeWrapper<'_> { + let accounting_result = match self.conn.transaction() { + Ok(txn) => { + let big_int_db_processor = &*self.big_int_db_processor; + let logger = &self.logger; + + Self::process_received_payments_and_return_txn( + big_int_db_processor, + received_payments, + timestamp, + txn, + logger, + ) + } + // Even though done with the accountancy, we still haven't updated the start block. + // We will proceed that as the last thing because of our concern about data continuity + // for cases if anything here or the whole Node crashes. + // Here we're taking over the uncommitted txn and will dispatch it to the place where + // it is more appropriate to finish the job. + // + // If things go wrong before the commit, everything from this txn will never make it + // into the database and once the Node is up again it can do a scan including the part + // we failed on last time, yet completely out of danger doubling any of those received + // transactions. + Err(e) => Err(ReceivableDaoError::from(e)), + }; + + match accounting_result { + Ok(txn) => txn, + Err(e) => { + Self::log_more_money_received_error_with_roll_back(&self.logger, received_payments); + panic!( + "Database corruption suspected during accounting newly received payments: {:?}", + e + ) + } + } } fn new_delinquencies( @@ -265,57 +307,98 @@ impl ReceivableDao for ReceivableDaoReal { } } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } impl ReceivableDaoReal { pub fn new(conn: Box) -> ReceivableDaoReal { ReceivableDaoReal { conn, - big_int_db_processor: BigIntDbProcessor::default(), + big_int_db_processor: Box::new(BigIntDbProcessorReal::default()), logger: Logger::new("ReceivableDaoReal"), } } - fn try_multi_insert_payment( - &mut self, + fn process_received_payments_and_return_txn<'txn>( + big_int_db_processor: &dyn BigIntDbProcessor, + received_payments: &[BlockchainTransaction], timestamp: SystemTime, - payments: &[BlockchainTransaction], - ) -> Result<(), ReceivableDaoError> { - let xactn = self.conn.transaction()?; - { - for transaction in payments { - // the plus signs are correct, 'Subtraction' in the wei_change converts x of u128 to -x of i128 which leads to an integer pair - // with the high bytes integer being negative - let main_sql = "update receivable set balance_high_b = balance_high_b + :balance_high_b, \ - balance_low_b = balance_low_b + :balance_low_b, last_received_timestamp = :last_received where wallet_address = :wallet"; - let overflow_update_clause = "update receivable set balance_high_b = :balance_high_b, balance_low_b = :balance_low_b, \ - last_received_timestamp = :last_received where wallet_address = :wallet"; - - self.big_int_db_processor.execute( - Either::Right(&xactn), - BigIntSqlConfig::new( - main_sql, - overflow_update_clause, - SQLParamsBuilder::default() - .key(WalletAddress(&transaction.from)) - .wei_change(Subtraction("balance", transaction.wei_amount)) - .other_params(vec![Param::new( - (":last_received", &to_time_t(timestamp)), - true, - )]) - .build(), - ), - )? + txn: TransactionSafeWrapper<'txn>, + logger: &Logger, + ) -> Result, ReceivableDaoError> { + // The plus signs are intended. 'Subtraction' provided by the '.wei_change()' causes x of u128 + // to become -x of i128 which produces a negative i64 integer in the column for the high bytes + let main_sql = "update receivable set balance_high_b = balance_high_b + :balance_high_b, \ + balance_low_b = balance_low_b + :balance_low_b, last_received_timestamp = :last_received \ + where wallet_address = :wallet"; + let update_clause_with_compensated_overflow = + "update receivable set balance_high_b = :balance_high_b, \ + balance_low_b = :balance_low_b, last_received_timestamp = :last_received \ + where wallet_address = :wallet"; + + match received_payments.iter().try_for_each(|received_payment| { + let last_received_timestamp = to_time_t(timestamp); + let params = SQLParamsBuilder::default() + .key(WalletAddress(&received_payment.from)) + .wei_change(WeiChange::new( + "balance", + received_payment.wei_amount, + WeiChangeDirection::Subtraction, + )) + .other_params(vec![ParamByUse::BeforeAndAfterOverflow( + DisplayableRusqliteParamPair::new(":last_received", &last_received_timestamp), + )]) + .build(); + + let result = big_int_db_processor.execute( + Either::Right(&txn), + BigIntSqlConfig::new(main_sql, update_clause_with_compensated_overflow, params), + ); + + match result { + Ok(_) => Ok(()), + Err(BigIntDatabaseError::General(err_msg)) => { + Err(ReceivableDaoError::RusqliteError(err_msg)) + } + Err(BigIntDatabaseError::RowChangeMismatch { .. }) => { + Self::verify_possibly_unknown_wallet(&txn, logger, received_payment) + } } + }) { + Ok(_) => Ok(txn), + Err(e) => Err(e), } - match xactn.commit() { - // Error response is untested here, because without a mockable Transaction, it's untestable. - Err(e) => Err(ReceivableDaoError::RusqliteError(format!("{:?}", e))), - Ok(_) => Ok(()), + } + + fn verify_possibly_unknown_wallet( + txn: &TransactionSafeWrapper, + logger: &Logger, + suspect: &BlockchainTransaction, + ) -> Result<(), ReceivableDaoError> { + let amount = suspect.wei_amount; + let address = &suspect.from; + let block_number = suspect.block_number; + + match Self::is_row_present(txn, address) { + false => { + info!(logger, + "Received {amount} wei from unknown debtor {address} in the block {block_number}."); + Ok(()) + } + true => Err(RusqliteError(format!( + "Update for received payment with {amount} wei ran without producing a change \ + in the database, despite the record for wallet {address} exists." + ))), } } + fn is_row_present(conn: &TransactionSafeWrapper, wallet: &Wallet) -> bool { + conn.prepare("select * from receivable where wallet_address = ?") + .expect("internal sqlite error") + .exists(&[&wallet]) + .expect("checking on a row failed") + } + fn create_receivable_account(row: &Row) -> rusqlite::Result { let wallet: Result = row.get(0); let balance_high_b_result = row.get(1); @@ -363,39 +446,39 @@ impl ReceivableDaoReal { ) } - fn more_money_received_pretty_error_log( - &self, + fn log_more_money_received_error_with_roll_back( + logger: &Logger, payments: &[BlockchainTransaction], - error: ReceivableDaoError, ) { - fn finalize_report(data: (Vec, u128)) -> String { - let (report_lines, sum) = data; - plus(report_lines, format!("{:10} {:42} {:18}", "TOTAL", "", sum)).join("\n") + fn finalize_report(mut report_lines: Vec, sum_of_all_txs: u128) -> String { + report_lines.push(format!("{:10} {:42} {:<18}", "TOTAL", "", sum_of_all_txs)); + report_lines.join("\n") } - fn record_one_more_transaction( - acc: (Vec, u128), - bc_tx: &BlockchainTransaction, + fn note_single_transaction( + (mut formatted_lines_so_far, sum_so_far): (Vec, u128), + blockchain_tx: &BlockchainTransaction, ) -> (Vec, u128) { - let lines_adjusted = plus( - acc.0, - format!( - "{:10} {:42} {:18}", - bc_tx.block_number, bc_tx.from, bc_tx.wei_amount - ), - ); - let sum_so_far = acc.1 + bc_tx.wei_amount; - (lines_adjusted, sum_so_far) + let lines_extended_by_new_one = { + let new_line = format!( + "{:<10} {:42} {:<18}", + blockchain_tx.block_number, blockchain_tx.from, blockchain_tx.wei_amount + ); + formatted_lines_so_far.push(new_line); + formatted_lines_so_far + }; + let money_total_so_far = sum_so_far + blockchain_tx.wei_amount; + (lines_extended_by_new_one, money_total_so_far) } + let init = ( vec![format!("{:10} {:42} {:18}", "Block #", "Wallet", "Amount")], 0_u128, ); - let aggregated = payments.iter().fold(init, record_one_more_transaction); + let (report_lines, received_in_total) = payments.iter().fold(init, note_single_transaction); error!( - self.logger, - "Payment reception failed, rolling back: {:?}\n{}", - error, - finalize_report(aggregated) + logger, + "Payment reception failed, rolling back:\n{}", + finalize_report(report_lines, received_in_total) ); } } @@ -415,32 +498,27 @@ mod tests { use crate::accountant::gwei_to_wei; use crate::accountant::test_utils::{ assert_account_creation_fn_fails_on_finding_wrong_columns_and_value_types, - make_receivable_account, + make_receivable_account, trick_rusqlite_with_read_only_conn, }; - use crate::database::db_initializer::{ - DbInitializationConfig, DbInitializer, DbInitializerReal, ExternalData, + use crate::database::db_initializer::{DbInitializationConfig, DbInitializer, DATABASE_FILE}; + use crate::database::db_initializer::{DbInitializerReal, ExternalData}; + use crate::database::rusqlite_wrappers::ConnectionWrapperReal; + use crate::database::test_utils::transaction_wrapper_mock::{ + AlteredStmtBySQLOrigin, PrepareMethodResultsProducer, StmtTypeDirective, + TransactionInnerWrapperMockBuilder, }; - use crate::db_config::persistent_configuration::PersistentConfigError; + use crate::database::test_utils::ConnectionWrapperMock; use crate::test_utils::assert_contains; use crate::test_utils::make_wallet; use masq_lib::messages::TopRecordsOrdering::{Age, Balance}; use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler}; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; use masq_lib::utils::NeighborhoodModeLight; - use rusqlite::ToSql; + use rusqlite::{ffi, Connection, ErrorCode, OpenFlags, ToSql}; + use std::panic::{catch_unwind, AssertUnwindSafe}; use std::path::Path; - - #[test] - fn conversion_from_pce_works() { - let pce = PersistentConfigError::BadHexFormat("booga".to_string()); - - let subject = ReceivableDaoError::from(pce); - - assert_eq!( - subject, - ReceivableDaoError::ConfigurationError("BadHexFormat(\"booga\")".to_string()) - ); - } + use std::sync::{Arc, Mutex}; + use std::time::{Duration, UNIX_EPOCH}; #[test] fn factory_produces_connection_that_is_familiar_with_our_defined_sqlite_functions() { @@ -481,10 +559,10 @@ mod tests { #[should_panic( expected = "Overflow detected with 340282366920938463463374607431768211455: cannot be converted from u128 to i128" )] - fn try_multi_insert_payment_handles_error_of_number_sign_check() { + fn more_money_received_handles_error_of_number_sign_check() { let home_dir = ensure_node_home_directory_exists( "receivable_dao", - "try_multi_insert_payment_handles_error_of_number_sign_check", + "more_money_received_handles_error_of_number_sign_check", ); let mut subject = ReceivableDaoReal::new( DbInitializerReal::default() @@ -497,15 +575,15 @@ mod tests { wei_amount: u128::MAX, }]; - let _ = subject.try_multi_insert_payment(SystemTime::now(), &payments.as_slice()); + let _ = subject.more_money_received(SystemTime::now(), &payments); } #[test] #[should_panic(expected = "no such table: receivable")] - fn try_multi_insert_payment_handles_error_adding_receivables() { + fn more_money_received_handles_error_adding_receivables() { let home_dir = ensure_node_home_directory_exists( "receivable_dao", - "try_multi_insert_payment_handles_error_adding_receivables", + "more_money_received_handles_error_adding_receivables", ); let conn = DbInitializerReal::default() .initialize(&home_dir, DbInitializationConfig::test_default()) @@ -522,7 +600,7 @@ mod tests { wei_amount: 18446744073709551615, }]; - let _ = subject.try_multi_insert_payment(SystemTime::now(), payments.as_slice()); + let _ = subject.more_money_received(SystemTime::now(), &payments); } #[test] @@ -626,10 +704,10 @@ mod tests { #[should_panic( expected = "Overflow detected with 340282366920938463463374607431768211455: cannot be converted from u128 to i128" )] - fn more_money_receivable_works_for_overflow() { + fn more_money_receivable_works_for_128_bits_overflow() { let home_dir = ensure_node_home_directory_exists( "receivable_dao", - "more_money_receivable_works_for_overflow", + "more_money_receivable_works_for_128_bits_overflow", ); let subject = ReceivableDaoReal::new( DbInitializerReal::default() @@ -640,6 +718,28 @@ mod tests { let _ = subject.more_money_receivable(SystemTime::now(), &make_wallet("booga"), u128::MAX); } + #[test] + fn more_money_receivable_handles_error() { + let home_dir = ensure_node_home_directory_exists( + "receivable_dao", + "more_money_receivable_handles_error", + ); + let wallet = make_wallet("blah"); + let conn = receivable_read_only_conn(&home_dir); + let wrapped_conn = ConnectionWrapperReal::new(conn); + let subject = ReceivableDaoReal::new(Box::new(wrapped_conn)); + + let result = subject.more_money_receivable(SystemTime::now(), &wallet, 123456); + + assert_eq!( + result, + Err(ReceivableDaoError::RusqliteError("Error from invalid upsert command for receivable table \ + and change of 123456 wei to 'wallet_address = 0x00000000000000000000000000000000626c6168' \ + with error 'attempt to write a readonly database'".to_string()) + ) + ) + } + #[test] fn more_money_received_works_for_existing_addresses_without_overflow() { //asserting on the correctness of the main sql @@ -705,8 +805,9 @@ mod tests { }, ]; - subject.more_money_received(payment_time, transactions); + let txn = subject.more_money_received(payment_time, &transactions); + txn.commit().unwrap(); let status1 = subject.account_status(&debtor1).unwrap(); assert_eq!(status1.wallet, debtor1); assert_eq!(status1.balance_wei, first_expected_result); @@ -724,92 +825,349 @@ mod tests { } #[test] - fn more_money_received_throws_away_payments_from_unknown_addresses() { - let home_dir = ensure_node_home_directory_exists( - "receivable_dao", - "more_money_received_throws_away_payments_from_unknown_addresses", - ); - let debtor = make_wallet("unknown_wallet"); + fn more_money_received_ignores_unknown_address_without_affecting_the_good_ones() { + init_test_logging(); + let test_name = + "more_money_received_ignores_unknown_address_without_affecting_the_good_ones"; + let home_dir = ensure_node_home_directory_exists("receivable_dao", test_name); + let previous_timestamp = UNIX_EPOCH; + let time_of_change = SystemTime::now() + .checked_sub(Duration::from_secs(1111)) + .unwrap(); + let first_tracked_wallet = make_wallet("abc"); + let first_initial_balance = 2345; + let unknown_wallet = make_wallet("def"); + let second_tracked_wallet = make_wallet("ghi"); + let second_initial_balance = 8901; + let logger = Logger::new(test_name); let mut subject = ReceivableDaoReal::new( DbInitializerReal::default() .initialize(&home_dir, DbInitializationConfig::test_default()) .unwrap(), ); - let transactions = vec![BlockchainTransaction { - from: debtor.clone(), - wei_amount: 2300_u128, - block_number: 33_u64, - }]; + subject + .more_money_receivable( + previous_timestamp, + &first_tracked_wallet, + first_initial_balance, + ) + .unwrap(); + subject + .more_money_receivable( + previous_timestamp, + &second_tracked_wallet, + second_initial_balance, + ) + .unwrap(); + subject.logger = logger; + let transaction_1 = BlockchainTransaction { + block_number: 4444, + from: first_tracked_wallet.clone(), + wei_amount: 1111, + }; + let transaction_2 = BlockchainTransaction { + block_number: 4446, + from: unknown_wallet.clone(), + wei_amount: 2222, + }; + let transaction_3 = BlockchainTransaction { + block_number: 4450, + from: second_tracked_wallet.clone(), + wei_amount: 9999, + }; + let transactions = vec![transaction_1, transaction_2, transaction_3]; - subject.more_money_received(SystemTime::now(), transactions); + let txn = subject.more_money_received(time_of_change, transactions.as_slice()); - let status = subject.account_status(&debtor); - assert!(status.is_none()); + txn.commit().unwrap(); + let actual_record_1 = subject.account_status(&first_tracked_wallet).unwrap(); + assert_eq!(actual_record_1.wallet, first_tracked_wallet); + assert_eq!( + actual_record_1.balance_wei, + first_initial_balance as i128 - 1111 + ); + assert_eq!( + to_time_t(actual_record_1.last_received_timestamp), + to_time_t(time_of_change) + ); + let actual_record_2 = subject.account_status(&unknown_wallet); + assert!(actual_record_2.is_none()); + let actual_record_3 = subject.account_status(&second_tracked_wallet).unwrap(); + assert_eq!(actual_record_3.wallet, second_tracked_wallet); + assert_eq!( + actual_record_3.balance_wei, + second_initial_balance as i128 - 9999 + ); + assert_eq!( + to_time_t(actual_record_3.last_received_timestamp), + to_time_t(time_of_change) + ); + let log_handler = TestLogHandler::new(); + log_handler.exists_log_containing(&format!( + "INFO: {test_name}: Received 2222 wei from unknown debtor {unknown_wallet} \ + in the block 4446." + )); + log_handler.exists_no_log_containing(&format!("ERROR: {test_name}: ")); } #[test] - fn more_money_received_logs_when_try_multi_insert_payment_fails() { + fn more_money_received_general_db_error() { init_test_logging(); - let home_dir = ensure_node_home_directory_exists( - "receivable_dao", - "more_money_received_logs_when_try_multi_insert_payment_fails", - ); - let mut subject = ReceivableDaoReal::new( - DbInitializerReal::default() - .initialize(&home_dir, DbInitializationConfig::test_default()) - .unwrap(), - ); - // Sabotage the database so there'll be an error + let test_name = "more_money_received_general_db_error"; + let data_dir = ensure_node_home_directory_exists("receivable_dao", test_name); + let time_of_change = SystemTime::now() + .checked_sub(Duration::from_secs(1111)) + .unwrap(); + let wallet = make_wallet("abc"); { - let mut conn = DbInitializerReal::default() - .initialize(&home_dir, DbInitializationConfig::panic_on_migration()) - .unwrap(); - let xactn = conn.transaction().unwrap(); - xactn - .prepare("drop table receivable") - .unwrap() - .execute([]) + DbInitializerReal::default() + .initialize(&data_dir, DbInitializationConfig::test_default()) .unwrap(); - xactn.commit().unwrap(); - } - let payments = vec![ - BlockchainTransaction { - block_number: 1234567890, - from: Wallet::new("0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"), - wei_amount: 123456789123456789, - }, - BlockchainTransaction { - block_number: 2345678901, - from: Wallet::new("0xBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"), - wei_amount: 234567891234567891, - }, - BlockchainTransaction { - block_number: 3456789012, - from: Wallet::new("0xCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC"), - wei_amount: 345678912345678912, - }, - ]; + }; + let conn = Connection::open_with_flags( + data_dir.join(DATABASE_FILE), + OpenFlags::SQLITE_OPEN_READ_ONLY, + ) + .unwrap(); + let conn = ConnectionWrapperReal::new(conn); + let mut subject = ReceivableDaoReal::new(Box::new(conn)); + let transaction = BlockchainTransaction { + block_number: 123_456, + from: wallet, + wei_amount: 45_678, + }; + let transactions = vec![transaction]; + + let caught_err = catch_unwind(AssertUnwindSafe(|| { + let _ = subject.more_money_received(time_of_change, transactions.as_slice()); + })) + .unwrap_err(); + + let panic_msg = caught_err.downcast_ref::().unwrap(); + let expected_panic_fragment = format!( + "Error from invalid update command for receivable table and change of -45678 wei to \ + 'wallet_address = 0x0000000000000000000000000000000000616263' with error 'attempt to \ + write a readonly database'" + ); + assert!( + panic_msg.contains(&expected_panic_fragment), + "Actual panic msg: {} does not contain this fragment {}", + panic_msg, + expected_panic_fragment + ); + // The background thread is used just for this log assertion + TestLogHandler::new().exists_no_log_containing(&format!("INFO: {test_name}: ")); + } - subject.more_money_received(SystemTime::now(), payments); + #[test] + #[should_panic( + expected = "Database corruption suspected during accounting newly received payments: \ + RusqliteError(\"SqliteFailure(Error { code: InternalMalfunction, extended_code: 0 }, \ + Some(\\\"blah\\\"))\")" + )] + fn more_money_received_hits_error_from_transaction_initialization() { + let mut subject = { + let conn = ConnectionWrapperMock::default().transaction_result(Err( + rusqlite::Error::SqliteFailure( + ffi::Error { + code: ErrorCode::InternalMalfunction, + extended_code: 0, + }, + Some("blah".to_string()), + ), + )); + ReceivableDaoReal::new(Box::new(conn)) + }; + let transaction = BlockchainTransaction { + block_number: 123_456, + from: make_wallet("abc"), + wei_amount: 1, + }; + let transactions = vec![transaction]; - TestLogHandler::new().exists_log_containing(&format!( - "ERROR: ReceivableDaoReal: Payment reception failed, rolling back: RusqliteError(\ - \"Error from invalid update command for receivable table and change of -123456789123456789 \ - wei to 'wallet_address = 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' with error 'no such table: receivable'\")\ - \n\ + subject.more_money_received(SystemTime::now(), &transactions); + } + + #[test] + fn more_money_received_leaves_transactions_uncommitted_if_panic_occurs() { + init_test_logging(); + let test_name = "more_money_received_leaves_transactions_uncommitted_if_panic_occurs"; + let data_dir = ensure_node_home_directory_exists("receivable_dao", test_name); + let prepare_params_arc = Arc::new(Mutex::new(vec![])); + let time_of_change = SystemTime::now() + .checked_sub(Duration::from_secs(1111)) + .unwrap(); + let first_wallet = make_wallet("abc"); + let first_initial_balance = 123_456; + let first_previous_timestamp = UNIX_EPOCH; + let second_wallet = make_wallet("def"); + let receivable_dao = { + let conn = DbInitializerReal::default() + .initialize(&data_dir, DbInitializationConfig::test_default()) + .unwrap(); + let dao = ReceivableDaoReal::new(conn); + dao.more_money_receivable( + first_previous_timestamp, + &first_wallet, + first_initial_balance, + ) + .unwrap(); + dao + }; + let prod_code_calls_conn = DbInitializerReal::default() + .initialize(&data_dir, DbInitializationConfig::test_default()) + .unwrap(); + let panic_causing_conn = { + let db_path = data_dir.join(DATABASE_FILE); + let flags = OpenFlags::SQLITE_OPEN_READ_ONLY; + let conn = Connection::open_with_flags(&db_path, flags).unwrap(); + Box::new(ConnectionWrapperReal::new(conn)) + }; + let prepare_results = + PrepareMethodResultsProducer::construct_with_prod_code_and_altered_stmts( + prod_code_calls_conn, + Some(panic_causing_conn), + vec![ + StmtTypeDirective::ExecuteProdCode, + StmtTypeDirective::ExecuteProdCode, + StmtTypeDirective::ExecuteProdCode, + StmtTypeDirective::UseAlteredStmt( + AlteredStmtBySQLOrigin::SQLIdenticalWithProdCode, + ), + ], + ); + let txn_inner_builder = TransactionInnerWrapperMockBuilder::default() + .prepare_params(&prepare_params_arc) + .prepare_results(prepare_results); + let mocked_transaction = TransactionSafeWrapper::new_with_builder(txn_inner_builder); + let mocked_conn = + Box::new(ConnectionWrapperMock::default().transaction_result(Ok(mocked_transaction))); + let mut subject = ReceivableDaoReal::new(mocked_conn); + subject.logger = Logger::new(test_name); + let first_transaction = BlockchainTransaction { + block_number: 123_456, + from: first_wallet.clone(), + wei_amount: 45_678, + }; + let second_transaction = BlockchainTransaction { + block_number: 789_123, + from: second_wallet, + wei_amount: 111_222, + }; + let transactions = vec![first_transaction, second_transaction]; + + let result = catch_unwind(AssertUnwindSafe(|| { + let _ = subject.more_money_received(time_of_change, &transactions); + })); + + let caught_panic = result.unwrap_err(); + let panic_msg = caught_panic.downcast_ref::().unwrap(); + let expected_panic_msg = + "Database corruption suspected during accounting newly received payments: \ + RusqliteError(\"Error from invalid update command for receivable table and change of \ + -111222 wei to 'wallet_address = 0x0000000000000000000000000000000000646566' with error \ + 'attempt to write a readonly database'\")"; + assert_eq!(panic_msg, &expected_panic_msg); + let prepare_params = prepare_params_arc.lock().unwrap(); + // Asserting that we did perform the first transaction completely which is a process + // composed of three SQL statements if it includes handling an overflow like here + assert_eq!(&prepare_params[0..3], + &[ + "update receivable set balance_high_b = balance_high_b + :balance_high_b, balance_low_b \ + = balance_low_b + :balance_low_b, last_received_timestamp = :last_received where wallet_address \ + = :wallet", + "select balance_high_b, balance_low_b from receivable where wallet_address = \ + '0x0000000000000000000000000000000000616263'", + "update receivable set balance_high_b = :balance_high_b, balance_low_b = :balance_low_b, \ + last_received_timestamp = :last_received where wallet_address = :wallet" + ]); + // The first transaction did not affect the db, was rolled back + let account_status = receivable_dao.account_status(&first_wallet); + assert_eq!( + account_status, + Some(ReceivableAccount { + wallet: first_wallet, + balance_wei: first_initial_balance as i128, + last_received_timestamp: first_previous_timestamp, + }) + ); + let log_handler = TestLogHandler::new(); + log_handler.exists_no_log_containing(&format!("INFO: {test_name}: ")); + log_handler.exists_log_containing(&format!( + "ERROR: {test_name}: Payment reception failed, rolling back:\n\ Block # Wallet Amount \n\ - 1234567890 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa 123456789123456789\n\ - 2345678901 0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb 234567891234567891\n\ - 3456789012 0xcccccccccccccccccccccccccccccccccccccccc 345678912345678912\n\ - TOTAL 703703592703703592" + 123456 0x0000000000000000000000000000000000616263 45678 \n\ + 789123 0x0000000000000000000000000000000000646566 111222 \n\ + TOTAL 156900" )); + // The test framework of TransactionWrapperInnerMock with its PrepareResultsDispatcher is + // one of the smarter tools you can meet. + // + // It allows to stimulate a certain error to be thrown out at the execution of + // this given Statement (with all the supplied parameters if any) that we make sure will be + // made inside the test tool at the right time and pushed out into the ongoing + // operation requiring it by the prod code, but given the conditions, causing an error right + // in the next moment. This way, the reaction is medium by which we can control the course + // of the code flow. + // + // As far as that it hasn't been a big deal. There is an extra concern though. If and how + // we can follow the real life behaviour given every transaction needs to be eventually + // committed by a function call in our code. + // + // Why is this important? You should know that the mocked wrapper has its foundation in + // its own transaction it carries around with it and that is supposed to follow how we work + // with the standard one: it can be used for any number of operations but the final step, + // the commit, must come in place, otherwise all those operations, even if succeeding + // on its own, will be discarded and have no impact on the database. We need to stay true to + // this because there might be tests, like this one, which are interested in the final state, + // looking into the database and wanting to see no changes there, because that is exactly + // what we would've seen with the pure version of the code. + // + // Another proof of the correct course of events in here is that even though we hadn't + // given any prepared result for the 'commit()' method, the test did not panic demanding + // a result to be used by the mock. Besides that, another way of looking at the evidence + // given by this test is that if the code had been supposed to commit the transactions + // separately, while the first one is proper and therefore it would've allowed that, we + // would necessarily have come across this call of 'commit()', crushing, yet we didn't. } #[test] - fn receivable_account_status_works_when_account_doesnt_exist() { + fn verify_possibly_unknown_wallet_returns_error_when_the_row_for_this_wallet_exists() { + let test_name = + "verify_possibly_unknown_wallet_returns_error_when_the_row_for_this_wallet_exists"; + let home_dir = ensure_node_home_directory_exists("receivable", test_name); + let mut conn = DbInitializerReal::default() + .initialize(&home_dir, DbInitializationConfig::test_default()) + .unwrap(); + let wallet = make_wallet("blah"); + conn.prepare( + "insert into receivable ( wallet_address, balance_high_b, balance_low_b, \ + last_received_timestamp ) values ( ?, 111, 222, 111222333 )", + ) + .unwrap() + .execute(&[&wallet]) + .unwrap(); + let suspect = BlockchainTransaction { + block_number: 1234, + from: wallet, + wei_amount: 1_000_000_000, + }; + let txn = conn.transaction().unwrap(); + let logger = Logger::new(test_name); + + let result = ReceivableDaoReal::verify_possibly_unknown_wallet(&txn, &logger, &suspect); + + let expected_panic_msg = "Update for received payment with 1000000000 wei ran \ + without producing a change in the database, despite the record for wallet \ + 0x00000000000000000000000000000000626c6168 exists."; + assert_eq!(result, Err(RusqliteError(expected_panic_msg.to_string()))) + } + + #[test] + fn receivable_account_status_works_when_account_does_not_exist() { let home_dir = ensure_node_home_directory_exists( "receivable_dao", - "receivable_account_status_works_when_account_doesnt_exist", + "receivable_account_status_works_when_account_does_not_exist", ); let wallet = make_wallet("booga"); let subject = ReceivableDaoReal::new( @@ -1524,6 +1882,10 @@ mod tests { stmt.execute(&[&account.wallet]).unwrap(); } + fn receivable_read_only_conn(path: &Path) -> Connection { + trick_rusqlite_with_read_only_conn(path, DbInitializerReal::create_receivable_table) + } + fn custom_query_test_body_for_receivable( test_name: &str, main_test_setup: F, diff --git a/node/src/accountant/db_access_objects/utils.rs b/node/src/accountant/db_access_objects/utils.rs index 85e5adade..c0063acb7 100644 --- a/node/src/accountant/db_access_objects/utils.rs +++ b/node/src/accountant/db_access_objects/utils.rs @@ -4,10 +4,10 @@ use crate::accountant::db_access_objects::payable_dao::PayableAccount; use crate::accountant::db_access_objects::receivable_dao::ReceivableAccount; use crate::accountant::db_big_integer::big_int_divider::BigIntDivider; use crate::accountant::{checked_conversion, gwei_to_wei, sign_conversion}; -use crate::database::connection_wrapper::ConnectionWrapper; use crate::database::db_initializer::{ connection_or_panic, DbInitializationConfig, DbInitializerReal, }; +use crate::database::rusqlite_wrappers::ConnectionWrapper; use crate::sub_lib::accountant::PaymentThresholds; use masq_lib::constants::WEIS_IN_GWEI; use masq_lib::messages::{ @@ -434,7 +434,7 @@ impl ThresholdUtils { #[cfg(test)] mod tests { use super::*; - use crate::database::connection_wrapper::ConnectionWrapperReal; + use crate::database::rusqlite_wrappers::ConnectionWrapperReal; use crate::sub_lib::accountant::DEFAULT_PAYMENT_THRESHOLDS; use crate::test_utils::make_wallet; use itertools::Itertools; diff --git a/node/src/accountant/db_big_integer/big_int_db_processor.rs b/node/src/accountant/db_big_integer/big_int_db_processor.rs index 474b6ebd2..3ef15278d 100644 --- a/node/src/accountant/db_big_integer/big_int_db_processor.rs +++ b/node/src/accountant/db_big_integer/big_int_db_processor.rs @@ -1,48 +1,60 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. use crate::accountant::checked_conversion; -use crate::accountant::db_access_objects::payable_dao::PayableDaoError; use crate::accountant::db_access_objects::receivable_dao::ReceivableDaoError; use crate::accountant::db_big_integer::big_int_divider::BigIntDivider; -use crate::database::connection_wrapper::ConnectionWrapper; +use crate::accountant::PayableDaoError; +use crate::database::rusqlite_wrappers::{ConnectionWrapper, TransactionSafeWrapper}; use crate::sub_lib::wallet::Wallet; use itertools::Either; -use rusqlite::{Error, Row, Statement, ToSql, Transaction}; +use rusqlite::{Error, Row, Statement, ToSql}; use std::fmt::{Debug, Display, Formatter}; use std::iter::once; use std::marker::PhantomData; use std::ops::Neg; +pub trait BigIntDbProcessor: Debug + Send +where + T: TableNameDAO, +{ + fn execute<'params>( + &self, + conn: Either<&dyn ConnectionWrapper, &TransactionSafeWrapper>, + config: BigIntSqlConfig<'params, T>, + ) -> Result<(), BigIntDatabaseError>; +} + #[derive(Debug)] -pub struct BigIntDbProcessor { +pub struct BigIntDbProcessorReal { overflow_handler: Box>, } -impl<'a, T: TableNameDAO> BigIntDbProcessor { - pub fn execute( +impl BigIntDbProcessor for BigIntDbProcessorReal +where + T: TableNameDAO, +{ + fn execute<'params>( &self, - conn: Either<&dyn ConnectionWrapper, &Transaction>, - config: BigIntSqlConfig<'a, T>, - ) -> Result<(), BigIntDbError> { + conn: Either<&dyn ConnectionWrapper, &TransactionSafeWrapper>, + config: BigIntSqlConfig<'params, T>, + ) -> Result<(), BigIntDatabaseError> { let main_sql = config.main_sql; - let mut stm = Self::prepare_statement(conn, main_sql); - let params = config - .params - .merge_other_and_wei_params((&config.params.wei_change_params).into()); - match stm.execute(params.as_slice()) { + let stm = Self::prepare_statement(conn, main_sql); + let params = config.params.non_overflow_params(); + match Self::execute_statement(stm, params) { Ok(1) => Ok(()), - Ok(x) => Err(BigIntDbError(format!("Expected 1 row to be changed for the unique key {} but got this count: {}", config.key_param_value(), x))), + Ok(detected_count_changed) => Err(BigIntDatabaseError::RowChangeMismatch{row_key: config.key_param_value().to_string(),detected_count_changed}), //SQLITE_CONSTRAINT_DATATYPE (3091), //the moment of Sqlite trying to store the number as REAL in a strict INT column Err(Error::SqliteFailure(e, _)) if e.extended_code == 3091 => { self.overflow_handler.update_with_overflow(conn, config) } - Err(e) => Err(BigIntDbError(format!( + Err(e) => Err(BigIntDatabaseError::General(format!( "Error from invalid {} command for {} table and change of {} wei to '{} = {}' with error '{}'", config.determine_command(), T::table_name(), config.balance_change(), - config.params.table_unique_key_name, + config.params.table_unique_key, config.key_param_value(), e ))), @@ -50,33 +62,47 @@ impl<'a, T: TableNameDAO> BigIntDbProcessor { } } -impl Default for BigIntDbProcessor { - fn default() -> BigIntDbProcessor { +impl Default for BigIntDbProcessorReal { + fn default() -> BigIntDbProcessorReal { Self { overflow_handler: Box::new(UpdateOverflowHandlerReal::default()), } } } -impl BigIntDbProcessor { - fn prepare_statement<'a>( - form_of_conn: Either<&'a dyn ConnectionWrapper, &'a Transaction>, - sql: &'a str, - ) -> Statement<'a> { +impl BigIntDbProcessorReal { + fn prepare_statement<'params>( + form_of_conn: Either<&'params dyn ConnectionWrapper, &'params TransactionSafeWrapper>, + sql: &'params str, + ) -> Statement<'params> { match form_of_conn { Either::Left(conn) => conn.prepare(sql), Either::Right(tx) => tx.prepare(sql), } .expect("internal rusqlite error") } + + fn execute_statement( + mut statement: Statement, + params: Vec, + ) -> rusqlite::Result { + let params_in_pure_rusqlite_format: Vec<(&str, &dyn ToSql)> = params + .into_iter() + .map(|param| (param.sql_subst_name, param.value)) + .collect(); + statement.execute(params_in_pure_rusqlite_format.as_slice()) + } } -pub trait UpdateOverflowHandler: Debug + Send { - fn update_with_overflow<'a>( +pub trait UpdateOverflowHandler: Debug + Send +where + T: TableNameDAO, +{ + fn update_with_overflow<'params>( &self, - conn: Either<&dyn ConnectionWrapper, &Transaction>, - config: BigIntSqlConfig<'a, T>, - ) -> Result<(), BigIntDbError>; + conn: Either<&dyn ConnectionWrapper, &TransactionSafeWrapper>, + config: BigIntSqlConfig<'params, T>, + ) -> Result<(), BigIntDatabaseError>; } #[derive(Debug)] @@ -93,11 +119,11 @@ impl Default for UpdateOverflowHandlerReal { } impl UpdateOverflowHandler for UpdateOverflowHandlerReal { - fn update_with_overflow<'a>( + fn update_with_overflow<'params>( &self, - conn: Either<&dyn ConnectionWrapper, &Transaction>, - config: BigIntSqlConfig<'a, T>, - ) -> Result<(), BigIntDbError> { + conn: Either<&dyn ConnectionWrapper, &TransactionSafeWrapper>, + config: BigIntSqlConfig<'params, T>, + ) -> Result<(), BigIntDatabaseError> { let update_divided_integer = |row: &Row| -> Result<(), rusqlite::Error> { let high_bytes_result = row.get::(0); let low_bytes_result = row.get::(1); @@ -110,34 +136,35 @@ impl UpdateOverflowHandler for UpdateOverflowHandlerReal former_low_bytes, requested_wei_change, ); - let wei_update_array: [(&str, &dyn ToSql); 2] = [ - ( - requested_wei_change.high.name.as_str(), + let wei_update_array: [RusqliteParamPairAsStruct; 2] = [ + RusqliteParamPairAsStruct::new( + requested_wei_change.high_bytes.name.as_str(), &high_bytes_corrected, ), - (requested_wei_change.low.name.as_str(), &low_bytes_corrected), + RusqliteParamPairAsStruct::new( + requested_wei_change.low_bytes.name.as_str(), + &low_bytes_corrected, + ), ]; - let execute_params = config - .params - .merge_other_and_wei_params_with_conditional_participants(wei_update_array); + let execute_params = config.params.overflow_params(wei_update_array); - Self::execute_update(conn, &config, &execute_params); + Self::execute_update(conn, &config, execute_params); Ok(()) } - array_of_results => Self::return_first_error(array_of_results), + improper_array_of_results => Self::return_first_error(improper_array_of_results), } }; let select_sql = config.select_sql(); - let mut select_stm = BigIntDbProcessor::::prepare_statement(conn, &select_sql); + let mut select_stm = BigIntDbProcessorReal::::prepare_statement(conn, &select_sql); match select_stm.query_row([], update_divided_integer) { Ok(()) => Ok(()), - Err(e) => Err(BigIntDbError(format!( + Err(e) => Err(BigIntDatabaseError::General(format!( "Updating balance for {} table and change of {} wei to '{} = {}' with error '{}'", T::table_name(), config.balance_change(), - config.params.table_unique_key_name, + config.params.table_unique_key, config.key_param_value(), e ))), @@ -146,15 +173,18 @@ impl UpdateOverflowHandler for UpdateOverflowHandlerReal } impl UpdateOverflowHandlerReal { - fn execute_update<'a>( - conn: Either<&dyn ConnectionWrapper, &Transaction>, - config: &BigIntSqlConfig<'a, T>, - execute_params: &[(&str, &dyn ToSql)], + fn execute_update<'params>( + conn: Either<&dyn ConnectionWrapper, &TransactionSafeWrapper>, + config: &BigIntSqlConfig<'params, T>, + sql_params: Vec, ) { - match BigIntDbProcessor::::prepare_statement(conn, config.overflow_update_clause) - .execute(execute_params) - .expect("logic broken given the previous non-overflow call accepted right") - { + let stmt = + BigIntDbProcessorReal::::prepare_statement(conn, config.overflow_update_clause); + let rows_changed = BigIntDbProcessorReal::::execute_statement(stmt, sql_params).expect( + "Logic problem in params selection for the SQL taking values with \ + an already compensated overflow", + ); + match rows_changed { 1 => (), x => panic!( "Broken code: this code was written to handle one changed row a time, not {}", @@ -168,9 +198,9 @@ impl UpdateOverflowHandlerReal { former_low_bytes: i64, requested_wei_change: &WeiChangeAsHighAndLowBytes, ) -> (i64, i64) { - let high_bytes_correction = former_high_bytes + requested_wei_change.high.value + 1; + let high_bytes_correction = former_high_bytes + requested_wei_change.high_bytes.value + 1; let low_bytes_correction = ((former_low_bytes as i128 - + requested_wei_change.low.value as i128) + + requested_wei_change.low_bytes.value as i128) & 0x7FFFFFFFFFFFFFFF) as i64; (high_bytes_correction, low_bytes_correction) } @@ -187,19 +217,19 @@ impl UpdateOverflowHandlerReal { } } -pub struct BigIntSqlConfig<'a, T> { - main_sql: &'a str, - overflow_update_clause: &'a str, - pub params: SQLParams<'a>, +pub struct BigIntSqlConfig<'params, T: TableNameDAO> { + main_sql: &'params str, + overflow_update_clause: &'params str, + params: SQLParams<'params>, phantom: PhantomData, } -impl<'a, T: TableNameDAO> BigIntSqlConfig<'a, T> { +impl<'params, T: TableNameDAO> BigIntSqlConfig<'params, T> { pub fn new( - main_sql: &'a str, - overflow_update_clause: &'a str, - params: SQLParams<'a>, - ) -> BigIntSqlConfig<'a, T> { + main_sql: &'params str, + overflow_update_clause: &'params str, + params: SQLParams<'params>, + ) -> BigIntSqlConfig<'params, T> { Self { main_sql, overflow_update_clause, @@ -211,21 +241,21 @@ impl<'a, T: TableNameDAO> BigIntSqlConfig<'a, T> { fn select_sql(&self) -> String { format!( "select {}, {} from {} where {} = '{}'", - &self.params.wei_change_params.high.name[1..], - &self.params.wei_change_params.low.name[1..], + &self.params.wei_change_params.high_bytes.name[1..], + &self.params.wei_change_params.low_bytes.name[1..], T::table_name(), - self.params.table_unique_key_name, + self.params.table_unique_key, self.key_param_value() ) } - fn key_param_value(&self) -> &'a dyn ExtendedParamsMarker { - self.params.params_except_wei_change[0].value_pair.1 + fn key_param_value(&self) -> &dyn DisplayableParamValue { + <&DisplayableRusqliteParamPair>::from(&self.params.other_than_wei_change_params[0]).value } fn balance_change(&self) -> i128 { let wei_params = &self.params.wei_change_params; - BigIntDivider::reconstitute(wei_params.high.value, wei_params.low.value) + BigIntDivider::reconstitute(wei_params.high_bytes.value, wei_params.low_bytes.value) } fn determine_command(&self) -> String { @@ -253,32 +283,58 @@ impl<'a, T: TableNameDAO> BigIntSqlConfig<'a, T> { } } -//to be able to display things that implement ToSql -pub trait ExtendedParamsMarker: ToSql + Display {} +pub struct RusqliteParamPairAsStruct<'params> { + sql_subst_name: &'params str, + value: &'params dyn ToSql, +} + +impl<'params> RusqliteParamPairAsStruct<'params> { + fn new( + sql_subst_name: &'params str, + value: &'params dyn ToSql, + ) -> RusqliteParamPairAsStruct<'params> { + Self { + sql_subst_name, + value, + } + } +} + +pub struct DisplayableRusqliteParamPair<'params> { + sql_subst_name: &'params str, + value: &'params dyn DisplayableParamValue, +} -macro_rules! impl_of_extended_params_marker{ - ($($implementer: ty),+) => { - $(impl ExtendedParamsMarker for $implementer {})+ +impl<'params> DisplayableRusqliteParamPair<'params> { + pub fn new( + sql_subst_name: &'params str, + value: &'params dyn DisplayableParamValue, + ) -> DisplayableRusqliteParamPair<'params> { + Self { + sql_subst_name, + value, + } } } -impl_of_extended_params_marker!(i64, &str, Wallet); +// To be able to display things that implement ToSql +pub trait DisplayableParamValue: ToSql + Display {} + +impl DisplayableParamValue for i64 {} +impl DisplayableParamValue for &str {} +impl DisplayableParamValue for Wallet {} #[derive(Default)] -pub struct SQLParamsBuilder<'a> { - key_spec_opt: Option>, +pub struct SQLParamsBuilder<'params> { + key_spec_opt: Option>, wei_change_spec_opt: Option, - other_params: Vec>, + other_params: Vec>, } -impl<'a> SQLParamsBuilder<'a> { - pub fn key(mut self, key_variant: KnownKeyVariants<'a>) -> Self { - let (definition_name, substitution_name_in_sql, value_itself) = key_variant.into(); - self.key_spec_opt = Some(UniqueKeySpec { - definition_name, - substitution_name_in_sql, - value_itself, - }); +impl<'params> SQLParamsBuilder<'params> { + pub fn key(mut self, key_variant: KeyVariants<'params>) -> Self { + let key_spec = TableUniqueKey::from(key_variant); + self.key_spec_opt = Some(key_spec); self } @@ -287,113 +343,110 @@ impl<'a> SQLParamsBuilder<'a> { self } - pub fn other_params(mut self, params: Vec>) -> Self { + pub fn other_params(mut self, params: Vec>) -> Self { self.other_params = params; self } - pub fn build(self) -> SQLParams<'a> { + pub fn build(self) -> SQLParams<'params> { let key_spec = self .key_spec_opt - .unwrap_or_else(|| panic!("SQLparams cannot miss the component of a key")); + .expect("SQLparams must have the key by now!"); + let wei_change_spec = self .wei_change_spec_opt - .unwrap_or_else(|| panic!("SQLparams cannot miss the component of wei change")); - let ((high_bytes_param_name, low_bytes_param_name), (high_bytes_value, low_bytes_value)) = - Self::expand_wei_params(wei_change_spec); - let key_as_the_first_param = (key_spec.substitution_name_in_sql, key_spec.value_itself); - let params = once(Param::new(key_as_the_first_param, true)) + .expect("SQLparams must have wei change by now!"); + + let wei_change_params = WeiChangeAsHighAndLowBytes::from(wei_change_spec); + + let key_param = ParamByUse::BeforeAndAfterOverflow(DisplayableRusqliteParamPair::new( + key_spec.substitution_name_in_sql, + key_spec.value, + )); + // Ensure keeping the key param at the first position + let other_than_wei_change_params = once(key_param) .chain(self.other_params.into_iter()) .collect(); + SQLParams { - table_unique_key_name: key_spec.definition_name, - wei_change_params: WeiChangeAsHighAndLowBytes { - high: StdNumParamFormNamed::new(high_bytes_param_name, high_bytes_value), - low: StdNumParamFormNamed::new(low_bytes_param_name, low_bytes_value), - }, - params_except_wei_change: params, + table_unique_key: key_spec.column_name, + wei_change_params, + other_than_wei_change_params, } } +} - fn expand_wei_params(wei_change_spec: WeiChange) -> ((String, String), (i64, i64)) { - let (name, num): (&'static str, i128) = match wei_change_spec { - WeiChange::Addition(name, num) => (name, checked_conversion::(num)), - WeiChange::Subtraction(name, num) => { - (name, checked_conversion::(num).neg()) - } - }; - let (high_bytes, low_bytes) = BigIntDivider::deconstruct(num); - let param_sub_name_for_high_bytes = - Self::proper_wei_change_param_name(name, ByteMagnitude::High); - let param_sub_name_for_low_bytes = - Self::proper_wei_change_param_name(name, ByteMagnitude::Low); - ( - (param_sub_name_for_high_bytes, param_sub_name_for_low_bytes), - (high_bytes, low_bytes), - ) - } - - fn proper_wei_change_param_name(base_word: &str, byte_magnitude: ByteMagnitude) -> String { - format!(":{}_{}_b", base_word, byte_magnitude) - } +struct TableUniqueKey<'params> { + column_name: &'params str, + substitution_name_in_sql: &'params str, + value: &'params dyn DisplayableParamValue, } -struct UniqueKeySpec<'a> { - definition_name: &'a str, - substitution_name_in_sql: &'a str, - value_itself: &'a dyn ExtendedParamsMarker, +impl<'params> TableUniqueKey<'params> { + fn new( + column_name: &'params str, + substitution_name_in_sql: &'params str, + value: &'params dyn DisplayableParamValue, + ) -> Self { + Self { + column_name, + substitution_name_in_sql, + value, + } + } } -pub enum KnownKeyVariants<'a> { - WalletAddress(&'a dyn ExtendedParamsMarker), - PendingPayableRowid(&'a dyn ExtendedParamsMarker), +pub enum KeyVariants<'params> { + WalletAddress(&'params dyn DisplayableParamValue), + PendingPayableRowid(&'params dyn DisplayableParamValue), #[cfg(test)] TestKey { - var_name: &'static str, - sub_name: &'static str, - val: &'a dyn ExtendedParamsMarker, + column_name: &'params str, + substitution_name: &'params str, + value: &'params dyn DisplayableParamValue, }, } -impl<'a> From> for (&'static str, &'static str, &'a dyn ExtendedParamsMarker) { - fn from(variant: KnownKeyVariants<'a>) -> Self { +impl<'params> From> for TableUniqueKey<'params> { + fn from(variant: KeyVariants<'params>) -> Self { match variant { - KnownKeyVariants::WalletAddress(val) => ("wallet_address", ":wallet", val), - KnownKeyVariants::PendingPayableRowid(val) => ("pending_payable_rowid", ":rowid", val), + KeyVariants::WalletAddress(val) => { + TableUniqueKey::new("wallet_address", ":wallet", val) + } + KeyVariants::PendingPayableRowid(val) => { + TableUniqueKey::new("pending_payable_rowid", ":rowid", val) + } #[cfg(test)] - KnownKeyVariants::TestKey { - var_name, - sub_name, - val, - } => (var_name, sub_name, val), - } - } -} - -enum ByteMagnitude { - High, - Low, -} - -impl Display for ByteMagnitude { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match self { - Self::High => write!(f, "high"), - Self::Low => write!(f, "low"), + KeyVariants::TestKey { + column_name: var_name, + substitution_name: sub_name, + value: val, + } => TableUniqueKey::new(var_name, sub_name, val), } } } -pub struct SQLParams<'a> { - table_unique_key_name: &'a str, +pub struct SQLParams<'params> { + table_unique_key: &'params str, wei_change_params: WeiChangeAsHighAndLowBytes, - params_except_wei_change: Vec>, + other_than_wei_change_params: Vec>, } #[derive(Debug, PartialEq)] struct WeiChangeAsHighAndLowBytes { - high: StdNumParamFormNamed, - low: StdNumParamFormNamed, + high_bytes: StdNumParamFormNamed, + low_bytes: StdNumParamFormNamed, +} + +impl WeiChangeAsHighAndLowBytes { + fn new(name: &str, high_bytes: i64, low_bytes: i64) -> Self { + let high_bytes = StdNumParamFormNamed::new(format!(":{}_high_b", name), high_bytes); + let low_bytes = StdNumParamFormNamed::new(format!(":{}_low_b", name), low_bytes); + Self { + high_bytes, + low_bytes, + } + } } #[derive(Debug, PartialEq)] @@ -402,67 +455,101 @@ struct StdNumParamFormNamed { value: i64, } +impl From for WeiChangeAsHighAndLowBytes { + fn from(wei_change: WeiChange) -> Self { + let size_checked_amount = checked_conversion::(wei_change.amount_to_change); + + let oriented_amount = match wei_change.direction { + WeiChangeDirection::Addition => size_checked_amount, + WeiChangeDirection::Subtraction => size_checked_amount.neg(), + }; + + let (high_bytes, low_bytes) = BigIntDivider::deconstruct(oriented_amount); + + WeiChangeAsHighAndLowBytes::new(wei_change.unsuffixed_name, high_bytes, low_bytes) + } +} + impl StdNumParamFormNamed { fn new(name: String, value: i64) -> Self { Self { name, value } } } -pub struct Param<'a> { - value_pair: (&'a str, &'a dyn ExtendedParamsMarker), - participates_in_overflow_clause: bool, +pub enum ParamByUse<'params> { + BeforeAndAfterOverflow(DisplayableRusqliteParamPair<'params>), + BeforeOverflowOnly(DisplayableRusqliteParamPair<'params>), } -impl<'a> Param<'a> { - pub fn new( - value_pair: (&'a str, &'a (dyn ExtendedParamsMarker + 'a)), - participates_in_overflow_clause: bool, - ) -> Self { - Self { - value_pair, - participates_in_overflow_clause, +impl<'params> From<&'params ParamByUse<'params>> for &DisplayableRusqliteParamPair<'params> { + fn from(param_by_use: &'params ParamByUse) -> Self { + match param_by_use { + ParamByUse::BeforeAndAfterOverflow(param_pair) => param_pair, + ParamByUse::BeforeOverflowOnly(param_pair) => param_pair, } } +} - fn as_rusqlite_params(&'a self) -> (&'a str, &'a dyn ToSql) { - (self.value_pair.0, &self.value_pair.1 as &dyn ToSql) +impl<'params> From<&'params ParamByUse<'params>> for RusqliteParamPairAsStruct<'params> { + fn from(param_by_use: &'params ParamByUse<'params>) -> Self { + match param_by_use { + ParamByUse::BeforeAndAfterOverflow(DisplayableRusqliteParamPair { + sql_subst_name, + value, + }) => RusqliteParamPairAsStruct::new(sql_subst_name, value), + ParamByUse::BeforeOverflowOnly(DisplayableRusqliteParamPair { + sql_subst_name, + value, + }) => RusqliteParamPairAsStruct::new(sql_subst_name, value), + } } } -impl<'a> From<&'a WeiChangeAsHighAndLowBytes> for [(&'a str, &'a dyn ToSql); 2] { - fn from(wei_change: &'a WeiChangeAsHighAndLowBytes) -> Self { +impl<'params> From<&'params WeiChangeAsHighAndLowBytes> + for [RusqliteParamPairAsStruct<'params>; 2] +{ + fn from(wei_change: &'params WeiChangeAsHighAndLowBytes) -> Self { [ - (wei_change.high.name.as_str(), &wei_change.high.value), - (wei_change.low.name.as_str(), &wei_change.low.value), + RusqliteParamPairAsStruct::new( + wei_change.high_bytes.name.as_str(), + &wei_change.high_bytes.value, + ), + RusqliteParamPairAsStruct::new( + wei_change.low_bytes.name.as_str(), + &wei_change.low_bytes.value, + ), ] } } -impl<'a> SQLParams<'a> { - fn merge_other_and_wei_params( - &'a self, - wei_change_params: [(&'a str, &'a dyn ToSql); 2], - ) -> Vec<(&'a str, &'a dyn ToSql)> { - Self::merge_params(self.params_except_wei_change.iter(), wei_change_params) +impl<'params> SQLParams<'params> { + fn non_overflow_params(&'params self) -> Vec { + Self::merge_params( + self.other_than_wei_change_params.iter(), + (&self.wei_change_params).into(), + ) } - fn merge_other_and_wei_params_with_conditional_participants( - &'a self, - wei_change_params: [(&'a str, &'a dyn ToSql); 2], - ) -> Vec<(&'a str, &'a dyn ToSql)> { - let preselection = self - .params_except_wei_change + fn overflow_params( + &'params self, + recomputed_wei_change_params: [RusqliteParamPairAsStruct<'params>; 2], + ) -> Vec> { + let other_params_selection_of_relevant_args = self + .other_than_wei_change_params .iter() - .filter(|param| param.participates_in_overflow_clause); - Self::merge_params(preselection, wei_change_params) + .filter(|param| matches!(param, ParamByUse::BeforeAndAfterOverflow { .. })); + Self::merge_params( + other_params_selection_of_relevant_args, + recomputed_wei_change_params, + ) } fn merge_params( - params: impl Iterator>, - wei_change_params: [(&'a str, &'a dyn ToSql); 2], - ) -> Vec<(&'a str, &'a dyn ToSql)> { + params: impl Iterator>, + wei_change_params: [RusqliteParamPairAsStruct<'params>; 2], + ) -> Vec> { params - .map(|param| param.as_rusqlite_params()) + .map(RusqliteParamPairAsStruct::from) .chain(wei_change_params.into_iter()) .collect() } @@ -472,46 +559,83 @@ pub trait TableNameDAO: Debug + Send { fn table_name() -> String; } -#[derive(Debug, PartialEq, Eq, Clone)] -pub enum WeiChange { - Addition(&'static str, u128), - //This means that the supplied amount is internally converted into a signed number and - //so even though the related SQL contains + signs the resulting math operation is subtraction - Subtraction(&'static str, u128), +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct WeiChange { + unsuffixed_name: &'static str, + amount_to_change: u128, + direction: WeiChangeDirection, +} + +impl WeiChange { + pub fn new( + unsuffixed_name: &'static str, + amount_to_change: u128, + direction: WeiChangeDirection, + ) -> Self { + Self { + unsuffixed_name, + amount_to_change, + direction, + } + } +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum WeiChangeDirection { + Addition, + Subtraction, } #[derive(Debug, PartialEq, Eq)] -pub struct BigIntDbError(pub String); +pub enum BigIntDatabaseError { + General(String), + RowChangeMismatch { + row_key: String, + detected_count_changed: usize, + }, +} -macro_rules! big_int_db_error_from { - ($($implementer: ident),+) => { - $(impl From for $implementer { - fn from(iu_err: BigIntDbError) -> Self { - $implementer::RusqliteError(iu_err.0) - } - })+ +impl From for PayableDaoError { + fn from(err: BigIntDatabaseError) -> Self { + PayableDaoError::RusqliteError(err.to_string()) } } -big_int_db_error_from!(PayableDaoError, ReceivableDaoError); +impl From for ReceivableDaoError { + fn from(err: BigIntDatabaseError) -> Self { + ReceivableDaoError::RusqliteError(err.to_string()) + } +} + +impl Display for BigIntDatabaseError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match &self { + BigIntDatabaseError::General(msg) => write!(f, "{}", msg), + BigIntDatabaseError::RowChangeMismatch { + row_key, + detected_count_changed, + } => write!( + f, + "Expected 1 row to be changed for the unique key {} but got this count: {}", + row_key, detected_count_changed + ), + } + } +} #[cfg(test)] mod tests { use super::*; - use crate::accountant::db_big_integer::big_int_db_processor::ByteMagnitude::{High, Low}; - use crate::accountant::db_big_integer::big_int_db_processor::KnownKeyVariants::TestKey; - use crate::accountant::db_big_integer::big_int_db_processor::WeiChange::{ - Addition, Subtraction, - }; + use crate::accountant::db_access_objects::payable_dao::PayableDaoError; + use crate::accountant::db_big_integer::big_int_db_processor::KeyVariants::TestKey; + use crate::accountant::db_big_integer::big_int_db_processor::WeiChangeDirection::Addition; use crate::accountant::db_big_integer::test_utils::restricted::{ create_new_empty_db, test_database_key, }; - use crate::database::connection_wrapper::{ConnectionWrapper, ConnectionWrapperReal}; + use crate::accountant::db_big_integer::test_utils::UpdateOverflowHandlerMock; + use crate::database::rusqlite_wrappers::{ConnectionWrapper, ConnectionWrapperReal}; use crate::test_utils::make_wallet; - use itertools::Either; - use itertools::Either::Left; use rusqlite::{Connection, ToSql}; - use std::cell::RefCell; use std::sync::{Arc, Mutex}; #[derive(Debug)] @@ -524,49 +648,104 @@ mod tests { } #[test] - fn conversion_from_local_error_to_particular_payable_dao_error_works() { - let subject = BigIntDbError(String::from("whatever")); - - let result: PayableDaoError = subject.into(); - + fn display_for_big_int_error_works() { assert_eq!( - result, - PayableDaoError::RusqliteError("whatever".to_string()) + BigIntDatabaseError::General("This is a general message".to_string()).to_string(), + "This is a general message".to_string() + ); + assert_eq!( + BigIntDatabaseError::RowChangeMismatch { + row_key: "Wallet123".to_string(), + detected_count_changed: 0 + } + .to_string(), + "Expected 1 row to be changed for the unique key Wallet123 but got this count: 0" ) } #[test] - fn conversion_from_local_error_to_particular_receivable_dao_error_works() { - let subject = BigIntDbError(String::from("whatever")); - - let result: ReceivableDaoError = subject.into(); + fn conversion_from_local_error_to_particular_payable_dao_error_works() { + assert_eq!( + PayableDaoError::from(BigIntDatabaseError::General(String::from("booga"))), + PayableDaoError::RusqliteError("booga".to_string()) + ); + assert_eq!( + PayableDaoError::from(BigIntDatabaseError::RowChangeMismatch { + row_key: "booga_key".to_string(), + detected_count_changed: 2 + }), + PayableDaoError::RusqliteError( + "Expected 1 row to be changed for the unique key \ + booga_key but got this count: 2" + .to_string() + ) + ); + } + #[test] + fn conversion_from_local_error_to_particular_receivable_dao_error_works() { assert_eq!( - result, - ReceivableDaoError::RusqliteError("whatever".to_string()) - ) + ReceivableDaoError::from(BigIntDatabaseError::General(String::from("blah"))), + ReceivableDaoError::RusqliteError("blah".to_string()) + ); + assert_eq!( + ReceivableDaoError::from(BigIntDatabaseError::RowChangeMismatch { + row_key: "blah_key".to_string(), + detected_count_changed: 2 + }), + ReceivableDaoError::RusqliteError( + "Expected 1 row to be changed for the unique key \ + blah_key but got this count: 2" + .to_string() + ) + ); } #[test] - fn display_for_byte_magnitude_works() { - assert_eq!(High.to_string(), "high".to_string()); - assert_eq!(Low.to_string(), "low".to_string()) + fn conversion_between_references_of_param_by_use_and_displayable_param_pair_is_enabled() { + let make_displayable_param_1 = + || DisplayableRusqliteParamPair::new(":elephant", &"trumpeting"); + let make_displayable_param_2 = || DisplayableRusqliteParamPair::new(":cat", &"meowing"); + let param_by_use_1 = ParamByUse::BeforeAndAfterOverflow(make_displayable_param_1()); + let param_by_use_2 = ParamByUse::BeforeOverflowOnly(make_displayable_param_2()); + + let result_from_before_and_after_overflow: &DisplayableRusqliteParamPair = + (¶m_by_use_1).into(); + let result_from_before_overflow_only: &DisplayableRusqliteParamPair = + (¶m_by_use_2).into(); + + let expected_result_1 = make_displayable_param_1(); + assert_eq!( + result_from_before_and_after_overflow.sql_subst_name, + expected_result_1.sql_subst_name + ); + assert_eq!( + result_from_before_and_after_overflow.value.to_string(), + expected_result_1.value.to_string() + ); + let expected_result_2 = make_displayable_param_2(); + assert_eq!( + result_from_before_overflow_only.sql_subst_name, + expected_result_2.sql_subst_name + ); + assert_eq!( + result_from_before_overflow_only.value.to_string(), + expected_result_2.value.to_string() + ) } #[test] - fn known_key_variants_to_tuple_of_names_and_val_works() { - let (wallet_name, wallet_sub_name, wallet_val): (&str, &str, &dyn ExtendedParamsMarker) = - KnownKeyVariants::WalletAddress(&"blah").into(); - let (rowid_name, rowid_sub_name, rowid_val): (&str, &str, &dyn ExtendedParamsMarker) = - KnownKeyVariants::PendingPayableRowid(&123).into(); + fn known_key_variants_to_table_unique_key_works() { + let key_1: TableUniqueKey = KeyVariants::WalletAddress(&"blah").into(); + let key_2: TableUniqueKey = KeyVariants::PendingPayableRowid(&123).into(); - assert_eq!(wallet_name, "wallet_address"); - assert_eq!(wallet_sub_name, ":wallet"); - assert_eq!(wallet_val.to_string(), "blah".to_string()); - assert_eq!(rowid_name, "pending_payable_rowid"); - assert_eq!(rowid_sub_name, ":rowid"); - assert_eq!(rowid_val.to_string(), 123.to_string()) - //cannot be compared by values directly + assert_eq!(key_1.column_name, "wallet_address"); + assert_eq!(key_1.substitution_name_in_sql, ":wallet"); + assert_eq!(key_1.value.to_string(), "blah".to_string()); + assert_eq!(key_2.column_name, "pending_payable_rowid"); + assert_eq!(key_2.substitution_name_in_sql, ":rowid"); + assert_eq!(key_2.value.to_string(), 123.to_string()) + // Values cannot be compared directly } #[test] @@ -574,125 +753,129 @@ mod tests { let subject = SQLParamsBuilder::default(); let result = subject - .wei_change(Addition("balance", 4546)) + .wei_change(WeiChange::new( + "balance", + 4546, + WeiChangeDirection::Addition, + )) .key(TestKey { - var_name: "some_key", - sub_name: ":some_key", - val: &"blah", + column_name: "some_key", + substitution_name: ":some_key", + value: &"blah", }) - .other_params(vec![Param::new(("other_thing", &46565), true)]); + .other_params(vec![ParamByUse::BeforeAndAfterOverflow( + DisplayableRusqliteParamPair { + sql_subst_name: "other_thing", + value: &46565, + }, + )]); - assert_eq!(result.wei_change_spec_opt, Some(Addition("balance", 4546))); + assert_eq!( + result.wei_change_spec_opt, + Some(WeiChange::new( + "balance", + 4546, + WeiChangeDirection::Addition + )) + ); let key_spec = result.key_spec_opt.unwrap(); - assert_eq!(key_spec.definition_name, "some_key"); + assert_eq!(key_spec.column_name, "some_key"); assert_eq!(key_spec.substitution_name_in_sql, ":some_key"); - assert_eq!(key_spec.value_itself.to_string(), "blah".to_string()); + assert_eq!(key_spec.value.to_string(), "blah".to_string()); + let param_pair = <&DisplayableRusqliteParamPair>::from(&result.other_params[0]); assert!(matches!( - result.other_params[0].value_pair, - ("other_thing", _) + param_pair, + DisplayableRusqliteParamPair { + sql_subst_name: "other_thing", + .. + } )); assert_eq!(result.other_params.len(), 1) } #[test] - fn sql_params_builder_builds_correct_params() { - let subject = SQLParamsBuilder::default(); - - let result = subject - .wei_change(Addition("balance", 115898)) - .key(TestKey { - var_name: "some_key", - sub_name: ":some_key", - val: &"blah", - }) - .other_params(vec![Param::new((":other_thing", &11111), true)]) - .build(); + fn sql_params_builder_builds_correct_params_with_addition_in_wei_change() { + let wei_change_input = WeiChange::new("balance", 115898, Addition); + let expected_resulted_wei_change_params = WeiChangeAsHighAndLowBytes { + high_bytes: StdNumParamFormNamed::new(":balance_high_b".to_string(), 0), + low_bytes: StdNumParamFormNamed::new(":balance_low_b".to_string(), 115898), + }; - assert_eq!(result.table_unique_key_name, "some_key"); - assert_eq!( - result.wei_change_params, - WeiChangeAsHighAndLowBytes { - high: StdNumParamFormNamed::new(":balance_high_b".to_string(), 0), - low: StdNumParamFormNamed::new(":balance_low_b".to_string(), 115898) - } - ); - assert_eq!(result.params_except_wei_change[0].value_pair.0, ":some_key"); - assert_eq!( - result.params_except_wei_change[0].value_pair.1.to_string(), - "blah".to_string() - ); - assert_eq!( - result.params_except_wei_change[1].value_pair.0, - ":other_thing" - ); - assert_eq!( - result.params_except_wei_change[1].value_pair.1.to_string(), - "11111".to_string() - ); - assert_eq!(result.params_except_wei_change.len(), 2) + test_correct_build_of_sql_params(wei_change_input, expected_resulted_wei_change_params) } #[test] - fn sql_params_builder_builds_correct_params_with_negative_wei_change() { + fn sql_params_builder_builds_correct_params_with_subtraction_in_wei_change() { + let wei_change_input = WeiChange::new("balance", 454684, WeiChangeDirection::Subtraction); + let expected_resulted_wei_change_params = WeiChangeAsHighAndLowBytes { + high_bytes: StdNumParamFormNamed::new(":balance_high_b".to_string(), -1), + low_bytes: StdNumParamFormNamed::new(":balance_low_b".to_string(), 9223372036854321124), + }; + + test_correct_build_of_sql_params(wei_change_input, expected_resulted_wei_change_params); + } + + fn test_correct_build_of_sql_params( + wei_change_input: WeiChange, + expected_ending_wei_change_params: WeiChangeAsHighAndLowBytes, + ) { let subject = SQLParamsBuilder::default(); let result = subject - .wei_change(Subtraction("balance", 454684)) + .wei_change(wei_change_input) .key(TestKey { - var_name: "some_key", - sub_name: ":some_key", - val: &"wooow", + column_name: "some_key", + substitution_name: ":some_key", + value: &"wooow", }) - .other_params(vec![Param::new((":other_thing", &46565), true)]) + .other_params(vec![ParamByUse::BeforeAndAfterOverflow( + DisplayableRusqliteParamPair::new(":other_thing", &46565), + )]) .build(); - - assert_eq!(result.table_unique_key_name, "some_key"); - assert_eq!( - result.wei_change_params, - WeiChangeAsHighAndLowBytes { - high: StdNumParamFormNamed::new(":balance_high_b".to_string(), -1), - low: StdNumParamFormNamed::new(":balance_low_b".to_string(), 9223372036854321124) - } - ); - assert_eq!(result.params_except_wei_change[0].value_pair.0, ":some_key"); - assert_eq!( - result.params_except_wei_change[0].value_pair.1.to_string(), - "wooow".to_string() - ); - assert_eq!( - result.params_except_wei_change[1].value_pair.0, - ":other_thing" - ); - assert_eq!( - result.params_except_wei_change[1].value_pair.1.to_string(), - "46565".to_string() - ); - assert_eq!(result.params_except_wei_change.len(), 2) + assert_eq!(result.table_unique_key, "some_key"); + assert_eq!(result.wei_change_params, expected_ending_wei_change_params); + let param_pair = + <&DisplayableRusqliteParamPair>::from(&result.other_than_wei_change_params[0]); + assert_eq!(param_pair.sql_subst_name, ":some_key"); + assert_eq!(param_pair.value.to_string(), "wooow".to_string()); + let param_pair = + <&DisplayableRusqliteParamPair>::from(&result.other_than_wei_change_params[1]); + assert_eq!(param_pair.sql_subst_name, ":other_thing"); + assert_eq!(param_pair.value.to_string(), "46565".to_string()); + assert_eq!(result.other_than_wei_change_params.len(), 2) } #[test] - #[should_panic(expected = "SQLparams cannot miss the component of a key")] + #[should_panic(expected = "SQLparams must have the key by now!")] fn sql_params_builder_cannot_be_built_without_key_spec() { let subject = SQLParamsBuilder::default(); let _ = subject - .wei_change(Addition("balance", 4546)) - .other_params(vec![Param::new(("laughter", &"hahaha"), true)]) + .wei_change(WeiChange::new( + "balance", + 4546, + WeiChangeDirection::Addition, + )) + .other_params(vec![ParamByUse::BeforeAndAfterOverflow( + DisplayableRusqliteParamPair::new("laughter", &"hahaha"), + )]) .build(); } #[test] - #[should_panic(expected = "SQLparams cannot miss the component of wei change")] + #[should_panic(expected = "SQLparams must have wei change by now!")] fn sql_params_builder_cannot_be_built_without_wei_change_spec() { let subject = SQLParamsBuilder::default(); let _ = subject .key(TestKey { - var_name: "wallet", - sub_name: ":wallet", - val: &make_wallet("wallet"), + column_name: "wallet", + substitution_name: ":wallet", + value: &make_wallet("wallet"), }) - .other_params(vec![Param::new(("other_thing", &46565), true)]) + .other_params(vec![ParamByUse::BeforeAndAfterOverflow( + DisplayableRusqliteParamPair::new("other_thing", &46565), + )]) .build(); } @@ -701,57 +884,61 @@ mod tests { let subject = SQLParamsBuilder::default(); let _ = subject - .wei_change(Addition("balance", 4546)) + .wei_change(WeiChange::new( + "balance", + 4546, + WeiChangeDirection::Addition, + )) .key(TestKey { - var_name: "id", - sub_name: ":id", - val: &45, + column_name: "id", + substitution_name: ":id", + value: &45, }) .build(); } #[test] - fn merge_other_and_wei_params_with_conditional_participants_can_filter_out_just_update_params() - { - let tuple_matrix = [ - ("blah", &456_i64 as &dyn ExtendedParamsMarker), - ("super key", &"abcxy"), - ("time", &779988), - ("error", &"no threat"), - ]; - let update_positive_1 = tuple_matrix[0]; - let update_positive_2 = tuple_matrix[1]; - let update_negative_1 = tuple_matrix[2]; - let update_negative_2 = tuple_matrix[3]; + fn overflow_params_do_not_contain_extra_params_meant_for_non_overflow_case() { let subject = SQLParams { - table_unique_key_name: "", + table_unique_key: "", wei_change_params: WeiChangeAsHighAndLowBytes { - high: StdNumParamFormNamed { + high_bytes: StdNumParamFormNamed { name: "".to_string(), value: 0, }, - low: StdNumParamFormNamed { + low_bytes: StdNumParamFormNamed { name: "".to_string(), value: 0, }, }, - params_except_wei_change: vec![ - Param::new(update_positive_2, true), - Param::new(update_negative_1, false), - Param::new(update_positive_1, true), - Param::new(update_negative_2, false), + other_than_wei_change_params: vec![ + ParamByUse::BeforeAndAfterOverflow(DisplayableRusqliteParamPair::new( + "blah", &456_i64, + )), + ParamByUse::BeforeOverflowOnly(DisplayableRusqliteParamPair::new("time", &779988)), + ParamByUse::BeforeAndAfterOverflow(DisplayableRusqliteParamPair::new( + "super key", + &"abcxy", + )), + ParamByUse::BeforeOverflowOnly(DisplayableRusqliteParamPair::new("booga", &"oh")), ], }; - let result = subject.merge_other_and_wei_params_with_conditional_participants([ - ("always_present_1", &12), - ("always_present_2", &77), + let result = subject.overflow_params([ + RusqliteParamPairAsStruct { + sql_subst_name: "always_present_1", + value: &12, + }, + RusqliteParamPairAsStruct { + sql_subst_name: "always_present_2", + value: &77, + }, ]); - assert_eq!(result[0].0, update_positive_2.0); - assert_eq!(result[1].0, update_positive_1.0); - assert_eq!(result[2].0, "always_present_1"); - assert_eq!(result[3].0, "always_present_2") + assert_eq!(result[0].sql_subst_name, "blah"); + assert_eq!(result[1].sql_subst_name, "super key"); + assert_eq!(result[2].sql_subst_name, "always_present_1"); + assert_eq!(result[3].sql_subst_name, "always_present_2") } #[test] @@ -786,12 +973,12 @@ mod tests { fn make_empty_sql_params<'a>() -> SQLParams<'a> { SQLParams { - table_unique_key_name: "", + table_unique_key: "", wei_change_params: WeiChangeAsHighAndLowBytes { - high: StdNumParamFormNamed::new("".to_string(), 0), - low: StdNumParamFormNamed::new("".to_string(), 0), + high_bytes: StdNumParamFormNamed::new("".to_string(), 0), + low_bytes: StdNumParamFormNamed::new("".to_string(), 0), }, - params_except_wei_change: vec![], + other_than_wei_change_params: vec![], } } @@ -877,35 +1064,6 @@ mod tests { .unwrap(); } - #[derive(Debug, Default)] - struct UpdateOverflowHandlerMock { - update_with_overflow_params: Arc>>, - update_with_overflow_results: RefCell>>, - } - - impl UpdateOverflowHandler for UpdateOverflowHandlerMock { - fn update_with_overflow<'a>( - &self, - _conn: Either<&dyn ConnectionWrapper, &Transaction>, - _config: BigIntSqlConfig<'a, T>, - ) -> Result<(), BigIntDbError> { - self.update_with_overflow_params.lock().unwrap().push(()); - self.update_with_overflow_results.borrow_mut().remove(0) - } - } - - impl UpdateOverflowHandlerMock { - fn update_with_overflow_params(mut self, params: &Arc>>) -> Self { - self.update_with_overflow_params = params.clone(); - self - } - - fn update_with_overflow_result(self, result: Result<(), BigIntDbError>) -> Self { - self.update_with_overflow_results.borrow_mut().push(result); - self - } - } - #[derive(Debug, PartialEq)] struct ConventionalUpsertUpdateAnalysisData { was_update_with_overflow: bool, @@ -929,12 +1087,12 @@ mod tests { let overflow_handler = UpdateOverflowHandlerMock::default() .update_with_overflow_params(&update_with_overflow_params_arc) .update_with_overflow_result(Ok(())); - let mut subject = BigIntDbProcessor::::default(); + let mut subject = BigIntDbProcessorReal::::default(); subject.overflow_handler = Box::new(overflow_handler); let act = |conn: &mut dyn ConnectionWrapper| { subject.execute( - Left(conn), + Either::Left(conn), BigIntSqlConfig::new( main_sql, "", @@ -961,7 +1119,7 @@ mod tests { update_with_overflow_params_arc: Arc>>, ) -> ConventionalUpsertUpdateAnalysisData where - F: Fn(&mut dyn ConnectionWrapper) -> Result<(), BigIntDbError>, + F: Fn(&mut dyn ConnectionWrapper) -> Result<(), BigIntDatabaseError>, { let mut conn = initiate_simple_connection_and_test_table("big_int_db_processor", test_name); if init_record != 0 { @@ -1034,7 +1192,7 @@ mod tests { let result = analyse_sql_commands_execution_without_details_of_overflow( "update_alone_works_for_addition", STANDARD_EXAMPLE_OF_UPDATE_CLAUSE, - Addition("balance", wei_change as u128), + WeiChange::new("balance", wei_change as u128, WeiChangeDirection::Addition), initial, ); @@ -1059,7 +1217,7 @@ mod tests { let result = analyse_sql_commands_execution_without_details_of_overflow( "update_alone_works_for_addition_with_overflow", STANDARD_EXAMPLE_OF_UPDATE_CLAUSE, - Addition("balance", wei_change as u128), + WeiChange::new("balance", wei_change as u128, WeiChangeDirection::Addition), initial, ); @@ -1085,7 +1243,11 @@ mod tests { let result = analyse_sql_commands_execution_without_details_of_overflow( "update_alone_works_for_subtraction", STANDARD_EXAMPLE_OF_UPDATE_CLAUSE, - Subtraction("balance", wei_change.abs() as u128), + WeiChange::new( + "balance", + wei_change.abs() as u128, + WeiChangeDirection::Subtraction, + ), initial, ); @@ -1111,7 +1273,11 @@ mod tests { let result = analyse_sql_commands_execution_without_details_of_overflow( "update_alone_works_for_subtraction_with_overflow", STANDARD_EXAMPLE_OF_UPDATE_CLAUSE, - Subtraction("balance", wei_change.abs() as u128), + WeiChange::new( + "balance", + wei_change.abs() as u128, + WeiChangeDirection::Subtraction, + ), initial, ); @@ -1141,7 +1307,7 @@ mod tests { let result = analyse_sql_commands_execution_without_details_of_overflow( "early_return_for_successful_insert_works", STANDARD_EXAMPLE_OF_INSERT_CLAUSE, - Addition("balance", wei_change as u128), + WeiChange::new("balance", wei_change as u128, WeiChangeDirection::Addition), initial, ); @@ -1166,7 +1332,11 @@ mod tests { let result = analyse_sql_commands_execution_without_details_of_overflow( "early_return_for_successful_insert_works_for_subtraction", STANDARD_EXAMPLE_OF_INSERT_CLAUSE, - Subtraction("balance", wei_change.abs() as u128), + WeiChange::new( + "balance", + wei_change.abs() as u128, + WeiChangeDirection::Subtraction, + ), initial, ); @@ -1195,7 +1365,7 @@ mod tests { let result = analyse_sql_commands_execution_without_details_of_overflow( "insert_blocked_simple_update_succeeds_for_addition", STANDARD_EXAMPLE_OF_INSERT_WITH_CONFLICT_CLAUSE, - Addition("balance", wei_change as u128), + WeiChange::new("balance", wei_change as u128, WeiChangeDirection::Addition), initial, ); @@ -1220,7 +1390,11 @@ mod tests { let result = analyse_sql_commands_execution_without_details_of_overflow( "insert_blocked_simple_update_succeeds_for_subtraction", STANDARD_EXAMPLE_OF_INSERT_WITH_CONFLICT_CLAUSE, - Subtraction("balance", wei_change.abs() as u128), + WeiChange::new( + "balance", + wei_change.abs() as u128, + WeiChangeDirection::Subtraction, + ), initial, ); @@ -1246,7 +1420,7 @@ mod tests { let result = analyse_sql_commands_execution_without_details_of_overflow( "insert_blocked_update_with_overflow_for_addition", STANDARD_EXAMPLE_OF_INSERT_WITH_CONFLICT_CLAUSE, - Addition("balance", wei_change as u128), + WeiChange::new("balance", wei_change as u128, WeiChangeDirection::Addition), initial, ); @@ -1272,7 +1446,11 @@ mod tests { let result = analyse_sql_commands_execution_without_details_of_overflow( "insert_blocked_update_with_overflow_for_subtraction", STANDARD_EXAMPLE_OF_INSERT_WITH_CONFLICT_CLAUSE, - Subtraction("balance", wei_change.abs() as u128), + WeiChange::new( + "balance", + wei_change.abs() as u128, + WeiChangeDirection::Subtraction, + ), initial, ); @@ -1298,7 +1476,7 @@ mod tests { fn update_alone_works_also_for_transaction_instead_of_connection() { let initial = BigIntDivider::reconstitute(10, 20); let wei_change = BigIntDivider::reconstitute(0, 30); - let subject = BigIntDbProcessor::::default(); + let subject = BigIntDbProcessorReal::::default(); let act = |conn: &mut dyn ConnectionWrapper| { let tx = conn.transaction().unwrap(); let result = subject.execute( @@ -1308,7 +1486,11 @@ mod tests { "", SQLParamsBuilder::default() .key(test_database_key(&"Joe")) - .wei_change(Addition("balance", wei_change as u128)) + .wei_change(WeiChange::new( + "balance", + wei_change as u128, + WeiChangeDirection::Addition, + )) .build(), ), ); @@ -1341,8 +1523,8 @@ mod tests { "big_int_db_processor", "main_sql_clause_error_handled", ); - let subject = BigIntDbProcessor::::default(); - let balance_change = Addition("balance", 4879898145125); + let subject = BigIntDbProcessorReal::::default(); + let balance_change = WeiChange::new("balance", 4879898145125, WeiChangeDirection::Addition); let config = BigIntSqlConfig::new( "insert into test_table (name, balance_high_b, balance_low_b) values (:name, :balance_wrong_a, :balance_wrong_b) on conflict (name) do \ update set balance_high_b = balance_high_b + 5, balance_low_b = balance_low_b + 10 where name = :name", @@ -1353,11 +1535,11 @@ mod tests { .build(), ); - let result = subject.execute(Left(conn.as_ref()), config); + let result = subject.execute(Either::Left(conn.as_ref()), config); assert_eq!( result, - Err(BigIntDbError( + Err(BigIntDatabaseError::General( "Error from invalid upsert command for test_table table and change of 4879898145125 \ wei to 'name = Joe' with error 'Invalid parameter name: :balance_high_b'" .to_string() @@ -1371,8 +1553,8 @@ mod tests { "big_int_db_processor", "different_count_of_changed_rows_than_expected_with_update_only_configuration", ); - let subject = BigIntDbProcessor::::default(); - let balance_change = Addition("balance", 12345); + let subject = BigIntDbProcessorReal::::default(); + let balance_change = WeiChange::new("balance", 12345, WeiChangeDirection::Addition); let config = BigIntSqlConfig::new( STANDARD_EXAMPLE_OF_UPDATE_CLAUSE, "", @@ -1382,14 +1564,14 @@ mod tests { .build(), ); - let result = subject.execute(Left(conn.as_ref()), config); + let result = subject.execute(Either::Left(conn.as_ref()), config); assert_eq!( result, - Err(BigIntDbError( - "Expected 1 row to be changed for the unique key Joe but got this count: 0" - .to_string() - )) + Err(BigIntDatabaseError::RowChangeMismatch { + row_key: "Joe".to_string(), + detected_count_changed: 0 + }) ); } @@ -1410,9 +1592,9 @@ mod tests { .build(), ); - let result = BigIntDbProcessor::::default() + let result = BigIntDbProcessorReal::::default() .overflow_handler - .update_with_overflow(Left(&*conn), update_config); + .update_with_overflow(Either::Left(&*conn), update_config); assert_eq!(result, Ok(())); let (final_high_bytes, final_low_bytes) = conn @@ -1436,7 +1618,7 @@ mod tests { let (final_high_bytes, final_low_bytes) = update_with_overflow_shared_test_body( "update_with_overflow_for_addition", big_initial, - Addition("balance", big_addend as u128), + WeiChange::new("balance", big_addend as u128, WeiChangeDirection::Addition), ); assert_eq!( @@ -1457,7 +1639,11 @@ mod tests { let (final_high_bytes, final_low_bytes) = update_with_overflow_shared_test_body( "update_with_overflow_for_subtraction_from_positive_num", big_initial, - Subtraction("balance", big_subtrahend as u128), + WeiChange::new( + "balance", + big_subtrahend as u128, + WeiChangeDirection::Subtraction, + ), ); assert_eq!( @@ -1481,7 +1667,11 @@ mod tests { let (final_high_bytes, final_low_bytes) = update_with_overflow_shared_test_body( "update_with_overflow_for_subtraction_from_negative_num", -big_initial, - Subtraction("balance", big_subtrahend as u128), + WeiChange::new( + "balance", + big_subtrahend as u128, + WeiChangeDirection::Subtraction, + ), ); assert_eq!( @@ -1502,7 +1692,7 @@ mod tests { "big_int_db_processor", "update_with_overflow_handles_unspecific_error", ); - let balance_change = Addition("balance", 100); + let balance_change = WeiChange::new("balance", 100, WeiChangeDirection::Addition); let update_config = BigIntSqlConfig::new( "this can be whatever because the test fails earlier on the select stm", STANDARD_EXAMPLE_OF_OVERFLOW_UPDATE_CLAUSE, @@ -1512,14 +1702,14 @@ mod tests { .build(), ); - let result = BigIntDbProcessor::::default() + let result = BigIntDbProcessorReal::::default() .overflow_handler - .update_with_overflow(Left(conn.as_ref()), update_config); + .update_with_overflow(Either::Left(conn.as_ref()), update_config); //this kind of error is impossible in the real use case but is easiest regarding an arrangement of the test assert_eq!( result, - Err(BigIntDbError( + Err(BigIntDatabaseError::General( "Updating balance for test_table table and change of 100 wei to 'name = Joe' with \ error 'Query returned no rows'" .to_string() @@ -1538,7 +1728,7 @@ mod tests { ); insert_single_record(&*conn, [&"Joe", &60, &5555]); insert_single_record(&*conn, [&"Jodie", &77, &0]); - let balance_change = Addition("balance", 100); + let balance_change = WeiChange::new("balance", 100, WeiChangeDirection::Addition); let update_config = BigIntSqlConfig::new( "", "update test_table set balance_high_b = balance_high_b + :balance_high_b, \ @@ -1549,9 +1739,9 @@ mod tests { .build(), ); - let _ = BigIntDbProcessor::::default() + let _ = BigIntDbProcessorReal::::default() .overflow_handler - .update_with_overflow(Left(conn.as_ref()), update_config); + .update_with_overflow(Either::Left(conn.as_ref()), update_config); } #[test] @@ -1569,7 +1759,7 @@ mod tests { .execute([]) .unwrap(); insert_single_record(&*conn, [&"Joe", &60, &"bad type"]); - let balance_change = Addition("balance", 100); + let balance_change = WeiChange::new("balance", 100, WeiChangeDirection::Addition); let update_config = BigIntSqlConfig::new( "this can be whatever because the test fails earlier on the select stm", "", @@ -1579,13 +1769,13 @@ mod tests { .build(), ); - let result = BigIntDbProcessor::::default() + let result = BigIntDbProcessorReal::::default() .overflow_handler - .update_with_overflow(Left(conn.as_ref()), update_config); + .update_with_overflow(Either::Left(conn.as_ref()), update_config); assert_eq!( result, - Err(BigIntDbError( + Err(BigIntDatabaseError::General( "Updating balance for test_table table and change of 100 wei to 'name = Joe' with error \ 'Invalid column type Text at index: 1, name: balance_low_b'" .to_string() diff --git a/node/src/accountant/db_big_integer/mod.rs b/node/src/accountant/db_big_integer/mod.rs index c602cea42..d4132118e 100644 --- a/node/src/accountant/db_big_integer/mod.rs +++ b/node/src/accountant/db_big_integer/mod.rs @@ -2,5 +2,4 @@ pub mod big_int_db_processor; pub mod big_int_divider; -#[cfg(test)] -mod test_utils; +pub mod test_utils; diff --git a/node/src/accountant/db_big_integer/test_utils.rs b/node/src/accountant/db_big_integer/test_utils.rs index 2a16e4575..41ca86d84 100644 --- a/node/src/accountant/db_big_integer/test_utils.rs +++ b/node/src/accountant/db_big_integer/test_utils.rs @@ -1,9 +1,19 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. +#![cfg(test)] + +use crate::accountant::db_big_integer::big_int_db_processor::{ + BigIntDatabaseError, BigIntDbProcessor, BigIntSqlConfig, TableNameDAO, UpdateOverflowHandler, +}; +use crate::database::rusqlite_wrappers::{ConnectionWrapper, TransactionSafeWrapper}; +use itertools::Either; +use std::cell::RefCell; +use std::sync::{Arc, Mutex}; + pub(in crate::accountant::db_big_integer) mod restricted { - use crate::accountant::db_big_integer::big_int_db_processor::KnownKeyVariants::TestKey; + use crate::accountant::db_big_integer::big_int_db_processor::KeyVariants::TestKey; use crate::accountant::db_big_integer::big_int_db_processor::{ - ExtendedParamsMarker, KnownKeyVariants, + DisplayableParamValue, KeyVariants, }; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; use rusqlite::Connection; @@ -14,11 +24,70 @@ pub(in crate::accountant::db_big_integer) mod restricted { Connection::open(db_path.as_path()).unwrap() } - pub fn test_database_key<'a>(val: &'a dyn ExtendedParamsMarker) -> KnownKeyVariants<'a> { + pub fn test_database_key<'a>(val: &'a dyn DisplayableParamValue) -> KeyVariants<'a> { TestKey { - var_name: "name", - sub_name: ":name", - val, + column_name: "name", + substitution_name: ":name", + value: val, } } } + +#[derive(Default, Debug)] +pub struct BigIntDbProcessorMock { + execute_results: RefCell>>, +} + +impl BigIntDbProcessor for BigIntDbProcessorMock +where + T: TableNameDAO, +{ + fn execute<'a>( + &self, + _conn: Either<&dyn ConnectionWrapper, &TransactionSafeWrapper>, + _config: BigIntSqlConfig<'a, T>, + ) -> Result<(), BigIntDatabaseError> { + // You can implement a params capture here but so far it hasn't been needed, + // we've done well with testing on the prod code + self.execute_results.borrow_mut().remove(0) + } +} + +impl BigIntDbProcessorMock { + pub fn execute_result(self, result: Result<(), BigIntDatabaseError>) -> Self { + self.execute_results.borrow_mut().push(result); + self + } +} + +#[derive(Debug, Default)] +pub(in crate::accountant::db_big_integer) struct UpdateOverflowHandlerMock { + update_with_overflow_params: Arc>>, + update_with_overflow_results: RefCell>>, +} + +impl UpdateOverflowHandler for UpdateOverflowHandlerMock +where + T: TableNameDAO, +{ + fn update_with_overflow<'a>( + &self, + _conn: Either<&dyn ConnectionWrapper, &TransactionSafeWrapper>, + _config: BigIntSqlConfig<'a, T>, + ) -> Result<(), BigIntDatabaseError> { + self.update_with_overflow_params.lock().unwrap().push(()); + self.update_with_overflow_results.borrow_mut().remove(0) + } +} + +impl UpdateOverflowHandlerMock { + pub fn update_with_overflow_params(mut self, params: &Arc>>) -> Self { + self.update_with_overflow_params = params.clone(); + self + } + + pub fn update_with_overflow_result(self, result: Result<(), BigIntDatabaseError>) -> Self { + self.update_with_overflow_results.borrow_mut().push(result); + self + } +} diff --git a/node/src/accountant/mod.rs b/node/src/accountant/mod.rs index cffa9f49e..29562d7f1 100644 --- a/node/src/accountant/mod.rs +++ b/node/src/accountant/mod.rs @@ -123,6 +123,7 @@ pub struct ReceivedPayments { // a problem? Do we want to correct the timestamp? Discuss. pub timestamp: SystemTime, pub payments: Vec, + pub new_start_block: u64, pub response_skeleton_opt: Option, } @@ -1001,14 +1002,18 @@ mod tests { }; use crate::accountant::test_utils::{ bc_from_earning_wallet, bc_from_wallets, make_payable_account, make_payables, - BannedDaoFactoryMock, MessageIdGeneratorMock, NullScanner, PayableDaoFactoryMock, - PayableDaoMock, PayableScannerBuilder, PaymentAdjusterMock, PendingPayableDaoFactoryMock, - PendingPayableDaoMock, ReceivableDaoFactoryMock, ReceivableDaoMock, ScannerMock, + BannedDaoFactoryMock, ConfigDaoFactoryMock, MessageIdGeneratorMock, NullScanner, + PayableDaoFactoryMock, PayableDaoMock, PayableScannerBuilder, PaymentAdjusterMock, + PendingPayableDaoFactoryMock, PendingPayableDaoMock, ReceivableDaoFactoryMock, + ReceivableDaoMock, ScannerMock, }; use crate::accountant::test_utils::{AccountantBuilder, BannedDaoMock}; use crate::accountant::Accountant; use crate::blockchain::blockchain_bridge::BlockchainBridge; 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::mocks::ConfigDaoMock; use crate::match_every_type_id; use crate::sub_lib::accountant::{ ExitServiceConsumed, PaymentThresholds, RoutingServiceConsumed, ScanIntervals, @@ -1080,6 +1085,7 @@ mod tests { let pending_payable_dao_factory_params_arc = Arc::new(Mutex::new(vec![])); let receivable_dao_factory_params_arc = Arc::new(Mutex::new(vec![])); let banned_dao_factory_params_arc = Arc::new(Mutex::new(vec![])); + let config_dao_factory_params_arc = Arc::new(Mutex::new(vec![])); let payable_dao_factory = PayableDaoFactoryMock::new() .make_params(&payable_dao_factory_params_arc) .make_result(PayableDaoMock::new()) // For Accountant @@ -1097,6 +1103,9 @@ mod tests { let banned_dao_factory = BannedDaoFactoryMock::new() .make_params(&banned_dao_factory_params_arc) .make_result(BannedDaoMock::new()); // For Receivable Scanner + let config_dao_factory = ConfigDaoFactoryMock::new() + .make_params(&config_dao_factory_params_arc) + .make_result(ConfigDaoMock::new()); // For receivable scanner let _ = Accountant::new( config, @@ -1105,6 +1114,7 @@ mod tests { pending_payable_dao_factory: Box::new(pending_payable_dao_factory), receivable_dao_factory: Box::new(receivable_dao_factory), banned_dao_factory: Box::new(banned_dao_factory), + config_dao_factory: Box::new(config_dao_factory), }, ); @@ -1121,6 +1131,7 @@ mod tests { vec![(), ()] ); assert_eq!(*banned_dao_factory_params_arc.lock().unwrap(), vec![()]); + assert_eq!(*config_dao_factory_params_arc.lock().unwrap(), vec![()]); } #[test] @@ -1145,6 +1156,8 @@ mod tests { ); let banned_dao_factory = Box::new(BannedDaoFactoryMock::new().make_result(BannedDaoMock::new())); + let config_dao_factory = + Box::new(ConfigDaoFactoryMock::new().make_result(ConfigDaoMock::new())); let result = Accountant::new( bootstrapper_config, @@ -1153,6 +1166,7 @@ mod tests { pending_payable_dao_factory, receivable_dao_factory, banned_dao_factory, + config_dao_factory, }, ); @@ -1252,6 +1266,7 @@ mod tests { config.suppress_initial_scans = true; let subject = AccountantBuilder::default() .bootstrapper_config(config) + .config_dao(ConfigDaoMock::new().set_result(Ok(()))) .build(); let (ui_gateway, _, ui_gateway_recording_arc) = make_recorder(); let subject_addr = subject.start(); @@ -1261,6 +1276,7 @@ mod tests { let received_payments = ReceivedPayments { timestamp: SystemTime::now(), payments: vec![], + new_start_block: 1234567, response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321, @@ -1496,11 +1512,12 @@ mod tests { let agent_id_stamp_first_phase = ArbitraryIdStamp::new(); let agent = BlockchainAgentMock::default().set_arbitrary_id_stamp(agent_id_stamp_first_phase); - let payable_payments_setup_msg = BlockchainAgentWithContextMessage { - protected_qualified_payables: protect_payables_in_test(vec![ - unadjusted_account_1.clone(), - unadjusted_account_2.clone(), - ]), + let initial_unadjusted_accounts = protect_payables_in_test(vec![ + unadjusted_account_1.clone(), + unadjusted_account_2.clone(), + ]); + let msg = BlockchainAgentWithContextMessage { + protected_qualified_payables: initial_unadjusted_accounts.clone(), agent: Box::new(agent), response_skeleton_opt: Some(response_skeleton), }; @@ -1528,7 +1545,7 @@ mod tests { let subject_addr = subject.start(); let system = System::new("test"); - subject_addr.try_send(payable_payments_setup_msg).unwrap(); + subject_addr.try_send(msg).unwrap(); let before = SystemTime::now(); assert_eq!(system.run(), 0); @@ -1541,7 +1558,7 @@ mod tests { actual_prepared_adjustment .original_setup_msg .protected_qualified_payables, - protect_payables_in_test(vec![unadjusted_account_1, unadjusted_account_2]) + initial_unadjusted_accounts ); assert_eq!( actual_prepared_adjustment @@ -1849,7 +1866,11 @@ mod tests { } #[test] - fn accountant_uses_receivables_dao_to_process_received_payments() { + fn accountant_processes_msg_with_received_payments_using_receivables_dao_and_then_updates_start_block( + ) { + let more_money_received_params_arc = Arc::new(Mutex::new(vec![])); + let commit_params_arc = Arc::new(Mutex::new(vec![])); + let set_by_guest_transaction_params_arc = Arc::new(Mutex::new(vec![])); let now = SystemTime::now(); let earning_wallet = make_wallet("earner3000"); let expected_receivable_1 = BlockchainTransaction { @@ -1862,13 +1883,22 @@ mod tests { from: make_wallet("wallet1"), wei_amount: 10000, }; - let more_money_received_params_arc = Arc::new(Mutex::new(vec![])); + let transaction_id = ArbitraryIdStamp::new(); + let txn_inner_builder = TransactionInnerWrapperMockBuilder::default() + .commit_params(&commit_params_arc) + .commit_result(Ok(())) + .set_arbitrary_id_stamp(transaction_id); + let wrapped_transaction = TransactionSafeWrapper::new_with_builder(txn_inner_builder); let receivable_dao = ReceivableDaoMock::new() - .more_money_received_parameters(&more_money_received_params_arc) - .more_money_received_result(Ok(())); + .more_money_received_params(&more_money_received_params_arc) + .more_money_received_result(wrapped_transaction); + let config_dao = ConfigDaoMock::new() + .set_by_guest_transaction_params(&set_by_guest_transaction_params_arc) + .set_by_guest_transaction_result(Ok(())); let accountant = AccountantBuilder::default() .bootstrapper_config(bc_from_earning_wallet(earning_wallet.clone())) .receivable_daos(vec![ForReceivableScanner(receivable_dao)]) + .config_dao(config_dao) .build(); let system = System::new("accountant_uses_receivables_dao_to_process_received_payments"); let subject = accountant.start(); @@ -1877,6 +1907,7 @@ mod tests { .try_send(ReceivedPayments { timestamp: now, payments: vec![expected_receivable_1.clone(), expected_receivable_2.clone()], + new_start_block: 123456789, response_skeleton_opt: None, }) .expect("unexpected actix error"); @@ -1887,6 +1918,17 @@ mod tests { assert_eq!( *more_money_received_params, vec![(now, vec![expected_receivable_1, expected_receivable_2])] + ); + let commit_params = commit_params_arc.lock().unwrap(); + assert_eq!(*commit_params, vec![()]); + let set_by_guest_transaction_params = set_by_guest_transaction_params_arc.lock().unwrap(); + assert_eq!( + *set_by_guest_transaction_params, + vec![( + transaction_id, + "start_block".to_string(), + Some("123456789".to_string()) + )] ) } diff --git a/node/src/accountant/payment_adjuster.rs b/node/src/accountant/payment_adjuster.rs index 8d230484a..88ee13e74 100644 --- a/node/src/accountant/payment_adjuster.rs +++ b/node/src/accountant/payment_adjuster.rs @@ -20,7 +20,7 @@ pub trait PaymentAdjuster { logger: &Logger, ) -> OutboundPaymentsInstructions; - as_any_in_trait!(); + as_any_ref_in_trait!(); } pub struct PaymentAdjusterReal {} @@ -43,7 +43,7 @@ impl PaymentAdjuster for PaymentAdjusterReal { todo!("this function is dead until the card GH-711 is played") } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } impl PaymentAdjusterReal { diff --git a/node/src/accountant/scanners/mid_scan_msg_handling/payable_scanner/agent_null.rs b/node/src/accountant/scanners/mid_scan_msg_handling/payable_scanner/agent_null.rs index 57615ba30..faf7d45cc 100644 --- a/node/src/accountant/scanners/mid_scan_msg_handling/payable_scanner/agent_null.rs +++ b/node/src/accountant/scanners/mid_scan_msg_handling/payable_scanner/agent_null.rs @@ -48,7 +48,7 @@ impl BlockchainAgent for BlockchainAgentNull { } #[cfg(test)] - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } impl BlockchainAgentNull { diff --git a/node/src/accountant/scanners/mid_scan_msg_handling/payable_scanner/blockchain_agent.rs b/node/src/accountant/scanners/mid_scan_msg_handling/payable_scanner/blockchain_agent.rs index 704704457..3ded6a16b 100644 --- a/node/src/accountant/scanners/mid_scan_msg_handling/payable_scanner/blockchain_agent.rs +++ b/node/src/accountant/scanners/mid_scan_msg_handling/payable_scanner/blockchain_agent.rs @@ -32,6 +32,6 @@ pub trait BlockchainAgent: Send { fn dup(&self) -> Box { intentionally_blank!() } - as_any_in_trait!(); + as_any_ref_in_trait!(); arbitrary_id_stamp_in_trait!(); } diff --git a/node/src/accountant/scanners/mod.rs b/node/src/accountant/scanners/mod.rs index 0e4174e83..2d5b1b7f4 100644 --- a/node/src/accountant/scanners/mod.rs +++ b/node/src/accountant/scanners/mod.rs @@ -46,8 +46,6 @@ use masq_lib::logger::TIME_FORMATTING_STRING; use masq_lib::messages::{ScanType, ToMessageBody, UiScanResponse}; use masq_lib::ui_gateway::{MessageTarget, NodeToUiMessage}; use masq_lib::utils::ExpectValue; -#[cfg(test)] -use std::any::Any; use std::cell::RefCell; use std::collections::HashMap; use std::rc::Rc; @@ -59,6 +57,7 @@ use masq_lib::type_obfuscation::Obfuscated; use crate::accountant::scanners::mid_scan_msg_handling::payable_scanner::{PreparedAdjustment, MultistagePayableScanner, SolvencySensitivePaymentInstructor}; use crate::accountant::scanners::mid_scan_msg_handling::payable_scanner::msgs::{BlockchainAgentWithContextMessage, QualifiedPayablesMessage}; use crate::blockchain::blockchain_interface::data_structures::errors::PayableTransactionError; +use crate::db_config::persistent_configuration::{PersistentConfiguration, PersistentConfigurationReal}; pub struct Scanners { pub payable: Box>, @@ -74,27 +73,36 @@ impl Scanners { when_pending_too_long_sec: u64, financial_statistics: Rc>, ) -> Self { + let payable = Box::new(PayableScanner::new( + dao_factories.payable_dao_factory.make(), + dao_factories.pending_payable_dao_factory.make(), + Rc::clone(&payment_thresholds), + Box::new(PaymentAdjusterReal::new()), + )); + + let pending_payable = Box::new(PendingPayableScanner::new( + dao_factories.payable_dao_factory.make(), + dao_factories.pending_payable_dao_factory.make(), + Rc::clone(&payment_thresholds), + when_pending_too_long_sec, + Rc::clone(&financial_statistics), + )); + + let persistent_configuration = + PersistentConfigurationReal::from(dao_factories.config_dao_factory.make()); + let receivable = Box::new(ReceivableScanner::new( + dao_factories.receivable_dao_factory.make(), + dao_factories.banned_dao_factory.make(), + Box::new(persistent_configuration), + Rc::clone(&payment_thresholds), + earning_wallet, + financial_statistics, + )); + Scanners { - payable: Box::new(PayableScanner::new( - dao_factories.payable_dao_factory.make(), - dao_factories.pending_payable_dao_factory.make(), - Rc::clone(&payment_thresholds), - Box::new(PaymentAdjusterReal::new()), - )), - pending_payable: Box::new(PendingPayableScanner::new( - dao_factories.payable_dao_factory.make(), - dao_factories.pending_payable_dao_factory.make(), - Rc::clone(&payment_thresholds), - when_pending_too_long_sec, - Rc::clone(&financial_statistics), - )), - receivable: Box::new(ReceivableScanner::new( - dao_factories.receivable_dao_factory.make(), - dao_factories.banned_dao_factory.make(), - Rc::clone(&payment_thresholds), - earning_wallet, - financial_statistics, - )), + payable, + pending_payable, + receivable, } } } @@ -115,7 +123,8 @@ where fn mark_as_started(&mut self, timestamp: SystemTime); fn mark_as_ended(&mut self, logger: &Logger); - as_any_in_trait!(); + as_any_ref_in_trait!(); + as_any_mut_in_trait!(); } pub struct ScannerCommon { @@ -249,7 +258,7 @@ impl Scanner for PayableScanner { time_marking_methods!(Payables); - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } impl SolvencySensitivePaymentInstructor for PayableScanner { @@ -618,7 +627,7 @@ impl Scanner for PendingP time_marking_methods!(PendingPayables); - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } impl PendingPayableScanner { @@ -817,6 +826,7 @@ pub struct ReceivableScanner { pub common: ScannerCommon, pub receivable_dao: Box, pub banned_dao: Box, + pub persistent_configuration: Box, pub earning_wallet: Rc, pub financial_statistics: Rc>, } @@ -844,32 +854,29 @@ impl Scanner for ReceivableScanner { }) } - fn finish_scan( - &mut self, - message: ReceivedPayments, - logger: &Logger, - ) -> Option { - if message.payments.is_empty() { + fn finish_scan(&mut self, msg: ReceivedPayments, logger: &Logger) -> Option { + if msg.payments.is_empty() { info!( logger, - "No new received payments were detected during the scanning process." - ) + "No newly received payments were detected during the scanning process." + ); + + match self + .persistent_configuration + .set_start_block(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 { - let total_newly_paid_receivable = message - .payments - .iter() - .fold(0, |so_far, now| so_far + now.wei_amount); - self.receivable_dao - .as_mut() - .more_money_received(message.timestamp, message.payments); - self.financial_statistics - .borrow_mut() - .total_paid_receivable_wei += total_newly_paid_receivable; + self.handle_new_received_payments(&msg, logger) } self.mark_as_ended(logger); - message - .response_skeleton_opt + msg.response_skeleton_opt .map(|response_skeleton| NodeToUiMessage { target: MessageTarget::ClientId(response_skeleton.client_id), body: UiScanResponse {}.tmb(response_skeleton.context_id), @@ -878,13 +885,15 @@ impl Scanner for ReceivableScanner { time_marking_methods!(Receivables); - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); + as_any_mut_in_trait_impl!(); } impl ReceivableScanner { pub fn new( receivable_dao: Box, banned_dao: Box, + persistent_configuration: Box, payment_thresholds: Rc, earning_wallet: Rc, financial_statistics: Rc>, @@ -894,10 +903,46 @@ impl ReceivableScanner { earning_wallet, receivable_dao, banned_dao, + persistent_configuration, financial_statistics, } } + fn handle_new_received_payments(&mut self, msg: &ReceivedPayments, logger: &Logger) { + let mut txn = self + .receivable_dao + .as_mut() + .more_money_received(msg.timestamp, &msg.payments); + + let new_start_block = msg.new_start_block; + match self + .persistent_configuration + .set_start_block_from_txn(new_start_block, &mut txn) + { + Ok(()) => (), + Err(e) => panic!( + "Attempt to set new start block to {} failed due to: {:?}", + new_start_block, e + ), + } + + match txn.commit() { + Ok(_) => { + debug!(logger, "Updated start block to: {}", new_start_block) + } + Err(e) => panic!("Commit of received transactions failed: {:?}", e), + } + + let total_newly_paid_receivable = msg + .payments + .iter() + .fold(0, |so_far, now| so_far + now.wei_amount); + + self.financial_statistics + .borrow_mut() + .total_paid_receivable_wei += total_newly_paid_receivable; + } + pub fn scan_for_delinquencies(&self, timestamp: SystemTime, logger: &Logger) { info!(logger, "Scanning for delinquencies"); self.find_and_ban_delinquents(timestamp, logger); @@ -1033,10 +1078,8 @@ pub trait ScanScheduler { intentionally_blank!() } - as_any_in_trait!(); - - #[cfg(test)] - fn as_any_mut(&mut self) -> &mut dyn Any; + as_any_ref_in_trait!(); + as_any_mut_in_trait!(); } impl ScanScheduler for PeriodicalScanScheduler { @@ -1049,14 +1092,9 @@ impl ScanScheduler for PeriodicalScanScheduler { self.interval } - as_any_in_trait_impl!(); - - #[cfg(test)] - fn as_any_mut(&mut self) -> &mut dyn Any { - self - } + as_any_ref_in_trait_impl!(); + as_any_mut_in_trait_impl!(); } - #[cfg(test)] mod tests { use crate::accountant::db_access_objects::payable_dao::{PayableAccount, PayableDaoError}; @@ -1075,10 +1113,10 @@ mod tests { use crate::accountant::test_utils::{ make_custom_payment_thresholds, make_payable_account, make_payables, make_pending_payable_fingerprint, make_receivable_account, BannedDaoFactoryMock, - BannedDaoMock, PayableDaoFactoryMock, PayableDaoMock, PayableScannerBuilder, - PayableThresholdsGaugeMock, PendingPayableDaoFactoryMock, PendingPayableDaoMock, - PendingPayableScannerBuilder, ReceivableDaoFactoryMock, ReceivableDaoMock, - ReceivableScannerBuilder, + BannedDaoMock, ConfigDaoFactoryMock, PayableDaoFactoryMock, PayableDaoMock, + PayableScannerBuilder, PayableThresholdsGaugeMock, PendingPayableDaoFactoryMock, + PendingPayableDaoMock, PendingPayableScannerBuilder, ReceivableDaoFactoryMock, + ReceivableDaoMock, ReceivableScannerBuilder, }; use crate::accountant::{ gwei_to_wei, PendingPayableId, ReceivedPayments, ReportTransactionReceipts, @@ -1090,17 +1128,24 @@ mod tests { BlockchainTransaction, RpcPayablesFailure, }; use crate::blockchain::test_utils::make_tx_hash; + use crate::database::rusqlite_wrappers::TransactionSafeWrapper; + use crate::database::test_utils::transaction_wrapper_mock::TransactionInnerWrapperMockBuilder; + use crate::db_config::mocks::ConfigDaoMock; + use crate::db_config::persistent_configuration::PersistentConfigError; use crate::sub_lib::accountant::{ DaoFactories, FinancialStatistics, PaymentThresholds, ScanIntervals, DEFAULT_PAYMENT_THRESHOLDS, }; use crate::test_utils::make_wallet; + use crate::test_utils::persistent_configuration_mock::PersistentConfigurationMock; + use crate::test_utils::unshared_test_utils::arbitrary_id_stamp::ArbitraryIdStamp; use actix::{Message, System}; use ethereum_types::U64; use masq_lib::logger::Logger; use masq_lib::messages::ScanType; use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler}; use regex::Regex; + use rusqlite::{ffi, ErrorCode}; use std::cell::RefCell; use std::ops::Sub; use std::panic::{catch_unwind, AssertUnwindSafe}; @@ -1118,9 +1163,14 @@ mod tests { let pending_payable_dao_factory = PendingPayableDaoFactoryMock::new() .make_result(PendingPayableDaoMock::new()) .make_result(PendingPayableDaoMock::new()); - let receivable_dao_factory = - ReceivableDaoFactoryMock::new().make_result(ReceivableDaoMock::new()); + let receivable_dao = ReceivableDaoMock::new(); + let receivable_dao_factory = ReceivableDaoFactoryMock::new().make_result(receivable_dao); let banned_dao_factory = BannedDaoFactoryMock::new().make_result(BannedDaoMock::new()); + let set_params_arc = Arc::new(Mutex::new(vec![])); + let config_dao_mock = ConfigDaoMock::new() + .set_params(&set_params_arc) + .set_result(Ok(())); + let config_dao_factory = ConfigDaoFactoryMock::new().make_result(config_dao_mock); let when_pending_too_long_sec = 1234; let financial_statistics = FinancialStatistics { total_paid_payable_wei: 1, @@ -1131,12 +1181,13 @@ mod tests { let payment_thresholds_rc = Rc::new(payment_thresholds); let initial_rc_count = Rc::strong_count(&payment_thresholds_rc); - let scanners = Scanners::new( + let mut scanners = Scanners::new( DaoFactories { payable_dao_factory: Box::new(payable_dao_factory), pending_payable_dao_factory: Box::new(pending_payable_dao_factory), receivable_dao_factory: Box::new(receivable_dao_factory), banned_dao_factory: Box::new(banned_dao_factory), + config_dao_factory: Box::new(config_dao_factory), }, Rc::clone(&payment_thresholds_rc), Rc::new(earning_wallet.clone()), @@ -1156,8 +1207,8 @@ mod tests { .unwrap(); let receivable_scanner = scanners .receivable - .as_any() - .downcast_ref::() + .as_any_mut() + .downcast_mut::() .unwrap(); assert_eq!( payable_scanner.common.payment_thresholds.as_ref(), @@ -1193,6 +1244,15 @@ mod tests { &payment_thresholds ); assert_eq!(receivable_scanner.common.initiated_at_opt.is_some(), false); + receivable_scanner + .persistent_configuration + .set_start_block(136890) + .unwrap(); + let set_params = set_params_arc.lock().unwrap(); + assert_eq!( + *set_params, + vec![("start_block".to_string(), Some("136890".to_string()))] + ); assert_eq!( Rc::strong_count(&payment_thresholds_rc), initial_rc_count + 3 @@ -2947,22 +3007,57 @@ mod tests { } #[test] - fn receivable_scanner_aborts_scan_if_no_payments_were_supplied() { + fn receivable_scanner_handles_no_new_payments_found() { init_test_logging(); let test_name = "receivable_scanner_aborts_scan_if_no_payments_were_supplied"; - let mut subject = ReceivableScannerBuilder::new().build(); + let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); + let new_start_block = 4321; + let persistent_config = PersistentConfigurationMock::new() + .set_start_block_params(&set_start_block_params_arc) + .set_start_block_result(Ok(())); + let mut subject = ReceivableScannerBuilder::new() + .persistent_configuration(persistent_config) + .build(); let msg = ReceivedPayments { timestamp: SystemTime::now(), payments: vec![], + new_start_block, response_skeleton_opt: None, }; let message_opt = subject.finish_scan(msg, &Logger::new(test_name)); 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]); TestLogHandler::new().exists_log_containing(&format!( - "INFO: {test_name}: No new received payments were detected during the scanning process." + "INFO: {test_name}: No newly received payments were detected during the scanning process." + )); + } + + #[test] + #[should_panic(expected = "Attempt to set new start block to 6709 failed due to: \ + UninterpretableValue(\"Illiterate database manager\")")] + fn no_transactions_received_but_start_block_setting_fails() { + 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 mut subject = ReceivableScannerBuilder::new() + .persistent_configuration(persistent_config) + .build(); + let msg = ReceivedPayments { + timestamp: now, + payments: vec![], + new_start_block: 6709, + response_skeleton_opt: None, + }; + // Not necessary, rather for preciseness + subject.mark_as_started(SystemTime::now()); + + subject.finish_scan(msg, &Logger::new(test_name)); } #[test] @@ -2971,11 +3066,23 @@ mod tests { let test_name = "receivable_scanner_handles_received_payments_message"; let now = SystemTime::now(); let more_money_received_params_arc = Arc::new(Mutex::new(vec![])); + let set_start_block_from_txn_params_arc = Arc::new(Mutex::new(vec![])); + let commit_params_arc = Arc::new(Mutex::new(vec![])); + let transaction_id = ArbitraryIdStamp::new(); + let txn_inner_builder = TransactionInnerWrapperMockBuilder::default() + .commit_params(&commit_params_arc) + .commit_result(Ok(())) + .set_arbitrary_id_stamp(transaction_id); + let transaction = TransactionSafeWrapper::new_with_builder(txn_inner_builder); + let persistent_config = PersistentConfigurationMock::new() + .set_start_block_from_txn_params(&set_start_block_from_txn_params_arc) + .set_start_block_from_txn_result(Ok(())); let receivable_dao = ReceivableDaoMock::new() - .more_money_received_parameters(&more_money_received_params_arc) - .more_money_receivable_result(Ok(())); + .more_money_received_params(&more_money_received_params_arc) + .more_money_received_result(transaction); let mut subject = ReceivableScannerBuilder::new() .receivable_dao(receivable_dao) + .persistent_configuration(persistent_config) .build(); let mut financial_statistics = subject.financial_statistics.borrow().clone(); financial_statistics.total_paid_receivable_wei += 2_222_123_123; @@ -2995,6 +3102,7 @@ mod tests { let msg = ReceivedPayments { timestamp: now, payments: receivables.clone(), + new_start_block: 7890123, response_skeleton_opt: None, }; subject.mark_as_started(SystemTime::now()); @@ -3005,16 +3113,100 @@ mod tests { .financial_statistics .borrow() .total_paid_receivable_wei; - let more_money_received_params = more_money_received_params_arc.lock().unwrap(); assert_eq!(message_opt, None); assert_eq!(subject.scan_started_at(), None); assert_eq!(total_paid_receivable, 2_222_123_123 + 45_780 + 3_333_345); + let more_money_received_params = more_money_received_params_arc.lock().unwrap(); assert_eq!(*more_money_received_params, vec![(now, receivables)]); + let set_by_guest_transaction_params = set_start_block_from_txn_params_arc.lock().unwrap(); + assert_eq!( + *set_by_guest_transaction_params, + vec![(7890123, transaction_id)] + ); + let commit_params = commit_params_arc.lock().unwrap(); + assert_eq!(*commit_params, vec![()]); TestLogHandler::new().exists_log_matching( "INFO: receivable_scanner_handles_received_payments_message: The Receivables scan ended in \\d+ms.", ); } + #[test] + #[should_panic(expected = "Attempt to set new start block to 7890123 failed due to: \ + DatabaseError(\"Fatigue\")")] + fn received_transactions_processed_but_start_block_setting_fails() { + init_test_logging(); + let test_name = "received_transactions_processed_but_start_block_setting_fails"; + 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 receivable_dao = ReceivableDaoMock::new().more_money_received_result(transaction); + let mut subject = ReceivableScannerBuilder::new() + .receivable_dao(receivable_dao) + .persistent_configuration(persistent_config) + .build(); + let receivables = vec![BlockchainTransaction { + block_number: 4578910, + from: make_wallet("abc"), + wei_amount: 45_780, + }]; + let msg = ReceivedPayments { + timestamp: now, + payments: receivables, + new_start_block: 7890123, + response_skeleton_opt: None, + }; + // Not necessary, rather for preciseness + subject.mark_as_started(SystemTime::now()); + + subject.finish_scan(msg, &Logger::new(test_name)); + } + + #[test] + #[should_panic( + expected = "Commit of received transactions failed: SqliteFailure(Error { code: \ + InternalMalfunction, extended_code: 0 }, Some(\"blah\"))" + )] + fn transaction_for_balance_start_block_updates_fails_on_its_commit() { + init_test_logging(); + let test_name = "transaction_for_balance_start_block_updates_fails_on_its_commit"; + let now = SystemTime::now(); + let commit_err = Err(rusqlite::Error::SqliteFailure( + ffi::Error { + code: ErrorCode::InternalMalfunction, + extended_code: 0, + }, + Some("blah".to_string()), + )); + 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 receivable_dao = ReceivableDaoMock::new().more_money_received_result(transaction); + let mut subject = ReceivableScannerBuilder::new() + .receivable_dao(receivable_dao) + .persistent_configuration(persistent_config) + .build(); + let receivables = vec![BlockchainTransaction { + block_number: 4578910, + from: make_wallet("abc"), + wei_amount: 45_780, + }]; + let msg = ReceivedPayments { + timestamp: now, + payments: receivables, + new_start_block: 7890123, + response_skeleton_opt: None, + }; + // Not necessary, rather for preciseness + subject.mark_as_started(SystemTime::now()); + + subject.finish_scan(msg, &Logger::new(test_name)); + } + #[test] fn signal_scanner_completion_and_log_if_timestamp_is_correct() { init_test_logging(); diff --git a/node/src/accountant/scanners/scanners_utils.rs b/node/src/accountant/scanners/scanners_utils.rs index a68718a36..e1c18707a 100644 --- a/node/src/accountant/scanners/scanners_utils.rs +++ b/node/src/accountant/scanners/scanners_utils.rs @@ -20,7 +20,6 @@ pub mod payable_scanner_utils { use crate::accountant::db_access_objects::pending_payable_dao::PendingPayable; use crate::blockchain::blockchain_interface::data_structures::errors::PayableTransactionError; use crate::blockchain::blockchain_interface::data_structures::{RpcPayablesFailure}; - pub type VecOfRowidOptAndHash = Vec<(Option, H256)>; #[derive(Debug, PartialEq, Eq)] @@ -291,7 +290,7 @@ pub mod payable_scanner_utils { payment_thresholds: &PaymentThresholds, x: u64, ) -> u128; - as_any_in_trait!(); + as_any_ref_in_trait!(); } #[derive(Default)] @@ -313,7 +312,7 @@ pub mod payable_scanner_utils { ) -> u128 { ThresholdUtils::calculate_finite_debt_limit_by_age(payment_thresholds, debt_age) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } } diff --git a/node/src/accountant/test_utils.rs b/node/src/accountant/test_utils.rs index f63370578..cb4106c82 100644 --- a/node/src/accountant/test_utils.rs +++ b/node/src/accountant/test_utils.rs @@ -32,6 +32,7 @@ use crate::blockchain::blockchain_bridge::PendingPayableFingerprint; use crate::blockchain::blockchain_interface::data_structures::BlockchainTransaction; use crate::blockchain::test_utils::make_tx_hash; use crate::bootstrapper::BootstrapperConfig; +use crate::database::rusqlite_wrappers::TransactionSafeWrapper; use crate::db_config::config_dao::{ConfigDao, ConfigDaoFactory}; use crate::db_config::mocks::ConfigDaoMock; use crate::sub_lib::accountant::{DaoFactories, FinancialStatistics}; @@ -40,6 +41,7 @@ use crate::sub_lib::blockchain_bridge::OutboundPaymentsInstructions; use crate::sub_lib::utils::NotifyLaterHandle; use crate::sub_lib::wallet::Wallet; use crate::test_utils::make_wallet; +use crate::test_utils::persistent_configuration_mock::PersistentConfigurationMock; use crate::test_utils::unshared_test_utils::make_bc_with_defaults; use actix::{Message, System}; use ethereum_types::H256; @@ -47,10 +49,11 @@ use itertools::Either; use masq_lib::logger::Logger; use masq_lib::messages::ScanType; use masq_lib::ui_gateway::NodeToUiMessage; -use rusqlite::{Connection, Row}; +use rusqlite::{Connection, OpenFlags, Row}; use std::any::type_name; use std::cell::RefCell; use std::fmt::Debug; +use std::path::Path; use std::rc::Rc; use std::sync::{Arc, Mutex}; use std::time::{Duration, SystemTime}; @@ -98,7 +101,7 @@ pub struct AccountantBuilder { receivable_dao_factory: Option, pending_payable_dao_factory: Option, banned_dao_factory: Option, - config_dao_factory: Option>, + config_dao_factory: Option, } impl Default for AccountantBuilder { @@ -324,7 +327,7 @@ impl AccountantBuilder { } pub fn config_dao(mut self, config_dao: ConfigDaoMock) -> Self { - self.config_dao_factory = Some(Box::new(ConfigDaoFactoryMock::new(config_dao))); + self.config_dao_factory = Some(ConfigDaoFactoryMock::new().make_result(config_dao)); self } @@ -350,6 +353,9 @@ impl AccountantBuilder { let banned_dao_factory = self .banned_dao_factory .unwrap_or(BannedDaoFactoryMock::new().make_result(BannedDaoMock::new())); + let config_dao_factory = self + .config_dao_factory + .unwrap_or(ConfigDaoFactoryMock::new().make_result(ConfigDaoMock::new())); let mut accountant = Accountant::new( config, DaoFactories { @@ -357,6 +363,7 @@ impl AccountantBuilder { pending_payable_dao_factory: Box::new(pending_payable_dao_factory), receivable_dao_factory: Box::new(receivable_dao_factory), banned_dao_factory: Box::new(banned_dao_factory), + config_dao_factory: Box::new(config_dao_factory), }, ); if let Some(logger) = self.logger { @@ -474,27 +481,32 @@ impl BannedDaoFactoryMock { } pub struct ConfigDaoFactoryMock { - called: Rc>, - mock: RefCell>, + make_params: Arc>>, + make_results: RefCell>>, } impl ConfigDaoFactory for ConfigDaoFactoryMock { fn make(&self) -> Box { - *self.called.borrow_mut() = true; - Box::new(self.mock.borrow_mut().take().unwrap()) + self.make_params.lock().unwrap().push(()); + self.make_results.borrow_mut().remove(0) } } impl ConfigDaoFactoryMock { - pub fn new(mock: ConfigDaoMock) -> Self { + pub fn new() -> Self { Self { - called: Rc::new(RefCell::new(false)), - mock: RefCell::new(Some(mock)), + make_params: Arc::new(Mutex::new(vec![])), + make_results: RefCell::new(vec![]), } } - pub fn called(mut self, called: &Rc>) -> Self { - self.called = called.clone(); + pub fn make_params(mut self, params: &Arc>>) -> Self { + self.make_params = params.clone(); + self + } + + pub fn make_result(self, result: ConfigDaoMock) -> Self { + self.make_results.borrow_mut().push(Box::new(result)); self } } @@ -656,7 +668,7 @@ pub struct ReceivableDaoMock { more_money_receivable_parameters: Arc>>, more_money_receivable_results: RefCell>>, more_money_received_parameters: Arc)>>>, - more_money_received_results: RefCell>>, + more_money_received_results: RefCell>>, new_delinquencies_parameters: Arc>>, new_delinquencies_results: RefCell>>, paid_delinquencies_parameters: Arc>>, @@ -680,11 +692,16 @@ impl ReceivableDao for ReceivableDaoMock { self.more_money_receivable_results.borrow_mut().remove(0) } - fn more_money_received(&mut self, now: SystemTime, transactions: Vec) { + fn more_money_received( + &mut self, + now: SystemTime, + transactions: &[BlockchainTransaction], + ) -> TransactionSafeWrapper { self.more_money_received_parameters .lock() .unwrap() - .push((now, transactions)); + .push((now, transactions.to_vec())); + self.more_money_received_results.borrow_mut().remove(0) } fn new_delinquencies( @@ -740,7 +757,7 @@ impl ReceivableDaoMock { self } - pub fn more_money_received_parameters( + pub fn more_money_received_params( mut self, parameters: &Arc)>>>, ) -> Self { @@ -748,7 +765,7 @@ impl ReceivableDaoMock { self } - pub fn more_money_received_result(self, result: Result<(), PayableDaoError>) -> Self { + pub fn more_money_received_result(self, result: TransactionSafeWrapper<'static>) -> Self { self.more_money_received_results.borrow_mut().push(result); self } @@ -1157,6 +1174,7 @@ impl PendingPayableScannerBuilder { pub struct ReceivableScannerBuilder { receivable_dao: ReceivableDaoMock, banned_dao: BannedDaoMock, + persistent_configuration: PersistentConfigurationMock, payment_thresholds: PaymentThresholds, earning_wallet: Wallet, financial_statistics: FinancialStatistics, @@ -1167,6 +1185,7 @@ impl ReceivableScannerBuilder { Self { receivable_dao: ReceivableDaoMock::new(), banned_dao: BannedDaoMock::new(), + persistent_configuration: PersistentConfigurationMock::new(), payment_thresholds: PaymentThresholds::default(), earning_wallet: make_wallet("earning_default"), financial_statistics: FinancialStatistics::default(), @@ -1178,6 +1197,14 @@ impl ReceivableScannerBuilder { self } + pub fn persistent_configuration( + mut self, + persistent_config: PersistentConfigurationMock, + ) -> Self { + self.persistent_configuration = persistent_config; + self + } + pub fn banned_dao(mut self, banned_dao: BannedDaoMock) -> Self { self.banned_dao = banned_dao; self @@ -1197,6 +1224,7 @@ impl ReceivableScannerBuilder { ReceivableScanner::new( Box::new(self.receivable_dao), Box::new(self.banned_dao), + Box::new(self.persistent_configuration), Rc::new(self.payment_thresholds), Rc::new(self.earning_wallet), Rc::new(RefCell::new(self.financial_statistics)), @@ -1393,6 +1421,18 @@ impl PayableThresholdsGaugeMock { } } +pub fn trick_rusqlite_with_read_only_conn( + path: &Path, + create_table: fn(&Connection), +) -> Connection { + let db_path = path.join("experiment.db"); + let conn = Connection::open_with_flags(&db_path, OpenFlags::default()).unwrap(); + create_table(&conn); + conn.close().unwrap(); + let conn = Connection::open_with_flags(&db_path, OpenFlags::SQLITE_OPEN_READ_ONLY).unwrap(); + conn +} + #[derive(Default)] pub struct PaymentAdjusterMock { search_for_indispensable_adjustment_params: @@ -1521,7 +1561,7 @@ where panic!("Called mark_as_ended() from NullScanner"); } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } formal_traits_for_payable_mid_scan_msg_handling!(NullScanner); diff --git a/node/src/actor_system_factory.rs b/node/src/actor_system_factory.rs index 870460dd1..45078267c 100644 --- a/node/src/actor_system_factory.rs +++ b/node/src/actor_system_factory.rs @@ -451,6 +451,7 @@ impl ActorFactory for ActorFactoryReal { let pending_payable_dao_factory = Box::new(Accountant::dao_factory(data_directory)); let receivable_dao_factory = Box::new(Accountant::dao_factory(data_directory)); let banned_dao_factory = Box::new(Accountant::dao_factory(data_directory)); + let config_dao_factory = Box::new(Accountant::dao_factory(data_directory)); Self::load_banned_cache(db_initializer, banned_cache_loader, data_directory); let arbiter = Arbiter::builder().stop_system_on_panic(true); let addr: Addr = arbiter.start(move |_| { @@ -461,6 +462,7 @@ impl ActorFactory for ActorFactoryReal { pending_payable_dao_factory, receivable_dao_factory, banned_dao_factory, + config_dao_factory, }, ) }); diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index 9b170709a..b0eab3bd4 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -338,16 +338,6 @@ 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(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"); } @@ -357,6 +347,7 @@ impl BlockchainBridge { .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."); @@ -1426,19 +1417,15 @@ 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(Some(10000u64))) - .start_block_result(Ok(6)) - .set_start_block_params(&set_start_block_params_arc) - .set_start_block_result(Ok(())); + .start_block_result(Ok(6)); 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(); @@ -1457,8 +1444,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![8675309u64]); let retrieve_transactions_params = retrieve_transactions_params_arc.lock().unwrap(); assert_eq!( *retrieve_transactions_params, @@ -1477,13 +1462,13 @@ mod tests { &ReceivedPayments { timestamp: received_payments.timestamp, payments: expected_transactions.transactions, + new_start_block: 8675309u64, response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321 }), } ); - TestLogHandler::new().exists_log_containing("INFO: BlockchainBridge: Using 'latest' block number instead of a literal number. QueryFailed(\"Failed to read the latest block number\")"); } @@ -1497,7 +1482,7 @@ mod tests { let amount = 42; let amount2 = 55; let expected_transactions = RetrievedBlockchainTransactions { - new_start_block: 1234, + new_start_block: 9876, transactions: vec![ BlockchainTransaction { block_number: 7, @@ -1518,12 +1503,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(Some(10000u64))) - .start_block_result(Ok(6)) - .set_start_block_params(&set_start_block_params_arc) - .set_start_block_result(Ok(())); + .start_block_result(Ok(6)); let subject = BlockchainBridge::new( Box::new(blockchain_interface_mock), Box::new(persistent_config), @@ -1548,8 +1530,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![1234u64]); let retrieve_transactions_params = retrieve_transactions_params_arc.lock().unwrap(); assert_eq!( *retrieve_transactions_params, @@ -1568,6 +1548,7 @@ mod tests { &ReceivedPayments { timestamp: received_payments.timestamp, payments: expected_transactions.transactions, + new_start_block: 9876, response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321 @@ -1587,12 +1568,9 @@ 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(6)) - .set_start_block_params(&set_start_block_params_arc) - .set_start_block_result(Ok(())); + .start_block_result(Ok(6)); let (accountant, _, accountant_recording_arc) = make_recorder(); let system = System::new( "processing_of_received_payments_continues_even_if_no_payments_are_detected", @@ -1621,8 +1599,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![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); @@ -1631,6 +1607,7 @@ mod tests { &ReceivedPayments { timestamp: received_payments.timestamp, payments: vec![], + new_start_block: 7, response_skeleton_opt: Some(ResponseSkeleton { client_id: 1234, context_id: 4321 @@ -1666,41 +1643,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(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/blockchain/blockchain_interface/blockchain_interface_null/mod.rs b/node/src/blockchain/blockchain_interface/blockchain_interface_null/mod.rs index e33ff06ab..69570c23b 100644 --- a/node/src/blockchain/blockchain_interface/blockchain_interface_null/mod.rs +++ b/node/src/blockchain/blockchain_interface/blockchain_interface_null/mod.rs @@ -65,7 +65,7 @@ impl BlockchainInterface for BlockchainInterfaceNull { &*self.lower_level_interface } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } impl Default for BlockchainInterfaceNull { diff --git a/node/src/blockchain/blockchain_interface/mod.rs b/node/src/blockchain/blockchain_interface/mod.rs index 250f0d2f8..d41513d3a 100644 --- a/node/src/blockchain/blockchain_interface/mod.rs +++ b/node/src/blockchain/blockchain_interface/mod.rs @@ -48,5 +48,5 @@ pub trait BlockchainInterface { fn lower_interface(&self) -> &dyn LowBlockchainInt; - as_any_in_trait!(); + as_any_ref_in_trait!(); } diff --git a/node/src/blockchain/blockchain_interface/test_utils.rs b/node/src/blockchain/blockchain_interface/test_utils.rs index 6817e74ab..6588047cd 100644 --- a/node/src/blockchain/blockchain_interface/test_utils.rs +++ b/node/src/blockchain/blockchain_interface/test_utils.rs @@ -5,7 +5,6 @@ use crate::blockchain::blockchain_interface::lower_level_interface::{ LatestBlockNumber, LowBlockchainInt, ResultForBalance, ResultForNonce, }; - use crate::blockchain::blockchain_interface::BlockchainInterface; use crate::sub_lib::wallet::Wallet; use crate::test_utils::http_test_server::TestServer; diff --git a/node/src/blockchain/test_utils.rs b/node/src/blockchain/test_utils.rs index b1a6eae30..4a8f4ecd6 100644 --- a/node/src/blockchain/test_utils.rs +++ b/node/src/blockchain/test_utils.rs @@ -77,8 +77,8 @@ pub struct BlockchainInterfaceMock { RefCell, PayableTransactionError>>>, get_transaction_receipt_params: Arc>>, get_transaction_receipt_results: RefCell>, - arbitrary_id_stamp_opt: Option, lower_interface_result: Option>, + arbitrary_id_stamp_opt: Option, } impl BlockchainInterface for BlockchainInterfaceMock { diff --git a/node/src/bootstrapper.rs b/node/src/bootstrapper.rs index 5d40c2379..936babf77 100644 --- a/node/src/bootstrapper.rs +++ b/node/src/bootstrapper.rs @@ -1293,6 +1293,7 @@ mod tests { #[test] fn initialize_as_unprivileged_passes_node_descriptor_to_ui_config() { + init_test_logging(); let _lock = INITIALIZATION.lock(); let data_dir = ensure_node_home_directory_exists( "bootstrapper", diff --git a/node/src/daemon/daemon_initializer.rs b/node/src/daemon/daemon_initializer.rs index 61519c047..d51478564 100644 --- a/node/src/daemon/daemon_initializer.rs +++ b/node/src/daemon/daemon_initializer.rs @@ -83,7 +83,7 @@ impl DaemonInitializer for DaemonInitializerReal { self.split(system, receiver); Ok(()) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } pub trait Rerunner { diff --git a/node/src/daemon/setup_reporter.rs b/node/src/daemon/setup_reporter.rs index 6ad031cc6..15a27d4bd 100644 --- a/node/src/daemon/setup_reporter.rs +++ b/node/src/daemon/setup_reporter.rs @@ -1189,8 +1189,8 @@ mod tests { use crate::daemon::dns_inspector::dns_inspector::DnsInspector; use crate::daemon::dns_inspector::DnsInspectionError; use crate::daemon::setup_reporter; - use crate::database::connection_wrapper::ConnectionWrapperReal; use crate::database::db_initializer::{DbInitializer, DbInitializerReal, DATABASE_FILE}; + use crate::database::rusqlite_wrappers::ConnectionWrapperReal; use crate::db_config::config_dao::{ConfigDao, ConfigDaoReal}; use crate::db_config::persistent_configuration::{ PersistentConfigError, PersistentConfiguration, PersistentConfigurationReal, diff --git a/node/src/database/config_dumper.rs b/node/src/database/config_dumper.rs index 3b2ed50c3..891eac1ad 100644 --- a/node/src/database/config_dumper.rs +++ b/node/src/database/config_dumper.rs @@ -50,7 +50,7 @@ impl DumpConfigRunner for DumpConfigRunnerReal { Ok(()) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } fn write_string(streams: &mut StdStreams, json: String) { @@ -159,8 +159,8 @@ fn distill_args( mod tests { use super::*; use crate::blockchain::bip39::Bip39; - use crate::database::connection_wrapper::ConnectionWrapperReal; use crate::database::db_initializer::ExternalData; + use crate::database::rusqlite_wrappers::ConnectionWrapperReal; use crate::db_config::config_dao::ConfigDao; use crate::db_config::persistent_configuration::{ PersistentConfiguration, PersistentConfigurationReal, diff --git a/node/src/database/connection_wrapper.rs b/node/src/database/connection_wrapper.rs deleted file mode 100644 index ca612c64b..000000000 --- a/node/src/database/connection_wrapper.rs +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. - -use rusqlite::{Connection, Error, Statement, Transaction}; -use std::fmt::Debug; - -pub trait ConnectionWrapper: Debug + Send { - fn prepare(&self, query: &str) -> Result; - fn transaction<'a: 'b, 'b>(&'a mut self) -> Result, rusqlite::Error>; -} - -#[derive(Debug)] -pub struct ConnectionWrapperReal { - conn: Connection, -} - -impl ConnectionWrapper for ConnectionWrapperReal { - fn prepare(&self, query: &str) -> Result { - self.conn.prepare(query) - } - fn transaction<'a: 'b, 'b>(&'a mut self) -> Result, Error> { - self.conn.transaction() - } -} - -impl ConnectionWrapperReal { - pub fn new(conn: Connection) -> Self { - Self { conn } - } -} diff --git a/node/src/database/db_initializer.rs b/node/src/database/db_initializer.rs index 2fad52508..8bfb9c1eb 100644 --- a/node/src/database/db_initializer.rs +++ b/node/src/database/db_initializer.rs @@ -1,5 +1,5 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. -use crate::database::connection_wrapper::{ConnectionWrapper, ConnectionWrapperReal}; +use crate::database::rusqlite_wrappers::{ConnectionWrapper, ConnectionWrapperReal}; use crate::database::db_migrations::db_migrator::{DbMigrator, DbMigratorReal}; use crate::db_config::secure_config_layer::EXAMPLE_ENCRYPTED; @@ -132,15 +132,15 @@ impl DbInitializerReal { } fn create_database_tables(&self, conn: &Connection, external_params: ExternalData) { - self.create_config_table(conn); - self.initialize_config(conn, external_params); - self.create_payable_table(conn); - self.create_pending_payable_table(conn); - self.create_receivable_table(conn); - self.create_banned_table(conn); + Self::create_config_table(conn); + Self::initialize_config(conn, external_params); + Self::create_payable_table(conn); + Self::create_pending_payable_table(conn); + Self::create_receivable_table(conn); + Self::create_banned_table(conn); } - fn create_config_table(&self, conn: &Connection) { + pub fn create_config_table(conn: &Connection) { conn.execute( "create table if not exists config ( name text primary key, @@ -151,7 +151,7 @@ impl DbInitializerReal { ) .expect("Can't create config table"); } - fn initialize_config(&self, conn: &Connection, external_params: ExternalData) { + fn initialize_config(conn: &Connection, external_params: ExternalData) { Self::set_config_value(conn, EXAMPLE_ENCRYPTED, None, true, "example_encrypted"); Self::set_config_value( conn, @@ -264,7 +264,7 @@ impl DbInitializerReal { Self::set_config_value(conn, "max_block_count", None, false, "maximum block count"); } - fn create_pending_payable_table(&self, conn: &Connection) { + pub fn create_pending_payable_table(conn: &Connection) { conn.execute( "create table if not exists pending_payable ( rowid integer primary key, @@ -285,7 +285,7 @@ impl DbInitializerReal { .expect("Can't create transaction hash index in pending payments"); } - fn create_payable_table(&self, conn: &Connection) { + pub fn create_payable_table(conn: &Connection) { conn.execute( "create table if not exists payable ( wallet_address text primary key, @@ -299,7 +299,7 @@ impl DbInitializerReal { .expect("Can't create payable table"); } - fn create_receivable_table(&self, conn: &Connection) { + pub fn create_receivable_table(conn: &Connection) { conn.execute( "create table if not exists receivable ( wallet_address text primary key, @@ -312,7 +312,7 @@ impl DbInitializerReal { .expect("Can't create receivable table"); } - fn create_banned_table(&self, conn: &Connection) { + pub fn create_banned_table(conn: &Connection) { conn.execute( "create table banned ( wallet_address text primary key )", [], @@ -623,113 +623,6 @@ impl Debug for DbInitializationConfig { } } -#[cfg(test)] -pub mod test_utils { - use crate::database::connection_wrapper::ConnectionWrapper; - use crate::database::db_initializer::DbInitializationConfig; - use crate::database::db_initializer::{DbInitializer, InitializationError}; - use rusqlite::Transaction; - use rusqlite::{Error, Statement}; - use std::cell::RefCell; - use std::path::{Path, PathBuf}; - use std::sync::{Arc, Mutex}; - - #[derive(Debug, Default)] - pub struct ConnectionWrapperMock<'b, 'a: 'b> { - prepare_params: Arc>>, - prepare_results: RefCell, Error>>>, - transaction_results: RefCell, Error>>>, - } - - unsafe impl<'a: 'b, 'b> Send for ConnectionWrapperMock<'a, 'b> {} - - impl<'a: 'b, 'b> ConnectionWrapperMock<'a, 'b> { - pub fn new() -> Self { - Self::default() - } - - pub fn prepare_result(self, result: Result, Error>) -> Self { - self.prepare_results.borrow_mut().push(result); - self - } - - pub fn transaction_result(self, result: Result, Error>) -> Self { - self.transaction_results.borrow_mut().push(result); - self - } - } - - impl<'a: 'b, 'b> ConnectionWrapper for ConnectionWrapperMock<'a, 'b> { - fn prepare(&self, query: &str) -> Result { - self.prepare_params - .lock() - .unwrap() - .push(String::from(query)); - self.prepare_results.borrow_mut().remove(0) - } - - fn transaction<'_a: '_b, '_b>(&'_a mut self) -> Result, Error> { - self.transaction_results.borrow_mut().remove(0) - } - } - - #[derive(Default)] - pub struct DbInitializerMock { - pub initialize_params: Arc>>, - pub initialize_results: - RefCell, InitializationError>>>, - } - - impl DbInitializer for DbInitializerMock { - fn initialize( - &self, - path: &Path, - init_config: DbInitializationConfig, - ) -> Result, InitializationError> { - self.initialize_params - .lock() - .unwrap() - .push((path.to_path_buf(), init_config)); - self.initialize_results.borrow_mut().remove(0) - } - - #[allow(unused_variables)] - fn initialize_to_version( - &self, - path: &Path, - target_version: usize, - init_config: DbInitializationConfig, - ) -> Result, InitializationError> { - intentionally_blank!() - /*all existing test calls only initialize() in the mocked version, - but we need to call initialize_to_version() for the real object - in order to carry out some important tests too*/ - } - } - - impl DbInitializerMock { - pub fn new() -> Self { - Self::default() - } - - pub fn initialize_parameters( - mut self, - parameters: Arc>>, - ) -> DbInitializerMock { - self.initialize_params = parameters; - self - } - - pub fn initialize_result( - self, - result: Result, InitializationError>, - ) -> DbInitializerMock { - self.initialize_results.borrow_mut().push(result); - self - } - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/node/src/database/db_migrations/db_migrator.rs b/node/src/database/db_migrations/db_migrator.rs index 4dba9ce97..746af3e26 100644 --- a/node/src/database/db_migrations/db_migrator.rs +++ b/node/src/database/db_migrations/db_migrator.rs @@ -1,6 +1,5 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. -use crate::database::connection_wrapper::ConnectionWrapper; use crate::database::db_initializer::ExternalData; use crate::database::db_migrations::migrations::migration_0_to_1::Migrate_0_to_1; use crate::database::db_migrations::migrations::migration_1_to_2::Migrate_1_to_2; @@ -14,8 +13,8 @@ use crate::database::db_migrations::migrations::migration_8_to_9::Migrate_8_to_9 use crate::database::db_migrations::migrator_utils::{ DBMigDeclarator, DBMigrationUtilities, DBMigrationUtilitiesReal, DBMigratorInnerConfiguration, }; +use crate::database::rusqlite_wrappers::{ConnectionWrapper, TransactionSafeWrapper}; use masq_lib::logger::Logger; -use rusqlite::Transaction; pub trait DbMigrator { fn migrate_database( @@ -110,7 +109,7 @@ impl DbMigratorReal { &self, record: &dyn DatabaseMigration, migration_utilities: &'a (dyn DBMigrationUtilities + 'a), - logger: &'a Logger, + logger: &Logger, ) -> rusqlite::Result<()> { info!( &self.logger, @@ -125,16 +124,16 @@ impl DbMigratorReal { pub fn update_schema_version( name_of_given_table: &str, - transaction: &Transaction, + transaction: &TransactionSafeWrapper, update_to: usize, ) -> rusqlite::Result<()> { - transaction.execute( - &format!( + transaction + .prepare(&format!( "UPDATE {} SET value = {} WHERE name = 'schema_version'", name_of_given_table, update_to - ), - [], - )?; + )) + .expect("internal rusqlite error") + .execute([])?; Ok(()) } @@ -149,12 +148,8 @@ impl DbMigratorReal { .iter() .skip_while(|entry| entry.old_version() != obsolete_schema) .take_while(|entry| entry.old_version() < target_version) - .map(Self::deref) - .collect::>() - } - - fn deref<'a, T: ?Sized>(value: &'a &T) -> &'a T { - *value + .copied() + .collect::>() } fn dispatch_bad_news( @@ -184,8 +179,6 @@ impl DbMigratorReal { #[cfg(test)] mod tests { - use crate::database::connection_wrapper::{ConnectionWrapper, ConnectionWrapperReal}; - use crate::database::db_initializer::test_utils::ConnectionWrapperMock; use crate::database::db_initializer::ExternalData; use crate::database::db_migrations::db_migrator::{ DatabaseMigration, DbMigrator, DbMigratorReal, @@ -196,6 +189,8 @@ mod tests { DBMigratorInnerConfiguration, }; use crate::database::db_migrations::test_utils::DBMigDeclaratorMock; + use crate::database::rusqlite_wrappers::{ConnectionWrapper, ConnectionWrapperReal}; + use crate::database::test_utils::ConnectionWrapperMock; use crate::test_utils::database_utils::make_external_data; use masq_lib::constants::CURRENT_SCHEMA_VERSION; use masq_lib::logger::Logger; @@ -577,11 +572,9 @@ mod tests { let assertion: (String, String) = connection_wrapper .transaction() .unwrap() - .query_row( - "SELECT name, value FROM test WHERE name='schema_version'", - [], - |row| Ok((row.get(0).unwrap(), row.get(1).unwrap())), - ) + .prepare("SELECT name, value FROM test WHERE name='schema_version'") + .unwrap() + .query_row([], |row| Ok((row.get(0).unwrap(), row.get(1).unwrap()))) .unwrap(); assert_eq!(assertion.1, "5") } diff --git a/node/src/database/db_migrations/migrations/migration_4_to_5.rs b/node/src/database/db_migrations/migrations/migration_4_to_5.rs index 7b3af68bd..dbc40f1b3 100644 --- a/node/src/database/db_migrations/migrations/migration_4_to_5.rs +++ b/node/src/database/db_migrations/migrations/migration_4_to_5.rs @@ -77,10 +77,10 @@ impl DatabaseMigration for Migrate_4_to_5 { #[cfg(test)] mod tests { use crate::accountant::db_access_objects::utils::{from_time_t, to_time_t}; - use crate::database::connection_wrapper::{ConnectionWrapper, ConnectionWrapperReal}; use crate::database::db_initializer::{ DbInitializationConfig, DbInitializer, DbInitializerReal, ExternalData, DATABASE_FILE, }; + use crate::database::rusqlite_wrappers::{ConnectionWrapper, ConnectionWrapperReal}; use crate::sub_lib::wallet::Wallet; use crate::test_utils::database_utils::{ assert_create_table_stm_contains_all_parts, diff --git a/node/src/database/db_migrations/migrations/migration_6_to_7.rs b/node/src/database/db_migrations/migrations/migration_6_to_7.rs index 12f14ecfc..81b6e15e6 100644 --- a/node/src/database/db_migrations/migrations/migration_6_to_7.rs +++ b/node/src/database/db_migrations/migrations/migration_6_to_7.rs @@ -228,11 +228,11 @@ impl<'a> Migrate_6_to_7_carrier<'a> { #[cfg(test)] mod tests { - use crate::database::connection_wrapper::ConnectionWrapper; use crate::database::db_initializer::{ DbInitializationConfig, DbInitializer, DbInitializerReal, DATABASE_FILE, }; use crate::database::db_migrations::db_migrator::{DbMigrator, DbMigratorReal}; + use crate::database::rusqlite_wrappers::ConnectionWrapper; use crate::db_config::persistent_configuration::{ PersistentConfiguration, PersistentConfigurationReal, }; diff --git a/node/src/database/db_migrations/migrator_utils.rs b/node/src/database/db_migrations/migrator_utils.rs index 422273b06..c0b4fe168 100644 --- a/node/src/database/db_migrations/migrator_utils.rs +++ b/node/src/database/db_migrations/migrator_utils.rs @@ -1,17 +1,17 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. -use crate::database::connection_wrapper::ConnectionWrapper; use crate::database::db_initializer::ExternalData; use crate::database::db_migrations::db_migrator::{DatabaseMigration, DbMigratorReal}; +use crate::database::rusqlite_wrappers::{ConnectionWrapper, TransactionSafeWrapper}; use masq_lib::constants::CURRENT_SCHEMA_VERSION; use masq_lib::logger::Logger; use masq_lib::utils::{to_string, ExpectValue}; -use rusqlite::{params_from_iter, Error, ToSql, Transaction}; +use rusqlite::{Error, ToSql}; use std::fmt::{Display, Formatter}; pub trait DBMigDeclarator { fn db_password(&self) -> Option; - fn transaction(&self) -> &Transaction; + fn transaction(&self) -> &TransactionSafeWrapper; fn execute_upon_transaction<'a>( &self, sql_statements: &[&'a dyn StatementObject], @@ -35,7 +35,7 @@ pub trait DBMigrationUtilities { } pub struct DBMigrationUtilitiesReal<'a> { - root_transaction: Option>, + root_transaction: Option>, db_migrator_configuration: DBMigratorInnerConfiguration, } @@ -50,7 +50,7 @@ impl<'a> DBMigrationUtilitiesReal<'a> { }) } - fn root_transaction_ref(&self) -> &Transaction<'a> { + fn root_transaction_ref(&self) -> &TransactionSafeWrapper { self.root_transaction.as_ref().expectv("root transaction") } } @@ -98,14 +98,14 @@ impl<'a> DBMigrationUtilities for DBMigrationUtilitiesReal<'a> { } struct DBMigDeclaratorReal<'a> { - root_transaction_ref: &'a Transaction<'a>, + root_transaction_ref: &'a TransactionSafeWrapper<'a>, external: &'a ExternalData, logger: &'a Logger, } impl<'a> DBMigDeclaratorReal<'a> { fn new( - root_transaction_ref: &'a Transaction<'a>, + root_transaction_ref: &'a TransactionSafeWrapper, external: &'a ExternalData, logger: &'a Logger, ) -> Self { @@ -122,7 +122,7 @@ impl DBMigDeclarator for DBMigDeclaratorReal<'_> { self.external.db_password_opt.clone() } - fn transaction(&self) -> &Transaction { + fn transaction(&self) -> &TransactionSafeWrapper { self.root_transaction_ref } @@ -155,17 +155,17 @@ impl DBMigDeclarator for DBMigDeclaratorReal<'_> { } pub trait StatementObject: Display { - fn execute(&self, transaction: &Transaction) -> rusqlite::Result<()>; + fn execute(&self, transaction: &TransactionSafeWrapper) -> rusqlite::Result<()>; } impl StatementObject for &str { - fn execute(&self, transaction: &Transaction) -> rusqlite::Result<()> { - transaction.execute(self, []).map(|_| ()) + fn execute(&self, transaction: &TransactionSafeWrapper) -> rusqlite::Result<()> { + transaction.execute(self, &[]).map(|_| ()) } } impl StatementObject for String { - fn execute(&self, transaction: &Transaction) -> rusqlite::Result<()> { + fn execute(&self, transaction: &TransactionSafeWrapper) -> rusqlite::Result<()> { self.as_str().execute(transaction) } } @@ -176,9 +176,16 @@ pub struct StatementWithRusqliteParams { } impl StatementObject for StatementWithRusqliteParams { - fn execute(&self, transaction: &Transaction) -> rusqlite::Result<()> { + fn execute(&self, transaction: &TransactionSafeWrapper) -> rusqlite::Result<()> { transaction - .execute(&self.sql_stm, params_from_iter(self.params.iter())) + .execute( + &self.sql_stm, + &self + .params + .iter() + .map(|param| param.as_ref()) + .collect::>(), + ) .map(|_| ()) } } @@ -227,11 +234,11 @@ impl DatabaseMigration for InterimMigrationPlaceholder { #[cfg(test)] mod tests { - use crate::database::connection_wrapper::ConnectionWrapperReal; use crate::database::db_migrations::migrator_utils::{ DBMigrationUtilities, DBMigrationUtilitiesReal, DBMigratorInnerConfiguration, StatementObject, StatementWithRusqliteParams, }; + use crate::database::rusqlite_wrappers::ConnectionWrapperReal; use crate::test_utils::database_utils::make_external_data; use masq_lib::logger::Logger; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; @@ -294,7 +301,7 @@ mod tests { let result = subject.transaction(); result - .execute("CREATE TABLE IF NOT EXISTS test (column TEXT)", []) + .execute("CREATE TABLE IF NOT EXISTS test (column TEXT)", &[]) .unwrap(); // no panic? Test passes! } diff --git a/node/src/database/db_migrations/test_utils.rs b/node/src/database/db_migrations/test_utils.rs index 11c68c5a7..08c6423c2 100644 --- a/node/src/database/db_migrations/test_utils.rs +++ b/node/src/database/db_migrations/test_utils.rs @@ -3,9 +3,9 @@ use crate::database::db_initializer::ExternalData; use crate::database::db_migrations::migrator_utils::{DBMigDeclarator, StatementObject}; +use crate::database::rusqlite_wrappers::TransactionSafeWrapper; use masq_lib::logger::Logger; use masq_lib::utils::to_string; -use rusqlite::Transaction; use std::cell::RefCell; use std::sync::{Arc, Mutex}; @@ -43,7 +43,7 @@ impl DBMigDeclarator for DBMigDeclaratorMock { self.db_password_results.borrow_mut().remove(0) } - fn transaction(&self) -> &Transaction { + fn transaction(&self) -> &TransactionSafeWrapper { unimplemented!("Not needed so far") } diff --git a/node/src/database/mod.rs b/node/src/database/mod.rs index 3ba58912e..3f8e5c079 100644 --- a/node/src/database/mod.rs +++ b/node/src/database/mod.rs @@ -1,6 +1,7 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. pub mod config_dumper; -pub mod connection_wrapper; pub mod db_initializer; pub mod db_migrations; +pub mod rusqlite_wrappers; +pub mod test_utils; diff --git a/node/src/database/rusqlite_wrappers.rs b/node/src/database/rusqlite_wrappers.rs new file mode 100644 index 000000000..ec867482f --- /dev/null +++ b/node/src/database/rusqlite_wrappers.rs @@ -0,0 +1,185 @@ +// Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. + +use crate::arbitrary_id_stamp_in_trait; +use crate::masq_lib::utils::ExpectValue; +use rusqlite::{Connection, Error, Statement, ToSql, Transaction}; +use std::fmt::Debug; + +// We were challenged multiple times to device mocks for testing stubborn, hard to tame, data +// structures from the 'rusqlite' library. After all, we've adopted two of them, the Connection, +// that came first, and the Transaction to come much later. Of these, only the former complies +// with the standard policy we follow for mock designs. +// +// The delay until the second one became a thing, even though we would've been glad having it +// on hand much earlier, was caused by vacuum of ideas on how we could create a mock of these +// parameters and have it accepted by the compiler. Passing a lot of time, we came up with a hybrid, +// at least. That said, it has costed us a considerably high price of giving up on simplicity. +// +// The firmest blocker of the design has always rooted in a relationship of serialized lifetimes, +// affecting each other, that has been so hard to maintain right. Yet the choices made +// within the third-party library have good reasoning, like that the database connection must +// stay alive as long as there are any active 'Transactions' or 'Statements'. +// +// It's a fact, though, that the use of an explicit, object-like transaction from the 'rusqlite' +// library is rare in our code. That might explain why we managed to live so long without feeling +// much constrained by absence of this mock. While we did write some code using that kind of +// transaction, we always acknowledged that we had to make an exception and leave that piece +// of code untested. The problem with a mocked transaction has been that we often need to combine +// it with a connection also needs to be a mock. Then the transaction springs from the connection. +// The following nontrivial issues with lifetimes have been briefly mentioned. +// +// The interface a transaction uses consists of three methods. Only 'prepare' causes troubles. +// It returns a `Statement`, keeping a reference to the parent connection. +// +// There is now no decent vision of how to cut back on this method, despite how large temptation +// surrounds it. Theoretically, we could make a replacement by the existing 'execute' method and +// so circumvent all the places where we now need to manipulate with 'Statement'. That is because +// 'execute' returns only simple data structures, easy to be stored in a mock. Even going that +// way, other issues would take place inevitably. +// +// One may consider creating another class to wrap up the `Statement` and acquire a full control. +// It's been tested. The interface requires even more methods and so obstacles. The steeply +// increasing difficulty ended the efforts. +// +// Critically speaking, this mock does not win with either simplicity or sparkling elegance. Its +// setup must also be expected to become harder the more complex tests it takes. The excuse for +// this is that it also opens up possibilities for thorough tests in progressively less rare +// occurrences where we need to be able to use an explicit 'rusqlite' transaction. Note that +// testing such situations used to seem undoable not so far ago. +// +// (See more explanation near the mock itself) + +pub trait ConnectionWrapper: Debug + Send { + fn prepare(&self, query: &str) -> Result; + fn transaction(&mut self) -> Result; +} + +#[derive(Debug)] +pub struct ConnectionWrapperReal { + conn: Connection, +} + +impl ConnectionWrapper for ConnectionWrapperReal { + fn prepare(&self, query: &str) -> Result { + self.conn.prepare(query) + } + fn transaction(&mut self) -> Result { + self.conn.transaction().map(TransactionSafeWrapper::new) + } +} + +impl ConnectionWrapperReal { + pub fn new(conn: Connection) -> Self { + Self { conn } + } +} + +// Whole point of this outer wrapper, that is common to both the real and mock transactions, is to +// make a chance to deconstruct all components of a transaction in place. It plays a crucial role +// during the final commit. Note that an usual mock based on the direct use of a trait object +// cannot be consumed by any of its methods because of the Rust rules for trait objects. They say +// clearly that we can access it via '&self', '&mut self' but not 'self'. However, to have a thing +// consume itself we need to be provided with the full ownership. +// +// Leaving remains of an already committed transaction around would expose us to a risk. Let's +// imagine somebody trying to make use of it the second time, while the inner element providing +// the connection has already been swallowed by a third-party function call, yet our wrapper would +// be able to live on. An error situation would've arisen. +// +// Second, caution is much desirable (while relevant only for the test tree, not production code) +// because the wrapper hides a potential hard-to-debug segmentation error that the OS might raise +// if the timing of deconstruct of the transaction we created and put inside isn't done correctly. +// The mock has an unsafe reference attached at a database transaction on one side and pointing +// back in to another field holding an exclusive connection that can be freely carried around as +// the whole mock moves. We must prevent the connection from being deconstructed before +// the transaction was. + +// The wrapper has a lifetime possibly having much different parameters depending on the use in +// the production code or if constructed as a mock inside a test. +// +// Real -> refers to an outer Connection +// Mock -> 'static +#[derive(Debug)] +pub struct TransactionSafeWrapper<'conn> { + inner: Box, +} + +impl<'conn> TransactionSafeWrapper<'conn> { + pub fn new(txn: Transaction<'conn>) -> Self { + Self { + inner: Box::new(TransactionInnerWrapperReal::new(txn)), + } + } + + pub fn prepare(&self, query: &str) -> Result { + self.inner.prepare(query) + } + + pub fn execute(&self, query: &str, params: &[&dyn ToSql]) -> Result { + self.inner.execute(query, params) + } + + pub fn commit(mut self) -> Result<(), Error> { + self.inner.commit() + } +} + +pub trait TransactionInnerWrapper: Debug { + fn prepare(&self, query: &str) -> Result; + fn execute(&self, query: &str, params: &[&dyn ToSql]) -> Result; + fn commit(&mut self) -> Result<(), Error>; + + arbitrary_id_stamp_in_trait!(); +} + +// Please note that this structure is meant to stay private + +#[derive(Debug)] +struct TransactionInnerWrapperReal<'conn> { + txn_opt: Option>, +} + +impl<'conn> TransactionInnerWrapperReal<'conn> { + fn new(transaction: Transaction<'conn>) -> TransactionInnerWrapperReal<'conn> { + Self { + txn_opt: Some(transaction), + } + } +} + +impl<'a> TransactionInnerWrapper for TransactionInnerWrapperReal<'a> { + fn prepare(&self, query: &str) -> Result { + self.txn_opt.as_ref().expectv("rusqlite txn").prepare(query) + } + + fn execute(&self, query: &str, params: &[&dyn ToSql]) -> Result { + self.txn_opt + .as_ref() + .expectv("rusqlite txn") + .execute(query, params) + } + + fn commit(&mut self) -> Result<(), Error> { + let transaction = self.txn_opt.take(); + transaction.expectv("rusqlite txn").commit() + } +} + +#[cfg(test)] +pub mod transaction_wrapper_test_only { + use crate::database::rusqlite_wrappers::TransactionSafeWrapper; + use crate::database::test_utils::transaction_wrapper_mock::TransactionInnerWrapperMockBuilder; + use crate::test_utils::unshared_test_utils::arbitrary_id_stamp::ArbitraryIdStamp; + + impl<'conn> TransactionSafeWrapper<'conn> { + pub fn new_with_builder(inner_wrapper_builder: TransactionInnerWrapperMockBuilder) -> Self { + Self { + inner: inner_wrapper_builder.build(), + } + } + + pub fn arbitrary_id_stamp(&self) -> ArbitraryIdStamp { + self.inner.arbitrary_id_stamp() + } + } +} diff --git a/node/src/database/test_utils/mod.rs b/node/src/database/test_utils/mod.rs new file mode 100644 index 000000000..e8b9060f1 --- /dev/null +++ b/node/src/database/test_utils/mod.rs @@ -0,0 +1,116 @@ +// Copyright (c) 2023, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. + +#![cfg(test)] +pub mod transaction_wrapper_mock; + +use crate::database::db_initializer::DbInitializationConfig; +use crate::database::db_initializer::{DbInitializer, InitializationError}; +use crate::database::rusqlite_wrappers::{ConnectionWrapper, TransactionSafeWrapper}; +use rusqlite::{Error, Statement}; +use std::cell::RefCell; +use std::fmt::Debug; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; + +#[derive(Debug, Default)] +pub struct ConnectionWrapperMock<'conn> { + prepare_params: Arc>>, + prepare_results: RefCell, Error>>>, + transaction_results: RefCell, Error>>>, +} + +// We don't know better how to deal with the third-party code for `Statement` that inherits +// attributes from `Connection` that lacks the Send marker. This unsafe block instructs the compiler +// we want to enforce it because it is a test utility and as far as we know it hasn't bitten us + +unsafe impl<'conn> Send for ConnectionWrapperMock<'conn> {} + +impl<'conn> ConnectionWrapperMock<'conn> { + pub fn new() -> Self { + Self::default() + } + + pub fn prepare_params(mut self, params: &Arc>>) -> Self { + self.prepare_params = params.clone(); + self + } + + pub fn prepare_result(self, result: Result, Error>) -> Self { + self.prepare_results.borrow_mut().push(result); + self + } + + pub fn transaction_result(self, result: Result, Error>) -> Self { + self.transaction_results.borrow_mut().push(result); + self + } +} + +impl ConnectionWrapper for ConnectionWrapperMock<'_> { + fn prepare(&self, query: &str) -> Result { + self.prepare_params + .lock() + .unwrap() + .push(String::from(query)); + self.prepare_results.borrow_mut().remove(0) + } + + fn transaction(&mut self) -> Result { + self.transaction_results.borrow_mut().remove(0) + } +} + +#[derive(Default)] +pub struct DbInitializerMock { + pub initialize_params: Arc>>, + pub initialize_results: RefCell, InitializationError>>>, +} + +impl DbInitializer for DbInitializerMock { + fn initialize( + &self, + path: &Path, + init_config: DbInitializationConfig, + ) -> Result, InitializationError> { + self.initialize_params + .lock() + .unwrap() + .push((path.to_path_buf(), init_config)); + self.initialize_results.borrow_mut().remove(0) + } + + #[allow(unused_variables)] + fn initialize_to_version( + &self, + path: &Path, + target_version: usize, + init_config: DbInitializationConfig, + ) -> Result, InitializationError> { + intentionally_blank!() + /*all existing test calls only initialize() in the mocked version, + but we need to call initialize_to_version() for the real object + in order to carry out some important tests too*/ + } +} + +impl DbInitializerMock { + pub fn new() -> Self { + Self::default() + } + + pub fn initialize_parameters( + mut self, + parameters: Arc>>, + ) -> DbInitializerMock { + self.initialize_params = parameters; + self + } + + pub fn initialize_result( + self, + result: Result, InitializationError>, + ) -> DbInitializerMock { + self.initialize_results.borrow_mut().push(result); + self + } +} diff --git a/node/src/database/test_utils/transaction_wrapper_mock.rs b/node/src/database/test_utils/transaction_wrapper_mock.rs new file mode 100644 index 000000000..d0577c72f --- /dev/null +++ b/node/src/database/test_utils/transaction_wrapper_mock.rs @@ -0,0 +1,328 @@ +// Copyright (c) 2023, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. + +#![cfg(test)] + +use crate::database::rusqlite_wrappers::{ + ConnectionWrapper, TransactionInnerWrapper, TransactionSafeWrapper, +}; +use crate::test_utils::unshared_test_utils::arbitrary_id_stamp::ArbitraryIdStamp; +use crate::{arbitrary_id_stamp_in_trait_impl, set_arbitrary_id_stamp_in_mock_impl}; +use itertools::Either; +use rusqlite::{Error, Statement, ToSql}; +use std::cell::RefCell; +use std::sync::{Arc, Mutex}; + +// The qualities of this builder are given by its wide usability contrasting with how it +// enables creating encapsulated environment, inside which it configures and eventually builds +// the otherwise inaccessible mock. It minimizes the exposure of the special internal mock +// to such an extend, that any developer should come to understand that the insulation +// was done on purpose and should be respected. It's important to prevent use of the inner +// transaction wrapper without its outer counterpart, because only that one gives it the needed +// safeness. +// +// Read more about the improved safeness in this layered structure in comments near to these +// wrappers. + +#[derive(Default)] +pub struct TransactionInnerWrapperMockBuilder { + prepare_params: Arc>>, + prepare_results_producer_opt: Option, + commit_params: Arc>>, + commit_results: RefCell>>, + arbitrary_id_stamp_opt: Option, +} + +impl TransactionInnerWrapperMockBuilder { + pub fn prepare_params(mut self, params: &Arc>>) -> Self { + self.prepare_params = params.clone(); + self + } + + pub fn prepare_results(mut self, results: PrepareMethodResultsProducer) -> Self { + self.prepare_results_producer_opt = Some(results); + self + } + + pub fn commit_params(mut self, params: &Arc>>) -> Self { + self.commit_params = params.clone(); + self + } + + pub fn commit_result(self, result: Result<(), Error>) -> Self { + self.commit_results.borrow_mut().push(result); + self + } + + pub fn build(self) -> Box { + Box::new(TransactionInnerWrapperMock { + prepare_params: self.prepare_params, + prepare_results_producer_opt: self.prepare_results_producer_opt, + commit_params: self.commit_params, + commit_results: self.commit_results, + arbitrary_id_stamp_opt: self.arbitrary_id_stamp_opt, + }) + } + + set_arbitrary_id_stamp_in_mock_impl!(); +} + +// Keep as a private class, seek a builder of it instead + +#[derive(Debug)] +struct TransactionInnerWrapperMock { + prepare_params: Arc>>, + // This field holds an object that can produce a requested result based on the configuration + // of this mock (from the setup of the test) reflecting the programmer's decision that he needs + // control over the results coming out of the 'prepare' method + prepare_results_producer_opt: Option, + commit_params: Arc>>, + commit_results: RefCell>>, + arbitrary_id_stamp_opt: Option, +} + +impl TransactionInnerWrapper for TransactionInnerWrapperMock { + fn prepare(&self, prod_code_query: &str) -> Result { + self.prepare_params + .lock() + .unwrap() + .push(prod_code_query.to_string()); + + self.prepare_results_producer_opt + .as_ref() + .unwrap() + .produce_statement(prod_code_query) + } + + fn execute(&self, _query: &str, _params: &[&dyn ToSql]) -> Result { + unimplemented!("not needed yet") + } + + fn commit(&mut self) -> Result<(), Error> { + self.commit_params.lock().unwrap().push(()); + + let next_result = self.commit_results.borrow_mut().remove(0); + // If we pull out a result that says success, we check for a transaction inside `PrepareMethodResultsProducer` + // that would belong with a connection meant for keeping the real prod code statements, if we find one, we will + // commit it now. This design enables to make writes to the database that would've happened in the real life + // scenario anyway. The database then will be reliable for assertions in the test, not omitting any steps that + // could somehow modify it. + if next_result.is_ok() { + if let Some(prepared_results) = self.prepare_results_producer_opt.as_mut() { + if let Either::Right(for_both) = &mut prepared_results.setup { + return for_both.commit_prod_code_stmts(); + } + } + } + + next_result + } + + arbitrary_id_stamp_in_trait_impl!(); +} + +// Trying to store a rusqlite 'Statement' inside the TransactionWrapperMock and then placing this +// combination into the ConnectionWrapperMock placed us before complex lifetime issues. A hypothesis +// is that because of the design of the language maintaining such relationships between objects +// may be unachievable. + +// Eventually, we decided to take a different approach: radically reduce the excessive use of +// borrowed references. We had to make the mock much smarter by giving it its own db connection, +// or in some cases two. This connection acts as a source for creating native rusqlite Statements. +// This was unavoidable because we couldn't find any alternative to having the rusqlite library +// build the 'Statement' using their internal methods. Additionally, our attempts to create +// a StatementWrapper failed due to solid compilation problems. These problems were caused by +// the way generic arguments are spread across the methods in the 'Statement' implementation, +// out of which quite a lot of them are being used by us. It couldn't be replicated in our wrapper. +// In Rust, no one may write a trait with generic arguments in its methods as long as a trait object +// is to be formed. + +// With that said, we're relieved to have at least one working solution now. Speaking of the 'prepare' +// method, an error would be hardly needed because the production code simply unwraps the results by +// using 'expect'. That is a function excluded from the requirement of writing tests for. + +// The 'Statement' produced by this method must be better understood. The 'prepare' method has +// a crucial influence on the result of the following function, executing such prepared 'Statement'. +// Luckily, we can steer the course of events that this next function call will have to go through, +// able to anticipate, and therefore count on an exact result to happen during the test. This approach +// can be applied for every related method such as 'execute', 'query_row', or 'query_map'. + +#[derive(Debug)] +struct SetupForOnlyAlteredStmts { + conn: Box, + queue_of_statements: RefCell>, +} + +#[derive(Debug)] +struct SetupForProdCodeAndAlteredStmts { + prod_code_stmts_conn: Box, + // This transaction must be carried along because otherwise all those + // successful SQL operations would be written into the database right away, + // which is not how the reality works. On the other hand we do want them to + // affect the database persistently if the commit point is reached, so that + // the test can laid down assertions that test the database with maximally + // realistic conditions. + txn_bearing_prod_code_stmts_opt: Option>, + queue_with_prod_code_and_altered_stmts: RefCell>, + // This connection is usually the most important, but using just the prod code + // connection, meant primarily for non-altered statements, should be also possible. + // + // Common strategies to use this additional connection: + // + // a) as a connection pointing to another database, usually declared as simple as possible, + // that allows the same kind of error we want to see in the test (often an unusual, corner + // case error), but with a database and carefully arranged situation in which + // the substituted parameters to come into the SQL statement (if any at all), need to + // participate in our arrangement too (rusqlite code would complain otherwise), and that + // combination will cause the error, + // + // b) to tickle the database while exercising statements that attempt to change the state of + // the database. Since this connection will be read only, it'll generate an error. Thereby, + // we can assert on this error. Usually used when all we need is any kind of error. + separate_conn_for_altered_stmts_opt: Option>, +} + +impl SetupForProdCodeAndAlteredStmts { + fn commit_prod_code_stmts(&mut self) -> Result<(), Error> { + self.txn_bearing_prod_code_stmts_opt + .take() + .expect("Dual setup with a missing txn should never happen") + .commit() + } + + fn make_altered_stmt(&self, altered_stm: &str) -> Result { + match self.separate_conn_for_altered_stmts_opt.as_ref() { + Some(special_conn) => special_conn.prepare(altered_stm), + None => + // In the prod code, all the db operations would've happened on a single wrapped txn, + // that's why we strive to manipulate a txn also here, not the conn directly. Most + // importantly, sometimes multiple subsequent operations take each the previous one as + // necessary base. If the continuity is broken the later statement might not work. If + // we record some changes on the transaction, other changes tried to be done from + // a different connection might meet a different state of the database and thwart the + // efforts. (This behaviour probably depends on the global setup of the db). + // + // + // Also imagine a 'Statement' that wouldn't cause an error whereupon any potential + // rollback of this txn should best drag off both the prod code and altered statements + // all together, disappearing. If we did not use this txn some of the changes would stay. + { + self.txn_bearing_prod_code_stmts_opt + .as_ref() + .expect("The txn is always created and should be present") + .prepare(altered_stm) + } + } + } +} + +impl Drop for SetupForProdCodeAndAlteredStmts { + fn drop(&mut self) { + // The self contains a reference that doesn't comply with Rust safeness anymore as it has gone + // through backward cast from a raw pointer. We're making sure by this Drop impl that one + // part of 'self', the borne txn will deconstruct earlier than what it is referencing, that + // is the DB Connection held by one of the other fields in the same struct. Failing that, we'll + // have to do with a segmentation error from the OS. + drop(self.txn_bearing_prod_code_stmts_opt.take()) + } +} + +#[derive(Debug)] +pub struct PrepareMethodResultsProducer { + setup: Either, +} + +impl PrepareMethodResultsProducer { + pub fn construct_with_altered_stmts_only( + conn: Box, + altered_stmts_queue: Vec, + ) -> Self { + let setup = SetupForOnlyAlteredStmts { + conn, + queue_of_statements: RefCell::new(altered_stmts_queue), + }; + + Self { + setup: Either::Left(setup), + } + } + + pub fn construct_with_prod_code_and_altered_stmts( + prod_code_stmts_conn: Box, + separate_conn_for_altered_stmts_opt: Option>, + stm_determining_queue: Vec, + ) -> Self { + let setup = { + let ptr = Box::into_raw(prod_code_stmts_conn); + let conn = unsafe { Box::from_raw(ptr) }; + + let mut setup = SetupForProdCodeAndAlteredStmts { + prod_code_stmts_conn: conn, + txn_bearing_prod_code_stmts_opt: None, + queue_with_prod_code_and_altered_stmts: RefCell::new(stm_determining_queue), + separate_conn_for_altered_stmts_opt, + }; + + let conn = unsafe { ptr.as_mut().unwrap() }; + let txn = conn.transaction().unwrap(); + + setup.txn_bearing_prod_code_stmts_opt = Some(txn); + + setup + }; + + Self { + setup: Either::Right(setup), + } + } + + fn produce_statement(&self, prod_code_stmt: &str) -> Result { + match self.setup.as_ref() { + Either::Left(setup) => Self::handle_stmt_for_only_altered(setup), + Either::Right(setup) => { + Self::handle_stmt_for_prod_code_and_altered(setup, prod_code_stmt) + } + } + } + + fn handle_stmt_for_only_altered(setup: &SetupForOnlyAlteredStmts) -> Result { + let stmt = setup.queue_of_statements.borrow_mut().remove(0); + setup.conn.prepare(&stmt) + } + + fn handle_stmt_for_prod_code_and_altered<'conn>( + setup: &'conn SetupForProdCodeAndAlteredStmts, + prod_code_stmt: &str, + ) -> Result, Error> { + let altered_stmt_opt = setup + .queue_with_prod_code_and_altered_stmts + .borrow_mut() + .remove(0); + + match altered_stmt_opt { + StmtTypeDirective::ExecuteProdCode => { + setup.prod_code_stmts_conn.prepare(prod_code_stmt) + } + StmtTypeDirective::UseAlteredStmt(altered_stmt) => { + let altered_stm_str = match &altered_stmt { + AlteredStmtBySQLOrigin::SQLIdenticalWithProdCode => prod_code_stmt, + AlteredStmtBySQLOrigin::Substitution { new_stmt } => new_stmt.as_str(), + }; + setup.make_altered_stmt(altered_stm_str) + } + } + } +} + +#[derive(Debug)] +pub enum StmtTypeDirective { + ExecuteProdCode, + UseAlteredStmt(AlteredStmtBySQLOrigin), +} + +#[derive(Debug)] +pub enum AlteredStmtBySQLOrigin { + // Use this on planning to have a conn pointing to a different db. This will eliminate having to + // think up a new stmt, instead, the same one as in the prod code will be used. + SQLIdenticalWithProdCode, + Substitution { new_stmt: String }, +} diff --git a/node/src/db_config/config_dao.rs b/node/src/db_config/config_dao.rs index 7d386f151..23bd1fce5 100644 --- a/node/src/db_config/config_dao.rs +++ b/node/src/db_config/config_dao.rs @@ -1,6 +1,6 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. use crate::accountant::db_access_objects::utils::DaoFactoryReal; -use crate::database::connection_wrapper::ConnectionWrapper; +use crate::database::rusqlite_wrappers::{ConnectionWrapper, TransactionSafeWrapper}; use masq_lib::utils::to_string; use rusqlite::types::ToSql; use rusqlite::{Row, Rows, Statement}; @@ -41,6 +41,12 @@ pub trait ConfigDao { fn get_all(&self) -> Result, ConfigDaoError>; fn get(&self, name: &str) -> Result; fn set(&self, name: &str, value: Option) -> Result<(), ConfigDaoError>; + fn set_by_guest_transaction( + &self, + txn: &mut TransactionSafeWrapper, + name: &str, + value: Option, + ) -> Result<(), ConfigDaoError>; } pub struct ConfigDaoReal { @@ -65,16 +71,18 @@ impl ConfigDao for ConfigDaoReal { } fn set(&self, name: &str, value: Option) -> Result<(), ConfigDaoError> { - let mut stmt = match self - .conn - .prepare("update config set value = ? where name = ?") - { - Ok(stmt) => stmt, - // The following line is untested, because we don't know how to trigger it. - Err(e) => return Err(ConfigDaoError::DatabaseError(format!("{}", e))), - }; - let params: &[&dyn ToSql] = &[&value, &name]; - handle_update_execution(stmt.execute(params)) + let prepare_stm = |stm: &str| self.conn.prepare(stm); + Self::set_value(prepare_stm, name, value) + } + + fn set_by_guest_transaction( + &self, + txn: &mut TransactionSafeWrapper, + name: &str, + value: Option, + ) -> Result<(), ConfigDaoError> { + let prepare_stm = |stm: &str| txn.prepare(stm); + Self::set_value(prepare_stm, name, value) } } @@ -82,6 +90,22 @@ impl ConfigDaoReal { pub fn new(conn: Box) -> ConfigDaoReal { ConfigDaoReal { conn } } + + fn set_value<'a, P>( + prepare_stmt: P, + name: &str, + value: Option, + ) -> Result<(), ConfigDaoError> + where + P: FnOnce(&str) -> Result, rusqlite::Error> + 'a, + { + let mut stmt = match prepare_stmt("update config set value = ? where name = ?") { + Ok(stmt) => stmt, + Err(e) => return Err(ConfigDaoError::DatabaseError(format!("{}", e))), + }; + let params: &[&dyn ToSql] = &[&value, &name]; + handle_update_execution(stmt.execute(params)) + } } pub trait ConfigDaoFactory { @@ -154,19 +178,18 @@ mod tests { use super::*; use crate::database::db_initializer::DbInitializationConfig; 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, ROPSTEN_TESTNET_CONTRACT_CREATION_BLOCK}; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; + use rusqlite::Connection; + use std::path::Path; #[test] fn get_all_returns_multiple_results() { let home_dir = ensure_node_home_directory_exists("config_dao", "get_all_returns_multiple_results"); - let subject = ConfigDaoReal::new( - DbInitializerReal::default() - .initialize(&home_dir, DbInitializationConfig::test_default()) - .unwrap(), - ); + let subject = make_subject(&home_dir); let result = subject.get_all().unwrap(); @@ -198,11 +221,7 @@ mod tests { "config_dao", "get_returns_not_present_if_row_doesnt_exist", ); - let subject = ConfigDaoReal::new( - DbInitializerReal::default() - .initialize(&home_dir, DbInitializationConfig::test_default()) - .unwrap(), - ); + let subject = make_subject(&home_dir); let result = subject.get("booga"); @@ -212,25 +231,22 @@ mod tests { #[test] fn set_and_get_work() { let home_dir = ensure_node_home_directory_exists("config_dao", "set_and_get_work"); - let subject = ConfigDaoReal::new( - DbInitializerReal::default() - .initialize(&home_dir, DbInitializationConfig::test_default()) - .unwrap(), - ); + let subject = make_subject(&home_dir); let modified_value = ConfigDaoRecord::new( "consuming_wallet_private_key", Some("Two wrongs don't make a right, but two Wrights make an airplane"), true, ); + subject .set( "consuming_wallet_private_key", Some("Two wrongs don't make a right, but two Wrights make an airplane".to_string()), ) .unwrap(); - let subject_get_all = subject.get_all().unwrap(); let subject_get = subject.get("consuming_wallet_private_key").unwrap(); + assert_contains(&subject_get_all, &modified_value); assert_eq!(subject_get, modified_value); } @@ -241,11 +257,7 @@ mod tests { "config_dao", "setting_nonexistent_value_returns_not_present", ); - let subject = ConfigDaoReal::new( - DbInitializerReal::default() - .initialize(&home_dir, DbInitializationConfig::test_default()) - .unwrap(), - ); + let subject = make_subject(&home_dir); let result = subject.set("booga", Some("bigglesworth".to_string())); @@ -258,12 +270,10 @@ mod tests { "config_dao", "setting_value_to_none_removes_value_but_not_row", ); - let subject = ConfigDaoReal::new( - DbInitializerReal::default() - .initialize(&home_dir, DbInitializationConfig::test_default()) - .unwrap(), - ); - let _ = subject.set("schema_version", None).unwrap(); + let subject = make_subject(&home_dir); + + subject.set("schema_version", None).unwrap(); + let result = subject.get("schema_version").unwrap(); assert_eq!(result, ConfigDaoRecord::new("schema_version", None, false)); } @@ -279,4 +289,68 @@ mod tests { )) ) } + + #[test] + fn set_by_guest_transaction_works() { + let home_dir = + ensure_node_home_directory_exists("config_dao", "set_by_guest_transaction_works"); + // We give a useless "Connection" to the DAO object, making it more obvious it was the guest + // connection that did all the job + let subject = ConfigDaoReal::new(Box::new(ConnectionWrapperMock::default())); + // Here we create a database, undoubtedly with the schema same as CURRENT_SCHEMA_VERSION + let mut conn = initialize_conn(&home_dir); + let mut txn = conn.transaction().unwrap(); + let different_than_current_schema_version = CURRENT_SCHEMA_VERSION + 3; + let dif_schema_version_str = different_than_current_schema_version.to_string(); + + subject + .set_by_guest_transaction( + &mut txn, + "schema_version", + Some(dif_schema_version_str.clone()), + ) + .unwrap(); + + let assert_dao = make_subject(&home_dir); + let result_before_commit = assert_dao.get("schema_version").unwrap(); + assert_eq!( + result_before_commit, + ConfigDaoRecord::new( + "schema_version", + Some(&CURRENT_SCHEMA_VERSION.to_string()), + false + ) + ); + txn.commit().unwrap(); + let result_after_commit = assert_dao.get("schema_version").unwrap(); + assert_eq!( + result_after_commit, + ConfigDaoRecord::new("schema_version", Some(&dif_schema_version_str), false) + ); + } + + #[test] + fn set_value_handles_error() { + let conn = Connection::open_in_memory().unwrap(); + let prepare_stm = |stm: &str| conn.prepare(stm); + + let result = ConfigDaoReal::set_value(prepare_stm, "bubbles", None); + + assert_eq!( + result, + Err(ConfigDaoError::DatabaseError( + "no such table: config".to_string() + )) + ) + } + + fn make_subject(home_dir: &Path) -> ConfigDaoReal { + ConfigDaoReal::new(initialize_conn(home_dir)) + } + + fn initialize_conn(home_dir: &Path) -> Box { + DbInitializerReal::default() + .initialize(home_dir, DbInitializationConfig::test_default()) + .unwrap() + } } diff --git a/node/src/db_config/config_dao_null.rs b/node/src/db_config/config_dao_null.rs index 2cc00e527..f1fc58cd4 100644 --- a/node/src/db_config/config_dao_null.rs +++ b/node/src/db_config/config_dao_null.rs @@ -1,6 +1,7 @@ // Copyright (c) 2019-2021, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. use crate::database::db_initializer::DbInitializerReal; +use crate::database::rusqlite_wrappers::TransactionSafeWrapper; use crate::db_config::config_dao::{ConfigDao, ConfigDaoError, ConfigDaoRecord}; use crate::neighborhood::DEFAULT_MIN_HOPS; use crate::sub_lib::accountant::{DEFAULT_PAYMENT_THRESHOLDS, DEFAULT_SCAN_INTERVALS}; @@ -41,6 +42,7 @@ insurmountable, but it would need to be considered and coded around. */ +#[derive(Debug, PartialEq, Eq)] pub struct ConfigDaoNull { data: HashMap, bool)>, } @@ -70,6 +72,15 @@ impl ConfigDao for ConfigDaoNull { fn set(&self, _name: &str, _value: Option) -> Result<(), ConfigDaoError> { Ok(()) } + + fn set_by_guest_transaction( + &self, + _txn: &mut TransactionSafeWrapper, + _name: &str, + _value: Option, + ) -> Result<(), ConfigDaoError> { + Ok(()) + } } impl Default for ConfigDaoNull { @@ -140,6 +151,7 @@ mod tests { use super::*; use crate::database::db_initializer::DbInitializationConfig; use crate::database::db_initializer::DbInitializer; + use crate::database::test_utils::transaction_wrapper_mock::TransactionInnerWrapperMockBuilder; use crate::db_config::config_dao::ConfigDaoReal; use crate::neighborhood::DEFAULT_MIN_HOPS; use masq_lib::blockchains::chains::Chain; @@ -283,4 +295,42 @@ mod tests { assert_eq!(value_pairs, expected_pairs); } + + #[test] + fn set_works_simple() { + let subject = ConfigDaoNull::default(); + + subject.set("param1", Some("value1".to_string())).unwrap(); + subject.set("schema_version", None).unwrap(); + subject + .set("schema_version", Some("456".to_string())) + .unwrap(); + + let subject_data_sorted = subject.data.iter().sorted().collect::>(); + let comparison_subject_data_sorted = subject.data.iter().sorted().collect::>(); + assert_eq!(subject_data_sorted, comparison_subject_data_sorted) + } + + #[test] + fn set_by_guest_transaction_works_simple() { + let subject = ConfigDaoNull::default(); + let txn_inner_builder = TransactionInnerWrapperMockBuilder::default(); + let mut txn = TransactionSafeWrapper::new_with_builder(txn_inner_builder); + let subject_data_before_sorted = subject.data.iter().sorted().collect::>(); + + subject + .set_by_guest_transaction(&mut txn, "param1", Some("value1".to_string())) + .unwrap(); + subject + .set_by_guest_transaction(&mut txn, "schema_version", None) + .unwrap(); + subject + .set_by_guest_transaction(&mut txn, "schema_version", Some("456".to_string())) + .unwrap(); + + let subject_data_after_sorted = subject.data.iter().sorted().collect::>(); + assert_eq!(subject_data_after_sorted, subject_data_before_sorted) + // Test didn't blow up so no method from the txn wrapper was called, + // therefore we don't even have to consider committing the transaction + } } diff --git a/node/src/db_config/mocks.rs b/node/src/db_config/mocks.rs index a3083807c..eeca439f1 100644 --- a/node/src/db_config/mocks.rs +++ b/node/src/db_config/mocks.rs @@ -1,7 +1,8 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. +use crate::database::rusqlite_wrappers::TransactionSafeWrapper; use crate::db_config::config_dao::{ConfigDao, ConfigDaoError, ConfigDaoRecord}; -use masq_lib::utils::to_string; +use crate::test_utils::unshared_test_utils::arbitrary_id_stamp::ArbitraryIdStamp; use std::cell::RefCell; use std::sync::{Arc, Mutex}; @@ -12,6 +13,8 @@ pub struct ConfigDaoMock { get_results: RefCell>>, set_params: Arc)>>>, set_results: RefCell>>, + set_by_guest_transaction_params: Arc)>>>, + set_by_guest_transaction_results: RefCell>>, } impl ConfigDao for ConfigDaoMock { @@ -28,9 +31,23 @@ impl ConfigDao for ConfigDaoMock { self.set_params .lock() .unwrap() - .push((name.to_string(), value.map(to_string))); + .push((name.to_string(), value)); self.set_results.borrow_mut().remove(0) } + + fn set_by_guest_transaction( + &self, + txn: &mut TransactionSafeWrapper, + name: &str, + value: Option, + ) -> Result<(), ConfigDaoError> { + self.set_by_guest_transaction_params.lock().unwrap().push(( + txn.arbitrary_id_stamp(), + name.to_string(), + value, + )); + self.set_by_guest_transaction_results.borrow_mut().remove(0) + } } impl ConfigDaoMock { @@ -41,6 +58,8 @@ impl ConfigDaoMock { get_results: RefCell::new(vec![]), set_params: Arc::new(Mutex::new(vec![])), set_results: RefCell::new(vec![]), + set_by_guest_transaction_params: Arc::new(Mutex::new(vec![])), + set_by_guest_transaction_results: RefCell::new(vec![]), } } @@ -68,4 +87,19 @@ impl ConfigDaoMock { self.set_results.borrow_mut().push(result); self } + + pub fn set_by_guest_transaction_params( + mut self, + params: &Arc)>>>, + ) -> Self { + self.set_by_guest_transaction_params = params.clone(); + self + } + + pub fn set_by_guest_transaction_result(self, result: Result<(), ConfigDaoError>) -> Self { + self.set_by_guest_transaction_results + .borrow_mut() + .push(result); + self + } } diff --git a/node/src/db_config/persistent_configuration.rs b/node/src/db_config/persistent_configuration.rs index 8842b896a..da3fa1583 100644 --- a/node/src/db_config/persistent_configuration.rs +++ b/node/src/db_config/persistent_configuration.rs @@ -3,7 +3,7 @@ use crate::arbitrary_id_stamp_in_trait; use crate::blockchain::bip32::Bip32EncryptionKeyProvider; use crate::blockchain::bip39::{Bip39, Bip39Error}; -use crate::database::connection_wrapper::ConnectionWrapper; +use crate::database::rusqlite_wrappers::{ConnectionWrapper, TransactionSafeWrapper}; use crate::db_config::config_dao::{ConfigDao, ConfigDaoError, ConfigDaoReal, ConfigDaoRecord}; use crate::db_config::secure_config_layer::{SecureConfigLayer, SecureConfigLayerError}; use crate::db_config::typed_config_layer::{ @@ -135,6 +135,11 @@ pub trait PersistentConfiguration { fn set_start_block(&mut self, value: u64) -> Result<(), PersistentConfigError>; fn max_block_count(&self) -> Result, PersistentConfigError>; fn set_max_block_count(&mut self, value: Option) -> Result<(), PersistentConfigError>; + fn set_start_block_from_txn( + &mut self, + value: u64, + transaction: &mut TransactionSafeWrapper, + ) -> Result<(), PersistentConfigError>; fn set_wallet_info( &mut self, consuming_wallet_private_key: &str, @@ -417,6 +422,14 @@ impl PersistentConfiguration for PersistentConfigurationReal { Ok(self.dao.set("max_block_count", encode_u64(value)?)?) } + fn set_start_block_from_txn( + &mut self, + value: u64, + transaction: &mut TransactionSafeWrapper, + ) -> Result<(), PersistentConfigError> { + self.simple_set_method_from_provided_txn("start_block", value, transaction) + } + fn set_wallet_info( &mut self, consuming_wallet_private_key: &str, @@ -552,6 +565,17 @@ impl PersistentConfigurationReal { Ok(self.dao.set(parameter_name, Some(value.to_string()))?) } + fn simple_set_method_from_provided_txn( + &mut self, + parameter_name: &str, + value: T, + txn: &mut TransactionSafeWrapper, + ) -> Result<(), PersistentConfigError> { + Ok(self + .dao + .set_by_guest_transaction(txn, parameter_name, Some(value.to_string()))?) + } + fn simple_get_method( &self, decoder: fn(Option) -> Result, TypedConfigLayerError>, @@ -592,10 +616,12 @@ mod tests { use crate::database::db_initializer::{ DbInitializationConfig, DbInitializer, DbInitializerReal, }; + use crate::database::test_utils::transaction_wrapper_mock::TransactionInnerWrapperMockBuilder; use crate::db_config::config_dao::ConfigDaoRecord; use crate::db_config::mocks::ConfigDaoMock; use crate::db_config::secure_config_layer::EXAMPLE_ENCRYPTED; use crate::test_utils::main_cryptde; + use crate::test_utils::unshared_test_utils::arbitrary_id_stamp::ArbitraryIdStamp; use bip39::{Language, MnemonicType}; use lazy_static::lazy_static; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; @@ -1498,7 +1524,6 @@ mod tests { let set_params_arc = Arc::new(Mutex::new(vec![])); let config_dao = Box::new( ConfigDaoMock::new() - .get_result(Ok(ConfigDaoRecord::new("start_block", Some("1234"), false))) .set_params(&set_params_arc) .set_result(Ok(())), ); @@ -1514,6 +1539,30 @@ mod tests { ) } + #[test] + fn set_start_block_from_txn_success() { + let set_params_arc = Arc::new(Mutex::new(vec![])); + let config_dao = Box::new( + ConfigDaoMock::new() + .set_by_guest_transaction_params(&set_params_arc) + .set_by_guest_transaction_result(Ok(())), + ); + let txn_id = ArbitraryIdStamp::new(); + let txn_inner_builder = + TransactionInnerWrapperMockBuilder::default().set_arbitrary_id_stamp(txn_id); + let mut txn = TransactionSafeWrapper::new_with_builder(txn_inner_builder); + let mut subject = PersistentConfigurationReal::new(config_dao); + + let result = subject.set_start_block_from_txn(1234, &mut txn); + + assert_eq!(result, Ok(())); + let set_params = set_params_arc.lock().unwrap(); + assert_eq!( + *set_params, + vec![(txn_id, "start_block".to_string(), Some("1234".to_string()))] + ) + } + #[test] fn gas_price() { let config_dao = Box::new(ConfigDaoMock::new().get_result(Ok(ConfigDaoRecord::new( @@ -1546,7 +1595,6 @@ mod tests { let set_params_arc = Arc::new(Mutex::new(vec![])); let config_dao = Box::new( ConfigDaoMock::new() - .get_result(Ok(ConfigDaoRecord::new("gas_price", Some("1234"), false))) .set_params(&set_params_arc) .set_result(Ok(())), ); diff --git a/node/src/run_modes_factories.rs b/node/src/run_modes_factories.rs index b3f3d3f08..61a3294ba 100644 --- a/node/src/run_modes_factories.rs +++ b/node/src/run_modes_factories.rs @@ -66,17 +66,17 @@ pub trait DaemonInitializerFactory { pub trait DumpConfigRunner { fn go(&self, streams: &mut StdStreams, args: &[String]) -> RunModeResult; - as_any_in_trait!(); + as_any_ref_in_trait!(); } pub trait ServerInitializer: futures::Future { fn go(&mut self, streams: &mut StdStreams, args: &[String]) -> RunModeResult; - as_any_in_trait!(); + as_any_ref_in_trait!(); } pub trait DaemonInitializer { fn go(&mut self, streams: &mut StdStreams, args: &[String]) -> RunModeResult; - as_any_in_trait!(); + as_any_ref_in_trait!(); } impl DumpConfigRunnerFactory for DumpConfigRunnerFactoryReal { diff --git a/node/src/server_initializer.rs b/node/src/server_initializer.rs index 95ec30799..9bc07212f 100644 --- a/node/src/server_initializer.rs +++ b/node/src/server_initializer.rs @@ -67,7 +67,7 @@ impl ServerInitializer for ServerInitializerReal { .initialize_as_unprivileged(¶ms.multi_config, streams), ) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } impl Future for ServerInitializerReal { diff --git a/node/src/sub_lib/accountant.rs b/node/src/sub_lib/accountant.rs index 15ceb0613..cf87c0803 100644 --- a/node/src/sub_lib/accountant.rs +++ b/node/src/sub_lib/accountant.rs @@ -10,6 +10,7 @@ use crate::accountant::{ }; use crate::actor_system_factory::SubsFactory; use crate::blockchain::blockchain_bridge::PendingPayableFingerprintSeeds; +use crate::db_config::config_dao::ConfigDaoFactory; use crate::sub_lib::peer_actors::{BindMessage, StartMessage}; use crate::sub_lib::wallet::Wallet; use actix::Recipient; @@ -72,6 +73,7 @@ pub struct DaoFactories { pub pending_payable_dao_factory: Box, pub receivable_dao_factory: Box, pub banned_dao_factory: Box, + pub config_dao_factory: Box, } #[derive(PartialEq, Eq, Debug, Clone, Copy)] @@ -174,7 +176,7 @@ pub enum SignConversionError { pub trait MessageIdGenerator { fn id(&self) -> u32; - as_any_in_trait!(); + as_any_ref_in_trait!(); } #[derive(Default)] @@ -184,7 +186,7 @@ impl MessageIdGenerator for MessageIdGeneratorReal { fn id(&self) -> u32 { MSG_ID_INCREMENTER.fetch_add(1, Ordering::Relaxed) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } #[cfg(test)] diff --git a/node/src/sub_lib/utils.rs b/node/src/sub_lib/utils.rs index 4b493cb5d..f6de206d7 100644 --- a/node/src/sub_lib/utils.rs +++ b/node/src/sub_lib/utils.rs @@ -149,7 +149,7 @@ where interval: Duration, ctx: &mut Context, ) -> Box; - as_any_in_trait!(); + as_any_ref_in_trait!(); } #[derive(Default)] @@ -179,7 +179,7 @@ where let handle = ctx.notify_later(msg, interval); Box::new(NLSpawnHandleHolderReal::new(handle)) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } pub trait NotifyHandle @@ -187,7 +187,7 @@ where A: Actor>, { fn notify<'a>(&'a self, msg: M, ctx: &'a mut Context); - as_any_in_trait!(); + as_any_ref_in_trait!(); } impl Default for Box> @@ -234,7 +234,7 @@ where fn notify<'a>(&'a self, msg: M, ctx: &'a mut Context) { ctx.notify(msg) } - as_any_in_trait_impl!(); + as_any_ref_in_trait_impl!(); } pub fn db_connection_launch_panic(err: InitializationError, data_directory: &Path) -> ! { diff --git a/node/src/test_utils/actor_system_factory.rs b/node/src/test_utils/actor_system_factory.rs index ba0baeccc..9c9116b51 100644 --- a/node/src/test_utils/actor_system_factory.rs +++ b/node/src/test_utils/actor_system_factory.rs @@ -1,7 +1,7 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. use crate::accountant::db_access_objects::banned_dao::BannedCacheLoader; -use crate::database::connection_wrapper::ConnectionWrapper; +use crate::database::rusqlite_wrappers::ConnectionWrapper; use std::sync::{Arc, Mutex}; #[derive(Default)] diff --git a/node/src/test_utils/database_utils.rs b/node/src/test_utils/database_utils.rs index ac64b0681..2005166c0 100644 --- a/node/src/test_utils/database_utils.rs +++ b/node/src/test_utils/database_utils.rs @@ -3,8 +3,8 @@ #![cfg(test)] use crate::accountant::db_access_objects::utils::VigilantRusqliteFlatten; -use crate::database::connection_wrapper::ConnectionWrapper; use crate::database::db_initializer::ExternalData; +use crate::database::rusqlite_wrappers::ConnectionWrapper; use crate::database::db_migrations::db_migrator::DbMigrator; use masq_lib::logger::Logger; diff --git a/node/src/test_utils/persistent_configuration_mock.rs b/node/src/test_utils/persistent_configuration_mock.rs index e93ce308c..7b7ace61d 100644 --- a/node/src/test_utils/persistent_configuration_mock.rs +++ b/node/src/test_utils/persistent_configuration_mock.rs @@ -2,6 +2,7 @@ #![cfg(test)] +use crate::database::rusqlite_wrappers::TransactionSafeWrapper; use crate::db_config::persistent_configuration::{PersistentConfigError, PersistentConfiguration}; use crate::sub_lib::accountant::{PaymentThresholds, ScanIntervals}; use crate::sub_lib::neighborhood::{Hops, NodeDescriptor, RatePack}; @@ -64,6 +65,8 @@ 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_results: RefCell>>, payment_thresholds_results: RefCell>>, set_payment_thresholds_params: Arc>>, set_payment_thresholds_results: RefCell>>, @@ -247,6 +250,17 @@ impl PersistentConfiguration for PersistentConfigurationMock { Self::result_from(&self.set_max_block_count_results) } + fn set_start_block_from_txn( + &mut self, + value: u64, + transaction: &mut TransactionSafeWrapper, + ) -> Result<(), PersistentConfigError> { + self.set_start_block_from_txn_params + .lock() + .unwrap() + .push((value, transaction.arbitrary_id_stamp())); + Self::result_from(&self.set_start_block_from_txn_results) + } fn set_wallet_info( &mut self, consuming_wallet_private_key: &str, @@ -570,6 +584,24 @@ impl PersistentConfigurationMock { self } + pub fn set_start_block_from_txn_params( + mut self, + params: &Arc>>, + ) -> Self { + self.set_start_block_from_txn_params = params.clone(); + self + } + + pub fn set_start_block_from_txn_result( + self, + result: Result<(), PersistentConfigError>, + ) -> Self { + self.set_start_block_from_txn_results + .borrow_mut() + .push(result); + self + } + pub fn payment_thresholds_result( self, result: Result, diff --git a/node/tests/utils.rs b/node/tests/utils.rs index b60017063..b8bf69fb3 100644 --- a/node/tests/utils.rs +++ b/node/tests/utils.rs @@ -5,10 +5,10 @@ use masq_lib::blockchains::chains::Chain; use masq_lib::constants::{CURRENT_LOGFILE_NAME, DEFAULT_CHAIN, DEFAULT_UI_PORT}; use masq_lib::test_utils::utils::{ensure_node_home_directory_exists, node_home_directory}; use masq_lib::utils::{add_masq_and_chain_directories, localhost}; -use node_lib::database::connection_wrapper::ConnectionWrapper; use node_lib::database::db_initializer::{ DbInitializationConfig, DbInitializer, DbInitializerReal, }; +use node_lib::database::rusqlite_wrappers::ConnectionWrapper; use node_lib::test_utils::await_value; use regex::{Captures, Regex}; use std::collections::hash_map::Entry; diff --git a/port_exposer/Cargo.lock b/port_exposer/Cargo.lock index 7d2fecf2c..1f2db3a9d 100644 --- a/port_exposer/Cargo.lock +++ b/port_exposer/Cargo.lock @@ -20,7 +20,7 @@ checksum = "6a987beff54b60ffa6d51982e1aa1146bc42f19bd26be28b0586f252fccf5317" [[package]] name = "port_exposer" -version = "0.7.3" +version = "0.8.0" dependencies = [ "default-net", ] diff --git a/port_exposer/Cargo.toml b/port_exposer/Cargo.toml index 1d44abc1d..042deb104 100644 --- a/port_exposer/Cargo.toml +++ b/port_exposer/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "port_exposer" -version = "0.7.3" +version = "0.8.0" authors = ["Dan Wiebe ", "MASQ"] license = "GPL-3.0-only" copyright = "Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved."