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.

Sign up for GitHub

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

Jump to bottom

rfc: Parallel Verification #763

Merged
merged 25 commits into from
Aug 14, 2020
Merged

rfc: Parallel Verification #763

merged 25 commits into from
Aug 14, 2020

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Jul 27, 2020 •
edited
Loading

An initial draft RFC for parallel verification.

Rendered

TODO:

@teor2345 teor2345 added A-docs Area: Documentation Poll::Ready C-design Category: Software design work A-consensus Area: Consensus rule updates labels Jul 27, 2020
@teor2345 teor2345 requested a review from a team July 27, 2020 03:45
@teor2345 teor2345 self-assigned this Jul 27, 2020
This was referenced Jul 27, 2020
Closed
Closed
@teor2345

This comment has been minimized.

Sign in to view
@teor2345 teor2345 force-pushed the rfc-parallel-verification branch 2 times, most recently from 60841b0 to 26e9d89 Compare July 27, 2020 06:07
@teor2345
rfc: Parallel Verification Draft
d2b2841 
An initial draft RFC for parallel verification.
@teor2345 teor2345 force-pushed the rfc-parallel-verification branch from 26e9d89 to b657cf5 Compare July 27, 2020 07:31
@teor2345
rfc: Integrate the CheckpointVerifier design
a237763 
Describe how the CheckpointVerifier interacts with chain state updates.
@teor2345 teor2345 force-pushed the rfc-parallel-verification branch from b657cf5 to a237763 Compare July 27, 2020 08:03
@yaahc yaahc mentioned this pull request Jul 27, 2020
Closed
18 tasks
@teor2345

This comment has been minimized.

Sign in to view
@teor2345 teor2345 mentioned this pull request Jul 29, 2020
Closed
6 tasks
@daira

This comment has been minimized.

Sign in to view
@teor2345

This comment has been minimized.

Sign in to view
This was referenced Jul 29, 2020
Merged
Closed
@yaahc

This comment has been minimized.

Sign in to view
@teor2345 teor2345 requested review from a team and yaahc August 13, 2020 00:04
yaahc
yaahc previously approved these changes Aug 13, 2020
View reviewed changes
Copy link
Contributor

@yaahc yaahc left a comment

Choose a reason for hiding this comment

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

LGTM, also I know we haven't discussed this but I think for RFC PRs we should require that everyone on the zebra team signs off on the PR (or abstains from voting if its not relevant to what they're working on) before merging.

@yaahc yaahc requested review from dconnolly and hdevalence August 13, 2020 00:31
@teor2345
Copy link
Contributor Author

LGTM, also I know we haven't discussed this but I think for RFC PRs we should require that everyone on the zebra team signs off on the PR (or abstains from voting if its not relevant to what they're working on) before merging.

Can we automate this check?

@teor2345
Copy link
Contributor Author

LGTM, also I know we haven't discussed this but I think for RFC PRs we should require that everyone on the zebra team signs off on the PR (or abstains from voting if its not relevant to what they're working on) before merging.

Can we automate this check?

It doesn't look like it:

  • the required number of reviews is per-protected branch
  • the CODEOWNERS file requires any owner, it doesn't support multiple required approvals

@yaahc
Copy link
Contributor

yaahc commented Aug 13, 2020

@teor2345 we could setup rfcbot like this rust-lang/rfcs#2965 (comment)

@hdevalence
Copy link
Contributor

I'd be OK with maintaining a social convention for this, given the small size of our team.

@teor2345
Copy link
Contributor Author

I'd be OK with maintaining a social convention for this, given the small size of our team.

In the absence of GitHub integration, I'm also happy with a social convention.
(I just like low-overhead reminders.)

@hdevalence hdevalence mentioned this pull request Aug 13, 2020
Merged
hdevalence
hdevalence previously requested changes Aug 13, 2020
View reviewed changes
Copy link
Contributor

@hdevalence hdevalence left a comment

Choose a reason for hiding this comment

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

This looks good and is well-scoped! I would like to try to keep using the structural/semantic/contextual language we used before. Using "contextual verification" rather than "state update" means that we stay focused on the verification aspect rather than on all the other details of state updates.

@teor2345
rfc: Delete coinbase checks which are unrelated to BlockHeight
626065a 
And remove some duplicate references to BlockHeight checks.
@teor2345
rfc: Focus on verification stages
fb53bb8 
And rewrite some stages for clarity.
@teor2345
rfc: Remove reference to zebra-network
ffefee3
@teor2345 teor2345 requested a review from hdevalence August 13, 2020 20:59
@hdevalence hdevalence dismissed their stale review August 14, 2020 18:03

changes were made

@hdevalence hdevalence merged commit 120c7ef into main Aug 14, 2020
@hdevalence hdevalence deleted the rfc-parallel-verification branch August 14, 2020 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daira daira daira left review comments

@yaahc yaahc yaahc approved these changes

@dconnolly dconnolly Awaiting requested review from dconnolly

@hdevalence hdevalence Awaiting requested review from hdevalence

Assignees

@teor2345 teor2345

Labels
A-consensus Area: Consensus rule updates A-docs Area: Documentation C-design Category: Software design work
Projects
None yet
Milestone
Block Validation
Development

Successfully merging this pull request may close these issues.
4 participants
@teor2345 @daira @yaahc @hdevalence