renepay: fine tuning test_renepay

- skip test on local htlc max limits which is not supported by askrene, on privacy
  leak issue.
- skip very extreme flow cases, where sats per HTLC in reserve must be taken
  into account.
- adjust the expected error response for messages coming from askrene.
- on getroutes failure return a command fail with the same error code as
  getroutes response.

Changelog-None

Signed-off-by: Lagrang3 <[email protected]>

Author: Lagrang3

plugins/renepay/mods.c

@@ -503,12 +503,20 @@ static struct command_result *getroutes_fail(struct command *cmd,
 	// 	return payment_fail(payment, PAY_STOPPED_RETRYING, "getroutes
 	// 	failed to find a feasible solution %s", explain_error(buf,
 	// tok));
-	const jsmntok_t *messtok = json_get_member(buf, tok, "message");
-	assert(messtok);
-	return payment_fail(
-	    payment, PAYMENT_PENDING,
-	    "getroutes failed to find a feasible solution: %.*s",
-	    json_tok_full_len(messtok), json_tok_full(buf, messtok));
+	enum jsonrpc_errcode errcode;
+	const char *msg;
+	const char *err =
+	    json_scan(tmpctx, buf, tok, "{code:%,message:%}",
+		      JSON_SCAN(json_to_jsonrpc_errcode, &errcode),
+		      JSON_SCAN_TAL(tmpctx, json_strdup, &msg));
+	if (err)
+		plugin_err(cmd->plugin,
+			   "Unable to parse getroutes error: %s, json: %.*s",
+			   err, json_tok_full_len(tok),
+			   json_tok_full(buf, tok));
+	return payment_fail(payment, errcode,
+			    "getroutes failed to find a feasible solution: %s",
+			    msg);
 }
 
 static struct command_result *getroutes_cb(struct payment *payment)

tests/test_renepay.py

@@ -105,13 +105,16 @@ def test_errors(node_factory, bitcoind):
     inv_deleted = l6.rpc.invoice(send_amount, "test_renepay2", "description2")["bolt11"]
     l6.rpc.delinvoice("test_renepay2", "unpaid")
 
-    failmsg = r"We don\'t have any channels"
+    # What happens if we don't have any channels?
+    # This would be a useful error message: failmsg = r"We don\'t have any channels"
+    failmsg = f"Unknown source node {l1.info['id']}"
     with pytest.raises(RpcError, match=failmsg):
         l1.rpc.call("renepay", {"invstring": inv})
     node_factory.join_nodes([l1, l2, l4], wait_for_announce=True, fundamount=1000000)
     node_factory.join_nodes([l1, l3, l5], wait_for_announce=True, fundamount=1000000)
 
-    failmsg = r"failed to find a feasible flow"
+    # What happens if the destination is unreacheable?
+    failmsg = r"There is no connection between source and destination"
     with pytest.raises(RpcError, match=failmsg):
         l1.rpc.call("renepay", {"invstring": inv})
 
@@ -256,6 +259,7 @@ def test_limits(node_factory):
     - CLTV delay is too high,
     - probability of success is too low.
     """
+    # FIXME: check error codes returned by askrene
     opts = [
         {"disable-mpp": None, "fee-base": 0, "fee-per-satoshi": 100},
     ]
@@ -272,32 +276,14 @@ def test_limits(node_factory):
     inv = l5.rpc.invoice("any", "any", "description")
     l3.rpc.call("pay", {"bolt11": inv["bolt11"], "amount_msat": 500000000})
 
-    # FIXME: pylightning should define these!
-    # PAY_STOPPED_RETRYING = 210
-    PAY_ROUTE_TOO_EXPENSIVE = 206
-
     inv = l6.rpc.invoice("any", "any", "description")
 
     # Fee too high.
-    failmsg = r"Fee exceeds our fee budget"
-    with pytest.raises(RpcError, match=failmsg) as err:
+    failmsg = r"Could not find route without excessive cost"
+    with pytest.raises(RpcError, match=failmsg):
         l1.rpc.call(
             "renepay", {"invstring": inv["bolt11"], "amount_msat": 1000000, "maxfee": 1}
         )
-    assert err.value.error["code"] == PAY_ROUTE_TOO_EXPENSIVE
-    # TODO(eduardo): which error code shall we use here?
-
-    # TODO(eduardo): shall we list attempts in renepay?
-    # status = l1.rpc.call('renepaystatus', {'invstring':inv['bolt11']})['paystatus'][0]['attempts']
-
-    failmsg = r"CLTV delay exceeds our CLTV budget"
-    # Delay too high.
-    with pytest.raises(RpcError, match=failmsg) as err:
-        l1.rpc.call(
-            "renepay",
-            {"invstring": inv["bolt11"], "amount_msat": 1000000, "maxdelay": 0},
-        )
-    assert err.value.error["code"] == PAY_ROUTE_TOO_EXPENSIVE
 
     inv2 = l6.rpc.invoice("800000sat", "inv2", "description")
     l1.rpc.call("renepay", {"invstring": inv2["bolt11"]})
@@ -640,6 +626,7 @@ def test_fees(node_factory):
     assert invoice["amount_received_msat"] == Millisatoshi("150000sat")
 
 
+@unittest.skip("Not supported by askrene")
 def test_local_htlcmax0(node_factory):
     """Testing a simple pay route when local channels have htlcmax=0."""
     l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True)
@@ -773,7 +760,8 @@ def test_privatechan(node_factory, bitcoind):
     assert invoice["amount_received_msat"] >= Millisatoshi("1000sat")
 
 
-@unittest.skipIf(TEST_NETWORK == 'liquid-regtest', "broken for some reason")
+# Skipping this test for now, we can make it pass with askrene.
+@unittest.skip
 def test_hardmpp2(node_factory, bitcoind):
     """Credits to @daywalker90 for this test case."""
     opts = {"disable-mpp": None, "fee-base": 0, "fee-per-satoshi": 10}