Skip to content

[consensus/simplex+marshal] Auto-certify locally proposed views + Improve Durability Guarantees#3543

Merged
patrick-ogrady merged 111 commits into
mainfrom
cl/rm-double-certify
Apr 20, 2026
Merged

[consensus/simplex+marshal] Auto-certify locally proposed views + Improve Durability Guarantees#3543
patrick-ogrady merged 111 commits into
mainfrom
cl/rm-double-certify

Conversation

@clabby
Copy link
Copy Markdown
Collaborator

@clabby clabby commented Apr 6, 2026

Overview

Updates simplex to prevent the local node from asking the application to certify its own proposals. Currently, this causes re-execution of blocks when using standard::Deferred or coding::Marshaled in the certify fallback path.

TODO

  • store in verified or notarized? If in notarized already, should store in verified?

@clabby clabby self-assigned this Apr 6, 2026
@clabby clabby added the bug Something isn't working label Apr 6, 2026
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 6, 2026

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 8ae64d5 Apr 20 2026, 09:06 PM

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

cloudflare-workers-and-pages Bot commented Apr 6, 2026

Deploying monorepo with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8ae64d5
Status: ✅  Deploy successful!
Preview URL: https://331761a1.monorepo-eu0.pages.dev
Branch Preview URL: https://cl-rm-double-certify.monorepo-eu0.pages.dev

View logs

@clabby clabby changed the title [consensus/simplex] Prevent certificatiion of our own proposals [consensus/simplex] Auto-certify locally proposed views Apr 6, 2026
@clabby clabby force-pushed the cl/rm-double-certify branch from 01ad0fe to 376f477 Compare April 7, 2026 00:00
@patrick-ogrady patrick-ogrady added this to the v2026.3.1 milestone Apr 7, 2026
Comment thread consensus/src/simplex/actors/voter/actor.rs Outdated
@clabby clabby force-pushed the cl/rm-double-certify branch 2 times, most recently from 48a3a86 to 6196044 Compare April 7, 2026 01:08
Comment thread consensus/src/simplex/mocks/application.rs
@clabby clabby added this to Tracker Apr 7, 2026
@clabby clabby moved this to Ready for Review in Tracker Apr 7, 2026
@clabby clabby marked this pull request as ready for review April 7, 2026 01:33
@clabby clabby force-pushed the cl/rm-double-certify branch from b94d753 to 1beb6c2 Compare April 7, 2026 03:53
Comment thread consensus/src/simplex/actors/voter/mod.rs
@clabby clabby marked this pull request as draft April 7, 2026 15:22
@clabby
Copy link
Copy Markdown
Collaborator Author

clabby commented Apr 7, 2026

Pulling into draft while we reason about a more strict guarantee.

clabby added 10 commits April 7, 2026 21:06
We currently track if the proposal was local as its own flag,
derived from the leader. Right now this doesn't buy us much.
The original intention was to enable a more fool-proof setup
where the proposer's node would keep track even in the event
of crashes, but for now, this re-centers the PR on a solution
that relies on the leader being known.
@clabby clabby force-pushed the cl/rm-double-certify branch from 1beb6c2 to fc71931 Compare April 8, 2026 01:06
@patrick-ogrady
Copy link
Copy Markdown
Contributor

patrick-ogrady commented Apr 20, 2026

If we persist block during propose rather than in broadcast, it looks a LOT cleaner. Remove proposed/last_built, call verified and then call forward (with recipients) on broadcast from consensus?

@patrick-ogrady
Copy link
Copy Markdown
Contributor

patrick-ogrady commented Apr 20, 2026

I think there is one more subtle edge case where a leader restarts and persisted a block in marshal but not a vote in simplex. The leader then re-proposes on restart but the new block isn't written (same index is a no-op)?

