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_backend: Use knowledge of inserted treestates to reduce time to spendability #982

Open
nuttycom opened this issue Sep 21, 2023 · 8 comments

Comments

@nuttycom
Copy link
Contributor

In order to minimize the amount of scanning work required at the chain tip, we should implement the following:

  • Add a priority that is higher than ChainTip, call it AnchorRange or something of the sort. The semantics of an AnchorRange scanning range are that when a wallet encounters such a range, it immediately downloads the treestate corresponding to the last block prior to the start of the range and inserts that frontier directly into the note commitment tree (using a to-be-defined WalletCommitmentTrees method).
  • After updating scan ranges, the following must hold:
    • the AnchorRange scanning range is at most PRUNING_DEPTH blocks long and has its end equal to the chain_tip + 1
    • any previously existing AnchorRange range with a starting height < stable_height has its priority reduced to Historic
    • if the the shard containing stable_height has no unscanned ranges below stable_height, no AnchorRange must exist, because in this case it's unnecessary to download or insert any additional tree state.

It might be possible to repurpose the ChainTip priority to have these semantics, but I think it's better to be explicit about the circumstance where the tree state should be downloaded, and we should continue to support wallets that don't want to download tree states; such wallets will treat AnchorRange ranges in the same fashion as they currently treat ChainTip ranges.

@nuttycom nuttycom added this to the SDK 2.0 Post-Release Cleanup milestone Sep 21, 2023
@str4d
Copy link
Contributor

str4d commented Sep 21, 2023

Do we even need to modify the scan ranges? This feels like it is trying to shoe-horn additional semantics or signals into suggest_scan_ranges, that complicate it over "scan these ranges in priority order".

The need to obtain a tree state at the anchor height is associated with a change to the chain tip height. We could therefore instead modify update_chain_tip to return metadata that includes "height at which the wallet should be given a tree state to make its desired anchor spendable". If the caller ignores that metadata, then they continue to work correctly because they scan ChainTip.

@nuttycom
Copy link
Contributor Author

The need to obtain a tree state at the anchor height is associated with a change to the chain tip height. We could therefore instead modify update_chain_tip to return metadata that includes "height at which the wallet should be given a tree state to make its desired anchor spendable". If the caller ignores that metadata, then they continue to work correctly because they scan ChainTip.

We'll have to keep track somewhere of the fact that we've been given the tree state & combine the information about that latest-provided-tree-state with information about scan ranges. We'll need this whichever way we do it, so I guess that just keeping ChainTip could work, because what we really want to know is that there are no unscanned ranges (of whatever priority) between max(anchor_shard_start, downloaded_tree_state_position)

@str4d
Copy link
Contributor

str4d commented Sep 21, 2023

Yeah, when a treestate at a specific height is provided, we could look at the height and if it intersects a ChainTip range then we can alter that range to deprioritise it or subdivide it as necessary.

@nuttycom
Copy link
Contributor Author

nuttycom commented Oct 5, 2023

A couple more insights:

This change will mean that we can no longer use whether or not a subtree is completely scanned as a condition for determining whether a note is spendable. Instead, we will likely want to store an extra piece of shard metadata, which is the height in the shard above which it's possible to build a witness.

@daira
Copy link
Contributor

daira commented Oct 6, 2023

Is scanning work at the chain tip actually a bottleneck? I would have thought that it's desirable to always scan the chain tip when we see new blocks, and that it takes very little time in the steady state. (If the database reopening on each FFI call is why it's taking a significant time, then that's what we should address, rather than adding complexity elsewhere.)

@nuttycom
Copy link
Contributor Author

nuttycom commented Oct 6, 2023

In my experience with Zashi, it can take more than a minute, sometimes even several minutes, to scan enough blocks to fill up the 2^16 notes in the tip shard; 2^16 notes is still ~0.1% of the chain, and in sparse block periods that can be weeks worth of blocks.

@nuttycom
Copy link
Contributor Author

nuttycom commented Nov 16, 2023

I want to generalize this task to "the wallet downloads the tree state corresponding to the previous block for every one of the user's notes that is discovered, and inserts this tree state into the note commitment tree". This doesn't reveal any information to the lightwalletd server that fetching the memos doesn't already reveal.

This would make it so that blocks prior to the discovered note could be downgraded to Historic scanning priority. Changes to the note selection logic would be required to make these notes selectable without having to scan the previous blocks in the subtree (the blocks within the subtree following the one containing the received note would still have to be scanned, but this could be further optimized by skipping trial decryption and just doing commitment tree updates.)

@nuttycom nuttycom changed the title zcash_client_backend: Support minimal scanning at the chain tip. zcash_client_backend: Import commitment tree frontiers to reduce time to spendability by 75% Dec 27, 2023
@nuttycom nuttycom changed the title zcash_client_backend: Import commitment tree frontiers to reduce time to spendability by 75% zcash_client_backend: Download treestates to reduce time to spendability by 75% Dec 27, 2023
@str4d str4d removed this from the SDK 2.0 Post-Release Cleanup milestone Jan 2, 2024
@str4d str4d added this to the Zashi 1.0 performance milestone Jan 11, 2024
@str4d str4d removed this from the Zashi 1.0 performance milestone Feb 8, 2024
@nuttycom nuttycom changed the title zcash_client_backend: Download treestates to reduce time to spendability by 75% zcash_client_backend: Use knowledge of inserted treestates to reduce time to spendability Mar 27, 2024
@nuttycom
Copy link
Contributor Author

After #1262 we have the treestates downloaded and inserted for every block range, so we just need to ensure that the scan queue logic can take advantage of them by deprioritizing the range prior to the first tree state for one of our notes in a block.

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

3 participants