Renepay fixes

plugins/renepay/pay.c

@@ -295,11 +295,11 @@ static struct command_result *flow_sendpay_failed(struct command *cmd,
 		      "{code:%,message:%}",
 		      JSON_SCAN(json_to_jsonrpc_errcode, &errcode),
 		      JSON_SCAN_TAL(tmpctx, json_strdup, &msg))) {
-		plugin_err(cmd->plugin, "Bad fail from sendpay: %.*s",
+		plugin_err(pay_plugin->plugin, "Bad fail from sendpay: %.*s",
 			   json_tok_full_len(err), json_tok_full(buf, err));
 	}
 	if (errcode != PAY_TRY_OTHER_ROUTE)
-		plugin_err(cmd->plugin, "Strange error from sendpay: %.*s",
+		plugin_err(pay_plugin->plugin, "Strange error from sendpay: %.*s",
 			   json_tok_full_len(err), json_tok_full(buf, err));
 
 	/* There is no new knowledge from this kind of failure.
@@ -350,7 +350,19 @@ static void sendpay_new_flows(struct payment *p)
 		json_add_sha256(req->js, "payment_hash", &p->payment_hash);
 		json_add_secret(req->js, "payment_secret", p->payment_secret);
 
-		json_add_amount_msat(req->js, "amount_msat", p->amount);
+		/* FIXME: sendpay has a check that we don't total more than
+		 * the exact amount, if we're setting partid (i.e. MPP).  However,
+		 * we always set partid, and we add a shadow amount *if we've
+		 * only have one part*, so we have to use that amount here.
+		 *
+		 * The spec was loosened so you are actually allowed
+		 * to overpay, so this check is now overzealous. */
+		if (amount_msat_greater(payflow_delivered(pf), p->amount)) {
+			json_add_amount_msat(req->js, "amount_msat",
+					     payflow_delivered(pf));
+		} else {
+			json_add_amount_msat(req->js, "amount_msat", p->amount);
+		}
 
 		json_add_u64(req->js, "partid", pf->key.partid);
 

plugins/renepay/pay_flow.c

@@ -115,7 +115,7 @@ static u32 shadow_one_flow(const struct gossmap *gossmap,
 	/* We only create shadow for extra CLTV delays, *not* for
 	 * amounts.  This is because with MPP our amounts are random
 	 * looking already. */
-	for (hop = 0; hop < MAX_SHADOW_LEN && pseudorand(1); hop++) {
+	for (hop = 0; hop < MAX_SHADOW_LEN && pseudorand(2); hop++) {
 		/* Try for a believable channel up to 10 times, then stop */
 		for (size_t i = 0; i < 10; i++) {
 			struct amount_sat cap;
@@ -124,7 +124,7 @@ static u32 shadow_one_flow(const struct gossmap *gossmap,
 			if (!gossmap_chan_set(chans[hop], dirs[hop])
 			    || !gossmap_chan_get_capacity(gossmap, chans[hop], &cap)
 			    /* This test is approximate, since amount would differ */
-			    || amount_msat_less_sat(amount, cap)) {
+			    || amount_msat_greater_sat(amount, cap)) {
 				chans[hop] = NULL;
 				continue;
 			}
@@ -172,8 +172,8 @@ static bool add_to_amounts(const struct gossmap *gossmap,
 	for (int i = num-2; i >= 0; i--) {
 		amounts[i] = amounts[i+1];
 		if (!amount_msat_add_fee(&amounts[i],
-					 flow_edge(f, i)->base_fee,
-					 flow_edge(f, i)->proportional_fee))
+					 flow_edge(f, i+1)->base_fee,
+					 flow_edge(f, i+1)->proportional_fee))
 			return false;
 	}
 
@@ -322,7 +322,7 @@ static void convert_and_attach_flows(struct payment *payment,
 		pf->cltv_delays[plen-1] = final_cltvs[i];
 		for (int j = (int)plen-2; j >= 0; j--) {
 			pf->cltv_delays[j] = pf->cltv_delays[j+1]
-				+ f->path[j]->half[f->dirs[j]].delay;
+				+ f->path[j+1]->half[f->dirs[j+1]].delay;
 		}
 		pf->amounts = tal_steal(pf, f->amounts);
 		pf->success_prob = f->success_prob;

tests/test_renepay.py

@@ -18,6 +18,42 @@ def test_simple(node_factory):
     assert details['destination'] == l2.info['id']
 
 
+def test_direction_matters(node_factory):
+    '''Make sure we use correct delay and fees for the direction we're going.'''
+    l1, l2, l3 = node_factory.line_graph(3,
+                                         wait_for_announce=True,
+                                         opts=[{},
+                                               {'fee-base': 2000, 'fee-per-satoshi': 20, 'cltv-delta': 20},
+                                               {'fee-base': 3000, 'fee-per-satoshi': 30, 'cltv-delta': 30}])
+    inv = l3.rpc.invoice(123000, 'test_renepay', 'description')['bolt11']
+    details = l1.rpc.call('renepay', {'invstring': inv})
+    assert details['status'] == 'complete'
+    assert details['amount_msat'] == Millisatoshi(123000)
+    assert details['destination'] == l3.info['id']
+
+
+def test_shadow(node_factory):
+    '''Make sure we shadow correctly.'''
+    l1, l2, l3 = node_factory.line_graph(3,
+                                         wait_for_announce=True,
+                                         opts=[{},
+                                               {'fee-base': 2000, 'fee-per-satoshi': 20, 'cltv-delta': 20},
+                                               {'fee-base': 3000, 'fee-per-satoshi': 30, 'cltv-delta': 30}])
+
+    # Shadow doesn't always happen (50% chance)!
+    for i in range(20):
+        inv = l2.rpc.invoice(123000, f'test_renepay{i}', 'description')['bolt11']
+        details = l1.rpc.call('renepay', {'invstring': inv})
+        assert details['status'] == 'complete'
+        assert details['amount_msat'] == Millisatoshi(123000)
+        assert details['destination'] == l2.info['id']
+
+        line = l1.daemon.wait_for_log("No MPP, so added .*msat shadow fee")
+        if 'added 0msat' not in line:
+            break
+        assert i != 19
+
+
 def test_mpp(node_factory):
     '''Test paying a remote node using two routes.
     1----2----4