feat: add Aptos capability view proto helpers#1992
Conversation
✅ API Diff Results -
|
product-security-plaid-production
Bot
requested review from
Farber98,
cedric-cordenier and
fernandezlautaro
and removed request for
Farber98 and
fernandezlautaro
There was a problem hiding this comment.
Pull request overview
Adds shared Aptos capability-view proto conversion helpers to map capability protos (ViewPayload, TypeTag) into Aptos domain types, centralizing validation and preserving short-address left-padding behavior.
Changes:
- Introduce
ConvertViewPayloadFromProtoandConvertTypeTagFromProtofor capability protos →pkg/types/chains/aptosdomain types. - Implement account-address left-padding (up to 32 bytes) during conversion.
- Add unit tests covering nested tag conversion and several invalid-input cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/capabilities/v2/chain-capabilities/aptos/proto_helpers.go | Implements proto→domain conversion helpers for Aptos view payloads and type tags, including short-address left-padding. |
| pkg/capabilities/v2/chain-capabilities/aptos/proto_helpers_test.go | Adds tests for nested vector/struct/generic conversion and validation error cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if payload.Module == nil { | ||
| return nil, fmt.Errorf("viewRequest.Payload.Module is required") | ||
| } | ||
| if payload.Module.Name == "" { | ||
| return nil, fmt.Errorf("viewRequest.Payload.Module.Name is required") | ||
| } | ||
| if payload.Function == "" { | ||
| return nil, fmt.Errorf("viewRequest.Payload.Function is required") | ||
| } | ||
|
|
||
| moduleAddress, err := convertAccountAddressFromProto(payload.Module.Address, "module") | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
payload.Module.Address can be empty/nil; convertAccountAddressFromProto will then return the all-zero address without error. Since the module address is required to identify the view function, this should be rejected explicitly (e.g., require len(payload.Module.Address) > 0 and return a "...Module.Address is required" error) to avoid silently calling address 0x0.
| case TypeTagKind_TYPE_TAG_KIND_STRUCT: | ||
| structTag := tag.GetStruct() | ||
| if structTag == nil { | ||
| return nil, fmt.Errorf("struct tag missing struct value") | ||
| } | ||
|
|
||
| structAddress, err := convertAccountAddressFromProto(structTag.Address, "struct") | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| typeParams := make([]typesaptos.TypeTag, 0, len(structTag.TypeParams)) | ||
| for i, tp := range structTag.TypeParams { | ||
| converted, err := ConvertTypeTagFromProto(tp) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("invalid struct type param at index %d: %w", i, err) | ||
| } | ||
| typeParams = append(typeParams, *converted) | ||
| } | ||
|
|
||
| return &typesaptos.TypeTag{Value: typesaptos.StructTag{ | ||
| Address: structAddress, | ||
| Module: structTag.Module, | ||
| Name: structTag.Name, | ||
| TypeParams: typeParams, | ||
| }}, nil |
There was a problem hiding this comment.
In the STRUCT branch, structTag.Address is allowed to be empty (converted to 0x0) and structTag.Module/structTag.Name are not validated. A Move struct type is not well-formed without these fields, so this converter should enforce non-empty Address, Module, and Name and return a clear validation error when missing.
| return nil, fmt.Errorf("viewRequest.Payload is required") | ||
| } | ||
| if payload.Module == nil { | ||
| return nil, fmt.Errorf("viewRequest.Payload.Module is required") | ||
| } | ||
| if payload.Module.Name == "" { | ||
| return nil, fmt.Errorf("viewRequest.Payload.Module.Name is required") | ||
| } | ||
| if payload.Function == "" { | ||
| return nil, fmt.Errorf("viewRequest.Payload.Function is required") |
There was a problem hiding this comment.
The validation errors in this helper reference viewRequest.Payload..., but the API accepts a *ViewPayload directly. Consider aligning the error strings with the function’s input (e.g., payload is required, payload.module is required) so callers who use the helper outside a ViewRequest context aren’t misled.
| return nil, fmt.Errorf("viewRequest.Payload is required") | |
| } | |
| if payload.Module == nil { | |
| return nil, fmt.Errorf("viewRequest.Payload.Module is required") | |
| } | |
| if payload.Module.Name == "" { | |
| return nil, fmt.Errorf("viewRequest.Payload.Module.Name is required") | |
| } | |
| if payload.Function == "" { | |
| return nil, fmt.Errorf("viewRequest.Payload.Function is required") | |
| return nil, fmt.Errorf("payload is required") | |
| } | |
| if payload.Module == nil { | |
| return nil, fmt.Errorf("payload.module is required") | |
| } | |
| if payload.Module.Name == "" { | |
| return nil, fmt.Errorf("payload.module.name is required") | |
| } | |
| if payload.Function == "" { | |
| return nil, fmt.Errorf("payload.function is required") |
| func TestConvertViewPayloadFromProto_RejectsInvalidPayloadInputs(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| _, err := aptoscap.ConvertViewPayloadFromProto(nil) | ||
| require.ErrorContains(t, err, "viewRequest.Payload is required") | ||
|
|
||
| _, err = aptoscap.ConvertViewPayloadFromProto(&aptoscap.ViewPayload{Function: "name"}) | ||
| require.ErrorContains(t, err, "viewRequest.Payload.Module is required") | ||
|
|
||
| _, err = aptoscap.ConvertViewPayloadFromProto(&aptoscap.ViewPayload{ | ||
| Module: &aptoscap.ModuleID{Address: []byte{0x01}, Name: "coin"}, | ||
| }) | ||
| require.ErrorContains(t, err, "viewRequest.Payload.Function is required") | ||
|
|
||
| _, err = aptoscap.ConvertViewPayloadFromProto(&aptoscap.ViewPayload{ | ||
| Module: &aptoscap.ModuleID{Address: []byte{0x01}}, | ||
| Function: "name", | ||
| }) | ||
| require.ErrorContains(t, err, "viewRequest.Payload.Module.Name is required") | ||
|
|
||
| _, err = aptoscap.ConvertViewPayloadFromProto(&aptoscap.ViewPayload{ | ||
| Module: &aptoscap.ModuleID{Address: make([]byte, typesaptos.AccountAddressLength+1), Name: "coin"}, | ||
| Function: "name", | ||
| }) | ||
| require.ErrorContains(t, err, "module address too long") | ||
| } |
There was a problem hiding this comment.
The negative tests don’t currently cover (a) missing/empty module address and (b) struct type tags with empty module/name (or empty address). If the helper is intended to centralize validation, please add test cases for these required-field scenarios to lock in the expected errors.
Summary
pkg/capabilities/v2/chain-capabilities/aptoshelpers for converting capabilityViewPayloadandTypeTagprotos into Aptos domain typeschainlink-commonTesting
GOWORK=off go test ./pkg/capabilities/v2/chain-capabilities/aptos -count=1