From 2be7ee33f84b0de1ade0c66228b295744dae6f15 Mon Sep 17 00:00:00 2001 From: Sean Gilligan Date: Wed, 6 Sep 2023 12:14:18 -0700 Subject: [PATCH] DefaultCoinSelector: extract `compareByDepth()` comparator --- .../bitcoinj/wallet/DefaultCoinSelector.java | 53 ++++++++++++------- .../wallet/DefaultCoinSelectorTest.java | 4 +- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/wallet/DefaultCoinSelector.java b/core/src/main/java/org/bitcoinj/wallet/DefaultCoinSelector.java index 730e78fe8..a76c76979 100644 --- a/core/src/main/java/org/bitcoinj/wallet/DefaultCoinSelector.java +++ b/core/src/main/java/org/bitcoinj/wallet/DefaultCoinSelector.java @@ -16,7 +16,6 @@ package org.bitcoinj.wallet; -import com.google.common.annotations.VisibleForTesting; import org.bitcoinj.base.BitcoinNetwork; import org.bitcoinj.base.Coin; import org.bitcoinj.base.Network; @@ -27,6 +26,7 @@ import org.bitcoinj.core.TransactionOutput; import java.math.BigInteger; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.List; /** @@ -54,7 +54,7 @@ public class DefaultCoinSelector implements CoinSelector { // When calculating the wallet balance, we may be asked to select all possible coins, if so, avoid sorting // them in order to improve performance. if (!target.equals(BitcoinNetwork.MAX_MONEY)) { - sortOutputs(sortedOutputs); + sortedOutputs.sort(DefaultCoinSelector::compareByDepth); } // Now iterate over the sorted outputs until we have got as close to the target as possible or a little // bit over (excessive value will be change). @@ -71,24 +71,37 @@ public class DefaultCoinSelector implements CoinSelector { return new CoinSelection(selected); } - @VisibleForTesting static void sortOutputs(ArrayList outputs) { - Collections.sort(outputs, (a, b) -> { - int depth1 = a.getParentTransactionDepthInBlocks(); - int depth2 = b.getParentTransactionDepthInBlocks(); - Coin aValue = a.getValue(); - Coin bValue = b.getValue(); - BigInteger aCoinDepth = BigInteger.valueOf(aValue.value).multiply(BigInteger.valueOf(depth1)); - BigInteger bCoinDepth = BigInteger.valueOf(bValue.value).multiply(BigInteger.valueOf(depth2)); - int c1 = bCoinDepth.compareTo(aCoinDepth); - if (c1 != 0) return c1; - // The "coin*days" destroyed are equal, sort by value alone to get the lowest transaction size. - int c2 = bValue.compareTo(aValue); - if (c2 != 0) return c2; - // They are entirely equivalent (possibly pending) so sort by hash to ensure a total ordering. - BigInteger aHash = a.getParentTransactionHash().toBigInteger(); - BigInteger bHash = b.getParentTransactionHash().toBigInteger(); - return aHash.compareTo(bHash); - }); + /** + * Comparator for sorting {@link TransactionOutput} by coin depth, value, and then hash. + * @param a The first object to be compared + * @param b The second object to be compared + * @return a negative integer, zero, or a positive integer as the first argument is + * less than, equal to, or greater than the second. + */ + public static int compareByDepth(TransactionOutput a, TransactionOutput b) { + int depth1 = a.getParentTransactionDepthInBlocks(); + int depth2 = b.getParentTransactionDepthInBlocks(); + Coin aValue = a.getValue(); + Coin bValue = b.getValue(); + BigInteger aCoinDepth = BigInteger.valueOf(aValue.value).multiply(BigInteger.valueOf(depth1)); + BigInteger bCoinDepth = BigInteger.valueOf(bValue.value).multiply(BigInteger.valueOf(depth2)); + int c1 = bCoinDepth.compareTo(aCoinDepth); + if (c1 != 0) return c1; + // The "coin*days" destroyed are equal, sort by value alone to get the lowest transaction size. + int c2 = bValue.compareTo(aValue); + if (c2 != 0) return c2; + // They are entirely equivalent (possibly pending) so sort by hash to ensure a total ordering. + BigInteger aHash = a.getParentTransactionHash().toBigInteger(); + BigInteger bHash = b.getParentTransactionHash().toBigInteger(); + return aHash.compareTo(bHash); + }; + + /** + * @deprecated Use {@link #compareByDepth(TransactionOutput, TransactionOutput)} with {@link List#sort(Comparator)} + */ + @Deprecated + static void sortOutputs(ArrayList outputs) { + Collections.sort(outputs, DefaultCoinSelector::compareByDepth); } /** Sub-classes can override this to just customize whether transactions are usable, but keep age sorting. */ diff --git a/core/src/test/java/org/bitcoinj/wallet/DefaultCoinSelectorTest.java b/core/src/test/java/org/bitcoinj/wallet/DefaultCoinSelectorTest.java index 0d3dbd799..9028f64fd 100644 --- a/core/src/test/java/org/bitcoinj/wallet/DefaultCoinSelectorTest.java +++ b/core/src/test/java/org/bitcoinj/wallet/DefaultCoinSelectorTest.java @@ -96,7 +96,7 @@ public class DefaultCoinSelectorTest extends TestWithWallet { ArrayList candidates = new ArrayList<>(); candidates.add(t2.getOutput(0)); candidates.add(t1.getOutput(0)); - DefaultCoinSelector.sortOutputs(candidates); + candidates.sort(DefaultCoinSelector::compareByDepth); assertEquals(t1.getOutput(0), candidates.get(0)); assertEquals(t2.getOutput(0), candidates.get(1)); } @@ -117,7 +117,7 @@ public class DefaultCoinSelectorTest extends TestWithWallet { candidates.add(t3.getOutput(0)); candidates.add(t2.getOutput(0)); candidates.add(t1.getOutput(0)); - DefaultCoinSelector.sortOutputs(candidates); + candidates.sort(DefaultCoinSelector::compareByDepth); assertEquals(t2.getOutput(0), candidates.get(0)); assertEquals(t1.getOutput(0), candidates.get(1)); assertEquals(t3.getOutput(0), candidates.get(2));