Conversation
|
✅ API Diff Results -
|
There was a problem hiding this comment.
Pull request overview
This PR extends the capabilities response metadata to optionally carry an OCR attestation (config digest, sequence number, and attributed signatures), enabling Capability DONs to include OCR-style attestations alongside responses.
Changes:
- Added
ocr_attestation(withResponseOCRAttestation+AttributedSignature) toResponseMetadatain the capabilities protobuf schema. - Updated Go capability types and pb helper conversions to serialize/deserialize OCR attestation data.
- Added test coverage for attestation round-tripping and invalid config digest length handling; refactored EVM keyring blob verification into a reusable function.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/capabilities/pb/capabilities_helpers_test.go | Adds subtests for invalid digest length and round-trip conversion including OCR attestation metadata. |
| pkg/capabilities/pb/capabilities_helpers.go | Adds OCR attestation marshaling/unmarshaling logic to capability response proto helpers. |
| pkg/capabilities/pb/capabilities.proto | Introduces ocr_attestation on ResponseMetadata and new messages for attestation + signatures. |
| pkg/capabilities/pb/capabilities.pb.go | Regenerated protobuf Go output for the updated schema. |
| pkg/capabilities/capabilities.go | Adds OCR attestation types to response metadata and introduces ResponseToReportData hashing helper. |
| keystore/corekeys/ocr2key/evm_keyring.go | Extracts EVM blob verification into EvmVerifyBlob and reuses it from the keyring method. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1de3504 to
82a189c
Compare
There was a problem hiding this comment.
Pull request overview
Adds OCR attestation metadata support to capability responses so Capability DONs can include verifiable OCR context (config digest, sequence number, signatures) alongside the response.
Changes:
- Extend
ResponseMetadataprotobuf schema with an optionalocr_attestationmessage (including attributed signatures). - Update Go conversion helpers to marshal/unmarshal OCR attestation between internal types and protobuf types, plus add round-trip/validation tests.
- Introduce response-to-report hashing helper and extract EVM blob verification into a reusable function.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/capabilities/pb/capabilities.proto | Adds ResponseOCRAttestation + AttributedSignature and wires it into ResponseMetadata. |
| pkg/capabilities/pb/capabilities.pb.go | Regenerated protobuf Go types to include the new messages/field. |
| pkg/capabilities/pb/capabilities_helpers.go | Adds proto ↔ internal mapping for OCR attestation on capability responses. |
| pkg/capabilities/pb/capabilities_helpers_test.go | Adds validation + round-trip coverage for response OCR attestation conversions. |
| pkg/capabilities/capabilities.go | Adds internal OCR attestation types and ResponseToReportData hashing helper. |
| keystore/corekeys/ocr2key/evm_keyring.go | Extracts EVM blob verification into EvmVerifyBlob and reuses it from the keyring method. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
82a189c to
76aac32
Compare
76aac32 to
bbf9252
Compare
There was a problem hiding this comment.
Pull request overview
Adds support for attaching OCR attestation data to CapabilityResponse so capability DON responses can be verified/attributed, and introduces hashing utilities/tests to produce deterministic report data for attestation workflows.
Changes:
- Extend
CapabilityResponseprotobuf with an optionalocr_attestation(config digest, sequence number, signatures). - Add Go helpers to marshal/unmarshal the new attestation field between proto and
capabilities.CapabilityResponse. - Introduce
ResponseToReportDatahashing + tests (including a golden digest) to produce deterministic 32-byte report data.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/capabilities/pb/capabilities.proto | Adds OCRAttestation + AttributedSignature messages and an optional ocr_attestation field on CapabilityResponse. |
| pkg/capabilities/pb/capabilities.pb.go | Regenerated Go protobuf output reflecting the new optional field and new messages. |
| pkg/capabilities/pb/capabilities_helpers.go | Implements proto ↔ domain conversions for OCR attestation on capability responses. |
| pkg/capabilities/pb/capabilities_helpers_test.go | Adds round-trip and validation tests for OCR attestation conversion. |
| pkg/capabilities/capabilities.go | Adds domain types for OCR attestation, ErrResponsePayloadNotAvailable, and ResponseToReportData hashing. |
| pkg/capabilities/capabilities_test.go | Adds tests validating deterministic hashing behavior and metadata constraints. |
| keystore/corekeys/ocr2key/evm_keyring.go | Refactors EVM blob verification into a shared helper function. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return rpt, nil | ||
| } | ||
|
|
||
| func ResponseToReportData(workflowExecutionID, referenceID string, responsePayload []byte, metadata ResponseMetadata) ([32]byte, error) { |
There was a problem hiding this comment.
How is this supposed to work is different nodes can attach different metering data? How are you going to get multiple signatures on a report that contains metering data in the signed payload?
There was a problem hiding this comment.
Why will different nodes attach different metering data? Right now spend unit and spend value are hardcoded for each read action. So the metering is identical across all nodes.
We need to include the metering into report to be able to trust a single response on the workflow DON to handle case of a malicious node.
Do we expect different nodes to provide different spend values in other capabilities?
There was a problem hiding this comment.
Yes, I think the original plan was to allow each node to independently determine the cost and let workflow DON medianize it. If you say it's always identical for ReadChain then it simplifies things. cc @patrickhuie19
https://smartcontract-it.atlassian.net/browse/PLEX-2611
Supports: