diff --git a/.gitignore b/.gitignore index cb5f4a1fca5e..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_* @@ -21,32 +22,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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 42634719073e..5e97fad78f9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,16 +9,21 @@ 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. ### 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 changes. +- JSON RPC: `listchannels`' `flags` field. This has been split into two fields, see Added. + ### Removed ### Fixed diff --git a/Makefile b/Makefile index 6eaa29719847..10bcbc757912 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../lightning-rfc/ -BOLTVERSION := fd9da9b95eb5d585252d7e749212151502e0cc17 +BOLTVERSION := 0891374d47ddffa64c5a2e6ad151247e3d6b7a59 -include config.vars @@ -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) 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/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; 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: 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(); } 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/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; } diff --git a/devtools/gossipwith.c b/devtools/gossipwith.c new file mode 100644 index 000000000000..1cb2e3563a09 --- /dev/null +++ b/devtools/gossipwith.c @@ -0,0 +1,229 @@ +/* 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, + char **args) +{ + 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)); + + /* 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)); + + 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] [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) + 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, argv+2); + exit(0); +} + 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 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/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/gossipd/gossipd.c b/gossipd/gossipd.c index 7d1d6247d70a..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,85 +905,24 @@ 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; 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 +932,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, + update = towire_channel_update(tmpctx, &dummy_sig, + &daemon->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); @@ -1038,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]; + + /* 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; - return !(hc->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; + 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->flags & ROUTING_FLAGS_DISABLED ? "DISABLED" : "ACTIVE") - : "UNDEFINED", - hc->last_timestamp, - (u32)time_now().ts.tv_sec, - hc->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; } /** @@ -1441,7 +1346,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; @@ -1759,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->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) @@ -1821,86 +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; - u16 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, &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 @@ -1909,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 @@ -1977,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) { @@ -2092,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); } @@ -2248,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 a0ab36a8054a..845fe53d9a13 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; @@ -749,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; } @@ -1003,7 +1012,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 +1023,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 +1042,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 +1052,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; @@ -1049,10 +1060,10 @@ 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, &flags, + &short_channel_id, ×tamp, + &message_flags, &channel_flags, &expiry, &htlc_minimum_msat, &fee_base_msat, &fee_proportional_millionths)) return false; @@ -1060,14 +1071,11 @@ 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 = 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); @@ -1085,7 +1093,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, @@ -1093,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; @@ -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); @@ -1276,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; @@ -1292,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); @@ -1459,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 f8552611ea95..979fd7929b4a 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 { @@ -248,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. */ @@ -308,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/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/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); diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 558e581bd13b..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" @@ -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); @@ -187,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)) @@ -222,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); @@ -273,6 +289,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, @@ -301,6 +319,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\" : " @@ -442,6 +463,8 @@ 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; + 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 9f36b8010c13..c85cf0928f9c 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,11 @@ 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; + bool *ok; }; struct json_connection { 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/param.c b/lightningd/param.c index a20c00938a7a..0618db79c335 100644 --- a/lightningd/param.c +++ b/lightningd/param.c @@ -223,13 +223,28 @@ 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) { #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 @@ -256,12 +271,18 @@ 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; } } 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..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. @@ -431,7 +441,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 +455,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,17 +517,67 @@ 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(); 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 @@ -528,6 +588,8 @@ int main(void) advanced(); advanced_fail(); json_tok_tests(); + usage(); + tal_free(tmpctx); printf("run-params ok\n"); } 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) { 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/test_gossip.py b/tests/test_gossip.py index ce53d1ac3abe..d3e534cc0540 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, only_one import json import logging import os +import struct import subprocess import time import unittest @@ -281,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 @@ -821,11 +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 - "00000099" # len - "12abbbba" # csum - "1002" # WIRE_GOSSIP_STORE_NODE_ANNOUNCEMENT - "00950101cf5d870bc7ecabcb7cd16898ef66891e5f0c6c5851bd85b670f03d325bc44d7544d367cd852e18ec03f7f4ff369b06860a3b12b07b29f36fb318ca11348bf8ec00005aab817c03f113414ebdc6c1fb0f33c99cd5a1d09dd79e7fdf2468cf1fe1af6674361695d23974b250757a7a6c6549544300000000000000000000000000000000000000000000000007010566933e2607" + f.write(bytearray.fromhex("03" # GOSSIP_VERSION "000001bc" # len "521ef598" # csum "1000" # WIRE_GOSSIP_STORE_CHANNEL_ANNOUNCEMENT @@ -833,9 +830,85 @@ 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'. 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 + + +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'}) + 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) + + # 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']) + + # 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) + + 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'] == []) + assert(l1.rpc.listnodes()['nodes'] == []) 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', 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'): diff --git a/tests/utils.py b/tests/utils.py index 7ec68a812f17..24a36c0f3b45 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): @@ -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. 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)) 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)); 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; 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,