Skip to content

feat(realtime): encrypt proposal blobs and consume forced inclusions#958

Open
AnshuJalan wants to merge 8 commits into
masterfrom
feat/realtime-privacy
Open

feat(realtime): encrypt proposal blobs and consume forced inclusions#958
AnshuJalan wants to merge 8 commits into
masterfrom
feat/realtime-privacy

Conversation

@AnshuJalan
Copy link
Copy Markdown
Collaborator

@AnshuJalan AnshuJalan commented May 7, 2026

Summary

Wires the realtime proposer for the cross-stack privacy feature and re-enables forced-inclusion consumption against the new RealTimeInbox queue.

  • Encryption — new realtime/src/privacy/ with ProposalCipher (AES-256-GCM with a fresh OsRng nonce per blob). Applied at both manifest-building sites — the L1 proposal builder and the async Raiko-request builder — so the bytes Raiko sees match what's hashed on L1.
  • Forced inclusion consumptionExecutionLayer.send_batch_to_l1 reads getForcedInclusionState() each propose tick, computes numForcedInclusions = min(pending, fi_max_per_proposal), and threads it into ProposeInput / ProposeInputV2. The contract reverts with UnprocessedForcedInclusionIsDue if the cap is below the due count.
  • ConfigSURGE_PRIVACY_MODE, SURGE_PRIVACY_SYMMETRIC_KEY, FI_MAX_PER_PROPOSAL new env vars on RealtimeConfig. Privacy mode without a key fails fast at startup. ABI regenerated to expose the new contract methods.
  • No ECIES in Catalyst — Catalyst never decrypts FI blobs; submitters encrypt them off-system to PK_sys.

Companion PRs:

  • surge-taiko-mono#300 — protocol contracts + driver decryption
  • raiko — prover decryption (cipher module + host CLI + guest hash-binding)

See PRIVACY_STACK.md for the cross-stack design reference.

Comment thread realtime/src/privacy/aes.rs Dismissed
Comment thread realtime/src/privacy/aes.rs Dismissed
@AnshuJalan
Copy link
Copy Markdown
Collaborator Author

@claude review

@claude
Copy link
Copy Markdown

claude Bot commented May 7, 2026

Claude finished @AnshuJalan's task in 3m 0s —— View job


PR Review: feat(realtime): encrypt proposal blobs and consume forced inclusions

Todo:

  • Read changed files
  • Review realtime/src/privacy/aes.rs (crypto implementation)
  • Review realtime/src/privacy/mod.rs (ProposalCipher)
  • Review realtime/src/l1/execution_layer.rs (forced inclusion logic)
  • Review realtime/src/l1/proposal_tx_builder.rs (encryption at proposal build)
  • Review realtime/src/node/proposal_manager/async_submitter.rs
  • Review config changes (realtime/src/utils/config.rs, realtime/src/l1/config.rs)
  • Review bindings/ABI changes
  • Address CodeQL security findings
  • Post final review

Overall this is a clean, well-structured addition. The AES-256-GCM crypto layer itself is correct, the fast-fail config validation is good, and the forced-inclusion wiring is straightforward. There is one critical correctness bug in privacy mode, one security hygiene issue with the key in Debug output, and a few smaller items below.

Critical: Double-Encryption Nonce Mismatch in Privacy Mode

This is the most important finding. When privacy_mode = true, cipher.wrap() is called twice on the same manifest data via two independent code paths, each generating a fresh random nonce. The two calls produce different nonces → different ciphertexts → different blob hashes.

Path 1async_submitter.rs:187-194 builds the sidecar to send to Raiko:

let blob_payload = ethereum_l1.execution_layer.proposal_cipher().wrap(&manifest_data)?;
let sidecar_builder: SidecarBuilder<BlobCoder> = SidecarBuilder::from_slice(&blob_payload);
// blob_hashes from this sidecar go to Raiko → proof binds to these hashes

