diff --git a/src/config_models/cli_args.rs b/src/config_models/cli_args.rs index e158c4f0..fec1b2fc 100644 --- a/src/config_models/cli_args.rs +++ b/src/config_models/cli_args.rs @@ -66,23 +66,6 @@ pub struct Args { #[clap(long, default_value = "1G", value_name = "SIZE")] pub max_mempool_size: ByteSize, - /// Prune the pool of UTXO notification when it exceeds this size in RAM. - /// - /// Units: B (bytes), K (kilobytes), M (megabytes), G (gigabytes) - /// - /// E.g. --max-utxo-notification-size 50M - #[clap(long, default_value = "50M", value_name = "SIZE")] - pub max_utxo_notification_size: ByteSize, - - /// Maximum number of unconfirmed expected UTXOs that can be stored for each peer. - /// - /// You may want to increase this number from its default value if - /// you're running a node that receives a very high number of UTXOs. - /// - /// E.g. --max_unconfirmed_utxo_notification_count_per_peer 5000 - #[clap(long, default_value = "1000", value_name = "COUNT")] - pub max_unconfirmed_utxo_notification_count_per_peer: usize, - /// Port on which to listen for peer connections. #[clap(long, default_value = "9798", value_name = "PORT")] pub peer_port: u16, diff --git a/src/main_loop.rs b/src/main_loop.rs index 7175331f..0aa25222 100644 --- a/src/main_loop.rs +++ b/src/main_loop.rs @@ -44,7 +44,7 @@ const PEER_DISCOVERY_INTERVAL_IN_SECONDS: u64 = 120; const SYNC_REQUEST_INTERVAL_IN_SECONDS: u64 = 3; const MEMPOOL_PRUNE_INTERVAL_IN_SECS: u64 = 30 * 60; // 30mins const MP_RESYNC_INTERVAL_IN_SECS: u64 = 59; -const UTXO_NOTIFICATION_POOL_PRUNE_INTERVAL_IN_SECS: u64 = 19 * 60; // 19 mins +const EXPECTED_UTXOS_PRUNE_INTERVAL_IN_SECS: u64 = 19 * 60; // 19 mins const SANCTION_PEER_TIMEOUT_FACTOR: u64 = 40; const POTENTIAL_PEER_MAX_COUNT_AS_A_FACTOR_OF_MAX_PEERS: usize = 20; @@ -821,7 +821,7 @@ impl MainLoopHandler { // Set removal of stale notifications for incoming UTXOs let utxo_notification_cleanup_timer_interval = - Duration::from_secs(UTXO_NOTIFICATION_POOL_PRUNE_INTERVAL_IN_SECS); + Duration::from_secs(EXPECTED_UTXOS_PRUNE_INTERVAL_IN_SECS); let utxo_notification_cleanup_timer = time::sleep(utxo_notification_cleanup_timer_interval); tokio::pin!(utxo_notification_cleanup_timer); @@ -979,7 +979,15 @@ impl MainLoopHandler { // Handle incoming UTXO notification cleanup, i.e. removing stale/too old UTXO notification from pool _ = &mut utxo_notification_cleanup_timer => { debug!("Timer: UTXO notification pool cleanup job"); - self.global_state_lock.lock_mut(|s| s.wallet_state.expected_utxos.prune_stale_utxo_notifications()).await; + + // Danger: possible loss of funds. + // + // See description of prune_stale_expected_utxos(). + // + // This call is disabled until such time as a thorough + // evaluation and perhaps reimplementation determines that + // it can be called safely without possible loss of funds. + // self.global_state_lock.lock_mut(|s| s.wallet_state.prune_stale_expected_utxos()).await; utxo_notification_cleanup_timer.as_mut().reset(tokio::time::Instant::now() + utxo_notification_cleanup_timer_interval); } diff --git a/src/mine_loop.rs b/src/mine_loop.rs index 37069f67..8efde9d4 100644 --- a/src/mine_loop.rs +++ b/src/mine_loop.rs @@ -39,8 +39,8 @@ use crate::models::blockchain::type_scripts::TypeScript; use crate::models::channel::*; use crate::models::consensus::timestamp::Timestamp; use crate::models::shared::SIZE_20MB_IN_BYTES; -use crate::models::state::wallet::utxo_notification_pool::ExpectedUtxo; -use crate::models::state::wallet::utxo_notification_pool::UtxoNotifier; +use crate::models::state::wallet::expected_utxo::ExpectedUtxo; +use crate::models::state::wallet::expected_utxo::UtxoNotifier; use crate::models::state::wallet::WalletSecret; use crate::models::state::GlobalState; use crate::models::state::GlobalStateLock; diff --git a/src/models/blockchain/transaction/mod.rs b/src/models/blockchain/transaction/mod.rs index 181627a8..8f349a94 100644 --- a/src/models/blockchain/transaction/mod.rs +++ b/src/models/blockchain/transaction/mod.rs @@ -47,7 +47,7 @@ use crate::models::blockchain::block::mutator_set_update::MutatorSetUpdate; use crate::models::consensus::mast_hash::MastHash; use crate::models::consensus::ValidityTree; use crate::models::consensus::WitnessType; -use crate::models::state::wallet::utxo_notification_pool::ExpectedUtxo; +use crate::models::state::wallet::expected_utxo::ExpectedUtxo; use crate::prelude::triton_vm; use crate::prelude::twenty_first; use crate::util_types::mutator_set::addition_record::AdditionRecord; diff --git a/src/models/blockchain/transaction/transaction_output.rs b/src/models/blockchain/transaction/transaction_output.rs index ad556675..80f44afc 100644 --- a/src/models/blockchain/transaction/transaction_output.rs +++ b/src/models/blockchain/transaction/transaction_output.rs @@ -6,8 +6,8 @@ use crate::models::blockchain::transaction::PublicAnnouncement; use crate::models::blockchain::type_scripts::neptune_coins::NeptuneCoins; use crate::models::state::wallet::address::ReceivingAddress; use crate::models::state::wallet::address::SpendingKey; -use crate::models::state::wallet::utxo_notification_pool::ExpectedUtxo; -use crate::models::state::wallet::utxo_notification_pool::UtxoNotifier; +use crate::models::state::wallet::expected_utxo::ExpectedUtxo; +use crate::models::state::wallet::expected_utxo::UtxoNotifier; use crate::models::state::wallet::wallet_state::WalletState; use crate::prelude::twenty_first::math::digest::Digest; use crate::prelude::twenty_first::util_types::algebraic_hasher::AlgebraicHasher; @@ -277,7 +277,7 @@ impl From<&TxOutputList> for Vec { impl From<&TxOutputList> for Vec { fn from(list: &TxOutputList) -> Self { - list.expected_utxos_iter().into_iter().collect() + list.expected_utxos_iter().collect() } } @@ -330,7 +330,7 @@ impl TxOutputList { } /// retrieves expected_utxos from possible sub-set of the list - pub fn expected_utxos_iter(&self) -> impl IntoIterator + '_ { + pub fn expected_utxos_iter(&self) -> impl Iterator + '_ { self.0.iter().filter_map(|u| match &u.utxo_notification { UtxoNotification::OffChain(eu) => Some(*eu.clone()), _ => None, @@ -346,7 +346,7 @@ impl TxOutputList { /// retrieves expected_utxos from possible sub-set of the list pub fn expected_utxos(&self) -> Vec { - self.expected_utxos_iter().into_iter().collect() + self.expected_utxos_iter().collect() } } diff --git a/src/models/channel.rs b/src/models/channel.rs index 91fc7dd2..5e7673df 100644 --- a/src/models/channel.rs +++ b/src/models/channel.rs @@ -10,7 +10,7 @@ use super::blockchain::block::block_height::BlockHeight; use super::blockchain::block::Block; use super::blockchain::transaction::Transaction; use super::peer::TransactionNotification; -use super::state::wallet::utxo_notification_pool::ExpectedUtxo; +use super::state::wallet::expected_utxo::ExpectedUtxo; #[derive(Clone, Debug)] pub enum MainToMiner { diff --git a/src/models/consensus/tasm/environment.rs b/src/models/consensus/tasm/environment.rs index faa68974..1cfac009 100644 --- a/src/models/consensus/tasm/environment.rs +++ b/src/models/consensus/tasm/environment.rs @@ -4,7 +4,7 @@ //! on the host machine's native architecture (i.e. your machine). //! //! It has been shamelessly copied from greenhat's omnizk compiler project: -//! https://github.com/greenhat/omnizk +//! use std::cell::RefCell; use std::collections::HashMap; diff --git a/src/models/consensus/timestamp.rs b/src/models/consensus/timestamp.rs index 9b815aee..de524c02 100644 --- a/src/models/consensus/timestamp.rs +++ b/src/models/consensus/timestamp.rs @@ -24,7 +24,7 @@ use tasm_lib::twenty_first::math::bfield_codec::BFieldCodec; /// milliseconds elapsed since the Unix epoch (00:00 UTC on 1 Jan 1970) using /// a single BFieldElement. #[derive( - Debug, Clone, Copy, BFieldCodec, PartialEq, Eq, Serialize, Deserialize, GetSize, Default, + Debug, Clone, Copy, Hash, BFieldCodec, PartialEq, Eq, Serialize, Deserialize, GetSize, Default, )] pub struct Timestamp(pub BFieldElement); diff --git a/src/models/state/archival_state.rs b/src/models/state/archival_state.rs index 466aa5f0..0a19b928 100644 --- a/src/models/state/archival_state.rs +++ b/src/models/state/archival_state.rs @@ -861,7 +861,8 @@ mod archival_state_tests { use crate::models::blockchain::type_scripts::neptune_coins::NeptuneCoins; use crate::models::consensus::timestamp::Timestamp; use crate::models::state::archival_state::ArchivalState; - use crate::models::state::wallet::utxo_notification_pool::UtxoNotifier; + use crate::models::state::wallet::expected_utxo::ExpectedUtxo; + use crate::models::state::wallet::expected_utxo::UtxoNotifier; use crate::models::state::wallet::WalletSecret; use crate::tests::shared::add_block_to_archival_state; use crate::tests::shared::make_mock_block_with_valid_pow; @@ -1587,28 +1588,26 @@ mod archival_state_tests { let mut genesis_state = genesis_state_lock.lock_guard_mut().await; genesis_state .wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( cb_utxo, cb_output_randomness, genesis_spending_key.privacy_preimage, UtxoNotifier::OwnMiner, - ) - .unwrap(); + )) + .await; } { let mut alice_state = alice_state_lock.lock_guard_mut().await; for rec_data in tx_outputs_for_alice { alice_state .wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( rec_data.utxo.clone(), rec_data.sender_randomness, alice_spending_key.privacy_preimage, UtxoNotifier::Cli, - ) - .unwrap(); + )) + .await; } } @@ -1617,14 +1616,13 @@ mod archival_state_tests { for rec_data in tx_outputs_for_bob { bob_state .wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( rec_data.utxo.clone(), rec_data.sender_randomness, bob_spending_key.privacy_preimage, UtxoNotifier::Cli, - ) - .unwrap(); + )) + .await; } } @@ -1783,14 +1781,13 @@ mod archival_state_tests { .lock_guard_mut() .await .wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( rec_data.utxo.clone(), rec_data.sender_randomness, genesis_spending_key.privacy_preimage, UtxoNotifier::Cli, - ) - .unwrap(); + )) + .await; } for rec_data in tx_outputs_from_bob { @@ -1798,28 +1795,26 @@ mod archival_state_tests { .lock_guard_mut() .await .wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( rec_data.utxo.clone(), rec_data.sender_randomness, genesis_spending_key.privacy_preimage, UtxoNotifier::Cli, - ) - .unwrap(); + )) + .await; } genesis_state_lock .lock_guard_mut() .await .wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( cb_utxo_block_2, cb_sender_randomness_block_2, genesis_spending_key.privacy_preimage, UtxoNotifier::Cli, - ) - .unwrap(); + )) + .await; // Update chain states for state_lock in [&genesis_state_lock, &alice_state_lock, &bob_state_lock] { diff --git a/src/models/state/mempool.rs b/src/models/state/mempool.rs index bc4b1763..2ba29732 100644 --- a/src/models/state/mempool.rs +++ b/src/models/state/mempool.rs @@ -456,7 +456,8 @@ mod tests { use crate::models::blockchain::transaction::TxOutput; use crate::models::blockchain::type_scripts::neptune_coins::NeptuneCoins; use crate::models::shared::SIZE_20MB_IN_BYTES; - use crate::models::state::wallet::utxo_notification_pool::UtxoNotifier; + use crate::models::state::wallet::expected_utxo::ExpectedUtxo; + use crate::models::state::wallet::expected_utxo::UtxoNotifier; use crate::models::state::wallet::WalletSecret; use crate::tests::shared::make_mock_block; use crate::tests::shared::make_mock_transaction_with_wallet; @@ -644,14 +645,13 @@ mod tests { // Update both states with block 1 other_global_state .wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( coinbase_utxo_1, cb_sender_randomness_1, other_receiver_spending_key.privacy_preimage, UtxoNotifier::OwnMiner, - ) - .expect("UTXO notification from miner must be accepted"); + )) + .await; other_global_state .set_new_tip(block_1.clone()) .await diff --git a/src/models/state/mod.rs b/src/models/state/mod.rs index 4236a6c3..5f8bc97d 100644 --- a/src/models/state/mod.rs +++ b/src/models/state/mod.rs @@ -24,7 +24,7 @@ use twenty_first::math::digest::Digest; use twenty_first::util_types::algebraic_hasher::AlgebraicHasher; use wallet::address::ReceivingAddress; use wallet::address::SpendingKey; -use wallet::utxo_notification_pool::UtxoNotifier; +use wallet::expected_utxo::UtxoNotifier; use wallet::wallet_state::WalletState; use wallet::wallet_status::WalletStatus; @@ -35,8 +35,8 @@ use crate::database::storage::storage_vec::Index; use crate::locks::tokio as sync_tokio; use crate::models::blockchain::transaction::UtxoNotifyMethod; use crate::models::peer::HandshakeData; +use crate::models::state::wallet::expected_utxo::ExpectedUtxo; use crate::models::state::wallet::monitored_utxo::MonitoredUtxo; -use crate::models::state::wallet::utxo_notification_pool::ExpectedUtxo; use crate::prelude::twenty_first; use crate::time_fn_call_async; use crate::util_types::mutator_set::mutator_set_accumulator::MutatorSetAccumulator; @@ -814,12 +814,14 @@ impl GlobalState { expected_utxos: impl IntoIterator, ) -> Result<()> { for expected_utxo in expected_utxos.into_iter() { - self.wallet_state.expected_utxos.add_expected_utxo( - expected_utxo.utxo, - expected_utxo.sender_randomness, - expected_utxo.receiver_preimage, - expected_utxo.received_from, - )?; + self.wallet_state + .add_expected_utxo(ExpectedUtxo::new( + expected_utxo.utxo, + expected_utxo.sender_randomness, + expected_utxo.receiver_preimage, + expected_utxo.received_from, + )) + .await; } Ok(()) } @@ -1291,14 +1293,13 @@ impl GlobalState { // Notify wallet to expect the coinbase UTXO, as we mined this block myself .wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( coinbase_info.utxo, coinbase_info.sender_randomness, coinbase_info.receiver_preimage, UtxoNotifier::OwnMiner, - ) - .expect("UTXO notification from miner must be accepted"); + )) + .await; } // Get parent of tip for mutator-set data needed for various updates. Parent of the @@ -1398,7 +1399,7 @@ mod global_state_tests { use crate::config_models::network::Network; use crate::models::blockchain::block::Block; - use crate::models::state::wallet::utxo_notification_pool::UtxoNotifier; + use crate::models::state::wallet::expected_utxo::UtxoNotifier; use crate::tests::shared::add_block_to_light_state; use crate::tests::shared::make_mock_block; use crate::tests::shared::make_mock_block_with_valid_pow; @@ -2111,14 +2112,13 @@ mod global_state_tests { .lock_guard_mut() .await .wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( rec_data.utxo.clone(), rec_data.sender_randomness, alice_spending_key.privacy_preimage, UtxoNotifier::Cli, - ) - .unwrap(); + )) + .await; } for rec_data in tx_outputs_for_bob { @@ -2126,14 +2126,13 @@ mod global_state_tests { .lock_guard_mut() .await .wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( rec_data.utxo.clone(), rec_data.sender_randomness, bob_spending_key.privacy_preimage, UtxoNotifier::Cli, - ) - .unwrap(); + )) + .await; } genesis_state_lock diff --git a/src/models/state/wallet/expected_utxo.rs b/src/models/state/wallet/expected_utxo.rs new file mode 100644 index 00000000..c14164cc --- /dev/null +++ b/src/models/state/wallet/expected_utxo.rs @@ -0,0 +1,75 @@ +use crate::models::blockchain::{shared::Hash, transaction::utxo::Utxo}; +use crate::{ + models::consensus::timestamp::Timestamp, + prelude::twenty_first, + util_types::mutator_set::{addition_record::AdditionRecord, commit}, +}; +use get_size::GetSize; +use serde::{Deserialize, Serialize}; +use twenty_first::{math::tip5::Digest, util_types::algebraic_hasher::AlgebraicHasher}; + +/// represents utxo and secrets necessary for recipient to claim it. +/// +/// [ExpectedUtxo] is intended for offchain temporary storage of utxos that a +/// wallet sends to itself, eg change outputs. +/// +/// The `ExpectedUtxo` will exist in the local +/// [RustyWalletDatabase](super::rusty_wallet_database::RustyWalletDatabase) +/// from the time the transaction is sent until it is mined in a block and +/// claimed by the wallet. +/// +/// note that when using `ExpectedUtxo` there is a risk of losing funds because +/// the wallet stores this state on disk and if the associated file(s) are lost +/// then the funds cannot be claimed. +/// +/// an alternative is to use onchain symmetric keys instead, which uses some +/// blockchain space and may leak some privacy if a key is ever used more than +/// once. +/// +/// ### about `receiver_preimage` +/// +/// See issue #176. +/// +/// +/// see [AnnouncedUtxo](crate::models::blockchain::transaction::AnnouncedUtxo), [UtxoNotification](crate::models::blockchain::transaction::UtxoNotification) +#[derive(Clone, Debug, PartialEq, Eq, Hash, GetSize, Serialize, Deserialize)] +pub struct ExpectedUtxo { + pub utxo: Utxo, + pub addition_record: AdditionRecord, + pub sender_randomness: Digest, + pub receiver_preimage: Digest, + pub received_from: UtxoNotifier, + pub notification_received: Timestamp, + pub mined_in_block: Option<(Digest, Timestamp)>, +} + +impl ExpectedUtxo { + pub fn new( + utxo: Utxo, + sender_randomness: Digest, + receiver_preimage: Digest, + received_from: UtxoNotifier, + ) -> Self { + Self { + addition_record: commit( + Hash::hash(&utxo), + sender_randomness, + receiver_preimage.hash::(), + ), + utxo, + sender_randomness, + receiver_preimage, + received_from, + notification_received: Timestamp::now(), + mined_in_block: None, + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash, GetSize, Serialize, Deserialize)] +pub enum UtxoNotifier { + OwnMiner, + Cli, + Myself, + Premine, +} diff --git a/src/models/state/wallet/mod.rs b/src/models/state/wallet/mod.rs index ba08e5b0..ba4107bb 100644 --- a/src/models/state/wallet/mod.rs +++ b/src/models/state/wallet/mod.rs @@ -1,8 +1,8 @@ pub mod address; pub mod coin_with_possible_timelock; +pub mod expected_utxo; pub mod monitored_utxo; pub mod rusty_wallet_database; -pub mod utxo_notification_pool; pub mod wallet_state; pub mod wallet_status; @@ -407,6 +407,7 @@ impl WalletSecret { #[cfg(test)] mod wallet_tests { + use expected_utxo::ExpectedUtxo; use num_traits::CheckedSub; use rand::random; use tracing_test::traced_test; @@ -423,7 +424,7 @@ mod wallet_tests { use crate::models::blockchain::transaction::TxOutput; use crate::models::blockchain::type_scripts::neptune_coins::NeptuneCoins; use crate::models::consensus::timestamp::Timestamp; - use crate::models::state::wallet::utxo_notification_pool::UtxoNotifier; + use crate::models::state::wallet::expected_utxo::UtxoNotifier; use crate::tests::shared::make_mock_block; use crate::tests::shared::make_mock_transaction_with_generation_key; use crate::tests::shared::mock_genesis_global_state; @@ -535,17 +536,16 @@ mod wallet_tests { make_mock_block(&genesis_block, None, own_recipient_address, rng.gen()); own_wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( block_1_coinbase_utxo.clone(), block_1_coinbase_sender_randomness, own_spending_key.privacy_preimage, UtxoNotifier::OwnMiner, - ) - .unwrap(); + )) + .await; assert_eq!( 1, - own_wallet_state.expected_utxos.len(), + own_wallet_state.wallet_db.expected_utxos().len().await, "Expected UTXO list must have length 1 before block registration" ); own_wallet_state @@ -556,9 +556,10 @@ mod wallet_tests { .await?; assert_eq!( 1, - own_wallet_state.expected_utxos.len(), + own_wallet_state.wallet_db.expected_utxos().len().await, "A: Expected UTXO list must have length 1 after block registration, due to potential reorganizations"); - let expected_utxos = own_wallet_state.expected_utxos.get_all_expected_utxos(); + let expected_utxos = own_wallet_state.wallet_db.expected_utxos().get_all().await; + assert_eq!(1, expected_utxos.len(), "B: Expected UTXO list must have length 1 after block registration, due to potential reorganizations"); assert_eq!( block_1.hash(), @@ -665,14 +666,13 @@ mod wallet_tests { // Add block to wallet state own_wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( cb_utxo, cb_output_randomness, own_spending_key.privacy_preimage, UtxoNotifier::OwnMiner, - ) - .unwrap(); + )) + .await; own_wallet_state .update_wallet_state_with_new_block( &genesis_block.kernel.body.mutator_set_accumulator, @@ -726,14 +726,13 @@ mod wallet_tests { rng.gen(), ); own_wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( cb_utxo_prime, cb_output_randomness_prime, own_spending_key.privacy_preimage, UtxoNotifier::OwnMiner, - ) - .unwrap(); + )) + .await; own_wallet_state .update_wallet_state_with_new_block( &previous_block.kernel.body.mutator_set_accumulator, @@ -954,14 +953,13 @@ mod wallet_tests { // Expect the UTXO outputs for receive_data in tx_outputs_to_other { own_wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( receive_data.utxo, receive_data.sender_randomness, own_spending_key.privacy_preimage, UtxoNotifier::Cli, - ) - .unwrap(); + )) + .await; } premine_receiver_global_state .set_new_tip(block_1.clone()) @@ -1014,14 +1012,13 @@ mod wallet_tests { let ret = make_mock_block(&previous_block, None, own_address, rng.gen()); next_block = ret.0; own_wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( ret.1, ret.2, own_spending_key.privacy_preimage, UtxoNotifier::OwnMiner, - ) - .unwrap(); + )) + .await; own_wallet_state .update_wallet_state_with_new_block( &previous_block.kernel.body.mutator_set_accumulator, @@ -1217,23 +1214,21 @@ mod wallet_tests { "Block must be valid after accumulating txs" ); own_wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( cb_utxo, cb_sender_randomness, own_spending_key.privacy_preimage, UtxoNotifier::OwnMiner, - ) - .unwrap(); + )) + .await; own_wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( tx_outputs_six.utxo, tx_outputs_six.sender_randomness, own_spending_key.privacy_preimage, UtxoNotifier::Cli, - ) - .unwrap(); + )) + .await; own_wallet_state .update_wallet_state_with_new_block( &block_2_b.kernel.body.mutator_set_accumulator, diff --git a/src/models/state/wallet/rusty_wallet_database.rs b/src/models/state/wallet/rusty_wallet_database.rs index 9cb0ac54..683f33a5 100644 --- a/src/models/state/wallet/rusty_wallet_database.rs +++ b/src/models/state/wallet/rusty_wallet_database.rs @@ -9,13 +9,18 @@ use crate::database::storage::storage_schema::SimpleRustyStorage; use crate::database::NeptuneLevelDb; use crate::prelude::twenty_first; +use super::expected_utxo::ExpectedUtxo; use super::monitored_utxo::MonitoredUtxo; pub struct RustyWalletDatabase { storage: SimpleRustyStorage, + // list of utxos we have already received in a block monitored_utxos: DbtVec, + // list of off-chain utxos we are expecting to receive in a future block + expected_utxos: DbtVec, + // records which block the database is synced to sync_label: DbtSingleton, @@ -31,18 +36,25 @@ impl RustyWalletDatabase { crate::LOG_LOCK_EVENT_CB, ); - let monitored_utxos_storage = storage + let monitored_utxos = storage .schema .new_vec::("monitored_utxos") .await; - let sync_label_storage = storage.schema.new_singleton::("sync_label").await; - let counter_storage = storage.schema.new_singleton::("counter").await; + + let expected_utxos = storage + .schema + .new_vec::("expected_utxos") + .await; + + let sync_label = storage.schema.new_singleton::("sync_label").await; + let counter = storage.schema.new_singleton::("counter").await; Self { storage, - monitored_utxos: monitored_utxos_storage, - sync_label: sync_label_storage, - counter: counter_storage, + monitored_utxos, + expected_utxos, + sync_label, + counter, } } @@ -56,6 +68,16 @@ impl RustyWalletDatabase { &mut self.monitored_utxos } + /// get expected_utxos. + pub fn expected_utxos(&self) -> &DbtVec { + &self.expected_utxos + } + + /// get mutable expected_utxos. + pub fn expected_utxos_mut(&mut self) -> &mut DbtVec { + &mut self.expected_utxos + } + /// Get the hash of the block to which this database is synced. pub async fn get_sync_label(&self) -> Digest { self.sync_label.get().await diff --git a/src/models/state/wallet/utxo_notification_pool.rs b/src/models/state/wallet/utxo_notification_pool.rs deleted file mode 100644 index 60406453..00000000 --- a/src/models/state/wallet/utxo_notification_pool.rs +++ /dev/null @@ -1,678 +0,0 @@ -use std::collections::HashMap; -use std::time::Duration; -use std::time::SystemTime; - -use anyhow::bail; -use anyhow::Result; -use bytesize::ByteSize; -use get_size::GetSize; -use itertools::Itertools; -use num_traits::Zero; -use priority_queue::DoublePriorityQueue; -use tracing::error; -use tracing::info; -use tracing::warn; -use twenty_first::math::tip5::Digest; -use twenty_first::util_types::algebraic_hasher::AlgebraicHasher; - -use crate::models::blockchain::shared::Hash; -use crate::models::blockchain::transaction::utxo::Utxo; -use crate::models::blockchain::transaction::AnnouncedUtxo; -use crate::models::blockchain::transaction::Transaction; -use crate::models::peer::InstanceId; -use crate::prelude::twenty_first; -use crate::util_types::mutator_set::addition_record::AdditionRecord; -use crate::util_types::mutator_set::commit; - -pub type Credibility = i32; - -// 28 days in secs -pub const UNRECEIVED_UTXO_NOTIFICATION_THRESHOLD_AGE_IN_SECS: u64 = 28 * 24 * 60 * 60; -pub const RECEIVED_UTXO_NOTIFICATION_THRESHOLD_AGE_IN_SECS: u64 = 3 * 24 * 60 * 60; - -/// represents utxo and secrets necessary for recipient to claim it. -/// -/// [ExpectedUtxo] is intended for offchain temporary storage of utxos that a -/// wallet sends to itself, eg change outputs. -/// -/// The `ExpectedUtxo` will exist in the local [UtxoNotificationPool] from the -/// time the transaction is sent until it is mined in a block and claimed by the -/// wallet. -/// -/// note that when using `ExpectedUtxo` there is a risk of losing funds because -/// the wallet stores this state on disk and if the associated file(s) are lost -/// then the funds cannot be claimed. -/// -/// an alternative is to use onchain symmetric keys instead, which uses some -/// blockchain space and may leak some privacy if a key is ever used more than -/// once. -/// -/// ### about `receiver_preimage` -/// -/// The `receiver_preimage` field in expected_utxo is not strictly necessary. -/// It is an optimization and a compromise. -/// -/// #### optimization -/// -/// An `ExpectedUtxo` really only needs `utxo`, `sender_randomness` and -/// `receiver_identifier`. These match the fields used in `PublicAnnouncement`. -/// -/// However it then becomes necessary to scan all known wallet keys (of all key -/// types) in order to find the matching key, in order to obtain the preimage. -/// -/// improvement: To avoid scanning a map of \[`receiver_identifier`\] --> -/// `derivation_index` could be stored in local wallet state for all known keys. -/// -/// However no such map presently exists, and so the most efficient and easy -/// thing is to include the preimage in [ExpectedUtxo] instead of a -/// `receiver_identifier`. This buys us an equivalent optimization for little -/// effort. -/// -/// #### compromise -/// -/// Because the preimage is necessary to create an [ExpectedUtxo] -/// `create_transaction()` must accept a `change_key: SpendingKey` parameter -/// rather than `change_address: ReceivingAddress`. One would normally expect -/// an output to require only an address. -/// -/// Further if [ExpectedUtxo] and `PublicAnnouncement` use the same fields then -/// they can share much of the same codepath when claiming. At present, we have -/// separate codepaths that perform largely the same function. -/// -/// We may revisit this area in the future, as it seems ripe for improvement. -/// In particular wallet receiver_identifier map idea indicates it is possible -/// to do this efficiently. As such it may be best to implement at least the -/// scanning based approach before mainnet. -/// -/// A branch with an implementation of the scanning approach exists: -/// danda/symmetric_keys_and_expected_utxos_without_receiver_preimage -/// -/// At time of writing many tests are not passing and need updating with the new -/// field. -/// -/// see [UtxoNotificationPool], [AnnouncedUtxo], [UtxoNotification](crate::models::blockchain::transaction::UtxoNotification) -#[derive(Clone, Debug, PartialEq, Eq, Hash, GetSize)] -pub struct ExpectedUtxo { - pub utxo: Utxo, - pub addition_record: AdditionRecord, - pub sender_randomness: Digest, - pub receiver_preimage: Digest, - pub received_from: UtxoNotifier, - pub notification_received: SystemTime, - pub mined_in_block: Option<(Digest, SystemTime)>, -} - -impl ExpectedUtxo { - pub fn new( - utxo: Utxo, - sender_randomness: Digest, - receiver_preimage: Digest, - received_from: UtxoNotifier, - ) -> Self { - Self { - addition_record: commit( - Hash::hash(&utxo), - sender_randomness, - receiver_preimage.hash::(), - ), - utxo, - sender_randomness, - receiver_preimage, - received_from, - notification_received: SystemTime::now(), - mined_in_block: None, - } - } -} - -#[derive(Clone, Debug, GetSize)] -pub struct UtxoNotificationPool { - max_total_size: usize, - max_unconfirmed_notification_count_per_peer: usize, - notifications: HashMap, - - peer_id_to_expected_utxos: HashMap>, - - #[get_size(ignore)] // This is relatively small compared to `notifications` - queue: DoublePriorityQueue, -} - -impl UtxoNotificationPool { - fn pop_min(&mut self) -> Option<(ExpectedUtxo, Credibility)> { - if let Some((utxo_digest, credibility)) = self.queue.pop_min() { - let expected_utxo = self.notifications.remove(&utxo_digest).unwrap(); - if let UtxoNotifier::PeerUnsigned((peer_id, _socket), _credibility) = - &expected_utxo.received_from - { - self.peer_id_to_expected_utxos - .entry(*peer_id) - .and_modify(|e| e.retain(|ar| *ar != expected_utxo.addition_record)); - if self.peer_id_to_expected_utxos[peer_id].is_empty() { - self.peer_id_to_expected_utxos.remove(peer_id); - } - } - debug_assert_eq!(self.notifications.len(), self.queue.len()); - Some((expected_utxo, credibility)) - } else { - None - } - } - - /// Minimize space used by data model. Reduces `capacity` of contained data structures - fn shrink_to_fit(&mut self) { - self.queue.shrink_to_fit(); - self.notifications.shrink_to_fit(); - self.peer_id_to_expected_utxos.shrink_to_fit(); - } - - /// Drop elements of lowest credibility until data model does not exceed its max allowed size - fn shrink_to_max_size(&mut self) { - while self.get_size() > self.max_total_size && self.pop_min().is_some() { - continue; - } - - // TODO: A call to this function might reallocate. Expensive! Is this a good idea? - // self.shrink_to_fit() - } - - /// Delete an expected UTXO from this data model - fn drop_expected_utxo(&mut self, addition_record: AdditionRecord) { - let maybe_removed = self.notifications.remove(&addition_record); - - if let Some(removed_exp_utxo) = maybe_removed { - self.queue.remove(&addition_record); - if let UtxoNotifier::PeerUnsigned((peer_id, _socket), _cred) = - removed_exp_utxo.received_from - { - self.peer_id_to_expected_utxos - .entry(peer_id) - .and_modify(|e| e.retain(|ar| *ar != addition_record)); - if self.peer_id_to_expected_utxos[&peer_id].is_empty() { - self.peer_id_to_expected_utxos.remove(&peer_id); - } - } - } - - debug_assert_eq!( - self.notifications.len(), - self.queue.len(), - "hashmap and queue length must agree for expected UTXO pool after drop" - ); - debug_assert!( - self.peer_id_to_expected_utxos.values().map(|x| x.len()).sum::() <= self.notifications.len(), - "Total number of UTXO notifications from peers may not exceed total number of expected UTXOs"); - } - - pub fn new(max_total_size: ByteSize, max_notification_count_per_peer: usize) -> Self { - Self { - max_total_size: max_total_size.0.try_into().unwrap(), - max_unconfirmed_notification_count_per_peer: max_notification_count_per_peer, - notifications: Default::default(), - queue: Default::default(), - peer_id_to_expected_utxos: Default::default(), - } - } - - pub fn len(&self) -> usize { - debug_assert_eq!( - self.notifications.len(), - self.queue.len(), - "Lengths of queue and hash map must match" - ); - self.notifications.len() - } - - pub fn is_empty(&self) -> bool { - self.len().is_zero() - } - - /// Scan the transaction for outputs that match with list of expected - /// incoming UTXOs, and returns expected UTXOs that are present in the - /// transaction. - /// Returns an iterator of [AnnouncedUtxo]. (addition record, UTXO, sender randomness, receiver_preimage) - pub fn scan_for_expected_utxos<'a>( - &'a self, - transaction: &'a Transaction, - ) -> impl Iterator + 'a { - transaction.kernel.outputs.iter().filter_map(|tx_output| { - self.notifications - .get(tx_output) - .map(|expected_utxo| expected_utxo.into()) - }) - } - - /// Return all expected UTXOs - pub fn get_all_expected_utxos(&self) -> Vec { - self.notifications.values().cloned().collect_vec() - } - - /// Add an expected incoming UTXO that the wallet will register if it shows up in a later block. - /// It's the caller's responsibility that the UTXO is spendable. - pub fn add_expected_utxo( - &mut self, - utxo: Utxo, - sender_randomness: Digest, - receiver_preimage: Digest, - received_from: UtxoNotifier, - ) -> Result { - // Check if UTXO notification exceeds peer's max number of allowed notifications - if let UtxoNotifier::PeerUnsigned((peer_id, peer_socket), _cred) = &received_from { - if let Some(expected_utxos_for_peer) = self.peer_id_to_expected_utxos.get(peer_id) { - if expected_utxos_for_peer.len() >= self.max_unconfirmed_notification_count_per_peer - { - warn!("Stored {} expected UTXOs for peer {peer_id}, exceeds capacity. Checking if any are confirmed...", self.max_unconfirmed_notification_count_per_peer); - // Check if the expected UTXOs have not been mined yet. If they haven't, then - // this insertion is not allowed. - - let number_of_unconfirmed_utxo_notifications = expected_utxos_for_peer - .iter() - .filter(|x| self.notifications.get(x).unwrap().mined_in_block.is_none()) - .count(); - if number_of_unconfirmed_utxo_notifications - >= self.max_unconfirmed_notification_count_per_peer - { - let error_msg = format!("Received too many UTXO notifications from peer with instance ID {} and socket {}", peer_id, peer_socket); - error!(error_msg); - bail!(error_msg) - } - - info!("Some were confirmed. Accepting notification."); - } - } - } - // TODO: Add check that we can actually unlock the UTXO's lock script. - // Also check that receiver preimage belongs to us etc. - // Or should this be the caller's responsibility? - let addition_record = commit( - Hash::hash(&utxo), - sender_randomness, - receiver_preimage.hash::(), - ); - - // Check that addition record is not already contained in notification set. - // If it is, do not allow its timestamp to be updated. Return early. - if self.notifications.contains_key(&addition_record) { - warn!("Received repeated addition record. Ignoring"); - return Ok(addition_record); - } - - let expected_utxo = ExpectedUtxo { - addition_record, - utxo, - sender_randomness, - receiver_preimage, - received_from: received_from.clone(), - notification_received: SystemTime::now(), - mined_in_block: None, - }; - let ret = self.notifications.insert(addition_record, expected_utxo); - - // Sanity check - assert!( - ret.is_none(), - "Addition record was already present in expected UTXO set" - ); - - self.queue - .push(addition_record, received_from.credibility()); - - debug_assert_eq!( - self.notifications.len(), - self.queue.len(), - "hashmap and queue length must agree for expected UTXO pool after add" - ); - - // Add addition record to list for peer - if let UtxoNotifier::PeerUnsigned((peer_id, _socket), _cred) = &received_from { - self.peer_id_to_expected_utxos - .entry(*peer_id) - .and_modify(|e| e.push(addition_record)) - .or_insert(vec![addition_record]); - } - - // Ensure that data model does not take up more space than allowed - self.shrink_to_max_size(); - - Ok(addition_record) - } - - /// Mark an expected incoming UTXO as received - pub fn mark_as_received( - &mut self, - addition_record: AdditionRecord, - block_digest: Digest, - ) -> Result<()> { - if let Some(entry) = self.notifications.get_mut(&addition_record) { - entry.mined_in_block = Some((block_digest, SystemTime::now())); - Ok(()) - } else { - let error_msg = "Requested to mark unknown UTXO notification as received"; - error!(error_msg); - bail!(error_msg); - } - } - - /// Delete UTXO notifications that exceed a certain age - pub fn prune_stale_utxo_notifications(&mut self) { - let cutoff_for_unreceived = SystemTime::now() - - Duration::from_secs(UNRECEIVED_UTXO_NOTIFICATION_THRESHOLD_AGE_IN_SECS); - let cutoff_for_received = SystemTime::now() - - Duration::from_secs(RECEIVED_UTXO_NOTIFICATION_THRESHOLD_AGE_IN_SECS); - let stale_notifications = self - .notifications - .iter() - .filter(|(_ar, expected)| match expected.mined_in_block { - Some((_bh, registered_timestamp)) => registered_timestamp < cutoff_for_received, - None => expected.notification_received < cutoff_for_unreceived, - }) - .map(|(ar, _)| *ar) - .collect_vec(); - - for stale_notification in stale_notifications { - self.drop_expected_utxo(stale_notification); - } - - // Minimize space used by data model. - self.shrink_to_fit(); - } -} - -#[derive(Clone, Debug, PartialEq, Eq, Hash, GetSize)] -pub enum UtxoNotifier { - OwnMiner, - Cli, - Myself, - // ((instanceId, stringified SocketAddr), peer credibility) - PeerUnsigned((InstanceId, String), Credibility), - Premine, -} - -const OWN_MINER_SUPPRESSION: Credibility = 1; -const CLI_SUPPRESSION: Credibility = 2; -const MYSELF_SUPPRESSION: Credibility = 1; -const UNSIGNED_PEER_SUPPRESSION: Credibility = 4; - -impl UtxoNotifier { - pub fn credibility(&self) -> Credibility { - match self { - UtxoNotifier::Premine => Credibility::MAX, - UtxoNotifier::OwnMiner => Credibility::MAX - OWN_MINER_SUPPRESSION, - UtxoNotifier::Cli => Credibility::MAX - CLI_SUPPRESSION, - UtxoNotifier::Myself => Credibility::MAX - MYSELF_SUPPRESSION, - UtxoNotifier::PeerUnsigned(_, credibility) => { - // Ensure that peer notifications always have lower priority - // than those reported other ways, and prevent overflow in this calculation - credibility.saturating_sub(UNSIGNED_PEER_SUPPRESSION) - } - } - } -} - -#[cfg(test)] -mod wallet_state_tests { - use rand::random; - use tracing_test::traced_test; - - use crate::models::blockchain::transaction::utxo::LockScript; - use crate::models::blockchain::type_scripts::neptune_coins::NeptuneCoins; - use crate::tests::shared::make_mock_transaction; - - use super::*; - - #[traced_test] - #[tokio::test] - async fn utxo_notification_insert_remove_scan() { - let mut notification_pool = UtxoNotificationPool::new(ByteSize::kb(1), 100); - assert!(notification_pool.is_empty()); - assert!(notification_pool.len().is_zero()); - let mock_utxo = Utxo { - lock_script_hash: LockScript::anyone_can_spend().hash(), - coins: NeptuneCoins::new(10).to_native_coins(), - }; - let sender_randomness: Digest = random(); - let receiver_preimage: Digest = random(); - let peer_instance_id: InstanceId = random(); - let expected_addition_record = commit( - Hash::hash(&mock_utxo), - sender_randomness, - receiver_preimage.hash::(), - ); - notification_pool - .add_expected_utxo( - mock_utxo.clone(), - sender_randomness, - receiver_preimage, - UtxoNotifier::PeerUnsigned((peer_instance_id, String::default()), 100), - ) - .unwrap(); - assert!(!notification_pool.is_empty()); - assert_eq!(1, notification_pool.len()); - assert_eq!( - 1, - notification_pool.peer_id_to_expected_utxos[&peer_instance_id].len() - ); - - let mock_tx_containing_expected_utxo = - make_mock_transaction(vec![], vec![expected_addition_record]); - - let ret_with_tx_containing_utxo = notification_pool - .scan_for_expected_utxos(&mock_tx_containing_expected_utxo) - .collect_vec(); - assert_eq!(1, ret_with_tx_containing_utxo.len()); - - // Call scan but with another input. Verify that it returns the empty list - let another_addition_record = commit( - Hash::hash(&mock_utxo), - random(), - receiver_preimage.hash::(), - ); - let tx_without_utxo = make_mock_transaction(vec![], vec![another_addition_record]); - let ret_with_tx_without_utxo = notification_pool - .scan_for_expected_utxos(&tx_without_utxo) - .collect_vec(); - assert!(ret_with_tx_without_utxo.is_empty()); - - // Verify that we can remove the expected UTXO again - notification_pool.drop_expected_utxo(expected_addition_record); - assert!(notification_pool.is_empty()); - assert!( - !notification_pool - .peer_id_to_expected_utxos - .contains_key(&peer_instance_id), - "Key for peer must be deleted after removal of expected UTXO" - ); - } - - #[traced_test] - #[tokio::test] - async fn utxo_notification_peer_spam_test() { - let max_number_of_stored_utxos_per_peer = 100; - let mut notification_pool = - UtxoNotificationPool::new(ByteSize::mb(1), max_number_of_stored_utxos_per_peer); - - let spamming_peer: InstanceId = random(); - let mock_utxo = Utxo { - lock_script_hash: LockScript::anyone_can_spend().hash(), - coins: NeptuneCoins::new(14).to_native_coins(), - }; - let receiver_preimage: Digest = random(); - let first_sender_randomness: Digest = random(); - let spamming_peer_credibility = 10; - - // Add 1 expected UTXO from spamming peer - notification_pool - .add_expected_utxo( - mock_utxo.clone(), - first_sender_randomness, - receiver_preimage, - UtxoNotifier::PeerUnsigned( - (spamming_peer, String::default()), - spamming_peer_credibility, - ), - ) - .unwrap(); - - // Add another N, until capacity from spamming peer - for _ in 1..max_number_of_stored_utxos_per_peer { - notification_pool - .add_expected_utxo( - mock_utxo.clone(), - random(), - receiver_preimage, - UtxoNotifier::PeerUnsigned( - (spamming_peer, String::default()), - spamming_peer_credibility, - ), - ) - .unwrap(); - } - - // The next insertion must fail - assert!(notification_pool - .add_expected_utxo( - mock_utxo.clone(), - random(), - receiver_preimage, - UtxoNotifier::PeerUnsigned( - (spamming_peer, String::default()), - spamming_peer_credibility - ), - ) - .is_err()); - - // An insertion from another peer must be allowed - let non_spamming_peer_credibility = 100; - let non_spamming_peer: InstanceId = random(); - notification_pool - .add_expected_utxo( - mock_utxo.clone(), - random(), - random(), - UtxoNotifier::PeerUnsigned( - (non_spamming_peer, String::default()), - non_spamming_peer_credibility, - ), - ) - .unwrap(); - - // Once one of the spamming peer's UTXOs are marked as received, another UTXO - // notification can be stored. - notification_pool - .mark_as_received( - commit( - Hash::hash(&mock_utxo), - first_sender_randomness, - receiver_preimage.hash::(), - ), - Digest::default(), - ) - .unwrap(); - notification_pool - .add_expected_utxo( - mock_utxo, - random(), - random(), - UtxoNotifier::PeerUnsigned( - (spamming_peer, String::default()), - spamming_peer_credibility, - ), - ) - .unwrap(); - assert_eq!( - 101, - notification_pool.peer_id_to_expected_utxos[&spamming_peer].len() - ); - assert_eq!( - 1, - notification_pool.peer_id_to_expected_utxos[&non_spamming_peer].len() - ); - - // Verify that `pop_min` returns the UTXO notification with lowest credibility - let (lowest_priority_notif, infimum_cred) = notification_pool.pop_min().unwrap(); - assert_eq!( - spamming_peer_credibility - UNSIGNED_PEER_SUPPRESSION, - infimum_cred - ); - assert_eq!( - UtxoNotifier::PeerUnsigned( - (spamming_peer, String::default()), - spamming_peer_credibility - ), - lowest_priority_notif.received_from - ); - } - - #[traced_test] - #[tokio::test] - async fn prune_stale_utxo_notifications_test() { - let mut notification_pool = UtxoNotificationPool::new(ByteSize::mb(1), 100); - let mock_utxo = Utxo { - lock_script_hash: LockScript::anyone_can_spend().hash(), - coins: NeptuneCoins::new(14).to_native_coins(), - }; - - // Add a UTXO notification - let mut addition_records = vec![]; - let ar = notification_pool - .add_expected_utxo( - mock_utxo.clone(), - random(), - random(), - UtxoNotifier::PeerUnsigned((random(), String::default()), 10), - ) - .unwrap(); - addition_records.push(ar); - - // Add three more - for _ in 0..3 { - let ar_new = notification_pool - .add_expected_utxo( - mock_utxo.clone(), - random(), - random(), - UtxoNotifier::PeerUnsigned((random(), String::default()), 10), - ) - .unwrap(); - addition_records.push(ar_new); - } - - // Test with a UTXO that was received - // Manipulate the time this entry was inserted - let two_weeks_as_sec = 60 * 60 * 24 * 7 * 2; - notification_pool - .notifications - .get_mut(&addition_records[0]) - .unwrap() - .mined_in_block = Some(( - Digest::default(), - SystemTime::now() - Duration::from_secs(two_weeks_as_sec), - )); - - assert_eq!(4, notification_pool.len()); - notification_pool.prune_stale_utxo_notifications(); - assert_eq!(3, notification_pool.len()); - - // Test with a UTXOs that were *not* received - // Manipulate the time of two more entries - let eight_weeks_as_secs = 60 * 60 * 24 * 7 * 8; - notification_pool - .notifications - .get_mut(&addition_records[1]) - .unwrap() - .notification_received = SystemTime::now() - Duration::from_secs(eight_weeks_as_secs); - notification_pool - .notifications - .get_mut(&addition_records[3]) - .unwrap() - .notification_received = SystemTime::now() - Duration::from_secs(eight_weeks_as_secs); - assert_eq!(3, notification_pool.len()); - notification_pool.prune_stale_utxo_notifications(); - assert_eq!(1, notification_pool.len()); - - assert_eq!( - addition_records[2], - notification_pool.get_all_expected_utxos()[0].addition_record, - "Expected UTXO notification must remain" - ) - } -} diff --git a/src/models/state/wallet/wallet_state.rs b/src/models/state/wallet/wallet_state.rs index dc58ddd6..01e5bdb2 100644 --- a/src/models/state/wallet/wallet_state.rs +++ b/src/models/state/wallet/wallet_state.rs @@ -25,7 +25,7 @@ use twenty_first::util_types::algebraic_hasher::AlgebraicHasher; use crate::config_models::cli_args::Args; use crate::config_models::data_directory::DataDirectory; use crate::database::storage::storage_schema::traits::*; -use crate::database::storage::storage_vec::traits::*; +use crate::database::storage::storage_vec::{traits::*, Index}; use crate::database::NeptuneLevelDb; use crate::models::blockchain::block::Block; use crate::models::blockchain::transaction::utxo::Utxo; @@ -51,9 +51,9 @@ use super::address::symmetric_key; use super::address::KeyType; use super::address::SpendingKey; use super::coin_with_possible_timelock::CoinWithPossibleTimeLock; +use super::expected_utxo::ExpectedUtxo; +use super::expected_utxo::UtxoNotifier; use super::rusty_wallet_database::RustyWalletDatabase; -use super::utxo_notification_pool::UtxoNotificationPool; -use super::utxo_notification_pool::UtxoNotifier; use super::wallet_status::WalletStatus; use super::wallet_status::WalletStatusElement; use super::WalletSecret; @@ -64,10 +64,6 @@ pub struct WalletState { pub wallet_secret: WalletSecret, pub number_of_mps_per_utxo: usize, - // Any task may read from expected_utxos, only main task may write - pub expected_utxos: UtxoNotificationPool, - - /// Path to directory containing wallet files wallet_directory_path: PathBuf, } @@ -101,7 +97,6 @@ impl Debug for WalletState { f.debug_struct("WalletState") .field("wallet_secret", &self.wallet_secret) .field("number_of_mps_per_utxo", &self.number_of_mps_per_utxo) - .field("expected_utxos", &self.expected_utxos) .field("wallet_directory_path", &self.wallet_directory_path) .finish() } @@ -204,10 +199,6 @@ impl WalletState { wallet_db: rusty_wallet_database, wallet_secret, number_of_mps_per_utxo: cli_args.number_of_mps_per_utxo, - expected_utxos: UtxoNotificationPool::new( - cli_args.max_utxo_notification_size, - cli_args.max_unconfirmed_utxo_notification_count_per_peer, - ), wallet_directory_path: data_dir.wallet_directory_path(), }; @@ -223,14 +214,13 @@ impl WalletState { for utxo in Block::premine_utxos(cli_args.network) { if utxo.lock_script_hash == own_receiving_address.lock_script().hash() { wallet_state - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( utxo, - Digest::default(), + Digest::default(), // sender_randomness own_spending_key.privacy_preimage(), UtxoNotifier::Premine, - ) - .unwrap(); + )) + .await; } } @@ -247,6 +237,14 @@ impl WalletState { wallet_state } + // note: does not verify we do not have any dups. + pub(crate) async fn add_expected_utxo(&mut self, expected_utxo: ExpectedUtxo) { + self.wallet_db + .expected_utxos_mut() + .push(expected_utxo) + .await; + } + /// Return a list of UTXOs spent by this wallet in the transaction async fn scan_for_spent_utxos( &self, @@ -303,6 +301,88 @@ impl WalletState { }) } + /// Scan the transaction for outputs that match with list of expected + /// incoming UTXOs, and returns expected UTXOs that are present in the + /// transaction. + /// + /// note: this algorithm is o(n) + o(m) where: + /// n = number of ExpectedUtxo in database. (all-time) + /// m = number of transaction outputs. + /// + /// see https://github.com/Neptune-Crypto/neptune-core/pull/175#issuecomment-2302511025 + /// + /// Returns an iterator of [AnnouncedUtxo]. (addition record, UTXO, sender randomness, receiver_preimage) + pub async fn scan_for_expected_utxos<'a>( + &'a self, + transaction: &'a Transaction, + ) -> impl Iterator + 'a { + let expected_utxos = self.wallet_db.expected_utxos().get_all().await; + let eu_map: HashMap<_, _> = expected_utxos + .into_iter() + .map(|eu| (eu.addition_record, eu)) + .collect(); + + transaction + .kernel + .outputs + .iter() + .filter_map(move |a| eu_map.get(a).map(|eu| eu.into())) + } + + /// Delete all ExpectedUtxo that exceed a certain age + /// + /// note: It is questionable if this method should ever be called + /// as presently implemented. + /// + /// issues: + /// 1. expiration does not consider if utxo has been + /// claimed by wallet or not. + /// 2. expiration thresholds are based on time, not + /// # of blocks. + /// 3. what if a deep re-org occurs after ExpectedUtxo + /// have been expired? possible loss of funds. + /// + /// Fundamentally, any time we remove an ExpectedUtxo we risk a possible + /// loss of funds in the future. + /// + /// for now, it may be best to simply leave all ExpectedUtxo in the wallet + /// database forever. This is the safest way to prevent a possible loss of + /// funds. + /// + /// note: DbtVec does not have a remove(). + /// So it is implemented by clearing all ExpectedUtxo from DB and + /// adding back those that are not stale. + pub async fn prune_stale_expected_utxos(&mut self) { + // prune un-received ExpectedUtxo after 28 days in secs + const UNRECEIVED_UTXO_SECS: u64 = 28 * 24 * 60 * 60; + + // prune received ExpectedUtxo after 3 days in secs. + const RECEIVED_UTXO_SECS: u64 = 3 * 24 * 60 * 60; + + let cutoff_for_unreceived = Timestamp::now() - Timestamp::seconds(UNRECEIVED_UTXO_SECS); + let cutoff_for_received = Timestamp::now() - Timestamp::seconds(RECEIVED_UTXO_SECS); + + let expected_utxos = self.wallet_db.expected_utxos().get_all().await; + + let keep_indexes = expected_utxos + .iter() + .enumerate() + .filter(|(_, eu)| match eu.mined_in_block { + Some((_bh, registered_timestamp)) => registered_timestamp >= cutoff_for_received, + None => eu.notification_received >= cutoff_for_unreceived, + }) + .map(|(idx, _)| idx); + + self.wallet_db.expected_utxos_mut().clear().await; + + for idx in keep_indexes.rev() { + self.wallet_db + .expected_utxos_mut() + .push(expected_utxos[idx].clone()) + .await; + } + } + // returns true if the utxo can be unlocked by one of the // known wallet keys. pub fn can_unlock(&self, utxo: &Utxo) -> bool { @@ -414,8 +494,8 @@ impl WalletState { let onchain_received_outputs = self.scan_for_announced_utxos(&transaction); let offchain_received_outputs = self - .expected_utxos .scan_for_expected_utxos(&transaction) + .await .collect_vec(); let all_received_outputs = @@ -727,11 +807,23 @@ impl WalletState { } // Mark all expected UTXOs that were received in this block as received - offchain_received_outputs.into_iter().for_each(|au| { - self.expected_utxos - .mark_as_received(au.addition_record, new_block.hash()) - .expect("Expected UTXO must be present when marking it as received") - }); + let updates = self + .wallet_db + .expected_utxos() + .get_all() + .await + .into_iter() + .enumerate() + .filter(|(_, eu)| { + offchain_received_outputs + .iter() + .any(|au| au.addition_record == eu.addition_record) + }) + .map(|(idx, mut eu)| { + eu.mined_in_block = Some((new_block.hash(), new_block.kernel.header.timestamp)); + (idx as Index, eu) + }); + self.wallet_db.expected_utxos_mut().set_many(updates).await; self.wallet_db.set_sync_label(new_block.hash()).await; self.wallet_db.persist().await; @@ -907,7 +999,7 @@ mod tests { use tracing_test::traced_test; use crate::config_models::network::Network; - use crate::models::state::wallet::utxo_notification_pool::ExpectedUtxo; + use crate::models::state::wallet::expected_utxo::ExpectedUtxo; use crate::tests::shared::make_mock_block; use crate::tests::shared::mock_genesis_global_state; use crate::tests::shared::mock_genesis_wallet_state; @@ -1196,18 +1288,126 @@ mod tests { } mod expected_utxos { + use crate::models::blockchain::transaction::utxo::LockScript; + use crate::tests::shared::make_mock_transaction; + use crate::util_types::mutator_set::commit; + use super::*; + #[traced_test] + #[tokio::test] + async fn insert_and_scan() { + let mut wallet = + mock_genesis_wallet_state(WalletSecret::new_random(), Network::RegTest).await; + + assert!(wallet.wallet_db.expected_utxos().is_empty().await); + assert!(wallet.wallet_db.expected_utxos().len().await.is_zero()); + + let mock_utxo = + Utxo::new_native_coin(LockScript::anyone_can_spend(), NeptuneCoins::new(10)); + + let sender_randomness: Digest = rand::random(); + let receiver_preimage: Digest = rand::random(); + let expected_addition_record = commit( + Hash::hash(&mock_utxo), + sender_randomness, + receiver_preimage.hash::(), + ); + wallet + .add_expected_utxo(ExpectedUtxo::new( + mock_utxo.clone(), + sender_randomness, + receiver_preimage, + UtxoNotifier::Myself, + )) + .await; + assert!(!wallet.wallet_db.expected_utxos().is_empty().await); + assert_eq!(1, wallet.wallet_db.expected_utxos().len().await); + + let mock_tx_containing_expected_utxo = + make_mock_transaction(vec![], vec![expected_addition_record]); + + let ret_with_tx_containing_utxo = wallet + .scan_for_expected_utxos(&mock_tx_containing_expected_utxo) + .await + .collect_vec(); + assert_eq!(1, ret_with_tx_containing_utxo.len()); + + // Call scan but with another input. Verify that it returns the empty list + let another_addition_record = commit( + Hash::hash(&mock_utxo), + rand::random(), + receiver_preimage.hash::(), + ); + let tx_without_utxo = make_mock_transaction(vec![], vec![another_addition_record]); + let ret_with_tx_without_utxo = wallet + .scan_for_expected_utxos(&tx_without_utxo) + .await + .collect_vec(); + assert!(ret_with_tx_without_utxo.is_empty()); + } + + #[traced_test] + #[tokio::test] + async fn prune_stale() { + let mut wallet = + mock_genesis_wallet_state(WalletSecret::new_random(), Network::RegTest).await; + + let mock_utxo = + Utxo::new_native_coin(LockScript::anyone_can_spend(), NeptuneCoins::new(14)); + + // Add a UTXO notification + let mut addition_records = vec![]; + let ar = wallet + .add_expected_utxo(ExpectedUtxo::new( + mock_utxo.clone(), + rand::random(), + rand::random(), + UtxoNotifier::Myself, + )) + .await; + addition_records.push(ar); + + // Add three more + for _ in 0..3 { + let ar_new = wallet + .add_expected_utxo(ExpectedUtxo::new( + mock_utxo.clone(), + rand::random(), + rand::random(), + UtxoNotifier::Myself, + )) + .await; + addition_records.push(ar_new); + } + + // Test with a UTXO that was received + // Manipulate the time this entry was inserted + let two_weeks_as_sec = 60 * 60 * 24 * 7 * 2; + let eu_idx = 0; + let mut eu = wallet.wallet_db.expected_utxos().get(eu_idx).await; + + // modify mined_in_block field. + eu.mined_in_block = Some(( + Digest::default(), + Timestamp::now() - Timestamp::seconds(two_weeks_as_sec), + )); + + // update db + wallet.wallet_db.expected_utxos_mut().set(eu_idx, eu).await; + + assert_eq!(4, wallet.wallet_db.expected_utxos().len().await); + wallet.prune_stale_expected_utxos().await; + assert_eq!(3, wallet.wallet_db.expected_utxos().len().await); + } + /// demonstrates/tests that if wallet-db is not persisted after an /// ExpectedUtxo is added, then the ExpectedUtxo will not exist after /// wallet is dropped from RAM and re-created from disk. /// - /// note: this test presently FAILS, which demonstrates validity of - /// issue #172. + /// This is a regression test for issue #172. /// /// https://github.com/Neptune-Crypto/neptune-core/issues/172 - /// - /// A future commit/pr will fix the issue so that this test passes. #[traced_test] #[tokio::test] #[allow(clippy::needless_return)] @@ -1226,7 +1426,6 @@ mod tests { } mod worker { - use crate::models::blockchain::transaction::utxo::LockScript; use crate::tests::shared::mock_genesis_wallet_state_with_data_dir; use crate::tests::shared::unit_test_data_directory; @@ -1251,7 +1450,7 @@ mod tests { let wallet_secret = WalletSecret::new_random(); let data_dir = unit_test_data_directory(network).unwrap(); - // create initial wallet in a new directory. + // create initial wallet in a new directory let mut wallet = mock_genesis_wallet_state_with_data_dir( wallet_secret.clone(), network, @@ -1262,20 +1461,19 @@ mod tests { let mock_utxo = Utxo::new_native_coin(LockScript::anyone_can_spend(), NeptuneCoins::new(14)); - assert!(wallet.expected_utxos.is_empty()); + assert!(wallet.wallet_db.expected_utxos().is_empty().await); - // Add a UTXO notification + // Add an ExpectedUtxo to the wallet. wallet - .expected_utxos - .add_expected_utxo( + .add_expected_utxo(ExpectedUtxo::new( mock_utxo.clone(), rand::random(), rand::random(), UtxoNotifier::Myself, - ) - .unwrap(); + )) + .await; - assert_eq!(1, wallet.expected_utxos.len()); + assert_eq!(1, wallet.wallet_db.expected_utxos().len().await); // persist wallet-db to disk, if testing that case. if persist { @@ -1294,7 +1492,10 @@ mod tests { // if wallet state was persisted to DB then we should have // 1 (restored) ExpectedUtxo, else 0. let expect = if persist { 1 } else { 0 }; - assert_eq!(expect, restored_wallet.expected_utxos.len()); + assert_eq!( + expect, + restored_wallet.wallet_db.expected_utxos().len().await + ); } } } diff --git a/src/rpc_server.rs b/src/rpc_server.rs index 6e77979e..8d101f43 100644 --- a/src/rpc_server.rs +++ b/src/rpc_server.rs @@ -814,10 +814,11 @@ mod rpc_server_tests { use ReceivingAddress; use crate::config_models::network::Network; + use crate::database::storage::storage_vec::traits::*; use crate::models::peer::PeerSanctionReason; use crate::models::state::wallet::address::generation_address::GenerationReceivingAddress; - use crate::models::state::wallet::utxo_notification_pool::ExpectedUtxo; - use crate::models::state::wallet::utxo_notification_pool::UtxoNotifier; + use crate::models::state::wallet::expected_utxo::ExpectedUtxo; + use crate::models::state::wallet::expected_utxo::UtxoNotifier; use crate::models::state::wallet::WalletSecret; use crate::rpc_server::NeptuneRPCServer; use crate::tests::shared::make_mock_block_with_valid_pow; @@ -1458,8 +1459,10 @@ mod rpc_server_tests { .lock_guard() .await .wallet_state - .expected_utxos - .len(); + .wallet_db + .expected_utxos() + .len() + .await; // --- Operation: perform send_to_many let result = rpc_server @@ -1477,8 +1480,10 @@ mod rpc_server_tests { .lock_guard() .await .wallet_state - .expected_utxos - .len(), + .wallet_db + .expected_utxos() + .len() + .await, num_expected_utxo + 2 );