Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No next lookup via gossipd #3547

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions gossipd/gossip_wire.csv
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ msgdata,gossip_get_channel_peer_reply,peer_id,?node_id,
msgdata,gossip_get_channel_peer_reply,stripped_update_len,u16,
msgdata,gossip_get_channel_peer_reply,stripped_update,u8,stripped_update_len

# Given a short_channel_id, return the latest (stripped) update for error msg.
msgtype,gossip_get_stripped_cupdate,3010
msgdata,gossip_get_stripped_cupdate,channel_id,short_channel_id,

msgtype,gossip_get_stripped_cupdate_reply,3110
msgdata,gossip_get_stripped_cupdate_reply,stripped_update_len,u16,
msgdata,gossip_get_stripped_cupdate_reply,stripped_update,u8,stripped_update_len

# gossipd->master: we're closing this channel.
msgtype,gossip_local_channel_close,3027
msgdata,gossip_local_channel_close,short_channel_id,short_channel_id,
Expand Down
44 changes: 44 additions & 0 deletions gossipd/gossipd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1377,6 +1377,46 @@ static struct io_plan *get_channel_peer(struct io_conn *conn,
return daemon_conn_read_next(conn, daemon->master);
}

/*~ lightningd: so, get me the latest update for this local channel,
* so I can include it in an error message. */
static struct io_plan *get_stripped_cupdate(struct io_conn *conn,
struct daemon *daemon, const u8 *msg)
{
struct short_channel_id scid;
struct local_chan *local_chan;
const u8 *stripped_update;

if (!fromwire_gossip_get_stripped_cupdate(msg, &scid))
master_badmsg(WIRE_GOSSIP_GET_STRIPPED_CUPDATE, msg);

local_chan = local_chan_map_get(&daemon->rstate->local_chan_map, &scid);
if (!local_chan) {
status_debug("Failed to resolve local channel %s",
type_to_string(tmpctx, struct short_channel_id, &scid));
stripped_update = NULL;
} else {
const struct half_chan *hc;

/* Since we're going to use it, make sure it's up-to-date. */
refresh_local_channel(daemon, local_chan, false);

hc = &local_chan->chan->half[local_chan->direction];
if (is_halfchan_defined(hc)) {
const u8 *update;

update = gossip_store_get(tmpctx, daemon->rstate->gs,
hc->bcast.index);
stripped_update = tal_dup_arr(tmpctx, u8, update + 2,
tal_count(update) - 2, 0);
} else
stripped_update = NULL;
}
daemon_conn_send(daemon->master,
take(towire_gossip_get_stripped_cupdate_reply(NULL,
stripped_update)));
return daemon_conn_read_next(conn, daemon->master);
}

