diff --git a/channeld/channel.c b/channeld/channel.c index 45b9eb1b2..efbbc6dcf 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -1758,7 +1758,6 @@ static u8 *channeld_read_peer_msg(struct peer *peer) &peer->channel_id, channeld_send_reply, channeld_io_error, - status_fail_errpkt, peer); } diff --git a/closingd/closing.c b/closingd/closing.c index 5cdaa98da..155824c65 100644 --- a/closingd/closing.c +++ b/closingd/closing.c @@ -87,7 +87,6 @@ static u8 *closing_read_peer_msg(const tal_t *ctx, while ((msg = read_peer_msg(ctx, cs, gossip_index, channel, sync_crypto_write_arg, status_fail_io, - status_fail_errpkt, NULL)) == NULL); return msg; diff --git a/common/read_peer_msg.c b/common/read_peer_msg.c index 0f343d593..3a3171f0f 100644 --- a/common/read_peer_msg.c +++ b/common/read_peer_msg.c @@ -45,11 +45,6 @@ u8 *read_peer_msg_(const tal_t *ctx, bool (*send_reply)(struct crypto_state *cs, int fd, const u8 *TAKES, void *arg), void (*io_error)(void *arg), - void (*err_pkt)(int peer_fd, int gossip_fd, - struct crypto_state *cs, u64 gossip_index, - const char *desc, - const struct channel_id *channel_id, - void *arg), void *arg) { u8 *msg; @@ -90,8 +85,9 @@ u8 *read_peer_msg_(const tal_t *ctx, * - MUST ignore the message. */ if (structeq(&chanid, channel) || channel_id_is_all(&chanid)) - err_pkt(peer_fd, gossip_fd, cs, gossip_index, - err, &chanid, arg); + peer_failed_received_errmsg(peer_fd, gossip_fd, + cs, gossip_index, + err, &chanid); return tal_free(msg); } @@ -126,14 +122,3 @@ void status_fail_io(void *unused) { peer_failed_connection_lost(); } - -/* Helper: calls peer_failed_received_errmsg() */ -void status_fail_errpkt(int peer_fd, int gossip_fd, - struct crypto_state *cs, u64 gossip_index, - const char *desc, - const struct channel_id *channel_id, - void *unused) -{ - peer_failed_received_errmsg(peer_fd, gossip_fd, - cs, gossip_index, desc, channel_id); -} diff --git a/common/read_peer_msg.h b/common/read_peer_msg.h index 7debad4dd..5c8399509 100644 --- a/common/read_peer_msg.h +++ b/common/read_peer_msg.h @@ -17,25 +17,18 @@ struct channel_id; * @send_reply: the way to send a reply packet (eg. sync_crypto_write_arg) * @io_error: what to do if there's an IO error (eg. status_fail_io) * (MUST NOT RETURN!) - * @err_pkt: what to do if there's an error packet (eg. status_fail_errorpkt) - * (MUST NOT RETURN!) * * This returns NULL if it handled the message, so it's normally called in * a loop. */ #define read_peer_msg(ctx, cs, gossip_index, chanid, send_reply, \ - io_error, err_pkt, arg) \ + io_error, arg) \ read_peer_msg_((ctx), PEER_FD, GOSSIP_FD, (cs), (gossip_index), \ (chanid), \ typesafe_cb_preargs(bool, void *, (send_reply), (arg), \ struct crypto_state *, int, \ const u8 *), \ typesafe_cb(void, void *, (io_error), (arg)), \ - typesafe_cb_preargs(void, void *, (err_pkt), (arg), \ - int, int, \ - struct crypto_state *, \ - u64, const char *, \ - const struct channel_id *), \ arg) /* Helper: sync_crypto_write, with extra args it ignores */ @@ -45,13 +38,6 @@ bool sync_crypto_write_arg(struct crypto_state *cs, int fd, const u8 *TAKES, /* Helper: calls peer_failed_connection_lost. */ void status_fail_io(void *unused); -/* Helper: calls peer_failed_received_errmsg() */ -void status_fail_errpkt(int peer_fd, int gossip_fd, - struct crypto_state *cs, u64 gossip_index, - const char *desc, - const struct channel_id *channel_id, - void *unused); - u8 *read_peer_msg_(const tal_t *ctx, int peer_fd, int gossip_fd, struct crypto_state *cs, u64 gossip_index, @@ -59,11 +45,6 @@ u8 *read_peer_msg_(const tal_t *ctx, bool (*send_reply)(struct crypto_state *cs, int fd, const u8 *TAKES, void *arg), void (*io_error)(void *arg), - void (*err_pkt)(int peer_fd, int gossip_fd, - struct crypto_state *cs, u64 gossip_index, - const char *desc, - const struct channel_id *channel_id, - void *arg), void *arg); #endif /* LIGHTNING_COMMON_READ_PEER_MSG_H */ diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 49a1b7d81..0b00fcfa2 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -288,19 +288,19 @@ static void connect_failed(struct lightningd *ld, const struct pubkey *id, } } -/* Opening failed: hand back to gossipd (with error if we have channel_id) */ +/* Opening failed: hand back to gossipd (sending errpkt if not NULL) */ static void uncommitted_channel_to_gossipd(struct lightningd *ld, struct uncommitted_channel *uc, - const struct channel_id *channel_id, const struct crypto_state *cs, u64 gossip_index, int peer_fd, int gossip_fd, + const u8 *errorpkt, const char *fmt, ...) { va_list ap; char *errstr; - u8 *error = NULL, *msg; + u8 *msg; va_start(ap, fmt); errstr = tal_vfmt(uc, fmt, ap); @@ -309,12 +309,11 @@ static void uncommitted_channel_to_gossipd(struct lightningd *ld, log_unusual(uc->log, "Opening channel: %s", errstr); if (uc->fc) command_fail(uc->fc->cmd, "%s", errstr); - if (channel_id) - error = towire_errorfmt(uc, channel_id, "%s", errstr); - /* Hand back to gossipd, with an error packet. */ - msg = towire_gossipctl_hand_back_peer(error, &uc->peer->id, cs, + + /* Hand back to gossipd, (maybe) with an error packet to send. */ + msg = towire_gossipctl_hand_back_peer(errstr, &uc->peer->id, cs, gossip_index, - error); + errorpkt); subd_send_msg(ld->gossip, take(msg)); subd_send_fd(ld->gossip, peer_fd); subd_send_fd(ld->gossip, gossip_fd); @@ -2472,41 +2471,6 @@ static void opening_fundee_finished(struct subd *openingd, tal_free(uc); } -/* Negotiation failed, but we can keep gossipping */ -static unsigned int opening_negotiation_failed(struct subd *openingd, - const u8 *msg, - const int *fds) -{ - struct uncommitted_channel *uc = openingd->channel; - struct crypto_state cs; - u64 gossip_index; - char *why; - struct lightningd *ld = openingd->ld; - - /* We need the peer fd and gossip fd. */ - if (tal_count(fds) == 0) - return 2; - - if (!fromwire_opening_negotiation_failed(msg, msg, NULL, - &cs, &gossip_index, &why)) { - log_broken(uc->log, "bad OPENING_NEGOTIATION_FAILED %s", - tal_hex(msg, msg)); - tal_free(openingd); - return 0; - } - - uncommitted_channel_to_gossipd(ld, uc, NULL, - &cs, gossip_index, - fds[0], fds[1], - "%s", why); - - /* Don't call opening_channel_errmsg, just free openingd */ - subd_release_channel(openingd, uc); - tal_free(uc); - return 0; -} - -/* peer_fd == -1 for local if daemon died */ static void opening_channel_errmsg(struct uncommitted_channel *uc, int peer_fd, int gossip_fd, const struct crypto_state *cs, @@ -2515,15 +2479,20 @@ static void opening_channel_errmsg(struct uncommitted_channel *uc, const char *desc, const u8 *err_for_them) { - if (uc->fc) - command_fail(uc->fc->cmd, "%sERROR %s", - peer_fd == -1 ? "" - : (err_for_them ? "sent " : "received "), - desc); + if (peer_fd == -1) { + log_info(uc->log, "%s", desc); + if (uc->fc) + command_fail(uc->fc->cmd, "%s", desc); + } else { + /* An error occurred (presumably negotiation fail). */ + const char *errsrc = err_for_them ? "sent" : "received"; - log_info(uc->log, "%sERROR %s", - peer_fd == -1 ? "" : (err_for_them ? "sent " : "received "), - desc); + uncommitted_channel_to_gossipd(uc->peer->ld, uc, + cs, gossip_index, + peer_fd, gossip_fd, + err_for_them, + "%s ERROR %s", errsrc, desc); + } tal_free(uc); } @@ -2603,17 +2572,23 @@ static u8 *peer_accept_channel(struct lightningd *ld, "Multiple channels unsupported"); uc->openingd = new_channel_subd(ld, "lightning_openingd", uc, uc->log, - opening_wire_type_name, - opening_negotiation_failed, + opening_wire_type_name, NULL, opening_channel_errmsg, take(&peer_fd), take(&gossip_fd), NULL); if (!uc->openingd) { - uncommitted_channel_to_gossipd(ld, uc, channel_id, + u8 *errpkt; + char *errmsg; + + errmsg = tal_fmt(uc, "INTERNAL ERROR:" + " Failed to subdaemon opening: %s", + strerror(errno)); + errpkt = towire_errorfmt(uc, channel_id, "%s", errmsg); + + uncommitted_channel_to_gossipd(ld, uc, cs, gossip_index, peer_fd, gossip_fd, - "Failed to subdaemon opening: %s", - strerror(errno)); + errpkt, "%s", errmsg); tal_free(uc); return NULL; } @@ -2682,15 +2657,17 @@ static void peer_offer_channel(struct lightningd *ld, fc->uc->openingd = new_channel_subd(ld, "lightning_openingd", fc->uc, fc->uc->log, - opening_wire_type_name, - opening_negotiation_failed, + opening_wire_type_name, NULL, opening_channel_errmsg, take(&peer_fd), take(&gossip_fd), NULL); if (!fc->uc->openingd) { + /* We don't send them an eror packet: for them, nothing + * happened! */ uncommitted_channel_to_gossipd(ld, fc->uc, NULL, - cs, gossip_index, - peer_fd, gossip_fd, + gossip_index, + peer_fd, gossip_fd, + NULL, "Failed to launch openingd: %s", strerror(errno)); tal_free(fc->uc); diff --git a/openingd/opening.c b/openingd/opening.c index cb17b34bf..4f06c03b0 100644 --- a/openingd/opening.c +++ b/openingd/opening.c @@ -67,37 +67,17 @@ struct state { }; /* For negotiation failures: we can still gossip with client. */ -static void negotiation_failed(struct state *state, bool send_error, - const char *fmt, ...) +static void negotiation_failed(struct state *state, const char *fmt, ...) { va_list ap; const char *errmsg; - u8 *msg; va_start(ap, fmt); errmsg = tal_vfmt(state, fmt, ap); va_end(ap); - /* Make sure it's correct length for towire_. */ - tal_resize(&errmsg, strlen(errmsg)+1); - - /* We don't send error in response to their error packet. */ - if (send_error) { - /* Tell peer we're bailing on this channel. */ - msg = towire_errorfmt(errmsg, &state->channel_id, "%s", errmsg); - sync_crypto_write(&state->cs, PEER_FD, take(msg)); - } - - /* Tell master we should return to gossiping. */ - msg = towire_opening_negotiation_failed(state, &state->cs, - state->gossip_index, - errmsg); - wire_sync_write(REQ_FD, msg); - fdpass_send(REQ_FD, PEER_FD); - fdpass_send(REQ_FD, GOSSIP_FD); - - tal_free(state); - exit(0); + peer_failed(&state->cs, state->gossip_index, &state->channel_id, + "%s", errmsg); } static void check_config_bounds(struct state *state, @@ -112,7 +92,7 @@ static void check_config_bounds(struct state *state, * unreasonably large. */ if (remoteconf->to_self_delay > state->max_to_self_delay) - negotiation_failed(state, true, + negotiation_failed(state, "to_self_delay %u larger than %u", remoteconf->to_self_delay, state->max_to_self_delay); @@ -130,7 +110,7 @@ static void check_config_bounds(struct state *state, /* Overflow check before capacity calc. */ if (remoteconf->channel_reserve_satoshis > state->funding_satoshis) - negotiation_failed(state, true, + negotiation_failed(state, "Invalid channel_reserve_satoshis %"PRIu64 " for funding_satoshis %"PRIu64, remoteconf->channel_reserve_satoshis, @@ -147,7 +127,7 @@ static void check_config_bounds(struct state *state, capacity_msat = remoteconf->max_htlc_value_in_flight_msat; if (remoteconf->htlc_minimum_msat * (u64)1000 > capacity_msat) - negotiation_failed(state, true, + negotiation_failed(state, "Invalid htlc_minimum_msat %"PRIu64 " for funding_satoshis %"PRIu64 " capacity_msat %"PRIu64, @@ -156,7 +136,7 @@ static void check_config_bounds(struct state *state, capacity_msat); if (capacity_msat < state->min_effective_htlc_capacity_msat) - negotiation_failed(state, true, + negotiation_failed(state, "Channel capacity with funding %"PRIu64" msat," " reserves %"PRIu64"/%"PRIu64" msat," " max_htlc_value_in_flight_msat %"PRIu64 @@ -170,7 +150,7 @@ static void check_config_bounds(struct state *state, /* We don't worry about how many HTLCs they accept, as long as > 0! */ if (remoteconf->max_accepted_htlcs == 0) - negotiation_failed(state, true, + negotiation_failed(state, "max_accepted_htlcs %u invalid", remoteconf->max_accepted_htlcs); @@ -205,20 +185,6 @@ static void temporary_channel_id(struct channel_id *channel_id) channel_id->id[i] = pseudorand(256); } -/* This handles the case where there's an error only for this channel */ -static void opening_errpkt(int peer_fd, int gossip_fd, - struct crypto_state *cs, u64 gossip_index, - const char *desc, - const struct channel_id *channel_id, - struct state *state) -{ - /* FIXME: Remove negotiation_failed */ - if (structeq(channel_id, &state->channel_id)) - negotiation_failed(state, false, "Error packet: %s", desc); - peer_failed_received_errmsg(peer_fd, gossip_fd, - cs, gossip_index, desc, channel_id); -} - /* Handle random messages we might get, returning the first non-handled one. */ static u8 *opening_read_peer_msg(struct state *state) { @@ -228,7 +194,6 @@ static u8 *opening_read_peer_msg(struct state *state) &state->channel_id, sync_crypto_write_arg, status_fail_io, - opening_errpkt, state)) == NULL); return msg; @@ -346,7 +311,7 @@ static u8 *funder_channel(struct state *state, * `open_channel`. */ if (minimum_depth > max_minimum_depth) - negotiation_failed(state, true, + negotiation_failed(state, "minimum_depth %u larger than %u", minimum_depth, max_minimum_depth); check_config_bounds(state, state->remoteconf); @@ -538,7 +503,7 @@ static u8 *fundee_channel(struct state *state, * unknown to the receiver. */ if (!structeq(&chain_hash, &state->chainparams->genesis_blockhash)) { - negotiation_failed(state, true, + negotiation_failed(state, "Unknown chain-hash %s", type_to_string(peer_msg, struct bitcoin_blkid, @@ -573,12 +538,12 @@ static u8 *fundee_channel(struct state *state, * too small for timely processing, or unreasonably large. */ if (state->feerate_per_kw < min_feerate) - negotiation_failed(state, true, + negotiation_failed(state, "feerate_per_kw %u below minimum %u", state->feerate_per_kw, min_feerate); if (state->feerate_per_kw > max_feerate) - negotiation_failed(state, true, + negotiation_failed(state, "feerate_per_kw %u above maximum %u", state->feerate_per_kw, max_feerate); diff --git a/openingd/opening_wire.csv b/openingd/opening_wire.csv index 2e1401bee..eced430fa 100644 --- a/openingd/opening_wire.csv +++ b/openingd/opening_wire.csv @@ -76,10 +76,3 @@ opening_fundee_reply,,feerate_per_kw,u32 # The (encrypted) funding signed message: send this and we're committed. opening_fundee_reply,,msglen,u16 opening_fundee_reply,,funding_signed_msg,msglen*u8 - -# We disagreed with opening parameters, but peer is ok for gossip (+ peerfd) -opening_negotiation_failed,6010 -opening_negotiation_failed,,crypto_state,struct crypto_state -opening_negotiation_failed,,gossip_index,u64 -opening_negotiation_failed,,msg,wirestring - diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 15c4adc6a..8b751c27d 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -154,9 +154,6 @@ bool fromwire_opening_fundee_reply(const tal_t *ctx UNNEEDED, const void *p UNNE /* Generated stub for fromwire_opening_funder_reply */ bool fromwire_opening_funder_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, size_t *plen UNNEEDED, struct channel_config *their_config UNNEEDED, struct bitcoin_tx **first_commit UNNEEDED, secp256k1_ecdsa_signature *first_commit_sig UNNEEDED, struct crypto_state *crypto_state UNNEEDED, u64 *gossip_index UNNEEDED, struct pubkey *revocation_basepoint UNNEEDED, struct pubkey *payment_basepoint UNNEEDED, struct pubkey *htlc_basepoint UNNEEDED, struct pubkey *delayed_payment_basepoint UNNEEDED, struct pubkey *their_per_commit_point UNNEEDED, u32 *minimum_depth UNNEEDED, struct pubkey *remote_fundingkey UNNEEDED, struct bitcoin_txid *funding_txid UNNEEDED, u32 *feerate_per_kw UNNEEDED) { fprintf(stderr, "fromwire_opening_funder_reply called!\n"); abort(); } -/* Generated stub for fromwire_opening_negotiation_failed */ -bool fromwire_opening_negotiation_failed(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, size_t *plen UNNEEDED, struct crypto_state *crypto_state UNNEEDED, u64 *gossip_index UNNEEDED, wirestring **msg UNNEEDED) -{ fprintf(stderr, "fromwire_opening_negotiation_failed called!\n"); abort(); } /* Generated stub for funding_tx */ struct bitcoin_tx *funding_tx(const tal_t *ctx UNNEEDED, u16 *outnum UNNEEDED,