eth: rework tx fetcher to use O(1) ops + manage network requests

core/tx_pool.go

@@ -18,7 +18,6 @@ package core
 
 import (
 	"errors"
-	"fmt"
 	"math"
 	"math/big"
 	"sort"
@@ -53,6 +52,10 @@ const (
 )
 
 var (
+	// ErrAlreadyKnown is returned if the transactions is already contained
+	// within the pool.
+	ErrAlreadyKnown = errors.New("already known")
+
 	// ErrInvalidSender is returned if the transaction contains an invalid signature.
 	ErrInvalidSender = errors.New("invalid sender")
 
@@ -579,7 +582,7 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e
 	if pool.all.Get(hash) != nil {
 		log.Trace("Discarding already known transaction", "hash", hash)
 		knownTxMeter.Mark(1)
-		return false, fmt.Errorf("known transaction: %x", hash)
+		return false, ErrAlreadyKnown
 	}
 	// If the transaction fails basic validation, discard it
 	if err := pool.validateTx(tx, local); err != nil {
@@ -786,7 +789,7 @@ func (pool *TxPool) addTxs(txs []*types.Transaction, local, sync bool) []error {
 	for i, tx := range txs {
 		// If the transaction is known, pre-set the error slot
 		if pool.all.Get(tx.Hash()) != nil {
-			errs[i] = fmt.Errorf("known transaction: %x", tx.Hash())
+			errs[i] = ErrAlreadyKnown
 			knownTxMeter.Mark(1)
 			continue
 		}

eth/fetcher/block_fetcher.go

@@ -27,6 +27,7 @@ import (
 	"github.com/ethereum/go-ethereum/consensus"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/metrics"
 )
 
 const (
@@ -42,6 +43,26 @@ const (
 	blockLimit   = 64  // Maximum number of unique blocks a peer may have delivered
 )
 
+var (
+	blockAnnounceInMeter   = metrics.NewRegisteredMeter("eth/fetcher/block/announces/in", nil)
+	blockAnnounceOutTimer  = metrics.NewRegisteredTimer("eth/fetcher/block/announces/out", nil)
+	blockAnnounceDropMeter = metrics.NewRegisteredMeter("eth/fetcher/block/announces/drop", nil)
+	blockAnnounceDOSMeter  = metrics.NewRegisteredMeter("eth/fetcher/block/announces/dos", nil)
+
+	blockBroadcastInMeter   = metrics.NewRegisteredMeter("eth/fetcher/block/broadcasts/in", nil)
+	blockBroadcastOutTimer  = metrics.NewRegisteredTimer("eth/fetcher/block/broadcasts/out", nil)
+	blockBroadcastDropMeter = metrics.NewRegisteredMeter("eth/fetcher/block/broadcasts/drop", nil)
+	blockBroadcastDOSMeter  = metrics.NewRegisteredMeter("eth/fetcher/block/broadcasts/dos", nil)
+
+	headerFetchMeter = metrics.NewRegisteredMeter("eth/fetcher/block/headers", nil)
+	bodyFetchMeter   = metrics.NewRegisteredMeter("eth/fetcher/block/bodies", nil)
+
+	headerFilterInMeter  = metrics.NewRegisteredMeter("eth/fetcher/block/filter/headers/in", nil)
+	headerFilterOutMeter = metrics.NewRegisteredMeter("eth/fetcher/block/filter/headers/out", nil)
+	bodyFilterInMeter    = metrics.NewRegisteredMeter("eth/fetcher/block/filter/bodies/in", nil)
+	bodyFilterOutMeter   = metrics.NewRegisteredMeter("eth/fetcher/block/filter/bodies/out", nil)
+)
+
 var (
 	errTerminated = errors.New("terminated")
 )

eth/fetcher/metrics.go

-// Copyright 2015 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/>.
-
-// Contains the metrics collected by the fetcher.
-
-package fetcher
-
-import (
-	"github.com/ethereum/go-ethereum/metrics"
-)
-
-var (
-	blockAnnounceInMeter   = metrics.NewRegisteredMeter("eth/fetcher/prop/block/announces/in", nil)
-	blockAnnounceOutTimer  = metrics.NewRegisteredTimer("eth/fetcher/prop/block/announces/out", nil)
-	blockAnnounceDropMeter = metrics.NewRegisteredMeter("eth/fetcher/prop/block/announces/drop", nil)
-	blockAnnounceDOSMeter  = metrics.NewRegisteredMeter("eth/fetcher/prop/block/announces/dos", nil)
-
-	blockBroadcastInMeter   = metrics.NewRegisteredMeter("eth/fetcher/prop/block/broadcasts/in", nil)
-	blockBroadcastOutTimer  = metrics.NewRegisteredTimer("eth/fetcher/prop/block/broadcasts/out", nil)
-	blockBroadcastDropMeter = metrics.NewRegisteredMeter("eth/fetcher/prop/block/broadcasts/drop", nil)
-	blockBroadcastDOSMeter  = metrics.NewRegisteredMeter("eth/fetcher/prop/block/broadcasts/dos", nil)
-
-	headerFetchMeter = metrics.NewRegisteredMeter("eth/fetcher/fetch/headers", nil)
-	bodyFetchMeter   = metrics.NewRegisteredMeter("eth/fetcher/fetch/bodies", nil)
-
-	headerFilterInMeter  = metrics.NewRegisteredMeter("eth/fetcher/filter/headers/in", nil)
-	headerFilterOutMeter = metrics.NewRegisteredMeter("eth/fetcher/filter/headers/out", nil)
-	bodyFilterInMeter    = metrics.NewRegisteredMeter("eth/fetcher/filter/bodies/in", nil)
-	bodyFilterOutMeter   = metrics.NewRegisteredMeter("eth/fetcher/filter/bodies/out", nil)
-
-	txAnnounceInMeter         = metrics.NewRegisteredMeter("eth/fetcher/prop/transaction/announces/in", nil)
-	txAnnounceDOSMeter        = metrics.NewRegisteredMeter("eth/fetcher/prop/transaction/announces/dos", nil)
-	txAnnounceSkipMeter       = metrics.NewRegisteredMeter("eth/fetcher/prop/transaction/announces/skip", nil)
-	txAnnounceUnderpriceMeter = metrics.NewRegisteredMeter("eth/fetcher/prop/transaction/announces/underprice", nil)
-	txBroadcastInMeter        = metrics.NewRegisteredMeter("eth/fetcher/prop/transaction/broadcasts/in", nil)
-	txFetchOutMeter           = metrics.NewRegisteredMeter("eth/fetcher/fetch/transaction/out", nil)
-	txFetchSuccessMeter       = metrics.NewRegisteredMeter("eth/fetcher/fetch/transaction/success", nil)
-	txFetchTimeoutMeter       = metrics.NewRegisteredMeter("eth/fetcher/fetch/transaction/timeout", nil)
-	txFetchInvalidMeter       = metrics.NewRegisteredMeter("eth/fetcher/fetch/transaction/invalid", nil)
-	txFetchDurationTimer      = metrics.NewRegisteredTimer("eth/fetcher/fetch/transaction/duration", nil)
-)

eth/fetcher/tx_fetcher.go

@@ -17,108 +17,236 @@
 package fetcher
 
 import (
-	"math/rand"
+	"bytes"
+	"fmt"
+	mrand "math/rand"
+	"sort"
 	"time"
 
 	mapset "github.com/deckarep/golang-set"
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/mclock"
 	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/metrics"
 )
 
-var (
-	// txAnnounceLimit is the maximum number of unique transaction a peer
+const (
+	// maxTxAnnounces is the maximum number of unique transaction a peer
 	// can announce in a short time.
-	txAnnounceLimit = 4096
+	maxTxAnnounces = 4096
+
+	// maxTxRetrievals is the maximum transaction number can be fetched in one
+	// request. The rationale to pick 256 is:
+	//   - In eth protocol, the softResponseLimit is 2MB. Nowadays according to
+	//     Etherscan the average transaction size is around 200B, so in theory
+	//     we can include lots of transaction in a single protocol packet.
+	//   - However the maximum size of a single transaction is raised to 128KB,
+	//     so pick a middle value here to ensure we can maximize the efficiency
+	//     of the retrieval and response size overflow won't happen in most cases.
+	maxTxRetrievals = 256
+
+	// maxTxUnderpricedSetSize is the size of the underpriced transaction set that
+	// is used to track recent transactions that have been dropped so we don't
+	// re-request them.
+	maxTxUnderpricedSetSize = 32768
+
+	// txArriveTimeout is the time allowance before an announced transaction is
+	// explicitly requested.
+	txArriveTimeout = 500 * time.Millisecond
+
+	// txGatherSlack is the interval used to collate almost-expired announces
+	// with network fetches.
+	txGatherSlack = 100 * time.Millisecond
+)
 
+var (
 	// txFetchTimeout is the maximum allotted time to return an explicitly
 	// requested transaction.
 	txFetchTimeout = 5 * time.Second
+)
 
-	// MaxTransactionFetch is the maximum transaction number can be fetched
-	// in one request. The rationale to pick this value is:
-	// In eth protocol, the softResponseLimit is 2MB. Nowdays according to
-	// Etherscan the average transaction size is around 200B, so in theory
-	// we can include lots of transaction in a single protocol packet. However
-	// the maximum size of a single transaction is raised to 128KB, so pick
-	// a middle value here to ensure we can maximize the efficiency of the
-	// retrieval and response size overflow won't happen in most cases.
-	MaxTransactionFetch = 256
-
-	// underpriceSetSize is the size of underprice set which used for maintaining
-	// the set of underprice transactions.
-	underpriceSetSize = 4096
+var (
+	txAnnounceInMeter          = metrics.NewRegisteredMeter("eth/fetcher/transaction/announces/in", nil)
+	txAnnounceKnownMeter       = metrics.NewRegisteredMeter("eth/fetcher/transaction/announces/known", nil)
+	txAnnounceUnderpricedMeter = metrics.NewRegisteredMeter("eth/fetcher/transaction/announces/underpriced", nil)
+	txAnnounceDOSMeter         = metrics.NewRegisteredMeter("eth/fetcher/transaction/announces/dos", nil)
+
+	txBroadcastInMeter          = metrics.NewRegisteredMeter("eth/fetcher/transaction/broadcasts/in", nil)
+	txBroadcastKnownMeter       = metrics.NewRegisteredMeter("eth/fetcher/transaction/broadcasts/known", nil)
+	txBroadcastUnderpricedMeter = metrics.NewRegisteredMeter("eth/fetcher/transaction/broadcasts/underpriced", nil)
+	txBroadcastOtherRejectMeter = metrics.NewRegisteredMeter("eth/fetcher/transaction/broadcasts/otherreject", nil)
+
+	txRequestOutMeter     = metrics.NewRegisteredMeter("eth/fetcher/transaction/request/out", nil)
+	txRequestFailMeter    = metrics.NewRegisteredMeter("eth/fetcher/transaction/request/fail", nil)
+	txRequestDoneMeter    = metrics.NewRegisteredMeter("eth/fetcher/transaction/request/done", nil)
+	txRequestTimeoutMeter = metrics.NewRegisteredMeter("eth/fetcher/transaction/request/timeout", nil)
+
+	txReplyInMeter          = metrics.NewRegisteredMeter("eth/fetcher/transaction/replies/in", nil)
+	txReplyKnownMeter       = metrics.NewRegisteredMeter("eth/fetcher/transaction/replies/known", nil)
+	txReplyUnderpricedMeter = metrics.NewRegisteredMeter("eth/fetcher/transaction/replies/underpriced", nil)
+	txReplyOtherRejectMeter = metrics.NewRegisteredMeter("eth/fetcher/transaction/replies/otherreject", nil)
+
+	txFetcherWaitingPeers   = metrics.NewRegisteredGauge("eth/fetcher/transaction/waiting/peers", nil)
+	txFetcherWaitingHashes  = metrics.NewRegisteredGauge("eth/fetcher/transaction/waiting/hashes", nil)
+	txFetcherQueueingPeers  = metrics.NewRegisteredGauge("eth/fetcher/transaction/queueing/peers", nil)
+	txFetcherQueueingHashes = metrics.NewRegisteredGauge("eth/fetcher/transaction/queueing/hashes", nil)
+	txFetcherFetchingPeers  = metrics.NewRegisteredGauge("eth/fetcher/transaction/fetching/peers", nil)
+	txFetcherFetchingHashes = metrics.NewRegisteredGauge("eth/fetcher/transaction/fetching/hashes", nil)
 )
 
-// txAnnounce is the notification of the availability of a single
-// new transaction in the network.
+// txAnnounce is the notification of the availability of a batch
+// of new transactions in the network.
 type txAnnounce struct {
 	origin string        // Identifier of the peer originating the notification
-	time     time.Time           // Timestamp of the announcement
-	fetchTxs func([]common.Hash) // Callback for retrieving transaction from specified peer
+	hashes []common.Hash // Batch of transaction hashes being announced
 }
 
-// txsAnnounce is the notification of the availability of a batch
-// of new transactions in the network.
-type txsAnnounce struct {
-	hashes   []common.Hash       // Batch of transaction hashes being announced
+// txRequest represents an in-flight transaction retrieval request destined to
+// a specific peers.
+type txRequest struct {
+	hashes []common.Hash            // Transactions having been requested
+	stolen map[common.Hash]struct{} // Deliveries by someone else (don't re-request)
+	time   mclock.AbsTime           // Timestamp of the request
+}
+
+// txDelivery is the notification that a batch of transactions have been added
+// to the pool and should be untracked.
+type txDelivery struct {
 	origin string        // Identifier of the peer originating the notification
-	time     time.Time           // Timestamp of the announcement
-	fetchTxs func([]common.Hash) // Callback for retrieving transaction from specified peer
+	hashes []common.Hash // Batch of transaction hashes having been delivered
+	direct bool          // Whether this is a direct reply or a broadcast
 }
 
-// TxFetcher is responsible for retrieving new transaction based
-// on the announcement.
+// txDrop is the notiication that a peer has disconnected.
+type txDrop struct {
+	peer string
+}
+
+// TxFetcher is responsible for retrieving new transaction based on announcements.
+//
+// The fetcher operates in 3 stages:
+//   - Transactions that are newly discovered are moved into a wait list.
+//   - After ~500ms passes, transactions from the wait list that have not been
+//     broadcast to us in whole are moved into a queueing area.
+//   - When a connected peer doesn't have in-flight retrieval requests, any
+//     transaction queued up (and announced by the peer) are allocated to the
+//     peer and moved into a fetching status until it's fulfilled or fails.
+//
+// The invariants of the fetcher are:
+//   - Each tracked transaction (hash) must only be present in one of the
+//     three stages. This ensures that the fetcher operates akin to a finite
+//     state automata and there's do data leak.
+//   - Each peer that announced transactions may be scheduled retrievals, but
+//     only ever one concurrently. This ensures we can immediately know what is
+//     missing from a reply and reschedule it.
 type TxFetcher struct {
-	notify  chan *txsAnnounce
-	cleanup chan []common.Hash
+	notify  chan *txAnnounce
+	cleanup chan *txDelivery
+	drop    chan *txDrop
 	quit    chan struct{}
 
-	// Announce states
-	announces   map[string]int                // Per peer transaction announce counts to prevent memory exhaustion
-	announced   map[common.Hash][]*txAnnounce // Announced transactions, scheduled for fetching
-	fetching    map[common.Hash]*txAnnounce   // Announced transactions, currently fetching
-	underpriced mapset.Set                    // Transaction set whose price is too low for accepting
+	underpriced mapset.Set // Transactions discarded as too cheap (don't re-fetch)
+
+	// Stage 1: Waiting lists for newly discovered transactions that might be
+	// broadcast without needing explicit request/reply round trips.
+	waitlist  map[common.Hash]map[string]struct{} // Transactions waiting for an potential broadcast
+	waittime  map[common.Hash]mclock.AbsTime      // Timestamps when transactions were added to the waitlist
+	waitslots map[string]map[common.Hash]struct{} // Waiting announcement sgroupped by peer (DoS protection)
+
+	// Stage 2: Queue of transactions that waiting to be allocated to some peer
+	// to be retrieved directly.
+	announces map[string]map[common.Hash]struct{} // Set of announced transactions, grouped by origin peer
+	announced map[common.Hash]map[string]struct{} // Set of download locations, grouped by transaction hash
+
+	// Stage 3: Set of transactions currently being retrieved, some which may be
+	// fulfilled and some rescheduled. Note, this step shares 'announces' from the
+	// previous stage to avoid having to duplicate (need it for DoS checks).
+	fetching   map[common.Hash]string              // Transaction set currently being retrieved
+	requests   map[string]*txRequest               // In-flight transaction retrievals
+	alternates map[common.Hash]map[string]struct{} // In-flight transaction alternate origins if retrieval fails
 
 	// Callbacks
 	hasTx    func(common.Hash) bool             // Retrieves a tx from the local txpool
 	addTxs   func([]*types.Transaction) []error // Insert a batch of transactions into local txpool
-	dropPeer func(string)                       // Drop the specified peer
-
-	// Hooks
-	announceHook     func([]common.Hash)        // Hook which is called when a batch transactions are announced
-	importTxsHook    func([]*types.Transaction) // Hook which is called when a batch of transactions are imported.
-	dropHook         func(string)               // Hook which is called when a peer is dropped
-	cleanupHook      func([]common.Hash)        // Hook which is called when internal status is cleaned
-	rejectUnderprice func(common.Hash)          // Hook which is called when underprice transaction is rejected
+	fetchTxs func(string, []common.Hash) error  // Retrieves a set of txs from a remote peer
+
+	step  chan struct{} // Notification channel when the fetcher loop iterates
+	clock mclock.Clock  // Time wrapper to simulate in tests
+	rand  *mrand.Rand   // Randomizer to use in tests instead of map range loops (soft-random)
 }
 
 // NewTxFetcher creates a transaction fetcher to retrieve transaction
 // based on hash announcements.
-func NewTxFetcher(hasTx func(common.Hash) bool, addTxs func([]*types.Transaction) []error, dropPeer func(string)) *TxFetcher {
+func NewTxFetcher(hasTx func(common.Hash) bool, addTxs func([]*types.Transaction) []error, fetchTxs func(string, []common.Hash) error) *TxFetcher {
+	return NewTxFetcherForTests(hasTx, addTxs, fetchTxs, mclock.System{}, nil)
+}
+
+// NewTxFetcherForTests is a testing method to mock out the realtime clock with
+// a simulated version and the internal randomness with a deterministic one.
+func NewTxFetcherForTests(
+	hasTx func(common.Hash) bool, addTxs func([]*types.Transaction) []error, fetchTxs func(string, []common.Hash) error,
+	clock mclock.Clock, rand *mrand.Rand) *TxFetcher {
 	return &TxFetcher{
-		notify:      make(chan *txsAnnounce),
-		cleanup:     make(chan []common.Hash),
+		notify:      make(chan *txAnnounce),
+		cleanup:     make(chan *txDelivery),
+		drop:        make(chan *txDrop),
 		quit:        make(chan struct{}),
-		announces:   make(map[string]int),
-		announced:   make(map[common.Hash][]*txAnnounce),
-		fetching:    make(map[common.Hash]*txAnnounce),
+		waitlist:    make(map[common.Hash]map[string]struct{}),
+		waittime:    make(map[common.Hash]mclock.AbsTime),
+		waitslots:   make(map[string]map[common.Hash]struct{}),
+		announces:   make(map[string]map[common.Hash]struct{}),
+		announced:   make(map[common.Hash]map[string]struct{}),
+		fetching:    make(map[common.Hash]string),
+		requests:    make(map[string]*txRequest),
+		alternates:  make(map[common.Hash]map[string]struct{}),
 		underpriced: mapset.NewSet(),
 		hasTx:       hasTx,
 		addTxs:      addTxs,
-		dropPeer:    dropPeer,
+		fetchTxs:    fetchTxs,
+		clock:       clock,
+		rand:        rand,
 	}
 }
 
-// Notify announces the fetcher of the potential availability of a
-// new transaction in the network.
-func (f *TxFetcher) Notify(peer string, hashes []common.Hash, time time.Time, fetchTxs func([]common.Hash)) error {
-	announce := &txsAnnounce{
-		hashes:   hashes,
-		time:     time,
+// Notify announces the fetcher of the potential availability of a new batch of
+// transactions in the network.
+func (f *TxFetcher) Notify(peer string, hashes []common.Hash) error {
+	// Keep track of all the announced transactions
+	txAnnounceInMeter.Mark(int64(len(hashes)))
+
+	// Skip any transaction announcements that we already know of, or that we've
+	// previously marked as cheap and discarded. This check is of course racey,
+	// because multiple concurrent notifies will still manage to pass it, but it's
+	// still valuable to check here because it runs concurrent  to the internal
+	// loop, so anything caught here is time saved internally.
+	var (
+		unknowns               = make([]common.Hash, 0, len(hashes))
+		duplicate, underpriced int64
+	)
+	for _, hash := range hashes {
+		switch {
+		case f.hasTx(hash):
+			duplicate++
+
+		case f.underpriced.Contains(hash):
+			underpriced++
+
+		default:
+			unknowns = append(unknowns, hash)
+		}
+	}
+	txAnnounceKnownMeter.Mark(duplicate)
+	txAnnounceUnderpricedMeter.Mark(underpriced)
+
+	// If anything's left to announce, push it into the internal loop
+	if len(unknowns) == 0 {
+		return nil
+	}
+	announce := &txAnnounce{
 		origin: peer,
-		fetchTxs: fetchTxs,
+		hashes: unknowns,
 	}
 	select {
 	case f.notify <- announce:
@@ -128,45 +256,75 @@ func (f *TxFetcher) Notify(peer string, hashes []common.Hash, time time.Time, fe
 	}
 }
 
-// EnqueueTxs imports a batch of received transaction into fetcher.
-func (f *TxFetcher) EnqueueTxs(peer string, txs []*types.Transaction) error {
+// Enqueue imports a batch of received transaction into the transaction pool
+// and the fetcher. This method may be called by both transaction broadcasts and
+// direct request replies. The differentiation is important so the fetcher can
+// re-shedule missing transactions as soon as possible.
+func (f *TxFetcher) Enqueue(peer string, txs []*types.Transaction, direct bool) error {
+	// Keep track of all the propagated transactions
+	if direct {
+		txReplyInMeter.Mark(int64(len(txs)))
+	} else {
+		txBroadcastInMeter.Mark(int64(len(txs)))
+	}
+	// Push all the transactions into the pool, tracking underpriced ones to avoid
+	// re-requesting them and dropping the peer in case of malicious transfers.
 	var (
-		drop   bool
-		hashes []common.Hash
+		added       = make([]common.Hash, 0, len(txs))
+		duplicate   int64
+		underpriced int64
+		otherreject int64
 	)
 	errs := f.addTxs(txs)
 	for i, err := range errs {
 		if err != nil {
-			// Drop peer if the received transaction isn't signed properly.
-			drop = (drop || err == core.ErrInvalidSender)
-			txFetchInvalidMeter.Mark(1)
-
 			// Track the transaction hash if the price is too low for us.
 			// Avoid re-request this transaction when we receive another
 			// announcement.
-			if err == core.ErrUnderpriced {
-				for f.underpriced.Cardinality() >= underpriceSetSize {
+			if err == core.ErrUnderpriced || err == core.ErrReplaceUnderpriced {
+				for f.underpriced.Cardinality() >= maxTxUnderpricedSetSize {
 					f.underpriced.Pop()
 				}
 				f.underpriced.Add(txs[i].Hash())
 			}
+			// Track a few interesting failure types
+			switch err {
+			case nil: // Noop, but need to handle to not count these
+
+			case core.ErrAlreadyKnown:
+				duplicate++
+
+			case core.ErrUnderpriced, core.ErrReplaceUnderpriced:
+				underpriced++
+
+			default:
+				otherreject++
 			}
-		hashes = append(hashes, txs[i].Hash())
 		}
-	if f.importTxsHook != nil {
-		f.importTxsHook(txs)
+		added = append(added, txs[i].Hash())
 	}
-	// Drop the peer if some transaction failed signature verification.
-	// We can regard this peer is trying to DOS us by feeding lots of
-	// random hashes.
-	if drop {
-		f.dropPeer(peer)
-		if f.dropHook != nil {
-			f.dropHook(peer)
+	if direct {
+		txReplyKnownMeter.Mark(duplicate)
+		txReplyUnderpricedMeter.Mark(underpriced)
+		txReplyOtherRejectMeter.Mark(otherreject)
+	} else {
+		txBroadcastKnownMeter.Mark(duplicate)
+		txBroadcastUnderpricedMeter.Mark(underpriced)
+		txBroadcastOtherRejectMeter.Mark(otherreject)
 	}
+	select {
+	case f.cleanup <- &txDelivery{origin: peer, hashes: added, direct: direct}:
+		return nil
+	case <-f.quit:
+		return errTerminated
 	}
+}
+
+// Drop should be called when a peer disconnects. It cleans up all the internal
+// data structures of the given node.
+func (f *TxFetcher) Drop(peer string) error {
 	select {
-	case f.cleanup <- hashes:
+	case f.drop <- &txDrop{peer: peer}:
 		return nil
 	case <-f.quit:
 		return errTerminated
@@ -186,134 +344,551 @@ func (f *TxFetcher) Stop() {
 }
 
 func (f *TxFetcher) loop() {
-	fetchTimer := time.NewTimer(0)
+	var (
+		waitTimer    = new(mclock.Timer)
+		timeoutTimer = new(mclock.Timer)
 
+		waitTrigger    = make(chan struct{}, 1)
+		timeoutTrigger = make(chan struct{}, 1)
+	)
 	for {
-		// Clean up any expired transaction fetches.
-		// There are many cases can lead to it:
-		// * We send the request to busy peer which can reply immediately
-		// * We send the request to malicious peer which doesn't reply deliberately
-		// * We send the request to normal peer for a batch of transaction, but some
-		//   transactions have been included into blocks. According to EIP these txs
-		//   won't be included.
-		// But it's fine to delete the fetching record and reschedule fetching iff we
-		// receive the annoucement again.
-		for hash, announce := range f.fetching {
-			if time.Since(announce.time) > txFetchTimeout {
-				delete(f.fetching, hash)
-				txFetchTimeoutMeter.Mark(1)
+		select {
+		case ann := <-f.notify:
+			// Drop part of the new announcements if there are too many accumulated.
+			// Note, we could but do not filter already known transactions here as
+			// the probability of something arriving between this call and the pre-
+			// filter outside is essentially zero.
+			used := len(f.waitslots[ann.origin]) + len(f.announces[ann.origin])
+			if used >= maxTxAnnounces {
+				// This can happen if a set of transactions are requested but not
+				// all fulfilled, so the remainder are rescheduled without the cap
+				// check. Should be fine as the limit is in the thousands and the
+				// request size in the hundreds.
+				txAnnounceDOSMeter.Mark(int64(len(ann.hashes)))
+				break
 			}
+			want := used + len(ann.hashes)
+			if want > maxTxAnnounces {
+				txAnnounceDOSMeter.Mark(int64(want - maxTxAnnounces))
+				ann.hashes = ann.hashes[:want-maxTxAnnounces]
 			}
-		select {
-		case anno := <-f.notify:
-			txAnnounceInMeter.Mark(int64(len(anno.hashes)))
+			// All is well, schedule the remainder of the transactions
+			idleWait := len(f.waittime) == 0
+			_, oldPeer := f.announces[ann.origin]
 
-			// Drop the new announce if there are too many accumulated.
-			count := f.announces[anno.origin] + len(anno.hashes)
-			if count > txAnnounceLimit {
-				txAnnounceDOSMeter.Mark(int64(count - txAnnounceLimit))
-				break
+			for _, hash := range ann.hashes {
+				// If the transaction is already downloading, add it to the list
+				// of possible alternates (in case the current retrieval fails) and
+				// also account it for the peer.
+				if f.alternates[hash] != nil {
+					f.alternates[hash][ann.origin] = struct{}{}
+
+					// Stage 2 and 3 share the set of origins per tx
+					if announces := f.announces[ann.origin]; announces != nil {
+						announces[hash] = struct{}{}
+					} else {
+						f.announces[ann.origin] = map[common.Hash]struct{}{hash: struct{}{}}
 					}
-			f.announces[anno.origin] = count
+					continue
+				}
+				// If the transaction is not downloading, but is already queued
+				// from a different peer, track it for the new peer too.
+				if f.announced[hash] != nil {
+					f.announced[hash][ann.origin] = struct{}{}
 
-			// All is well, schedule the announce if transaction is not yet downloading
-			empty := len(f.announced) == 0
-			for _, hash := range anno.hashes {
-				if _, ok := f.fetching[hash]; ok {
+					// Stage 2 and 3 share the set of origins per tx
+					if announces := f.announces[ann.origin]; announces != nil {
+						announces[hash] = struct{}{}
+					} else {
+						f.announces[ann.origin] = map[common.Hash]struct{}{hash: struct{}{}}
+					}
 					continue
 				}
-				if f.underpriced.Contains(hash) {
-					txAnnounceUnderpriceMeter.Mark(1)
-					if f.rejectUnderprice != nil {
-						f.rejectUnderprice(hash)
+				// If the transaction is already known to the fetcher, but not
+				// yet downloading, add the peer as an alternate origin in the
+				// waiting list.
+				if f.waitlist[hash] != nil {
+					f.waitlist[hash][ann.origin] = struct{}{}
+
+					if waitslots := f.waitslots[ann.origin]; waitslots != nil {
+						waitslots[hash] = struct{}{}
+					} else {
+						f.waitslots[ann.origin] = map[common.Hash]struct{}{hash: struct{}{}}
 					}
 					continue
 				}
-				f.announced[hash] = append(f.announced[hash], &txAnnounce{
-					origin:   anno.origin,
-					time:     anno.time,
-					fetchTxs: anno.fetchTxs,
-				})
+				// Transaction unknown to the fetcher, insert it into the waiting list
+				f.waitlist[hash] = map[string]struct{}{ann.origin: struct{}{}}
+				f.waittime[hash] = f.clock.Now()
+
+				if waitslots := f.waitslots[ann.origin]; waitslots != nil {
+					waitslots[hash] = struct{}{}
+				} else {
+					f.waitslots[ann.origin] = map[common.Hash]struct{}{hash: struct{}{}}
+				}
+			}
+			// If a new item was added to the waitlist, schedule it into the fetcher
+			if idleWait && len(f.waittime) > 0 {
+				f.rescheduleWait(waitTimer, waitTrigger)
 			}
-			if empty && len(f.announced) > 0 {
-				f.reschedule(fetchTimer)
+			// If this peer is new and announced something already queued, maybe
+			// request transactions from them
+			if !oldPeer && len(f.announces[ann.origin]) > 0 {
+				f.scheduleFetches(timeoutTimer, timeoutTrigger, map[string]struct{}{ann.origin: struct{}{}})
+			}
+
+		case <-waitTrigger:
+			// At least one transaction's waiting time ran out, push all expired
+			// ones into the retrieval queues
+			actives := make(map[string]struct{})
+			for hash, instance := range f.waittime {
+				if time.Duration(f.clock.Now()-instance)+txGatherSlack > txArriveTimeout {
+					// Transaction expired without propagation, schedule for retrieval
+					if f.announced[hash] != nil {
+						panic("announce tracker already contains waitlist item")
+					}
+					f.announced[hash] = f.waitlist[hash]
+					for peer := range f.waitlist[hash] {
+						if announces := f.announces[peer]; announces != nil {
+							announces[hash] = struct{}{}
+						} else {
+							f.announces[peer] = map[common.Hash]struct{}{hash: struct{}{}}
+						}
+						delete(f.waitslots[peer], hash)
+						if len(f.waitslots[peer]) == 0 {
+							delete(f.waitslots, peer)
+						}
+						actives[peer] = struct{}{}
+					}
+					delete(f.waittime, hash)
+					delete(f.waitlist, hash)
 				}
-			if f.announceHook != nil {
-				f.announceHook(anno.hashes)
 			}
-		case <-fetchTimer.C:
-			// At least one tx's timer ran out, check for needing retrieval
-			request := make(map[string][]common.Hash)
+			// If transactions are still waiting for propagation, reschedule the wait timer
+			if len(f.waittime) > 0 {
+				f.rescheduleWait(waitTimer, waitTrigger)
+			}
+			// If any peers became active and are idle, request transactions from them
+			if len(actives) > 0 {
+				f.scheduleFetches(timeoutTimer, timeoutTrigger, actives)
+			}
 
-			for hash, announces := range f.announced {
-				if time.Since(announces[0].time) > arriveTimeout-gatherSlack {
-					// Pick a random peer to retrieve from, reset all others
-					announce := announces[rand.Intn(len(announces))]
-					f.forgetHash(hash)
+		case <-timeoutTrigger:
+			// Clean up any expired retrievals and avoid re-requesting them from the
+			// same peer (either overloaded or malicious, useless in both cases). We
+			// could also penalize (Drop), but there's nothing to gain, and if could
+			// possibly further increase the load on it.
+			for peer, req := range f.requests {
+				if time.Duration(f.clock.Now()-req.time)+txGatherSlack > txFetchTimeout {
+					txRequestTimeoutMeter.Mark(int64(len(req.hashes)))
 
-					// Skip fetching if we already receive the transaction.
-					if f.hasTx(hash) {
-						txAnnounceSkipMeter.Mark(1)
+					// Reschedule all the not-yet-delivered fetches to alternate peers
+					for _, hash := range req.hashes {
+						// Skip rescheduling hashes already delivered by someone else
+						if req.stolen != nil {
+							if _, ok := req.stolen[hash]; ok {
 								continue
 							}
-					// If the transaction still didn't arrive, queue for fetching
-					request[announce.origin] = append(request[announce.origin], hash)
-					f.fetching[hash] = announce
 						}
+						// Move the delivery back from fetching to queued
+						if _, ok := f.announced[hash]; ok {
+							panic("announced tracker already contains alternate item")
 						}
-			// Send out all block header requests
-			for peer, hashes := range request {
-				log.Trace("Fetching scheduled transactions", "peer", peer, "txs", hashes)
-				fetchTxs := f.fetching[hashes[0]].fetchTxs
-				fetchTxs(hashes)
-				txFetchOutMeter.Mark(int64(len(hashes)))
+						if f.alternates[hash] != nil { // nil if tx was broadcast during fetch
+							f.announced[hash] = f.alternates[hash]
 						}
-			// Schedule the next fetch if blocks are still pending
-			f.reschedule(fetchTimer)
-		case hashes := <-f.cleanup:
-			for _, hash := range hashes {
-				f.forgetHash(hash)
-				anno, exist := f.fetching[hash]
-				if !exist {
-					txBroadcastInMeter.Mark(1) // Directly transaction propagation
+						delete(f.announced[hash], peer)
+						if len(f.announced[hash]) == 0 {
+							delete(f.announced, hash)
+						}
+						delete(f.announces[peer], hash)
+						delete(f.alternates, hash)
+						delete(f.fetching, hash)
+					}
+					if len(f.announces[peer]) == 0 {
+						delete(f.announces, peer)
+					}
+					// Keep track of the request as dangling, but never expire
+					f.requests[peer].hashes = nil
+				}
+			}
+			// Schedule a new transaction retrieval
+			f.scheduleFetches(timeoutTimer, timeoutTrigger, nil)
+
+			// No idea if we sheduled something or not, trigger the timer if needed
+			// TODO(karalabe): this is kind of lame, can't we dump it into scheduleFetches somehow?
+			f.rescheduleTimeout(timeoutTimer, timeoutTrigger)
+
+		case delivery := <-f.cleanup:
+			// Independent if the delivery was direct or broadcast, remove all
+			// traces of the hash from internal trackers
+			for _, hash := range delivery.hashes {
+				if _, ok := f.waitlist[hash]; ok {
+					for peer, txset := range f.waitslots {
+						delete(txset, hash)
+						if len(txset) == 0 {
+							delete(f.waitslots, peer)
+						}
+					}
+					delete(f.waitlist, hash)
+					delete(f.waittime, hash)
+				} else {
+					for peer, txset := range f.announces {
+						delete(txset, hash)
+						if len(txset) == 0 {
+							delete(f.announces, peer)
+						}
+					}
+					delete(f.announced, hash)
+					delete(f.alternates, hash)
+
+					// If a transaction currently being fetched from a different
+					// origin was delivered (delivery stolen), mark it so the
+					// actual delivery won't double schedule it.
+					if origin, ok := f.fetching[hash]; ok && (origin != delivery.origin || !delivery.direct) {
+						stolen := f.requests[origin].stolen
+						if stolen == nil {
+							f.requests[origin].stolen = make(map[common.Hash]struct{})
+							stolen = f.requests[origin].stolen
+						}
+						stolen[hash] = struct{}{}
+					}
+					delete(f.fetching, hash)
+				}
+			}
+			// In case of a direct delivery, also reschedule anything missing
+			// from the original query
+			if delivery.direct {
+				// Mark the reqesting successful (independent of individual status)
+				txRequestDoneMeter.Mark(int64(len(delivery.hashes)))
+
+				// Make sure something was pending, nuke it
+				req := f.requests[delivery.origin]
+				if req == nil {
+					log.Warn("Unexpected transaction delivery", "peer", delivery.origin)
+					break
+				}
+				delete(f.requests, delivery.origin)
+
+				// Anything not delivered should be re-scheduled (with or without
+				// this peer, depending on the response cutoff)
+				delivered := make(map[common.Hash]struct{})
+				for _, hash := range delivery.hashes {
+					delivered[hash] = struct{}{}
+				}
+				cutoff := len(req.hashes) // If nothing is delivered, assume everything is missing, don't retry!!!
+				for i, hash := range req.hashes {
+					if _, ok := delivered[hash]; ok {
+						cutoff = i
+					}
+				}
+				// Reschedule missing hashes from alternates, not-fulfilled from alt+self
+				for i, hash := range req.hashes {
+					// Skip rescheduling hashes already delivered by someone else
+					if req.stolen != nil {
+						if _, ok := req.stolen[hash]; ok {
 							continue
 						}
-				txFetchDurationTimer.UpdateSince(anno.time)
-				txFetchSuccessMeter.Mark(1)
+					}
+					if _, ok := delivered[hash]; !ok {
+						if i < cutoff {
+							delete(f.alternates[hash], delivery.origin)
+							delete(f.announces[delivery.origin], hash)
+							if len(f.announces[delivery.origin]) == 0 {
+								delete(f.announces, delivery.origin)
+							}
+						}
+						if len(f.alternates[hash]) > 0 {
+							if _, ok := f.announced[hash]; ok {
+								panic(fmt.Sprintf("announced tracker already contains alternate item: %v", f.announced[hash]))
+							}
+							f.announced[hash] = f.alternates[hash]
+						}
+					}
+					delete(f.alternates, hash)
 					delete(f.fetching, hash)
 				}
-			if f.cleanupHook != nil {
-				f.cleanupHook(hashes)
+				// Something was delivered, try to rechedule requests
+				f.scheduleFetches(timeoutTimer, timeoutTrigger, nil) // Partial delivery may enable others to deliver too
+			}
+
+		case drop := <-f.drop:
+			// A peer was dropped, remove all traces of it
+			if _, ok := f.waitslots[drop.peer]; ok {
+				for hash := range f.waitslots[drop.peer] {
+					delete(f.waitlist[hash], drop.peer)
+					if len(f.waitlist[hash]) == 0 {
+						delete(f.waitlist, hash)
+						delete(f.waittime, hash)
+					}
+				}
+				delete(f.waitslots, drop.peer)
+				if len(f.waitlist) > 0 {
+					f.rescheduleWait(waitTimer, waitTrigger)
+				}
+			}
+			// Clean up any active requests
+			var request *txRequest
+			if request = f.requests[drop.peer]; request != nil {
+				for _, hash := range request.hashes {
+					// Skip rescheduling hashes already delivered by someone else
+					if request.stolen != nil {
+						if _, ok := request.stolen[hash]; ok {
+							continue
+						}
+					}
+					// Undelivered hash, reschedule if there's an alternative origin available
+					delete(f.alternates[hash], drop.peer)
+					if len(f.alternates[hash]) == 0 {
+						delete(f.alternates, hash)
+					} else {
+						f.announced[hash] = f.alternates[hash]
+						delete(f.alternates, hash)
+					}
+					delete(f.fetching, hash)
+				}
+				delete(f.requests, drop.peer)
+			}
+			// Clean up general announcement tracking
+			if _, ok := f.announces[drop.peer]; ok {
+				for hash := range f.announces[drop.peer] {
+					delete(f.announced[hash], drop.peer)
+					if len(f.announced[hash]) == 0 {
+						delete(f.announced, hash)
+					}
+				}
+				delete(f.announces, drop.peer)
 			}
+			// If a request was cancelled, check if anything needs to be rescheduled
+			if request != nil {
+				f.scheduleFetches(timeoutTimer, timeoutTrigger, nil)
+				f.rescheduleTimeout(timeoutTimer, timeoutTrigger)
+			}
+
 		case <-f.quit:
 			return
 		}
+		// No idea what happened, but bump some sanity metrics
+		txFetcherWaitingPeers.Update(int64(len(f.waitslots)))
+		txFetcherWaitingHashes.Update(int64(len(f.waitlist)))
+		txFetcherQueueingPeers.Update(int64(len(f.announces) - len(f.requests)))
+		txFetcherQueueingHashes.Update(int64(len(f.announced)))
+		txFetcherFetchingPeers.Update(int64(len(f.requests)))
+		txFetcherFetchingHashes.Update(int64(len(f.fetching)))
+
+		// Loop did something, ping the step notifier if needed (tests)
+		if f.step != nil {
+			f.step <- struct{}{}
+		}
 	}
 }
 
-// rescheduleFetch resets the specified fetch timer to the next blockAnnounce timeout.
-func (f *TxFetcher) reschedule(fetch *time.Timer) {
-	// Short circuit if no transactions are announced
-	if len(f.announced) == 0 {
-		return
+// rescheduleWait iterates over all the transactions currently in the waitlist
+// and schedules the movement into the fetcher for the earliest.
+//
+// The method has a granularity of 'gatherSlack', since there's not much point in
+// spinning over all the transactions just to maybe find one that should trigger
+// a few ms earlier.
+func (f *TxFetcher) rescheduleWait(timer *mclock.Timer, trigger chan struct{}) {
+	if *timer != nil {
+		(*timer).Stop()
+	}
+	now := f.clock.Now()
+
+	earliest := now
+	for _, instance := range f.waittime {
+		if earliest > instance {
+			earliest = instance
+			if txArriveTimeout-time.Duration(now-earliest) < gatherSlack {
+				break
+			}
+		}
+	}
+	*timer = f.clock.AfterFunc(txArriveTimeout-time.Duration(now-earliest), func() {
+		trigger <- struct{}{}
+	})
+}
+
+// rescheduleTimeout iterates over all the transactions currently in flight and
+// schedules a cleanup run when the first would trigger.
+//
+// The method has a granularity of 'gatherSlack', since there's not much point in
+// spinning over all the transactions just to maybe find one that should trigger
+// a few ms earlier.
+//
+// This method is a bit "flaky" "by design". In theory the timeout timer only ever
+// should be rescheduled if some request is pending. In practice, a timeout will
+// cause the timer to be rescheduled every 5 secs (until the peer comes through or
+// disconnects). This is a limitation of the fetcher code because we don't trac
+// pending requests and timed out requests separatey. Without double tracking, if
+// we simply didn't reschedule the timer on all-timeout then the timer would never
+// be set again since len(request) > 0 => something's running.
+func (f *TxFetcher) rescheduleTimeout(timer *mclock.Timer, trigger chan struct{}) {
+	if *timer != nil {
+		(*timer).Stop()
+	}
+	now := f.clock.Now()
+
+	earliest := now
+	for _, req := range f.requests {
+		// If this request already timed out, skip it altogether
+		if req.hashes == nil {
+			continue
+		}
+		if earliest > req.time {
+			earliest = req.time
+			if txFetchTimeout-time.Duration(now-earliest) < gatherSlack {
+				break
 			}
-	// Otherwise find the earliest expiring announcement
-	earliest := time.Now()
-	for _, announces := range f.announced {
-		if earliest.After(announces[0].time) {
-			earliest = announces[0].time
 		}
 	}
-	fetch.Reset(arriveTimeout - time.Since(earliest))
+	*timer = f.clock.AfterFunc(txFetchTimeout-time.Duration(now-earliest), func() {
+		trigger <- struct{}{}
+	})
 }
 
-func (f *TxFetcher) forgetHash(hash common.Hash) {
-	// Remove all pending announces and decrement DOS counters
-	for _, announce := range f.announced[hash] {
-		f.announces[announce.origin]--
-		if f.announces[announce.origin] <= 0 {
-			delete(f.announces, announce.origin)
+// scheduleFetches starts a batch of retrievals for all available idle peers.
+func (f *TxFetcher) scheduleFetches(timer *mclock.Timer, timeout chan struct{}, whitelist map[string]struct{}) {
+	// Gather the set of peers we want to retrieve from (default to all)
+	actives := whitelist
+	if actives == nil {
+		actives = make(map[string]struct{})
+		for peer := range f.announces {
+			actives[peer] = struct{}{}
+		}
+	}
+	if len(actives) == 0 {
+		return
 	}
+	// For each active peer, try to schedule some transaction fetches
+	idle := len(f.requests) == 0
+
+	f.forEachPeer(actives, func(peer string) {
+		if f.requests[peer] != nil {
+			return // continue in the for-each
+		}
+		if len(f.announces[peer]) == 0 {
+			return // continue in the for-each
+		}
+		hashes := make([]common.Hash, 0, maxTxRetrievals)
+		f.forEachHash(f.announces[peer], func(hash common.Hash) bool {
+			if _, ok := f.fetching[hash]; !ok {
+				// Mark the hash as fetching and stash away possible alternates
+				f.fetching[hash] = peer
+
+				if _, ok := f.alternates[hash]; ok {
+					panic(fmt.Sprintf("alternate tracker already contains fetching item: %v", f.alternates[hash]))
 				}
+				f.alternates[hash] = f.announced[hash]
 				delete(f.announced, hash)
+
+				// Accumulate the hash and stop if the limit was reached
+				hashes = append(hashes, hash)
+				if len(hashes) >= maxTxRetrievals {
+					return false // break in the for-each
+				}
+			}
+			return true // continue in the for-each
+		})
+		// If any hashes were allocated, request them from the peer
+		if len(hashes) > 0 {
+			f.requests[peer] = &txRequest{hashes: hashes, time: f.clock.Now()}
+			txRequestOutMeter.Mark(int64(len(hashes)))
+
+			go func(peer string, hashes []common.Hash) {
+				// Try to fetch the transactions, but in case of a request
+				// failure (e.g. peer disconnected), reschedule the hashes.
+				if err := f.fetchTxs(peer, hashes); err != nil {
+					txRequestFailMeter.Mark(int64(len(hashes)))
+					f.Drop(peer)
+				}
+			}(peer, hashes)
+		}
+	})
+	// If a new request was fired, schedule a timeout timer
+	if idle && len(f.requests) > 0 {
+		f.rescheduleTimeout(timer, timeout)
+	}
+}
+
+// forEachPeer does a range loop over a map of peers in production, but during
+// testing it does a deterministic sorted random to allow reproducing issues.
+func (f *TxFetcher) forEachPeer(peers map[string]struct{}, do func(peer string)) {
+	// If we're running production, use whatever Go's map gives us
+	if f.rand == nil {
+		for peer := range peers {
+			do(peer)
+		}
+		return
+	}
+	// We're running the test suite, make iteration deterministic
+	list := make([]string, 0, len(peers))
+	for peer := range peers {
+		list = append(list, peer)
+	}
+	sort.Strings(list)
+	rotateStrings(list, f.rand.Intn(len(list)))
+	for _, peer := range list {
+		do(peer)
+	}
+}
+
+// forEachHash does a range loop over a map of hashes in production, but during
+// testing it does a deterministic sorted random to allow reproducing issues.
+func (f *TxFetcher) forEachHash(hashes map[common.Hash]struct{}, do func(hash common.Hash) bool) {
+	// If we're running production, use whatever Go's map gives us
+	if f.rand == nil {
+		for hash := range hashes {
+			if !do(hash) {
+				return
+			}
+		}
+		return
+	}
+	// We're running the test suite, make iteration deterministic
+	list := make([]common.Hash, 0, len(hashes))
+	for hash := range hashes {
+		list = append(list, hash)
+	}
+	sortHashes(list)
+	rotateHashes(list, f.rand.Intn(len(list)))
+	for _, hash := range list {
+		if !do(hash) {
+			return
+		}
+	}
+}
+
+// rotateStrings rotates the contents of a slice by n steps. This method is only
+// used in tests to simulate random map iteration but keep it deterministic.
+func rotateStrings(slice []string, n int) {
+	orig := make([]string, len(slice))
+	copy(orig, slice)
+
+	for i := 0; i < len(orig); i++ {
+		slice[i] = orig[(i+n)%len(orig)]
+	}
+}
+
+// sortHashes sorts a slice of hashes. This method is only used in tests in order
+// to simulate random map iteration but keep it deterministic.
+func sortHashes(slice []common.Hash) {
+	for i := 0; i < len(slice); i++ {
+		for j := i + 1; j < len(slice); j++ {
+			if bytes.Compare(slice[i][:], slice[j][:]) > 0 {
+				slice[i], slice[j] = slice[j], slice[i]
+			}
+		}
+	}
+}
+
+// rotateHashes rotates the contents of a slice by n steps. This method is only
+// used in tests to simulate random map iteration but keep it deterministic.
+func rotateHashes(slice []common.Hash, n int) {
+	orig := make([]common.Hash, len(slice))
+	copy(orig, slice)
+
+	for i := 0; i < len(orig); i++ {
+		slice[i] = orig[(i+n)%len(orig)]
+	}
 }

eth/fetcher/tx_fetcher_test.go

@@ -17,302 +17,1512 @@
 package fetcher
 
 import (
-	"crypto/ecdsa"
+	"errors"
 	"math/big"
 	"math/rand"
-	"sync"
-	"sync/atomic"
 	"testing"
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/mclock"
 	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/types"
-	"github.com/ethereum/go-ethereum/crypto"
 )
 
-func init() {
-	rand.Seed(int64(time.Now().Nanosecond()))
+var (
+	// testTxs is a set of transactions to use during testing that have meaninful hashes.
+	testTxs = []*types.Transaction{
+		types.NewTransaction(rand.Uint64(), common.Address{byte(rand.Intn(256))}, new(big.Int), 0, new(big.Int), nil),
+		types.NewTransaction(rand.Uint64(), common.Address{byte(rand.Intn(256))}, new(big.Int), 0, new(big.Int), nil),
+		types.NewTransaction(rand.Uint64(), common.Address{byte(rand.Intn(256))}, new(big.Int), 0, new(big.Int), nil),
+		types.NewTransaction(rand.Uint64(), common.Address{byte(rand.Intn(256))}, new(big.Int), 0, new(big.Int), nil),
+	}
+	// testTxsHashes is the hashes of the test transactions above
+	testTxsHashes = []common.Hash{testTxs[0].Hash(), testTxs[1].Hash(), testTxs[2].Hash(), testTxs[3].Hash()}
+)
 
-	txAnnounceLimit = 64
-	MaxTransactionFetch = 16
+type doTxNotify struct {
+	peer   string
+	hashes []common.Hash
+}
+type doTxEnqueue struct {
+	peer   string
+	txs    []*types.Transaction
+	direct bool
+}
+type doWait struct {
+	time time.Duration
+	step bool
 }
+type doDrop string
+type doFunc func()
 
-func makeTransactions(key *ecdsa.PrivateKey, target int) []*types.Transaction {
-	var txs []*types.Transaction
+type isWaiting map[string][]common.Hash
+type isScheduled struct {
+	tracking map[string][]common.Hash
+	fetching map[string][]common.Hash
+	dangling map[string][]common.Hash
+}
+type isUnderpriced int
 
-	for i := 0; i < target; i++ {
-		random := rand.Uint32()
-		tx := types.NewTransaction(uint64(random), common.Address{0x1, 0x2, 0x3}, big.NewInt(int64(random)), 100, big.NewInt(int64(random)), nil)
-		tx, _ = types.SignTx(tx, types.NewEIP155Signer(big.NewInt(1)), key)
-		txs = append(txs, tx)
-	}
-	return txs
+// txFetcherTest represents a test scenario that can be executed by the test
+// runner.
+type txFetcherTest struct {
+	init  func() *TxFetcher
+	steps []interface{}
 }
 
-func makeUnsignedTransactions(key *ecdsa.PrivateKey, target int) []*types.Transaction {
-	var txs []*types.Transaction
+// Tests that transaction announcements are added to a waitlist, and none
+// of them are scheduled for retrieval until the wait expires.
+func TestTransactionFetcherWaiting(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				nil,
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Initial announcement to get something into the waitlist
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x01}, {0x02}}},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x01}, {0x02}},
+			}),
+			// Announce from a new peer to check that no overwrite happens
+			doTxNotify{peer: "B", hashes: []common.Hash{{0x03}, {0x04}}},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x01}, {0x02}},
+				"B": {{0x03}, {0x04}},
+			}),
+			// Announce clashing hashes but unique new peer
+			doTxNotify{peer: "C", hashes: []common.Hash{{0x01}, {0x04}}},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x01}, {0x02}},
+				"B": {{0x03}, {0x04}},
+				"C": {{0x01}, {0x04}},
+			}),
+			// Announce existing and clashing hashes from existing peer
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x01}, {0x03}, {0x05}}},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x01}, {0x02}, {0x03}, {0x05}},
+				"B": {{0x03}, {0x04}},
+				"C": {{0x01}, {0x04}},
+			}),
+			isScheduled{tracking: nil, fetching: nil},
 
-	for i := 0; i < target; i++ {
-		random := rand.Uint32()
-		tx := types.NewTransaction(uint64(random), common.Address{0x1, 0x2, 0x3}, big.NewInt(int64(random)), 100, big.NewInt(int64(random)), nil)
-		txs = append(txs, tx)
-	}
-	return txs
+			// Wait for the arrival timeout which should move all expired items
+			// from the wait list to the scheduler
+			doWait{time: txArriveTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}, {0x03}, {0x05}},
+					"B": {{0x03}, {0x04}},
+					"C": {{0x01}, {0x04}},
+				},
+				fetching: map[string][]common.Hash{ // Depends on deterministic test randomizer
+					"A": {{0x02}, {0x03}, {0x05}},
+					"C": {{0x01}, {0x04}},
+				},
+			},
+			// Queue up a non-fetchable transaction and then trigger it with a new
+			// peer (weird case to test 1 line in the fetcher)
+			doTxNotify{peer: "C", hashes: []common.Hash{{0x06}, {0x07}}},
+			isWaiting(map[string][]common.Hash{
+				"C": {{0x06}, {0x07}},
+			}),
+			doWait{time: txArriveTimeout, step: true},
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}, {0x03}, {0x05}},
+					"B": {{0x03}, {0x04}},
+					"C": {{0x01}, {0x04}, {0x06}, {0x07}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x02}, {0x03}, {0x05}},
+					"C": {{0x01}, {0x04}},
+				},
+			},
+			doTxNotify{peer: "D", hashes: []common.Hash{{0x06}, {0x07}}},
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}, {0x03}, {0x05}},
+					"B": {{0x03}, {0x04}},
+					"C": {{0x01}, {0x04}, {0x06}, {0x07}},
+					"D": {{0x06}, {0x07}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x02}, {0x03}, {0x05}},
+					"C": {{0x01}, {0x04}},
+					"D": {{0x06}, {0x07}},
+				},
+			},
+		},
+	})
 }
 
-type txfetcherTester struct {
-	fetcher *TxFetcher
+// Tests that transaction announcements skip the waiting list if they are
+// already scheduled.
+func TestTransactionFetcherSkipWaiting(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				nil,
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Push an initial announcement through to the scheduled stage
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x01}, {0x02}}},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x01}, {0x02}},
+			}),
+			isScheduled{tracking: nil, fetching: nil},
 
-	priceLimit *big.Int
-	sender     *ecdsa.PrivateKey
-	senderAddr common.Address
-	signer     types.Signer
-	txs        map[common.Hash]*types.Transaction
-	dropped    map[string]struct{}
-	lock       sync.RWMutex
+			doWait{time: txArriveTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+			},
+			// Announce overlaps from the same peer, ensure the new ones end up
+			// in stage one, and clashing ones don't get double tracked
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x02}, {0x03}}},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x03}},
+			}),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+			},
+			// Announce overlaps from a new peer, ensure new transactions end up
+			// in stage one and clashing ones get tracked for the new peer
+			doTxNotify{peer: "B", hashes: []common.Hash{{0x02}, {0x03}, {0x04}}},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x03}},
+				"B": {{0x03}, {0x04}},
+			}),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+					"B": {{0x02}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+			},
+		},
+	})
 }
 
