Fix price reader to return available prices and not fail completely i…

…f some prices are missing [CCIP-4791] (#429)

commit/plugin_e2e_test.go

@@ -291,8 +291,10 @@ func TestPlugin_E2E_AllNodesAgree_TokenPrices(t *testing.T) {
 				m.EXPECT().
 					// tokens need to be ordered, plugin checks all tokens from commit offchain config
 					GetFeedPricesUSD(params.ctx, []ccipocr3.UnknownEncodedAddress{arbAddr, ethAddr}).
-					Return([]*big.Int{arbPrice, ethPrice}, nil).
-					Maybe()
+					Return(map[ccipocr3.UnknownEncodedAddress]*big.Int{
+						arbAddr: arbPrice,
+						ethAddr: ethPrice,
+					}, nil).Maybe()
 
 				m.EXPECT().
 					GetFeeQuoterTokenUpdates(params.ctx, mock.Anything, mock.Anything).
@@ -331,7 +333,10 @@ func TestPlugin_E2E_AllNodesAgree_TokenPrices(t *testing.T) {
 				m.EXPECT().
 					// tokens need to be ordered, plugin checks all tokens from commit offchain config
 					GetFeedPricesUSD(params.ctx, []ccipocr3.UnknownEncodedAddress{arbAddr, ethAddr}).
-					Return([]*big.Int{arbPrice, ethPrice}, nil).
+					Return(map[ccipocr3.UnknownEncodedAddress]*big.Int{
+						arbAddr: arbPrice,
+						ethAddr: ethPrice,
+					}, nil).
 					Maybe()
 
 				// Arb is fresh, will not be updated
@@ -357,8 +362,10 @@ func TestPlugin_E2E_AllNodesAgree_TokenPrices(t *testing.T) {
 				m.EXPECT().
 					// tokens need to be ordered, plugin checks all tokens from commit offchain config
 					GetFeedPricesUSD(params.ctx, []ccipocr3.UnknownEncodedAddress{arbAddr, ethAddr}).
-					Return([]*big.Int{arbPrice, ethPrice}, nil).
-					Maybe()
+					Return(map[ccipocr3.UnknownEncodedAddress]*big.Int{
+						arbAddr: arbPrice,
+						ethAddr: ethPrice,
+					}, nil).Maybe()
 
 				m.EXPECT().
 					GetFeeQuoterTokenUpdates(params.ctx, mock.Anything, mock.Anything).
@@ -713,8 +720,7 @@ func preparePriceReaderMock(ctx context.Context, priceReader *readerpkg_mock.Moc
 
 	priceReader.EXPECT().
 		GetFeedPricesUSD(ctx, mock.Anything).
-		Return([]*big.Int{}, nil).
-		Maybe()
+		Return(map[ccipocr3.UnknownEncodedAddress]*big.Int{}, nil).Maybe()
 }
 
 type nodeSetup struct {

commit/tokenprice/observation.go

@@ -80,15 +80,11 @@ func (p *processor) ObserveFeedTokenPrices(ctx context.Context) []cciptypes.Toke
 		return []cciptypes.TokenPrice{}
 	}
 
-	// If we couldn't fetch all prices log and return only the ones we could fetch
-	if len(tokenPrices) != len(tokensToQuery) {
-		p.lggr.Errorw("token prices length mismatch", "got", tokenPrices, "want", tokensToQuery)
-		return []cciptypes.TokenPrice{}
-	}
-
 	tokenPricesUSD := make([]cciptypes.TokenPrice, 0, len(tokenPrices))
-	for i, token := range tokensToQuery {
-		tokenPricesUSD = append(tokenPricesUSD, cciptypes.NewTokenPrice(token, tokenPrices[i]))
+	for _, token := range tokensToQuery {
+		if tokenPrices[token] != nil {
+			tokenPricesUSD = append(tokenPricesUSD, cciptypes.NewTokenPrice(token, tokenPrices[token]))
+		}
 	}
 
 	return tokenPricesUSD

commit/tokenprice/observation_test.go

@@ -54,7 +54,9 @@ func Test_Observation(t *testing.T) {
 
 				tokenPriceReader := readerpkg_mock.NewMockPriceReader(t)
 				tokenPriceReader.EXPECT().GetFeedPricesUSD(mock.Anything, []cciptypes.UnknownEncodedAddress{tokenA, tokenB}).
-					Return([]*big.Int{bi100, bi200}, nil)
+					Return(map[cciptypes.UnknownEncodedAddress]*big.Int{
+						tokenA: bi100,
+						tokenB: bi200}, nil)
 
 				tokenPriceReader.EXPECT().GetFeeQuoterTokenUpdates(mock.Anything, mock.Anything, mock.Anything).Return(
 					map[cciptypes.UnknownEncodedAddress]plugintypes.TimestampedBig{

pkg/reader/price_reader.go

@@ -23,7 +23,8 @@ type PriceReader interface {
 	//	1 ETH = 2,000 USD per full token, each full token is 1e18 units -> 2000 * 1e18 * 1e18 / 1e18 = 2_000e18
 	//	1 LINK = 5.00 USD per full token, each full token is 1e18 units -> 5 * 1e18 * 1e18 / 1e18 = 5e18
 	// The order of the returned prices corresponds to the order of the provided tokens.
-	GetFeedPricesUSD(ctx context.Context, tokens []ccipocr3.UnknownEncodedAddress) ([]*big.Int, error)
+	GetFeedPricesUSD(ctx context.Context,
+		tokens []ccipocr3.UnknownEncodedAddress) (map[ccipocr3.UnknownEncodedAddress]*big.Int, error)
 
 	// GetFeeQuoterTokenUpdates returns the latest token prices from the FeeQuoter on the specified chain
 	GetFeeQuoterTokenUpdates(
@@ -70,7 +71,7 @@ type LatestRoundData struct {
 }
 
 // ContractTokenMap maps contracts to their token indices
-type ContractTokenMap map[commontypes.BoundContract][]int
+type ContractTokenMap map[commontypes.BoundContract][]ccipocr3.UnknownEncodedAddress
 
 // Number of batch operations performed (getLatestRoundData and getDecimals)
 const priceReaderOperationCount = 2
@@ -150,8 +151,8 @@ func (pr *priceReader) GetFeeQuoterTokenUpdates(
 func (pr *priceReader) GetFeedPricesUSD(
 	ctx context.Context,
 	tokens []ccipocr3.UnknownEncodedAddress,
-) ([]*big.Int, error) {
-	prices := make([]*big.Int, len(tokens))
+) (map[ccipocr3.UnknownEncodedAddress]*big.Int, error) {
+	prices := make(map[ccipocr3.UnknownEncodedAddress]*big.Int, len(tokens))
 	if pr.feedChainReader() == nil {
 		pr.lggr.Debug("node does not support feed chain")
 		return prices, nil
@@ -170,40 +171,42 @@ func (pr *priceReader) GetFeedPricesUSD(
 	}
 
 	// Process results by contract
-	for boundContract, tokenIndices := range contractTokenMap {
+	for boundContract, tokens := range contractTokenMap {
 		contractResults, ok := results[boundContract]
 		if !ok || len(contractResults) != priceReaderOperationCount {
-			return nil, fmt.Errorf("invalid results for contract %s", boundContract.Address)
+			pr.lggr.Errorf("invalid results for contract %s", boundContract.Address)
+			continue
 		}
 
 		// Get price data
 		latestRoundData, err := pr.getPriceData(contractResults[0], boundContract)
 		if err != nil {
-			return nil, err
+			pr.lggr.Errorw("calling getPriceData", err)
+			continue
 		}
 
 		// Get decimals
 		decimals, err := pr.getDecimals(contractResults[1], boundContract)
 		if err != nil {
-			return nil, err
+			pr.lggr.Errorw("calling getPriceData", err)
+			continue
 		}
 
 		// Normalize price for this contract
 		normalizedContractPrice := pr.normalizePrice(latestRoundData.Answer, *decimals)
 
 		// Apply the normalized price to all tokens using this contract
-		for _, tokenIdx := range tokenIndices {
-			token := tokens[tokenIdx]
+		for _, token := range tokens {
 			tokenInfo := pr.tokenInfo[token]
-			prices[tokenIdx] = calculateUsdPer1e18TokenAmount(normalizedContractPrice, tokenInfo.Decimals)
+			price := calculateUsdPer1e18TokenAmount(normalizedContractPrice, tokenInfo.Decimals)
+			if price == nil {
+				pr.lggr.Errorw("failed to calculate price", "token", token)
+				continue
+			}
+			prices[token] = price
 		}
 	}
 
-	// Verify no nil prices
-	if err := pr.validatePrices(prices); err != nil {
-		return nil, err
-	}
-
 	return prices, nil
 }
 
@@ -250,7 +253,7 @@ func (pr *priceReader) prepareBatchRequest(
 	batchRequest := make(commontypes.BatchGetLatestValuesRequest)
 	contractTokenMap := make(ContractTokenMap)
 
-	for i, token := range tokens {
+	for _, token := range tokens {
 		tokenInfo, ok := pr.tokenInfo[token]
 		if !ok {
 			return nil, nil, fmt.Errorf("get tokenInfo for %s: missing token info", token)
@@ -277,7 +280,7 @@ func (pr *priceReader) prepareBatchRequest(
 		}
 
 		// Track which tokens use this contract
-		contractTokenMap[boundContract] = append(contractTokenMap[boundContract], i)
+		contractTokenMap[boundContract] = append(contractTokenMap[boundContract], token)
 	}
 
 	return batchRequest, contractTokenMap, nil
@@ -294,15 +297,6 @@ func (pr *priceReader) normalizePrice(price *big.Int, decimals uint8) *big.Int {
 	return answer
 }
 
-func (pr *priceReader) validatePrices(prices []*big.Int) error {
-	for _, price := range prices {
-		if price == nil {
-			return fmt.Errorf("failed to get all token prices successfully, some prices are nil")
-		}
-	}
-	return nil
-}
-
 func (pr *priceReader) feedChainReader() contractreader.ContractReaderFacade {
 	return pr.chainReaders[pr.feedChain]
 }

pkg/reader/price_reader_test.go

@@ -6,6 +6,8 @@ import (
 	"math/big"
 	"testing"
 
+	"github.com/smartcontractkit/chainlink-common/pkg/logger"
+
 	"github.com/stretchr/testify/require"
 
 	"github.com/stretchr/testify/assert"
@@ -59,7 +61,7 @@ func TestOnchainTokenPricesReader_GetTokenPricesUSD(t *testing.T) {
 		inputTokens   []cciptypes.UnknownEncodedAddress
 		tokenInfo     map[cciptypes.UnknownEncodedAddress]pluginconfig.TokenInfo
 		mockPrices    map[cciptypes.UnknownEncodedAddress]*big.Int
-		want          []*big.Int
+		want          map[cciptypes.UnknownEncodedAddress]*big.Int
 		errorAccounts []cciptypes.UnknownEncodedAddress
 		wantErr       bool
 	}{
@@ -70,7 +72,7 @@ func TestOnchainTokenPricesReader_GetTokenPricesUSD(t *testing.T) {
 			},
 			inputTokens: []cciptypes.UnknownEncodedAddress{ArbAddr},
 			mockPrices:  map[cciptypes.UnknownEncodedAddress]*big.Int{ArbAddr: ArbPrice},
-			want:        []*big.Int{ArbPrice},
+			want:        map[cciptypes.UnknownEncodedAddress]*big.Int{ArbAddr: ArbPrice},
 		},
 		{
 			name: "On-chain multiple prices",
@@ -80,19 +82,18 @@ func TestOnchainTokenPricesReader_GetTokenPricesUSD(t *testing.T) {
 			},
 			inputTokens: []cciptypes.UnknownEncodedAddress{ArbAddr, EthAddr},
 			mockPrices:  map[cciptypes.UnknownEncodedAddress]*big.Int{ArbAddr: ArbPrice, EthAddr: EthPrice},
-			want:        []*big.Int{ArbPrice, EthPrice},
+			want:        map[cciptypes.UnknownEncodedAddress]*big.Int{ArbAddr: ArbPrice, EthAddr: EthPrice},
 		},
 		{
-			name: "Missing price should error",
+			name: "Missing price doesn't fail, return available prices",
 			tokenInfo: map[cciptypes.UnknownEncodedAddress]pluginconfig.TokenInfo{
 				ArbAddr: ArbInfo,
 				EthAddr: EthInfo,
 			},
 			inputTokens:   []cciptypes.UnknownEncodedAddress{ArbAddr, EthAddr},
 			mockPrices:    map[cciptypes.UnknownEncodedAddress]*big.Int{ArbAddr: ArbPrice},
 			errorAccounts: []cciptypes.UnknownEncodedAddress{EthAddr},
-			want:          nil,
-			wantErr:       true,
+			want:          map[cciptypes.UnknownEncodedAddress]*big.Int{ArbAddr: ArbPrice},
 		},
 		{
 			name: "Empty input tokens list",
@@ -101,7 +102,7 @@ func TestOnchainTokenPricesReader_GetTokenPricesUSD(t *testing.T) {
 			},
 			inputTokens: []cciptypes.UnknownEncodedAddress{},
 			mockPrices:  map[cciptypes.UnknownEncodedAddress]*big.Int{},
-			want:        []*big.Int{},
+			want:        map[cciptypes.UnknownEncodedAddress]*big.Int{},
 		},
 		{
 			name: "Repeated token in input",
@@ -110,7 +111,7 @@ func TestOnchainTokenPricesReader_GetTokenPricesUSD(t *testing.T) {
 			},
 			inputTokens: []cciptypes.UnknownEncodedAddress{ArbAddr, ArbAddr},
 			mockPrices:  map[cciptypes.UnknownEncodedAddress]*big.Int{ArbAddr: ArbPrice},
-			want:        []*big.Int{ArbPrice, ArbPrice},
+			want:        map[cciptypes.UnknownEncodedAddress]*big.Int{ArbAddr: ArbPrice},
 		},
 		{
 			name: "Zero price should succeed",
@@ -119,7 +120,7 @@ func TestOnchainTokenPricesReader_GetTokenPricesUSD(t *testing.T) {
 			},
 			inputTokens: []cciptypes.UnknownEncodedAddress{ArbAddr},
 			mockPrices:  map[cciptypes.UnknownEncodedAddress]*big.Int{ArbAddr: big.NewInt(0)},
-			want:        []*big.Int{big.NewInt(0)},
+			want:        map[cciptypes.UnknownEncodedAddress]*big.Int{ArbAddr: big.NewInt(0)},
 		},
 		{
 			name: "Multiple error accounts",
@@ -131,15 +132,15 @@ func TestOnchainTokenPricesReader_GetTokenPricesUSD(t *testing.T) {
 			inputTokens:   []cciptypes.UnknownEncodedAddress{ArbAddr, EthAddr, BtcAddr},
 			mockPrices:    map[cciptypes.UnknownEncodedAddress]*big.Int{ArbAddr: ArbPrice},
 			errorAccounts: []cciptypes.UnknownEncodedAddress{EthAddr, BtcAddr},
-			want:          nil,
-			wantErr:       true,
+			want:          map[cciptypes.UnknownEncodedAddress]*big.Int{ArbAddr: ArbPrice},
 		},
 	}
 
 	for _, tc := range testCases {
 		contractReader := createMockReader(t, tc.mockPrices, tc.errorAccounts, tc.tokenInfo)
 		feedChain := cciptypes.ChainSelector(1)
 		tokenPricesReader := priceReader{
+			lggr: logger.Test(t),
 			chainReaders: map[cciptypes.ChainSelector]contractreader.ContractReaderFacade{
 				feedChain: contractReader,
 			},