paymod: Compute fee and CLTV delta limits for a payment

So far we got away with not caring about these but since we're implementing
modifiers that impact these limits, we better keep track of them.

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 */

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;
 

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);
 

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)