Skip to content

Commit

Permalink
chore: cleanup consts, improve doc comments
Browse files Browse the repository at this point in the history
* addresses suggestions in PR #175.
* fixes `cargo doc` warnings
  • Loading branch information
dan-da committed Aug 22, 2024
1 parent 232e852 commit 7ab2675
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 54 deletions.
2 changes: 1 addition & 1 deletion src/models/consensus/tasm/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
//! <https://github.com/greenhat/omnizk>

use std::cell::RefCell;
use std::collections::HashMap;
Expand Down
55 changes: 7 additions & 48 deletions src/models/state/wallet/expected_utxo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
/// <https://github.com/Neptune-Crypto/neptune-core/issues/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,
Expand Down
13 changes: 8 additions & 5 deletions src/models/state/wallet/wallet_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down

0 comments on commit 7ab2675

Please sign in to comment.