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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
67 changes: 54 additions & 13 deletions commit/chainfee/validate_observation.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"math/big"

"github.com/pkg/errors"

b-gopalswami marked this conversation as resolved.
Show resolved Hide resolved
"github.com/smartcontractkit/chainlink-ccip/internal/plugincommon"

"golang.org/x/exp/maps"
Expand All @@ -21,32 +23,71 @@ 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 errors.Wrap(err, "failed to validate observed chains")
b-gopalswami marked this conversation as resolved.
Show resolved Hide resolved
}

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

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 errors.Wrap(err, "failed to validate chain fee updates")
}

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Add validation for timpestamps making sure they're not zero or more than current utc timestamp.

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")
}
}
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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to add extra checks here to make sure that all keys in all maps are exactly the same, otherwise we can end up with same lengths but for different chains on different maps.

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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go 1.23.3
require (
github.com/deckarep/golang-set/v2 v2.6.0
github.com/patrickmn/go-cache v2.1.0+incompatible
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.20.0
github.com/prometheus/client_model v0.6.1
github.com/smartcontractkit/chain-selectors v1.0.34
Expand Down Expand Up @@ -38,7 +39,6 @@ require (
github.com/mr-tron/base58 v1.2.0 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/pelletier/go-toml/v2 v2.2.2 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/prometheus/common v0.59.1 // indirect
github.com/prometheus/procfs v0.15.1 // indirect
Expand Down
Loading