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

feat(config): Allow to add keys to be scanned by the zebra-scan crate to config #7949

Merged
merged 3 commits into from
Nov 16, 2023

Conversation

oxarbitrage
Copy link
Contributor

Motivation

We want the user to add sapling keys to the config file so Zebra can scan the blockchain with them.

Close #7941

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?

Solution

Add a config type to the zebra-scan crate with a sapling_keys_to_scan field of type IndexMap<String, u32> where the string is the key and the number the birthday height.

This field is used as:

[scan.sapling_keys_to_scan]
"somekey" = 1
"someotherkey" = 100000

The configuration is behind a zebra-scan rust feature so build should be done as follows to see the new section:

$ cargo build --features="zebra-scan"

Testing

This PR has been tested manually.

Review

This is a very basic version introducing the less amount of code possible, open to make changes.

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

@oxarbitrage oxarbitrage requested a review from a team as a code owner November 15, 2023 19:21
@oxarbitrage oxarbitrage requested review from upbqdn and removed request for a team November 15, 2023 19:21
@github-actions github-actions bot added the C-feature Category: New features label Nov 15, 2023
@oxarbitrage oxarbitrage added P-Medium ⚡ A-blockchain-scanner Area: Blockchain scanner of shielded transactions labels Nov 15, 2023
@oxarbitrage oxarbitrage self-assigned this Nov 15, 2023
zebra-scan/src/config.rs Outdated Show resolved Hide resolved
teor2345
teor2345 previously approved these changes Nov 15, 2023
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 great!

Adding zebra-scan as a dependency in production zebrad builds might be a blocker. Everything else is optional, I've tried to move as much work out of the MVP and onto the tracking issue as possible.

zebra-scan/src/config.rs Outdated Show resolved Hide resolved
zebrad/Cargo.toml Outdated Show resolved Hide resolved
zebra-scan/src/config.rs Outdated Show resolved Hide resolved
zebrad/Cargo.toml Outdated Show resolved Hide resolved
zebrad/src/config.rs Outdated Show resolved Hide resolved
zebrad/src/config.rs Outdated Show resolved Hide resolved
Co-authored-by: teor <teor@riseup.net>
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 to me, thanks for the quick changes

@teor2345
Copy link
Contributor

@mergify mergify bot merged commit a22c8d5 into main Nov 16, 2023
105 checks passed
@mergify mergify bot deleted the issue7941 branch November 16, 2023 01:32
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
Development

Successfully merging this pull request may close these issues.

feat(config): Add keys to be scanned to Zebra config file
2 participants