Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zcash_client_sqlite: init_blocks_table and put_sapling_subtree_roots conflict #874

Closed
str4d opened this issue Jul 12, 2023 · 2 comments
Closed

Comments

@str4d
Copy link
Contributor

str4d commented Jul 12, 2023

In #831, zcash_client_sqlite::wallet::init::init_blocks_table was modified to insert the nodes of the given wallet birthday frontier into the ShardTree:

if let Some(nonempty_frontier) = block_end_tree.to_frontier().value() {
let shard_store =
SqliteShardStore::<_, sapling::Node, SAPLING_SHARD_HEIGHT>::from_connection(
wdb.conn.0,
SAPLING_TABLES_PREFIX,
)?;
let mut shard_tree: ShardTree<
_,
{ sapling::NOTE_COMMITMENT_TREE_DEPTH },
SAPLING_SHARD_HEIGHT,
> = ShardTree::new(shard_store, PRUNING_DEPTH.try_into().unwrap());
shard_tree.insert_frontier_nodes(
nonempty_frontier.clone(),
Retention::Checkpoint {
id: height,
is_marked: false,
},
)?;
}

Meanwhile, as the first step to the non-linear sync process being added in #872, we need to fetch the subtree roots before scanning and add them to the ShardTree using the WalletDb::put_sapling_subtree_roots method added in #861:

fn put_sapling_subtree_roots(
&mut self,
start_index: u64,
roots: &[CommitmentTreeRoot<sapling::Node>],
) -> Result<(), ShardTreeError<Self::Error>> {
let tx = self
.conn
.transaction()
.map_err(|e| ShardTreeError::Storage(Either::Right(e)))?;
put_shard_roots::<_, { sapling::NOTE_COMMITMENT_TREE_DEPTH }, SAPLING_SHARD_HEIGHT>(
&tx,
SAPLING_TABLES_PREFIX,
start_index,
roots,
)?;
tx.commit()
.map_err(|e| ShardTreeError::Storage(Either::Right(e)))?;
Ok(())
}

These appear to conflict: when testing #872 on mainnet, put_sapling_subtree_roots returns an insertion error.

@str4d
Copy link
Contributor Author

str4d commented Jul 12, 2023

When the added logic from init_blocks_table is commented out, the issue goes away. My initial guess is that this is caused by a bug in ShardTree::insert_frontier_nodes, where it is inserting a leaf into the cap corresponding to a subtree that is not complete.

str4d added a commit to nuttycom/librustzcash that referenced this issue Jul 13, 2023
The Merkle hashes used for the note commitment trees are domain
separated by level, so when pretending that the subtree roots are leaves
of the cap tree, we need to adjust for their level not being zero.

Closes zcash#874.

Co-authored-by: Sean Bowe <[email protected]>
@str4d
Copy link
Contributor Author

str4d commented Jul 13, 2023

The problem was that put_shard_roots pretends the subtree roots are leaves of a DEPTH - SHARD_HEIGHT tree so it can update the cap, but the Sapling tree Merkle hash is level-dependent.

str4d added a commit to nuttycom/librustzcash that referenced this issue Jul 18, 2023
The Merkle hashes used for the note commitment trees are domain
separated by level, so when pretending that the subtree roots are leaves
of the cap tree, we need to adjust for their level not being zero.

Closes zcash#874.

Co-authored-by: Sean Bowe <[email protected]>
str4d added a commit to nuttycom/librustzcash that referenced this issue Jul 18, 2023
The Merkle hashes used for the note commitment trees are domain
separated by level, so when pretending that the subtree roots are leaves
of the cap tree, we need to adjust for their level not being zero.

Closes zcash#874.

Co-authored-by: Sean Bowe <[email protected]>
str4d added a commit to nuttycom/librustzcash that referenced this issue Jul 18, 2023
The Merkle hashes used for the note commitment trees are domain
separated by level, so when pretending that the subtree roots are leaves
of the cap tree, we need to adjust for their level not being zero.

Closes zcash#874.

Co-authored-by: Sean Bowe <[email protected]>
@str4d str4d closed this as completed in cb887ef Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant