Skip to content

apollo_consensus_orchestrator: send only the commitment-info delta the recorder is missing#14667

Open
itamar-starkware wants to merge 1 commit into
mainfrom
cende-commitment-delta
Open

apollo_consensus_orchestrator: send only the commitment-info delta the recorder is missing#14667
itamar-starkware wants to merge 1 commit into
mainfrom
cende-commitment-delta

Conversation

@itamar-starkware

Copy link
Copy Markdown
Contributor

Query the recorder's get_last_stored_commitment_height and bound collect_recent_state_commitment_infos' lower end to max(height - N, last_stored + 1), so the blob carries only the commitment infos the recorder has not stored yet instead of the full fixed window. Falls back to the full window when the query fails/returns nothing, so correctness never depends on the optimization. os_input-gated.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

itamar-starkware commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@yoavGrs yoavGrs left a comment

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.

@yoavGrs made 1 comment.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on itamar-starkware).


a discussion (no related file):
Block until we have the Python side.

@yoavGrs yoavGrs left a comment

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.

@yoavGrs made 1 comment.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on itamar-starkware).


crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs line 706 at r1 (raw file):

        );
        // Send only the delta the recorder hasn't stored yet, bounded above by the fixed window.
        let fixed_window_lowest_height = height.0.saturating_sub(N_BLOCK_HASHES_BACK_IN_BLOB);
  1. If cende_height == lowest_height: validate the batcher has commitment infos for this height.
  2. Do it BEFORE decision_reached in the batcher.

@yoavGrs yoavGrs self-assigned this Jul 1, 2026
…e recorder is missing

Query the recorder's get_last_stored_commitment_height and bound collect_recent_state_commitment_infos' lower end to max(height - N, last_stored + 1), so the blob carries only the commitment infos the recorder has not stored yet instead of the full fixed window. Falls back to the full window when the query fails/returns nothing, so correctness never depends on the optimization. os_input-gated.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@yoavGrs
yoavGrs force-pushed the cende-commitment-delta branch from 3c75039 to 7c7370f Compare July 15, 2026 09:49
@cursor

cursor Bot commented Jul 15, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes which commitment infos appear in os_input cende blobs; incorrect delta logic could omit data the recorder needs, though failures revert to the prior full-window path.

Overview
Under os_input, cende blobs no longer always carry the full N_BLOCK_HASHES_BACK_IN_BLOB window of state commitment infos. The orchestrator asks the recorder for get_last_stored_commitment_height and collect_recent_state_commitment_infos starts at max(height - N, last_stored + 1), so only heights the recorder has not persisted yet are included.

CendeContext / CendeAmbassador gain query_last_stored_commitment_height (HTTP to /cende_recorder/get_last_stored_commitment_height). Any parse, HTTP, or transport failure logs a warning and falls back to the previous full-window behavior, so the change is an optimization rather than a hard dependency on the new endpoint.

Tests cover delta-only collection, single-block when caught up, window cap, query failure fallback, and empty delta when the recorder is already at the current height; default mocks return None from the query so existing tests keep sending the full window.

Reviewed by Cursor Bugbot for commit 7c7370f. Bugbot is set up for automated code reviews on this repo. Configure here.

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.

3 participants