From c2cbd78799431aaa5f5a66c6683c621071dbd467 Mon Sep 17 00:00:00 2001 From: Gabriel Paradiso Date: Wed, 3 Apr 2024 17:52:03 +0200 Subject: [PATCH] fix: adjust iteration and add tests on updateAllowedSendersInBatches --- .../handlers/functions/allowlist/allowlist.go | 4 +- .../allowlist/allowlist_internal_test.go | 94 +++++++++++++++++++ 2 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 core/services/gateway/handlers/functions/allowlist/allowlist_internal_test.go diff --git a/core/services/gateway/handlers/functions/allowlist/allowlist.go b/core/services/gateway/handlers/functions/allowlist/allowlist.go index cecd9d6ab4a..1743e0b3f12 100644 --- a/core/services/gateway/handlers/functions/allowlist/allowlist.go +++ b/core/services/gateway/handlers/functions/allowlist/allowlist.go @@ -254,7 +254,7 @@ func (a *onchainAllowlist) updateFromContractV1(ctx context.Context, blockNum *b // updateAllowedSendersInBatches will update the node's inmemory state and the orm layer representing the allowlist. // it will get the current node's in memory allowlist and start fetching and adding from the tos contract in batches. // the iteration order will give priority to new allowed senders -func (a *onchainAllowlist) updateAllowedSendersInBatches(ctx context.Context, tosContract *functions_allow_list.TermsOfServiceAllowList, blockNum *big.Int) error { +func (a *onchainAllowlist) updateAllowedSendersInBatches(ctx context.Context, tosContract functions_allow_list.TermsOfServiceAllowListInterface, blockNum *big.Int) error { // currentAllowedSenderList will be the starting point from which we will be adding the new allowed senders currentAllowedSenderList := make(map[common.Address]struct{}, 0) @@ -277,7 +277,7 @@ func (a *onchainAllowlist) updateAllowedSendersInBatches(ctx context.Context, to <-throttleTicker.C idxStart := uint64(i) - uint64(a.config.OnchainAllowlistBatchSize) - idxEnd := uint64(i) + idxEnd := uint64(i) - 1 if idxEnd >= currentAllowedSenderCount { idxEnd = currentAllowedSenderCount - 1 } diff --git a/core/services/gateway/handlers/functions/allowlist/allowlist_internal_test.go b/core/services/gateway/handlers/functions/allowlist/allowlist_internal_test.go new file mode 100644 index 00000000000..c1531eee245 --- /dev/null +++ b/core/services/gateway/handlers/functions/allowlist/allowlist_internal_test.go @@ -0,0 +1,94 @@ +package allowlist + +import ( + "context" + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/accounts/abi/bind" + "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink-common/pkg/services" + "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/functions/generated/functions_allow_list" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + "github.com/smartcontractkit/chainlink/v2/core/logger" + amocks "github.com/smartcontractkit/chainlink/v2/core/services/gateway/handlers/functions/allowlist/mocks" +) + +func TestUpdateAllowedSendersInBatches(t *testing.T) { + ctx := context.Background() + config := OnchainAllowlistConfig{ + ContractAddress: testutils.NewAddress(), + ContractVersion: 1, + BlockConfirmations: 1, + UpdateFrequencySec: 2, + UpdateTimeoutSec: 1, + StoredAllowlistBatchSize: 2, + OnchainAllowlistBatchSize: 10, + FetchingDelayInRangeSec: 1, + } + + // allowlistSize defines how big the mocked allowlist will be + allowlistSize := 50 + // allowlist represents the actual allowlist the tos contract will return + allowlist := make([]common.Address, 0, allowlistSize) + // expectedAllowlist will be used to compare the actual status with what we actually want + expectedAllowlist := make(map[common.Address]struct{}, 0) + + // we load both the expectedAllowlist and the allowlist the contract will return with some new addresses + for i := 0; i < allowlistSize; i++ { + addr := testutils.NewAddress() + allowlist = append(allowlist, addr) + expectedAllowlist[addr] = struct{}{} + } + + tosContract := NewTosContractMock(allowlist) + + // with the orm mock we can validate the actual order in which the allowlist is fetched giving priority to newest addresses + orm := amocks.NewORM(t) + firstCall := orm.On("CreateAllowedSenders", allowlist[40:50]).Times(1).Return(nil) + secondCall := orm.On("CreateAllowedSenders", allowlist[30:40]).Times(1).Return(nil).NotBefore(firstCall) + thirdCall := orm.On("CreateAllowedSenders", allowlist[20:30]).Times(1).Return(nil).NotBefore(secondCall) + forthCall := orm.On("CreateAllowedSenders", allowlist[10:20]).Times(1).Return(nil).NotBefore(thirdCall) + orm.On("CreateAllowedSenders", allowlist[0:10]).Times(1).Return(nil).NotBefore(forthCall) + + onchainAllowlist := &onchainAllowlist{ + config: config, + orm: orm, + blockConfirmations: big.NewInt(int64(config.BlockConfirmations)), + lggr: logger.TestLogger(t).Named("OnchainAllowlist"), + stopCh: make(services.StopChan), + } + + // we set the onchain allowlist to an empty state before updating it in batches + emptyMap := make(map[common.Address]struct{}) + onchainAllowlist.allowlist.Store(&emptyMap) + + err := onchainAllowlist.updateAllowedSendersInBatches(ctx, tosContract, big.NewInt(0)) + require.NoError(t, err) + + currentAllowlist := onchainAllowlist.allowlist.Load() + require.Equal(t, &expectedAllowlist, currentAllowlist) +} + +type tosContractMock struct { + functions_allow_list.TermsOfServiceAllowListInterface + + onchainAllowlist []common.Address +} + +func NewTosContractMock(onchainAllowlist []common.Address) *tosContractMock { + return &tosContractMock{ + onchainAllowlist: onchainAllowlist, + } +} + +func (t *tosContractMock) GetAllowedSendersCount(opts *bind.CallOpts) (uint64, error) { + return uint64(len(t.onchainAllowlist)), nil +} + +func (t *tosContractMock) GetAllowedSendersInRange(opts *bind.CallOpts, allowedSenderIdxStart uint64, allowedSenderIdxEnd uint64) ([]common.Address, error) { + // we replicate the onchain behaviour of including start and end indexes + return t.onchainAllowlist[allowedSenderIdxStart : allowedSenderIdxEnd+1], nil +}