Parse raw bytes instead of hex over capReg grpc wire for transmitter address#1983
Parse raw bytes instead of hex over capReg grpc wire for transmitter address#1983yashnevatia wants to merge 2 commits intomainfrom
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 the Capabilities Registry gRPC marshalling for OCR3 configs so transmitter addresses are passed as raw bytes (matching the protobuf repeated bytes transmitters) instead of being hex-encoded/decoded into ocrtypes.Account strings.
Changes:
- Remove hex encoding when decoding
capabilitiespb.OCR3Config.Transmittersintoocrtypes.ContractConfig. - Remove hex decoding when encoding
ocrtypes.ContractConfig.Transmittersback into the protobuf reply. - Drop the unused
encoding/heximport.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| transmitters := make([][]byte, len(cfg.Transmitters)) | ||
| for i, t := range cfg.Transmitters { | ||
| transmitters[i], err = hex.DecodeString(string(t)) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to decode transmitter: %w", err) | ||
| } | ||
| transmitters[i] = []byte(t) | ||
| } |
There was a problem hiding this comment.
By switching from hex.DecodeString(string(t)) to []byte(t), the server now treats cfg.Transmitters as containing raw address bytes encoded directly in a string. If any current core.CapabilitiesRegistry implementations still populate cfg.Transmitters as hex strings (as implied by the prior implementation), this will put ASCII hex on the wire rather than the intended raw bytes and won’t error anymore, leading to hard-to-debug misconfiguration. Consider supporting both encodings during rollout (decode hex when applicable), or enforce/validate the expected format (e.g., reject obvious hex strings when raw bytes are required, or add an explicit migration/version check).
| transmitters := make([]ocrtypes.Account, len(pbCfg.Transmitters)) | ||
| for i, t := range pbCfg.Transmitters { | ||
| transmitters[i] = ocrtypes.Account(hex.EncodeToString(t)) | ||
| transmitters[i] = ocrtypes.Account(t) | ||
| } |
There was a problem hiding this comment.
This change switches the in-process representation of ocrtypes.ContractConfig.Transmitters from a hex string (previously produced via hex.EncodeToString) to a raw-byte string (ocrtypes.Account(t) where t is protobuf []byte). Because ocrtypes.Account is a string type, this is a behavioral/API change for any downstream code that expects human-readable/hex transmitter addresses, and mixed-version plugin/core deployments (Hashicorp go-plugin handshake here doesn’t enforce a protocol version) could silently disagree on the encoding. Consider adding a transitional decode that accepts both formats (e.g., if the string is valid hex then decode, otherwise treat as raw bytes), or document/guard the change with an explicit versioned capability/handshake so incompatible components fail fast.
jmank88
left a comment
jmank88
left a comment
There was a problem hiding this comment.
Are you sure that this works for all cases? IIRC protobufs only accept utf8 encoded strings - not arbitrary bytes like Go does. Can we add a test?
| transmitters[i], err = hex.DecodeString(string(t)) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to decode transmitter: %w", err) | ||
| decoded, hexErr := hex.DecodeString(string(t)) |
There was a problem hiding this comment.
From the discussion that we had it sounds like hex.DecodeString never works because the transmitter string isn't in hex, so If the transmitter is base64 encoded lets just decode it here into a hex string and keep it in that format.
"error":"failed to get capability config: rpc error: code = Unknown desc = failed to decode transmitter: encoding/hex: invalid byte: U+0093"This is the flow
Pushing on chain:
OCR2OracleConfigwithTransmitters as []common.AddressOCR2OracleConfigToMapto push this data to chain which stores transmitter address as base64 stringReading on chain: