From 30b290cb8fd698c2d340dacba3de9c42745b2c1a Mon Sep 17 00:00:00 2001 From: John Barboza Date: Wed, 7 Mar 2018 08:23:43 -0500 Subject: [PATCH] Explicit error message when disconnect fails Whether the peer is not connected or not gossiping. --- gossipd/gossip.c | 4 ++-- gossipd/gossip_wire.csv | 1 + lightningd/peer_control.c | 9 +++++++-- tests/test_lightningd.py | 9 ++++++--- wallet/test/run-wallet.c | 2 +- 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 37092998a..ab47975c7 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -1029,8 +1029,8 @@ static struct io_plan *disconnect_peer(struct io_conn *conn, struct daemon *daem } else { status_trace("disconnect_peer: peer %s %s", type_to_string(trc, struct pubkey, &id), - !peer ? "not found" : "not local to this daemon"); - msg = towire_gossipctl_peer_disconnect_replyfail(msg); + !peer ? "not connected" : "not gossiping"); + msg = towire_gossipctl_peer_disconnect_replyfail(msg, peer ? true : false); daemon_conn_send(&daemon->master, take(msg)); } return daemon_conn_read_next(conn, &daemon->master); diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index 857b07c08..de2d620a9 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -222,3 +222,4 @@ gossipctl_peer_disconnect_reply,3123 # Gossipd -> master: reply to gossip_peer_disconnect if we couldn't find the peer. gossipctl_peer_disconnect_replyfail,3223 +gossipctl_peer_disconnect_replyfail,,isconnected,bool diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index d3ef8a389..73bb9774a 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -880,11 +880,16 @@ static void gossip_peer_disconnected (struct subd *gossip, const u8 *resp, const int *fds, struct command *cmd) { + bool isconnected; + if (!fromwire_gossipctl_peer_disconnect_reply(resp)) { - if (!fromwire_gossipctl_peer_disconnect_replyfail(resp)) + if (!fromwire_gossipctl_peer_disconnect_replyfail(resp, &isconnected)) fatal("Gossip daemon gave invalid reply %s", tal_hex(gossip, resp)); - command_fail(cmd, "Peer not connected"); + if (isconnected) + command_fail(cmd, "Peer is not in gossip mode"); + else + command_fail(cmd, "Peer not connected"); } else { /* Successfully disconnected */ command_success(cmd, null_response(cmd)); diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index b6ef4bcf5..d7c03a845 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -3821,15 +3821,18 @@ class LightningDTests(BaseLightningDTests): assert l2.rpc.getpeer(l1.info['id']) is None # Make sure you cannot disconnect after disconnecting - self.assertRaises(ValueError, l1.rpc.disconnect, l2.info['id']) - self.assertRaises(ValueError, l2.rpc.disconnect, l1.info['id']) + self.assertRaisesRegex(ValueError, "Peer not connected", + l1.rpc.disconnect, l2.info['id']) + self.assertRaisesRegex(ValueError, "Peer not connected", + l2.rpc.disconnect, l1.info['id']) # Fund channel l1 -> l3 self.fund_channel(l1, l3, 10**6) bitcoind.generate_block(5) # disconnecting a non gossiping peer results in error - self.assertRaises(ValueError, l1.rpc.disconnect, l3.info['id']) + self.assertRaisesRegex(ValueError, "Peer is not in gossip mode", + l1.rpc.disconnect, l3.info['id']) if __name__ == '__main__': diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index fbbf82fa5..ff3e39ed5 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -383,7 +383,7 @@ u8 *towire_gossipctl_peer_disconnect(const tal_t *ctx UNNEEDED, const struct pub bool fromwire_gossipctl_peer_disconnect_reply(const void *p UNNEEDED) { fprintf(stderr, "fromwire_gossipctl_peer_disconnect_reply called!\n"); abort(); } /* Generated stub for fromwire_gossipctl_peer_disconnect_replyfail*/ -bool fromwire_gossipctl_peer_disconnect_replyfail(const void *p UNNEEDED) +bool fromwire_gossipctl_peer_disconnect_replyfail(const void *p UNNEEDED, bool *isconnected UNNEEDED) { fprintf(stderr, "fromwire_gossipctl_peer_disconnect_replyfail called!\n"); abort(); } /* AUTOGENERATED MOCKS END */