From 70cd2ffb968149fa2567df9ee60b8b16e69c4542 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Tue, 26 Nov 2013 18:22:01 +0100 Subject: [PATCH] Wallet: throw exceptions when completing a transaction fails. If there's insufficient balance (typical error) then InsufficientMoneyException is thrown (checked). If the SendRequest is bad in some way, like asking to create a spend that would violate the protocol rules, IllegalArgumentException is thrown (unchecked). Also make VerificationException (and thus ProtocolException+ScriptException) unchecked. Resolves issue 425. --- .../core/InsufficientMoneyException.java | 32 +++++ .../com/google/bitcoin/core/Transaction.java | 1 - .../bitcoin/core/VerificationException.java | 2 +- .../java/com/google/bitcoin/core/Wallet.java | 99 ++++++--------- .../channels/IPaymentChannelClient.java | 3 +- .../channels/PaymentChannelClient.java | 12 +- .../PaymentChannelClientConnection.java | 3 +- .../channels/PaymentChannelClientState.java | 8 +- .../channels/PaymentChannelServer.java | 15 ++- .../channels/PaymentChannelServerState.java | 19 +-- .../StoredPaymentChannelServerStates.java | 2 +- .../google/bitcoin/core/BlockChainTest.java | 14 ++- .../com/google/bitcoin/core/WalletTest.java | 119 ++++++++++-------- .../channels/ChannelConnectionTest.java | 2 +- .../channels/PaymentChannelStateTest.java | 8 +- .../bitcoin/examples/ForwardingService.java | 3 + .../com/google/bitcoin/tools/WalletTool.java | 8 +- .../wallettemplate/SendMoneyController.java | 14 +-- 18 files changed, 203 insertions(+), 161 deletions(-) diff --git a/core/src/main/java/com/google/bitcoin/core/InsufficientMoneyException.java b/core/src/main/java/com/google/bitcoin/core/InsufficientMoneyException.java index 0f121106..c42b66ac 100644 --- a/core/src/main/java/com/google/bitcoin/core/InsufficientMoneyException.java +++ b/core/src/main/java/com/google/bitcoin/core/InsufficientMoneyException.java @@ -16,8 +16,40 @@ package com.google.bitcoin.core; +import javax.annotation.Nullable; +import java.math.BigInteger; + +import static com.google.common.base.Preconditions.checkNotNull; + /** * Thrown to indicate that you don't have enough money available to perform the requested operation. */ public class InsufficientMoneyException extends Exception { + /** Contains the number of satoshis that would have been required to complete the operation. */ + @Nullable + public final BigInteger missing; + + protected InsufficientMoneyException() { + this.missing = null; + } + + public InsufficientMoneyException(BigInteger missing) { + this(missing, "Insufficient money, missing " + missing + " satoshis"); + } + + public InsufficientMoneyException(BigInteger missing, String message) { + super(message); + this.missing = checkNotNull(missing); + } + + /** + * Thrown when we were trying to empty the wallet, and the total amount of money we were trying to empty after + * being reduced for the fee was smaller than the min payment. Note that the missing field will be null in this + * case. + */ + public static class CouldNotAdjustDownwards extends InsufficientMoneyException { + public CouldNotAdjustDownwards() { + super(); + } + } } diff --git a/core/src/main/java/com/google/bitcoin/core/Transaction.java b/core/src/main/java/com/google/bitcoin/core/Transaction.java index 1acc701e..4a13e4bd 100644 --- a/core/src/main/java/com/google/bitcoin/core/Transaction.java +++ b/core/src/main/java/com/google/bitcoin/core/Transaction.java @@ -795,7 +795,6 @@ public class Transaction extends ChildMessage implements Serializable { * @param aesKey The AES key to use to decrypt the key before signing. Null if no decryption is required. */ public synchronized void signInputs(SigHash hashType, Wallet wallet, @Nullable KeyParameter aesKey) throws ScriptException { - // TODO: This should be a method of the TransactionInput that (possibly?) operates with a copy of this object. Preconditions.checkState(inputs.size() > 0); Preconditions.checkState(outputs.size() > 0); diff --git a/core/src/main/java/com/google/bitcoin/core/VerificationException.java b/core/src/main/java/com/google/bitcoin/core/VerificationException.java index 967edfec..4d4ab830 100644 --- a/core/src/main/java/com/google/bitcoin/core/VerificationException.java +++ b/core/src/main/java/com/google/bitcoin/core/VerificationException.java @@ -17,7 +17,7 @@ package com.google.bitcoin.core; @SuppressWarnings("serial") -public class VerificationException extends Exception { +public class VerificationException extends RuntimeException { public VerificationException(String msg) { super(msg); } 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 9311b309..f186dc98 100644 --- a/core/src/main/java/com/google/bitcoin/core/Wallet.java +++ b/core/src/main/java/com/google/bitcoin/core/Wallet.java @@ -26,7 +26,6 @@ import com.google.bitcoin.store.WalletProtobufSerializer; import com.google.bitcoin.utils.ListenerRegistration; import com.google.bitcoin.utils.Threading; import com.google.bitcoin.wallet.*; -import com.google.common.base.Preconditions; import com.google.common.collect.*; import com.google.common.primitives.Ints; import com.google.common.util.concurrent.FutureCallback; @@ -1640,15 +1639,12 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi * @param nanocoins How much currency to send, in nanocoins. * @return either the created Transaction or null if there are insufficient coins. * coins as spent until commitTx is called on the result. + * @throws InsufficientMoneyException if the request could not be completed due to not enough balance. */ - @Nullable - public Transaction createSend(Address address, BigInteger nanocoins) { + public Transaction createSend(Address address, BigInteger nanocoins) throws InsufficientMoneyException { SendRequest req = SendRequest.to(address, nanocoins); - if (completeTx(req)) { - return req.tx; - } else { - return null; // No money. - } + completeTx(req); + return req.tx; } /** @@ -1657,18 +1653,15 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi * announced to the network. The given {@link SendRequest} is completed first using * {@link Wallet#completeTx(Wallet.SendRequest)} to make it valid. * - * @return the Transaction that was created, or null if there are insufficient coins in the wallet. + * @return the Transaction that was created + * @throws InsufficientMoneyException if the request could not be completed due to not enough balance. */ - @Nullable - public Transaction sendCoinsOffline(SendRequest request) { + public Transaction sendCoinsOffline(SendRequest request) throws InsufficientMoneyException { lock.lock(); try { - if (!completeTx(request)) - return null; // Not enough money! :-( + completeTx(request); commitTx(request.tx); return request.tx; - } catch (VerificationException e) { - throw new RuntimeException(e); // Cannot happen unless there's a bug, as we just created this ourselves. } finally { lock.unlock(); } @@ -1694,9 +1687,9 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi * @param to Which address to send coins to. * @param value How much value to send. You can use Utils.toNanoCoins() to calculate this. * @return An object containing the transaction that was created, and a future for the broadcast of it. + * @throws InsufficientMoneyException if the request could not be completed due to not enough balance. */ - @Nullable - public SendResult sendCoins(TransactionBroadcaster broadcaster, Address to, BigInteger value) { + public SendResult sendCoins(TransactionBroadcaster broadcaster, Address to, BigInteger value) throws InsufficientMoneyException { SendRequest request = SendRequest.to(to, value); return sendCoins(broadcaster, request); } @@ -1715,9 +1708,9 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi * @param broadcaster the target to use for broadcast. * @param request the SendRequest that describes what to do, get one using static methods on SendRequest itself. * @return An object containing the transaction that was created, and a future for the broadcast of it. + * @throws InsufficientMoneyException if the request could not be completed due to not enough balance. */ - @Nullable - public SendResult sendCoins(TransactionBroadcaster broadcaster, SendRequest request) { + public SendResult sendCoins(TransactionBroadcaster broadcaster, SendRequest request) throws InsufficientMoneyException { // Should not be locked here, as we're going to call into the broadcaster and that might want to hold its // own lock. sendCoinsOffline handles everything that needs to be locked. checkState(!lock.isHeldByCurrentThread()); @@ -1725,8 +1718,6 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi // Commit the TX to the wallet immediately so the spent coins won't be reused. // TODO: We should probably allow the request to specify tx commit only after the network has accepted it. Transaction tx = sendCoinsOffline(request); - if (tx == null) - return null; // Not enough money. SendResult result = new SendResult(); result.tx = tx; // The tx has been committed to the pending pool by this point (via sendCoinsOffline -> commitTx), so it has @@ -1745,9 +1736,9 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi * @param request the SendRequest that describes what to do, get one using static methods on SendRequest itself. * @return An object containing the transaction that was created, and a future for the broadcast of it. * @throws IllegalStateException if no transaction broadcaster has been configured. + * @throws InsufficientMoneyException if the request could not be completed due to not enough balance. */ - @Nullable - public SendResult sendCoins(SendRequest request) { + public SendResult sendCoins(SendRequest request) throws InsufficientMoneyException { TransactionBroadcaster broadcaster = vTransactionBroadcaster; checkState(broadcaster != null, "No transaction broadcaster is configured"); return sendCoins(broadcaster, request); @@ -1760,13 +1751,11 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi * if one may be required for the transaction to be confirmed. * * @return The {@link Transaction} that was created or null if there was insufficient balance to send the coins. + * @throws InsufficientMoneyException if the request could not be completed due to not enough balance. * @throws IOException if there was a problem broadcasting the transaction */ - @Nullable - public Transaction sendCoins(Peer peer, SendRequest request) throws IOException { + public Transaction sendCoins(Peer peer, SendRequest request) throws IOException, InsufficientMoneyException { Transaction tx = sendCoinsOffline(request); - if (tx == null) - return null; // Not enough money. peer.sendMessage(tx); return tx; } @@ -1777,13 +1766,14 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi * the fee parameter. * * @param req a SendRequest that contains the incomplete transaction and details for how to make it valid. - * @throws IllegalArgumentException if you try and complete the same SendRequest twice. - * @return whether or not the requested send is affordable. + * @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. */ - public boolean completeTx(SendRequest req) { + public void completeTx(SendRequest req) throws InsufficientMoneyException { lock.lock(); try { - Preconditions.checkArgument(!req.completed, "Given SendRequest has already been completed."); + checkArgument(!req.completed, "Given SendRequest has already been completed."); // Calculate the amount of value we need to import. BigInteger value = BigInteger.ZERO; for (TransactionOutput output : req.tx.getOutputs()) { @@ -1810,10 +1800,8 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi if (req.ensureMinRequiredFee && !req.emptyWallet) { // min fee checking is handled later for emptyWallet for (TransactionOutput output : req.tx.getOutputs()) if (output.getValue().compareTo(Utils.CENT) < 0) { - if (output.getValue().compareTo(output.getMinNonDustValue()) < 0) { - log.error("Tried to send dust with ensureMinRequiredFee set - no way to complete this"); - return false; - } + if (output.getValue().compareTo(output.getMinNonDustValue()) < 0) + throw new IllegalArgumentException("Tried to send dust with ensureMinRequiredFee set - no way to complete this"); needAtLeastReferenceFee = true; break; } @@ -1833,13 +1821,7 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi if (!req.emptyWallet) { // This can throw InsufficientMoneyException. FeeCalculation feeCalculation; - try { - feeCalculation = new FeeCalculation(req, value, originalInputs, needAtLeastReferenceFee, candidates); - } catch (InsufficientMoneyException e) { - // TODO: Propagate this after 0.9 is released and stop returning a boolean. - log.error("Insufficent money in wallet to pay the required fee"); - return false; - } + feeCalculation = new FeeCalculation(req, value, originalInputs, needAtLeastReferenceFee, candidates); bestCoinSelection = feeCalculation.bestCoinSelection; bestChangeOutput = feeCalculation.bestChangeOutput; } else { @@ -1859,10 +1841,8 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi final BigInteger baseFee = req.fee == null ? BigInteger.ZERO : req.fee; final BigInteger feePerKb = req.feePerKb == null ? BigInteger.ZERO : req.feePerKb; Transaction tx = req.tx; - if (!adjustOutputDownwardsForFee(tx, bestCoinSelection, baseFee, feePerKb)) { - log.error("Could not adjust output downwards to pay min fee."); - return false; - } + if (!adjustOutputDownwardsForFee(tx, bestCoinSelection, baseFee, feePerKb)) + throw new InsufficientMoneyException.CouldNotAdjustDownwards(); } totalInput = totalInput.add(bestCoinSelection.valueGathered); @@ -1878,21 +1858,14 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi } // Now sign the inputs, thus proving that we are entitled to redeem the connected outputs. - try { - req.tx.signInputs(Transaction.SigHash.ALL, this, req.aesKey); - } catch (ScriptException e) { - // If this happens it means an output script in a wallet tx could not be understood. That should never - // happen, if it does it means the wallet has got into an inconsistent state. - throw new RuntimeException(e); - } + req.tx.signInputs(Transaction.SigHash.ALL, this, req.aesKey); // Check size. int size = req.tx.bitcoinSerialize().length; if (size > Transaction.MAX_STANDARD_TX_SIZE) { - // TODO: Throw an unchecked protocol exception here. - log.error("Transaction could not be created without exceeding max size: {} vs {}", size, - Transaction.MAX_STANDARD_TX_SIZE); - return false; + throw new IllegalArgumentException( + String.format("Transaction could not be created without exceeding max size: %d vs %d", size, + Transaction.MAX_STANDARD_TX_SIZE)); } // Label the transaction as being self created. We can use this later to spend its change output even before @@ -1906,7 +1879,6 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi req.completed = true; req.fee = calculatedFee; log.info(" completed: {}", req.tx); - return true; } finally { lock.unlock(); } @@ -3165,7 +3137,7 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi // We keep track of the last size of the transaction we calculated but only if the act of adding inputs and // change resulted in the size crossing a 1000 byte boundary. Otherwise it stays at zero. int lastCalculatedSize = 0; - BigInteger valueNeeded; + BigInteger valueNeeded, valueMissing = null; while (true) { resetTxInputs(req, originalInputs); @@ -3188,8 +3160,10 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi CoinSelector selector = req.coinSelector == null ? coinSelector : req.coinSelector; CoinSelection selection = selector.select(valueNeeded, candidates); // Can we afford this? - if (selection.valueGathered.compareTo(valueNeeded) < 0) + if (selection.valueGathered.compareTo(valueNeeded) < 0) { + valueMissing = valueNeeded.subtract(selection.valueGathered); break; + } checkState(selection.gathered.size() > 0 || originalInputs.size() > 0); // We keep track of an upper bound on transaction size to calculate fees that need to be added. @@ -3292,8 +3266,9 @@ public class Wallet implements Serializable, BlockChainListener, PeerFilterProvi resetTxInputs(req, originalInputs); if (selection3 == null && selection2 == null && selection1 == null) { - log.warn("Insufficient value in wallet for send: needed {}", bitcoinValueToFriendlyString(valueNeeded)); - throw new InsufficientMoneyException(); + checkNotNull(valueMissing); + log.warn("Insufficient value in wallet for send: needed {} more", bitcoinValueToFriendlyString(valueMissing)); + throw new InsufficientMoneyException(valueMissing); } BigInteger lowestFee = null; diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/IPaymentChannelClient.java b/core/src/main/java/com/google/bitcoin/protocols/channels/IPaymentChannelClient.java index 17ff9c17..0c2e644e 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/IPaymentChannelClient.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/IPaymentChannelClient.java @@ -16,6 +16,7 @@ package com.google.bitcoin.protocols.channels; +import com.google.bitcoin.core.InsufficientMoneyException; import com.google.common.util.concurrent.ListenableFuture; import org.bitcoin.paymentchannel.Protos; @@ -31,7 +32,7 @@ public interface IPaymentChannelClient { * Called when a message is received from the server. Processes the given message and generates events based on its * content. */ - void receiveMessage(Protos.TwoWayChannelMessage msg) throws ValueOutOfRangeException; + void receiveMessage(Protos.TwoWayChannelMessage msg) throws InsufficientMoneyException; /** *

Called when the connection to the server terminates.

diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClient.java b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClient.java index 1cfcdd2d..20d4daa3 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClient.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClient.java @@ -129,7 +129,7 @@ public class PaymentChannelClient implements IPaymentChannelClient { @Nullable @GuardedBy("lock") - private CloseReason receiveInitiate(Protos.Initiate initiate, BigInteger contractValue, Protos.Error.Builder errorBuilder) throws VerificationException, ValueOutOfRangeException { + private CloseReason receiveInitiate(Protos.Initiate initiate, BigInteger contractValue, Protos.Error.Builder errorBuilder) throws VerificationException, InsufficientMoneyException { log.info("Got INITIATE message:\n{}", initiate.toString()); checkState(initiate.getExpireTimeSecs() > 0 && initiate.getMinAcceptedChannelSize() >= 0); @@ -163,7 +163,13 @@ public class PaymentChannelClient implements IPaymentChannelClient { state = new PaymentChannelClientState(wallet, myKey, new ECKey(null, initiate.getMultisigKey().toByteArray()), contractValue, initiate.getExpireTimeSecs()); - state.initiate(); + try { + state.initiate(); + } catch (ValueOutOfRangeException e) { + log.error("Value out of range when trying to initiate", e); + errorBuilder.setCode(Protos.Error.ErrorCode.CHANNEL_VALUE_TOO_LARGE); + return CloseReason.SERVER_REQUESTED_TOO_MUCH_VALUE; + } minPayment = initiate.getMinPayment(); step = InitStep.WAITING_FOR_REFUND_RETURN; @@ -230,7 +236,7 @@ public class PaymentChannelClient implements IPaymentChannelClient { * {@inheritDoc} */ @Override - public void receiveMessage(Protos.TwoWayChannelMessage msg) throws ValueOutOfRangeException { + public void receiveMessage(Protos.TwoWayChannelMessage msg) throws InsufficientMoneyException { lock.lock(); try { checkState(connectionOpen); diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClientConnection.java b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClientConnection.java index 3878b699..3e2e4bc3 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClientConnection.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClientConnection.java @@ -17,6 +17,7 @@ package com.google.bitcoin.protocols.channels; import com.google.bitcoin.core.ECKey; +import com.google.bitcoin.core.InsufficientMoneyException; import com.google.bitcoin.core.Sha256Hash; import com.google.bitcoin.core.Wallet; import com.google.bitcoin.protocols.niowrapper.NioClient; @@ -89,7 +90,7 @@ public class PaymentChannelClientConnection { public void messageReceived(ProtobufParser handler, Protos.TwoWayChannelMessage msg) { try { channelClient.receiveMessage(msg); - } catch (ValueOutOfRangeException e) { + } catch (InsufficientMoneyException e) { // We should only get this exception during INITIATE, so channelOpen wasn't called yet. channelOpenFuture.setException(e); } diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClientState.java b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClientState.java index 93adbf11..5ed8d26b 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClientState.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClientState.java @@ -235,9 +235,10 @@ public class PaymentChannelClientState { * overriding {@link PaymentChannelClientState#editContractSendRequest(com.google.bitcoin.core.Wallet.SendRequest)}. * By default unconfirmed coins are allowed to be used, as for micropayments the risk should be relatively low. * - * @throws ValueOutOfRangeException If the value being used cannot be afforded or is too small to be accepted by the network + * @throws ValueOutOfRangeException if the value being used is too small to be accepted by the network + * @throws InsufficientMoneyException if the wallet doesn't contain enough balance to initiate */ - public synchronized void initiate() throws ValueOutOfRangeException { + public synchronized void initiate() throws ValueOutOfRangeException, InsufficientMoneyException { final NetworkParameters params = wallet.getParams(); Transaction template = new Transaction(params); // We always place the client key before the server key because, if either side wants some privacy, they can @@ -252,8 +253,7 @@ public class PaymentChannelClientState { Wallet.SendRequest req = Wallet.SendRequest.forTx(template); req.coinSelector = AllowUnconfirmedCoinSelector.get(); editContractSendRequest(req); - if (!wallet.completeTx(req)) - throw new ValueOutOfRangeException("Cannot afford this channel"); + wallet.completeTx(req); BigInteger multisigFee = req.fee; multisigContract = req.tx; // Build a refund transaction that protects us in the case of a bad server that's just trying to cause havoc diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServer.java b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServer.java index 9f7fffc0..c7b4b588 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServer.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServer.java @@ -271,6 +271,12 @@ public class PaymentChannelServer { log.error("Initial payment value was out of range", e); error(e.getMessage(), Protos.Error.ErrorCode.BAD_TRANSACTION, CloseReason.REMOTE_SENT_INVALID_MESSAGE); return; + } catch (InsufficientMoneyException e) { + // This shouldn't happen because the server shouldn't allow itself to get into this situation in the + // first place, by specifying a min up front payment. + log.error("Tried to settle channel and could not afford the fees whilst updating payment", e); + error(e.getMessage(), Protos.Error.ErrorCode.BAD_TRANSACTION, CloseReason.REMOTE_SENT_INVALID_MESSAGE); + return; } conn.sendToClient(Protos.TwoWayChannelMessage.newBuilder() .setType(Protos.TwoWayChannelMessage.MessageType.CHANNEL_OPEN) @@ -301,7 +307,7 @@ public class PaymentChannelServer { } @GuardedBy("lock") - private void receiveUpdatePaymentMessage(Protos.UpdatePayment msg, boolean sendAck) throws VerificationException, ValueOutOfRangeException { + private void receiveUpdatePaymentMessage(Protos.UpdatePayment msg, boolean sendAck) throws VerificationException, ValueOutOfRangeException, InsufficientMoneyException { log.info("Got a payment update"); BigInteger lastBestPayment = state.getBestValueToMe(); @@ -371,6 +377,9 @@ public class PaymentChannelServer { } catch (ValueOutOfRangeException e) { log.error("Caught value out of range exception handling message from client", e); error(e.getMessage(), Protos.Error.ErrorCode.BAD_TRANSACTION, CloseReason.REMOTE_SENT_INVALID_MESSAGE); + } catch (InsufficientMoneyException e) { + log.error("Caught insufficient money exception handling message from client", e); + error(e.getMessage(), Protos.Error.ErrorCode.BAD_TRANSACTION, CloseReason.REMOTE_SENT_INVALID_MESSAGE); } catch (IllegalStateException e) { log.error("Caught illegal state exception handling message from client", e); error(e.getMessage(), Protos.Error.ErrorCode.SYNTAX_ERROR, CloseReason.REMOTE_SENT_INVALID_MESSAGE); @@ -394,7 +403,7 @@ public class PaymentChannelServer { } @GuardedBy("lock") - private void receiveCloseMessage() throws ValueOutOfRangeException { + private void receiveCloseMessage() throws InsufficientMoneyException { log.info("Got CLOSE message, closing channel"); if (state != null) { settlePayment(CloseReason.CLIENT_REQUESTED_CLOSE); @@ -404,7 +413,7 @@ public class PaymentChannelServer { } @GuardedBy("lock") - private void settlePayment(final CloseReason clientRequestedClose) throws ValueOutOfRangeException { + private void settlePayment(final CloseReason clientRequestedClose) throws InsufficientMoneyException { // Setting channelSettling here prevents us from sending another CLOSE when state.close() calls // close() on us here below via the stored channel state. // TODO: Strongly separate the lifecycle of the payment channel from the TCP connection in these classes. diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServerState.java b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServerState.java index 6d2d0244..82729cbe 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServerState.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServerState.java @@ -283,7 +283,7 @@ public class PaymentChannelServerState { * @throws VerificationException If the signature does not verify or size is out of range (incl being rejected by the network as dust). * @return true if there is more value left on the channel, false if it is now fully used up. */ - public synchronized boolean incrementPayment(BigInteger refundSize, byte[] signatureBytes) throws VerificationException, ValueOutOfRangeException { + public synchronized boolean incrementPayment(BigInteger refundSize, byte[] signatureBytes) throws VerificationException, ValueOutOfRangeException, InsufficientMoneyException { checkState(state == State.READY); checkNotNull(refundSize); checkNotNull(signatureBytes); @@ -360,9 +360,9 @@ public class PaymentChannelServerState { * @return a future which completes when the provided multisig contract successfully broadcasts, or throws if the * broadcast fails for some reason. Note that if the network simply rejects the transaction, this future * will never complete, a timeout should be used. - * @throws ValueOutOfRangeException If the payment tx would have cost more in fees to spend than it is worth. + * @throws InsufficientMoneyException If the payment tx would have cost more in fees to spend than it is worth. */ - public synchronized ListenableFuture close() throws ValueOutOfRangeException { + public synchronized ListenableFuture close() throws InsufficientMoneyException { if (storedServerChannel != null) { StoredServerChannel temp = storedServerChannel; storedServerChannel = null; @@ -394,20 +394,21 @@ public class PaymentChannelServerState { // die. We could probably add features to the SendRequest API to make this a bit more efficient. signMultisigInput(tx, Transaction.SigHash.NONE, true); // Let wallet handle adding additional inputs/fee as necessary. - if (!wallet.completeTx(req)) - throw new ValueOutOfRangeException("Unable to complete transaction - unable to pay required fee"); + wallet.completeTx(req); feePaidForPayment = req.fee; log.info("Calculated fee is {}", feePaidForPayment); - if (feePaidForPayment.compareTo(bestValueToMe) >= 0) - throw new ValueOutOfRangeException(String.format("Had to pay more in fees (%s) than the channel was worth (%s)", - feePaidForPayment, bestValueToMe)); + if (feePaidForPayment.compareTo(bestValueToMe) >= 0) { + final String msg = String.format("Had to pay more in fees (%s) than the channel was worth (%s)", + feePaidForPayment, bestValueToMe); + throw new InsufficientMoneyException(feePaidForPayment.subtract(bestValueToMe), msg); + } // Now really sign the multisig input. signMultisigInput(tx, Transaction.SigHash.ALL, false); // Some checks that shouldn't be necessary but it can't hurt to check. tx.verify(); // Sanity check syntax. for (TransactionInput input : tx.getInputs()) input.verify(); // Run scripts and ensure it is valid. - } catch (ValueOutOfRangeException e) { + } catch (InsufficientMoneyException e) { throw e; // Don't fall through. } catch (Exception e) { log.error("Could not verify self-built tx\nMULTISIG {}\nCLOSE {}", multisigContract, tx != null ? tx : ""); diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelServerStates.java b/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelServerStates.java index b9985965..c980d4b4 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelServerStates.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelServerStates.java @@ -84,7 +84,7 @@ public class StoredPaymentChannelServerStates implements WalletExtension { channel.closeConnectedHandler(); try { channel.getOrCreateState(wallet, broadcaster).close(); - } catch (ValueOutOfRangeException e) { + } catch (InsufficientMoneyException e) { e.printStackTrace(); } catch (VerificationException e) { e.printStackTrace(); diff --git a/core/src/test/java/com/google/bitcoin/core/BlockChainTest.java b/core/src/test/java/com/google/bitcoin/core/BlockChainTest.java index 3789fa39..c92ef119 100644 --- a/core/src/test/java/com/google/bitcoin/core/BlockChainTest.java +++ b/core/src/test/java/com/google/bitcoin/core/BlockChainTest.java @@ -308,8 +308,11 @@ public class BlockChainTest { assertTrue(!coinbaseTransaction.isMature()); // Attempt to spend the coinbase - this should fail as the coinbase is not mature yet. - Transaction coinbaseSpend = wallet.createSend(addressToSendTo, Utils.toNanoCoins(49, 0)); - assertNull(coinbaseSpend); + try { + wallet.createSend(addressToSendTo, Utils.toNanoCoins(49, 0)); + fail(); + } catch (InsufficientMoneyException e) { + } // Check that the coinbase is unavailable to spend for the next spendableCoinbaseDepth - 2 blocks. for (int i = 0; i < unitTestParams.getSpendableCoinbaseDepth() - 2; i++) { @@ -328,8 +331,11 @@ public class BlockChainTest { assertTrue(!coinbaseTransaction.isMature()); // Attempt to spend the coinbase - this should fail. - coinbaseSpend = wallet.createSend(addressToSendTo, Utils.toNanoCoins(49, 0)); - assertNull(coinbaseSpend); + try { + wallet.createSend(addressToSendTo, Utils.toNanoCoins(49, 0)); + fail(); + } catch (InsufficientMoneyException e) { + } } // Give it one more block - should now be able to spend coinbase transaction. Non relevant tx. 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 c2f2724e..8b119862 100644 --- a/core/src/test/java/com/google/bitcoin/core/WalletTest.java +++ b/core/src/test/java/com/google/bitcoin/core/WalletTest.java @@ -125,12 +125,22 @@ public class WalletTest extends TestWithWallet { // arbitrary transaction in isolation, we'll check that the fee was set by examining the size of the change. // Receive some money as a pending transaction. - receiveAPendingTransaction(wallet, toAddress); + receiveATransaction(wallet, toAddress); + + // Try to send too much and fail. + Address destination = new ECKey().toAddress(params); + BigInteger vHuge = toNanoCoins(10, 0); + Wallet.SendRequest req = Wallet.SendRequest.to(destination, vHuge); + try { + wallet.completeTx(req); + fail(); + } catch (InsufficientMoneyException e) { + assertEquals(toNanoCoins(9, 0), e.missing); + } // Prepare to send. - Address destination = new ECKey().toAddress(params); BigInteger v2 = toNanoCoins(0, 50); - Wallet.SendRequest req = Wallet.SendRequest.to(destination, v2); + req = Wallet.SendRequest.to(destination, v2); req.fee = toNanoCoins(0, 1); if (testEncryption) { @@ -189,7 +199,7 @@ public class WalletTest extends TestWithWallet { spendUnconfirmedChange(wallet, t2, req.aesKey); } - private void receiveAPendingTransaction(Wallet wallet, Address toAddress) throws Exception { + private void receiveATransaction(Wallet wallet, Address toAddress) throws Exception { BigInteger v1 = Utils.toNanoCoins(1, 0); final ListenableFuture availFuture = wallet.getBalanceFuture(v1, Wallet.BalanceType.AVAILABLE); final ListenableFuture estimatedFuture = wallet.getBalanceFuture(v1, Wallet.BalanceType.ESTIMATED); @@ -292,10 +302,9 @@ public class WalletTest extends TestWithWallet { t2.addOutput(v4, a2); SendRequest req = SendRequest.forTx(t2); req.ensureMinRequiredFee = false; - boolean complete = wallet.completeTx(req); + wallet.completeTx(req); // Do some basic sanity checks. - assertTrue(complete); assertEquals(1, t2.getInputs().size()); assertEquals(myAddress, t2.getInputs().get(0).getScriptSig().getFromAddress(params)); assertEquals(TransactionConfidence.ConfidenceType.UNKNOWN, t2.getConfidence().getConfidenceType()); @@ -1024,8 +1033,7 @@ public class WalletTest extends TestWithWallet { t2.addOutput(o2); SendRequest req = SendRequest.forTx(t2); req.ensureMinRequiredFee = false; - boolean complete = wallet.completeTx(req); - assertTrue(complete); + wallet.completeTx(req); // Commit t2, so it is placed in the pending pool wallet.commitTx(t2); @@ -1213,7 +1221,7 @@ public class WalletTest extends TestWithWallet { } } - @Test + @Test(expected = IllegalArgumentException.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); @@ -1226,7 +1234,7 @@ public class WalletTest extends TestWithWallet { tx.addOutput(v, new Address(params, bits)); } Wallet.SendRequest req = Wallet.SendRequest.forTx(tx); - assertFalse(wallet.completeTx(req)); + wallet.completeTx(req); } @Test @@ -1249,7 +1257,11 @@ public class WalletTest extends TestWithWallet { wallet.receiveFromBlock(tx3, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, 2); // No way we can add nearly enough fee - assertNull(wallet.createSend(notMyAddr, BigInteger.ONE)); + try { + wallet.createSend(notMyAddr, BigInteger.ONE); + fail(); + } catch (IllegalArgumentException e) { + } // Spend it all without fee enforcement SendRequest req = SendRequest.to(notMyAddr, BigInteger.TEN.add(BigInteger.ONE.add(BigInteger.ONE))); req.ensureMinRequiredFee = false; @@ -1278,7 +1290,7 @@ public class WalletTest extends TestWithWallet { // ...but not more fee than what we request SendRequest request3 = SendRequest.to(notMyAddr, CENT.subtract(BigInteger.ONE)); request3.fee = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(BigInteger.ONE); - assertTrue(wallet.completeTx(request3)); + wallet.completeTx(request3); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(BigInteger.ONE), request3.fee); Transaction spend3 = request3.tx; assertEquals(2, spend3.getOutputs().size()); @@ -1289,7 +1301,7 @@ public class WalletTest extends TestWithWallet { // ...unless we need it SendRequest request4 = SendRequest.to(notMyAddr, CENT.subtract(BigInteger.ONE)); request4.fee = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.subtract(BigInteger.ONE); - assertTrue(wallet.completeTx(request4)); + wallet.completeTx(request4); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request4.fee); Transaction spend4 = request4.tx; assertEquals(2, spend4.getOutputs().size()); @@ -1298,7 +1310,7 @@ public class WalletTest extends TestWithWallet { Utils.COIN.subtract(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE)); SendRequest request5 = SendRequest.to(notMyAddr, Utils.COIN.subtract(CENT.subtract(BigInteger.ONE))); - assertTrue(wallet.completeTx(request5)); + wallet.completeTx(request5); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request5.fee); Transaction spend5 = request5.tx; // If we would have a change output < 0.01, it should add the fee @@ -1308,7 +1320,7 @@ public class WalletTest extends TestWithWallet { Utils.COIN.subtract(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE)); SendRequest request6 = SendRequest.to(notMyAddr, Utils.COIN.subtract(CENT)); - assertTrue(wallet.completeTx(request6)); + wallet.completeTx(request6); assertEquals(BigInteger.ZERO, request6.fee); Transaction spend6 = request6.tx; // ...but not if change output == 0.01 @@ -1318,7 +1330,7 @@ public class WalletTest extends TestWithWallet { SendRequest request7 = SendRequest.to(notMyAddr, Utils.COIN.subtract(CENT.subtract(BigInteger.valueOf(2)).multiply(BigInteger.valueOf(2)))); request7.tx.addOutput(CENT.subtract(BigInteger.ONE), notMyAddr); - assertTrue(wallet.completeTx(request7)); + wallet.completeTx(request7); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request7.fee); Transaction spend7 = request7.tx; // If change is 0.1-nanocoin and we already have a 0.1-nanocoin output, fee should be reference fee @@ -1328,7 +1340,7 @@ public class WalletTest extends TestWithWallet { Utils.COIN.subtract(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE)); SendRequest request8 = SendRequest.to(notMyAddr, Utils.COIN.subtract(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE)); - assertTrue(wallet.completeTx(request8)); + wallet.completeTx(request8); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request8.fee); Transaction spend8 = request8.tx; // If we would have a change output == REFERENCE_DEFAULT_MIN_TX_FEE that would cause a fee, throw it away and make it fee @@ -1338,7 +1350,7 @@ public class WalletTest extends TestWithWallet { SendRequest request9 = SendRequest.to(notMyAddr, Utils.COIN.subtract( Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Transaction.MIN_NONDUST_OUTPUT))); - assertTrue(wallet.completeTx(request9)); + wallet.completeTx(request9); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Transaction.MIN_NONDUST_OUTPUT), request9.fee); Transaction spend9 = request9.tx; // ...in fact, also add fee if we would get back less than MIN_NONDUST_OUTPUT @@ -1349,7 +1361,7 @@ public class WalletTest extends TestWithWallet { SendRequest request10 = SendRequest.to(notMyAddr, Utils.COIN.subtract( Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Transaction.MIN_NONDUST_OUTPUT).add(BigInteger.ONE))); - assertTrue(wallet.completeTx(request10)); + wallet.completeTx(request10); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request10.fee); Transaction spend10 = request10.tx; // ...but if we get back any more than that, we should get a refund (but still pay fee) @@ -1361,7 +1373,7 @@ public class WalletTest extends TestWithWallet { SendRequest request11 = SendRequest.to(notMyAddr, Utils.COIN.subtract( Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Transaction.MIN_NONDUST_OUTPUT).add(BigInteger.valueOf(2)))); request11.fee = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(BigInteger.ONE); - assertTrue(wallet.completeTx(request11)); + wallet.completeTx(request11); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(BigInteger.ONE), request11.fee); Transaction spend11 = request11.tx; // ...of course fee should be min(request.fee, MIN_TX_FEE) so we should get MIN_TX_FEE.add(ONE) here @@ -1413,7 +1425,7 @@ public class WalletTest extends TestWithWallet { request15.tx.addOutput(CENT, notMyAddr); assertTrue(request15.tx.bitcoinSerialize().length > 1000); request15.feePerKb = BigInteger.ONE; - assertTrue(wallet.completeTx(request15)); + wallet.completeTx(request15); assertEquals(BigInteger.valueOf(2), request15.fee); Transaction spend15 = request15.tx; // If a transaction is over 1kb, 2 satoshis should be added. @@ -1429,7 +1441,7 @@ public class WalletTest extends TestWithWallet { for (int i = 0; i < 29; i++) request16.tx.addOutput(CENT, notMyAddr); assertTrue(request16.tx.bitcoinSerialize().length > 1000); - assertTrue(wallet.completeTx(request16)); + wallet.completeTx(request16); // Of course the fee shouldn't be added if feePerKb == 0 assertEquals(BigInteger.ZERO, request16.fee); Transaction spend16 = request16.tx; @@ -1446,7 +1458,7 @@ public class WalletTest extends TestWithWallet { request17.tx.addOutput(CENT, notMyAddr); request17.tx.addOutput(new TransactionOutput(params, request17.tx, CENT, new byte[15])); request17.feePerKb = BigInteger.ONE; - assertTrue(wallet.completeTx(request17)); + wallet.completeTx(request17); assertEquals(BigInteger.ONE, request17.fee); assertEquals(1, request17.tx.getInputs().size()); // Calculate its max length to make sure it is indeed 999 @@ -1474,7 +1486,7 @@ public class WalletTest extends TestWithWallet { request18.tx.addOutput(CENT, notMyAddr); request18.tx.addOutput(new TransactionOutput(params, request18.tx, CENT, new byte[17])); request18.feePerKb = BigInteger.ONE; - assertTrue(wallet.completeTx(request18)); + wallet.completeTx(request18); assertEquals(BigInteger.valueOf(2), request18.fee); assertEquals(1, request18.tx.getInputs().size()); // Calculate its max length to make sure it is indeed 1001 @@ -1501,7 +1513,7 @@ public class WalletTest extends TestWithWallet { for (int i = 0; i < 99; i++) request19.tx.addOutput(CENT, notMyAddr); // If we send now, we shouldn't need a fee and should only have to spend our COIN - assertTrue(wallet.completeTx(request19)); + wallet.completeTx(request19); assertEquals(BigInteger.ZERO, request19.fee); assertEquals(1, request19.tx.getInputs().size()); assertEquals(100, request19.tx.getOutputs().size()); @@ -1509,7 +1521,7 @@ public class WalletTest extends TestWithWallet { request19.tx.clearInputs(); request19 = SendRequest.forTx(request19.tx); request19.feePerKb = BigInteger.ONE; - assertTrue(wallet.completeTx(request19)); + wallet.completeTx(request19); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request19.fee); assertEquals(2, request19.tx.getInputs().size()); BigInteger outValue19 = BigInteger.ZERO; @@ -1526,7 +1538,7 @@ public class WalletTest extends TestWithWallet { for (int i = 0; i < 99; i++) request20.tx.addOutput(CENT, notMyAddr); // If we send now, we shouldn't have a fee and should only have to spend our COIN - assertTrue(wallet.completeTx(request20)); + wallet.completeTx(request20); assertEquals(BigInteger.ZERO, request20.fee); assertEquals(1, request20.tx.getInputs().size()); assertEquals(100, request20.tx.getOutputs().size()); @@ -1534,7 +1546,7 @@ public class WalletTest extends TestWithWallet { request20.tx.clearInputs(); request20 = SendRequest.forTx(request20.tx); request20.feePerKb = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE; - assertTrue(wallet.completeTx(request20)); + wallet.completeTx(request20); // 4kb tx. assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.multiply(BigInteger.valueOf(4)), request20.fee); assertEquals(2, request20.tx.getInputs().size()); @@ -1552,7 +1564,7 @@ public class WalletTest extends TestWithWallet { request21.tx.addOutput(CENT, notMyAddr); request21.tx.addOutput(CENT.subtract(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE), notMyAddr); // If we send without a feePerKb, we should still require REFERENCE_DEFAULT_MIN_TX_FEE because we have an output < 0.01 - assertTrue(wallet.completeTx(request21)); + wallet.completeTx(request21); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request21.fee); assertEquals(2, request21.tx.getInputs().size()); BigInteger outValue21 = BigInteger.ZERO; @@ -1567,7 +1579,7 @@ public class WalletTest extends TestWithWallet { for (int i = 0; i < 70; i++) request25.tx.addOutput(CENT, notMyAddr); // If we send now, we shouldn't need a fee and should only have to spend our COIN - assertTrue(wallet.completeTx(request25)); + wallet.completeTx(request25); assertEquals(BigInteger.ZERO, request25.fee); assertEquals(1, request25.tx.getInputs().size()); assertEquals(72, request25.tx.getOutputs().size()); @@ -1576,7 +1588,7 @@ public class WalletTest extends TestWithWallet { request25 = SendRequest.forTx(request25.tx); request25.feePerKb = CENT.divide(BigInteger.valueOf(3)); request25.ensureMinRequiredFee = false; - assertTrue(wallet.completeTx(request25)); + wallet.completeTx(request25); assertEquals(CENT.subtract(BigInteger.ONE), request25.fee); assertEquals(2, request25.tx.getInputs().size()); BigInteger outValue25 = BigInteger.ZERO; @@ -1604,7 +1616,7 @@ public class WalletTest extends TestWithWallet { Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Transaction.MIN_NONDUST_OUTPUT)), notMyAddr); assertTrue(request26.tx.bitcoinSerialize().length > 1000); request26.feePerKb = BigInteger.ONE; - assertTrue(wallet.completeTx(request26)); + wallet.completeTx(request26); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.add(Transaction.MIN_NONDUST_OUTPUT), request26.fee); Transaction spend26 = request26.tx; // If a transaction is over 1kb, the set fee should be added @@ -1637,7 +1649,7 @@ public class WalletTest extends TestWithWallet { // Create a spend that will throw away change (category 3 type 2 in which the change causes fee which is worth more than change) SendRequest request1 = SendRequest.to(notMyAddr, CENT.add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE).subtract(BigInteger.ONE)); - assertTrue(wallet.completeTx(request1)); + wallet.completeTx(request1); assertEquals(BigInteger.ONE, request1.fee); assertEquals(request1.tx.getInputs().size(), i); // We should have spent all inputs @@ -1648,7 +1660,7 @@ public class WalletTest extends TestWithWallet { // ... and create a spend that will throw away change (category 3 type 1 in which the change causes dust output) SendRequest request2 = SendRequest.to(notMyAddr, CENT.add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE).subtract(BigInteger.ONE)); - assertTrue(wallet.completeTx(request2)); + wallet.completeTx(request2); assertEquals(BigInteger.ONE, request2.fee); assertEquals(request2.tx.getInputs().size(), i - 1); // We should have spent all inputs - 1 @@ -1660,14 +1672,14 @@ public class WalletTest extends TestWithWallet { // ... and create a spend that will throw away change (category 3 type 1 in which the change causes dust output) // but that also could have been category 2 if it wanted SendRequest request3 = SendRequest.to(notMyAddr, CENT.add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE).subtract(BigInteger.ONE)); - assertTrue(wallet.completeTx(request3)); + wallet.completeTx(request3); assertEquals(BigInteger.ONE, request3.fee); assertEquals(request3.tx.getInputs().size(), i - 2); // We should have spent all inputs - 2 // SendRequest request4 = SendRequest.to(notMyAddr, CENT.add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE).subtract(BigInteger.ONE)); request4.feePerKb = Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.divide(BigInteger.valueOf(request3.tx.bitcoinSerialize().length)); - assertTrue(wallet.completeTx(request4)); + wallet.completeTx(request4); assertEquals(BigInteger.ONE, request4.fee); assertEquals(request4.tx.getInputs().size(), i - 2); // We should have spent all inputs - 2 @@ -1680,7 +1692,7 @@ public class WalletTest extends TestWithWallet { // ...that is just slightly less than is needed for category 1 SendRequest request5 = SendRequest.to(notMyAddr, CENT.add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE).subtract(BigInteger.ONE)); - assertTrue(wallet.completeTx(request5)); + wallet.completeTx(request5); assertEquals(BigInteger.ONE, request5.fee); assertEquals(1, request5.tx.getOutputs().size()); // We should have no change output @@ -1691,7 +1703,7 @@ public class WalletTest extends TestWithWallet { // ... that puts us in category 1 (no fee!) SendRequest request6 = SendRequest.to(notMyAddr, CENT.add(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE).subtract(BigInteger.ONE)); - assertTrue(wallet.completeTx(request6)); + wallet.completeTx(request6); assertEquals(BigInteger.ZERO, request6.fee); assertEquals(2, request6.tx.getOutputs().size()); // We should have a change output @@ -1718,7 +1730,7 @@ public class WalletTest extends TestWithWallet { // The selector will choose 2 with MIN_TX_FEE fee SendRequest request1 = SendRequest.to(notMyAddr, CENT.add(BigInteger.ONE)); - assertTrue(wallet.completeTx(request1)); + wallet.completeTx(request1); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request1.fee); assertEquals(request1.tx.getInputs().size(), i); // We should have spent all inputs assertEquals(2, request1.tx.getOutputs().size()); // and gotten change back @@ -1753,7 +1765,7 @@ public class WalletTest extends TestWithWallet { // It spends COIN + 1(fee) and because its output is thus < CENT, we have to pay MIN_TX_FEE // When it tries category 1, its too large and requires COIN + 2 (fee) // This adds the next input, but still has a < CENT output which means it cant reach category 1 - assertTrue(wallet.completeTx(request1)); + wallet.completeTx(request1); assertEquals(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE, request1.fee); assertEquals(2, request1.tx.getInputs().size()); @@ -1768,7 +1780,7 @@ public class WalletTest extends TestWithWallet { request2.tx.addOutput(new TransactionOutput(params, request2.tx, CENT, new byte[16])); request2.feePerKb = BigInteger.ONE; // The process is the same as above, but now we can complete category 1 with one more input, and pay a fee of 2 - assertTrue(wallet.completeTx(request2)); + wallet.completeTx(request2); assertEquals(BigInteger.valueOf(2), request2.fee); assertEquals(4, request2.tx.getInputs().size()); } @@ -1793,7 +1805,7 @@ public class WalletTest extends TestWithWallet { SendRequest request1 = SendRequest.to(notMyAddr, CENT); // If we just complete as-is, we will use one of the COIN outputs to get higher priority, // resulting in a change output - assertNotNull(wallet.completeTx(request1)); + wallet.completeTx(request1); assertEquals(1, request1.tx.getInputs().size()); assertEquals(2, request1.tx.getOutputs().size()); assertEquals(CENT, request1.tx.getOutput(0).getValue()); @@ -1803,7 +1815,7 @@ public class WalletTest extends TestWithWallet { SendRequest request2 = SendRequest.to(notMyAddr, CENT); request2.tx.addInput(tx3.getOutput(0)); // Now completeTx will result in one input, one output - assertTrue(wallet.completeTx(request2)); + wallet.completeTx(request2); assertEquals(1, request2.tx.getInputs().size()); assertEquals(1, request2.tx.getOutputs().size()); assertEquals(CENT, request2.tx.getOutput(0).getValue()); @@ -1815,7 +1827,7 @@ public class WalletTest extends TestWithWallet { request3.tx.addInput(new TransactionInput(params, request3.tx, new byte[]{}, new TransactionOutPoint(params, 0, tx3.getHash()))); // Now completeTx will result in two inputs, two outputs and a fee of a CENT // Note that it is simply assumed that the inputs are correctly signed, though in fact the first is not - assertTrue(wallet.completeTx(request3)); + wallet.completeTx(request3); assertEquals(2, request3.tx.getInputs().size()); assertEquals(2, request3.tx.getOutputs().size()); assertEquals(CENT, request3.tx.getOutput(0).getValue()); @@ -1826,7 +1838,7 @@ public class WalletTest extends TestWithWallet { // Now if we manually sign it, completeTx will not replace our signature request4.tx.signInputs(SigHash.ALL, wallet); byte[] scriptSig = request4.tx.getInput(0).getScriptBytes(); - assertTrue(wallet.completeTx(request4)); + wallet.completeTx(request4); assertEquals(1, request4.tx.getInputs().size()); assertEquals(1, request4.tx.getOutputs().size()); assertEquals(CENT, request4.tx.getOutput(0).getValue()); @@ -1873,7 +1885,7 @@ public class WalletTest extends TestWithWallet { wallet.receiveFromBlock(tx, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, i); } SendRequest request = SendRequest.emptyWallet(new ECKey().toAddress(params)); - assertTrue(wallet.completeTx(request)); + wallet.completeTx(request); wallet.commitTx(request.tx); assertEquals(BigInteger.ZERO, wallet.getBalance()); } @@ -1886,7 +1898,7 @@ public class WalletTest extends TestWithWallet { Transaction tx = createFakeTx(params, CENT, myAddress); wallet.receiveFromBlock(tx, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, 0); SendRequest request = SendRequest.emptyWallet(outputKey); - assertTrue(wallet.completeTx(request)); + wallet.completeTx(request); wallet.commitTx(request.tx); assertEquals(BigInteger.ZERO, wallet.getBalance()); assertEquals(CENT, request.tx.getOutput(0).getValue()); @@ -1899,7 +1911,7 @@ public class WalletTest extends TestWithWallet { tx = createFakeTx(params, CENT, myAddress); wallet.receivePending(tx, null); request = SendRequest.emptyWallet(outputKey); - assertTrue(wallet.completeTx(request)); + wallet.completeTx(request); wallet.commitTx(request.tx); assertEquals(BigInteger.ZERO, wallet.getBalance()); assertEquals(CENT, request.tx.getOutput(0).getValue()); @@ -1909,7 +1921,7 @@ public class WalletTest extends TestWithWallet { tx = createFakeTx(params, CENT.subtract(BigInteger.ONE), myAddress); wallet.receiveFromBlock(tx, block2, AbstractBlockChain.NewBlockType.BEST_CHAIN, 0); request = SendRequest.emptyWallet(outputKey); - assertTrue(wallet.completeTx(request)); + wallet.completeTx(request); wallet.commitTx(request.tx); assertEquals(BigInteger.ZERO, wallet.getBalance()); assertEquals(CENT.subtract(BigInteger.ONE).subtract(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE), request.tx.getOutput(0).getValue()); @@ -1920,9 +1932,12 @@ public class WalletTest extends TestWithWallet { tx = createFakeTx(params, outputValue, myAddress); wallet.receiveFromBlock(tx, block3, AbstractBlockChain.NewBlockType.BEST_CHAIN, 0); request = SendRequest.emptyWallet(outputKey); - assertFalse(wallet.completeTx(request)); + try { + wallet.completeTx(request); + fail(); + } catch (InsufficientMoneyException.CouldNotAdjustDownwards e) {} request.ensureMinRequiredFee = false; - assertTrue(wallet.completeTx(request)); + wallet.completeTx(request); wallet.commitTx(request.tx); assertEquals(BigInteger.ZERO, wallet.getBalance()); assertEquals(outputValue, request.tx.getOutput(0).getValue()); @@ -2046,7 +2061,7 @@ public class WalletTest extends TestWithWallet { ECKey dest = new ECKey(); Wallet.SendRequest req = Wallet.SendRequest.emptyWallet(dest.toAddress(params)); - assertTrue(wallet.completeTx(req)); + wallet.completeTx(req); byte[] dummySig = TransactionSignature.dummy().encodeToBitcoin(); // Selected inputs can be in any order. for (int i = 0; i < req.tx.getInputs().size(); i++) { diff --git a/core/src/test/java/com/google/bitcoin/protocols/channels/ChannelConnectionTest.java b/core/src/test/java/com/google/bitcoin/protocols/channels/ChannelConnectionTest.java index 540dbd75..e64ab41e 100644 --- a/core/src/test/java/com/google/bitcoin/protocols/channels/ChannelConnectionTest.java +++ b/core/src/test/java/com/google/bitcoin/protocols/channels/ChannelConnectionTest.java @@ -559,7 +559,7 @@ public class ChannelConnectionTest extends TestWithWallet { .setMinPayment(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.longValue())) .setType(MessageType.INITIATE).build()); fail(); - } catch (ValueOutOfRangeException expected) { + } catch (InsufficientMoneyException expected) { // This should be thrown. } } diff --git a/core/src/test/java/com/google/bitcoin/protocols/channels/PaymentChannelStateTest.java b/core/src/test/java/com/google/bitcoin/protocols/channels/PaymentChannelStateTest.java index 566ba728..ddb022b4 100644 --- a/core/src/test/java/com/google/bitcoin/protocols/channels/PaymentChannelStateTest.java +++ b/core/src/test/java/com/google/bitcoin/protocols/channels/PaymentChannelStateTest.java @@ -106,8 +106,7 @@ public class PaymentChannelStateTest extends TestWithWallet { try { channelState.initiate(); fail(); - } catch (ValueOutOfRangeException e) { - assertTrue(e.getMessage().contains("afford")); + } catch (InsufficientMoneyException e) { } } @@ -698,8 +697,7 @@ public class PaymentChannelStateTest extends TestWithWallet { try { serverState.close(); fail(); - } catch (ValueOutOfRangeException e) { - assertTrue(e.getMessage().contains("unable to pay required fee")); + } catch (InsufficientMoneyException e) { } // Now give the server enough coins to pay the fee @@ -711,7 +709,7 @@ public class PaymentChannelStateTest extends TestWithWallet { try { serverState.close(); fail(); - } catch (ValueOutOfRangeException e) { + } catch (InsufficientMoneyException e) { assertTrue(e.getMessage().contains("more in fees")); } diff --git a/examples/src/main/java/com/google/bitcoin/examples/ForwardingService.java b/examples/src/main/java/com/google/bitcoin/examples/ForwardingService.java index 8f66d217..065b0f2c 100644 --- a/examples/src/main/java/com/google/bitcoin/examples/ForwardingService.java +++ b/examples/src/main/java/com/google/bitcoin/examples/ForwardingService.java @@ -139,6 +139,9 @@ public class ForwardingService { } catch (KeyCrypterException e) { // We don't use encrypted wallets in this example - can never happen. throw new RuntimeException(e); + } catch (InsufficientMoneyException e) { + // This should never happen - we're only trying to forward what we received! + throw new RuntimeException(e); } } } diff --git a/tools/src/main/java/com/google/bitcoin/tools/WalletTool.java b/tools/src/main/java/com/google/bitcoin/tools/WalletTool.java index 9d58d26a..3ae4f069 100644 --- a/tools/src/main/java/com/google/bitcoin/tools/WalletTool.java +++ b/tools/src/main/java/com/google/bitcoin/tools/WalletTool.java @@ -453,10 +453,8 @@ public class WalletTool { } req.aesKey = wallet.getKeyCrypter().deriveKey(password); } - if (!wallet.completeTx(req)) { - System.err.println("Insufficient funds: have " + Utils.bitcoinValueToFriendlyString(wallet.getBalance())); - return; - } + wallet.completeTx(req); + try { if (lockTimeStr != null) { t.setLockTime(Transaction.parseLockTimeStr(lockTimeStr)); @@ -499,6 +497,8 @@ public class WalletTool { throw new RuntimeException(e); } catch (ExecutionException e) { throw new RuntimeException(e); + } catch (InsufficientMoneyException e) { + System.err.println("Insufficient funds: have " + Utils.bitcoinValueToFriendlyString(wallet.getBalance())); } } diff --git a/wallettemplate/src/main/java/wallettemplate/SendMoneyController.java b/wallettemplate/src/main/java/wallettemplate/SendMoneyController.java index e0a1ff96..ec859e3f 100644 --- a/wallettemplate/src/main/java/wallettemplate/SendMoneyController.java +++ b/wallettemplate/src/main/java/wallettemplate/SendMoneyController.java @@ -36,15 +36,7 @@ public class SendMoneyController { try { Address destination = new Address(Main.params, address.getText()); Wallet.SendRequest req = Wallet.SendRequest.emptyWallet(destination); - sendResult = Main.bitcoin.wallet().sendCoins(req); - if (sendResult == null) { - // We couldn't empty the wallet for some reason. TODO: When bitcoinj issue 425 is fixed, be more helpful - informationalAlert("Could not empty the wallet", - "You may have too little money left in the wallet to make a transaction."); - overlayUi.done(); - return; - } - + Main.bitcoin.wallet().sendCoins(req); Futures.addCallback(sendResult.broadcastComplete, new FutureCallback() { @Override public void onSuccess(Transaction result) { @@ -67,6 +59,10 @@ public class SendMoneyController { } catch (AddressFormatException e) { // Cannot happen because we already validated it when the text field changed. throw new RuntimeException(e); + } catch (InsufficientMoneyException e) { + informationalAlert("Could not empty the wallet", + "You may have too little money left in the wallet to make a transaction."); + overlayUi.done(); } }