From 021dff775f1c34e230e0dbdc82f21c7c059f4bad Mon Sep 17 00:00:00 2001 From: Michael Street <5597260+MStreet3@users.noreply.github.com> Date: Thu, 19 Dec 2024 05:15:15 -0500 Subject: [PATCH] fix(deployment): enables multiple ocr3 contracts per chain --- deployment/address_book.go | 10 +- deployment/address_book_test.go | 29 +++++ deployment/keystone/changeset/deploy_ocr3.go | 2 + .../keystone/changeset/deploy_ocr3_test.go | 117 +++++++++++++++++- deployment/keystone/deploy.go | 43 ++++++- deployment/keystone/state.go | 10 +- 6 files changed, 203 insertions(+), 8 deletions(-) diff --git a/deployment/address_book.go b/deployment/address_book.go index 3ce0332a4c3..82057240630 100644 --- a/deployment/address_book.go +++ b/deployment/address_book.go @@ -83,7 +83,7 @@ func NewTypeAndVersion(t ContractType, v semver.Version) TypeAndVersion { type AddressBook interface { Save(chainSelector uint64, address string, tv TypeAndVersion) error Addresses() (map[uint64]map[string]TypeAndVersion, error) - AddressesForChain(chain uint64) (map[string]TypeAndVersion, error) + AddressesForChain(chain uint64, opts ...func(map[string]TypeAndVersion)) (map[string]TypeAndVersion, error) // Allows for merging address books (e.g. new deployments with existing ones) Merge(other AddressBook) error Remove(ab AddressBook) error @@ -149,7 +149,7 @@ func (m *AddressBookMap) Addresses() (map[uint64]map[string]TypeAndVersion, erro return m.cloneAddresses(m.addressesByChain), nil } -func (m *AddressBookMap) AddressesForChain(chainSelector uint64) (map[string]TypeAndVersion, error) { +func (m *AddressBookMap) AddressesForChain(chainSelector uint64, opts ...func(map[string]TypeAndVersion)) (map[string]TypeAndVersion, error) { _, err := chainsel.GetChainIDFromSelector(chainSelector) if err != nil { return nil, errors.Wrapf(ErrInvalidChainSelector, "chain selector %d", chainSelector) @@ -165,7 +165,11 @@ func (m *AddressBookMap) AddressesForChain(chainSelector uint64) (map[string]Typ // maps are mutable and pass via a pointer // creating a copy of the map to prevent concurrency // read and changes outside object-bound - return maps.Clone(m.addressesByChain[chainSelector]), nil + cloned := maps.Clone(m.addressesByChain[chainSelector]) + for _, opt := range opts { + opt(cloned) + } + return cloned, nil } // Merge will merge the addresses from another address book into this one. diff --git a/deployment/address_book_test.go b/deployment/address_book_test.go index e022e89a9ab..922432dffe4 100644 --- a/deployment/address_book_test.go +++ b/deployment/address_book_test.go @@ -72,6 +72,35 @@ func TestAddressBook_Save(t *testing.T) { }) } +func TestAddressBook_AddressesForChain(t *testing.T) { + ab := NewMemoryAddressBook() + ocr3Cap100 := NewTypeAndVersion("OCR3Capability", Version1_0_0) + copyOCR3Cap100 := NewTypeAndVersion("OCR3Capability", Version1_0_0) + + addr1 := common.HexToAddress("0x1").String() + addr2 := common.HexToAddress("0x2").String() + + err := ab.Save(chainsel.TEST_90000001.Selector, addr1, ocr3Cap100) + require.NoError(t, err) + + err = ab.Save(chainsel.TEST_90000001.Selector, addr2, copyOCR3Cap100) + require.NoError(t, err) + + addresses, err := ab.AddressesForChain(chainsel.TEST_90000001.Selector, + func(m map[string]TypeAndVersion) { + for k, v := range m { + if v.Type == "OCR3Capability" { + if k != addr1 { + delete(m, k) + } + } + } + }, + ) + require.NoError(t, err) + require.Len(t, addresses, 1) +} + func TestAddressBook_Merge(t *testing.T) { onRamp100 := NewTypeAndVersion("OnRamp", Version1_0_0) onRamp110 := NewTypeAndVersion("OnRamp", Version1_1_0) diff --git a/deployment/keystone/changeset/deploy_ocr3.go b/deployment/keystone/changeset/deploy_ocr3.go index 057bba4c12d..b9f74110bc9 100644 --- a/deployment/keystone/changeset/deploy_ocr3.go +++ b/deployment/keystone/changeset/deploy_ocr3.go @@ -37,6 +37,7 @@ var _ deployment.ChangeSet[ConfigureOCR3Config] = ConfigureOCR3Contract type ConfigureOCR3Config struct { ChainSel uint64 NodeIDs []string + OCR3ContractAddr *string OCR3Config *kslib.OracleConfig DryRun bool WriteGeneratedConfig io.Writer // if not nil, write the generated config to this writer as JSON [OCR2OracleConfig] @@ -55,6 +56,7 @@ func ConfigureOCR3Contract(env deployment.Environment, cfg ConfigureOCR3Config) NodeIDs: cfg.NodeIDs, OCR3Config: cfg.OCR3Config, DryRun: cfg.DryRun, + OCR3Addr: cfg.OCR3ContractAddr, UseMCMS: cfg.UseMCMS(), }) if err != nil { diff --git a/deployment/keystone/changeset/deploy_ocr3_test.go b/deployment/keystone/changeset/deploy_ocr3_test.go index 7a276886242..7d512ae89c8 100644 --- a/deployment/keystone/changeset/deploy_ocr3_test.go +++ b/deployment/keystone/changeset/deploy_ocr3_test.go @@ -84,6 +84,122 @@ func TestConfigureOCR3(t *testing.T) { assert.Nil(t, csOut.Proposals) }) + t.Run("success multiple OCR3 contracts", func(t *testing.T) { + + te := SetupTestEnv(t, TestConfig{ + WFDonConfig: DonConfig{N: 4}, + AssetDonConfig: DonConfig{N: 4}, + WriterDonConfig: DonConfig{N: 4}, + NumChains: 1, + }) + + registrySel := te.Env.AllChainSelectors()[0] + + existingContracts, err := te.Env.ExistingAddresses.AddressesForChain(registrySel) + require.NoError(t, err) + require.Len(t, existingContracts, 3) + + // Find existing OCR3 contract + var existingOCR3Addr string + for addr, tv := range existingContracts { + if tv.Type == kslib.OCR3Capability { + existingOCR3Addr = addr + break + } + } + + // Deploy a new OCR3 contract + resp, err := changeset.DeployOCR3(te.Env, registrySel) + require.NoError(t, err) + require.NotNil(t, resp) + require.NoError(t, te.Env.ExistingAddresses.Merge(resp.AddressBook)) + + // Verify after merge there are three original contracts plus one new one + addrs, err := te.Env.ExistingAddresses.AddressesForChain(registrySel) + require.NoError(t, err) + require.Len(t, addrs, 4) + + // Find new OCR3 contract + var newOCR3Addr string + for addr, tv := range addrs { + if tv.Type == kslib.OCR3Capability && addr != existingOCR3Addr { + newOCR3Addr = addr + break + } + } + + var wfNodes []string + for id, _ := range te.WFNodes { + wfNodes = append(wfNodes, id) + } + + w := &bytes.Buffer{} + cfg := changeset.ConfigureOCR3Config{ + ChainSel: te.RegistrySelector, + NodeIDs: wfNodes, + OCR3ContractAddr: &newOCR3Addr, // Use the new OCR3 contract to configure + OCR3Config: &c, + WriteGeneratedConfig: w, + } + + csOut, err := changeset.ConfigureOCR3Contract(te.Env, cfg) + require.NoError(t, err) + var got kslib.OCR2OracleConfig + err = json.Unmarshal(w.Bytes(), &got) + require.NoError(t, err) + assert.Len(t, got.Signers, 4) + assert.Len(t, got.Transmitters, 4) + assert.Nil(t, csOut.Proposals) + }) + + t.Run("fails to configure with multiple OCR3 contracts because not found", func(t *testing.T) { + te := SetupTestEnv(t, TestConfig{ + WFDonConfig: DonConfig{N: 4}, + AssetDonConfig: DonConfig{N: 4}, + WriterDonConfig: DonConfig{N: 4}, + NumChains: 1, + }) + + registrySel := te.Env.AllChainSelectors()[0] + + existingContracts, err := te.Env.ExistingAddresses.AddressesForChain(registrySel) + require.NoError(t, err) + require.Len(t, existingContracts, 3) + + // Deploy a new OCR3 contract + resp, err := changeset.DeployOCR3(te.Env, registrySel) + require.NoError(t, err) + require.NotNil(t, resp) + require.NoError(t, te.Env.ExistingAddresses.Merge(resp.AddressBook)) + + // Verify after merge there are three original contracts plus one new one + addrs, err := te.Env.ExistingAddresses.AddressesForChain(registrySel) + require.NoError(t, err) + require.Len(t, addrs, 4) + + // Specify a non-existent OCR3 contract, means all deployed contracts will be filtered + // out and the configuration will fail + notFoundAddr := "0x1234567890123456789012345678901234567890" + + var wfNodes []string + for id, _ := range te.WFNodes { + wfNodes = append(wfNodes, id) + } + + w := &bytes.Buffer{} + cfg := changeset.ConfigureOCR3Config{ + ChainSel: te.RegistrySelector, + NodeIDs: wfNodes, + OCR3ContractAddr: ¬FoundAddr, + OCR3Config: &c, + WriteGeneratedConfig: w, + } + + _, err = changeset.ConfigureOCR3Contract(te.Env, cfg) + require.Error(t, err) + require.ErrorContains(t, err, "no ocr3 contract found") + }) + t.Run("mcms", func(t *testing.T) { te := SetupTestEnv(t, TestConfig{ WFDonConfig: DonConfig{N: 4}, @@ -137,5 +253,4 @@ func TestConfigureOCR3(t *testing.T) { }) require.NoError(t, err) }) - } diff --git a/deployment/keystone/deploy.go b/deployment/keystone/deploy.go index 3d415c5d2da..6aab475bea9 100644 --- a/deployment/keystone/deploy.go +++ b/deployment/keystone/deploy.go @@ -351,7 +351,11 @@ type ConfigureOCR3Config struct { ChainSel uint64 NodeIDs []string OCR3Config *OracleConfig - DryRun bool + + // OCR3Addr is the address of the OCR3 contract to configure. If nil, and there are more than + // one deployed contracts on the chain, then the configured contract is non-deterministic. + OCR3Addr *string + DryRun bool UseMCMS bool } @@ -367,9 +371,16 @@ func ConfigureOCR3ContractFromJD(env *deployment.Environment, cfg ConfigureOCR3C if !ok { return nil, fmt.Errorf("chain %d not found in environment", cfg.ChainSel) } + + filterFunc := identityFilterer + if cfg.OCR3Addr != nil { + filterFunc = makeOCR3CapabilityFilterer(*cfg.OCR3Addr) + } + contractSetsResp, err := GetContractSets(env.Logger, &GetContractSetsRequest{ - Chains: env.Chains, - AddressBook: env.ExistingAddresses, + Chains: env.Chains, + AddressBook: env.ExistingAddresses, + AddressFilterer: filterFunc, }) if err != nil { return nil, fmt.Errorf("failed to get contract sets: %w", err) @@ -382,6 +393,13 @@ func ConfigureOCR3ContractFromJD(env *deployment.Environment, cfg ConfigureOCR3C if contract == nil { return nil, fmt.Errorf("no ocr3 contract found for chain %d", cfg.ChainSel) } + + if cfg.OCR3Addr != nil { + if *cfg.OCR3Addr != contract.Address().String() { + return nil, fmt.Errorf("Requested address to configure %s does not match contract address %s", *cfg.OCR3Addr, contract.Address().String()) + } + } + nodes, err := deployment.NodeInfo(cfg.NodeIDs, env.Offchain) if err != nil { return nil, err @@ -971,3 +989,22 @@ func configureForwarder(lggr logger.Logger, chain deployment.Chain, contractSet } return opMap, nil } + +// makeOCR3CapabilityFilterer returns a filter func that deletes any OCR3Capability contract entries +// that do not match a given OCR3 Address. If no address is given, no filtering is done. +func makeOCR3CapabilityFilterer(ocr3Addr string) func(map[string]deployment.TypeAndVersion) { + return func(m map[string]deployment.TypeAndVersion) { + for k, v := range m { + if v.Type == OCR3Capability { + if k != ocr3Addr { + delete(m, k) + } + } + } + } +} + +// identityFilterer is a filter func that does nothing +func identityFilterer(m map[string]deployment.TypeAndVersion) { + // no-op +} diff --git a/deployment/keystone/state.go b/deployment/keystone/state.go index 0ac7cdc89ed..851dea650a1 100644 --- a/deployment/keystone/state.go +++ b/deployment/keystone/state.go @@ -19,6 +19,10 @@ import ( type GetContractSetsRequest struct { Chains map[uint64]deployment.Chain AddressBook deployment.AddressBook + + // AddressFilterer is a function that filters the addresses a given address book. Filtering + // mutates the input map so the input map should be a copy of the original map. + AddressFilterer func(map[string]deployment.TypeAndVersion) } type GetContractSetsResponse struct { @@ -62,8 +66,12 @@ func GetContractSets(lggr logger.Logger, req *GetContractSetsRequest) (*GetContr resp := &GetContractSetsResponse{ ContractSets: make(map[uint64]ContractSet), } + var opts []func(map[string]deployment.TypeAndVersion) + if req.AddressFilterer != nil { + opts = append(opts, req.AddressFilterer) + } for id, chain := range req.Chains { - addrs, err := req.AddressBook.AddressesForChain(id) + addrs, err := req.AddressBook.AddressesForChain(id, opts...) if err != nil { return nil, fmt.Errorf("failed to get addresses for chain %d: %w", id, err) }