Conversation
|
👋 yashnevatia, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
✅ API Diff Results -
|
There was a problem hiding this comment.
Pull request overview
Updates this repo’s capabilities protobuf bindings to align with the newer chainlink-protos/cre version (per LINK-777), regenerating the checked-in .pb.go outputs accordingly.
Changes:
- Bump
github.com/smartcontractkit/chainlink-protos/cre/gotov0.0.0-20260312152957-059f906b6597. - Regenerate EVM capabilities protobuf Go bindings (notably updating embedded chain selector descriptors).
- Regenerate Aptos capabilities protobuf Go bindings, adding an optional
ledger_versionfield toViewRequest.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/capabilities/v2/chain-capabilities/evm/client.pb.go | Regenerated EVM protobuf Go output; updates raw descriptor content (incl. chain selector list). |
| pkg/capabilities/v2/chain-capabilities/aptos/client.pb.go | Regenerated Aptos protobuf Go output; introduces optional ledger_version on ViewRequest. |
| go.mod | Updates dependency on chainlink-protos/cre/go to the newer pseudo-version. |
| go.sum | Updates checksums to match the new chainlink-protos/cre/go version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (x *ViewRequest) GetLedgerVersion() uint64 { | ||
| if x != nil && x.LedgerVersion != nil { | ||
| return *x.LedgerVersion | ||
| } | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
LedgerVersion is documented as "nil means use latest ledger version", but the generated GetLedgerVersion() accessor returns 0 when the pointer is nil, which makes it easy for callers to accidentally lose presence information and send ledger version 0 (a potentially valid value) instead of "latest". Consider changing the proto/API semantics to avoid relying on nil presence (e.g., use an explicit sentinel like 0==latest, or a wrapper/oneof that forces callers to check presence), and/or ensure callers use req.LedgerVersion != nil rather than GetLedgerVersion() when deciding whether to pin a ledger version.
Requires
Supports