Skip to content

Commit 04544db

Browse files
vyzaldysanchezvyzaldysanchez
authored
Support writer- prefix on cap writer names (#18723)
* Supports `writer-` prefix on cap writer names * Fixes lint * Improves validation
1 parent 49fde9e commit 04544db

File tree

2 files changed

+123
-60
lines changed

2 files changed

+123
-60
lines changed

‎deployment/keystone/changeset/add_capabilities.go‎

Lines changed: 61 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@ package changeset
33
import (
44
"errors"
55
"fmt"
6-
"strconv"
6+
"regexp"
77
"strings"
88

9-
chainselectors "github.com/smartcontractkit/chain-selectors"
109
"github.com/smartcontractkit/mcms"
1110
mcmssdk "github.com/smartcontractkit/mcms/sdk"
1211
mcmstypes "github.com/smartcontractkit/mcms/types"
@@ -20,15 +19,20 @@ import (
2019
)
2120

2221
const (
23-
CapabilityTypeTarget = uint8(3) // See: https://github.com/smartcontractkit/chainlink/blob/3684365e78ef911d7668e724aa782d3b3f3e8801/deployment/keystone/changeset/internal/capability_definitions.go#L15
24-
CapabilityTypeTargetNamePrefix = "write_"
22+
CapabilityTypeTarget = uint8(3) // See: https://github.com/smartcontractkit/chainlink/blob/3684365e78ef911d7668e724aa782d3b3f3e8801/deployment/keystone/changeset/internal/capability_definitions.go#L15
23+
CapabilityTypeTargetNamePrefix1 = "write_"
24+
CapabilityTypeTargetNamePrefix2 = "write-"
2525
)
2626

2727
var (
28-
ErrEmptyWriteCapName = errors.New("capability labelled name must not be empty")
29-
ErrInvalidWriteCapName = errors.New("capability labelled name must start with " + CapabilityTypeTargetNamePrefix)
30-
ErrEmptyTrimmedWriteCapName = errors.New("capability labelled name must not be empty after removing prefix " + CapabilityTypeTargetNamePrefix)
31-
ErrInvalidWriteCapNameFormat = errors.New("capability labelled name is not a valid chain name or chain ID")
28+
ErrEmptyWriteCapName = errors.New("capability labelled name must not be empty")
29+
ErrInvalidWriteCapName = errors.New("capability labelled name must start with 'write_' or 'write-' and contain a valid chain name or chain ID")
30+
ErrEmptyWriteCapNetworkNameOrChainID = errors.New("network_name/chain_ID must not be empty")
31+
ErrInvalidWriteCapNameFormat = errors.New("capability labelled name is not a valid chain name or chain ID")
32+
33+
writeCapNameRegex = regexp.MustCompile(`^write[_-](.+)$`)
34+
writeCapChainFamilyNameRegex = regexp.MustCompile(`^([a-z]+)$`)
35+
writeCapNetworkNameOrChainIDRegex = regexp.MustCompile(`^([a-z]+(-[a-z]+)*(-\d+)?|\d+)$`)
3236
)
3337

3438
// AddCapabilitiesRequest is a request to add capabilities
@@ -56,35 +60,11 @@ func (r *AddCapabilitiesRequest) Validate(env cldf.Environment) error {
5660
if c.CapabilityType != CapabilityTypeTarget {
5761
continue
5862
}
59-
if c.LabelledName == "" {
60-
capNameErr = errors.Join(ErrEmptyWriteCapName, capNameErr)
61-
continue
62-
}
63-
if !strings.HasPrefix(c.LabelledName, CapabilityTypeTargetNamePrefix) {
64-
capNameErr = errors.Join(ErrInvalidWriteCapName, capNameErr)
65-
continue
66-
}
67-
extracted := strings.TrimPrefix(c.LabelledName, CapabilityTypeTargetNamePrefix)
68-
if extracted == "" {
69-
capNameErr = errors.Join(ErrEmptyTrimmedWriteCapName, capNameErr)
63+
64+
if err := ValidateWriteTargetName(c.LabelledName); err != nil {
65+
capNameErr = errors.Join(err, capNameErr)
7066
continue
7167
}
72-
_, err := chainselectors.ChainIdFromName(extracted)
73-
if err != nil {
74-
// Validate if the extracted value is the chain ID instead, since the labelled name can contain
75-
// both the chain ID or the chain name.
76-
// See: https://github.com/smartcontractkit/chainlink/blob/3684365e78ef911d7668e724aa782d3b3f3e8801/core/services/relay/evm/write_target.go#L75
77-
chainID, chainIDErr := strconv.ParseUint(extracted, 10, 64)
78-
if chainIDErr == nil {
79-
_, chainIDErr = chainselectors.NameFromChainId(chainID)
80-
if chainIDErr == nil {
81-
// If it is a valid chain ID, we don't error and continue
82-
continue
83-
}
84-
}
85-
86-
capNameErr = errors.Join(ErrInvalidWriteCapNameFormat, capNameErr)
87-
}
8868
}
8969

9070
if capNameErr != nil {
@@ -178,3 +158,49 @@ func AddCapabilities(env cldf.Environment, req *AddCapabilitiesRequest) (cldf.Ch
178158
}
179159
return out, nil
180160
}
161+
162+
// ValidateWriteTargetName checks if a write target name matches the expected format generated by NewWriteTargetID (before the `@version`).
163+
// See source here: https://github.com/smartcontractkit/chainlink-framework/blob/main/capabilities/writetarget/write_target.go#L132
164+
func ValidateWriteTargetName(name string) error {
165+
if name == "" {
166+
return ErrEmptyWriteCapName
167+
}
168+
169+
// Use the same regex pattern to match `write_` or `write-` prefix
170+
matches := writeCapNameRegex.FindStringSubmatch(name)
171+
if len(matches) < 2 {
172+
return ErrInvalidWriteCapName
173+
}
174+
175+
core := matches[1]
176+
if core == "" {
177+
return ErrEmptyWriteCapNetworkNameOrChainID
178+
}
179+
180+
var chainFamilyName, networkNameOrChainID string
181+
// Try to split on the first '-' (chainFamilyName is optional)
182+
dashIdx := strings.Index(core, "-")
183+
if dashIdx == -1 {
184+
// No chain family, so core is just networkNameOrChainID
185+
chainFamilyName = ""
186+
networkNameOrChainID = core
187+
} else {
188+
chainFamilyName = core[:dashIdx]
189+
networkNameOrChainID = core[dashIdx+1:]
190+
}
191+
192+
// chainFamilyName is optional, but if provided, it must match the regex
193+
if chainFamilyName != "" && !writeCapChainFamilyNameRegex.MatchString(chainFamilyName) {
194+
return fmt.Errorf("chain family name '%s' is not valid", chainFamilyName)
195+
}
196+
197+
if networkNameOrChainID == "" {
198+
return ErrEmptyWriteCapNetworkNameOrChainID
199+
}
200+
201+
if !writeCapNetworkNameOrChainIDRegex.MatchString(networkNameOrChainID) {
202+
return fmt.Errorf("network name or chain ID '%s' is not valid", networkNameOrChainID)
203+
}
204+
205+
return nil
206+
}

‎deployment/keystone/changeset/add_capabilities_test.go‎

Lines changed: 62 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package changeset_test
22

33
import (
4+
"errors"
45
"fmt"
56
"testing"
67

@@ -88,34 +89,59 @@ func TestAddCapabilitiesRequest_Validate_WriterCapability(t *testing.T) {
8889
expectedError error
8990
}{
9091
{
91-
name: "valid request with chain ID on capability name",
92+
name: "valid request with chain ID on capability name and `writer_` prefix",
9293
req: func(te test.EnvWrapper) (*changeset.AddCapabilitiesRequest, error) {
9394
chainID, err := chainselectors.GetChainIDFromSelector(chainselectors.TEST_90000001.Selector)
9495
if err != nil {
9596
return nil, err
9697
}
9798
return &changeset.AddCapabilitiesRequest{
9899
RegistryChainSel: te.RegistrySelector,
99-
Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: fmt.Sprintf("%s%s", changeset.CapabilityTypeTargetNamePrefix, chainID), Version: "1.0.0", CapabilityType: changeset.CapabilityTypeTarget}},
100+
Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: fmt.Sprintf("%s%s", changeset.CapabilityTypeTargetNamePrefix1, chainID), Version: "1.0.0", CapabilityType: changeset.CapabilityTypeTarget}},
101+
RegistryRef: te.CapabilityRegistryAddressRef(),
102+
}, nil
103+
},
104+
expectedError: nil,
105+
},
106+
{
107+
name: "valid request with chain ID on capability name and `writer-` prefix",
108+
req: func(te test.EnvWrapper) (*changeset.AddCapabilitiesRequest, error) {
109+
chainID, err := chainselectors.GetChainIDFromSelector(chainselectors.TEST_90000001.Selector)
110+
if err != nil {
111+
return nil, err
112+
}
113+
return &changeset.AddCapabilitiesRequest{
114+
RegistryChainSel: te.RegistrySelector,
115+
Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: fmt.Sprintf("%s%s", changeset.CapabilityTypeTargetNamePrefix2, chainID), Version: "1.0.0", CapabilityType: changeset.CapabilityTypeTarget}},
116+
RegistryRef: te.CapabilityRegistryAddressRef(),
117+
}, nil
118+
},
119+
expectedError: nil,
120+
},
121+
{
122+
name: "valid request with chain name on capability name and `writer_` prefix",
123+
req: func(te test.EnvWrapper) (*changeset.AddCapabilitiesRequest, error) {
124+
chainName := "random-chain-name"
125+
return &changeset.AddCapabilitiesRequest{
126+
RegistryChainSel: te.RegistrySelector,
127+
Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: fmt.Sprintf("%s%s", changeset.CapabilityTypeTargetNamePrefix1, chainName), Version: "1.0.0", CapabilityType: changeset.CapabilityTypeTarget}},
128+
RegistryRef: te.CapabilityRegistryAddressRef(),
129+
}, nil
130+
},
131+
expectedError: nil,
132+
},
133+
{
134+
name: "valid request with chain name on capability name and `writer-` prefix",
135+
req: func(te test.EnvWrapper) (*changeset.AddCapabilitiesRequest, error) {
136+
chainName := "random-chain-name-1"
137+
return &changeset.AddCapabilitiesRequest{
138+
RegistryChainSel: te.RegistrySelector,
139+
Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: fmt.Sprintf("%s%s", changeset.CapabilityTypeTargetNamePrefix2, chainName), Version: "1.0.0", CapabilityType: changeset.CapabilityTypeTarget}},
100140
RegistryRef: te.CapabilityRegistryAddressRef(),
101141
}, nil
102142
},
103143
expectedError: nil,
104144
},
105-
// Cannot test this since `chainselectors.ChainIdFromName()` uses the chains from the `.yaml` files,
106-
// and the chain name is not set in the `test_selectors.yaml` file.
107-
// {
108-
// name: "valid request with chain name on capability name",
109-
// req: func(te test.EnvWrapper) (*changeset.AddCapabilitiesRequest, error) {
110-
// chain := te.Env.BlockChains.EVMChains()[chainselectors.TEST_90000001.Selector]
111-
// return &changeset.AddCapabilitiesRequest{
112-
// RegistryChainSel: te.RegistrySelector,
113-
// Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: fmt.Sprintf("%s%s", changeset.CapabilityTypeTargetNamePrefix, chain.Name()), Version: "1.0.0", CapabilityType: changeset.CapabilityTypeTarget}},
114-
// RegistryRef: te.CapabilityRegistryAddressRef(),
115-
// }, nil
116-
// },
117-
// expectError: false,
118-
// },
119145
{
120146
name: "empty capability name",
121147
req: func(te test.EnvWrapper) (*changeset.AddCapabilitiesRequest, error) {
@@ -132,11 +158,11 @@ func TestAddCapabilitiesRequest_Validate_WriterCapability(t *testing.T) {
132158
req: func(te test.EnvWrapper) (*changeset.AddCapabilitiesRequest, error) {
133159
return &changeset.AddCapabilitiesRequest{
134160
RegistryChainSel: te.RegistrySelector,
135-
Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: changeset.CapabilityTypeTargetNamePrefix, Version: "1.0.0", CapabilityType: changeset.CapabilityTypeTarget}},
161+
Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: changeset.CapabilityTypeTargetNamePrefix1, Version: "1.0.0", CapabilityType: changeset.CapabilityTypeTarget}},
136162
RegistryRef: te.CapabilityRegistryAddressRef(),
137163
}, nil
138164
},
139-
expectedError: changeset.ErrEmptyTrimmedWriteCapName,
165+
expectedError: changeset.ErrInvalidWriteCapName,
140166
},
141167
{
142168
name: "missing prefix on capability name",
@@ -150,26 +176,37 @@ func TestAddCapabilitiesRequest_Validate_WriterCapability(t *testing.T) {
150176
expectedError: changeset.ErrInvalidWriteCapName,
151177
},
152178
{
153-
name: "invalid chain name on capability name",
179+
name: "mixed chars after prefix as chain family",
180+
req: func(te test.EnvWrapper) (*changeset.AddCapabilitiesRequest, error) {
181+
return &changeset.AddCapabilitiesRequest{
182+
RegistryChainSel: te.RegistrySelector,
183+
Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: changeset.CapabilityTypeTargetNamePrefix2 + "test23-test", Version: "1.0.0", CapabilityType: 3}},
184+
RegistryRef: te.CapabilityRegistryAddressRef(),
185+
}, nil
186+
},
187+
expectedError: errors.New("chain family name 'test23' is not valid"),
188+
},
189+
{
190+
name: "mixed chars after prefix as network name",
154191
req: func(te test.EnvWrapper) (*changeset.AddCapabilitiesRequest, error) {
155192
return &changeset.AddCapabilitiesRequest{
156193
RegistryChainSel: te.RegistrySelector,
157-
Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: changeset.CapabilityTypeTargetNamePrefix + "test-cap", Version: "1.0.0", CapabilityType: 3}},
194+
Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: changeset.CapabilityTypeTargetNamePrefix1 + "test-cap123", Version: "1.0.0", CapabilityType: 3}},
158195
RegistryRef: te.CapabilityRegistryAddressRef(),
159196
}, nil
160197
},
161-
expectedError: changeset.ErrInvalidWriteCapNameFormat,
198+
expectedError: errors.New("network name or chain ID 'cap123' is not valid"),
162199
},
163200
{
164-
name: "invalid chain ID on capability name",
201+
name: "with chain family but without network name or chain ID",
165202
req: func(te test.EnvWrapper) (*changeset.AddCapabilitiesRequest, error) {
166203
return &changeset.AddCapabilitiesRequest{
167204
RegistryChainSel: te.RegistrySelector,
168-
Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: changeset.CapabilityTypeTargetNamePrefix + "12345", Version: "1.0.0", CapabilityType: 3}},
205+
Capabilities: []kcr.CapabilitiesRegistryCapability{{LabelledName: changeset.CapabilityTypeTargetNamePrefix1 + "family-", Version: "1.0.0", CapabilityType: 3}},
169206
RegistryRef: te.CapabilityRegistryAddressRef(),
170207
}, nil
171208
},
172-
expectedError: changeset.ErrInvalidWriteCapNameFormat,
209+
expectedError: changeset.ErrEmptyWriteCapNetworkNameOrChainID,
173210
},
174211
}
175212

@@ -189,7 +226,7 @@ func TestAddCapabilitiesRequest_Validate_WriterCapability(t *testing.T) {
189226
require.NoError(t, err)
190227
err = req.Validate(te.Env)
191228
if tt.expectedError != nil {
192-
assert.ErrorIs(t, err, tt.expectedError)
229+
assert.ErrorContains(t, err, tt.expectedError.Error())
193230
} else {
194231
assert.NoError(t, err)
195232
}

0 commit comments

Comments
 (0)