offchain - batch caller verbose errors and token pools fix (#533)

## Motivation

- 1.0.0 Token pools do not have a `typeAndVersion` method which leaded
to rpc call failures.

- Also in this scenario the rpc does not reply with an error but with an
empty response `""` or `"0x"` which leads to `unpack errors` that make
the debugging process harder.

## Solution

- Detect `1.0.0` pools by proper error handling of the empty response
errors.



This PR introduces wETH transfers via a LockUnlock 1.0.0 token pool in
the integration test. While implementing this, we cleaned up a lot of
code that wasn't needed like manual price updates on the destination
chain. The commit plugin will update the prices. This PR improves the
coverage of our integration test by adding

- multiple token tx
- 1.0.0 token pools
- dest price updates through commitStore
- multiple priced assets
- lockUnlock token pools

---------

Co-authored-by: Rens Rooimans <[email protected]>

core/services/ocr2/plugins/ccip/clo_ccip_integration_test.go

@@ -36,6 +36,7 @@ func Test_CLOSpecApprovalFlow_dynamicPriceGetter(t *testing.T) {
 	dstLinkAddr := ccipTH.Dest.LinkToken.Address()
 	srcNativeAddr, err := ccipTH.Source.Router.GetWrappedNative(nil)
 	require.NoError(t, err)
+	aggDstNativeAddr := ccipTH.Dest.WrappedNative.Address()
 
 	aggSrcNatAddr, _, aggSrcNat, err := mock_v3_aggregator_contract.DeployMockV3AggregatorContract(ccipTH.Source.User, ccipTH.Source.Chain, 18, big.NewInt(2e18))
 	require.NoError(t, err)
@@ -63,6 +64,14 @@ func Test_CLOSpecApprovalFlow_dynamicPriceGetter(t *testing.T) {
 	require.Equal(t, big.NewInt(50), tmp.RoundId)
 	require.Equal(t, big.NewInt(8000000), tmp.Answer)
 
+	// deploy dest wrapped native aggregator
+	aggDstNativeAggrAddr, _, aggDstNativeAggr, err := mock_v3_aggregator_contract.DeployMockV3AggregatorContract(ccipTH.Dest.User, ccipTH.Dest.Chain, 18, big.NewInt(3e18))
+	require.NoError(t, err)
+	ccipTH.Dest.Chain.Commit()
+	_, err = aggDstNativeAggr.UpdateRoundData(ccipTH.Dest.User, big.NewInt(50), big.NewInt(500000), big.NewInt(1000), big.NewInt(1000))
+	require.NoError(t, err)
+	ccipTH.Dest.Chain.Commit()
+
 	priceGetterConfig := config.DynamicPriceGetterConfig{
 		AggregatorPrices: map[common.Address]config.AggregatorPriceConfig{
 			srcLinkAddr: {
@@ -77,6 +86,10 @@ func Test_CLOSpecApprovalFlow_dynamicPriceGetter(t *testing.T) {
 				ChainID:                   ccipTH.Dest.ChainID,
 				AggregatorContractAddress: aggDstLnkAddr,
 			},
+			aggDstNativeAddr: {
+				ChainID:                   ccipTH.Dest.ChainID,
+				AggregatorContractAddress: aggDstNativeAggrAddr,
+			},
 		},
 		StaticPrices: map[common.Address]config.StaticPriceConfig{},
 	}

core/services/ocr2/plugins/ccip/integration_test.go

@@ -58,7 +58,6 @@ func TestIntegration_CCIP(t *testing.T) {
 				tokenPricesUSDPipeline = testPricePipeline
 			} else {
 				// Set up a test price getter.
-
 				// Set up the aggregators here to avoid modifying ccipTH.
 				aggSrcNatAddr, _, aggSrcNat, err := mock_v3_aggregator_contract.DeployMockV3AggregatorContract(ccipTH.Source.User, ccipTH.Source.Chain, 18, big.NewInt(2e18))
 				require.NoError(t, err)
@@ -94,6 +93,10 @@ func TestIntegration_CCIP(t *testing.T) {
 							ChainID:                   ccipTH.Dest.ChainID,
 							AggregatorContractAddress: aggDstLnkAddr,
 						},
+						ccipTH.Dest.WrappedNative.Address(): {
+							ChainID:                   ccipTH.Dest.ChainID,
+							AggregatorContractAddress: aggDstLnkAddr,
+						},
 					},
 					StaticPrices: map[common.Address]config.StaticPriceConfig{},
 				}
@@ -114,19 +117,25 @@ func TestIntegration_CCIP(t *testing.T) {
 				require.NoError(t, err2)
 
 				sourceBalances, err2 := testhelpers.GetBalances(t, []testhelpers.BalanceReq{
-					{Name: testhelpers.SourcePool, Addr: ccipTH.Source.Pool.Address(), Getter: ccipTH.GetSourceLinkBalance},
+					{Name: testhelpers.SourcePool, Addr: ccipTH.Source.LinkTokenPool.Address(), Getter: ccipTH.GetSourceLinkBalance},
 					{Name: testhelpers.OnRamp, Addr: ccipTH.Source.OnRamp.Address(), Getter: ccipTH.GetSourceLinkBalance},
 					{Name: testhelpers.SourceRouter, Addr: ccipTH.Source.Router.Address(), Getter: ccipTH.GetSourceLinkBalance},
-					{Name: testhelpers.SourcePrices, Addr: ccipTH.Source.PriceRegistry.Address(), Getter: ccipTH.GetSourceLinkBalance},
+					{Name: testhelpers.SourcePriceRegistry, Addr: ccipTH.Source.PriceRegistry.Address(), Getter: ccipTH.GetSourceLinkBalance},
 				})
 				require.NoError(t, err2)
 				destBalances, err2 := testhelpers.GetBalances(t, []testhelpers.BalanceReq{
 					{Name: testhelpers.Receiver, Addr: ccipTH.Dest.Receivers[0].Receiver.Address(), Getter: ccipTH.GetDestLinkBalance},
-					{Name: testhelpers.DestPool, Addr: ccipTH.Dest.Pool.Address(), Getter: ccipTH.GetDestLinkBalance},
+					{Name: testhelpers.DestPool, Addr: ccipTH.Dest.LinkTokenPool.Address(), Getter: ccipTH.GetDestLinkBalance},
 					{Name: testhelpers.OffRamp, Addr: ccipTH.Dest.OffRamp.Address(), Getter: ccipTH.GetDestLinkBalance},
 				})
 				require.NoError(t, err2)
 
+				ccipTH.Source.User.Value = tokenAmount
+				_, err2 = ccipTH.Source.WrappedNative.Deposit(ccipTH.Source.User)
+				require.NoError(t, err2)
+				ccipTH.Source.Chain.Commit()
+				ccipTH.Source.User.Value = nil
+
 				msg := router.ClientEVM2AnyMessage{
 					Receiver: testhelpers.MustEncodeAddress(t, ccipTH.Dest.Receivers[0].Receiver.Address()),
 					Data:     []byte("hello"),
@@ -135,6 +144,10 @@ func TestIntegration_CCIP(t *testing.T) {
 							Token:  ccipTH.Source.LinkToken.Address(),
 							Amount: tokenAmount,
 						},
+						{
+							Token:  ccipTH.Source.WrappedNative.Address(),
+							Amount: tokenAmount,
+						},
 					},
 					FeeToken:  ccipTH.Source.LinkToken.Address(),
 					ExtraArgs: extraArgs,
@@ -147,6 +160,10 @@ func TestIntegration_CCIP(t *testing.T) {
 				_, err2 = ccipTH.Source.LinkToken.Approve(ccipTH.Source.User, ccipTH.Source.Router.Address(), new(big.Int).Add(fee, tokenAmount))
 				require.NoError(t, err2)
 				ccipTH.Source.Chain.Commit()
+				_, err2 = ccipTH.Source.WrappedNative.Approve(ccipTH.Source.User, ccipTH.Source.Router.Address(), tokenAmount)
+				require.NoError(t, err2)
+				ccipTH.Source.Chain.Commit()
+
 				ccipTH.SendRequest(t, msg)
 				// Should eventually see this executed.
 				ccipTH.AllNodesHaveReqSeqNum(t, currentSeqNum)
@@ -160,25 +177,24 @@ func TestIntegration_CCIP(t *testing.T) {
 				// 1) The total pool input == total pool output
 				// 2) Pool flow equals tokens sent
 				// 3) Sent tokens arrive at the receiver
-
 				ccipTH.AssertBalances(t, []testhelpers.BalanceAssertion{
 					{
 						Name:     testhelpers.SourcePool,
-						Address:  ccipTH.Source.Pool.Address(),
+						Address:  ccipTH.Source.LinkTokenPool.Address(),
 						Expected: testhelpers.MustAddBigInt(sourceBalances[testhelpers.SourcePool], tokenAmount.String()).String(),
 						Getter:   ccipTH.GetSourceLinkBalance,
 					},
 					{
-						Name:     testhelpers.SourcePrices,
+						Name:     testhelpers.SourcePriceRegistry,
 						Address:  ccipTH.Source.PriceRegistry.Address(),
-						Expected: sourceBalances[testhelpers.SourcePrices].String(),
+						Expected: sourceBalances[testhelpers.SourcePriceRegistry].String(),
 						Getter:   ccipTH.GetSourceLinkBalance,
 					},
 					{
 						// Fees end up in the onramp.
 						Name:     testhelpers.OnRamp,
 						Address:  ccipTH.Source.OnRamp.Address(),
-						Expected: testhelpers.MustAddBigInt(sourceBalances[testhelpers.SourcePrices], fee.String()).String(),
+						Expected: testhelpers.MustAddBigInt(sourceBalances[testhelpers.SourcePriceRegistry], fee.String()).String(),
 						Getter:   ccipTH.GetSourceLinkBalance,
 					},
 					{
@@ -187,8 +203,6 @@ func TestIntegration_CCIP(t *testing.T) {
 						Expected: sourceBalances[testhelpers.SourceRouter].String(),
 						Getter:   ccipTH.GetSourceLinkBalance,
 					},
-				})
-				ccipTH.AssertBalances(t, []testhelpers.BalanceAssertion{
 					{
 						Name:     testhelpers.Receiver,
 						Address:  ccipTH.Dest.Receivers[0].Receiver.Address(),
@@ -197,7 +211,7 @@ func TestIntegration_CCIP(t *testing.T) {
 					},
 					{
 						Name:     testhelpers.DestPool,
-						Address:  ccipTH.Dest.Pool.Address(),
+						Address:  ccipTH.Dest.LinkTokenPool.Address(),
 						Expected: testhelpers.MustSubBigInt(destBalances[testhelpers.DestPool], tokenAmount.String()).String(),
 						Getter:   ccipTH.GetDestLinkBalance,
 					},

core/services/ocr2/plugins/ccip/internal/ccipdata/batchreader/token_pool_batch_reader.go

@@ -2,7 +2,9 @@ package batchreader
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"strings"
 	"sync"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -88,7 +90,18 @@ func (br *EVMTokenPoolBatchedReader) GetInboundTokenPoolRateLimits(ctx context.C
 		}
 	}
 
-	return batchCallLatestBlockNumber[cciptypes.TokenBucketRateLimit](ctx, br.evmBatchCaller, evmCalls)
+	results, err := br.evmBatchCaller.BatchCall(ctx, 0, evmCalls)
+	if err != nil {
+		return nil, fmt.Errorf("batch call limit: %w", err)
+	}
+
+	resultsParsed, err := rpclib.ParseOutputs[cciptypes.TokenBucketRateLimit](results, func(d rpclib.DataAndErr) (cciptypes.TokenBucketRateLimit, error) {
+		return rpclib.ParseOutput[cciptypes.TokenBucketRateLimit](d, 0)
+	})
+	if err != nil {
+		return nil, fmt.Errorf("parse outputs: %w", err)
+	}
+	return resultsParsed, nil
 }
 
 // loadTokenPoolReaders loads the token pools into the factory's cache
@@ -149,17 +162,23 @@ func getBatchedTypeAndVersion(ctx context.Context, evmBatchCaller rpclib.EvmBatc
 		))
 	}
 
-	return batchCallLatestBlockNumber[string](ctx, evmBatchCaller, evmCalls)
-}
-
-func batchCallLatestBlockNumber[T any](ctx context.Context, evmBatchCaller rpclib.EvmBatchCaller, evmCalls []rpclib.EvmCall) ([]T, error) {
 	results, err := evmBatchCaller.BatchCall(ctx, 0, evmCalls)
 	if err != nil {
 		return nil, fmt.Errorf("batch call limit: %w", err)
 	}
 
-	result, err := rpclib.ParseOutputs[T](results, func(d rpclib.DataAndErr) (T, error) {
-		return rpclib.ParseOutput[T](d, 0)
+	result, err := rpclib.ParseOutputs[string](results, func(d rpclib.DataAndErr) (string, error) {
+		tAndV, err1 := rpclib.ParseOutput[string](d, 0)
+		if err1 != nil {
+			// typeAndVersion method do not exist for 1.0 pools. We are going to get an ErrEmptyOutput in that case.
+			// Some chains, like the simulated chains, will simply revert with "execution reverted"
+			if errors.Is(err1, rpclib.ErrEmptyOutput) || strings.Contains(err1.Error(), "execution reverted") {
+				return "LegacyPool " + ccipdata.V1_0_0, nil
+			}
+			return "", err1
+		}
+
+		return tAndV, nil
 	})
 	if err != nil {
 		return nil, fmt.Errorf("parse outputs: %w", err)

core/services/ocr2/plugins/ccip/internal/ccipdata/batchreader/token_pool_batch_reader_test.go

@@ -2,6 +2,7 @@ package batchreader
 
 import (
 	"context"
+	"fmt"
 	"math/big"
 	"testing"
 
@@ -45,10 +46,17 @@ func TestTokenPoolFactory(t *testing.T) {
 
 		var batchCallResult []rpclib.DataAndErr
 		for _, poolType := range poolTypes {
-			batchCallResult = append(batchCallResult, rpclib.DataAndErr{
-				Outputs: []any{poolType + " " + versionStr},
-				Err:     nil,
-			})
+			if versionStr == ccipdata.V1_0_0 {
+				// simulating the behaviour for 1.0.0 pools where typeAndVersion method does not exist
+				batchCallResult = append(batchCallResult, rpclib.DataAndErr{
+					Err: fmt.Errorf("unpack result: %w", rpclib.ErrEmptyOutput),
+				})
+			} else {
+				batchCallResult = append(batchCallResult, rpclib.DataAndErr{
+					Outputs: []any{poolType + " " + versionStr},
+					Err:     nil,
+				})
+			}
 		}
 
 		batchCallerMock.On("BatchCall", ctx, uint64(0), mock.Anything).Return(batchCallResult, nil).Once()

core/services/ocr2/plugins/ccip/internal/rpclib/evm.go

@@ -16,6 +16,8 @@ import (
 	"github.com/smartcontractkit/chainlink/v2/core/logger"
 )
 
+var ErrEmptyOutput = errors.New("rpc call output is empty (make sure that the contract method exists and rpc is healthy)")
+
 //go:generate mockery --quiet --name EvmBatchCaller --output ./rpclibmocks --outpkg rpclibmocks --filename evm_mock.go --case=underscore
 type EvmBatchCaller interface {
 	// BatchCall executes all the provided EvmCall and returns the results in the same order
@@ -125,17 +127,26 @@ func (c *defaultEvmBatchCaller) batchCall(ctx context.Context, blockNumber uint6
 		}
 
 		if packedOutputs[i] == "" {
-			return nil, fmt.Errorf("%s: RPC did not properly set any output and also did not set an error", call)
+			// Some RPCs instead of returning "0x" are returning an empty string.
+			// We are overriding this behaviour for consistent handling of this scenario.
+			packedOutputs[i] = "0x"
 		}
+
 		b, err := hexutil.Decode(packedOutputs[i])
 		if err != nil {
 			return nil, fmt.Errorf("decode result %s: packedOutputs %s: %w", call, packedOutputs[i], err)
 		}
 
 		unpackedOutputs, err := call.abi.Unpack(call.methodName, b)
 		if err != nil {
-			return nil, fmt.Errorf("unpack result %s: %w", call, err)
+			if len(b) == 0 {
+				results[i].Err = fmt.Errorf("unpack result %s: %s: %w", call, err.Error(), ErrEmptyOutput)
+			} else {
+				results[i].Err = fmt.Errorf("unpack result %s: %w", call, err)
+			}
+			continue
 		}
+
 		results[i].Outputs = unpackedOutputs
 	}