Skip to content

Commit b792412

Browse files
karalabekaralabe
authored
Merge pull request #3430 from karalabe/miner-pending-race
miner: clean up unconfirmed mined block tracking
2 parents 49c6f10 + 07311f3 commit b792412

File tree

3 files changed

+217
-74
lines changed

3 files changed

+217
-74
lines changed

‎miner/unconfirmed.go‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
// Copyright 2016 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+
"container/ring"
21+
"sync"
22+
23+
"github.com/ethereum/go-ethereum/common"
24+
"github.com/ethereum/go-ethereum/core/types"
25+
"github.com/ethereum/go-ethereum/logger"
26+
"github.com/ethereum/go-ethereum/logger/glog"
27+
)
28+
29+
// headerRetriever is used by the unconfirmed block set to verify whether a previously
30+
// mined block is part of the canonical chain or not.
31+
type headerRetriever interface {
32+
// GetHeaderByNumber retrieves the canonical header associated with a block number.
33+
GetHeaderByNumber(number uint64) *types.Header
34+
}
35+
36+
// unconfirmedBlock is a small collection of metadata about a locally mined block
37+
// that is placed into a unconfirmed set for canonical chain inclusion tracking.
38+
type unconfirmedBlock struct {
39+
index uint64
40+
hash common.Hash
41+
}
42+
43+
// unconfirmedBlocks implements a data structure to maintain locally mined blocks
44+
// have have not yet reached enough maturity to guarantee chain inclusion. It is
45+
// used by the miner to provide logs to the user when a previously mined block
46+
// has a high enough guarantee to not be reorged out of te canonical chain.
47+
type unconfirmedBlocks struct {
48+
chain headerRetriever // Blockchain to verify canonical status through
49+
depth uint // Depth after which to discard previous blocks
50+
blocks *ring.Ring // Block infos to allow canonical chain cross checks
51+
lock sync.RWMutex // Protects the fields from concurrent access
52+
}
53+
54+
// newUnconfirmedBlocks returns new data structure to track currently unconfirmed blocks.
55+
func newUnconfirmedBlocks(chain headerRetriever, depth uint) *unconfirmedBlocks {
56+
return &unconfirmedBlocks{
57+
chain: chain,
58+
depth: depth,
59+
}
60+
}
61+
62+
// Insert adds a new block to the set of unconfirmed ones.
63+
func (set *unconfirmedBlocks) Insert(index uint64, hash common.Hash) {
64+
// If a new block was mined locally, shift out any old enough blocks
65+
set.Shift(index)
66+
67+
// Create the new item as its own ring
68+
item := ring.New(1)
69+
item.Value = &unconfirmedBlock{
70+
index: index,
71+
hash: hash,
72+
}
73+
// Set as the initial ring or append to the end
74+
set.lock.Lock()
75+
defer set.lock.Unlock()
76+
77+
if set.blocks == nil {
78+
set.blocks = item
79+
} else {
80+
set.blocks.Move(-1).Link(item)
81+
}
82+
// Display a log for the user to notify of a new mined block unconfirmed
83+
glog.V(logger.Info).Infof("🔨 mined potential block #%d [%x…], waiting for %d blocks to confirm", index, hash.Bytes()[:4], set.depth)
84+
}
85+
86+
// Shift drops all unconfirmed blocks from the set which exceed the unconfirmed sets depth
87+
// allowance, checking them against the canonical chain for inclusion or staleness
88+
// report.
89+
func (set *unconfirmedBlocks) Shift(height uint64) {
90+
set.lock.Lock()
91+
defer set.lock.Unlock()
92+
93+
for set.blocks != nil {
94+
// Retrieve the next unconfirmed block and abort if too fresh
95+
next := set.blocks.Value.(*unconfirmedBlock)
96+
if next.index+uint64(set.depth) > height {
97+
break
98+
}
99+
// Block seems to exceed depth allowance, check for canonical status
100+
header := set.chain.GetHeaderByNumber(next.index)
101+
switch {
102+
case header == nil:
103+
glog.V(logger.Warn).Infof("failed to retrieve header of mined block #%d [%x…]", next.index, next.hash.Bytes()[:4])
104+
case header.Hash() == next.hash:
105+
glog.V(logger.Info).Infof("🔗 mined block #%d [%x…] reached canonical chain", next.index, next.hash.Bytes()[:4])
106+
default:
107+
glog.V(logger.Info).Infof("⑂ mined block #%d [%x…] became a side fork", next.index, next.hash.Bytes()[:4])
108+
}
109+
// Drop the block out of the ring
110+
if set.blocks.Value == set.blocks.Next().Value {
111+
set.blocks = nil
112+
} else {
113+
set.blocks = set.blocks.Move(-1)
114+
set.blocks.Unlink(1)
115+
set.blocks = set.blocks.Move(1)
116+
}
117+
}
118+
}

