Skip to content

Commit

Permalink
Changes from review; restore GetSortedChainTokens & make pipeline IsT…
Browse files Browse the repository at this point in the history
…okenConfigured more robust
  • Loading branch information
justinkaseman committed Apr 6, 2024
1 parent 7c03179 commit 954bdd4
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 46 deletions.
6 changes: 4 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,9 @@ func (r *CommitReportingPlugin) observePriceUpdates(
return nil, nil, nil
}

sortedChainTokens, err := ccipcommon.GetSortedChainTokens(ctx, r.offRampReaders, r.destPriceRegistryReader, r.priceGetter)
sortedChainTokens, filteredChainTokens, err := ccipcommon.GetFilteredSortedChainTokens(ctx, r.offRampReaders, r.destPriceRegistryReader, r.priceGetter)
lggr.Debugw("Filtered bridgeable tokens with no configured price getter", filteredChainTokens)

if err != nil {
return nil, nil, fmt.Errorf("get destination tokens: %w", err)
}
Expand Down Expand Up @@ -324,7 +326,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, r.priceGetter)
sortedChainTokens, _, err := ccipcommon.GetFilteredSortedChainTokens(ctx, r.offRampReaders, r.destPriceRegistryReader, r.priceGetter)
if err != nil {
return false, nil, fmt.Errorf("get destination tokens: %w", err)
}
Expand Down
57 changes: 41 additions & 16 deletions core/services/ocr2/plugins/ccip/internal/ccipcommon/shortcuts.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,39 +35,63 @@ type BackfillArgs struct {

// 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.
// 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) {
func GetSortedChainTokens(ctx context.Context, offRamps []ccipdata.OffRampReader, priceRegistry cciptypes.PriceRegistryReader) (chainTokens []cciptypes.Address, err error) {
destFeeTokens, destBridgeableTokens, err := getTokensWithBatchLimit(ctx, offRamps, priceRegistry, offRampBatchSizeLimit)
if err != nil {
return nil, err
return nil, fmt.Errorf("get tokens with batch limit: %w", err)
}
// fee token can overlap with bridgeable tokens
// we need to dedup them to arrive at chain token set
return flattenedAndSortedChainTokens(destFeeTokens, destBridgeableTokens), nil
}

// GetFilteredSortedChainTokens 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. Bridgeable tokens are only included if they are configured on the pricegetter
// Fee tokens are not filtered as they must always be priced
func GetFilteredSortedChainTokens(ctx context.Context, offRamps []ccipdata.OffRampReader, priceRegistry cciptypes.PriceRegistryReader, priceGetter cciptypes.PriceGetter) (chainTokens []cciptypes.Address, excludedTokens []cciptypes.Address, err error) {
destFeeTokens, destBridgeableTokens, err := getTokensWithBatchLimit(ctx, offRamps, priceRegistry, offRampBatchSizeLimit)
if err != nil {
return nil, nil, fmt.Errorf("get tokens with batch limit: %w", 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)
fmt.Println(isConfigured)
fmt.Println(dbt)
destTokensWithPrice, destTokensWithoutPrice, err := filterForPricedTokens(ctx, destBridgeableTokens, priceGetter)
if err != nil {
return nil, nil, fmt.Errorf("filter for priced tokens: %w", err)
}

return flattenedAndSortedChainTokens(destFeeTokens, destTokensWithPrice), destTokensWithoutPrice, nil
}

func filterForPricedTokens(ctx context.Context, chainTokens []cciptypes.Address, priceGetter cciptypes.PriceGetter) (tokensWithPrice []cciptypes.Address, tokensWithoutPrice []cciptypes.Address, err error) {
tokensWithPrice = []cciptypes.Address{}
tokensWithoutPrice = []cciptypes.Address{}

for _, token := range chainTokens {
isConfigured, err := priceGetter.IsTokenConfigured(ctx, token)

Check failure on line 70 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 70 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 70 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 70 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 70 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)
if err != nil {
return nil, nil, fmt.Errorf("unable to check if token configured: %w", err)
}
if isConfigured {
dbtWithPriceGetter = append(dbtWithPriceGetter, dbt)
tokensWithPrice = append(tokensWithPrice, token)
} else {
tokensWithoutPrice = append(tokensWithoutPrice, token)
}
}
fmt.Println(dbtWithPriceGetter)
return flattenedAndSortedChainTokens(destFeeTokens, dbtWithPriceGetter)

return tokensWithPrice, tokensWithoutPrice, nil
}

func flattenedAndSortedChainTokens(destFeeTokens []cciptypes.Address, destBridgeableTokens []cciptypes.Address) (chainTokens []cciptypes.Address, err error) {
func flattenedAndSortedChainTokens(slices ...[]cciptypes.Address) (chainTokens []cciptypes.Address) {
// 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)
chainTokens = FlattenUniqueSlice(slices...)

// 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 chainTokens
}

func getTokensWithBatchLimit(ctx context.Context, offRamps []ccipdata.OffRampReader, priceRegistry cciptypes.PriceRegistryReader, batchSize int) (destFeeTokens []cciptypes.Address, destBridgeableTokens []cciptypes.Address, err error) {
Expand Down Expand Up @@ -107,7 +131,8 @@ func getTokensWithBatchLimit(ctx context.Context, offRamps []ccipdata.OffRampRea
return nil, nil, err
}

return destFeeTokens, destBridgeableTokens, nil
// same token can be returned by multiple offRamps
return destFeeTokens, flattenedAndSortedChainTokens(destBridgeableTokens), nil
}

// GetDestinationTokens returns the destination chain fee tokens from the provided price registry
Expand Down
123 changes: 104 additions & 19 deletions core/services/ocr2/plugins/ccip/internal/ccipcommon/shortcuts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,12 @@ 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 @@ -96,7 +94,6 @@ 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 @@ -107,7 +104,6 @@ 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 @@ -118,9 +114,92 @@ 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]},
},
}

