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 eabd2e97..7da9fb23 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 @@ -180,7 +180,7 @@ public class PaymentChannelClient { @GuardedBy("lock") private void receiveChannelOpen() throws VerificationException { - checkState(step == InitStep.WAITING_FOR_CHANNEL_OPEN || (step == InitStep.WAITING_FOR_INITIATE && storedChannel != null)); + checkState(step == InitStep.WAITING_FOR_CHANNEL_OPEN || (step == InitStep.WAITING_FOR_INITIATE && storedChannel != null), step); log.info("Got CHANNEL_OPEN message, ready to pay"); if (step == InitStep.WAITING_FOR_INITIATE) 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 3ca20eac..13d886ae 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 @@ -156,18 +156,21 @@ public class PaymentChannelServer { if (channels != null) { StoredServerChannel storedServerChannel = channels.getChannel(contractHash); if (storedServerChannel != null) { - if (storedServerChannel.setConnectedHandler(this)) { - log.info("Got resume version message, responding with VERSIONS and CHANNEL_OPEN"); - state = storedServerChannel.getOrCreateState(wallet, broadcaster); - step = InitStep.CHANNEL_OPEN; - conn.sendToClient(Protos.TwoWayChannelMessage.newBuilder() - .setType(Protos.TwoWayChannelMessage.MessageType.CHANNEL_OPEN) - .build()); - conn.channelOpen(contractHash); - return; - } else { - log.error(" ... but that contract is already in use."); + final PaymentChannelServer existingHandler = storedServerChannel.setConnectedHandler(this, false); + if (existingHandler != this) { + log.warn(" ... and that channel is already in use, disconnecting other user."); + existingHandler.close(); + storedServerChannel.setConnectedHandler(this, true); } + + log.info("Got resume version message, responding with VERSIONS and CHANNEL_OPEN"); + state = storedServerChannel.getOrCreateState(wallet, broadcaster); + step = InitStep.CHANNEL_OPEN; + conn.sendToClient(Protos.TwoWayChannelMessage.newBuilder() + .setType(Protos.TwoWayChannelMessage.MessageType.CHANNEL_OPEN) + .build()); + conn.channelOpen(contractHash); + return; } else { log.error(" ... but we do not have any record of that contract! Resume failed."); } @@ -367,7 +370,7 @@ public class PaymentChannelServer { if (channels != null) { StoredServerChannel storedServerChannel = channels.getChannel(state.getMultisigContract().getHash()); if (storedServerChannel != null) { - storedServerChannel.setConnectedHandler(null); + storedServerChannel.clearConnectedHandler(); } } } 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 024660d5..776717d8 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 @@ -475,9 +475,10 @@ public class PaymentChannelServerState { * a call to {@link PaymentChannelServerState#close()} completes successfully. A channel may only be stored after it * has fully opened (ie state == State.READY). * - * @param connectedHandler The {@link PaymentChannelClientState} object which is managing this object. This will + * @param connectedHandler Optional {@link PaymentChannelServer} object that manages this object. This will * set the appropriate pointer in the newly created {@link StoredServerChannel} before it is - * committed to wallet. + * committed to wallet. If set, closing the state object will propagate the close to the + * handler which can then do a TCP disconnect. */ public synchronized void storeChannelInWallet(@Nullable PaymentChannelServer connectedHandler) { checkState(state == State.READY); @@ -488,7 +489,8 @@ public class PaymentChannelServerState { StoredPaymentChannelServerStates channels = (StoredPaymentChannelServerStates) wallet.addOrGetExistingExtension(new StoredPaymentChannelServerStates(wallet, broadcaster)); storedServerChannel = new StoredServerChannel(this, multisigContract, clientOutput, refundTransactionUnlockTimeSecs, serverKey, bestValueToMe, bestValueSignature); - checkState(storedServerChannel.setConnectedHandler(connectedHandler)); + if (connectedHandler != null) + checkState(storedServerChannel.setConnectedHandler(connectedHandler, false) == connectedHandler); channels.putChannel(storedServerChannel); wallet.addOrUpdateExtension(channels); } 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 cd0288e0..1184dfd6 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 @@ -78,9 +78,9 @@ public class StoredPaymentChannelServerStates implements WalletExtension { lock.unlock(); } synchronized (channel) { - if (channel.connectedHandler != null) // connectedHandler will be reset to null in connectionClosed - channel.connectedHandler.close(); // Closes the actual connection, not the channel - try {//TODO add event listener to PaymentChannelServerStateManager + channel.closeConnectedHandler(); + try { + //TODO add event listener to PaymentChannelServerStateManager channel.getOrCreateState(wallet, broadcaster).close(); } catch (ValueOutOfRangeException e) { e.printStackTrace(); diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/StoredServerChannel.java b/core/src/main/java/com/google/bitcoin/protocols/channels/StoredServerChannel.java index 12f0c224..e6717636 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/StoredServerChannel.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/StoredServerChannel.java @@ -38,7 +38,7 @@ public class StoredServerChannel { // In-memory pointer to the event handler which handles this channel if the client is connected. // Used as a flag to prevent duplicate connections and to disconnect the channel if its expire time approaches. - PaymentChannelServer connectedHandler = null; + private PaymentChannelServer connectedHandler = null; PaymentChannelServerState state = null; StoredServerChannel(PaymentChannelServerState state, Transaction contract, TransactionOutput clientOutput, @@ -63,13 +63,27 @@ public class StoredServerChannel { /** * Attempts to connect the given handler to this, returning true if it is the new handler, false if there was - * already one attached. A null connectedHandler clears this's connected handler no matter its current state. + * already one attached. */ - synchronized boolean setConnectedHandler(PaymentChannelServer connectedHandler) { - if (this.connectedHandler != null && connectedHandler != null) - return false; + synchronized PaymentChannelServer setConnectedHandler(PaymentChannelServer connectedHandler, boolean override) { + if (this.connectedHandler != null && !override) + return this.connectedHandler; this.connectedHandler = connectedHandler; - return true; + return connectedHandler; + } + + /** Clears a handler that was connected with setConnectedHandler. */ + synchronized void clearConnectedHandler() { + this.connectedHandler = null; + } + + /** + * If a handler is connected, call its {@link com.google.bitcoin.protocols.channels.PaymentChannelServer#close()} + * method thus disconnecting the TCP connection. + */ + synchronized void closeConnectedHandler() { + if (connectedHandler != null) + connectedHandler.close(); } /** 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 25c05407..10a1dcc2 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 @@ -325,9 +325,7 @@ public class ChannelConnectionTest extends TestWithWallet { PaymentChannelClient openClient = client; ChannelTestUtils.RecordingPair openPair = pair; - // Now open up a new client with the same id and make sure it doesn't attempt to reopen the channel. - // If a client connects to a server with the same channel ID as one that's currently in use, the - // server responds by opening up a new channel instead of letting two client connections conflict. + // Now open up a new client with the same id and make sure the server disconnects the previous client. pair = ChannelTestUtils.makeRecorders(serverWallet, mockBroadcaster); client = new PaymentChannelClient(wallet, myKey, Utils.COIN, someServerId, pair.clientRecorder); server = pair.server; @@ -339,14 +337,13 @@ public class ChannelConnectionTest extends TestWithWallet { Protos.TwoWayChannelMessage msg = pair.clientRecorder.checkNextMsg(MessageType.CLIENT_VERSION); assertFalse(msg.getClientVersion().hasPreviousChannelContractHash()); } - - // Make sure the server won't allow two simultaneous opens either. It will try to reinitiate instead. + // Make sure the server allows two simultaneous opens. It will close the first and allow resumption of the second. pair = ChannelTestUtils.makeRecorders(serverWallet, mockBroadcaster); client = new PaymentChannelClient(wallet, myKey, Utils.COIN, someServerId, pair.clientRecorder); server = pair.server; client.connectionOpen(); server.connectionOpen(); - // Swap out the clients version message for a custom one that incorrectly tries to resume ... + // Swap out the clients version message for a custom one that tries to resume ... pair.clientRecorder.getNextMsg(); server.receiveMessage(Protos.TwoWayChannelMessage.newBuilder() .setType(MessageType.CLIENT_VERSION) @@ -354,48 +351,12 @@ public class ChannelConnectionTest extends TestWithWallet { .setPreviousChannelContractHash(ByteString.copyFrom(contractHash.getBytes())) .setMajor(0).setMinor(42)) .build()); - // We get the usual setup sequence. - client.receiveMessage(pair.serverRecorder.checkNextMsg(MessageType.SERVER_VERSION)); - client.receiveMessage(pair.serverRecorder.checkNextMsg(MessageType.INITIATE)); - server.receiveMessage(pair.clientRecorder.checkNextMsg(MessageType.PROVIDE_REFUND)); - client.receiveMessage(pair.serverRecorder.checkNextMsg(MessageType.RETURN_REFUND)); - broadcastTxPause.release(); - server.receiveMessage(pair.clientRecorder.checkNextMsg(MessageType.PROVIDE_CONTRACT)); - broadcasts.take(); - client.receiveMessage(pair.serverRecorder.checkNextMsg(MessageType.CHANNEL_OPEN)); - Sha256Hash secondContractHash = (Sha256Hash) pair.serverRecorder.q.take(); - pair.clientRecorder.checkOpened(); - assertNull(pair.serverRecorder.q.poll()); - assertNull(pair.clientRecorder.q.poll()); - client.close(); - client.connectionClosed(); - pair.server.close(); - pair.server.connectionClosed(); + // We get the usual resume sequence. + pair.serverRecorder.checkNextMsg(MessageType.SERVER_VERSION); + pair.serverRecorder.checkNextMsg(MessageType.CHANNEL_OPEN); + // Verify the previous one was closed. + openPair.serverRecorder.checkNextMsg(MessageType.CLOSE); - // Now open again with the same id and make sure it reopens the second (because the 1st is still open). - pair = ChannelTestUtils.makeRecorders(serverWallet, mockBroadcaster); - client = new PaymentChannelClient(wallet, myKey, Utils.COIN, someServerId, pair.clientRecorder); - server = pair.server; - client.connectionOpen(); - server.connectionOpen(); - { - Protos.TwoWayChannelMessage msg = pair.clientRecorder.checkNextMsg(MessageType.CLIENT_VERSION); - assertEquals(secondContractHash, new Sha256Hash(msg.getClientVersion().getPreviousChannelContractHash().toByteArray())); - server.receiveMessage(msg); - } - client.receiveMessage(pair.serverRecorder.checkNextMsg(MessageType.SERVER_VERSION)); - client.receiveMessage(pair.serverRecorder.checkNextMsg(MessageType.CHANNEL_OPEN)); - pair.clientRecorder.checkOpened(); - // Close it. - assertEquals(2, clientStoredChannels.mapChannels.size()); - broadcastTxPause.release(); - client.close(); - server.receiveMessage(pair.clientRecorder.checkNextMsg(MessageType.CLOSE)); - assertEquals(CloseReason.CLIENT_REQUESTED_CLOSE, pair.clientRecorder.q.take()); - server.connectionClosed(); - client.connectionClosed(); - - assertFalse(clientStoredChannels.getChannel(Sha256Hash.create(new byte[]{}), secondContractHash).active); assertTrue(clientStoredChannels.getChannel(Sha256Hash.create(new byte[]{}), contractHash).active); // And finally close the first channel too. @@ -406,13 +367,10 @@ public class ChannelConnectionTest extends TestWithWallet { Utils.rollMockClock(60 * 60 * 24 + 60*5); // Client announces refund 5 minutes after expire time StoredPaymentChannelClientStates newClientStates = new StoredPaymentChannelClientStates(wallet, mockBroadcaster); newClientStates.deserializeWalletExtension(wallet, clientStoredChannels.serializeWalletExtension()); - // Expect two pairs of contract/refund ... - for (int i = 0; i < 2; i++) { - broadcastTxPause.release(); - assertTrue(broadcasts.take().getOutput(0).getScriptPubKey().isSentToMultiSig()); - broadcastTxPause.release(); - assertEquals(TransactionConfidence.Source.SELF, broadcasts.take().getConfidence().getSource()); - } + broadcastTxPause.release(); + assertTrue(broadcasts.take().getOutput(0).getScriptPubKey().isSentToMultiSig()); + broadcastTxPause.release(); + assertEquals(TransactionConfidence.Source.SELF, broadcasts.take().getConfidence().getSource()); assertTrue(broadcasts.isEmpty()); assertTrue(newClientStates.mapChannels.isEmpty()); // Server also knows it's too late.