feat(go): port binaryCid + targetProofCid + sourceContractCid + EvidenceTerm to Go kit#18
Conversation
…nceTerm to Go kit Port four cross-impl protocol fields from Rust to the Go kit so .proof bundles + bridge mementos minted in Go are byte-equal to the Rust reference (and stay byte-equal to TS / C++ / Python at the JCS hash boundary): * binaryCid (.proof envelope back-pin per protocol/specs/2026-04-30-proof-file-format.md v1.3.0). Optional; empty BinaryCID is omitted entirely. Builder.Input gains BinaryCID string; bodyPairsUnsigned conditionally appends the pair. * targetProofCid + sourceContractCid (BridgeDeclaration locked key order per protocol/specs/2026-04-30-ir-formal-grammar.md, promoted normative in PR #10). Wired through BridgeSpec / BridgeDeclaration / BridgeDeclaration.MarshalJSON in ir/property.go, PrimitiveBridgeDeclaration in ir/extensions.go, BridgeMintArgs + MintBridge body in claim_envelope/bridge_minter.go, and CallSite.{BridgeSourceContractCID, BridgeTargetProofCID} + EnumerateCallsitesStage in verifier/. * EvidenceTerm: already conformant in Go (ir/property.go). Confirmed field-by-field against provekit-ir-types::EvidenceTerm and Rust's parse.rs; the Go MarshalJSON hardcodes "kind":"evidence" matching Rust's serde-derived shape. No-op port; documented in PR body. Status: - Go kit emits new fields (stored, JSON-marshaled, included in JCS). - Verifier records new fields on CallSite for downstream stages but does not enforce targetProofCid resolution today (matches the Rust CallSite shape; enforcement is the fix/verifier-enforce-target-proof-cid track). Tests: - ir/canonical_form_test.go: TestCanonicalFormBridgeWithPinning + TestCanonicalFormBridgeWithPinningAndNotes pin the locked bridge-decl JSON byte sequence. - proof_envelope/builder_test.go: cross-impl conformance vs Rust's build_minimal_proof_round_trips. Asserts map-head 0xA7 (no binaryCid) / 0xA8 (with binaryCid) / 0xA7 again when BinaryCID is empty (omit-when-empty). go build ./... + go test ./... clean across provekit-ir-symbolic, provekit-self-contracts, provekit-lift-go-tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 46 minutes and 4 seconds.Comment |
There was a problem hiding this comment.
Pull request overview
This PR ports newer protocol pinning fields into the Go implementation so Go-authored bridge declarations, bridge mementos, and .proof envelopes can move toward byte-for-byte conformance with the other language kits and the updated v1.3 grammar/specs.
Changes:
- Added
binaryCidsupport to proof-envelope building plus conformance tests for omit-when-empty behavior. - Added
sourceContractCidandtargetProofCidacross IR bridge declarations, bridge minting, and verifier call-site extraction. - Added golden tests to pin JSON field order and serialized bridge shape for cross-implementation consistency.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
implementations/go/provekit-ir-symbolic/verifier/types.go |
Extends CallSite with stored bridge pinning fields and updates comments. |
implementations/go/provekit-ir-symbolic/verifier/enumerate_callsites.go |
Plumbs sourceContractCid and targetProofCid from bridge bodies into call sites. |
implementations/go/provekit-ir-symbolic/proof_envelope/builder_test.go |
Adds proof-envelope tests for binaryCid presence/omission and map size. |
implementations/go/provekit-ir-symbolic/proof_envelope/builder.go |
Adds optional BinaryCID to proof-envelope input and emitted CBOR body. |
implementations/go/provekit-ir-symbolic/ir/property.go |
Extends bridge IR structs/serialization with new pinning fields and key order. |
implementations/go/provekit-ir-symbolic/ir/extensions.go |
Extends primitive bridge declarations with optional new pinning fields. |
implementations/go/provekit-ir-symbolic/ir/canonical_form_test.go |
Adds golden JSON tests for bridge declarations with pinning fields. |
implementations/go/provekit-ir-symbolic/claim_envelope/bridge_minter.go |
Extends bridge minting args/body with the new bridge pinning fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SourceContractCid: spec.SourceContractCid, | ||
| TargetContractCid: spec.TargetContractCid, | ||
| TargetProofCid: spec.TargetProofCid, |
| if args.SourceContractCID != "" { | ||
| body["sourceContractCid"] = args.SourceContractCID | ||
| } | ||
| if args.TargetProofCID != "" { | ||
| body["targetProofCid"] = args.TargetProofCID | ||
| } |
| // resolution); the load-all-proofs stage does not enforce them today, | ||
| // matching the Rust CallSite shape. |
| type BridgeSpec struct { | ||
| SourceSymbol string | ||
| SourceLayer string | ||
| SourceContractCid string | ||
| TargetContractCid string | ||
| TargetProofCid string |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2fff221c15
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if args.SourceContractCID != "" { | ||
| body["sourceContractCid"] = args.SourceContractCID | ||
| } | ||
| if args.TargetProofCID != "" { | ||
| body["targetProofCid"] = args.TargetProofCID |
There was a problem hiding this comment.
Require new bridge pinning fields when minting bridges
MintBridge now adds sourceContractCid and targetProofCid only when the caller sets them, so existing call sites that were updated to the new struct but leave these fields empty will silently mint bridge mementos without the new pins. That defeats the purpose of this port (the verifier later receives empty BridgeSourceContractCID / BridgeTargetProofCID) and creates under-specified bridges even though this function already treats other bridge identity fields as required. This should fail fast (or always emit validated non-empty CID values) instead of quietly omitting the keys.
Useful? React with 👍 / 👎.
Summary
Cross-impl conformance: port the four protocol fields from the Rust
reference into the Go kit so Go-minted
.proofbundles + bridgemementos stay JCS-byte-identical to Rust / TS / C++ / Python.
Per-field landing site
binaryCid(proof envelope back-pin)Spec:
protocol/specs/2026-04-30-proof-file-format.md(v1.3.0).proof_envelope/builder.goInput.BinaryCIDproof_envelope/builder.gobodyPairsUnsignedBinaryCID != ""(mirrors RustOption<String>skip-when-None)binaryCidenforcement (running-binary hash check)targetProofCid+sourceContractCid(bridge fields)Spec:
protocol/specs/2026-04-30-ir-formal-grammar.md(BridgeDeclaration locked key order, promoted normative in PR #10).ir/property.goBridgeSpecir/property.goBridgeDeclarationir/property.goBridgeDeclaration.MarshalJSONkind, name, sourceSymbol, sourceLayer, sourceContractCid, targetContractCid, targetProofCid, targetLayer, notes?ir/property.goBridge()collectorir/extensions.goPrimitiveBridgeDeclarationomitempty(so kit primitives that have not yet been re-emitted by their tooling stay byte-equal across kits)claim_envelope/bridge_minter.goBridgeMintArgs+MintBridgebody mapverifier/types.goCallSite.BridgeSourceContractCID+.BridgeTargetProofCIDverifier/enumerate_callsites.gotargetProofCidresolution (verifier enforcement)CallSiteshape; enforcement is owned by the parallelfix/verifier-enforce-target-proof-cidtrack.EvidenceTermir/property.goEvidenceTerm+MarshalJSONprovekit-ir-types::EvidenceTerm(kind,proofType,certificate.{tool, version, formulaHash, proofData}) and Rust'sparse_evidence_at. The Go MarshalJSON hardcodes"kind":"evidence"which matches Rust's serde-derived shape exactly. No code change required.Golden test status
Two new cross-impl pinning tests added:
ir/canonical_form_test.go—TestCanonicalFormBridgeWithPinningandTestCanonicalFormBridgeWithPinningAndNotespin the exact byte sequence of aBridgeDeclarationJSON, including the locked key order and thenotesomit-when-empty rule.proof_envelope/builder_test.go— three tests mirroring Rust'sbuild_minimal_proof_round_trips. Assert map-head0xA7for the minimal envelope,0xA8whenbinaryCidis added, and0xA7again whenBinaryCID == ""(omit-when-empty).A full byte-equal cross-impl
.proofround-trip would require pinning the deterministic ed25519 signature; that is beyond the scope of this PR (and would re-pin if the Rust deterministic-signing seed changes). The map-head + key-presence assertions are the smallest defensible cross-impl conformance check.Third-rail issues / scope notes
Cargo.lock/ Rust source / TypeScript source / Python source / spec files / workflows — untouched (parallel agents own those).metadatafield onInput(Rust hasOption<BTreeMap<String, String>> metadata) — out of scope for this PR; not in the task brief. Adding it later is a clean follow-up: same conditional-append pattern asbinaryCid.PrimitiveBridge()factory signature inir/extensions.gowas NOT extended with the two new fields — kept variadic-stable so existing callers compile. The struct fields are present + JSON-emitted viaomitempty. Producers that mint bridge memento bodies throughclaim_envelope.MintBridge(the canonical mint path) get the fields end-to-end; the kit-primitive convenience factory is a soft-port.EvidenceTermparse-back round-trip (consuming a Rust-emitted evidence JSON) — Go has the marshal side but no parse path today; that is a separate gap from this PR's scope.Test plan
go build ./...clean acrossprovekit-ir-symbolic,provekit-self-contracts,provekit-lift-go-testsgo test ./...clean across all three (existing tests pass; 5 new tests pass)proof_envelope/builder_test.goir/canonical_form_test.go🤖 Generated with Claude Code