Change the stack semantics in withdrawal.eligibleInputs

votingpool/input_selection.go

@@ -96,7 +96,8 @@ func (c byAddress) Less(i, j int) bool {
 }
 
 // getEligibleInputs returns eligible inputs with addresses between startAddress
-// and the last used address of lastSeriesID.
+// and the last used address of lastSeriesID. They're reverse ordered based on
+// their address.
 func (p *Pool) getEligibleInputs(store *wtxmgr.Store, startAddress WithdrawalAddress,
 	lastSeriesID uint32, dustThreshold btcutil.Amount, chainHeight int32,
 	minConf int) ([]credit, error) {
@@ -125,8 +126,6 @@ func (p *Pool) getEligibleInputs(store *wtxmgr.Store, startAddress WithdrawalAdd
 					eligibles = append(eligibles, candidate)
 				}
 			}
-			// Make sure the eligibles are correctly sorted.
-			sort.Sort(byAddress(eligibles))
 			inputs = append(inputs, eligibles...)
 		}
 		nAddr, err := nextAddr(p, address.seriesID, address.branch, address.index, lastSeriesID+1)
@@ -138,6 +137,7 @@ func (p *Pool) getEligibleInputs(store *wtxmgr.Store, startAddress WithdrawalAdd
 		}
 		address = *nAddr
 	}
+	sort.Sort(sort.Reverse(byAddress(inputs)))
 	return inputs, nil
 }
 

votingpool/input_selection_wb_test.go

@@ -74,8 +74,8 @@ func TestGetEligibleInputs(t *testing.T) {
 			len(eligibles), expNoEligibleInputs)
 	}
 
-	// Check that the returned eligibles are sorted by address.
-	if !sort.IsSorted(byAddress(eligibles)) {
+	// Check that the returned eligibles are reverse sorted by address.
+	if !sort.IsSorted(sort.Reverse(byAddress(eligibles))) {
 		t.Fatal("Eligible inputs are not sorted.")
 	}
 

votingpool/withdrawal.go

@@ -482,17 +482,14 @@ func (w *withdrawal) pushRequest(request OutputRequest) {
 // popInput removes and returns the first input from the stack of eligible
 // inputs.
 func (w *withdrawal) popInput() credit {
-	input := w.eligibleInputs[0]
-	w.eligibleInputs = w.eligibleInputs[1:]
+	input := w.eligibleInputs[len(w.eligibleInputs)-1]
+	w.eligibleInputs = w.eligibleInputs[:len(w.eligibleInputs)-1]
 	return input
 }
 
 // pushInput adds a new input to the top of the stack of eligible inputs.
-// TODO: Reverse the stack semantics here as the current one generates a lot of
-// extra garbage since it always creates a new single-element slice and append
-// the rest of the items to it.
 func (w *withdrawal) pushInput(input credit) {
-	w.eligibleInputs = append([]credit{input}, w.eligibleInputs...)
+	w.eligibleInputs = append(w.eligibleInputs, input)
 }
 
 // If this returns it means we have added an output and the necessary inputs to fulfil that

votingpool/withdrawal_wb_test.go

@@ -89,7 +89,7 @@ func TestOutputSplittingOversizeTx(t *testing.T) {
 	smallInput := int64(2)
 	request := TstNewOutputRequest(
 		t, 1, "34eVkREKgvvGASZW7hkgE2uNc1yycntMK6", requestAmount, pool.Manager().ChainParams())
-	seriesID, eligible := TstCreateCreditsOnNewSeries(t, pool, []int64{bigInput, smallInput})
+	seriesID, eligible := TstCreateCreditsOnNewSeries(t, pool, []int64{smallInput, bigInput})
 	changeStart := TstNewChangeAddress(t, pool, seriesID, 0)
 	w := newWithdrawal(0, []OutputRequest{request}, eligible, *changeStart)
 	restoreCalculateTxFee := replaceCalculateTxFee(TstConstantFee(0))
@@ -437,7 +437,7 @@ func TestRollbackLastOutputWhenNewInputAdded(t *testing.T) {
 	defer tearDown()
 
 	net := pool.Manager().ChainParams()
-	series, eligible := TstCreateCreditsOnNewSeries(t, pool, []int64{1, 2, 3, 4, 5, 6})
+	series, eligible := TstCreateCreditsOnNewSeries(t, pool, []int64{6, 5, 4, 3, 2, 1})
 	requests := []OutputRequest{
 		// This is manually ordered by outBailmentIDHash, which is the order in
 		// which they're going to be fulfilled by w.fulfillRequests().
@@ -468,23 +468,24 @@ func TestRollbackLastOutputWhenNewInputAdded(t *testing.T) {
 	}
 
 	// First tx should have one output with amount of 1, the first input from
-	// the list of eligible inputs, and no change output.
+	// the stack of eligible inputs (last slice item), and no change output.
 	firstTx := w.transactions[0]
 	req1 := requests[0]
 	checkTxOutputs(t, firstTx,
 		[]*withdrawalTxOut{&withdrawalTxOut{request: req1, amount: req1.Amount}})
-	checkTxInputs(t, firstTx, eligible[0:1])
+	checkTxInputs(t, firstTx, eligible[5:6])
 
 	// Second tx should have outputs for the two last requests (in the same
 	// order they were passed to newWithdrawal), and the 3 inputs needed to
-	// fulfill that (also in the same order as they were passed to
-	// newWithdrawal) and no change output.
+	// fulfill that (in reverse order as they were passed to newWithdrawal, as
+	// that's how fulfillRequests() consumes them) and no change output.
 	secondTx := w.transactions[1]
 	wantOutputs := []*withdrawalTxOut{
 		&withdrawalTxOut{request: requests[1], amount: requests[1].Amount},
 		&withdrawalTxOut{request: requests[2], amount: requests[2].Amount}}
 	checkTxOutputs(t, secondTx, wantOutputs)
-	checkTxInputs(t, secondTx, eligible[1:4])
+	wantInputs := []credit{eligible[4], eligible[3], eligible[2]}
+	checkTxInputs(t, secondTx, wantInputs)
 }
 
 func TestWithdrawalTxRemoveOutput(t *testing.T) {