diff --git a/Cargo.lock b/Cargo.lock index 2cf0ecbf3..5b8aadd83 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -855,7 +855,7 @@ dependencies = [ [[package]] name = "incrementalmerkletree" version = "0.3.0-beta.1" -source = "git+https://github.com/nuttycom/incrementalmerkletree?rev=027a98e53ad59699662d258ac22cd2095ae02aea#027a98e53ad59699662d258ac22cd2095ae02aea" +source = "git+https://github.com/nuttycom/incrementalmerkletree?rev=f19cf9c381d70547d170216f844b62c86befb6c3#f19cf9c381d70547d170216f844b62c86befb6c3" dependencies = [ "serde", ] diff --git a/Cargo.toml b/Cargo.toml index 0e0b3bf60..a0e6bbcb1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -87,7 +87,7 @@ codegen-units = 1 [patch.crates-io] hdwallet = { git = "https://github.com/nuttycom/hdwallet", rev = "576683b9f2865f1118c309017ff36e01f84420c9" } -incrementalmerkletree = { git = "https://github.com/nuttycom/incrementalmerkletree", rev = "027a98e53ad59699662d258ac22cd2095ae02aea" } +incrementalmerkletree = { git = "https://github.com/nuttycom/incrementalmerkletree", rev = "f19cf9c381d70547d170216f844b62c86befb6c3" } zcash_address = { git = "https://github.com/zcash/librustzcash.git", rev = "9c1ed86c5aa8ae3b6d6dcc1478f2d6ba1264488f" } zcash_encoding = { git = "https://github.com/zcash/librustzcash.git", rev = "9c1ed86c5aa8ae3b6d6dcc1478f2d6ba1264488f" } zcash_history = { git = "https://github.com/zcash/librustzcash.git", rev = "9c1ed86c5aa8ae3b6d6dcc1478f2d6ba1264488f" } diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index 1260f148a..c51c27567 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -76,6 +76,7 @@ BASE_SCRIPTS= [ 'wallet_import_export.py', 'wallet_isfromme.py', 'wallet_orchard_change.py', + 'wallet_orchard_init.py', 'wallet_orchard_persistence.py', 'wallet_nullifiers.py', 'wallet_sapling.py', diff --git a/qa/rpc-tests/wallet_orchard_init.py b/qa/rpc-tests/wallet_orchard_init.py new file mode 100755 index 000000000..b5db9f4e1 --- /dev/null +++ b/qa/rpc-tests/wallet_orchard_init.py @@ -0,0 +1,162 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or https://www.opensource.org/licenses/mit-license.php . + +import os +import os.path + +from decimal import Decimal + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + NU5_BRANCH_ID, + assert_equal, + get_coinbase_address, + nuparams, + start_nodes, + stop_nodes, + wait_bitcoinds, + wait_and_assert_operationid_status, +) + +# Test wallet behaviour with the Orchard protocol +class OrchardWalletInitTest(BitcoinTestFramework): + def __init__(self): + super().__init__() + self.num_nodes = 4 + + def setup_nodes(self): + return start_nodes(self.num_nodes, self.options.tmpdir, [[ + nuparams(NU5_BRANCH_ID, 205), + '-regtestwalletsetbestchaineveryblock' + ]] * self.num_nodes) + + def run_test(self): + # Sanity-check the test harness + assert_equal(self.nodes[0].getblockcount(), 200) + + # Get a new Orchard account on node 0 + acct0 = self.nodes[0].z_getnewaccount()['account'] + ua0 = self.nodes[0].z_getaddressforaccount(acct0, ['orchard'])['address'] + + # Activate NU5 + self.nodes[0].generate(5) + self.sync_all() + + # Get a recipient address + acct1 = self.nodes[1].z_getnewaccount()['account'] + ua1 = self.nodes[1].z_getaddressforaccount(acct1, ['orchard'])['address'] + + # Send a transaction to node 1 so that it has an Orchard note to spend. + recipients = [{"address": ua1, "amount": 10}] + myopid = self.nodes[0].z_sendmany(get_coinbase_address(self.nodes[0]), recipients, 1, 0, 'AllowRevealedSenders') + wait_and_assert_operationid_status(self.nodes[0], myopid) + + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + + # Check the value sent to ua1 was received + assert_equal( + {'pools': {'orchard': {'valueZat': 10_0000_0000}}, 'minimum_confirmations': 1}, + self.nodes[1].z_getbalanceforaccount(acct1)) + + # Create an Orchard spend, so that the note commitment tree root gets altered. + recipients = [{"address": ua0, "amount": 1}] + myopid = self.nodes[1].z_sendmany(ua1, recipients, 1, 0) + wait_and_assert_operationid_status(self.nodes[1], myopid) + + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + + # Verify the balance on both nodes + assert_equal( + {'pools': {'orchard': {'valueZat': 9_0000_0000}}, 'minimum_confirmations': 1}, + self.nodes[1].z_getbalanceforaccount(acct1)) + + assert_equal( + {'pools': {'orchard': {'valueZat': 1_0000_0000}}, 'minimum_confirmations': 1}, + self.nodes[0].z_getbalanceforaccount(acct0)) + + # Split the network. We'll advance the state of nodes 0/1 so that after + # we re-join the network, we need to roll back more than one block after + # the wallet is restored, into a chain state that the wallet never observed. + self.split_network() + + self.sync_all() + self.nodes[0].generate(2) + self.sync_all() + + # Shut down the network and delete node 0's wallet + stop_nodes(self.nodes) + wait_bitcoinds() + + tmpdir = self.options.tmpdir + os.remove(os.path.join(tmpdir, "node0", "regtest", "wallet.dat")) + + # Restart the network, still split; the split here is note necessary to reproduce + # the error but it is sufficient, and we will later use the split to check the + # corresponding rewind. + self.setup_network(True) + + assert_equal( + {'pools': {'orchard': {'valueZat': 9_0000_0000}}, 'minimum_confirmations': 1}, + self.nodes[1].z_getbalanceforaccount(acct1)) + + # Get a new account with an Orchard UA on node 0 + acct0new = self.nodes[0].z_getnewaccount()['account'] + ua0new = self.nodes[0].z_getaddressforaccount(acct0, ['orchard'])['address'] + + # Send a transaction to the Orchard account. When we mine this transaction, + # the bug causes the state of note commitment tree in the wallet to not match + # the state of the global note commitment tree. + recipients = [{"address": ua0new, "amount": 1}] + myopid = self.nodes[1].z_sendmany(ua1, recipients, 1, 0) + rollback_tx = wait_and_assert_operationid_status(self.nodes[1], myopid) + + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + + assert_equal( + {'pools': {'orchard': {'valueZat': 1_0000_0000}}, 'minimum_confirmations': 1}, + self.nodes[0].z_getbalanceforaccount(acct0new)) + + # Node 2 has no progress since the network was first split, so this should roll + # everything back to the original fork point. + self.nodes[2].generate(10) + + # Re-join the network + self.join_network() + + assert_equal(set([rollback_tx]), set(self.nodes[1].getrawmempool())) + + # Resend un-mined transactions and sync the network + self.nodes[1].resendwallettransactions() + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + + assert_equal( + {'pools': {'orchard': {'valueZat': 1_0000_0000}}, 'minimum_confirmations': 1}, + self.nodes[0].z_getbalanceforaccount(acct0new)) + + # Spend from the note that was just received + recipients = [{"address": ua1, "amount": Decimal('0.3')}] + myopid = self.nodes[0].z_sendmany(ua0new, recipients, 1, 0) + wait_and_assert_operationid_status(self.nodes[0], myopid) + + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + + assert_equal( + {'pools': {'orchard': {'valueZat': 8_3000_0000}}, 'minimum_confirmations': 1}, + self.nodes[1].z_getbalanceforaccount(acct1)) + + +if __name__ == '__main__': + OrchardWalletInitTest().main() + diff --git a/src/rust/include/rust/orchard/wallet.h b/src/rust/include/rust/orchard/wallet.h index a65962878..dae875dab 100644 --- a/src/rust/include/rust/orchard/wallet.h +++ b/src/rust/include/rust/orchard/wallet.h @@ -5,6 +5,7 @@ #ifndef ZCASH_RUST_INCLUDE_RUST_ORCHARD_WALLET_H #define ZCASH_RUST_INCLUDE_RUST_ORCHARD_WALLET_H +#include "rust/orchard/incremental_merkle_tree.h" #include "rust/orchard/keys.h" #include "rust/builder.h" @@ -65,19 +66,27 @@ bool orchard_wallet_get_last_checkpoint( * * The `blockHeight` argument provides the height to which the witness tree should be * rewound, such that after the rewind this height corresponds to the latest block - * appended to the tree. The number of blocks that were removed from the witness - * tree in the rewind process is returned via `blocksRewoundRet`. + * appended to the tree. * - * Returns `true` if the rewind is successful, in which case the number of blocks that were - * removed from the witness tree in the rewind process is returned via `blocksRewoundRet`; - * this returns `false` and leaves `blocksRewoundRet` unmodified. + * Returns `true` if the rewind is successful, in which case `resultHeight` will contain + * the height to which the tree has been rewound; otherwise, this returns `false` and + * leaves `resultHeight` unmodified. */ bool orchard_wallet_rewind( OrchardWalletPtr* wallet, uint32_t blockHeight, - uint32_t* blocksRewoundRet + uint32_t* resultHeight ); +/** + * Initialize the wallet's note commitment tree to the empty tree starting from the + * specified Merkle frontier. This will return `false` and leave the wallet unmodified if + * it would cause any checkpoint or witness state to be invalidated. + */ +bool orchard_wallet_init_from_frontier( + OrchardWalletPtr* wallet, + const OrchardMerkleFrontierPtr* frontier); + /** * A C struct used to transfer action_idx/IVK pairs back from Rust across the FFI * boundary. This must have the same in-memory representation as the `FFIActionIVK` type diff --git a/src/rust/src/wallet.rs b/src/rust/src/wallet.rs index 765390e53..526d07327 100644 --- a/src/rust/src/wallet.rs +++ b/src/rust/src/wallet.rs @@ -1,5 +1,5 @@ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; -use incrementalmerkletree::{bridgetree::BridgeTree, Frontier, Position, Tree}; +use incrementalmerkletree::{bridgetree, bridgetree::BridgeTree, Frontier, Position, Tree}; use libc::c_uchar; use std::collections::{BTreeMap, BTreeSet}; use std::convert::TryInto; @@ -42,14 +42,14 @@ pub struct LastObserved { } /// A pointer to a particular action in an Orchard transaction output. -#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct OutPoint { txid: TxId, action_idx: usize, } /// A pointer to a previous output being spent in an Orchard action. -#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct InPoint { txid: TxId, action_idx: usize, @@ -195,6 +195,14 @@ pub enum BundleLoadError { InvalidActionIndex(usize), } +#[derive(Debug, Clone)] +pub enum SpendRetrievalError { + DecryptedNoteNotFound(OutPoint), + NoIvkForRecipient(Address), + FvkNotFound(IncomingViewingKey), + NoteNotPositioned(OutPoint), +} + /// A struct used to return metadata about how a bundle was determined /// to be involved with the wallet. #[derive(Debug, Clone)] @@ -266,47 +274,45 @@ impl Wallet { self.last_checkpoint } - /// Rewinds the note commitment tree to the given height, removes notes and - /// spentness information for transactions mined in the removed blocks, and returns - /// the number of blocks by which the tree has been rewound if successful. Returns - /// `RewindError` if not enough checkpoints exist to execute the full rewind - /// requested. If the requested height is greater than or equal to the height of the - /// latest checkpoint, this returns `Ok(0)` and leaves the wallet unmodified. - pub fn rewind(&mut self, to_height: BlockHeight) -> Result { + /// Rewinds the note commitment tree to the given height, removes notes and spentness + /// information for transactions mined in the removed blocks, and returns the height to which + /// the tree has been rewound if successful. Returns `RewindError` if not enough checkpoints + /// exist to execute the full rewind requested and the wallet has witness information that + /// would be invalidated by the rewind. If the requested height is greater than or equal to the + /// height of the latest checkpoint, this returns a successful result containing the height of + /// the last checkpoint. + /// + /// In the case that no checkpoints exist but the note commitment tree also records no witness + /// information, we allow the wallet to continue to rewind, under the assumption that the state + /// of the note commitment tree will be overwritten prior to the next append. + pub fn rewind(&mut self, to_height: BlockHeight) -> Result { if let Some(checkpoint_height) = self.last_checkpoint { if to_height >= checkpoint_height { - return Ok(0); + return Ok(checkpoint_height); } let blocks_to_rewind = ::from(checkpoint_height) - ::from(to_height); - - // if the rewind length exceeds the number of checkpoints we have stored, - // return failure. let checkpoint_count = self.witness_tree.checkpoints().len(); - if blocks_to_rewind as usize > checkpoint_count { - return Err(RewindError::InsufficientCheckpoints(checkpoint_count)); - } - for _ in 0..blocks_to_rewind { - // we've already checked that we have enough checkpoints to fully - // rewind by the requested amount. - assert!(self.witness_tree.rewind()); + // If the rewind fails, we have no more checkpoints. This is fine in the + // case that we have a recently-initialized tree, so long as we have no + // witnessed indices. In the case that we have any witnessed notes, we + // have hit the maximum rewind limit, and this is an error. + if !self.witness_tree.rewind() { + assert!(self.witness_tree.checkpoints().is_empty()); + if !self.witness_tree.witnessed_indices().is_empty() { + return Err(RewindError::InsufficientCheckpoints(checkpoint_count)); + } + } } - // reset our last observed height to ensure that notes added in the future are - // from a new block - let last_observed = LastObserved { - block_height: checkpoint_height - blocks_to_rewind, - block_tx_idx: None, - }; - // retain notes that correspond to transactions that are not "un-mined" after // the rewind let to_retain: BTreeSet<_> = self .wallet_note_positions .iter() .filter_map(|(txid, n)| { - if n.tx_height <= last_observed.block_height { + if n.tx_height <= to_height { Some(*txid) } else { None @@ -315,21 +321,39 @@ impl Wallet { .collect(); self.mined_notes.retain(|_, v| to_retain.contains(&v.txid)); + // nullifier and received note data are retained, because these values are stable // once we've observed a note for the first time. The block height at which we // observed the note is removed along with the note positions, because the // transaction will no longer have been observed as having been mined. self.wallet_note_positions .retain(|txid, _| to_retain.contains(txid)); - self.last_observed = Some(last_observed); + + // reset our last observed height to ensure that notes added in the future are + // from a new block + self.last_observed = Some(LastObserved { + block_height: to_height, + block_tx_idx: None, + }); + self.last_checkpoint = if checkpoint_count > blocks_to_rewind as usize { - Some(checkpoint_height - blocks_to_rewind) + Some(to_height) } else { - // checkpoint_count == blocks_to_rewind + // checkpoint_count <= blocks_to_rewind None }; - Ok(blocks_to_rewind) + Ok(to_height) + } else if self.witness_tree.witnessed_indices().is_empty() { + // If we have no witnessed notes, it's okay to keep "rewinding" even though + // we have no checkpoints. We then allow last_observed to assume the height + // to which we have reset the tree state. + self.last_observed = Some(LastObserved { + block_height: to_height, + block_tx_idx: None, + }); + + Ok(to_height) } else { Err(RewindError::InsufficientCheckpoints(0)) } @@ -636,42 +660,55 @@ impl Wallet { /// /// Returns `None` if the `OutPoint` is not known to the wallet, or the Orchard bundle /// containing the note has not been passed to `Wallet::append_bundle_commitments`. - pub fn get_spend_info(&self, outpoint: OutPoint) -> Option { + pub fn get_spend_info( + &self, + outpoint: OutPoint, + ) -> Result { // TODO: Take `confirmations` parameter and obtain the Merkle path to the root at // that checkpoint, not the latest root. - self.wallet_received_notes + let dnote = self + .wallet_received_notes .get(&outpoint.txid) .and_then(|tx_notes| tx_notes.decrypted_notes.get(&outpoint.action_idx)) - .and_then(|dnote| { + .ok_or(SpendRetrievalError::DecryptedNoteNotFound(outpoint))?; + + let fvk = self + .key_store + .ivk_for_address(&dnote.note.recipient()) + .ok_or_else(|| SpendRetrievalError::NoIvkForRecipient(dnote.note.recipient())) + .and_then(|ivk| { self.key_store - .ivk_for_address(&dnote.note.recipient()) - .and_then(|ivk| self.key_store.viewing_keys.get(ivk)) - .zip( - self.wallet_note_positions - .get(&outpoint.txid) - .and_then(|tx_notes| tx_notes.note_positions.get(&outpoint.action_idx)), - ) - .map(|(fvk, position)| { - assert_eq!( - self.witness_tree - .get_witnessed_leaf(*position) - .expect("tree has witnessed the leaf for this note."), - &MerkleHashOrchard::from_cmx(&dnote.note.commitment().into()), - ); - let path = self - .witness_tree - .authentication_path(*position) - .expect("wallet always has paths to positioned notes"); - OrchardSpendInfo::from_parts( - fvk.clone(), - dnote.note, - MerklePath::from_parts( - u64::from(*position).try_into().unwrap(), - path.try_into().unwrap(), - ), - ) - }) - }) + .viewing_keys + .get(ivk) + .ok_or_else(|| SpendRetrievalError::FvkNotFound(ivk.clone())) + })?; + + let position = self + .wallet_note_positions + .get(&outpoint.txid) + .and_then(|tx_notes| tx_notes.note_positions.get(&outpoint.action_idx)) + .ok_or(SpendRetrievalError::NoteNotPositioned(outpoint))?; + + assert_eq!( + self.witness_tree + .get_witnessed_leaf(*position) + .expect("tree has witnessed the leaf for this note."), + &MerkleHashOrchard::from_cmx(&dnote.note.commitment().into()), + ); + + let path = self + .witness_tree + .authentication_path(*position) + .expect("wallet always has paths to positioned notes"); + + Ok(OrchardSpendInfo::from_parts( + fvk.clone(), + dnote.note, + MerklePath::from_parts( + u64::from(*position).try_into().unwrap(), + path.try_into().unwrap(), + ), + )) } } @@ -728,14 +765,14 @@ pub extern "C" fn orchard_wallet_get_last_checkpoint( pub extern "C" fn orchard_wallet_rewind( wallet: *mut Wallet, to_height: BlockHeight, - blocks_rewound: *mut u32, + result_height: *mut BlockHeight, ) -> bool { let wallet = unsafe { wallet.as_mut() }.expect("Wallet pointer may not be null"); - let blocks_rewound = - unsafe { blocks_rewound.as_mut() }.expect("Return value pointer may not be null."); + let result_height = + unsafe { result_height.as_mut() }.expect("Return value pointer may not be null."); match wallet.rewind(to_height) { - Ok(rewound) => { - *blocks_rewound = rewound; + Ok(result) => { + *result_height = result; true } Err(e) => { @@ -1142,15 +1179,16 @@ pub extern "C" fn orchard_wallet_get_spend_info( let outpoint = OutPoint { txid, action_idx }; - if let Some(ret) = wallet.get_spend_info(outpoint) { - Box::into_raw(Box::new(ret)) - } else { - tracing::error!( - "Requested note in action {} of transaction {} wasn't in the wallet", - outpoint.action_idx, - outpoint.txid - ); - ptr::null_mut() + match wallet.get_spend_info(outpoint) { + Ok(ret) => Box::into_raw(Box::new(ret)), + Err(e) => { + tracing::error!( + "Error obtaining spend info for outpoint {:?}: {:?}", + outpoint, + e + ); + ptr::null_mut() + } } } @@ -1273,3 +1311,32 @@ pub extern "C" fn orchard_wallet_load_note_commitment_tree( } } } + +#[no_mangle] +pub extern "C" fn orchard_wallet_init_from_frontier( + wallet: *mut Wallet, + frontier: *const bridgetree::Frontier, +) -> bool { + let wallet = unsafe { wallet.as_mut() }.expect("Wallet pointer may not be null."); + let frontier = unsafe { frontier.as_ref() }.expect("Wallet pointer may not be null."); + + if wallet.witness_tree.checkpoints().is_empty() + && wallet.witness_tree.witnessed_indices().is_empty() + { + wallet.witness_tree = frontier.value().map_or_else( + || BridgeTree::new(MAX_CHECKPOINTS), + |nonempty_frontier| { + BridgeTree::from_frontier(MAX_CHECKPOINTS, nonempty_frontier.clone()) + }, + ); + true + } else { + // if we have any checkpoints in the tree, or if we have any witnessed notes, + // don't allow reinitialization + error!( + "Invalid attempt to reinitialize note commitment tree: {} checkpoints present.", + wallet.witness_tree.checkpoints().len() + ); + false + } +} diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 0668626c4..9fd8e4b10 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -70,12 +70,12 @@ void SyncWithWallets(const CTransaction &tx, const CBlock *pblock, const int nHe struct CachedBlockData { CBlockIndex *pindex; - std::pair oldTrees; + MerkleFrontiers oldTrees; std::list txConflicted; CachedBlockData( CBlockIndex *pindex, - std::pair oldTrees, + MerkleFrontiers oldTrees, std::list txConflicted): pindex(pindex), oldTrees(oldTrees), txConflicted(txConflicted) {} }; @@ -160,27 +160,40 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip) // Iterate backwards over the connected blocks we need to notify. while (pindex && pindex != pindexFork) { + MerkleFrontiers oldFrontiers; // Get the Sprout commitment tree as of the start of this block. - SproutMerkleTree oldSproutTree; - assert(pcoinsTip->GetSproutAnchorAt(pindex->hashSproutAnchor, oldSproutTree)); + assert(pcoinsTip->GetSproutAnchorAt(pindex->hashSproutAnchor, oldFrontiers.sprout)); // Get the Sapling commitment tree as of the start of this block. // We can get this from the `hashFinalSaplingRoot` of the last block // However, this is only reliable if the last block was on or after // the Sapling activation height. Otherwise, the last anchor was the // empty root. - SaplingMerkleTree oldSaplingTree; if (chainParams.GetConsensus().NetworkUpgradeActive( pindex->pprev->nHeight, Consensus::UPGRADE_SAPLING)) { assert(pcoinsTip->GetSaplingAnchorAt( - pindex->pprev->hashFinalSaplingRoot, oldSaplingTree)); + pindex->pprev->hashFinalSaplingRoot, oldFrontiers.sapling)); } else { - assert(pcoinsTip->GetSaplingAnchorAt(SaplingMerkleTree::empty_root(), oldSaplingTree)); + assert(pcoinsTip->GetSaplingAnchorAt(SaplingMerkleTree::empty_root(), oldFrontiers.sapling)); + } + + // Get the Orchard Merkle frontier as of the start of this block. + // We can get this from the `hashFinalOrchardRoot` of the last block + // However, this is only reliable if the last block was on or after + // the Orchard activation height. Otherwise, the last anchor was the + // empty root. + if (chainParams.GetConsensus().NetworkUpgradeActive( + pindex->pprev->nHeight, Consensus::UPGRADE_NU5)) { + assert(pcoinsTip->GetOrchardAnchorAt( + pindex->pprev->hashFinalOrchardRoot, oldFrontiers.orchard)); + } else { + assert(pcoinsTip->GetOrchardAnchorAt( + OrchardMerkleFrontier::empty_root(), oldFrontiers.orchard)); } blockStack.emplace_back( pindex, - std::make_pair(oldSproutTree, oldSaplingTree), + oldFrontiers, recentlyConflicted.first.at(pindex)); pindex = pindex->pprev; diff --git a/src/validationinterface.h b/src/validationinterface.h index 4dce6df3c..ca5757c97 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -23,6 +23,12 @@ class CValidationInterface; class CValidationState; class uint256; +struct MerkleFrontiers { + SproutMerkleTree sprout; + SaplingMerkleTree sapling; + OrchardMerkleFrontier orchard; +}; + // These functions dispatch to one or all registered wallets /** Register a wallet to receive updates from core */ @@ -37,7 +43,7 @@ protected: virtual void UpdatedBlockTip(const CBlockIndex *pindex) {} virtual void SyncTransaction(const CTransaction &tx, const CBlock *pblock, const int nHeight) {} virtual void EraseFromWallet(const uint256 &hash) {} - virtual void ChainTip(const CBlockIndex *pindex, const CBlock *pblock, std::optional> added) {} + virtual void ChainTip(const CBlockIndex *pindex, const CBlock *pblock, std::optional added) {} virtual void UpdatedTransaction(const uint256 &hash) {} virtual void Inventory(const uint256 &hash) {} virtual void ResendWalletTransactions(int64_t nBestBlockTime) {} @@ -59,7 +65,7 @@ struct CMainSignals { /** Notifies listeners of an updated transaction without new data (for now: a coinbase potentially becoming visible). */ boost::signals2::signal UpdatedTransaction; /** Notifies listeners of a change to the tip of the active block chain. */ - boost::signals2::signal>)> ChainTip; + boost::signals2::signal)> ChainTip; /** Notifies listeners about an inventory item being seen on the network. */ boost::signals2::signal Inventory; /** Tells listeners to broadcast their data. */ diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 87c8dee7c..0a6fbb6a7 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -60,11 +60,10 @@ public: void IncrementNoteWitnesses(const Consensus::Params& consensus, const CBlockIndex* pindex, const CBlock* pblock, - SproutMerkleTree& sproutTree, - SaplingMerkleTree& saplingTree, + MerkleFrontiers& frontiers, bool performOrchardWalletUpdates) { CWallet::IncrementNoteWitnesses( - consensus, pindex, pblock, sproutTree, saplingTree, performOrchardWalletUpdates); + consensus, pindex, pblock, frontiers, performOrchardWalletUpdates); } @@ -96,8 +95,7 @@ std::pair CreateValidBlock(TestWallet& wallet, const libzcash::SproutSpendingKey& sk, const CBlockIndex& index, CBlock& block, - SproutMerkleTree& sproutTree, - SaplingMerkleTree& saplingTree) { + MerkleFrontiers& frontiers) { auto wtx = GetValidSproutReceive(sk, 50, true); auto note = GetSproutNote(sk, wtx, 0, 1); auto nullifier = note.nullifier(sk); @@ -111,7 +109,7 @@ std::pair CreateValidBlock(TestWallet& wallet, wallet.LoadWalletTx(wtx); block.vtx.push_back(wtx); - wallet.IncrementNoteWitnesses(Params().GetConsensus(), &index, &block, sproutTree, saplingTree, true); + wallet.IncrementNoteWitnesses(Params().GetConsensus(), &index, &block, frontiers, true); return std::make_pair(jsoutpt, saplingNotes[0]); } @@ -705,10 +703,11 @@ TEST(WalletTests, GetConflictedSaplingNotes) { // Generate note A libzcash::SaplingNote note(pk, 50000, zip_212_enabled[ver]); auto cm = note.cmu().value(); - SaplingMerkleTree saplingTree; - saplingTree.append(cm); - auto anchor = saplingTree.root(); - auto witness = saplingTree.witness(); + + MerkleFrontiers frontiers; + frontiers.sapling.append(cm); + auto anchor = frontiers.sapling.root(); + auto witness = frontiers.sapling.witness(); // Generate tx to create output note B auto builder = TransactionBuilder(consensusParams, 1, std::nullopt); @@ -719,7 +718,6 @@ TEST(WalletTests, GetConflictedSaplingNotes) { // Fake-mine the transaction EXPECT_EQ(-1, chainActive.Height()); - SproutMerkleTree sproutTree; CBlock block; block.vtx.push_back(wtx); block.hashMerkleRoot = block.BuildMerkleTree(); @@ -738,7 +736,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { wallet.LoadWalletTx(wtx); // Simulate receiving new block and ChainTip signal - wallet.IncrementNoteWitnesses(Params().GetConsensus(),&fakeIndex, &block, sproutTree, saplingTree, true); + wallet.IncrementNoteWitnesses(Params().GetConsensus(),&fakeIndex, &block, frontiers, true); wallet.UpdateSaplingNullifierNoteMapForBlock(&block); // Retrieve the updated wtx from wallet @@ -764,7 +762,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { ASSERT_EQ(static_cast(maybe_nf), true); auto nullifier2 = maybe_nf.value(); - anchor = saplingTree.root(); + anchor = frontiers.sapling.root(); // Create transaction to spend note B auto builder2 = TransactionBuilder(consensusParams, 2, std::nullopt); @@ -844,8 +842,7 @@ TEST(WalletTests, GetConflictedOrchardNotes) { CWalletTx wtx {&wallet, tx}; // Fake-mine the transaction - SproutMerkleTree sproutTree; - SaplingMerkleTree saplingTree; + MerkleFrontiers frontiers; OrchardMerkleFrontier orchardTree; orchardTree.AppendBundle(wtx.GetOrchardBundle()); @@ -870,7 +867,7 @@ TEST(WalletTests, GetConflictedOrchardNotes) { wallet.LoadWalletTx(wtx); // Simulate receiving new block and ChainTip signal - wallet.IncrementNoteWitnesses(Params().GetConsensus(),&fakeIndex, &block, sproutTree, saplingTree, true); + wallet.IncrementNoteWitnesses(Params().GetConsensus(),&fakeIndex, &block, frontiers, true); // Fetch the Orchard note so we can spend it. std::vector sproutEntries; @@ -1103,12 +1100,13 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) { ASSERT_TRUE(nf); uint256 nullifier = nf.value(); + MerkleFrontiers frontiers = { .sapling = testNote.tree }; + // Verify dummy note is unspent EXPECT_FALSE(wallet.IsSaplingSpent(nullifier)); // Fake-mine the transaction EXPECT_EQ(-1, chainActive.Height()); - SproutMerkleTree sproutTree; CBlock block; block.vtx.push_back(wtx); block.hashMerkleRoot = block.BuildMerkleTree(); @@ -1138,7 +1136,7 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) { } // Simulate receiving new block and ChainTip signal - wallet.IncrementNoteWitnesses(Params().GetConsensus(), &fakeIndex, &block, sproutTree, testNote.tree, true); + wallet.IncrementNoteWitnesses(Params().GetConsensus(), &fakeIndex, &block, frontiers, true); wallet.UpdateSaplingNullifierNoteMapForBlock(&block); // Retrieve the updated wtx from wallet @@ -1221,10 +1219,10 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { // Generate Sapling note A libzcash::SaplingNote note(pk, 50000, zip_212_enabled[ver]); auto cm = note.cmu().value(); - SaplingMerkleTree saplingTree; - saplingTree.append(cm); - auto anchor = saplingTree.root(); - auto witness = saplingTree.witness(); + MerkleFrontiers frontiers; + frontiers.sapling.append(cm); + auto anchor = frontiers.sapling.root(); + auto witness = frontiers.sapling.witness(); // Generate transaction, which sends funds to note B auto builder = TransactionBuilder(consensusParams, 1, std::nullopt); @@ -1238,7 +1236,6 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { // Fake-mine the transaction EXPECT_EQ(-1, chainActive.Height()); - SproutMerkleTree sproutTree; CBlock block; block.vtx.push_back(wtx); block.hashMerkleRoot = block.BuildMerkleTree(); @@ -1258,7 +1255,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { // Simulate receiving new block and ChainTip signal. // This triggers calculation of nullifiers for notes belonging to this wallet // in the output descriptions of wtx. - wallet.IncrementNoteWitnesses(Params().GetConsensus(), &fakeIndex, &block, sproutTree, saplingTree, true); + wallet.IncrementNoteWitnesses(Params().GetConsensus(), &fakeIndex, &block, frontiers, true); wallet.UpdateSaplingNullifierNoteMapForBlock(&block); // Retrieve the updated wtx from wallet @@ -1296,7 +1293,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { // NOTE: Not updating the anchor results in a core dump. Shouldn't builder just return error? // *** Error in `./zcash-gtest': double free or corruption (out): 0x00007ffd8755d990 *** - anchor = saplingTree.root(); + anchor = frontiers.sapling.root(); // Create transaction to spend note B auto builder2 = TransactionBuilder(consensusParams, 2, std::nullopt); @@ -1393,9 +1390,8 @@ TEST(WalletTests, CachedWitnessesEmptyChain) { CBlock block; block.vtx.push_back(wtx); CBlockIndex index(block); - SproutMerkleTree sproutTree; - SaplingMerkleTree saplingTree; - wallet.IncrementNoteWitnesses(Params().GetConsensus(), &index, &block, sproutTree, saplingTree, true); + MerkleFrontiers frontiers; + wallet.IncrementNoteWitnesses(Params().GetConsensus(), &index, &block, frontiers, true); ::GetWitnessesAndAnchors(wallet, sproutNotes, saplingNotes, sproutWitnesses, saplingWitnesses); @@ -1415,8 +1411,7 @@ TEST(WalletTests, CachedWitnessesChainTip) { std::pair anchors1; CBlock block1; - SproutMerkleTree sproutTree; - SaplingMerkleTree saplingTree; + MerkleFrontiers frontiers; auto sk = libzcash::SproutSpendingKey::random(); wallet.AddSproutSpendingKey(sk); @@ -1425,7 +1420,7 @@ TEST(WalletTests, CachedWitnessesChainTip) { // First block (case tested in _empty_chain) CBlockIndex index1(block1); index1.nHeight = 1; - auto outpts = CreateValidBlock(wallet, sk, index1, block1, sproutTree, saplingTree); + auto outpts = CreateValidBlock(wallet, sk, index1, block1, frontiers); // Called to fetch anchor std::vector sproutNotes {outpts.first}; @@ -1466,9 +1461,8 @@ TEST(WalletTests, CachedWitnessesChainTip) { block2.vtx.push_back(wtx); CBlockIndex index2(block2); index2.nHeight = 2; - SproutMerkleTree sproutTree2 {sproutTree}; - SaplingMerkleTree saplingTree2 {saplingTree}; - wallet.IncrementNoteWitnesses(Params().GetConsensus(), &index2, &block2, sproutTree2, saplingTree2, true); + MerkleFrontiers frontiers2 = { .sprout = frontiers.sprout, .sapling = frontiers.sapling }; + wallet.IncrementNoteWitnesses(Params().GetConsensus(), &index2, &block2, frontiers2, true); auto anchors2 = GetWitnessesAndAnchors(wallet, sproutNotes, saplingNotes, sproutWitnesses, saplingWitnesses); EXPECT_NE(anchors2.first, anchors2.second); @@ -1489,7 +1483,7 @@ TEST(WalletTests, CachedWitnessesChainTip) { EXPECT_NE(anchors1.second, anchors3.second); // Re-incrementing with the same block should give the same result - wallet.IncrementNoteWitnesses(Params().GetConsensus(), &index2, &block2, sproutTree, saplingTree, true); + wallet.IncrementNoteWitnesses(Params().GetConsensus(), &index2, &block2, frontiers, true); auto anchors4 = GetWitnessesAndAnchors(wallet, sproutNotes, saplingNotes, sproutWitnesses, saplingWitnesses); EXPECT_NE(anchors4.first, anchors4.second); @@ -1499,7 +1493,7 @@ TEST(WalletTests, CachedWitnessesChainTip) { EXPECT_EQ(anchors2.second, anchors4.second); // Incrementing with the same block again should not change the cache - wallet.IncrementNoteWitnesses(Params().GetConsensus(), &index2, &block2, sproutTree, saplingTree, true); + wallet.IncrementNoteWitnesses(Params().GetConsensus(), &index2, &block2, frontiers, true); std::vector> sproutWitnesses5; std::vector> saplingWitnesses5; @@ -1518,8 +1512,7 @@ TEST(WalletTests, CachedWitnessesDecrementFirst) { TestWallet wallet(Params()); LOCK(wallet.cs_wallet); - SproutMerkleTree sproutTree; - SaplingMerkleTree saplingTree; + MerkleFrontiers frontiers; auto sk = libzcash::SproutSpendingKey::random(); wallet.AddSproutSpendingKey(sk); @@ -1529,7 +1522,7 @@ TEST(WalletTests, CachedWitnessesDecrementFirst) { CBlock block1; CBlockIndex index1(block1); index1.nHeight = 1; - CreateValidBlock(wallet, sk, index1, block1, sproutTree, saplingTree); + CreateValidBlock(wallet, sk, index1, block1, frontiers); } std::pair anchors2; @@ -1539,7 +1532,7 @@ TEST(WalletTests, CachedWitnessesDecrementFirst) { { // Second block (case tested in _chain_tip) index2.nHeight = 2; - auto outpts = CreateValidBlock(wallet, sk, index2, block2, sproutTree, saplingTree); + auto outpts = CreateValidBlock(wallet, sk, index2, block2, frontiers); // Called to fetch anchor std::vector sproutNotes {outpts.first}; @@ -1585,7 +1578,7 @@ TEST(WalletTests, CachedWitnessesDecrementFirst) { EXPECT_NE(anchors2.second, anchors4.second); // Re-incrementing with the same block should give the same result - wallet.IncrementNoteWitnesses(Params().GetConsensus(), &index2, &block2, sproutTree, saplingTree, true); + wallet.IncrementNoteWitnesses(Params().GetConsensus(), &index2, &block2, frontiers, true); auto anchors5 = GetWitnessesAndAnchors(wallet, sproutNotes, saplingNotes, sproutWitnesses, saplingWitnesses); @@ -1607,10 +1600,8 @@ TEST(WalletTests, CachedWitnessesCleanIndex) { std::vector saplingNotes; std::vector sproutAnchors; std::vector saplingAnchors; - SproutMerkleTree sproutTree; - SproutMerkleTree sproutRiTree = sproutTree; - SaplingMerkleTree saplingTree; - SaplingMerkleTree saplingRiTree = saplingTree; + MerkleFrontiers frontiers; + MerkleFrontiers riFrontiers = { .sprout = frontiers.sprout, .sapling = frontiers.sapling }; std::vector> sproutWitnesses; std::vector> saplingWitnesses; @@ -1623,11 +1614,11 @@ TEST(WalletTests, CachedWitnessesCleanIndex) { indices.resize(numBlocks); for (size_t i = 0; i < numBlocks; i++) { indices[i].nHeight = i; - auto oldSproutRoot = sproutTree.root(); - auto oldSaplingRoot = saplingTree.root(); - auto outpts = CreateValidBlock(wallet, sk, indices[i], blocks[i], sproutTree, saplingTree); - EXPECT_NE(oldSproutRoot, sproutTree.root()); - EXPECT_NE(oldSaplingRoot, saplingTree.root()); + auto oldSproutRoot = frontiers.sprout.root(); + auto oldSaplingRoot = frontiers.sapling.root(); + auto outpts = CreateValidBlock(wallet, sk, indices[i], blocks[i], frontiers); + EXPECT_NE(oldSproutRoot, frontiers.sprout.root()); + EXPECT_NE(oldSaplingRoot, frontiers.sapling.root()); sproutNotes.push_back(outpts.first); saplingNotes.push_back(outpts.second); @@ -1643,9 +1634,8 @@ TEST(WalletTests, CachedWitnessesCleanIndex) { // Now pretend we are reindexing: the chain is cleared, and each block is // used to increment witnesses again. for (size_t i = 0; i < numBlocks; i++) { - SproutMerkleTree sproutRiPrevTree {sproutRiTree}; - SaplingMerkleTree saplingRiPrevTree {saplingRiTree}; - wallet.IncrementNoteWitnesses(Params().GetConsensus(), &(indices[i]), &(blocks[i]), sproutRiTree, saplingRiTree, true); + MerkleFrontiers riPrevFrontiers{riFrontiers}; + wallet.IncrementNoteWitnesses(Params().GetConsensus(), &(indices[i]), &(blocks[i]), riFrontiers, true); auto anchors = GetWitnessesAndAnchors(wallet, sproutNotes, saplingNotes, sproutWitnesses, saplingWitnesses); for (size_t j = 0; j < numBlocks; j++) { @@ -1672,7 +1662,7 @@ TEST(WalletTests, CachedWitnessesCleanIndex) { } { - wallet.IncrementNoteWitnesses(Params().GetConsensus(), &(indices[i]), &(blocks[i]), sproutRiPrevTree, saplingRiPrevTree, true); + wallet.IncrementNoteWitnesses(Params().GetConsensus(), &(indices[i]), &(blocks[i]), riPrevFrontiers, true); auto anchors = GetWitnessesAndAnchors(wallet, sproutNotes, saplingNotes, sproutWitnesses, saplingWitnesses); for (size_t j = 0; j < numBlocks; j++) { EXPECT_TRUE((bool) sproutWitnesses[j]); @@ -2059,6 +2049,8 @@ TEST(WalletTests, UpdatedSaplingNoteData) { builder.AddSaplingOutput(extfvk.fvk.ovk, pa2, 25000, {}); auto tx = builder.Build().GetTxOrThrow(); + MerkleFrontiers frontiers = { .sapling = testNote.tree }; + // Wallet contains extfvk1 but not extfvk2 CWalletTx wtx {&wallet, tx}; ASSERT_TRUE(wallet.AddSaplingZKey(sk)); @@ -2067,7 +2059,6 @@ TEST(WalletTests, UpdatedSaplingNoteData) { // Fake-mine the transaction EXPECT_EQ(-1, chainActive.Height()); - SproutMerkleTree sproutTree; CBlock block; block.vtx.push_back(wtx); block.hashMerkleRoot = block.BuildMerkleTree(); @@ -2086,7 +2077,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) { wallet.LoadWalletTx(wtx); // Simulate receiving new block and ChainTip signal - wallet.IncrementNoteWitnesses(Params().GetConsensus(), &fakeIndex, &block, sproutTree, testNote.tree, true); + wallet.IncrementNoteWitnesses(Params().GetConsensus(), &fakeIndex, &block, frontiers, true); wallet.UpdateSaplingNullifierNoteMapForBlock(&block); // Retrieve the updated wtx from wallet @@ -2104,7 +2095,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) { // The payment note has not been witnessed yet, so let's fake the witness. SaplingOutPoint sop0(wtx2.GetHash(), 0); SaplingOutPoint sop1(wtx2.GetHash(), 1); - wtx2.mapSaplingNoteData[sop0].witnesses.push_front(testNote.tree.witness()); + wtx2.mapSaplingNoteData[sop0].witnesses.push_front(frontiers.sapling.witness()); wtx2.mapSaplingNoteData[sop0].witnessHeight = 0; // The txs are different as wtx is aware of just the change output, @@ -2132,7 +2123,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) { EXPECT_EQ(wtx.mapSaplingNoteData[sop0].witnesses.front(), wtx2.mapSaplingNoteData[sop0].witnesses.front()); // wtx2 never had its change output witnessed even though it has been in wtx EXPECT_EQ(0, wtx2.mapSaplingNoteData[sop1].witnesses.size()); - EXPECT_EQ(wtx.mapSaplingNoteData[sop1].witnesses.front(), testNote.tree.witness()); + EXPECT_EQ(wtx.mapSaplingNoteData[sop1].witnesses.front(), frontiers.sapling.witness()); // Tear down chainActive.SetTip(NULL); @@ -2215,7 +2206,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { // Fake-mine the transaction EXPECT_EQ(-1, chainActive.Height()); - SaplingMerkleTree saplingTree; + MerkleFrontiers frontiers; SproutMerkleTree sproutTree; CBlock block; block.vtx.push_back(wtx); @@ -2235,7 +2226,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { wallet.LoadWalletTx(wtx); // Simulate receiving new block and ChainTip signal - wallet.IncrementNoteWitnesses(Params().GetConsensus(), &fakeIndex, &block, sproutTree, saplingTree, true); + wallet.IncrementNoteWitnesses(Params().GetConsensus(), &fakeIndex, &block, frontiers, true); wallet.UpdateSaplingNullifierNoteMapForBlock(&block); // Retrieve the updated wtx from wallet @@ -2248,8 +2239,8 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { auto maybe_note = maybe_pt.value().note(ivk); ASSERT_EQ(static_cast(maybe_note), true); auto note = maybe_note.value(); - auto anchor = saplingTree.root(); - auto witness = saplingTree.witness(); + auto anchor = frontiers.sapling.root(); + auto witness = frontiers.sapling.witness(); // Create a Sapling-only transaction // 0.0004 z-ZEC in, 0.00025 z-ZEC out, default fee, 0.00005 z-ZEC change diff --git a/src/wallet/orchard.h b/src/wallet/orchard.h index d6aa43ce2..a846a95c3 100644 --- a/src/wallet/orchard.h +++ b/src/wallet/orchard.h @@ -13,6 +13,7 @@ #include "rust/orchard/keys.h" #include "rust/orchard/wallet.h" #include "zcash/address/orchard.hpp" +#include "zcash/IncrementalMerkleTree.hpp" class OrchardWallet; class OrchardWalletNoteCommitmentTreeWriter; @@ -218,6 +219,16 @@ public: return orchard_wallet_reset(inner.get()); } + /** + * Overwrite the first bridge of the Orchard note commitment tree to have the + * provided frontier as its latest state. This will fail with an assertion error + * if any checkpoints exist in the tree. + */ + void InitNoteCommitmentTree(const OrchardMerkleFrontier& frontier) { + assert(!GetLastCheckpointHeight().has_value()); + assert(orchard_wallet_init_from_frontier(inner.get(), frontier.inner.get())); + } + /** * Checkpoint the note commitment tree. This returns `false` and leaves the note * commitment tree unmodified if the block height specified is not the successor @@ -245,9 +256,9 @@ public: * previously identified as having been spent by transactions in the * latest block. */ - bool Rewind(int nBlockHeight, uint32_t& blocksRewoundRet) { + bool Rewind(int nBlockHeight, uint32_t& uResultHeight) { assert(nBlockHeight >= 0); - return orchard_wallet_rewind(inner.get(), (uint32_t) nBlockHeight, &blocksRewoundRet); + return orchard_wallet_rewind(inner.get(), (uint32_t) nBlockHeight, &uResultHeight); } static void PushOrchardActionIVK(void* rec, RawOrchardActionIVK actionIVK) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4d2206f53..9727ed6e8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1392,15 +1392,14 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, void CWallet::ChainTipAdded(const CBlockIndex *pindex, const CBlock *pblock, - SproutMerkleTree sproutTree, - SaplingMerkleTree saplingTree, + MerkleFrontiers frontiers, bool performOrchardWalletUpdates) { const auto chainParams = Params(); IncrementNoteWitnesses( chainParams.GetConsensus(), pindex, pblock, - sproutTree, saplingTree, performOrchardWalletUpdates); + frontiers, performOrchardWalletUpdates); UpdateSaplingNullifierNoteMapForBlock(pblock); // SetBestChain() can be expensive for large wallets, so do only @@ -1432,11 +1431,11 @@ void CWallet::ChainTip(const CBlockIndex *pindex, const CBlock *pblock, // If this is None, it indicates a rollback and we will decrement the // witnesses / rewind the tree - std::optional> added) + std::optional added) { const auto& consensus = Params().GetConsensus(); if (added.has_value()) { - ChainTipAdded(pindex, pblock, added->first, added->second, true); + ChainTipAdded(pindex, pblock, added.value(), true); // Prevent migration transactions from being created when node is syncing after launch, // and also when node wakes up from suspension/hibernation and incoming blocks are old. if (!IsInitialBlockDownload(consensus) && @@ -2621,8 +2620,7 @@ void CWallet::IncrementNoteWitnesses( const Consensus::Params& consensus, const CBlockIndex* pindex, const CBlock* pblockIn, - SproutMerkleTree& sproutTree, - SaplingMerkleTree& saplingTree, + MerkleFrontiers& frontiers, bool performOrchardWalletUpdates) { LOCK(cs_wallet); @@ -2632,6 +2630,9 @@ void CWallet::IncrementNoteWitnesses( } if (performOrchardWalletUpdates && consensus.NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_NU5)) { + if (!orchardWallet.GetLastCheckpointHeight().has_value()) { + orchardWallet.InitNoteCommitmentTree(frontiers.orchard); + } assert(orchardWallet.CheckpointNoteCommitmentTree(pindex->nHeight)); } @@ -2654,7 +2655,7 @@ void CWallet::IncrementNoteWitnesses( const JSDescription& jsdesc = tx.vJoinSplit[i]; for (uint8_t j = 0; j < jsdesc.commitments.size(); j++) { const uint256& note_commitment = jsdesc.commitments[j]; - sproutTree.append(note_commitment); + frontiers.sprout.append(note_commitment); // Increment existing witnesses for (std::pair& wtxItem : mapWallet) { @@ -2664,14 +2665,14 @@ void CWallet::IncrementNoteWitnesses( // If this is our note, witness it if (txIsOurs) { JSOutPoint jsoutpt {hash, i, j}; - ::WitnessNoteIfMine(mapWallet[hash].mapSproutNoteData, pindex->nHeight, nWitnessCacheSize, jsoutpt, sproutTree.witness()); + ::WitnessNoteIfMine(mapWallet[hash].mapSproutNoteData, pindex->nHeight, nWitnessCacheSize, jsoutpt, frontiers.sprout.witness()); } } } // Sapling for (uint32_t i = 0; i < tx.vShieldedOutput.size(); i++) { const uint256& note_commitment = tx.vShieldedOutput[i].cmu; - saplingTree.append(note_commitment); + frontiers.sapling.append(note_commitment); // Increment existing witnesses for (std::pair& wtxItem : mapWallet) { @@ -2681,7 +2682,7 @@ void CWallet::IncrementNoteWitnesses( // If this is our note, witness it if (txIsOurs) { SaplingOutPoint outPoint {hash, i}; - ::WitnessNoteIfMine(mapWallet[hash].mapSaplingNoteData, pindex->nHeight, nWitnessCacheSize, outPoint, saplingTree.witness()); + ::WitnessNoteIfMine(mapWallet[hash].mapSaplingNoteData, pindex->nHeight, nWitnessCacheSize, outPoint, frontiers.sapling.witness()); } } } @@ -2746,23 +2747,35 @@ void DecrementNoteWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t n void CWallet::DecrementNoteWitnesses(const Consensus::Params& consensus, const CBlockIndex* pindex) { LOCK(cs_wallet); + bool hasSprout = false; + bool hasSapling = false; for (std::pair& wtxItem : mapWallet) { + hasSprout |= !wtxItem.second.mapSproutNoteData.empty(); ::DecrementNoteWitnesses(wtxItem.second.mapSproutNoteData, pindex->nHeight, nWitnessCacheSize); + hasSapling |= !wtxItem.second.mapSaplingNoteData.empty(); ::DecrementNoteWitnesses(wtxItem.second.mapSaplingNoteData, pindex->nHeight, nWitnessCacheSize); } - nWitnessCacheSize -= 1; - // TODO: If nWitnessCache is zero, we need to regenerate the caches (#1302) - assert(nWitnessCacheSize > 0); + if (nWitnessCacheSize > 0) { + nWitnessCacheSize -= 1; + } + // TODO: If nWitnessCache is zero, we need to regenerate the caches (#1302); + // however, if we have never observed Sprout or Sapling notes, this is okay + // because then the witness cache size can remain at 0. + assert(!(hasSprout || hasSapling) || nWitnessCacheSize > 0); // ORCHARD: rewind to the last checkpoint. if (consensus.NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_NU5)) { // pindex->nHeight is the height of the block being removed, so we rewind // to the previous block height - uint32_t blocksRewound{0}; + uint32_t uResultHeight{0}; assert(pindex->nHeight >= 1); - assert(orchardWallet.Rewind(pindex->nHeight - 1, blocksRewound)); - assert(blocksRewound == 1); - if (consensus.NetworkUpgradeActive(pindex->nHeight - 1, Consensus::UPGRADE_NU5)) { + assert(orchardWallet.Rewind(pindex->nHeight - 1, uResultHeight)); + assert(uResultHeight == pindex->nHeight - 1); + // If we have no checkpoints after the rewind, then the latest anchor of the + // wallet's Orchard note commitment tree will be in an indeterminate state and it + // will be overwritten in the next `IncrementNoteWitnesses` call, so we can skip + // the check against `hashFinalOrchardRoot`. + if (orchardWallet.GetLastCheckpointHeight().has_value()) { assert(pindex->pprev->hashFinalOrchardRoot == orchardWallet.GetLatestAnchor()); } } @@ -4368,13 +4381,14 @@ int CWallet::ScanForWalletTransactions( // since we do not support Orchard key import. if (isInitScan) { int rewindHeight = std::max(nu5_height.value(), pindex->nHeight - 1); - uint32_t blocksRewound{0}; LogPrintf( "CWallet::ScanForWalletTransactions(): Rewinding Orchard wallet to height %d; current is %d", rewindHeight, orchardWallet.GetLastCheckpointHeight().value()); - if (orchardWallet.Rewind(rewindHeight, blocksRewound)) { + uint32_t uResultHeight{0}; + if (orchardWallet.Rewind(rewindHeight, uResultHeight)) { // rewind was successful or a no-op, so perform Orchard wallet updates + assert(uResultHeight == rewindHeight); performOrchardWalletUpdates = true; } else { // Orchard witnesses will not be able to be correctly updated, because we @@ -4410,18 +4424,20 @@ int CWallet::ScanForWalletTransactions( } } - SproutMerkleTree sproutTree; - SaplingMerkleTree saplingTree; + MerkleFrontiers frontiers; // This should never fail: we should always be able to get the tree // state on the path to the tip of our chain - assert(pcoinsTip->GetSproutAnchorAt(pindex->hashSproutAnchor, sproutTree)); + assert(pcoinsTip->GetSproutAnchorAt(pindex->hashSproutAnchor, frontiers.sprout)); if (pindex->pprev) { if (consensus.NetworkUpgradeActive(pindex->pprev->nHeight, Consensus::UPGRADE_SAPLING)) { - assert(pcoinsTip->GetSaplingAnchorAt(pindex->pprev->hashFinalSaplingRoot, saplingTree)); + assert(pcoinsTip->GetSaplingAnchorAt(pindex->pprev->hashFinalSaplingRoot, frontiers.sapling)); + } + if (consensus.NetworkUpgradeActive(pindex->pprev->nHeight, Consensus::UPGRADE_NU5)) { + assert(pcoinsTip->GetOrchardAnchorAt(pindex->pprev->hashFinalOrchardRoot, frontiers.orchard)); } } // Increment note witness caches - ChainTipAdded(pindex, &block, sproutTree, saplingTree, performOrchardWalletUpdates); + ChainTipAdded(pindex, &block, frontiers, performOrchardWalletUpdates); pindex = chainActive.Next(pindex); if (GetTime() >= nNow + 60) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 33a093a25..1122b2990 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1086,8 +1086,7 @@ protected: const Consensus::Params& consensus, const CBlockIndex* pindex, const CBlock* pblock, - SproutMerkleTree& sproutTree, - SaplingMerkleTree& saplingTree, + MerkleFrontiers& frontiers, bool performOrchardWalletUpdates ); /** @@ -1158,8 +1157,7 @@ private: void ChainTipAdded( const CBlockIndex *pindex, const CBlock *pblock, - SproutMerkleTree sproutTree, - SaplingMerkleTree saplingTree, + MerkleFrontiers frontiers, bool performOrchardWalletUpdates); /* Add a transparent secret key to the wallet. Internal use only. */ @@ -1806,7 +1804,7 @@ public: void ChainTip( const CBlockIndex *pindex, const CBlock *pblock, - std::optional> added); + std::optional added); void RunSaplingMigration(int blockHeight); void AddPendingSaplingMigrationTx(const CTransaction& tx); /** Saves witness caches and best block locator to disk. */ diff --git a/src/zcash/IncrementalMerkleTree.hpp b/src/zcash/IncrementalMerkleTree.hpp index 5c6d51899..e8fa7e912 100644 --- a/src/zcash/IncrementalMerkleTree.hpp +++ b/src/zcash/IncrementalMerkleTree.hpp @@ -259,12 +259,16 @@ typedef libzcash::IncrementalMerkleTree SaplingWitness; typedef libzcash::IncrementalWitness SaplingTestingWitness; +class OrchardWallet; + class OrchardMerkleFrontier { private: /// An incremental Sinsemilla tree; this pointer may never be null. /// Memory is allocated by Rust. std::unique_ptr inner; + + friend class OrchardWallet; public: OrchardMerkleFrontier() : inner(orchard_merkle_frontier_empty(), orchard_merkle_frontier_free) {} diff --git a/src/zcbenchmarks.cpp b/src/zcbenchmarks.cpp index fb3009eca..65dfec8ad 100644 --- a/src/zcbenchmarks.cpp +++ b/src/zcbenchmarks.cpp @@ -335,8 +335,7 @@ double benchmark_increment_sprout_note_witnesses(size_t nTxs) const Consensus::Params& consensusParams = Params().GetConsensus(); CWallet wallet(Params()); - SproutMerkleTree sproutTree; - SaplingMerkleTree saplingTree; + MerkleFrontiers frontiers; auto sproutSpendingKey = libzcash::SproutSpendingKey::random(); wallet.AddSproutSpendingKey(sproutSpendingKey); @@ -353,7 +352,7 @@ double benchmark_increment_sprout_note_witnesses(size_t nTxs) index1.nHeight = 1; // Increment to get transactions witnessed - wallet.ChainTip(&index1, &block1, std::make_pair(sproutTree, saplingTree)); + wallet.ChainTip(&index1, &block1, frontiers); // Second block CBlock block2; @@ -369,7 +368,7 @@ double benchmark_increment_sprout_note_witnesses(size_t nTxs) struct timeval tv_start; timer_start(tv_start); - wallet.ChainTip(&index2, &block2, std::make_pair(sproutTree, saplingTree)); + wallet.ChainTip(&index2, &block2, frontiers); return timer_stop(tv_start); } @@ -397,8 +396,7 @@ double benchmark_increment_sapling_note_witnesses(size_t nTxs) const Consensus::Params& consensusParams = Params().GetConsensus(); CWallet wallet(Params()); - SproutMerkleTree sproutTree; - SaplingMerkleTree saplingTree; + MerkleFrontiers frontiers; auto saplingSpendingKey = GetTestMasterSaplingSpendingKey(); wallet.AddSaplingSpendingKey(saplingSpendingKey); @@ -415,7 +413,7 @@ double benchmark_increment_sapling_note_witnesses(size_t nTxs) index1.nHeight = 1; // Increment to get transactions witnessed - wallet.ChainTip(&index1, &block1, std::make_pair(sproutTree, saplingTree)); + wallet.ChainTip(&index1, &block1, frontiers); // Second block CBlock block2; @@ -431,7 +429,7 @@ double benchmark_increment_sapling_note_witnesses(size_t nTxs) struct timeval tv_start; timer_start(tv_start); - wallet.ChainTip(&index2, &block2, std::make_pair(sproutTree, saplingTree)); + wallet.ChainTip(&index2, &block2, frontiers); return timer_stop(tv_start); }