From 4b48dbfda9e2312efb9a0feb8b91f8e044d07496 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 9 Oct 2013 18:15:04 +0200 Subject: [PATCH] Payment channels: plumb through the actual amount of value sent on a channel, as it can sometimes be different to how much was requested. --- .../channels/PaymentChannelClient.java | 18 ++++++--- .../channels/PaymentChannelClientState.java | 27 +++++++++---- .../channels/PaymentChannelServer.java | 3 +- .../channels/PaymentChannelStateTest.java | 40 +++++++++---------- 4 files changed, 54 insertions(+), 34 deletions(-) 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 cb5b26f6..d73923d6 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 @@ -214,8 +214,8 @@ public class PaymentChannelClient { switch (msg.getType()) { case SERVER_VERSION: checkState(step == InitStep.WAITING_FOR_VERSION_NEGOTIATION && msg.hasServerVersion()); - // Server might send back a major version lower than our own if they want to fallback to a lower version - // We can't handle that, so we just close the channel + // Server might send back a major version lower than our own if they want to fallback to a + // lower version. We can't handle that, so we just close the channel. if (msg.getServerVersion().getMajor() != 0) { errorBuilder = Protos.Error.newBuilder() .setCode(Protos.Error.ErrorCode.NO_ACCEPTABLE_VERSION); @@ -243,6 +243,7 @@ public class PaymentChannelClient { errorBuilder = Protos.Error.newBuilder() .setCode(Protos.Error.ErrorCode.CHANNEL_VALUE_TOO_LARGE); closeReason = CloseReason.SERVER_REQUESTED_TOO_MUCH_VALUE; + log.error("Server requested too much value"); break; } @@ -411,29 +412,34 @@ public class PaymentChannelClient { } /** - * Increments the total value which we pay the server. + * Increments the total value which we pay the server. Note that the amount of money sent may not be the same as the + * amount of money actually requested. It can be larger if the amount left over in the channel would be too small to + * be accepted by the Bitcoin network. ValueOutOfRangeException will be thrown, however, if there's not enough money + * left in the channel to make the payment at all. * * @param size How many satoshis to increment the payment by (note: not the new total). * @throws ValueOutOfRangeException If the size is negative or would pay more than this channel's total value * ({@link PaymentChannelClientConnection#state()}.getTotalValue()) * @throws IllegalStateException If the channel has been closed or is not yet open * (see {@link PaymentChannelClientConnection#getChannelOpenFuture()} for the second) + * @return amount of money actually sent. */ - public void incrementPayment(BigInteger size) throws ValueOutOfRangeException, IllegalStateException { + public BigInteger incrementPayment(BigInteger size) throws ValueOutOfRangeException, IllegalStateException { lock.lock(); try { if (state() == null || !connectionOpen || step != InitStep.CHANNEL_OPEN) throw new IllegalStateException("Channel is not fully initialized/has already been closed"); - byte[] signature = state().incrementPaymentBy(size); + PaymentChannelClientState.IncrementedPayment payment = state().incrementPaymentBy(size); Protos.UpdatePayment.Builder updatePaymentBuilder = Protos.UpdatePayment.newBuilder() - .setSignature(ByteString.copyFrom(signature)) + .setSignature(ByteString.copyFrom(payment.signature.encodeToBitcoin())) .setClientChangeValue(state.getValueRefunded().longValue()); conn.sendToServer(Protos.TwoWayChannelMessage.newBuilder() .setUpdatePayment(updatePaymentBuilder) .setType(Protos.TwoWayChannelMessage.MessageType.UPDATE_PAYMENT) .build()); + return payment.amount; } finally { lock.unlock(); } 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 8d8060de..ace449bc 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 @@ -360,6 +360,12 @@ public class PaymentChannelClientState { } } + /** Container for a signature and an amount that was sent. */ + public static class IncrementedPayment { + public TransactionSignature signature; + public BigInteger amount; + } + /** *

Updates the outputs on the payment contract transaction and re-signs it. The state must be READY in order to * call this method. The signature that is returned should be sent to the server so it has the ability to broadcast @@ -372,19 +378,23 @@ public class PaymentChannelClientState { * {@link PaymentChannelClientState#getValueRefunded()}

* * @param size How many satoshis to increment the payment by (note: not the new total). - * @throws ValueOutOfRangeException If size is negative or the new value being returned as change is smaller than - * min nondust output size (including if the new total payment is larger than this - * channel's totalValue) + * @throws ValueOutOfRangeException If size is negative or the channel does not have sufficient money in it to + * complete this payment. */ - public synchronized byte[] incrementPaymentBy(BigInteger size) throws ValueOutOfRangeException { + public synchronized IncrementedPayment incrementPaymentBy(BigInteger size) throws ValueOutOfRangeException { checkState(state == State.READY); checkNotExpired(); checkNotNull(size); // Validity of size will be checked by makeUnsignedChannelContract. if (size.compareTo(BigInteger.ZERO) < 0) throw new ValueOutOfRangeException("Tried to decrement payment"); BigInteger newValueToMe = valueToMe.subtract(size); - if (Transaction.MIN_NONDUST_OUTPUT.compareTo(newValueToMe) > 0 && !newValueToMe.equals(BigInteger.ZERO)) - throw new ValueOutOfRangeException("New value being sent back as change was smaller than minimum nondust output"); + if (newValueToMe.compareTo(Transaction.MIN_NONDUST_OUTPUT) < 0 && newValueToMe.compareTo(BigInteger.ZERO) > 0) { + log.info("New value being sent back as change was smaller than minimum nondust output, sending all"); + size = valueToMe; + newValueToMe = BigInteger.ZERO; + } + if (newValueToMe.compareTo(BigInteger.ZERO) < 0) + throw new ValueOutOfRangeException("Channel has too little money to pay " + size + " satoshis"); Transaction tx = makeUnsignedChannelContract(newValueToMe); log.info("Signing new payment tx {}", tx); Transaction.SigHash mode; @@ -397,7 +407,10 @@ public class PaymentChannelClientState { TransactionSignature sig = tx.calculateSignature(0, myKey, multisigScript, mode, true); valueToMe = newValueToMe; updateChannelInWallet(); - return sig.encodeToBitcoin(); + IncrementedPayment payment = new IncrementedPayment(); + payment.signature = sig; + payment.amount = size; + return payment; } private synchronized void updateChannelInWallet() { 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 6c0e9828..6ec87ce9 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 @@ -180,7 +180,8 @@ public class PaymentChannelServer { log.error(" ... but we do not have any stored channels! Resume failed."); } } - log.info("Got initial version message, responding with VERSIONS and INITIATE"); + log.info("Got initial version message, responding with VERSIONS and INITIATE: min value={}", + minAcceptedChannelSize.longValue()); myKey = new ECKey(); wallet.addKey(myKey); 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 ff3e1cc1..1c774c24 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 @@ -170,7 +170,7 @@ public class PaymentChannelStateTest extends TestWithWallet { BigInteger size = halfCoin.divide(BigInteger.TEN).divide(BigInteger.TEN); BigInteger totalPayment = BigInteger.ZERO; for (int i = 0; i < 4; i++) { - byte[] signature = clientState.incrementPaymentBy(size); + byte[] signature = clientState.incrementPaymentBy(size).signature.encodeToBitcoin(); totalPayment = totalPayment.add(size); serverState.incrementPayment(halfCoin.subtract(totalPayment), signature); } @@ -178,7 +178,7 @@ public class PaymentChannelStateTest extends TestWithWallet { // Now confirm the contract transaction and make sure payments still work chain.add(makeSolvedTestBlock(blockStore.getChainHead().getHeader(), multisigContract)); - byte[] signature = clientState.incrementPaymentBy(size); + byte[] signature = clientState.incrementPaymentBy(size).signature.encodeToBitcoin(); totalPayment = totalPayment.add(size); serverState.incrementPayment(halfCoin.subtract(totalPayment), signature); @@ -273,7 +273,7 @@ public class PaymentChannelStateTest extends TestWithWallet { // Pay a tiny bit serverState.incrementPayment(Utils.CENT.divide(BigInteger.valueOf(2)).subtract(Utils.CENT.divide(BigInteger.TEN)), - clientState.incrementPaymentBy(Utils.CENT.divide(BigInteger.TEN))); + clientState.incrementPaymentBy(Utils.CENT.divide(BigInteger.TEN)).signature.encodeToBitcoin()); // Advance time until our we get close enough to lock time that server should rebroadcast Utils.rollMockClock(60*60*22); @@ -469,7 +469,7 @@ public class PaymentChannelStateTest extends TestWithWallet { fail(); } catch (ValueOutOfRangeException e) {} - byte[] signature = clientState.incrementPaymentBy(size); + byte[] signature = clientState.incrementPaymentBy(size).signature.encodeToBitcoin(); totalPayment = totalPayment.add(size); byte[] signatureCopy = Arrays.copyOf(signature, signature.length); @@ -500,7 +500,7 @@ public class PaymentChannelStateTest extends TestWithWallet { serverState.incrementPayment(halfCoin.subtract(totalPayment), signature); // Pay the rest (signed with SIGHASH_NONE|SIGHASH_ANYONECANPAY) - byte[] signature2 = clientState.incrementPaymentBy(halfCoin.subtract(totalPayment)); + byte[] signature2 = clientState.incrementPaymentBy(halfCoin.subtract(totalPayment)).signature.encodeToBitcoin(); totalPayment = totalPayment.add(halfCoin.subtract(totalPayment)); assertEquals(totalPayment, halfCoin); @@ -602,7 +602,7 @@ public class PaymentChannelStateTest extends TestWithWallet { BigInteger totalPayment = BigInteger.ZERO; // We can send as little as we want - its up to the server to get the fees right - byte[] signature = clientState.incrementPaymentBy(BigInteger.ONE); + byte[] signature = clientState.incrementPaymentBy(BigInteger.ONE).signature.encodeToBitcoin(); totalPayment = totalPayment.add(BigInteger.ONE); serverState.incrementPayment(Utils.CENT.subtract(totalPayment), signature); @@ -612,21 +612,20 @@ public class PaymentChannelStateTest extends TestWithWallet { fail(); } catch (ValueOutOfRangeException e) {} - // We cannot, however, send just under the total value - our refund would make it unspendable - try { - clientState.incrementPaymentBy(Utils.CENT.subtract(Transaction.MIN_NONDUST_OUTPUT)); - fail(); - } catch (ValueOutOfRangeException e) {} - // The server also won't accept it if we do that + // We cannot send just under the total value - our refund would make it unspendable. So the client + // will correct it for us to be larger than the requested amount, to make the change output zero. + PaymentChannelClientState.IncrementedPayment payment = + clientState.incrementPaymentBy(Utils.CENT.subtract(Transaction.MIN_NONDUST_OUTPUT)); + assertEquals(Utils.CENT.subtract(BigInteger.ONE), payment.amount); + totalPayment = totalPayment.add(payment.amount); + + // The server also won't accept it if we do that. try { serverState.incrementPayment(Transaction.MIN_NONDUST_OUTPUT.subtract(BigInteger.ONE), signature); fail(); } catch (ValueOutOfRangeException e) {} - signature = clientState.incrementPaymentBy(Utils.CENT.subtract(BigInteger.ONE)); - totalPayment = totalPayment.add(Utils.CENT.subtract(BigInteger.ONE)); - assertEquals(totalPayment, Utils.CENT); - serverState.incrementPayment(Utils.CENT.subtract(totalPayment), signature); + serverState.incrementPayment(Utils.CENT.subtract(totalPayment), payment.signature.encodeToBitcoin()); // And close the channel. serverState.close(); @@ -686,7 +685,8 @@ public class PaymentChannelStateTest extends TestWithWallet { assertEquals(PaymentChannelServerState.State.READY, serverState.getState()); // Both client and server are now in the ready state, split the channel in half - byte[] signature = clientState.incrementPaymentBy(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.subtract(BigInteger.ONE)); + byte[] signature = clientState.incrementPaymentBy(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.subtract(BigInteger.ONE)) + .signature.encodeToBitcoin(); BigInteger totalRefund = Utils.CENT.subtract(Transaction.REFERENCE_DEFAULT_MIN_TX_FEE.subtract(BigInteger.ONE)); serverState.incrementPayment(totalRefund, signature); @@ -711,7 +711,7 @@ public class PaymentChannelStateTest extends TestWithWallet { assertTrue(e.getMessage().contains("more in fees")); } - signature = clientState.incrementPaymentBy(BigInteger.ONE.shiftLeft(1)); + signature = clientState.incrementPaymentBy(BigInteger.ONE.shiftLeft(1)).signature.encodeToBitcoin(); totalRefund = totalRefund.subtract(BigInteger.ONE.shiftLeft(1)); serverState.incrementPayment(totalRefund, signature); @@ -783,7 +783,7 @@ public class PaymentChannelStateTest extends TestWithWallet { BigInteger size = halfCoin.divide(BigInteger.TEN).divide(BigInteger.TEN); BigInteger totalPayment = BigInteger.ZERO; for (int i = 0; i < 5; i++) { - byte[] signature = clientState.incrementPaymentBy(size); + byte[] signature = clientState.incrementPaymentBy(size).signature.encodeToBitcoin(); totalPayment = totalPayment.add(size); serverState.incrementPayment(halfCoin.subtract(totalPayment), signature); } @@ -800,7 +800,7 @@ public class PaymentChannelStateTest extends TestWithWallet { // Now if we try to spend again the server will reject it since it saw a double-spend try { - byte[] signature = clientState.incrementPaymentBy(size); + byte[] signature = clientState.incrementPaymentBy(size).signature.encodeToBitcoin(); totalPayment = totalPayment.add(size); serverState.incrementPayment(halfCoin.subtract(totalPayment), signature); fail();