diff --git a/opera/marshal_test.go b/opera/marshal_test.go index ab62b0ec..09d31046 100644 --- a/opera/marshal_test.go +++ b/opera/marshal_test.go @@ -1,6 +1,7 @@ package opera import ( + "fmt" "math/big" "testing" @@ -38,6 +39,35 @@ func TestUpdateRules(t *testing.T) { require.Error(err, "should fail on invalid rules") } +func TestUpdateRules_ValidityCheckIsConductedIfCheckIsEnabledInUpdatedRuleSet(t *testing.T) { + for _, enabledBefore := range []bool{true, false} { + for _, enabledAfter := range []bool{true, false} { + for _, validUpdate := range []bool{true, false} { + t.Run(fmt.Sprintf("before=%t,after=%t,valid=%t", enabledBefore, enabledAfter, validUpdate), func(t *testing.T) { + require := require.New(t) + + base := MainNetRules() + base.Upgrades.CheckRuleChanges = enabledBefore + + maxParents := 1 + if validUpdate { + maxParents = 5 + } + + update := fmt.Sprintf(`{"Dag":{"MaxParents":%d}, "Upgrades":{"CheckRuleChanges":%t}}`, maxParents, enabledAfter) + + _, err := UpdateRules(base, []byte(update)) + if enabledAfter && !validUpdate { + require.Error(err) + } else { + require.NoError(err) + } + }) + } + } + } +} + func TestMainNetRulesRLP(t *testing.T) { rules := MainNetRules() require := require.New(t) diff --git a/opera/rules.go b/opera/rules.go index 83951420..f5e7d556 100644 --- a/opera/rules.go +++ b/opera/rules.go @@ -429,6 +429,15 @@ func (r Rules) Copy() Rules { return cp } +// Validate checks the rules for consistency and safety. Rules are considered safe if +// they do not risk stalling the network or preventing future rule updates. +// +// Note: the validation is very liberal to allow a maximum flexibility in the rules. +// The merely check for the most critical configuration errors that may lead to network +// stalls or rule update issues. However, many valid configurations may still result +// in undesirable network behavior. Rule-setters need to be aware of the implications +// of their choices and should always test their rules in a controlled environment. +// This validation is not a substitute for proper testing. func (r Rules) Validate() error { return validate(r) } diff --git a/opera/validate.go b/opera/validate.go index bae81fdf..168b5dca 100644 --- a/opera/validate.go +++ b/opera/validate.go @@ -13,7 +13,7 @@ func validate(rules Rules) error { validateDagRules(rules.Dag), validateEmitterRules(rules.Emitter), validateEpochsRules(rules.Epochs), - validateBlockRules(rules.Blocks), + validateBlocksRules(rules.Blocks), validateEconomyRules(rules.Economy), validateUpgrades(rules.Upgrades), ) @@ -72,14 +72,14 @@ func validateEpochsRules(rules EpochsRules) error { return errors.Join(issues...) } -func validateBlockRules(rules BlocksRules) error { +func validateBlocksRules(rules BlocksRules) error { var issues []error if rules.MaxBlockGas < MinimumMaxBlockGas { - issues = append(issues, errors.New("MaxBlockGas is too low")) + issues = append(issues, errors.New("Blocks.MaxBlockGas is too low")) } if rules.MaxBlockGas > MaximumMaxBlockGas { - issues = append(issues, errors.New("MaxBlockGas is too high")) + issues = append(issues, errors.New("Blocks.MaxBlockGas is too high")) } // The empty-block skip period is not restricted. There are no too low or too high values. @@ -140,7 +140,11 @@ func validateGasRules(rules GasRules) error { issues = append(issues, errors.New("Gas.MaxEventGas is too low")) } - if rules.MaxEventGas-rules.EventGas < upperBoundForRuleChangeGasCosts { + if rules.EventGas > rules.MaxEventGas { + issues = append(issues, errors.New("Gas.EventGas is too high")) + } + + if rules.MaxEventGas < upperBoundForRuleChangeGasCosts+rules.EventGas { issues = append(issues, errors.New("Gas.EventGas is too high")) } diff --git a/opera/validate_test.go b/opera/validate_test.go index fa6ba050..cc78e690 100644 --- a/opera/validate_test.go +++ b/opera/validate_test.go @@ -1,8 +1,12 @@ package opera import ( + "math" + "math/big" "testing" + "time" + "github.com/Fantom-foundation/go-opera/inter" "github.com/stretchr/testify/require" ) @@ -17,3 +21,448 @@ func TestDefaultRulesAreValid(t *testing.T) { }) } } + +func TestValidate_DetectsIssues(t *testing.T) { + issues := map[string]struct { + rules Rules + issue string + }{ + "dag rules issue": { + rules: Rules{Dag: DagRules{MaxParents: 1}}, + issue: "Dag.MaxParents is too low", + }, + "emitter rules issue": { + rules: Rules{Emitter: EmitterRules{Interval: inter.Timestamp(11 * time.Second)}}, + issue: "Emitter.Interval is too high", + }, + "epochs rules issue": { + rules: Rules{Epochs: EpochsRules{MaxEpochDuration: inter.Timestamp(24*time.Hour) + 1}}, + issue: "Epochs.MaxEpochDuration is too high", + }, + "blocks rules issue": { + rules: Rules{Blocks: BlocksRules{MaxBlockGas: 0}}, + issue: "Blocks.MaxBlockGas is too low", + }, + "economy rules issue": { + rules: Rules{Economy: EconomyRules{Gas: GasRules{MaxEventGas: 1}}}, + issue: "Gas.MaxEventGas is too low", + }, + "upgrades issue": { + rules: Rules{Upgrades: Upgrades{Llr: true}}, + issue: "LLR upgrade is not supported", + }, + } + + for name, test := range issues { + t.Run(name, func(t *testing.T) { + err := test.rules.Validate() + require.Error(t, err) + require.Contains(t, err.Error(), test.issue) + }) + } +} + +func TestDagRulesValidation_DetectsIssues(t *testing.T) { + issues := map[string]struct { + rules DagRules + issue string + }{ + "zero parents are not enough": { + rules: DagRules{MaxParents: 0}, + issue: "MaxParents is too low", + }, + "one parent is not enough": { + rules: DagRules{MaxParents: 1}, + issue: "MaxParents is too low", + }, + "zero free parents are not enough": { + rules: DagRules{MaxFreeParents: 0}, + issue: "MaxFreeParents is too low", + }, + "one free parent is not enough": { + rules: DagRules{MaxFreeParents: 1}, + issue: "MaxFreeParents is too low", + }, + "too much extra data": { + rules: DagRules{MaxExtraData: 1<<20 + 1}, + issue: "MaxExtraData is too high", + }, + } + + for name, test := range issues { + t.Run(name, func(t *testing.T) { + err := validateDagRules(test.rules) + require.Error(t, err) + require.Contains(t, err.Error(), test.issue) + }) + } +} + +func TestDagRulesValidation_AcceptsValidRules(t *testing.T) { + rules := []DagRules{ + {MaxParents: 2, MaxFreeParents: 2, MaxExtraData: 0}, + {MaxParents: 2, MaxFreeParents: 2, MaxExtraData: 1 << 20}, + {MaxParents: 10, MaxFreeParents: 10, MaxExtraData: 1 << 20}, + } + + for _, test := range rules { + require.NoError(t, validateDagRules(test)) + } +} + +func TestEmitterRulesValidation_DetectsIssues(t *testing.T) { + issues := map[string]struct { + rules EmitterRules + issue string + }{ + "more than 10 seconds emitter times is too high": { + rules: EmitterRules{Interval: inter.Timestamp(10*time.Second) + 1}, + issue: "Interval is too high", + }, + "hour long emitter times is too high": { + rules: EmitterRules{Interval: inter.Timestamp(10 * time.Hour)}, + issue: "Interval is too high", + }, + "stall threshold must be at least 10 seconds": { + rules: EmitterRules{StallThreshold: inter.Timestamp(10*time.Second) - 1}, + issue: "StallThreshold is too low", + }, + "stall interval must be at least 10 seconds": { + rules: EmitterRules{StallThreshold: inter.Timestamp(10*time.Second) - 1}, + issue: "StallThreshold is too low", + }, + "stall intervals of more than 1 minute are too high": { + rules: EmitterRules{StalledInterval: inter.Timestamp(1*time.Minute) + 1}, + issue: "StalledInterval is too high", + }, + } + + for name, test := range issues { + t.Run(name, func(t *testing.T) { + err := validateEmitterRules(test.rules) + require.Error(t, err) + require.Contains(t, err.Error(), test.issue) + }) + } +} + +func TestEmitterRulesValidation_AcceptsValidRules(t *testing.T) { + rules := []EmitterRules{ + { + Interval: inter.Timestamp(10 * time.Second), + StallThreshold: inter.Timestamp(10 * time.Second), + StalledInterval: inter.Timestamp(10 * time.Second), + }, + { + Interval: inter.Timestamp(10 * time.Millisecond), + StallThreshold: inter.Timestamp(10 * time.Second), + StalledInterval: inter.Timestamp(1 * time.Minute), + }, + { + Interval: inter.Timestamp(0), + StallThreshold: inter.Timestamp(1 * time.Hour), + StalledInterval: inter.Timestamp(30 * time.Second), + }, + } + + for _, test := range rules { + require.NoError(t, validateEmitterRules(test)) + } +} + +func TestEpochsRulesValidation_DetectsIssues(t *testing.T) { + issues := map[string]struct { + rules EpochsRules + issue string + }{ + "more than day long epochs are too high": { + rules: EpochsRules{MaxEpochDuration: inter.Timestamp(24*time.Hour) + 1}, + issue: "MaxEpochDuration is too high", + }, + } + + for name, test := range issues { + t.Run(name, func(t *testing.T) { + err := validateEpochsRules(test.rules) + require.Error(t, err) + require.Contains(t, err.Error(), test.issue) + }) + } +} + +func TestEpochsRulesValidation_AcceptsValidRules(t *testing.T) { + rules := []EpochsRules{ + {MaxEpochDuration: inter.Timestamp(24 * time.Hour)}, + {MaxEpochDuration: inter.Timestamp(1 * time.Hour)}, + {MaxEpochDuration: inter.Timestamp(0)}, + } + + for _, test := range rules { + require.NoError(t, validateEpochsRules(test)) + } +} + +func TestBlocksRulesValidation_DetectsIssues(t *testing.T) { + issues := map[string]struct { + rules BlocksRules + issue string + }{ + "no block gas": { + rules: BlocksRules{MaxBlockGas: 0}, + issue: "MaxBlockGas is too low", + }, + "max block gas less than minimum is too low": { + rules: BlocksRules{MaxBlockGas: MinimumMaxBlockGas - 1}, + issue: "MaxBlockGas is too low", + }, + "max block gas more than maximum is too high": { + rules: BlocksRules{MaxBlockGas: MaximumMaxBlockGas + 1}, + issue: "MaxBlockGas is too high", + }, + "uint64 max block gas": { + rules: BlocksRules{MaxBlockGas: math.MaxUint64}, + issue: "MaxBlockGas is too high", + }, + } + + for name, test := range issues { + t.Run(name, func(t *testing.T) { + err := validateBlocksRules(test.rules) + require.Error(t, err) + require.Contains(t, err.Error(), test.issue) + }) + } +} + +func TestBlocksRulesValidation_AcceptsValidRules(t *testing.T) { + rules := []BlocksRules{ + {MaxBlockGas: MinimumMaxBlockGas}, + {MaxBlockGas: MaximumMaxBlockGas}, + {MaxBlockGas: MaximumMaxBlockGas / 2}, + } + + for _, test := range rules { + require.NoError(t, validateBlocksRules(test)) + } +} + +func TestGasRulesValidation_DetectsIssues(t *testing.T) { + issues := map[string]struct { + rules GasRules + issue string + }{ + "zero max event gas is too low": { + rules: GasRules{MaxEventGas: 0}, + issue: "MaxEventGas is too low", + }, + "zero event gas is too low": { + rules: GasRules{EventGas: 0}, + issue: "EventGas is too low", + }, + "less than rule-update gas is too low": { + rules: GasRules{MaxEventGas: upperBoundForRuleChangeGasCosts - 1}, + issue: "MaxEventGas is too low", + }, + "too high event gas costs": { + rules: GasRules{EventGas: 1}, + issue: "EventGas is too high", + }, + "insufficient capacity for rule update": { + rules: GasRules{MaxEventGas: upperBoundForRuleChangeGasCosts, EventGas: 1}, + issue: "EventGas is too high", + }, + } + + for name, test := range issues { + t.Run(name, func(t *testing.T) { + err := validateGasRules(test.rules) + require.Error(t, err) + require.Contains(t, err.Error(), test.issue) + }) + } +} + +func TestGasRulesValidation_AcceptsValidRules(t *testing.T) { + rules := []GasRules{ + {MaxEventGas: upperBoundForRuleChangeGasCosts, EventGas: 0}, + {MaxEventGas: upperBoundForRuleChangeGasCosts + 10, EventGas: 10}, + {MaxEventGas: upperBoundForRuleChangeGasCosts + 15, EventGas: 10}, + {MaxEventGas: 1000 * upperBoundForRuleChangeGasCosts, EventGas: 10000}, + } + + for _, test := range rules { + require.NoError(t, validateGasRules(test)) + } +} + +func TestEconomyRulesValidation_DetectsIssues(t *testing.T) { + issues := map[string]struct { + rules EconomyRules + issue string + }{ + "min gas price must not be nil": { + rules: EconomyRules{MinGasPrice: nil}, + issue: "MinGasPrice is nil", + }, + "negative min gas price is too low": { + rules: EconomyRules{MinGasPrice: big.NewInt(-1)}, + issue: "MinGasPrice is negative", + }, + "too high min gas price is too high": { + rules: EconomyRules{MinGasPrice: new(big.Int).Add(maxMinimumGasPrice, big.NewInt(1))}, + issue: "MinGasPrice is too high", + }, + "min base fee must not be nil": { + rules: EconomyRules{MinBaseFee: nil}, + issue: "MinBaseFee is nil", + }, + "negative min base fee is too low": { + rules: EconomyRules{MinBaseFee: big.NewInt(-1)}, + issue: "MinBaseFee is negative", + }, + "too high min base fee is too high": { + rules: EconomyRules{MinBaseFee: new(big.Int).Add(maxMinimumGasPrice, big.NewInt(1))}, + issue: "MinBaseFee is too high", + }, + "too low event gas": { + rules: EconomyRules{Gas: GasRules{MaxEventGas: 1}}, + issue: "MaxEventGas is too low", + }, + "too low short-gas allocation per second": { + rules: EconomyRules{ShortGasPower: GasPowerRules{AllocPerSec: 1}}, + issue: "ShortGasPower.AllocPerSec is too low", + }, + "too low long-gas allocation per second": { + rules: EconomyRules{LongGasPower: GasPowerRules{AllocPerSec: 1}}, + issue: "LongGasPower.AllocPerSec is too low", + }, + } + + for name, test := range issues { + t.Run(name, func(t *testing.T) { + err := validateEconomyRules(test.rules) + require.Error(t, err) + require.Contains(t, err.Error(), test.issue) + }) + } +} + +func TestEconomyRulesValidation_AcceptsValidRules(t *testing.T) { + valid := EconomyRules{ + Gas: GasRules{ + MaxEventGas: upperBoundForRuleChangeGasCosts, + }, + MinGasPrice: big.NewInt(0), + MinBaseFee: big.NewInt(0), + ShortGasPower: GasPowerRules{ + AllocPerSec: 10 * upperBoundForRuleChangeGasCosts, + MaxAllocPeriod: inter.Timestamp(time.Second), + StartupAllocPeriod: inter.Timestamp(time.Second), + }, + LongGasPower: GasPowerRules{ + AllocPerSec: 12 * upperBoundForRuleChangeGasCosts, + MaxAllocPeriod: inter.Timestamp(2 * time.Second), + StartupAllocPeriod: inter.Timestamp(3 * time.Second), + }, + } + + require.NoError(t, validateEconomyRules(valid)) +} + +func TestGasPowerRulesValidation_DetectsIssues(t *testing.T) { + issues := map[string]struct { + rules GasPowerRules + issue string + }{ + "zero gas allocation per second is too low": { + rules: GasPowerRules{AllocPerSec: 0}, + issue: "AllocPerSec is too low", + }, + "too low allocation per second": { + rules: GasPowerRules{AllocPerSec: 10*upperBoundForRuleChangeGasCosts - 1}, + issue: "AllocPerSec is too low", + }, + "no allocation period is too low": { + rules: GasPowerRules{MaxAllocPeriod: 0}, + issue: "AllocPeriod is too low", + }, + "less than a second allocation period is too low": { + rules: GasPowerRules{MaxAllocPeriod: inter.Timestamp(time.Second) - 1}, + issue: "AllocPeriod is too low", + }, + "more than a minute allocation period is too high": { + rules: GasPowerRules{MaxAllocPeriod: inter.Timestamp(time.Minute) + 1}, + issue: "AllocPeriod is too high", + }, + "less than a second startup period is too low": { + rules: GasPowerRules{StartupAllocPeriod: inter.Timestamp(time.Second) - 1}, + issue: "StartupAllocPeriod is too low", + }, + } + + for name, test := range issues { + t.Run(name, func(t *testing.T) { + err := validateGasPowerRules("", test.rules) + require.Error(t, err) + require.Contains(t, err.Error(), test.issue) + }) + } +} + +func TestGasPowerRulesValidation_AcceptsValidRules(t *testing.T) { + sec := inter.Timestamp(time.Second) + min := inter.Timestamp(time.Minute) + rules := []GasPowerRules{ + {AllocPerSec: 10 * upperBoundForRuleChangeGasCosts, MaxAllocPeriod: 1 * sec, StartupAllocPeriod: 1 * sec}, + {AllocPerSec: 10 * upperBoundForRuleChangeGasCosts, MaxAllocPeriod: 1 * min, StartupAllocPeriod: 1 * sec}, + {AllocPerSec: 10 * upperBoundForRuleChangeGasCosts, MaxAllocPeriod: 1 * min, StartupAllocPeriod: 1 * min}, + {AllocPerSec: math.MaxUint64, MaxAllocPeriod: 1 * min, StartupAllocPeriod: 1 * min}, + } + + for _, test := range rules { + require.NoError(t, validateGasPowerRules("", test)) + } +} + +func TestUpgradesValidation_DetectsIssues(t *testing.T) { + issues := map[string]struct { + upgrade Upgrades + issue string + }{ + "LLR upgrade is not supported": { + upgrade: Upgrades{Llr: true}, + issue: "LLR upgrade is not supported", + }, + "Sonic upgrade requires London": { + upgrade: Upgrades{Sonic: true}, + issue: "Sonic upgrade requires London", + }, + "London upgrade requires Berlin": { + upgrade: Upgrades{London: true}, + issue: "London upgrade requires Berlin", + }, + "Sonic upgrade is required": { + upgrade: Upgrades{}, + issue: "Sonic upgrade is required", + }, + } + + for name, test := range issues { + t.Run(name, func(t *testing.T) { + err := validateUpgrades(test.upgrade) + require.Error(t, err) + require.Contains(t, err.Error(), test.issue) + }) + } +} + +func TestUpgradesValidation_AcceptsValidRules(t *testing.T) { + upgrades := []Upgrades{ + {Berlin: true, London: true, Sonic: true}, + {Berlin: true, London: true, Sonic: true, CheckRuleChanges: true}, + } + + for _, test := range upgrades { + require.NoError(t, validateUpgrades(test)) + } +}