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

Parallelise note commitment tree point conversions #5425

Closed
teor2345 opened this issue Oct 19, 2022 · 1 comment
Closed

Parallelise note commitment tree point conversions #5425

teor2345 opened this issue Oct 19, 2022 · 1 comment
Labels
A-cryptography Area: Cryptography related A-state Area: State / database changes C-enhancement Category: This is an improvement I-cost Zebra infrastructure costs I-slow Problems with performance or responsiveness

Comments

@teor2345
Copy link
Contributor

teor2345 commented Oct 19, 2022

Motivation

We can speed up note commitment tree updates by running point conversions in parallel with tree updates.

Designs

Run the following operations in parallel rayon iterators or threads:

  • each point conversion, or batches of point conversions, in any order, in multiple threads (cm_x.into())
  • appending notes, in blockchain order, in a single thread (self.inner.append(...))

We can start appending notes as soon as the first point conversion is completed.

This change can be made for Sprout, Sapling, and Orchard. Sprout is a lower priority because it's not currently being used much.

Orchard example:

pub fn append(&mut self, cm_x: NoteCommitmentUpdate) -> Result<(), NoteCommitmentTreeError> {
if self.inner.append(&cm_x.into()) {

zcashd example of out of order note insertion using shardtree:
zcash/incrementalmerkletree#50

@teor2345 teor2345 added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage I-slow Problems with performance or responsiveness P-Optional ✨ A-state Area: State / database changes A-cryptography Area: Cryptography related I-cost Zebra infrastructure costs labels Oct 19, 2022
@teor2345
Copy link
Contributor Author

teor2345 commented Oct 19, 2022

This is currently optional. We've talked about it a few times, so I wanted to write it down so we had a record.

But I think we can close it, unless we decide we need it to fix #5420, and that #5420 is an important enough fix to schedule soon.

Edit: or we decide we want to fix #5709, which we need to fix before this ticket will help with sync speed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cryptography Area: Cryptography related A-state Area: State / database changes C-enhancement Category: This is an improvement I-cost Zebra infrastructure costs I-slow Problems with performance or responsiveness
Projects
None yet
Development

No branches or pull requests

2 participants