Qortal/altcoinj
1
0
Fork 1
mirror of https://github.com/Qortal/altcoinj.git synced 2025-08-01 12:31:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

Payment channels: tweaks to channel resume behaviour.

Browse Source 
Don't create a new channel automatically when the client wants to resume but there's already an open connection using that contract. Instead, disconnect the other client. This fixes unintuitive behaviour that could occur if a TCP connection silently died and the server didn't notice.
This commit is contained in:
Mike Hearn
2013-07-25 18:11:53 +02:00
parent ce1d8315ea
commit 630b36c5c8
6 changed files with 56 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelClient.java
View File

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

27
core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServer.java
View File

@@ -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();
}
}
}

8
core/src/main/java/com/google/bitcoin/protocols/channels/PaymentChannelServerState.java
View File

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

6
core/src/main/java/com/google/bitcoin/protocols/channels/StoredPaymentChannelServerStates.java
View File

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

26
core/src/main/java/com/google/bitcoin/protocols/channels/StoredServerChannel.java
View File

@@ -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();
}
/**

66
core/src/test/java/com/google/bitcoin/protocols/channels/ChannelConnectionTest.java
View File

@@ -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.