From 021fd29129833f74f4288432cdff37911fcad8b3 Mon Sep 17 00:00:00 2001 From: ZmnSCPxj jxPCSnmZ Date: Wed, 15 Jan 2020 16:04:03 +0800 Subject: [PATCH 1/6] common/json.c: Implement `json_to_u32`. --- common/json.c | 15 +++++++++++++++ common/json.h | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/common/json.c b/common/json.c index 8cd37cdd02f9..af99b2e82574 100644 --- a/common/json.c +++ b/common/json.c @@ -105,6 +105,21 @@ bool json_to_u16(const char *buffer, const jsmntok_t *tok, return true; } +bool json_to_u32(const char *buffer, const jsmntok_t *tok, + uint32_t *num) +{ + uint64_t u64; + + if (!json_to_u64(buffer, tok, &u64)) + return false; + *num = u64; + + /* Just in case it doesn't fit. */ + if (*num != u64) + return false; + return true; +} + bool json_to_int(const char *buffer, const jsmntok_t *tok, int *num) { char *end; diff --git a/common/json.h b/common/json.h index 355af0103711..80548671af5c 100644 --- a/common/json.h +++ b/common/json.h @@ -37,6 +37,10 @@ bool json_to_number(const char *buffer, const jsmntok_t *tok, bool json_to_u64(const char *buffer, const jsmntok_t *tok, uint64_t *num); +/* Extract number from this (may be a string, or a number literal) */ +bool json_to_u32(const char *buffer, const jsmntok_t *tok, + uint32_t *num); + /* Extract number from this (may be a string, or a number literal) */ bool json_to_u16(const char *buffer, const jsmntok_t *tok, uint16_t *num); From f9decf171418ad7d05a8f911cb716e39d088fa3a Mon Sep 17 00:00:00 2001 From: ZmnSCPxj Date: Thu, 26 Dec 2019 10:19:09 +0000 Subject: [PATCH 2/6] lightningd/peer_control.c: Implement waitblockheight. This is needed to fully implement handling of blockheight disagreements between us and payee. If payee believes the blockheight is higher than ours, then `pay` should wait for our node to achieve that blockheight. Changelog-Add: Implement `waitblockheight` to wait for a specific blockheight. --- contrib/pyln-client/pyln/client/lightning.py | 10 ++ doc/Makefile | 1 + doc/index.rst | 3 +- doc/lightning-waitblockheight.7 | 36 +++++++ doc/lightning-waitblockheight.7.md | 37 +++++++ lightningd/lightningd.c | 2 + lightningd/lightningd.h | 3 + lightningd/peer_control.c | 107 +++++++++++++++++++ lightningd/peer_control.h | 4 + lightningd/test/run-find_my_abspath.c | 3 + tests/test_misc.py | 40 ++++++- 11 files changed, 244 insertions(+), 2 deletions(-) create mode 100644 doc/lightning-waitblockheight.7 create mode 100644 doc/lightning-waitblockheight.7.md diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py index 230f8be64598..63b15bac29d3 100644 --- a/contrib/pyln-client/pyln/client/lightning.py +++ b/contrib/pyln-client/pyln/client/lightning.py @@ -997,6 +997,16 @@ def waitanyinvoice(self, lastpay_index=None): } return self.call("waitanyinvoice", payload) + def waitblockheight(self, blockheight, timeout=None): + """ + Wait for the blockchain to reach the specified block height. + """ + payload = { + "blockheight": blockheight, + "timeout": timeout + } + return self.call("waitblockheight", payload) + def waitinvoice(self, label): """ Wait for an incoming payment matching the invoice with {label} diff --git a/doc/Makefile b/doc/Makefile index 80a9c3692add..db2b300293a7 100644 --- a/doc/Makefile +++ b/doc/Makefile @@ -42,6 +42,7 @@ MANPAGES := doc/lightning-cli.1 \ doc/lightning-txsend.7 \ doc/lightning-waitinvoice.7 \ doc/lightning-waitanyinvoice.7 \ + doc/lightning-waitblockheight.7 \ doc/lightning-waitsendpay.7 \ doc/lightning-withdraw.7 diff --git a/doc/index.rst b/doc/index.rst index 1eab0f6139b1..d54b644f3e47 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -27,8 +27,8 @@ c-lightning Documentation :maxdepth: 1 :caption: Manpages - lightningd lightningd-config + lightningd lightning-autocleaninvoice lightning-check lightning-checkmessage @@ -64,6 +64,7 @@ c-lightning Documentation lightning-txprepare lightning-txsend lightning-waitanyinvoice + lightning-waitblockheight lightning-waitinvoice lightning-waitsendpay lightning-withdraw diff --git a/doc/lightning-waitblockheight.7 b/doc/lightning-waitblockheight.7 new file mode 100644 index 000000000000..72f88f796f8f --- /dev/null +++ b/doc/lightning-waitblockheight.7 @@ -0,0 +1,36 @@ +.TH "LIGHTNING-WAITBLOCKHEIGHT" "7" "" "" "lightning-waitblockheight" +.SH NAME +lightning-waitblockheight -- Command for waiting for blocks on the blockchain + +lightning-waitblockheight -- Command for waiting for blocks on the blockchain + +.SH SYNOPSIS + +\fBwaitblockheight\fR \fIblockheight\fR [\fItimeout\fR] + +.SH DESCRIPTION + +The \fBwaitblockheight\fR RPC command waits until the blockchain +has reached the specified \fIblockheight\fR. +It will only wait up to \fItimeout\fR seconds (default 60). + +If the \fIblockheight\fR is a present or past block height, then this +command returns immediately. + +.SH RETURN VALUE + +Once the specified block height has been achieved by the blockchain, +an object with the single field \fIblockheight\fR is returned, which is +the block height at the time the command returns. + +If \fItimeout\fR seconds is reached without the specified blockheight +being reached, this command will fail. + +.SH AUTHOR + +ZmnSCPxj <\fIZmnSCPxj@protonmail.com\fR> is mainly responsible. + +.SH RESOURCES + +Main web site: \fIhttps://github.com/ElementsProject/lightning\fR + diff --git a/doc/lightning-waitblockheight.7.md b/doc/lightning-waitblockheight.7.md new file mode 100644 index 000000000000..0e260b6f1bc7 --- /dev/null +++ b/doc/lightning-waitblockheight.7.md @@ -0,0 +1,37 @@ +lightning-waitblockheight -- Command for waiting for blocks on the blockchain +============================================================================= + +SYNOPSIS +-------- + +**waitblockheight** *blockheight* \[*timeout*\] + +DESCRIPTION +----------- + +The **waitblockheight** RPC command waits until the blockchain +has reached the specified *blockheight*. +It will only wait up to *timeout* seconds (default 60). + +If the *blockheight* is a present or past block height, then this +command returns immediately. + +RETURN VALUE +------------ + +Once the specified block height has been achieved by the blockchain, +an object with the single field *blockheight* is returned, which is +the block height at the time the command returns. + +If *timeout* seconds is reached without the specified blockheight +being reached, this command will fail. + +AUTHOR +------ + +ZmnSCPxj <> is mainly responsible. + +RESOURCES +--------- + +Main web site: diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 0c5499e38966..f8247afc6237 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -185,6 +185,7 @@ static struct lightningd *new_lightningd(const tal_t *ctx) list_head_init(&ld->sendpay_commands); list_head_init(&ld->close_commands); list_head_init(&ld->ping_commands); + list_head_init(&ld->waitblockheight_commands); /*~ Tal also explicitly supports arrays: it stores the number of * elements, which can be accessed with tal_count() (or tal_bytelen() @@ -597,6 +598,7 @@ void notify_new_block(struct lightningd *ld, u32 block_height) htlcs_notify_new_block(ld, block_height); channel_notify_new_block(ld, block_height); gossip_notify_new_block(ld, block_height); + waitblockheight_notify_new_block(ld, block_height); } static void on_sigint(int _ UNUSED) diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index adfa8b573dad..240fa2e37c16 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -251,6 +251,9 @@ struct lightningd { bool encrypted_hsm; mode_t initial_umask; + + /* Outstanding waitblockheight commands. */ + struct list_head waitblockheight_commands; }; /* Turning this on allows a tal allocation to return NULL, rather than aborting. diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 1e1bc98c483a..4116dae7b16e 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -1750,6 +1751,112 @@ static const struct json_command getinfo_command = { }; AUTODATA(json_command, &getinfo_command); +/* Wait for at least a specific blockheight, then return, or time out. */ +struct waitblockheight_waiter { + /* struct lightningd::waitblockheight_commands. */ + struct list_node list; + /* Command structure. This is the parent of the close command. */ + struct command *cmd; + /* The block height being waited for. */ + u32 block_height; + /* Whether we have been removed from the list. */ + bool removed; +}; +/* Completes a pending waitblockheight. */ +static struct command_result * +waitblockheight_complete(struct command *cmd, + u32 block_height) +{ + struct json_stream *response; + + response = json_stream_success(cmd); + json_add_num(response, "blockheight", block_height); + return command_success(cmd, response); +} +/* Called when command is destroyed without being resolved. */ +static void +destroy_waitblockheight_waiter(struct waitblockheight_waiter *w) +{ + if (!w->removed) + list_del(&w->list); +} +/* Called on timeout. */ +static void +timeout_waitblockheight_waiter(struct waitblockheight_waiter *w) +{ + list_del(&w->list); + w->removed = true; + tal_steal(tmpctx, w); + was_pending(command_fail(w->cmd, LIGHTNINGD, + "Timed out.")); +} +/* Called by lightningd at each new block. */ +void waitblockheight_notify_new_block(struct lightningd *ld, + u32 block_height) +{ + struct waitblockheight_waiter *w, *n; + char *to_delete = tal(NULL, char); + + /* Use safe since we could resolve commands and thus + * trigger removal of list elements. + */ + list_for_each_safe(&ld->waitblockheight_commands, w, n, list) { + /* Skip commands that have not been reached yet. */ + if (w->block_height > block_height) + continue; + + list_del(&w->list); + w->removed = true; + tal_steal(to_delete, w); + was_pending(waitblockheight_complete(w->cmd, + block_height)); + } + tal_free(to_delete); +} +static struct command_result *json_waitblockheight(struct command *cmd, + const char *buffer, + const jsmntok_t *obj, + const jsmntok_t *params) +{ + unsigned int *target_block_height; + u32 block_height; + unsigned int *timeout; + struct waitblockheight_waiter *w; + + if (!param(cmd, buffer, params, + p_req("blockheight", param_number, &target_block_height), + p_opt_def("timeout", param_number, &timeout, 60), + NULL)) + return command_param_failed(); + + /* Check if already reached anyway. */ + block_height = get_block_height(cmd->ld->topology); + if (*target_block_height <= block_height) + return waitblockheight_complete(cmd, block_height); + + /* Create a new waitblockheight command. */ + w = tal(cmd, struct waitblockheight_waiter); + tal_add_destructor(w, &destroy_waitblockheight_waiter); + list_add(&cmd->ld->waitblockheight_commands, &w->list); + w->cmd = cmd; + w->block_height = *target_block_height; + w->removed = false; + /* Install the timeout. */ + (void) new_reltimer(cmd->ld->timers, w, time_from_sec(*timeout), + &timeout_waitblockheight_waiter, w); + + return command_still_pending(cmd); +} + +static const struct json_command waitblockheight_command = { + "waitblockheight", + "utility", + &json_waitblockheight, + "Wait for the blockchain to reach {blockheight}, up to " + "{timeout} seconds." +}; +AUTODATA(json_command, &waitblockheight_command); + static struct command_result *param_channel_or_all(struct command *cmd, const char *name, const char *buffer, diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index 68e8fb60c345..100756699027 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -95,4 +95,8 @@ struct htlc_in_map *load_channels_from_wallet(struct lightningd *ld); void peer_dev_memleak(struct command *cmd); #endif /* DEVELOPER */ +/* Triggered at each new block. */ +void waitblockheight_notify_new_block(struct lightningd *ld, + u32 block_height); + #endif /* LIGHTNING_LIGHTNINGD_PEER_CONTROL_H */ diff --git a/lightningd/test/run-find_my_abspath.c b/lightningd/test/run-find_my_abspath.c index deb55d49301c..9534db7d0f8a 100644 --- a/lightningd/test/run-find_my_abspath.c +++ b/lightningd/test/run-find_my_abspath.c @@ -192,6 +192,9 @@ bool wallet_network_check(struct wallet *w UNNEEDED) /* Generated stub for wallet_new */ struct wallet *wallet_new(struct lightningd *ld UNNEEDED, struct timers *timers UNNEEDED) { fprintf(stderr, "wallet_new called!\n"); abort(); } +/* Generated stub for waitblockheight_notify_new_block */ +void waitblockheight_notify_new_block(struct lightningd *ld UNNEEDED, u32 blockheight UNNEEDED) +{ fprintf(stderr, "waitblockheight_notify_new_block called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ struct log *crashlog; diff --git a/tests/test_misc.py b/tests/test_misc.py index a0b4e4bde8f8..ea40d3e10902 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1993,7 +1993,7 @@ def test_new_node_is_mainnet(node_factory): assert os.path.isfile(os.path.join(basedir, "lightningd-bitcoin.pid")) -def test_unicode_rpc(node_factory): +def test_unicode_rpc(node_factory, executor, bitcoind): node = node_factory.get_node() desc = "Some candy 🍬 and a nice glass of milk 🥛." @@ -2019,3 +2019,41 @@ def test_unix_socket_path_length(node_factory, bitcoind, directory, executor, db # Let's just call it again to make sure it really works. l1.rpc.listconfigs() l1.stop() + + +def test_waitblockheight(node_factory, executor, bitcoind): + node = node_factory.get_node() + + sync_blockheight(bitcoind, [node]) + + blockheight = node.rpc.getinfo()['blockheight'] + + # Should succeed without waiting. + node.rpc.waitblockheight(blockheight - 2) + node.rpc.waitblockheight(blockheight - 1) + node.rpc.waitblockheight(blockheight) + + # Should not succeed yet. + fut2 = executor.submit(node.rpc.waitblockheight, blockheight + 2) + fut1 = executor.submit(node.rpc.waitblockheight, blockheight + 1) + assert not fut1.done() + assert not fut2.done() + + # Should take about ~1second and time out. + with pytest.raises(RpcError): + node.rpc.waitblockheight(blockheight + 2, 1) + + # Others should still not be done. + assert not fut1.done() + assert not fut2.done() + + # Trigger just one more block. + bitcoind.generate_block(1) + sync_blockheight(bitcoind, [node]) + fut1.result(5) + assert not fut2.done() + + # Trigger two blocks. + bitcoind.generate_block(1) + sync_blockheight(bitcoind, [node]) + fut2.result(5) From 2d58e10a255300bb354d02545dfa6fb1a56386e6 Mon Sep 17 00:00:00 2001 From: ZmnSCPxj Date: Mon, 23 Dec 2019 16:32:43 +0000 Subject: [PATCH 3/6] pay: Factor out execution of `getroute` from starting of payment attempt. --- plugins/pay.c | 56 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/plugins/pay.c b/plugins/pay.c index b53dc9b71ad0..5fb5291ef7c5 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -724,35 +724,18 @@ static const char **dup_excludes(const tal_t *ctx, const char **excludes) return ret; } -static struct command_result *start_pay_attempt(struct command *cmd, - struct pay_command *pc, - const char *fmt, ...) +/* Get a route from the lightningd. */ +static struct command_result *execute_getroute(struct command *cmd, + struct pay_command *pc) { + struct pay_attempt *attempt = current_attempt(pc); + + u32 max_hops = ROUTING_MAX_HOPS; struct amount_msat msat; const char *dest; - u32 max_hops = ROUTING_MAX_HOPS; u32 cltv; - struct pay_attempt *attempt; - va_list ap; - size_t n; struct json_out *params; - n = tal_count(pc->ps->attempts); - tal_resize(&pc->ps->attempts, n+1); - attempt = &pc->ps->attempts[n]; - - va_start(ap, fmt); - attempt->start = time_now(); - /* Mark it unfinished */ - attempt->end.ts.tv_sec = -1; - attempt->excludes = dup_excludes(pc->ps, pc->excludes); - attempt->route = NULL; - attempt->failure = NULL; - attempt->result = NULL; - attempt->sendpay = false; - attempt->why = tal_vfmt(pc->ps, fmt, ap); - va_end(ap); - /* routehint set below. */ /* If we have a routehint, try that first; we need to do extra @@ -804,6 +787,33 @@ static struct command_result *start_pay_attempt(struct command *cmd, take(params)); } +static struct command_result *start_pay_attempt(struct command *cmd, + struct pay_command *pc, + const char *fmt, ...) +{ + struct pay_attempt *attempt; + va_list ap; + size_t n; + + n = tal_count(pc->ps->attempts); + tal_resize(&pc->ps->attempts, n+1); + attempt = &pc->ps->attempts[n]; + + va_start(ap, fmt); + attempt->start = time_now(); + /* Mark it unfinished */ + attempt->end.ts.tv_sec = -1; + attempt->excludes = dup_excludes(pc->ps, pc->excludes); + attempt->route = NULL; + attempt->failure = NULL; + attempt->result = NULL; + attempt->sendpay = false; + attempt->why = tal_vfmt(pc->ps, fmt, ap); + va_end(ap); + + return execute_getroute(cmd, pc); +} + /* BOLT #7: * * If a route is computed by simply routing to the intended recipient and From 5ca37274dd27ad5681e8751b88cb820ef6ff84f3 Mon Sep 17 00:00:00 2001 From: ZmnSCPxj Date: Mon, 23 Dec 2019 17:49:30 +0000 Subject: [PATCH 4/6] pay: Implement tracking of blockheight for starting payment attempts. --- plugins/pay.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/plugins/pay.c b/plugins/pay.c index 5fb5291ef7c5..6ad4bf5a8fcd 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -37,6 +37,9 @@ struct pay_attempt { struct json_out *failure; /* The non-failure result (NULL on failure) */ const char *result; + /* The blockheight at which the payment attempt was + * started. */ + u32 start_block; }; struct pay_status { @@ -787,6 +790,54 @@ static struct command_result *execute_getroute(struct command *cmd, take(params)); } +static struct command_result * +getstartblockheight_done(struct command *cmd, + const char *buf, + const jsmntok_t *result, + struct pay_command *pc) +{ + const jsmntok_t *blockheight_tok; + u64 blockheight; + + blockheight_tok = json_get_member(buf, result, "blockheight"); + if (!blockheight_tok) + plugin_err("getstartblockheight: " + "getinfo gave no 'blockheight'? '%.*s'", + result->end - result->start, buf); + + if (!json_to_u64(buf, blockheight_tok, &blockheight)) + plugin_err("getstartblockheight: " + "getinfo gave non-number 'blockheight'? '%.*s'", + result->end - result->start, buf); + + current_attempt(pc)->start_block = (u32) blockheight; + assert(((u64) current_attempt(pc)->start_block) == blockheight); + + return execute_getroute(cmd, pc); +} + +static struct command_result * +getstartblockheight_error(struct command *cmd, + const char *buf, + const jsmntok_t *error, + struct pay_command *pc) +{ + /* Should never happen. */ + plugin_err("getstartblockheight: getinfo failed!? '%.*s'", + error->end - error->start, buf); +} + +static struct command_result * +execute_getstartblockheight(struct command *cmd, + struct pay_command *pc) +{ + return send_outreq(cmd, "getinfo", + &getstartblockheight_done, + &getstartblockheight_error, + pc, + take(json_out_obj(NULL, NULL, NULL))); +} + static struct command_result *start_pay_attempt(struct command *cmd, struct pay_command *pc, const char *fmt, ...) @@ -811,7 +862,7 @@ static struct command_result *start_pay_attempt(struct command *cmd, attempt->why = tal_vfmt(pc->ps, fmt, ap); va_end(ap); - return execute_getroute(cmd, pc); + return execute_getstartblockheight(cmd, pc); } /* BOLT #7: From 637ec60b7236af488fae367f206543a4dd8a8e73 Mon Sep 17 00:00:00 2001 From: ZmnSCPxj Date: Sat, 4 Jan 2020 02:01:37 +0000 Subject: [PATCH 5/6] test_pay.py: Add test for blockheight disagreement. --- tests/test_pay.py | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tests/test_pay.py b/tests/test_pay.py index 9493431098ff..8445c122e412 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -2744,3 +2744,57 @@ def test_createonion_limits(node_factory): with pytest.raises(RpcError, match=r'Payloads exceed maximum onion packet size.'): hops[0]['payload'] += '01' l1.rpc.createonion(hops=hops, assocdata="BB" * 32) + + +@pytest.mark.xfail(strict=True) +@unittest.skipIf(not DEVELOPER, "needs use_shadow") +def test_blockheight_disagreement(node_factory, bitcoind, executor): + """ + While a payment is in-transit from payer to payee, a block + might be mined, so that the blockheight the payer used to + initiate the payment is no longer the blockheight when the + payee receives it. + This leads to a failure which *used* to be + `final_expiry_too_soon`, a non-permanent failure, but + which is *now* `incorrect_or_unknown_payment_details`, + a permanent failure. + `pay` treats permanent failures as, well, permanent, and + gives up on receiving such failure from the payee, but + this particular subcase of blockheight disagreement is + actually a non-permanent failure (the payer only needs + to synchronize to the same blockheight as the payee). + """ + l1, l2 = node_factory.line_graph(2) + + sync_blockheight(bitcoind, [l1, l2]) + + # Arrange l1 to stop getting new blocks. + def no_more_blocks(req): + return {"result": None, + "error": {"code": -8, "message": "Block height out of range"}, "id": req['id']} + l1.daemon.rpcproxy.mock_rpc('getblockhash', no_more_blocks) + + # Increase blockheight and make sure l2 knows it. + # Why 2? Because `pay` uses min_final_cltv_expiry + 1. + # But 2 blocks coming in close succession, plus slow + # forwarding nodes and block propagation, are still + # possible on the mainnet, thus this test. + bitcoind.generate_block(2) + sync_blockheight(bitcoind, [l2]) + + # Have l2 make an invoice. + inv = l2.rpc.invoice(1000, 'l', 'd')['bolt11'] + + # Have l1 pay l2 + def pay(l1, inv): + l1.rpc.dev_pay(inv, use_shadow=False) + fut = executor.submit(pay, l1, inv) + + # Make sure l1 sends out the HTLC. + l1.daemon.wait_for_logs([r'NEW:: HTLC LOCAL']) + + # Unblock l1 from new blocks. + l1.daemon.rpcproxy.mock_rpc('getblockhash', None) + + # pay command should complete without error + fut.result() From 5b7c215eb9d1accd66c724c1adfd000d0995d2cd Mon Sep 17 00:00:00 2001 From: ZmnSCPxj Date: Mon, 23 Dec 2019 18:38:22 +0000 Subject: [PATCH 6/6] pay: Implement retry in case of final CLTV being too soon for receiver. Changelog-Fixed: Detect a previously non-permanent error (`final_cltv_too_soon`) that has been merged into a permanent error (`incorrect_or_unknown_payment_details`), and retry that failure case in `pay`. --- plugins/pay.c | 168 +++++++++++++++++++++++++++++++++++++++++++--- tests/test_pay.py | 1 - 2 files changed, 158 insertions(+), 11 deletions(-) diff --git a/plugins/pay.c b/plugins/pay.c index 6ad4bf5a8fcd..bc7a706ec308 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -14,6 +14,7 @@ #include #include #include +#include /* Public key of this node. */ static struct node_id my_id; @@ -323,11 +324,106 @@ static struct command_result *next_routehint(struct command *cmd, "Could not find a route"); } +static struct command_result * +waitblockheight_done(struct command *cmd, + const char *buf UNUSED, + const jsmntok_t *result UNUSED, + struct pay_command *pc) +{ + return start_pay_attempt(cmd, pc, + "Retried due to blockheight " + "disagreement with payee"); +} +static struct command_result * +waitblockheight_error(struct command *cmd, + const char *buf UNUSED, + const jsmntok_t *error UNUSED, + struct pay_command *pc) +{ + if (time_after(time_now(), pc->stoptime)) + return waitsendpay_expired(cmd, pc); + else + /* Ehhh just retry it. */ + return waitblockheight_done(cmd, buf, error, pc); +} + +static struct command_result * +execute_waitblockheight(struct command *cmd, + u32 blockheight, + struct pay_command *pc) +{ + struct json_out *params; + struct timeabs now = time_now(); + struct timerel remaining; + + if (time_after(now, pc->stoptime)) + return waitsendpay_expired(cmd, pc); + + remaining = time_between(pc->stoptime, now); + + params = json_out_new(tmpctx); + json_out_start(params, NULL, '{'); + json_out_add_u32(params, "blockheight", blockheight); + json_out_add_u64(params, "timeout", time_to_sec(remaining)); + json_out_end(params, '}'); + json_out_finished(params); + + return send_outreq(cmd, "waitblockheight", + &waitblockheight_done, + &waitblockheight_error, + pc, + params); +} + +/* Gets the remote height from a + * WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS + * failure. + * Return 0 if unable to find such a height. + */ +static u32 +get_remote_block_height(const char *buf, const jsmntok_t *error) +{ + const jsmntok_t *raw_message_tok; + const u8 *raw_message; + size_t raw_message_len; + u16 type; + + /* Is there even a raw_message? */ + raw_message_tok = json_delve(buf, error, ".data.raw_message"); + if (!raw_message_tok) + return 0; + if (raw_message_tok->type != JSMN_STRING) + return 0; + + raw_message = json_tok_bin_from_hex(tmpctx, buf, raw_message_tok); + if (!raw_message) + return 0; + + /* BOLT #4: + * + * 1. type: PERM|15 (`incorrect_or_unknown_payment_details`) + * 2. data: + * * [`u64`:`htlc_msat`] + * * [`u32`:`height`] + * + */ + raw_message_len = tal_count(raw_message); + + type = fromwire_u16(&raw_message, &raw_message_len); /* type */ + if (type != WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS) + return 0; + + (void) fromwire_u64(&raw_message, &raw_message_len); /* htlc_msat */ + + return fromwire_u32(&raw_message, &raw_message_len); /* height */ +} + static struct command_result *waitsendpay_error(struct command *cmd, const char *buf, const jsmntok_t *error, struct pay_command *pc) { + struct pay_attempt *attempt = current_attempt(pc); const jsmntok_t *codetok, *failcodetok, *nodeidtok, *scidtok, *dirtok; int code, failcode; bool node_err = false; @@ -339,6 +435,64 @@ static struct command_result *waitsendpay_error(struct command *cmd, plugin_err("waitsendpay error gave no 'code'? '%.*s'", error->end - error->start, buf + error->start); + if (code != PAY_UNPARSEABLE_ONION) { + failcodetok = json_delve(buf, error, ".data.failcode"); + if (!json_to_int(buf, failcodetok, &failcode)) + plugin_err("waitsendpay error gave no 'failcode'? '%.*s'", + error->end - error->start, buf + error->start); + } + + /* Special case for WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS. + * + * One possible trigger for this failure is that the receiver + * thinks the final timeout it gets is too near the future. + * + * For the most part, we respect the indicated `final_cltv` + * in the invoice, and our shadow routing feature also tends + * to give more timing budget to the receiver than the + * `final_cltv`. + * + * However, there is an edge case possible on real networks: + * + * * We send out a payment respecting the `final_cltv` of + * the receiver. + * * Miners mine a new block while the payment is in transit. + * * By the time the payment reaches the receiver, the + * payment violates the `final_cltv` because the receiver + * is now using a different basis blockheight. + * + * This is a transient error. + * Unfortunately, WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS + * is marked with the PERM bit. + * This means that we would give up on this since `waitsendpay` + * would return PAY_DESTINATION_PERM_FAIL instead of + * PAY_TRY_OTHER_ROUTE. + * Thus the `pay` plugin would not retry this case. + * + * Thus, we need to add this special-case checking here, where + * the blockheight when we started the pay attempt was not + * the same as what the payee reports. + * + * In the past this particular failure had its own failure code, + * equivalent to 17. + * In case the receiver is a really old software, we also + * special-case it here. + */ + if ((code != PAY_UNPARSEABLE_ONION) && + ((failcode == 17) || + ((failcode == WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS) && + (attempt->start_block < get_remote_block_height(buf, error))))) { + u32 target_blockheight; + + if (failcode == 17) + target_blockheight = attempt->start_block + 1; + else + target_blockheight = get_remote_block_height(buf, error); + + return execute_waitblockheight(cmd, target_blockheight, + pc); + } + /* FIXME: Handle PAY_UNPARSEABLE_ONION! */ /* Many error codes are final. */ @@ -346,11 +500,6 @@ static struct command_result *waitsendpay_error(struct command *cmd, return forward_error(cmd, buf, error, pc); } - failcodetok = json_delve(buf, error, ".data.failcode"); - if (!json_to_int(buf, failcodetok, &failcode)) - plugin_err("waitsendpay error gave no 'failcode'? '%.*s'", - error->end - error->start, buf + error->start); - if (failcode & NODE) { nodeidtok = json_delve(buf, error, ".data.erring_node"); if (!nodeidtok) @@ -797,7 +946,7 @@ getstartblockheight_done(struct command *cmd, struct pay_command *pc) { const jsmntok_t *blockheight_tok; - u64 blockheight; + u32 blockheight; blockheight_tok = json_get_member(buf, result, "blockheight"); if (!blockheight_tok) @@ -805,13 +954,12 @@ getstartblockheight_done(struct command *cmd, "getinfo gave no 'blockheight'? '%.*s'", result->end - result->start, buf); - if (!json_to_u64(buf, blockheight_tok, &blockheight)) + if (!json_to_u32(buf, blockheight_tok, &blockheight)) plugin_err("getstartblockheight: " - "getinfo gave non-number 'blockheight'? '%.*s'", + "getinfo gave non-unsigned-32-bit 'blockheight'? '%.*s'", result->end - result->start, buf); - current_attempt(pc)->start_block = (u32) blockheight; - assert(((u64) current_attempt(pc)->start_block) == blockheight); + current_attempt(pc)->start_block = blockheight; return execute_getroute(cmd, pc); } diff --git a/tests/test_pay.py b/tests/test_pay.py index 8445c122e412..567da67b4b17 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -2746,7 +2746,6 @@ def test_createonion_limits(node_factory): l1.rpc.createonion(hops=hops, assocdata="BB" * 32) -@pytest.mark.xfail(strict=True) @unittest.skipIf(not DEVELOPER, "needs use_shadow") def test_blockheight_disagreement(node_factory, bitcoind, executor): """