move broadcast code to new file and use RespendTimeout instead of a t…

…ries counter

modules/consts.go

@@ -0,0 +1,10 @@
+package modules
+
+import "github.com/NebulousLabs/Sia/types"
+
+// Consts that are required by multiple modules
+const (
+	// maxTxnAge determines the maximum age of a transaction (in block height)
+	// allowed before the transaction is pruned from the transaction pool.
+	MaxTxnAge = types.BlockHeight(24)
+)

modules/transactionpool/consts.go

@@ -16,10 +16,6 @@ const (
 
 // Constants related to the size and ease-of-entry of the transaction pool.
 const (
-	// maxTxnAge determines the maximum age of a transaction (in block height)
-	// allowed before the transaction is pruned from the transaction pool.
-	maxTxnAge = types.BlockHeight(24)
-
 	// TransactionPoolFeeExponentiation defines the polynomial rate of growth
 	// required to keep putting transactions into the transaction pool. If the
 	// exponentiation is 2, then doubling the size of the transaction pool

modules/transactionpool/update.go

@@ -324,7 +324,7 @@ func (tp *TransactionPool) ProcessConsensusChange(cc modules.ConsensusChange) {
 		var validTxns []types.Transaction
 		for _, txn := range tSet {
 			seenHeight, seen := tp.transactionHeights[txn.ID()]
-			if tp.blockHeight-seenHeight <= maxTxnAge || !seen {
+			if tp.blockHeight-seenHeight <= modules.MaxTxnAge || !seen {
 				validTxns = append(validTxns, txn)
 			} else {
 				delete(tp.transactionHeights, txn.ID())

modules/transactionpool/update_test.go

@@ -265,7 +265,7 @@ func TestValidRevertedTransaction(t *testing.T) {
 }
 
 // TestTransactionPoolPruning verifies that the transaction pool correctly
-// prunes transactions older than maxTxnAge.
+// prunes transactions older than MaxTxnAge.
 func TestTransactionPoolPruning(t *testing.T) {
 	if testing.Short() {
 		t.SkipNow()
@@ -298,7 +298,7 @@ func TestTransactionPoolPruning(t *testing.T) {
 		t.Fatal("testers did not have the same block height after one minute")
 	}
 
-	// disconnect tpt, create an unconfirmed transaction on tpt, mine maxTxnAge
+	// disconnect tpt, create an unconfirmed transaction on tpt, mine MaxTxnAge
 	// blocks on tpt2 and reconnect. The unconfirmed transactions should be
 	// removed from tpt's pool.
 	err = tpt.gateway.Disconnect(tpt2.gateway.Address())
@@ -310,7 +310,7 @@ func TestTransactionPoolPruning(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	for i := types.BlockHeight(0); i < maxTxnAge+1; i++ {
+	for i := types.BlockHeight(0); i < modules.MaxTxnAge+1; i++ {
 		_, err = tpt2.miner.AddBlock()
 		if err != nil {
 			t.Fatal(err)

modules/wallet/broadcast.go

@@ -0,0 +1,31 @@
+package wallet
+
+import "github.com/NebulousLabs/Sia/types"
+
+// broadcastedTSet is a helper struct to keep track of transaction sets and to
+// help rebroadcast them.
+type broadcastedTSet struct {
+	firstTry     types.BlockHeight            // first time the tSet was broadcasted
+	lastTry      types.BlockHeight            // last time the tSet was broadcasted
+	confirmedTxn map[types.TransactionID]bool // tracks confirmed txns of set
+	transactions []types.Transaction          // the tSet
+}
+
+// newBroadcastedTSet creates a broadcastedTSet from a normal tSet
+func (w *Wallet) newBroadcastedTSet(tSet []types.Transaction) (bts *broadcastedTSet, err error) {
+	bts = &broadcastedTSet{}
+	// Set the height of the first and last try
+	bts.firstTry, err = dbGetConsensusHeight(w.dbTx)
+	if err != nil {
+		return
+	}
+	bts.lastTry = bts.firstTry
+
+	// Initialize confirmedTxn and transactions
+	bts.confirmedTxn = make(map[types.TransactionID]bool)
+	for _, txn := range tSet {
+		bts.confirmedTxn[txn.ID()] = false
+		bts.transactions = append(bts.transactions, txn)
+	}
+	return
+}

modules/wallet/broadcast_test.go

@@ -0,0 +1,89 @@
+package wallet
+
+import (
+	"testing"
+
+	"github.com/NebulousLabs/Sia/crypto"
+	"github.com/NebulousLabs/Sia/modules"
+	"github.com/NebulousLabs/Sia/types"
+)
+
+// TestRebroadcastTransactions checks if transactions are correctly
+// rebroadcasted after some time if they haven't been confirmed
+func TestRebroadcastTransactions(t *testing.T) {
+	if testing.Short() {
+		t.SkipNow()
+	}
+	wt, err := createWalletTester(t.Name(), &ProductionDependencies{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer wt.closeWt()
+
+	// Get an address to send money to
+	uc, err := wt.wallet.NextAddress()
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Send money to the address
+	_, err = wt.wallet.SendSiacoins(types.SiacoinPrecision, uc.UnlockHash())
+	if err != nil {
+		t.Fatal(err)
+	}
+	// The wallet should track the new tSet
+	if len(wt.wallet.broadcastedTSets) != 1 {
+		t.Fatalf("len(broadcastedTSets) should be %v but was %v",
+			1, len(wt.wallet.broadcastedTSets))
+	}
+	// Mine enough blocks for the wallet to stop tracking the tSet
+	for i := 0; i < rebroadcastInterval+1; i++ {
+		if _, err := wt.miner.AddBlock(); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if len(wt.wallet.broadcastedTSets) > 0 {
+		t.Fatalf("len(broadcastedTSets) should be 0 but was %v",
+			len(wt.wallet.broadcastedTSets))
+	}
+
+	// Send some more money to the address
+	tSet, err := wt.wallet.SendSiacoins(types.SiacoinPrecision, uc.UnlockHash())
+	if err != nil {
+		t.Fatal(err)
+	}
+	// The wallet should track the new tSet
+	if len(wt.wallet.broadcastedTSets) != 1 {
+		t.Fatalf("len(broadcastedTSets) should be %v but was %v",
+			1, len(wt.wallet.broadcastedTSets))
+	}
+	// Mine a block to get the tSet confirmed
+	if _, err := wt.miner.AddBlock(); err != nil {
+		t.Fatal(err)
+	}
+	// Corrupt the new tSet to make sure the wallet believes it is not confirmed
+	tSetID := modules.TransactionSetID(crypto.HashAll(tSet))
+	bts := wt.wallet.broadcastedTSets[tSetID]
+	for tid := range bts.confirmedTxn {
+		bts.confirmedTxn[tid] = false
+	}
+	// Mine the same number of blocks. This time the wallet should still track
+	// the tSet afterwards.
+	for i := 0; i < rebroadcastInterval+1; i++ {
+		if _, err := wt.miner.AddBlock(); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if len(wt.wallet.broadcastedTSets) != 1 {
+		t.Fatalf("The wallet should still track the tSet")
+	}
+	// Continue mining to make sure that the wallet stops tracking the tSet
+	// once the max number of retries is reached
+	for i := types.BlockHeight(0); i < rebroadcastTimeout; i++ {
+		if _, err := wt.miner.AddBlock(); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if _, exists := wt.wallet.broadcastedTSets[tSetID]; exists {
+		t.Fatalf("Wallet should drop txnSet after %v blocks", rebroadcastTimeout)
+	}
+}

modules/wallet/consts.go

@@ -2,6 +2,8 @@ package wallet
 
 import (
 	"github.com/NebulousLabs/Sia/build"
+	"github.com/NebulousLabs/Sia/modules"
+	"github.com/NebulousLabs/Sia/types"
 )
 
 const (
@@ -19,14 +21,24 @@ const (
 	// rebroadcastInterval is the number of blocks the wallet will wait until
 	// it rebroadcasts an unconfirmed transaction by adding it to the
 	// transaction pool again.
-	rebroadcastInterval = 10
+	rebroadcastInterval = 6
 
-	// rebroadcastMaxTries is the maximum number of times a transaction set
-	// will be rebroadcasted before the wallet stops tracking it
-	rebroadcastMaxTries = 10
+	// RespendTimeout records the number of blocks that the wallet will wait
+	// before spending an output that has been spent in the past. If the
+	// transaction spending the output has not made it to the transaction pool
+	// after the limit, the assumption is that it never will.
+	respendTimeout = 72
 )
 
 var (
+	// rebroadcastTimeout is the amount of blocks after which we stop trying to
+	// rebroadcast transactions. The reason why we can't just use
+	// respendTimeout as the rebroadcastTimeout is, that the transaction pool
+	// will boot transactions after MaxTxnAge. We need to make sure that we
+	// leave at least MaxTxnAge blocks after the last broadcast to allow for
+	// the transasction to be pruned before the wallet tries to respend it.
+	rebroadcastTimeout = types.BlockHeight(respendTimeout - modules.MaxTxnAge)
+
 	// lookaheadBuffer together with lookaheadRescanThreshold defines the constant part
 	// of the maxLookahead
 	lookaheadBuffer = build.Select(build.Var{

modules/wallet/transactionbuilder.go

@@ -100,7 +100,7 @@ func (w *Wallet) checkOutput(tx *bolt.Tx, currentHeight types.BlockHeight, id ty
 	// Check that this output has not recently been spent by the wallet.
 	spendHeight, err := dbGetSpentOutput(tx, types.OutputID(id))
 	if err == nil {
-		if spendHeight+RespendTimeout > currentHeight {
+		if spendHeight+respendTimeout > currentHeight {
 			return errSpendHeightTooHigh
 		}
 	}
@@ -287,8 +287,8 @@ func (tb *transactionBuilder) FundSiafunds(amount types.Currency) error {
 			spendHeight = 0
 		}
 		// Prevent an underflow error.
-		allowedHeight := consensusHeight - RespendTimeout
-		if consensusHeight < RespendTimeout {
+		allowedHeight := consensusHeight - respendTimeout
+		if consensusHeight < respendTimeout {
 			allowedHeight = 0
 		}
 		if spendHeight > allowedHeight {

modules/wallet/update.go

@@ -510,7 +510,7 @@ func (w *Wallet) rebroadcastOldTransactions(tx *bolt.Tx, cc modules.ConsensusCha
 		}
 		// If the transaction set has been confirmed for one broadcast cycle it
 		// should be safe to remove it
-		if confirmed && consensusHeight > bts.height+rebroadcastInterval {
+		if confirmed && consensusHeight > bts.lastTry+rebroadcastInterval {
 			delete(w.broadcastedTSets, tSetID)
 			continue
 		}
@@ -521,17 +521,16 @@ func (w *Wallet) rebroadcastOldTransactions(tx *bolt.Tx, cc modules.ConsensusCha
 		}
 		// If the transaction set is not confirmed and hasn't been broadcasted
 		// for rebroadcastInterval blocks we try to broadcast it again
-		if consensusHeight > bts.height+rebroadcastInterval {
-			bts.height = consensusHeight
-			bts.tries++
+		if consensusHeight > bts.lastTry+rebroadcastInterval {
+			bts.lastTry = consensusHeight
 			go func() {
-				w.mu.Lock()
-				defer w.mu.Unlock()
 				if err := w.tpool.AcceptTransactionSet(bts.transactions); err != nil {
 					w.log.Println("WARNING: Rebroadcast failed: ", err)
 				}
 			}()
-			if bts.tries >= rebroadcastMaxTries {
+			// Delete the transaction set once we have tried for RespendTimeout
+			// blocks
+			if consensusHeight >= bts.firstTry+rebroadcastTimeout {
 				delete(w.broadcastedTSets, tSetID)
 			}
 		}

modules/wallet/wallet.go

@@ -20,14 +20,6 @@ import (
 	"github.com/NebulousLabs/Sia/types"
 )
 
-const (
-	// RespendTimeout records the number of blocks that the wallet will wait
-	// before spending an output that has been spent in the past. If the
-	// transaction spending the output has not made it to the transaction pool
-	// after the limit, the assumption is that it never will.
-	RespendTimeout = 40
-)
-
 var (
 	errNilConsensusSet = errors.New("wallet cannot initialize with a nil consensus set")
 	errNilTpool        = errors.New("wallet cannot initialize with a nil transaction pool")
@@ -43,32 +35,6 @@ type spendableKey struct {
 	SecretKeys       []crypto.SecretKey
 }
 
-// broadcastedTSet is a helper struct to keep track of transaction sets and to
-// help rebroadcast them.
-type broadcastedTSet struct {
-	height       types.BlockHeight
-	tries        int
-	confirmedTxn map[types.TransactionID]bool
-	transactions []types.Transaction
-}
-
-// newBroadcastedTSet
-func (w *Wallet) newBroadcastedTSet(tSet []types.Transaction) (bts *broadcastedTSet, err error) {
-	bts = &broadcastedTSet{}
-	// Set the height
-	bts.height, err = dbGetConsensusHeight(w.dbTx)
-	if err != nil {
-		return
-	}
-	// Initialize confirmedTxn and transactions
-	bts.confirmedTxn = make(map[types.TransactionID]bool)
-	for _, txn := range tSet {
-		bts.confirmedTxn[txn.ID()] = false
-		bts.transactions = append(bts.transactions, txn)
-	}
-	return
-}
-
 // Wallet is an object that tracks balances, creates keys and addresses,
 // manages building and sending transactions.
 type Wallet struct {