From eec96b9cf8827ad65b13e78f84f07066b6e69080 Mon Sep 17 00:00:00 2001 From: trueptolemy <823220586@qq.com> Date: Sat, 4 May 2019 22:40:55 +0800 Subject: [PATCH] Channled: Add CHANNEL_GOT_ANNOUNCEMENT_REPLY to make sure master stores the sigs When Channeld sends sigs to Master, it will wait for CHANNEL_GOT_ANNOUNCEMENT_REPLY to make sure master stores the sigs. After that, Channeld won't send sigs to matser in the future. --- CHANGELOG.md | 2 + channeld/channel_wire.csv | 3 + channeld/channeld.c | 154 +++++++++++++++++------------------ lightningd/channel_control.c | 9 ++ 4 files changed, 91 insertions(+), 77 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b16c8a66e983..d66dbbf3e378 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - plugin: the `connected` hook can now send an `error_message` to the rejected peer. - Protocol: we now enforce `option_upfront_shutdown_script` if a peer negotiates it. - JSON API: `listforwards` now includes the local_failed forwards with failcode (Issue [#2435](https://github.com/ElementsProject/lightning/issues/2435), PR [#2524](https://github.com/ElementsProject/lightning/pull/2524)) +- DB: Store the signatures of channel announcement sent from remote peer into DB, and init channel with signatures from DB directly when reenable the channel. +(Issue [#2409](https://github.com/ElementsProject/lightning/issues/2409)) ### Changed diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv index af8ff28b332d..7a1a12797dd5 100644 --- a/channeld/channel_wire.csv +++ b/channeld/channel_wire.csv @@ -101,6 +101,9 @@ channel_got_announcement,1017 channel_got_announcement,,remote_ann_node_sig,secp256k1_ecdsa_signature channel_got_announcement,,remote_ann_bitcoin_sig,secp256k1_ecdsa_signature +# Wait for reply, to make sure it's on disk.(We store the announcement only once!) +channel_got_announcement_reply,1117 + # When we receive funding_locked. channel_got_funding_locked,1019 channel_got_funding_locked,,next_per_commit_point,struct pubkey diff --git a/channeld/channeld.c b/channeld/channeld.c index fe1175a0b248..51913468a819 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -237,6 +237,59 @@ static const u8 *hsm_req(const tal_t *ctx, const u8 *req TAKES) return msg; } +/* This queues other traffic from the fd until we get reply. */ +static u8 *wait_sync_reply(const tal_t *ctx, + const u8 *msg, + int replytype, + int fd, + struct msg_queue *queue, + const char *who) +{ + u8 *reply; + + status_trace("Sending %s %u", who, fromwire_peektype(msg)); + + if (!wire_sync_write(fd, msg)) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "Could not set sync write to %s: %s", + who, strerror(errno)); + + status_trace("... , awaiting %u", replytype); + + for (;;) { + reply = wire_sync_read(ctx, fd); + if (!reply) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "Could not set sync read from %s: %s", + who, strerror(errno)); + if (fromwire_peektype(reply) == replytype) { + status_trace("Got it!"); + break; + } + + status_trace("Nope, got %u instead", fromwire_peektype(reply)); + msg_enqueue(queue, take(reply)); + } + + return reply; +} + +static u8 *master_wait_sync_reply(const tal_t *ctx, + struct peer *peer, const u8 *msg, + enum channel_wire_type replytype) +{ + return wait_sync_reply(ctx, msg, replytype, + MASTER_FD, peer->from_master, "master"); +} + +static u8 *gossipd_wait_sync_reply(const tal_t *ctx, + struct peer *peer, const u8 *msg, + enum gossip_peerd_wire_type replytype) +{ + return wait_sync_reply(ctx, msg, replytype, + GOSSIP_FD, peer->from_gossipd, "gossipd"); +} + /* * The maximum msat that this node will accept for an htlc. * It's flagged as an optional field in `channel_update`. @@ -461,6 +514,7 @@ static void announce_channel(struct peer *peer) static void channel_announcement_negotiate(struct peer *peer) { + u8 *msg; /* Don't do any announcement work if we're shutting down */ if (peer->shutdown_sent[LOCAL]) return; @@ -512,14 +566,12 @@ static void channel_announcement_negotiate(struct peer *peer) * But we won't do this when restart, because we load announcement * signatures form DB when we reenable Channeld! */ if(peer->remote_ann_stored == false){ - /* Ask Master to save remote announcement into DB. - * We will never waste time on waiting for remote peer announcement - * reply when restart. - */ - wire_sync_write(MASTER_FD, - take(towire_channel_got_announcement(NULL, - &peer->announcement_node_sigs[REMOTE], - &peer->announcement_bitcoin_sigs[REMOTE]))); + msg = towire_channel_got_announcement(NULL, + &peer->announcement_node_sigs[REMOTE], + &peer->announcement_bitcoin_sigs[REMOTE]); + master_wait_sync_reply(tmpctx, peer, + take(msg), + WIRE_CHANNEL_GOT_ANNOUNCEMENT_REPLY); peer->remote_ann_stored = true; } announce_channel(peer); @@ -575,8 +627,8 @@ static void handle_peer_announcement_signatures(struct peer *peer, const u8 *msg secp256k1_ecdsa_signature *remote_bitcoin_sigs; struct short_channel_id *remote_scid = tal(tmpctx, struct short_channel_id); - remote_node_sigs = talz(tmpctx, secp256k1_ecdsa_signature); - remote_bitcoin_sigs = talz(tmpctx, secp256k1_ecdsa_signature); + remote_node_sigs = tal(tmpctx, secp256k1_ecdsa_signature); + remote_bitcoin_sigs = tal(tmpctx, secp256k1_ecdsa_signature); if (!fromwire_announcement_signatures(msg, &chanid, @@ -607,13 +659,13 @@ static void handle_peer_announcement_signatures(struct peer *peer, const u8 *msg if (!short_channel_id_eq(remote_scid, &peer->short_channel_ids[REMOTE])) peer_failed(&peer->cs, - &peer->channel_id, - "Wrong announcement: " - "first received short_channel_ids: %s" - " , now second get %s", - type_to_string(peer, struct short_channel_id, + &peer->channel_id, + "Wrong announcement: " + "first received short_channel_ids: %s" + " , now second get %s", + type_to_string(peer, struct short_channel_id, &peer->short_channel_ids[REMOTE]), - type_to_string(peer, struct short_channel_id, + type_to_string(peer, struct short_channel_id, remote_scid)); /* make sure new node signature and bitcoin signature equal @@ -625,17 +677,17 @@ static void handle_peer_announcement_signatures(struct peer *peer, const u8 *msg remote_bitcoin_sigs, sizeof(peer->announcement_bitcoin_sigs[REMOTE]))) peer_failed(&peer->cs, - &peer->channel_id, - "Wrong announcement: first received node_sigs %s, " - "now second get %s , first received bitcoin_sigs %s," - "now second get %s", - type_to_string(tmpctx, secp256k1_ecdsa_signature, + &peer->channel_id, + "Wrong announcement: first received node_sigs %s, " + "now second get %s , first received bitcoin_sigs %s," + "now second get %s", + type_to_string(tmpctx, secp256k1_ecdsa_signature, &peer->announcement_node_sigs[REMOTE]), - type_to_string(tmpctx, secp256k1_ecdsa_signature, + type_to_string(tmpctx, secp256k1_ecdsa_signature, remote_node_sigs), - type_to_string(tmpctx, secp256k1_ecdsa_signature, + type_to_string(tmpctx, secp256k1_ecdsa_signature, &peer->announcement_bitcoin_sigs[REMOTE]), - type_to_string(tmpctx, secp256k1_ecdsa_signature, + type_to_string(tmpctx, secp256k1_ecdsa_signature, remote_bitcoin_sigs)); } else { peer->have_sigs[REMOTE] = true; @@ -847,59 +899,6 @@ static void maybe_send_shutdown(struct peer *peer) billboard_update(peer); } -/* This queues other traffic from the fd until we get reply. */ -static u8 *wait_sync_reply(const tal_t *ctx, - const u8 *msg, - int replytype, - int fd, - struct msg_queue *queue, - const char *who) -{ - u8 *reply; - - status_trace("Sending %s %u", who, fromwire_peektype(msg)); - - if (!wire_sync_write(fd, msg)) - status_failed(STATUS_FAIL_INTERNAL_ERROR, - "Could not set sync write to %s: %s", - who, strerror(errno)); - - status_trace("... , awaiting %u", replytype); - - for (;;) { - reply = wire_sync_read(ctx, fd); - if (!reply) - status_failed(STATUS_FAIL_INTERNAL_ERROR, - "Could not set sync read from %s: %s", - who, strerror(errno)); - if (fromwire_peektype(reply) == replytype) { - status_trace("Got it!"); - break; - } - - status_trace("Nope, got %u instead", fromwire_peektype(reply)); - msg_enqueue(queue, take(reply)); - } - - return reply; -} - -static u8 *master_wait_sync_reply(const tal_t *ctx, - struct peer *peer, const u8 *msg, - enum channel_wire_type replytype) -{ - return wait_sync_reply(ctx, msg, replytype, - MASTER_FD, peer->from_master, "master"); -} - -static u8 *gossipd_wait_sync_reply(const tal_t *ctx, - struct peer *peer, const u8 *msg, - enum gossip_peerd_wire_type replytype) -{ - return wait_sync_reply(ctx, msg, replytype, - GOSSIP_FD, peer->from_gossipd, "gossipd"); -} - static u8 *foreign_channel_update(const tal_t *ctx, struct peer *peer, const struct short_channel_id *scid) @@ -2847,6 +2846,7 @@ static void req_in(struct peer *peer, const u8 *msg) case WIRE_CHANNEL_GOT_REVOKE_REPLY: case WIRE_CHANNEL_GOT_FUNDING_LOCKED: case WIRE_CHANNEL_GOT_ANNOUNCEMENT: + case WIRE_CHANNEL_GOT_ANNOUNCEMENT_REPLY: case WIRE_CHANNEL_GOT_SHUTDOWN: case WIRE_CHANNEL_SHUTDOWN_COMPLETE: case WIRE_CHANNEL_DEV_REENABLE_COMMIT_REPLY: diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index a3c59a1820d0..0b4bc953aa0f 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -139,6 +139,12 @@ static void peer_got_announcement(struct channel *channel, const u8 *msg) return; } + /* To handle the case that Master was shut down before sends reply. + * In this case, master may receive announcement more than once! + */ + tal_free(channel->remote_ann_node_sig); + tal_free(channel->remote_ann_bitcoin_sig); + channel->remote_ann_node_sig = tal_steal(channel, remote_ann_node_sig); channel->remote_ann_bitcoin_sig @@ -146,6 +152,8 @@ static void peer_got_announcement(struct channel *channel, const u8 *msg) /* save remote peer announcement signatures into DB! */ wallet_announcement_save(channel->peer->ld->wallet, channel); + subd_send_msg(channel->owner, + take(towire_channel_got_announcement_reply(msg))); } static void peer_got_shutdown(struct channel *channel, const u8 *msg) @@ -281,6 +289,7 @@ static unsigned channel_msg(struct subd *sd, const u8 *msg, const int *fds) case WIRE_CHANNEL_OFFER_HTLC_REPLY: case WIRE_CHANNEL_DEV_REENABLE_COMMIT_REPLY: case WIRE_CHANNEL_DEV_MEMLEAK_REPLY: + case WIRE_CHANNEL_GOT_ANNOUNCEMENT_REPLY: break; }