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

change(state): Add note subtree index handling to zebra-state, but don't write them to the finalized state yet #7334

Merged
merged 14 commits into from
Aug 28, 2023
Merged

change(state): Add note subtree index handling to zebra-state, but don't write them to the finalized state yet #7334

merged 14 commits into from
Aug 28, 2023

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented Aug 15, 2023 •
edited
Loading

Motivation

This is needed to support "spend before sync" in lightwalletd.

Part of #6953.

Solution

  • Add is_complete_subtree(), subtree_index, and subtree_root methods to Sapling and Orchard NoteCommitmentTrees.
  • Find end heights of subtrees
  • Find start heights of the next subtree when a subtree is completed
  • Find the start height of the first subtree in the pool if subtree_index is 0
  • Insert start heights without roots or end heights
  • Update subtree_indexes with end heights and subtree roots with merge_cf
  • Increment database format minor version

Review

Anyone can review.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

Follow Up Work

  • Add subtrees below initial tip height

@arya2 arya2 added C-enhancement Category: This is an improvement P-Medium ⚡ A-state Area: State / database changes A-concurrency Area: Async code, needs extra work to make it work properly. A-compatibility Area: Compatibility with other nodes or wallets, or standard rules labels Aug 15, 2023
@arya2 arya2 requested review from upbqdn and oxarbitrage August 15, 2023 22:52
@arya2 arya2 self-assigned this Aug 15, 2023
@arya2 arya2 temporarily deployed to docs August 15, 2023 22:52 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 15, 2023 22:52 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 15, 2023 22:52 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented Aug 15, 2023 •
edited
Loading

Visit the preview URL for this PR (updated for commit aea9320):

https://zebra-docs-book--preview-0di6tduu.web.app

(expires Thu, 24 Aug 2023 22:59:27 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 8d460e315475673a76f0ab870fc114279ba7381c

@github-actions
Copy link

github-actions bot commented Aug 15, 2023 •
edited
Loading

Visit the preview URL for this PR (updated for commit aea9320):

https://zebra-docs-external--preview-t4y2orst.web.app

(expires Thu, 24 Aug 2023 23:00:33 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 705897b721e180be8dabb12f6fb28ccb185183d6

@github-actions
Copy link

github-actions bot commented Aug 15, 2023 •
edited
Loading

Visit the preview URL for this PR (updated for commit aea9320):

https://zebra-docs-internal--preview-brkz2hqb.web.app

(expires Thu, 24 Aug 2023 23:01:36 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 5a34f7d8d9b292211c2deeb6afaff9045ab060e0

@arya2 arya2 temporarily deployed to docs August 16, 2023 17:30 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 17:30 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 17:30 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 20:13 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 20:13 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 20:13 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 20:30 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 20:30 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 20:30 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 20:56 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 20:56 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 20:56 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 21:11 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 21:12 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 21:12 — with GitHub Actions Inactive
@arya2 arya2 temporarily deployed to docs August 16, 2023 21:40 — with GitHub Actions Inactive
@arya2 arya2 force-pushed the subtree-boundaries branch from 5170969 to e430154 Compare August 24, 2023 01:16
@arya2 arya2 changed the base branch from main to subtree-boundaries-zebra-chain August 24, 2023 01:16
@arya2 arya2 force-pushed the subtree-boundaries branch from e8d7bb0 to 8b53133 Compare August 25, 2023 18:18
@arya2 arya2 force-pushed the subtree-boundaries branch from 8b53133 to 7d90cfb Compare August 25, 2023 18:38
@arya2 arya2 requested a review from teor2345 August 25, 2023 18:58
@arya2 arya2 mentioned this pull request Aug 25, 2023
Closed
6 tasks
@arya2 arya2 changed the title 1. change(state): Add note subtree indexes 1. change(state): Add note subtree indexes to zebra-state Aug 25, 2023
Base automatically changed from subtree-boundaries-zebra-chain to main August 28, 2023 00:48
@teor2345 teor2345 removed the do-not-merge Tells Mergify not to merge this PR label Aug 28, 2023
@teor2345 teor2345 changed the title 1. change(state): Add note subtree indexes to zebra-state change(state): Add note subtree index handling to zebra-state, but don't write them to the finalized state yet Aug 28, 2023
teor2345
teor2345 approved these changes Aug 28, 2023
View reviewed changes
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, let's see if we can get this merged.

mergify bot added a commit that referenced this pull request Aug 28, 2023
@mergify
Merge of #7334
af904ac
@mergify mergify bot mentioned this pull request Aug 28, 2023
Closed
@mergify mergify bot merged commit 94d9155 into main Aug 28, 2023
@mergify mergify bot deleted the subtree-boundaries branch August 28, 2023 08:50
@upbqdn upbqdn mentioned this pull request Sep 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teor2345 teor2345 teor2345 approved these changes

@oxarbitrage oxarbitrage Awaiting requested review from oxarbitrage

@upbqdn upbqdn Awaiting requested review from upbqdn

Assignees

@arya2 arya2

Labels
A-compatibility Area: Compatibility with other nodes or wallets, or standard rules A-concurrency Area: Async code, needs extra work to make it work properly. A-state Area: State / database changes C-enhancement Category: This is an improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@arya2 @upbqdn @teor2345