miner: rename pending to unconformed, add bounds and ops tests

3 files changed, 117 insertions, 30 deletions

diff --git a/miner/pending.go b/miner/unconfirmed.go
index e4668d8d9..86a30de35 100644
--- a/miner/pending.go
+++ b/miner/unconfirmed.go
@@ -21,45 +21,52 @@ import (
 	"sync"
 
 	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/core"
+	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/logger"
 	"github.com/ethereum/go-ethereum/logger/glog"
 )
 
-// pendingBlock is a small collection of metadata about a locally mined block
-// that is placed into a pending set for canonical chain inclusion tracking.
-type pendingBlock struct {
+// headerRetriever is used by the unconfirmed block set to verify whether a previously
+// mined block is part of the canonical chain or not.
+type headerRetriever interface {
+	// GetHeaderByNumber retrieves the canonical header associated with a block number.
+	GetHeaderByNumber(number uint64) *types.Header
+}
+
+// unconfirmedBlock is a small collection of metadata about a locally mined block
+// that is placed into a unconfirmed set for canonical chain inclusion tracking.
+type unconfirmedBlock struct {
 	index uint64
 	hash  common.Hash
 }
 
-// pendingBlockSet implements a data structure to maintain locally mined blocks
+// unconfirmedBlocks implements a data structure to maintain locally mined blocks
 // have have not yet reached enough maturity to guarantee chain inclusion. It is
 // used by the miner to provide logs to the user when a previously mined block
 // has a high enough guarantee to not be reorged out of te canonical chain.
-type pendingBlockSet struct {
-	chain  *core.BlockChain // Blockchain to verify canonical status through
-	depth  uint             // Depth after which to discard previous blocks
-	blocks *ring.Ring       // Block infos to allow canonical chain cross checks
-	lock   sync.RWMutex     // Protects the fields from concurrent access
+type unconfirmedBlocks struct {
+	chain  headerRetriever // Blockchain to verify canonical status through
+	depth  uint            // Depth after which to discard previous blocks
+	blocks *ring.Ring      // Block infos to allow canonical chain cross checks
+	lock   sync.RWMutex    // Protects the fields from concurrent access
 }
 
-// newPendingBlockSet returns new data structure to track currently pending blocks.
-func newPendingBlockSet(chain *core.BlockChain, depth uint) *pendingBlockSet {
-	return &pendingBlockSet{
+// newUnconfirmedBlocks returns new data structure to track currently unconfirmed blocks.
+func newUnconfirmedBlocks(chain headerRetriever, depth uint) *unconfirmedBlocks {
+	return &unconfirmedBlocks{
 		chain: chain,
 		depth: depth,
 	}
 }
 
-// Insert adds a new block to the set of pending ones.
-func (set *pendingBlockSet) Insert(index uint64, hash common.Hash) {
+// Insert adds a new block to the set of unconfirmed ones.
+func (set *unconfirmedBlocks) Insert(index uint64, hash common.Hash) {
 	// If a new block was mined locally, shift out any old enough blocks
 	set.Shift(index)
 
 	// Create the new item as its own ring
 	item := ring.New(1)
-	item.Value = &pendingBlock{
+	item.Value = &unconfirmedBlock{
 		index: index,
 		hash:  hash,
 	}
@@ -72,25 +79,20 @@ func (set *pendingBlockSet) Insert(index uint64, hash common.Hash) {
 	} else {
 		set.blocks.Move(-1).Link(item)
 	}
-	// Display a log for the user to notify of a new mined block pending
+	// Display a log for the user to notify of a new mined block unconfirmed
 	glog.V(logger.Info).Infof("🔨  mined potential block #%d [%x…], waiting for %d blocks to confirm", index, hash.Bytes()[:4], set.depth)
 }
 
-// Shift drops all pending blocks from the set which exceed the pending sets depth
+// Shift drops all unconfirmed blocks from the set which exceed the unconfirmed sets depth
 // allowance, checking them against the canonical chain for inclusion or staleness
 // report.
-func (set *pendingBlockSet) Shift(height uint64) {
+func (set *unconfirmedBlocks) Shift(height uint64) {
 	set.lock.Lock()
 	defer set.lock.Unlock()
 
-	// Short circuit if there are no pending blocks to shift
-	if set.blocks == nil {
-		return
-	}
-	// Otherwise shift all blocks below the depth allowance
 	for set.blocks != nil {
-		// Retrieve the next pending block and abort if too fresh
-		next := set.blocks.Value.(*pendingBlock)
+		// Retrieve the next unconfirmed block and abort if too fresh
+		next := set.blocks.Value.(*unconfirmedBlock)
 		if next.index+uint64(set.depth) > height {
 			break
 		}
diff --git a/miner/unconfirmed_test.go b/miner/unconfirmed_test.go
new file mode 100644
index 000000000..456af1764
--- /dev/null
+++ b/miner/unconfirmed_test.go
@@ -0,0 +1,85 @@
+// Copyright 2016 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+package miner
+
+import (
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/types"
+)
+
+// noopHeaderRetriever is an implementation of headerRetriever that always
+// returns nil for any requested headers.
+type noopHeaderRetriever struct{}
+
+func (r *noopHeaderRetriever) GetHeaderByNumber(number uint64) *types.Header {
+	return nil
+}
+
+// Tests that inserting blocks into the unconfirmed set accumulates them until
+// the desired depth is reached, after which they begin to be dropped.
+func TestUnconfirmedInsertBounds(t *testing.T) {
+	limit := uint(10)
+
+	pool := newUnconfirmedBlocks(new(noopHeaderRetriever), limit)
+	for depth := uint64(0); depth < 2*uint64(limit); depth++ {
+		// Insert multiple blocks for the same level just to stress it
+		for i := 0; i < int(depth); i++ {
+			pool.Insert(depth, common.Hash([32]byte{byte(depth), byte(i)}))
+		}
+		// Validate that no blocks below the depth allowance are left in
+		pool.blocks.Do(func(block interface{}) {
+			if block := block.(*unconfirmedBlock); block.index+uint64(limit) <= depth {
+				t.Errorf("depth %d: block %x not dropped", depth, block.hash)
+			}
+		})
+	}
+}
+
+// Tests that shifting blocks out of the unconfirmed set works both for normal
+// cases as well as for corner cases such as empty sets, empty shifts or full
+// shifts.
+func TestUnconfirmedShifts(t *testing.T) {
+	// Create a pool with a few blocks on various depths
+	limit, start := uint(10), uint64(25)
+
+	pool := newUnconfirmedBlocks(new(noopHeaderRetriever), limit)
+	for depth := start; depth < start+uint64(limit); depth++ {
+		pool.Insert(depth, common.Hash([32]byte{byte(depth)}))
+	}
+	// Try to shift below the limit and ensure no blocks are dropped
+	pool.Shift(start + uint64(limit) - 1)
+	if n := pool.blocks.Len(); n != int(limit) {
+		t.Errorf("unconfirmed count mismatch: have %d, want %d", n, limit)
+	}
+	// Try to shift half the blocks out and verify remainder
+	pool.Shift(start + uint64(limit) - 1 + uint64(limit/2))
+	if n := pool.blocks.Len(); n != int(limit)/2 {
+		t.Errorf("unconfirmed count mismatch: have %d, want %d", n, limit/2)
+	}
+	// Try to shift all the remaining blocks out and verify emptyness
+	pool.Shift(start + 2*uint64(limit))
+	if n := pool.blocks.Len(); n != 0 {
+		t.Errorf("unconfirmed count mismatch: have %d, want %d", n, 0)
+	}
+	// Try to shift out from the empty set and make sure it doesn't break
+	pool.Shift(start + 3*uint64(limit))
+	if n := pool.blocks.Len(); n != 0 {
+		t.Errorf("unconfirmed count mismatch: have %d, want %d", n, 0)
+	}
+}
diff --git a/miner/worker.go b/miner/worker.go
index 4f4a6c437..f29566c0a 100644
--- a/miner/worker.go
+++ b/miner/worker.go
@@ -117,7 +117,7 @@ type worker struct {
 	txQueueMu sync.Mutex
 	txQueue   map[common.Hash]*types.Transaction
 
-	minedBlocks *pendingBlockSet // set of locally mined blocks pending canonicalness confirmations
+	unconfirmed *unconfirmedBlocks // set of locally mined blocks pending canonicalness confirmations
 
 	// atomic status counters
 	mining int32
@@ -140,7 +140,7 @@ func newWorker(config *params.ChainConfig, coinbase common.Address, eth Backend,
 		coinbase:       coinbase,
 		txQueue:        make(map[common.Hash]*types.Transaction),
 		agents:         make(map[Agent]struct{}),
-		minedBlocks:    newPendingBlockSet(eth.BlockChain(), 5),
+		unconfirmed:    newUnconfirmedBlocks(eth.BlockChain(), 5),
 		fullValidation: false,
 	}
 	worker.events = worker.mux.Subscribe(core.ChainHeadEvent{}, core.ChainSideEvent{}, core.TxPreEvent{})
@@ -341,7 +341,7 @@ func (self *worker) wait() {
 				}(block, work.state.Logs(), work.receipts)
 			}
 			// Insert the block into the set of pending ones to wait for confirmations
-			self.minedBlocks.Insert(block.NumberU64(), block.Hash())
+			self.unconfirmed.Insert(block.NumberU64(), block.Hash())
 
 			if mustCommitNewWork {
 				self.commitNewWork()
@@ -514,7 +514,7 @@ func (self *worker) commitNewWork() {
 	// We only care about logging if we're actually mining.
 	if atomic.LoadInt32(&self.mining) == 1 {
 		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))
-		self.minedBlocks.Shift(work.Block.NumberU64() - 1)
+		self.unconfirmed.Shift(work.Block.NumberU64() - 1)
 	}
 	self.push(work)
 }