Skip to content

Commit

Permalink
Remove HistoryDepth to FinalityDepth validation
Browse files Browse the repository at this point in the history
  • Loading branch information
HelloKashif committed Jun 11, 2024
1 parent bb40d51 commit 468c6ed
Show file tree
Hide file tree
Showing 4 changed files with 7,147 additions and 59 deletions.
5 changes: 5 additions & 0 deletions .changeset/healthy-plants-guess.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

#removed history depth to finality depth validation
101 changes: 65 additions & 36 deletions core/chains/evm/config/toml/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func (cs EVMConfigs) totalChains() int {
}
return total
}

func (cs EVMConfigs) Chains(ids ...string) (r []commontypes.ChainStatus, total int, err error) {
total = cs.totalChains()
for _, ch := range cs {
Expand Down Expand Up @@ -298,11 +299,15 @@ func (c *EVMConfig) ValidateConfig() (err error) {
is := c.ChainType.ChainType()
if is != must {
if must == "" {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "ChainType", Value: c.ChainType.ChainType(),
Msg: "must not be set with this chain id"})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "ChainType", Value: c.ChainType.ChainType(),
Msg: "must not be set with this chain id",
})
} else {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "ChainType", Value: c.ChainType.ChainType(),
Msg: fmt.Sprintf("only %q can be used with this chain id", must)})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "ChainType", Value: c.ChainType.ChainType(),
Msg: fmt.Sprintf("only %q can be used with this chain id", must),
})
}
}
}
Expand All @@ -319,8 +324,10 @@ func (c *EVMConfig) ValidateConfig() (err error) {
break
}
if !hasPrimary {
err = multierr.Append(err, commonconfig.ErrMissing{Name: "Nodes",
Msg: "must have at least one primary node with WSURL"})
err = multierr.Append(err, commonconfig.ErrMissing{
Name: "Nodes",
Msg: "must have at least one primary node with WSURL",
})
}
}

Expand Down Expand Up @@ -372,25 +379,29 @@ type Chain struct {

func (c *Chain) ValidateConfig() (err error) {
if !c.ChainType.ChainType().IsValid() {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "ChainType", Value: c.ChainType.ChainType(),
Msg: chaintype.ErrInvalidChainType.Error()})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "ChainType", Value: c.ChainType.ChainType(),
Msg: chaintype.ErrInvalidChainType.Error(),
})
}

if c.GasEstimator.BumpTxDepth != nil && *c.GasEstimator.BumpTxDepth > *c.Transactions.MaxInFlight {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "GasEstimator.BumpTxDepth", Value: *c.GasEstimator.BumpTxDepth,
Msg: "must be less than or equal to Transactions.MaxInFlight"})
}
if *c.HeadTracker.HistoryDepth < *c.FinalityDepth {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "HeadTracker.HistoryDepth", Value: *c.HeadTracker.HistoryDepth,
Msg: "must be equal to or greater than FinalityDepth"})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "GasEstimator.BumpTxDepth", Value: *c.GasEstimator.BumpTxDepth,
Msg: "must be less than or equal to Transactions.MaxInFlight",
})
}
if *c.FinalityDepth < 1 {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "FinalityDepth", Value: *c.FinalityDepth,
Msg: "must be greater than or equal to 1"})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "FinalityDepth", Value: *c.FinalityDepth,
Msg: "must be greater than or equal to 1",
})
}
if *c.MinIncomingConfirmations < 1 {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "MinIncomingConfirmations", Value: *c.MinIncomingConfirmations,
Msg: "must be greater than or equal to 1"})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "MinIncomingConfirmations", Value: *c.MinIncomingConfirmations,
Msg: "must be greater than or equal to 1",
})
}