-func newTxFetcherTester() *txfetcherTester {
-	key, _ := crypto.GenerateKey()
-	addr := crypto.PubkeyToAddress(key.PublicKey)
-	t := &txfetcherTester{
-		sender:     key,
-		senderAddr: addr,
-		signer:     types.NewEIP155Signer(big.NewInt(1)),
-		txs:        make(map[common.Hash]*types.Transaction),
-		dropped:    make(map[string]struct{}),
-	}
-	t.fetcher = NewTxFetcher(t.hasTx, t.addTxs, t.dropPeer)
-	t.fetcher.Start()
-	return t
+// Tests that only a single transaction request gets scheduled to a peer
+// and subsequent announces block or get allotted to someone else.
+func TestTransactionFetcherSingletonRequesting(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				nil,
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Push an initial announcement through to the scheduled stage
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x01}, {0x02}}},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x01}, {0x02}},
+			}),
+			isScheduled{tracking: nil, fetching: nil},
+
+			doWait{time: txArriveTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+			},
+			// Announce a new set of transactions from the same peer and ensure
+			// they do not start fetching since the peer is already busy
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x03}, {0x04}}},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x03}, {0x04}},
+			}),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+			},
+			doWait{time: txArriveTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}, {0x03}, {0x04}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+			},
+			// Announce a duplicate set of transactions from a new peer and ensure
+			// uniquely new ones start downloading, even if clashing.
+			doTxNotify{peer: "B", hashes: []common.Hash{{0x02}, {0x03}, {0x05}, {0x06}}},
+			isWaiting(map[string][]common.Hash{
+				"B": {{0x05}, {0x06}},
+			}),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}, {0x03}, {0x04}},
+					"B": {{0x02}, {0x03}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+					"B": {{0x03}},
+				},
+			},
+		},
+	})
 }
 
