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

suggestion: Use the same ChainVerifier for downloaded and submitted blocks #5533

Merged
merged 5 commits into from
Nov 2, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Nov 2, 2022

Motivation

Zebra isn't really designed to use multiple instances of the same verifiers. It might work, but it is easier to design, test, and maintain if we just have one running verifier of each kind.

This is a suggestion for PR #5526.

Solution

See the explanations I'm going to post on each part of the PR.

Review

@arya2 wrote the original PR.

I'm happy to revert any of these changes that aren't needed - let me know what you think!

Reviewer Checklist

  • Does the code do what the ticket and PR says?
  • How do you know it works? Does it have tests?

@teor2345 teor2345 added C-enhancement Category: This is an improvement A-rpc Area: Remote Procedure Call interfaces labels Nov 2, 2022
@teor2345 teor2345 requested a review from arya2 November 2, 2022 04:29
@teor2345 teor2345 self-assigned this Nov 2, 2022
@teor2345 teor2345 requested a review from a team as a code owner November 2, 2022 04:29
@teor2345 teor2345 changed the base branch from main to submitblock November 2, 2022 04:30
@teor2345 teor2345 requested review from a team as code owners November 2, 2022 04:30
@teor2345 teor2345 requested review from dconnolly and removed request for a team November 2, 2022 04:30
@github-actions github-actions bot added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Nov 2, 2022
@teor2345 teor2345 force-pushed the submitblock-chain-verifier branch from 38464e0 to 634fff1 Compare November 2, 2022 04:33
@codecov
Copy link

codecov bot commented Nov 2, 2022

Codecov Report

Merging #5533 (7cbd1f8) into submitblock (82ffa42) will increase coverage by 0.02%.
The diff coverage is 90.00%.

Additional details and impacted files
@@               Coverage Diff               @@
##           submitblock    #5533      +/-   ##
===============================================
+ Coverage        79.25%   79.28%   +0.02%     
===============================================
  Files              305      305              
  Lines            37884    37879       -5     
===============================================
+ Hits             30026    30031       +5     
+ Misses            7858     7848      -10     

Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

These are good changes and decisions, thank you!

@arya2 arya2 merged commit 6cd8456 into submitblock Nov 2, 2022
@arya2 arya2 deleted the submitblock-chain-verifier branch November 2, 2022 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Area: Remote Procedure Call interfaces C-enhancement Category: This is an improvement 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.

2 participants