diff --git a/cli/operator/node.go b/cli/operator/node.go index 07c4ef6e5f..18d21520ac 100644 --- a/cli/operator/node.go +++ b/cli/operator/node.go @@ -105,6 +105,9 @@ func buildNode(ctx context.Context, cfg *config, logger *zap.Logger) (*node, err SSV: ssvNetworkConfig, Beacon: consensusClient.BeaconConfig(), } + if err := networkConfig.Validate(); err != nil { + return nil, fmt.Errorf("invalid network config: %w", err) + } var executionAddrList []string for _, addr := range strings.Split(cfg.ExecutionClient.Addr, ";") { diff --git a/networkconfig/network.go b/networkconfig/network.go index 5dc09b7655..d5fb9ba2ac 100644 --- a/networkconfig/network.go +++ b/networkconfig/network.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "math" + "time" "github.com/attestantio/go-eth2-client/spec/phase0" spectypes "github.com/ssvlabs/ssv-spec/types" @@ -81,6 +82,51 @@ func (n Network) BooleForkAtSlot(slot phase0.Slot) bool { return n.BooleForkAtEpoch(n.EstimatedEpochAtSlot(slot)) } +// Validate checks the assembled network configuration for internal inconsistencies that fork +// activation logic can't catch on its own, returning an error for configurations that would +// panic or misbehave. It stays logger-free by design: the caller decides whether/how to abort. +func (n Network) Validate() error { + // A zero SlotsPerEpoch is a malformed config regardless of fork scheduling: slot/epoch + // conversions divide by it unconditionally (e.g. Beacon.EstimatedEpochAtSlot), so it would + // panic on the first duty long before any fork logic runs. + if n.SlotsPerEpoch == 0 { + return fmt.Errorf("slots per epoch must be positive") + } + + if n.BooleForkScheduled() { + if maxEpoch := n.maxEpochConvertibleToSlot(); n.SSV.Forks.Boole > maxEpoch { + return fmt.Errorf("boole fork epoch %d overflows slot conversion (max supported epoch %d)", n.SSV.Forks.Boole, maxEpoch) + } + + // unmarshalFromConfig defaults NextDomainType to DomainType when the field is absent, so + // equality while the fork is still ahead is exactly the signature of a scheduled fork that + // forgot to set NextDomainType: it would "activate" with zero observable domain change, and + // a restart is guaranteed before activation (see BooleForkScheduled), so refusing to start + // is the last safe moment to catch it. Once the fork is active, equality is legitimate + // steady state — a config written post-fork may set DomainType to the post-fork domain and + // omit NextDomainType. + // The wall-clock read must be guarded: EstimatedCurrentSlot panics while the clock is + // still before GenesisTime, and before genesis a scheduled fork is by definition not + // yet active. + booleForkActive := !time.Now().Before(n.GenesisTime) && n.BooleFork() + if n.NextDomainType == n.DomainType && !booleForkActive { + return fmt.Errorf( + "boole fork is scheduled at epoch %d but NextDomainType equals DomainType: the fork would activate with no observable domain change", + n.SSV.Forks.Boole, + ) + } + } + + return nil +} + +// maxEpochConvertibleToSlot returns the highest epoch whose first slot still fits the slot +// range; FirstSlotAtEpoch would overflow beyond it. Callers must rule out a zero SlotsPerEpoch +// first, or the division panics. +func (n Network) maxEpochConvertibleToSlot() phase0.Epoch { + return phase0.Epoch(math.MaxUint64 / n.SlotsPerEpoch) +} + // InBooleTransitionWindow checks if the slot is in the Boole transition window, // i.e., in `PRIOR_WINDOW` or `SUBSEQUENT_WINDOW` according to https://github.com/ssvlabs/SIPs/pull/43. func (n Network) InBooleTransitionWindow(slot phase0.Slot) bool { @@ -120,8 +166,7 @@ func (n Network) inBooleSubsequentWindowWithSlots(slot phase0.Slot, windowSlots // Avoid FirstSlotAtEpoch overflow when Boole is beyond the representable epoch range; // without this guard the multiplication would wrap and could treat small slots as in-window. - maxEpoch := phase0.Epoch(math.MaxUint64 / n.SlotsPerEpoch) - if n.SSV.Forks.Boole > maxEpoch { + if n.SSV.Forks.Boole > n.maxEpochConvertibleToSlot() { return false } diff --git a/networkconfig/network_test.go b/networkconfig/network_test.go index e227a595cb..e26bc1280a 100644 --- a/networkconfig/network_test.go +++ b/networkconfig/network_test.go @@ -202,6 +202,131 @@ func TestBooleForkAtSlot(t *testing.T) { } } +func TestNetworkValidate(t *testing.T) { + domainAlan := spectypes.DomainType{0x01, 0x02, 0x03, 0x04} + domainBoole := spectypes.DomainType{0x05, 0x06, 0x07, 0x08} + + tests := []struct { + name string + boole phase0.Epoch + currentEpoch phase0.Epoch // wall-clock epoch the config is validated at + preGenesis bool // put GenesisTime in the future (overrides currentEpoch) + slotsPerEpoch uint64 + domainType spectypes.DomainType + nextDomainType spectypes.DomainType + expectErr bool + }{ + { + name: "hard_error_zero_slots_per_epoch", + boole: 10, + slotsPerEpoch: 0, + domainType: domainAlan, + nextDomainType: domainBoole, + expectErr: true, + }, + { + name: "hard_error_zero_slots_per_epoch_unscheduled", + boole: phase0.Epoch(math.MaxUint64), + slotsPerEpoch: 0, + domainType: domainAlan, + nextDomainType: domainAlan, + expectErr: true, + }, + { + name: "hard_error_overflow", + boole: phase0.Epoch(math.MaxUint64/32) + 1, + slotsPerEpoch: 32, + domainType: domainAlan, + nextDomainType: domainBoole, + expectErr: true, + }, + { + name: "hard_error_next_domain_equals_domain_before_fork", + boole: 10, + currentEpoch: 5, + slotsPerEpoch: 32, + domainType: domainAlan, + nextDomainType: domainAlan, + expectErr: true, + }, + { + // The unchanged-domain check must not panic on EstimatedCurrentSlot when the + // clock is still before genesis; pre-genesis the fork is not yet active, so the + // misconfiguration is still reported as an error. + name: "hard_error_next_domain_equals_domain_pre_genesis", + boole: 10, + preGenesis: true, + slotsPerEpoch: 32, + domainType: domainAlan, + nextDomainType: domainAlan, + expectErr: true, + }, + { + name: "clean_next_domain_equals_domain_after_fork", + boole: 10, + currentEpoch: 20, + slotsPerEpoch: 32, + domainType: domainBoole, + nextDomainType: domainBoole, + }, + { + name: "clean_scheduled", + boole: 10, + slotsPerEpoch: 32, + domainType: domainAlan, + nextDomainType: domainBoole, + }, + { + name: "clean_genesis_fork", + boole: 0, + slotsPerEpoch: 32, + domainType: domainAlan, + nextDomainType: domainBoole, + }, + { + name: "clean_at_max_epoch_boundary", + boole: phase0.Epoch(math.MaxUint64 / 32), + slotsPerEpoch: 32, + domainType: domainAlan, + nextDomainType: domainBoole, + }, + { + name: "clean_unscheduled", + boole: phase0.Epoch(math.MaxUint64), + slotsPerEpoch: 32, + domainType: domainAlan, + nextDomainType: domainAlan, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + beacon := *TestNetwork.Beacon + beacon.SlotsPerEpoch = test.slotsPerEpoch + slotsSinceGenesis := uint64(test.currentEpoch) * test.slotsPerEpoch + beacon.GenesisTime = time.Now().Add(-time.Duration(slotsSinceGenesis) * beacon.SlotDuration) + if test.preGenesis { + beacon.GenesisTime = time.Now().Add(time.Hour) + } + netCfg := Network{ + Beacon: &beacon, + SSV: &SSV{ + DomainType: test.domainType, + NextDomainType: test.nextDomainType, + Forks: SSVForks{Boole: test.boole}, + }, + } + + err := netCfg.Validate() + if test.expectErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} + func beaconAtEpoch(epoch phase0.Epoch) *Beacon { beacon := *TestNetwork.Beacon slotsSinceGenesis := uint64(epoch) * beacon.SlotsPerEpoch diff --git a/operator/validator/controller.go b/operator/validator/controller.go index c2461cd42f..cfc1e523f0 100644 --- a/operator/validator/controller.go +++ b/operator/validator/controller.go @@ -1223,6 +1223,15 @@ func SetupRunners( case ssvtypes.RoleAggregator: // Post-Boole, aggregator duties route through the merged AggregatorCommitteeRunner // (committee-scoped) instead of this legacy per-validator runner. + // + // Gating on BooleFork() here is a deliberate snapshot taken at share-add time: once a + // share is added, this runner set is fixed until the share is re-added, so a share added + // shortly before the fork keeps its legacy runner across the transition. The resulting + // cross-population asymmetry (some operators still running the legacy runner while others + // have moved to the committee runner) is bounded to the Boole subsequent window + // (SlotsPerEpoch + booleSubsequentWindowLateSlots; 34 slots on mainnet — see InBooleTransitionWindow) + // and only affects already-decided pre-fork aggregator stragglers — no correctness impact. + // These legacy runners retire wholesale together with the Alan topics. if !options.NetworkConfig.BooleFork() { aggregatorValueChecker := ssv.NewAggregatorChecker(options.NetworkConfig.Beacon, share.ValidatorPubKey, share.ValidatorIndex) runners[role], err = runner.NewAggregatorRunner(runner.AggregatorRunnerOptions{ @@ -1235,6 +1244,15 @@ func SetupRunners( case ssvtypes.RoleSyncCommitteeContribution: // Post-Boole, sync committee contribution duties route through the merged // AggregatorCommitteeRunner (committee-scoped) instead of this legacy per-validator runner. + // + // Gating on BooleFork() here is a deliberate snapshot taken at share-add time: once a + // share is added, this runner set is fixed until the share is re-added, so a share added + // shortly before the fork keeps its legacy runner across the transition. The resulting + // cross-population asymmetry (some operators still running the legacy runner while others + // have moved to the committee runner) is bounded to the Boole subsequent window + // (SlotsPerEpoch + booleSubsequentWindowLateSlots; 34 slots on mainnet — see InBooleTransitionWindow) + // and only affects already-decided pre-fork sync committee contribution stragglers — no + // correctness impact. These legacy runners retire wholesale together with the Alan topics. if !options.NetworkConfig.BooleFork() { syncCommitteeContributionValueChecker := ssv.NewSyncCommitteeContributionChecker(options.NetworkConfig.Beacon, share.ValidatorPubKey, share.ValidatorIndex) runners[role], err = runner.NewSyncCommitteeAggregatorRunner(runner.SyncCommitteeAggregatorRunnerOptions{ diff --git a/protocol/v2/qbft/spectest/error_code_map_alan.go b/protocol/v2/qbft/spectest/error_code_map_alan.go index b7f335d4af..36a0bb3ab3 100644 --- a/protocol/v2/qbft/spectest/error_code_map_alan.go +++ b/protocol/v2/qbft/spectest/error_code_map_alan.go @@ -4,6 +4,13 @@ package qbft import spectypes "github.com/ssvlabs/ssv-spec/types" +// maxShiftableLegacyErrorCode is the highest v1.2.2 error code eligible for the -2 shift below. +// spectypes.TimeoutInstanceErrorCode is the last error code that existed in the spec before the +// Boole-era additions (AggComm*/Committee* codes), which have no v1.2.2 counterpart and thus were +// never subject to the shift. Its v1.2.2 value is its current (v1.2.3+) value plus the 2-code +// offset described below: 70 + 2 = 72. +const maxShiftableLegacyErrorCode = int(spectypes.TimeoutInstanceErrorCode) + 2 + func adjustExpectedErrorCode(code int) int { // Alan fixtures use v1.2.2 error-code numbering. v1.2.3 removed two enum // members after code 9, so most legacy codes shift down by 2. @@ -14,7 +21,7 @@ func adjustExpectedErrorCode(code int) int { return spectypes.ValidatorExitNoConsensusPhaseErrorCode } - if code >= 12 && code <= 79 { + if code >= 12 && code <= maxShiftableLegacyErrorCode { return code - 2 }