Skip to content

[consensus/marshal] commonware-glue marshal changes#3764

Merged
patrick-ogrady merged 3 commits into
mainfrom
cl/glue-marshal-changes
May 22, 2026
Merged

[consensus/marshal] commonware-glue marshal changes#3764
patrick-ogrady merged 3 commits into
mainfrom
cl/glue-marshal-changes

Conversation

@clabby
Copy link
Copy Markdown
Collaborator

@clabby clabby commented May 12, 2026

Overview

Ports the changes to marshal from #3381:

  • Add a GetProcessedHeight function to marshal's mailbox, so that the startup routine can query marshal's processed height to determine if it needs to roll back one or more databases.

@clabby clabby self-assigned this May 12, 2026
@clabby clabby added this to Tracker May 12, 2026
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 12, 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 d99635d May 22 2026, 07:48 PM

@clabby clabby force-pushed the cl/glue-marshal-changes branch from 6ed2e53 to f72fb17 Compare May 12, 2026 16:10
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 12, 2026

Deploying monorepo with  Cloudflare Pages  Cloudflare Pages

Latest commit: d99635d
Status: ✅  Deploy successful!
Preview URL: https://12cfe1dc.monorepo-eu0.pages.dev
Branch Preview URL: https://cl-glue-marshal-changes.monorepo-eu0.pages.dev

View logs

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 12, 2026

Benchmark results

Tip

PASSED: No benchmark exceeded the regression threshold.

Benchmark comparison table
Benchmark Baseline (main) Current Delta Threshold Status
qmdb::merkleize/variant=any::unordered::fixed::mmr keys=10000 ch=false sync=false 1.577 ms 1.604 ms +1.71% 10.00% ✅ PASS
qmdb::merkleize/variant=current::ordered::fixed::mmb chunk=256 keys=10000 ch=true sync=false 3.085 ms 3.112 ms +0.85% 10.00% ✅ PASS

Baseline commit(s): 527763b5f80a

Comment thread consensus/src/marshal/ancestry.rs Outdated
@clabby clabby force-pushed the cl/glue-marshal-changes branch from f72fb17 to 298de8d Compare May 13, 2026 18:30
BrendanChou
BrendanChou previously approved these changes May 13, 2026
Comment thread consensus/src/marshal/mocks/harness.rs
Comment thread consensus/src/marshal/core/actor.rs Outdated
},
Recipients::One(peer) => {
let peers = NonEmptyVec::new(peer);
resolver.fetch_targeted(request, peers).await;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we also change the API of resolver to use Recipients? I guess not (?)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

My thinking was no, since it's generic over the transport?

BrendanChou
BrendanChou previously approved these changes May 13, 2026
@clabby clabby force-pushed the cl/glue-marshal-changes branch from 833f307 to eef3a95 Compare May 13, 2026 23:13
@clabby clabby enabled auto-merge May 13, 2026 23:13
Comment thread consensus/src/marshal/core/actor.rs Outdated
// Skip if height is at or below the floor
if height <= self.last_processed_height {
// Skip if height is below the floor
if height < self.last_processed_height {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the rationale behind this change? if we've processed a block, we must have a finalization at or above this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

When we kick off state sync, we set the floor far in the future. That block has not necessarily been processed yet. We need to allow for marshal to fetch the floor, in the case that it is missing.

Copy link
Copy Markdown
Contributor

@patrick-ogrady patrick-ogrady May 14, 2026

Choose a reason for hiding this comment

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

Got it. That becomes very similar to something I was considering for my state sync cleanup.

@clabby clabby force-pushed the cl/glue-marshal-changes branch from eef3a95 to abb360e Compare May 14, 2026 14:34
@clabby clabby disabled auto-merge May 14, 2026 15:31
@clabby clabby added this pull request to the merge queue May 14, 2026
@clabby clabby removed this pull request from the merge queue due to a manual request May 14, 2026
@clabby clabby force-pushed the cl/glue-marshal-changes branch from abb360e to 468907b Compare May 15, 2026 00:13
Comment thread consensus/src/marshal/core/actor.rs Outdated
@clabby clabby force-pushed the cl/glue-marshal-changes branch from 468907b to 69ee63f Compare May 15, 2026 18:05
Comment thread consensus/src/marshal/core/mailbox.rs
Comment thread consensus/src/marshal/core/mailbox.rs Outdated
@clabby clabby force-pushed the cl/glue-marshal-changes branch 2 times, most recently from c48fb77 to d13f1c6 Compare May 19, 2026 18:04
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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d13f1c6. Configure here.

Comment thread consensus/src/marshal/core/actor.rs Outdated
@clabby clabby force-pushed the cl/glue-marshal-changes branch 5 times, most recently from dd097ec to 0258591 Compare May 21, 2026 16:38
@clabby clabby force-pushed the cl/glue-marshal-changes branch from 0258591 to 7199ec6 Compare May 22, 2026 15:14
@clabby clabby moved this to Ready for Review in Tracker May 22, 2026
@clabby clabby force-pushed the cl/glue-marshal-changes branch from 7199ec6 to bb6cdbb Compare May 22, 2026 15:46
@patrick-ogrady patrick-ogrady merged commit c1cd5a3 into main May 22, 2026
176 checks passed
@patrick-ogrady patrick-ogrady deleted the cl/glue-marshal-changes branch May 22, 2026 19:56
@github-project-automation github-project-automation Bot moved this from Ready for Review to Done in Tracker May 22, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 22, 2026

Codecov Report

❌ Patch coverage is 55.17241% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.76%. Comparing base (edc5cce) to head (d99635d).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
consensus/src/marshal/core/mailbox.rs 16.66% 10 Missing ⚠️
consensus/src/marshal/core/actor.rs 0.00% 3 Missing ⚠️
@@            Coverage Diff             @@
##             main    #3764      +/-   ##
==========================================
- Coverage   95.77%   95.76%   -0.02%     
==========================================
  Files         486      486              
  Lines      200338   200706     +368     
  Branches     4858     4858              
==========================================
+ Hits       191872   192201     +329     
- Misses       6834     6873      +39     
  Partials     1632     1632              
Files with missing lines Coverage Δ
consensus/src/marshal/ancestry.rs 93.54% <100.00%> (+0.52%) ⬆️
consensus/src/marshal/core/actor.rs 92.26% <0.00%> (-0.21%) ⬇️
consensus/src/marshal/core/mailbox.rs 91.77% <16.66%> (-0.96%) ⬇️

... and 41 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 edc5cce...d99635d. 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

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants