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

poc(scanner): add a populated state test for ZECpages viewing key #7916

Merged
merged 42 commits into from
Nov 8, 2023

Conversation

oxarbitrage
Copy link
Contributor

Motivation

As part of the proof of concept we want to make sure we can scan for the ZECpages viewing key even if we get no responses scanning the test data.

Close #7903

Solution

Add a test for the mentioned use case.

Review

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
Copy link
Contributor Author

@arya2 can you push a commit to the branch ?

arya2
arya2 previously approved these changes Nov 7, 2023
@oxarbitrage oxarbitrage added P-Medium ⚡ C-testing Category: These are tests A-blockchain-scanner Area: Blockchain scanner of shielded transactions labels Nov 7, 2023
zebra-scan/src/tests.rs Outdated Show resolved Hide resolved
@oxarbitrage
Copy link
Contributor Author

So, we can merge this to issue7727 or merge #7758 and then rebase this to main.

@arya2
Copy link
Contributor

arya2 commented Nov 7, 2023

So, we can merge this to issue7727 or merge #7758 and then rebase this to main.

Merging #7758 first and rebasing off main would give us smaller PRs, but these are tests. Either works.

@oxarbitrage
Copy link
Contributor Author

Yea, i think @teor2345 will have to approve #7758 as we both worked on it, so ill let them decide.

zebra-scan/src/tests.rs Outdated Show resolved Hide resolved
zebra-scan/src/tests.rs Outdated Show resolved Hide resolved
zebra-scan/src/tests.rs Outdated Show resolved Hide resolved
zebra-scan/src/tests.rs Outdated Show resolved Hide resolved
teor2345
teor2345 previously approved these changes Nov 8, 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.

Thanks! I'm not sure how the merge order works here, let's see if I can just approve it and Mergify works it out.

Base automatically changed from issue7727 to main November 8, 2023 22:05
@mergify mergify bot dismissed stale reviews from teor2345 and arya2 November 8, 2023 22:05

The base branch was changed.

deny.toml Outdated Show resolved Hide resolved
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.

Ok that merge looks better

@mergify mergify bot merged commit e5e89ec into main Nov 8, 2023
103 checks passed
@mergify mergify bot deleted the issue7903 branch November 8, 2023 23:46
@arya2 arya2 mentioned this pull request Nov 29, 2023
44 tasks
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-testing Category: These are tests C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test(scan): Scan the first 10 blocks with the ZECpages viewing key
3 participants