diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 07c6cdf9ea5b..f2ed37960ad3 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -578,16 +578,14 @@ void json_add_u64(struct json_stream *result UNNEEDED, const char *fieldname UNN uint64_t value UNNEEDED) { fprintf(stderr, "json_add_u64 called!\n"); abort(); } /* Generated stub for json_add_uncommitted_channel */ -void json_add_uncommitted_channel(struct json_stream *response UNNEEDED, - const struct uncommitted_channel *uc UNNEEDED, - /* Only set for listpeerchannels */ - const struct peer *peer UNNEEDED) +void json_add_uncommitted_channel(struct json_stream *response UNNEEDED, + const struct uncommitted_channel *uc UNNEEDED, + const struct peer *peer UNNEEDED) { fprintf(stderr, "json_add_uncommitted_channel called!\n"); abort(); } /* Generated stub for json_add_unsaved_channel */ -void json_add_unsaved_channel(struct json_stream *response UNNEEDED, - const struct channel *channel UNNEEDED, - /* Only set for listpeerchannels */ - const struct peer *peer UNNEEDED) +void json_add_unsaved_channel(struct json_stream *response UNNEEDED, + const struct channel *channel UNNEEDED, + const struct peer *peer UNNEEDED) { fprintf(stderr, "json_add_unsaved_channel called!\n"); abort(); } /* Generated stub for json_array_end */ void json_array_end(struct json_stream *js UNNEEDED) @@ -671,14 +669,6 @@ struct jsonrpc_request *jsonrpc_request_start_( void kill_uncommitted_channel(struct uncommitted_channel *uc UNNEEDED, const char *why UNNEEDED) { fprintf(stderr, "kill_uncommitted_channel called!\n"); abort(); } -/* Generated stub for lightningd_deprecated_out_ok */ -bool lightningd_deprecated_out_ok(struct lightningd *ld UNNEEDED, - bool deprecated_apis UNNEEDED, - const char *subsys UNNEEDED, - const char *api UNNEEDED, - const char *start UNNEEDED, - const char *end UNNEEDED) -{ fprintf(stderr, "lightningd_deprecated_out_ok called!\n"); abort(); } /* Generated stub for lockin_complete */ void lockin_complete(struct channel *channel UNNEEDED, enum channel_state expected_state UNNEEDED) diff --git a/plugins/keysend.c b/plugins/keysend.c index 5930fc32dc72..5124a4980649 100644 --- a/plugins/keysend.c +++ b/plugins/keysend.c @@ -233,7 +233,6 @@ struct payment_modifier *pay_mods[] = { &shadowroute_pay_mod, &routehints_pay_mod, &exemptfee_pay_mod, - &waitblockheight_pay_mod, &retry_pay_mod, NULL, }; diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index 3794aa684cf9..59c3ce808ac3 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -118,6 +118,7 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd, p->invstring = parent->invstring; p->description = parent->description; p->mods = parent->mods; + p->chainlag = parent->chainlag; } else { assert(cmd != NULL); p->partid = 0; @@ -132,6 +133,7 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd, p->local_invreq_id = NULL; p->groupid = 0; p->mods = NULL; + p->chainlag = 0; } /* Initialize all modifier data so we can point to the fields when @@ -276,16 +278,53 @@ struct payment_tree_result payment_collect_result(struct payment *p) return res; } +static struct command_result *payment_waitblockheight_cb(struct command *cmd, + const char *buffer, + const jsmntok_t *toks, + struct payment *p) +{ + u32 syncheight; + json_scan(tmpctx, buffer, toks, "{blockheight:%}", + JSON_SCAN(json_to_u32, &syncheight)); + paymod_log(p, LOG_DBG, "waitblockheight reports syncheight=%d", + syncheight); + p->chainlag = p->start_block - syncheight; + if (p->chainlag > 0) + paymod_log(p, LOG_INFORM, + "Starting the payment with chainlag=%d " + "(syncheight=%d < headercount=%d)", + p->chainlag, syncheight, p->start_block); + + payment_continue(p); + return command_still_pending(cmd); +} + static struct command_result * payment_getblockheight_success(struct command *cmd, const char *buffer, const jsmntok_t *toks, struct payment *p) { - const jsmntok_t *blockheighttok = - json_get_member(buffer, toks, "blockheight"); - json_to_number(buffer, blockheighttok, &p->start_block); - payment_continue(p); + struct out_req *req; + u32 blockcount, headercount; + + json_scan(tmpctx, buffer, toks, "{blockcount:%,headercount:%}", + JSON_SCAN(json_to_u32, &blockcount), + JSON_SCAN(json_to_u32, &headercount)); + paymod_log(p, LOG_DBG, + "Received getchaininfo blockcount=%d, headercount=%d", + blockcount, headercount); + + p->start_block = headercount; + + /* Now we just need to ask `lightningd` what height it has + * synced up to, and we remember that as chainlag. */ + req = jsonrpc_request_start(p->plugin, NULL, "waitblockheight", + &payment_waitblockheight_cb, + &payment_rpc_failure, p); + json_add_u32(req->js, "blockheight", 0); + send_outreq(p->plugin, req); + return command_still_pending(cmd); } @@ -323,17 +362,15 @@ void payment_start_at_blockheight(struct payment *p, u32 blockheight) return payment_continue(p); } - /* `waitblockheight 0` can be used as a query for the current - * block height. - * This is slightly better than `getinfo` since `getinfo` - * counts the channels and addresses and pushes more data - * onto the RPC but all we care about is the blockheight. + /* Check with the backend what it believes the network's + * height to be. We'll base all of our offsets based on that + * height, allowing us to send while still syncing. */ struct out_req *req; - req = jsonrpc_request_start(p->plugin, NULL, "waitblockheight", + req = jsonrpc_request_start(p->plugin, NULL, "getchaininfo", &payment_getblockheight_success, &payment_rpc_failure, p); - json_add_u32(req->js, "blockheight", 0); + json_add_u32(req->js, "last_height", 0); send_outreq(p->plugin, req); } @@ -1640,6 +1677,13 @@ static struct command_result *payment_createonion_success(struct command *cmd, struct secret *secrets; struct payment *root = payment_root(p); + /* The delay on the first hop needs to be offset by chainlag, + * as it would otherwise use the current height in + * `lightningd`. All other hops have already been adjusted + * during the payload encoding. + */ + u32 delay = first->delay + p->chainlag; + p->createonion_response = json_to_createonion_response(p, buffer, toks); req = jsonrpc_request_start(p->plugin, NULL, "sendonion", @@ -1649,7 +1693,7 @@ static struct command_result *payment_createonion_success(struct command *cmd, json_object_start(req->js, "first_hop"); json_add_amount_msat(req->js, "amount_msat", first->amount); - json_add_num(req->js, "delay", first->delay); + json_add_num(req->js, "delay", delay); json_add_node_id(req->js, "id", &first->node_id); json_add_short_channel_id(req->js, "channel", first->scid); json_object_end(req->js); @@ -1711,6 +1755,9 @@ static void payment_add_hop_onion_payload(struct payment *p, const u8 *payment_metadata) { struct createonion_request *cr = p->createonion_request; + + /* The start_block takes chainlag into consideration, so no + * need to adjust it here. */ u32 cltv = p->start_block + next->delay + 1; u64 msat = next->amount.millisatoshis; /* Raw: TLV payload generation*/ struct tlv_field **fields; @@ -1724,6 +1771,7 @@ static void payment_add_hop_onion_payload(struct payment *p, fields = &dst->tlv_payload->fields; tlvstream_set_tu64(fields, TLV_PAYLOAD_AMT_TO_FORWARD, msat); + tlvstream_set_tu32(fields, TLV_PAYLOAD_OUTGOING_CLTV_VALUE, cltv); @@ -3419,100 +3467,6 @@ static struct direct_pay_data *direct_pay_init(struct payment *p) REGISTER_PAYMENT_MODIFIER(directpay, struct direct_pay_data *, direct_pay_init, direct_pay_cb); -static struct command_result *waitblockheight_rpc_cb(struct command *cmd, - const char *buffer, - const jsmntok_t *toks, - struct payment *p) -{ - const jsmntok_t *blockheighttok, *codetok; - - u32 blockheight; - int code; - struct payment *subpayment; - - blockheighttok = json_get_member(buffer, toks, "blockheight"); - - if (!blockheighttok || - !json_to_number(buffer, blockheighttok, &blockheight)) { - codetok = json_get_member(buffer, toks, "code"); - json_to_int(buffer, codetok, &code); - if (code == WAIT_TIMEOUT) { - payment_fail( - p, - "Timed out while attempting to sync to blockheight " - "returned by destination. Please finish syncing " - "with the blockchain and try again."); - - } else { - plugin_err( - p->plugin, - "Unexpected result from waitblockheight: %.*s", - json_tok_full_len(toks), - json_tok_full(buffer, toks)); - } - } else { - subpayment = payment_new(p, NULL, p, p->modifiers); - payment_start_at_blockheight(subpayment, blockheight); - payment_set_step(p, PAYMENT_STEP_RETRY); - subpayment->why = tal_fmt( - subpayment, "Retrying after waiting for blockchain sync."); - paymod_log( - p, LOG_DBG, - "Retrying after waitblockheight, new partid %" PRIu32, - subpayment->partid); - payment_continue(p); - } - return command_still_pending(cmd); -} - -static void waitblockheight_cb(void *d, struct payment *p) -{ - struct out_req *req; - struct timeabs now = time_now(); - struct timerel remaining; - u32 blockheight; - if (p->step != PAYMENT_STEP_FAILED) - return payment_continue(p); - - /* If we don't have an error message to parse we can't wait for blockheight. */ - if (p->result == NULL) - return payment_continue(p); - - /* Check if we'd be waiting more than 0 seconds. If we have - * less than a second then waitblockheight would return - * immediately resulting in a loop. */ - if (time_after(now, p->deadline)) - return payment_continue(p); - - remaining = time_between(p->deadline, now); - if (time_to_sec(remaining) < 1) - return payment_continue(p); - - /* *Was* it a blockheight disagreement that caused the failure? */ - if (!failure_is_blockheight_disagreement(p, &blockheight)) - return payment_continue(p); - - paymod_log(p, LOG_INFORM, - "Remote node appears to be on a longer chain, which causes " - "CLTV timeouts to be incorrect. Waiting up to %" PRIu64 - " seconds to catch up to block %d before retrying.", - time_to_sec(remaining), blockheight); - - /* Set temporarily set the state of the payment to not failed, so - * interim status queries don't show this as terminally failed. We're - * in control for this payment so nobody else could be fooled by - * this. The callback will set it to retry anyway. */ - payment_set_step(p, PAYMENT_STEP_RETRY); - - req = jsonrpc_request_start(p->plugin, NULL, "waitblockheight", - waitblockheight_rpc_cb, - waitblockheight_rpc_cb, p); - json_add_u32(req->js, "blockheight", blockheight); - json_add_u32(req->js, "timeout", time_to_sec(remaining)); - send_outreq(p->plugin, req); -} - -REGISTER_PAYMENT_MODIFIER(waitblockheight, void *, NULL, waitblockheight_cb); static u32 payment_max_htlcs(const struct payment *p) { diff --git a/plugins/libplugin-pay.h b/plugins/libplugin-pay.h index f5247ae7bcbf..24048ee16eee 100644 --- a/plugins/libplugin-pay.h +++ b/plugins/libplugin-pay.h @@ -326,6 +326,11 @@ struct payment { * explanation if a payment is aborted. */ char *aborterror; + /* How many blocks are we lagging behind the rest of the + network? This needs to be taken into consideration when + sending payments before being fully caught up.*/ + u32 chainlag; + /* Callback to be called when the entire payment process * completes successfully. */ void (*on_payment_success)(struct payment *p); @@ -445,7 +450,6 @@ REGISTER_PAYMENT_MODIFIER_HEADER(routehints, struct routehints_data); REGISTER_PAYMENT_MODIFIER_HEADER(exemptfee, struct exemptfee_data); REGISTER_PAYMENT_MODIFIER_HEADER(shadowroute, struct shadow_route_data); REGISTER_PAYMENT_MODIFIER_HEADER(directpay, struct direct_pay_data); -extern struct payment_modifier waitblockheight_pay_mod; REGISTER_PAYMENT_MODIFIER_HEADER(presplit, struct presplit_mod_data); REGISTER_PAYMENT_MODIFIER_HEADER(adaptive_splitter, struct adaptive_split_mod_data); diff --git a/plugins/pay.c b/plugins/pay.c index e44897105d2d..d03ac854d4f6 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -999,7 +999,6 @@ struct payment_modifier *paymod_mods[] = { */ &routehints_pay_mod, &payee_incoming_limit_pay_mod, - &waitblockheight_pay_mod, &retry_pay_mod, &adaptive_splitter_pay_mod, NULL, diff --git a/plugins/test/run-route-calc.c b/plugins/test/run-route-calc.c index 5a8f74a0a9de..5cee7ad8e5c8 100644 --- a/plugins/test/run-route-calc.c +++ b/plugins/test/run-route-calc.c @@ -155,6 +155,13 @@ void json_object_end(struct json_stream *js UNNEEDED) /* Generated stub for json_object_start */ void json_object_start(struct json_stream *ks UNNEEDED, const char *fieldname UNNEEDED) { fprintf(stderr, "json_object_start called!\n"); abort(); } +/* Generated stub for json_scan */ +const char *json_scan(const tal_t *ctx UNNEEDED, + const char *buffer UNNEEDED, + const jsmntok_t *tok UNNEEDED, + const char *guide UNNEEDED, + ...) +{ fprintf(stderr, "json_scan called!\n"); abort(); } /* Generated stub for json_strdup */ char *json_strdup(const tal_t *ctx UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED) { fprintf(stderr, "json_strdup called!\n"); abort(); } diff --git a/plugins/test/run-route-overlong.c b/plugins/test/run-route-overlong.c index a6e0a5a00e22..3e55dc4cb7a7 100644 --- a/plugins/test/run-route-overlong.c +++ b/plugins/test/run-route-overlong.c @@ -152,6 +152,13 @@ void json_object_end(struct json_stream *js UNNEEDED) /* Generated stub for json_object_start */ void json_object_start(struct json_stream *ks UNNEEDED, const char *fieldname UNNEEDED) { fprintf(stderr, "json_object_start called!\n"); abort(); } +/* Generated stub for json_scan */ +const char *json_scan(const tal_t *ctx UNNEEDED, + const char *buffer UNNEEDED, + const jsmntok_t *tok UNNEEDED, + const char *guide UNNEEDED, + ...) +{ fprintf(stderr, "json_scan called!\n"); abort(); } /* Generated stub for json_strdup */ char *json_strdup(const tal_t *ctx UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED) { fprintf(stderr, "json_strdup called!\n"); abort(); } diff --git a/tests/test_pay.py b/tests/test_pay.py index e34eead9aea5..03398513ee06 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -3405,61 +3405,6 @@ def test_createonion_limits(node_factory): ) -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.dev_pay(inv, dev_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']) - - height = bitcoind.rpc.getblockchaininfo()['blocks'] - l1.daemon.wait_for_log('Remote node appears to be on a longer chain.*catch up to block {}'.format(height)) - - # Unblock l1 from new blocks. - l1.daemon.rpcproxy.mock_rpc('getblockhash', None) - - # pay command should complete without error - fut.result() - - def test_sendpay_msatoshi_arg(node_factory): """sendpay msatoshi arg was used for non-MPP to indicate the amount they asked for. But using it with anything other than the final amount @@ -4778,6 +4723,48 @@ def test_fetchinvoice_autoconnect(node_factory, bitcoind): assert l3.rpc.listpeers(l2.info['id'])['peers'] != [] +def test_pay_blockheight_mismatch(node_factory, bitcoind): + """Test that we can send a payment even if not caught up with the chain. + + We removed the requirement for the node to be fully synced up with + the blockchain in v24.05, allowing us to send a payment while still + processing blocks. This test pins the sender at a lower height, + but `getnetworkinfo` still reports the correct height. Since CLTV + computations are based on headers and not our own sync height, the + recipient should still be happy with the parameters we chose. + + """ + + send, direct, recv = node_factory.line_graph(3, wait_for_announce=True) + sync_blockheight(bitcoind, [send, recv]) + + # Pin `send` at the current height. by not returning the next + # blockhash. This error is special-cased not to count as the + # backend failing since it is used to poll for the next block. + def mock_getblockhash(req): + return { + "id": req['id'], + "error": { + "code": -8, + "message": "Block height out of range" + } + } + + send.daemon.rpcproxy.mock_rpc('getblockhash', mock_getblockhash) + bitcoind.generate_block(100) + + sync_blockheight(bitcoind, [recv]) + + inv = recv.rpc.invoice(42, 'lbl', 'desc')['bolt11'] + send.rpc.pay(inv) + + # The direct_override payment modifier does some trickery on the + # route calculation, so we better ensure direct payments still + # work correctly. + inv = direct.rpc.invoice(13, 'lbl', 'desc')['bolt11'] + send.rpc.pay(inv) + + def test_pay_waitblockheight_timeout(node_factory, bitcoind): plugin = os.path.join(os.path.dirname(__file__), 'plugins', 'endlesswaitblockheight.py') l1, l2 = node_factory.line_graph(2, opts=[{}, {'plugin': plugin}]) diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index c3a3c65a1de4..d5170c062e70 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -128,12 +128,6 @@ struct command_result *command_check_done(struct command *cmd) /* Generated stub for command_check_only */ bool command_check_only(const struct command *cmd UNNEEDED) { fprintf(stderr, "command_check_only called!\n"); abort(); } -/* Generated stub for command_deprecated_in_ok */ -bool command_deprecated_in_ok(struct command *cmd UNNEEDED, - const char *param UNNEEDED, - const char *depr_start UNNEEDED, - const char *depr_end UNNEEDED) -{ fprintf(stderr, "command_deprecated_in_ok called!\n"); abort(); } /* Generated stub for command_fail */ struct command_result *command_fail(struct command *cmd UNNEEDED, enum jsonrpc_errcode code UNNEEDED, const char *fmt UNNEEDED, ...) @@ -516,16 +510,14 @@ void json_add_u64(struct json_stream *result UNNEEDED, const char *fieldname UNN uint64_t value UNNEEDED) { fprintf(stderr, "json_add_u64 called!\n"); abort(); } /* Generated stub for json_add_uncommitted_channel */ -void json_add_uncommitted_channel(struct json_stream *response UNNEEDED, - const struct uncommitted_channel *uc UNNEEDED, - /* Only set for listpeerchannels */ - const struct peer *peer UNNEEDED) +void json_add_uncommitted_channel(struct json_stream *response UNNEEDED, + const struct uncommitted_channel *uc UNNEEDED, + const struct peer *peer UNNEEDED) { fprintf(stderr, "json_add_uncommitted_channel called!\n"); abort(); } /* Generated stub for json_add_unsaved_channel */ -void json_add_unsaved_channel(struct json_stream *response UNNEEDED, - const struct channel *channel UNNEEDED, - /* Only set for listpeerchannels */ - const struct peer *peer UNNEEDED) +void json_add_unsaved_channel(struct json_stream *response UNNEEDED, + const struct channel *channel UNNEEDED, + const struct peer *peer UNNEEDED) { fprintf(stderr, "json_add_unsaved_channel called!\n"); abort(); } /* Generated stub for json_array_end */ void json_array_end(struct json_stream *js UNNEEDED) @@ -600,14 +592,6 @@ bool json_tok_streq(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, void kill_uncommitted_channel(struct uncommitted_channel *uc UNNEEDED, const char *why UNNEEDED) { fprintf(stderr, "kill_uncommitted_channel called!\n"); abort(); } -/* Generated stub for lightningd_deprecated_out_ok */ -bool lightningd_deprecated_out_ok(struct lightningd *ld UNNEEDED, - bool deprecated_apis UNNEEDED, - const char *subsys UNNEEDED, - const char *api UNNEEDED, - const char *start UNNEEDED, - const char *end UNNEEDED) -{ fprintf(stderr, "lightningd_deprecated_out_ok called!\n"); abort(); } /* Generated stub for lockin_complete */ void lockin_complete(struct channel *channel UNNEEDED, enum channel_state expected_state UNNEEDED)