Skip to content

fix(driver): schedule timeout upon CommitCertificate for invalid value#1481

Merged
romac merged 7 commits intocirclefin:mainfrom
cason:commit-certificate-timeout
Feb 25, 2026
Merged

fix(driver): schedule timeout upon CommitCertificate for invalid value#1481
romac merged 7 commits intocirclefin:mainfrom
cason:commit-certificate-timeout

Conversation

@cason
Copy link
Contributor

@cason cason commented Feb 17, 2026

Closes: #1482

Minor fix for the logic commented below, which is not applied if the proposal is not valid, only if not found:

} else {
// Proposal exists but validity is not confirmed, start precommit timer
return Some(RoundInput::PrecommitAny);
}

Test unit updated in order to fail in the case a CommitCertificate is received for an invalid value v.

@cason cason requested review from ancazamfir, nenadmilosevic95 and romac and removed request for ancazamfir and romac February 17, 2026 09:10
@github-actions

This comment was marked as outdated.

@github-actions github-actions bot added the need-triage This issue needs to be triaged label Feb 17, 2026
@github-actions github-actions bot closed this Feb 17, 2026
@romac romac reopened this Feb 17, 2026
@romac romac removed the need-triage This issue needs to be triaged label Feb 17, 2026
match self.store_and_multiplex_commit_certificate(certificate) {
Some(round_input) => self.apply_input(round, round_input),
None => Ok(None),
None => Ok(None), // FIXME: Any scenario where this can happen?
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can now change the return type of store_and_multiplex_commit_certificate to RoundInput<Ctx> and simplify this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@nenadmilosevic95 nenadmilosevic95 left a comment

Choose a reason for hiding this comment

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

LGTM! Nice catch!

@romac romac added this pull request to the merge queue Feb 25, 2026
Merged via the queue into circlefin:main with commit e819677 Feb 25, 2026
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

core: schedule Precommit timeout upon CommitCertificate for invalid value

3 participants