Payment channels: plumb through the actual amount of value sent on a channel, as it can sometimes be different to how much was requested.

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
+                        // Server might send back a major version lower than our own if they want to fallback to a
-                        // We can't handle that, so we just close the channel
+                        // 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();
         }

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;
+    }
+
     /**
      * <p>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()}</p>
      *
      * @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
+     * @throws ValueOutOfRangeException If size is negative or the channel does not have sufficient money in it to
-     *                                  min nondust output size (including if the new total payment is larger than this
+     *                                  complete this payment.
-     *                                  channel's totalValue)
      */
-    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))
+        if (newValueToMe.compareTo(Transaction.MIN_NONDUST_OUTPUT) < 0 && newValueToMe.compareTo(BigInteger.ZERO) > 0) {
-            throw new ValueOutOfRangeException("New value being sent back as change was smaller than minimum nondust output");
+            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() {

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);

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
+        // We cannot send just under the total value - our refund would make it unspendable. So the client
-        try {
+        // will correct it for us to be larger than the requested amount, to make the change output zero.
-            clientState.incrementPaymentBy(Utils.CENT.subtract(Transaction.MIN_NONDUST_OUTPUT));
+        PaymentChannelClientState.IncrementedPayment payment =
-            fail();
+                clientState.incrementPaymentBy(Utils.CENT.subtract(Transaction.MIN_NONDUST_OUTPUT));
-        } catch (ValueOutOfRangeException e) {}
+        assertEquals(Utils.CENT.subtract(BigInteger.ONE), payment.amount);
-        // The server also won't accept it if we do that
+        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));
+        serverState.incrementPayment(Utils.CENT.subtract(totalPayment), payment.signature.encodeToBitcoin());
-        totalPayment = totalPayment.add(Utils.CENT.subtract(BigInteger.ONE));
-        assertEquals(totalPayment, Utils.CENT);
-        serverState.incrementPayment(Utils.CENT.subtract(totalPayment), signature);
 
         // 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();