feat: extra step to fetch latest added addresses while batching

core/services/gateway/handlers/functions/allowlist/allowlist.go

@@ -253,7 +253,8 @@ 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
+// the iteration order will give priority to new allowed senders, if new addresses are added while iterating over the batches
+// an extra step will be executed to keep this up to date.
 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
@@ -284,33 +285,70 @@ func (a *onchainAllowlist) updateAllowedSendersInBatches(ctx context.Context, to
 			idxEnd = currentAllowedSenderCount - 1
 		}
 
-		allowedSendersBatch, err := tosContract.GetAllowedSendersInRange(&bind.CallOpts{
+		// before continuing we evaluate if the size of the list changed, if that happens we trigger an extra step
+		// getting the latest added addresses from the list
+		updatedAllowedSenderCount, err := tosContract.GetAllowedSendersCount(&bind.CallOpts{
 			Pending:     false,
 			BlockNumber: blockNum,
 			Context:     ctx,
-		}, idxStart, idxEnd)
+		})
 		if err != nil {
-			return errors.Wrap(err, "error calling GetAllowedSendersInRange")
+			return errors.Wrap(err, "unexpected error while fetching the updated functions_allow_list.GetAllowedSendersCount")
 		}
 
-		// add the fetched batch to the currentAllowedSenderList and replace the existing allowlist
-		for _, addr := range allowedSendersBatch {
-			currentAllowedSenderList[addr] = struct{}{}
+		if updatedAllowedSenderCount > currentAllowedSenderCount {
+			lastBatchIdxStart := updatedAllowedSenderCount - (updatedAllowedSenderCount - currentAllowedSenderCount)
+			lastBatchIdxEnd := updatedAllowedSenderCount - 1
+			currentAllowedSenderCount = updatedAllowedSenderCount
+
+			err = a.updateAllowedSendersBatch(ctx, tosContract, blockNum, lastBatchIdxStart, lastBatchIdxEnd, currentAllowedSenderList)
+			if err != nil {
+				return err
+			}
 		}
-		a.allowlist.Store(&currentAllowedSenderList)
-		a.lggr.Infow("allowlist updated in batches successfully", "len", len(currentAllowedSenderList))
 
-		// persist each batch to the underalying orm layer
-		err = a.orm.CreateAllowedSenders(ctx, allowedSendersBatch)
+		err = a.updateAllowedSendersBatch(ctx, tosContract, blockNum, idxStart, idxEnd, currentAllowedSenderList)
 		if err != nil {
-			a.lggr.Errorf("failed to update stored allowedSenderList: %w", err)
+			return err
 		}
 	}
 	throttleTicker.Stop()
 
 	return nil
 }
 
+func (a *onchainAllowlist) updateAllowedSendersBatch(
+	ctx context.Context,
+	tosContract functions_allow_list.TermsOfServiceAllowListInterface,
+	blockNum *big.Int,
+	idxStart uint64,
+	idxEnd uint64,
+	currentAllowedSenderList map[common.Address]struct{},
+) error {
+	allowedSendersBatch, err := tosContract.GetAllowedSendersInRange(&bind.CallOpts{
+		Pending:     false,
+		BlockNumber: blockNum,
+		Context:     ctx,
+	}, idxStart, idxEnd)
+	if err != nil {
+		return errors.Wrap(err, "error calling GetAllowedSendersInRange")
+	}
+
+	// add the fetched batch to the currentAllowedSenderList and replace the existing allowlist
+	for _, addr := range allowedSendersBatch {
+		currentAllowedSenderList[addr] = struct{}{}
+	}
+	a.allowlist.Store(&currentAllowedSenderList)
+	a.lggr.Infow("allowlist updated in batches successfully", "len", len(currentAllowedSenderList))
+
+	// persist each batch to the underalying orm layer
+	err = a.orm.CreateAllowedSenders(ctx, allowedSendersBatch)
+	if err != nil {
+		a.lggr.Errorf("failed to update stored allowedSenderList: %w", err)
+	}
+	return nil
+}
+
 // syncBlockedSenders fetches the list of blocked addresses from the contract in batches
 // and removes the addresses from the functions_allowlist table if present
 func (a *onchainAllowlist) syncBlockedSenders(ctx context.Context, tosContract *functions_allow_list.TermsOfServiceAllowList, blockNum *big.Int) error {

core/services/gateway/handlers/functions/allowlist/allowlist_internal_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 
 	"github.com/smartcontractkit/chainlink-common/pkg/services"
@@ -17,59 +18,130 @@ import (
 )
 
 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)
+	t.Run("OK-simple_update_in_batches", func(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", context.Background(), allowlist[40:50]).Times(1).Return(nil)
+		secondCall := orm.On("CreateAllowedSenders", context.Background(), allowlist[30:40]).Times(1).Return(nil).NotBefore(firstCall)
+		thirdCall := orm.On("CreateAllowedSenders", context.Background(), allowlist[20:30]).Times(1).Return(nil).NotBefore(secondCall)
+		forthCall := orm.On("CreateAllowedSenders", context.Background(), allowlist[10:20]).Times(1).Return(nil).NotBefore(thirdCall)
+		orm.On("CreateAllowedSenders", context.Background(), 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)
+	})
+
+	t.Run("OK-new_address_added_while_updating_in_batches", func(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 initial mocked allowlist will be
+		allowlistSize := 50
+		// allowlist represents the actual allowlist the tos contract will return
+		allowlist := make([]common.Address, 0)
+		// 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", context.Background(), allowlist[40:50]).Times(1).Run(func(args mock.Arguments) {
+
+			// after the first call we update the tosContract by adding a new address
+			addr := testutils.NewAddress()
+			allowlist = append(allowlist, addr)
+			expectedAllowlist[addr] = struct{}{}
+			*tosContract = *NewTosContractMock(allowlist)
+		}).Return(nil)
+
+		// this is the extra step that will fetch the new address we want to validate
+		extraStepCall := orm.On("CreateAllowedSenders", context.Background(), allowlist[50:51]).Times(1).Return(nil).NotBefore(firstCall)
+
+		secondCall := orm.On("CreateAllowedSenders", context.Background(), allowlist[30:40]).Times(1).Return(nil).NotBefore(extraStepCall)
+		thirdCall := orm.On("CreateAllowedSenders", context.Background(), allowlist[20:30]).Times(1).Return(nil).NotBefore(secondCall)
+		forthCall := orm.On("CreateAllowedSenders", context.Background(), allowlist[10:20]).Times(1).Return(nil).NotBefore(thirdCall)
+		orm.On("CreateAllowedSenders", context.Background(), 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)
+	})
 
-	currentAllowlist := onchainAllowlist.allowlist.Load()
-	require.Equal(t, &expectedAllowlist, currentAllowlist)
 }
 
 type tosContractMock struct {