/*~ We queue incoming channel_announcement pending confirmation from lightningd
* that it really is an unspent output. Here's its reply. */
static struct io_plan *handle_txout_reply(struct io_conn *conn,
Expand Down Expand Up @@ -1574,6 +1614,9 @@ static struct io_plan *recv_req(struct io_conn *conn,
case WIRE_GOSSIP_GET_CHANNEL_PEER:
return get_channel_peer(conn, daemon, msg);

case WIRE_GOSSIP_GET_STRIPPED_CUPDATE:
return get_stripped_cupdate(conn, daemon, msg);

case WIRE_GOSSIP_GET_TXOUT_REPLY:
return handle_txout_reply(conn, daemon, msg);

Expand Down Expand Up @@ -1621,6 +1664,7 @@ static struct io_plan *recv_req(struct io_conn *conn,
case WIRE_GOSSIP_GETCHANNELS_REPLY:
case WIRE_GOSSIP_PING_REPLY:
case WIRE_GOSSIP_GET_CHANNEL_PEER_REPLY:
case WIRE_GOSSIP_GET_STRIPPED_CUPDATE_REPLY:
case WIRE_GOSSIP_GET_INCOMING_CHANNELS_REPLY:
case WIRE_GOSSIP_GET_TXOUT:
case WIRE_GOSSIP_DEV_MEMLEAK_REPLY:
Expand Down
2 changes: 2 additions & 0 deletions lightningd/gossip_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
case WIRE_GOSSIP_GETCHANNELS_REQUEST:
case WIRE_GOSSIP_PING:
case WIRE_GOSSIP_GET_CHANNEL_PEER:
case WIRE_GOSSIP_GET_STRIPPED_CUPDATE:
case WIRE_GOSSIP_GET_TXOUT_REPLY:
case WIRE_GOSSIP_OUTPOINT_SPENT:
case WIRE_GOSSIP_PAYMENT_FAILURE:
Expand All @@ -158,6 +159,7 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
case WIRE_GOSSIP_GET_INCOMING_CHANNELS_REPLY:
case WIRE_GOSSIP_DEV_MEMLEAK_REPLY:
case WIRE_GOSSIP_DEV_COMPACT_STORE_REPLY:
case WIRE_GOSSIP_GET_STRIPPED_CUPDATE_REPLY:
break;

case WIRE_GOSSIP_PING_REPLY:
Expand Down
119 changes: 98 additions & 21 deletions lightningd/peer_htlcs.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,74 @@ static struct failed_htlc *mk_failed_htlc(const tal_t *ctx,
return f;
}

static void tell_channeld_htlc_failed(const struct htlc_in *hin,
const struct failed_htlc *failed_htlc)
{
/* Tell peer, if we can. */
if (!hin->key.channel->owner)
return;

/* onchaind doesn't care, it can't do anything but wait */
if (channel_on_chain(hin->key.channel))
return;

subd_send_msg(hin->key.channel->owner,
take(towire_channel_fail_htlc(NULL, failed_htlc)));
}

struct failmsg_update_cbdata {
struct htlc_in *hin;
const u8 *failmsg_needs_update;
};

static void failmsg_update_reply(struct subd *gossipd,
const u8 *msg,
const int *unused,
struct failmsg_update_cbdata *cbdata)
{
u8 *failmsg;
u8 *stripped_update;
struct failed_htlc *failed_htlc;

/* In theory, we could have no update because channel suddenly closed,
* but it's v. unlikely */
if (!fromwire_gossip_get_stripped_cupdate_reply(msg, msg,
&stripped_update)
|| !tal_count(stripped_update)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't we using tal_bytelen for u8*?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a line-ball, IMHO. Especially here, where it will actually (due to our explicit fromwire_ generation policy) be NULL instead of empty.

log_broken(gossipd->log,
"bad fromwire_gossip_get_stripped_cupdate %s",
tal_hex(msg, msg));
failmsg = towire_temporary_node_failure(NULL);
} else {
/* End of failmsg is two zero bytes (empty update). */
assert(tal_count(cbdata->failmsg_needs_update) >= 2);
failmsg = tal_dup_arr(msg, u8,
cbdata->failmsg_needs_update,
tal_count(cbdata->failmsg_needs_update)-2,
0);
towire_u16(&failmsg, tal_count(stripped_update));
towire_u8_array(&failmsg,
stripped_update, tal_count(stripped_update));
}

/* Now we replace dummy failonion with this real one */
tal_free(cbdata->hin->failonion);
cbdata->hin->failonion
= create_onionreply(cbdata->hin,
cbdata->hin->shared_secret,
failmsg);

wallet_htlc_update(gossipd->ld->wallet,
cbdata->hin->dbid, cbdata->hin->hstate,
cbdata->hin->preimage,
cbdata->hin->badonion,
cbdata->hin->failonion, NULL);

failed_htlc = mk_failed_htlc(tmpctx,
cbdata->hin, cbdata->hin->failonion);
tell_channeld_htlc_failed(cbdata->hin, failed_htlc);
}

static void fail_in_htlc(struct htlc_in *hin,
const struct onionreply *failonion TAKES)
{
Expand All @@ -140,17 +208,8 @@ static void fail_in_htlc(struct htlc_in *hin,
htlc_in_update_state(hin->key.channel, hin, SENT_REMOVE_HTLC);
htlc_in_check(hin, __func__);

/* Tell peer, if we can. */
if (!hin->key.channel->owner)
return;

/* onchaind doesn't care, it can't do anything but wait */
if (channel_on_chain(hin->key.channel))
return;

failed_htlc = mk_failed_htlc(tmpctx, hin, hin->failonion);
subd_send_msg(hin->key.channel->owner,
take(towire_channel_fail_htlc(NULL, failed_htlc)));
tell_channeld_htlc_failed(hin, failed_htlc);
}

/* Immediately fail HTLC with a BADONION code */
Expand All @@ -167,17 +226,7 @@ static void local_fail_in_htlc_badonion(struct htlc_in *hin,
htlc_in_check(hin, __func__);

failed_htlc = mk_failed_htlc_badonion(tmpctx, hin, badonion);

/* Tell peer, if we can. */
if (!hin->key.channel->owner)
return;

/* onchaind doesn't care, it can't do anything but wait */
if (channel_on_chain(hin->key.channel))
return;

subd_send_msg(hin->key.channel->owner,
take(towire_channel_fail_htlc(NULL, failed_htlc)));
tell_channeld_htlc_failed(hin, failed_htlc);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice simplification 👍

}

/* This is used for cases where we can immediately fail the HTLC. */
Expand All @@ -193,6 +242,34 @@ void local_fail_in_htlc(struct htlc_in *hin, const u8 *failmsg TAKES)
fail_in_htlc(hin, take(failonion));
}

/* This is used for cases where we can immediately fail the HTLC, but
* need to append a channel_update. */
void local_fail_in_htlc_needs_update(struct htlc_in *hin,
const u8 *failmsg_needs_update TAKES,
const struct short_channel_id *failmsg_scid)
{
struct failmsg_update_cbdata *cbdata;

/* To avoid the state where we have no failonion, we use a temporary
* one, and update once we get the reply from gossipd. */
assert(!hin->preimage);

hin->failonion = create_onionreply(hin,
hin->shared_secret,
towire_temporary_node_failure(tmpctx));
/* We update state now to signal it's in progress, for persistence. */
htlc_in_update_state(hin->key.channel, hin, SENT_REMOVE_HTLC);
htlc_in_check(hin, __func__);

cbdata = tal(hin, struct failmsg_update_cbdata);
cbdata->hin = hin;
cbdata->failmsg_needs_update
= tal_dup_talarr(cbdata, u8, failmsg_needs_update);
subd_req(cbdata, hin->key.channel->peer->ld->gossip,
take(towire_gossip_get_stripped_cupdate(NULL, failmsg_scid)),
-1, 0, failmsg_update_reply, cbdata);
Copy link
Contributor

@niftynei niftynei Feb 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like your gossip daemon doesnt exist in all cases? failing test_restart_many_payments

EBUG:root:lightningd-1: 2020-02-25T06:08:23.095Z DEBUG 0382ce59ebf18be7d84677c2e35f23294b9992ceca95491fcf8a56c6cb2d9de199-chan#3: HTLC in 1 RCVD_ADD_ACK_REVOCATION->SENT_REMOVE_HTLC
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: FATAL SIGNAL 11 (version v0.8.1-36-g03045e1-modded)
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: common/daemon.c:44 (send_backtrace) 0x5634d3e4c1b0
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: common/daemon.c:52 (crashdump) 0x5634d3e4c200
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: (null):0 ((null)) 0x7fb9d3ca4f1f
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: lightningd/subd.c:750 (subd_send_msg) 0x5634d3e43132
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: lightningd/subd.c:770 (subd_req_) 0x5634d3e431f6
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: lightningd/peer_htlcs.c:268 (local_fail_in_htlc_needs_update) 0x5634d3e35438
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: lightningd/peer_htlcs.c:296 (fail_out_htlc) 0x5634d3e3558d
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: lightningd/peer_htlcs.c:516 (destroy_hout_subd_died) 0x5634d3e35c86
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: ccan/ccan/tal/tal.c:240 (notify) 0x5634d3eb2472
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: ccan/ccan/tal/tal.c:402 (del_tree) 0x5634d3eb2961
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: ccan/ccan/tal/tal.c:486 (tal_free) 0x5634d3eb2ced
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: lightningd/peer_htlcs.c:2107 (free_htlcs) 0x5634d3e3a85a
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: lightningd/lightningd.c:520 (shutdown_subdaemons) 0x5634d3e1a7a1
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: lightningd/lightningd.c:940 (main) 0x5634d3e1b217
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: (null):0 ((null)) 0x7fb9d3c87b96
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: (null):0 ((null)) 0x5634d3e01b09
DEBUG:root:lightningd-1: 2020-02-25T06:08:23.108Z **BROKEN** lightningd: backtrace: (null):0 ((null)) 0xffffffffffffffff
DEBUG:root:Received response for stop call: {'error': 'Connection to RPC server lost.'}
----------------------------- Captured stderr call -----------------------------
lightningd: FATAL SIGNAL 11 (version v0.8.1-36-g03045e1-modded)
0x5634d3e4c15a send_backtrace
	common/daemon.c:39
0x5634d3e4c200 crashdump
	common/daemon.c:52
0x7fb9d3ca4f1f ???
	???:0
0x5634d3e43132 subd_send_msg
	lightningd/subd.c:750
0x5634d3e431f6 subd_req_
	lightningd/subd.c:770
0x5634d3e35438 local_fail_in_htlc_needs_update
	lightningd/peer_htlcs.c:268
0x5634d3e3558d fail_out_htlc
	lightningd/peer_htlcs.c:296
0x5634d3e35c86 destroy_hout_subd_died
	lightningd/peer_htlcs.c:516
0x5634d3eb2472 notify
	ccan/ccan/tal/tal.c:240
0x5634d3eb2961 del_tree
	ccan/ccan/tal/tal.c:402
0x5634d3eb2ced tal_free
	ccan/ccan/tal/tal.c:486
0x5634d3e3a85a free_htlcs
	lightningd/peer_htlcs.c:2107
0x5634d3e1a7a1 shutdown_subdaemons
	lightningd/lightningd.c:520
0x5634d3e1b217 main
	lightningd/lightningd.c:940
0x7fb9d3c87b96 ???
	???:0
0x5634d3e01b09 ???
	???:0
0xffffffffffffffff ???
	???:0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I think this is best addressed by freeing HTLCs and peer daemons first, then gossipd. Thanks!

}

/* Helper to create (common) WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS */
const u8 *failmsg_incorrect_or_unknown(const tal_t *ctx,
const struct htlc_in *hin)
Expand Down
3 changes: 3 additions & 0 deletions lightningd/peer_htlcs.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ void htlcs_resubmit(struct lightningd *ld,
/* For HTLCs which terminate here, invoice payment calls one of these. */
void fulfill_htlc(struct htlc_in *hin, const struct preimage *preimage);
void local_fail_in_htlc(struct htlc_in *hin, const u8 *failmsg TAKES);
void local_fail_in_htlc_needs_update(struct htlc_in *hin,
const u8 *failmsg_needs_update TAKES,
const struct short_channel_id *failmsg_scid);

/* This json process will be used as the serialize method for
* forward_event_notification_gen and be used in
Expand Down
24 changes: 6 additions & 18 deletions wallet/test/run-wallet.c
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ bool fromwire_custommsg_in(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u8
/* Generated stub for fromwire_gossip_get_channel_peer_reply */
bool fromwire_gossip_get_channel_peer_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct node_id **peer_id UNNEEDED, u8 **stripped_update UNNEEDED)
{ fprintf(stderr, "fromwire_gossip_get_channel_peer_reply called!\n"); abort(); }
/* Generated stub for fromwire_gossip_get_stripped_cupdate_reply */
bool fromwire_gossip_get_stripped_cupdate_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u8 **stripped_update UNNEEDED)
{ fprintf(stderr, "fromwire_gossip_get_stripped_cupdate_reply called!\n"); abort(); }
/* Generated stub for fromwire_hsm_ecdh_resp */
bool fromwire_hsm_ecdh_resp(const void *p UNNEEDED, struct secret *ss UNNEEDED)
{ fprintf(stderr, "fromwire_hsm_ecdh_resp called!\n"); abort(); }
Expand Down Expand Up @@ -577,9 +580,6 @@ void topology_add_sync_waiter_(const tal_t *ctx UNNEEDED,
void *arg) UNNEEDED,
void *arg UNNEEDED)
{ fprintf(stderr, "topology_add_sync_waiter_ called!\n"); abort(); }
/* Generated stub for towire_amount_below_minimum */
u8 *towire_amount_below_minimum(const tal_t *ctx UNNEEDED, struct amount_msat htlc_msat UNNEEDED, const u8 *channel_update UNNEEDED)
{ fprintf(stderr, "towire_amount_below_minimum called!\n"); abort(); }
/* Generated stub for towire_channel_dev_memleak */
u8 *towire_channel_dev_memleak(const tal_t *ctx UNNEEDED)
{ fprintf(stderr, "towire_channel_dev_memleak called!\n"); abort(); }
Expand Down Expand Up @@ -645,6 +645,9 @@ u8 *towire_final_incorrect_htlc_amount(const tal_t *ctx UNNEEDED, struct amount_
/* Generated stub for towire_gossip_get_channel_peer */
u8 *towire_gossip_get_channel_peer(const tal_t *ctx UNNEEDED, const struct short_channel_id *channel_id UNNEEDED)
{ fprintf(stderr, "towire_gossip_get_channel_peer called!\n"); abort(); }
/* Generated stub for towire_gossip_get_stripped_cupdate */
u8 *towire_gossip_get_stripped_cupdate(const tal_t *ctx UNNEEDED, const struct short_channel_id *channel_id UNNEEDED)
{ fprintf(stderr, "towire_gossip_get_stripped_cupdate called!\n"); abort(); }
/* Generated stub for towire_hsm_ecdh_req */
u8 *towire_hsm_ecdh_req(const tal_t *ctx UNNEEDED, const struct pubkey *point UNNEEDED)
{ fprintf(stderr, "towire_hsm_ecdh_req called!\n"); abort(); }
Expand All @@ -657,24 +660,9 @@ u8 *towire_incorrect_cltv_expiry(const tal_t *ctx UNNEEDED, u32 cltv_expiry UNNE
/* Generated stub for towire_incorrect_or_unknown_payment_details */
u8 *towire_incorrect_or_unknown_payment_details(const tal_t *ctx UNNEEDED, struct amount_msat htlc_msat UNNEEDED, u32 height UNNEEDED)
{ fprintf(stderr, "towire_incorrect_or_unknown_payment_details called!\n"); abort(); }
/* Generated stub for towire_invalid_onion_hmac */
u8 *towire_invalid_onion_hmac(const tal_t *ctx UNNEEDED, const struct sha256 *sha256_of_onion UNNEEDED)
{ fprintf(stderr, "towire_invalid_onion_hmac called!\n"); abort(); }
/* Generated stub for towire_invalid_onion_key */
u8 *towire_invalid_onion_key(const tal_t *ctx UNNEEDED, const struct sha256 *sha256_of_onion UNNEEDED)
{ fprintf(stderr, "towire_invalid_onion_key called!\n"); abort(); }
/* Generated stub for towire_invalid_onion_payload */
u8 *towire_invalid_onion_payload(const tal_t *ctx UNNEEDED, varint type UNNEEDED, u16 offset UNNEEDED)
{ fprintf(stderr, "towire_invalid_onion_payload called!\n"); abort(); }
/* Generated stub for towire_invalid_onion_version */
u8 *towire_invalid_onion_version(const tal_t *ctx UNNEEDED, const struct sha256 *sha256_of_onion UNNEEDED)
{ fprintf(stderr, "towire_invalid_onion_version called!\n"); abort(); }
/* Generated stub for towire_invalid_realm */
u8 *towire_invalid_realm(const tal_t *ctx UNNEEDED)
{ fprintf(stderr, "towire_invalid_realm called!\n"); abort(); }
/* Generated stub for towire_mpp_timeout */
u8 *towire_mpp_timeout(const tal_t *ctx UNNEEDED)
{ fprintf(stderr, "towire_mpp_timeout called!\n"); abort(); }
/* Generated stub for towire_onchain_dev_memleak */
u8 *towire_onchain_dev_memleak(const tal_t *ctx UNNEEDED)
{ fprintf(stderr, "towire_onchain_dev_memleak called!\n"); abort(); }
Expand Down