Skip to content

[consensus] Inferred Certification#3647

Draft
patrick-ogrady wants to merge 6 commits into
mainfrom
inferred-certification
Draft

[consensus] Inferred Certification#3647
patrick-ogrady wants to merge 6 commits into
mainfrom
inferred-certification

Conversation

@patrick-ogrady
Copy link
Copy Markdown
Contributor

Related: #3433

Open question: should we emit a finalize vote if we infer certification (we only emit a notarize vote if we explicitly verify but the only way that is cancelled is with a notarization...with this, a future notarization can cause us to infer so there is a chance we don't form finalizations)?

@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
commonware-mcp 63753df Apr 21 2026, 10:02 PM

@0xAysh
Copy link
Copy Markdown
Contributor

0xAysh commented Apr 23, 2026

I think yes we should emit finalization vote. With current code finalization path does not require a view to be explicitly verified (it just reqiures the view to not be already finalized/nullified locally, and to have natorization+certificateation for the view). So if inference marks a view as certified, emitting finalization certification seems consistent and helps liveness. I also just thought to call verify fn on inferred views but try_verify() is scoped only for current views not for ancestors and we deviate from the core problem if we try to adjust the scope of it. Also I skimmed your implementation and would love to understand your approach and thought process.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026

Codecov Report

❌ Patch coverage is 94.33657% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.86%. Comparing base (598246c) to head (63753df).
⚠️ Report is 69 commits behind head on main.

Files with missing lines Patch % Lines
consensus/src/simplex/actors/voter/mod.rs 88.06% 18 Missing and 11 partials ⚠️
consensus/src/simplex/actors/voter/actor.rs 95.52% 1 Missing and 2 partials ⚠️
consensus/src/simplex/actors/voter/state.rs 99.00% 2 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##             main    #3647      +/-   ##
==========================================
- Coverage   95.88%   95.86%   -0.03%     
==========================================
  Files         441      442       +1     
  Lines      172366   171965     -401     
  Branches     4006     4016      +10     
==========================================
- Hits       165276   164853     -423     
- Misses       5824     5845      +21     
- Partials     1266     1267       +1     
Files with missing lines Coverage Δ
consensus/src/simplex/actors/voter/round.rs 99.51% <100.00%> (-0.37%) ⬇️
consensus/src/simplex/actors/voter/actor.rs 97.90% <95.52%> (+0.26%) ⬆️
consensus/src/simplex/actors/voter/state.rs 98.81% <99.00%> (-0.01%) ⬇️
consensus/src/simplex/actors/voter/mod.rs 91.69% <88.06%> (+0.01%) ⬆️

... and 39 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 598246c...63753df. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants