From c7cdad284e2887fc5a3ea47977b5d06242528a2a Mon Sep 17 00:00:00 2001 From: Philip Robinson Date: Tue, 23 Nov 2021 11:06:19 +0200 Subject: [PATCH 1/6] docs: ignore RFC code blocks (#3603) Description --- The code blocks in the covenants RFC were failing automated rust tests in CI --- RFC/src/RFC-0250_Covenants.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/RFC/src/RFC-0250_Covenants.md b/RFC/src/RFC-0250_Covenants.md index e3eb65cc15..48891d1188 100644 --- a/RFC/src/RFC-0250_Covenants.md +++ b/RFC/src/RFC-0250_Covenants.md @@ -291,7 +291,7 @@ before being executed. For instance, -``` +```ignore xor( filter_output_hash_eq(Hash(0e0411c70df0ea4243a363fcbf161ebe6e2c1f074faf1c6a316a386823c3753c)), filter_relative_height(10), @@ -300,7 +300,7 @@ xor( is represented in hex bytes as `23 30 01 a8b3f48e39449e89f7ff699b3eb2b080a2479b09a600a19d8ba48d765fe5d47d 35 07 0a`. Let's unpack that as follows: -``` +```ignore 23 // xor - consume two covenant args 30 // filter_output_hash_eq - consume a hash arg 01 // 32-byte hash @@ -365,7 +365,7 @@ one or more outputs. Spend within 10 blocks or burn -``` +```ignore not(filter_relative_height(10)) ``` @@ -377,13 +377,13 @@ the miner. Output features as detailed in [RFC-310-AssetImplementation] (early draft stages, still to be finalised) contain the NFT details. This covenant preserves both the covenant protecting the token, and the token itself. -``` +```ignore filter_fields_preserved([field::features, field::covenant]) ``` ### Side-chain checkpointing -``` +```ignore and( filter_field_int_eq(field::feature_flags, 16) // SIDECHAIN CHECKPOINT = 16 filter_fields_preserved([field::features, field::covenant, field::script]) @@ -392,7 +392,7 @@ and( ### Restrict spending to a particular commitment if not spent within 100 blocks -``` +```ignore or( not(filter_relative_height(100)), filter_fields_hashed_eq([field::commmitment], Hash(xxxx)) @@ -401,7 +401,7 @@ or( ### Output must preserve covenant, features and script or be burnt -``` +```ignore xor( filter_fields_preserved([field::features, field::covenant, field::script]), and( @@ -413,7 +413,7 @@ xor( ### Commission for NFT transfer -``` +```ignore // Must be different outputs xor( and( From befa6215741c37c3c40f7088cdccb4221750a033 Mon Sep 17 00:00:00 2001 From: Philip Robinson Date: Tue, 23 Nov 2021 11:43:12 +0200 Subject: [PATCH 2/6] feat: use CipherSeed wallet birthday for recovery start point (#3602) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Description --- This PR makes use of the wallet birthday encoded into the wallet’s CipherSeed as a starting point for wallet recovery. This is instead of starting at the genesis block and will reduce the amount of work the wallet needs to do significantly by exclude all the blocks before its birthday. This PR implements a new RPC method on the BaseNodeSyncRpcService service called `get_height_at_time` which accepts a Unix Epoch time. The base node will then use a binary search strategy to determine what the block height was at that time. When a fresh Wallet Recovery is started if there isn’t a current progress metadata already stored in the database the wallet will calculate the unix epoch time of two days prior CipherSeeds birthday. This is to account for any timezone issues and does not add much in terms of work to the process. The wallet will then request the height at this time, request the header for that height and will be able to start the recovery process from that point. How Has This Been Tested? --- Tests provided for RPC service and CipherSeed birthday db storage. UTXO Recovery tested manually --- base_layer/core/src/base_node/sync/rpc/mod.rs | 3 + .../core/src/base_node/sync/rpc/service.rs | 55 ++++++++++++++ base_layer/core/tests/base_node_rpc.rs | 76 +++++++++++++++++-- base_layer/key_manager/src/cipher_seed.rs | 12 ++- base_layer/key_manager/src/key_manager.rs | 2 +- base_layer/wallet/src/storage/database.rs | 20 +++++ base_layer/wallet/src/storage/sqlite_db.rs | 6 ++ .../src/utxo_scanner_service/utxo_scanning.rs | 43 ++++++++--- base_layer/wallet/tests/wallet/mod.rs | 37 ++++++++- .../features/WalletRecovery.feature | 1 - 10 files changed, 236 insertions(+), 19 deletions(-) diff --git a/base_layer/core/src/base_node/sync/rpc/mod.rs b/base_layer/core/src/base_node/sync/rpc/mod.rs index 3c0d5ea698..1fc9ec13ac 100644 --- a/base_layer/core/src/base_node/sync/rpc/mod.rs +++ b/base_layer/core/src/base_node/sync/rpc/mod.rs @@ -90,6 +90,9 @@ pub trait BaseNodeSyncService: Send + Sync + 'static { #[rpc(method = 8)] async fn sync_utxos(&self, request: Request) -> Result, RpcStatus>; + + #[rpc(method = 9)] + async fn get_height_at_time(&self, request: Request) -> Result, RpcStatus>; } #[cfg(feature = "base_node")] diff --git a/base_layer/core/src/base_node/sync/rpc/service.rs b/base_layer/core/src/base_node/sync/rpc/service.rs index 9e20df058a..8a48cb3ce1 100644 --- a/base_layer/core/src/base_node/sync/rpc/service.rs +++ b/base_layer/core/src/base_node/sync/rpc/service.rs @@ -462,4 +462,59 @@ impl BaseNodeSyncService for BaseNodeSyncRpcServ Ok(Streaming::new(rx)) } + + async fn get_height_at_time(&self, request: Request) -> Result, RpcStatus> { + let requested_epoch_time: u64 = request.into_message(); + + let tip_header = self + .db() + .fetch_tip_header() + .await + .map_err(RpcStatus::log_internal_error(LOG_TARGET))?; + let mut left_height = 0u64; + let mut right_height = tip_header.height(); + + while left_height <= right_height { + let mut mid_height = (left_height + right_height) / 2; + + if mid_height == 0 { + return Ok(Response::new(0u64)); + } + // If the two bounds are adjacent then perform the test between the right and left sides + if left_height == mid_height { + mid_height = right_height; + } + + let mid_header = self + .db() + .fetch_header(mid_height) + .await + .map_err(RpcStatus::log_internal_error(LOG_TARGET))? + .ok_or_else(|| { + RpcStatus::not_found(format!("Header not found during search at height {}", mid_height)) + })?; + let before_mid_header = self + .db() + .fetch_header(mid_height - 1) + .await + .map_err(RpcStatus::log_internal_error(LOG_TARGET))? + .ok_or_else(|| { + RpcStatus::not_found(format!("Header not found during search at height {}", mid_height - 1)) + })?; + + if requested_epoch_time < mid_header.timestamp.as_u64() && + requested_epoch_time >= before_mid_header.timestamp.as_u64() + { + return Ok(Response::new(before_mid_header.height)); + } else if mid_height == right_height { + return Ok(Response::new(right_height)); + } else if requested_epoch_time <= mid_header.timestamp.as_u64() { + right_height = mid_height; + } else { + left_height = mid_height; + } + } + + Ok(Response::new(0u64)) + } } diff --git a/base_layer/core/tests/base_node_rpc.rs b/base_layer/core/tests/base_node_rpc.rs index 23e2f267fb..7643b5ca28 100644 --- a/base_layer/core/tests/base_node_rpc.rs +++ b/base_layer/core/tests/base_node_rpc.rs @@ -42,7 +42,7 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use std::convert::TryFrom; +use std::{convert::TryFrom, sync::Arc, time::Duration}; use randomx_rs::RandomXFlag; use tempfile::{tempdir, TempDir}; @@ -61,6 +61,8 @@ use tari_core::{ }, rpc::{BaseNodeWalletRpcService, BaseNodeWalletService}, state_machine_service::states::{ListeningInfo, StateInfo, StatusInfo}, + sync::rpc::BaseNodeSyncRpcService, + BaseNodeSyncService, }, blocks::ChainBlock, consensus::{ConsensusManager, ConsensusManagerBuilder, NetworkConsensus}, @@ -80,7 +82,7 @@ use tari_core::{ }; use crate::helpers::{ - block_builders::{chain_block, create_genesis_block_with_coinbase_value}, + block_builders::{chain_block, chain_block_with_new_coinbase, create_genesis_block_with_coinbase_value}, nodes::{BaseNodeBuilder, NodeInterfaces}, }; @@ -88,6 +90,7 @@ mod helpers; async fn setup() -> ( BaseNodeWalletRpcService, + BaseNodeSyncRpcService, NodeInterfaces, RpcRequestMock, ConsensusManager, @@ -118,13 +121,15 @@ async fn setup() -> ( }); let request_mock = RpcRequestMock::new(base_node.comms.peer_manager()); - let service = BaseNodeWalletRpcService::new( + let wallet_service = BaseNodeWalletRpcService::new( base_node.blockchain_db.clone().into(), base_node.mempool_handle.clone(), base_node.state_machine_handle.clone(), ); + let base_node_service = BaseNodeSyncRpcService::new(base_node.blockchain_db.clone().into()); ( - service, + wallet_service, + base_node_service, base_node, request_mock, consensus_manager, @@ -138,7 +143,7 @@ async fn setup() -> ( #[allow(clippy::identity_op)] async fn test_base_node_wallet_rpc() { // Testing the submit_transaction() and transaction_query() rpc calls - let (service, mut base_node, request_mock, consensus_manager, block0, utxo0, _temp_dir) = setup().await; + let (service, _, mut base_node, request_mock, consensus_manager, block0, utxo0, _temp_dir) = setup().await; let (txs1, utxos1) = schema_to_transaction(&[txn_schema!(from: vec![utxo0.clone()], to: vec![1 * T, 1 * T])]); let tx1 = (*txs1[0]).clone(); @@ -290,3 +295,64 @@ async fn test_base_node_wallet_rpc() { .any(|u| u.as_transaction_output(&factories).unwrap().commitment == output.commitment)); } } + +#[tokio::test] +async fn test_get_height_at_time() { + let factories = CryptoFactories::default(); + + let (_, service, base_node, request_mock, consensus_manager, block0, _utxo0, _temp_dir) = setup().await; + + let mut prev_block = block0.clone(); + let mut times = Vec::new(); + times.push(prev_block.header().timestamp); + for _ in 0..10 { + tokio::time::sleep(Duration::from_secs(2)).await; + let new_block = base_node + .blockchain_db + .prepare_new_block(chain_block_with_new_coinbase(&prev_block, vec![], &consensus_manager, &factories).0) + .unwrap(); + + prev_block = base_node + .blockchain_db + .add_block(Arc::new(new_block)) + .unwrap() + .assert_added(); + times.push(prev_block.header().timestamp); + } + + let req = request_mock.request_with_context(Default::default(), times[0].as_u64() - 100); + let resp = service.get_height_at_time(req).await.unwrap().into_message(); + assert_eq!(resp, 0); + + let req = request_mock.request_with_context(Default::default(), times[0].as_u64()); + let resp = service.get_height_at_time(req).await.unwrap().into_message(); + assert_eq!(resp, 0); + + let req = request_mock.request_with_context(Default::default(), times[0].as_u64() + 1); + let resp = service.get_height_at_time(req).await.unwrap().into_message(); + assert_eq!(resp, 0); + + let req = request_mock.request_with_context(Default::default(), times[7].as_u64()); + let resp = service.get_height_at_time(req).await.unwrap().into_message(); + assert_eq!(resp, 7); + + let req = request_mock.request_with_context(Default::default(), times[7].as_u64() - 1); + let resp = service.get_height_at_time(req).await.unwrap().into_message(); + assert_eq!(resp, 6); + + let req = request_mock.request_with_context(Default::default(), times[7].as_u64() + 1); + let resp = service.get_height_at_time(req).await.unwrap().into_message(); + assert_eq!(resp, 7); + + let req = request_mock.request_with_context(Default::default(), times[10].as_u64()); + let resp = service.get_height_at_time(req).await.unwrap().into_message(); + assert_eq!(resp, 10); + + let req = request_mock.request_with_context(Default::default(), times[10].as_u64() - 1); + let resp = service.get_height_at_time(req).await.unwrap().into_message(); + assert_eq!(resp, 9); + + let req = request_mock.request_with_context(Default::default(), times[10].as_u64() + 1); + let resp = service.get_height_at_time(req).await.unwrap().into_message(); + assert_eq!(resp, 10); +} diff --git a/base_layer/key_manager/src/cipher_seed.rs b/base_layer/key_manager/src/cipher_seed.rs index 543071041d..dfffe7fe98 100644 --- a/base_layer/key_manager/src/cipher_seed.rs +++ b/base_layer/key_manager/src/cipher_seed.rs @@ -85,7 +85,7 @@ pub const CIPHER_SEED_MAC_BYTES: usize = 5; pub struct CipherSeed { version: u8, birthday: u16, - pub entropy: [u8; CIPHER_SEED_ENTROPY_BYTES], + entropy: [u8; CIPHER_SEED_ENTROPY_BYTES], salt: [u8; CIPHER_SEED_SALT_BYTES], } @@ -108,7 +108,7 @@ impl CipherSeed { pub fn encipher(&self, passphrase: Option) -> Result, KeyManagerError> { let mut plaintext = self.birthday.to_le_bytes().to_vec(); - plaintext.append(&mut self.entropy.clone().to_vec()); + plaintext.append(&mut self.entropy().clone().to_vec()); let passphrase = passphrase.unwrap_or_else(|| DEFAULT_CIPHER_SEED_PASSPHRASE.to_string()); @@ -236,6 +236,14 @@ impl CipherSeed { Ok(()) } + + pub fn entropy(&self) -> [u8; CIPHER_SEED_ENTROPY_BYTES] { + self.entropy + } + + pub fn birthday(&self) -> u16 { + self.birthday + } } impl Drop for CipherSeed { diff --git a/base_layer/key_manager/src/key_manager.rs b/base_layer/key_manager/src/key_manager.rs index 3ca14c409a..b95e750374 100644 --- a/base_layer/key_manager/src/key_manager.rs +++ b/base_layer/key_manager/src/key_manager.rs @@ -74,7 +74,7 @@ where /// Derive a new private key from master key: derived_key=SHA256(master_key||branch_seed||index) pub fn derive_key(&self, key_index: u64) -> Result, ByteArrayError> { - let concatenated = format!("{}{}", self.seed.entropy.to_vec().to_hex(), key_index.to_string()); + let concatenated = format!("{}{}", self.seed.entropy().to_vec().to_hex(), key_index.to_string()); match K::from_bytes(D::digest(&concatenated.into_bytes()).as_slice()) { Ok(k) => Ok(DerivedKey { k, key_index }), Err(e) => Err(e), diff --git a/base_layer/wallet/src/storage/database.rs b/base_layer/wallet/src/storage/database.rs index b0f9021f3b..ae22123969 100644 --- a/base_layer/wallet/src/storage/database.rs +++ b/base_layer/wallet/src/storage/database.rs @@ -55,6 +55,7 @@ pub enum DbKey { MasterSeed, PassphraseHash, EncryptionSalt, + WalletBirthday, } pub enum DbValue { @@ -67,6 +68,7 @@ pub enum DbValue { MasterSeed(CipherSeed), PassphraseHash(String), EncryptionSalt(String), + WalletBirthday(String), } #[derive(Clone)] @@ -306,6 +308,22 @@ where T: WalletBackend + 'static .map_err(|err| WalletStorageError::BlockingTaskSpawnError(err.to_string()))??; Ok(c) } + + pub async fn get_wallet_birthday(&self) -> Result { + let db_clone = self.db.clone(); + + let result = tokio::task::spawn_blocking(move || match db_clone.fetch(&DbKey::WalletBirthday) { + Ok(None) => Err(WalletStorageError::ValueNotFound(DbKey::WalletBirthday)), + Ok(Some(DbValue::WalletBirthday(b))) => Ok(b + .parse::() + .map_err(|_| WalletStorageError::ConversionError("Could not parse wallet birthday".to_string()))?), + Ok(Some(other)) => unexpected_result(DbKey::WalletBirthday, other), + Err(e) => log_error(DbKey::WalletBirthday, e), + }) + .await + .map_err(|err| WalletStorageError::BlockingTaskSpawnError(err.to_string()))??; + Ok(result) + } } impl Display for DbKey { @@ -319,6 +337,7 @@ impl Display for DbKey { DbKey::BaseNodeChainMetadata => f.write_str(&"Last seen Chain metadata from base node".to_string()), DbKey::PassphraseHash => f.write_str(&"PassphraseHash".to_string()), DbKey::EncryptionSalt => f.write_str(&"EncryptionSalt".to_string()), + DbKey::WalletBirthday => f.write_str(&"WalletBirthday".to_string()), } } } @@ -335,6 +354,7 @@ impl Display for DbValue { DbValue::BaseNodeChainMetadata(v) => f.write_str(&format!("Last seen Chain metadata from base node:{}", v)), DbValue::PassphraseHash(h) => f.write_str(&format!("PassphraseHash: {}", h)), DbValue::EncryptionSalt(s) => f.write_str(&format!("EncryptionSalt: {}", s)), + DbValue::WalletBirthday(b) => f.write_str(&format!("WalletBirthday: {}", b)), } } } diff --git a/base_layer/wallet/src/storage/sqlite_db.rs b/base_layer/wallet/src/storage/sqlite_db.rs index 27295998fa..4f9c2aa364 100644 --- a/base_layer/wallet/src/storage/sqlite_db.rs +++ b/base_layer/wallet/src/storage/sqlite_db.rs @@ -83,7 +83,9 @@ impl WalletSqliteDatabase { match cipher.as_ref() { None => { let seed_bytes = seed.encipher(None)?; + let birthday = seed.birthday(); WalletSettingSql::new(DbKey::MasterSeed.to_string(), seed_bytes.to_hex()).set(conn)?; + WalletSettingSql::new(DbKey::WalletBirthday.to_string(), birthday.to_string()).set(conn)?; }, Some(cipher) => { let seed_bytes = seed.encipher(None)?; @@ -305,6 +307,9 @@ impl WalletSqliteDatabase { DbKey::EncryptionSalt => { return Err(WalletStorageError::OperationNotSupported); }, + DbKey::WalletBirthday => { + return Err(WalletStorageError::OperationNotSupported); + }, }; if start.elapsed().as_millis() > 0 { trace!( @@ -346,6 +351,7 @@ impl WalletBackend for WalletSqliteDatabase { DbKey::BaseNodeChainMetadata => self.get_chain_metadata(&conn)?.map(DbValue::BaseNodeChainMetadata), DbKey::PassphraseHash => WalletSettingSql::get(key.to_string(), &conn)?.map(DbValue::PassphraseHash), DbKey::EncryptionSalt => WalletSettingSql::get(key.to_string(), &conn)?.map(DbValue::EncryptionSalt), + DbKey::WalletBirthday => WalletSettingSql::get(key.to_string(), &conn)?.map(DbValue::WalletBirthday), }; if start.elapsed().as_millis() > 0 { trace!( diff --git a/base_layer/wallet/src/utxo_scanner_service/utxo_scanning.rs b/base_layer/wallet/src/utxo_scanner_service/utxo_scanning.rs index e6fb3f1011..e807208b9f 100644 --- a/base_layer/wallet/src/utxo_scanner_service/utxo_scanning.rs +++ b/base_layer/wallet/src/utxo_scanner_service/utxo_scanning.rs @@ -338,14 +338,15 @@ where TBackend: WalletBackend + 'static } async fn get_start_utxo_mmr_pos(&self, client: &mut BaseNodeSyncRpcClient) -> Result { - let metadata = self.get_metadata().await?.unwrap_or_default(); - if metadata.height_hash.is_empty() { - // Set a value in here so that if the recovery fails on the genesis block the client will know a - // recover was started. Important on Console wallet that otherwise makes this decision based on the - // presence of the data file - self.set_metadata(metadata).await?; - return Ok(0); - } + let metadata = match self.get_metadata().await? { + None => { + let birthday_metadata = self.get_birthday_metadata(client).await?; + self.set_metadata(birthday_metadata.clone()).await?; + return Ok(birthday_metadata.utxo_index); + }, + Some(m) => m, + }; + // if it's none, we return 0 above. let request = FindChainSplitRequest { block_hashes: vec![metadata.height_hash], @@ -635,6 +636,30 @@ where TBackend: WalletBackend + 'static self.peer_index += 1; peer } + + async fn get_birthday_metadata( + &self, + client: &mut BaseNodeSyncRpcClient, + ) -> Result { + let birthday = self.resources.db.get_wallet_birthday().await?; + // Calculate the unix epoch time of two days before the wallet birthday. This is to avoid any weird time zone + // issues + let epoch_time = (birthday.saturating_sub(2) as u64) * 60 * 60 * 24; + let block_height = client.get_height_at_time(epoch_time).await?; + let header = client.get_header_by_height(block_height).await?; + let header = BlockHeader::try_from(header).map_err(|_| UtxoScannerError::ConversionError)?; + + info!( + target: LOG_TARGET, + "Fresh wallet recovery starting at Block {}", block_height + ); + Ok(ScanningMetadata { + total_amount: Default::default(), + number_of_utxos: 0, + utxo_index: header.output_mmr_size, + height_hash: header.hash(), + }) + } } pub struct UtxoScannerService @@ -783,7 +808,7 @@ fn convert_response_to_transaction_outputs( Ok((outputs, current_utxo_index)) } -#[derive(Default, Serialize, Deserialize)] +#[derive(Clone, Default, Serialize, Deserialize)] struct ScanningMetadata { pub total_amount: MicroTari, pub number_of_utxos: u64, diff --git a/base_layer/wallet/tests/wallet/mod.rs b/base_layer/wallet/tests/wallet/mod.rs index bd6e6f8d79..fb79579c02 100644 --- a/base_layer/wallet/tests/wallet/mod.rs +++ b/base_layer/wallet/tests/wallet/mod.rs @@ -46,7 +46,7 @@ use tari_core::transactions::{ transaction_entities::OutputFeatures, CryptoFactories, }; -use tari_key_manager::cipher_seed::CipherSeed; +use tari_key_manager::{cipher_seed::CipherSeed, mnemonic::Mnemonic}; use tari_p2p::{initialization::P2pConfig, transport::TransportType, Network, DEFAULT_DNS_NAME_SERVER}; use tari_shutdown::{Shutdown, ShutdownSignal}; use tari_test_utils::random; @@ -69,6 +69,7 @@ use tari_wallet::{ handle::TransactionEvent, storage::sqlite_db::TransactionServiceSqliteDatabase, }, + utxo_scanner_service::utxo_scanning::UtxoScannerService, Wallet, WalletConfig, WalletSqlite, @@ -774,3 +775,37 @@ fn test_db_file_locking() { assert!(run_migration_and_create_sqlite_connection(&wallet_path, 16).is_ok()); } + +#[tokio::test] +async fn test_recovery_birthday() { + let dir = tempdir().unwrap(); + let factories = CryptoFactories::default(); + let shutdown = Shutdown::new(); + + let seed_words: Vec = [ + "cactus", "pool", "fuel", "skull", "chair", "casino", "season", "disorder", "flat", "crash", "wrist", + "whisper", "decorate", "narrow", "oxygen", "remember", "minor", "among", "happy", "cricket", "embark", "blue", + "ship", "sick", + ] + .to_vec() + .iter() + .map(|w| w.to_string()) + .collect(); + + let recovery_seed = CipherSeed::from_mnemonic(seed_words.as_slice(), None).unwrap(); + let birthday = recovery_seed.birthday(); + + let wallet = create_wallet( + dir.path(), + "wallet_db", + factories.clone(), + shutdown.to_signal(), + None, + Some(recovery_seed), + ) + .await + .unwrap(); + + let db_birthday = wallet.db.get_wallet_birthday().await.unwrap(); + assert_eq!(birthday, db_birthday); +} diff --git a/integration_tests/features/WalletRecovery.feature b/integration_tests/features/WalletRecovery.feature index d003de4718..5b4484e708 100644 --- a/integration_tests/features/WalletRecovery.feature +++ b/integration_tests/features/WalletRecovery.feature @@ -1,7 +1,6 @@ @wallet-recovery @wallet Feature: Wallet Recovery - Scenario: Wallet recovery with connected base node staying online Given I have a seed node NODE And I have 1 base nodes connected to all seed nodes From cc846cdf0495082d4e11939b311e57f05fd7425e Mon Sep 17 00:00:00 2001 From: Hansie Odendaal <39146854+hansieodendaal@users.noreply.github.com> Date: Tue, 23 Nov 2021 12:14:33 +0200 Subject: [PATCH 3/6] test: improve cucumber scenario robustness (#3599) Description --- Improved cucumber scenario robustness by explicitly waiting on the command mode to finish before proceeding with the next step for: - `Scenario: Wallet imports spent outputs that become invalidated` - `Scenario: Wallet imports reorged outputs that become invalidated` Motivation and Context --- The above-mentioned tests failed now and again, at least 1 in 10. How Has This Been Tested? --- Repeatedly running these tests 10 times on two different computers. - `npm test -- --name "Wallet imports spent outputs that become invalidated"` - `npm test -- --name "Wallet imports reorged outputs that become invalidated"` --- integration_tests/helpers/walletProcess.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/integration_tests/helpers/walletProcess.js b/integration_tests/helpers/walletProcess.js index 549b4993d2..aad8ed7048 100644 --- a/integration_tests/helpers/walletProcess.js +++ b/integration_tests/helpers/walletProcess.js @@ -252,6 +252,7 @@ class WalletProcess { } async exportSpentOutputs() { + await this.stop(); const args = [ "--init", "--base-path", @@ -262,11 +263,13 @@ class WalletProcess { "--command", "export-spent-utxos --csv-file exported_outputs.csv", ]; + let output = { buffer: "" }; outputProcess = __dirname + "/../temp/out/tari_console_wallet"; - await this.run(outputProcess, args, true); + await this.run(outputProcess, args, true, "\n", output, true); } async exportUnspentOutputs() { + await this.stop(); const args = [ "--init", "--base-path", @@ -277,8 +280,9 @@ class WalletProcess { "--command", "export-utxos --csv-file exported_outputs.csv", ]; + let output = { buffer: "" }; outputProcess = __dirname + "/../temp/out/tari_console_wallet"; - await this.run(outputProcess, args, true); + await this.run(outputProcess, args, true, "\n", output, true); } async readExportedOutputs() { From 3b3da21830fc098b8038b30b1d947e98f9198ede Mon Sep 17 00:00:00 2001 From: David Main <51991544+StriderDM@users.noreply.github.com> Date: Tue, 23 Nov 2021 12:49:21 +0200 Subject: [PATCH 4/6] feat!: expose reason for transaction cancellation for callback in wallet_ffi (#3601) Description --- Exposes reason for transaction cancellation to wallet_ffi via the transaction cancellation callback. Additionally removes outdated docs and fixes Clippy errors. Motivation and Context --- How Has This Been Tested? --- cargo test --all --- .../src/ui/state/wallet_event_monitor.rs | 2 +- base_layer/p2p/src/services/liveness/state.rs | 4 +- .../wallet/src/transaction_service/handle.rs | 3 +- .../src/transaction_service/protocols/mod.rs | 11 ++++ .../transaction_broadcast_protocol.rs | 38 +++++++++---- .../protocols/transaction_receive_protocol.rs | 7 ++- .../protocols/transaction_send_protocol.rs | 6 ++- .../transaction_validation_protocol.rs | 3 +- .../wallet/src/transaction_service/service.rs | 6 ++- .../tests/transaction_service/service.rs | 14 ++--- .../transaction_protocols.rs | 4 +- base_layer/wallet/tests/wallet/mod.rs | 2 +- base_layer/wallet_ffi/src/callback_handler.rs | 12 ++--- .../wallet_ffi/src/callback_handler_tests.rs | 18 +++++-- base_layer/wallet_ffi/src/lib.rs | 54 +++++++------------ base_layer/wallet_ffi/wallet.h | 21 ++++++-- integration_tests/helpers/ffi/ffiInterface.js | 2 +- integration_tests/helpers/ffi/wallet.js | 4 +- 18 files changed, 130 insertions(+), 81 deletions(-) diff --git a/applications/tari_console_wallet/src/ui/state/wallet_event_monitor.rs b/applications/tari_console_wallet/src/ui/state/wallet_event_monitor.rs index dca471ee9b..c1b51207d0 100644 --- a/applications/tari_console_wallet/src/ui/state/wallet_event_monitor.rs +++ b/applications/tari_console_wallet/src/ui/state/wallet_event_monitor.rs @@ -100,7 +100,7 @@ impl WalletEventMonitor { self.trigger_balance_refresh(); notifier.transaction_mined(tx_id); }, - TransactionEvent::TransactionCancelled(tx_id) => { + TransactionEvent::TransactionCancelled(tx_id, _) => { self.trigger_tx_state_refresh(tx_id).await; self.trigger_balance_refresh(); notifier.transaction_cancelled(tx_id); diff --git a/base_layer/p2p/src/services/liveness/state.rs b/base_layer/p2p/src/services/liveness/state.rs index 4e89d8e91f..3fc0478dba 100644 --- a/base_layer/p2p/src/services/liveness/state.rs +++ b/base_layer/p2p/src/services/liveness/state.rs @@ -152,7 +152,7 @@ impl LivenessState { self.failed_pings .entry(node_id) .and_modify(|v| { - *v = *v + 1; + *v += 1; }) .or_insert(1); } @@ -167,7 +167,7 @@ impl LivenessState { /// a latency sample is added and calculated. The given `peer` must match the recorded peer pub fn record_pong(&mut self, nonce: u64, sent_by: &NodeId) -> Option { self.inc_pongs_received(); - self.failed_pings.remove_entry(&sent_by); + self.failed_pings.remove_entry(sent_by); let (node_id, _) = self.inflight_pings.get(&nonce)?; if node_id == sent_by { diff --git a/base_layer/wallet/src/transaction_service/handle.rs b/base_layer/wallet/src/transaction_service/handle.rs index 0f91f8fee8..6e914aebde 100644 --- a/base_layer/wallet/src/transaction_service/handle.rs +++ b/base_layer/wallet/src/transaction_service/handle.rs @@ -36,6 +36,7 @@ use tari_service_framework::reply_channel::SenderService; use crate::transaction_service::{ error::TransactionServiceError, + protocols::TxRejection, storage::models::{CompletedTransaction, InboundTransaction, OutboundTransaction, WalletTransaction}, }; @@ -150,7 +151,7 @@ pub enum TransactionEvent { TransactionDirectSendResult(TxId, bool), TransactionCompletedImmediately(TxId), TransactionStoreForwardSendResult(TxId, bool), - TransactionCancelled(TxId), + TransactionCancelled(TxId, TxRejection), TransactionBroadcast(TxId), TransactionImported(TxId), TransactionMined { diff --git a/base_layer/wallet/src/transaction_service/protocols/mod.rs b/base_layer/wallet/src/transaction_service/protocols/mod.rs index 15bdb1dd1c..664aab6146 100644 --- a/base_layer/wallet/src/transaction_service/protocols/mod.rs +++ b/base_layer/wallet/src/transaction_service/protocols/mod.rs @@ -20,6 +20,17 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub enum TxRejection { + Unknown, // 0 + UserCancelled, // 1 + Timeout, // 2 + DoubleSpend, // 3 + Orphan, // 4 + TimeLocked, // 5 + InvalidTransaction, // 6 +} + pub mod transaction_broadcast_protocol; pub mod transaction_receive_protocol; pub mod transaction_send_protocol; diff --git a/base_layer/wallet/src/transaction_service/protocols/transaction_broadcast_protocol.rs b/base_layer/wallet/src/transaction_service/protocols/transaction_broadcast_protocol.rs index 7bcf3ccb3e..4983a7d140 100644 --- a/base_layer/wallet/src/transaction_service/protocols/transaction_broadcast_protocol.rs +++ b/base_layer/wallet/src/transaction_service/protocols/transaction_broadcast_protocol.rs @@ -48,6 +48,7 @@ use crate::{ transaction_service::{ error::{TransactionServiceError, TransactionServiceProtocolError}, handle::TransactionEvent, + protocols::TxRejection, service::TransactionServiceResources, storage::{database::TransactionBackend, models::CompletedTransaction}, }, @@ -215,10 +216,31 @@ where self.cancel_transaction().await; + let reason = match response.rejection_reason { + TxSubmissionRejectionReason::None | TxSubmissionRejectionReason::ValidationFailed => { + TransactionServiceError::MempoolRejectionInvalidTransaction + }, + TxSubmissionRejectionReason::DoubleSpend => TransactionServiceError::MempoolRejectionDoubleSpend, + TxSubmissionRejectionReason::Orphan => TransactionServiceError::MempoolRejectionOrphan, + TxSubmissionRejectionReason::TimeLocked => TransactionServiceError::MempoolRejectionTimeLocked, + _ => TransactionServiceError::UnexpectedBaseNodeResponse, + }; + + let cancellation_event_reason = match reason { + TransactionServiceError::MempoolRejectionInvalidTransaction => TxRejection::InvalidTransaction, + TransactionServiceError::MempoolRejectionDoubleSpend => TxRejection::DoubleSpend, + TransactionServiceError::MempoolRejectionOrphan => TxRejection::Orphan, + TransactionServiceError::MempoolRejectionTimeLocked => TxRejection::TimeLocked, + _ => TxRejection::Unknown, + }; + let _ = self .resources .event_publisher - .send(Arc::new(TransactionEvent::TransactionCancelled(self.tx_id))) + .send(Arc::new(TransactionEvent::TransactionCancelled( + self.tx_id, + cancellation_event_reason, + ))) .map_err(|e| { trace!( target: LOG_TARGET, @@ -228,15 +250,6 @@ where e }); - let reason = match response.rejection_reason { - TxSubmissionRejectionReason::None | TxSubmissionRejectionReason::ValidationFailed => { - TransactionServiceError::MempoolRejectionInvalidTransaction - }, - TxSubmissionRejectionReason::DoubleSpend => TransactionServiceError::MempoolRejectionDoubleSpend, - TxSubmissionRejectionReason::Orphan => TransactionServiceError::MempoolRejectionOrphan, - TxSubmissionRejectionReason::TimeLocked => TransactionServiceError::MempoolRejectionTimeLocked, - _ => TransactionServiceError::UnexpectedBaseNodeResponse, - }; return Err(TransactionServiceProtocolError::new(self.tx_id, reason)); } else if response.rejection_reason == TxSubmissionRejectionReason::AlreadyMined { info!( @@ -342,7 +355,10 @@ where let _ = self .resources .event_publisher - .send(Arc::new(TransactionEvent::TransactionCancelled(self.tx_id))) + .send(Arc::new(TransactionEvent::TransactionCancelled( + self.tx_id, + TxRejection::InvalidTransaction, + ))) .map_err(|e| { trace!( target: LOG_TARGET, diff --git a/base_layer/wallet/src/transaction_service/protocols/transaction_receive_protocol.rs b/base_layer/wallet/src/transaction_service/protocols/transaction_receive_protocol.rs index 2be90c9b48..1ce1e4ba9c 100644 --- a/base_layer/wallet/src/transaction_service/protocols/transaction_receive_protocol.rs +++ b/base_layer/wallet/src/transaction_service/protocols/transaction_receive_protocol.rs @@ -39,7 +39,7 @@ use tari_common_types::transaction::{TransactionDirection, TransactionStatus, Tx use tari_comms::types::CommsPublicKey; use tokio::sync::{mpsc, oneshot}; -use crate::connectivity_service::WalletConnectivityInterface; +use crate::{connectivity_service::WalletConnectivityInterface, transaction_service::protocols::TxRejection}; use tari_common_types::types::HashOutput; use tari_core::transactions::{ transaction_entities::Transaction, @@ -504,7 +504,10 @@ where let _ = self .resources .event_publisher - .send(Arc::new(TransactionEvent::TransactionCancelled(self.id))) + .send(Arc::new(TransactionEvent::TransactionCancelled( + self.id, + TxRejection::Timeout, + ))) .map_err(|e| { trace!( target: LOG_TARGET, diff --git a/base_layer/wallet/src/transaction_service/protocols/transaction_send_protocol.rs b/base_layer/wallet/src/transaction_service/protocols/transaction_send_protocol.rs index 34d68644df..5812f2ac58 100644 --- a/base_layer/wallet/src/transaction_service/protocols/transaction_send_protocol.rs +++ b/base_layer/wallet/src/transaction_service/protocols/transaction_send_protocol.rs @@ -26,6 +26,7 @@ use crate::{ config::TransactionRoutingMechanism, error::{TransactionServiceError, TransactionServiceProtocolError}, handle::{TransactionEvent, TransactionServiceResponse}, + protocols::TxRejection, service::TransactionServiceResources, storage::{ database::TransactionBackend, @@ -826,7 +827,10 @@ where let _ = self .resources .event_publisher - .send(Arc::new(TransactionEvent::TransactionCancelled(self.id))) + .send(Arc::new(TransactionEvent::TransactionCancelled( + self.id, + TxRejection::Timeout, + ))) .map_err(|e| { trace!( target: LOG_TARGET, diff --git a/base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs b/base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs index 639a246e7a..bf34834081 100644 --- a/base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs +++ b/base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs @@ -64,6 +64,7 @@ pub struct TransactionValidationProtocol { - if let TransactionEvent::TransactionCancelled(_) = &*event.unwrap() { + if let TransactionEvent::TransactionCancelled(..) = &*event.unwrap() { cancelled = true; break; } @@ -2380,7 +2380,7 @@ fn test_transaction_cancellation() { loop { tokio::select! { event = alice_event_stream.recv() => { - if let TransactionEvent::TransactionCancelled(_) = &*event.unwrap() { + if let TransactionEvent::TransactionCancelled(..) = &*event.unwrap() { cancelled = true; break; } @@ -3524,7 +3524,7 @@ fn test_coinbase_abandoned() { loop { tokio::select! { event = alice_event_stream.recv() => { - if let TransactionEvent::TransactionCancelled(tx_id) = &*event.unwrap() { + if let TransactionEvent::TransactionCancelled(tx_id, _) = &*event.unwrap() { if tx_id == &tx_id1 { count += 1; } @@ -3684,7 +3684,7 @@ fn test_coinbase_abandoned() { count += 1; } }, - TransactionEvent::TransactionCancelled(tx_id) => { + TransactionEvent::TransactionCancelled(tx_id, _) => { if tx_id == &tx_id2 { count += 1; } @@ -3771,7 +3771,7 @@ fn test_coinbase_abandoned() { count += 1; } }, - TransactionEvent::TransactionCancelled(tx_id) => { + TransactionEvent::TransactionCancelled(tx_id, _) => { if tx_id == &tx_id1 { count += 1; } @@ -4755,7 +4755,7 @@ fn test_transaction_timeout_cancellation() { loop { tokio::select! { event = carol_event_stream.recv() => { - if let TransactionEvent::TransactionCancelled(t) = &*event.unwrap() { + if let TransactionEvent::TransactionCancelled(t, _) = &*event.unwrap() { if t == &tx_id { transaction_cancelled = true; break; @@ -5074,7 +5074,7 @@ fn transaction_service_tx_broadcast() { loop { tokio::select! { event = alice_event_stream.recv() => { - if let TransactionEvent::TransactionCancelled(tx_id) = &*event.unwrap(){ + if let TransactionEvent::TransactionCancelled(tx_id, _) = &*event.unwrap(){ if tx_id == &tx_id2 { tx2_cancelled = true; break; diff --git a/base_layer/wallet/tests/transaction_service/transaction_protocols.rs b/base_layer/wallet/tests/transaction_service/transaction_protocols.rs index 102c744769..e38cf6f022 100644 --- a/base_layer/wallet/tests/transaction_service/transaction_protocols.rs +++ b/base_layer/wallet/tests/transaction_service/transaction_protocols.rs @@ -370,7 +370,7 @@ async fn tx_broadcast_protocol_submit_rejection() { loop { tokio::select! { event = event_stream.recv() => { - if let TransactionEvent::TransactionCancelled(_) = &*event.unwrap() { + if let TransactionEvent::TransactionCancelled(..) = &*event.unwrap() { cancelled = true; } }, @@ -547,7 +547,7 @@ async fn tx_broadcast_protocol_submit_success_followed_by_rejection() { loop { tokio::select! { event = event_stream.recv() => { - if let TransactionEvent::TransactionCancelled(_) = &*event.unwrap() { + if let TransactionEvent::TransactionCancelled(..) = &*event.unwrap() { cancelled = true; } }, diff --git a/base_layer/wallet/tests/wallet/mod.rs b/base_layer/wallet/tests/wallet/mod.rs index fb79579c02..d54c307c01 100644 --- a/base_layer/wallet/tests/wallet/mod.rs +++ b/base_layer/wallet/tests/wallet/mod.rs @@ -633,7 +633,7 @@ fn test_store_and_forward_send_tx() { event = carol_event_stream.recv() => { match &*event.unwrap() { TransactionEvent::ReceivedTransaction(_) => tx_recv = true, - TransactionEvent::TransactionCancelled(_) => tx_cancelled = true, + TransactionEvent::TransactionCancelled(..) => tx_cancelled = true, _ => (), } if tx_recv && tx_cancelled { diff --git a/base_layer/wallet_ffi/src/callback_handler.rs b/base_layer/wallet_ffi/src/callback_handler.rs index fffbe5800e..9418b38e1b 100644 --- a/base_layer/wallet_ffi/src/callback_handler.rs +++ b/base_layer/wallet_ffi/src/callback_handler.rs @@ -88,7 +88,7 @@ where TBackend: TransactionBackend + 'static callback_transaction_mined_unconfirmed: unsafe extern "C" fn(*mut CompletedTransaction, u64), callback_direct_send_result: unsafe extern "C" fn(TxId, bool), callback_store_and_forward_send_result: unsafe extern "C" fn(TxId, bool), - callback_transaction_cancellation: unsafe extern "C" fn(*mut CompletedTransaction), + callback_transaction_cancellation: unsafe extern "C" fn(*mut CompletedTransaction, u64), callback_txo_validation_complete: unsafe extern "C" fn(u64, u8), callback_balance_updated: unsafe extern "C" fn(*mut Balance), callback_transaction_validation_complete: unsafe extern "C" fn(u64, u8), @@ -123,7 +123,7 @@ where TBackend: TransactionBackend + 'static callback_transaction_mined_unconfirmed: unsafe extern "C" fn(*mut CompletedTransaction, u64), callback_direct_send_result: unsafe extern "C" fn(TxId, bool), callback_store_and_forward_send_result: unsafe extern "C" fn(TxId, bool), - callback_transaction_cancellation: unsafe extern "C" fn(*mut CompletedTransaction), + callback_transaction_cancellation: unsafe extern "C" fn(*mut CompletedTransaction, u64), callback_txo_validation_complete: unsafe extern "C" fn(TxId, u8), callback_balance_updated: unsafe extern "C" fn(*mut Balance), callback_transaction_validation_complete: unsafe extern "C" fn(TxId, u8), @@ -242,8 +242,8 @@ where TBackend: TransactionBackend + 'static self.receive_store_and_forward_send_result(tx_id, result); self.trigger_balance_refresh().await; }, - TransactionEvent::TransactionCancelled(tx_id) => { - self.receive_transaction_cancellation(tx_id).await; + TransactionEvent::TransactionCancelled(tx_id, reason) => { + self.receive_transaction_cancellation(tx_id, reason as u64).await; self.trigger_balance_refresh().await; }, TransactionEvent::TransactionBroadcast(tx_id) => { @@ -425,7 +425,7 @@ where TBackend: TransactionBackend + 'static } } - async fn receive_transaction_cancellation(&mut self, tx_id: TxId) { + async fn receive_transaction_cancellation(&mut self, tx_id: TxId, reason: u64) { let mut transaction = None; if let Ok(tx) = self.db.get_cancelled_completed_transaction(tx_id).await { transaction = Some(tx); @@ -451,7 +451,7 @@ where TBackend: TransactionBackend + 'static ); let boxing = Box::into_raw(Box::new(tx)); unsafe { - (self.callback_transaction_cancellation)(boxing); + (self.callback_transaction_cancellation)(boxing, reason); } }, } diff --git a/base_layer/wallet_ffi/src/callback_handler_tests.rs b/base_layer/wallet_ffi/src/callback_handler_tests.rs index e544fab345..4d90f2b3ee 100644 --- a/base_layer/wallet_ffi/src/callback_handler_tests.rs +++ b/base_layer/wallet_ffi/src/callback_handler_tests.rs @@ -63,6 +63,7 @@ mod test { }; use crate::{callback_handler::CallbackHandler, output_manager_service_mock::MockOutputManagerService}; + use tari_wallet::transaction_service::protocols::TxRejection; struct CallbackState { pub received_tx_callback_called: bool, @@ -168,7 +169,7 @@ mod test { drop(lock); } - unsafe extern "C" fn tx_cancellation_callback(tx: *mut CompletedTransaction) { + unsafe extern "C" fn tx_cancellation_callback(tx: *mut CompletedTransaction, _reason: u64) { let mut lock = CALLBACK_STATE.lock().unwrap(); match (*tx).tx_id { 3 => lock.tx_cancellation_callback_called_inbound = true, @@ -415,7 +416,10 @@ mod test { mock_output_manager_service_state.set_balance(balance.clone()); // Balance updated should be detected with following event, total = 4 times transaction_event_sender - .send(Arc::new(TransactionEvent::TransactionCancelled(3u64))) + .send(Arc::new(TransactionEvent::TransactionCancelled( + 3u64, + TxRejection::UserCancelled, + ))) .unwrap(); let start = Instant::now(); while start.elapsed().as_secs() < 10 { @@ -431,11 +435,17 @@ mod test { assert_eq!(callback_balance_updated, 4); transaction_event_sender - .send(Arc::new(TransactionEvent::TransactionCancelled(4u64))) + .send(Arc::new(TransactionEvent::TransactionCancelled( + 4u64, + TxRejection::UserCancelled, + ))) .unwrap(); transaction_event_sender - .send(Arc::new(TransactionEvent::TransactionCancelled(5u64))) + .send(Arc::new(TransactionEvent::TransactionCancelled( + 5u64, + TxRejection::UserCancelled, + ))) .unwrap(); oms_event_sender diff --git a/base_layer/wallet_ffi/src/lib.rs b/base_layer/wallet_ffi/src/lib.rs index d5ec154748..2a65bf8fd5 100644 --- a/base_layer/wallet_ffi/src/lib.rs +++ b/base_layer/wallet_ffi/src/lib.rs @@ -72,35 +72,6 @@ //! 6. This wallet will then monitor the Base Layer to see when the transaction is mined which means the //! `CompletedTransaction` status will become `Mined` and the funds will then move from the `PendingIncomingBalance` //! to the `AvailableBalance`. -//! -//! ## Using the test functions -//! The above two flows both require a second wallet for this wallet to interact with. Because we do not yet have a live -//! Test Net and the communications layer is not quite ready the library supplies four functions to help simulate the -//! second wallets role in these flows. The following will describe how to use these functions to produce the flows. -//! -//! ### Send Transaction with test functions -//! 1. Send Transaction as above to produce a `PendingOutboundTransaction`. -//! 2. Call the `complete_sent_transaction(...)` function with the tx_id of the sent transaction to simulate a reply. -//! This will move the `PendingOutboundTransaction` to become a `CompletedTransaction` with the `Completed` status. -//! 3. Call the 'broadcast_transaction(...)` function with the tx_id of the sent transaction and its status will move -//! from 'Completed' to 'Broadcast' which means it has been broadcast to the Base Layer Mempool but not mined yet. -//! from 'Completed' to 'Broadcast' which means it has been broadcast to the Base Layer Mempool but not mined yet. -//! 4. Call the `mined_transaction(...)` function with the tx_id of the sent transaction which will change -//! the status of the `CompletedTransaction` from `Broadcast` to `Mined`. The pending funds will also become -//! finalized as spent and available funds respectively. -//! -//! ### Receive Transaction with test functions -//! Under normal operation another wallet would initiate a Receive Transaction flow by sending you a transaction. We -//! will use the `receive_test_transaction(...)` function to initiate the flow: -//! -//! 1. Calling `receive_test_transaction(...)` will produce an `InboundTransaction`, the amount of the transaction will -//! appear under the `PendingIncomingBalance`. -//! 2. To simulate detecting the `InboundTransaction` being broadcast to the Base Layer Mempool call -//! `broadcast_transaction(...)` function. This will change the `InboundTransaction` to a -//! `CompletedTransaction` with the `Broadcast` status. The funds will still reflect in the pending balance. -//! 3. Call the `mined_transaction(...)` function with the tx_id of the received transaction which will -//! change the status of the `CompletedTransaction` from `Broadcast` to `Mined`. The pending funds will also -//! become finalized as spent and available funds respectively #![recursion_limit = "1024"] @@ -3044,9 +3015,24 @@ unsafe fn init_logging( /// when a Broadcast transaction is detected as mined AND confirmed. /// `callback_transaction_mined_unconfirmed` - The callback function pointer matching the function signature. This will /// be called when a Broadcast transaction is detected as mined but not yet confirmed. -/// `callback_discovery_process_complete` - The callback function pointer matching the function signature. This will be -/// called when a `send_transacion(..)` call is made to a peer whose address is not known and a discovery process must -/// be conducted. The outcome of the discovery process is relayed via this callback +/// `callback_direct_send_result` - The callback function pointer matching the function signature. This is called +/// when a direct send is completed. The first parameter is the transaction id and the second is whether if was +/// successful or not. +/// `callback_store_and_forward_send_result` - The callback function pointer matching the function +/// signature. This is called when a direct send is completed. The first parameter is the transaction id and the second +/// is whether if was successful or not. +/// `callback_transaction_cancellation` - The callback function pointer matching +/// the function signature. This is called when a transaction is cancelled. The first parameter is a pointer to the +/// cancelled transaction, the second is a reason as to why said transaction failed that is mapped to the `TxRejection` +/// enum: pub enum TxRejection { +/// Unknown, // 0 +/// UserCancelled, // 1 +/// Timeout, // 2 +/// DoubleSpend, // 3 +/// Orphan, // 4 +/// TimeLocked, // 5 +/// InvalidTransaction, // 6 +/// } /// `callback_txo_validation_complete` - The callback function pointer matching the function signature. This is called /// when a TXO validation process is completed. The request_key is used to identify which request this /// callback references and the second parameter is a u8 that represent the ClassbackValidationResults enum. @@ -3085,7 +3071,7 @@ pub unsafe extern "C" fn wallet_create( callback_transaction_mined_unconfirmed: unsafe extern "C" fn(*mut TariCompletedTransaction, u64), callback_direct_send_result: unsafe extern "C" fn(c_ulonglong, bool), callback_store_and_forward_send_result: unsafe extern "C" fn(c_ulonglong, bool), - callback_transaction_cancellation: unsafe extern "C" fn(*mut TariCompletedTransaction), + callback_transaction_cancellation: unsafe extern "C" fn(*mut TariCompletedTransaction, u64), callback_txo_validation_complete: unsafe extern "C" fn(u64, u8), callback_balance_updated: unsafe extern "C" fn(*mut TariBalance), callback_transaction_validation_complete: unsafe extern "C" fn(u64, u8), @@ -5850,7 +5836,7 @@ mod test { // assert!(true); //optimized out by compiler } - unsafe extern "C" fn tx_cancellation_callback(tx: *mut TariCompletedTransaction) { + unsafe extern "C" fn tx_cancellation_callback(tx: *mut TariCompletedTransaction, _reason: u64) { assert!(!tx.is_null()); assert_eq!( type_of((*tx).clone()), diff --git a/base_layer/wallet_ffi/wallet.h b/base_layer/wallet_ffi/wallet.h index c4be5b890c..d7266603e2 100644 --- a/base_layer/wallet_ffi/wallet.h +++ b/base_layer/wallet_ffi/wallet.h @@ -423,9 +423,22 @@ void comms_config_destroy(struct TariCommsConfig *wc); /// when a Broadcast transaction is detected as mined AND confirmed. /// `callback_transaction_mined_unconfirmed` - The callback function pointer matching the function signature. This will /// be called when a Broadcast transaction is detected as mined but not yet confirmed. -/// `callback_discovery_process_complete` - The callback function pointer matching the function signature. This will be -/// called when a `send_transacion(..)` call is made to a peer whose address is not known and a discovery process must -/// be conducted. The outcome of the discovery process is relayed via this callback +/// `callback_direct_send_result` - The callback function pointer matching the function signature. This is called +/// when a direct send is completed. The first parameter is the transaction id and the second is whether if was successful or not. +/// `callback_store_and_forward_send_result` - The callback function pointer matching the function signature. This is called +/// when a direct send is completed. The first parameter is the transaction id and the second is whether if was successful or not. +/// `callback_transaction_cancellation` - The callback function pointer matching the function signature. This is called +/// when a transaction is cancelled. The first parameter is a pointer to the cancelled transaction, the second is a reason as to +/// why said transaction failed that is mapped to the `TxRejection` enum: +/// pub enum TxRejection { +/// Unknown, // 0 +/// UserCancelled, // 1 +/// Timeout, // 2 +/// DoubleSpend, // 3 +/// Orphan, // 4 +/// TimeLocked, // 5 +/// InvalidTransaction, // 6 +/// } /// `callback_txo_validation_complete` - The callback function pointer matching the function signature. This is called /// when a TXO validation process is completed. The request_key is used to identify which request this /// callback references and the second parameter is a u8 that represent the CallbackValidationResults enum. @@ -469,7 +482,7 @@ struct TariWallet *wallet_create(struct TariCommsConfig *config, void (*callback_transaction_mined_unconfirmed)(struct TariCompletedTransaction *, unsigned long long), void (*callback_direct_send_result)(unsigned long long, bool), void (*callback_store_and_forward_send_result)(unsigned long long, bool), - void (*callback_transaction_cancellation)(struct TariCompletedTransaction *), + void (*callback_transaction_cancellation)(struct TariCompletedTransaction *, unsigned long long), void (*callback_txo_validation_complete)(unsigned long long, unsigned char), void (*callback_balance_updated)(struct TariBalance *), void (*callback_transaction_validation_complete)(unsigned long long, unsigned char), diff --git a/integration_tests/helpers/ffi/ffiInterface.js b/integration_tests/helpers/ffi/ffiInterface.js index 78430ac90e..607252a07e 100644 --- a/integration_tests/helpers/ffi/ffiInterface.js +++ b/integration_tests/helpers/ffi/ffiInterface.js @@ -1129,7 +1129,7 @@ class InterfaceFFI { } static createCallbackTransactionCancellation(fn) { - return ffi.Callback(this.void, [this.ptr], fn); + return ffi.Callback(this.void, [this.ptr, this.ulonglong], fn); } static createCallbackTxoValidationComplete(fn) { return ffi.Callback(this.void, [this.ulonglong, this.uchar], fn); diff --git a/integration_tests/helpers/ffi/wallet.js b/integration_tests/helpers/ffi/wallet.js index 0797ad81d5..ff1b783ca1 100644 --- a/integration_tests/helpers/ffi/wallet.js +++ b/integration_tests/helpers/ffi/wallet.js @@ -245,11 +245,11 @@ class Wallet { this.minedunconfirmed += 1; }; - onTransactionCancellation = (ptr) => { + onTransactionCancellation = (ptr, reason) => { let tx = new CompletedTransaction(); tx.pointerAssign(ptr); console.log( - `${new Date().toISOString()} Transaction with txID ${tx.getTransactionID()} was cancelled` + `${new Date().toISOString()} Transaction with txID ${tx.getTransactionID()} was cancelled with reason code ${reason}.` ); tx.destroy(); this.cancelled += 1; From 65157b00237a7cd6b3b68d84f958ed33da3a7297 Mon Sep 17 00:00:00 2001 From: Stan Bondi Date: Tue, 23 Nov 2021 13:25:18 +0200 Subject: [PATCH 5/6] feat: add ban peers metric (#3605) Description --- Add metric for peer banning Motivation and Context --- Visibility into whether a node is banning peers How Has This Been Tested? --- Simple change, code compiles --- comms/src/connectivity/manager.rs | 3 +++ comms/src/connectivity/metrics.rs | 17 +++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/comms/src/connectivity/manager.rs b/comms/src/connectivity/manager.rs index ad75db44b0..e5c0e46bee 100644 --- a/comms/src/connectivity/manager.rs +++ b/comms/src/connectivity/manager.rs @@ -733,6 +733,9 @@ impl ConnectivityManagerActor { self.peer_manager.ban_peer_by_node_id(node_id, duration, reason).await?; + #[cfg(feature = "metrics")] + super::metrics::banned_peers_counter(node_id).inc(); + self.publish_event(ConnectivityEvent::PeerBanned(node_id.clone())); if let Some(conn) = self.pool.get_connection_mut(node_id) { diff --git a/comms/src/connectivity/metrics.rs b/comms/src/connectivity/metrics.rs index 1470b4c9c7..0cc90578d0 100644 --- a/comms/src/connectivity/metrics.rs +++ b/comms/src/connectivity/metrics.rs @@ -20,9 +20,9 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use crate::connection_manager::ConnectionDirection; +use crate::{connection_manager::ConnectionDirection, peer_manager::NodeId}; use once_cell::sync::Lazy; -use tari_metrics::{IntGauge, IntGaugeVec}; +use tari_metrics::{IntCounter, IntCounterVec, IntGauge, IntGaugeVec}; pub fn connections(direction: ConnectionDirection) -> IntGauge { static METER: Lazy = Lazy::new(|| { @@ -43,3 +43,16 @@ pub fn uptime() -> IntGauge { METER.clone() } + +pub fn banned_peers_counter(peer: &NodeId) -> IntCounter { + static METER: Lazy = Lazy::new(|| { + tari_metrics::register_int_counter_vec( + "comms::connectivity::banned_peers", + "The number of peer bans by peer", + &["peer_id"], + ) + .unwrap() + }); + + METER.with_label_values(&[peer.to_string().as_str()]) +} From fff45db4bd1a6fa436ad3525e96ae08f26a856e8 Mon Sep 17 00:00:00 2001 From: David Main <51991544+StriderDM@users.noreply.github.com> Date: Tue, 23 Nov 2021 15:23:49 +0200 Subject: [PATCH 6/6] fix: seed word parsing (#3607) Description --- Moved `detect_language` into `MnemonicLanguage` and made it public. Prevented a `TariSeedWords` object from becoming invalid in the event an invalid or inconsistent word was attempted to be pushed to it in wallet_ffi. Differentiated between an invalid word and an inconsistent word. Added word to the string of WordNotFound error. Motivation and Context --- General fixes How Has This Been Tested? --- cargo test --all --- base_layer/key_manager/src/error.rs | 4 +- base_layer/key_manager/src/mnemonic.rs | 110 +++++++++++++------------ base_layer/wallet_ffi/src/enums.rs | 1 + base_layer/wallet_ffi/src/lib.rs | 72 ++++++++++++---- 4 files changed, 115 insertions(+), 72 deletions(-) diff --git a/base_layer/key_manager/src/error.rs b/base_layer/key_manager/src/error.rs index 14b79ea058..3d67c59eb1 100644 --- a/base_layer/key_manager/src/error.rs +++ b/base_layer/key_manager/src/error.rs @@ -51,8 +51,8 @@ pub enum MnemonicError { defined natural languages" )] UnknownLanguage, - #[error("Only 2048 words for each language was selected to form Mnemonic word lists")] - WordNotFound, + #[error("Word not found: `{0}`")] + WordNotFound(String), #[error("A mnemonic word does not exist for the requested index")] IndexOutOfBounds, #[error("A problem encountered constructing a secret key from bytes or mnemonic sequence: `{0}`")] diff --git a/base_layer/key_manager/src/mnemonic.rs b/base_layer/key_manager/src/mnemonic.rs index 6e7e6ef8f3..9338e82217 100644 --- a/base_layer/key_manager/src/mnemonic.rs +++ b/base_layer/key_manager/src/mnemonic.rs @@ -48,7 +48,7 @@ impl MnemonicLanguage { /// Detects the mnemonic language of a specific word by searching all defined mnemonic word lists pub fn from(mnemonic_word: &str) -> Result { let words = vec![mnemonic_word.to_string()]; - detect_language(&words) + MnemonicLanguage::detect_language(&words) } /// Returns an iterator for the MnemonicLanguage enum group to allow iteration over all defined languages @@ -77,6 +77,51 @@ impl MnemonicLanguage { MnemonicLanguage::Spanish => MNEMONIC_SPANISH_WORDS.len(), } } + + /// Detects the language of a list of words + pub fn detect_language(words: &[String]) -> Result { + let count = words.iter().len(); + match count.cmp(&1) { + Ordering::Less => { + return Err(MnemonicError::UnknownLanguage); + }, + Ordering::Equal => { + let word = words.get(0).ok_or(MnemonicError::EncodeInvalidLength)?; + for language in MnemonicLanguage::iterator() { + if find_mnemonic_index_from_word(word, language).is_ok() { + return Ok(*language); + } + } + return Err(MnemonicError::UnknownLanguage); + }, + Ordering::Greater => { + for word in words { + let mut languages = Vec::with_capacity(MnemonicLanguage::iterator().len()); + // detect all languages in which a word falls into + for language in MnemonicLanguage::iterator() { + if find_mnemonic_index_from_word(word, language).is_ok() { + languages.push(*language); + } + } + // check if at least one of the languages is consistent for all other words against languages + // yielded from the initial word for this iteration + for language in languages { + let mut consistent = true; + for compare in words { + if compare != word && find_mnemonic_index_from_word(compare, &language).is_err() { + consistent = false; + } + } + if consistent { + return Ok(language); + } + } + } + }, + } + + Err(MnemonicError::UnknownLanguage) + } } /// Finds and returns the index of a specific word in a mnemonic word list defined by the specified language @@ -106,7 +151,7 @@ fn find_mnemonic_index_from_word(word: &str, language: &MnemonicLanguage) -> Res } match search_result { Ok(v) => Ok(v), - Err(_err) => Err(MnemonicError::WordNotFound), + Err(_err) => Err(MnemonicError::WordNotFound(word.to_string())), } } @@ -154,54 +199,10 @@ pub fn from_bytes(bytes: Vec, language: &MnemonicLanguage) -> Result Result { - let count = words.iter().len(); - match count.cmp(&1) { - Ordering::Less => { - return Err(MnemonicError::UnknownLanguage); - }, - Ordering::Equal => { - let word = words.get(0).ok_or(MnemonicError::EncodeInvalidLength)?; - for language in MnemonicLanguage::iterator() { - if find_mnemonic_index_from_word(word, language).is_ok() { - return Ok(*language); - } - } - return Err(MnemonicError::UnknownLanguage); - }, - Ordering::Greater => { - for word in words { - let mut languages = Vec::with_capacity(MnemonicLanguage::iterator().len()); - // detect all languages in which a word falls into - for language in MnemonicLanguage::iterator() { - if find_mnemonic_index_from_word(word, language).is_ok() { - languages.push(*language); - } - } - // check if at least one of the languages is consistent for all other words against languages yielded - // from the initial word for this iteration - for language in languages { - let mut consistent = true; - for compare in words { - if compare != word && find_mnemonic_index_from_word(compare, &language).is_err() { - consistent = false; - } - } - if consistent { - return Ok(language); - } - } - } - }, - } - - Err(MnemonicError::UnknownLanguage) -} - /// Generates a vector of bytes that represent the provided mnemonic sequence of words, the language of the mnemonic /// sequence is detected pub fn to_bytes(mnemonic_seq: &[String]) -> Result, MnemonicError> { - let language = self::detect_language(mnemonic_seq)?; + let language = MnemonicLanguage::detect_language(mnemonic_seq)?; to_bytes_with_language(mnemonic_seq, &language) } @@ -336,7 +337,10 @@ mod test { "opera".to_string(), "abandon".to_string(), ]; - assert_eq!(detect_language(&words1), Ok(MnemonicLanguage::English)); + assert_eq!( + MnemonicLanguage::detect_language(&words1), + Ok(MnemonicLanguage::English) + ); // English/Spanish + English/French + Italian/Spanish let words2 = vec![ @@ -346,7 +350,7 @@ mod test { "abandon".to_string(), "tipico".to_string(), ]; - assert_eq!(detect_language(&words2).is_err(), true); + assert_eq!(MnemonicLanguage::detect_language(&words2).is_err(), true); // bounds check (last word is invalid) let words3 = vec![ @@ -356,16 +360,16 @@ mod test { "abandon".to_string(), "topazio".to_string(), ]; - assert_eq!(detect_language(&words3).is_err(), true); + assert_eq!(MnemonicLanguage::detect_language(&words3).is_err(), true); // building up a word list: English/French + French -> French let mut words = Vec::with_capacity(3); words.push("concert".to_string()); - assert_eq!(detect_language(&words), Ok(MnemonicLanguage::English)); + assert_eq!(MnemonicLanguage::detect_language(&words), Ok(MnemonicLanguage::English)); words.push("abandon".to_string()); - assert_eq!(detect_language(&words), Ok(MnemonicLanguage::English)); + assert_eq!(MnemonicLanguage::detect_language(&words), Ok(MnemonicLanguage::English)); words.push("barbier".to_string()); - assert_eq!(detect_language(&words), Ok(MnemonicLanguage::French)); + assert_eq!(MnemonicLanguage::detect_language(&words), Ok(MnemonicLanguage::French)); } #[test] diff --git a/base_layer/wallet_ffi/src/enums.rs b/base_layer/wallet_ffi/src/enums.rs index 4d54dfc38e..bb1d90b598 100644 --- a/base_layer/wallet_ffi/src/enums.rs +++ b/base_layer/wallet_ffi/src/enums.rs @@ -27,4 +27,5 @@ pub enum SeedWordPushResult { SeedPhraseComplete, InvalidSeedPhrase, InvalidObject, + NoLanguageMatch, } diff --git a/base_layer/wallet_ffi/src/lib.rs b/base_layer/wallet_ffi/src/lib.rs index 2a65bf8fd5..566ec880d4 100644 --- a/base_layer/wallet_ffi/src/lib.rs +++ b/base_layer/wallet_ffi/src/lib.rs @@ -1020,11 +1020,13 @@ pub unsafe extern "C" fn seed_words_get_at( /// /// ## Returns /// 'c_uchar' - Returns a u8 version of the `SeedWordPushResult` enum indicating whether the word was not a valid seed -/// word, if the push was successful and whether the push was successful and completed the full Seed Phrase +/// word, if the push was successful and whether the push was successful and completed the full Seed Phrase. +/// `seed_words` is only modified in the event of a `SuccessfulPush`. /// '0' -> InvalidSeedWord /// '1' -> SuccessfulPush /// '2' -> SeedPhraseComplete /// '3' -> InvalidSeedPhrase +/// '4' -> NoLanguageMatch, /// # Safety /// The ```string_destroy``` method must be called when finished with a string from rust to prevent a memory leak #[no_mangle] @@ -1082,28 +1084,64 @@ pub unsafe extern "C" fn seed_words_push_word( }, } - if MnemonicLanguage::from(word_string.as_str()).is_err() { - log::error!(target: LOG_TARGET, "{} is not a valid mnemonic seed word", word_string); - return SeedWordPushResult::InvalidSeedWord as u8; + // Seed words is currently empty, this is the first word + if (*seed_words).0.is_empty() { + (*seed_words).0.push(word_string); + return SeedWordPushResult::SuccessfulPush as u8; } - (*seed_words).0.push(word_string); - if (*seed_words).0.len() >= 24 { - return if let Err(e) = CipherSeed::from_mnemonic(&(*seed_words).0, None) { + // Try push to a temporary copy first to prevent existing object becoming invalid + let mut temp = (*seed_words).0.clone(); + + if let Ok(language) = MnemonicLanguage::detect_language(&temp) { + temp.push(word_string.clone()); + // Check words in temp are still consistent for a language, note that detected language can change + // depending on word added + if MnemonicLanguage::detect_language(&temp).is_ok() { + if temp.len() >= 24 { + if let Err(e) = CipherSeed::from_mnemonic(&temp, None) { + log::error!( + target: LOG_TARGET, + "Problem building valid private seed from seed phrase: {:?}", + e + ); + error = LibWalletError::from(WalletError::KeyManagerError(e)).code; + ptr::swap(error_out, &mut error as *mut c_int); + return SeedWordPushResult::InvalidSeedPhrase as u8; + }; + } + + (*seed_words).0.push(word_string); + + // Note: test for a validity was already done so we can just check length here + if (*seed_words).0.len() < 24 { + SeedWordPushResult::SuccessfulPush as u8 + } else { + SeedWordPushResult::SeedPhraseComplete as u8 + } + } else { log::error!( target: LOG_TARGET, - "Problem building valid private seed from seed phrase: {:?}", - e + "Words in seed phrase do not match any language after trying to add word: `{:?}`, previously words \ + were detected to be in: `{:?}`", + word_string, + language ); - error = LibWalletError::from(WalletError::KeyManagerError(e)).code; - ptr::swap(error_out, &mut error as *mut c_int); - SeedWordPushResult::InvalidSeedPhrase as u8 - } else { - SeedWordPushResult::SeedPhraseComplete as u8 - }; + SeedWordPushResult::NoLanguageMatch as u8 + } + } else { + // Seed words are invalid, shouldn't normally be reachable + log::error!( + target: LOG_TARGET, + "Words in seed phrase do not match any language prior to adding word: `{:?}`", + word_string + ); + let error_msg = "Invalid seed words object, no language can be detected."; + log::error!(target: LOG_TARGET, "{}", error_msg); + error = LibWalletError::from(InterfaceError::InvalidArgument(error_msg.to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + SeedWordPushResult::InvalidObject as u8 } - - SeedWordPushResult::SuccessfulPush as u8 } /// Frees memory for a TariSeedWords