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

fix(tests): Add an integration test for error conversions from zebra-state to zebra-consensus #6665

Merged
merged 1 commit into from
May 16, 2023

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented May 11, 2023

Motivation

The transaction verifier in zebra-consensus converts and propagates boxed ValidateContextErrors from zebra-state, but is missing unit tests.

Closes #6399.

Solution

  • Add a unit test in zebra-consensus that shows errors from the state service propagate correctly through the transaction verifier

Review

Anyone can review.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

@arya2 arya2 added C-enhancement Category: This is an improvement P-Low ❄️ C-testing Category: These are tests C-audit Category: Issues arising from audit findings labels May 11, 2023
@arya2 arya2 requested a review from a team as a code owner May 11, 2023 17:36
@arya2 arya2 self-assigned this May 11, 2023
@arya2 arya2 requested review from upbqdn and removed request for a team May 11, 2023 17:36
@github-actions github-actions bot added C-bug Category: This is a bug C-feature Category: New features labels May 11, 2023
@codecov
Copy link

codecov bot commented May 11, 2023

Codecov Report

Merging #6665 (1219724) into main (25ca4cc) will increase coverage by 0.11%.
The diff coverage is 91.25%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6665      +/-   ##
==========================================
+ Coverage   77.98%   78.09%   +0.11%     
==========================================
  Files         309      309              
  Lines       40664    40743      +79     
==========================================
+ Hits        31711    31819     +108     
+ Misses       8953     8924      -29     

@arya2 arya2 added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label May 11, 2023
@mpguerra mpguerra mentioned this pull request May 11, 2023
36 tasks
mergify bot added a commit that referenced this pull request May 15, 2023
@arya2
Copy link
Contributor Author

arya2 commented May 16, 2023

@mergify refresh

@mergify
Copy link
Contributor

mergify bot commented May 16, 2023

refresh

✅ Pull request refreshed

mergify bot added a commit that referenced this pull request May 16, 2023
@arya2
Copy link
Contributor Author

arya2 commented May 16, 2023

@arya2
Copy link
Contributor Author

arya2 commented May 16, 2023

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented May 16, 2023

refresh

✅ Pull request refreshed

mergify bot added a commit that referenced this pull request May 16, 2023
@teor2345
Copy link
Contributor

Another unrelated test failure:

ZcashFoundation/zebra/actions/runs/4993544630/jobs/8942826854#step:14:3930

This seems to be a peer failure, but maybe "all ready peers are missing inventory" shouldn't hang for 4 minutes before trying again.

@mergify mergify bot merged commit 3779eb1 into main May 16, 2023
@mergify mergify bot deleted the test-box-error-downcasts branch May 16, 2023 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-audit Category: Issues arising from audit findings C-bug Category: This is a bug C-enhancement Category: This is an improvement C-feature Category: New features 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.

Show that duplicate nullifier errors are propagated through the transaction verifier
3 participants