From b47995ed97ef560eff2766a003591c6e17843276 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 21 May 2014 19:38:12 +0200 Subject: [PATCH] Wallet: throw more appropriate exception types during completion. Resolves issue 560. --- .../java/com/google/bitcoin/core/Wallet.java | 22 +++++++++++-------- .../com/google/bitcoin/core/WalletTest.java | 9 ++++---- .../src/main/java/wallettemplate/Main.java | 2 +- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/com/google/bitcoin/core/Wallet.java b/core/src/main/java/com/google/bitcoin/core/Wallet.java index f284666b..41935cc3 100644 --- a/core/src/main/java/com/google/bitcoin/core/Wallet.java +++ b/core/src/main/java/com/google/bitcoin/core/Wallet.java @@ -1884,6 +1884,11 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha return tx; } + public static class CompletionException extends RuntimeException {} + public static class DustySendRequested extends CompletionException {} + public static class CouldNotAdjustDownwards extends CompletionException {} + public static class ExceededMaxTransactionSize extends CompletionException {} + /** * Given a spend request containing an incomplete transaction, makes it valid by adding outputs and signed inputs * according to the instructions in the request. The transaction in the request is modified by this method, as is @@ -1891,8 +1896,10 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha * * @param req a SendRequest that contains the incomplete transaction and details for how to make it valid. * @throws InsufficientMoneyException if the request could not be completed due to not enough balance. - * @throws IllegalArgumentException if you try and complete the same SendRequest twice, or if the given send request - * cannot be completed without violating the protocol rules. + * @throws IllegalArgumentException if you try and complete the same SendRequest twice + * @throws DustySendRequested if the resultant transaction would violate the dust rules (an output that's too small to be worthwhile) + * @throws CouldNotAdjustDownwards if emptying the wallet was requested and the output can't be shrunk for fees without violating a protocol rule. + * @throws ExceededMaxTransactionSize if the resultant transaction is too big for Bitcoin to process (try breaking up the amounts of value) */ public void completeTx(SendRequest req) throws InsufficientMoneyException { lock.lock(); @@ -1925,7 +1932,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha for (TransactionOutput output : req.tx.getOutputs()) if (output.getValue().compareTo(Utils.CENT) < 0) { if (output.getValue().compareTo(output.getMinNonDustValue()) < 0) - throw new IllegalArgumentException("Tried to send dust with ensureMinRequiredFee set - no way to complete this"); + throw new DustySendRequested(); needAtLeastReferenceFee = true; break; } @@ -1967,7 +1974,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha final BigInteger feePerKb = req.feePerKb == null ? BigInteger.ZERO : req.feePerKb; Transaction tx = req.tx; if (!adjustOutputDownwardsForFee(tx, bestCoinSelection, baseFee, feePerKb)) - throw new InsufficientMoneyException.CouldNotAdjustDownwards(); + throw new CouldNotAdjustDownwards(); } totalInput = totalInput.add(bestCoinSelection.valueGathered); @@ -1991,11 +1998,8 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha // Check size. int size = req.tx.bitcoinSerialize().length; - if (size > Transaction.MAX_STANDARD_TX_SIZE) { - throw new IllegalArgumentException( - String.format("Transaction could not be created without exceeding max size: %d vs %d", size, - Transaction.MAX_STANDARD_TX_SIZE)); - } + if (size > Transaction.MAX_STANDARD_TX_SIZE) + throw new ExceededMaxTransactionSize(); // Label the transaction as being self created. We can use this later to spend its change output even before // the transaction is confirmed. We deliberately won't bother notifying listeners here as there's not much diff --git a/core/src/test/java/com/google/bitcoin/core/WalletTest.java b/core/src/test/java/com/google/bitcoin/core/WalletTest.java index 73521442..12a25e7d 100644 --- a/core/src/test/java/com/google/bitcoin/core/WalletTest.java +++ b/core/src/test/java/com/google/bitcoin/core/WalletTest.java @@ -1463,7 +1463,7 @@ public class WalletTest extends TestWithWallet { } } - @Test(expected = IllegalArgumentException.class) + @Test(expected = Wallet.ExceededMaxTransactionSize.class) public void respectMaxStandardSize() throws Exception { // Check that we won't create txns > 100kb. Average tx size is ~220 bytes so this would have to be enormous. sendMoneyToWallet(Utils.toNanoCoins(100, 0), AbstractBlockChain.NewBlockType.BEST_CHAIN); @@ -1498,11 +1498,12 @@ public class WalletTest extends TestWithWallet { Transaction tx3 = createFakeTx(params, BigInteger.TEN, myAddress); wallet.receiveFromBlock(tx3, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, 2); - // No way we can add nearly enough fee + // Not allowed to send dust. try { wallet.createSend(notMyAddr, BigInteger.ONE); fail(); - } catch (IllegalArgumentException e) { + } catch (Wallet.DustySendRequested e) { + // Expected. } // Spend it all without fee enforcement SendRequest req = SendRequest.to(notMyAddr, BigInteger.TEN.add(BigInteger.ONE.add(BigInteger.ONE))); @@ -2179,7 +2180,7 @@ public class WalletTest extends TestWithWallet { try { wallet.completeTx(request); fail(); - } catch (InsufficientMoneyException.CouldNotAdjustDownwards e) {} + } catch (Wallet.CouldNotAdjustDownwards e) {} request.ensureMinRequiredFee = false; wallet.completeTx(request); wallet.commitTx(request.tx); diff --git a/wallettemplate/src/main/java/wallettemplate/Main.java b/wallettemplate/src/main/java/wallettemplate/Main.java index 540d69f0..fd7fc04e 100644 --- a/wallettemplate/src/main/java/wallettemplate/Main.java +++ b/wallettemplate/src/main/java/wallettemplate/Main.java @@ -87,7 +87,7 @@ public class Main extends Application { // last months worth or more (takes a few seconds). bitcoin.setCheckpoints(getClass().getResourceAsStream("checkpoints")); // As an example! - bitcoin.useTor(); + // bitcoin.useTor(); } // Now configure and start the appkit. This will take a second or two - we could show a temporary splash screen