‎miner/unconfirmed_test.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// Copyright 2016 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+
"testing"
21+
22+
"github.com/ethereum/go-ethereum/common"
23+
"github.com/ethereum/go-ethereum/core/types"
24+
)
25+
26+
// noopHeaderRetriever is an implementation of headerRetriever that always
27+
// returns nil for any requested headers.
28+
type noopHeaderRetriever struct{}
29+
30+
func (r *noopHeaderRetriever) GetHeaderByNumber(number uint64) *types.Header {
31+
return nil
32+
}
33+
34+
// Tests that inserting blocks into the unconfirmed set accumulates them until
35+
// the desired depth is reached, after which they begin to be dropped.
36+
func TestUnconfirmedInsertBounds(t *testing.T) {
37+
limit := uint(10)
38+
39+
pool := newUnconfirmedBlocks(new(noopHeaderRetriever), limit)
40+
for depth := uint64(0); depth < 2*uint64(limit); depth++ {
41+
// Insert multiple blocks for the same level just to stress it
42+
for i := 0; i < int(depth); i++ {
43+
pool.Insert(depth, common.Hash([32]byte{byte(depth), byte(i)}))
44+
}
45+
// Validate that no blocks below the depth allowance are left in
46+
pool.blocks.Do(func(block interface{}) {
47+
if block := block.(*unconfirmedBlock); block.index+uint64(limit) <= depth {
48+
t.Errorf("depth %d: block %x not dropped", depth, block.hash)
49+
}
50+
})
51+
}
52+
}
53+
54+
// Tests that shifting blocks out of the unconfirmed set works both for normal
55+
// cases as well as for corner cases such as empty sets, empty shifts or full
56+
// shifts.
57+
func TestUnconfirmedShifts(t *testing.T) {
58+
// Create a pool with a few blocks on various depths
59+
limit, start := uint(10), uint64(25)
60+
61+
pool := newUnconfirmedBlocks(new(noopHeaderRetriever), limit)
62+
for depth := start; depth < start+uint64(limit); depth++ {
63+
pool.Insert(depth, common.Hash([32]byte{byte(depth)}))
64+
}
65+
// Try to shift below the limit and ensure no blocks are dropped
66+
pool.Shift(start + uint64(limit) - 1)
67+
if n := pool.blocks.Len(); n != int(limit) {
68+
t.Errorf("unconfirmed count mismatch: have %d, want %d", n, limit)
69+
}
70+
// Try to shift half the blocks out and verify remainder
71+
pool.Shift(start + uint64(limit) - 1 + uint64(limit/2))
72+
if n := pool.blocks.Len(); n != int(limit)/2 {
73+
t.Errorf("unconfirmed count mismatch: have %d, want %d", n, limit/2)
74+
}
75+
// Try to shift all the remaining blocks out and verify emptyness
76+
pool.Shift(start + 2*uint64(limit))
77+
if n := pool.blocks.Len(); n != 0 {
78+
t.Errorf("unconfirmed count mismatch: have %d, want %d", n, 0)
79+
}
80+
// Try to shift out from the empty set and make sure it doesn't break
81+
pool.Shift(start + 3*uint64(limit))
82+
if n := pool.blocks.Len(); n != 0 {
83+
t.Errorf("unconfirmed count mismatch: have %d, want %d", n, 0)
84+
}
85+
}

