From cd35835c5a2c8c8b38569088c58af6bdc6eb38a7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 12 Dec 2019 10:21:36 +1030 Subject: [PATCH] sendpay/sendonion: add optional partid arg, finesse msatoshi argument. msatoshi was used to indicate the amount the invoice asked for, but for parallel sendpay it's required, as it allows our sanity check of limiting the total payments in flight, ie. it becomes 'total_msat'. There's a special case for sendonion, which always tells us the value is 0. Signed-off-by: Rusty Russell --- lightningd/pay.c | 34 +++++++++++++++------------ tests/test_pay.py | 58 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 14 deletions(-) diff --git a/lightningd/pay.c b/lightningd/pay.c index e4d83726b..8043b04cb 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -280,11 +280,11 @@ void payment_succeeded(struct lightningd *ld, struct htlc_out *hout, struct wallet_payment *payment; wallet_payment_set_status(ld->wallet, &hout->payment_hash, - /* FIXME: Set partid! */ 0, + hout->partid, PAYMENT_COMPLETE, rval); payment = wallet_payment_by_hash(tmpctx, ld->wallet, &hout->payment_hash, - /* FIXME: Set partid! */ 0); + hout->partid); assert(payment); tell_waiters_success(ld, &hout->payment_hash, payment); @@ -982,7 +982,7 @@ send_payment(struct lightningd *ld, final_tlv, route[i].amount, base_expiry + route[i].delay, - route[i].amount, payment_secret); + total_msat, payment_secret); if (!onion) { return command_fail(cmd, PAY_DESTINATION_PERM_FAIL, "Destination does not support" @@ -1094,6 +1094,7 @@ static struct command_result *json_sendonion(struct command *cmd, struct lightningd *ld = cmd->ld; const char *label; struct secret *path_secrets; + u64 *partid; if (!param(cmd, buffer, params, p_req("onion", param_bin_from_hex, &onion), @@ -1101,6 +1102,7 @@ static struct command_result *json_sendonion(struct command *cmd, p_req("payment_hash", param_sha256, &payment_hash), p_opt("label", param_escaped_string, &label), p_opt("shared_secrets", param_secrets_array, &path_secrets), + p_opt_def("partid", param_u64, &partid, 0), NULL)) return command_param_failed(); @@ -1112,7 +1114,7 @@ static struct command_result *json_sendonion(struct command *cmd, "with failcode=%d", failcode); - return send_payment_core(ld, cmd, payment_hash, /* FIXME: Set partid! */0, + return send_payment_core(ld, cmd, payment_hash, *partid, first_hop, AMOUNT_MSAT(0), AMOUNT_MSAT(0), label, NULL, &packet, NULL, NULL, NULL, path_secrets); @@ -1163,6 +1165,7 @@ static struct command_result *json_sendpay(struct command *cmd, struct route_hop *route; struct amount_msat *msat; const char *b11str, *label = NULL; + u64 *partid; struct secret *payment_secret; /* For generating help, give new-style. */ @@ -1175,6 +1178,7 @@ static struct command_result *json_sendpay(struct command *cmd, p_opt("bolt11", param_string, &b11str), p_opt("payment_secret", param_secret, &payment_secret), + p_opt_def("partid", param_u64, &partid, 0), NULL)) return command_param_failed(); } else if (params->type == JSMN_ARRAY) { @@ -1186,6 +1190,7 @@ static struct command_result *json_sendpay(struct command *cmd, p_opt("bolt11", param_string, &b11str), p_opt("payment_secret", param_secret, &payment_secret), + p_opt_def("partid", param_u64, &partid, 0), NULL)) return command_param_failed(); } else { @@ -1199,6 +1204,7 @@ static struct command_result *json_sendpay(struct command *cmd, p_opt("bolt11", param_string, &b11str), p_opt("payment_secret", param_secret, &payment_secret), + p_opt_def("partid", param_u64, &partid, 0), NULL)) return command_param_failed(); @@ -1265,20 +1271,21 @@ static struct command_result *json_sendpay(struct command *cmd, * requesting. The final hop amount is what we actually give, which can * be from the msatoshi to twice msatoshi. */ - /* if not: msatoshi <= finalhop.amount <= 2 * msatoshi, fail. */ + if (*partid && !msat) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "Must specify msatoshi with partid"); + + /* if not: finalhop.amount <= 2 * msatoshi, fail. */ if (msat) { struct amount_msat limit = route[routetok->size-1].amount; - if (amount_msat_less(*msat, limit)) + if (!amount_msat_add(&limit, limit, limit)) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "msatoshi %s less than final %s", - type_to_string(tmpctx, - struct amount_msat, - msat), + "Unbelievable final amount %s", type_to_string(tmpctx, struct amount_msat, &route[routetok->size-1].amount)); - limit.millisatoshis *= 2; /* Raw: sanity check */ + if (amount_msat_greater(*msat, limit)) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "msatoshi %s more than twice final %s", @@ -1299,10 +1306,9 @@ static struct command_result *json_sendpay(struct command *cmd, } #endif - return send_payment(cmd->ld, cmd, rhash, /* FIXME: Set partid! */ 0, + return send_payment(cmd->ld, cmd, rhash, *partid, route, - msat ? *msat : route[routetok->size-1].amount, - /* FIXME: Set total_msat! */ + route[routetok->size-1].amount, msat ? *msat : route[routetok->size-1].amount, label, b11str, payment_secret); } diff --git a/tests/test_pay.py b/tests/test_pay.py index cd20ed7c8..009a7cb4f 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -2567,3 +2567,61 @@ def test_sendonion_rpc(node_factory): except RpcError as e: assert(e.error['code'] == 204) assert(e.error['data']['raw_message'] == "400f00000000000003e80000006c") + + +@unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") +def test_partial_payment(node_factory, bitcoind, executor): + # We want to test two payments at the same time, before we send commit + l1, l2, l3, l4 = node_factory.get_nodes(4, [{}] + [{'disconnect': ['=WIRE_UPDATE_ADD_HTLC-nocommit']}] * 2 + [{}]) + + # Two routes to l4: one via l2, and one via l3. + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + l1.fund_channel(l2, 100000) + l1.rpc.connect(l3.info['id'], 'localhost', l3.port) + l1.fund_channel(l3, 100000) + l2.rpc.connect(l4.info['id'], 'localhost', l4.port) + scid24 = l2.fund_channel(l4, 100000) + l3.rpc.connect(l4.info['id'], 'localhost', l4.port) + scid34 = l3.fund_channel(l4, 100000) + bitcoind.generate_block(5) + + # Wait until l1 knows about all channels. + wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 8) + + inv = l4.rpc.invoice(1000, 'inv', 'inv') + paysecret = l4.rpc.decodepay(inv['bolt11'])['payment_secret'] + + # Separate routes for each part of the payment. + r134 = l1.rpc.getroute(l4.info['id'], 500, 1, exclude=[scid24 + '/0', scid24 + '/1'])['route'] + r124 = l1.rpc.getroute(l4.info['id'], 500, 1, exclude=[scid34 + '/0', scid34 + '/1'])['route'] + + # These can happen in parallel. + l1.rpc.call('sendpay', [r134, inv['payment_hash'], None, 1000, inv['bolt11'], paysecret, 1]) + + # Can't mix non-parallel payment! + with pytest.raises(RpcError, match=r'Already have parallel payment in progress'): + l1.rpc.call('sendpay', {'route': r124, + 'payment_hash': inv['payment_hash'], + 'msatoshi': 1000, + 'payment_secret': paysecret}) + + # It will not allow a parallel with different msatoshi! + with pytest.raises(RpcError, match=r'msatoshi was previously 1000msat, now 999msat'): + l1.rpc.call('sendpay', [r124, inv['payment_hash'], None, 999, inv['bolt11'], paysecret, 2]) + + # This will work fine. + l1.rpc.call('sendpay', [r124, inv['payment_hash'], None, 1000, inv['bolt11'], paysecret, 2]) + + # Any more would exceed total payment + with pytest.raises(RpcError, match=r'Already have 1000msat of 1000msat payments in progress'): + l1.rpc.call('sendpay', [r124, inv['payment_hash'], None, 1000, inv['bolt11'], paysecret, 3]) + + # But repeat is a NOOP. + l1.rpc.call('sendpay', [r124, inv['payment_hash'], None, 1000, inv['bolt11'], paysecret, 1]) + l1.rpc.call('sendpay', [r134, inv['payment_hash'], None, 1000, inv['bolt11'], paysecret, 2]) + + # Now continue, payments will fail because receiver doesn't do MPP. + l2.rpc.dev_reenable_commit(l4.info['id']) + l3.rpc.dev_reenable_commit(l4.info['id']) + + # FIXME: waitsendpay needs a 'partid' field.