What we want to do here is load a verified for that round and pass that back as digest (or don't propose at all)?

Comment thread consensus/src/marshal/mocks/harness.rs Outdated
Comment thread consensus/src/marshal/mocks/harness.rs Outdated
Comment thread consensus/src/marshal/mocks/harness.rs
Comment thread consensus/src/simplex/actors/voter/mod.rs
Comment thread consensus/src/marshal/core/actor.rs
Comment thread consensus/src/marshal/mocks/harness.rs Outdated
Comment thread consensus/src/marshal/standard/inline.rs Outdated
Comment thread consensus/src/simplex/actors/voter/mod.rs
Comment thread consensus/src/simplex/actors/voter/mod.rs
Comment thread consensus/src/marshal/coding/marshaled.rs
@patrick-ogrady
Copy link
Copy Markdown
Contributor

bugbot run

Comment thread consensus/src/marshal/standard/inline.rs
@patrick-ogrady
Copy link
Copy Markdown
Contributor

bugbot run

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8ae64d5. Configure here.

@patrick-ogrady patrick-ogrady merged commit 455820a into main Apr 20, 2026
178 checks passed
@patrick-ogrady patrick-ogrady deleted the cl/rm-double-certify branch April 20, 2026 22:49
@github-project-automation github-project-automation Bot moved this from Ready for Review to Done in Tracker Apr 20, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

❌ Patch coverage is 95.89540% with 124 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.87%. Comparing base (d2e7d69) to head (8ae64d5).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
consensus/src/marshal/standard/mod.rs 93.93% 23 Missing and 5 partials ⚠️
consensus/src/marshal/coding/marshaled.rs 51.11% 19 Missing and 3 partials ⚠️
consensus/src/marshal/mocks/harness.rs 98.08% 17 Missing ⚠️
consensus/src/marshal/standard/inline.rs 95.73% 13 Missing and 3 partials ⚠️
consensus/src/marshal/standard/deferred.rs 93.43% 9 Missing and 4 partials ⚠️
consensus/src/marshal/mocks/verifying.rs 82.97% 7 Missing and 1 partial ⚠️
p2p/src/authenticated/discovery/network.rs 0.00% 5 Missing ⚠️
p2p/src/authenticated/lookup/network.rs 0.00% 5 Missing ⚠️
consensus/src/simplex/engine.rs 50.00% 3 Missing ⚠️
consensus/src/marshal/coding/mod.rs 99.43% 1 Missing and 1 partial ⚠️
... and 4 more
@@            Coverage Diff             @@
##             main    #3543      +/-   ##
==========================================
+ Coverage   95.81%   95.87%   +0.06%     
==========================================
  Files         440      440              
  Lines      167647   172053    +4406     
  Branches     3909     4002      +93     
==========================================
+ Hits       160630   164958    +4328     
- Misses       5798     5827      +29     
- Partials     1219     1268      +49     
Files with missing lines Coverage Δ
consensus/src/lib.rs 0.00% <ø> (ø)
consensus/src/marshal/application/validation.rs 100.00% <100.00%> (ø)
...nsus/src/marshal/application/verification_tasks.rs 100.00% <100.00%> (+42.10%) ⬆️
consensus/src/marshal/coding/variant.rs 92.30% <ø> (-7.70%) ⬇️
consensus/src/marshal/core/cache.rs 93.45% <100.00%> (+0.19%) ⬆️
consensus/src/marshal/core/mailbox.rs 100.00% <100.00%> (+7.75%) ⬆️
consensus/src/marshal/mod.rs 54.54% <ø> (ø)
consensus/src/marshal/resolver/handler.rs 94.05% <100.00%> (+0.39%) ⬆️
consensus/src/simplex/actors/batcher/actor.rs 93.58% <100.00%> (ø)
consensus/src/simplex/actors/voter/actor.rs 97.19% <100.00%> (-0.41%) ⬇️
... and 22 more

... and 21 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 d2e7d69...8ae64d5. 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.

crazywriter1 pushed a commit to crazywriter1/tempo that referenced this pull request Apr 23, 2026
…#3697)

Updates all commonware dependencies to the most recent head at
commonwarexyz/monorepo@2a7dd42.

This notably contains the patches
commonwarexyz/monorepo#3543 and
commonwarexyz/monorepo#3641 to improve
durability and persistence of proposed and verified blocks in the
marshal actor.

In certain scenarios (especially around epoch transitions, but also in
single-node tests), there were races between simplex engines asking for
new proposals (overwriting the last-proposed-block mutex), and
broadcasts of proposal parents.

This patch removes this logic, instead relying on the marshal actor to
provide the blocks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants