starknet_transaction_prover: relay opaque additional_data from check response#14410
starknet_transaction_prover: relay opaque additional_data from check response#14410avi-starkware wants to merge 1 commit into
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
PR SummaryMedium Risk Overview Malformed Reviewed by Cursor Bugbot for commit f459598. Bugbot is set up for automated code reviews on this repo. Configure here. |
remollemo
left a comment
There was a problem hiding this comment.
@remollemo reviewed 3 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on ittaysw and Yoni-Starkware).
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware reviewed all commit messages and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on avi-starkware and ittaysw).
crates/starknet_transaction_prover/src/proving/virtual_snos_prover.rs line 56 at r1 (raw file):
/// ECDSA signature `s` component. pub sig_s: Felt, }
Why do you need to expose this in the proving service?
I was thinking about returning a dict from the interceptor ("additional_data") and outputting it as-is.
The prover should not be aware to the content (signature etc)
Code quote:
/// Screening signature the external blocking check service attaches to an allowed,
/// screened transaction. Relayed verbatim to the prover's client, which submits it
/// on-chain alongside the proven actions.
///
/// `issued_at` is unix seconds; the felts serialize as `0x`-prefixed hex strings.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct ScreeningSignature {
/// Unix timestamp (seconds) at which the signature was issued.
pub issued_at: u64,
/// ECDSA signature `r` component.
pub sig_r: Felt,
/// ECDSA signature `s` component.
pub sig_s: Felt,
}…response The external check's allow response may carry an opaque additional_data object for screened transactions. BlockingCheckResult::Allowed carries it verbatim as a serde_json object; the prover never interprets its contents. additional_data must be a JSON object — a non-object value fails parsing and maps to Inconclusive (deferring to the fail-open policy).
6e0d222 to
f459598
Compare
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware reviewed 3 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on ittaysw).

The external check's allow response may carry a screening signature
({ allowed: true, signature: { issued_at, sig_r, sig_s } }) for screened
transactions. BlockingCheckResult::Allowed now carries it as an Option.
An allow response with a malformed signature fails envelope parsing and
maps to Inconclusive (deferring to the fail-open policy) — a corrupt
signed-allow means the screening path is misbehaving, so it is not
treated as a clean allow.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com