Feat/filter labeled addresses

.changeset/young-poems-tan.md

@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+fix(chainlink/deployment): loads unlabeled contracts from address book by default

deployment/address_book.go

@@ -315,28 +315,37 @@ func tvKey(tv TypeAndVersion) typeVersionKey {
 	}
 }
 
-// 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 _, 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=%s)",
-						wantTV.Type, wantTV.Version.String(), wantTV.Labels.String())
-				}
-			}
+// EnsureDeduped ensures that each contract in the bundle only appears once
+// in the address map.  It returns an error if there are more than one instance of a contract.
+// Returns true if every value in the bundle is found once, false otherwise.
+func EnsureDeduped(addrs map[string]TypeAndVersion, bundle []TypeAndVersion) (bool, error) {
+	var (
+		grouped = toTypeAndVersionMap(addrs)
+		found   = make([]TypeAndVersion, 0)
+	)
+	for _, btv := range bundle {
+		key := tvKey(btv)
+		matched, ok := grouped[key]
+		if ok {
+			found = append(found, btv)
+		}
+		if len(matched) > 1 {
+			return false, fmt.Errorf("found more than one instance of contract %s v%s (labels=%s)",
+				key.Type, key.Version, key.Labels)
 		}
 	}
 
-	// Ensure we found *all* wantTypes exactly once
-	return len(counts) == len(wantTypes), nil
+	// Indicate if each TypeAndVersion in the bundle is found at least once
+	return len(found) == len(bundle), nil
+}
+
+// toTypeAndVersionMap groups contract addresses by unique TypeAndVersion.
+func toTypeAndVersionMap(addrs map[string]TypeAndVersion) map[typeVersionKey][]string {
+	tvkMap := make(map[typeVersionKey][]string)
+	for k, v := range addrs {
+		tvkMap[tvKey(v)] = append(tvkMap[tvKey(v)], k)
+	}
+	return tvkMap
 }
 
 // AddLabel adds a string to the LabelSet in the TypeAndVersion.

deployment/address_book_labels.go

@@ -65,3 +65,7 @@ func (ls LabelSet) Equal(other LabelSet) bool {
 	}
 	return true
 }
+
+func (ls LabelSet) IsEmpty() bool {
+	return len(ls) == 0
+}

deployment/address_book_test.go

@@ -244,7 +244,7 @@ func TestAddressBook_ConcurrencyAndDeadlock(t *testing.T) {
 	wg.Wait()
 }
 