ctx := testutils.Context(t)
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {

priceRegistry := ccipdatamocks.NewPriceRegistryReader(t)
priceRegistry.On("GetFeeTokens", ctx).Return(tc.feeTokens, nil).Once()

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)
assert.NoError(t, err)

sort.Slice(tc.expectedChainTokens, func(i, j int) bool {
return tc.expectedChainTokens[i] < tc.expectedChainTokens[j]
})
assert.Equal(t, tc.expectedChainTokens, chainTokens)
})
}
}

func TestGetFilteredChainTokens(t *testing.T) {
const numTokens = 6
var tokens []cciptypes.Address
for i := 0; i < numTokens; i++ {
tokens = append(tokens, ccipcalc.EvmAddrToGeneric(utils.RandomAddress()))
}

testCases := []struct {
name string
feeTokens []cciptypes.Address
destTokens [][]cciptypes.Address
tokenHasPriceGetter [numTokens]bool
expectedChainTokens []cciptypes.Address
expectedFilteredTokens []cciptypes.Address
}{
{
name: "empty",
feeTokens: []cciptypes.Address{},
destTokens: [][]cciptypes.Address{{}},
tokenHasPriceGetter: [numTokens]bool{false, false, false, false, false, false},
expectedChainTokens: []cciptypes.Address{},
expectedFilteredTokens: []cciptypes.Address{},
},
{
name: "single offRamp",
feeTokens: []cciptypes.Address{tokens[0]},
destTokens: [][]cciptypes.Address{
{tokens[1], tokens[2], tokens[3]},
},
tokenHasPriceGetter: [numTokens]bool{true, true, true, true, false, false},
expectedChainTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3]},
expectedFilteredTokens: []cciptypes.Address{},
},
{
name: "multiple offRamps with distinct tokens",
feeTokens: []cciptypes.Address{tokens[0]},
destTokens: [][]cciptypes.Address{
{tokens[1], tokens[2]},
{tokens[3], tokens[4]},
{tokens[5]},
},
tokenHasPriceGetter: [numTokens]bool{true, true, true, true, true, true},
expectedChainTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3], tokens[4], tokens[5]},
expectedFilteredTokens: []cciptypes.Address{},
},
{
name: "overlapping 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]},
},
tokenHasPriceGetter: [numTokens]bool{true, true, true, true, true, true},
expectedChainTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3], tokens[4], tokens[5]},
expectedFilteredTokens: []cciptypes.Address{},
},
{
name: "unconfigured tokens",
feeTokens: []cciptypes.Address{tokens[0]},
Expand All @@ -129,8 +208,9 @@ 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]},
expectedChainTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3], tokens[4]},
tokenHasPriceGetter: [numTokens]bool{true, true, true, true, true, false},
expectedChainTokens: []cciptypes.Address{tokens[0], tokens[1], tokens[2], tokens[3], tokens[4]},
expectedFilteredTokens: []cciptypes.Address{tokens[5]},
},
}

