From e17b0ebcb40d8734d7292cd52f2e08e5659456b3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 19 Feb 2018 11:36:14 +1030 Subject: [PATCH] channeld: map htlc add/remove errors to names. I couldn't figure out what 'Bad peer_add_htlc: 7' meant! Signed-off-by: Rusty Russell --- channeld/Makefile | 23 +++++++++++++-------- channeld/channel.c | 18 +++++++++------- channeld/full_channel.c | 27 ++++++++++++++++++++++++ channeld/full_channel.h | 39 +++++------------------------------ channeld/full_channel_error.h | 39 +++++++++++++++++++++++++++++++++++ tests/test_lightningd.py | 4 ++-- 6 files changed, 98 insertions(+), 52 deletions(-) create mode 100644 channeld/full_channel_error.h diff --git a/channeld/Makefile b/channeld/Makefile index bcccc4604..e50956f97 100644 --- a/channeld/Makefile +++ b/channeld/Makefile @@ -8,21 +8,22 @@ default: channeld-all channeld-all: lightningd/lightning_channeld -# lightningd/channel needs these: LIGHTNINGD_CHANNEL_HEADERS_GEN := \ + channeld/gen_full_channel_error_names.h \ channeld/gen_channel_wire.h LIGHTNINGD_CHANNEL_HEADERS_NOGEN := \ channeld/channeld_htlc.h \ channeld/commit_tx.h \ - channeld/full_channel.h + channeld/full_channel.h \ + channeld/full_channel_error.h LIGHTNINGD_CHANNEL_HEADERS := $(LIGHTNINGD_CHANNEL_HEADERS_GEN) $(LIGHTNINGD_CHANNEL_HEADERS_NOGEN) LIGHTNINGD_CHANNEL_SRC := channeld/channel.c \ channeld/commit_tx.c \ channeld/full_channel.c \ - $(LIGHTNINGD_CHANNEL_HEADERS_GEN:.h=.c) + channeld/gen_channel_wire.c LIGHTNINGD_CHANNEL_OBJS := $(LIGHTNINGD_CHANNEL_SRC:.c=.o) # Make sure these depend on everything. @@ -72,19 +73,23 @@ CHANNELD_COMMON_OBJS := \ wire/fromwire.o \ wire/towire.o +LIGHTNINGD_CHANNEL_SRC_GEN := $(filter channeld/gen_%, $(LIGHTNINGD_CHANNEL_SRC)) + +LIGHTNINGD_CHANNEL_SRC_NOGEN := $(filter-out channeld/gen_%, $(LIGHTNINGD_CHANNEL_SRC)) + # Control daemon uses this: LIGHTNINGD_CHANNEL_CONTROL_HEADERS := $(LIGHTNINGD_CHANNEL_HEADERS_GEN) -LIGHTNINGD_CHANNEL_CONTROL_SRC := $(LIGHTNINGD_CHANNEL_HEADERS_GEN:.h=.c) +LIGHTNINGD_CHANNEL_CONTROL_SRC := $(LIGHTNINGD_CHANNEL_SRC_GEN) LIGHTNINGD_CHANNEL_CONTROL_OBJS := $(LIGHTNINGD_CHANNEL_CONTROL_SRC:.c=.o) -LIGHTNINGD_CHANNEL_GEN_SRC := $(filter channeld/gen_%, $(LIGHTNINGD_CHANNEL_SRC) $(LIGHTNINGD_CHANNEL_CONTROL_SRC)) - -LIGHTNINGD_CHANNEL_SRC_NOGEN := $(filter-out channeld/gen_%, $(LIGHTNINGD_CHANNEL_SRC)) - # Add to headers which any object might need. LIGHTNINGD_HEADERS_GEN += $(LIGHTNINGD_CHANNEL_HEADERS_GEN) LIGHTNINGD_HEADERS_NOGEN += $(LIGHTNINGD_CHANNEL_HEADERS_NOGEN) +channeld/gen_full_channel_error_names.h: channeld/full_channel_error.h ccan/ccan/cdump/tools/cdump-enumstr + ccan/ccan/cdump/tools/cdump-enumstr channeld/full_channel_error.h > $@ + + $(LIGHTNINGD_CHANNEL_OBJS): $(LIGHTNINGD_HEADERS) channeld/gen_channel_wire.h: $(WIRE_GEN) channeld/channel_wire.csv @@ -93,7 +98,7 @@ channeld/gen_channel_wire.h: $(WIRE_GEN) channeld/channel_wire.csv channeld/gen_channel_wire.c: $(WIRE_GEN) channeld/channel_wire.csv $(WIRE_GEN) ${@:.c=.h} channel_wire_type < channeld/channel_wire.csv > $@ -LIGHTNINGD_CHANNEL_OBJS := $(LIGHTNINGD_CHANNEL_SRC:.c=.o) $(LIGHTNINGD_CHANNEL_GEN_SRC:.c=.o) +LIGHTNINGD_CHANNEL_OBJS := $(LIGHTNINGD_CHANNEL_SRC:.c=.o) lightningd/lightning_channeld: $(LIGHTNINGD_CHANNEL_OBJS) $(WIRE_ONION_OBJS) $(CHANNELD_COMMON_OBJS) $(WIRE_OBJS) $(BITCOIN_OBJS) $(LIGHTNINGD_HSM_CLIENT_OBJS) diff --git a/channeld/channel.c b/channeld/channel.c index f6f4d7bc7..108789318 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -596,7 +596,8 @@ static void handle_peer_add_htlc(struct peer *peer, const u8 *msg) peer_failed(PEER_FD, &peer->cs, &peer->channel_id, - "Bad peer_add_htlc: %u", add_err); + "Bad peer_add_htlc: %s", + channel_add_err_name(add_err)); /* If this is wrong, we don't complain yet; when it's confirmed we'll * send it to the master which handles all HTLC failures. */ @@ -1389,7 +1390,7 @@ static void handle_peer_fulfill_htlc(struct peer *peer, const u8 *msg) &peer->cs, &peer->channel_id, "Bad update_fulfill_htlc: failed to fulfill %" - PRIu64 " error %u", id, e); + PRIu64 " error %s", id, channel_remove_err_name(e)); } abort(); } @@ -1427,7 +1428,8 @@ static void handle_peer_fail_htlc(struct peer *peer, const u8 *msg) &peer->cs, &peer->channel_id, "Bad update_fail_htlc: failed to remove %" - PRIu64 " error %u", id, e); + PRIu64 " error %s", id, + channel_remove_err_name(e)); } abort(); } @@ -1500,7 +1502,7 @@ static void handle_peer_fail_malformed_htlc(struct peer *peer, const u8 *msg) &peer->cs, &peer->channel_id, "Bad update_fail_malformed_htlc: failed to remove %" - PRIu64 " error %u", id, e); + PRIu64 " error %s", id, channel_remove_err_name(e)); } abort(); } @@ -2004,8 +2006,9 @@ static void handle_offer_htlc(struct peer *peer, const u8 *inmsg) e = channel_add_htlc(peer->channel, LOCAL, peer->htlc_id, amount_msat, cltv_expiry, &payment_hash, onion_routing_packet, NULL); - status_trace("Adding HTLC %"PRIu64" msat=%"PRIu64" cltv=%u gave %i", - peer->htlc_id, amount_msat, cltv_expiry, e); + status_trace("Adding HTLC %"PRIu64" msat=%"PRIu64" cltv=%u gave %s", + peer->htlc_id, amount_msat, cltv_expiry, + channel_add_err_name(e)); switch (e) { case CHANNEL_ERR_ADD_OK: @@ -2284,7 +2287,8 @@ static void handle_fail(struct peer *peer, const u8 *inmsg) case CHANNEL_ERR_HTLC_NOT_IRREVOCABLE: case CHANNEL_ERR_BAD_PREIMAGE: status_failed(STATUS_FAIL_MASTER_IO, - "HTLC %"PRIu64" removal failed: %i", id, e); + "HTLC %"PRIu64" removal failed: %s", id, + channel_remove_err_name(e)); } abort(); } diff --git a/channeld/full_channel.c b/channeld/full_channel.c index f92591cce..bbadf1e98 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -17,7 +17,10 @@ #include #include #include +#include #include + /* Needs to be at end, since it doesn't include its own hdrs */ + #include "gen_full_channel_error_names.h" struct channel *new_full_channel(const tal_t *ctx, const struct bitcoin_txid *funding_txid, @@ -1055,3 +1058,27 @@ bool channel_force_htlcs(struct channel *channel, return true; } + +const char *channel_add_err_name(enum channel_add_err e) +{ + static char invalidbuf[sizeof("INVALID ") + STR_MAX_CHARS(e)]; + + for (size_t i = 0; enum_channel_add_err_names[i].name; i++) { + if (enum_channel_add_err_names[i].v == e) + return enum_channel_add_err_names[i].name; + } + sprintf(invalidbuf, "INVALID %i", e); + return invalidbuf; +} + +const char *channel_remove_err_name(enum channel_remove_err e) +{ + static char invalidbuf[sizeof("INVALID ") + STR_MAX_CHARS(e)]; + + for (size_t i = 0; enum_channel_remove_err_names[i].name; i++) { + if (enum_channel_remove_err_names[i].v == e) + return enum_channel_remove_err_names[i].name; + } + sprintf(invalidbuf, "INVALID %i", e); + return invalidbuf; +} diff --git a/channeld/full_channel.h b/channeld/full_channel.h index d76d3b000..5a6af9b85 100644 --- a/channeld/full_channel.h +++ b/channeld/full_channel.h @@ -3,6 +3,7 @@ #define LIGHTNING_LIGHTNINGD_CHANNEL_FULL_CHANNEL_H #include "config.h" #include +#include #include #include @@ -77,25 +78,6 @@ struct bitcoin_tx **channel_txs(const tal_t *ctx, u32 actual_feerate(const struct channel *channel, const struct signature *theirsig); -enum channel_add_err { - /* All OK! */ - CHANNEL_ERR_ADD_OK, - /* Bad expiry value */ - CHANNEL_ERR_INVALID_EXPIRY, - /* Not really a failure, if expected: it's an exact duplicate. */ - CHANNEL_ERR_DUPLICATE, - /* Same ID, but otherwise different. */ - CHANNEL_ERR_DUPLICATE_ID_DIFFERENT, - /* Would exceed the specified max_htlc_value_in_flight_msat */ - CHANNEL_ERR_MAX_HTLC_VALUE_EXCEEDED, - /* Can't afford it */ - CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED, - /* HTLC is below htlc_minimum_msat */ - CHANNEL_ERR_HTLC_BELOW_MINIMUM, - /* HTLC would push past max_accepted_htlcs */ - CHANNEL_ERR_TOO_MANY_HTLCS, -}; - /** * channel_add_htlc: append an HTLC to channel if it can afford it * @channel: The channel @@ -128,21 +110,6 @@ enum channel_add_err channel_add_htlc(struct channel *channel, */ struct htlc *channel_get_htlc(struct channel *channel, enum side sender, u64 id); -enum channel_remove_err { - /* All OK! */ - CHANNEL_ERR_REMOVE_OK, - /* No such HTLC. */ - CHANNEL_ERR_NO_SUCH_ID, - /* Already have fulfilled it */ - CHANNEL_ERR_ALREADY_FULFILLED, - /* Preimage doesn't hash to value. */ - CHANNEL_ERR_BAD_PREIMAGE, - /* HTLC is not committed */ - CHANNEL_ERR_HTLC_UNCOMMITTED, - /* HTLC is not committed and prior revoked on both sides */ - CHANNEL_ERR_HTLC_NOT_IRREVOCABLE -}; - /** * channel_fail_htlc: remove an HTLC, funds to the side which offered it. * @channel: The channel state @@ -279,4 +246,8 @@ bool channel_force_htlcs(struct channel *channel, * Uses status_trace() on every HTLC. */ void dump_htlcs(const struct channel *channel, const char *prefix); + +const char *channel_add_err_name(enum channel_add_err e); +const char *channel_remove_err_name(enum channel_remove_err e); + #endif /* LIGHTNING_LIGHTNINGD_CHANNEL_FULL_CHANNEL_H */ diff --git a/channeld/full_channel_error.h b/channeld/full_channel_error.h new file mode 100644 index 000000000..899f4623d --- /dev/null +++ b/channeld/full_channel_error.h @@ -0,0 +1,39 @@ +/* Error enums separated out for easy autogen of names*/ +#ifndef LIGHTNING_CHANNELD_FULL_CHANNEL_ERROR_H +#define LIGHTNING_CHANNELD_FULL_CHANNEL_ERROR_H + +enum channel_add_err { + /* All OK! */ + CHANNEL_ERR_ADD_OK, + /* Bad expiry value */ + CHANNEL_ERR_INVALID_EXPIRY, + /* Not really a failure, if expected: it's an exact duplicate. */ + CHANNEL_ERR_DUPLICATE, + /* Same ID, but otherwise different. */ + CHANNEL_ERR_DUPLICATE_ID_DIFFERENT, + /* Would exceed the specified max_htlc_value_in_flight_msat */ + CHANNEL_ERR_MAX_HTLC_VALUE_EXCEEDED, + /* Can't afford it */ + CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED, + /* HTLC is below htlc_minimum_msat */ + CHANNEL_ERR_HTLC_BELOW_MINIMUM, + /* HTLC would push past max_accepted_htlcs */ + CHANNEL_ERR_TOO_MANY_HTLCS, +}; + +enum channel_remove_err { + /* All OK! */ + CHANNEL_ERR_REMOVE_OK, + /* No such HTLC. */ + CHANNEL_ERR_NO_SUCH_ID, + /* Already have fulfilled it */ + CHANNEL_ERR_ALREADY_FULFILLED, + /* Preimage doesn't hash to value. */ + CHANNEL_ERR_BAD_PREIMAGE, + /* HTLC is not committed */ + CHANNEL_ERR_HTLC_UNCOMMITTED, + /* HTLC is not committed and prior revoked on both sides */ + CHANNEL_ERR_HTLC_NOT_IRREVOCABLE +}; + +#endif /* LIGHTNING_CHANNELD_FULL_CHANNEL_ERROR_H */ diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index c01410c4c..a05576178 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -2065,9 +2065,9 @@ class LightningDTests(BaseLightningDTests): # We add one to the blockcount for a bit of fuzz (FIXME: Shadowroute would fix this!) shadow_route = 1 - l1.daemon.wait_for_log("Adding HTLC 0 msat=5010198 cltv={} gave 0" + l1.daemon.wait_for_log("Adding HTLC 0 msat=5010198 cltv={} gave CHANNEL_ERR_ADD_OK" .format(bitcoind.rpc.getblockcount() + 20 + 9 + shadow_route)) - l2.daemon.wait_for_log("Adding HTLC 0 msat=4999999 cltv={} gave 0" + l2.daemon.wait_for_log("Adding HTLC 0 msat=4999999 cltv={} gave CHANNEL_ERR_ADD_OK" .format(bitcoind.rpc.getblockcount() + 9 + shadow_route)) l3.daemon.wait_for_log("test_forward_different_fees_and_cltv: Actual amount 4999999msat, HTLC expiry {}" .format(bitcoind.rpc.getblockcount() + 9 + shadow_route))