-func (t *txfetcherTester) hasTx(hash common.Hash) bool {
-	t.lock.RLock()
-	defer t.lock.RUnlock()
+// Tests that if a transaction retrieval fails, all the transactions get
+// instantly schedule back to someone else or the announcements dropped
+// if no alternate source is available.
+func TestTransactionFetcherFailedRescheduling(t *testing.T) {
+	// Create a channel to control when tx requests can fail
+	proceed := make(chan struct{})
 
-	return t.txs[hash] != nil
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				nil,
+				func(origin string, hashes []common.Hash) error {
+					<-proceed
+					return errors.New("peer disconnected")
+				},
+			)
+		},
+		steps: []interface{}{
+			// Push an initial announcement through to the scheduled stage
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x01}, {0x02}}},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x01}, {0x02}},
+			}),
+			isScheduled{tracking: nil, fetching: nil},
+
+			doWait{time: txArriveTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+			},
+			// While the original peer is stuck in the request, push in an second
+			// data source.
+			doTxNotify{peer: "B", hashes: []common.Hash{{0x02}}},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+					"B": {{0x02}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+			},
+			// Wait until the original request fails and check that transactions
+			// are either rescheduled or dropped
+			doFunc(func() {
+				proceed <- struct{}{} // Allow peer A to return the failure
+			}),
+			doWait{time: 0, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"B": {{0x02}},
+				},
+				fetching: map[string][]common.Hash{
+					"B": {{0x02}},
+				},
+			},
+			doFunc(func() {
+				proceed <- struct{}{} // Allow peer B to return the failure
+			}),
+			doWait{time: 0, step: true},
+			isWaiting(nil),
+			isScheduled{nil, nil, nil},
+		},
+	})
 }
 
-func (t *txfetcherTester) addTxs(txs []*types.Transaction) []error {
-	t.lock.Lock()
-	defer t.lock.Unlock()
+// Tests that if a transaction retrieval succeeds, all alternate origins
+// are cleaned up.
+func TestTransactionFetcherCleanup(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Push an initial announcement through to the scheduled stage
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0]}},
+			isWaiting(map[string][]common.Hash{
+				"A": {testTxsHashes[0]},
+			}),
+			isScheduled{tracking: nil, fetching: nil},
 
-	var errors []error
-	for _, tx := range txs {
-		// Make sure the transaction is signed properly
-		_, err := types.Sender(t.signer, tx)
-		if err != nil {
-			errors = append(errors, core.ErrInvalidSender)
-			continue
-		}
-		// Make sure the price is high enough to accpet
-		if t.priceLimit != nil && tx.GasPrice().Cmp(t.priceLimit) < 0 {
-			errors = append(errors, core.ErrUnderpriced)
-			continue
-		}
-		t.txs[tx.Hash()] = tx
-		errors = append(errors, nil)
-	}
-	return errors
+			doWait{time: txArriveTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {testTxsHashes[0]},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {testTxsHashes[0]},
+				},
+			},
+			// Request should be delivered
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[0]}, direct: true},
+			isScheduled{nil, nil, nil},
+		},
+	})
 }
 
-func (t *txfetcherTester) dropPeer(id string) {
-	t.lock.Lock()
-	defer t.lock.Unlock()
+// Tests that if a transaction retrieval succeeds, but the response is empty (no
+// transactions available, then all are nuked instead of being rescheduled (yes,
+// this was a bug)).
+func TestTransactionFetcherCleanupEmpty(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Push an initial announcement through to the scheduled stage
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0]}},
+			isWaiting(map[string][]common.Hash{
+				"A": {testTxsHashes[0]},
+			}),
+			isScheduled{tracking: nil, fetching: nil},
 
-	t.dropped[id] = struct{}{}
+			doWait{time: txArriveTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {testTxsHashes[0]},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {testTxsHashes[0]},
+				},
+			},
+			// Deliver an empty response and ensure the transaction is cleared, not rescheduled
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{}, direct: true},
+			isScheduled{nil, nil, nil},
+		},
+	})
 }
 
