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

8. feat(state): add a query function for transparent address balances #4097

Merged
merged 12 commits into from
Apr 14, 2022
Merged

8. feat(state): add a query function for transparent address balances #4097

merged 12 commits into from
Apr 14, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Apr 12, 2022 •
edited
Loading

Motivation

This PR adds a state query function for the get_address_balance RPC in ticket #3157.

We still need to add a state request and a RPC method.

Specifications

RPC:

But only the arguments and fields in ticket #3157.

Designs

There are a few tricky parts of this PR:

  • making sure the balances from finalized state are consistent with each other
  • combining the finalized and non-finalized balances

We could try to hold a database read snapshot, but that could cause locking issues. Instead, I just retry a few times if the balance might have changed. (If the query fails for this reason, Zebra is still syncing, and the results would probably be wrong anyway.)

Solution

  • Add a query function for address balances
    • Add a query method for non-finalized address balance changes
    • Add a query method for finalized state address balances
  • Retry interrupted finalized balance queries
  • Pop chain root blocks until it matches the finalized tip

Related changes:

  • Make address index types consistent
  • Simplify non-finalized address index updates
    • Update snapshots for address index queries
  • Simplify non-finalized UTXO query

Review

I think @jvff is writing the RPC side of this query.

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

Do the state queries for the other address RPCs.

@teor2345 teor2345 added C-enhancement Category: This is an improvement P-High 🔥 A-state Area: State / database changes lightwalletd any work associated with lightwalletd labels Apr 12, 2022
@teor2345 teor2345 requested a review from a team as a code owner April 12, 2022 04:43
@teor2345 teor2345 self-assigned this Apr 12, 2022
@teor2345 teor2345 requested review from oxarbitrage and jvff and removed request for a team and oxarbitrage April 12, 2022 04:43
@teor2345 teor2345 changed the title feat(state): add a query function for transparent address balances 8. feat(state): add a query function for transparent address balances Apr 13, 2022
@teor2345 teor2345 requested review from a team as code owners April 13, 2022 00:44
@teor2345 teor2345 requested review from gustavovalverde and upbqdn and removed request for a team April 13, 2022 00:44
@teor2345 teor2345 changed the base branch from addr-index-base to db-tx-loc-by-taddr-loc April 13, 2022 00:44
@teor2345 teor2345 removed request for a team, gustavovalverde and upbqdn April 13, 2022 00:45
@teor2345 teor2345 force-pushed the db-tx-loc-by-taddr-loc branch from b8cdc92 to 1059362 Compare April 13, 2022 05:03
@teor2345 teor2345 mentioned this pull request Apr 13, 2022
Merged
3 tasks
Base automatically changed from db-tx-loc-by-taddr-loc to main April 13, 2022 23:48
jvff
jvff previously approved these changes Apr 14, 2022
View reviewed changes
Copy link
Contributor

@jvff jvff left a comment

Choose a reason for hiding this comment

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

Looks great! 🎉

@teor2345
Copy link
Contributor Author

Full validation sync failed due to #3991, but we stopped requiring that check due to #4114:
https://github.com/ZcashFoundation/zebra/runs/6017141629?check_suite_focus=true#step:9:1635

@teor2345
Copy link
Contributor Author

@Mergifyio update

@teor2345
Copy link
Contributor Author

Mergify hung on "evaluating rules" for 90 minutes:
https://github.com/ZcashFoundation/zebra/pull/4097/checks?check_run_id=6016785912

@teor2345
Copy link
Contributor Author

@Mergifyio refresh

@teor2345 teor2345 requested a review from jvff April 14, 2022 07:20
@teor2345
Copy link
Contributor Author

Full validation sync failed due to #3991:
https://github.com/ZcashFoundation/zebra/runs/6018158375?check_suite_focus=true#step:9:7199

I'll re-run the job.

@teor2345
Copy link
Contributor Author

teor2345 commented Apr 14, 2022 •
edited
Loading

Now it's failing with #4114, sigh

At least that job isn't required any more.

mergify bot added a commit that referenced this pull request Apr 14, 2022
@mergify
Merge of #4097
6574a02
@mergify mergify bot mentioned this pull request Apr 14, 2022
Closed
36 tasks
@mergify mergify bot merged commit 8e29219 into main Apr 14, 2022
@mergify mergify bot deleted the addr-index-query branch April 14, 2022 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jvff jvff jvff approved these changes

Assignees

@teor2345 teor2345

Labels
A-state Area: State / database changes C-enhancement Category: This is an improvement lightwalletd any work associated with lightwalletd
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@teor2345 @jvff