diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index 0a788974c..a1b2b00df 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -32,6 +32,8 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd, p->payment_hash = parent->payment_hash; p->partid = payment_root(p->parent)->next_partid++; p->plugin = parent->plugin; + p->fee_budget = parent->fee_budget; + p->cltv_budget = parent->cltv_budget; } else { assert(cmd != NULL); p->partid = 0; @@ -220,10 +222,42 @@ static struct command_result *payment_getroute_result(struct command *cmd, struct payment *p) { const jsmntok_t *rtok = json_get_member(buffer, toks, "route"); + struct amount_msat fee; assert(rtok != NULL); p->route = tal_route_from_json(p, buffer, rtok); p->step = PAYMENT_STEP_GOT_ROUTE; + /* Ensure that our fee and CLTV budgets are respected. */ + + if (!amount_msat_sub(&fee, p->route[0].amount, p->amount)) { + plugin_err( + p->plugin, + "gossipd returned a route with a negative fee: sending %s " + "to deliver %s", + type_to_string(tmpctx, struct amount_msat, + &p->route[0].amount), + type_to_string(tmpctx, struct amount_msat, &p->amount)); + payment_fail(p); + return command_still_pending(cmd); + } + + if (amount_msat_greater(fee, p->fee_budget)) { + plugin_log(p->plugin, LOG_INFORM, + "Fee exceeds our fee budget: %s > %s, discarding route", + type_to_string(tmpctx, struct amount_msat, &fee), + type_to_string(tmpctx, struct amount_msat, &p->fee_budget)); + payment_fail(p); + return command_still_pending(cmd); + } + + if (p->route[0].delay > p->cltv_budget) { + plugin_log(p->plugin, LOG_INFORM, + "CLTV delay exceeds our CLTV budget: %d > %d", + p->route[0].delay, p->cltv_budget); + payment_fail(p); + return command_still_pending(cmd); + } + /* Allow modifiers to modify the route, before * payment_compute_onion_payloads uses the route to generate the * onion_payloads */ diff --git a/plugins/libplugin-pay.h b/plugins/libplugin-pay.h index 249ddb034..fca8cd748 100644 --- a/plugins/libplugin-pay.h +++ b/plugins/libplugin-pay.h @@ -201,7 +201,13 @@ struct payment { struct timeabs start_time, end_time; struct timeabs deadline; - struct amount_msat extra_budget; + /* Maximum remaining fees we're willing to pay to complete this + * (sub-)payment. */ + struct amount_msat fee_budget; + + /* Maximum end-to-end CLTV delta we're willing to wait for this + * (sub-)payment to complete. */ + u32 cltv_budget; struct short_channel_id *exclusions; diff --git a/plugins/pay.c b/plugins/pay.c index 804d42768..aec557bd3 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -1851,12 +1851,19 @@ static struct command_result *json_paymod(struct command *cmd, const char *b11str; struct bolt11 *b11; char *fail; + u64 *maxfee_pct_millionths; + u32 *maxdelay; + p = payment_new(NULL, cmd, NULL /* No parent */, paymod_mods); /* If any of the modifiers need to add params to the JSON-RPC call we * would add them to the `param()` call below, and have them be * initialized directly that way. */ if (!param(cmd, buf, params, p_req("bolt11", param_string, &b11str), + p_opt_def("maxdelay", param_number, &maxdelay, + maxdelay_default), + p_opt_def("maxfeepercent", param_millionths, + &maxfee_pct_millionths, 500000), NULL)) return command_param_failed(); @@ -1898,6 +1905,14 @@ static struct command_result *json_paymod(struct command *cmd, p->invoice = tal_steal(p, b11); p->bolt11 = tal_steal(p, b11str); p->why = "Initial attempt"; + p->cltv_budget = *maxdelay; + + if (!amount_msat_fee(&p->fee_budget, p->amount, 0, *maxfee_pct_millionths)) { + tal_free(p); + return command_fail( + cmd, JSONRPC2_INVALID_PARAMS, + "Overflow when computing fee budget, fee rate too high."); + } payment_start(p); list_add_tail(&payments, &p->list); diff --git a/tests/test_pay.py b/tests/test_pay.py index 5bdf1eb3e..10aff37b2 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -1678,8 +1678,15 @@ def test_pay_routeboost(node_factory, bitcoind): l1, l2 = node_factory.line_graph(2, announce_channels=True, wait_for_announce=True) l3, l4, l5 = node_factory.line_graph(3, announce_channels=False, wait_for_announce=False) + # COMPAT_V090 made the return value of pay and paystatus more consistent, + # but broke tests relying on the inconsistent interface. Remove this once + # COMPAT_V090 gets removed. + COMPAT_V090 = env('COMPAT') == '1' + PAY_ROUTE_NOT_FOUND = 205 + # This should a "could not find a route" because that's true. - with pytest.raises(RpcError, match=r'Could not find a route'): + error = r'Could not find a route' if COMPAT_V090 else r'Ran out of routes' + with pytest.raises(RpcError, match=error): l1.rpc.pay(l5.rpc.invoice(10**8, 'test_retry', 'test_retry')['bolt11']) l2.rpc.connect(l3.info['id'], 'localhost', l3.port) @@ -1718,25 +1725,33 @@ def test_pay_routeboost(node_factory, bitcoind): assert 'label' not in only_one(status['pay']) assert 'routehint_modifications' not in only_one(status['pay']) assert 'local_exclusions' not in only_one(status['pay']) - # First attempt will fail, then it will try route hint attempts = only_one(status['pay'])['attempts'] - assert len(attempts) == 2 - assert attempts[0]['strategy'] == "Initial attempt" - # FIXME! - PAY_ROUTE_NOT_FOUND = 205 - assert attempts[0]['failure']['code'] == PAY_ROUTE_NOT_FOUND - assert attempts[1]['strategy'] == "Trying route hint" - assert 'success' in attempts[1] - assert attempts[1]['age_in_seconds'] <= time.time() - start - assert attempts[1]['duration_in_seconds'] <= end - start - assert only_one(attempts[1]['routehint']) - assert only_one(attempts[1]['routehint'])['id'] == l3.info['id'] scid34 = only_one(l3.rpc.listpeers(l4.info['id'])['peers'])['channels'][0]['short_channel_id'] - assert only_one(attempts[1]['routehint'])['channel'] == scid34 - assert only_one(attempts[1]['routehint'])['fee_base_msat'] == 1 - assert only_one(attempts[1]['routehint'])['fee_proportional_millionths'] == 10 - assert only_one(attempts[1]['routehint'])['cltv_expiry_delta'] == 6 - + if COMPAT_V090: + assert len(attempts) == 2 + assert attempts[0]['strategy'] == "Initial attempt" + # FIXME! + assert attempts[0]['failure']['code'] == PAY_ROUTE_NOT_FOUND + assert attempts[1]['strategy'] == "Trying route hint" + assert 'success' in attempts[1] + assert attempts[1]['age_in_seconds'] <= time.time() - start + assert attempts[1]['duration_in_seconds'] <= end - start + assert only_one(attempts[1]['routehint']) + assert only_one(attempts[1]['routehint'])['id'] == l3.info['id'] + assert only_one(attempts[1]['routehint'])['channel'] == scid34 + assert only_one(attempts[1]['routehint'])['fee_base_msat'] == 1 + assert only_one(attempts[1]['routehint'])['fee_proportional_millionths'] == 10 + assert only_one(attempts[1]['routehint'])['cltv_expiry_delta'] == 6 + else: + # The behavior changed to try with the route hint first, so we end up + # with a single successful attempt: + assert(len(attempts) == 1) + a = attempts[0] + assert(a['strategy'] == "Initial attempt") + assert('success' in a) + assert('payment_preimage' in a['success']) + + print("XXX longer route developeronly") # With dev-route option we can test longer routehints. if DEVELOPER: scid45 = only_one(l4.rpc.listpeers(l5.info['id'])['peers'])['channels'][0]['short_channel_id'] @@ -1756,11 +1771,18 @@ def test_pay_routeboost(node_factory, bitcoind): 'dev-routes': [routel3l4l5]}) l1.rpc.dev_pay(inv['bolt11'], use_shadow=False) status = l1.rpc.call('paystatus', [inv['bolt11']]) - assert len(only_one(status['pay'])['attempts']) == 2 - assert 'failure' in only_one(status['pay'])['attempts'][0] - assert 'success' not in only_one(status['pay'])['attempts'][0] - assert 'failure' not in only_one(status['pay'])['attempts'][1] - assert 'success' in only_one(status['pay'])['attempts'][1] + pay = only_one(status['pay']) + attempts = pay['attempts'] + if COMPAT_V090: + assert len(pay['attempts']) == 2 + assert 'failure' in attempts[0] + assert 'success' not in attempts[0] + assert 'failure' not in attempts[1] + assert 'success' in attempts[1] + else: + assert(len(attempts) == 1) + assert 'failure' not in attempts[0] + assert 'success' in attempts[0] # Finally, it should fall back to second routehint if first fails. # (Note, this is not public because it's not 6 deep) @@ -3055,9 +3077,15 @@ def test_pay_modifiers(node_factory): # Make sure that the dummy param is in the help (and therefore assigned to # the modifier data). hlp = l1.rpc.help("paymod")['help'][0] - assert(hlp['command'] == 'paymod bolt11') + assert(hlp['command'] == 'paymod bolt11 [maxdelay] [maxfeepercent]') inv = l2.rpc.invoice(123, 'lbl', 'desc')['bolt11'] r = l1.rpc.paymod(inv) assert(r['status'] == 'complete') assert(sha256(unhexlify(r['payment_preimage'])).hexdigest() == r['payment_hash']) + + +def test_pay_exemptfee(node_factory): + """Tiny payment, huge fee + """ + l1, l2, l3 = node_factory.line_graph(3)