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

Scan blocks with sapling keys and write the results to the database #8040

Merged
merged 14 commits into from
Dec 3, 2023

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Dec 1, 2023

Motivation

This PR scans the blockchain and adds the scanner results to the scanner database.

Close #8021

I think it might also
close #7953
but @upbqdn feel free to replace that code with your PR, and re-open that ticket if it's not finished.

PR Author Checklist

Check before marking the PR as ready for review:

  • Will the PR name make sense to users?
  • Does the PR have a priority label?
  • Have you added or updated tests?
  • Is the documentation up to date?
For significant changes:
  • Is there a summary in the CHANGELOG?
  • Can these changes be split into multiple PRs?

If a checkbox isn't relevant to the PR, mark it as done.

The changelog summary for 1.5.0 already talks about scanning.

Complex Code or Requirements

I tried scanning with all the keys to make sure it would work, but we only really need to scan with the full viewing key. I couldn't find the encoding for individual viewing keys, so I left that until later.

There's also a bunch of spawn_blocking() calls in here.

Solution

Changes:

  • Parse keys once at the start of the scan
    • Create a function that parses a key into a list of keys
    • Take a list of keys from the same key in the scanner for now
  • Pass a ChainTipChange to the scanner function, this turned out to be mostly useless but it's good for progress
  • Get a block from the state instead of the tip height
  • Use a dummy sapling tree size
  • Scan each block and add the results to storage
    • Convert a scanned block to a sapling result

Fixes:

  • Fix availability of tokio::time in scanner
  • Make it easier to pass keys and blocks to conversion functions
  • Increase scanner wait times
  • Don't log secret keys, only log every 100,000 blocks
  • Move blocking tasks into spawn_blocking()

Testing

  • Update the acceptance test

I manually tested this, getting it in CI is ticket #8037

Review

@oxarbitrage might want to review the scanner bits, and @upbqdn the block/state bits?

Reviewer Checklist

Check before approving the PR:

  • Does the PR scope match the ticket?
  • Are there enough tests to make sure it works? Do the tests cover the PR motivation?
  • Are all the PR blockers dealt with?
    PR blockers can be dealt with in new tickets or PRs.

And check the PR Author checklist is complete.

Follow Up Work

#8022 then lots of testing:

@teor2345 teor2345 added P-Medium ⚡ C-feature Category: New features A-blockchain-scanner Area: Blockchain scanner of shielded transactions labels Dec 1, 2023
@teor2345 teor2345 self-assigned this Dec 1, 2023
@teor2345 teor2345 requested a review from a team as a code owner December 1, 2023 04:58
@teor2345 teor2345 requested review from arya2 and removed request for a team December 1, 2023 04:58
@teor2345
Copy link
Contributor Author

teor2345 commented Dec 1, 2023

Seems to work in my manual tests, and quickly too:

2023-12-01T04:57:54.983238Z INFO {zebrad="850bd6a" net="Main"}: zebra_scan::scan: Scanning the blockchain: now at block Height(600000), current tip Some((Height(2315746), block::Hash("00000000017abab20936b148ac1ed54a38cb87019f7ac489baeab2db256ea59e")))
2023-12-01T04:57:54.983251Z INFO {zebrad="850bd6a" net="Main"}: zebra_scan::scan: Scanning the blockchain for key 0, started at block Height(419200)

This is only running on one core, and doing each key twice (dfvk and ivks).

@teor2345 teor2345 changed the title Scan blocks and write the results to the database Scan blocks with sapling keys and write the results to the database Dec 1, 2023
@oxarbitrage oxarbitrage requested review from upbqdn and oxarbitrage and removed request for arya2 December 1, 2023 12:43
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

utACK. Looks really good to me, i left a few questions and optionals.

zebra-scan/src/scan.rs Show resolved Hide resolved
zebra-scan/src/scan.rs Show resolved Hide resolved
zebrad/tests/acceptance.rs Show resolved Hide resolved
zebrad/tests/acceptance.rs Show resolved Hide resolved
zebra-scan/Cargo.toml Show resolved Hide resolved
zebra-scan/src/scan.rs Show resolved Hide resolved
zebra-scan/src/scan.rs Show resolved Hide resolved
@mergify mergify bot merged commit 4306a00 into main Dec 3, 2023
129 checks passed
@mergify mergify bot deleted the scan-write-db branch December 3, 2023 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-blockchain-scanner Area: Blockchain scanner of shielded transactions C-feature Category: New features
Projects
None yet
2 participants