‎miner/worker.go‎

Lines changed: 14 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -55,26 +55,20 @@ type Agent interface {
5555
GetHashRate() int64
5656
}
5757

58-
type uint64RingBuffer struct {
59-
ints []uint64 //array of all integers in buffer
60-
next int //where is the next insertion? assert 0 <= next < len(ints)
61-
}
62-
6358
// Work is the workers current environment and holds
6459
// all of the current state information
6560
type Work struct {
6661
config *params.ChainConfig
6762
signer types.Signer
6863

69-
state *state.StateDB // apply state changes here
70-
ancestors *set.Set // ancestor set (used for checking uncle parent validity)
71-
family *set.Set // family set (used for checking uncle invalidity)
72-
uncles *set.Set // uncle set
73-
tcount int // tx count in cycle
74-
ownedAccounts *set.Set
75-
lowGasTxs types.Transactions
76-
failedTxs types.Transactions
77-
localMinedBlocks *uint64RingBuffer // the most recent block numbers that were mined locally (used to check block inclusion)
64+
state *state.StateDB // apply state changes here
65+
ancestors *set.Set // ancestor set (used for checking uncle parent validity)
66+
family *set.Set // family set (used for checking uncle invalidity)
67+
uncles *set.Set // uncle set
68+
tcount int // tx count in cycle
69+
ownedAccounts *set.Set
70+
lowGasTxs types.Transactions
71+
failedTxs types.Transactions
7872

7973
Block *types.Block // the new block
8074

@@ -123,6 +117,8 @@ type worker struct {
123117
txQueueMu sync.Mutex
124118
txQueue map[common.Hash]*types.Transaction
125119

120+
unconfirmed *unconfirmedBlocks // set of locally mined blocks pending canonicalness confirmations
121+
126122
// atomic status counters
127123
mining int32
128124
atWork int32
@@ -144,6 +140,7 @@ func newWorker(config *params.ChainConfig, coinbase common.Address, eth Backend,
144140
coinbase: coinbase,
145141
txQueue: make(map[common.Hash]*types.Transaction),
146142
agents: make(map[Agent]struct{}),
143+
unconfirmed: newUnconfirmedBlocks(eth.BlockChain(), 5),
147144
fullValidation: false,
148145
}
149146
worker.events = worker.mux.Subscribe(core.ChainHeadEvent{}, core.ChainSideEvent{}, core.TxPreEvent{})
@@ -269,18 +266,6 @@ func (self *worker) update() {
269266
}
270267
}
271268

272-
func newLocalMinedBlock(blockNumber uint64, prevMinedBlocks *uint64RingBuffer) (minedBlocks *uint64RingBuffer) {
273-
if prevMinedBlocks == nil {
274-
minedBlocks = &uint64RingBuffer{next: 0, ints: make([]uint64, miningLogAtDepth+1)}
275-
} else {
276-
minedBlocks = prevMinedBlocks
277-
}
278-
279-
minedBlocks.ints[minedBlocks.next] = blockNumber
280-
minedBlocks.next = (minedBlocks.next + 1) % len(minedBlocks.ints)
281-
return minedBlocks
282-
}
283-
284269
func (self *worker) wait() {
285270
for {
286271
mustCommitNewWork := true
@@ -355,17 +340,8 @@ func (self *worker) wait() {
355340
}
356341
}(block, work.state.Logs(), work.receipts)
357342
}
358-
359-
// check staleness and display confirmation
360-
var stale, confirm string
361-
canonBlock := self.chain.GetBlockByNumber(block.NumberU64())
362-
if canonBlock != nil && canonBlock.Hash() != block.Hash() {
363-
stale = "stale "
364-
} else {
365-
confirm = "Wait 5 blocks for confirmation"
366-
work.localMinedBlocks = newLocalMinedBlock(block.Number().Uint64(), work.localMinedBlocks)
367-
}
368-
glog.V(logger.Info).Infof("🔨 Mined %sblock (#%v / %x). %s", stale, block.Number(), block.Hash().Bytes()[:4], confirm)
343+
// Insert the block into the set of pending ones to wait for confirmations
344+
self.unconfirmed.Insert(block.NumberU64(), block.Hash())
369345

370346
if mustCommitNewWork {
371347
self.commitNewWork()
@@ -417,9 +393,6 @@ func (self *worker) makeCurrent(parent *types.Block, header *types.Header) error
417393
// Keep track of transactions which return errors so they can be removed
418394
work.tcount = 0
419395
work.ownedAccounts = accountAddressesSet(accounts)
420-
if self.current != nil {
421-
work.localMinedBlocks = self.current.localMinedBlocks
422-
}
423396
self.current = work
424397
return nil
425398
}
@@ -435,38 +408,6 @@ func (w *worker) setGasPrice(p *big.Int) {
435408
w.mux.Post(core.GasPriceChanged{Price: w.gasPrice})
436409
}
437410

438-
func (self *worker) isBlockLocallyMined(current *Work, deepBlockNum uint64) bool {
439-
//Did this instance mine a block at {deepBlockNum} ?
440-
var isLocal = false
441-
for idx, blockNum := range current.localMinedBlocks.ints {
442-
if deepBlockNum == blockNum {
443-
isLocal = true
444-
current.localMinedBlocks.ints[idx] = 0 //prevent showing duplicate logs
445-
break
446-
}
447-
}
448-
//Short-circuit on false, because the previous and following tests must both be true
449-
if !isLocal {
450-
return false
451-
}
452-
453-
//Does the block at {deepBlockNum} send earnings to my coinbase?
454-
var block = self.chain.GetBlockByNumber(deepBlockNum)
455-
return block != nil && block.Coinbase() == self.coinbase
456-
}
457-
458-
func (self *worker) logLocalMinedBlocks(current, previous *Work) {
459-
if previous != nil && current.localMinedBlocks != nil {
460-
nextBlockNum := current.Block.NumberU64()
461-
for checkBlockNum := previous.Block.NumberU64(); checkBlockNum < nextBlockNum; checkBlockNum++ {
462-
inspectBlockNum := checkBlockNum - miningLogAtDepth
463-
if self.isBlockLocallyMined(current, inspectBlockNum) {
464-
glog.V(logger.Info).Infof("🔨 🔗 Mined %d blocks back: block #%v", miningLogAtDepth, inspectBlockNum)
465-
}
466-
}
467-
}
468-
}
469-
470411
func (self *worker) commitNewWork() {
471412
self.mu.Lock()
472413
defer self.mu.Unlock()
@@ -513,7 +454,6 @@ func (self *worker) commitNewWork() {
513454
}
514455
}
515456
}
516-
previous := self.current
517457
// Could potentially happen if starting to mine in an odd state.
518458
err := self.makeCurrent(parent, header)
519459
if err != nil {
@@ -574,7 +514,7 @@ func (self *worker) commitNewWork() {
574514
// We only care about logging if we're actually mining.
575515
if atomic.LoadInt32(&self.mining) == 1 {
576516
glog.V(logger.Info).Infof("commit new work on block %v with %d txs & %d uncles. Took %v\n", work.Block.Number(), work.tcount, len(uncles), time.Since(tstart))
577-
self.logLocalMinedBlocks(work, previous)
517+
self.unconfirmed.Shift(work.Block.NumberU64() - 1)
578518
}
579519
self.push(work)
580520
}

0 commit comments

Comments
 (0)