apollo_committer,apollo_batcher: compress state commitment infos at the committer#14670
apollo_committer,apollo_batcher: compress state commitment infos at the committer#14670itamar-starkware wants to merge 1 commit into
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Artifacts upload workflows: |
yoavGrs
left a comment
There was a problem hiding this comment.
@yoavGrs reviewed 15 files and made 12 comments.
Reviewable status: 15 of 19 files reviewed, 12 unresolved discussions (waiting on itamar-starkware).
crates/apollo_committer/src/committer.rs line 154 at r1 (raw file):
/// Compresses the state commitment infos to `base64(zstd(serde_json(..)))`, once at the source, so /// the witness stays compact across the committer->batcher RPC (which has an 8 MB response cap),
Instead of determining the size, please point to the constant with this value.
Code quote:
which has an 8 MB response capcrates/apollo_committer/src/committer.rs line 157 at r1 (raw file):
/// batcher storage, and the cende blob. #[cfg(feature = "os_input")] fn compress_state_commitment_infos(infos: &StateCommitmentInfos) -> CommitterResult<String> {
We already have StateCommitmentInfos::compress,
Code quote:
fn compress_state_commitment_infos(infos: &StateCommitmentInfos) -> CommitterResult<String> {crates/apollo_committer/src/committer.rs line 162 at r1 (raw file):
let compressed = zstd::encode_all(json.as_slice(), STATE_COMMITMENT_INFOS_ZSTD_LEVEL) .map_err(|err| CommitterError::StateCommitmentInfosCompression(err.to_string()))?; Ok(base64::encode(compressed))
Can't do without it?
Code quote:
base64::encodecrates/apollo_committer/Cargo.toml line 25 at r1 (raw file):
async-trait.workspace = true base64 = { workspace = true, optional = true } serde_json.workspace = true
How is it not optional?
Code quote:
serde_json.workspace = truecrates/apollo_batcher_types/src/communication.rs line 57 at r1 (raw file):
/// Gets the block hash for a given block number. async fn get_block_hash(&self, block_number: BlockNumber) -> BatcherClientResult<BlockHash>; /// Gets the compressed (`base64(zstd(serde_json(..)))`) state commitment infos for a block.
The compression method should be documented by the functionality of the compressed type
Code quote:
(`base64(zstd(serde_json(..)))`)crates/apollo_committer/src/request_paths_and_commit_block_tests.rs line 167 at r1 (raw file):
/// Decompresses the response witness (`base64(zstd(serde_json(..)))`) so the assertions below can /// inspect the trie commitment infos. fn decompress_response_commitment_infos(
We already have StateCommitmentInfos::decompress,
crates/apollo_storage/src/state_commitment_infos_test.rs line 10 at r1 (raw file):
// Storage persists the witness verbatim, so a plain string stands in for the real payload. fn sample_state_commitment_infos() -> String {
Consider
Suggestion:
dummy_state_commitment_infoscrates/apollo_consensus_orchestrator/src/cende/mod.rs line 81 at r1 (raw file):
#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct StateCommitmentInfosAndNumber { /// Compressed (`base64(zstd(serde_json(..)))`) commitment infos from the committer, forwarded
The compression method should be documented by the functionality of the compressed type,
Code quote:
Compressed (`base64(zstd(serde_json(..)))`) crates/apollo_committer_types/src/errors.rs line 66 at r1 (raw file):
MissingPatriciaPaths { height: BlockNumber }, // Holds the error message rather than `#[from] serde_json::Error` because `CommitterError` // crosses the RPC boundary and must stay `Clone`/`Serialize`/`Deserialize`.
Consider delete the comments.
Code quote:
// Holds the error message rather than `#[from] serde_json::Error` because `CommitterError`
// crosses the RPC boundary and must stay `Clone`/`Serialize`/`Deserialize`.crates/apollo_committer_types/src/committer_types.rs line 53 at r1 (raw file):
/// The OS-input commitment infos, compressed by the committer into a /// `base64(zstd(serde_json(StateCommitmentInfos)))` string (kept compact across this RPC). pub state_commitment_infos: String,
Please define struct CompressedStateCommitmentInfos(String) (or with Vec<u8> instead of String) , and impl converters from / to StateCommitmentInfos.
Suggestion:
state_commitment_infos: CompressedStateCommitmentInfos,crates/apollo_storage/src/lib.rs line 1174 at r1 (raw file):
accessed_keys: FileHandler<VersionZeroWrapper<AccessedKeys>, Mode>, #[cfg(feature = "os_input")] state_commitment_infos: FileHandler<VersionZeroWrapper<String>, Mode>,
Same here, a dedicated type.
Suggestion:
FileHandler<VersionZeroWrapper<StateCommitmentInfosCompressed>crates/apollo_storage/src/lib.rs line 1329 at r1 (raw file):
#[cfg(feature = "os_input")] // Returns the commitment infos at the given location or an error in case they don't exist.
Please don't delete it.
Code quote:
or an error in case they don't existebf250e to
977a6bb
Compare
3c75039 to
7c7370f
Compare
PR SummaryMedium Risk Overview The committer calls
Reviewed by Cursor Bugbot for commit 8cd1880. Bugbot is set up for automated code reviews on this repo. Configure here. |
7c7370f to
e9a2273
Compare
977a6bb to
e74d7ae
Compare
…mmitment infos Compress the OS-input state commitment infos into a CompressedStateCommitmentInfos (zstd of bincode) once at the committer, and forward it verbatim across the committer->batcher RPC, batcher storage, and the cende blob so the witness stays within the RPC response cap. Co-Authored-By: Itamar Shechter <itamar@starkware.co> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
e74d7ae to
8cd1880
Compare

Compress each block's StateCommitmentInfos to a base64(zstd) String at the committer (in ReadPathsAndCommitBlockResponse) instead of at the consensus orchestrator, so the witness travels compressed across the committer->batcher RPC (capped at 8MB max_response_body_bytes, which wedged the chain on large state diffs), storage, and cende. The compressed String propagates end-to-end; the orchestrator- and storage-layer compression are removed.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com