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.

Sign up for GitHub

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

Jump to bottom

Refactor FinalizedState::commit_finalized_direct #2588

Closed
conradoplg opened this issue Aug 9, 2021 · 1 comment · Fixed by #3579
Closed

Refactor FinalizedState::commit_finalized_direct #2588

conradoplg opened this issue Aug 9, 2021 · 1 comment · Fixed by #3579
Labels
C-cleanup Category: This is a cleanup C-enhancement Category: This is an improvement

Comments

@conradoplg
Copy link
Collaborator

conradoplg commented Aug 9, 2021 •
edited by teor2345
Loading

Motivation

FinalizedState::commit_finalized_direct is too big and does multiple things. Refactor it to (as suggested by @teor2345 in discussion):

  1. Move the note commitment tree and history tree updates outside the database batch
  2. Move any other calculations or lookups outside the database batch (such as UTXO lookups)
  3. Split out the chain order assertion, the calculations, and the write batch into separate methods

If we move the calculations, the batch closure becomes infallible, because it only contains serialization and deserialization.

Specifications

Designs

Related Work

Follow up to #2582
@conradoplg conradoplg added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Aug 9, 2021
@conradoplg conradoplg mentioned this issue Aug 9, 2021
Merged
3 tasks
@teor2345 teor2345 mentioned this issue Aug 13, 2021
Merged
3 tasks
@mpguerra mpguerra added C-cleanup Category: This is a cleanup P-Low labels Aug 13, 2021
@teor2345 teor2345 mentioned this issue Feb 18, 2022
Merged
2 tasks
@teor2345
Copy link
Contributor

Closed by PR #3579, but I can't get the closing keywords to work.

@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-cleanup Category: This is a cleanup C-enhancement Category: This is an improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

3. refactor(state): move database reads and writes to a new zebra_db module ZcashFoundation/zebra
3 participants
@conradoplg @mpguerra @teor2345