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 382aec78..eabd2e97 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 @@ -346,7 +346,7 @@ public class PaymentChannelClient { StoredPaymentChannelClientStates channels = (StoredPaymentChannelClientStates) wallet.getExtensions().get(StoredPaymentChannelClientStates.EXTENSION_ID); if (channels != null) - storedChannel = channels.getInactiveChannelById(serverId); + storedChannel = channels.getUsableChannelForServerID(serverId); step = InitStep.WAITING_FOR_VERSION_NEGOTIATION; diff --git a/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelClientStates.java b/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelClientStates.java index 4edbe058..8ea1f14a 100644 --- a/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelClientStates.java +++ b/core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelClientStates.java @@ -23,6 +23,7 @@ import com.google.common.collect.HashMultimap; import com.google.protobuf.ByteString; import net.jcip.annotations.GuardedBy; +import javax.annotation.Nullable; import java.math.BigInteger; import java.util.Date; import java.util.Set; @@ -61,12 +62,16 @@ public class StoredPaymentChannelClientStates implements WalletExtension { /** * Finds an inactive channel with the given id and returns it, or returns null. */ - public StoredClientChannel getInactiveChannelById(Sha256Hash id) { + @Nullable + public StoredClientChannel getUsableChannelForServerID(Sha256Hash id) { lock.lock(); try { Set setChannels = mapChannels.get(id); for (StoredClientChannel channel : setChannels) { synchronized (channel) { + // Check if the channel is usable (has money, inactive) and if so, activate it. + if (channel.valueToMe.equals(BigInteger.ZERO)) + continue; if (!channel.active) { channel.active = true; return channel; @@ -244,7 +249,7 @@ class StoredClientChannel { @Override public String toString() { final String newline = String.format("%n"); - return String.format("Stored client channel %s (%s)%n" + + return String.format("Stored client channel for server ID %s (%s)%n" + " Key: %s%n" + " Value to me: %d%n" + " Refund fees: %d%n" + 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 19a62818..25c05407 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 @@ -575,4 +575,43 @@ public class ChannelConnectionTest extends TestWithWallet { assertEquals(Protos.Error.ErrorCode.SYNTAX_ERROR, error.getError().getCode()); assertEquals(CloseReason.REMOTE_SENT_INVALID_MESSAGE, pair.clientRecorder.q.take()); } + + @Test + public void testDontResumeEmptyChannels() throws Exception { + // Check that if the client has an empty channel that's being kept around in case we need to broadcast the + // refund, we don't accidentally try to resume it). + + // Open up a normal channel. + Sha256Hash someServerId = Sha256Hash.ZERO_HASH; + ChannelTestUtils.RecordingPair pair = ChannelTestUtils.makeRecorders(serverWallet, mockBroadcaster); + pair.server.connectionOpen(); + PaymentChannelClient client = new PaymentChannelClient(wallet, myKey, Utils.COIN, someServerId, pair.clientRecorder); + PaymentChannelServer server = pair.server; + client.connectionOpen(); + server.receiveMessage(pair.clientRecorder.checkNextMsg(MessageType.CLIENT_VERSION)); + 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 contractHash = (Sha256Hash) pair.serverRecorder.q.take(); + pair.clientRecorder.checkOpened(); + assertNull(pair.serverRecorder.q.poll()); + assertNull(pair.clientRecorder.q.poll()); + // Send the whole channel at once. + client.incrementPayment(Utils.COIN); + server.receiveMessage(pair.clientRecorder.checkNextMsg(MessageType.UPDATE_PAYMENT)); + // The channel is now empty. + assertEquals(BigInteger.ZERO, client.state().getValueRefunded()); + client.connectionClosed(); + + // Now try opening a new channel with the same server ID and verify the client asks for a new channel. + client = new PaymentChannelClient(wallet, myKey, Utils.COIN, someServerId, pair.clientRecorder); + client.connectionOpen(); + Protos.TwoWayChannelMessage msg = pair.clientRecorder.checkNextMsg(MessageType.CLIENT_VERSION); + assertFalse(msg.getClientVersion().hasPreviousChannelContractHash()); + } }