Skip to content

Commit 5288c41

Browse files
gzliudangzliudan
committed
fix(miner): avoid XDPoS-only paths on non-XDPoS engines
Avoid panics in update() by guarding XDPoS-only channels when the engine is not XDPoS. Skip work commits when XDPoS config is enabled but the engine is not XDPoS, with height-based log throttling. Add regression tests for non-XDPoS update loop and XDPoS config/engine mismatch.
1 parent 50210d9 commit 5288c41

File tree

2 files changed

+173
-30
lines changed

2 files changed

+173
-30
lines changed

‎miner/worker.go‎

Lines changed: 49 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,21 @@ type worker struct {
161161
atWork int32
162162
announceTxs bool
163163
lastParentBlockCommit string
164+
165+
// last block number logged for XDPoS config/engine mismatch
166+
lastXDPoSMismatchBlock *big.Int
167+
}
168+
169+
func (w *worker) lockAll() {
170+
w.mu.Lock()
171+
w.uncleMu.Lock()
172+
w.currentMu.Lock()
173+
}
174+
175+
func (w *worker) unlockAll() {
176+
w.currentMu.Unlock()
177+
w.uncleMu.Unlock()
178+
w.mu.Unlock()
164179
}
165180

166181
func newWorker(config *params.ChainConfig, engine consensus.Engine, coinbase common.Address, eth Backend, mux *event.TypeMux, announceTxs bool) *worker {
@@ -286,8 +301,12 @@ func (w *worker) update() {
286301

287302
// timeout waiting for v1 initial value
288303
minePeriod := 2
289-
MinePeriodCh := w.engine.(*XDPoS.XDPoS).MinePeriodCh
290-
NewRoundCh := w.engine.(*XDPoS.XDPoS).NewRoundCh
304+
var MinePeriodCh <-chan int
305+
var NewRoundCh <-chan types.Round
306+
if xdposEngine, ok := w.engine.(*XDPoS.XDPoS); ok {
307+
MinePeriodCh = xdposEngine.MinePeriodCh
308+
NewRoundCh = xdposEngine.NewRoundCh
309+
}
291310

292311
timeout := time.NewTimer(time.Duration(minePeriod) * time.Second)
293312
defer timeout.Stop()
@@ -646,37 +665,41 @@ func (w *worker) makeCurrent(parent *types.Block, header *types.Header) error {
646665
return nil
647666
}
648667

649-
// checkPreCommitWithLock checks whether a new work commit is needed with locks,
650-
// returns the parent block and shouldReturn.
651-
func (w *worker) checkPreCommitWithLock() (*types.Block, bool) {
652-
w.mu.Lock()
653-
defer w.mu.Unlock()
654-
w.uncleMu.Lock()
655-
defer w.uncleMu.Unlock()
656-
w.currentMu.Lock()
657-
defer w.currentMu.Unlock()
658-
659-
return w.checkPreCommit()
660-
}
668+
// preCommitCheck checks whether a new work commit is needed.
669+
// If locked is false, the worker locks are acquired internally.
670+
// If locked is true, callers must already hold w.mu, w.uncleMu, and w.currentMu.
671+
func (w *worker) preCommitCheck(locked bool) (*types.Block, bool) {
672+
if !locked {
673+
w.lockAll()
674+
defer w.unlockAll()
675+
}
661676

662-
// checkPreCommit checks whether a new work commit is needed,
663-
// returns the parent block and shouldReturn.
664-
func (w *worker) checkPreCommit() (*types.Block, bool) {
665-
c := w.engine.(*XDPoS.XDPoS)
677+
var xdposEngine *XDPoS.XDPoS
678+
if engine, ok := w.engine.(*XDPoS.XDPoS); ok {
679+
xdposEngine = engine
680+
}
666681
var parent *types.Block
667682
currentHeader := w.chain.CurrentBlock()
668683
// Guard against nil header (early startup or uninitialised chain).
669684
if currentHeader == nil {
670685
return nil, true
671686
}
672-
if c != nil {
673-
parent = c.FindParentBlockToAssign(w.chain, currentHeader)
687+
if xdposEngine != nil {
688+
parent = xdposEngine.FindParentBlockToAssign(w.chain, currentHeader)
674689
} else {
675690
parent = w.chain.GetBlock(currentHeader.Hash(), currentHeader.Number.Uint64())
676691
}
677692
if parent == nil {
678693
return nil, true
679694
}
695+
if w.config.XDPoS != nil && xdposEngine == nil {
696+
blockNum := new(big.Int).Set(parent.Number())
697+
if w.lastXDPoSMismatchBlock == nil || blockNum.Cmp(w.lastXDPoSMismatchBlock) > 0 {
698+
w.lastXDPoSMismatchBlock = blockNum
699+
log.Warn("XDPoS config enabled but consensus engine is not XDPoS; skipping work commit", "block", blockNum)
700+
}
701+
return parent, true
702+
}
680703
if parent.Hash().Hex() == w.lastParentBlockCommit {
681704
return parent, true
682705
}
@@ -687,8 +710,8 @@ func (w *worker) checkPreCommit() (*types.Block, bool) {
687710
// Only try to commit new work if we are mining
688711
if atomic.LoadInt32(&w.mining) == 1 {
689712
// check if we are right after parent's coinbase in the list
690-
if w.config.XDPoS != nil {
691-
ok, err := c.YourTurn(w.chain, parent.Header(), w.coinbase)
713+
if w.config.XDPoS != nil && xdposEngine != nil {
714+
ok, err := xdposEngine.YourTurn(w.chain, parent.Header(), w.coinbase)
692715
if err != nil {
693716
log.Warn("Failed when trying to commit new work", "err", err)
694717
return parent, true
@@ -704,7 +727,7 @@ func (w *worker) checkPreCommit() (*types.Block, bool) {
704727
}
705728

706729
func (w *worker) commitNewWork() {
707-
parent, shouldReturn := w.checkPreCommitWithLock()
730+
parent, shouldReturn := w.preCommitCheck(false)
708731
if parent == nil || shouldReturn {
709732
return
710733
}
@@ -720,14 +743,10 @@ func (w *worker) commitNewWork() {
720743
time.Sleep(wait)
721744
}
722745

723-
w.mu.Lock()
724-
defer w.mu.Unlock()
725-
w.uncleMu.Lock()
726-
defer w.uncleMu.Unlock()
727-
w.currentMu.Lock()
728-
defer w.currentMu.Unlock()
746+
w.lockAll()
747+
defer w.unlockAll()
729748

730-
parent, shouldReturn = w.checkPreCommit()
749+
parent, shouldReturn = w.preCommitCheck(true)
731750
if parent == nil || shouldReturn {
732751
return
733752
}

‎miner/worker_test.go‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// Copyright 2026 The go-ethereum Authors
2+
// This file is part of the go-ethereum library.
3+
//
4+
// The go-ethereum library is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU Lesser General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
//
9+
// The go-ethereum library is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU Lesser General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU Lesser General Public License
15+
// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
16+
17+
package miner
18+
19+
import (
20+
"math/big"
21+
"testing"
22+
"time"
23+
24+
"github.com/XinFinOrg/XDPoSChain/common"
25+
"github.com/XinFinOrg/XDPoSChain/consensus/XDPoS/utils"
26+
"github.com/XinFinOrg/XDPoSChain/consensus/ethash"
27+
"github.com/XinFinOrg/XDPoSChain/core"
28+
"github.com/XinFinOrg/XDPoSChain/core/rawdb"
29+
"github.com/XinFinOrg/XDPoSChain/core/types"
30+
"github.com/XinFinOrg/XDPoSChain/core/vm"
31+
"github.com/XinFinOrg/XDPoSChain/event"
32+
"github.com/XinFinOrg/XDPoSChain/params"
33+
)
34+
35+
func newBlockingSubscription() event.Subscription {
36+
return event.NewSubscription(func(unsub <-chan struct{}) error {
37+
<-unsub
38+
return nil
39+
})
40+
}
41+
42+
func TestWorkerUpdateNonXDPoSStaysRunning(t *testing.T) {
43+
worker := &worker{
44+
engine: ethash.NewFaker(),
45+
chainHeadSub: newBlockingSubscription(),
46+
chainSideSub: newBlockingSubscription(),
47+
resetCh: make(chan time.Duration, 1),
48+
}
49+
50+
done := make(chan struct{})
51+
go func() {
52+
worker.update()
53+
close(done)
54+
}()
55+
56+
select {
57+
case <-done:
58+
t.Fatal("worker.update returned early for non-XDPoS engine")
59+
case <-time.After(200 * time.Millisecond):
60+
// Expected: update keeps running for non-XDPoS engines.
61+
}
62+
63+
worker.chainHeadSub.Unsubscribe()
64+
select {
65+
case <-done:
66+
// Expected: update exits after subscription error.
67+
case <-time.After(time.Second):
68+
t.Fatal("worker.update did not return after unsubscribe")
69+
}
70+
}
71+
72+
func TestWorkerCheckPreCommitXDPoSMismatch(t *testing.T) {
73+
config := &params.ChainConfig{
74+
ChainID: big.NewInt(1),
75+
XDPoS: &params.XDPoSConfig{
76+
V2: &params.V2{
77+
SwitchBlock: big.NewInt(0),
78+
AllConfigs: map[uint64]*params.V2Config{
79+
0: {MinePeriod: 2},
80+
},
81+
},
82+
},
83+
}
84+
signer := common.HexToAddress("0x0000000000000000000000000000000000000001")
85+
extraData := make([]byte, 0, utils.ExtraVanity+common.AddressLength+utils.ExtraSeal)
86+
extraData = append(extraData, make([]byte, utils.ExtraVanity)...)
87+
extraData = append(extraData, signer.Bytes()...)
88+
extraData = append(extraData, make([]byte, utils.ExtraSeal)...)
89+
genesis := &core.Genesis{
90+
Config: config,
91+
GasLimit: params.TargetGasLimit,
92+
Difficulty: big.NewInt(1),
93+
Alloc: types.GenesisAlloc{},
94+
ExtraData: extraData,
95+
}
96+
db := rawdb.NewMemoryDatabase()
97+
if _, err := genesis.Commit(db); err != nil {
98+
t.Fatalf("failed to commit genesis: %v", err)
99+
}
100+
engine := ethash.NewFaker()
101+
chain, err := core.NewBlockChain(db, nil, genesis, engine, vm.Config{})
102+
if err != nil {
103+
t.Fatalf("failed to create blockchain: %v", err)
104+
}
105+
defer chain.Stop()
106+
107+
worker := &worker{
108+
config: config,
109+
engine: engine,
110+
chain: chain,
111+
announceTxs: true,
112+
}
113+
114+
parent, shouldReturn := worker.preCommitCheck(false)
115+
if parent == nil {
116+
t.Fatal("expected parent block, got nil")
117+
}
118+
if !shouldReturn {
119+
t.Fatal("expected checkPreCommit to skip when XDPoS config is enabled but engine is not XDPoS")
120+
}
121+
if parent.Number().Sign() != 0 {
122+
t.Fatalf("expected genesis parent, got number %v", parent.Number())
123+
}
124+
}

0 commit comments

Comments
 (0)