Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CCIP-4703: Adding check for ChainFeeUpdates field #410

Merged
merged 22 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4addc48
CCIP-4703: Adding check for ChainFeeUpdates field
b-gopalswami Dec 30, 2024
a28b6b7
Merge branch 'main' of github.com:smartcontractkit/chainlink-ccip int…
b-gopalswami Dec 31, 2024
5bcb9b8
Separating observed chains validation
b-gopalswami Dec 31, 2024
5146533
Lint
b-gopalswami Dec 31, 2024
b36980f
go mod tidy
b-gopalswami Dec 31, 2024
7cb45b1
Merge branch 'main' of github.com:smartcontractkit/chainlink-ccip int…
b-gopalswami Jan 6, 2025
22de002
review comments and adding unit test
b-gopalswami Jan 6, 2025
eaf6b01
Lint fix
b-gopalswami Jan 6, 2025
adf719d
goimports with local
b-gopalswami Jan 6, 2025
053c08f
Merge branch 'main' into cl33-07
b-gopalswami Jan 7, 2025
8a43f25
Merge branch 'main' of github.com:smartcontractkit/chainlink-ccip int…
b-gopalswami Jan 8, 2025
941d029
adding additional checks per review
b-gopalswami Jan 8, 2025
0df3535
Merge branch 'cl33-07' of github.com:smartcontractkit/chainlink-ccip …
b-gopalswami Jan 8, 2025
a0ac3ea
Lint fix
b-gopalswami Jan 8, 2025
9cb84c1
Trigger workflow with dummy commit
b-gopalswami Jan 9, 2025
960a2e8
Add filter by intersecting unique chains
b-gopalswami Jan 9, 2025
a3acd67
lint
b-gopalswami Jan 10, 2025
b9a20ab
Merge branch 'main' of github.com:smartcontractkit/chainlink-ccip int…
b-gopalswami Jan 10, 2025
deac5db
Fix chain fee validation.
asoliman92 Jan 13, 2025
cb2ed66
linting
asoliman92 Jan 13, 2025
71c5c6a
Merge branch 'main' into cl33-07
asoliman92 Jan 13, 2025
f1b4bb2
Merge branch 'main' into cl33-07
b-gopalswami Jan 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 55 additions & 13 deletions commit/chainfee/validate_observation.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,32 +21,74 @@ func (p *processor) ValidateObservation(
return fmt.Errorf("failed to validate FChain: %w", err)
}

