Skip to content

Commit

Permalink
Wallet: throw more appropriate exception types during completion.
Browse files Browse the repository at this point in the history
Resolves issue 560.
  • Loading branch information
mikehearn committed May 21, 2014
1 parent 028a1cc commit b47995e
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 14 deletions.
22 changes: 13 additions & 9 deletions core/src/main/java/com/google/bitcoin/core/Wallet.java
Original file line number Diff line number Diff line change
Expand Up @@ -1884,15 +1884,22 @@ public Transaction sendCoins(Peer peer, SendRequest request) throws Insufficient
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
* the fee parameter.
*
* @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();
Expand Down Expand Up @@ -1925,7 +1932,7 @@ public void completeTx(SendRequest req) throws InsufficientMoneyException {
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;
}
Expand Down Expand Up @@ -1967,7 +1974,7 @@ public void completeTx(SendRequest req) throws InsufficientMoneyException {
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);
Expand All @@ -1991,11 +1998,8 @@ public void completeTx(SendRequest req) throws InsufficientMoneyException {

// 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
Expand Down
9 changes: 5 additions & 4 deletions core/src/test/java/com/google/bitcoin/core/WalletTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1463,7 +1463,7 @@ public void ageMattersDuringSelection() throws Exception {
}
}

@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);
Expand Down Expand Up @@ -1498,11 +1498,12 @@ public void feeSolverAndCoinSelectionTest() throws Exception {
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)));
Expand Down Expand Up @@ -2179,7 +2180,7 @@ public void testEmptyWallet() throws Exception {
try {
wallet.completeTx(request);
fail();
} catch (InsufficientMoneyException.CouldNotAdjustDownwards e) {}
} catch (Wallet.CouldNotAdjustDownwards e) {}
request.ensureMinRequiredFee = false;
wallet.completeTx(request);
wallet.commitTx(request.tx);
Expand Down
2 changes: 1 addition & 1 deletion wallettemplate/src/main/java/wallettemplate/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ private void init(Stage mainWindow) throws IOException {
// last months worth or more (takes a few seconds).
bitcoin.setCheckpoints(getClass().getResourceAsStream("checkpoints"));
// As an example!
bitcoin.useTor();
// bitcoin.useTor();

This comment has been minimized.

Copy link
@schildbach

schildbach May 28, 2014

Member

Why did you uncomment usage of Tor and what has it got with exception types during transaction completion? It should be noted in the commit comment imho.

This comment has been minimized.

Copy link
@mikehearn

mikehearn via email May 28, 2014

Author Member
}

// Now configure and start the appkit. This will take a second or two - we could show a temporary splash screen
Expand Down

0 comments on commit b47995e

Please sign in to comment.