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

add getblock RPC verbosity = 3 for lightwalletd #6747

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LarryRuane
Copy link
Collaborator

@LarryRuane LarryRuane commented Aug 9, 2023

This implements a new verbosity level=3 for getblock that returns exactly what lightwalletd needs to fetch a block:

  • full block in raw hex form
  • list of txids
  • block hash

These are all returned by other verbosity levels; this is just a new combination of them. Without this combination of result items, lightwalletd needs to do two RPC calls for every block it fetches.

There is no unit test yet but it would be easy to write one.

See related lightwalletd issue zcash/lightwalletd#392 and PR zcash/lightwalletd#449 (which makes use of this PR's new functionality). These two PRs can be merged and deployed in either order.

@LarryRuane
Copy link
Collaborator Author

Rebased, also fixed unit test.

This implements a new verbosity level for `getblock`, 3, that returns
exactly what lightwalletd needs to fetch a block:

 - full block in raw hex form
 - list of txids
 - block hash

These are all returned by other verbosity levels; this is just a new
combination of them. Without this combination of result items,
lightwalletd needs to do two RPC calls for every block it fetches.

There is no unit test yet but it would be easy to write one.

See related lightwalletd issue 392.
@LarryRuane
Copy link
Collaborator Author

Rebased on the latest master commit (2112e46) and made a proper performance measurement. Syncing lightwalletd from scratch using an optimized-build already-synced zcashd on my Ubuntu desktop, without this PR and the corresponding lightwalletd PR, that is, latest master branch for both, took 48003 seconds (13 hrs 20 min 3 sec). With both PRs, the time was 31713 seconds (8 hrs 48 min 33 sec), which is 66% of the time (or to say it the other way around, without these PRs, the time is 1.51 times greater). This is intuitively reasonable, since with this PR, lightwalletd is doing only half the number of getblock RPCs.

I've heard some complaints about lightwalletd sync time (I don't have references, it was some time ago), so I think the performance improvement justifies the additional code complexity (which is fairly minimal; it's just putting together existing functionality in a slightly different way).

@LarryRuane LarryRuane requested a review from daira March 27, 2024 19:36
@LarryRuane
Copy link
Collaborator Author

It looks like the one CI failure is unrelated to this PR.

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

Successfully merging this pull request may close these issues.

None yet

2 participants