From 013646403cd6e4f8b79b6db798605b10bef4294e Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Mon, 3 Nov 2025 17:21:54 +0800 Subject: [PATCH] engine_v2, params: fix unsynchronized reads of V2.CurrentConfig, close XFN-53 (#1642) --- consensus/XDPoS/engines/engine_v2/engine.go | 12 +++++---- consensus/XDPoS/engines/engine_v2/timeout.go | 2 +- .../XDPoS/engines/engine_v2/verifyHeader.go | 1 + internal/ethapi/api.go | 4 +-- params/config.go | 26 ++++++++++++++----- 5 files changed, 31 insertions(+), 14 deletions(-) diff --git a/consensus/XDPoS/engines/engine_v2/engine.go b/consensus/XDPoS/engines/engine_v2/engine.go index be0971a2ae..0469d3aec8 100644 --- a/consensus/XDPoS/engines/engine_v2/engine.go +++ b/consensus/XDPoS/engines/engine_v2/engine.go @@ -142,14 +142,15 @@ func (x *XDPoS_v2) UpdateParams(header *types.Header) { x.config.V2.UpdateConfig(uint64(round)) // Setup timeoutTimer - duration := time.Duration(x.config.V2.CurrentConfig.TimeoutPeriod) * time.Second - err = x.timeoutWorker.SetParams(duration, x.config.V2.CurrentConfig.ExpTimeoutConfig.Base, x.config.V2.CurrentConfig.ExpTimeoutConfig.MaxExponent) + currentConfig := x.config.V2.GetCurrentConfig() + duration := time.Duration(currentConfig.TimeoutPeriod) * time.Second + err = x.timeoutWorker.SetParams(duration, currentConfig.ExpTimeoutConfig.Base, currentConfig.ExpTimeoutConfig.MaxExponent) if err != nil { log.Error("[UpdateParams] set params failed", "err", err) } // avoid deadlock go func() { - x.minePeriodCh <- x.config.V2.CurrentConfig.MinePeriod + x.minePeriodCh <- currentConfig.MinePeriod }() } @@ -254,10 +255,11 @@ func (x *XDPoS_v2) initial(chain consensus.ChainReader, header *types.Header) er } // Initial timeout - log.Warn("[initial] miner wait period", "period", x.config.V2.CurrentConfig.MinePeriod) + currentConfig := x.config.V2.GetCurrentConfig() + log.Warn("[initial] miner wait period", "period", currentConfig.MinePeriod) // avoid deadlock go func() { - x.minePeriodCh <- x.config.V2.CurrentConfig.MinePeriod + x.minePeriodCh <- currentConfig.MinePeriod }() // Kick-off the countdown timer diff --git a/consensus/XDPoS/engines/engine_v2/timeout.go b/consensus/XDPoS/engines/engine_v2/timeout.go index b32c723c76..59d3b60a6e 100644 --- a/consensus/XDPoS/engines/engine_v2/timeout.go +++ b/consensus/XDPoS/engines/engine_v2/timeout.go @@ -302,7 +302,7 @@ func (x *XDPoS_v2) OnCountdownTimeout(time time.Time, chain interface{}) error { } x.timeoutCount++ - if x.timeoutCount%x.config.V2.CurrentConfig.TimeoutSyncThreshold == 0 { + if x.timeoutCount%x.config.V2.GetCurrentConfig().TimeoutSyncThreshold == 0 { log.Warn("[OnCountdownTimeout] timeout sync threadhold reached, send syncInfo message") syncInfo := x.getSyncInfo() x.broadcastToBftChannel(syncInfo) diff --git a/consensus/XDPoS/engines/engine_v2/verifyHeader.go b/consensus/XDPoS/engines/engine_v2/verifyHeader.go index 8af4d6f6a0..03216610fc 100644 --- a/consensus/XDPoS/engines/engine_v2/verifyHeader.go +++ b/consensus/XDPoS/engines/engine_v2/verifyHeader.go @@ -2,6 +2,7 @@ package engine_v2 import ( "bytes" + "fmt" "math/big" "time" diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 9f2cf3a4ae..567300a2e7 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -494,8 +494,8 @@ func (s *PublicBlockChainAPI) GetStorageAt(ctx context.Context, address common.A } // GetBlockReceipts returns the block receipts for the given block hash or number or tag. -func (api *PublicBlockChainAPI) GetBlockReceipts(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) ([]map[string]interface{}, error) { - block, err := api.b.BlockByNumberOrHash(ctx, blockNrOrHash) +func (s *PublicBlockChainAPI) GetBlockReceipts(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) ([]map[string]interface{}, error) { + block, err := s.b.BlockByNumberOrHash(ctx, blockNrOrHash) if err != nil { return nil, err } diff --git a/params/config.go b/params/config.go index 515f4d9dbd..87ea2c7bb8 100644 --- a/params/config.go +++ b/params/config.go @@ -493,7 +493,7 @@ func (v2 *V2) Description(indent int) string { banner += fmt.Sprintf("%s- SwitchEpoch: %v\n", prefix, v2.SwitchEpoch) banner += fmt.Sprintf("%s- SwitchBlock: %v\n", prefix, v2.SwitchBlock) banner += fmt.Sprintf("%s- SkipV2Validation: %v\n", prefix, v2.SkipV2Validation) - banner += fmt.Sprintf("%s- %s", prefix, v2.CurrentConfig.Description("CurrentConfig", indent+2)) + banner += fmt.Sprintf("%s- %s", prefix, v2.GetCurrentConfig().Description("CurrentConfig", indent+2)) return banner } @@ -538,18 +538,32 @@ func (v *V2) UpdateConfig(round uint64) { v.CurrentConfig = v.AllConfigs[index] } -func (v *V2) Config(round uint64) *V2Config { +// GetCurrentConfig returns a opy of the current config, it assumes v2 is not nil +func (v2 *V2) GetCurrentConfig() *V2Config { + v2.lock.RLock() + defer v2.lock.RUnlock() + + if v2.CurrentConfig == nil { + return nil + } + + // avoid CurrentConfig is changed by other goroutines + cpyConfig := *v2.CurrentConfig + return &cpyConfig +} + +func (v2 *V2) Config(round uint64) *V2Config { configRound := round var index uint64 //find the right config - for i := range v.configIndex { - if v.configIndex[i] <= configRound { - index = v.configIndex[i] + for i := range v2.configIndex { + if v2.configIndex[i] <= configRound { + index = v2.configIndex[i] break } } - return v.AllConfigs[index] + return v2.AllConfigs[index] } func (v *V2) BuildConfigIndex() {