Turn isTxTooBig into a method on withdrawalTx

Also get rid of the replaceIsTxTooBig test helper by making tests use
replaceCalculateTxSize instead

votingpool/common_test.go

@@ -108,14 +108,6 @@ func replaceCalculateTxFee(f func(*withdrawalTx) btcutil.Amount) func() {
 	return func() { calculateTxFee = orig }
 }
 
-// replaceIsTxTooBig replaces the isTxTooBig func with the given one
-// and returns a function that restores it to the original one.
-func replaceIsTxTooBig(f func(*withdrawalTx) bool) func() {
-	orig := isTxTooBig
-	isTxTooBig = f
-	return func() { isTxTooBig = orig }
-}
-
 // replaceCalculateTxSize replaces the calculateTxSize func with the given one
 // and returns a function that restores it to the original one.
 func replaceCalculateTxSize(f func(*withdrawalTx) int) func() {

votingpool/withdrawal.go

@@ -317,6 +317,15 @@ func (tx *withdrawalTx) ntxid() Ntxid {
 	return Ntxid(msgtx.TxSha().String())
 }
 
+// isTooBig returns true if the size (in bytes) of the given tx is greater
+// than or equal to txMaxSize.
+func (tx *withdrawalTx) isTooBig() bool {
+	// In bitcoind a tx is considered standard only if smaller than
+	// MAX_STANDARD_TX_SIZE; that's why we consider anything >= txMaxSize to
+	// be too big.
+	return calculateTxSize(tx) >= txMaxSize
+}
+
 // inputTotal returns the sum amount of all inputs in this tx.
 func (tx *withdrawalTx) inputTotal() (total btcutil.Amount) {
 	for _, input := range tx.inputs {
@@ -540,7 +549,7 @@ func (w *withdrawal) fulfillNextRequest() error {
 	output.status = statusSuccess
 	w.current.addOutput(request)
 
-	if isTxTooBig(w.current) {
+	if w.current.isTooBig() {
 		return w.handleOversizeTx()
 	}
 
@@ -556,7 +565,7 @@ func (w *withdrawal) fulfillNextRequest() error {
 		w.current.addInput(w.popInput())
 		fee = calculateTxFee(w.current)
 
-		if isTxTooBig(w.current) {
+		if w.current.isTooBig() {
 			return w.handleOversizeTx()
 		}
 	}
@@ -990,16 +999,6 @@ var calculateTxFee = func(tx *withdrawalTx) btcutil.Amount {
 	return btcutil.Amount(1+calculateTxSize(tx)/1000) * feeIncrement
 }
 
-// isTxTooBig returns true if the size (in bytes) of the given tx is greater
-// than or equal to txMaxSize. It is defined as a variable so it can be
-// replaced for testing purposes.
-var isTxTooBig = func(tx *withdrawalTx) bool {
-	// In bitcoind a tx is considered standard only if smaller than
-	// MAX_STANDARD_TX_SIZE; that's why we consider anything >= txMaxSize to
-	// be too big.
-	return calculateTxSize(tx) >= txMaxSize
-}
-
 // calculateTxSize returns an estimate of the serialized size (in bytes) of the
 // given transaction. It assumes all tx inputs are P2SH multi-sig.  We use a
 // variable instead of a function so that it can be replaced in tests.

votingpool/withdrawal_wb_test.go

@@ -95,11 +95,14 @@ func TestOutputSplittingOversizeTx(t *testing.T) {
 	w := newWithdrawal(0, []OutputRequest{request}, eligible, *changeStart)
 	restoreCalculateTxFee := replaceCalculateTxFee(TstConstantFee(0))
 	defer restoreCalculateTxFee()
-	restoreIsTxTooBig := replaceIsTxTooBig(func(tx *withdrawalTx) bool {
+	restoreCalcTxSize := replaceCalculateTxSize(func(tx *withdrawalTx) int {
 		// Trigger an output split right after the second input is added.
-		return len(tx.inputs) == 2
+		if len(tx.inputs) == 2 {
+			return txMaxSize + 1
+		}
+		return txMaxSize - 1
 	})
-	defer restoreIsTxTooBig()
+	defer restoreCalcTxSize()
 
 	if err := w.fulfillRequests(); err != nil {
 		t.Fatal(err)
@@ -401,10 +404,14 @@ func TestRollbackLastOutputWhenNewOutputAdded(t *testing.T) {
 	w := newWithdrawal(0, requests, eligible, *changeStart)
 	restoreCalculateTxFee := replaceCalculateTxFee(TstConstantFee(0))
 	defer restoreCalculateTxFee()
-	restoreIsTxTooBig := replaceIsTxTooBig(func(tx *withdrawalTx) bool {
+	restoreCalcTxSize := replaceCalculateTxSize(func(tx *withdrawalTx) int {
-		return len(tx.outputs) > 1
+		// Trigger an output split right after the second output is added.
+		if len(tx.outputs) > 1 {
+			return txMaxSize + 1
+		}
+		return txMaxSize - 1
 	})
-	defer restoreIsTxTooBig()
+	defer restoreCalcTxSize()
 
 	if err := w.fulfillRequests(); err != nil {
 		t.Fatal("Unexpected error:", err)
@@ -451,11 +458,14 @@ func TestRollbackLastOutputWhenNewInputAdded(t *testing.T) {
 	w := newWithdrawal(0, requests, eligible, *changeStart)
 	restoreCalculateTxFee := replaceCalculateTxFee(TstConstantFee(0))
 	defer restoreCalculateTxFee()
-	restoreIsTxTooBig := replaceIsTxTooBig(func(tx *withdrawalTx) bool {
+	restoreCalcTxSize := replaceCalculateTxSize(func(tx *withdrawalTx) int {
 		// Make a transaction too big as soon as a fourth input is added to it.
-		return len(tx.inputs) > 3
+		if len(tx.inputs) > 3 {
+			return txMaxSize + 1
+		}
+		return txMaxSize - 1
 	})
-	defer restoreIsTxTooBig()
+	defer restoreCalcTxSize()
 
 	// The rollback should be triggered right after the 4th input is added in
 	// order to fulfill the second request.
@@ -994,7 +1004,7 @@ func TestTxTooBig(t *testing.T) {
 	tx := createWithdrawalTx(t, pool, []int64{5}, []int64{1})
 
 	restoreCalcTxSize := replaceCalculateTxSize(func(tx *withdrawalTx) int { return txMaxSize - 1 })
-	if isTxTooBig(tx) {
+	if tx.isTooBig() {
 		t.Fatalf("Tx is smaller than max size (%d < %d) but was considered too big",
 			calculateTxSize(tx), txMaxSize)
 	}
@@ -1002,14 +1012,14 @@ func TestTxTooBig(t *testing.T) {
 
 	// A tx whose size is equal to txMaxSize should be considered too big.
 	restoreCalcTxSize = replaceCalculateTxSize(func(tx *withdrawalTx) int { return txMaxSize })
-	if !isTxTooBig(tx) {
+	if !tx.isTooBig() {
 		t.Fatalf("Tx size is equal to the max size (%d == %d) but was not considered too big",
 			calculateTxSize(tx), txMaxSize)
 	}
 	restoreCalcTxSize()
 
 	restoreCalcTxSize = replaceCalculateTxSize(func(tx *withdrawalTx) int { return txMaxSize + 1 })
-	if !isTxTooBig(tx) {
+	if !tx.isTooBig() {
 		t.Fatalf("Tx size is bigger than max size (%d > %d) but was not considered too big",
 			calculateTxSize(tx), txMaxSize)
 	}