Path 2proposal_tx_builder.rs:262 builds the sidecar for the actual L1 transaction:

let blob_payload = self.cipher.wrap(&manifest_data)?;
// nonce_2 ≠ nonce_1 → blob_hash_2 ≠ blob_hash_1

Raiko's proof commits to blob_hash_1. The L1 transaction posts blobs identified by blob_hash_2. Every propose call in privacy mode will submit a proof that doesn't match the on-chain blob hashes and will revert.

Fix: Encrypt the manifest once (in async_submitter.rs) and carry the resulting payload on Proposal so proposal_tx_builder.rs can consume it directly without re-encrypting. Example sketch:

// Proposal struct gains an optional pre-encrypted blob payload
pub struct Proposal {
    // ...
    pub encrypted_blob_payload: Option<Vec<u8>>,
}

Then proposal_tx_builder.rs uses batch.encrypted_blob_payload.as_deref() when present, falling back to cipher.wrap only when absent (plaintext mode — which is deterministic, so the double-call is harmless there). Fix this →


Security: RealtimeConfig Debug Derive Exposes Symmetric Key

realtime/src/utils/config.rs:8: #[derive(Debug, Clone)] is on RealtimeConfig, which contains privacy_symmetric_key: Option<[u8; 32]>. Rust's derived Debug will print the key bytes verbatim. If RealtimeConfig is ever logged via {:?}, the key appears in logs/traces.

The Display impl correctly omits it. The Debug impl should be made safe as well — either implement Debug manually or wrap the key in a newtype:

struct RedactedKey(pub [u8; 32]);
impl fmt::Debug for RedactedKey {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        f.write_str("<redacted>")
    }
}

Fix this →


Potential Lint Failure: cast_possible_truncation at execution_layer.rs:253

let num_forced_inclusions = pending.min(u64::from(self.fi_max_per_proposal)) as u16;

pending is u64; as u16 is a truncating cast. The workspace Cargo.toml denies cast_possible_truncation. The value is logically bounded by fi_max_per_proposal: u16, but the compiler has no way to prove that, so clippy will fire. This will fail CI. Use u16::try_from(...) instead:

let num_forced_inclusions =
    u16::try_from(pending.min(u64::from(self.fi_max_per_proposal)))
        .unwrap_or(self.fi_max_per_proposal);

Or, since the min is provably within range, a expect("bounded by fi_max_per_proposal") is also acceptable. Fix this →


CodeQL Findings: Both Are False Positives

  • Line 24 (let mut nonce = [0u8; NONCE_LEN]): This is a zero-initialized stack buffer immediately overwritten by OsRng.fill_bytes(&mut nonce) on the next line. CodeQL sees an initialized array and flags it; the actual nonce is always random.
  • Line 62 (let nonce = [0x37u8; NONCE_LEN]): This is inside #[cfg(test)] and intentionally uses a deterministic nonce for the round-trip test. The module doc at line 29–30 explicitly marks encrypt_with_nonce as test-only. No action needed.

If you want to suppress the alerts cleanly without noise in the source, a #[allow(clippy::...)] attribute on the test function and a brief comment pointing at CodeQL issue numbers would be sufficient.


Resilience: Silent FI State Failure May Burn Gas

execution_layer.rs:244-250: if getForcedInclusionState RPC fails, the code silently falls back to num_forced_inclusions = 0. When there are actually "due" forced inclusions, the resulting propose call will revert with UnprocessedForcedInclusionIsDue, wasting gas. Consider returning an Err and skipping the proposal for this slot rather than committing to a predictably-reverting transaction.


Minor Nits

  • proposal_tx_builder.rs:95: #[allow(clippy::too_many_arguments)] on build_propose_blob which only has 4 arguments — can be removed.
  • realtime/src/utils/config.rs:175: Display for RealtimeConfig prints privacy_mode but not whether privacy_symmetric_key is set. A line like Privacy key: {} (set/not set) would make the startup log more informative during debugging.
  • async_submitter.rs:250: std::fs::write("/tmp/raiko_request.json", ...) runs on every bypass invocation and could silently fail to write if the filesystem is read-only. The error is dropped via ? which would propagate correctly — this is fine, just worth noting in ops docs.