observerSupportedChains, err := p.chainSupport.SupportedChains(ao.OracleID)
if err != nil {
return fmt.Errorf("failed to get supported chains: %w", err)
if err := p.ValidateObservedChains(ao); err != nil {
return fmt.Errorf("failed to validate observed chains: %w", err)
}

observedChains := append(maps.Keys(obs.FeeComponents), maps.Keys(obs.NativeTokenPrices)...)
if err := validateFeeComponents(ao); err != nil {
return fmt.Errorf("failed to validate fee components: %w", err)
}

for _, chain := range observedChains {
if !observerSupportedChains.Contains(chain) {
return fmt.Errorf("chain %d is not supported by observer", chain)
if err := validateChainFeeUpdates(ao); err != nil {
return fmt.Errorf("failed to validate chain fee updates: %w", err)
}

for _, token := range obs.NativeTokenPrices {
if token.Int == nil || token.Int.Cmp(zero) <= 0 {
return fmt.Errorf("nil or non-positive %s", "execution fee")
}
}

b-gopalswami marked this conversation as resolved.
Show resolved Hide resolved
for _, feeComponent := range obs.FeeComponents {
if feeComponent.ExecutionFee == nil || feeComponent.ExecutionFee.Cmp(zero) <= 0 {
return nil
}

func validateChainFeeUpdates(
ao plugincommon.AttributedObservation[Observation],
) error {
b-gopalswami marked this conversation as resolved.
Show resolved Hide resolved
for _, update := range ao.Observation.ChainFeeUpdates {
if update.ChainFee.ExecutionFeePriceUSD == nil || update.ChainFee.ExecutionFeePriceUSD.Cmp(big.NewInt(0)) <= 0 {
return fmt.Errorf("nil or non-positive %s", "execution fee price")
}

if update.ChainFee.DataAvFeePriceUSD == nil || update.ChainFee.DataAvFeePriceUSD.Cmp(big.NewInt(0)) < 0 {
return fmt.Errorf("nil or negative %s", "data availability fee price")
}
if update.Timestamp.IsZero() {
return fmt.Errorf("zero timestamp")
}
}
return nil
}

func validateFeeComponents(
ao plugincommon.AttributedObservation[Observation],
) error {
b-gopalswami marked this conversation as resolved.
Show resolved Hide resolved
for _, feeComponent := range ao.Observation.FeeComponents {
if feeComponent.ExecutionFee == nil || feeComponent.ExecutionFee.Cmp(big.NewInt(0)) <= 0 {
return fmt.Errorf("nil or non-positive %s", "execution fee")
}

if feeComponent.DataAvailabilityFee == nil || feeComponent.DataAvailabilityFee.Cmp(zero) < 0 {
if feeComponent.DataAvailabilityFee == nil || feeComponent.DataAvailabilityFee.Cmp(big.NewInt(0)) < 0 {
return fmt.Errorf("nil or negative %s", "data availability fee")
}
}
return nil
}

for _, token := range obs.NativeTokenPrices {
if token.Int == nil || token.Int.Cmp(zero) <= 0 {
return fmt.Errorf("nil or non-positive %s", "execution fee")
func (p *processor) ValidateObservedChains(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function shouldn't be public since it'll only get called from the chainfee processor

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this an interface method?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I misread it. Fixed it.

ao plugincommon.AttributedObservation[Observation],
) error {
obs := ao.Observation
observerSupportedChains, err := p.chainSupport.SupportedChains(ao.OracleID)
if err != nil {
return fmt.Errorf("failed to get supported chains: %w", err)
}

b-gopalswami marked this conversation as resolved.
Show resolved Hide resolved
observedChains := append(maps.Keys(obs.FeeComponents), maps.Keys(obs.NativeTokenPrices)...)
observedChains = append(observedChains, maps.Keys(obs.ChainFeeUpdates)...)
for _, chain := range observedChains {
if !observerSupportedChains.Contains(chain) {
return fmt.Errorf("chain %d is not supported by observer", chain)
}
}

Expand Down
178 changes: 178 additions & 0 deletions commit/chainfee/validate_observation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
package chainfee

import (
"math/big"
"testing"
"time"

"github.com/stretchr/testify/require"

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

"github.com/smartcontractkit/chainlink-ccip/internal/plugincommon"
"github.com/smartcontractkit/chainlink-ccip/pkg/types/ccipocr3"
)

func Test_validateFeeComponentsAndChainFeeUpdates(t *testing.T) {
fourHoursAgo := time.Now().Add(-4 * time.Hour).UTC().Truncate(time.Hour)
tests := []struct {
name string
feeComponents map[ccipocr3.ChainSelector]types.ChainFeeComponents
chainFeeUpdates map[ccipocr3.ChainSelector]Update
expectedFeeComponentError string
expectedChainFeeUpdatesError string
}{
{
name: "valid fee components",
feeComponents: map[ccipocr3.ChainSelector]types.ChainFeeComponents{
1: {
ExecutionFee: big.NewInt(10),
DataAvailabilityFee: big.NewInt(20),
},
},
expectedFeeComponentError: "",
},
{
name: "nil execution fee",
feeComponents: map[ccipocr3.ChainSelector]types.ChainFeeComponents{
1: {
ExecutionFee: nil,
DataAvailabilityFee: big.NewInt(20),
},
},
expectedFeeComponentError: "nil or non-positive execution fee",
},
{
name: "non-positive execution fee",
feeComponents: map[ccipocr3.ChainSelector]types.ChainFeeComponents{
1: {
ExecutionFee: big.NewInt(0),
DataAvailabilityFee: big.NewInt(20),
},
},
expectedFeeComponentError: "nil or non-positive execution fee",
},
{
name: "nil data availability fee",
feeComponents: map[ccipocr3.ChainSelector]types.ChainFeeComponents{
1: {
ExecutionFee: big.NewInt(10),
DataAvailabilityFee: nil,
},
},
expectedFeeComponentError: "nil or negative data availability fee",
},
{
name: "negative data availability fee",
feeComponents: map[ccipocr3.ChainSelector]types.ChainFeeComponents{
1: {
ExecutionFee: big.NewInt(10),
DataAvailabilityFee: big.NewInt(-1),
},
},
expectedFeeComponentError: "nil or negative data availability fee",
},
{
name: "valid chain fee updates",
chainFeeUpdates: map[ccipocr3.ChainSelector]Update{
1: {
ChainFee: ComponentsUSDPrices{
ExecutionFeePriceUSD: big.NewInt(10),
DataAvFeePriceUSD: big.NewInt(20),
},
Timestamp: fourHoursAgo,
},
},
expectedChainFeeUpdatesError: "",
},
{
name: "nil execution fee price - chain fee updates",
chainFeeUpdates: map[ccipocr3.ChainSelector]Update{
1: {
ChainFee: ComponentsUSDPrices{
ExecutionFeePriceUSD: nil,
DataAvFeePriceUSD: big.NewInt(20),
},
Timestamp: fourHoursAgo,
},
},
expectedChainFeeUpdatesError: "nil or non-positive execution fee price",
},
{
name: "non-positive execution fee price - chain fee updates",
chainFeeUpdates: map[ccipocr3.ChainSelector]Update{
1: {
ChainFee: ComponentsUSDPrices{
ExecutionFeePriceUSD: big.NewInt(0),
DataAvFeePriceUSD: big.NewInt(20),
},
Timestamp: fourHoursAgo,
},
},
expectedChainFeeUpdatesError: "nil or non-positive execution fee price",
},
{
name: "nil data availability fee price - chain fee updates",
chainFeeUpdates: map[ccipocr3.ChainSelector]Update{
1: {
ChainFee: ComponentsUSDPrices{
ExecutionFeePriceUSD: big.NewInt(10),
DataAvFeePriceUSD: nil,
},
Timestamp: fourHoursAgo,
},
},
expectedChainFeeUpdatesError: "nil or negative data availability fee price",
},
{
name: "negative data availability fee price - chain fee updates",
chainFeeUpdates: map[ccipocr3.ChainSelector]Update{
1: {
ChainFee: ComponentsUSDPrices{
ExecutionFeePriceUSD: big.NewInt(10),
DataAvFeePriceUSD: big.NewInt(-1),
},
Timestamp: fourHoursAgo,
},
},
expectedChainFeeUpdatesError: "nil or negative data availability fee price",
},
{
name: "zero timestamp - chain fee updates",
chainFeeUpdates: map[ccipocr3.ChainSelector]Update{
1: {
ChainFee: ComponentsUSDPrices{
ExecutionFeePriceUSD: big.NewInt(10),
DataAvFeePriceUSD: big.NewInt(20),
},
},
},
expectedChainFeeUpdatesError: "zero timestamp",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ao := plugincommon.AttributedObservation[Observation]{
Observation: Observation{
FeeComponents: tt.feeComponents,
ChainFeeUpdates: tt.chainFeeUpdates,
},
}
err := validateFeeComponents(ao)
if tt.expectedFeeComponentError == "" {
require.NoError(t, err)
} else {
require.Error(t, err)
require.Contains(t, err.Error(), tt.expectedFeeComponentError)
}
err = validateChainFeeUpdates(ao)
if tt.expectedChainFeeUpdatesError == "" {
require.NoError(t, err)
} else {
require.Error(t, err)
require.Contains(t, err.Error(), tt.expectedChainFeeUpdatesError)
}
})
}
}
Loading