From 7ab26752ca27b2e969309a37cfdb432931eaca30 Mon Sep 17 00:00:00 2001 From: danda Date: Mon, 19 Aug 2024 21:36:10 -0700 Subject: [PATCH] chore: cleanup consts, improve doc comments * addresses suggestions in PR #175. * fixes `cargo doc` warnings --- src/models/consensus/tasm/environment.rs | 2 +- src/models/state/wallet/expected_utxo.rs | 55 +++--------------------- src/models/state/wallet/wallet_state.rs | 13 +++--- 3 files changed, 16 insertions(+), 54 deletions(-) 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/state/wallet/expected_utxo.rs b/src/models/state/wallet/expected_utxo.rs index 8bfe039c..c14164cc 100644 --- a/src/models/state/wallet/expected_utxo.rs +++ b/src/models/state/wallet/expected_utxo.rs @@ -8,18 +8,15 @@ 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. +/// 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 @@ -31,48 +28,10 @@ pub const RECEIVED_UTXO_NOTIFICATION_THRESHOLD_AGE_IN_SECS: u64 = 3 * 24 * 60 * /// /// ### 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 issue #176. +/// /// -/// see [UtxoNotificationPool], [AnnouncedUtxo], [UtxoNotification](crate::models::blockchain::transaction::UtxoNotification) +/// 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, diff --git a/src/models/state/wallet/wallet_state.rs b/src/models/state/wallet/wallet_state.rs index 868929c6..c3b8cbe4 100644 --- a/src/models/state/wallet/wallet_state.rs +++ b/src/models/state/wallet/wallet_state.rs @@ -51,7 +51,6 @@ 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; @@ -344,10 +343,14 @@ impl WalletState { /// 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); + // 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;