Summary

Severity Finding
🔴 Critical Double-encryption: async_submitter.rs and proposal_tx_builder.rs independently call cipher.wrap(), generating mismatched blob hashes in privacy mode
🟡 Medium RealtimeConfig derived Debug exposes the raw AES key bytes
🟡 Medium as u16 cast violates cast_possible_truncation = deny workspace lint
🟢 Info Both CodeQL nonce alerts are false positives
🟢 Minor FI state failure silently proceeds to a likely-reverting L1 call
🟢 Nit Stale #[allow(clippy::too_many_arguments)] on build_propose_blob


- uses: docker/login-action@v3
- name: Login to JFrog Artifactory
uses: docker/login-action@v3
steps:
- uses: docker/login-action@v3
- name: Login to JFrog Artifactory
uses: docker/login-action@v3
AnshuJalan added a commit that referenced this pull request May 8, 2026
Previously async_submitter and proposal_tx_builder each independently
called build_blob_payload (which calls cipher.wrap, which mints a fresh
random AES-GCM nonce). In privacy mode this produced two different
ciphertexts, two different blob hashes, and a Raiko proof that committed
to a hash the L1 transaction could never reproduce — every privacy-mode
propose call would have reverted.

Fix: store the blob_payload that async_submitter computes (just before
the Raiko request) on Proposal. proposal_tx_builder consumes that field
when building the L1 sidecar, falling back to compute-on-the-fly only
when blob_payload is absent (safe in plaintext mode where the output is
deterministic; logs a warning if the cipher is enabled, since the
mismatch would only show up at L1-revert time).

Caught by Claude PR review on #958.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AnshuJalan added a commit that referenced this pull request May 8, 2026
Five smaller items from the #958 review left after the critical
double-encryption fix:

- Manual Debug impl on RealtimeConfig that redacts privacy_symmetric_key,
  raiko_api_key (replaces the derived Debug, which would print raw key
  bytes if a config was ever logged via {:?}).
- Display now reports whether the privacy key is set (set/unset),
  matching the existing privacy_mode line, for clearer startup logs.
