Skip to content

Commit

Permalink
(refactor): Filter out destination chain bridgeable tokens that are n…
Browse files Browse the repository at this point in the history
…ot configured on pricegetter
  • Loading branch information
justinkaseman committed Apr 5, 2024
1 parent b1d022f commit 7c03179
Show file tree
Hide file tree
Showing 10 changed files with 162 additions and 23 deletions.
5 changes: 5 additions & 0 deletions .changeset/weak-months-turn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"ccip": minor
---

Filter out destination chain bridgeable tokens that are not configured on pricegetter
4 changes: 2 additions & 2 deletions core/services/ocr2/plugins/ccip/ccipcommit/ocr2.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func (r *CommitReportingPlugin) observePriceUpdates(
return nil, nil, nil
}

sortedChainTokens, err := ccipcommon.GetSortedChainTokens(ctx, r.offRampReaders, r.destPriceRegistryReader)
sortedChainTokens, err := ccipcommon.GetSortedChainTokens(ctx, r.offRampReaders, r.destPriceRegistryReader, r.priceGetter)
if err != nil {
return nil, nil, fmt.Errorf("get destination tokens: %w", err)
}
Expand Down Expand Up @@ -324,7 +324,7 @@ func (r *CommitReportingPlugin) Report(ctx context.Context, epochAndRound types.

parsableObservations := ccip.GetParsableObservations[ccip.CommitObservation](lggr, observations)

sortedChainTokens, err := ccipcommon.GetSortedChainTokens(ctx, r.offRampReaders, r.destPriceRegistryReader)
sortedChainTokens, err := ccipcommon.GetSortedChainTokens(ctx, r.offRampReaders, r.destPriceRegistryReader, r.priceGetter)
if err != nil {
return false, nil, fmt.Errorf("get destination tokens: %w", err)
}
Expand Down
17 changes: 17 additions & 0 deletions core/services/ocr2/plugins/ccip/ccipcommit/ocr2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ func TestCommitReportingPlugin_Observation(t *testing.T) {
sourceChainCursed bool
commitStoreSeqNum uint64
tokenPrices map[cciptypes.Address]*big.Int
priceGetterConfTokens []cciptypes.Address
sendReqs []cciptypes.EVM2EVMMessageWithTxMeta
tokenDecimals map[cciptypes.Address]uint8
fee *big.Int
Expand All @@ -98,6 +99,10 @@ func TestCommitReportingPlugin_Observation(t *testing.T) {
bridgedTokens[1]: bridgedTokenPrices[bridgedTokens[1]],
sourceNativeTokenAddr: big.NewInt(2e18),
},
priceGetterConfTokens: []cciptypes.Address{
bridgedTokens[0],
bridgedTokens[1],
},
sendReqs: []cciptypes.EVM2EVMMessageWithTxMeta{
{EVM2EVMMessage: cciptypes.EVM2EVMMessage{SequenceNumber: 54}},
{EVM2EVMMessage: cciptypes.EVM2EVMMessage{SequenceNumber: 55}},
Expand All @@ -121,6 +126,10 @@ func TestCommitReportingPlugin_Observation(t *testing.T) {
bridgedTokens[1]: bridgedTokenPrices[bridgedTokens[1]],
sourceNativeTokenAddr: big.NewInt(2e18),
},
priceGetterConfTokens: []cciptypes.Address{
bridgedTokens[0],
bridgedTokens[1],
},
sendReqs: []cciptypes.EVM2EVMMessageWithTxMeta{
{EVM2EVMMessage: cciptypes.EVM2EVMMessage{SequenceNumber: 54}},
{EVM2EVMMessage: cciptypes.EVM2EVMMessage{SequenceNumber: 55}},
Expand All @@ -145,6 +154,11 @@ func TestCommitReportingPlugin_Observation(t *testing.T) {
bridgedTokens[1]: bridgedTokenPrices[bridgedTokens[1]],
sourceNativeTokenAddr: big.NewInt(2e18),
},
priceGetterConfTokens: []cciptypes.Address{
sourceNativeTokenAddr,
bridgedTokens[0],
bridgedTokens[1],
},
sendReqs: []cciptypes.EVM2EVMMessageWithTxMeta{
{EVM2EVMMessage: cciptypes.EVM2EVMMessage{SequenceNumber: 54}},
{EVM2EVMMessage: cciptypes.EVM2EVMMessage{SequenceNumber: 55}},
Expand Down Expand Up @@ -210,6 +224,9 @@ func TestCommitReportingPlugin_Observation(t *testing.T) {
if !tc.priceReportingDisabled && len(tc.tokenPrices) > 0 {
queryTokens := ccipcommon.FlattenUniqueSlice([]cciptypes.Address{sourceNativeTokenAddr}, destTokens)
priceGet.On("TokenPricesUSD", mock.Anything, queryTokens).Return(tc.tokenPrices, nil)
for _, confToken := range tc.priceGetterConfTokens {
priceGet.On("IsTokenConfigured", mock.Anything, confToken).Return(true, nil)
}
}

gasPriceEstimator := prices.NewMockGasPriceEstimatorCommit(t)
Expand Down
58 changes: 39 additions & 19 deletions core/services/ocr2/plugins/ccip/internal/ccipcommon/shortcuts.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,51 @@ type BackfillArgs struct {
SourceStartBlock, DestStartBlock uint64
}

func GetSortedChainTokens(ctx context.Context, offRamps []ccipdata.OffRampReader, priceRegistry cciptypes.PriceRegistryReader) (chainTokens []cciptypes.Address, err error) {
return getSortedChainTokensWithBatchLimit(ctx, offRamps, priceRegistry, offRampBatchSizeLimit)
}

// GetChainTokens returns union of all tokens supported on the destination chain, including fee tokens from the provided price registry
// and the bridgeable tokens from all the offRamps living on the chain.
func getSortedChainTokensWithBatchLimit(ctx context.Context, offRamps []ccipdata.OffRampReader, priceRegistry cciptypes.PriceRegistryReader, batchSize int) (chainTokens []cciptypes.Address, err error) {
// Bridgeable tokens are only included if they are configured on the pricegetter
func GetSortedChainTokens(ctx context.Context, offRamps []ccipdata.OffRampReader, priceRegistry cciptypes.PriceRegistryReader, priceGetter cciptypes.PriceGetter) (chainTokens []cciptypes.Address, err error) {
destFeeTokens, destBridgeableTokens, err := getTokensWithBatchLimit(ctx, offRamps, priceRegistry, offRampBatchSizeLimit)
if err != nil {
return nil, err
}

// Only include destination bridgeable tokens if they are configured on the price getter
var dbtWithPriceGetter []cciptypes.Address
for _, dbt := range destBridgeableTokens {
isConfigured, _ := priceGetter.IsTokenConfigured(ctx, dbt)

Check failure on line 48 in core/services/ocr2/plugins/ccip/internal/ccipcommon/shortcuts.go

View workflow job for this annotation

GitHub Actions / Core Tests (go_core_fuzz)

priceGetter.IsTokenConfigured undefined (type "github.com/smartcontractkit/chainlink-common/pkg/types/ccip".PriceGetter has no field or method IsTokenConfigured)

Check failure on line 48 in core/services/ocr2/plugins/ccip/internal/ccipcommon/shortcuts.go

View workflow job for this annotation

GitHub Actions / Core Tests (go_core_race_tests)

priceGetter.IsTokenConfigured undefined (type "github.com/smartcontractkit/chainlink-common/pkg/types/ccip".PriceGetter has no field or method IsTokenConfigured)

Check failure on line 48 in core/services/ocr2/plugins/ccip/internal/ccipcommon/shortcuts.go

View workflow job for this annotation

GitHub Actions / lint

priceGetter.IsTokenConfigured undefined (type "github.com/smartcontractkit/chainlink-common/pkg/types/ccip".PriceGetter has no field or method IsTokenConfigured)

Check failure on line 48 in core/services/ocr2/plugins/ccip/internal/ccipcommon/shortcuts.go

View workflow job for this annotation

GitHub Actions / Core Tests (go_core_tests)

priceGetter.IsTokenConfigured undefined (type "github.com/smartcontractkit/chainlink-common/pkg/types/ccip".PriceGetter has no field or method IsTokenConfigured)

Check failure on line 48 in core/services/ocr2/plugins/ccip/internal/ccipcommon/shortcuts.go

View workflow job for this annotation

GitHub Actions / Flakey Test Detection

priceGetter.IsTokenConfigured undefined (type "github.com/smartcontractkit/chainlink-common/pkg/types/ccip".PriceGetter has no field or method IsTokenConfigured)
fmt.Println(isConfigured)
fmt.Println(dbt)

if isConfigured {
dbtWithPriceGetter = append(dbtWithPriceGetter, dbt)
}
}
fmt.Println(dbtWithPriceGetter)
return flattenedAndSortedChainTokens(destFeeTokens, dbtWithPriceGetter)
}

func flattenedAndSortedChainTokens(destFeeTokens []cciptypes.Address, destBridgeableTokens []cciptypes.Address) (chainTokens []cciptypes.Address, err error) {
// same token can be returned by multiple offRamps, and fee token can overlap with bridgeable tokens,
// we need to dedup them to arrive at chain token set
chainTokens = FlattenUniqueSlice(destFeeTokens, destBridgeableTokens)

// return the tokens in deterministic order to aid with testing and debugging
sort.Slice(chainTokens, func(i, j int) bool {
return chainTokens[i] < chainTokens[j]
})

return chainTokens, nil
}

func getTokensWithBatchLimit(ctx context.Context, offRamps []ccipdata.OffRampReader, priceRegistry cciptypes.PriceRegistryReader, batchSize int) (destFeeTokens []cciptypes.Address, destBridgeableTokens []cciptypes.Address, err error) {
if batchSize == 0 {
return nil, fmt.Errorf("batch size must be greater than 0")
return nil, nil, fmt.Errorf("batch size must be greater than 0")
}

eg := new(errgroup.Group)
eg.SetLimit(batchSize)

var destFeeTokens []cciptypes.Address
var destBridgeableTokens []cciptypes.Address
mu := &sync.RWMutex{}

eg.Go(func() error {
Expand All @@ -75,19 +104,10 @@ func getSortedChainTokensWithBatchLimit(ctx context.Context, offRamps []ccipdata
}

if err := eg.Wait(); err != nil {
return nil, err
return nil, nil, err
}

// same token can be returned by multiple offRamps, and fee token can overlap with bridgeable tokens,
// we need to dedup them to arrive at chain token set
chainTokens = FlattenUniqueSlice(destFeeTokens, destBridgeableTokens)

// return the tokens in deterministic order to aid with testing and debugging
sort.Slice(chainTokens, func(i, j int) bool {
return chainTokens[i] < chainTokens[j]
})

return chainTokens, nil
return destFeeTokens, destBridgeableTokens, nil
}

// GetDestinationTokens returns the destination chain fee tokens from the provided price registry
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/ethereum/go-ethereum/common"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"

cciptypes "github.com/smartcontractkit/chainlink-common/pkg/types/ccip"

Expand All @@ -17,6 +18,7 @@ import (
"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipcalc"
"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata"
ccipdatamocks "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata/mocks"
"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/pricegetter"
)

func TestGetMessageIDsAsHexString(t *testing.T) {
Expand Down Expand Up @@ -78,12 +80,14 @@ func TestGetChainTokens(t *testing.T) {
name string
feeTokens []cciptypes.Address
destTokens [][]cciptypes.Address
confTokens []cciptypes.Address
expectedChainTokens []cciptypes.Address
}{
{
name: "empty",
feeTokens: []cciptypes.Address{},
destTokens: [][]cciptypes.Address{{}},
confTokens: []cciptypes.Address{},
expectedChainTokens: []cciptypes.Address{},
},
{
Expand All @@ -92,6 +96,7 @@ func TestGetChainTokens(t *testing.T) {
destTokens: [][]cciptypes.Address{
{tokens[1], tokens[2], tokens[3]},
},
confTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3]},
expectedChainTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3]},
},
{
Expand All @@ -102,6 +107,7 @@ func TestGetChainTokens(t *testing.T) {
{tokens[3], tokens[4]},
{tokens[5]},
},
confTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3], tokens[4], tokens[5]},
expectedChainTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3], tokens[4], tokens[5]},
},
{
Expand All @@ -112,8 +118,20 @@ func TestGetChainTokens(t *testing.T) {
{tokens[0], tokens[2], tokens[3], tokens[4], tokens[5]},
{tokens[5]},
},
confTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3], tokens[4], tokens[5]},
expectedChainTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3], tokens[4], tokens[5]},
},
{
name: "unconfigured tokens",
feeTokens: []cciptypes.Address{tokens[0]},
destTokens: [][]cciptypes.Address{
{tokens[0], tokens[1], tokens[2], tokens[3]},
{tokens[0], tokens[2], tokens[3], tokens[4], tokens[5]},
{tokens[5]},
},
confTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3], tokens[4]},
expectedChainTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3], tokens[4]},
},
}

