From 226a371cbd928ba05bfd2c1d14861928521ba324 Mon Sep 17 00:00:00 2001 From: sword_smith Date: Thu, 9 Jan 2025 14:04:08 +0100 Subject: [PATCH] fix(`WalletState`): Filter out unspendable UTXOs before monitoring Before this commit, the wallet would add UTXOs to the database of monitored UTXOs even if it has an unrecognized or even unsatisfiable type script. This commit adds a filter to the process of upgrading expected UTXOs to monitored UTXOs, guaranteeing that all type scripts are known. Also: - Add test for above refactor. - Improve some functions' names. - Factor out common test code. Co-authored-by: Alan Szepieniec --- src/mine_loop.rs | 2 +- src/models/blockchain/transaction/utxo.rs | 13 +- src/models/state/archival_state.rs | 12 +- src/models/state/mod.rs | 16 +- src/models/state/wallet/mod.rs | 4 +- src/models/state/wallet/transaction_output.rs | 13 ++ src/models/state/wallet/wallet_state.rs | 192 ++++++++++++++---- src/models/state/wallet/wallet_status.rs | 10 +- src/rpc_server.rs | 6 +- 9 files changed, 208 insertions(+), 60 deletions(-) diff --git a/src/mine_loop.rs b/src/mine_loop.rs index 70d653100..7b6f4b9c7 100644 --- a/src/mine_loop.rs +++ b/src/mine_loop.rs @@ -1118,7 +1118,7 @@ pub(crate) mod mine_loop_tests { .await .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(now) + .synced_unspent_liquid_amount(now) .is_zero(), "Assumed to be premine-recipient" ); diff --git a/src/models/blockchain/transaction/utxo.rs b/src/models/blockchain/transaction/utxo.rs index 4ec2cf75b..d9fd1edc5 100644 --- a/src/models/blockchain/transaction/utxo.rs +++ b/src/models/blockchain/transaction/utxo.rs @@ -161,7 +161,7 @@ impl Utxo { /// Determine whether the UTXO has coins that contain only known type /// scripts. If other type scripts are included, then we cannot spend /// this UTXO. - pub fn has_known_type_scripts(&self) -> bool { + pub fn all_type_scripts_are_known(&self) -> bool { let known_type_script_hashes = [NativeCurrency.hash(), TimeLock.hash()]; self.coins .iter() @@ -175,7 +175,7 @@ impl Utxo { pub fn can_spend_at(&self, timestamp: Timestamp) -> bool { crate::macros::log_slow_scope!(); // unknown type script - if !self.has_known_type_scripts() { + if !self.all_type_scripts_are_known() { return false; } @@ -204,7 +204,7 @@ impl Utxo { /// Determine whether the only thing preventing the UTXO from being spendable /// is the timelock whose according release date is in the future. pub fn is_timelocked_but_otherwise_spendable_at(&self, timestamp: Timestamp) -> bool { - if !self.has_known_type_scripts() { + if !self.all_type_scripts_are_known() { return false; } @@ -307,6 +307,13 @@ mod test { (lock_script_hash, coins).into() } + impl Utxo { + pub(crate) fn with_coin(mut self, coin: Coin) -> Self { + self.coins.push(coin); + self + } + } + #[test] fn hash_utxo_test() { let output = make_random_utxo(); diff --git a/src/models/state/archival_state.rs b/src/models/state/archival_state.rs index d3dc1cab3..b8f97c02f 100644 --- a/src/models/state/archival_state.rs +++ b/src/models/state/archival_state.rs @@ -1963,7 +1963,7 @@ mod archival_state_tests { .await .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(in_seven_months) + .synced_unspent_liquid_amount(in_seven_months) ); assert_eq!( NeptuneCoins::new(5), @@ -1971,7 +1971,7 @@ mod archival_state_tests { .await .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(in_seven_months) + .synced_unspent_liquid_amount(in_seven_months) ); let block_subsidy = Block::block_subsidy(block_1.header().height); @@ -1988,7 +1988,7 @@ mod archival_state_tests { .await .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(in_seven_months) + .synced_unspent_liquid_amount(in_seven_months) ); let after_cb_timelock_expiration = block_1.header().timestamp + Timestamp::months(37); @@ -1999,7 +1999,7 @@ mod archival_state_tests { .await .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(after_cb_timelock_expiration) + .synced_unspent_liquid_amount(after_cb_timelock_expiration) ); println!("Transactions were received in good order."); @@ -2177,14 +2177,14 @@ mod archival_state_tests { .await .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(in_seven_months) + .synced_unspent_liquid_amount(in_seven_months) .is_zero()); assert!(bob .lock_guard() .await .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(in_seven_months) + .synced_unspent_liquid_amount(in_seven_months) .is_zero()); // Verify that all ingoing UTXOs are recorded in wallet of receiver of genesis UTXO diff --git a/src/models/state/mod.rs b/src/models/state/mod.rs index 7002ded48..adb7668bc 100644 --- a/src/models/state/mod.rs +++ b/src/models/state/mod.rs @@ -1820,7 +1820,7 @@ mod global_state_tests { assert!(!alice .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(launch + seven_months) + .synced_unspent_liquid_amount(launch + seven_months) .is_zero()); // Verify that this is unsynced with mock_block_1a @@ -2338,7 +2338,7 @@ mod global_state_tests { .await .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(in_seven_months) + .synced_unspent_liquid_amount(in_seven_months) ); assert_eq!( NeptuneCoins::new(7), @@ -2346,7 +2346,7 @@ mod global_state_tests { .await .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(in_seven_months) + .synced_unspent_liquid_amount(in_seven_months) ); // TODO: No idea why this isn't working. // { @@ -3088,7 +3088,7 @@ mod global_state_tests { let alice_initial_balance = alice_state_mut .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(seven_months_post_launch); + .synced_unspent_liquid_amount(seven_months_post_launch); assert_eq!(alice_initial_balance, NeptuneCoins::new(20)); // create change key for alice. change_key_type is a test param. @@ -3185,7 +3185,7 @@ mod global_state_tests { alice_state_mut .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(seven_months_post_launch) + .synced_unspent_liquid_amount(seven_months_post_launch) ); block_1 @@ -3204,7 +3204,7 @@ mod global_state_tests { bob_state_mut .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(seven_months_post_launch) + .synced_unspent_liquid_amount(seven_months_post_launch) ); } @@ -3241,7 +3241,7 @@ mod global_state_tests { let alice_initial_balance = alice_state_mut .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(seven_months_post_launch); + .synced_unspent_liquid_amount(seven_months_post_launch); // lucky alice's wallet begins with 20 balance from premine. assert_eq!(alice_initial_balance, NeptuneCoins::new(20)); @@ -3272,7 +3272,7 @@ mod global_state_tests { alice_state_mut .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(seven_months_post_launch) + .synced_unspent_liquid_amount(seven_months_post_launch) ); } } diff --git a/src/models/state/wallet/mod.rs b/src/models/state/wallet/mod.rs index e054424df..c407c1b95 100644 --- a/src/models/state/wallet/mod.rs +++ b/src/models/state/wallet/mod.rs @@ -900,7 +900,7 @@ mod wallet_tests { let bobs_original_balance = bob .get_wallet_status_for_tip() .await - .synced_unspent_available_amount(in_seven_months); + .synced_unspent_liquid_amount(in_seven_months); assert!( !bobs_original_balance.is_zero(), "Premine must have non-zero synced balance" @@ -958,7 +958,7 @@ mod wallet_tests { .unwrap(), bob.get_wallet_status_for_tip() .await - .synced_unspent_available_amount(in_seven_months), + .synced_unspent_liquid_amount(in_seven_months), "Preminer must have spent 15: 12 + 1 for sent, 2 for fees" ); diff --git a/src/models/state/wallet/transaction_output.rs b/src/models/state/wallet/transaction_output.rs index 9c518e921..7579a9608 100644 --- a/src/models/state/wallet/transaction_output.rs +++ b/src/models/state/wallet/transaction_output.rs @@ -368,12 +368,25 @@ mod tests { use super::*; use crate::config_models::cli_args; use crate::config_models::network::Network; + use crate::models::blockchain::transaction::utxo::Coin; use crate::models::blockchain::type_scripts::neptune_coins::NeptuneCoins; use crate::models::state::wallet::address::generation_address::GenerationReceivingAddress; use crate::models::state::wallet::address::KeyType; use crate::models::state::wallet::WalletSecret; use crate::tests::shared::mock_genesis_global_state; + impl TxOutput { + pub(crate) fn with_coin(self, coin: Coin) -> Self { + Self { + utxo: self.utxo.with_coin(coin), + sender_randomness: self.sender_randomness, + receiver_digest: self.receiver_digest, + notification_method: self.notification_method, + owned: self.owned, + } + } + } + #[tokio::test] async fn test_utxoreceiver_auto_not_owned_output() { let global_state_lock = mock_genesis_global_state( diff --git a/src/models/state/wallet/wallet_state.rs b/src/models/state/wallet/wallet_state.rs index e50a6b210..e018ff124 100644 --- a/src/models/state/wallet/wallet_state.rs +++ b/src/models/state/wallet/wallet_state.rs @@ -439,7 +439,7 @@ impl WalletState { ) -> NeptuneCoins { let wallet_status = self.get_wallet_status_from_lock(tip_digest).await; - wallet_status.synced_unspent_available_amount(timestamp) + wallet_status.synced_unspent_liquid_amount(timestamp) } pub async fn unconfirmed_balance( @@ -472,6 +472,10 @@ impl WalletState { // note: does not verify we do not have any dups. pub(crate) async fn add_expected_utxo(&mut self, expected_utxo: ExpectedUtxo) { + if !expected_utxo.utxo.all_type_scripts_are_known() { + warn!("adding expected UTXO with *unknown type script* to expected UTXOs database"); + } + self.wallet_db .expected_utxos_mut() .push(expected_utxo) @@ -831,8 +835,6 @@ impl WalletState { /// /// Assume the given block is valid and that the wallet state is not synced /// with the new block yet but is synced with the previous block (if any). - /// The previous block necessary (if it exists) to supply the - /// mutator set accumulator and guesser fee. pub async fn update_wallet_state_with_new_block( &mut self, previous_mutator_set_accumulator: &MutatorSetAccumulator, @@ -928,11 +930,12 @@ impl WalletState { .await .collect_vec(); - let all_received_outputs = - onchain_received_outputs.chain(offchain_received_outputs.iter().cloned()); + let all_spendable_received_outputs = onchain_received_outputs + .chain(offchain_received_outputs.iter().cloned()) + .filter(|announced_utxo| announced_utxo.utxo.all_type_scripts_are_known()); let addition_record_to_utxo_info: HashMap = - all_received_outputs + all_spendable_received_outputs .map(|au| { ( au.addition_record(), @@ -941,13 +944,6 @@ impl WalletState { }) .collect(); - debug!( - "announced outputs received: onchain: {}, offchain: {}, total: {}", - addition_record_to_utxo_info.len() - offchain_received_outputs.len(), - offchain_received_outputs.len(), - addition_record_to_utxo_info.len() - ); - // Derive the membership proofs for received UTXOs, and in // the process update existing membership proofs with // updates from this block @@ -1278,7 +1274,7 @@ impl WalletState { let wallet_status = self.get_wallet_status_from_lock(tip_digest).await; // First check that we have enough. Otherwise return an error. - let available = wallet_status.synced_unspent_available_amount(timestamp); + let available = wallet_status.synced_unspent_liquid_amount(timestamp); if available < total_spend { bail!( "Insufficient funds. Requested: {}, Available: {}", @@ -1361,10 +1357,13 @@ mod tests { use crate::config_models::cli_args; use crate::config_models::network::Network; use crate::job_queue::triton_vm::TritonVmJobQueue; + use crate::models::blockchain::transaction::transaction_kernel::TransactionKernelModifier; + use crate::models::blockchain::transaction::utxo::Coin; use crate::models::state::tx_proving_capability::TxProvingCapability; use crate::models::state::wallet::expected_utxo::ExpectedUtxo; use crate::models::state::wallet::transaction_output::TxOutput; use crate::models::state::wallet::utxo_notification::UtxoNotificationMedium; + use crate::models::state::GlobalStateLock; use crate::tests::shared::invalid_block_with_transaction; use crate::tests::shared::make_mock_block; use crate::tests::shared::make_mock_block_with_nonce_preimage_and_guesser_fraction; @@ -1453,10 +1452,10 @@ mod tests { // there, as it is timelocked. let one_coin = NeptuneCoins::new(1); assert!(alice_ws_genesis - .synced_unspent_available_amount(launch_timestamp) + .synced_unspent_liquid_amount(launch_timestamp) .is_zero()); assert!(!alice_ws_genesis - .synced_unspent_available_amount(released_timestamp) + .synced_unspent_liquid_amount(released_timestamp) .is_zero()); assert!( alice @@ -1510,11 +1509,16 @@ mod tests { ); } - #[tokio::test] - #[traced_test] - async fn test_update_wallet_state_repeated_addition_records() { + /// Test-setup. + /// + /// Generate a new wallet and state for Bob, who proceeds to mine one block. + /// Bob updates his wallet state with this block and as a result has a + /// nonzero balance. + /// + /// Note that this function is probabilistic. Block is invalid, both wrt. + /// PoW and proof. + async fn bob_mines_one_block(network: Network) -> (Block, GlobalStateLock) { let mut rng = thread_rng(); - let network = Network::Main; let cli = cli_args::Args::default(); let bob_wallet_secret = WalletSecret::new_random(); @@ -1522,11 +1526,6 @@ mod tests { let mut bob_global_lock = mock_genesis_global_state(network, 0, bob_wallet_secret, cli.clone()).await; - let alice_wallet_secret = WalletSecret::new_random(); - let alice_key = alice_wallet_secret.nth_generation_spending_key_for_tests(0); - let mut alice_global_lock = - mock_genesis_global_state(network, 0, alice_wallet_secret, cli).await; - // `bob` both composes and guesses the PoW solution of this block. let (block1, composer_fee_eutxos) = make_mock_block(&Block::genesis_block(network), None, bob_key, rng.gen()).await; @@ -1537,7 +1536,29 @@ mod tests { .set_new_self_mined_tip(block1.clone(), composer_fee_eutxos) .await .unwrap(); - alice_global_lock + + (block1, bob_global_lock) + } + + #[tokio::test] + #[traced_test] + async fn test_update_wallet_state_repeated_addition_records() { + let network = Network::Main; + let cli = cli_args::Args::default(); + + let alice_wallet_secret = WalletSecret::new_random(); + let alice_key = alice_wallet_secret.nth_generation_spending_key_for_tests(0); + let mut alice = mock_genesis_global_state(network, 0, alice_wallet_secret, cli).await; + + let (block1, mut bob) = bob_mines_one_block(network).await; + let bob_key = bob + .lock_guard() + .await + .wallet_state + .wallet_secret + .nth_generation_spending_key_for_tests(0); + + alice .lock_guard_mut() .await .set_new_tip(block1.clone()) @@ -1548,12 +1569,12 @@ mod tests { let fee = NeptuneCoins::new(1); let txoutput = TxOutput::onchain_native_currency( NeptuneCoins::new(7), - rng.gen(), + random(), alice_key.to_address().into(), false, ); let tx_outputs = vec![txoutput.clone(), txoutput.clone()]; - let (tx_block2, _) = bob_global_lock + let (tx_block2, _) = bob .lock_guard_mut() .await .create_transaction_with_prover_capability( @@ -1570,13 +1591,12 @@ mod tests { // Make block 2, verify that Alice registers correct balance. let block2 = invalid_block_with_transaction(&block1, tx_block2.clone()); - bob_global_lock - .lock_guard_mut() + bob.lock_guard_mut() .await .set_new_tip(block2.clone()) .await .unwrap(); - alice_global_lock + alice .lock_guard_mut() .await .set_new_tip(block2.clone()) @@ -1584,7 +1604,7 @@ mod tests { .unwrap(); assert_eq!( NeptuneCoins::new(14), - alice_global_lock + alice .lock_guard() .await .wallet_state @@ -1595,7 +1615,7 @@ mod tests { // Repeat the outputs to Alice in block 3 and verify correct new // balance. - let (tx_block3, _) = bob_global_lock + let (tx_block3, _) = bob .lock_guard_mut() .await .create_transaction_with_prover_capability( @@ -1610,7 +1630,7 @@ mod tests { .await .unwrap(); let block3 = invalid_block_with_transaction(&block2, tx_block3.clone()); - alice_global_lock + alice .lock_guard_mut() .await .set_new_tip(block3.clone()) @@ -1618,7 +1638,7 @@ mod tests { .unwrap(); assert_eq!( NeptuneCoins::new(28), - alice_global_lock + alice .lock_guard() .await .wallet_state @@ -1628,6 +1648,108 @@ mod tests { ); } + #[tokio::test] + #[traced_test] + async fn test_unrecognized_type_script() { + let network = Network::Main; + let cli = cli_args::Args::default(); + + let alice_wallet_secret = WalletSecret::new_random(); + let alice_key = alice_wallet_secret.nth_generation_spending_key_for_tests(0); + let mut alice = mock_genesis_global_state(network, 0, alice_wallet_secret, cli).await; + + let (block1, mut bob) = bob_mines_one_block(network).await; + let bob_key = bob + .lock_guard() + .await + .wallet_state + .wallet_secret + .nth_generation_spending_key_for_tests(0); + + alice + .lock_guard_mut() + .await + .set_new_tip(block1.clone()) + .await + .unwrap(); + + let unrecognized_typescript = Coin { + type_script_hash: random(), + state: vec![random(), random()], + }; + let txo = TxOutput::offchain_native_currency( + NeptuneCoins::new(3), + random(), + alice_key.to_address().into(), + false, + ); + let fee = NeptuneCoins::new(10); + let (mut tx_block2, _) = bob + .lock_guard_mut() + .await + .create_transaction_with_prover_capability( + vec![txo.clone()].into(), + bob_key.into(), + UtxoNotificationMedium::OnChain, + fee, + network.launch_date() + Timestamp::minutes(11), + TxProvingCapability::PrimitiveWitness, + &TritonVmJobQueue::dummy(), + ) + .await + .unwrap(); + let bad_txo = txo.clone().with_coin(unrecognized_typescript); + let expected_bad_utxos = alice + .lock_guard() + .await + .wallet_state + .extract_expected_utxos(vec![bad_txo.clone()].into(), UtxoNotifier::Cli); + alice + .lock_guard_mut() + .await + .wallet_state + .add_expected_utxos(expected_bad_utxos) + .await; + let bad_addition_record = commit( + Tip5::hash(&bad_txo.utxo()), + txo.sender_randomness(), + txo.receiver_digest(), + ); + let bad_kernel = TransactionKernelModifier::default() + .outputs(vec![bad_addition_record]) + .modify(tx_block2.kernel.clone()); + tx_block2.kernel = bad_kernel; + let block2 = invalid_block_with_transaction(&block1, tx_block2.clone()); + alice + .lock_guard_mut() + .await + .set_new_tip(block2.clone()) + .await + .unwrap(); + assert!( + alice + .lock_guard() + .await + .wallet_state + .confirmed_balance(block2.hash(), tx_block2.kernel.timestamp) + .await + .is_zero(), + "UTXO with unknown typescript may not count towards balance" + ); + assert!( + alice + .lock_guard() + .await + .wallet_state + .wallet_db + .monitored_utxos() + .len() + .await + .is_zero(), + "UTXO with unknown typescript may not added to MUTXO list" + ); + } + #[tokio::test] #[traced_test] async fn never_store_same_utxo_twice_different_blocks() { diff --git a/src/models/state/wallet/wallet_status.rs b/src/models/state/wallet/wallet_status.rs index 288e2a096..b7f9ac269 100644 --- a/src/models/state/wallet/wallet_status.rs +++ b/src/models/state/wallet/wallet_status.rs @@ -40,7 +40,7 @@ pub struct WalletStatus { } impl WalletStatus { - pub fn synced_unspent_available_amount(&self, timestamp: Timestamp) -> NeptuneCoins { + pub fn synced_unspent_liquid_amount(&self, timestamp: Timestamp) -> NeptuneCoins { self.synced_unspent .iter() .map(|(wse, _msmp)| &wse.utxo) @@ -56,18 +56,24 @@ impl WalletStatus { .map(|utxo| utxo.get_native_currency_amount()) .sum::() } + + /// Sum of value of monitored unsynced, unspent UTXOs. Does not check for + /// spendability, as that can only be determined once the monitored UTXO + /// is synced. pub fn unsynced_unspent_amount(&self) -> NeptuneCoins { self.unsynced_unspent .iter() .map(|wse| wse.utxo.get_native_currency_amount()) .sum::() } + pub fn synced_spent_amount(&self) -> NeptuneCoins { self.synced_spent .iter() .map(|wse| wse.utxo.get_native_currency_amount()) .sum::() } + pub fn unsynced_spent_amount(&self) -> NeptuneCoins { self.unsynced_spent .iter() @@ -87,7 +93,7 @@ impl Display for WalletStatus { let synced_unspent_available: String = format!( "synced, unspent available UTXOS: count: {}, amount: {:?}\n[{}]", synced_unspent_available_count, - self.synced_unspent_available_amount(now), + self.synced_unspent_liquid_amount(now), self.synced_unspent .iter() .filter(|(wse, _mnmp)| wse.utxo.can_spend_at(now)) diff --git a/src/rpc_server.rs b/src/rpc_server.rs index 278ac9e04..3fa535927 100644 --- a/src/rpc_server.rs +++ b/src/rpc_server.rs @@ -976,7 +976,7 @@ impl RPC for NeptuneRPCServer { .await .get_wallet_status_for_tip() .await; - amount <= wallet_status.synced_unspent_available_amount(now) + amount <= wallet_status.synced_unspent_liquid_amount(now) } // documented in trait. do not add doc-comment. @@ -990,7 +990,7 @@ impl RPC for NeptuneRPCServer { .await .get_wallet_status_for_tip() .await; - wallet_status.synced_unspent_available_amount(now) + wallet_status.synced_unspent_liquid_amount(now) } // documented in trait. do not add doc-comment. @@ -1186,7 +1186,7 @@ impl RPC for NeptuneRPCServer { let available_balance = { log_slow_scope!(fn_name!() + "::synced_unspent_available_amount()"); - wallet_status.synced_unspent_available_amount(now) + wallet_status.synced_unspent_liquid_amount(now) }; let timelocked_balance = { log_slow_scope!(fn_name!() + "::synced_unspent_timelocked_amount()");