// AutoPurge configs depend on ChainType so handling validation on per chain basis
Expand Down Expand Up @@ -560,37 +571,53 @@ type GasEstimator struct {

func (e *GasEstimator) ValidateConfig() (err error) {
if uint64(*e.BumpPercent) < legacypool.DefaultConfig.PriceBump {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "BumpPercent", Value: *e.BumpPercent,
Msg: fmt.Sprintf("may not be less than Geth's default of %d", legacypool.DefaultConfig.PriceBump)})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "BumpPercent", Value: *e.BumpPercent,
Msg: fmt.Sprintf("may not be less than Geth's default of %d", legacypool.DefaultConfig.PriceBump),
})
}
if e.TipCapDefault.Cmp(e.TipCapMin) < 0 {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "TipCapDefault", Value: e.TipCapDefault,
Msg: "must be greater than or equal to TipCapMinimum"})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "TipCapDefault", Value: e.TipCapDefault,
Msg: "must be greater than or equal to TipCapMinimum",
})
}
if e.FeeCapDefault.Cmp(e.TipCapDefault) < 0 {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "FeeCapDefault", Value: e.TipCapDefault,
Msg: "must be greater than or equal to TipCapDefault"})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "FeeCapDefault", Value: e.TipCapDefault,
Msg: "must be greater than or equal to TipCapDefault",
})
}
if *e.Mode == "FixedPrice" && *e.BumpThreshold == 0 && *e.EIP1559DynamicFees && e.FeeCapDefault.Cmp(e.PriceMax) != 0 {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "FeeCapDefault", Value: e.FeeCapDefault,
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "FeeCapDefault", Value: e.FeeCapDefault,
Msg: fmt.Sprintf("must be equal to PriceMax (%s) since you are using FixedPrice estimation with gas bumping disabled in "+
"EIP1559 mode - PriceMax will be used as the FeeCap for transactions instead of FeeCapDefault", e.PriceMax)})
"EIP1559 mode - PriceMax will be used as the FeeCap for transactions instead of FeeCapDefault", e.PriceMax),
})
} else if e.FeeCapDefault.Cmp(e.PriceMax) > 0 {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "FeeCapDefault", Value: e.FeeCapDefault,
Msg: fmt.Sprintf("must be less than or equal to PriceMax (%s)", e.PriceMax)})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "FeeCapDefault", Value: e.FeeCapDefault,
Msg: fmt.Sprintf("must be less than or equal to PriceMax (%s)", e.PriceMax),
})
}

if e.PriceMin.Cmp(e.PriceDefault) > 0 {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "PriceMin", Value: e.PriceMin,
Msg: "must be less than or equal to PriceDefault"})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "PriceMin", Value: e.PriceMin,
Msg: "must be less than or equal to PriceDefault",
})
}
if e.PriceMax.Cmp(e.PriceDefault) < 0 {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "PriceMax", Value: e.PriceMin,
Msg: "must be greater than or equal to PriceDefault"})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "PriceMax", Value: e.PriceMin,
Msg: "must be greater than or equal to PriceDefault",
})
}
if *e.Mode == "BlockHistory" && *e.BlockHistory.BlockHistorySize <= 0 {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "BlockHistory.BlockHistorySize", Value: *e.BlockHistory.BlockHistorySize,
Msg: "must be greater than or equal to 1 with BlockHistory Mode"})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "BlockHistory.BlockHistorySize", Value: *e.BlockHistory.BlockHistorySize,
Msg: "must be greater than or equal to 1 with BlockHistory Mode",
})
}

