From eecbbf1af71f7daf31bc981c04329d27b98ced96 Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Tue, 27 Oct 2026 17:31:20 +0800 Subject: [PATCH 1/3] fix(common/countdown): keep the full round difference in the timeout exponent The exponent was derived from a uint8 conversion of the round difference, which truncates it modulo 256. A difference of 257 therefore collapsed the timeout to its minimum instead of saturating at maxExponent, and the window recurs every 256 rounds. Compute the difference in uint64 instead. This defect is pre-existing since the package was introduced (#764) and is independent of #2831. close #2835 --- common/countdown/exp_duration.go | 5 +++- common/countdown/exp_duration_test.go | 42 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/common/countdown/exp_duration.go b/common/countdown/exp_duration.go index 7cc4d3d68c0e..9e0867bb348e 100644 --- a/common/countdown/exp_duration.go +++ b/common/countdown/exp_duration.go @@ -56,7 +56,10 @@ func (d *ExpTimeoutDuration) GetTimeoutDuration(currentRound, highestRound types power := float64(1) // below statement must be true, just to prevent negative result if highestRound < currentRound { - exp := min(uint8(currentRound-highestRound)-1, maxExponent) + // The difference is computed in uint64: converting it to uint8 first + // truncates it modulo 256, so a round difference of 257 would collapse + // the timeout to its minimum instead of saturating at maxExponent. + exp := min(uint64(currentRound-highestRound)-1, uint64(maxExponent)) power = math.Pow(base, float64(exp)) } return duration * time.Duration(power) diff --git a/common/countdown/exp_duration_test.go b/common/countdown/exp_duration_test.go index ac9b8c5097c7..8c5bf6013de5 100644 --- a/common/countdown/exp_duration_test.go +++ b/common/countdown/exp_duration_test.go @@ -51,6 +51,48 @@ func TestExpDuration(t *testing.T) { assert.Equal(t, duration, result) } +// TestExpDurationRoundDiff pins the exponent at literal round differences +// instead of re-deriving it from the implementation formula. A round +// difference of 257 or more whose low 8 bits fall in [1, maxExponent] makes +// the uint8-truncated exponent too small (e.g. 513); 256 sits exactly on the +// wrap point, where uint8(256)-1 wraps to 255 and the old formula happens to +// be correct, so it is kept as a control. +func TestExpDurationRoundDiff(t *testing.T) { + const duration = time.Second + base := float64(2.0) + tests := []struct { + name string + maxExponent uint8 + diff uint64 + want time.Duration + }{ + {"maxExponent=0 diff=1", 0, 1, duration}, + {"maxExponent=0 diff=256", 0, 256, duration}, + {"maxExponent=0 diff=513", 0, 513, duration}, + {"maxExponent=3 diff=1", 3, 1, duration}, + {"maxExponent=3 diff=2", 3, 2, 2 * duration}, + {"maxExponent=3 diff=4 saturates", 3, 4, 8 * duration}, + {"maxExponent=3 diff=5 saturates", 3, 5, 8 * duration}, + {"maxExponent=3 diff=256 saturates", 3, 256, 8 * duration}, + {"maxExponent=3 diff=257 saturates", 3, 257, 8 * duration}, + {"maxExponent=3 diff=258 saturates", 3, 258, 8 * duration}, + {"maxExponent=3 diff=259 saturates", 3, 259, 8 * duration}, + {"maxExponent=3 diff=513 saturates", 3, 513, 8 * duration}, + {"maxExponent=31 diff=1", 31, 1, duration}, + {"maxExponent=31 diff=32 saturates", 31, 32, duration * (1 << 31)}, + {"maxExponent=31 diff=256 saturates", 31, 256, duration * (1 << 31)}, + {"maxExponent=31 diff=257 saturates", 31, 257, duration * (1 << 31)}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + helper, err := NewExpTimeoutDuration(duration, base, test.maxExponent) + assert.NoError(t, err) + got := helper.GetTimeoutDuration(types.Round(test.diff), types.Round(0)) + assert.Equal(t, test.want, got) + }) + } +} + func TestInvalidParameter(t *testing.T) { base := float64(2.0) max_exponent := uint8(32) From f67eeb6923a017c90735b7e6d3c1e7cc7cfb2d6e Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Tue, 27 Oct 2026 17:32:09 +0800 Subject: [PATCH 2/3] fix(common/countdown): saturate the timeout at the maximum time.Duration duration*base^exp can overflow int64 for a long round difference and a large base, wrapping around to a negative timeout; the timer then retriggers immediately. Saturate at math.MaxInt64 instead. This is a separate defect from the truncation in the previous commit: truncation recurs every 256 rounds, while the overflow depends on the exponent, and each can occur without the other. It is pre-existing since the package was introduced (#764). The saturated value never fires in practice; a chain whose own maximum exponential timeout is unrepresentable is rejected by the chain config validation at startup. --- common/countdown/exp_duration.go | 12 +++++++++++- common/countdown/exp_duration_test.go | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/common/countdown/exp_duration.go b/common/countdown/exp_duration.go index 9e0867bb348e..ef3a27eaab7f 100644 --- a/common/countdown/exp_duration.go +++ b/common/countdown/exp_duration.go @@ -62,7 +62,17 @@ func (d *ExpTimeoutDuration) GetTimeoutDuration(currentRound, highestRound types exp := min(uint64(currentRound-highestRound)-1, uint64(maxExponent)) power = math.Pow(base, float64(exp)) } - return duration * time.Duration(power) + // duration*power can exceed the range of time.Duration for a long round + // difference and a large base; saturate instead of wrapping around to a + // negative timeout, which would retrigger the timer immediately. A chain + // whose maximum exponential timeout is unrepresentable is rejected by the + // chain config validation at startup, so this branch is a backstop for + // configs that bypass that check. + multiplier := int64(power) + if multiplier > 0 && int64(duration) > math.MaxInt64/multiplier { + return time.Duration(math.MaxInt64) + } + return time.Duration(int64(duration) * multiplier) } // SetParams replaces the whole parameter set; an invalid set is rejected diff --git a/common/countdown/exp_duration_test.go b/common/countdown/exp_duration_test.go index 8c5bf6013de5..c775fa4af0c2 100644 --- a/common/countdown/exp_duration_test.go +++ b/common/countdown/exp_duration_test.go @@ -93,6 +93,26 @@ func TestExpDurationRoundDiff(t *testing.T) { } } +// TestExpDurationOverflowSaturates checks that a timeout outside the int64 range +// saturates at the maximum duration instead of wrapping around to a negative +// (or arbitrarily small) one. +func TestExpDurationOverflowSaturates(t *testing.T) { + helper, err := NewExpTimeoutDuration(59*time.Second, float64(2.0), 31) + assert.NoError(t, err) + // 59s * 2^27 fits in a time.Duration (about 251 years), 59s * 2^28 overflows. + assert.Equal(t, 59*time.Second*(1<<27), helper.GetTimeoutDuration(types.Round(28), types.Round(0))) + assert.Equal(t, time.Duration(math.MaxInt64), helper.GetTimeoutDuration(types.Round(29), types.Round(0))) + assert.Equal(t, time.Duration(math.MaxInt64), helper.GetTimeoutDuration(types.Round(31), types.Round(0))) + assert.Equal(t, time.Duration(math.MaxInt64), helper.GetTimeoutDuration(types.Round(256), types.Round(0))) + assert.Equal(t, time.Duration(math.MaxInt64), helper.GetTimeoutDuration(types.Round(257), types.Round(0))) + assert.Equal(t, time.Duration(math.MaxInt64), helper.GetTimeoutDuration(types.Round(513), types.Round(0))) + // The guard must be a strict inequality: at the exact boundary the product + // is MaxInt64-1 and still fits, so it must not saturate. + exact, err := NewExpTimeoutDuration(9223372036854775806/3, float64(3.0), 1) + assert.NoError(t, err) + assert.Equal(t, time.Duration(9223372036854775806), exact.GetTimeoutDuration(types.Round(2), types.Round(0))) +} + func TestInvalidParameter(t *testing.T) { base := float64(2.0) max_exponent := uint8(32) From 0063916d4181f4cfdaa45777b0f8cfb5e6ce9961 Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Tue, 27 Oct 2026 17:33:02 +0800 Subject: [PATCH 3/3] fix(params): reject XDPoS v2 exp timeouts that overflow time.Duration common/countdown arms the pacemaker timer with timeout_period * base^exp. The engine converts the period with time.Duration(TimeoutPeriod) * time.Second first, then common/countdown multiplies by base^exp; when either step leaves the int64 range the timer saturates at math.MaxInt64 (~292 years) and never fires, so the node silently stops producing timeout messages. validateV2ExpTimeoutConfig only bounded max_exponent and base^max_exponent, which admits such a genesis: with base=2, maxExponent=30 and the 30s mainnet timeout period the product already overflows from round difference 30 on. Bound both steps at the time.Duration range in whole seconds, so a chain whose own maximum exponential timeout is unrepresentable fails at startup instead of running until the round difference grows. The per-step bound matters: with base < 1, base^max_exponent shrinks the multiplier, so a single combined check would let an overflowing period through. No network shipped in this repository is affected: all 19 built-in V2Config entries and every genesis file use base=1 and maxExponent=0, where the product is just timeout_period seconds. Also correct the TimeoutPeriod comment, which claims milliseconds while every consumer multiplies by time.Second. go-ethereum has no common/countdown package and no equivalent chain-config check. The closest upstream precedent is geth #31407 (eth/catalyst: sanitize simulated beacon period to avoid overflowing time.Duration), which clamps the period with an integer bound instead; this commit rejects the config instead, because a saturated pacemaker stalls silently while a rejected config fails loudly at startup. Checking the largest exponent covers every smaller one only while base >= 1; with base < 1 the largest timeout is at exponent 0, which the period conversion check above bounds on its own. --- params/config_backfill.go | 16 +++++- params/config_test.go | 114 ++++++++++++++++++++++++++++++++++++++ params/config_xdpos.go | 2 +- 3 files changed, 130 insertions(+), 2 deletions(-) diff --git a/params/config_backfill.go b/params/config_backfill.go index 9a6a7c12c8c1..0a8f008dcf5a 100644 --- a/params/config_backfill.go +++ b/params/config_backfill.go @@ -33,6 +33,7 @@ import ( "math" "math/big" "strings" + "time" "github.com/XinFinOrg/XDPoSChain/common" "github.com/XinFinOrg/XDPoSChain/log" @@ -753,9 +754,22 @@ func validateV2ExpTimeoutConfig(cfg *V2Config, fieldPath string) error { if cfg.ExpTimeoutConfig.MaxExponent >= 32 { return fmt.Errorf("%s: max_exponent (%d) >= max_exponent_upperbound (%d)", fieldPath, cfg.ExpTimeoutConfig.MaxExponent, 32) } - if math.Pow(cfg.ExpTimeoutConfig.Base, float64(cfg.ExpTimeoutConfig.MaxExponent)) >= float64(math.MaxUint32) { + power := math.Pow(cfg.ExpTimeoutConfig.Base, float64(cfg.ExpTimeoutConfig.MaxExponent)) + if power >= float64(math.MaxUint32) { return fmt.Errorf("%s: base^max_exponent (%f^%d) should be less than 2^32", fieldPath, cfg.ExpTimeoutConfig.Base, cfg.ExpTimeoutConfig.MaxExponent) } + // The engine converts the period with time.Duration(TimeoutPeriod) * time.Second + // and common/countdown then multiplies it by base^exp. Both steps must fit in a + // time.Duration: a saturated timeout is math.MaxInt64 (~292 years), so the timer + // never fires and the pacemaker silently stops. Reject such a config at startup. + const maxTimeoutSeconds = math.MaxInt64 / int64(time.Second) + if int64(cfg.TimeoutPeriod) > maxTimeoutSeconds { + return fmt.Errorf("%s: timeout_period (%ds) exceeds the time.Duration range", fieldPath, cfg.TimeoutPeriod) + } + ns := int64(cfg.TimeoutPeriod) * int64(time.Second) + if multiplier := int64(power); multiplier > 0 && ns > math.MaxInt64/multiplier { + return fmt.Errorf("%s: timeout_period (%ds) * base^max_exponent (%f^%d) exceeds the time.Duration range", fieldPath, cfg.TimeoutPeriod, cfg.ExpTimeoutConfig.Base, cfg.ExpTimeoutConfig.MaxExponent) + } return nil } diff --git a/params/config_test.go b/params/config_test.go index c3b2312ebdee..57206aebaa58 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -19,9 +19,11 @@ package params import ( "encoding/json" "errors" + "math" "math/big" "reflect" "slices" + "strconv" "strings" "testing" @@ -2069,3 +2071,115 @@ func TestActiveSystemContractsTracksXDCActivationBlocks(t *testing.T) { }) } } + +// expTimeoutTestChainConfig builds an XDC chain config that satisfies every +// CheckConfigForkOrder requirement except the exponential timeout bounds under +// test, so the validation reaches the checks this test targets. +func expTimeoutTestChainConfig(timeoutPeriod int, base float64, maxExponent uint8) *ChainConfig { + newV2Config := func() *V2Config { + return &V2Config{ + SwitchRound: 0, + MinePeriod: 2, + TimeoutPeriod: timeoutPeriod, + ExpTimeoutConfig: ExpTimeoutConfig{ + Base: base, + MaxExponent: maxExponent, + }, + } + } + return &ChainConfig{ + ChainID: big.NewInt(1234), + TIPTRC21FeeBlock: big.NewInt(0), + Gas50xBlock: big.NewInt(0), + TRC21IssuerSMC: TestnetChainConfig.TRC21IssuerSMC, + XDCXListingSMC: TestnetChainConfig.XDCXListingSMC, + RelayerRegistrationSMC: TestnetChainConfig.RelayerRegistrationSMC, + LendingRegistrationSMC: TestnetChainConfig.LendingRegistrationSMC, + XDPoS: &XDPoSConfig{ + Epoch: 900, + FoundationWalletAddr: TestnetChainConfig.XDPoS.FoundationWalletAddr, + MaxMasternodesV2: 108, + V2: &V2{ + SwitchEpoch: 1, + SwitchBlock: big.NewInt(900), + CurrentConfig: newV2Config(), + AllConfigs: map[uint64]*V2Config{ + 0: newV2Config(), + }, + }, + }, + } +} + +// TestValidateV2ExpTimeoutConfigDurationRange checks that a chain config whose +// largest exponential timeout does not fit in a time.Duration is rejected: the +// countdown timer would otherwise saturate at math.MaxInt64 and never fire. +func TestValidateV2ExpTimeoutConfigDurationRange(t *testing.T) { + tests := []struct { + name string + timeout int64 + base float64 + maxExponent uint8 + wantErr bool + }{ + {name: "built-in network shape", timeout: 30, base: 1, maxExponent: 0}, + {name: "largest timeout still fits", timeout: 59, base: 2, maxExponent: 27}, + // At the exact product boundary the timeout is representable and must + // be accepted: 3e9 ns * 3074457345 == 9223372035000000000 <= MaxInt64. + {name: "largest product fits at the exact boundary", timeout: 3, base: 3074457345, maxExponent: 1}, + {name: "largest timeout overflows", timeout: 59, base: 2, maxExponent: 28, wantErr: true}, + {name: "long period with large exponent", timeout: 30, base: 2, maxExponent: 30, wantErr: true}, + // The engine converts the period to nanoseconds before applying + // base^exp, so the conversion overflows even though base < 1 shrinks + // the multiplier. + {name: "period conversion overflows with shrinking base", timeout: 18_000_000_000, base: 0.5, maxExponent: 1, wantErr: true}, + {name: "period conversion boundary still fits", timeout: 9_223_372_036, base: 1, maxExponent: 0}, + {name: "period conversion boundary overflows", timeout: 9_223_372_037, base: 1, maxExponent: 0, wantErr: true}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if test.timeout > math.MaxInt32 && strconv.IntSize == 32 { + t.Skip("timeout_period exceeds the 32-bit int range") + } + cfg := expTimeoutTestChainConfig(int(test.timeout), test.base, test.maxExponent) + err := cfg.CheckConfigForkOrder() + if !test.wantErr { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + return + } + if !errors.Is(err, ErrWrongForkSwitchOrder) { + t.Fatalf("unexpected error: have %v want %v", err, ErrWrongForkSwitchOrder) + } + if !strings.Contains(err.Error(), "XDPoS.V2.CurrentConfig.ExpTimeoutConfig") { + t.Fatalf("error does not name the field: %v", err) + } + if !strings.Contains(err.Error(), "time.Duration range") { + t.Fatalf("unexpected error: %v", err) + } + }) + } + t.Run("all configs entry is validated", func(t *testing.T) { + cfg := expTimeoutTestChainConfig(30, 1, 0) + cfg.XDPoS.V2.AllConfigs[10] = &V2Config{ + SwitchRound: 10, + MinePeriod: 2, + TimeoutPeriod: 59, + ExpTimeoutConfig: ExpTimeoutConfig{ + Base: 2, + MaxExponent: 28, + }, + } + err := cfg.CheckConfigForkOrder() + if !errors.Is(err, ErrWrongForkSwitchOrder) { + t.Fatalf("unexpected error: have %v want %v", err, ErrWrongForkSwitchOrder) + } + if !strings.Contains(err.Error(), "XDPoS.V2.AllConfigs[10].ExpTimeoutConfig") { + t.Fatalf("error does not name the all-configs entry: %v", err) + } + if !strings.Contains(err.Error(), "time.Duration range") { + t.Fatalf("unexpected error: %v", err) + } + }) +} diff --git a/params/config_xdpos.go b/params/config_xdpos.go index ae36144be3fe..2f1c701bec5b 100644 --- a/params/config_xdpos.go +++ b/params/config_xdpos.go @@ -131,7 +131,7 @@ type V2Config struct { SwitchRound uint64 `json:"switchRound"` // v1 to v2 switch block number MinePeriod int `json:"minePeriod"` // Miner mine period to mine a block TimeoutSyncThreshold int `json:"timeoutSyncThreshold"` // send syncInfo after number of timeout - TimeoutPeriod int `json:"timeoutPeriod"` // Duration in ms + TimeoutPeriod int `json:"timeoutPeriod"` // Duration in seconds CertThreshold float64 `json:"certificateThreshold"` // Necessary number of messages from master nodes to form a certificate MasternodeReward float64 `json:"masternodeReward"` // Block reward per master node (core validator) - unit Ether