zcash_client_sqlite: Fix shardtree error caused by pruning after frontier insertion.

Fixes #1398
This commit is contained in:
Kris Nuttycombe 2024-05-24 17:40:38 -06:00
parent de66c5b154
commit ecea9ca196
10 changed files with 85 additions and 90 deletions

6
Cargo.lock generated
View File

@ -1061,8 +1061,7 @@ dependencies = [
[[package]]
name = "incrementalmerkletree"
version = "0.5.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "eb1872810fb725b06b8c153dde9e86f3ec26747b9b60096da7a869883b549cbe"
source = "git+https://github.com/zcash/incrementalmerkletree?rev=8b4b1315d64d171bcf701e5de59d0707dc029f9c#8b4b1315d64d171bcf701e5de59d0707dc029f9c"
dependencies = [
"either",
"proptest",
@ -2176,8 +2175,7 @@ dependencies = [
[[package]]
name = "shardtree"
version = "0.3.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3b3cdd24424ce0b381646737fedddc33c4dcf7dcd2d545056b53f7982097bef5"
source = "git+https://github.com/zcash/incrementalmerkletree?rev=8b4b1315d64d171bcf701e5de59d0707dc029f9c#8b4b1315d64d171bcf701e5de59d0707dc029f9c"
dependencies = [
"assert_matches",
"bitflags 2.5.0",

View File

@ -45,7 +45,7 @@ zcash_proofs = { version = "0.15", path = "zcash_proofs", default-features = fal
ff = "0.13"
group = "0.13"
incrementalmerkletree = "0.5.1"
shardtree = "0.3"
shardtree = "0.3.1"
zcash_spec = "0.1"
# Payment protocols
@ -122,3 +122,7 @@ zip32 = "0.1.1"
lto = true
panic = 'abort'
codegen-units = 1
[patch.crates-io]
incrementalmerkletree = { git = "https://github.com/zcash/incrementalmerkletree", rev = "8b4b1315d64d171bcf701e5de59d0707dc029f9c" }
shardtree = { git = "https://github.com/zcash/incrementalmerkletree", rev = "8b4b1315d64d171bcf701e5de59d0707dc029f9c" }

View File

@ -5,7 +5,7 @@ use std::convert::TryFrom;
use std::fmt::{self, Debug};
use std::hash::Hash;
use incrementalmerkletree::{Position, Retention};
use incrementalmerkletree::{Marking, Position, Retention};
use sapling::{
note_encryption::{CompactOutputDescription, SaplingDomain},
SaplingIvk,
@ -1105,7 +1105,11 @@ fn find_received<
let retention = match (decrypted_note.is_some(), is_checkpoint) {
(is_marked, true) => Retention::Checkpoint {
id: block_height,
is_marked,
marking: if is_marked {
Marking::Marked
} else {
Marking::None
},
},
(true, false) => Retention::Marked,
(false, false) => Retention::Ephemeral,
@ -1297,7 +1301,7 @@ mod tests {
use std::convert::Infallible;
use incrementalmerkletree::{Position, Retention};
use incrementalmerkletree::{Marking, Position, Retention};
use sapling::Nullifier;
use zcash_keys::keys::UnifiedSpendingKey;
use zcash_primitives::{
@ -1390,7 +1394,7 @@ mod tests {
Retention::Ephemeral,
Retention::Checkpoint {
id: scanned_block.height(),
is_marked: true
marking: Marking::Marked
}
]
);
@ -1466,7 +1470,7 @@ mod tests {
Retention::Marked,
Retention::Checkpoint {
id: scanned_block.height(),
is_marked: false
marking: Marking::None
}
]
);
@ -1525,7 +1529,7 @@ mod tests {
Retention::Ephemeral,
Retention::Checkpoint {
id: scanned_block.height(),
is_marked: false
marking: Marking::None
}
]
);

View File

@ -13,6 +13,7 @@ const SER_V1: u8 = 1;
const NIL_TAG: u8 = 0;
const LEAF_TAG: u8 = 1;
const PARENT_TAG: u8 = 2;
const PRUNED_TAG: u8 = 3;
/// Writes a [`PrunableTree`] to the provided [`Write`] instance.
///
@ -43,6 +44,10 @@ pub fn write_shard<H: HashSer, W: Write>(writer: &mut W, tree: &PrunableTree<H>)
writer.write_u8(NIL_TAG)?;
Ok(())
}
Node::Pruned => {
writer.write_u8(PRUNED_TAG)?;
Ok(())
}
}
}
@ -74,6 +79,7 @@ fn read_shard_v1<H: HashSer, R: Read>(mut reader: &mut R) -> io::Result<Prunable
Ok(Tree::leaf((value, flags)))
}
NIL_TAG => Ok(Tree::empty()),
PRUNED_TAG => Ok(Tree::empty_pruned()),
other => Err(io::Error::new(
io::ErrorKind::InvalidData,
format!("Node tag not recognized: {}", other),

View File

@ -32,7 +32,7 @@
// Catch documentation errors caused by code changes.
#![deny(rustdoc::broken_intra_doc_links)]
use incrementalmerkletree::{Position, Retention};
use incrementalmerkletree::{Marking, Position, Retention};
use maybe_rayon::{
prelude::{IndexedParallelIterator, ParallelIterator},
slice::ParallelSliceMut,
@ -929,7 +929,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
from_state.final_sapling_tree().clone(),
Retention::Checkpoint {
id: from_state.block_height(),
is_marked: false,
marking: Marking::Reference,
},
)?;
@ -978,7 +978,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
from_state.final_orchard_tree().clone(),
Retention::Checkpoint {
id: from_state.block_height(),
is_marked: false,
marking: Marking::Reference,
},
)?;

View File

@ -6,7 +6,7 @@ use std::{collections::BTreeMap, convert::Infallible};
use std::fs::File;
use group::ff::Field;
use incrementalmerkletree::{Position, Retention};
use incrementalmerkletree::{Marking, Position, Retention};
use nonempty::NonEmpty;
use prost::Message;
use rand_chacha::ChaChaRng;
@ -181,23 +181,6 @@ impl<Cache> TestBuilder<Cache> {
self
}
pub(crate) fn with_account_birthday(
mut self,
birthday: impl FnOnce(
&mut ChaChaRng,
&LocalNetwork,
Option<&InitialChainState>,
) -> AccountBirthday,
) -> Self {
assert!(self.account_birthday.is_none());
self.account_birthday = Some(birthday(
&mut self.rng,
&self.network,
self.initial_chain_state.as_ref(),
));
self
}
pub(crate) fn with_account_from_sapling_activation(mut self, prev_hash: BlockHash) -> Self {
assert!(self.account_birthday.is_none());
self.account_birthday = Some(AccountBirthday::from_parts(
@ -245,7 +228,7 @@ impl<Cache> TestBuilder<Cache> {
initial_state.chain_state.final_sapling_tree().clone(),
Retention::Checkpoint {
id: initial_state.chain_state.block_height(),
is_marked: false,
marking: Marking::Reference,
},
)
})
@ -262,7 +245,7 @@ impl<Cache> TestBuilder<Cache> {
initial_state.chain_state.final_orchard_tree().clone(),
Retention::Checkpoint {
id: initial_state.chain_state.block_height(),
is_marked: false,
marking: Marking::Reference,
},
)
})

View File

@ -64,7 +64,7 @@
//! wallet.
//! - `memo` the shielded memo associated with the output, if any.
use incrementalmerkletree::Retention;
use incrementalmerkletree::{Marking, Retention};
use rusqlite::{self, named_params, OptionalExtension};
use secrecy::{ExposeSecret, SecretVec};
use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
@ -450,7 +450,7 @@ pub(crate) fn add_account<P: consensus::Parameters>(
// there exists a prior block for which frontier is the tree state at the end of
// the block.
id: birthday.height() - 1,
is_marked: false,
marking: Marking::Reference,
},
)?;
}
@ -477,7 +477,7 @@ pub(crate) fn add_account<P: consensus::Parameters>(
// there exists a prior block for which frontier is the tree state at the end of
// the block.
id: birthday.height() - 1,
is_marked: false,
marking: Marking::Reference,
},
)?;
}

