From 2a87ac5affe771596d22ad53691a510fd335a9be Mon Sep 17 00:00:00 2001 From: Margaret Ma Date: Thu, 30 Jan 2025 19:15:33 -0500 Subject: [PATCH] change metadata to labels --- deployment/address_book.go | 84 +++++--- deployment/address_book_labels.go | 51 +++++ ...ta_test.go => address_book_labels_test.go} | 30 +-- deployment/address_book_metadata.go | 38 ---- deployment/address_book_test.go | 203 ++++++++++++------ deployment/common/changeset/internal/mcms.go | 6 +- .../common/changeset/internal/mcms_test.go | 6 +- 7 files changed, 265 insertions(+), 153 deletions(-) create mode 100644 deployment/address_book_labels.go rename deployment/{address_book_metadata_test.go => address_book_labels_test.go} (69%) delete mode 100644 deployment/address_book_metadata.go diff --git a/deployment/address_book.go b/deployment/address_book.go index 232b6a8f136..dae4374b0fd 100644 --- a/deployment/address_book.go +++ b/deployment/address_book.go @@ -33,16 +33,16 @@ var ( ) type TypeAndVersion struct { - Type ContractType - Version semver.Version - Metadata MetadataSet + Type ContractType + Version semver.Version + Labels LabelSet } func (tv TypeAndVersion) String() string { - if len(tv.Metadata) == 0 { + if len(tv.Labels) == 0 { return fmt.Sprintf("%s %s", tv.Type, tv.Version.String()) } - mdSlice := tv.Metadata.AsSlice() + mdSlice := tv.Labels.AsSlice() sort.Strings(mdSlice) // stable order return fmt.Sprintf("%s %s %s", tv.Type, @@ -52,7 +52,16 @@ func (tv TypeAndVersion) String() string { } func (tv TypeAndVersion) Equal(other TypeAndVersion) bool { - return tv.String() == other.String() + // Compare Type + if tv.Type != other.Type { + return false + } + // Compare Versions + if !tv.Version.Equal(&other.Version) { + return false + } + // Compare Labels + return tv.Labels.Equal(other.Labels) } func MustTypeAndVersionFromString(s string) TypeAndVersion { @@ -74,22 +83,22 @@ func TypeAndVersionFromString(s string) (TypeAndVersion, error) { if err != nil { return TypeAndVersion{}, err } - metadata := make(MetadataSet) + labels := make(LabelSet) if len(parts) > 2 { - metadata = NewMetadataSet(parts[2:]...) + labels = NewLabelSet(parts[2:]...) } return TypeAndVersion{ - Type: ContractType(parts[0]), - Version: *v, - Metadata: metadata, + Type: ContractType(parts[0]), + Version: *v, + Labels: labels, }, nil } func NewTypeAndVersion(t ContractType, v semver.Version) TypeAndVersion { return TypeAndVersion{ - Type: t, - Version: v, - Metadata: make(MetadataSet), // empty set, + Type: t, + Version: v, + Labels: make(LabelSet), // empty set, } } @@ -291,38 +300,53 @@ func AddressBookContains(ab AddressBook, chain uint64, addrToFind string) (bool, return false, nil } -// typeVersionKey creates a comparable key from Type and Version type typeVersionKey struct { Type ContractType Version string + Labels string // store labels in a canonical form (comma-joined sorted list) +} + +func tvKey(tv TypeAndVersion) typeVersionKey { + sortedLabels := make([]string, 0, len(tv.Labels)) + for lbl := range tv.Labels { + sortedLabels = append(sortedLabels, lbl) + } + sort.Strings(sortedLabels) + return typeVersionKey{ + Type: tv.Type, + Version: tv.Version.String(), + Labels: strings.Join(sortedLabels, ","), + } } // AddressesContainBundle checks if the addresses // contains a single instance of all the addresses in the bundle. // It returns an error if there are more than one instance of a contract. func AddressesContainBundle(addrs map[string]TypeAndVersion, wantTypes []TypeAndVersion) (bool, error) { + // Count how many times each wanted TypeAndVersion is found counts := make(map[typeVersionKey]int) - for _, wantType := range wantTypes { - key := typeVersionKey{Type: wantType.Type, Version: wantType.Version.String()} - for _, haveType := range addrs { - sameType := (wantType.Type == haveType.Type) - sameVersion := wantType.Version.String() == haveType.Version.String() - - if sameType && sameVersion { - counts[key]++ - if counts[key] > 1 { - return false, fmt.Errorf("found more than one instance of contract %s %s", key.Type, key.Version) + for _, wantTV := range wantTypes { + wantKey := tvKey(wantTV) + for _, haveTV := range addrs { + if wantTV.Equal(haveTV) { + // They match exactly (Type, Version, Labels) + counts[wantKey]++ + if counts[wantKey] > 1 { + return false, fmt.Errorf("found more than one instance of contract %s %s (labels=%v)", + wantTV.Type, wantTV.Version.String(), wantTV.Labels) } } } } + + // Ensure we found *all* wantTypes exactly once return len(counts) == len(wantTypes), nil } -// AddMetadata adds a string to the metadata set in the TypeAndVersion. -func (tv *TypeAndVersion) AddMetadata(md string) { - if tv.Metadata == nil { - tv.Metadata = make(MetadataSet) +// AddLabel adds a string to the LabelSet in the TypeAndVersion. +func (tv *TypeAndVersion) AddLabel(label string) { + if tv.Labels == nil { + tv.Labels = make(LabelSet) } - tv.Metadata.Add(md) + tv.Labels.Add(label) } diff --git a/deployment/address_book_labels.go b/deployment/address_book_labels.go new file mode 100644 index 00000000000..4917a9cb0eb --- /dev/null +++ b/deployment/address_book_labels.go @@ -0,0 +1,51 @@ +package deployment + +// LabelSet represents a set of labels on an address book entry. +type LabelSet map[string]struct{} + +// NewLabelSet initializes a new LabelSet with any number of labels. +func NewLabelSet(labels ...string) LabelSet { + set := make(LabelSet) + for _, lb := range labels { + set[lb] = struct{}{} + } + return set +} + +// Add inserts a labels into the set. +func (ls LabelSet) Add(labels string) { + ls[labels] = struct{}{} +} + +// Remove deletes a labels from the set, if it exists. +func (ls LabelSet) Remove(labels string) { + delete(ls, labels) +} + +// Contains checks if the set contains the given labels. +func (ls LabelSet) Contains(labels string) bool { + _, ok := ls[labels] + return ok +} + +// AsSlice returns the labels in a slice. Useful for printing or serialization. +func (ls LabelSet) AsSlice() []string { + out := make([]string, 0, len(ls)) + for labels := range ls { + out = append(out, labels) + } + return out +} + +// Equal checks if two LabelSets are equal. +func (ls LabelSet) Equal(other LabelSet) bool { + if len(ls) != len(other) { + return false + } + for label := range ls { + if _, ok := other[label]; !ok { + return false + } + } + return true +} diff --git a/deployment/address_book_metadata_test.go b/deployment/address_book_labels_test.go similarity index 69% rename from deployment/address_book_metadata_test.go rename to deployment/address_book_labels_test.go index 0af92063e3d..688c22b85ce 100644 --- a/deployment/address_book_metadata_test.go +++ b/deployment/address_book_labels_test.go @@ -6,14 +6,14 @@ import ( "github.com/stretchr/testify/assert" ) -func TestNewMetadataSet(t *testing.T) { - t.Run("no metadata", func(t *testing.T) { - ms := NewMetadataSet() +func TestNewLabelSet(t *testing.T) { + t.Run("no labels", func(t *testing.T) { + ms := NewLabelSet() assert.Empty(t, ms, "expected empty set") }) - t.Run("some metadata", func(t *testing.T) { - ms := NewMetadataSet("foo", "bar") + t.Run("some labels", func(t *testing.T) { + ms := NewLabelSet("foo", "bar") assert.Len(t, ms, 2) assert.True(t, ms.Contains("foo")) assert.True(t, ms.Contains("bar")) @@ -21,21 +21,21 @@ func TestNewMetadataSet(t *testing.T) { }) } -func TestMetadataSet_Add(t *testing.T) { - ms := NewMetadataSet("initial") +func TestLabelSet_Add(t *testing.T) { + ms := NewLabelSet("initial") ms.Add("new") assert.True(t, ms.Contains("initial"), "expected 'initial' in set") assert.True(t, ms.Contains("new"), "expected 'new' in set") - assert.Len(t, ms, 2, "expected 2 distinct metadata in set") + assert.Len(t, ms, 2, "expected 2 distinct labels in set") // Add duplicate "new" again; size should remain 2 ms.Add("new") assert.Len(t, ms, 2, "expected size to remain 2 after adding a duplicate") } -func TestMetadataSet_Remove(t *testing.T) { - ms := NewMetadataSet("remove_me", "keep") +func TestLabelSet_Remove(t *testing.T) { + ms := NewLabelSet("remove_me", "keep") ms.Remove("remove_me") assert.False(t, ms.Contains("remove_me"), "expected 'remove_me' to be removed") @@ -47,20 +47,20 @@ func TestMetadataSet_Remove(t *testing.T) { assert.Len(t, ms, 1, "expected size to remain 1 after removing a non-existent item") } -func TestMetadataSet_Contains(t *testing.T) { - ms := NewMetadataSet("foo", "bar") +func TestLabelSet_Contains(t *testing.T) { + ms := NewLabelSet("foo", "bar") assert.True(t, ms.Contains("foo")) assert.True(t, ms.Contains("bar")) assert.False(t, ms.Contains("baz")) } -func TestMetadataSet_AsSlice(t *testing.T) { - ms := NewMetadataSet("foo", "bar") +func TestLabelSet_AsSlice(t *testing.T) { + ms := NewLabelSet("foo", "bar") slice := ms.AsSlice() // We can't rely on order in a map-based set, so we only check membership and length - assert.Len(t, slice, 2, "expected 2 distinct metadata in slice") + assert.Len(t, slice, 2, "expected 2 distinct labels in slice") // Convert slice to a map for quick membership checks found := make(map[string]bool) diff --git a/deployment/address_book_metadata.go b/deployment/address_book_metadata.go deleted file mode 100644 index 91d58ced613..00000000000 --- a/deployment/address_book_metadata.go +++ /dev/null @@ -1,38 +0,0 @@ -package deployment - -// MetadataSet represents a set of metadata on an address book entry. -type MetadataSet map[string]struct{} - -// NewMetadataSet initializes a new MetadataSet with any number of metadata. -func NewMetadataSet(metadata ...string) MetadataSet { - set := make(MetadataSet) - for _, md := range metadata { - set[md] = struct{}{} - } - return set -} - -// Add inserts a metadata into the set. -func (ms MetadataSet) Add(metadata string) { - ms[metadata] = struct{}{} -} - -// Remove deletes a metadata from the set, if it exists. -func (ms MetadataSet) Remove(metadata string) { - delete(ms, metadata) -} - -// Contains checks if the set contains the given metadata. -func (ms MetadataSet) Contains(metadata string) bool { - _, ok := ms[metadata] - return ok -} - -// AsSlice returns the labels in a slice. Useful for printing or serialization. -func (ms MetadataSet) AsSlice() []string { - out := make([]string, 0, len(ms)) - for metadata := range ms { - out = append(out, metadata) - } - return out -} diff --git a/deployment/address_book_test.go b/deployment/address_book_test.go index 8a7907a3a43..526f02f9042 100644 --- a/deployment/address_book_test.go +++ b/deployment/address_book_test.go @@ -244,37 +244,112 @@ func TestAddressBook_ConcurrencyAndDeadlock(t *testing.T) { wg.Wait() } -func TestAddressesContainsBundle(t *testing.T) { +func TestAddressesContainBundle(t *testing.T) { + t.Parallel() + + // Define some TypeAndVersion values onRamp100 := NewTypeAndVersion("OnRamp", Version1_0_0) onRamp110 := NewTypeAndVersion("OnRamp", Version1_1_0) onRamp120 := NewTypeAndVersion("OnRamp", Version1_2_0) + + // Create one with labels + onRamp100WithLabels := NewTypeAndVersion("OnRamp", Version1_0_0) + onRamp100WithLabels.Labels.Add("sa") + onRamp100WithLabels.Labels.Add("staging") + addr1 := common.HexToAddress("0x1").String() addr2 := common.HexToAddress("0x2").String() addr3 := common.HexToAddress("0x3").String() - // More than one instance should error - _, err := AddressesContainBundle(map[string]TypeAndVersion{ - addr1: onRamp100, - addr2: onRamp100, - }, []TypeAndVersion{onRamp100}) - require.Error(t, err) + tests := []struct { + name string + addrs map[string]TypeAndVersion // input address map + wantTypes []TypeAndVersion // the “bundle” we want + wantErr bool + wantErrMsg string + wantResult bool // expected boolean return when no error + }{ + { + name: "More than one instance => error", + addrs: map[string]TypeAndVersion{ + addr1: onRamp100, + addr2: onRamp100, // duplicate + }, + wantTypes: []TypeAndVersion{onRamp100}, + wantErr: true, + // an example substring check: + wantErrMsg: "found more than one instance of contract", + }, + { + name: "No instance => result false, no error", + addrs: map[string]TypeAndVersion{ + addr1: onRamp110, + addr2: onRamp110, + }, + wantTypes: []TypeAndVersion{onRamp100}, + wantErr: false, + wantResult: false, + }, + { + name: "2 elements => success", + addrs: map[string]TypeAndVersion{ + addr1: onRamp100, + addr2: onRamp110, + addr3: onRamp120, + }, + wantTypes: []TypeAndVersion{onRamp100, onRamp110}, + wantErr: false, + wantResult: true, + }, + { + name: "Mismatched labels => false", + addrs: map[string]TypeAndVersion{ + addr1: onRamp100, // no labels + }, + wantTypes: []TypeAndVersion{onRamp100WithLabels}, + wantErr: false, + wantResult: false, // label mismatch => not found + }, + { + name: "Exact label match => success", + addrs: map[string]TypeAndVersion{ + addr1: onRamp100WithLabels, + }, + wantTypes: []TypeAndVersion{onRamp100WithLabels}, + wantErr: false, + wantResult: true, + }, + { + name: "Duplicate labeled => error", + addrs: map[string]TypeAndVersion{ + addr1: onRamp100WithLabels, + addr2: onRamp100WithLabels, // same type/version/labels => duplicate + }, + wantTypes: []TypeAndVersion{onRamp100WithLabels}, + wantErr: true, + wantErrMsg: "more than one instance of contract", + }, + } - // No such instances should be false - exists, err := AddressesContainBundle(map[string]TypeAndVersion{ - addr2: onRamp110, - addr1: onRamp110, - }, []TypeAndVersion{onRamp100}) - require.NoError(t, err) - assert.Equal(t, exists, false) - - // 2 elements - exists, err = AddressesContainBundle(map[string]TypeAndVersion{ - addr1: onRamp100, - addr2: onRamp110, - addr3: onRamp120, - }, []TypeAndVersion{onRamp100, onRamp110}) - require.NoError(t, err) - assert.Equal(t, exists, true) + for _, tt := range tests { + tt := tt // capture range variable + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + gotResult, gotErr := AddressesContainBundle(tt.addrs, tt.wantTypes) + + if tt.wantErr { + require.Error(t, gotErr, "expected an error but got none") + if tt.wantErrMsg != "" { + require.Contains(t, gotErr.Error(), tt.wantErrMsg) + } + return + } + require.NoError(t, gotErr, "did not expect an error but got one") + assert.Equal(t, tt.wantResult, gotResult, + "expected result %v but got %v", tt.wantResult, gotResult) + }) + } } func TestTypeAndVersionFromString(t *testing.T) { @@ -289,15 +364,15 @@ func TestTypeAndVersionFromString(t *testing.T) { wantMeta []string }{ { - name: "valid - no metadata", + name: "valid - no labels", input: "CallProxy 1.0.0", wantErr: false, wantType: "CallProxy", wantVersion: Version1_0_0, - wantMeta: nil, // no metadata + wantMeta: nil, // no labels }, { - name: "valid - multiple metadata, normal spacing", + name: "valid - multiple labels, normal spacing", input: "CallProxy 1.0.0 SA staging", wantErr: false, wantType: "CallProxy", @@ -305,7 +380,7 @@ func TestTypeAndVersionFromString(t *testing.T) { wantMeta: []string{"SA", "staging"}, }, { - name: "valid - multiple metadata, extra spacing", + name: "valid - multiple labels, extra spacing", input: " CallProxy 1.0.0 SA staging ", wantErr: false, wantType: "CallProxy", @@ -342,46 +417,46 @@ func TestTypeAndVersionFromString(t *testing.T) { // Check Version require.Equal(t, tt.wantVersion.String(), gotTV.Version.String(), "incorrect version") - // Check metadata - gotMeta := gotTV.Metadata.AsSlice() - require.Equal(t, len(tt.wantMeta), len(gotMeta), "metadata length mismatch") + // Check labels + gotMeta := gotTV.Labels.AsSlice() + require.Equal(t, len(tt.wantMeta), len(gotMeta), "labels length mismatch") for _, wantMd := range tt.wantMeta { - require.Contains(t, gotMeta, wantMd, "missing metadata item") + require.Contains(t, gotMeta, wantMd, "missing labels item") } }) } } -func TestTypeAndVersion_AddMetadata(t *testing.T) { +func TestTypeAndVersion_AddLabels(t *testing.T) { t.Parallel() tests := []struct { - name string - initialMetadata []string - toAdd []string - wantContains []string - wantLen int + name string + initialLabels []string + toAdd []string + wantContains []string + wantLen int }{ { - name: "add single metadata to empty set", - initialMetadata: nil, - toAdd: []string{"foo"}, - wantContains: []string{"foo"}, - wantLen: 1, + name: "add single labels to empty set", + initialLabels: nil, + toAdd: []string{"foo"}, + wantContains: []string{"foo"}, + wantLen: 1, }, { - name: "add multiple metadata to existing set", - initialMetadata: []string{"alpha"}, - toAdd: []string{"beta", "gamma"}, - wantContains: []string{"alpha", "beta", "gamma"}, - wantLen: 3, + name: "add multiple labels to existing set", + initialLabels: []string{"alpha"}, + toAdd: []string{"beta", "gamma"}, + wantContains: []string{"alpha", "beta", "gamma"}, + wantLen: 3, }, { - name: "add duplicate metadata", - initialMetadata: []string{"dup"}, - toAdd: []string{"dup", "dup", "new"}, - wantContains: []string{"dup", "new"}, - wantLen: 2, + name: "add duplicate labels", + initialLabels: []string{"dup"}, + toAdd: []string{"dup", "dup", "new"}, + wantContains: []string{"dup", "new"}, + wantLen: 2, }, } @@ -390,25 +465,25 @@ func TestTypeAndVersion_AddMetadata(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - // Construct a TypeAndVersion with any initial metadata + // Construct a TypeAndVersion with any initial labels tv := TypeAndVersion{ - Type: "CallProxy", - Version: Version1_0_0, - Metadata: NewMetadataSet(tt.initialMetadata...), + Type: "CallProxy", + Version: Version1_0_0, + Labels: NewLabelSet(tt.initialLabels...), } - // Call AddMetadata for each item in toAdd - for _, md := range tt.toAdd { - tv.AddMetadata(md) + // Call AddLabel for each item in toAdd + for _, label := range tt.toAdd { + tv.AddLabel(label) } - // Check final metadata length - require.Len(t, tv.Metadata, tt.wantLen, "metadata size mismatch") + // Check final labels length + require.Len(t, tv.Labels, tt.wantLen, "labels size mismatch") - // Check that expected metadata is present + // Check that expected labels is present for _, md := range tt.wantContains { - require.True(t, tv.Metadata.Contains(md), - "expected metadata %q was not found in tv.Metadata", md) + require.True(t, tv.Labels.Contains(md), + "expected labels %q was not found in tv.Labels", md) } }) } diff --git a/deployment/common/changeset/internal/mcms.go b/deployment/common/changeset/internal/mcms.go index fc673084396..f40d73acc83 100644 --- a/deployment/common/changeset/internal/mcms.go +++ b/deployment/common/changeset/internal/mcms.go @@ -18,7 +18,7 @@ type DeployMCMSOption func(*deployment.TypeAndVersion) // WithLabel is a functional option that sets a label on the TypeAndVersion. func WithLabel(label string) DeployMCMSOption { return func(tv *deployment.TypeAndVersion) { - tv.AddMetadata(label) + tv.AddLabel(label) } } @@ -139,7 +139,7 @@ func DeployMCMSWithTimelockContracts( tv := deployment.NewTypeAndVersion(types.RBACTimelock, deployment.Version1_0_0) if config.Label != nil { - tv.AddMetadata(*config.Label) + tv.AddLabel(*config.Label) } return deployment.ContractDeploy[*owner_helpers.RBACTimelock]{ @@ -161,7 +161,7 @@ func DeployMCMSWithTimelockContracts( tv := deployment.NewTypeAndVersion(types.CallProxy, deployment.Version1_0_0) if config.Label != nil { - tv.AddMetadata(*config.Label) + tv.AddLabel(*config.Label) } return deployment.ContractDeploy[*owner_helpers.CallProxy]{ diff --git a/deployment/common/changeset/internal/mcms_test.go b/deployment/common/changeset/internal/mcms_test.go index 10d06fd8780..f404a645851 100644 --- a/deployment/common/changeset/internal/mcms_test.go +++ b/deployment/common/changeset/internal/mcms_test.go @@ -33,7 +33,7 @@ func TestDeployMCMSWithConfig(t *testing.T) { proposalutils.SingleGroupMCMS(t), ) require.NoError(t, err) - require.Empty(t, mcmNoLabel.Tv.Metadata, "expected no label to be set") + require.Empty(t, mcmNoLabel.Tv.Labels, "expected no label to be set") // 2) Test WITH a label label := "SA" @@ -46,8 +46,8 @@ func TestDeployMCMSWithConfig(t *testing.T) { internal.WithLabel(label), ) require.NoError(t, err) - require.NotNil(t, mcmWithLabel.Tv.Metadata, "expected metadata to be set") - require.Contains(t, mcmWithLabel.Tv.Metadata, label, "label mismatch") + require.NotNil(t, mcmWithLabel.Tv.Labels, "expected labels to be set") + require.Contains(t, mcmWithLabel.Tv.Labels, label, "label mismatch") } func TestDeployMCMSWithTimelockContracts(t *testing.T) {