-func TestAddressesContainBundle(t *testing.T) {
+func Test_EnsureDeduped(t *testing.T) {
 	t.Parallel()
 
 	// Define some TypeAndVersion values
@@ -336,7 +336,7 @@ func TestAddressesContainBundle(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			gotResult, gotErr := AddressesContainBundle(tt.addrs, tt.wantTypes)
+			gotResult, gotErr := EnsureDeduped(tt.addrs, tt.wantTypes)
 
 			if tt.wantErr {
 				require.Error(t, gotErr, "expected an error but got none")
@@ -491,3 +491,56 @@ func TestTypeAndVersion_AddLabels(t *testing.T) {
 		})
 	}
 }
+
+func Test_toTypeAndVersionMap(t *testing.T) {
+	v100 := semver.MustParse("1.0.0")
+
+	tests := []struct {
+		name  string
+		addrs map[string]TypeAndVersion
+		want  map[typeVersionKey][]string
+	}{
+		{
+			name: "OK_single entry",
+			addrs: map[string]TypeAndVersion{
+				"addr1": {Type: "type1", Version: *v100},
+			},
+			want: map[typeVersionKey][]string{
+				{Type: "type1", Version: "1.0.0", Labels: ""}: {"addr1"},
+			},
+		},
+		{
+			name: "OK_multiple entries same type no labels",
+			addrs: map[string]TypeAndVersion{
+				"addr1": {Type: "type1", Version: *v100},
+				"addr2": {Type: "type1", Version: *v100},
+			},
+			want: map[typeVersionKey][]string{
+				{Type: "type1", Version: "1.0.0", Labels: ""}: {"addr1", "addr2"},
+			},
+		},
+		{
+			name: "OK_multiple entries same type with labels",
+			addrs: map[string]TypeAndVersion{
+				"addr1": {Type: "type1", Version: *v100, Labels: NewLabelSet("test")},
+				"addr2": {Type: "type1", Version: *v100},
+			},
+			want: map[typeVersionKey][]string{
+				{Type: "type1", Version: "1.0.0", Labels: "test"}: {"addr1"},
+				{Type: "type1", Version: "1.0.0", Labels: ""}:     {"addr2"},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := toTypeAndVersionMap(tt.addrs)
+			require.Equal(t, len(tt.want), len(got))
+			for k, gotAddresses := range got {
+				wantAddresses, ok := tt.want[k]
+				require.True(t, ok)
+				require.ElementsMatch(t, wantAddresses, gotAddresses)
+			}
+		})
+	}
+}

deployment/common/changeset/state.go

@@ -79,78 +79,96 @@ func MaybeLoadMCMSWithTimelockState(env deployment.Environment, chainSelectors [
 
 // MaybeLoadMCMSWithTimelockChainState looks for the addresses corresponding to
 // contracts deployed with DeployMCMSWithTimelock and loads them into a
-// MCMSWithTimelockState struct. If none of the contracts are found, the state struct will be nil.
+// MCMSWithTimelockState struct.  If none of the contracts are found, the state struct will be nil.
+//
 // An error indicates:
 // - Found but was unable to load a contract
 // - It only found part of the bundle of contracts
 // - If found more than one instance of a contract (we expect one bundle in the given addresses)
-func MaybeLoadMCMSWithTimelockChainState(chain deployment.Chain, addresses map[string]deployment.TypeAndVersion) (*MCMSWithTimelockState, error) {
-	state := MCMSWithTimelockState{
-		MCMSWithTimelockContracts: &proposalutils.MCMSWithTimelockContracts{},
-	}
-	// We expect one of each contract on the chain.
-	timelock := deployment.NewTypeAndVersion(types.RBACTimelock, deployment.Version1_0_0)
-	callProxy := deployment.NewTypeAndVersion(types.CallProxy, deployment.Version1_0_0)
-	proposer := deployment.NewTypeAndVersion(types.ProposerManyChainMultisig, deployment.Version1_0_0)
-	canceller := deployment.NewTypeAndVersion(types.CancellerManyChainMultisig, deployment.Version1_0_0)
-	bypasser := deployment.NewTypeAndVersion(types.BypasserManyChainMultisig, deployment.Version1_0_0)
-	// the same contract can have different roles
-	multichain := deployment.NewTypeAndVersion(types.ManyChainMultisig, deployment.Version1_0_0)
+func MaybeLoadMCMSWithTimelockChainState(
+	chain deployment.Chain,
+	addresses map[string]deployment.TypeAndVersion,
+) (*MCMSWithTimelockState, error) {
+	var (
+		state = MCMSWithTimelockState{
+			MCMSWithTimelockContracts: &proposalutils.MCMSWithTimelockContracts{},
+		}
+
+		// We expect one of each contract on the chain.
+		timelock  = deployment.NewTypeAndVersion(types.RBACTimelock, deployment.Version1_0_0)
+		callProxy = deployment.NewTypeAndVersion(types.CallProxy, deployment.Version1_0_0)
+		proposer  = deployment.NewTypeAndVersion(types.ProposerManyChainMultisig, deployment.Version1_0_0)
+		canceller = deployment.NewTypeAndVersion(types.CancellerManyChainMultisig, deployment.Version1_0_0)
+		bypasser  = deployment.NewTypeAndVersion(types.BypasserManyChainMultisig, deployment.Version1_0_0)
+
+		// the same contract can have different roles
+		multichain    = deployment.NewTypeAndVersion(types.ManyChainMultisig, deployment.Version1_0_0)
+		proposerMCMS  = deployment.NewTypeAndVersion(types.ManyChainMultisig, deployment.Version1_0_0)
+		bypasserMCMS  = deployment.NewTypeAndVersion(types.ManyChainMultisig, deployment.Version1_0_0)
+		cancellerMCMS = deployment.NewTypeAndVersion(types.ManyChainMultisig, deployment.Version1_0_0)
+	)
 
 	// Convert map keys to a slice
-	wantTypes := []deployment.TypeAndVersion{timelock, proposer, canceller, bypasser, callProxy}
+	proposerMCMS.Labels.Add(types.ProposerRole.String())
+	bypasserMCMS.Labels.Add(types.BypasserRole.String())
+	cancellerMCMS.Labels.Add(types.CancellerRole.String())
+	wantTypes := []deployment.TypeAndVersion{timelock, proposer, canceller, bypasser, callProxy,
+		proposerMCMS, bypasserMCMS, cancellerMCMS,
+	}
 
 	// Ensure we either have the bundle or not.
-	_, err := deployment.AddressesContainBundle(addresses, wantTypes)
+	_, err := deployment.EnsureDeduped(addresses, wantTypes)
 	if err != nil {
 		return nil, fmt.Errorf("unable to check MCMS contracts on chain %s error: %w", chain.Name(), err)
 	}
 
-	for address, tvStr := range addresses {
+	for address, tv := range addresses {
 		switch {
-		case tvStr.Type == timelock.Type && tvStr.Version.String() == timelock.Version.String():
+		case tv.Type == timelock.Type && tv.Version.String() == timelock.Version.String():
 			tl, err := owner_helpers.NewRBACTimelock(common.HexToAddress(address), chain.Client)
 			if err != nil {
 				return nil, err
 			}
 			state.Timelock = tl
-		case tvStr.Type == callProxy.Type && tvStr.Version.String() == callProxy.Version.String():
+		case tv.Type == callProxy.Type && tv.Version.String() == callProxy.Version.String():
 			cp, err := owner_helpers.NewCallProxy(common.HexToAddress(address), chain.Client)
 			if err != nil {
 				return nil, err
 			}
 			state.CallProxy = cp
-		case tvStr.Type == proposer.Type && tvStr.Version.String() == proposer.Version.String():
+		case tv.Type == proposer.Type && tv.Version.String() == proposer.Version.String():
 			mcms, err := owner_helpers.NewManyChainMultiSig(common.HexToAddress(address), chain.Client)
 			if err != nil {
 				return nil, err
 			}
 			state.ProposerMcm = mcms
-		case tvStr.Type == bypasser.Type && tvStr.Version.String() == bypasser.Version.String():
+		case tv.Type == bypasser.Type && tv.Version.String() == bypasser.Version.String():
 			mcms, err := owner_helpers.NewManyChainMultiSig(common.HexToAddress(address), chain.Client)
 			if err != nil {
 				return nil, err
 			}
 			state.BypasserMcm = mcms
-		case tvStr.Type == canceller.Type && tvStr.Version.String() == canceller.Version.String():
+		case tv.Type == canceller.Type && tv.Version.String() == canceller.Version.String():
 			mcms, err := owner_helpers.NewManyChainMultiSig(common.HexToAddress(address), chain.Client)
 			if err != nil {
 				return nil, err
 			}
 			state.CancellerMcm = mcms
-		case tvStr.Type == multichain.Type && tvStr.Version.String() == multichain.Version.String():
-			// the same contract can have different roles so we use the labels to determine which role it is
+		case tv.Type == multichain.Type && tv.Version.String() == multichain.Version.String():
+			// Contract of type ManyChainMultiSig must be labeled to assign to the proper state
+			// field.  If a specifically typed contract already occupies the field, then this
+			// contract will be ignored.
 			mcms, err := owner_helpers.NewManyChainMultiSig(common.HexToAddress(address), chain.Client)
 			if err != nil {
 				return nil, err
 			}
-			if tvStr.Labels.Contains(types.ProposerRole.String()) {
+			if tv.Labels.Contains(types.ProposerRole.String()) && state.ProposerMcm == nil {
 				state.ProposerMcm = mcms
 			}
-			if tvStr.Labels.Contains(types.BypasserRole.String()) {
+			if tv.Labels.Contains(types.BypasserRole.String()) && state.BypasserMcm == nil {
 				state.BypasserMcm = mcms
 			}
-			if tvStr.Labels.Contains(types.CancellerRole.String()) {
+			if tv.Labels.Contains(types.CancellerRole.String()) && state.CancellerMcm == nil {
 				state.CancellerMcm = mcms
 			}
 		}
@@ -199,7 +217,7 @@ func MaybeLoadLinkTokenChainState(chain deployment.Chain, addresses map[string]d
 	wantTypes := []deployment.TypeAndVersion{linkToken}
 
 	// Ensure we either have the bundle or not.
-	_, err := deployment.AddressesContainBundle(addresses, wantTypes)
+	_, err := deployment.EnsureDeduped(addresses, wantTypes)
 	if err != nil {
 		return nil, fmt.Errorf("unable to check link token on chain %s error: %w", chain.Name(), err)
 	}
@@ -235,7 +253,7 @@ func MaybeLoadStaticLinkTokenState(chain deployment.Chain, addresses map[string]
 	wantTypes := []deployment.TypeAndVersion{staticLinkToken}
 
 	// Ensure we either have the bundle or not.
-	_, err := deployment.AddressesContainBundle(addresses, wantTypes)
+	_, err := deployment.EnsureDeduped(addresses, wantTypes)
 	if err != nil {
 		return nil, fmt.Errorf("unable to check static link token on chain %s error: %w", chain.Name(), err)
 	}

deployment/common/changeset/state/evm.go

@@ -101,7 +101,7 @@ func MaybeLoadMCMSWithTimelockChainState(chain deployment.Chain, addresses map[s
 	wantTypes := []deployment.TypeAndVersion{timelock, proposer, canceller, bypasser, callProxy}
 
 	// Ensure we either have the bundle or not.
-	_, err := deployment.AddressesContainBundle(addresses, wantTypes)
+	_, err := deployment.EnsureDeduped(addresses, wantTypes)
 	if err != nil {
 		return nil, fmt.Errorf("unable to check MCMS contracts on chain %s error: %w", chain.Name(), err)
 	}
@@ -199,7 +199,7 @@ func MaybeLoadLinkTokenChainState(chain deployment.Chain, addresses map[string]d
 	wantTypes := []deployment.TypeAndVersion{linkToken}
 
 	// Ensure we either have the bundle or not.
-	_, err := deployment.AddressesContainBundle(addresses, wantTypes)
+	_, err := deployment.EnsureDeduped(addresses, wantTypes)
 	if err != nil {
 		return nil, fmt.Errorf("unable to check link token on chain %s error: %w", chain.Name(), err)
 	}
@@ -235,7 +235,7 @@ func MaybeLoadStaticLinkTokenState(chain deployment.Chain, addresses map[string]
 	wantTypes := []deployment.TypeAndVersion{staticLinkToken}
 
 	// Ensure we either have the bundle or not.
-	_, err := deployment.AddressesContainBundle(addresses, wantTypes)
+	_, err := deployment.EnsureDeduped(addresses, wantTypes)
 	if err != nil {
 		return nil, fmt.Errorf("unable to check static link token on chain %s error: %w", chain.Name(), err)
 	}

deployment/common/changeset/state/solana.go

@@ -200,7 +200,7 @@ func MaybeLoadMCMSWithTimelockChainStateSolana(chain deployment.SolChain, addres
 	}
 
 	// Ensure we either have the bundle or not.
-	_, err := deployment.AddressesContainBundle(addresses, wantTypes)
+	_, err := deployment.EnsureDeduped(addresses, wantTypes)
 	if err != nil {
 		return nil, fmt.Errorf("unable to check MCMS contracts on chain %s error: %w", chain.Name(), err)
 	}