View File

@ -1010,17 +1010,11 @@ pub(crate) fn put_shard_roots<
let insert_into_cap = tracing::info_span!("insert_into_cap").entered();
let cap_result = cap
.batch_insert(
.batch_insert::<(), _>(
Position::from(start_index),
roots.iter().map(|r| {
(
LevelShifter(r.root_hash().clone()),
Retention::Checkpoint {
id: (),
is_marked: false,
},
)
}),
roots
.iter()
.map(|r| (LevelShifter(r.root_hash().clone()), Retention::Reference)),
)
.map_err(ShardTreeError::Insert)?
.expect("slice of inserted roots was verified to be nonempty");
@ -1084,7 +1078,7 @@ mod tests {
check_append, check_checkpoint_rewind, check_remove_mark, check_rewind_remove_mark,
check_root_hashes, check_witness_consistency, check_witnesses,
},
Position, Retention,
Marking, Position, Retention,
};
use shardtree::ShardTree;
use zcash_client_backend::data_api::chain::CommitmentTreeRoot;
@ -1235,7 +1229,7 @@ mod tests {
'c' => Retention::Marked,
'h' => Retention::Checkpoint {
id: checkpoint_height,
is_marked: false,
marking: Marking::None,
},
_ => Retention::Ephemeral,
},

