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

Introduce failed_certificates field in candidate block #1137

Merged
merged 50 commits into from
Dec 1, 2023
Merged

Introduce failed_certificates field in candidate block #1137

merged 50 commits into from
Dec 1, 2023

Conversation

goshawk-3
Copy link
Contributor

fixes #1116

@goshawk-3 goshawk-3 changed the title Failed iterations Introduce failed_certificates field in candidate block Nov 15, 2023
goshawk added 18 commits November 15, 2023 11:22
consensus: Rename FinalResult to Ready and Result to Pending in Handl…
8be1672 
…eMsgOutput enum
consensus: Collect any reduction message from former iteration in a s…
a5c23c1 
…eparate method
consensus: Enable NIL voting for 2nd reduction
c9e4cc7
consensus: Wrap up send_agreement function into AgreementSender struct
1679187
consensus: Send an agreement msg if quorum of a former iteration is r…
584bdba 
…eached
node-data: Impl failed_iterations field of type IterationsInfo in Blo…
c51f350 
…ck::Header
consensus: Use nil_quorum fn for empty hash
36abc52
consensus: Pass failed_iterations in generate_block fn
b9a80b4
node: Verify failed iterations list
885c970
consensus: Iterations greater than RELAX_ITERATION_THRESHOLD shall no…
9bc09d5 
…t be included into failed_iterations
consensus: Refactor step_votes_registry:
d0188f8 
- Rename SvEntry to AgreementInfo
- Use Ledger::Certificate in AgreementInfo
- Rename StepVotesRegistry to AgreementInfoRegistry
node-data: Make Certificate Copyable
35a97c4
consensus: Add flags to mark quorum_reached state
b72f74f
consensus: Record signatures with/without quorum
9c265c1
consensus: Aggregator always returns quorum_reached flag with current…
35a1c0a 
… step_votes
consensus: Make quorum_check optional for verify_votes fn
86a0560
node: Enable quorum check for both winner cert and prev_block_cert. D…
c447f22 
…isable it for failed_certificates check
consensus: Resolve unit-test failure
c14e0c6
@goshawk-3 goshawk-3 marked this pull request as ready for review November 23, 2023 10:20
fed-franz
fed-franz requested changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@fed-franz fed-franz left a comment

Choose a reason for hiding this comment

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

Overall LGTM, just a few minor comments.

But I'd like a clarification on collect_from_past before approving.

consensus/src/firststep/handler.rs Show resolved Hide resolved
@goshawk-3 goshawk-3 requested a review from fed-franz November 29, 2023 10:52
herr-seppia
herr-seppia requested changes Nov 30, 2023
View reviewed changes
Copy link
Member

@herr-seppia herr-seppia left a comment

Choose a reason for hiding this comment

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

LGTM overall, but I've some doubt that need to be addressed

consensus/src/aggregator.rs Outdated Show resolved Hide resolved
consensus/src/agreement/verifiers.rs Outdated Show resolved Hide resolved
consensus/src/commons.rs Show resolved Hide resolved
consensus/src/firststep/handler.rs Outdated Show resolved Hide resolved
consensus/src/firststep/handler.rs Show resolved Hide resolved
consensus/src/secondstep/handler.rs Show resolved Hide resolved
consensus/src/secondstep/step.rs Show resolved Hide resolved
consensus/src/selection/handler.rs Show resolved Hide resolved
consensus/src/step_votes_reg.rs Outdated Show resolved Hide resolved
node-data/src/encoding.rs Show resolved Hide resolved
fed-franz
fed-franz approved these changes Nov 30, 2023
View reviewed changes
Copy link
Contributor

@fed-franz fed-franz left a comment

Choose a reason for hiding this comment

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

LGTM

@goshawk-3 goshawk-3 requested a review from herr-seppia December 1, 2023 09:26
@herr-seppia herr-seppia mentioned this pull request Dec 1, 2023
Closed
@goshawk-3 goshawk-3 merged commit 5ae8540 into master Dec 1, 2023
3 checks passed
@goshawk-3 goshawk-3 deleted the failed_iterations branch December 1, 2023 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@herr-seppia herr-seppia herr-seppia approved these changes

@fed-franz fed-franz fed-franz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add failed_iterations fields
3 participants
@goshawk-3 @fed-franz @herr-seppia