return
Expand Down Expand Up @@ -767,8 +794,10 @@ func (t *HeadTracker) setFrom(f *HeadTracker) {

func (t *HeadTracker) ValidateConfig() (err error) {
if *t.MaxAllowedFinalityDepth < 1 {
err = multierr.Append(err, commonconfig.ErrInvalid{Name: "MaxAllowedFinalityDepth", Value: *t.MaxAllowedFinalityDepth,
Msg: "must be greater than or equal to 1"})
err = multierr.Append(err, commonconfig.ErrInvalid{
Name: "MaxAllowedFinalityDepth", Value: *t.MaxAllowedFinalityDepth,
Msg: "must be greater than or equal to 1",
})
}

return
Expand Down

Large diffs are not rendered by default.

63 changes: 40 additions & 23 deletions core/services/chainlink/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ var (
HTTPURL: mustURL("http://broadcast.mirror"),
SendOnly: ptr(true),
},
}},
},
},
{
ChainID: ubig.NewI(42),
Chain: evmcfg.Chain{
Expand All @@ -127,7 +128,8 @@ var (
Name: ptr("foo"),
WSURL: mustURL("wss://web.socket/test/foo"),
},
}},
},
},
{
ChainID: ubig.NewI(137),
Chain: evmcfg.Chain{
Expand All @@ -140,7 +142,8 @@ var (
Name: ptr("bar"),
WSURL: mustURL("wss://web.socket/test/bar"),
},
}},
},
},
},
Cosmos: []*coscfg.TOMLConfig{
{
Expand All @@ -150,15 +153,17 @@ var (
},
Nodes: []*coscfg.Node{
{Name: ptr("primary"), TendermintURL: commoncfg.MustParseURL("http://columbus.cosmos.com")},
}},
},
},
{
ChainID: ptr("Malaga-420"),
Chain: coscfg.Chain{
BlocksUntilTxTimeout: ptr[int64](20),
},
Nodes: []*coscfg.Node{
{Name: ptr("secondary"), TendermintURL: commoncfg.MustParseURL("http://bombay.cosmos.com")},
}},
},
},
},
Solana: []*solcfg.TOMLConfig{
{
Expand Down Expand Up @@ -294,11 +299,13 @@ func TestConfig_Marshal(t *testing.T) {
SendInterval: commoncfg.MustNewDuration(time.Minute),
SendTimeout: commoncfg.MustNewDuration(5 * time.Second),
UseBatchSend: ptr(true),
Endpoints: []toml.TelemetryIngressEndpoint{{
Network: ptr("EVM"),
ChainID: ptr("1"),
ServerPubKey: ptr("test-pub-key"),
URL: mustURL("prom.test")},
Endpoints: []toml.TelemetryIngressEndpoint{
{
Network: ptr("EVM"),
ChainID: ptr("1"),
ServerPubKey: ptr("test-pub-key"),
URL: mustURL("prom.test"),
},
},
}

Expand Down Expand Up @@ -635,7 +642,8 @@ func TestConfig_Marshal(t *testing.T) {
HTTPURL: mustURL("http://broadcast.mirror"),
SendOnly: ptr(true),
},
}},
},
},
}
full.Solana = []*solcfg.TOMLConfig{
{
Expand Down Expand Up @@ -1282,11 +1290,10 @@ func TestConfig_Validate(t *testing.T) {
- WSURL: missing: required for primary nodes
- HTTPURL: missing: required for all nodes
- 1.HTTPURL: missing: required for all nodes
- 1: 10 errors:
- 1: 9 errors:
- ChainType: invalid value (Foo): must not be set with this chain id
- Nodes: missing: must have at least one node
- ChainType: invalid value (Foo): must be one of arbitrum, celo, gnosis, kroma, metis, optimismBedrock, scroll, wemix, xlayer, zkevm, zksync or omitted
- HeadTracker.HistoryDepth: invalid value (30): must be equal to or greater than FinalityDepth
- GasEstimator.BumpThreshold: invalid value (0): cannot be 0 if auto-purge feature is enabled for Foo
- Transactions.AutoPurge.Threshold: missing: needs to be set if auto-purge feature is enabled for Foo
- Transactions.AutoPurge.MinAttempts: missing: needs to be set if auto-purge feature is enabled for Foo
Expand Down Expand Up @@ -1411,10 +1418,14 @@ func Test_generalConfig_LogConfiguration(t *testing.T) {
wantWarning string
}{
{name: "empty", wantEffective: emptyEffectiveTOML, wantSecrets: emptyEffectiveSecretsTOML},
{name: "full", inputSecrets: secretsFullTOML, inputConfig: fullTOML,
wantConfig: fullTOML, wantEffective: fullTOML, wantSecrets: secretsFullRedactedTOML, wantWarning: deprecated},
{name: "multi-chain", inputSecrets: secretsMultiTOML, inputConfig: multiChainTOML,
wantConfig: multiChainTOML, wantEffective: multiChainEffectiveTOML, wantSecrets: secretsMultiRedactedTOML},
{
name: "full", inputSecrets: secretsFullTOML, inputConfig: fullTOML,
wantConfig: fullTOML, wantEffective: fullTOML, wantSecrets: secretsFullRedactedTOML, wantWarning: deprecated,
},
{
name: "multi-chain", inputSecrets: secretsMultiTOML, inputConfig: multiChainTOML,
wantConfig: multiChainTOML, wantEffective: multiChainEffectiveTOML, wantSecrets: secretsMultiRedactedTOML,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -1510,14 +1521,17 @@ func TestSecrets_Validate(t *testing.T) {
toml string
exp string
}{
{name: "partial",
{
name: "partial",
toml: `
Database.AllowSimplePasswords = true`,
exp: `invalid secrets: 2 errors:
- Database.URL: empty: must be provided and non-empty
- Password.Keystore: empty: must be provided and non-empty`},
- Password.Keystore: empty: must be provided and non-empty`,
},

{name: "invalid-urls",
{
name: "invalid-urls",
toml: `[Database]
URL = "postgresql://user:passlocalhost:5432/asdf"
BackupURL = "foo-bar?password=asdf"
Expand All @@ -1543,14 +1557,17 @@ AllowSimplePasswords = false`,
Must not comprise:
Leading or trailing whitespace (note that a trailing newline in the password file, if present, will be ignored)
- Password.Keystore: empty: must be provided and non-empty`},
- Password.Keystore: empty: must be provided and non-empty`,
},

{name: "invalid-urls-allowed",
{
name: "invalid-urls-allowed",
toml: `[Database]
URL = "postgresql://user:passlocalhost:5432/asdf"
BackupURL = "foo-bar?password=asdf"
AllowSimplePasswords = true`,
exp: `invalid secrets: Password.Keystore: empty: must be provided and non-empty`},
exp: `invalid secrets: Password.Keystore: empty: must be provided and non-empty`,
},
} {
t.Run(tt.name, func(t *testing.T) {
var s Secrets
Expand Down

0 comments on commit 468c6ed

Please sign in to comment.