- execution_layer: replace `as u16` cast (which the workspace lint
  cast_possible_truncation = deny would have rejected) with u16::try_from
  + a fallback to fi_max_per_proposal (the value is bounded but the
  compiler can't prove it).
- execution_layer: surface FI-state RPC failure as Err to the caller
  instead of silently falling back to numForcedInclusions = 0. The old
  behavior would burn blob-tx gas on a guaranteed-revert with
  UnprocessedForcedInclusionIsDue when there were due FIs; skipping the
  slot is cheaper and lets the next tick retry.
- Drop stale #[allow(clippy::too_many_arguments)] from build_propose_tx
  and build_propose_blob (now 4 args each, well under the threshold).

Plus cargo fmt formatting fixups.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread realtime/Cargo.toml Outdated
AnshuJalan and others added 7 commits May 21, 2026 14:32
Wires the realtime proposer for the privacy stack and re-enables forced
inclusion consumption against the new RealTimeInbox queue.

Encryption: new realtime/src/privacy/ with ProposalCipher (AES-256-GCM with a
fresh CSPRNG nonce per blob). Applied at both manifest-building sites — the
L1 proposal builder and the async Raiko-request builder — so the bytes Raiko
sees match the bytes hashed on L1. Catalyst does NOT decrypt; FI blobs are
encrypted off-system to PK_sys by their submitters.

Forced inclusion: ExecutionLayer reads getForcedInclusionState() each
propose tick, computes numForcedInclusions = min(pending, fi_max_per_proposal),
and threads it into ProposeInput / ProposeInputV2. The contract enforces
consumption of any "due" FI; if the cap is below the due count, the propose
call reverts with UnprocessedForcedInclusionIsDue.

Config: SURGE_PRIVACY_MODE, SURGE_PRIVACY_SYMMETRIC_KEY, FI_MAX_PER_PROPOSAL
new env vars on RealtimeConfig. Privacy mode without a key fails fast at
startup. ABI regenerated to expose the new contract methods.

Tests: 5 new privacy round-trip tests (random nonce, fresh-nonce-per-call,
plaintext fallback). Companion PRs: surge-taiko-mono (protocol + driver) and
raiko (prover decryption).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The first cut emitted [scheme || frame(64) || compressed_manifest] which
caused raiko's blob_tx_slice_param_for_source to read the scheme byte as
the first byte of the bytes32(version) field, fail the version check, and
return None. The driver's ExtractVersionAndSize did the same. Both fell
back to default-empty manifests but with diverging block params, tripping
the validate_input_block_param assertion inside the Zisk guest.

New helper build_blob_payload re-emits the Shasta frame around the
cipher-wrapped inner: [bytes32(1) || bytes32(inner_len) || scheme || body].
Both the L1 proposal-tx builder and the async Raiko-request submitter call
through the same helper so the L1 blob hash and the bytes Raiko sees stay
identical. Three new tests cover the round-trip layout (plaintext,
encrypted, short-input rejection); the prior 5 still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…or_source

Inlines a copy of raiko's parser (raiko/lib/src/input.rs:442) and asserts
build_blob_payload's output passes the version + size frame check, then
yields a slice that begins with the scheme byte. This is the exact gate
that rejected the original buggy payload (captured in
surge-taiko-mono/blob-payloads-share/04-blob-after-decode_blob_data.hex
where decoded[0..] started with 0x01 b0 b9 78 ... — scheme byte at the
wrong offset).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously async_submitter and proposal_tx_builder each independently
called build_blob_payload (which calls cipher.wrap, which mints a fresh
random AES-GCM nonce). In privacy mode this produced two different
ciphertexts, two different blob hashes, and a Raiko proof that committed
to a hash the L1 transaction could never reproduce — every privacy-mode
propose call would have reverted.

Fix: store the blob_payload that async_submitter computes (just before
the Raiko request) on Proposal. proposal_tx_builder consumes that field
when building the L1 sidecar, falling back to compute-on-the-fly only
when blob_payload is absent (safe in plaintext mode where the output is
deterministic; logs a warning if the cipher is enabled, since the
mismatch would only show up at L1-revert time).

Caught by Claude PR review on #958.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five smaller items from the #958 review left after the critical
double-encryption fix:

- Manual Debug impl on RealtimeConfig that redacts privacy_symmetric_key,
  raiko_api_key (replaces the derived Debug, which would print raw key
  bytes if a config was ever logged via {:?}).
- Display now reports whether the privacy key is set (set/unset),
  matching the existing privacy_mode line, for clearer startup logs.
- execution_layer: replace `as u16` cast (which the workspace lint
  cast_possible_truncation = deny would have rejected) with u16::try_from
  + a fallback to fi_max_per_proposal (the value is bounded but the
  compiler can't prove it).
- execution_layer: surface FI-state RPC failure as Err to the caller
  instead of silently falling back to numForcedInclusions = 0. The old
  behavior would burn blob-tx gas on a guaranteed-revert with
  UnprocessedForcedInclusionIsDue when there were due FIs; skipping the
  slot is cheaper and lets the next tick retry.
- Drop stale #[allow(clippy::too_many_arguments)] from build_propose_tx
  and build_propose_blob (now 4 args each, well under the threshold).

Plus cargo fmt formatting fixups.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AnshuJalan AnshuJalan force-pushed the feat/realtime-privacy branch from ba8d39d to 9cd3364 Compare May 21, 2026 09:33
Comment thread .github/workflows/node_docker_build.yml Dismissed
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.

6 participants