Expand All @@ -142,8 +222,8 @@ func TestGetChainTokens(t *testing.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)
for i, token := range tokens {
priceGet.On("IsTokenConfigured", mock.Anything, token).Return(tc.tokenHasPriceGetter[i], nil).Maybe()
}

var offRamps []ccipdata.OffRampReader
Expand All @@ -153,34 +233,38 @@ func TestGetChainTokens(t *testing.T) {
offRamps = append(offRamps, offRamp)
}

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

sort.Slice(tc.expectedChainTokens, func(i, j int) bool {
return tc.expectedChainTokens[i] < tc.expectedChainTokens[j]
})
assert.Equal(t, tc.expectedChainTokens, chainTokens)
assert.Equal(t, tc.expectedFilteredTokens, filteredTokens)
})
}
}

func TestGetChainTokensWithBatchLimit(t *testing.T) {
numTokens := 100
numFeeTokens := 10
var tokens []cciptypes.Address
for i := 0; i < numTokens; i++ {
tokens = append(tokens, ccipcalc.EvmAddrToGeneric(utils.RandomAddress()))
}

expectedTokens := make([]cciptypes.Address, numTokens)
copy(expectedTokens, tokens)
sort.Slice(expectedTokens, func(i, j int) bool {
return expectedTokens[i] < expectedTokens[j]
expectedFeeTokens := make([]cciptypes.Address, numFeeTokens)
copy(expectedFeeTokens, tokens[0:numFeeTokens])
expectedBridgeableTokens := make([]cciptypes.Address, numTokens)
copy(expectedBridgeableTokens, tokens)
sort.Slice(expectedBridgeableTokens, func(i, j int) bool {
return expectedBridgeableTokens[i] < expectedBridgeableTokens[j]
})

testCases := []struct {
name string
batchSize int
numOffRamps uint
numOffRamps int
expectError bool
}{
{
Expand Down Expand Up @@ -226,23 +310,24 @@ func TestGetChainTokensWithBatchLimit(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {

priceRegistry := ccipdatamocks.NewPriceRegistryReader(t)
priceRegistry.On("GetFeeTokens", ctx).Return(tokens[0:10], nil).Maybe()
priceRegistry.On("GetFeeTokens", ctx).Return(expectedFeeTokens, nil).Maybe()

var offRamps []ccipdata.OffRampReader
for i := 0; i < int(tc.numOffRamps); i++ {
for i := 0; i < tc.numOffRamps; i++ {
offRamp := ccipdatamocks.NewOffRampReader(t)
offRamp.On("GetTokens", ctx).Return(cciptypes.OffRampTokens{DestinationTokens: tokens[i%numTokens:]}, nil).Maybe()
offRamps = append(offRamps, offRamp)
}

chainTokens, err := getSortedChainTokensWithBatchLimit(ctx, offRamps, priceRegistry, tc.batchSize)
destFeeTokens, destBridgeableTokens, err := getTokensWithBatchLimit(ctx, offRamps, priceRegistry, tc.batchSize)
if tc.expectError {
assert.Error(t, err)
return
}

assert.NoError(t, err)
assert.Equal(t, expectedTokens, chainTokens)
assert.Equal(t, expectedFeeTokens, destFeeTokens)
assert.Equal(t, expectedBridgeableTokens, destBridgeableTokens)
})
}
}
Expand Down
16 changes: 8 additions & 8 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 @@ -96,6 +96,6 @@ func (d *PipelineGetter) TokenPricesUSD(ctx context.Context, tokens []cciptypes.
// 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))
contains := strings.Contains(strings.ToLower(d.source), strings.ToLower(string(token)))
return contains, nil
}

0 comments on commit 954bdd4

Please sign in to comment.