ctx := testutils.Context(t)
Expand All @@ -123,14 +141,19 @@ func TestGetChainTokens(t *testing.T) {
priceRegistry := ccipdatamocks.NewPriceRegistryReader(t)
priceRegistry.On("GetFeeTokens", ctx).Return(tc.feeTokens, nil).Once()

priceGet := pricegetter.NewMockPriceGetter(t)
for _, confToken := range tc.confTokens {
priceGet.On("IsTokenConfigured", mock.Anything, confToken).Return(true, nil)
}

var offRamps []ccipdata.OffRampReader
for _, destTokens := range tc.destTokens {
offRamp := ccipdatamocks.NewOffRampReader(t)
offRamp.On("GetTokens", ctx).Return(cciptypes.OffRampTokens{DestinationTokens: destTokens}, nil).Once()
offRamps = append(offRamps, offRamp)
}

chainTokens, err := GetSortedChainTokens(ctx, offRamps, priceRegistry)
chainTokens, err := GetSortedChainTokens(ctx, offRamps, priceRegistry, priceGet)
assert.NoError(t, err)

sort.Slice(tc.expectedChainTokens, func(i, j int) bool {
Expand Down
17 changes: 17 additions & 0 deletions core/services/ocr2/plugins/ccip/internal/pricegetter/evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,20 @@ func (d *DynamicPriceGetter) TokenPricesUSD(ctx context.Context, tokens []ccipty

return prices, nil
}

// IsTokenConfigured implements the PriceGetter interface.
// It returns if a token has a price resolution rule configured on the PriceGetterConfig
func (d *DynamicPriceGetter) IsTokenConfigured(ctx context.Context, token cciptypes.Address) (bool, error) {
evmAddr, err := ccipcalc.GenericAddrToEvm(token)
if err != nil {
return false, err
}

if _, isAgg := d.cfg.AggregatorPrices[evmAddr]; isAgg {
return isAgg, nil
} else if _, isStatic := d.cfg.StaticPrices[evmAddr]; isStatic {
return isStatic, nil
} else {
return false, nil
}
}
12 changes: 11 additions & 1 deletion core/services/ocr2/plugins/ccip/internal/pricegetter/evm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,20 @@ func TestDynamicPriceGetter(t *testing.T) {
}
require.NoError(t, err)
ctx := testutils.Context(t)
// Check unconfigured token
unconfiguredTk := cciptypes.Address(utils.RandomAddress().String())
isConfigured, err := pg.IsTokenConfigured(ctx, unconfiguredTk)
require.NoError(t, err)
assert.False(t, isConfigured)
// Build list of tokens to query.
tokens := make([]cciptypes.Address, 0, len(test.param.expectedTokenPrices))
for tk := range test.param.expectedTokenPrices {
tokens = append(tokens, cciptypes.Address(tk.String()))
tokenAddr := cciptypes.Address(tk.String())
tokens = append(tokens, tokenAddr)
// Expect that token is configured
isConfigured, err := pg.IsTokenConfigured(ctx, tokenAddr)
require.NoError(t, err)
assert.True(t, isConfigured)
}
prices, err := pg.TokenPricesUSD(ctx, tokens)
if test.param.priceResolutionErrorExpected {
Expand Down
28 changes: 28 additions & 0 deletions core/services/ocr2/plugins/ccip/internal/pricegetter/mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package pricegetter
import (
"context"
"math/big"
"strings"
"time"

mapset "github.com/deckarep/golang-set/v2"
Expand Down Expand Up @@ -91,3 +92,10 @@ func (d *PipelineGetter) TokenPricesUSD(ctx context.Context, tokens []cciptypes.

return tokenPrices, nil
}

// IsTokenConfigured implements the PriceGetter interface.
// It returns if a token has a pipeline job configured on the TokenPricesUSDPipeline
func (d *PipelineGetter) IsTokenConfigured(ctx context.Context, token cciptypes.Address) (bool, error) {
contains := strings.Contains(d.source, string(token))
return contains, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,17 @@ func TestDataSource(t *testing.T) {
`, linkEth.URL, usdcEth.URL, linkTokenAddress, usdcTokenAddress)

priceGetter := newTestPipelineGetter(t, source)

// Link token is configured
hasLinkToken, err := priceGetter.IsTokenConfigured(context.Background(), linkTokenAddress)
require.NoError(t, err)
assert.True(t, hasLinkToken)

// USDC token is configured
hasUSDCToken, err := priceGetter.IsTokenConfigured(context.Background(), usdcTokenAddress)
require.NoError(t, err)
assert.True(t, hasUSDCToken)

// Ask for all prices present in spec.
prices, err := priceGetter.TokenPricesUSD(context.Background(), []cciptypes.Address{
linkTokenAddress,
Expand Down

0 comments on commit 7c03179

Please sign in to comment.