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/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..4843c9b4 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; @@ -36,7 +36,7 @@ use crate::locks::tokio as sync_tokio; use crate::models::blockchain::transaction::UtxoNotifyMethod; use crate::models::peer::HandshakeData; use crate::models::state::wallet::monitored_utxo::MonitoredUtxo; -use crate::models::state::wallet::utxo_notification_pool::ExpectedUtxo; +use crate::models::state::wallet::expected_utxo::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..8bfe039c --- /dev/null +++ b/src/models/state/wallet/expected_utxo.rs @@ -0,0 +1,116 @@ +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}; + +// 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, 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/wallet_state.rs b/src/models/state/wallet/wallet_state.rs index dc58ddd6..868929c6 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,10 @@ use super::address::symmetric_key; use super::address::KeyType; use super::address::SpendingKey; use super::coin_with_possible_timelock::CoinWithPossibleTimeLock; +use super::expected_utxo; +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 +65,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 +98,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 +200,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 +215,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 +238,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 +302,74 @@ 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. + /// 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; + + transaction.kernel.outputs.iter().filter_map(move |a| { + expected_utxos + .iter() + .find(|eu| eu.addition_record == *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) { + let cutoff_for_unreceived = Timestamp::now() + - Timestamp::seconds(expected_utxo::UNRECEIVED_UTXO_NOTIFICATION_THRESHOLD_AGE_IN_SECS); + let cutoff_for_received = Timestamp::now() + - Timestamp::seconds(expected_utxo::RECEIVED_UTXO_NOTIFICATION_THRESHOLD_AGE_IN_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 +481,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 +794,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 +986,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 +1275,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 +1413,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 +1437,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 +1448,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 +1479,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 );