Skip to content

Commit a4bc2c3

Browse files
karalabekaralabe
authored
Merge pull request #17540 from karalabe/miner-uncle-fix
miner: track uncles more aggressively
2 parents 75ae5af + f751c6e commit a4bc2c3

File tree

2 files changed

+63
-51
lines changed

2 files changed

+63
-51
lines changed

‎miner/worker.go‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ package miner
1818

1919
import (
2020
"bytes"
21-
"fmt"
21+
"errors"
2222
"math/big"
2323
"sync"
2424
"sync/atomic"
@@ -620,13 +620,16 @@ func (w *worker) makeCurrent(parent *types.Block, header *types.Header) error {
620620
func (w *worker) commitUncle(env *environment, uncle *types.Header) error {
621621
hash := uncle.Hash()
622622
if env.uncles.Contains(hash) {
623-
return fmt.Errorf("uncle not unique")
623+
return errors.New("uncle not unique")
624+
}
625+
if env.header.ParentHash == uncle.ParentHash {
626+
return errors.New("uncle is sibling")
624627
}
625628
if !env.ancestors.Contains(uncle.ParentHash) {
626-
return fmt.Errorf("uncle's parent unknown (%x)", uncle.ParentHash[0:4])
629+
return errors.New("uncle's parent unknown")
627630
}
628631
if env.family.Contains(hash) {
629-
return fmt.Errorf("uncle already in family (%x)", hash)
632+
return errors.New("uncle already included")
630633
}
631634
env.uncles.Add(uncle.Hash())
632635
return nil
@@ -852,29 +855,24 @@ func (w *worker) commitNewWork(interrupt *int32, noempty bool) {
852855
if w.config.DAOForkSupport && w.config.DAOForkBlock != nil && w.config.DAOForkBlock.Cmp(header.Number) == 0 {
853856
misc.ApplyDAOHardFork(env.state)
854857
}
855-
856-
// compute uncles for the new block.
857-
var (
858-
uncles []*types.Header
859-
badUncles []common.Hash
860-
)
858+
// Accumulate the uncles for the current block
859+
for hash, uncle := range w.possibleUncles {
860+
if uncle.NumberU64()+staleThreshold <= header.Number.Uint64() {
861+
delete(w.possibleUncles, hash)
862+
}
863+
}
864+
uncles := make([]*types.Header, 0, 2)
861865
for hash, uncle := range w.possibleUncles {
862866
if len(uncles) == 2 {
863867
break
864868
}
865869
if err := w.commitUncle(env, uncle.Header()); err != nil {
866-
log.Trace("Bad uncle found and will be removed", "hash", hash)
867-
log.Trace(fmt.Sprint(uncle))
868-
869-
badUncles = append(badUncles, hash)
870+
log.Trace("Possible uncle rejected", "hash", hash, "reason", err)
870871
} else {
871872
log.Debug("Committing new uncle to block", "hash", hash)
872873
uncles = append(uncles, uncle.Header())
873874
}
874875
}
875-
for _, hash := range badUncles {
876-
delete(w.possibleUncles, hash)
877-
}
878876

879877
if !noempty {
880878
// Create an empty block based on temporary copied state for sealing in advance without waiting block

‎miner/worker_test.go‎

Lines changed: 48 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ var (
4545
testBankAddress = crypto.PubkeyToAddress(testBankKey.PublicKey)
4646
testBankFunds = big.NewInt(1000000000000000000)
4747

48-
acc1Key, _ = crypto.GenerateKey()
49-
acc1Addr = crypto.PubkeyToAddress(acc1Key.PublicKey)
48+
testUserKey, _ = crypto.GenerateKey()
49+
testUserAddress = crypto.PubkeyToAddress(testUserKey.PublicKey)
5050

5151
// Test transactions
5252
pendingTxs []*types.Transaction
@@ -62,9 +62,9 @@ func init() {
6262
Period: 10,
6363
Epoch: 30000,
6464
}
65-
tx1, _ := types.SignTx(types.NewTransaction(0, acc1Addr, big.NewInt(1000), params.TxGas, nil, nil), types.HomesteadSigner{}, testBankKey)
65+
tx1, _ := types.SignTx(types.NewTransaction(0, testUserAddress, big.NewInt(1000), params.TxGas, nil, nil), types.HomesteadSigner{}, testBankKey)
6666
pendingTxs = append(pendingTxs, tx1)
67-
tx2, _ := types.SignTx(types.NewTransaction(1, acc1Addr, big.NewInt(1000), params.TxGas, nil, nil), types.HomesteadSigner{}, testBankKey)
67+
tx2, _ := types.SignTx(types.NewTransaction(1, testUserAddress, big.NewInt(1000), params.TxGas, nil, nil), types.HomesteadSigner{}, testBankKey)
6868
newTxs = append(newTxs, tx2)
6969
}
7070

@@ -77,7 +77,7 @@ type testWorkerBackend struct {
7777
uncleBlock *types.Block
7878
}
7979

80-
func newTestWorkerBackend(t *testing.T, chainConfig *params.ChainConfig, engine consensus.Engine) *testWorkerBackend {
80+
func newTestWorkerBackend(t *testing.T, chainConfig *params.ChainConfig, engine consensus.Engine, n int) *testWorkerBackend {
8181
var (
8282
db = ethdb.NewMemDatabase()
8383
gspec = core.Genesis{
@@ -92,14 +92,28 @@ func newTestWorkerBackend(t *testing.T, chainConfig *params.ChainConfig, engine
9292
copy(gspec.ExtraData[32:], testBankAddress[:])
9393
case *ethash.Ethash:
9494
default:
95-
t.Fatal("unexpect consensus engine type")
95+
t.Fatalf("unexpected consensus engine type: %T", engine)
9696
}
9797
genesis := gspec.MustCommit(db)
9898

9999
chain, _ := core.NewBlockChain(db, nil, gspec.Config, engine, vm.Config{})
100100
txpool := core.NewTxPool(testTxPoolConfig, chainConfig, chain)
101-
blocks, _ := core.GenerateChain(chainConfig, genesis, engine, db, 1, func(i int, gen *core.BlockGen) {
102-
gen.SetCoinbase(acc1Addr)
101+
102+
// Generate a small n-block chain and an uncle block for it
103+
if n > 0 {
104+
blocks, _ := core.GenerateChain(chainConfig, genesis, engine, db, n, func(i int, gen *core.BlockGen) {
105+
gen.SetCoinbase(testBankAddress)
106+
})
107+
if _, err := chain.InsertChain(blocks); err != nil {
108+
t.Fatalf("failed to insert origin chain: %v", err)
109+
}
110+
}
111+
parent := genesis
112+
if n > 0 {
113+
parent = chain.GetBlockByHash(chain.CurrentBlock().ParentHash())
114+
}
115+
blocks, _ := core.GenerateChain(chainConfig, parent, engine, db, 1, func(i int, gen *core.BlockGen) {
116+
gen.SetCoinbase(testUserAddress)
103117
})
104118

105119
return &testWorkerBackend{
@@ -116,8 +130,8 @@ func (b *testWorkerBackend) PostChainEvents(events []interface{}) {
116130
b.chain.PostChainEvents(events, nil)
117131
}
118132

119-
func newTestWorker(t *testing.T, chainConfig *params.ChainConfig, engine consensus.Engine) (*worker, *testWorkerBackend) {
120-
backend := newTestWorkerBackend(t, chainConfig, engine)
133+
func newTestWorker(t *testing.T, chainConfig *params.ChainConfig, engine consensus.Engine, blocks int) (*worker, *testWorkerBackend) {
134+
backend := newTestWorkerBackend(t, chainConfig, engine, blocks)
121135
backend.txPool.AddLocals(pendingTxs)
122136
w := newWorker(chainConfig, engine, backend, new(event.TypeMux), time.Second, params.GenesisGasLimit, params.GenesisGasLimit)
123137
w.setEtherbase(testBankAddress)
@@ -134,24 +148,24 @@ func TestPendingStateAndBlockClique(t *testing.T) {
134148
func testPendingStateAndBlock(t *testing.T, chainConfig *params.ChainConfig, engine consensus.Engine) {
135149
defer engine.Close()
136150

137-
w, b := newTestWorker(t, chainConfig, engine)
151+
w, b := newTestWorker(t, chainConfig, engine, 0)
138152
defer w.close()
139153

140154
// Ensure snapshot has been updated.
141155
time.Sleep(100 * time.Millisecond)
142156
block, state := w.pending()
143157
if block.NumberU64() != 1 {
144-
t.Errorf("block number mismatch, has %d, want %d", block.NumberU64(), 1)
158+
t.Errorf("block number mismatch: have %d, want %d", block.NumberU64(), 1)
145159
}
146-
if balance := state.GetBalance(acc1Addr); balance.Cmp(big.NewInt(1000)) != 0 {
147-
t.Errorf("account balance mismatch, has %d, want %d", balance, 1000)
160+
if balance := state.GetBalance(testUserAddress); balance.Cmp(big.NewInt(1000)) != 0 {
161+
t.Errorf("account balance mismatch: have %d, want %d", balance, 1000)
148162
}
149163
b.txPool.AddLocals(newTxs)
150164
// Ensure the new tx events has been processed
151165
time.Sleep(100 * time.Millisecond)
152166
block, state = w.pending()
153-
if balance := state.GetBalance(acc1Addr); balance.Cmp(big.NewInt(2000)) != 0 {
154-
t.Errorf("account balance mismatch, has %d, want %d", balance, 2000)
167+
if balance := state.GetBalance(testUserAddress); balance.Cmp(big.NewInt(2000)) != 0 {
168+
t.Errorf("account balance mismatch: have %d, want %d", balance, 2000)
155169
}
156170
}
157171

@@ -165,7 +179,7 @@ func TestEmptyWorkClique(t *testing.T) {
165179
func testEmptyWork(t *testing.T, chainConfig *params.ChainConfig, engine consensus.Engine) {
166180
defer engine.Close()
167181

168-
w, _ := newTestWorker(t, chainConfig, engine)
182+
w, _ := newTestWorker(t, chainConfig, engine, 0)
169183
defer w.close()
170184

171185
var (
@@ -179,10 +193,10 @@ func testEmptyWork(t *testing.T, chainConfig *params.ChainConfig, engine consens
179193
receiptLen, balance = 1, big.NewInt(1000)
180194
}
181195
if len(task.receipts) != receiptLen {
182-
t.Errorf("receipt number mismatch has %d, want %d", len(task.receipts), receiptLen)
196+
t.Errorf("receipt number mismatch: have %d, want %d", len(task.receipts), receiptLen)
183197
}
184-
if task.state.GetBalance(acc1Addr).Cmp(balance) != 0 {
185-
t.Errorf("account balance mismatch has %d, want %d", task.state.GetBalance(acc1Addr), balance)
198+
if task.state.GetBalance(testUserAddress).Cmp(balance) != 0 {
199+
t.Errorf("account balance mismatch: have %d, want %d", task.state.GetBalance(testUserAddress), balance)
186200
}
187201
}
188202

@@ -219,19 +233,19 @@ func TestStreamUncleBlock(t *testing.T) {
219233
ethash := ethash.NewFaker()
220234
defer ethash.Close()
221235

222-
w, b := newTestWorker(t, ethashChainConfig, ethash)
236+
w, b := newTestWorker(t, ethashChainConfig, ethash, 1)
223237
defer w.close()
224238

225239
var taskCh = make(chan struct{})
226240

227241
taskIndex := 0
228242
w.newTaskHook = func(task *task) {
229-
if task.block.NumberU64() == 1 {
243+
if task.block.NumberU64() == 2 {
230244
if taskIndex == 2 {
231-
has := task.block.Header().UncleHash
245+
have := task.block.Header().UncleHash
232246
want := types.CalcUncleHash([]*types.Header{b.uncleBlock.Header()})
233-
if has != want {
234-
t.Errorf("uncle hash mismatch, has %s, want %s", has.Hex(), want.Hex())
247+
if have != want {
248+
t.Errorf("uncle hash mismatch: have %s, want %s", have.Hex(), want.Hex())
235249
}
236250
}
237251
taskCh <- struct{}{}
@@ -248,12 +262,12 @@ func TestStreamUncleBlock(t *testing.T) {
248262
// Ensure worker has finished initialization
249263
for {
250264
b := w.pendingBlock()
251-
if b != nil && b.NumberU64() == 1 {
265+
if b != nil && b.NumberU64() == 2 {
252266
break
253267
}
254268
}
255-
256269
w.start()
270+
257271
// Ignore the first two works
258272
for i := 0; i < 2; i += 1 {
259273
select {
@@ -282,7 +296,7 @@ func TestRegenerateMiningBlockClique(t *testing.T) {
282296
func testRegenerateMiningBlock(t *testing.T, chainConfig *params.ChainConfig, engine consensus.Engine) {
283297
defer engine.Close()
284298

285-
w, b := newTestWorker(t, chainConfig, engine)
299+
w, b := newTestWorker(t, chainConfig, engine, 0)
286300
defer w.close()
287301

288302
var taskCh = make(chan struct{})
@@ -293,10 +307,10 @@ func testRegenerateMiningBlock(t *testing.T, chainConfig *params.ChainConfig, en
293307
if taskIndex == 2 {
294308
receiptLen, balance := 2, big.NewInt(2000)
295309
if len(task.receipts) != receiptLen {
296-
t.Errorf("receipt number mismatch has %d, want %d", len(task.receipts), receiptLen)
310+
t.Errorf("receipt number mismatch: have %d, want %d", len(task.receipts), receiptLen)
297311
}
298-
if task.state.GetBalance(acc1Addr).Cmp(balance) != 0 {
299-
t.Errorf("account balance mismatch has %d, want %d", task.state.GetBalance(acc1Addr), balance)
312+
if task.state.GetBalance(testUserAddress).Cmp(balance) != 0 {
313+
t.Errorf("account balance mismatch: have %d, want %d", task.state.GetBalance(testUserAddress), balance)
300314
}
301315
}
302316
taskCh <- struct{}{}
@@ -347,7 +361,7 @@ func TestAdjustIntervalClique(t *testing.T) {
347361
func testAdjustInterval(t *testing.T, chainConfig *params.ChainConfig, engine consensus.Engine) {
348362
defer engine.Close()
349363

350-
w, _ := newTestWorker(t, chainConfig, engine)
364+
w, _ := newTestWorker(t, chainConfig, engine, 0)
351365
defer w.close()
352366

353367
w.skipSealHook = func(task *task) bool {
@@ -387,10 +401,10 @@ func testAdjustInterval(t *testing.T, chainConfig *params.ChainConfig, engine co
387401

388402
// Check interval
389403
if minInterval != wantMinInterval {
390-
t.Errorf("resubmit min interval mismatch want %s has %s", wantMinInterval, minInterval)
404+
t.Errorf("resubmit min interval mismatch: have %v, want %v ", minInterval, wantMinInterval)
391405
}
392406
if recommitInterval != wantRecommitInterval {
393-
t.Errorf("resubmit interval mismatch want %s has %s", wantRecommitInterval, recommitInterval)
407+
t.Errorf("resubmit interval mismatch: have %v, want %v", recommitInterval, wantRecommitInterval)
394408
}
395409
result = append(result, float64(recommitInterval.Nanoseconds()))
396410
index += 1

0 commit comments

Comments
 (0)