-// makeTxFetcher retrieves a batch of transaction associated with a simulated peer.
-func (t *txfetcherTester) makeTxFetcher(peer string, txs []*types.Transaction) func(hashes []common.Hash) {
-	closure := make(map[common.Hash]*types.Transaction)
-	for _, tx := range txs {
-		closure[tx.Hash()] = tx
-	}
-	return func(hashes []common.Hash) {
-		var txs []*types.Transaction
-		for _, hash := range hashes {
-			tx := closure[hash]
-			if tx == nil {
-				continue
+// Tests that non-returned transactions are either re-sheduled from a
+// different peer, or self if they are after the cutoff point.
+func TestTransactionFetcherMissingRescheduling(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Push an initial announcement through to the scheduled stage
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0], testTxsHashes[1], testTxsHashes[2]}},
+			isWaiting(map[string][]common.Hash{
+				"A": {testTxsHashes[0], testTxsHashes[1], testTxsHashes[2]},
+			}),
+			isScheduled{tracking: nil, fetching: nil},
+
+			doWait{time: txArriveTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {testTxsHashes[0], testTxsHashes[1], testTxsHashes[2]},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {testTxsHashes[0], testTxsHashes[1], testTxsHashes[2]},
+				},
+			},
+			// Deliver the middle transaction requested, the one before which
+			// should be dropped and the one after re-requested.
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[0]}, direct: true}, // This depends on the deterministic random
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {testTxsHashes[2]},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {testTxsHashes[2]},
+				},
+			},
+		},
+	})
+}
+
+// Tests that out of two transactions, if one is missing and the last is
+// delivered, the peer gets properly cleaned out from the internal state.
+func TestTransactionFetcherMissingCleanup(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Push an initial announcement through to the scheduled stage
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0], testTxsHashes[1]}},
+			isWaiting(map[string][]common.Hash{
+				"A": {testTxsHashes[0], testTxsHashes[1]},
+			}),
+			isScheduled{tracking: nil, fetching: nil},
+
+			doWait{time: txArriveTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {testTxsHashes[0], testTxsHashes[1]},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {testTxsHashes[0], testTxsHashes[1]},
+				},
+			},
+			// Deliver the middle transaction requested, the one before which
+			// should be dropped and the one after re-requested.
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[1]}, direct: true}, // This depends on the deterministic random
+			isScheduled{nil, nil, nil},
+		},
+	})
+}
+
+// Tests that transaction broadcasts properly clean up announcements.
+func TestTransactionFetcherBroadcasts(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Set up three transactions to be in different stats, waiting, queued and fetching
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0]}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[1]}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[2]}},
+
+			isWaiting(map[string][]common.Hash{
+				"A": {testTxsHashes[2]},
+			}),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {testTxsHashes[0], testTxsHashes[1]},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {testTxsHashes[0]},
+				},
+			},
+			// Broadcast all the transactions and ensure everything gets cleaned
+			// up, but the dangling request is left alone to avoid doing multiple
+			// concurrent requests.
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[0], testTxs[1], testTxs[2]}, direct: false},
+			isWaiting(nil),
+			isScheduled{
+				tracking: nil,
+				fetching: nil,
+				dangling: map[string][]common.Hash{
+					"A": {testTxsHashes[0]},
+				},
+			},
+			// Deliver the requested hashes
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[0], testTxs[1], testTxs[2]}, direct: true},
+			isScheduled{nil, nil, nil},
+		},
+	})
+}
+
+// Tests that the waiting list timers properly reset and reschedule.
+func TestTransactionFetcherWaitTimerResets(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				nil,
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x01}}},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x01}},
+			}),
+			isScheduled{nil, nil, nil},
+			doWait{time: txArriveTimeout / 2, step: false},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x01}},
+			}),
+			isScheduled{nil, nil, nil},
+
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x02}}},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x01}, {0x02}},
+			}),
+			isScheduled{nil, nil, nil},
+			doWait{time: txArriveTimeout / 2, step: true},
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x02}},
+			}),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}},
+				},
+			},
+
+			doWait{time: txArriveTimeout / 2, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}},
+				},
+			},
+		},
+	})
+}
+
+// Tests that if a transaction request is not replied to, it will time
+// out and be re-scheduled for someone else.
+func TestTransactionFetcherTimeoutRescheduling(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Push an initial announcement through to the scheduled stage
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0]}},
+			isWaiting(map[string][]common.Hash{
+				"A": {testTxsHashes[0]},
+			}),
+			isScheduled{tracking: nil, fetching: nil},
+
+			doWait{time: txArriveTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {testTxsHashes[0]},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {testTxsHashes[0]},
+				},
+			},
+			// Wait until the delivery times out, everything should be cleaned up
+			doWait{time: txFetchTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: nil,
+				fetching: nil,
+				dangling: map[string][]common.Hash{
+					"A": {},
+				},
+			},
+			// Ensure that followup announcements don't get scheduled
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[1]}},
+			doWait{time: txArriveTimeout, step: true},
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {testTxsHashes[1]},
+				},
+				fetching: nil,
+				dangling: map[string][]common.Hash{
+					"A": {},
+				},
+			},
+			// If the dangling request arrives a bit later, do not choke
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[0]}, direct: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {testTxsHashes[1]},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {testTxsHashes[1]},
+				},
+			},
+		},
+	})
+}
+
+// Tests that the fetching timeout timers properly reset and reschedule.
+func TestTransactionFetcherTimeoutTimerResets(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				nil,
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x01}}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxNotify{peer: "B", hashes: []common.Hash{{0x02}}},
+			doWait{time: txArriveTimeout, step: true},
+
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}},
+					"B": {{0x02}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}},
+					"B": {{0x02}},
+				},
+			},
+			doWait{time: txFetchTimeout - txArriveTimeout, step: true},
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"B": {{0x02}},
+				},
+				fetching: map[string][]common.Hash{
+					"B": {{0x02}},
+				},
+				dangling: map[string][]common.Hash{
+					"A": {},
+				},
+			},
+			doWait{time: txArriveTimeout, step: true},
+			isScheduled{
+				tracking: nil,
+				fetching: nil,
+				dangling: map[string][]common.Hash{
+					"A": {},
+					"B": {},
+				},
+			},
+		},
+	})
+}
+
+// Tests that if thousands of transactions are announces, only a small
+// number of them will be requested at a time.
+func TestTransactionFetcherRateLimiting(t *testing.T) {
+	// Create a slew of transactions and to announce them
+	var hashes []common.Hash
+	for i := 0; i < maxTxAnnounces; i++ {
+		hashes = append(hashes, common.Hash{byte(i / 256), byte(i % 256)})
 	}
-			txs = append(txs, tx)
+
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				nil,
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Announce all the transactions, wait a bit and ensure only a small
+			// percentage gets requested
+			doTxNotify{peer: "A", hashes: hashes},
+			doWait{time: txArriveTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": hashes,
+				},
+				fetching: map[string][]common.Hash{
+					"A": hashes[1643 : 1643+maxTxRetrievals],
+				},
+			},
+		},
+	})
+}
+
+// Tests that then number of transactions a peer is allowed to announce and/or
+// request at the same time is hard capped.
+func TestTransactionFetcherDoSProtection(t *testing.T) {
+	// Create a slew of transactions and to announce them
+	var hashesA []common.Hash
+	for i := 0; i < maxTxAnnounces+1; i++ {
+		hashesA = append(hashesA, common.Hash{0x01, byte(i / 256), byte(i % 256)})
 	}
-		// Return on a new thread
-		go t.fetcher.EnqueueTxs(peer, txs)
+	var hashesB []common.Hash
+	for i := 0; i < maxTxAnnounces+1; i++ {
+		hashesB = append(hashesB, common.Hash{0x02, byte(i / 256), byte(i % 256)})
 	}
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				nil,
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Announce half of the transaction and wait for them to be scheduled
+			doTxNotify{peer: "A", hashes: hashesA[:maxTxAnnounces/2]},
+			doTxNotify{peer: "B", hashes: hashesB[:maxTxAnnounces/2-1]},
+			doWait{time: txArriveTimeout, step: true},
+
+			// Announce the second half and keep them in the wait list
+			doTxNotify{peer: "A", hashes: hashesA[maxTxAnnounces/2 : maxTxAnnounces]},
+			doTxNotify{peer: "B", hashes: hashesB[maxTxAnnounces/2-1 : maxTxAnnounces-1]},
+
+			// Ensure the hashes are split half and half
+			isWaiting(map[string][]common.Hash{
+				"A": hashesA[maxTxAnnounces/2 : maxTxAnnounces],
+				"B": hashesB[maxTxAnnounces/2-1 : maxTxAnnounces-1],
+			}),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": hashesA[:maxTxAnnounces/2],
+					"B": hashesB[:maxTxAnnounces/2-1],
+				},
+				fetching: map[string][]common.Hash{
+					"A": hashesA[1643 : 1643+maxTxRetrievals],
+					"B": append(append([]common.Hash{}, hashesB[maxTxAnnounces/2-3:maxTxAnnounces/2-1]...), hashesB[:maxTxRetrievals-2]...),
+				},
+			},
+			// Ensure that adding even one more hash results in dropping the hash
+			doTxNotify{peer: "A", hashes: []common.Hash{hashesA[maxTxAnnounces]}},
+			doTxNotify{peer: "B", hashes: hashesB[maxTxAnnounces-1 : maxTxAnnounces+1]},
+
+			isWaiting(map[string][]common.Hash{
+				"A": hashesA[maxTxAnnounces/2 : maxTxAnnounces],
+				"B": hashesB[maxTxAnnounces/2-1 : maxTxAnnounces],
+			}),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": hashesA[:maxTxAnnounces/2],
+					"B": hashesB[:maxTxAnnounces/2-1],
+				},
+				fetching: map[string][]common.Hash{
+					"A": hashesA[1643 : 1643+maxTxRetrievals],
+					"B": append(append([]common.Hash{}, hashesB[maxTxAnnounces/2-3:maxTxAnnounces/2-1]...), hashesB[:maxTxRetrievals-2]...),
+				},
+			},
+		},
+	})
 }
 
-func TestSequentialTxAnnouncements(t *testing.T) {
-	tester := newTxFetcherTester()
-	txs := makeTransactions(tester.sender, txAnnounceLimit)
+// Tests that underpriced transactions don't get rescheduled after being rejected.
+func TestTransactionFetcherUnderpricedDedup(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					errs := make([]error, len(txs))
+					for i := 0; i < len(errs); i++ {
+						if i%2 == 0 {
+							errs[i] = core.ErrUnderpriced
+						} else {
+							errs[i] = core.ErrReplaceUnderpriced
+						}
+					}
+					return errs
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Deliver a transaction through the fetcher, but reject as underpriced
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0], testTxsHashes[1]}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[0], testTxs[1]}, direct: true},
+			isScheduled{nil, nil, nil},
 
-	retrieveTxs := tester.makeTxFetcher("peer", txs)
+			// Try to announce the transaction again, ensure it's not scheduled back
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0], testTxsHashes[1], testTxsHashes[2]}}, // [2] is needed to force a step in the fetcher
+			isWaiting(map[string][]common.Hash{
+				"A": {testTxsHashes[2]},
+			}),
+			isScheduled{nil, nil, nil},
+		},
+	})
+}
+
+// Tests that underpriced transactions don't get rescheduled after being rejected,
+// but at the same time there's a hard cap on the number of transactions that are
+// tracked.
+func TestTransactionFetcherUnderpricedDoSProtection(t *testing.T) {
+	// Temporarily disable fetch timeouts as they massively mess up the simulated clock
+	defer func(timeout time.Duration) { txFetchTimeout = timeout }(txFetchTimeout)
+	txFetchTimeout = 24 * time.Hour
 
-	newTxsCh := make(chan struct{})
-	tester.fetcher.importTxsHook = func(transactions []*types.Transaction) {
-		newTxsCh <- struct{}{}
+	// Create a slew of transactions to max out the underpriced set
+	var txs []*types.Transaction
+	for i := 0; i < maxTxUnderpricedSetSize+1; i++ {
+		txs = append(txs, types.NewTransaction(rand.Uint64(), common.Address{byte(rand.Intn(256))}, new(big.Int), 0, new(big.Int), nil))
 	}
-	for _, tx := range txs {
-		tester.fetcher.Notify("peer", []common.Hash{tx.Hash()}, time.Now().Add(-arriveTimeout), retrieveTxs)
-		select {
-		case <-newTxsCh:
-		case <-time.NewTimer(time.Second).C:
-			t.Fatalf("timeout")
+	hashes := make([]common.Hash, len(txs))
+	for i, tx := range txs {
+		hashes[i] = tx.Hash()
 	}
+	// Generate a set of steps to announce and deliver the entire set of transactions
+	var steps []interface{}
+	for i := 0; i < maxTxUnderpricedSetSize/maxTxRetrievals; i++ {
+		steps = append(steps, doTxNotify{peer: "A", hashes: hashes[i*maxTxRetrievals : (i+1)*maxTxRetrievals]})
+		steps = append(steps, isWaiting(map[string][]common.Hash{
+			"A": hashes[i*maxTxRetrievals : (i+1)*maxTxRetrievals],
+		}))
+		steps = append(steps, doWait{time: txArriveTimeout, step: true})
+		steps = append(steps, isScheduled{
+			tracking: map[string][]common.Hash{
+				"A": hashes[i*maxTxRetrievals : (i+1)*maxTxRetrievals],
+			},
+			fetching: map[string][]common.Hash{
+				"A": hashes[i*maxTxRetrievals : (i+1)*maxTxRetrievals],
+			},
+		})
+		steps = append(steps, doTxEnqueue{peer: "A", txs: txs[i*maxTxRetrievals : (i+1)*maxTxRetrievals], direct: true})
+		steps = append(steps, isWaiting(nil))
+		steps = append(steps, isScheduled{nil, nil, nil})
+		steps = append(steps, isUnderpriced((i+1)*maxTxRetrievals))
 	}
-	if len(tester.txs) != len(txs) {
-		t.Fatalf("Imported transaction number mismatch, want %d, got %d", len(txs), len(tester.txs))
+	testTransactionFetcher(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					errs := make([]error, len(txs))
+					for i := 0; i < len(errs); i++ {
+						errs[i] = core.ErrUnderpriced
 					}
+					return errs
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: append(steps, []interface{}{
+			// The preparation of the test has already been done in `steps`, add the last check
+			doTxNotify{peer: "A", hashes: []common.Hash{hashes[maxTxUnderpricedSetSize]}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{txs[maxTxUnderpricedSetSize]}, direct: true},
+			isUnderpriced(maxTxUnderpricedSetSize),
+		}...),
+	})
 }
 
-func TestConcurrentAnnouncements(t *testing.T) {
-	tester := newTxFetcherTester()
-	txs := makeTransactions(tester.sender, txAnnounceLimit)
+// Tests that unexpected deliveries don't corrupt the internal state.
+func TestTransactionFetcherOutOfBoundDeliveries(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Deliver something out of the blue
+			isWaiting(nil),
+			isScheduled{nil, nil, nil},
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[0]}, direct: false},
+			isWaiting(nil),
+			isScheduled{nil, nil, nil},
+
+			// Set up a few hashes into various stages
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0]}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[1]}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[2]}},
 
-	txFetcherFn1 := tester.makeTxFetcher("peer1", txs)
-	txFetcherFn2 := tester.makeTxFetcher("peer2", txs)
+			isWaiting(map[string][]common.Hash{
+				"A": {testTxsHashes[2]},
+			}),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {testTxsHashes[0], testTxsHashes[1]},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {testTxsHashes[0]},
+				},
+			},
+			// Deliver everything and more out of the blue
+			doTxEnqueue{peer: "B", txs: []*types.Transaction{testTxs[0], testTxs[1], testTxs[2], testTxs[3]}, direct: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: nil,
+				fetching: nil,
+				dangling: map[string][]common.Hash{
+					"A": {testTxsHashes[0]},
+				},
+			},
+		},
+	})
+}
 
-	var (
-		count uint32
-		done  = make(chan struct{})
+// Tests that dropping a peer cleans out all internal data structures in all the
+// live or danglng stages.
+func TestTransactionFetcherDrop(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
 			)
-	tester.fetcher.importTxsHook = func(transactions []*types.Transaction) {
-		atomic.AddUint32(&count, uint32(len(transactions)))
-		if atomic.LoadUint32(&count) >= uint32(txAnnounceLimit) {
-			done <- struct{}{}
-		}
-	}
-	for _, tx := range txs {
-		tester.fetcher.Notify("peer1", []common.Hash{tx.Hash()}, time.Now().Add(-arriveTimeout), txFetcherFn1)
-		tester.fetcher.Notify("peer2", []common.Hash{tx.Hash()}, time.Now().Add(-arriveTimeout+time.Millisecond), txFetcherFn2)
-		tester.fetcher.Notify("peer2", []common.Hash{tx.Hash()}, time.Now().Add(-arriveTimeout-time.Millisecond), txFetcherFn2)
-	}
-	select {
-	case <-done:
-	case <-time.NewTimer(time.Second).C:
-		t.Fatalf("timeout")
-	}
+		},
+		steps: []interface{}{
+			// Set up a few hashes into various stages
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x01}}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x02}}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x03}}},
+
+			isWaiting(map[string][]common.Hash{
+				"A": {{0x03}},
+			}),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}, {0x02}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}},
+				},
+			},
+			// Drop the peer and ensure everything's cleaned out
+			doDrop("A"),
+			isWaiting(nil),
+			isScheduled{nil, nil, nil},
+
+			// Push the node into a dangling (timeout) state
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0]}},
+			doWait{time: txArriveTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {testTxsHashes[0]},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {testTxsHashes[0]},
+				},
+			},
+			doWait{time: txFetchTimeout, step: true},
+			isWaiting(nil),
+			isScheduled{
+				tracking: nil,
+				fetching: nil,
+				dangling: map[string][]common.Hash{
+					"A": {},
+				},
+			},
+			// Drop the peer and ensure everything's cleaned out
+			doDrop("A"),
+			isWaiting(nil),
+			isScheduled{nil, nil, nil},
+		},
+	})
 }
 
-func TestBatchAnnouncements(t *testing.T) {
-	tester := newTxFetcherTester()
-	txs := makeTransactions(tester.sender, txAnnounceLimit)
+// Tests that dropping a peer instantly reschedules failed announcements to any
+// available peer.
+func TestTransactionFetcherDropRescheduling(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Set up a few hashes into various stages
+			doTxNotify{peer: "A", hashes: []common.Hash{{0x01}}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxNotify{peer: "B", hashes: []common.Hash{{0x01}}},
 
-	retrieveTxs := tester.makeTxFetcher("peer", txs)
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"A": {{0x01}},
+					"B": {{0x01}},
+				},
+				fetching: map[string][]common.Hash{
+					"A": {{0x01}},
+				},
+			},
+			// Drop the peer and ensure everything's cleaned out
+			doDrop("A"),
+			isWaiting(nil),
+			isScheduled{
+				tracking: map[string][]common.Hash{
+					"B": {{0x01}},
+				},
+				fetching: map[string][]common.Hash{
+					"B": {{0x01}},
+				},
+			},
+		},
+	})
+}
 
-	var count uint32
-	var done = make(chan struct{})
-	tester.fetcher.importTxsHook = func(txs []*types.Transaction) {
-		atomic.AddUint32(&count, uint32(len(txs)))
+// This test reproduces a crash caught by the fuzzer. The root cause was a
+// dangling transaction timing out and clashing on readd with a concurrently
+// announced one.
+func TestTransactionFetcherFuzzCrash01(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Get a transaction into fetching mode and make it dangling with a broadcast
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0]}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[0]}},
 
-		if atomic.LoadUint32(&count) >= uint32(txAnnounceLimit) {
-			done <- struct{}{}
-		}
-	}
-	// Send all announces which exceeds the limit.
-	var hashes []common.Hash
-	for _, tx := range txs {
-		hashes = append(hashes, tx.Hash())
-	}
-	tester.fetcher.Notify("peer", hashes, time.Now(), retrieveTxs)
+			// Notify the dangling transaction once more and crash via a timeout
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0]}},
+			doWait{time: txFetchTimeout, step: true},
+		},
+	})
+}
 
-	select {
-	case <-done:
-	case <-time.NewTimer(time.Second).C:
-		t.Fatalf("timeout")
-	}
+// This test reproduces a crash caught by the fuzzer. The root cause was a
+// dangling transaction getting peer-dropped and clashing on readd with a
+// concurrently announced one.
+func TestTransactionFetcherFuzzCrash02(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Get a transaction into fetching mode and make it dangling with a broadcast
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0]}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[0]}},
+
+			// Notify the dangling transaction once more, re-fetch, and crash via a drop and timeout
+			doTxNotify{peer: "B", hashes: []common.Hash{testTxsHashes[0]}},
+			doWait{time: txArriveTimeout, step: true},
+			doDrop("A"),
+			doWait{time: txFetchTimeout, step: true},
+		},
+	})
 }
 
-func TestPropagationAfterAnnounce(t *testing.T) {
-	tester := newTxFetcherTester()
-	txs := makeTransactions(tester.sender, txAnnounceLimit)
+// This test reproduces a crash caught by the fuzzer. The root cause was a
+// dangling transaction getting rescheduled via a partial delivery, clashing
+// with a concurrent notify.
+func TestTransactionFetcherFuzzCrash03(t *testing.T) {
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error { return nil },
+			)
+		},
+		steps: []interface{}{
+			// Get a transaction into fetching mode and make it dangling with a broadcast
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0], testTxsHashes[1]}},
+			doWait{time: txFetchTimeout, step: true},
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[0], testTxs[1]}},
 
-	var cleaned = make(chan struct{})
-	tester.fetcher.cleanupHook = func(hashes []common.Hash) {
-		cleaned <- struct{}{}
-	}
-	retrieveTxs := tester.makeTxFetcher("peer", txs)
-	for _, tx := range txs {
-		tester.fetcher.Notify("peer", []common.Hash{tx.Hash()}, time.Now(), retrieveTxs)
-		tester.fetcher.EnqueueTxs("peer", []*types.Transaction{tx})
+			// Notify the dangling transaction once more, partially deliver, clash&crash with a timeout
+			doTxNotify{peer: "B", hashes: []common.Hash{testTxsHashes[0]}},
+			doWait{time: txArriveTimeout, step: true},
 
-		// It's ok to read the map directly since no write
-		// will happen in the same time.
-		<-cleaned
-		if len(tester.fetcher.announced) != 0 {
-			t.Fatalf("Announcement should be cleaned, got %d", len(tester.fetcher.announced))
-		}
-	}
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[1]}, direct: true},
+			doWait{time: txFetchTimeout, step: true},
+		},
+	})
 }
 
