From 2327f04b54ab3ac45f4bb40a08f645781669f829 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 20 Sep 2018 14:37:59 -0700 Subject: [PATCH 01/27] HACKING.md: Update rec'd test cmd to `full-check` Otherwise you won't run `check-source` and your Travis build will fail on you. Also remove comment about how it's currently disabled cuz that's a lie. --- doc/HACKING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/HACKING.md b/doc/HACKING.md index bdfb8488f975..71f09f94a5d4 100644 --- a/doc/HACKING.md +++ b/doc/HACKING.md @@ -172,7 +172,7 @@ PYTEST_PAR=n - runs pytests in parallel A modern desktop can build and run through all the tests in a couple of minutes with: - make -j12 check PYTEST_PAR=24 DEVELOPER=1 VALGRIND=0 + make -j12 full-check PYTEST_PAR=24 DEVELOPER=1 VALGRIND=0 Adjust `-j` and `PYTEST_PAR` accordingly for your hardware. @@ -180,7 +180,7 @@ There are three kinds of tests: * **source tests** - run by `make check-source`, looks for whitespace, header order, and checks formatted quotes from BOLTs if BOLTDIR - exists (currently disabled, since BOLTs are being re-edited). + exists. * **unit tests** - standalone programs that can be run individually. They are `run-*.c` files in test/ subdirectories used to test routines From b287f2f0070fe0e0a128aaf805fac6e9511112c6 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 20 Sep 2018 13:07:53 -0700 Subject: [PATCH 02/27] BOLT 11 human-readable formatting changes --- Makefile | 2 +- common/bolt11.c | 36 +++++++++++++++++++++++------------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index 6eaa29719847..8c6448e9b78c 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../lightning-rfc/ -BOLTVERSION := fd9da9b95eb5d585252d7e749212151502e0cc17 +BOLTVERSION := 21e3688e843f82267b3970cda69fa93158dc9517 -include config.vars diff --git a/common/bolt11.c b/common/bolt11.c index 3bb23ab3e07b..fad4b54fcfea 100644 --- a/common/bolt11.c +++ b/common/bolt11.c @@ -498,7 +498,8 @@ struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str, /* BOLT #11: * - * A reader MUST fail if it does not understand the `prefix`. + * A reader: + * - MUST fail if it does not understand the `prefix` */ if (!strstarts(prefix, "ln")) return decode_fail(b11, fail, @@ -510,14 +511,13 @@ struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str, /* BOLT #11: * - * A reader SHOULD fail if `amount` contains a non-digit or - * is followed by anything except a `multiplier` in the table - * above. */ + * - If the `amount` is empty: + * */ amountstr = tal_strdup(tmpctx, hrp + strlen(prefix)); if (streq(amountstr, "")) { /* BOLT #11: * - * A reader SHOULD indicate if amount is unspecified + * - SHOULD indicate if amount is unspecified */ b11->msatoshi = NULL; } else { @@ -537,10 +537,9 @@ struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str, /* BOLT #11: * - * A reader SHOULD fail if `amount` contains a non-digit or - * is followed by anything except a `multiplier` in the table - * above. - */ + * MUST fail if `amount` contains a non-digit or is followed by + * anything except a `multiplier` in the table above + **/ amount = strtoull(amountstr, &end, 10); if (amount == ULLONG_MAX && errno == ERANGE) return decode_fail(b11, fail, @@ -549,7 +548,12 @@ struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str, return decode_fail(b11, fail, "Invalid amount postfix '%s'", end); - /* Convert to millisatoshis. */ + /* BOLT #11: + * + * - If the `multiplier` is present: + * - MUST multiply `amount` by the `multiplier` + * value to derive the amount required for payment + **/ b11->msatoshi = tal(b11, u64); *b11->msatoshi = amount * m10 / 10; } @@ -873,9 +877,15 @@ char *bolt11_encode_(const tal_t *ctx, /* BOLT #11: * - * A writer MUST encode `amount` as a positive decimal integer - * with no leading zeroes and SHOULD use the shortest representation - * possible. + * A writer: + * - MUST encode `prefix` using the currency it requires + * for successful payment + * - If it requires a specific minimum amount for successful payment: + * - MUST include that `amount` + * - MUST encode `amount` as a positive decimal integer + * with no leading zeroes + * - SHOULD use the shortest representation possible by + * using the largest multiplier or omitting the multiplier */ if (b11->msatoshi) { char postfix; From 73ea6d0038d8580e88c4bb162ef57c313391bcb8 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 20 Sep 2018 13:13:53 -0700 Subject: [PATCH 03/27] BOLT 2 updates for fix placment of chain_hash req See https://github.com/lightningnetwork/lightning-rfc/commit/4b62d26af93a07166d6a9de2cb5eff80849d9c19 --- Makefile | 2 +- openingd/openingd.c | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 8c6448e9b78c..29d25591cdc9 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../lightning-rfc/ -BOLTVERSION := 21e3688e843f82267b3970cda69fa93158dc9517 +BOLTVERSION := 4b62d26af93a07166d6a9de2cb5eff80849d9c19 -include config.vars diff --git a/openingd/openingd.c b/openingd/openingd.c index 4478f545d453..72060c76c364 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -440,7 +440,6 @@ static u8 *funder_channel(struct state *state, /* BOLT #2: * * The receiver: - *... * - if `minimum_depth` is unreasonably large: * - MAY reject the channel. */ @@ -704,10 +703,9 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) /* BOLT #2: * - * The receiver: - * - if the `chain_hash` value, within the `open_channel`, message is - * set to a hash of a chain that is unknown to the receiver: - * - MUST reject the channel. + * The receiving node MUST fail the channel if: + * - the `chain_hash` value is set to a hash of a chain + * that is unknown to the receiver. */ if (!bitcoin_blkid_eq(&chain_hash, &state->chainparams->genesis_blockhash)) { @@ -733,6 +731,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) /* BOLT #2: * * The receiving node MUST fail the channel if: + * ... * - `push_msat` is greater than `funding_satoshis` * 1000. */ if (state->push_msat > state->funding_satoshis * 1000) { From b1f15c260525bf7fdefbcad9f046f34bb76fe5b7 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 20 Sep 2018 13:19:38 -0700 Subject: [PATCH 04/27] BOLT updates: broken link fixes See https://github.com/lightningnetwork/lightning-rfc/commit/a9195a84d07b9350f0eb1262ed0c1a82bc42e4ef --- Makefile | 2 +- channeld/commit_tx.c | 4 ++-- common/initial_commit_tx.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 29d25591cdc9..4fdd27876296 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../lightning-rfc/ -BOLTVERSION := 4b62d26af93a07166d6a9de2cb5eff80849d9c19 +BOLTVERSION := a9195a84d07b9350f0eb1262ed0c1a82bc42e4ef -include config.vars diff --git a/channeld/commit_tx.c b/channeld/commit_tx.c index 33ed4f4be31f..ad44e9982217 100644 --- a/channeld/commit_tx.c +++ b/channeld/commit_tx.c @@ -200,7 +200,7 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx, * * 5. If the `to_local` amount is greater or equal to * `dust_limit_satoshis`, add a [`to_local` - * output](#to-local-output). + * output](#to_local-output). */ if (self_pay_msat / 1000 >= dust_limit_satoshis) { u8 *wscript = to_self_wscript(tmpctx, to_self_delay,keyset); @@ -218,7 +218,7 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx, * * 6. If the `to_remote` amount is greater or equal to * `dust_limit_satoshis`, add a [`to_remote` - * output](#to-remote-output). + * output](#to_remote-output). */ if (other_pay_msat / 1000 >= dust_limit_satoshis) { /* BOLT #3: diff --git a/common/initial_commit_tx.c b/common/initial_commit_tx.c index 6142f6fb3c3b..bdbff1232060 100644 --- a/common/initial_commit_tx.c +++ b/common/initial_commit_tx.c @@ -157,7 +157,7 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx, * * 5. If the `to_local` amount is greater or equal to * `dust_limit_satoshis`, add a [`to_local` - * output](#to-local-output). + * output](#to_local-output). */ if (self_pay_msat / 1000 >= dust_limit_satoshis) { u8 *wscript = to_self_wscript(tmpctx, to_self_delay,keyset); @@ -170,7 +170,7 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx, * * 6. If the `to_remote` amount is greater or equal to * `dust_limit_satoshis`, add a [`to_remote` - * output](#to-remote-output). + * output](#to_remote-output). */ if (other_pay_msat / 1000 >= dust_limit_satoshis) { /* BOLT #3: From b1ceaf99101448f3bbc7dd228732047387b8376e Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Wed, 19 Sep 2018 17:59:46 -0700 Subject: [PATCH 05/27] gossipd: Update BOLT-split flags in channel_update BOLT 7's been updated to split the flags field in `channel_update` into two: `channel_flags` and `message_flags`. This changeset does the minimal necessary to get to building with the new flags. --- CHANGELOG.md | 3 ++ Makefile | 2 +- gossipd/gossipd.c | 29 ++++++++++------ gossipd/routing.c | 47 +++++++++++++++----------- gossipd/routing.h | 8 +++-- gossipd/test/run-bench-find_route.c | 4 +-- gossipd/test/run-find_route-specific.c | 24 +++++++------ gossipd/test/run-find_route.c | 6 ++-- hsmd/hsmd.c | 7 ++-- lightningd/gossip_control.c | 9 +++-- lightningd/gossip_msg.c | 6 ++-- lightningd/gossip_msg.h | 3 +- tests/utils.py | 2 +- wire/gen_peer_wire_csv | 3 +- wire/test/run-peer-wire.c | 9 +++-- 15 files changed, 101 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42634719073e..b8d6b99d4a4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - JSON API: `listpeers` has new field `scratch_txid`: the latest tx in channel. +- JSON API: `listchannels` has two new fields: `message_flags` and `channel_flags`. This replaces `flags`. - Bitcoind: more parallelism in requests, for very slow nodes. - Testing: fixed logging, cleaner interception of bitcoind, minor fixes. @@ -19,6 +20,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. Note: You should always set `allow-deprecated-apis=false` to test for changes. +- JSON RPC: `listchannels`' `flags` field. This has been split into two fields, see Added. + ### Removed ### Fixed diff --git a/Makefile b/Makefile index 4fdd27876296..4e730bf22b7b 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../lightning-rfc/ -BOLTVERSION := a9195a84d07b9350f0eb1262ed0c1a82bc42e4ef +BOLTVERSION := 0891374d47ddffa64c5a2e6ad151247e3d6b7a59 -include config.vars diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 7d1d6247d70a..6b842ebac20c 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -1008,7 +1008,11 @@ static u8 *create_channel_update(const tal_t *ctx, secp256k1_ecdsa_signature dummy_sig; u8 *update, *msg; u32 timestamp = time_now().ts.tv_sec; - u16 flags; + u8 message_flags, channel_flags; + + /* `message_flags` are optional. + * Currently, not set by c-lightning */ + message_flags = 0; /* So valgrind doesn't complain */ memset(&dummy_sig, 0, sizeof(dummy_sig)); @@ -1018,15 +1022,16 @@ static u8 *create_channel_update(const tal_t *ctx, && timestamp == chan->half[direction].last_timestamp) timestamp++; - flags = direction; + channel_flags = direction; if (disable) - flags |= ROUTING_FLAGS_DISABLED; + channel_flags |= ROUTING_FLAGS_DISABLED; update = towire_channel_update(ctx, &dummy_sig, &rstate->chain_hash, &chan->scid, timestamp, - flags, cltv_expiry_delta, + message_flags, channel_flags, + cltv_expiry_delta, htlc_minimum_msat, fee_base_msat, fee_proportional_millionths); @@ -1055,7 +1060,7 @@ static bool update_redundant(const struct half_chan *hc, if (!is_halfchan_defined(hc)) return false; - return !(hc->flags & ROUTING_FLAGS_DISABLED) == !disable + return !(hc->channel_flags & ROUTING_FLAGS_DISABLED) == !disable && hc->delay == cltv_delta && hc->htlc_minimum_msat == htlc_minimum_msat && hc->base_fee == fee_base_msat @@ -1108,11 +1113,11 @@ static void apply_delayed_local_update(struct local_update *local_update) &local_update->scid), local_update->direction, is_halfchan_defined(hc) - ? (hc->flags & ROUTING_FLAGS_DISABLED ? "DISABLED" : "ACTIVE") + ? (hc->channel_flags & ROUTING_FLAGS_DISABLED ? "DISABLED" : "ACTIVE") : "UNDEFINED", hc->last_timestamp, (u32)time_now().ts.tv_sec, - hc->flags, + hc->channel_flags, local_update->disable); tal_free(local_update); return; @@ -1441,7 +1446,8 @@ static void append_half_channel(struct gossip_getchannels_entry **entries, e->source = chan->nodes[idx]->id; e->destination = chan->nodes[!idx]->id; e->satoshis = chan->satoshis; - e->flags = c->flags; + e->channel_flags = c->channel_flags; + e->message_flags = c->message_flags; e->local_disabled = chan->local_disabled; e->public = is_chan_public(chan); e->short_channel_id = chan->scid; @@ -1767,7 +1773,7 @@ static void gossip_send_keepalive_update(struct routing_state *rstate, /* Generate a new update, with up to date timestamp */ update = create_channel_update(tmpctx, rstate, chan, - hc->flags & ROUTING_FLAGS_DIRECTION, + hc->channel_flags & ROUTING_FLAGS_DIRECTION, false, hc->delay, hc->htlc_minimum_msat, @@ -1834,7 +1840,7 @@ static void gossip_disable_outgoing_halfchan(struct daemon *daemon, { u8 direction; struct half_chan *hc; - u16 flags; + u8 message_flags, channel_flags; u32 timestamp; struct bitcoin_blkid chain_hash; secp256k1_ecdsa_signature sig; @@ -1858,7 +1864,8 @@ static void gossip_disable_outgoing_halfchan(struct daemon *daemon, if (!fromwire_channel_update( hc->channel_update, &sig, &chain_hash, - &local_update->scid, ×tamp, &flags, + &local_update->scid, ×tamp, + &message_flags, &channel_flags, &local_update->cltv_delta, &local_update->htlc_minimum_msat, &local_update->fee_base_msat, diff --git a/gossipd/routing.c b/gossipd/routing.c index a0ab36a8054a..67fed194a940 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -259,13 +259,17 @@ static void destroy_chan(struct chan *chan, struct routing_state *rstate) static void init_half_chan(struct routing_state *rstate, struct chan *chan, - int idx) + int channel_idx) { - struct half_chan *c = &chan->half[idx]; + struct half_chan *c = &chan->half[channel_idx]; c->channel_update = NULL; c->unroutable_until = 0; - c->flags = idx; + + /* Set the channel direction */ + c->channel_flags = channel_idx; + // TODO: wireup message_flags + c->message_flags = 0; /* We haven't seen channel_update: make it halfway to prune time, * which should be older than any update we'd see. */ c->last_timestamp = time_now().ts.tv_sec - rstate->prune_timeout/2; @@ -1003,7 +1007,8 @@ static void set_connection_values(struct chan *chan, u32 base_fee, u32 proportional_fee, u32 delay, - u16 flags, + u8 message_flags, + u8 channel_flags, u64 timestamp, u32 htlc_minimum_msat) { @@ -1013,9 +1018,10 @@ static void set_connection_values(struct chan *chan, c->htlc_minimum_msat = htlc_minimum_msat; c->base_fee = base_fee; c->proportional_fee = proportional_fee; - c->flags = flags; + c->message_flags = message_flags; + c->channel_flags = channel_flags; c->last_timestamp = timestamp; - assert((c->flags & ROUTING_FLAGS_DIRECTION) == idx); + assert((c->channel_flags & ROUTING_FLAGS_DIRECTION) == idx); /* If it was temporarily unroutable, re-enable */ c->unroutable_until = 0; @@ -1031,7 +1037,7 @@ static void set_connection_values(struct chan *chan, &chan->scid), idx, c->proportional_fee); - c->flags |= ROUTING_FLAGS_DISABLED; + c->channel_flags |= ROUTING_FLAGS_DISABLED; } } @@ -1041,7 +1047,7 @@ bool routing_add_channel_update(struct routing_state *rstate, secp256k1_ecdsa_signature signature; struct short_channel_id short_channel_id; u32 timestamp; - u16 flags; + u8 message_flags, channel_flags; u16 expiry; u64 htlc_minimum_msat; u32 fee_base_msat; @@ -1052,7 +1058,8 @@ bool routing_add_channel_update(struct routing_state *rstate, bool have_broadcast_announce; if (!fromwire_channel_update(update, &signature, &chain_hash, - &short_channel_id, ×tamp, &flags, + &short_channel_id, ×tamp, + &message_flags, &channel_flags, &expiry, &htlc_minimum_msat, &fee_base_msat, &fee_proportional_millionths)) return false; @@ -1064,10 +1071,11 @@ bool routing_add_channel_update(struct routing_state *rstate, have_broadcast_announce = is_halfchan_defined(&chan->half[0]) || is_halfchan_defined(&chan->half[1]); - direction = flags & 0x1; + direction = channel_flags & 0x1; set_connection_values(chan, direction, fee_base_msat, fee_proportional_millionths, expiry, - flags, timestamp, htlc_minimum_msat); + message_flags, channel_flags, + timestamp, htlc_minimum_msat); /* Replace any old one. */ tal_free(chan->half[direction].channel_update); @@ -1101,7 +1109,7 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update, secp256k1_ecdsa_signature signature; struct short_channel_id short_channel_id; u32 timestamp; - u16 flags; + u8 message_flags, channel_flags; u16 expiry; u64 htlc_minimum_msat; u32 fee_base_msat; @@ -1115,7 +1123,8 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update, serialized = tal_dup_arr(tmpctx, u8, update, len, 0); if (!fromwire_channel_update(serialized, &signature, &chain_hash, &short_channel_id, - ×tamp, &flags, &expiry, + ×tamp, &message_flags, + &channel_flags, &expiry, &htlc_minimum_msat, &fee_base_msat, &fee_proportional_millionths)) { err = towire_errorfmt(rstate, NULL, @@ -1123,7 +1132,7 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update, tal_hex(tmpctx, serialized)); return err; } - direction = flags & 0x1; + direction = channel_flags & 0x1; /* BOLT #7: * @@ -1160,7 +1169,7 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update, type_to_string(tmpctx, struct short_channel_id, &short_channel_id), - flags)); + channel_flags)); return NULL; } } @@ -1198,7 +1207,7 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update, type_to_string(tmpctx, struct short_channel_id, &short_channel_id), - flags, + channel_flags, tal_hex(tmpctx, c->channel_update), tal_hex(tmpctx, serialized)); } @@ -1224,10 +1233,10 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update, status_trace("Received channel_update for channel %s(%d) now %s was %s (from %s)", type_to_string(tmpctx, struct short_channel_id, &short_channel_id), - flags & 0x01, - flags & ROUTING_FLAGS_DISABLED ? "DISABLED" : "ACTIVE", + channel_flags & 0x01, + channel_flags & ROUTING_FLAGS_DISABLED ? "DISABLED" : "ACTIVE", is_halfchan_defined(c) - ? (c->flags & ROUTING_FLAGS_DISABLED ? "DISABLED" : "ACTIVE") + ? (c->channel_flags & ROUTING_FLAGS_DISABLED ? "DISABLED" : "ACTIVE") : "UNDEFINED", source); diff --git a/gossipd/routing.h b/gossipd/routing.h index f8552611ea95..6fb15d941a2e 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -31,7 +31,11 @@ struct half_chan { /* Flags as specified by the `channel_update`s, among other * things indicated direction wrt the `channel_id` */ - u16 flags; + u8 channel_flags; + + /* Flags as specified by the `channel_update`s, indicates + * optional fields. */ + u8 message_flags; /* If greater than current time, this connection should not * be used for routing. */ @@ -81,7 +85,7 @@ static inline bool is_halfchan_defined(const struct half_chan *hc) static inline bool is_halfchan_enabled(const struct half_chan *hc) { - return is_halfchan_defined(hc) && !(hc->flags & ROUTING_FLAGS_DISABLED); + return is_halfchan_defined(hc) && !(hc->channel_flags & ROUTING_FLAGS_DISABLED); } struct node { diff --git a/gossipd/test/run-bench-find_route.c b/gossipd/test/run-bench-find_route.c index 9984bce8b825..0347217c26bc 100644 --- a/gossipd/test/run-bench-find_route.c +++ b/gossipd/test/run-bench-find_route.c @@ -63,7 +63,7 @@ void broadcast_del(struct broadcast_state *bstate UNNEEDED, u64 index UNNEEDED, bool fromwire_channel_announcement(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, secp256k1_ecdsa_signature *node_signature_1 UNNEEDED, secp256k1_ecdsa_signature *node_signature_2 UNNEEDED, secp256k1_ecdsa_signature *bitcoin_signature_1 UNNEEDED, secp256k1_ecdsa_signature *bitcoin_signature_2 UNNEEDED, u8 **features UNNEEDED, struct bitcoin_blkid *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct pubkey *node_id_1 UNNEEDED, struct pubkey *node_id_2 UNNEEDED, struct pubkey *bitcoin_key_1 UNNEEDED, struct pubkey *bitcoin_key_2 UNNEEDED) { fprintf(stderr, "fromwire_channel_announcement called!\n"); abort(); } /* Generated stub for fromwire_channel_update */ -bool fromwire_channel_update(const void *p UNNEEDED, secp256k1_ecdsa_signature *signature UNNEEDED, struct bitcoin_blkid *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, u32 *timestamp UNNEEDED, u16 *flags UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, u64 *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED) +bool fromwire_channel_update(const void *p UNNEEDED, secp256k1_ecdsa_signature *signature UNNEEDED, struct bitcoin_blkid *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, u32 *timestamp UNNEEDED, u8 *message_flags UNNEEDED, u8 *channel_flags UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, u64 *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED) { fprintf(stderr, "fromwire_channel_update called!\n"); abort(); } /* Generated stub for fromwire_gossip_local_add_channel */ bool fromwire_gossip_local_add_channel(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct pubkey *remote_node_id UNNEEDED, u64 *satoshis UNNEEDED) @@ -160,7 +160,7 @@ static void add_connection(struct routing_state *rstate, c->base_fee = base_fee; c->proportional_fee = proportional_fee; c->delay = delay; - c->flags = get_channel_direction(from, to); + c->channel_flags = get_channel_direction(from, to); } static struct pubkey nodeid(size_t n) diff --git a/gossipd/test/run-find_route-specific.c b/gossipd/test/run-find_route-specific.c index 79d25b5b356b..90b8d70404ae 100644 --- a/gossipd/test/run-find_route-specific.c +++ b/gossipd/test/run-find_route-specific.c @@ -3,7 +3,7 @@ * Expect route 03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf -> 0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae -> 02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06 * * getchannels: - * {'channels': [{'active': True, 'short_id': '6990:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'last_update': 1504064344}, {'active': True, 'short_id': '6989:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'flags': 0, 'destination': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, {'active': True, 'short_id': '6990:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'flags': 0, 'destination': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, {'active': True, 'short_id': '6989:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'last_update': 1504064344}]} + * {'channels': [{'active': True, 'short_id': '6990:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'message_flags': 0, 'channel_flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'last_update': 1504064344}, {'active': True, 'short_id': '6989:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'message_flags': 0, 'channel_flags': 0, 'destination': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, {'active': True, 'short_id': '6990:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'message_flags': 0, 'channel_flags': 0, 'destination': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, {'active': True, 'short_id': '6989:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'message_flags': 0, 'channel_flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'last_update': 1504064344}]} */ #include @@ -27,7 +27,7 @@ void broadcast_del(struct broadcast_state *bstate UNNEEDED, u64 index UNNEEDED, bool fromwire_channel_announcement(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, secp256k1_ecdsa_signature *node_signature_1 UNNEEDED, secp256k1_ecdsa_signature *node_signature_2 UNNEEDED, secp256k1_ecdsa_signature *bitcoin_signature_1 UNNEEDED, secp256k1_ecdsa_signature *bitcoin_signature_2 UNNEEDED, u8 **features UNNEEDED, struct bitcoin_blkid *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct pubkey *node_id_1 UNNEEDED, struct pubkey *node_id_2 UNNEEDED, struct pubkey *bitcoin_key_1 UNNEEDED, struct pubkey *bitcoin_key_2 UNNEEDED) { fprintf(stderr, "fromwire_channel_announcement called!\n"); abort(); } /* Generated stub for fromwire_channel_update */ -bool fromwire_channel_update(const void *p UNNEEDED, secp256k1_ecdsa_signature *signature UNNEEDED, struct bitcoin_blkid *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, u32 *timestamp UNNEEDED, u16 *flags UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, u64 *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED) +bool fromwire_channel_update(const void *p UNNEEDED, secp256k1_ecdsa_signature *signature UNNEEDED, struct bitcoin_blkid *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, u32 *timestamp UNNEEDED, u8 *message_flags UNNEEDED, u8 *channel_flags UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, u64 *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED) { fprintf(stderr, "fromwire_channel_update called!\n"); abort(); } /* Generated stub for fromwire_gossip_local_add_channel */ bool fromwire_gossip_local_add_channel(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct pubkey *remote_node_id UNNEEDED, u64 *satoshis UNNEEDED) @@ -172,38 +172,42 @@ int main(void) rstate = new_routing_state(tmpctx, &zerohash, &a, 0); - /* [{'active': True, 'short_id': '6990:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'last_update': 1504064344}, */ + /* [{'active': True, 'short_id': '6990:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'message_flags': 0, 'channel_flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'last_update': 1504064344}, */ nc = get_or_make_connection(rstate, &c, &b, "6990:2:1", 1000); nc->base_fee = 0; nc->proportional_fee = 10; nc->delay = 5; - nc->flags = 1; + nc->channel_flags = 1; + nc->message_flags = 0; nc->last_timestamp = 1504064344; - /* {'active': True, 'short_id': '6989:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'flags': 0, 'destination': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, */ + /* {'active': True, 'short_id': '6989:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'message_flags': 0, 'channel_flags': 0, 'destination': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, */ nc = get_or_make_connection(rstate, &b, &a, "6989:2:1", 1000); nc->base_fee = 0; nc->proportional_fee = 10; nc->delay = 5; - nc->flags = 0; + nc->channel_flags = 0; + nc->message_flags = 0; nc->last_timestamp = 1504064344; - /* {'active': True, 'short_id': '6990:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'flags': 0, 'destination': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, */ + /* {'active': True, 'short_id': '6990:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'message_flags': 0, 'channel_flags': 0, 'destination': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, */ nc = get_or_make_connection(rstate, &b, &c, "6990:2:1", 1000); nc->base_fee = 0; nc->proportional_fee = 10; nc->delay = 5; - nc->flags = 0; + nc->channel_flags = 0; + nc->message_flags = 0; nc->last_timestamp = 1504064344; nc->htlc_minimum_msat = 100; - /* {'active': True, 'short_id': '6989:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'last_update': 1504064344}]} */ + /* {'active': True, 'short_id': '6989:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'message_flags': 0, 'channel_flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'last_update': 1504064344}]} */ nc = get_or_make_connection(rstate, &a, &b, "6989:2:1", 1000); nc->base_fee = 0; nc->proportional_fee = 10; nc->delay = 5; - nc->flags = 1; + nc->channel_flags = 1; + nc->message_flags = 0; nc->last_timestamp = 1504064344; route = find_route(tmpctx, rstate, &a, &c, 100000, riskfactor, 0.0, NULL, &fee); diff --git a/gossipd/test/run-find_route.c b/gossipd/test/run-find_route.c index 372106fabe0a..88ed4660c022 100644 --- a/gossipd/test/run-find_route.c +++ b/gossipd/test/run-find_route.c @@ -25,7 +25,7 @@ void broadcast_del(struct broadcast_state *bstate UNNEEDED, u64 index UNNEEDED, bool fromwire_channel_announcement(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, secp256k1_ecdsa_signature *node_signature_1 UNNEEDED, secp256k1_ecdsa_signature *node_signature_2 UNNEEDED, secp256k1_ecdsa_signature *bitcoin_signature_1 UNNEEDED, secp256k1_ecdsa_signature *bitcoin_signature_2 UNNEEDED, u8 **features UNNEEDED, struct bitcoin_blkid *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct pubkey *node_id_1 UNNEEDED, struct pubkey *node_id_2 UNNEEDED, struct pubkey *bitcoin_key_1 UNNEEDED, struct pubkey *bitcoin_key_2 UNNEEDED) { fprintf(stderr, "fromwire_channel_announcement called!\n"); abort(); } /* Generated stub for fromwire_channel_update */ -bool fromwire_channel_update(const void *p UNNEEDED, secp256k1_ecdsa_signature *signature UNNEEDED, struct bitcoin_blkid *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, u32 *timestamp UNNEEDED, u16 *flags UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, u64 *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED) +bool fromwire_channel_update(const void *p UNNEEDED, secp256k1_ecdsa_signature *signature UNNEEDED, struct bitcoin_blkid *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, u32 *timestamp UNNEEDED, u8 *message_flags UNNEEDED, u8 *channel_flags UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, u64 *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED) { fprintf(stderr, "fromwire_channel_update called!\n"); abort(); } /* Generated stub for fromwire_gossip_local_add_channel */ bool fromwire_gossip_local_add_channel(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct pubkey *remote_node_id UNNEEDED, u64 *satoshis UNNEEDED) @@ -127,7 +127,7 @@ static void add_connection(struct routing_state *rstate, c->base_fee = base_fee; c->proportional_fee = proportional_fee; c->delay = delay; - c->flags = get_channel_direction(from, to); + c->channel_flags = get_channel_direction(from, to); c->htlc_minimum_msat = 0; } @@ -258,7 +258,7 @@ int main(void) assert(fee == 1 + 3); /* Make B->C inactive, force it back via D */ - get_connection(rstate, &b, &c)->flags |= ROUTING_FLAGS_DISABLED; + get_connection(rstate, &b, &c)->channel_flags |= ROUTING_FLAGS_DISABLED; route = find_route(tmpctx, rstate, &a, &c, 3000000, riskfactor, 0.0, NULL, &fee); assert(route); assert(tal_count(route) == 2); diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c index d047e8e917b5..95642c528a11 100644 --- a/hsmd/hsmd.c +++ b/hsmd/hsmd.c @@ -653,7 +653,8 @@ static struct io_plan *handle_channel_update_sig(struct io_conn *conn, struct short_channel_id scid; u32 timestamp, fee_base_msat, fee_proportional_mill; u64 htlc_minimum_msat; - u16 flags, cltv_expiry_delta; + u8 message_flags, channel_flags; + u16 cltv_expiry_delta; struct bitcoin_blkid chain_hash; u8 *cu; @@ -661,7 +662,7 @@ static struct io_plan *handle_channel_update_sig(struct io_conn *conn, return bad_req(conn, c, msg_in); if (!fromwire_channel_update(cu, &sig, &chain_hash, - &scid, ×tamp, &flags, + &scid, ×tamp, &message_flags, &channel_flags, &cltv_expiry_delta, &htlc_minimum_msat, &fee_base_msat, &fee_proportional_mill)) { return bad_req_fmt(conn, c, msg_in, "Bad inner channel_update"); @@ -676,7 +677,7 @@ static struct io_plan *handle_channel_update_sig(struct io_conn *conn, sign_hash(&node_pkey, &hash, &sig); cu = towire_channel_update(tmpctx, &sig, &chain_hash, - &scid, timestamp, flags, + &scid, timestamp, message_flags, channel_flags, cltv_expiry_delta, htlc_minimum_msat, fee_base_msat, fee_proportional_mill); return req_reply(conn, c, take(towire_hsm_cupdate_sig_reply(NULL, cu))); diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 5ebf00b7b09a..a8217edddcb5 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -346,9 +347,13 @@ static void json_listchannels_reply(struct subd *gossip UNUSED, const u8 *reply, &entries[i].short_channel_id)); json_add_bool(response, "public", entries[i].public); json_add_u64(response, "satoshis", entries[i].satoshis); - json_add_num(response, "flags", entries[i].flags); + json_add_num(response, "message_flags", entries[i].message_flags); + json_add_num(response, "channel_flags", entries[i].channel_flags); + /* Prior to spec v0891374d47ddffa64c5a2e6ad151247e3d6b7a59, these two were a single u16 field */ + if (deprecated_apis) + json_add_num(response, "flags", ((u16)entries[i].message_flags << 8) | entries[i].channel_flags); json_add_bool(response, "active", - !(entries[i].flags & ROUTING_FLAGS_DISABLED) + !(entries[i].channel_flags & ROUTING_FLAGS_DISABLED) && !entries[i].local_disabled); json_add_num(response, "last_update", entries[i].last_update_timestamp); diff --git a/lightningd/gossip_msg.c b/lightningd/gossip_msg.c index c29d9a9c76d3..60628cf2f549 100644 --- a/lightningd/gossip_msg.c +++ b/lightningd/gossip_msg.c @@ -84,7 +84,8 @@ void fromwire_gossip_getchannels_entry(const u8 **pptr, size_t *max, fromwire_pubkey(pptr, max, &entry->source); fromwire_pubkey(pptr, max, &entry->destination); entry->satoshis = fromwire_u64(pptr, max); - entry->flags = fromwire_u16(pptr, max); + entry->message_flags = fromwire_u8(pptr, max); + entry->channel_flags = fromwire_u8(pptr, max); entry->public = fromwire_bool(pptr, max); entry->local_disabled = fromwire_bool(pptr, max); entry->last_update_timestamp = fromwire_u32(pptr, max); @@ -100,7 +101,8 @@ void towire_gossip_getchannels_entry(u8 **pptr, towire_pubkey(pptr, &entry->source); towire_pubkey(pptr, &entry->destination); towire_u64(pptr, entry->satoshis); - towire_u16(pptr, entry->flags); + towire_u8(pptr, entry->message_flags); + towire_u8(pptr, entry->channel_flags); towire_bool(pptr, entry->public); towire_bool(pptr, entry->local_disabled); towire_u32(pptr, entry->last_update_timestamp); diff --git a/lightningd/gossip_msg.h b/lightningd/gossip_msg.h index b6c61ca046b8..2fa30ac08a37 100644 --- a/lightningd/gossip_msg.h +++ b/lightningd/gossip_msg.h @@ -22,7 +22,8 @@ struct gossip_getchannels_entry { struct pubkey source, destination; u64 satoshis; struct short_channel_id short_channel_id; - u16 flags; + u8 message_flags; + u8 channel_flags; bool public; bool local_disabled; u32 last_update_timestamp; diff --git a/tests/utils.py b/tests/utils.py index 7ec68a812f17..02387f4a77fc 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -557,7 +557,7 @@ def get_channel_scid(self, other): def is_channel_active(self, chanid): channels = self.rpc.listchannels()['channels'] - active = [(c['short_channel_id'], c['flags']) for c in channels if c['active']] + active = [(c['short_channel_id'], c['channel_flags']) for c in channels if c['active']] return (chanid, 0) in active and (chanid, 1) in active def wait_for_channel_onchain(self, peerid): diff --git a/wire/gen_peer_wire_csv b/wire/gen_peer_wire_csv index 0e07d96131aa..4afd7776fa63 100644 --- a/wire/gen_peer_wire_csv +++ b/wire/gen_peer_wire_csv @@ -143,7 +143,8 @@ channel_update,0,signature,64 channel_update,64,chain_hash,32 channel_update,96,short_channel_id,8 channel_update,104,timestamp,4 -channel_update,108,flags,2 +channel_update,108,message_flags,1 +channel_update,109,channel_flags,1 channel_update,110,cltv_expiry_delta,2 channel_update,112,htlc_minimum_msat,8 channel_update,120,fee_base_msat,4 diff --git a/wire/test/run-peer-wire.c b/wire/test/run-peer-wire.c index 00fef90521a6..d21778a30ad6 100644 --- a/wire/test/run-peer-wire.c +++ b/wire/test/run-peer-wire.c @@ -123,7 +123,8 @@ struct msg_revoke_and_ack { struct msg_channel_update { secp256k1_ecdsa_signature signature; u32 timestamp; - u16 flags; + u8 message_flags; + u8 channel_flags; u16 expiry; u64 htlc_minimum_msat; u32 fee_base_msat; @@ -376,7 +377,8 @@ static void *towire_struct_channel_update(const tal_t *ctx, &s->chain_hash, &s->short_channel_id, s->timestamp, - s->flags, + s->message_flags, + s->channel_flags, s->expiry, s->htlc_minimum_msat, s->fee_base_msat, @@ -392,7 +394,8 @@ static struct msg_channel_update *fromwire_struct_channel_update(const tal_t *ct &s->chain_hash, &s->short_channel_id, &s->timestamp, - &s->flags, + &s->message_flags, + &s->channel_flags, &s->expiry, &s->htlc_minimum_msat, &s->fee_base_msat, From edb74260f4d1e22d122a5ae521c0f84898d6c4e1 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 20 Sep 2018 15:32:10 +0930 Subject: [PATCH 06/27] pytest: more reliable onchain tests. We extract the tx from the logs, and then we wait until that hits the mempool. This is more reliable than 'sendrawtx' in the logs, which might catch a previous sendrawtx; it's also more explicit that we expect that tx exactly. Signed-off-by: Rusty Russell --- tests/test_closing.py | 103 +++++++++++++++++++++--------------------- tests/utils.py | 14 ++++++ 2 files changed, 66 insertions(+), 51 deletions(-) diff --git a/tests/test_closing.py b/tests/test_closing.py index 271b998f68da..45cb348eac44 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -327,12 +327,12 @@ def test_penalty_inhtlc(node_factory, bitcoind, executor): # l2 should spend all of the outputs (except to-us). # Could happen in any order, depending on commitment tx. - l2.daemon.wait_for_logs([ - 'Propose handling THEIR_REVOKED_UNILATERAL/DELAYED_OUTPUT_TO_THEM by OUR_PENALTY_TX .* after 0 blocks', - 'sendrawtx exit 0', - 'Propose handling THEIR_REVOKED_UNILATERAL/THEIR_HTLC by OUR_PENALTY_TX .* after 0 blocks', - 'sendrawtx exit 0' - ]) + needle = l2.daemon.logsearch_start + l2.wait_for_onchaind_broadcast('OUR_PENALTY_TX', + 'THEIR_REVOKED_UNILATERAL/DELAYED_OUTPUT_TO_THEM') + l2.daemon.logsearch_start = needle + l2.wait_for_onchaind_broadcast('OUR_PENALTY_TX', + 'THEIR_REVOKED_UNILATERAL/THEIR_HTLC') # FIXME: test HTLC tx race! @@ -403,13 +403,15 @@ def test_penalty_outhtlc(node_factory, bitcoind, executor): # l2 should spend all of the outputs (except to-us). # Could happen in any order, depending on commitment tx. - l2.daemon.wait_for_logs([ - 'Ignoring output.*: THEIR_REVOKED_UNILATERAL/OUTPUT_TO_US', - 'Propose handling THEIR_REVOKED_UNILATERAL/DELAYED_OUTPUT_TO_THEM by OUR_PENALTY_TX .* after 0 blocks', - 'sendrawtx exit 0', - 'Propose handling THEIR_REVOKED_UNILATERAL/OUR_HTLC by OUR_PENALTY_TX .* after 0 blocks', - 'sendrawtx exit 0' - ]) + needle = l2.daemon.logsearch_start + l2.wait_for_onchaind_broadcast('OUR_PENALTY_TX', + 'THEIR_REVOKED_UNILATERAL/DELAYED_OUTPUT_TO_THEM') + l2.daemon.logsearch_start = needle + l2.wait_for_onchaind_broadcast('OUR_PENALTY_TX', + 'THEIR_REVOKED_UNILATERAL/OUR_HTLC') + + l2.daemon.logsearch_start = needle + l2.daemon.wait_for_log('Ignoring output.*: THEIR_REVOKED_UNILATERAL/OUTPUT_TO_US') # FIXME: test HTLC tx race! @@ -452,10 +454,8 @@ def test_onchain_first_commit(node_factory, bitcoind): # 10 later, l1 should collect its to-self payment. bitcoind.generate_block(10) - l1.daemon.wait_for_logs([ - 'Broadcasting OUR_DELAYED_RETURN_TO_WALLET .* to resolve OUR_UNILATERAL/DELAYED_OUTPUT_TO_US', - 'sendrawtx exit 0' - ]) + l1.wait_for_onchaind_broadcast('OUR_DELAYED_RETURN_TO_WALLET', + 'OUR_UNILATERAL/DELAYED_OUTPUT_TO_US') # 94 later, l2 is done. bitcoind.generate_block(94) @@ -483,9 +483,8 @@ def test_onchain_unwatch(node_factory, bitcoind): # 10 later, l1 should collect its to-self payment. bitcoind.generate_block(10) - l1.daemon.wait_for_log('Broadcasting OUR_DELAYED_RETURN_TO_WALLET .* to resolve ' - 'OUR_UNILATERAL/DELAYED_OUTPUT_TO_US') - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_onchaind_broadcast('OUR_DELAYED_RETURN_TO_WALLET', + 'OUR_UNILATERAL/DELAYED_OUTPUT_TO_US') # First time it sees it, onchaind cares. bitcoind.generate_block(1) @@ -615,8 +614,8 @@ def test_onchain_dust_out(node_factory, bitcoind, executor): # 6 later, l1 should collect its to-self payment. bitcoind.generate_block(6) - l1.daemon.wait_for_log('Broadcasting OUR_DELAYED_RETURN_TO_WALLET .* to resolve OUR_UNILATERAL/DELAYED_OUTPUT_TO_US') - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_onchaind_broadcast('OUR_DELAYED_RETURN_TO_WALLET', + 'OUR_UNILATERAL/DELAYED_OUTPUT_TO_US') # 94 later, l2 is done. bitcoind.generate_block(94) @@ -672,10 +671,12 @@ def test_onchain_timeout(node_factory, bitcoind, executor): 'Propose handling OUR_UNILATERAL/OUR_HTLC by OUR_HTLC_TIMEOUT_TX .* after 6 blocks']) bitcoind.generate_block(4) - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_onchaind_broadcast('OUR_DELAYED_RETURN_TO_WALLET', + 'OUR_UNILATERAL/DELAYED_OUTPUT_TO_US') bitcoind.generate_block(1) - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_onchaind_broadcast('OUR_HTLC_TIMEOUT_TX', + 'OUR_UNILATERAL/OUR_HTLC') # We use 3 blocks for "reasonable depth" bitcoind.generate_block(3) @@ -688,8 +689,8 @@ def test_onchain_timeout(node_factory, bitcoind, executor): # 2 later, l1 spends HTLC (5 blocks total). bitcoind.generate_block(2) - l1.daemon.wait_for_log('Broadcasting OUR_DELAYED_RETURN_TO_WALLET .* to resolve OUR_HTLC_TIMEOUT_TX/DELAYED_OUTPUT_TO_US') - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_onchaind_broadcast('OUR_DELAYED_RETURN_TO_WALLET', + 'OUR_HTLC_TIMEOUT_TX/DELAYED_OUTPUT_TO_US') # 89 later, l2 is done. bitcoind.generate_block(89) @@ -755,8 +756,8 @@ def try_pay(): l2.daemon.wait_for_log('OUR_UNILATERAL/THEIR_HTLC') # l2 should fulfill HTLC onchain, and spend to-us (any order) - l2.daemon.wait_for_log('Propose handling OUR_UNILATERAL/THEIR_HTLC by OUR_HTLC_SUCCESS_TX .* after 0 blocks') - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_onchaind_broadcast('OUR_HTLC_SUCCESS_TX', + 'OUR_UNILATERAL/THEIR_HTLC') # Payment should succeed. l1.bitcoin.generate_block(1) @@ -770,17 +771,13 @@ def try_pay(): # Three more, l2 can spend to-us. bitcoind.generate_block(3) - l2.daemon.wait_for_logs([ - 'Broadcasting OUR_DELAYED_RETURN_TO_WALLET .* to resolve OUR_UNILATERAL/DELAYED_OUTPUT_TO_US', - 'sendrawtx exit 0' - ]) + l2.wait_for_onchaind_broadcast('OUR_DELAYED_RETURN_TO_WALLET', + 'OUR_UNILATERAL/DELAYED_OUTPUT_TO_US') # One more block, HTLC tx is now spendable. l1.bitcoin.generate_block(1) - l2.daemon.wait_for_logs([ - 'Broadcasting OUR_DELAYED_RETURN_TO_WALLET .* to resolve OUR_HTLC_SUCCESS_TX/DELAYED_OUTPUT_TO_US', - 'sendrawtx exit 0' - ]) + l2.wait_for_onchaind_broadcast('OUR_DELAYED_RETURN_TO_WALLET', + 'OUR_HTLC_SUCCESS_TX/DELAYED_OUTPUT_TO_US') # 100 blocks after last spend, l2 should be done. l1.bitcoin.generate_block(100) @@ -823,7 +820,8 @@ def test_onchain_feechange(node_factory, bitcoind, executor): l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/OUR_HTLC by OUR_HTLC_TIMEOUT_TO_US .* after 6 blocks') bitcoind.generate_block(6) - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_onchaind_broadcast('OUR_HTLC_TIMEOUT_TO_US', + 'THEIR_UNILATERAL/OUR_HTLC') # Make sure that gets included. @@ -904,9 +902,9 @@ def test_onchain_all_dust(node_factory, bitcoind, executor): l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/OUR_HTLC by IGNORING_TINY_PAYMENT .* after 6 blocks') bitcoind.generate_block(5) - l1.daemon.wait_for_logs(['Broadcasting IGNORING_TINY_PAYMENT .* to resolve THEIR_UNILATERAL/OUR_HTLC', - 'sendrawtx exit 0', - 'Ignoring output 0 of .*: THEIR_UNILATERAL/OUR_HTLC']) + l1.wait_for_onchaind_broadcast('IGNORING_TINY_PAYMENT', + 'THEIR_UNILATERAL/OUR_HTLC') + l1.daemon.wait_for_log('Ignoring output 0 of .*: THEIR_UNILATERAL/OUR_HTLC') # 100 deep and l2 forgets. bitcoind.generate_block(93) @@ -1009,7 +1007,9 @@ def test_permfail_new_commit(node_factory, bitcoind, executor): # OK, time out HTLC. bitcoind.generate_block(5) - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_onchaind_broadcast('OUR_HTLC_TIMEOUT_TO_US', + 'THEIR_UNILATERAL/OUR_HTLC') + bitcoind.generate_block(1) l1.daemon.wait_for_log('Resolved THEIR_UNILATERAL/OUR_HTLC by our proposal OUR_HTLC_TIMEOUT_TO_US') l2.daemon.wait_for_log('Ignoring output.*: OUR_UNILATERAL/THEIR_HTLC') @@ -1045,16 +1045,17 @@ def test_permfail_htlc_in(node_factory, bitcoind, executor): l2.daemon.wait_for_log('Propose handling OUR_UNILATERAL/THEIR_HTLC by THEIR_HTLC_TIMEOUT_TO_THEM \\(IGNORING\\) after 6 blocks') l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/OUR_HTLC by OUR_HTLC_TIMEOUT_TO_US (.*) after 6 blocks') # l2 then gets preimage, uses it instead of ignoring - l2.daemon.wait_for_log('Propose handling OUR_UNILATERAL/THEIR_HTLC by OUR_HTLC_SUCCESS_TX .* after 0 blocks') - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_onchaind_broadcast('OUR_HTLC_SUCCESS_TX', + 'OUR_UNILATERAL/THEIR_HTLC') bitcoind.generate_block(1) # OK, l1 sees l2 fulfill htlc. l1.daemon.wait_for_log('THEIR_UNILATERAL/OUR_HTLC gave us preimage') l2.daemon.wait_for_log('Propose handling OUR_HTLC_SUCCESS_TX/DELAYED_OUTPUT_TO_US by OUR_DELAYED_RETURN_TO_WALLET .* after 5 blocks') - bitcoind.generate_block(6) + bitcoind.generate_block(5) - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_onchaind_broadcast('OUR_DELAYED_RETURN_TO_WALLET', + 'OUR_HTLC_SUCCESS_TX/DELAYED_OUTPUT_TO_US') t.cancel() @@ -1094,8 +1095,8 @@ def test_permfail_htlc_out(node_factory, bitcoind, executor): l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/THEIR_HTLC by THEIR_HTLC_TIMEOUT_TO_THEM \\(IGNORING\\) after 6 blocks') # l1 then gets preimage, uses it instead of ignoring - l1.daemon.wait_for_log('Propose handling THEIR_UNILATERAL/THEIR_HTLC by THEIR_HTLC_FULFILL_TO_US .* after 0 blocks') - l1.daemon.wait_for_log('sendrawtx exit 0') + l1.wait_for_onchaind_broadcast('THEIR_HTLC_FULFILL_TO_US', + 'THEIR_UNILATERAL/THEIR_HTLC') # l2 sees l1 fulfill tx. bitcoind.generate_block(1) @@ -1105,8 +1106,8 @@ def test_permfail_htlc_out(node_factory, bitcoind, executor): # l2 can send OUR_DELAYED_RETURN_TO_WALLET after 3 more blocks. bitcoind.generate_block(3) - l2.daemon.wait_for_log('Broadcasting OUR_DELAYED_RETURN_TO_WALLET .* to resolve OUR_UNILATERAL/DELAYED_OUTPUT_TO_US') - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_onchaind_broadcast('OUR_DELAYED_RETURN_TO_WALLET', + 'OUR_UNILATERAL/DELAYED_OUTPUT_TO_US') # Now, 100 blocks they should be done. bitcoind.generate_block(95) @@ -1184,8 +1185,8 @@ def check_billboard(): wait_for(lambda: (closetxid, "confirmed") in set([(o['txid'], o['status']) for o in l1.rpc.listfunds()['outputs']])) # It should send the to-wallet tx. - l2.daemon.wait_for_log('Broadcasting OUR_DELAYED_RETURN_TO_WALLET') - l2.daemon.wait_for_log('sendrawtx exit 0') + l2.wait_for_onchaind_broadcast('OUR_DELAYED_RETURN_TO_WALLET', + 'OUR_UNILATERAL/DELAYED_OUTPUT_TO_US') # 100 after l1 sees tx, it should be done. bitcoind.generate_block(95) diff --git a/tests/utils.py b/tests/utils.py index 02387f4a77fc..24a36c0f3b45 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -632,6 +632,20 @@ def mock_estimatesmartfee(r): if wait_for_effect: wait_for(lambda: self.daemon.rpcproxy.mock_counts['estimatesmartfee'] >= 3) + def wait_for_onchaind_broadcast(self, name, resolve=None): + """Wait for onchaind to drop tx name to resolve (if any)""" + if resolve: + r = self.daemon.wait_for_log('Broadcasting {} .* to resolve {}' + .format(name, resolve)) + else: + r = self.daemon.wait_for_log('Broadcasting {} .* to resolve ' + .format(name)) + + rawtx = re.search('.* \(([0-9a-fA-F]*)\) ', r).group(1) + txid = self.bitcoin.rpc.decoderawtransaction(rawtx, True)['txid'] + + wait_for(lambda: txid in self.bitcoin.rpc.getrawmempool()) + class NodeFactory(object): """A factory to setup and start `lightningd` daemons. From 1b80cb42691715e775dc15926198ecce50c16b52 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 21 Sep 2018 17:19:33 +0930 Subject: [PATCH 07/27] Makefile: don't rebuild all the time The code to regenerate the local BOLT copy was causing eternal rebuild. So only build if it's wrong. Signed-off-by: Rusty Russell --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 4e730bf22b7b..10bcbc757912 100644 --- a/Makefile +++ b/Makefile @@ -243,12 +243,12 @@ check-makefile: # Any mention of BOLT# must be followed by an exact quote, modulo whitespace. bolt-check/%: % bolt-precheck tools/check-bolt - @[ ! -d .tmp.lightningrfc ] || tools/check-bolt .tmp.lightningrfc $< + @if [ -d .tmp.lightningrfc ]; then tools/check-bolt .tmp.lightningrfc $<; else Not checking BOLTs: BOLTDIR $(BOLTDIR) does not exist >&2; fi LOCAL_BOLTDIR=.tmp.lightningrfc bolt-precheck: - @rm -rf $(LOCAL_BOLTDIR); if [ ! -d $(BOLTDIR) ]; then echo Not checking BOLTs: BOLTDIR $(BOLTDIR) does not exist >&2; exit 0; fi; set -e; if [ -n "$(BOLTVERSION)" ]; then git clone -q $(BOLTDIR) $(LOCAL_BOLTDIR) && cd $(LOCAL_BOLTDIR) && git checkout -q $(BOLTVERSION); else cp -a $(BOLTDIR) $(LOCAL_BOLTDIR); fi + @[ -d $(BOLTDIR) ] || exit 0; set -e; if [ -z "$(BOLTVERSION)" ]; then rm -rf $(LOCAL_BOLTDIR); ln -sf $(BOLTDIR) $(LOCAL_BOLTDIR); exit 0; fi; [ "$$(git -C $(LOCAL_BOLTDIR) rev-list --max-count=1 HEAD 2>/dev/null)" != "$(BOLTVERSION)" ] || exit 0; rm -rf $(LOCAL_BOLTDIR) && git clone -q $(BOLTDIR) $(LOCAL_BOLTDIR) && cd $(LOCAL_BOLTDIR) && git checkout -q $(BOLTVERSION) check-source-bolt: $(ALL_TEST_PROGRAMS:%=bolt-check/%.c) From 0925daa0877fd8f23685d8b0855d37f0fedffa88 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 21 Sep 2018 09:59:44 +0930 Subject: [PATCH 08/27] gossipwith: simple tool to snarf gossip from a node. Signed-off-by: Rusty Russell --- devtools/.gitignore | 1 + devtools/Makefile | 11 ++- devtools/gossipwith.c | 220 ++++++++++++++++++++++++++++++++++++++++++ tests/test_gossip.py | 25 ++++- 4 files changed, 253 insertions(+), 4 deletions(-) create mode 100644 devtools/gossipwith.c diff --git a/devtools/.gitignore b/devtools/.gitignore index eb53afe43646..0019a5abf38b 100644 --- a/devtools/.gitignore +++ b/devtools/.gitignore @@ -2,3 +2,4 @@ bolt11-cli decodemsg onion dump-gossipstore +gossipwith diff --git a/devtools/Makefile b/devtools/Makefile index c2af6a2a7a38..e70db606b4f1 100644 --- a/devtools/Makefile +++ b/devtools/Makefile @@ -1,6 +1,7 @@ DEVTOOLS_SRC := devtools/gen_print_wire.c devtools/gen_print_onion_wire.c devtools/print_wire.c DEVTOOLS_OBJS := $(DEVTOOLS_SRC:.c=.o) -DEVTOOLS_TOOL_SRC := devtools/bolt11-cli.c devtools/decodemsg.c devtools/onion.c devtools/dump-gossipstore.c +DEVTOOLS := devtools/bolt11-cli devtools/decodemsg devtools/onion devtools/dump-gossipstore devtools/gossipwith +DEVTOOLS_TOOL_SRC := $(DEVTOOLS:=.c) DEVTOOLS_TOOL_OBJS := $(DEVTOOLS_TOOL_SRC:.c=.o) DEVTOOLS_COMMON_OBJS := \ @@ -15,7 +16,7 @@ DEVTOOLS_COMMON_OBJS := \ common/version.o \ common/wireaddr.o -devtools-all: devtools/bolt11-cli devtools/decodemsg devtools/onion devtools/dump-gossipstore +devtools-all: $(DEVTOOLS) devtools/gen_print_wire.h: $(WIRE_GEN) wire/gen_peer_wire_csv $(WIRE_GEN) --bolt --printwire --header $@ wire_type < wire/gen_peer_wire_csv > $@ @@ -40,12 +41,16 @@ devtools/onion.c: ccan/config.h devtools/onion: $(DEVTOOLS_OBJS) $(DEVTOOLS_COMMON_OBJS) $(JSMN_OBJS) $(CCAN_OBJS) $(BITCOIN_OBJS) wire/fromwire.o wire/towire.o devtools/onion.o common/sphinx.o +devtools/gossipwith: $(DEVTOOLS_OBJS) $(DEVTOOLS_COMMON_OBJS) $(JSMN_OBJS) $(CCAN_OBJS) $(BITCOIN_OBJS) wire/fromwire.o wire/towire.o wire/gen_peer_wire.o devtools/gossipwith.o common/cryptomsg.o common/cryptomsg.o common/crypto_sync.o + $(DEVTOOLS_OBJS) $(DEVTOOLS_TOOL_OBJS): wire/wire.h devtools/gen_print_wire.h devtools/gen_print_onion_wire.h devtools/gen_print_wire.o: devtools/gen_print_wire.h wire/gen_peer_wire.h devtools/print_wire.h devtools/gen_print_onion_wire.o: devtools/gen_print_onion_wire.h devtools/print_wire.h +devtools/bolt11-cli: $(DEVTOOLS_OBJS) $(DEVTOOLS_COMMON_OBJS) $(JSMN_OBJS) $(CCAN_OBJS) $(BITCOIN_OBJS) wire/fromwire.o wire/towire.o devtools/bolt11-cli.o + # Make sure these depend on everything. -ALL_PROGRAMS += devtools/bolt11-cli devtools/decodemsg devtools/onion devtools/dump-gossipstore +ALL_PROGRAMS += $(DEVTOOLS) ALL_OBJS += $(DEVTOOLS_OBJS) $(DEVTOOLS_TOOL_OBJS) check-source: $(DEVTOOLS_SRC:%=check-src-include-order/%) $(DEVTOOLS_TOOLS_SRC:%=check-src-include-order/%) diff --git a/devtools/gossipwith.c b/devtools/gossipwith.c new file mode 100644 index 000000000000..a970998145d0 --- /dev/null +++ b/devtools/gossipwith.c @@ -0,0 +1,220 @@ +/* Simple tool to route gossip from a peer. */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define io_write_ simple_write +#define io_read_ simple_read + +static struct io_plan *simple_write(struct io_conn *conn, + const void *data, size_t len, + struct io_plan *(*next)(struct io_conn *, void *), + void *arg); + +static struct io_plan *simple_read(struct io_conn *conn, + void *data, size_t len, + struct io_plan *(*next)(struct io_conn *, void *), + void *next_arg); + +#include "../connectd/handshake.c" + +/* This makes the handshake prototypes work. */ +struct io_conn { + int fd; +}; + +static struct secret notsosecret; +static bool initial_sync = false; +static unsigned long max_messages = -1UL; + +/* Empty stubs to make us compile */ +void status_peer_io(enum log_level iodir, const u8 *p) +{ +} + +void status_fmt(enum log_level level, const char *fmt, ...) +{ +} + +#if DEVELOPER +void dev_sabotage_fd(int fd) +{ + abort(); +} + +void dev_blackhole_fd(int fd) +{ + abort(); +} + +enum dev_disconnect dev_disconnect(int pkt_type) +{ + return DEV_DISCONNECT_NORMAL; +} +#endif + +void peer_failed_connection_lost(void) +{ + exit(0); +} + +bool hsm_do_ecdh(struct secret *ss, const struct pubkey *point) +{ + if (secp256k1_ecdh(secp256k1_ctx, ss->data, &point->pubkey, + notsosecret.data) != 1) + errx(1, "ECDH failed"); + return true; +} + +/* We don't want to discard *any* messages. */ +bool is_unknown_msg_discardable(const u8 *cursor) +{ + return false; +} + +static struct io_plan *simple_write(struct io_conn *conn, + const void *data, size_t len, + struct io_plan *(*next)(struct io_conn *, void *), + void *arg) +{ + if (!write_all(conn->fd, data, len)) + err(1, "Writing data"); + return next(conn, arg); +} + +static struct io_plan *simple_read(struct io_conn *conn, + void *data, size_t len, + struct io_plan *(*next)(struct io_conn *, void *), + void *next_arg) +{ + if (!read_all(conn->fd, data, len)) + err(1, "Reading data"); + return next(conn, next_arg); +} + +static struct io_plan *handshake_success(struct io_conn *conn, + const struct pubkey *them, + const struct wireaddr_internal *addr, + const struct crypto_state *orig_cs, + void *unused) +{ + u8 *msg; + struct crypto_state cs = *orig_cs; + u8 *local_features; + + if (initial_sync) { + local_features = tal(conn, u8); + local_features[0] = (1 << 3); + } else + local_features = NULL; + + msg = towire_init(NULL, NULL, local_features); + + sync_crypto_write(&cs, conn->fd, take(msg)); + /* Ignore their init message. */ + tal_free(sync_crypto_read(NULL, &cs, conn->fd)); + + /* Now write out whatever we get. */ + while ((msg = sync_crypto_read(NULL, &cs, conn->fd)) != NULL) { + be16 len = cpu_to_be16(tal_bytelen(msg)); + + if (!write_all(STDOUT_FILENO, &len, sizeof(len)) + || !write_all(STDOUT_FILENO, msg, tal_bytelen(msg))) + err(1, "Writing out msg"); + tal_free(msg); + + if (--max_messages == 0) + exit(0); + } + err(1, "Reading msg"); +} + +int main(int argc, char *argv[]) +{ + struct io_conn *conn = tal(NULL, struct io_conn); + struct wireaddr_internal addr; + int af; + struct pubkey us, them; + const char *err_msg; + const char *at; + struct addrinfo *ai; + + setup_locale(); + secp256k1_ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY | + SECP256K1_CONTEXT_SIGN); + + opt_register_noarg("--initial-sync", opt_set_bool, &initial_sync, + "Stream complete gossip history at start"); + opt_register_arg("--max-messages", opt_set_ulongval, opt_show_ulongval, + &max_messages, + "Terminate after reading this many messages (> 0)"); + opt_register_noarg("--help|-h", opt_usage_and_exit, + "id@addr[:port]\n" + "Connect to a lightning peer and relay gossip messages from it", + "Print this message."); + + opt_parse(&argc, argv, opt_log_stderr_exit); + if (argc != 2) + opt_usage_exit_fail("Need an id@addr to connect to"); + at = strchr(argv[1], '@'); + if (!at) + opt_usage_exit_fail("Need id@addr"); + + if (!pubkey_from_hexstr(argv[1], at - argv[1], &them)) + opt_usage_exit_fail("Invalid id %.*s", + (int)(at - argv[1]), argv[1]); + + if (!parse_wireaddr_internal(at+1, &addr, DEFAULT_PORT, NULL, + true, false, &err_msg)) + opt_usage_exit_fail("%s '%s'", err_msg, argv[1]); + + switch (addr.itype) { + case ADDR_INTERNAL_SOCKNAME: + af = AF_LOCAL; + ai = wireaddr_internal_to_addrinfo(conn, &addr); + break; + case ADDR_INTERNAL_ALLPROTO: + case ADDR_INTERNAL_AUTOTOR: + case ADDR_INTERNAL_FORPROXY: + opt_usage_exit_fail("Don't support proxy use"); + + case ADDR_INTERNAL_WIREADDR: + switch (addr.u.wireaddr.type) { + case ADDR_TYPE_TOR_V2: + case ADDR_TYPE_TOR_V3: + opt_usage_exit_fail("Don't support proxy use"); + break; + case ADDR_TYPE_IPV4: + af = AF_INET; + break; + case ADDR_TYPE_IPV6: + af = AF_INET6; + break; + case ADDR_TYPE_PADDING: + abort(); + } + ai = wireaddr_to_addrinfo(tmpctx, &addr.u.wireaddr); + } + conn->fd = socket(af, SOCK_STREAM, 0); + if (conn->fd < 0) + err(1, "Creating socket"); + + memset(¬sosecret, 0x42, sizeof(notsosecret)); + if (!pubkey_from_secret(¬sosecret, &us)) + errx(1, "Creating pubkey"); + + if (connect(conn->fd, ai->ai_addr, ai->ai_addrlen) != 0) + err(1, "Connecting to %s", at+1); + + initiator_handshake(conn, &us, &them, &addr, handshake_success, NULL); + exit(0); +} + diff --git a/tests/test_gossip.py b/tests/test_gossip.py index ce53d1ac3abe..98c40f4421a3 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -1,9 +1,10 @@ from fixtures import * # noqa: F401,F403 -from utils import wait_for +from utils import wait_for, TIMEOUT import json import logging import os +import struct import subprocess import time import unittest @@ -839,3 +840,25 @@ def test_gossip_store_load(node_factory): # May preceed the Started msg waited for in 'start'. wait_for(lambda: l1.daemon.is_in_log('gossip_store: Read 1/1/1/0 cannounce/cupdate/nannounce/cdelete from store in 744 bytes')) assert not l1.daemon.is_in_log('gossip_store.*truncating') + + +def test_gossipwith(node_factory): + l1, l2 = node_factory.line_graph(2, announce=True) + + out = subprocess.run(['devtools/gossipwith', + '--initial-sync', + '--max-messages=5', + '{}@localhost:{}'.format(l1.info['id'], l1.port)], + check=True, + timeout=TIMEOUT, stdout=subprocess.PIPE).stdout + + num_msgs = 0 + while len(out): + l, t = struct.unpack('>HH', out[0:4]) + # channel_announcement node_announcement or channel_update + assert t == 256 or t == 257 or t == 258 + out = out[2 + l:] + num_msgs += 1 + + # one channel announcement, two channel_updates, two node announcements. + assert num_msgs == 5 From 47f5bc4deb79b146afe88d920671c7cf56e0a5bc Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 21 Sep 2018 09:59:48 +0930 Subject: [PATCH 09/27] gossipwith: add ability to send message. Just cmdline for now, rather than a proper stdin io loop. Signed-off-by: Rusty Russell --- devtools/gossipwith.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/devtools/gossipwith.c b/devtools/gossipwith.c index a970998145d0..1cb2e3563a09 100644 --- a/devtools/gossipwith.c +++ b/devtools/gossipwith.c @@ -104,7 +104,7 @@ static struct io_plan *handshake_success(struct io_conn *conn, const struct pubkey *them, const struct wireaddr_internal *addr, const struct crypto_state *orig_cs, - void *unused) + char **args) { u8 *msg; struct crypto_state cs = *orig_cs; @@ -122,6 +122,15 @@ static struct io_plan *handshake_success(struct io_conn *conn, /* Ignore their init message. */ tal_free(sync_crypto_read(NULL, &cs, conn->fd)); + /* Did they ask us to send any messages? Do so now. */ + while (*args) { + u8 *m = tal_hexdata(NULL, *args, strlen(*args)); + if (!m) + errx(1, "Invalid hexdata '%s'", *args); + sync_crypto_write(&cs, conn->fd, take(m)); + args++; + } + /* Now write out whatever we get. */ while ((msg = sync_crypto_read(NULL, &cs, conn->fd)) != NULL) { be16 len = cpu_to_be16(tal_bytelen(msg)); @@ -157,12 +166,12 @@ int main(int argc, char *argv[]) &max_messages, "Terminate after reading this many messages (> 0)"); opt_register_noarg("--help|-h", opt_usage_and_exit, - "id@addr[:port]\n" + "id@addr[:port] [hex-msg-tosend...]\n" "Connect to a lightning peer and relay gossip messages from it", "Print this message."); opt_parse(&argc, argv, opt_log_stderr_exit); - if (argc != 2) + if (argc < 2) opt_usage_exit_fail("Need an id@addr to connect to"); at = strchr(argv[1], '@'); if (!at) @@ -214,7 +223,7 @@ int main(int argc, char *argv[]) if (connect(conn->fd, ai->ai_addr, ai->ai_addrlen) != 0) err(1, "Connecting to %s", at+1); - initiator_handshake(conn, &us, &them, &addr, handshake_success, NULL); + initiator_handshake(conn, &us, &them, &addr, handshake_success, argv+2); exit(0); } From d16c3dcdc7d92fe62a7ca53e299acc0a8fdf8d9b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 21 Sep 2018 09:59:48 +0930 Subject: [PATCH 10/27] devtools/decodemsg: take series of msgs from stdin. Useful in combination with gossipwith. Signed-off-by: Rusty Russell --- devtools/decodemsg.c | 53 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/devtools/decodemsg.c b/devtools/decodemsg.c index f5bdf0c96c02..4c332d716978 100644 --- a/devtools/decodemsg.c +++ b/devtools/decodemsg.c @@ -1,10 +1,12 @@ #include #include +#include #include #include #include #include #include +#include int main(int argc, char *argv[]) { @@ -15,22 +17,49 @@ int main(int argc, char *argv[]) opt_register_noarg("--onion", opt_set_bool, &onion, "Decode an error message instead of a peer message"); opt_register_noarg("--help|-h", opt_usage_and_exit, - "" - "Decode a lightning spec wire message from hex.", + "[]" + "Decode a lightning spec wire message from hex, or a series of messages from stdin", "Print this message."); opt_parse(&argc, argv, opt_log_stderr_exit); - if (argc != 2) - errx(1, "Need a hex message"); + if (argc > 2) + opt_usage_and_exit("Too many arguments"); - /* Arg is hex string */ - m = tal_hexdata(NULL, argv[1], strlen(argv[1])); - if (!m) - errx(1, "'%s' is not valid hex", argv[1]); + if (argc == 2) { + /* Arg is hex string */ + m = tal_hexdata(NULL, argv[1], strlen(argv[1])); + if (!m) + errx(1, "'%s' is not valid hex", argv[1]); - if (onion) - printonion_type_message(m); - else - printwire_type_message(m); + if (onion) + printonion_type_message(m); + else + printwire_type_message(m); + } else { + u8 *f = grab_fd(NULL, STDIN_FILENO); + size_t off = 0; + + while (off != tal_count(f)) { + be16 len; + + if (off + sizeof(len) > tal_count(f)) { + warnx("Truncated file"); + break; + } + memcpy(&len, f + off, sizeof(len)); + off += sizeof(len); + if (off + be16_to_cpu(len) > tal_count(f)) { + warnx("Truncated file"); + break; + } + m = tal_dup_arr(f, u8, f + off, be16_to_cpu(len), 0); + if (onion) + printonion_type_message(m); + else + printwire_type_message(m); + off += be16_to_cpu(len); + tal_free(m); + } + } return 0; } From 6228c25643c41218f0d265196fc7f18e096fbab2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 21 Sep 2018 09:59:48 +0930 Subject: [PATCH 11/27] test_gossip: basic 'node notices close' test. Signed-off-by: Rusty Russell --- tests/test_gossip.py | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 98c40f4421a3..b3c3706f986b 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -1,5 +1,5 @@ from fixtures import * # noqa: F401,F403 -from utils import wait_for, TIMEOUT +from utils import wait_for, TIMEOUT, only_one import json import logging @@ -862,3 +862,28 @@ def test_gossipwith(node_factory): # one channel announcement, two channel_updates, two node announcements. assert num_msgs == 5 + + +def test_gossip_notices_close(node_factory, bitcoind): + l1 = node_factory.get_node() + l2, l3 = node_factory.line_graph(2) + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + + bitcoind.generate_block(5) + + # Make sure l1 learns about channel. + wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 2) + wait_for(lambda: len(l1.rpc.listnodes()['nodes']) == 2) + l1.rpc.disconnect(l2.info['id']) + + l2.rpc.close(l3.info['id']) + wait_for(lambda: only_one(l2.rpc.listpeers(l3.info['id'])['peers'])['channels'][0]['state'] == 'CLOSINGD_COMPLETE') + bitcoind.generate_block(1) + + wait_for(lambda: l1.rpc.listchannels()['channels'] == []) + wait_for(lambda: l1.rpc.listnodes()['nodes'] == []) + + l1.stop() + l1.start() + assert(l1.rpc.listchannels()['channels'] == []) + assert(l1.rpc.listnodes()['nodes'] == []) From 24c386c086e60e820cdcb021e38ae5d1ffb9fccf Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 21 Sep 2018 09:59:48 +0930 Subject: [PATCH 12/27] test_gossip: gossip retransmit on spent UTXO test. Aka #1934. Signed-off-by: Rusty Russell --- tests/test_gossip.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/test_gossip.py b/tests/test_gossip.py index b3c3706f986b..9b24e1b6cbe7 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -4,6 +4,7 @@ import json import logging import os +import pytest import struct import subprocess import time @@ -864,10 +865,15 @@ def test_gossipwith(node_factory): assert num_msgs == 5 +@pytest.mark.xfail(strict=True) def test_gossip_notices_close(node_factory, bitcoind): - l1 = node_factory.get_node() + # We want IO logging so we can replay a channel_announce to l1. + l1 = node_factory.get_node(options={'log-level': 'io'}) l2, l3 = node_factory.line_graph(2) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + # FIXME: sending SIGUSR1 immediately may kill it before handler installed. + l1.daemon.wait_for_log('Handed peer, entering loop') + subprocess.run(['kill', '-USR1', l1.subd_pid('openingd')]) bitcoind.generate_block(5) @@ -876,6 +882,11 @@ def test_gossip_notices_close(node_factory, bitcoind): wait_for(lambda: len(l1.rpc.listnodes()['nodes']) == 2) l1.rpc.disconnect(l2.info['id']) + # Grab channel_announcement from io logs (ends in ') + channel_announcement = l1.daemon.is_in_log('\[IN\] 0100').split(' ')[-1][:-1] + channel_update = l1.daemon.is_in_log('\[IN\] 0102').split(' ')[-1][:-1] + node_announcement = l1.daemon.is_in_log('\[IN\] 0101').split(' ')[-1][:-1] + l2.rpc.close(l3.info['id']) wait_for(lambda: only_one(l2.rpc.listpeers(l3.info['id'])['peers'])['channels'][0]['state'] == 'CLOSINGD_COMPLETE') bitcoind.generate_block(1) @@ -883,6 +894,22 @@ def test_gossip_notices_close(node_factory, bitcoind): wait_for(lambda: l1.rpc.listchannels()['channels'] == []) wait_for(lambda: l1.rpc.listnodes()['nodes'] == []) + # FIXME: This is a hack: we should have a framework for canned conversations + # This doesn't naturally terminate, so we give it 5 seconds. + try: + subprocess.run(['devtools/gossipwith', + '{}@localhost:{}'.format(l1.info['id'], l1.port), + channel_announcement, + channel_update, + node_announcement], + timeout=5, stdout=subprocess.PIPE) + except subprocess.TimeoutExpired: + pass + + # l1 should reject it. + assert(l1.rpc.listchannels()['channels'] == []) + assert(l1.rpc.listnodes()['nodes'] == []) + l1.stop() l1.start() assert(l1.rpc.listchannels()['channels'] == []) From 0855422110845f74b3ddf0ff2ebc898317db76f3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 21 Sep 2018 10:01:30 +0930 Subject: [PATCH 13/27] gossip_control: when searching for a txout, make sure it's not spent! There's no reason for the db to ever return non-NULL if it's spent. And there's only one caller, for which that is definitely true. Suggested-by: @cdecker Fixes: #1934 Signed-off-by: Rusty Russell --- tests/test_gossip.py | 2 -- wallet/wallet.c | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 9b24e1b6cbe7..5437b64c58e4 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -4,7 +4,6 @@ import json import logging import os -import pytest import struct import subprocess import time @@ -865,7 +864,6 @@ def test_gossipwith(node_factory): assert num_msgs == 5 -@pytest.mark.xfail(strict=True) def test_gossip_notices_close(node_factory, bitcoind): # We want IO logging so we can replay a channel_announce to l1. l1 = node_factory.get_node(options={'log-level': 'io'}) diff --git a/wallet/wallet.c b/wallet/wallet.c index 23b94020f03a..9f2216811e2a 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -2139,7 +2139,8 @@ struct outpoint *wallet_outpoint_for_scid(struct wallet *w, tal_t *ctx, "FROM utxoset " "WHERE blockheight = ?" " AND txindex = ?" - " AND outnum = ?"); + " AND outnum = ?" + " AND spendheight IS NULL"); sqlite3_bind_int(stmt, 1, short_channel_id_blocknum(scid)); sqlite3_bind_int(stmt, 2, short_channel_id_txnum(scid)); sqlite3_bind_int(stmt, 3, short_channel_id_outnum(scid)); From 48de77d56e5c99f950f1aaf448ac6315fb0694c9 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 21 Sep 2018 10:01:45 +0930 Subject: [PATCH 14/27] gossipd: invalidate old gossip_stores. Incrementing version number means stores which were prior to the previous commit will be removed, and refreshed. The simplest fix, if not the most efficient. Signed-off-by: Rusty Russell --- gossipd/gossip_store.h | 2 +- tests/test_gossip.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gossipd/gossip_store.h b/gossipd/gossip_store.h index 6a78d2ca9d7c..03406fcf65c1 100644 --- a/gossipd/gossip_store.h +++ b/gossipd/gossip_store.h @@ -11,7 +11,7 @@ /** * gossip_store -- On-disk storage related information */ -#define GOSSIP_STORE_VERSION 2 +#define GOSSIP_STORE_VERSION 3 struct gossip_store; struct routing_state; diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 5437b64c58e4..cb71503cf2e3 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -822,7 +822,7 @@ def test_gossip_store_load(node_factory): """Make sure we can read canned gossip store""" l1 = node_factory.get_node(start=False) with open(os.path.join(l1.daemon.lightning_dir, 'gossip_store'), 'wb') as f: - f.write(bytearray.fromhex("02" # GOSSIP_VERSION + f.write(bytearray.fromhex("03" # GOSSIP_VERSION "00000099" # len "12abbbba" # csum "1002" # WIRE_GOSSIP_STORE_NODE_ANNOUNCEMENT From 8455b12781bd64d6025af562d3fc52f1c4b1799c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 21 Sep 2018 10:01:45 +0930 Subject: [PATCH 15/27] Revert "gossipd: handle premature node_announcements in the store." This reverts commit e2f426903d653e310e483a610dd1d0015e536c9c. With the new store version, this can't happen. Signed-off-by: Rusty Russell --- gossipd/gossip_store.c | 31 ++++++------------------------- gossipd/routing.c | 16 ++++------------ gossipd/routing.h | 6 +----- tests/test_gossip.py | 10 +++++----- 4 files changed, 16 insertions(+), 47 deletions(-) diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c index be25ecd65d31..2e070e69ffba 100644 --- a/gossipd/gossip_store.c +++ b/gossipd/gossip_store.c @@ -251,12 +251,9 @@ void gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) /* We set/check version byte on creation */ off_t known_good = 1; const char *bad; - size_t stats[] = {0, 0, 0, 0, 0}; + size_t stats[] = {0, 0, 0, 0}; int fd = gs->fd; gs->fd = -1; - bool unknown_node; - size_t num_delayed_na = 0; - u8 **delayed_na = tal_arr(tmpctx, u8 *, num_delayed_na); if (lseek(fd, known_good, SEEK_SET) < 0) { status_unusual("gossip_store: lseek failure"); @@ -297,18 +294,11 @@ void gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) stats[1]++; } else if (fromwire_gossip_store_node_announcement(msg, msg, &gossip_msg)) { - if (!routing_add_node_announcement(rstate, gossip_msg, - &unknown_node)) { - if (!unknown_node) { - bad = "Bad node_announcement"; - goto truncate; - } - /* Defer until later. */ - tal_resize(&delayed_na, num_delayed_na+1); - delayed_na[num_delayed_na++] - = tal_steal(delayed_na, gossip_msg); - } else - stats[2]++; + if (!routing_add_node_announcement(rstate, gossip_msg)) { + bad = "Bad node_announcement"; + goto truncate; + } + stats[2]++; } else if (fromwire_gossip_store_channel_delete(msg, &scid)) { struct chan *c = get_channel(rstate, &scid); if (!c) { @@ -328,15 +318,6 @@ void gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) gs->count++; tal_free(msg); } - - for (size_t i = 0; i < tal_count(delayed_na); i++) { - if (routing_add_node_announcement(rstate, delayed_na[i], NULL)) { - stats[2]++; - stats[4]++; - } - } - status_trace("Successfully processed %zu/%zu unknown node_announcement", - stats[4], tal_count(delayed_na)); goto out; truncate: diff --git a/gossipd/routing.c b/gossipd/routing.c index 67fed194a940..05e5eb7f871a 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -1285,9 +1285,7 @@ static struct wireaddr *read_addresses(const tal_t *ctx, const u8 *ser) return wireaddrs; } -bool routing_add_node_announcement(struct routing_state *rstate, - const u8 *msg TAKES, - bool *unknown_node) +bool routing_add_node_announcement(struct routing_state *rstate, const u8 *msg TAKES) { struct node *node; secp256k1_ecdsa_signature signature; @@ -1301,21 +1299,15 @@ bool routing_add_node_announcement(struct routing_state *rstate, if (!fromwire_node_announcement(tmpctx, msg, &signature, &features, ×tamp, &node_id, rgb_color, alias, - &addresses)) { - if (unknown_node) - *unknown_node = false; + &addresses)) return false; - } node = get_node(rstate, &node_id); /* May happen if we accepted the node_announcement due to a local * channel, for which we didn't have the announcement yet. */ - if (node == NULL) { - if (unknown_node) - *unknown_node = true; + if (node == NULL) return false; - } wireaddrs = read_addresses(tmpctx, addresses); tal_free(node->addresses); @@ -1468,7 +1460,7 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann) status_trace("Received node_announcement for node %s", type_to_string(tmpctx, struct pubkey, &node_id)); - applied = routing_add_node_announcement(rstate, serialized, NULL); + applied = routing_add_node_announcement(rstate, serialized); assert(applied); return NULL; } diff --git a/gossipd/routing.h b/gossipd/routing.h index 6fb15d941a2e..349d43e9c916 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -312,13 +312,9 @@ bool routing_add_channel_update(struct routing_state *rstate, * Directly add the node being announced to the network view, without verifying * it. This must be from a trusted source, e.g., gossip_store. For untrusted * sources (peers) please use @see{handle_node_announcement}. - * - * Populates *unknown_node if it isn't NULL and this returns false to indicate - * if failure was due to an unknown node_id. */ bool routing_add_node_announcement(struct routing_state *rstate, - const u8 *msg TAKES, - bool *unknown_node); + const u8 *msg TAKES); /** diff --git a/tests/test_gossip.py b/tests/test_gossip.py index cb71503cf2e3..12287a858ea5 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -823,10 +823,6 @@ def test_gossip_store_load(node_factory): l1 = node_factory.get_node(start=False) with open(os.path.join(l1.daemon.lightning_dir, 'gossip_store'), 'wb') as f: f.write(bytearray.fromhex("03" # GOSSIP_VERSION - "00000099" # len - "12abbbba" # csum - "1002" # WIRE_GOSSIP_STORE_NODE_ANNOUNCEMENT - "00950101cf5d870bc7ecabcb7cd16898ef66891e5f0c6c5851bd85b670f03d325bc44d7544d367cd852e18ec03f7f4ff369b06860a3b12b07b29f36fb318ca11348bf8ec00005aab817c03f113414ebdc6c1fb0f33c99cd5a1d09dd79e7fdf2468cf1fe1af6674361695d23974b250757a7a6c6549544300000000000000000000000000000000000000000000000007010566933e2607" "000001bc" # len "521ef598" # csum "1000" # WIRE_GOSSIP_STORE_CHANNEL_ANNOUNCEMENT @@ -834,7 +830,11 @@ def test_gossip_store_load(node_factory): "00000086" # len "88c703c8" # csum "1001" # WIRE_GOSSIP_STORE_CHANNEL_UPDATE - "008201021ea7c2eadf8a29eb8690511a519b5656e29aa0a853771c4e38e65c5abf43d907295a915e69e451f4c7a0c3dc13dd943cfbe3ae88c0b96667cd7d58955dbfedcf43497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea33090000000013a63c0000b500015b8d9b440000009000000000000003e8000003e800000001")) + "008201021ea7c2eadf8a29eb8690511a519b5656e29aa0a853771c4e38e65c5abf43d907295a915e69e451f4c7a0c3dc13dd943cfbe3ae88c0b96667cd7d58955dbfedcf43497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea33090000000013a63c0000b500015b8d9b440000009000000000000003e8000003e800000001" + "00000099" # len + "12abbbba" # csum + "1002" # WIRE_GOSSIP_STORE_NODE_ANNOUNCEMENT + "00950101cf5d870bc7ecabcb7cd16898ef66891e5f0c6c5851bd85b670f03d325bc44d7544d367cd852e18ec03f7f4ff369b06860a3b12b07b29f36fb318ca11348bf8ec00005aab817c03f113414ebdc6c1fb0f33c99cd5a1d09dd79e7fdf2468cf1fe1af6674361695d23974b250757a7a6c6549544300000000000000000000000000000000000000000000000007010566933e2607")) l1.start() # May preceed the Started msg waited for in 'start'. From 3ce53ab9eddd397d57b6afc5faefe6703e56ac26 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Fri, 21 Sep 2018 17:35:16 -0700 Subject: [PATCH 16/27] tools/gen-wire: 3th -> 3rd Small grammar fix in wire gen tool --- tools/generate-wire.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index e2801c5ad6bc..04e68f4bdc38 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -401,7 +401,7 @@ def print_fromwire(self, is_header): .format(f.name, f.name, f.fieldtype.name, basetype, f.name)) elif f.is_assignable(): - subcalls.append("//3th case {name}".format(name=f.name)) + subcalls.append("//3rd case {name}".format(name=f.name)) if f.is_len_var: subcalls.append('{} = fromwire_{}(&cursor, &plen);' .format(f.name, basetype)) From fc12f65a3d9733e9a6649f7aa557b155e1cb1e61 Mon Sep 17 00:00:00 2001 From: Rene Pickhardt Date: Tue, 25 Sep 2018 07:21:00 +0200 Subject: [PATCH 17/27] Invoiceapidoc patch Added the fallback address to the API documentation of the invoice command --- lightningd/invoice.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 31d746b1b114..b82a801402dc 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -267,7 +267,8 @@ static void json_invoice(struct command *cmd, static const struct json_command invoice_command = { "invoice", json_invoice, "Create an invoice for {msatoshi} with {label} " "and {description} with optional {expiry} seconds " - "(default 1 hour) and optional {preimage} " + "(default 1 hour), optional {fallbacks} address list" + "(default empty list) and optional {preimage} " "(default autogenerated)"}; AUTODATA(json_command, &invoice_command); From c3f433ec66d74dbd9eff7ed43409678def34a041 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Tue, 25 Sep 2018 07:47:23 +0200 Subject: [PATCH 18/27] json: Support streaming JSON messages It turns out we were heavily relying on the fact that after each message from the client there'd be a flush, and that there would not be anything after the JSON object we read. This will no longer be the case once we start streaming things or we are very quick in issuing the JSON-RPC requests. This just takes one of the error paths (incomplete read) and makes it into a successful path if we have indeed read a full root element. --- common/json.c | 19 +++++++++++++++---- common/test/run-json.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/common/json.c b/common/json.c index d1abe52f022e..f741f796245d 100644 --- a/common/json.c +++ b/common/json.c @@ -188,23 +188,34 @@ jsmntok_t *json_parse_input(const char *input, int len, bool *valid) int ret; toks = tal_arr(input, jsmntok_t, 10); + toks[0].type = JSMN_UNDEFINED; -again: jsmn_init(&parser); +again: ret = jsmn_parse(&parser, input, len, toks, tal_count(toks) - 1); switch (ret) { case JSMN_ERROR_INVAL: *valid = false; return tal_free(toks); - case JSMN_ERROR_PART: - *valid = true; - return tal_free(toks); case JSMN_ERROR_NOMEM: tal_resize(&toks, tal_count(toks) * 2); goto again; } + /* Check whether we read at least one full root element, i.e., root + * element has its end set. */ + if (toks[0].type == JSMN_UNDEFINED || toks[0].end == -1) { + *valid = true; + return tal_free(toks); + } + + /* If we read a partial element at the end of the stream we'll get a + * ret=JSMN_ERROR_PART, but due to the previous check we know we read at + * least one full element, so count tokens that are part of this root + * element. */ + ret = json_next(toks) - toks; + /* Cut to length and return. */ *valid = true; tal_resize(&toks, ret + 1); diff --git a/common/test/run-json.c b/common/test/run-json.c index 956fdda35125..d1b855ed4b33 100644 --- a/common/test/run-json.c +++ b/common/test/run-json.c @@ -145,6 +145,37 @@ static void test_json_partial(void) tal_free(ctx); } +/* Test that we can segment and parse a stream of json objects correctly. */ +static void test_json_stream(void) +{ + bool valid; + char *input, *talstr; + jsmntok_t *toks; + + /* Multiple full messages in a single buffer (happens when buffer + * boundary coincides with message boundary, or read returned after + * timeout. */ + input = "{\"x\":\"x\"}{\"y\":\"y\"}"; + talstr = tal_strndup(NULL, input, strlen(input)); + toks = json_parse_input(talstr, strlen(talstr), &valid); + assert(toks); + assert(tal_count(toks) == 4); + assert(toks[0].start == 0 && toks[0].end == 9); + assert(valid); + tal_free(talstr); + + /* Multiple messages, and the last one is partial, far more likely than + * accidentally getting the boundaries to match. */ + input = "{\"x\":\"x\"}{\"y\":\"y\"}{\"z\":\"z"; + talstr = tal_strndup(NULL, input, strlen(input)); + toks = json_parse_input(talstr, strlen(talstr), &valid); + assert(toks); + assert(tal_count(toks) == 4); + assert(toks[0].start == 0 && toks[0].end == 9); + assert(valid); + tal_free(talstr); +} + int main(void) { setup_locale(); @@ -153,6 +184,7 @@ int main(void) test_json_filter(); test_json_escape(); test_json_partial(); + test_json_stream(); assert(!taken_any()); take_cleanup(); } From d91b94a81211e4c82e6be22bb13be15559263371 Mon Sep 17 00:00:00 2001 From: Mark Beckwith Date: Wed, 12 Sep 2018 10:56:39 -0500 Subject: [PATCH 19/27] param: add command mode Added the concept of a "command mode". The behavior of param() changes based on the mode. Added and tested the command mode of CMD_USAGE for setting the usage of a command without running it. Only infrastructure and test. No functional changes. Signed-off-by: Mark Beckwith --- lightningd/jsonrpc.c | 1 + lightningd/jsonrpc.h | 12 ++++++++ lightningd/param.c | 20 +++++++++++++ lightningd/test/run-param.c | 56 +++++++++++++++++++++++++++++++++++-- 4 files changed, 86 insertions(+), 3 deletions(-) diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 558e581bd13b..06f6e4a86f3d 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -442,6 +442,7 @@ static void parse_request(struct json_connection *jcon, const jsmntok_t tok[]) c->id = tal_strndup(c, json_tok_contents(jcon->buffer, id), json_tok_len(id)); + c->mode = CMD_NORMAL; list_add(&jcon->commands, &c->list); tal_add_destructor(c, destroy_cmd); diff --git a/lightningd/jsonrpc.h b/lightningd/jsonrpc.h index 9f36b8010c13..e155258a281c 100644 --- a/lightningd/jsonrpc.h +++ b/lightningd/jsonrpc.h @@ -10,6 +10,14 @@ struct bitcoin_txid; struct wireaddr; struct wallet_tx; +/* The command mode tells param() how to process. */ +enum command_mode { + /* Normal command processing */ + CMD_NORMAL, + /* Create command usage string, nothing else. */ + CMD_USAGE +}; + /* Context for a command (from JSON, but might outlive the connection!) * You can allocate off this for temporary objects. */ struct command { @@ -25,6 +33,10 @@ struct command { struct json_connection *jcon; /* Have we been marked by command_still_pending? For debugging... */ bool pending; + /* Tell param() how to process the command */ + enum command_mode mode; + /* This is created if mode is CMD_USAGE */ + const char *usage; }; struct json_connection { diff --git a/lightningd/param.c b/lightningd/param.c index a20c00938a7a..c1480d7a5a1d 100644 --- a/lightningd/param.c +++ b/lightningd/param.c @@ -223,6 +223,21 @@ static bool check_params(struct param *params) } #endif +static char *param_usage(const tal_t *ctx, + const struct param *params) +{ + char *usage = tal_strdup(ctx, ""); + for (size_t i = 0; i < tal_count(params); i++) { + if (i != 0) + tal_append_fmt(&usage, " "); + if (params[i].required) + tal_append_fmt(&usage, "%s", params[i].name); + else + tal_append_fmt(&usage, "[%s]", params[i].name); + } + return usage; +} + static bool param_arr(struct command *cmd, const char *buffer, const jsmntok_t tokens[], struct param *params) @@ -263,5 +278,10 @@ bool param(struct command *cmd, const char *buffer, } va_end(ap); + if (cmd->mode == CMD_USAGE) { + cmd->usage = param_usage(cmd, params); + return false; + } + return param_arr(cmd, buffer, tokens, params); } diff --git a/lightningd/test/run-param.c b/lightningd/test/run-param.c index 033ec56dd870..7b069deb0308 100644 --- a/lightningd/test/run-param.c +++ b/lightningd/test/run-param.c @@ -431,7 +431,7 @@ static void advanced(void) p_opt("msat_opt2", json_tok_msat, &msat_opt2), NULL)); assert(label != NULL); - assert(!strcmp(label->s, "lightning")); + assert(streq(label->s, "lightning")); assert(*msat == 24); assert(tok); assert(msat_opt1); @@ -445,8 +445,8 @@ static void advanced(void) p_req("label", json_tok_label, &label), p_opt("foo", json_tok_label, &foo), NULL)); - assert(!strcmp(label->s, "3")); - assert(!strcmp(foo->s, "foo")); + assert(streq(label->s, "3")); + assert(streq(foo->s, "foo")); } { u64 *msat; @@ -507,6 +507,54 @@ static void json_tok_tests(void) test_cb(json_tok_percent, double, "[ 'wow' ]", 0, false); } +static void test_invoice(struct command *cmd, const char *buffer, + const jsmntok_t *params) +{ + u64 *msatoshi_val; + struct json_escaped *label_val; + const char *desc_val; + u64 *expiry; + const jsmntok_t *fallbacks; + const jsmntok_t *preimagetok; + + assert(cmd->mode == CMD_USAGE); + if(!param(cmd, buffer, params, + p_req("msatoshi", json_tok_msat, &msatoshi_val), + p_req("label", json_tok_label, &label_val), + p_req("description", json_tok_escaped_string, &desc_val), + p_opt("expiry", json_tok_u64, &expiry), + p_opt("fallbacks", json_tok_array, &fallbacks), + p_opt("preimage", json_tok_tok, &preimagetok), NULL)) + return; + + /* should not be here since we are in the mode of CMD_USAGE + * and it always returns false. */ + abort(); +} + +static void usage(void) +{ + /* Do this to simulate a call to our pretend handler (test_invoice) */ + struct json_command invoice_command = { + "invoice", + test_invoice, + "", + false, + "" + }; + + cmd->mode = CMD_USAGE; + cmd->json_cmd = &invoice_command; + + cmd->json_cmd->dispatch(cmd, NULL, NULL); + + assert(streq(cmd->usage, + "msatoshi label description " + "[expiry] [fallbacks] [preimage]")); + + cmd->mode = CMD_NORMAL; +} + int main(void) { setup_locale(); @@ -528,6 +576,8 @@ int main(void) advanced(); advanced_fail(); json_tok_tests(); + usage(); + tal_free(tmpctx); printf("run-params ok\n"); } From 30b67c03343deb4b7e5463c0cefa12ed94076c7b Mon Sep 17 00:00:00 2001 From: Mark Beckwith Date: Fri, 14 Sep 2018 09:51:04 -0500 Subject: [PATCH 20/27] param: call param() all the time Now call param() even for commands that don't accept any parameters. This is a bugfix of sorts. For example, before you could call: bitcoin-cli getinfo blah and the blah parameter would be ignored. Now you will get an error: "too many parameters: got 1, expected 0" Signed-off-by: Mark Beckwith --- lightningd/jsonrpc.c | 9 +++++++++ lightningd/memdump.c | 7 +++++++ lightningd/test/run-param.c | 12 ++++++++++++ wallet/walletrpc.c | 8 ++++++++ 4 files changed, 36 insertions(+) diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 06f6e4a86f3d..a7f78a8bd3e8 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -78,6 +78,9 @@ static void json_stop(struct command *cmd, { struct json_result *response = new_json_result(cmd); + if (!param(cmd, buffer, params, NULL)) + return; + /* This can't have closed yet! */ cmd->jcon->stop = true; json_add_string(response, NULL, "Shutting down"); @@ -121,6 +124,9 @@ AUTODATA(json_command, &dev_rhash_command); static void json_crash(struct command *cmd UNUSED, const char *buffer UNUSED, const jsmntok_t *params UNUSED) { + if (!param(cmd, buffer, params, NULL)) + return; + fatal("Crash at user request"); } @@ -137,6 +143,9 @@ static void json_getinfo(struct command *cmd, { struct json_result *response = new_json_result(cmd); + if (!param(cmd, buffer, params, NULL)) + return; + json_object_start(response, NULL); json_add_pubkey(response, "id", &cmd->ld->id); json_add_string(response, "alias", (const char *)cmd->ld->alias); diff --git a/lightningd/memdump.c b/lightningd/memdump.c index 5e9914c00bf4..e8b9f6bb1055 100644 --- a/lightningd/memdump.c +++ b/lightningd/memdump.c @@ -10,6 +10,7 @@ #include #include #include +#include #include static void json_add_ptr(struct json_result *response, const char *name, @@ -57,6 +58,9 @@ static void json_memdump(struct command *cmd, { struct json_result *response = new_json_result(cmd); + if (!param(cmd, buffer, params, NULL)) + return; + add_memdump(response, NULL, NULL, cmd); command_success(cmd, response); @@ -151,6 +155,9 @@ static void json_memleak(struct command *cmd, { struct json_result *response = new_json_result(cmd); + if (!param(cmd, buffer, params, NULL)) + return; + if (!getenv("LIGHTNINGD_DEV_MEMLEAK")) { command_fail(cmd, LIGHTNINGD, "Leak detection needs $LIGHTNINGD_DEV_MEMLEAK"); diff --git a/lightningd/test/run-param.c b/lightningd/test/run-param.c index 7b069deb0308..8e735fdc2881 100644 --- a/lightningd/test/run-param.c +++ b/lightningd/test/run-param.c @@ -253,6 +253,16 @@ static void null_params(void) assert(*intptrs[6] == 888); } +static void no_params(void) +{ + struct json *j = json_parse(cmd, "[]"); + assert(param(cmd, j->buffer, j->toks, NULL)); + + j = json_parse(cmd, "[ 'unexpected' ]"); + assert(!param(cmd, j->buffer, j->toks, NULL)); +} + + #if DEVELOPER /* * Check to make sure there are no programming mistakes. @@ -560,12 +570,14 @@ int main(void) setup_locale(); setup_tmpctx(); cmd = tal(tmpctx, struct command); + cmd->mode = CMD_NORMAL; fail_msg = tal_arr(cmd, char, 10000); zero_params(); sanity(); tok_tok(); null_params(); + no_params(); #if DEVELOPER bad_programmer(); #endif diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 2e0ad1c1f5e0..69372eeb6e4f 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -388,6 +388,10 @@ static void json_listfunds(struct command *cmd, const char *buffer UNUSED, wallet_get_utxos(cmd, cmd->ld->wallet, output_state_available); char* out; struct pubkey funding_pubkey; + + if (!param(cmd, buffer, params, NULL)) + return; + json_object_start(response, NULL); json_array_start(response, "outputs"); for (size_t i = 0; i < tal_count(utxos); i++) { @@ -505,6 +509,10 @@ static void json_dev_rescan_outputs(struct command *cmd, const jsmntok_t *params UNUSED) { struct txo_rescan *rescan = tal(cmd, struct txo_rescan); + + if (!param(cmd, buffer, params, NULL)) + return; + rescan->response = new_json_result(cmd); rescan->cmd = cmd; From 1a4f355a7bc268376dcad4f81df0b67acf68d772 Mon Sep 17 00:00:00 2001 From: Mark Beckwith Date: Sat, 15 Sep 2018 12:12:15 -0500 Subject: [PATCH 21/27] param: add ok flag to struct command Callers to param() can now optionally set a flag to see if command_fail was called. This is necessary because the `cmd` is freed in case of failure. I spent a bit of time trying to extend the lifetime of the `cmd` to the end of parse_request(), but the destructors still needed to be called when they were, and it was getting ugly. So I took this minimal approach. Signed-off-by: Mark Beckwith --- lightningd/jsonrpc.c | 6 ++++++ lightningd/jsonrpc.h | 1 + 2 files changed, 7 insertions(+) diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index a7f78a8bd3e8..39223f761b86 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -282,6 +282,8 @@ static void connection_complete_ok(struct json_connection *jcon, { assert(id != NULL); assert(result != NULL); + if (cmd->ok) + *(cmd->ok) = true; /* This JSON is simple enough that we build manually */ json_done(jcon, cmd, take(tal_fmt(jcon, @@ -310,6 +312,9 @@ static void connection_complete_error(struct json_connection *jcon, assert(id != NULL); + if (cmd->ok) + *(cmd->ok) = false; + json_done(jcon, cmd, take(tal_fmt(tmpctx, "{ \"jsonrpc\": \"2.0\", " " \"error\" : " @@ -452,6 +457,7 @@ static void parse_request(struct json_connection *jcon, const jsmntok_t tok[]) json_tok_contents(jcon->buffer, id), json_tok_len(id)); c->mode = CMD_NORMAL; + c->ok = NULL; list_add(&jcon->commands, &c->list); tal_add_destructor(c, destroy_cmd); diff --git a/lightningd/jsonrpc.h b/lightningd/jsonrpc.h index e155258a281c..c85cf0928f9c 100644 --- a/lightningd/jsonrpc.h +++ b/lightningd/jsonrpc.h @@ -37,6 +37,7 @@ struct command { enum command_mode mode; /* This is created if mode is CMD_USAGE */ const char *usage; + bool *ok; }; struct json_connection { From cbde3e20f7fb4264ab0ccd88a04210a4852ad35c Mon Sep 17 00:00:00 2001 From: Mark Beckwith Date: Thu, 13 Sep 2018 10:57:24 -0500 Subject: [PATCH 22/27] cli: help command now also prints usage The help command now adds command usage to its output by calling each command handler in CMD_USAGE mode. Instead of seeing, for example: decodepay Decode {bolt11}, using {description} if necessary we see: decodepay bolt11 [description] Decode {bolt11}, using {description} if necessary Signed-off-by: Mark Beckwith --- lightningd/jsonrpc.c | 11 +++++++++-- lightningd/param.c | 5 +++-- tests/test_misc.py | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 39223f761b86..626e1f874a06 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -62,7 +62,7 @@ static void json_help(struct command *cmd, static const struct json_command help_command = { "help", json_help, - "List available commands, or give verbose help on one command.", + "List available commands, or give verbose help on one {command}.", .verbose = "help [command]\n" "Without [command]:\n" @@ -196,7 +196,10 @@ static void json_help(struct command *cmd, struct json_result *response = new_json_result(cmd); struct json_command **cmdlist = get_cmdlist(); const jsmntok_t *cmdtok; + char *usage; + /* FIXME: This is never called with a command parameter because lightning-cli + * attempts to launch the man page and then exits. */ if (!param(cmd, buffer, params, p_opt("command", json_tok_tok, &cmdtok), NULL)) @@ -231,11 +234,15 @@ static void json_help(struct command *cmd, return; } + cmd->mode = CMD_USAGE; json_array_start(response, "help"); for (i = 0; i < num_cmdlist; i++) { + cmdlist[i]->dispatch(cmd, NULL, NULL); + usage = tal_fmt(cmd, "%s %s", cmdlist[i]->name, + cmd->usage); json_add_object(response, "command", JSMN_STRING, - cmdlist[i]->name, + usage, "description", JSMN_STRING, cmdlist[i]->description, NULL); diff --git a/lightningd/param.c b/lightningd/param.c index c1480d7a5a1d..0618db79c335 100644 --- a/lightningd/param.c +++ b/lightningd/param.c @@ -244,7 +244,7 @@ static bool param_arr(struct command *cmd, const char *buffer, { #if DEVELOPER if (!check_params(params)) { - command_fail(cmd, PARAM_DEV_ERROR, "developer error"); + command_fail(cmd, PARAM_DEV_ERROR, "developer error: check_params"); return false; } #endif @@ -271,7 +271,8 @@ bool param(struct command *cmd, const char *buffer, param_cbx cbx = va_arg(ap, param_cbx); void *arg = va_arg(ap, void *); if (!param_add(¶ms, name, required, cbx, arg)) { - command_fail(cmd, PARAM_DEV_ERROR, "developer error"); + command_fail(cmd, PARAM_DEV_ERROR, + "developer error: param_add %s", name); va_end(ap); return false; } diff --git a/tests/test_misc.py b/tests/test_misc.py index c57f6a44ed43..177adac6ec8a 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -622,7 +622,7 @@ def test_cli(node_factory): .format(l1.daemon.lightning_dir), 'help']).decode('utf-8') # Test some known output. - assert 'help\n List available commands, or give verbose help on one command' in out + assert 'help [command]\n List available commands, or give verbose help on one {command}' in out # Test JSON output. out = subprocess.check_output(['cli/lightning-cli', From eee31a1519e78477e57a97393360468c0d5b2687 Mon Sep 17 00:00:00 2001 From: Mark Beckwith Date: Tue, 25 Sep 2018 09:34:15 -0500 Subject: [PATCH 23/27] git: ignore all current and future unit tests I wanted to add run-secret_eq_consttime and run-find_my_abspath to the .gitignore file, but instead I replaced all the names of tests with a simple regex. This works because git doesn't ignore files that are already in git. So source and header file are safe. It doesn't ignore *.c files that haven't been added to git either. Signed-off-by: Mark Beckwith --- .gitignore | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/.gitignore b/.gitignore index cb5f4a1fca5e..706a608ddfdf 100644 --- a/.gitignore +++ b/.gitignore @@ -21,32 +21,11 @@ __pycache__ config.vars # Ignore some generated binaries -lightningd/test/run-channel -lightningd/test/run-cryptomsg -lightningd/test/run-commit_tx -lightningd/test/run-find_my_path -lightningd/test/run-funding_tx -lightningd/test/run-param -lightningd/test/run-key_derive -common/test/run-ip_port_parsing -wire/test/run-peer-wire -bitcoin/test/run-tx-encode -channeld/test/run-full_channel -common/test/run-bolt11 -common/test/run-derive_basepoints -common/test/run-features -common/test/run-json -common/test/run-sphinx -connectd/test/run-initiator-success -connectd/test/run-responder-success -daemon/test/run-maxfee +*/test/run-* +!*/test/run-*.c external/libbacktrace-build/ external/libbacktrace.a external/libbacktrace.la -gossipd/test/run-find_route-specific -gossipd/test/run-initiator-success -gossipd/test/run-responder-success -onchaind/test/run-grind_feerate test/test_protocol test/test_sphinx tests/.pytest.restart From 66105e83ea859650ba5ab77c31656e4c26bb32de Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 24 Sep 2018 13:58:26 +0930 Subject: [PATCH 24/27] gossipd: simplify "broadcast channel_announcement now we have channel_update" logic It's simpler and more robust to just check that it's not yet announced (the broadcast index will be 0). Signed-off-by: Rusty Russell --- gossipd/routing.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/gossipd/routing.c b/gossipd/routing.c index 05e5eb7f871a..f60384a2c6d9 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -1055,7 +1055,6 @@ bool routing_add_channel_update(struct routing_state *rstate, struct bitcoin_blkid chain_hash; struct chan *chan; u8 direction; - bool have_broadcast_announce; if (!fromwire_channel_update(update, &signature, &chain_hash, &short_channel_id, ×tamp, @@ -1067,10 +1066,6 @@ bool routing_add_channel_update(struct routing_state *rstate, if (!chan) return false; - /* We broadcast announce once we have one update */ - have_broadcast_announce = is_halfchan_defined(&chan->half[0]) - || is_halfchan_defined(&chan->half[1]); - direction = channel_flags & 0x1; set_connection_values(chan, direction, fee_base_msat, fee_proportional_millionths, expiry, @@ -1093,7 +1088,7 @@ bool routing_add_channel_update(struct routing_state *rstate, * - MUST consider whether to send the `channel_announcement` after * receiving the first corresponding `channel_update`. */ - if (!have_broadcast_announce) + if (chan->channel_announcement_index == 0) add_channel_announce_to_broadcast(rstate, chan, timestamp); persistent_broadcast(rstate, chan->half[direction].channel_update, From 16e16a725e7d1c67bbe1a3f04a2a38a5c7062cf2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 24 Sep 2018 13:58:27 +0930 Subject: [PATCH 25/27] gossipd: apply private updates to announce channel. We trade channel_update before channel_announce makes the channel public, and currently forget them when we finally get the channel_announce. We should instead apply them, and not rely on retransmission (which we remove in the next patch!). This earlier channel_update means test_gossip_jsonrpc triggers too early, so have that wait for node_announcement. Signed-off-by: Rusty Russell --- gossipd/routing.c | 15 ++++++++++----- tests/test_gossip.py | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/gossipd/routing.c b/gossipd/routing.c index f60384a2c6d9..793357db4b56 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -753,11 +753,16 @@ bool routing_add_channel_announcement(struct routing_state *rstate, /* Channel is now public. */ chan->channel_announce = tal_dup_arr(chan, u8, msg, tal_count(msg), 0); - /* Clear any private updates: new updates will trigger broadcast of - * this channel_announce. */ - for (size_t i = 0; i < ARRAY_SIZE(chan->half); i++) - chan->half[i].channel_update - = tal_free(chan->half[i].channel_update); + /* Apply any private updates. */ + for (size_t i = 0; i < ARRAY_SIZE(chan->half); i++) { + const u8 *update = chan->half[i].channel_update; + if (!update) + continue; + + /* Remove from channel, otherwise it will be freed! */ + chan->half[i].channel_update = NULL; + routing_add_channel_update(rstate, take(update)); + } return true; } diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 12287a858ea5..d3e534cc0540 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -282,7 +282,7 @@ def test_gossip_jsonrpc(node_factory): 'peer_in WIRE_ANNOUNCEMENT_SIGNATURES']) # Just wait for the update to kick off and then check the effect - needle = "Received channel_update for channel" + needle = "Received node_announcement for node" l1.daemon.wait_for_log(needle) l2.daemon.wait_for_log(needle) # Need to increase timeout, intervals cannot be shortened with DEVELOPER=0 From e450c6bbdb075791cb05a002c726ea141f0764dc Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 25 Sep 2018 15:13:56 +0930 Subject: [PATCH 26/27] gossipd: remove time-delayed local channel_update, produce DISABLE on-demand. We have a lot of infrastructure to delay local channel_updates to avoid spamming on each peer reconnect; we had to keep tracking of pending ones though, in case we needed the very latest for sending an error when failing an HTLC. Instead, it's far simpler to set the local_disabled flag on a channel when we disconnect, but only send a disabling channel_update if we actually fail an HTLC. Note: handle_channel_update() TAKES update (due to tal_arr_dup), but we didn't use that before. Now we do, add annotation. Signed-off-by: Rusty Russell --- CHANGELOG.md | 2 + gossipd/gossipd.c | 503 ++++++++++++++-------------------------------- gossipd/routing.c | 2 +- gossipd/routing.h | 2 +- tests/test_pay.py | 8 +- 5 files changed, 165 insertions(+), 352 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8d6b99d4a4f..5e97fad78f9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed +- Protocol: `channel_update` sent to disable channel only if we reject an HTLC. + ### Deprecated Note: You should always set `allow-deprecated-apis=false` to test for diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 6b842ebac20c..56aa9e314bdc 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -61,25 +61,6 @@ static u32 max_scids_encode_bytes = -1U; static bool suppress_gossip = false; #endif -struct local_update { - /* daemon->local_updates */ - struct list_node list; - - /* Because we're handed to a one-arg timer */ - struct daemon *daemon; - - /* Which channel this is */ - struct short_channel_id scid; - - /* Which direction we own */ - int direction; - - u16 cltv_delta; - u64 htlc_minimum_msat; - u32 fee_base_msat, fee_proportional_millionths; - bool disable; -}; - struct daemon { /* Who am I? */ struct pubkey id; @@ -111,9 +92,6 @@ struct daemon { /* To make sure our node_announcement timestamps increase */ u32 last_announce_timestamp; - - /* Unapplied local updates waiting for their timers. */ - struct list_head local_updates; }; struct peer { @@ -159,25 +137,22 @@ struct peer { struct daemon_conn *remote; }; -/* FIXME: Reorder */ -static void peer_disable_channels(struct daemon *daemon, struct node *node); -static u8 *create_channel_update(const tal_t *ctx, - struct routing_state *rstate, - const struct chan *chan, - int direction, - bool disable, - u16 cltv_expiry_delta, - u64 htlc_minimum_msat, - u32 fee_base_msat, - u32 fee_proportional_millionths); -static struct local_update *find_local_update(struct daemon *daemon, - const struct short_channel_id *scid); +static void peer_disable_channels(struct daemon *daemon, struct node *node) +{ + for (size_t i = 0; i < tal_count(node->chans); i++) { + struct chan *c = node->chans[i]; + if (pubkey_eq(&other_node(node, c)->id, &daemon->id)) + c->local_disabled = true; + } +} static void destroy_peer(struct peer *peer) { struct node *node; list_del_from(&peer->daemon->peers, &peer->list); + + /* If we have a channel with this peer, disable it. */ node = get_node(peer->daemon->rstate, &peer->id); if (node) peer_disable_channels(peer->daemon, node); @@ -930,80 +905,15 @@ static bool maybe_queue_gossip(struct peer *peer) return false; } -static void handle_get_update(struct peer *peer, const u8 *msg) -{ - struct short_channel_id scid; - struct chan *chan; - const u8 *update; - struct routing_state *rstate = peer->daemon->rstate; - - if (!fromwire_gossip_get_update(msg, &scid)) { - status_trace("peer %s sent bad gossip_get_update %s", - type_to_string(tmpctx, struct pubkey, &peer->id), - tal_hex(tmpctx, msg)); - return; - } - - chan = get_channel(rstate, &scid); - if (!chan) { - status_unusual("peer %s scid %s: unknown channel", - type_to_string(tmpctx, struct pubkey, &peer->id), - type_to_string(tmpctx, struct short_channel_id, - &scid)); - update = NULL; - } else { - struct local_update *l; - - /* We want the update that comes from our end. */ - if (pubkey_eq(&chan->nodes[0]->id, &peer->daemon->id)) - update = chan->half[0].channel_update; - else if (pubkey_eq(&chan->nodes[1]->id, &peer->daemon->id)) - update = chan->half[1].channel_update; - else { - status_unusual("peer %s scid %s: not our channel?", - type_to_string(tmpctx, struct pubkey, - &peer->id), - type_to_string(tmpctx, - struct short_channel_id, - &scid)); - update = NULL; - goto out; - } - - /* We might have a pending update which overrides: always send - * that now, since this is used to populate errors which should - * contain the latest information. */ - l = find_local_update(peer->daemon, &scid); - if (l) - update = create_channel_update(tmpctx, - rstate, - chan, l->direction, - l->disable, - l->cltv_delta, - l->htlc_minimum_msat, - l->fee_base_msat, - l->fee_proportional_millionths); - } - -out: - status_trace("peer %s schanid %s: %s update", - type_to_string(tmpctx, struct pubkey, &peer->id), - type_to_string(tmpctx, struct short_channel_id, &scid), - update ? "got" : "no"); - - msg = towire_gossip_get_update_reply(NULL, update); - daemon_conn_send(peer->remote, take(msg)); -} - -static u8 *create_channel_update(const tal_t *ctx, - struct routing_state *rstate, +static void update_local_channel(struct daemon *daemon, const struct chan *chan, int direction, bool disable, u16 cltv_expiry_delta, u64 htlc_minimum_msat, u32 fee_base_msat, - u32 fee_proportional_millionths) + u32 fee_proportional_millionths, + const char *caller) { secp256k1_ecdsa_signature dummy_sig; u8 *update, *msg; @@ -1026,8 +936,8 @@ static u8 *create_channel_update(const tal_t *ctx, if (disable) channel_flags |= ROUTING_FLAGS_DISABLED; - update = towire_channel_update(ctx, &dummy_sig, - &rstate->chain_hash, + update = towire_channel_update(tmpctx, &dummy_sig, + &daemon->rstate->chain_hash, &chan->scid, timestamp, message_flags, channel_flags, @@ -1043,201 +953,191 @@ static u8 *create_channel_update(const tal_t *ctx, } msg = wire_sync_read(tmpctx, HSM_FD); - if (!msg || !fromwire_hsm_cupdate_sig_reply(ctx, msg, &update)) { + if (!msg || !fromwire_hsm_cupdate_sig_reply(NULL, msg, &update)) { status_failed(STATUS_FAIL_HSM_IO, "Reading cupdate_sig_req: %s", strerror(errno)); } - return update; + /* We always tell peer, even if it's not public yet */ + if (!is_chan_public(chan)) { + struct peer *peer = find_peer(daemon, + &chan->nodes[!direction]->id); + if (peer) + queue_peer_msg(peer, update); + } + + msg = handle_channel_update(daemon->rstate, take(update), caller); + if (msg) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "%s: rejected local channel update %s: %s", + caller, + /* This works because handle_channel_update + * only steals onto tmpctx */ + tal_hex(tmpctx, update), + tal_hex(tmpctx, msg)); } -/* Return true if the only change would be the timestamp. */ -static bool update_redundant(const struct half_chan *hc, - bool disable, u16 cltv_delta, u64 htlc_minimum_msat, - u32 fee_base_msat, u32 fee_proportional_millionths) +static void maybe_update_local_channel(struct daemon *daemon, + struct chan *chan, int direction) { - if (!is_halfchan_defined(hc)) - return false; + const struct half_chan *hc = &chan->half[direction]; - return !(hc->channel_flags & ROUTING_FLAGS_DISABLED) == !disable - && hc->delay == cltv_delta - && hc->htlc_minimum_msat == htlc_minimum_msat - && hc->base_fee == fee_base_msat - && hc->proportional_fee == fee_proportional_millionths; + /* Don't generate a channel_update for an uninitialized channel. */ + if (!hc->channel_update) + return; + + /* Nothing to update? */ + if (!chan->local_disabled == !(hc->channel_flags & ROUTING_FLAGS_DISABLED)) + return; + + update_local_channel(daemon, chan, direction, + chan->local_disabled, + hc->delay, + hc->htlc_minimum_msat, + hc->base_fee, + hc->proportional_fee, + __func__); } -static struct local_update *find_local_update(struct daemon *daemon, - const struct short_channel_id *scid) +static bool local_direction(struct daemon *daemon, + const struct chan *chan, + int *direction) { - struct local_update *i; - - list_for_each(&daemon->local_updates, i, list) { - if (short_channel_id_eq(scid, &i->scid)) - return i; + for (*direction = 0; *direction < 2; (*direction)++) { + if (pubkey_eq(&chan->nodes[*direction]->id, &daemon->id)) + return true; } - return NULL; + return false; } -/* Frees local_update */ -static void apply_delayed_local_update(struct local_update *local_update) +static void handle_get_update(struct peer *peer, const u8 *msg) { + struct short_channel_id scid; struct chan *chan; - const struct half_chan *hc; - u8 *cupdate, *err; + const u8 *update; + struct routing_state *rstate = peer->daemon->rstate; + int direction; - /* Can theoretically happen if channel just closed. */ - chan = get_channel(local_update->daemon->rstate, &local_update->scid); - if (!chan) { - status_trace("Delayed local_channel_update for unknown %s", - type_to_string(tmpctx, struct short_channel_id, - &local_update->scid)); - tal_free(local_update); + if (!fromwire_gossip_get_update(msg, &scid)) { + status_trace("peer %s sent bad gossip_get_update %s", + type_to_string(tmpctx, struct pubkey, &peer->id), + tal_hex(tmpctx, msg)); return; } - /* Convenience variable */ - hc = &chan->half[local_update->direction]; - - /* Avoid redundant updates on public channels: on non-public channels - * we'd need to consider pending updates, so don't bother. */ - if (is_chan_public(chan) - && update_redundant(hc, - local_update->disable, - local_update->cltv_delta, - local_update->htlc_minimum_msat, - local_update->fee_base_msat, - local_update->fee_proportional_millionths)) { - status_trace("Suppressing redundant channel update for %s:(%u) %s %"PRIu64"/%u vs %u/%u", - type_to_string(tmpctx, struct short_channel_id, - &local_update->scid), - local_update->direction, - is_halfchan_defined(hc) - ? (hc->channel_flags & ROUTING_FLAGS_DISABLED ? "DISABLED" : "ACTIVE") - : "UNDEFINED", - hc->last_timestamp, - (u32)time_now().ts.tv_sec, - hc->channel_flags, - local_update->disable); - tal_free(local_update); - return; + chan = get_channel(rstate, &scid); + if (!chan) { + status_unusual("peer %s scid %s: unknown channel", + type_to_string(tmpctx, struct pubkey, &peer->id), + type_to_string(tmpctx, struct short_channel_id, + &scid)); + update = NULL; + goto out; } - cupdate = create_channel_update(tmpctx, local_update->daemon->rstate, - chan, local_update->direction, - local_update->disable, - local_update->cltv_delta, - local_update->htlc_minimum_msat, - local_update->fee_base_msat, - local_update->fee_proportional_millionths); - - err = handle_channel_update(local_update->daemon->rstate, cupdate, - "apply_delayed_local_update"); - if (err) - status_failed(STATUS_FAIL_INTERNAL_ERROR, - "Rejected local channel update %s: %s", - tal_hex(tmpctx, cupdate), - tal_hex(tmpctx, err)); - - /* We always tell peer, even if it's not public yet */ - if (!is_chan_public(chan)) { - struct peer *peer = find_peer(local_update->daemon, - &chan->nodes[!local_update - ->direction]->id); - if (peer) - queue_peer_msg(peer, take(cupdate)); + /* We want the update that comes from our end. */ + if (!local_direction(peer->daemon, chan, &direction)) { + status_unusual("peer %s scid %s: not our channel?", + type_to_string(tmpctx, struct pubkey, &peer->id), + type_to_string(tmpctx, + struct short_channel_id, + &scid)); + update = NULL; + goto out; } - status_trace("Channel update for %s(%u)%s", - type_to_string(tmpctx, struct short_channel_id, - &local_update->scid), - local_update->direction, - is_chan_public(chan) ? "" : " (private)"); + /* Since we're going to send it out, make sure it's up-to-date. */ + maybe_update_local_channel(peer->daemon, chan, direction); - /* That channel_update might trigger our first channel_announcement */ - maybe_send_own_node_announce(local_update->daemon); - tal_free(local_update); -} + update = chan->half[direction].channel_update; +out: + status_trace("peer %s schanid %s: %s update", + type_to_string(tmpctx, struct pubkey, &peer->id), + type_to_string(tmpctx, struct short_channel_id, &scid), + update ? "got" : "no"); -static void destroy_local_update(struct local_update *local_update) -{ - list_del_from(&local_update->daemon->local_updates, - &local_update->list); + msg = towire_gossip_get_update_reply(NULL, update); + daemon_conn_send(peer->remote, take(msg)); } -static void queue_local_update(struct daemon *daemon, - struct local_update *local_update, - bool instant) +/* Return true if the information has changed. */ +static bool halfchan_new_info(const struct half_chan *hc, + u16 cltv_delta, u64 htlc_minimum_msat, + u32 fee_base_msat, u32 fee_proportional_millionths) { - /* Free any old unapplied update. */ - tal_free(find_local_update(daemon, &local_update->scid)); - - list_add_tail(&daemon->local_updates, &local_update->list); - tal_add_destructor(local_update, destroy_local_update); + if (!is_halfchan_defined(hc)) + return true; - if (instant) - apply_delayed_local_update(local_update); - else - /* Delay 1/4 a broadcast interval */ - new_reltimer(&daemon->timers, local_update, - time_from_msec(daemon->broadcast_interval/4), - apply_delayed_local_update, local_update); + return hc->delay != cltv_delta + || hc->htlc_minimum_msat != htlc_minimum_msat + || hc->base_fee != fee_base_msat + || hc->proportional_fee != fee_proportional_millionths; } static void handle_local_channel_update(struct peer *peer, const u8 *msg) { struct chan *chan; - struct local_update *local_update; - bool delay; - const struct pubkey *my_id = &peer->daemon->rstate->local_id; - - local_update = tal(peer->daemon, struct local_update); - local_update->daemon = peer->daemon; + struct short_channel_id scid; + bool disable; + u16 cltv_expiry_delta; + u64 htlc_minimum_msat; + u32 fee_base_msat; + u32 fee_proportional_millionths; + int direction; if (!fromwire_gossip_local_channel_update(msg, - &local_update->scid, - &local_update->disable, - &local_update->cltv_delta, - &local_update->htlc_minimum_msat, - &local_update->fee_base_msat, - &local_update->fee_proportional_millionths)) { + &scid, + &disable, + &cltv_expiry_delta, + &htlc_minimum_msat, + &fee_base_msat, + &fee_proportional_millionths)) { status_broken("peer %s bad local_channel_update %s", type_to_string(tmpctx, struct pubkey, &peer->id), tal_hex(tmpctx, msg)); - tal_free(local_update); return; } /* Can theoretically happen if channel just closed. */ - chan = get_channel(peer->daemon->rstate, &local_update->scid); + chan = get_channel(peer->daemon->rstate, &scid); if (!chan) { status_trace("peer %s local_channel_update for unknown %s", type_to_string(tmpctx, struct pubkey, &peer->id), type_to_string(tmpctx, struct short_channel_id, - &local_update->scid)); - tal_free(local_update); + &scid)); return; } - if (pubkey_eq(&chan->nodes[0]->id, my_id)) - local_update->direction = 0; - else if (pubkey_eq(&chan->nodes[1]->id, my_id)) - local_update->direction = 1; - else { + if (!local_direction(peer->daemon, chan, &direction)) { status_broken("peer %s bad local_channel_update for non-local %s", type_to_string(tmpctx, struct pubkey, &peer->id), type_to_string(tmpctx, struct short_channel_id, - &local_update->scid)); - tal_free(local_update); + &scid)); return; } - /* Don't delay the initialization update. */ - delay = !is_halfchan_defined(&chan->half[local_update->direction]); + /* We could change configuration on restart; update immediately. + * Or, if we're *enabling* an announced-disabled channel. + * Or, if it's an unannounced channel (only sending to peer). */ + if (halfchan_new_info(&chan->half[direction], + cltv_expiry_delta, htlc_minimum_msat, + fee_base_msat, fee_proportional_millionths) + || ((chan->half[direction].channel_flags & ROUTING_FLAGS_DISABLED) + && !disable) + || !is_chan_public(chan)) { + update_local_channel(peer->daemon, chan, direction, + disable, + cltv_expiry_delta, + htlc_minimum_msat, + fee_base_msat, + fee_proportional_millionths, + __func__); + } - /* channeld has reconnected, remove local disable. */ - chan->local_disabled = false; - queue_local_update(peer->daemon, local_update, delay); + /* Normal case: just toggle local_disabled, and generate broadcast in + * maybe_update_local_channel when/if someone asks about it. */ + chan->local_disabled = disable; } /** @@ -1765,30 +1665,24 @@ static struct io_plan *dev_gossip_suppress(struct io_conn *conn, } #endif /* DEVELOPER */ -static void gossip_send_keepalive_update(struct routing_state *rstate, +static void gossip_send_keepalive_update(struct daemon *daemon, const struct chan *chan, const struct half_chan *hc) { - u8 *update, *err; - - /* Generate a new update, with up to date timestamp */ - update = create_channel_update(tmpctx, rstate, chan, - hc->channel_flags & ROUTING_FLAGS_DIRECTION, - false, - hc->delay, - hc->htlc_minimum_msat, - hc->base_fee, - hc->proportional_fee); - status_trace("Sending keepalive channel_update for %s", type_to_string(tmpctx, struct short_channel_id, &chan->scid)); - err = handle_channel_update(rstate, update, "keepalive"); - if (err) - status_failed(STATUS_FAIL_INTERNAL_ERROR, - "rejected keepalive channel_update: %s", - tal_hex(tmpctx, err)); + /* As a side-effect, this will create an update which matches the + * local_disabled state */ + update_local_channel(daemon, chan, + hc->channel_flags & ROUTING_FLAGS_DIRECTION, + chan->local_disabled, + hc->delay, + hc->htlc_minimum_msat, + hc->base_fee, + hc->proportional_fee, + __func__); } static void gossip_refresh_network(struct daemon *daemon) @@ -1827,87 +1721,16 @@ static void gossip_refresh_network(struct daemon *daemon) continue; } - gossip_send_keepalive_update(daemon->rstate, n->chans[i], - hc); + gossip_send_keepalive_update(daemon, n->chans[i], hc); } } route_prune(daemon->rstate); } -static void gossip_disable_outgoing_halfchan(struct daemon *daemon, - struct chan *chan) -{ - u8 direction; - struct half_chan *hc; - u8 message_flags, channel_flags; - u32 timestamp; - struct bitcoin_blkid chain_hash; - secp256k1_ecdsa_signature sig; - struct local_update *local_update; - struct routing_state *rstate = daemon->rstate; - - direction = pubkey_eq(&chan->nodes[0]->id, &rstate->local_id)?0:1; - assert(chan); - hc = &chan->half[direction]; - - if (!is_halfchan_defined(hc)) - return; - - status_trace("Disabling channel %s/%d, active %d -> %d", - type_to_string(tmpctx, struct short_channel_id, &chan->scid), - direction, is_halfchan_enabled(hc), 0); - - local_update = tal(daemon, struct local_update); - local_update->daemon = daemon; - local_update->direction = direction; - - if (!fromwire_channel_update( - hc->channel_update, &sig, &chain_hash, - &local_update->scid, ×tamp, - &message_flags, &channel_flags, - &local_update->cltv_delta, - &local_update->htlc_minimum_msat, - &local_update->fee_base_msat, - &local_update->fee_proportional_millionths)) { - status_failed( - STATUS_FAIL_INTERNAL_ERROR, - "Unable to parse previously accepted channel_update"); - } - - local_update->disable = true; - - queue_local_update(daemon, local_update, false); -} - -/** - * Disable both directions of a local channel. - * - * Disables both directions of a local channel as a result of a close or lost - * connection. A disabling `channel_update` will be queued for the outgoing - * direction as well, but that will be a little delayed. We can't do that for - * the incoming direction, so we set local_disabled and the other endpoint - * should take care of publicly disabling it with a `channel_update`. - * - * It is important to disable the incoming edge as well since we might otherwise - * return that edge as a `contact_point` as part of an invoice. - */ -static void gossip_disable_local_channel(struct daemon *daemon, - struct chan *chan) -{ - struct routing_state *rstate = daemon->rstate; - - assert(pubkey_eq(&rstate->local_id, &chan->nodes[0]->id) || - pubkey_eq(&rstate->local_id, &chan->nodes[1]->id)); - - chan->local_disabled = true; - gossip_disable_outgoing_halfchan(daemon, chan); -} - static void gossip_disable_local_channels(struct daemon *daemon) { - struct node *local_node = - get_node(daemon->rstate, &daemon->rstate->local_id); + struct node *local_node = get_node(daemon->rstate, &daemon->id); size_t i; /* We don't have a local_node, so we don't have any channels yet @@ -1916,8 +1739,7 @@ static void gossip_disable_local_channels(struct daemon *daemon) return; for (i = 0; i < tal_count(local_node->chans); i++) - gossip_disable_local_channel(daemon, - local_node->chans[i]); + local_node->chans[i]->local_disabled = true; } /* Parse an incoming gossip init message and assign config variables @@ -1984,18 +1806,6 @@ static struct io_plan *resolve_channel_req(struct io_conn *conn, return daemon_conn_read_next(conn, &daemon->master); } -static void peer_disable_channels(struct daemon *daemon, struct node *node) -{ - struct chan *c; - size_t i; - for (i=0; ichans); i++) { - c = node->chans[i]; - if (pubkey_eq(&other_node(node, c)->id, - &daemon->rstate->local_id)) - gossip_disable_local_channel(daemon, c); - } -} - static struct io_plan *handle_txout_reply(struct io_conn *conn, struct daemon *daemon, const u8 *msg) { @@ -2099,7 +1909,7 @@ static struct io_plan *handle_local_channel_close(struct io_conn *conn, chan = get_channel(rstate, &scid); if (chan) - gossip_disable_local_channel(daemon, chan); + chan->local_disabled = true; return daemon_conn_read_next(conn, &daemon->master); } @@ -2255,7 +2065,6 @@ int main(int argc, char *argv[]) daemon = tal(NULL, struct daemon); list_head_init(&daemon->peers); - list_head_init(&daemon->local_updates); timers_init(&daemon->timers, time_mono()); daemon->broadcast_interval = 30000; daemon->last_announce_timestamp = 0; diff --git a/gossipd/routing.c b/gossipd/routing.c index 793357db4b56..845fe53d9a13 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -1101,7 +1101,7 @@ bool routing_add_channel_update(struct routing_state *rstate, return true; } -u8 *handle_channel_update(struct routing_state *rstate, const u8 *update, +u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES, const char *source) { u8 *serialized; diff --git a/gossipd/routing.h b/gossipd/routing.h index 349d43e9c916..979fd7929b4a 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -252,7 +252,7 @@ void handle_pending_cannouncement(struct routing_state *rstate, const u8 *txscript); /* Returns NULL if all OK, otherwise an error for the peer which sent. */ -u8 *handle_channel_update(struct routing_state *rstate, const u8 *update, +u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES, const char *source); /* Returns NULL if all OK, otherwise an error for the peer which sent. */ diff --git a/tests/test_pay.py b/tests/test_pay.py index d5ea5022363c..4567eda8046c 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -91,11 +91,13 @@ def test_pay_disconnect(node_factory, bitcoind): inv = l2.rpc.invoice(123000, 'test_pay_disconnect', 'description') rhash = inv['payment_hash'] + wait_for(lambda: [c['active'] for c in l1.rpc.listchannels()['channels']] == [True, True]) + # Can't use `pay` since that'd notice that we can't route, due to disabling channel_update route = l1.rpc.getroute(l2.info['id'], 123000, 1)["route"] l2.stop() - l1.daemon.wait_for_log('Disabling channel .*, active 1 -> 0') + wait_for(lambda: [c['active'] for c in l1.rpc.listchannels()['channels']] == [False, False]) # Can't pay while its offline. with pytest.raises(RpcError): @@ -146,8 +148,8 @@ def try_route(src, dst): l2.rpc.dev_suppress_gossip() l3.stop() - # Make sure that l2 has processed the local update which disables. - l2.daemon.wait_for_log('Received channel_update for channel {}\(.*\) now DISABLED was ACTIVE \(from apply_delayed_local_update\)'.format(chanid2)) + # Make sure that l2 has seen disconnect, considers channel disabled. + wait_for(lambda: [c['active'] for c in l2.rpc.listchannels(chanid2)['channels']] == [False, False]) l1.rpc.sendpay(route, inv['payment_hash']) with pytest.raises(RpcError, match=r'WIRE_TEMPORARY_CHANNEL_FAILURE'): From dfe0378464f08e436481c477cfb1c4771092e86d Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Tue, 25 Sep 2018 18:39:40 -0700 Subject: [PATCH 27/27] git: ignore vim's c-tags 'tags' file --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 706a608ddfdf..7bfeaf75e4e4 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ *.rej .cppcheck-suppress TAGS +tags ccan/tools/configurator/configurator ccan/ccan/cdump/tools/cdump-enumstr gen_*