diff --git a/votingpool/input_selection.go b/votingpool/input_selection.go index 403cdf3..0e89d5c 100644 --- a/votingpool/input_selection.go +++ b/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 } diff --git a/votingpool/input_selection_wb_test.go b/votingpool/input_selection_wb_test.go index 3235db1..4c6ddd1 100644 --- a/votingpool/input_selection_wb_test.go +++ b/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.") } diff --git a/votingpool/withdrawal.go b/votingpool/withdrawal.go index f47abde..ac426e0 100644 --- a/votingpool/withdrawal.go +++ b/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 diff --git a/votingpool/withdrawal_wb_test.go b/votingpool/withdrawal_wb_test.go index 43aca65..894f0f5 100644 --- a/votingpool/withdrawal_wb_test.go +++ b/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) {