-func TestEnqueueTransactions(t *testing.T) {
-	tester := newTxFetcherTester()
-	txs := makeTransactions(tester.sender, txAnnounceLimit)
+// This test reproduces a crash caught by the fuzzer. The root cause was a
+// dangling transaction getting rescheduled via a disconnect, clashing with
+// a concurrent notify.
+func TestTransactionFetcherFuzzCrash04(t *testing.T) {
+	// Create a channel to control when tx requests can fail
+	proceed := make(chan struct{})
 
-	done := make(chan struct{})
-	tester.fetcher.importTxsHook = func(transactions []*types.Transaction) {
-		if len(transactions) == txAnnounceLimit {
-			done <- struct{}{}
-		}
-	}
-	go tester.fetcher.EnqueueTxs("peer", txs)
-	select {
-	case <-done:
-	case <-time.NewTimer(time.Second).C:
-		t.Fatalf("timeout")
-	}
+	testTransactionFetcherParallel(t, txFetcherTest{
+		init: func() *TxFetcher {
+			return NewTxFetcher(
+				func(common.Hash) bool { return false },
+				func(txs []*types.Transaction) []error {
+					return make([]error, len(txs))
+				},
+				func(string, []common.Hash) error {
+					<-proceed
+					return errors.New("peer disconnected")
+				},
+			)
+		},
+		steps: []interface{}{
+			// Get a transaction into fetching mode and make it dangling with a broadcast
+			doTxNotify{peer: "A", hashes: []common.Hash{testTxsHashes[0]}},
+			doWait{time: txArriveTimeout, step: true},
+			doTxEnqueue{peer: "A", txs: []*types.Transaction{testTxs[0]}},
+
+			// Notify the dangling transaction once more, re-fetch, and crash via an in-flight disconnect
+			doTxNotify{peer: "B", hashes: []common.Hash{testTxsHashes[0]}},
+			doWait{time: txArriveTimeout, step: true},
+			doFunc(func() {
+				proceed <- struct{}{} // Allow peer A to return the failure
+			}),
+			doWait{time: 0, step: true},
+			doWait{time: txFetchTimeout, step: true},
+		},
+	})
 }
 
-func TestInvalidTxAnnounces(t *testing.T) {
-	tester := newTxFetcherTester()
+func testTransactionFetcherParallel(t *testing.T, tt txFetcherTest) {
+	t.Parallel()
+	testTransactionFetcher(t, tt)
+}
 
-	var txs []*types.Transaction
-	txs = append(txs, makeUnsignedTransactions(tester.sender, 1)...)
-	txs = append(txs, makeTransactions(tester.sender, 1)...)
+func testTransactionFetcher(t *testing.T, tt txFetcherTest) {
+	// Create a fetcher and hook into it's simulated fields
+	clock := new(mclock.Simulated)
+	wait := make(chan struct{})
 
-	txFetcherFn := tester.makeTxFetcher("peer", txs)
+	fetcher := tt.init()
+	fetcher.clock = clock
+	fetcher.step = wait
+	fetcher.rand = rand.New(rand.NewSource(0x3a29))
 
-	dropped := make(chan string, 1)
-	tester.fetcher.dropHook = func(s string) { dropped <- s }
+	fetcher.Start()
+	defer fetcher.Stop()
 
-	for _, tx := range txs {
-		tester.fetcher.Notify("peer", []common.Hash{tx.Hash()}, time.Now(), txFetcherFn)
+	// Crunch through all the test steps and execute them
+	for i, step := range tt.steps {
+		switch step := step.(type) {
+		case doTxNotify:
+			if err := fetcher.Notify(step.peer, step.hashes); err != nil {
+				t.Errorf("step %d: %v", i, err)
 			}
+			<-wait // Fetcher needs to process this, wait until it's done
 			select {
-	case s := <-dropped:
-		if s != "peer" {
-			t.Fatalf("invalid dropped peer")
+			case <-wait:
+				panic("wtf")
+			case <-time.After(time.Millisecond):
 			}
-	case <-time.NewTimer(time.Second).C:
-		t.Fatalf("timeout")
+
+		case doTxEnqueue:
+			if err := fetcher.Enqueue(step.peer, step.txs, step.direct); err != nil {
+				t.Errorf("step %d: %v", i, err)
 			}
-}
+			<-wait // Fetcher needs to process this, wait until it's done
 
-func TestRejectUnderpriced(t *testing.T) {
-	tester := newTxFetcherTester()
-	tester.priceLimit = big.NewInt(10000)
+		case doWait:
+			clock.Run(step.time)
+			if step.step {
+				<-wait // Fetcher supposed to do something, wait until it's done
+			}
 
-	done := make(chan struct{})
-	tester.fetcher.importTxsHook = func([]*types.Transaction) { done <- struct{}{} }
-	reject := make(chan struct{})
-	tester.fetcher.rejectUnderprice = func(common.Hash) { reject <- struct{}{} }
+		case doDrop:
+			if err := fetcher.Drop(string(step)); err != nil {
+				t.Errorf("step %d: %v", i, err)
+			}
+			<-wait // Fetcher needs to process this, wait until it's done
 
-	tx := types.NewTransaction(0, common.Address{0x1, 0x2, 0x3}, big.NewInt(int64(100)), 100, big.NewInt(int64(100)), nil)
-	tx, _ = types.SignTx(tx, types.NewEIP155Signer(big.NewInt(1)), tester.sender)
-	txFetcherFn := tester.makeTxFetcher("peer", []*types.Transaction{tx})
+		case doFunc:
+			step()
 
-	// Send the announcement first time
-	tester.fetcher.Notify("peer", []common.Hash{tx.Hash()}, time.Now().Add(-arriveTimeout), txFetcherFn)
-	<-done
+		case isWaiting:
+			// We need to check that the waiting list (stage 1) internals
+			// match with the expected set. Check the peer->hash mappings
+			// first.
+			for peer, hashes := range step {
+				waiting := fetcher.waitslots[peer]
+				if waiting == nil {
+					t.Errorf("step %d: peer %s missing from waitslots", i, peer)
+					continue
+				}
+				for _, hash := range hashes {
+					if _, ok := waiting[hash]; !ok {
+						t.Errorf("step %d, peer %s: hash %x missing from waitslots", i, peer, hash)
+					}
+				}
+				for hash := range waiting {
+					if !containsHash(hashes, hash) {
+						t.Errorf("step %d, peer %s: hash %x extra in waitslots", i, peer, hash)
+					}
+				}
+			}
+			for peer := range fetcher.waitslots {
+				if _, ok := step[peer]; !ok {
+					t.Errorf("step %d: peer %s extra in waitslots", i, peer)
+				}
+			}
+			// Peer->hash sets correct, check the hash->peer and timeout sets
+			for peer, hashes := range step {
+				for _, hash := range hashes {
+					if _, ok := fetcher.waitlist[hash][peer]; !ok {
+						t.Errorf("step %d, hash %x: peer %s missing from waitlist", i, hash, peer)
+					}
+					if _, ok := fetcher.waittime[hash]; !ok {
+						t.Errorf("step %d: hash %x missing from waittime", i, hash)
+					}
+				}
+			}
+			for hash, peers := range fetcher.waitlist {
+				if len(peers) == 0 {
+					t.Errorf("step %d, hash %x: empty peerset in waitlist", i, hash)
+				}
+				for peer := range peers {
+					if !containsHash(step[peer], hash) {
+						t.Errorf("step %d, hash %x: peer %s extra in waitlist", i, hash, peer)
+					}
+				}
+			}
+			for hash := range fetcher.waittime {
+				var found bool
+				for _, hashes := range step {
+					if containsHash(hashes, hash) {
+						found = true
+						break
+					}
+				}
+				if !found {
+					t.Errorf("step %d,: hash %x extra in waittime", i, hash)
+				}
+			}
 
-	// Resend the announcement, shouldn't schedule fetching this time
-	tester.fetcher.Notify("peer", []common.Hash{tx.Hash()}, time.Now().Add(-arriveTimeout), txFetcherFn)
-	select {
-	case <-reject:
-	case <-time.NewTimer(time.Second).C:
-		t.Fatalf("timeout")
+		case isScheduled:
+			// Check that all scheduled announces are accounted for and no
+			// extra ones are present.
+			for peer, hashes := range step.tracking {
+				scheduled := fetcher.announces[peer]
+				if scheduled == nil {
+					t.Errorf("step %d: peer %s missing from announces", i, peer)
+					continue
+				}
+				for _, hash := range hashes {
+					if _, ok := scheduled[hash]; !ok {
+						t.Errorf("step %d, peer %s: hash %x missing from announces", i, peer, hash)
+					}
+				}
+				for hash := range scheduled {
+					if !containsHash(hashes, hash) {
+						t.Errorf("step %d, peer %s: hash %x extra in announces", i, peer, hash)
+					}
+				}
+			}
+			for peer := range fetcher.announces {
+				if _, ok := step.tracking[peer]; !ok {
+					t.Errorf("step %d: peer %s extra in announces", i, peer)
+				}
+			}
+			// Check that all announces required to be fetching are in the
+			// appropriate sets
+			for peer, hashes := range step.fetching {
+				request := fetcher.requests[peer]
+				if request == nil {
+					t.Errorf("step %d: peer %s missing from requests", i, peer)
+					continue
+				}
+				for _, hash := range hashes {
+					if !containsHash(request.hashes, hash) {
+						t.Errorf("step %d, peer %s: hash %x missing from requests", i, peer, hash)
+					}
+				}
+				for _, hash := range request.hashes {
+					if !containsHash(hashes, hash) {
+						t.Errorf("step %d, peer %s: hash %x extra in requests", i, peer, hash)
+					}
+				}
+			}
+			for peer := range fetcher.requests {
+				if _, ok := step.fetching[peer]; !ok {
+					if _, ok := step.dangling[peer]; !ok {
+						t.Errorf("step %d: peer %s extra in requests", i, peer)
+					}
+				}
+			}
+			for peer, hashes := range step.fetching {
+				for _, hash := range hashes {
+					if _, ok := fetcher.fetching[hash]; !ok {
+						t.Errorf("step %d, peer %s: hash %x missing from fetching", i, peer, hash)
+					}
+				}
+			}
+			for hash := range fetcher.fetching {
+				var found bool
+				for _, req := range fetcher.requests {
+					if containsHash(req.hashes, hash) {
+						found = true
+						break
+					}
+				}
+				if !found {
+					t.Errorf("step %d: hash %x extra in fetching", i, hash)
+				}
+			}
+			for _, hashes := range step.fetching {
+				for _, hash := range hashes {
+					alternates := fetcher.alternates[hash]
+					if alternates == nil {
+						t.Errorf("step %d: hash %x missing from alternates", i, hash)
+						continue
+					}
+					for peer := range alternates {
+						if _, ok := fetcher.announces[peer]; !ok {
+							t.Errorf("step %d: peer %s extra in alternates", i, peer)
+							continue
+						}
+						if _, ok := fetcher.announces[peer][hash]; !ok {
+							t.Errorf("step %d, peer %s: hash %x extra in alternates", i, hash, peer)
+							continue
+						}
+					}
+					for p := range fetcher.announced[hash] {
+						if _, ok := alternates[p]; !ok {
+							t.Errorf("step %d, hash %x: peer %s missing from alternates", i, hash, p)
+							continue
+						}
+					}
+				}
+			}
+			for peer, hashes := range step.dangling {
+				request := fetcher.requests[peer]
+				if request == nil {
+					t.Errorf("step %d: peer %s missing from requests", i, peer)
+					continue
+				}
+				for _, hash := range hashes {
+					if !containsHash(request.hashes, hash) {
+						t.Errorf("step %d, peer %s: hash %x missing from requests", i, peer, hash)
+					}
+				}
+				for _, hash := range request.hashes {
+					if !containsHash(hashes, hash) {
+						t.Errorf("step %d, peer %s: hash %x extra in requests", i, peer, hash)
+					}
+				}
+			}
+			// Check that all transaction announces that are scheduled for
+			// retrieval but not actively being downloaded are tracked only
+			// in the stage 2 `announced` map.
+			var queued []common.Hash
+			for _, hashes := range step.tracking {
+				for _, hash := range hashes {
+					var found bool
+					for _, hs := range step.fetching {
+						if containsHash(hs, hash) {
+							found = true
+							break
+						}
+					}
+					if !found {
+						queued = append(queued, hash)
+					}
+				}
+			}
+			for _, hash := range queued {
+				if _, ok := fetcher.announced[hash]; !ok {
+					t.Errorf("step %d: hash %x missing from announced", i, hash)
+				}
+			}
+			for hash := range fetcher.announced {
+				if !containsHash(queued, hash) {
+					t.Errorf("step %d: hash %x extra in announced", i, hash)
+				}
+			}
+
+		case isUnderpriced:
+			if fetcher.underpriced.Cardinality() != int(step) {
+				t.Errorf("step %d: underpriced set size mismatch: have %d, want %d", i, fetcher.underpriced.Cardinality(), step)
+			}
+
+		default:
+			t.Fatalf("step %d: unknown step type %T", i, step)
+		}
+		// After every step, cross validate the internal uniqueness invariants
+		// between stage one and stage two.
+		for hash := range fetcher.waittime {
+			if _, ok := fetcher.announced[hash]; ok {
+				t.Errorf("step %d: hash %s present in both stage 1 and 2", i, hash)
+			}
+		}
+	}
+}
+
+// containsHash returns whether a hash is contained within a hash slice.
+func containsHash(slice []common.Hash, hash common.Hash) bool {
+	for _, have := range slice {
+		if have == hash {
+			return true
+		}
 	}
+	return false
 }

eth/handler.go

@@ -51,7 +51,7 @@ const (
 	// The number is referenced from the size of tx pool.
 	txChanSize = 4096
 
-	// minimim number of peers to broadcast new blocks to
+	// minimim number of peers to broadcast entire blocks and transactions too.
 	minBroadcastPeers = 4
 )
 
@@ -192,7 +192,15 @@ func NewProtocolManager(config *params.ChainConfig, checkpoint *params.TrustedCh
 		return n, err
 	}
 	manager.blockFetcher = fetcher.NewBlockFetcher(blockchain.GetBlockByHash, validator, manager.BroadcastBlock, heighter, inserter, manager.removePeer)
-	manager.txFetcher = fetcher.NewTxFetcher(txpool.Has, txpool.AddRemotes, manager.removePeer)
+
+	fetchTx := func(peer string, hashes []common.Hash) error {
+		p := manager.peers.Peer(peer)
+		if p == nil {
+			return errors.New("unknown peer")
+		}
+		return p.RequestTxs(hashes)
+	}
+	manager.txFetcher = fetcher.NewTxFetcher(txpool.Has, txpool.AddRemotes, fetchTx)
 
 	return manager, nil
 }
@@ -240,6 +248,8 @@ func (pm *ProtocolManager) removePeer(id string) {
 
 	// Unregister the peer from the downloader and Ethereum peer set
 	pm.downloader.UnregisterPeer(id)
+	pm.txFetcher.Drop(id)
+
 	if err := pm.peers.Unregister(id); err != nil {
 		log.Error("Peer removal failed", "peer", id, "err", err)
 	}
@@ -263,7 +273,7 @@ func (pm *ProtocolManager) Start(maxPeers int) {
 
 	// start sync handlers
 	go pm.syncer()
-	go pm.txsyncLoop()
+	go pm.txsyncLoop64() // TODO(karalabe): Legacy initial tx echange, drop with eth/64.
 }
 
 func (pm *ProtocolManager) Stop() {
@@ -292,7 +302,7 @@ func (pm *ProtocolManager) Stop() {
 }
 
 func (pm *ProtocolManager) newPeer(pv int, p *p2p.Peer, rw p2p.MsgReadWriter, getPooledTx func(hash common.Hash) *types.Transaction) *peer {
-	return newPeer(pv, p, newMeteredMsgWriter(rw), getPooledTx)
+	return newPeer(pv, p, rw, getPooledTx)
 }
 
 // handle is the callback invoked to manage the life cycle of an eth peer. When
@@ -316,9 +326,6 @@ func (pm *ProtocolManager) handle(p *peer) error {
 		p.Log().Debug("Ethereum handshake failed", "err", err)
 		return err
 	}
-	if rw, ok := p.rw.(*meteredMsgReadWriter); ok {
-		rw.Init(p.version)
-	}
 	// Register the peer locally
 	if err := pm.peers.Register(p); err != nil {
 		p.Log().Error("Ethereum peer registration failed", "err", err)
@@ -740,20 +747,10 @@ func (pm *ProtocolManager) handleMsg(p *peer) error {
 			return errResp(ErrDecode, "msg %v: %v", msg, err)
 		}
 		// Schedule all the unknown hashes for retrieval
-		var unknown []common.Hash
 		for _, hash := range hashes {
-			// Mark the hashes as present at the remote node
 			p.MarkTransaction(hash)
-
-			// Filter duplicated transaction announcement.
-			// Notably we only dedupliate announcement in txpool, check the rationale
-			// behind in EIP https://github.com/ethereum/EIPs/pull/2464.
-			if pm.txpool.Has(hash) {
-				continue
-			}
-			unknown = append(unknown, hash)
 		}
-		pm.txFetcher.Notify(p.id, unknown, time.Now(), p.AsyncRequestTxs)
+		pm.txFetcher.Notify(p.id, hashes)
 
 	case msg.Code == GetPooledTransactionsMsg && p.version >= eth65:
 		// Decode the retrieval message
@@ -765,6 +762,7 @@ func (pm *ProtocolManager) handleMsg(p *peer) error {
 		var (
 			hash   common.Hash
 			bytes  int
+			hashes []common.Hash
 			txs    []rlp.RawValue
 		)
 		for bytes < softResponseLimit {
@@ -783,13 +781,14 @@ func (pm *ProtocolManager) handleMsg(p *peer) error {
 			if encoded, err := rlp.EncodeToBytes(tx); err != nil {
 				log.Error("Failed to encode transaction", "err", err)
 			} else {
+				hashes = append(hashes, hash)
 				txs = append(txs, encoded)
 				bytes += len(encoded)
 			}
 		}
-		return p.SendTransactionRLP(txs)
+		return p.SendPooledTransactionsRLP(hashes, txs)
 
-	case msg.Code == TxMsg:
+	case msg.Code == TransactionMsg || (msg.Code == PooledTransactionsMsg && p.version >= eth65):
 		// Transactions arrived, make sure we have a valid and fresh chain to handle them
 		if atomic.LoadUint32(&pm.acceptTxs) == 0 {
 			break
@@ -806,7 +805,7 @@ func (pm *ProtocolManager) handleMsg(p *peer) error {
 			}
 			p.MarkTransaction(tx.Hash())
 		}
-		pm.txFetcher.EnqueueTxs(p.id, txs)
+		pm.txFetcher.Enqueue(p.id, txs, msg.Code == PooledTransactionsMsg)
 
 	default:
 		return errResp(ErrInvalidMsgCode, "%v", msg.Code)
@@ -854,9 +853,9 @@ func (pm *ProtocolManager) BroadcastBlock(block *types.Block, propagate bool) {
 	}
 }
 
-// BroadcastTxs will propagate a batch of transactions to all peers which are not known to
+// BroadcastTransactions will propagate a batch of transactions to all peers which are not known to
 // already have the given transaction.
-func (pm *ProtocolManager) BroadcastTxs(txs types.Transactions, propagate bool) {
+func (pm *ProtocolManager) BroadcastTransactions(txs types.Transactions, propagate bool) {
 	var (
 		txset = make(map[*peer][]common.Hash)
 		annos = make(map[*peer][]common.Hash)
@@ -894,7 +893,7 @@ func (pm *ProtocolManager) BroadcastTxs(txs types.Transactions, propagate bool)
 	}
 	for peer, hashes := range annos {
 		if peer.version >= eth65 {
-			peer.AsyncSendTransactionHashes(hashes)
+			peer.AsyncSendPooledTransactionHashes(hashes)
 		} else {
 			peer.AsyncSendTransactions(hashes)
 		}
@@ -918,11 +917,11 @@ func (pm *ProtocolManager) txBroadcastLoop() {
 		case event := <-pm.txsCh:
 			// For testing purpose only, disable propagation
 			if pm.broadcastTxAnnouncesOnly {
-				pm.BroadcastTxs(event.Txs, false)
+				pm.BroadcastTransactions(event.Txs, false)
 				continue
 			}
-			pm.BroadcastTxs(event.Txs, true)  // First propagate transactions to peers
-			pm.BroadcastTxs(event.Txs, false) // Only then announce to the rest
+			pm.BroadcastTransactions(event.Txs, true)  // First propagate transactions to peers
+			pm.BroadcastTransactions(event.Txs, false) // Only then announce to the rest
 
 		// Err() channel will be closed when unsubscribing.
 		case <-pm.txsSub.Err():

eth/metrics.go

-// Copyright 2015 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 eth
-
-import (
-	"github.com/ethereum/go-ethereum/metrics"
-	"github.com/ethereum/go-ethereum/p2p"
-)
-
-var (
-	propTxnInPacketsMeter     = metrics.NewRegisteredMeter("eth/prop/txns/in/packets", nil)
-	propTxnInTrafficMeter     = metrics.NewRegisteredMeter("eth/prop/txns/in/traffic", nil)
-	propTxnOutPacketsMeter    = metrics.NewRegisteredMeter("eth/prop/txns/out/packets", nil)
-	propTxnOutTrafficMeter    = metrics.NewRegisteredMeter("eth/prop/txns/out/traffic", nil)
-	propHashInPacketsMeter    = metrics.NewRegisteredMeter("eth/prop/hashes/in/packets", nil)
-	propHashInTrafficMeter    = metrics.NewRegisteredMeter("eth/prop/hashes/in/traffic", nil)
-	propHashOutPacketsMeter   = metrics.NewRegisteredMeter("eth/prop/hashes/out/packets", nil)
-	propHashOutTrafficMeter   = metrics.NewRegisteredMeter("eth/prop/hashes/out/traffic", nil)
-	propBlockInPacketsMeter   = metrics.NewRegisteredMeter("eth/prop/blocks/in/packets", nil)
-	propBlockInTrafficMeter   = metrics.NewRegisteredMeter("eth/prop/blocks/in/traffic", nil)
-	propBlockOutPacketsMeter  = metrics.NewRegisteredMeter("eth/prop/blocks/out/packets", nil)
-	propBlockOutTrafficMeter  = metrics.NewRegisteredMeter("eth/prop/blocks/out/traffic", nil)
-	reqHeaderInPacketsMeter   = metrics.NewRegisteredMeter("eth/req/headers/in/packets", nil)
-	reqHeaderInTrafficMeter   = metrics.NewRegisteredMeter("eth/req/headers/in/traffic", nil)
-	reqHeaderOutPacketsMeter  = metrics.NewRegisteredMeter("eth/req/headers/out/packets", nil)
-	reqHeaderOutTrafficMeter  = metrics.NewRegisteredMeter("eth/req/headers/out/traffic", nil)
-	reqBodyInPacketsMeter     = metrics.NewRegisteredMeter("eth/req/bodies/in/packets", nil)
-	reqBodyInTrafficMeter     = metrics.NewRegisteredMeter("eth/req/bodies/in/traffic", nil)
-	reqBodyOutPacketsMeter    = metrics.NewRegisteredMeter("eth/req/bodies/out/packets", nil)
-	reqBodyOutTrafficMeter    = metrics.NewRegisteredMeter("eth/req/bodies/out/traffic", nil)
-	reqStateInPacketsMeter    = metrics.NewRegisteredMeter("eth/req/states/in/packets", nil)
-	reqStateInTrafficMeter    = metrics.NewRegisteredMeter("eth/req/states/in/traffic", nil)
-	reqStateOutPacketsMeter   = metrics.NewRegisteredMeter("eth/req/states/out/packets", nil)
-	reqStateOutTrafficMeter   = metrics.NewRegisteredMeter("eth/req/states/out/traffic", nil)
-	reqReceiptInPacketsMeter  = metrics.NewRegisteredMeter("eth/req/receipts/in/packets", nil)
-	reqReceiptInTrafficMeter  = metrics.NewRegisteredMeter("eth/req/receipts/in/traffic", nil)
-	reqReceiptOutPacketsMeter = metrics.NewRegisteredMeter("eth/req/receipts/out/packets", nil)
-	reqReceiptOutTrafficMeter = metrics.NewRegisteredMeter("eth/req/receipts/out/traffic", nil)
-	miscInPacketsMeter        = metrics.NewRegisteredMeter("eth/misc/in/packets", nil)
-	miscInTrafficMeter        = metrics.NewRegisteredMeter("eth/misc/in/traffic", nil)
-	miscOutPacketsMeter       = metrics.NewRegisteredMeter("eth/misc/out/packets", nil)
-	miscOutTrafficMeter       = metrics.NewRegisteredMeter("eth/misc/out/traffic", nil)
-)
-
-// meteredMsgReadWriter is a wrapper around a p2p.MsgReadWriter, capable of
-// accumulating the above defined metrics based on the data stream contents.
-type meteredMsgReadWriter struct {
-	p2p.MsgReadWriter     // Wrapped message stream to meter
-	version           int // Protocol version to select correct meters
-}
-
-// newMeteredMsgWriter wraps a p2p MsgReadWriter with metering support. If the
-// metrics system is disabled, this function returns the original object.
-func newMeteredMsgWriter(rw p2p.MsgReadWriter) p2p.MsgReadWriter {
-	if !metrics.Enabled {
-		return rw
-	}
-	return &meteredMsgReadWriter{MsgReadWriter: rw}
-}
-
-// Init sets the protocol version used by the stream to know which meters to
-// increment in case of overlapping message ids between protocol versions.
-func (rw *meteredMsgReadWriter) Init(version int) {
-	rw.version = version
-}
-
-func (rw *meteredMsgReadWriter) ReadMsg() (p2p.Msg, error) {
-	// Read the message and short circuit in case of an error
-	msg, err := rw.MsgReadWriter.ReadMsg()
-	if err != nil {
-		return msg, err
-	}
-	// Account for the data traffic
-	packets, traffic := miscInPacketsMeter, miscInTrafficMeter
-	switch {
-	case msg.Code == BlockHeadersMsg:
-		packets, traffic = reqHeaderInPacketsMeter, reqHeaderInTrafficMeter
-	case msg.Code == BlockBodiesMsg:
-		packets, traffic = reqBodyInPacketsMeter, reqBodyInTrafficMeter
-
-	case rw.version >= eth63 && msg.Code == NodeDataMsg:
-		packets, traffic = reqStateInPacketsMeter, reqStateInTrafficMeter
-	case rw.version >= eth63 && msg.Code == ReceiptsMsg:
-		packets, traffic = reqReceiptInPacketsMeter, reqReceiptInTrafficMeter
-
-	case msg.Code == NewBlockHashesMsg:
-		packets, traffic = propHashInPacketsMeter, propHashInTrafficMeter
-	case msg.Code == NewBlockMsg:
-		packets, traffic = propBlockInPacketsMeter, propBlockInTrafficMeter
-	case msg.Code == TxMsg:
-		packets, traffic = propTxnInPacketsMeter, propTxnInTrafficMeter
-	}
-	packets.Mark(1)
-	traffic.Mark(int64(msg.Size))
-
-	return msg, err
-}
-
-func (rw *meteredMsgReadWriter) WriteMsg(msg p2p.Msg) error {
-	// Account for the data traffic
-	packets, traffic := miscOutPacketsMeter, miscOutTrafficMeter
-	switch {
-	case msg.Code == BlockHeadersMsg:
-		packets, traffic = reqHeaderOutPacketsMeter, reqHeaderOutTrafficMeter
-	case msg.Code == BlockBodiesMsg:
-		packets, traffic = reqBodyOutPacketsMeter, reqBodyOutTrafficMeter
-
-	case rw.version >= eth63 && msg.Code == NodeDataMsg:
-		packets, traffic = reqStateOutPacketsMeter, reqStateOutTrafficMeter
-	case rw.version >= eth63 && msg.Code == ReceiptsMsg:
-		packets, traffic = reqReceiptOutPacketsMeter, reqReceiptOutTrafficMeter
-
-	case msg.Code == NewBlockHashesMsg:
-		packets, traffic = propHashOutPacketsMeter, propHashOutTrafficMeter
-	case msg.Code == NewBlockMsg:
-		packets, traffic = propBlockOutPacketsMeter, propBlockOutTrafficMeter
-	case msg.Code == TxMsg:
-		packets, traffic = propTxnOutPacketsMeter, propTxnOutTrafficMeter
-	}
-	packets.Mark(1)
-	traffic.Mark(int64(msg.Size))
-
-	// Send the packet to the p2p layer
-	return rw.MsgReadWriter.WriteMsg(msg)
-}

eth/peer.go

@@ -27,7 +27,6 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/forkid"
 	"github.com/ethereum/go-ethereum/core/types"
-	"github.com/ethereum/go-ethereum/eth/fetcher"
 	"github.com/ethereum/go-ethereum/p2p"
 	"github.com/ethereum/go-ethereum/rlp"
 )
@@ -43,17 +42,13 @@ const (
 	maxKnownBlocks = 1024  // Maximum block hashes to keep in the known list (prevent DOS)
 
 	// maxQueuedTxs is the maximum number of transactions to queue up before dropping
-	// broadcasts.
+	// older broadcasts.
 	maxQueuedTxs = 4096
 
 	// maxQueuedTxAnns is the maximum number of transaction announcements to queue up
-	// before dropping broadcasts.
+	// before dropping older announcements.
 	maxQueuedTxAnns = 4096
 
-	// maxQueuedTxRetrieval is the maximum number of tx retrieval requests to queue up
-	// before dropping requests.
-	maxQueuedTxRetrieval = 4096
-
 	// maxQueuedBlocks is the maximum number of block propagations to queue up before
 	// dropping broadcasts. There's not much point in queueing stale blocks, so a few
 	// that might cover uncles should be enough.
@@ -102,14 +97,15 @@ type peer struct {
 	td   *big.Int
 	lock sync.RWMutex
 
-	knownTxs        mapset.Set                           // Set of transaction hashes known to be known by this peer
 	knownBlocks     mapset.Set        // Set of block hashes known to be known by this peer
 	queuedBlocks    chan *propEvent   // Queue of blocks to broadcast to the peer
 	queuedBlockAnns chan *types.Block // Queue of blocks to announce to the peer
-	txPropagation   chan []common.Hash                   // Channel used to queue transaction propagation requests
+
+	knownTxs    mapset.Set                           // Set of transaction hashes known to be known by this peer
+	txBroadcast chan []common.Hash                   // Channel used to queue transaction propagation requests
 	txAnnounce  chan []common.Hash                   // Channel used to queue transaction announcement requests
-	txRetrieval     chan []common.Hash                   // Channel used to queue transaction retrieval requests
 	getPooledTx func(common.Hash) *types.Transaction // Callback used to retrieve transaction from txpool
+
 	term chan struct{} // Termination channel to stop the broadcaster
 }
 
@@ -123,17 +119,16 @@ func newPeer(version int, p *p2p.Peer, rw p2p.MsgReadWriter, getPooledTx func(ha
 		knownBlocks:     mapset.NewSet(),
 		queuedBlocks:    make(chan *propEvent, maxQueuedBlocks),
 		queuedBlockAnns: make(chan *types.Block, maxQueuedBlockAnns),
-		txPropagation:   make(chan []common.Hash),
+		txBroadcast:     make(chan []common.Hash),
 		txAnnounce:      make(chan []common.Hash),
-		txRetrieval:     make(chan []common.Hash),
 		getPooledTx:     getPooledTx,
 		term:            make(chan struct{}),
 	}
 }
 
-// broadcastBlocks is a write loop that multiplexes block propagations,
-// announcements into the remote peer. The goal is to have an async writer
-// that does not lock up node internals.
+// broadcastBlocks is a write loop that multiplexes blocks and block accouncements
+// to the remote peer. The goal is to have an async writer that does not lock up
+// node internals and at the same time rate limits queued data.
 func (p *peer) broadcastBlocks() {
 	for {
 		select {
@@ -155,105 +150,60 @@ func (p *peer) broadcastBlocks() {
 	}
 }
 
-// broadcastTxs is a write loop that multiplexes transaction propagations,
-// announcements into the remote peer. The goal is to have an async writer
-// that does not lock up node internals.
-func (p *peer) broadcastTxs() {
+// broadcastTransactions is a write loop that schedules transaction broadcasts
+// to the remote peer. The goal is to have an async writer that does not lock up
+// node internals and at the same time rate limits queued data.
+func (p *peer) broadcastTransactions() {
 	var (
-		txProps []common.Hash      // Queue of transaction propagations to the peer
-		txAnnos []common.Hash      // Queue of transaction announcements to the peer
-		done    chan struct{}      // Non-nil if background network sender routine is active.
-		errch   = make(chan error) // Channel used to receive network error
+		queue []common.Hash      // Queue of hashes to broadcast as full transactions
+		done  chan struct{}      // Non-nil if background broadcaster is running
+		fail  = make(chan error) // Channel used to receive network error
 	)
-	scheduleTask := func() {
-		// Short circuit if there already has a inflight task.
-		if done != nil {
-			return
-		}
-		// Spin up transaction propagation task if there is any
-		// queued hashes.
-		if len(txProps) > 0 {
+	for {
+		// If there's no in-flight broadcast running, check if a new one is needed
+		if done == nil && len(queue) > 0 {
+			// Pile transaction until we reach our allowed network limit
 			var (
 				hashes []common.Hash
 				txs    []*types.Transaction
 				size   common.StorageSize
 			)
-			for i := 0; i < len(txProps) && size < txsyncPackSize; i++ {
-				if tx := p.getPooledTx(txProps[i]); tx != nil {
+			for i := 0; i < len(queue) && size < txsyncPackSize; i++ {
+				if tx := p.getPooledTx(queue[i]); tx != nil {
 					txs = append(txs, tx)
 					size += tx.Size()
 				}
-				hashes = append(hashes, txProps[i])
+				hashes = append(hashes, queue[i])
 			}
-			txProps = txProps[:copy(txProps, txProps[len(hashes):])]
+			queue = queue[:copy(queue, queue[len(hashes):])]
+
+			// If there's anything available to transfer, fire up an async writer
 			if len(txs) > 0 {
 				done = make(chan struct{})
 				go func() {
-					if err := p.SendNewTransactions(txs); err != nil {
-						errch <- err
+					if err := p.sendTransactions(txs); err != nil {
+						fail <- err
 						return
 					}
 					close(done)
 					p.Log().Trace("Sent transactions", "count", len(txs))
 				}()
-				return
-			}
-		}
-		// Spin up transaction announcement task if there is any
-		// queued hashes.
-		if len(txAnnos) > 0 {
-			var (
-				hashes  []common.Hash
-				pending []common.Hash
-				size    common.StorageSize
-			)
-			for i := 0; i < len(txAnnos) && size < txsyncPackSize; i++ {
-				if tx := p.getPooledTx(txAnnos[i]); tx != nil {
-					pending = append(pending, txAnnos[i])
-					size += common.HashLength
-				}
-				hashes = append(hashes, txAnnos[i])
-			}
-			txAnnos = txAnnos[:copy(txAnnos, txAnnos[len(hashes):])]
-			if len(pending) > 0 {
-				done = make(chan struct{})
-				go func() {
-					if err := p.SendNewTransactionHashes(pending); err != nil {
-						errch <- err
-						return
-					}
-					close(done)
-					p.Log().Trace("Sent transaction announcements", "count", len(pending))
-				}()
-			}
 			}
 		}
-
-	for {
-		scheduleTask()
+		// Transfer goroutine may or may not have been started, listen for events
 		select {
-		case hashes := <-p.txPropagation:
-			if len(txProps) == maxQueuedTxs {
-				continue
-			}
-			if len(txProps)+len(hashes) > maxQueuedTxs {
-				hashes = hashes[:maxQueuedTxs-len(txProps)]
+		case hashes := <-p.txBroadcast:
+			// New batch of transactions to be broadcast, queue them (with cap)
+			queue = append(queue, hashes...)
+			if len(queue) > maxQueuedTxs {
+				// Fancy copy and resize to ensure buffer doesn't grow indefinitely
+				queue = queue[:copy(queue, queue[len(queue)-maxQueuedTxs:])]
 			}
-			txProps = append(txProps, hashes...)
-
-		case hashes := <-p.txAnnounce:
-			if len(txAnnos) == maxQueuedTxAnns {
-				continue
-			}
-			if len(txAnnos)+len(hashes) > maxQueuedTxAnns {
-				hashes = hashes[:maxQueuedTxAnns-len(txAnnos)]
-			}
-			txAnnos = append(txAnnos, hashes...)
 
 		case <-done:
 			done = nil
 
-		case <-errch:
+		case <-fail:
 			return
 
 		case <-p.term:
@@ -262,60 +212,60 @@ func (p *peer) broadcastTxs() {
 	}
 }
 
-// retrievalTxs is a write loop which is responsible for retrieving transaction
-// from the remote peer. The goal is to have an async writer that does not lock
-// up node internals. If there are too many requests queued, then new arrival
-// requests will be dropped silently so that we can ensure the memory assumption
-// is fixed for each peer.
-func (p *peer) retrievalTxs() {
+// announceTransactions is a write loop that schedules transaction broadcasts
+// to the remote peer. The goal is to have an async writer that does not lock up
+// node internals and at the same time rate limits queued data.
+func (p *peer) announceTransactions() {
 	var (
-		requests []common.Hash      // Queue of transaction requests to the peer
-		done     chan struct{}      // Non-nil if background network sender routine is active.
-		errch    = make(chan error) // Channel used to receive network error
+		queue []common.Hash      // Queue of hashes to announce as transaction stubs
+		done  chan struct{}      // Non-nil if background announcer is running
+		fail  = make(chan error) // Channel used to receive network error
 	)
-	// pick chooses a reasonble number of transaction hashes for retrieval.
-	pick := func() []common.Hash {
-		var ret []common.Hash
-		if len(requests) > fetcher.MaxTransactionFetch {
-			ret = requests[:fetcher.MaxTransactionFetch]
-		} else {
-			ret = requests[:]
-		}
-		requests = requests[:copy(requests, requests[len(ret):])]
-		return ret
-	}
-	// send sends transactions retrieval request.
-	send := func(hashes []common.Hash, done chan struct{}) {
-		if err := p.RequestTxs(hashes); err != nil {
-			errch <- err
+	for {
+		// If there's no in-flight announce running, check if a new one is needed
+		if done == nil && len(queue) > 0 {
+			// Pile transaction hashes until we reach our allowed network limit
+			var (
+				hashes  []common.Hash
+				pending []common.Hash
+				size    common.StorageSize
+			)
+			for i := 0; i < len(queue) && size < txsyncPackSize; i++ {
+				if p.getPooledTx(queue[i]) != nil {
+					pending = append(pending, queue[i])
+					size += common.HashLength
+				}
+				hashes = append(hashes, queue[i])
+			}
+			queue = queue[:copy(queue, queue[len(hashes):])]
+
+			// If there's anything available to transfer, fire up an async writer
+			if len(pending) > 0 {
+				done = make(chan struct{})
+				go func() {
+					if err := p.sendPooledTransactionHashes(pending); err != nil {
+						fail <- err
 						return
 					}
 					close(done)
-		p.Log().Trace("Sent transaction retrieval request", "count", len(hashes))
-	}
-	for {
-		select {
-		case hashes := <-p.txRetrieval:
-			if len(requests) == maxQueuedTxRetrieval {
-				continue
+					p.Log().Trace("Sent transaction announcements", "count", len(pending))
+				}()
 			}
-			if len(requests)+len(hashes) > maxQueuedTxRetrieval {
-				hashes = hashes[:maxQueuedTxRetrieval-len(requests)]
 		}
-			requests = append(requests, hashes...)
-			if done == nil {
-				done = make(chan struct{})
-				go send(pick(), done)
+		// Transfer goroutine may or may not have been started, listen for events
+		select {
+		case hashes := <-p.txAnnounce:
+			// New batch of transactions to be broadcast, queue them (with cap)
+			queue = append(queue, hashes...)
+			if len(queue) > maxQueuedTxAnns {
+				// Fancy copy and resize to ensure buffer doesn't grow indefinitely
+				queue = queue[:copy(queue, queue[len(queue)-maxQueuedTxs:])]
 			}
 
 		case <-done:
 			done = nil
-			if pending := pick(); len(pending) > 0 {
-				done = make(chan struct{})
-				go send(pending, done)
-			}
 
-		case <- errch:
+		case <-fail:
 			return
 
 		case <-p.term:
@@ -379,22 +329,22 @@ func (p *peer) MarkTransaction(hash common.Hash) {
 	p.knownTxs.Add(hash)
 }
 
-// SendNewTransactionHashes sends a batch of transaction hashes to the peer and
-// includes the hashes in its transaction hash set for future reference.
-func (p *peer) SendNewTransactionHashes(hashes []common.Hash) error {
-	// Mark all the transactions as known, but ensure we don't overflow our limits
-	for p.knownTxs.Cardinality() > max(0, maxKnownTxs-len(hashes)) {
-		p.knownTxs.Pop()
-	}
-	for _, hash := range hashes {
-		p.knownTxs.Add(hash)
-	}
-	return p2p.Send(p.rw, NewPooledTransactionHashesMsg, hashes)
+// SendTransactions64 sends transactions to the peer and includes the hashes
+// in its transaction hash set for future reference.
+//
+// This method is legacy support for initial transaction exchange in eth/64 and
+// prior. For eth/65 and higher use SendPooledTransactionHashes.
+func (p *peer) SendTransactions64(txs types.Transactions) error {
+	return p.sendTransactions(txs)
 }
 
-// SendNewTransactions sends transactions to the peer and includes the hashes
+// sendTransactions sends transactions to the peer and includes the hashes
 // in its transaction hash set for future reference.
-func (p *peer) SendNewTransactions(txs types.Transactions) error {
+//
+// This method is a helper used by the async transaction sender. Don't call it
+// directly as the queueing (memory) and transmission (bandwidth) costs should
+// not be managed directly.
+func (p *peer) sendTransactions(txs types.Transactions) error {
 	// Mark all the transactions as known, but ensure we don't overflow our limits
 	for p.knownTxs.Cardinality() > max(0, maxKnownTxs-len(txs)) {
 		p.knownTxs.Pop()
@@ -402,18 +352,15 @@ func (p *peer) SendNewTransactions(txs types.Transactions) error {
 	for _, tx := range txs {
 		p.knownTxs.Add(tx.Hash())
 	}
-	return p2p.Send(p.rw, TxMsg, txs)
-}
-
-func (p *peer) SendTransactionRLP(txs []rlp.RawValue) error {
-	return p2p.Send(p.rw, TxMsg, txs)
+	return p2p.Send(p.rw, TransactionMsg, txs)
 }
 
-// AsyncSendTransactions queues list of transactions propagation to a remote
-// peer. If the peer's broadcast queue is full, the event is silently dropped.
+// AsyncSendTransactions queues a list of transactions (by hash) to eventually
+// propagate to a remote peer. The number of pending sends are capped (new ones
+// will force old sends to be dropped)
 func (p *peer) AsyncSendTransactions(hashes []common.Hash) {
 	select {
-	case p.txPropagation <- hashes:
+	case p.txBroadcast <- hashes:
 		// Mark all the transactions as known, but ensure we don't overflow our limits
 		for p.knownTxs.Cardinality() > max(0, maxKnownTxs-len(hashes)) {
 			p.knownTxs.Pop()
@@ -426,9 +373,27 @@ func (p *peer) AsyncSendTransactions(hashes []common.Hash) {
 	}
 }
 
-// AsyncSendTransactions queues list of transactions propagation to a remote
-// peer. If the peer's broadcast queue is full, the event is silently dropped.
-func (p *peer) AsyncSendTransactionHashes(hashes []common.Hash) {
+// sendPooledTransactionHashes sends transaction hashes to the peer and includes
+// them in its transaction hash set for future reference.
+//
+// This method is a helper used by the async transaction announcer. Don't call it
+// directly as the queueing (memory) and transmission (bandwidth) costs should
+// not be managed directly.
+func (p *peer) sendPooledTransactionHashes(hashes []common.Hash) error {
+	// Mark all the transactions as known, but ensure we don't overflow our limits
+	for p.knownTxs.Cardinality() > max(0, maxKnownTxs-len(hashes)) {
+		p.knownTxs.Pop()
+	}
+	for _, hash := range hashes {
+		p.knownTxs.Add(hash)
+	}
+	return p2p.Send(p.rw, NewPooledTransactionHashesMsg, hashes)
+}
+
+// AsyncSendPooledTransactionHashes queues a list of transactions hashes to eventually
+// announce to a remote peer.  The number of pending sends are capped (new ones
+// will force old sends to be dropped)
+func (p *peer) AsyncSendPooledTransactionHashes(hashes []common.Hash) {
 	select {
 	case p.txAnnounce <- hashes:
 		// Mark all the transactions as known, but ensure we don't overflow our limits
@@ -443,6 +408,22 @@ func (p *peer) AsyncSendTransactionHashes(hashes []common.Hash) {
 	}
 }
 
+// SendPooledTransactionsRLP sends requested transactions to the peer and adds the
+// hashes in its transaction hash set for future reference.
+//
+// Note, the method assumes the hashes are correct and correspond to the list of
+// transactions being sent.
+func (p *peer) SendPooledTransactionsRLP(hashes []common.Hash, txs []rlp.RawValue) error {
+	// Mark all the transactions as known, but ensure we don't overflow our limits
+	for p.knownTxs.Cardinality() > max(0, maxKnownTxs-len(hashes)) {
+		p.knownTxs.Pop()
+	}
+	for _, hash := range hashes {
+		p.knownTxs.Add(hash)
+	}
+	return p2p.Send(p.rw, PooledTransactionsMsg, txs)
+}
+
 // SendNewBlockHashes announces the availability of a number of blocks through
 // a hash notification.
 func (p *peer) SendNewBlockHashes(hashes []common.Hash, numbers []uint64) error {
@@ -577,16 +558,6 @@ func (p *peer) RequestTxs(hashes []common.Hash) error {
 	return p2p.Send(p.rw, GetPooledTransactionsMsg, hashes)
 }
 
-// AsyncRequestTxs queues a tx retrieval request to a remote peer. If
-// the peer's retrieval queue is full, the event is silently dropped.
-func (p *peer) AsyncRequestTxs(hashes []common.Hash) {
-	select {
-	case p.txRetrieval <- hashes:
-	case <-p.term:
-		p.Log().Debug("Dropping transaction retrieval request", "count", len(hashes))
-	}
-}
-
 // Handshake executes the eth protocol handshake, negotiating version number,
 // network IDs, difficulties, head and genesis blocks.
 func (p *peer) Handshake(network uint64, td *big.Int, head common.Hash, genesis common.Hash, forkID forkid.ID, forkFilter forkid.Filter) error {
@@ -746,9 +717,10 @@ func (ps *peerSet) Register(p *peer) error {
 		return errAlreadyRegistered
 	}
 	ps.peers[p.id] = p
+
 	go p.broadcastBlocks()
-	go p.broadcastTxs()
-	go p.retrievalTxs()
+	go p.broadcastTransactions()
+	go p.announceTransactions()
 
 	return nil
 }

eth/protocol.go

@@ -51,7 +51,7 @@ const protocolMaxMsgSize = 10 * 1024 * 1024 // Maximum cap on the size of a prot
 const (
 	StatusMsg          = 0x00
 	NewBlockHashesMsg  = 0x01
-	TxMsg              = 0x02
+	TransactionMsg     = 0x02
 	GetBlockHeadersMsg = 0x03
 	BlockHeadersMsg    = 0x04
 	GetBlockBodiesMsg  = 0x05
@@ -64,10 +64,11 @@ const (
 
 	// New protocol message codes introduced in eth65
 	//
-	// Previously these message ids(0x08, 0x09) were used by some
-	// legacy and unsupported eth protocols, reown them here.
+	// Previously these message ids were used by some legacy and unsupported
+	// eth protocols, reown them here.
 	NewPooledTransactionHashesMsg = 0x08
 	GetPooledTransactionsMsg      = 0x09
+	PooledTransactionsMsg         = 0x0a
 )
 
 type errCode int

eth/protocol_test.go

@@ -62,7 +62,7 @@ func TestStatusMsgErrors63(t *testing.T) {
 		wantError error
 	}{
 		{
-			code: TxMsg, data: []interface{}{},
+			code: TransactionMsg, data: []interface{}{},
 			wantError: errResp(ErrNoStatusMsg, "first msg has code 2 (!= 0)"),
 		},
 		{
@@ -114,7 +114,7 @@ func TestStatusMsgErrors64(t *testing.T) {
 		wantError error
 	}{
 		{
-			code: TxMsg, data: []interface{}{},
+			code: TransactionMsg, data: []interface{}{},
 			wantError: errResp(ErrNoStatusMsg, "first msg has code 2 (!= 0)"),
 		},
 		{
@@ -258,7 +258,7 @@ func testRecvTransactions(t *testing.T, protocol int) {
 	defer p.close()
 
 	tx := newTestTransaction(testAccount, 0, 0)
-	if err := p2p.Send(p.app, TxMsg, []interface{}{tx}); err != nil {
+	if err := p2p.Send(p.app, TransactionMsg, []interface{}{tx}); err != nil {
 		t.Fatalf("send error: %v", err)
 	}
 	select {
@@ -282,13 +282,16 @@ func testSendTransactions(t *testing.T, protocol int) {
 	pm, _ := newTestProtocolManagerMust(t, downloader.FullSync, 0, nil, nil)
 	defer pm.Stop()
 
-	// Fill the pool with big transactions.
+	// Fill the pool with big transactions (use a subscription to wait until all
+	// the transactions are announced to avoid spurious events causing extra
+	// broadcasts).
 	const txsize = txsyncPackSize / 10
 	alltxs := make([]*types.Transaction, 100)
 	for nonce := range alltxs {
 		alltxs[nonce] = newTestTransaction(testAccount, uint64(nonce), txsize)
 	}
 	pm.txpool.AddRemotes(alltxs)
+	time.Sleep(100 * time.Millisecond) // Wait until new tx even gets out of the system (lame)
 
 	// Connect several peers. They should all receive the pending transactions.
 	var wg sync.WaitGroup
@@ -300,8 +303,6 @@ func testSendTransactions(t *testing.T, protocol int) {
 			seen[tx.Hash()] = false
 		}
 		for n := 0; n < len(alltxs) && !t.Failed(); {
-			var txs []*types.Transaction
-			var hashes []common.Hash
 			var forAllHashes func(callback func(hash common.Hash))
 			switch protocol {
 			case 63:
@@ -310,11 +311,15 @@ func testSendTransactions(t *testing.T, protocol int) {
 				msg, err := p.app.ReadMsg()
 				if err != nil {
 					t.Errorf("%v: read error: %v", p.Peer, err)
-				} else if msg.Code != TxMsg {
+					continue
+				} else if msg.Code != TransactionMsg {
 					t.Errorf("%v: got code %d, want TxMsg", p.Peer, msg.Code)
+					continue
 				}
+				var txs []*types.Transaction
 				if err := msg.Decode(&txs); err != nil {
 					t.Errorf("%v: %v", p.Peer, err)
+					continue
 				}
 				forAllHashes = func(callback func(hash common.Hash)) {
 					for _, tx := range txs {
@@ -325,11 +330,15 @@ func testSendTransactions(t *testing.T, protocol int) {
 				msg, err := p.app.ReadMsg()
 				if err != nil {
 					t.Errorf("%v: read error: %v", p.Peer, err)
+					continue
 				} else if msg.Code != NewPooledTransactionHashesMsg {
-					t.Errorf("%v: got code %d, want TxMsg", p.Peer, msg.Code)
+					t.Errorf("%v: got code %d, want NewPooledTransactionHashesMsg", p.Peer, msg.Code)
+					continue
 				}
+				var hashes []common.Hash
 				if err := msg.Decode(&hashes); err != nil {
 					t.Errorf("%v: %v", p.Peer, err)
+					continue
 				}
 				forAllHashes = func(callback func(hash common.Hash)) {
 					for _, h := range hashes {

eth/sync.go

@@ -39,12 +39,17 @@ const (
 
 type txsync struct {
 	p   *peer
-	hashes []common.Hash
 	txs []*types.Transaction
 }
 
 // syncTransactions starts sending all currently pending transactions to the given peer.
 func (pm *ProtocolManager) syncTransactions(p *peer) {
+	// Assemble the set of transaction to broadcast or announce to the remote
+	// peer. Fun fact, this is quite an expensive operation as it needs to sort
+	// the transactions if the sorting is not cached yet. However, with a random
+	// order, insertions could overflow the non-executable queues and get dropped.
+	//
+	// TODO(karalabe): Figure out if we could get away with random order somehow
 	var txs types.Transactions
 	pending, _ := pm.txpool.Pending()
 	for _, batch := range pending {
@@ -53,17 +58,29 @@ func (pm *ProtocolManager) syncTransactions(p *peer) {
 	if len(txs) == 0 {
 		return
 	}
+	// The eth/65 protocol introduces proper transaction announcements, so instead
+	// of dripping transactions across multiple peers, just send the entire list as
+	// an announcement and let the remote side decide what they need (likely nothing).
+	if p.version >= eth65 {
+		hashes := make([]common.Hash, len(txs))
+		for i, tx := range txs {
+			hashes[i] = tx.Hash()
+		}
+		p.AsyncSendPooledTransactionHashes(hashes)
+		return
+	}
+	// Out of luck, peer is running legacy protocols, drop the txs over
 	select {
 	case pm.txsyncCh <- &txsync{p: p, txs: txs}:
 	case <-pm.quitSync:
 	}
 }
 
-// txsyncLoop takes care of the initial transaction sync for each new
+// txsyncLoop64 takes care of the initial transaction sync for each new
 // connection. When a new peer appears, we relay all currently pending
 // transactions. In order to minimise egress bandwidth usage, we send
 // the transactions in small packs to one peer at a time.
-func (pm *ProtocolManager) txsyncLoop() {
+func (pm *ProtocolManager) txsyncLoop64() {
 	var (
 		pending = make(map[enode.ID]*txsync)
 		sending = false               // whether a send is active
@@ -72,30 +89,13 @@ func (pm *ProtocolManager) txsyncLoop() {
 	)
 	// send starts a sending a pack of transactions from the sync.
 	send := func(s *txsync) {
+		if s.p.version >= eth65 {
+			panic("initial transaction syncer running on eth/65+")
+		}
 		// Fill pack with transactions up to the target size.
 		size := common.StorageSize(0)
 		pack.p = s.p
-		pack.hashes = pack.hashes[:0]
 		pack.txs = pack.txs[:0]
-		if s.p.version >= eth65 {
-			// Eth65 introduces transaction announcement https://github.com/ethereum/EIPs/pull/2464,
-			// only txhashes are transferred here.
-			for i := 0; i < len(s.txs) && size < txsyncPackSize; i++ {
-				pack.hashes = append(pack.hashes, s.txs[i].Hash())
-				size += common.HashLength
-			}
-			// Remove the transactions that will be sent.
-			s.txs = s.txs[:copy(s.txs, s.txs[len(pack.hashes):])]
-			if len(s.txs) == 0 {
-				delete(pending, s.p.ID())
-			}
-			// Send the pack in the background.
-			s.p.Log().Trace("Sending batch of transaction announcements", "count", len(pack.hashes), "bytes", size)
-			sending = true
-			go func() { done <- pack.p.SendNewTransactionHashes(pack.hashes) }()
-		} else {
-			// Legacy eth protocol doesn't have transaction announcement protocol
-			// message, transfer the whole pending transaction slice.
 		for i := 0; i < len(s.txs) && size < txsyncPackSize; i++ {
 			pack.txs = append(pack.txs, s.txs[i])
 			size += s.txs[i].Size()
@@ -108,8 +108,7 @@ func (pm *ProtocolManager) txsyncLoop() {
 		// Send the pack in the background.
 		s.p.Log().Trace("Sending batch of transactions", "count", len(pack.txs), "bytes", size)
 		sending = true
-			go func() { done <- pack.p.SendNewTransactions(pack.txs) }()
-		}
+		go func() { done <- pack.p.SendTransactions64(pack.txs) }()
 	}
 
 	// pick chooses the next pending sync.

fuzzbuzz.yaml

@@ -26,6 +26,14 @@ targets:
       function: Fuzz
       package: github.com/ethereum/go-ethereum/tests/fuzzers/trie
       checkout: github.com/ethereum/go-ethereum/
+  - name: txfetcher
+    language: go
+    version: "1.13"
+    corpus: ./fuzzers/txfetcher/corpus
+    harness:
+      function: Fuzz
+      package: github.com/ethereum/go-ethereum/tests/fuzzers/txfetcher
+      checkout: github.com/ethereum/go-ethereum/
   - name: whisperv6
     language: go
     version: "1.13"

tests/fuzzers/txfetcher/corpus/0151ee1d0db4c74d3bcdfa4f7396a4c8538748c9-2

+ 
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/021d1144e359233c496e22c3250609b11b213e9f-4

+ TESTING KEY-----
+MIICXgIBAAKBgQDuLnQAI3mDgey3VBzWnB2L39JUU4txjeVE6myuDqkM/uGlfjb9
+SjY1bIw4iAJm2gsvvZhIrCHS3l6afab4pZB
+l2+XsDlrKBxKKtDrGxlG4LjncdabFn9gvLZad2bSysqz/qTAUStTtqJQIDAQAB
+AoGAGRzwwir7XvBOAy5tuV6ef6anZzus1s1Y1Clb6HbnWWF/wbZGOpet
+3m4vD6MXc7jpTLryzTQIvVdfQbRc6+MUVeLKZTXtdZrh+k7hx0nTP8Jcb
+uqFk541awmMogY/EfbWd6IOkp+4xqjlFBEDytgbIECQQDvH/6nk+hgN4H
+qzzVtxxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLmQcj2PprIMmPcQrooz84SHEg1Ak/7KCxmD/sfgS5TeuNi8DoUBEmiSJwm7FX
+ftxuvL7XvjwjN5B30pNEbc6Iuyt7y4MQJBAIt21su43sjXNueLKH8+ph2UfQuU9txblTu14q3N7gHRZB4ZMhFYyDy8CKrN2cPg/Fvyt0Xl
/DoCzjA0CQQDU
+y2pGsuSmgUtWj3NM9xuwYPm+Z/F84K6+ARYiZ6PYj013sovGKUFfYAqVXVlxtIoX
+qUn3Xh9ps8ZfjLZO7BAkEAlT4R5Yl6cGhaJQYZHOde3JMhNRcVFMO8dDaFo
+f9Oeos0UotgiDktdQHxdNEwLjQlJBz+OtwwA=---E RATTIEY-
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/0d28327b1fb52c1ba02a6eb96675c31633921bb2-2

+&^oȗ-----BEGIN RSA TESTING KEY-----
+MIICXgIBAAKBgQDuLnQAI3mDgey3VBzWnB2L39JUU4txjeVE6myuDqkM/uGlfjb9
+SjY1bIw4iA5sBBZzHi3z0h1YV8QPuxEbi4nW91IJm2gsvvZhIrCHS3l6afab4pZB
+l2+XsDulrKBxKKtD1rGxlG4LjncdabFn9gvLZad2bSysqz/qTAUStTvqJQIDAQAB
+AoGAGRzwwir7XvBOAy5tM/uV6e+Zf6anZzus1s1Y1ClbjbE6HXbnWWF/wbZGOpet
+3Zm4vD6MXc7jpTLryzTQIvVdfQbRc6+MUVeLKwZatTXtdZrhu+Jk7hx0nTPy8Jcb
+uJqFk541aEw+mMogY/xEcfbWd6IOkp+4xqjlFLBEDytgbIECQQDvH/E6nk+hgN4H
+qzzVtxxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLmQcj2PprIMmPcQrooz8vp
+jy4SHEg1AkEA/v13/5M47K9vCxmb8QeD/asydfsgS5TeuNi8DoUBEmiSJwma7FXY
+fFUtxuvL7XvjwjN5B30pNEbc6Iuyt7y4MQJBAIt21su4b3sjXNueLKH85Q+phy2U
+fQtuUE9txblTu14q3N7gHRZB4ZMhFYyDy8CKrN2cPg/Fvyt0Xlp/DoCzjA0CQQDU
+y2ptGsuSmgUtWj3NM9xuwYPm+Z/F84K6+ARYiZ6PYj013sovGKUFfYAqVXVlxtIX
+qyUBnu3X9ps8ZfjLZO7BAkEAlT4R5Yl6cGhaJQYZHOde3JEMhNRcVFMO8dJDaFeo
+f9Oeos0UUothgiDktdQHxdNEwLjQf7lJJBzV+5OtwswCWA==
+-----END RSA TESTING KEY-----Q_
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/0fcd827b57ded58e91f7ba2ac2b7ea4d25ebedca-7

+ap
�

￿���V�#�&��
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/1adfa6b9ddf5766220c8ff7ede2926ca241bb947-3

+TAKBgDuLnQA3gey3VBznB39JUtxjeE6myuDkM/uGlfjb
+S1w4iA5sBzzh8uxEbi4nW91IJm2gsvvZhICHS3l6ab4pZB
+l2DulrKBxKKtD1rGxlG4LncabFn9vLZad2bSysqz/qTAUSTvqJQIDAQAB
+AoGAGRzwwir7XvBOAy5tM/uV6e+Zf6anZzus1s1Y1ClbjbE6HXbnWWF/wbZGOpet
+3Z4vMXc7jpTLryzTQIvVdfQbRc6+MUVeLKZatTXtdZrhu+Jk7hx0nTPy8Jcb
+uJqFk54MogxEcfbWd6IOkp+4xqFLBEDtgbIECnk+hgN4H
+qzzxxr397vWrjrIgbJpQvBv8QeeuNi8DoUBEmiSJwa7FXY
+FUtxuvL7XvjwjN5B30pEbc6Iuyt7y4MQJBAIt21su4b3sjphy2tuUE9xblTu14qgHZ6+AiZovGKU--FfYAqVXVlxtIX
+qyU3X9ps8ZfjLZ45l6cGhaJQYZHOde3JEMhNRcVFMO8dJDaFeo
+f9Oeos0UUothgiDktdQHxdNEwLjQf7lJJBzV+5OtwswCWA==
+-----END RSA T
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/1e14c7ea1faef92890988061b5abe96db7190f98-7

+0000000000000000000000000000000000000000000000000000000000000000000000000
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/1fd84ee194e791783a7f18f0a6deab8efe05fc04-2

+--broken encoding: IBM420_ltr
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/21e76b9fca21d94d97f860c1c82f40697a83471b-8

+DtQvfQ+MULKZTXk78c
+/fWkpxlQQ/+hgNzVtx9vWgJsafG7b0dA4AFjwVbFLmQcj2PprIMmPNQrooX
+L
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/23ddcd66aa92fe3d78b7f5b6e7cddb1b55c5f5df-3

+4txjeVE6myuDqkM/uGlfjb9
+SjY1bIw4iA5sBBZzHi3z0h1YV8QPuxEbi4nW91IJm2gsvvZeIrCHS3l6afab4pZB
+l2+XsDlrKBxKKtD1rGxlG4jncdabFn9gvLZad2bSysqz/qTAUSTvqJQIDAQAB
+AoGAGRzwwXvBOAy5tM/uV6e+Zf6aZzus1s1Y1ClbjbE6HXbnWWF/wbZGOpet
+3Z4vD6Mc7pLryzTQIVdfQbRc6+MUVeLKZaTXtdZru+Jk70PJJqFk541aEw+mMogY/xEcfbWd6IOkp+4xqjlFLBEDytgbIECQQDvH/E6nk+gN4H
+qzzVtxxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLmQ2PprIMPcQroo8vpjSHg1Ev14KxmQeDydfsgeuN8UBESJwm7F
+UtuL7Xvjw50pNEbc6Iuyty4QJA21su4sjXNueLQphy2U
+fQtuUE9txblTu14qN7gHRZB4ZMhFYyDy8CKrN2cPg/Fvyt0Xlp/DoCzjA0CQQDU
+y2ptGsuSmgUtWj3NM9xuwYPm+Z/F84K6ARYiZPYj1oGUFfYAVVxtI
+qyBnu3X9pfLZOAkEAlT4R5Yl6cJQYZHOde3JEhNRcVFMO8dJFo
+f9Oeos0UUhgiDkQxdEwLjQf7lJJz5OtwC=
+-NRSA TESINGKEY-Q_
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/30494b85bb60ad7f099fa49d427007a761620d8f-5

+jXbnWWF/wbZGOpet
+3Zm4vD6MXc7jpTLryzTQIvVdfQbRc6+MUVeLKwZatTXtdZrhu+Jk7hx0nTPy8Jcb
+uJqFk541aEw+mMogY/xEcfbWd6IOkp+4xqjlFLBEDytgbIECQQDvH/E6nk+hgN4H
+qzzVtxxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLmQcj2PprIMmPcQrooz8vp
+jy4SHEg1AkEA/v13/5M47K9vCxb8QeD/asydfsgS5TeuNi8DoUBEmiSJwma7FXY
+fFUtxuvL7XvjwjN5B30pNEbc6Iuyt7y4MQJBAIt21su4b3sjXNueLKH85Q+phy2U
+fQtuUE9txblTu14q3N7gHRZB4ZMhFYyDy8CKrN2cPg/Fvyt0Xl
/DoCzjA0CQQDU
+y2ptGsuSmgUtWj3NM9xuwYPm+Z/F84K6+ARYiZ6Yj013sovGKUFfYAqVXVlxtIX
+qyUBnu3Xh9ps8ZfjLZO7BAkEAlT4R5Yl6cGhaJQYZHOde3JEMhNRcVFMO8dDaFeo
+f9Oeos0UotgiDktdQHxdNEwLjQfl
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/316024ca3aaf09c1de5258733ff5fe3d799648d3-4

+^oȗ----BEGIN RA TTING KEY-----
+IIXgIBAAKBQDuLnQI3mDgey3VBzWnB2L39JUU4txjeVE6myuDqkM/uGlfjb9
+SjY1bIw4iA5sBBZzHi3z0h1YV8QPuxEbi4nW91IJmgsvvZhrCHSl6afab4pZB
+l2+XsDulrKBxKKtD1rGxlG4LjcdabF9gvLZad2bSysqz/qTAUStTvqJQDAQAB
+AoGAGRzwwir7XvBOAy5tM/uV6e+Zf6anZzus1s1Y1ClbjbE6HXbnWWF/wbZGOpet
+3Z4vD6MXc7jpTLryzTQIvVdfQbRc6+MUVeLKwZatTXtdZrhu+Jk7hx0nTPy8Jcb
+uJqFk541aEw+mMogY/xEcfbWd6IOkp+4xqjlFLBEDytgbIECQQDvH/E6nk+hgN4H
+qzzVtxxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLmQcj2PprIMmPcQrooz8vp
+jy4SHEg1AkEA/v13/5M47K9vCxmb8QeD/asydfsgS5TeuNi8DoUBEmiSJwma7FXY
+fFUtxuvL7XvjwjN5B30pNEbc6Iuyt7y4MQJBAIt21su4b3sjXNueLKH85Q+phy2U
+fQtuUE9txblTu14q3N7gHRZB4ZMhFYyDy8CKrN2cPg/Fvyt0Xlp/DoCzjA0CQQDU
+y2ptGsuSmgUtWj3NM9xuwYPm+Z/F84K6+ARYiZ6PYj043sovGKUFfYAqVXVlxtIX
+qyUBnu3X9ps8ZfjLZO7BAkEAlT4R5Yl6cGhaJQYZHOde3JEMhNRcVFMO8dJDaFeo
+f9Oeos0UUothgiDktdQHxdNEwLjQf7lJJBzV+5OtwswCWA==
+-----END RSA TESTING KEY-----Q_
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/33ec1dc0bfeb93d16edee3c07125fec6ac1aa17d-2

+
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/3a010483a4ad8d7215447ce27e0fac3791235c99-4

+
+lGAGRzwwir7XvBOAy5tM/uV6e+Zf6anZzus1s1Y1ClbjbE6HXbnWWF/wbZGOpet
+3Zm4vD6MXc7jpTLryzTQIvVdfQbRc6+MUVeLKwZatTXtdZrhu+Jk7hx0nTPy8Jcb
+uJqFk541aEw+mMogY/xEcfbWd6IOkp+4xqjlFLBEDytgbIECQQDvH/E6nk+hgN4H
+qzzVtxxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLmQcj2PprIMmPcQrooz8vp
+jy4SHEg1AkEA/v13/5M47K9vCxmb8QeD/asydfsgS5TeuNi8DoUBEmiSJwma7FXY
+fFUtxuvL7XvjwjN5
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/3c37f6d58b8029971935f127f53e6aaeba558445-6

+w�€
�

�
�
�
�
�
�
�
�	
�
+
�
�
�

�
�
�

�
�
�
�
�
�
�
�
�
�
�
�
�
�
� 
�!
�"
�#
�$
�%
�&
�'
�(
�)
�*
�+
�,
�-
�.
�/
0
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/3c73b63bafa9f535c882ec17189adaf02b58f432-6

+LvhaJQHOe3EhRcdaFofeoogkjQfJB
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/3f99c546a3962256176d566c19e3fffb62072078-1

+&^o
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/436154e5bb6487673f6642e6d2a582c01b083c08-8

+apfffffffffffffffffffffffffffffffebadce6f48a0_3bbfd2364
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/4a0a12f5b033c8c160cc3b5133692ea1e92c6cdf-7

+DtQvfQ+MULKZTXk78c
+/fWkpxlyEQQ/+hgNzVtx9vWgJsafG7b0dA4AFjwVbFLmQcj2PprIMmPNQg1Ak/7KCxmDgS5TDEmSJwFX
+txLjbt4xTgeXVlXsjLZ
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/5570ef82893a9b9b9158572d43a7de7537121d2d-1

+
�ٯ0,1,2,3,4,5,6,7,-3420794409,(2,a)
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/61e89c3fbdf9eff74bd250ea73cc2e61f8ca0d97-5

+88242871'392752200424491531672177074144720616417147514758635765020556616
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/6782da8f1a432a77306d60d2ac2470c35b98004f-3

+21888242871'392752200424452601091531672177074144720616417147514758635765020556616 
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/6a5059bc86872526241d21ab5dae9f0afd3b9ae1-3

+¿½
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/717928e0e2d478c680c6409b173552ca98469ba5-6

+LvhaJcdaFofenogkjQfJB
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/7312a0f31ae5d773ed4fd74abc7521eb14754683-8

+DtQvfQ+MULKZTXk78c
+/fWkpxlyEQQ/+hgNzVtx9vWgJsafG7b0dA4AFjwVbFLmQcj2PprIMmPNQg1AkS5TDEmSJwFVlXsjLZ
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/78480977d5c07386b06e9b37f5c82f5ed86c2f09-3

+ TESTING KEY-----
+MIICXgIBAAKBgQDuLnQAI3mDgey3VBzWnB2L39JUU4txjeVE6myuDqkM/uGlfjb9
+SjY1bIw4iAJm2gsvvZhIrCHS3l6afab4pZB
+l2+XsDulrKBxKKtD1rGxlG4LjncdabFn9gvLZad2bSysqz/qTAUStTvqJQIDAQAB
+AoGAGRzwwir7XvBOAy5tM/uV6e+Zf6anZzus1s1Y1ClbjbE6HXbnWWF/wbZGOpet
+3Zm4vD6MXc7jpTLryzTQIvVdfQbRc6+MUVeLKwZatTXtdZrhu+Jk7hx0nTPy8Jcb
+uJqFk541aEw+mMogY/xEcfbWd6IOkp+4xqjlFLBEDytgbIECQQDvH/E6nk+hgN4H
+qzzVtxxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLmQcj2PprIMmPcQrooz8vp
+jy4SHEg1AkEA/v13/5M47K9vCxmb8QeD/asydfsgS5TeuNi8DoUBEmiSJwma7FXY
+fFUtxuvL7XvjwjN5B30pNEbc6Iuyt7y4MQJBAIt21su4b3sjXNueLKH85Q+phy2U
+fQtuUE9txblTu14q3N7gHRZB4ZMhFYyDy8CKrN2cPg/Fvyt0Xl
/DoCzjA0CQQDU
+y2ptGsuSmgUtWj3NM9xuwYPm+Z/F84K6+ARYiZ6PYj013sovGKUFfYAqVXVlxtIX
+qyUBnu3Xh9ps8ZfjLZO7BAkEAlT4R5Yl6cGhaJQYZHOde3JEMhNRcVFMO8dDaFeo
+f9Oeos0UotgiDktdQHxdNEwLjQflJJBzV+5OtwswCA=----EN RATESTI EY-----Q
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/7a113cd3c178934cdb64353af86d51462d7080a4-5

+l6afab4pZB
+l2+XsDlrKBxKKtDrGxlG4LjncdabFn9gvLZad2bSysqz/qTAUStTtqJQIDAQAB
+AoGAGRzwwir7XvBOAy5tuV6ef6anZzus1s1Y1Clb6HbnWWF/wbZGOpet
+3m4vD6MXc7jpTLryzTQIvVdfQbRc6+MUVeLKZTXtdZrh+k7hx0nTP8Jcb
+uqFk541awmMogY/EfbWd6IOkp+4xqjlFBEDytgbIECQQDvH/6nk+hgN4H
+qzzVtxxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLmQcj2PprIMmPcQrooz84SHEg1Ak/7KCxmD/sfgS5TeuNi8DoUBEmiSJwm7FX
+ftxuvL7XvjwjN5B30pNEbc6Iuyt7y4MQJBAIt21su43sjXNueLKH8+ph2UfQuU9txblTu14q3N7gHRZB4ZMhFYyDy8CKrN2cPg/Fvyt0Xl
/DoCzjA0CQQDU
+y2pGsuSmgUtWj3NM9xuwYPm+Z/F84K6+ARYiZ6PYj13sovGKUFfYAqVXVlxtIoX
+qUn3X9ps8ZfjLZO7BAkEAlT4R5Yl6cGhaJQYZHOde3JMhNRcVFMO8dDaFo
+f9Oeos0UotgiDktdQHxdNEwLjQlJBz+OtwwA=---E ATTIEY-
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/7ea9f71020f3eb783f743f744eba8d8ca4b2582f-3

+
+l2+DulrKBxKKtD1rGxlG4LjncdabFn9gvLZad2bSysqz/qTAUStTvqJQIDAQAB
+AoGAGRzwwir7XvBOAy5tM/uV6e+Zf6anZzus1s1Y1ClbjbE6HXbnWWF/wbZGOpet
+3Zm4vD6MXc7jpTLryzTQIvVdfQbRc6+MUVeLKwZatTXtdZrhu+Jk7hx0nTPy8Jcb
+uJqFk541aEw+mMogY/xEcfbWd6IOkp+4xqjlFLBEDytgbIECQQDvH/E6nk+hgN4H
+qzzVtxxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLmQcj2PprIMmPcQrooz8vp
+jy4SHEg1AkEA/v13/5M47K9vCxmb8QeD/asydfsgS5TeuNi8DoUBEmiSJwma7FXY
+fFUtxuvL7XvjwjN5B30pNEbc6Iuyt7y4MQJBAIt21su4b3sjXNueLKH85Q+phy2U
+fQtuUE9txblTu14q3N7gHRZB4ZMhFYyDy8CKrN2cPg/Fvyt0Xlp/DoCzjA0CQQDU

tests/fuzzers/txfetcher/corpus/84f8c275f3ffbaf8c32c21782af13de10e7de28b-3

+KKtDlbjVeLKwZatTXtdZrhu+Jk7hx0xxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLQcmPcQETT YQ
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/9034aaf45143996a2b14465c352ab0c6fa26b221-2

+½
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/92cefdc6251d04896349a464b29be03d6bb04c3d-2

+39402006196394479212279040100143613805079739270465446667948293404245721771496870329047266088258938001861606973112319 
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/9ebcbbfdaf0e98c87652e57226a4d8a35170c67d-4

+l2+DulrKBxKKtD1rGxlG4LjncdabFn9gvLZad2bSysqz/qTAUStTvqJQIDAQAB
+AoGAGRzwwir7XvBOAy5tM/uV6e+Zf6anZzus1s1Y1ClbjbE6HXbnWWF/wbZGOpwVbFLmQet
+3Zm4vD6MXc7jpTLryzTQIvVdfQbRc6+MUVeLKwZatTXtdZrhu+Jk7hx0nTPy8Jcb
+uJqFk541aEw+mMogY/xEcfbWd6IOkp+4xqjlFLBEDytgbIECQQDvH/E6nk+hgN4H
+qzzVtxxr397vWrjr
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/9ff520eb8b8319a5fdafbe4d1cbb02a75058d93b-7

+&w�€
�

�
�
�
�
�
�
�
�	
�
+
�
�
�

�
�
�

�
�
�
�
�
�
�
�
�
�
�
�
�
� 
�!
�"
�#
�$
�%
�&
�'
�(
�)
�*
�+
�,
�-
�.
�/
0
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/a8f7c254eb64a40fd2a77b79979c7bbdac6a760c-4

+lxtIX
+qyU3X9ps8ZfjLZ45l6cGhaJQYZHOde3JEMhNRcVFMO8dJDaFe
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/aa7444d8e326158046862590a0db993c07aef372-7

+00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000@0000000000000
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/ae4593626d8796e079a358c2395a4f6c9ddd6a44-6

+9pmM gY/xEcfbWd6IOkp+4xqjlFLBEDytgbparsing /E6nk+hgN4H
+qzzVtxxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLmQcj2PprLANGcQrooz8vp
+jy4SHEg1AkEA/v13/@M47K9vCxb8QeD/asydfsgS5TeuNi8DoUBEmiSJwma7FXY
+fFUtxuvL7XvjwjN5B30pNEbc6Iuyt7y4MQJBAIt21su4b3sjXNueLKH85Q+phy2U
+fQtuUE9txblTu14q3N7gHRZB4ZMhFYyDy8CKrN2cPg/Fvyt0Xl
/DoCz�	jA0CQQDU
+y2ptGsuSmgUtWj3NM9xuwYPm+Z/F84K6+ARYiZ6Yj013sovGKUFfYAqVXVlxtIX
+qyUBnu3Xh9ps8ZfjLZO7BAkEAlT4R5Yl6cGhaJQYFZHOde3JEMhNRcVFMO8dDaFeo
+f9Oeos0Uot
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/b631ef3291fa405cd6517d11f4d1b9b6d02912d4-2

+&o
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/b858cb282617fb0956d960215c8e84d1ccf909c6-2

+ 
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/bc9d570aacf3acd39600feda8e72a293a4667da4-1

+� 
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/be7eed35b245b5d5d2adcdb4c67f07794eb86b24-3

+4LZmbRc6+MUVeLKXtdZr+Jk7hhgN4H
+qzzVtxxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLQcmPcQ SN_
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/c010b0cd70c7edbc5bd332fc9e2e91c6a1cbcdc4-5

+
+Xc7jpTLryzTQIvVdfQbRc6+MUVeLKwZatTXtdZrhu+Jk7hx0nTPy8Jcb
+uJqFk541aEw+mMogY/xEcfbWd6IOkp+4xqjlFLBEDytgbIECQQDvH/E6nhgN4H
+qzzVtxx7vWrjrIgPbJpvfb
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/c1690698607eb0f4c4244e9f9629968be4beb6bc-8

+&Ƚ
�
�	
�
+
�
�
�

�
�
�

�
�
�
�
�
�
�
�
�
�
�
�
�
� 
�!
�"
�#
�$
�%
�&
�'
�(
�)
�*
�+
�,
�-
�.
�/
0
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/cd1d73b4e101bc7b979e3f6f135cb12d4594d348-5

+822452601031714757585602556
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/dd441bd24581332c9ce19e008260a69287aa3cbc-6

+Dtf1nWk78c
+/fWklyEQQ/+hgNzVtxxmDgS5TDETgeXVlXsjLZ
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/def879fe0fd637a745c00c8f1da340518db8688c-2

+ 
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/ec0a25eba8966b8f628d821b3cfbdf2dfd4bbb4c-3

+&^oȗ-----BEGIN RSA TESTING KEY-----
+MIICXgIBAAKBgQDuLnQAI3mDgey3VBzWnB2L39JUU4txjeVE6myuDqkM/uGlfjb9
+SjY1bIw4iA5sBBZzHi3z0h1YV8PuxEbi4nW91IJm2gsvvZhIrHS3l6afab4pZB
+l2+XsDulrKBxKKtD1rGxlG4Ljncdabn9vLZad2bSysqz/qTAUStvqJQIDAQAB
+AoGAGRzwwir7XvBOAy5tM/uV6e+Zf6anZzus1s1K1ClbjbE6HXbnWWF/wbZGOpet
+3Zm4vD6MXc7jpTLryzQIvVdfQbRc6+MUVeLKwZatTXtZru+Jk7hx0nTPy8Jcb
+uJqFk541aEw+mMogY/xEcfbW6IOkp+4xqjlFLBEDytgbIECQQDvH/E6nk+hg4
+qzzVtxxr397vWrjrIgPbJpQvBsafG7b0dA4AFjwVbFLcj2pIMPQroozvjg1AkEA/v13/5M47K9vCxmb8QeD/aydfsgS5TeuNi8DoUBEmiSJwmaXY
+fFUtxv7XvjwjN5B30pNEbc6Iuyt7y4MQJBAIt21su4bjeLKH8Q+ph2
+fQtuUE9txblTu14q3N7gHRZB4ZMhFYyDy8CKrN2cPg/Fvyt0Xlp/DoCzjA0CQQDU
+y2ptGsuSmgUtWj3NM9xuwYPm+Z/F84K6+AYiZ6PYj013sovGKFYqVXVlxtIX
+qyUBnu3X9s8ZfjZO7BAkl4R5Yl6cGhaJQYZHOe3JEMhVFaFf9Oes0UUothgiDktdQxdNLj7+5CWA==
+-----END RSASQ
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/efb7410d02418befeba25a43d676cc6124129125-4

+88242871'392752200424452601091531672177074144720616417147514758635765020556616
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/f7a3cd00fa0e57742e7dbbb8283dcaea067eaf7b-5

+Xyt0Xl
/DoCzjA0CQQDU
+y2ptGsuSmgUtWj3NM9xuwYPm+Z/F84K6+ARYi
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/f94d60a6c556ce485ab60088291760b8be25776c-6

+HZB4cQZde3JMNRcVFMO8dDFo
+f9OeosiDdQQl
\ No newline at end of file

tests/fuzzers/txfetcher/corpus/fb3775aa24e5667e658920c05ba4b7b19ff256fb-5

+HZB4c2cPclieoverpGsumgUtWj3NMYPZ/F8t5YlNR8dDFoiDdQQl
\ No newline at end of file

tests/fuzzers/txfetcher/txfetcher_fuzzer.go

+// Copyright 2020 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 txfetcher
+
+import (
+	"bytes"
+	"fmt"
+	"math/big"
+	"math/rand"
+	"time"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/mclock"
+	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/eth/fetcher"
+)
+
+var (
+	peers []string
+	txs   []*types.Transaction
+)
+
+func init() {
+	// Random is nice, but we need it deterministic
+	rand := rand.New(rand.NewSource(0x3a29))
+
+	peers = make([]string, 10)
+	for i := 0; i < len(peers); i++ {
+		peers[i] = fmt.Sprintf("Peer #%d", i)
+	}
+	txs = make([]*types.Transaction, 65536) // We need to bump enough to hit all the limits
+	for i := 0; i < len(txs); i++ {
+		txs[i] = types.NewTransaction(rand.Uint64(), common.Address{byte(rand.Intn(256))}, new(big.Int), 0, new(big.Int), nil)
+	}
+}
+
+func Fuzz(input []byte) int {
+	// Don't generate insanely large test cases, not much value in them
+	if len(input) > 16*1024 {
+		return -1
+	}
+	r := bytes.NewReader(input)
+
+	// Reduce the problem space for certain fuzz runs. Small tx space is better
+	// for testing clashes and in general the fetcher, but we should still run
+	// some tests with large spaces to hit potential issues on limits.
+	limit, err := r.ReadByte()
+	if err != nil {
+		return 0
+	}
+	switch limit % 4 {
+	case 0:
+		txs = txs[:4]
+	case 1:
+		txs = txs[:256]
+	case 2:
+		txs = txs[:4096]
+	case 3:
+		// Full run
+	}
+	// Create a fetcher and hook into it's simulated fields
+	clock := new(mclock.Simulated)
+	rand := rand.New(rand.NewSource(0x3a29)) // Same used in package tests!!!
+
+	f := fetcher.NewTxFetcherForTests(
+		func(common.Hash) bool { return false },
+		func(txs []*types.Transaction) []error {
+			return make([]error, len(txs))
+		},
+		func(string, []common.Hash) error { return nil },
+		clock, rand,
+	)
+	f.Start()
+	defer f.Stop()
+
+	// Try to throw random junk at the fetcher
+	for {
+		// Read the next command and abort if we're done
+		cmd, err := r.ReadByte()
+		if err != nil {
+			return 0
+		}
+		switch cmd % 4 {
+		case 0:
+			// Notify a new set of transactions:
+			//   Byte 1:             Peer index to announce with
+			//   Byte 2:             Number of hashes to announce
+			//   Byte 3-4, 5-6, etc: Transaction indices (2 byte) to announce
+			peerIdx, err := r.ReadByte()
+			if err != nil {
+				return 0
+			}
+			peer := peers[int(peerIdx)%len(peers)]
+
+			announceCnt, err := r.ReadByte()
+			if err != nil {
+				return 0
+			}
+			announce := int(announceCnt) % (2 * len(txs)) // No point in generating too many duplicates
+
+			var (
+				announceIdxs = make([]int, announce)
+				announces    = make([]common.Hash, announce)
+			)
+			for i := 0; i < len(announces); i++ {
+				annBuf := make([]byte, 2)
+				if n, err := r.Read(annBuf); err != nil || n != 2 {
+					return 0
+				}
+				announceIdxs[i] = (int(annBuf[0])*256 + int(annBuf[1])) % len(txs)
+				announces[i] = txs[announceIdxs[i]].Hash()
+			}
+			fmt.Println("Notify", peer, announceIdxs)
+			if err := f.Notify(peer, announces); err != nil {
+				panic(err)
+			}
+
+		case 1:
+			// Deliver a new set of transactions:
+			//   Byte 1:             Peer index to announce with
+			//   Byte 2:             Number of hashes to announce
+			//   Byte 3-4, 5-6, etc: Transaction indices (2 byte) to announce
+			peerIdx, err := r.ReadByte()
+			if err != nil {
+				return 0
+			}
+			peer := peers[int(peerIdx)%len(peers)]
+
+			deliverCnt, err := r.ReadByte()
+			if err != nil {
+				return 0
+			}
+			deliver := int(deliverCnt) % (2 * len(txs)) // No point in generating too many duplicates
+
+			var (
+				deliverIdxs = make([]int, deliver)
+				deliveries  = make([]*types.Transaction, deliver)
+			)
+			for i := 0; i < len(deliveries); i++ {
+				deliverBuf := make([]byte, 2)
+				if n, err := r.Read(deliverBuf); err != nil || n != 2 {
+					return 0
+				}
+				deliverIdxs[i] = (int(deliverBuf[0])*256 + int(deliverBuf[1])) % len(txs)
+				deliveries[i] = txs[deliverIdxs[i]]
+			}
+			directFlag, err := r.ReadByte()
+			if err != nil {
+				return 0
+			}
+			direct := (directFlag % 2) == 0
+
+			fmt.Println("Enqueue", peer, deliverIdxs, direct)
+			if err := f.Enqueue(peer, deliveries, direct); err != nil {
+				panic(err)
+			}
+
+		case 2:
+			// Drop a peer:
+			//   Byte 1: Peer index to drop
+			peerIdx, err := r.ReadByte()
+			if err != nil {
+				return 0
+			}
+			peer := peers[int(peerIdx)%len(peers)]
+
+			fmt.Println("Drop", peer)
+			if err := f.Drop(peer); err != nil {
+				panic(err)
+			}
+
+		case 3:
+			// Move the simulated clock forward
+			//   Byte 1: 100ms increment to move forward
+			tickCnt, err := r.ReadByte()
+			if err != nil {
+				return 0
+			}
+			tick := time.Duration(tickCnt) * 100 * time.Millisecond
+
+			fmt.Println("Sleep", tick)
+			clock.Run(tick)
+		}
+	}
+}