View File

@ -4,7 +4,7 @@
use std::collections::{BTreeSet, HashSet};
use incrementalmerkletree::Retention;
use incrementalmerkletree::{Marking, Retention};
use rusqlite::{self, named_params, params};
use schemer;
use schemer_rusqlite::RusqliteMigration;
@ -173,7 +173,7 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
nonempty_frontier.clone(),
Retention::Checkpoint {
id: block_height,
is_marked: false,
marking: Marking::Reference,
},
)
.map_err(|e| match e {

View File

@ -791,46 +791,65 @@ pub(crate) mod tests {
}
pub(crate) fn test_with_nu5_birthday_offset<T: ShieldedPoolTester>(
initial_shard_blocks: u32,
birthday_offset: u32,
prior_block_hash: BlockHash,
insert_prior_roots: bool,
) -> (TestState<BlockCache>, T::Fvk, AccountBirthday, u32) {
let st = TestBuilder::new()
.with_block_cache()
.with_account_birthday(|rng, network, initial_chain_state| {
// We're constructing the birthday without adding any chain data.
assert!(initial_chain_state.is_none());
.with_initial_chain_state(|rng, network| {
// We set the Sapling and Orchard frontiers at the birthday height to be
// 1234 notes into the second shard.
let frontier_position = Position::from((0x1 << 16) + 1234);
let birthday_height =
network.activation_height(NetworkUpgrade::Nu5).unwrap() + birthday_offset;
let initial_shard_end =
network.activation_height(NetworkUpgrade::Nu5).unwrap() + initial_shard_blocks;
let birthday_height = initial_shard_end + birthday_offset;
// Construct a fake chain state for the end of the block with the given
// birthday_offset from the Nu5 birthday.
let (_, sapling_initial_tree) = Frontier::random_with_prior_subtree_roots(
rng,
(frontier_position + 1).into(),
NonZeroU8::new(16).unwrap(),
);
// birthday_offset from the end of the last shard.
let (prior_sapling_roots, sapling_initial_tree) =
Frontier::random_with_prior_subtree_roots(
rng,
(frontier_position + 1).into(),
NonZeroU8::new(16).unwrap(),
);
#[cfg(feature = "orchard")]
let (_, orchard_initial_tree) = Frontier::random_with_prior_subtree_roots(
rng,
(frontier_position + 1).into(),
NonZeroU8::new(16).unwrap(),
);
let (prior_orchard_roots, orchard_initial_tree) =
Frontier::random_with_prior_subtree_roots(
rng,
(frontier_position + 1).into(),
NonZeroU8::new(16).unwrap(),
);
AccountBirthday::from_parts(
ChainState::new(
InitialChainState {
chain_state: ChainState::new(
birthday_height,
prior_block_hash,
sapling_initial_tree,
#[cfg(feature = "orchard")]
orchard_initial_tree,
),
None,
)
prior_sapling_roots: if insert_prior_roots {
prior_sapling_roots
.into_iter()
.map(|root| CommitmentTreeRoot::from_parts(initial_shard_end, root))
.collect()
} else {
vec![]
},
#[cfg(feature = "orchard")]
prior_orchard_roots: if insert_prior_roots {
prior_orchard_roots
.into_iter()
.map(|root| CommitmentTreeRoot::from_parts(initial_shard_end, root))
.collect()
} else {
vec![]
},
}
})
.with_account_having_current_birthday()
.build();
let birthday = st.test_account().unwrap().birthday().clone();
@ -856,7 +875,7 @@ pub(crate) mod tests {
// Use a non-zero birthday offset because Sapling and NU5 are activated at the same height.
let (st, _, birthday, sap_active) =
test_with_nu5_birthday_offset::<T>(76, BlockHash([0; 32]));
test_with_nu5_birthday_offset::<T>(50, 26, BlockHash([0; 32]), true);
let birthday_height = birthday.height().into();
let expected = vec![
@ -924,7 +943,7 @@ pub(crate) mod tests {
// Use a non-zero birthday offset because Sapling and NU5 are activated at the same height.
let (mut st, _, birthday, sap_active) =
test_with_nu5_birthday_offset::<T>(76, BlockHash([0; 32]));
test_with_nu5_birthday_offset::<T>(50, 26, BlockHash([0; 32]), false);
// Set up the following situation:
//
@ -969,7 +988,7 @@ pub(crate) mod tests {
// Use a non-zero birthday offset because Sapling and NU5 are activated at the same height.
let (mut st, _, birthday, sap_active) =
test_with_nu5_birthday_offset::<T>(76, BlockHash([0; 32]));
test_with_nu5_birthday_offset::<T>(76, 1000, BlockHash([0; 32]), true);
// Set up the following situation:
//
@ -978,19 +997,6 @@ pub(crate) mod tests {
// wallet_birthday
let prior_tip_height = birthday.height();
// Set up some shard root history before the wallet birthday.
let last_shard_start = birthday.height() - 1000;
T::put_subtree_roots(
&mut st,
0,
&[CommitmentTreeRoot::from_parts(
last_shard_start,
// fake a hash, the value doesn't matter
T::empty_tree_leaf(),
)],
)
.unwrap();
// Update the chain tip.
let tip_height = prior_tip_height + 500;
st.wallet_mut().update_chain_tip(tip_height).unwrap();
@ -1499,7 +1505,7 @@ pub(crate) mod tests {
/// in the last note in that block.
/// * The next block crosses the shard boundary, with two notes in the prior
/// shard and two blocks in the subsequent shard
/// * An additional 100 blocks are scanned, to ensure that the checkpoint
/// * An additional 110 blocks are scanned, to ensure that the checkpoint
/// is pruned.
///
/// The diagram below shows the arrangement. the position of the X indicates the
@ -1657,7 +1663,7 @@ pub(crate) mod tests {
#[test]
#[cfg(feature = "orchard")]
fn orchard_block_spanning_tip_boundary() {
fn orchard_block_spanning_tip_boundary_complete() {
let mut st = prepare_orchard_block_spanning_test(true);
let account = st.test_account().cloned().unwrap();
let birthday = account.birthday();