From 52cbb18005f8c7eaaec3e5368638f898bf7e971a Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Tue, 25 Jul 2023 13:06:45 -0400 Subject: [PATCH] check admin address for mercury access (#9909) --- .../plugins/ocr2keeper/evm21/feed_lookup.go | 12 +++-- .../ocr2keeper/evm21/feed_lookup_test.go | 23 +++++--- .../ocr2keeper/evm21/mocks/registry.go | 53 ++++++++++--------- .../ocr2/plugins/ocr2keeper/evm21/registry.go | 15 +++--- 4 files changed, 62 insertions(+), 41 deletions(-) diff --git a/core/services/ocr2/plugins/ocr2keeper/evm21/feed_lookup.go b/core/services/ocr2/plugins/ocr2keeper/evm21/feed_lookup.go index 30cb008b566..49ea387c058 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evm21/feed_lookup.go +++ b/core/services/ocr2/plugins/ocr2keeper/evm21/feed_lookup.go @@ -182,12 +182,18 @@ func (r *EvmRegistry) doLookup(ctx context.Context, wg *sync.WaitGroup, lookup * // allowedToUseMercury retrieves upkeep's administrative offchain config and decode a mercuryEnabled bool to indicate if // this upkeep is allowed to use Mercury service. func (r *EvmRegistry) allowedToUseMercury(opts *bind.CallOpts, upkeepId *big.Int) (bool, error) { - allowed, ok := r.mercury.allowListCache.Get(upkeepId.String()) + var au activeUpkeep + var ok bool + if au, ok = r.active[upkeepId.String()]; !ok { + return false, fmt.Errorf("upkeep %s is not active", upkeepId) + } + + allowed, ok := r.mercury.allowListCache.Get(au.Admin.Hex()) if ok { return allowed.(bool), nil } - cfg, err := r.registry.GetUpkeepPrivilegeConfig(opts, upkeepId) + cfg, err := r.registry.GetAdminPrivilegeConfig(opts, au.Admin) if err != nil { return false, fmt.Errorf("failed to get upkeep privilege config for upkeep ID %s: %v", upkeepId, err) } @@ -197,7 +203,7 @@ func (r *EvmRegistry) allowedToUseMercury(opts *bind.CallOpts, upkeepId *big.Int if err != nil { return false, fmt.Errorf("failed to unmarshal privilege config for upkeep ID %s: %v", upkeepId, err) } - r.mercury.allowListCache.Set(upkeepId.String(), a.MercuryEnabled, cache.DefaultExpiration) + r.mercury.allowListCache.Set(au.Admin.Hex(), a.MercuryEnabled, cache.DefaultExpiration) return a.MercuryEnabled, nil } diff --git a/core/services/ocr2/plugins/ocr2keeper/evm21/feed_lookup_test.go b/core/services/ocr2/plugins/ocr2keeper/evm21/feed_lookup_test.go index 22fafa5d9fe..61671aa3896 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evm21/feed_lookup_test.go +++ b/core/services/ocr2/plugins/ocr2keeper/evm21/feed_lookup_test.go @@ -85,6 +85,7 @@ func setupEVMRegistry(t *testing.T) *EvmRegistry { func TestEvmRegistry_FeedLookup(t *testing.T) { upkeepId, ok := new(big.Int).SetString("71022726777042968814359024671382968091267501884371696415772139504780367423725", 10) assert.True(t, ok, t.Name()) + admin := common.HexToAddress("0x6cA639822c6C241Fa9A7A6b5032F6F7F1C513CAD") tests := []struct { name string input []ocr2keepers.CheckResult @@ -235,13 +236,17 @@ func TestEvmRegistry_FeedLookup(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := setupEVMRegistry(t) + r.active[upkeepId.String()] = activeUpkeep{ + ID: upkeepId, + Admin: admin, + } if !tt.cachedAdminCfg && !tt.hasError { mockRegistry := mocks.NewRegistry(t) cfg := AdminOffchainConfig{MercuryEnabled: tt.hasPermission} b, err := json.Marshal(cfg) assert.Nil(t, err) - mockRegistry.On("GetUpkeepPrivilegeConfig", mock.Anything, upkeepId).Return(b, nil) + mockRegistry.On("GetAdminPrivilegeConfig", mock.Anything, admin).Return(b, nil) r.registry = mockRegistry } @@ -323,7 +328,9 @@ func TestEvmRegistry_DecodeFeedLookup(t *testing.T) { } func TestEvmRegistry_AllowedToUseMercury(t *testing.T) { - upkeepId := big.NewInt(123456789) + upkeepId, ok := new(big.Int).SetString("71022726777042968814359024671382968091267501884371696415772139504780367423725", 10) + assert.True(t, ok, t.Name()) + admin := common.HexToAddress("0x6cA639822c6C241Fa9A7A6b5032F6F7F1C513CAD") tests := []struct { name string cached bool @@ -353,27 +360,31 @@ func TestEvmRegistry_AllowedToUseMercury(t *testing.T) { { name: "failure - cannot unmarshal privilege config", cached: false, - errorMessage: "failed to unmarshal privilege config for upkeep ID 123456789: invalid character '\\x00' looking for beginning of value", + errorMessage: "failed to unmarshal privilege config for upkeep ID 71022726777042968814359024671382968091267501884371696415772139504780367423725: invalid character '\\x00' looking for beginning of value", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := setupEVMRegistry(t) + r.active[upkeepId.String()] = activeUpkeep{ + ID: upkeepId, + Admin: admin, + } if tt.errorMessage != "" { mockRegistry := mocks.NewRegistry(t) - mockRegistry.On("GetUpkeepPrivilegeConfig", mock.Anything, upkeepId).Return([]byte{0, 1}, nil) + mockRegistry.On("GetAdminPrivilegeConfig", mock.Anything, admin).Return([]byte{0, 1}, nil) r.registry = mockRegistry } else { if tt.cached { - r.mercury.allowListCache.Set(upkeepId.String(), tt.allowed, cache.DefaultExpiration) + r.mercury.allowListCache.Set(admin.Hex(), tt.allowed, cache.DefaultExpiration) } else { mockRegistry := mocks.NewRegistry(t) cfg := AdminOffchainConfig{MercuryEnabled: tt.allowed} b, err := json.Marshal(cfg) assert.Nil(t, err) - mockRegistry.On("GetUpkeepPrivilegeConfig", mock.Anything, upkeepId).Return(b, nil) + mockRegistry.On("GetAdminPrivilegeConfig", mock.Anything, admin).Return(b, nil) r.registry = mockRegistry } } diff --git a/core/services/ocr2/plugins/ocr2keeper/evm21/mocks/registry.go b/core/services/ocr2/plugins/ocr2keeper/evm21/mocks/registry.go index 1827d4cb57f..7e6f9fb5827 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evm21/mocks/registry.go +++ b/core/services/ocr2/plugins/ocr2keeper/evm21/mocks/registry.go @@ -6,6 +6,7 @@ import ( big "math/big" bind "github.com/ethereum/go-ethereum/accounts/abi/bind" + common "github.com/ethereum/go-ethereum/common" generated "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/generated" @@ -73,6 +74,32 @@ func (_m *Registry) GetActiveUpkeepIDs(opts *bind.CallOpts, startIndex *big.Int, return r0, r1 } +// GetAdminPrivilegeConfig provides a mock function with given fields: opts, admin +func (_m *Registry) GetAdminPrivilegeConfig(opts *bind.CallOpts, admin common.Address) ([]byte, error) { + ret := _m.Called(opts, admin) + + var r0 []byte + var r1 error + if rf, ok := ret.Get(0).(func(*bind.CallOpts, common.Address) ([]byte, error)); ok { + return rf(opts, admin) + } + if rf, ok := ret.Get(0).(func(*bind.CallOpts, common.Address) []byte); ok { + r0 = rf(opts, admin) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]byte) + } + } + + if rf, ok := ret.Get(1).(func(*bind.CallOpts, common.Address) error); ok { + r1 = rf(opts, admin) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetState provides a mock function with given fields: opts func (_m *Registry) GetState(opts *bind.CallOpts) (i_keeper_registry_master_wrapper_2_1.GetState, error) { ret := _m.Called(opts) @@ -121,32 +148,6 @@ func (_m *Registry) GetUpkeep(opts *bind.CallOpts, id *big.Int) (i_keeper_regist return r0, r1 } -// GetUpkeepPrivilegeConfig provides a mock function with given fields: opts, upkeepId -func (_m *Registry) GetUpkeepPrivilegeConfig(opts *bind.CallOpts, upkeepId *big.Int) ([]byte, error) { - ret := _m.Called(opts, upkeepId) - - var r0 []byte - var r1 error - if rf, ok := ret.Get(0).(func(*bind.CallOpts, *big.Int) ([]byte, error)); ok { - return rf(opts, upkeepId) - } - if rf, ok := ret.Get(0).(func(*bind.CallOpts, *big.Int) []byte); ok { - r0 = rf(opts, upkeepId) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]byte) - } - } - - if rf, ok := ret.Get(1).(func(*bind.CallOpts, *big.Int) error); ok { - r1 = rf(opts, upkeepId) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // GetUpkeepTriggerConfig provides a mock function with given fields: opts, upkeepId func (_m *Registry) GetUpkeepTriggerConfig(opts *bind.CallOpts, upkeepId *big.Int) ([]byte, error) { ret := _m.Called(opts, upkeepId) diff --git a/core/services/ocr2/plugins/ocr2keeper/evm21/registry.go b/core/services/ocr2/plugins/ocr2keeper/evm21/registry.go index 0ebffbf6af2..c5d18a4f75f 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evm21/registry.go +++ b/core/services/ocr2/plugins/ocr2keeper/evm21/registry.go @@ -63,7 +63,7 @@ type Registry interface { GetUpkeep(opts *bind.CallOpts, id *big.Int) (UpkeepInfo, error) GetState(opts *bind.CallOpts) (iregistry21.GetState, error) GetActiveUpkeepIDs(opts *bind.CallOpts, startIndex *big.Int, maxCount *big.Int) ([]*big.Int, error) - GetUpkeepPrivilegeConfig(opts *bind.CallOpts, upkeepId *big.Int) ([]byte, error) + GetAdminPrivilegeConfig(opts *bind.CallOpts, admin common.Address) ([]byte, error) GetUpkeepTriggerConfig(opts *bind.CallOpts, upkeepId *big.Int) ([]byte, error) CheckCallback(opts *bind.TransactOpts, id *big.Int, values [][]byte, extraData []byte) (*coreTypes.Transaction, error) ParseLog(log coreTypes.Log) (generated.AbigenLog, error) @@ -159,11 +159,13 @@ type activeUpkeep struct { ID *big.Int PerformGasLimit uint32 CheckData []byte + Admin common.Address } type MercuryConfig struct { - cred *models.MercuryCredentials - abi abi.ABI + cred *models.MercuryCredentials + abi abi.ABI + // allowListCache stores the admin address' privilege. in 2.1, this only includes a JSON bytes for allowed to use mercury allowListCache *cache.Cache } @@ -500,7 +502,7 @@ func (r *EvmRegistry) processUpkeepStateLog(l logpoller.Log) error { r.lggr.Warnf("failed to update trigger config for upkeep ID %s: %s", l.Id.String(), err) } case *iregistry21.IKeeperRegistryMasterUpkeepRegistered: - trigger := getUpkeepType(ocr2keepers.UpkeepIdentifier(l.Id.Bytes())) + trigger := getUpkeepType(l.Id.Bytes()) r.lggr.Debugf("KeeperRegistryUpkeepRegistered log detected for upkeep ID %s (trigger=%d) in transaction %s", l.Id.String(), trigger, hash) r.addToActive(l.Id, false) if err := r.updateTriggerConfig(l.Id, nil); err != nil { @@ -530,7 +532,7 @@ func (r *EvmRegistry) removeFromActive(id *big.Int) { delete(r.active, id.String()) r.mu.Unlock() - trigger := getUpkeepType(ocr2keepers.UpkeepIdentifier(id.Bytes())) + trigger := getUpkeepType(id.Bytes()) switch trigger { case logTrigger: if err := r.logEventProvider.UnregisterFilter(id); err != nil { @@ -860,6 +862,7 @@ func (r *EvmRegistry) getUpkeepConfigs(ctx context.Context, ids []*big.Int) ([]a ID: ids[i], PerformGasLimit: info.ExecuteGas, CheckData: info.CheckData, + Admin: info.Admin, } } } @@ -869,7 +872,7 @@ func (r *EvmRegistry) getUpkeepConfigs(ctx context.Context, ids []*big.Int) ([]a func (r *EvmRegistry) updateTriggerConfig(id *big.Int, cfg []byte) error { uid := id.String() - switch getUpkeepType(ocr2keepers.UpkeepIdentifier(id.Bytes())) { + switch getUpkeepType(id.Bytes()) { case logTrigger: if len(cfg) == 0 { fetched, err := r.fetchTriggerConfig(id)