Protocol: make var_onion, payment_secret and basic_mpp non-EXPERIMENTAL.

Thanks to @t-bast, who made this possible by interop testing with Eclair! Changelog-Added: Protocol: can now send and receive TLV-style onion messages. Changelog-Added: Protocol: can now send and receive BOLT11 payment_secrets. Changelog-Added: Protocol: can now receive basic multi-part payments. Changelog-Added: RPC: low-level commands sendpay and waitsendpay can now be used to manually send multi-part payments. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-11-19 01:43:36 +01:00 · 2019-12-12 21:27:07 +10:30 · 2019-12-12 21:27:07 +10:30 · 839909d2cf
commit 839909d2cf
parent 2e97afd1ac
20 changed files with 95 additions and 187 deletions
--- a/channeld/channeld.c
+++ b/channeld/channeld.c
@ -960,11 +960,9 @@ static u8 *make_failmsg(const tal_t *ctx,
 		/* FIXME: wire this into tlv parser somehow. */
 		msg = towire_invalid_onion_payload(ctx, 0, 0);
 		goto done;
-#if EXPERIMENTAL_FEATURES
 	case WIRE_MPP_TIMEOUT:
 		msg = towire_mpp_timeout(ctx);
 		goto done;
-#endif /* EXPERIMENTAL_FEATURES */
 	}
 	status_failed(STATUS_FAIL_INTERNAL_ERROR,
 		      "Asked to create failmsg %u (%s)",
--- a/common/features.c
+++ b/common/features.c
@ -8,11 +8,9 @@ static const u32 our_features[] = {
 	OPTIONAL_FEATURE(OPT_DATA_LOSS_PROTECT),
 	OPTIONAL_FEATURE(OPT_UPFRONT_SHUTDOWN_SCRIPT),
 	OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES),
-#if EXPERIMENTAL_FEATURES
 	OPTIONAL_FEATURE(OPT_VAR_ONION),
 	OPTIONAL_FEATURE(OPT_PAYMENT_SECRET),
 	OPTIONAL_FEATURE(OPT_BASIC_MPP),
-#endif
 	OPTIONAL_FEATURE(OPT_GOSSIP_QUERIES_EX),
 	OPTIONAL_FEATURE(OPT_STATIC_REMOTEKEY),
 };
--- a/common/onion.c
+++ b/common/onion.c
@ -101,9 +101,7 @@ u8 *onion_final_hop(const tal_t *ctx,
 		struct tlv_tlv_payload *tlv = tlv_tlv_payload_new(tmpctx);
 		struct tlv_tlv_payload_amt_to_forward tlv_amt;
 		struct tlv_tlv_payload_outgoing_cltv_value tlv_cltv;
-#if EXPERIMENTAL_FEATURES
 		struct tlv_tlv_payload_payment_data tlv_pdata;
-#endif

 		/* BOLT #4:
 		 *
@ -118,17 +116,11 @@ u8 *onion_final_hop(const tal_t *ctx,
 		tlv->amt_to_forward = &tlv_amt;
 		tlv->outgoing_cltv_value = &tlv_cltv;

-#if EXPERIMENTAL_FEATURES
 		if (payment_secret) {
 			tlv_pdata.payment_secret = *payment_secret;
 			tlv_pdata.total_msat = total_msat.millisatoshis; /* Raw: TLV convert */
 			tlv->payment_data = &tlv_pdata;
 		}
-#else
-		/* Wihtout EXPERIMENTAL_FEATURES, we can't send payment_secret */
-		if (payment_secret)
-			return NULL;
-#endif
 		return make_tlv_hop(ctx, tlv);
 	} else {
 		static struct short_channel_id all_zero_scid;
@ -170,10 +162,6 @@ static bool pull_payload_length(const u8 **cursor,
 		return true;
 	}

-#if !EXPERIMENTAL_FEATURES
-	/* Only handle legacy format */
-	return false;
-#else
 	/* BOLT #4:
 	 * - `tlv_payload` format, identified by any length over `1`. In this
 	 *   case the `hop_payload_length` is equal to the numeric value of
@ -191,7 +179,6 @@ static bool pull_payload_length(const u8 **cursor,
 	}

 	return false;
-#endif /* EXPERIMENTAL_FEATURES */
 }

 size_t onion_payload_length(const u8 *raw_payload, size_t len,
@ -289,7 +276,6 @@ struct onion_payload *onion_decode(const tal_t *ctx,

 		p->payment_secret = NULL;

-#if EXPERIMENTAL_FEATURES
 		if (tlv->payment_data) {
 			p->payment_secret = tal_dup(p, struct secret,
 						    &tlv->payment_data->payment_secret);
@ -298,7 +284,6 @@ struct onion_payload *onion_decode(const tal_t *ctx,
 			p->total_msat->millisatoshis /* Raw: tu64 on wire */
 				= tlv->payment_data->total_msat;
 		}
-#endif
 		tal_free(tlv);
 		return p;
 	}
--- a/common/sphinx.c
+++ b/common/sphinx.c
@ -498,11 +498,6 @@ struct route_step *process_onionpacket(

 	payload_size = onion_payload_length(paddedheader, ROUTING_INFO_SIZE,
 					    &valid, NULL);
-#if !EXPERIMENTAL_FEATURES
-	/* We don't even attempt to handle non-legacy or malformed payloads */
-	if (!valid)
-		return tal_free(step);
-#endif

 	/* Can't decode?  Treat it as terminal. */
 	if (!valid) {
--- a/contrib/pyln-client/pyln/client/lightning.py
+++ b/contrib/pyln-client/pyln/client/lightning.py
@ -886,12 +886,15 @@ class LightningRpc(UnixDomainSocketRpc):
        if 'description' in kwargs:
            return self._deprecated_sendpay(route, payment_hash, *args, **kwargs)

-        def _sendpay(route, payment_hash, label=None, msatoshi=None):
+        def _sendpay(route, payment_hash, label=None, msatoshi=None, bolt11=None, payment_secret=None, partid=None):
            payload = {
                "route": route,
                "payment_hash": payment_hash,
                "label": label,
                "msatoshi": msatoshi,
+                "bolt11": bolt11,
+                "payment_secret": payment_secret,
+                "partid": partid,
            }
            return self.call("sendpay", payload)

@ -935,13 +938,14 @@ class LightningRpc(UnixDomainSocketRpc):
        }
        return self.call("waitinvoice", payload)

-    def waitsendpay(self, payment_hash, timeout=None):
+    def waitsendpay(self, payment_hash, timeout=None, partid=None):
        """
        Wait for payment for preimage of {payment_hash} to complete
        """
        payload = {
            "payment_hash": payment_hash,
-            "timeout": timeout
+            "timeout": timeout,
+            "partid": partid,
        }
        return self.call("waitsendpay", payload)

--- a/doc/lightning-sendpay.7
+++ b/doc/lightning-sendpay.7
@ -4,7 +4,7 @@ lightning-sendpay - Low-level command for sending a payment via a route
 .SH SYNOPSIS

 \fBsendpay\fR \fIroute\fR \fIpayment_hash\fR [\fIlabel\fR] [\fImsatoshi\fR]
-[\fIbolt11\fR]
+[\fIbolt11\fR] [\fIpartid\fR]

 .SH DESCRIPTION

@ -31,21 +31,23 @@ The \fIlabel\fR and \fIbolt11\fR parameters, if provided, will be returned in

 The \fImsatoshi\fR amount, if provided, is the amount that will be recorded
 as the target payment value\. If not specified, it will be the final
-amount to the destination\. If specified, then the final amount at the
-destination must be from the specified \fImsatoshi\fR to twice the specified
-\fImsatoshi\fR, inclusive\. This is intended to obscure payments by
-overpaying slightly at the destination; the actual target payment is
-what should be specified as the \fImsatoshi\fR argument\. \fImsatoshi\fR is in
-millisatoshi precision; it can be a whole number, or a whole number
+amount to the destination\. By default it is in millisatoshi precision; it can be a whole number, or a whole number
 ending in \fImsat\fR or \fIsat\fR, or a number with three decimal places ending
 in \fIsat\fR, or a number with 1 to 11 decimal places ending in \fIbtc\fR\.


+The \fIpartid\fR value, if provided and non-zero, allows for multiple parallel
+partial payments with the same \fIpayment_hash\fR\.  The \fImsatoshi\fR amount
+(which must be provided) for each \fBsendpay\fR with matching
+\fIpayment_hash\fR must be equal, and \fBsendpay\fR will fail if there are
+already \fImsatoshi\fR worth of payments pending\.
+
+
 Once a payment has succeeded, calls to \fBsendpay\fR with the same
 \fIpayment_hash\fR but a different \fImsatoshi\fR or destination will fail;
 this prevents accidental multiple payments\. Calls to \fBsendpay\fR with
 the same \fIpayment_hash\fR, \fImsatoshi\fR, and destination as a previous
-successful payment (even if a different route) will return immediately
+successful payment (even if a different route or \fIpartid\fR) will return immediately
 with success\.

 .SH RETURN VALUE
@ -65,6 +67,7 @@ retried\.

 The following error codes may occur:

+.RS
 .IP \[bu]
 -1: Catchall nonspecific error\.
 .IP \[bu]
@ -81,9 +84,11 @@ will be routing failure object\.
 204: Failure along route; retry a different route\. The \fIdata\fR field
 of the error will be routing failure object\.

+.RE

 A routing failure object has the fields below:

+.RS
 .IP \[bu]
 \fIerring_index\fR\. The index of the node along the route that reported
 the error\. 0 for the local node, 1 for the first hop, and so on\.
@ -101,6 +106,7 @@ received from the remote node\. Only present if error is from the
 remote node and the \fIfailcode\fR has the UPDATE bit set, as per BOLT
 #4\.

+.RE
 .SH AUTHOR

 Rusty Russell \fI<rusty@rustcorp.com.au\fR> is mainly responsible\.
@ -115,7 +121,3 @@ Rusty Russell \fI<rusty@rustcorp.com.au\fR> is mainly responsible\.

 Main web site: \fIhttps://github.com/ElementsProject/lightning\fR

-.HL
-
-Last updated 2019-08-01 14:59:36 CEST
-
--- a/doc/lightning-sendpay.7.md
+++ b/doc/lightning-sendpay.7.md
@ -5,7 +5,7 @@ SYNOPSIS
 --------

 **sendpay** *route* *payment\_hash* \[*label*\] \[*msatoshi*\]
-\[*bolt11*\]
+\[*bolt11*\] \[*partid*\]

 DESCRIPTION
 -----------
@ -29,20 +29,21 @@ The *label* and *bolt11* parameters, if provided, will be returned in

 The *msatoshi* amount, if provided, is the amount that will be recorded
 as the target payment value. If not specified, it will be the final
-amount to the destination. If specified, then the final amount at the
-destination must be from the specified *msatoshi* to twice the specified
-*msatoshi*, inclusive. This is intended to obscure payments by
-overpaying slightly at the destination; the actual target payment is
-what should be specified as the *msatoshi* argument. *msatoshi* is in
-millisatoshi precision; it can be a whole number, or a whole number
+amount to the destination. By default it is in millisatoshi precision; it can be a whole number, or a whole number
 ending in *msat* or *sat*, or a number with three decimal places ending
 in *sat*, or a number with 1 to 11 decimal places ending in *btc*.

+The *partid* value, if provided and non-zero, allows for multiple parallel
+partial payments with the same *payment_hash*.  The *msatoshi* amount
+(which must be provided) for each **sendpay** with matching
+*payment_hash* must be equal, and **sendpay** will fail if there are
+already *msatoshi* worth of payments pending.
+
 Once a payment has succeeded, calls to **sendpay** with the same
 *payment\_hash* but a different *msatoshi* or destination will fail;
 this prevents accidental multiple payments. Calls to **sendpay** with
 the same *payment\_hash*, *msatoshi*, and destination as a previous
-successful payment (even if a different route) will return immediately
+successful payment (even if a different route or *partid*) will return immediately
 with success.

 RETURN VALUE
--- a/doc/lightning-waitsendpay.7
+++ b/doc/lightning-waitsendpay.7
@ -3,7 +3,7 @@
 lightning-waitsendpay - Command for sending a payment via a route
 .SH SYNOPSIS

-\fBwaitsendpay\fR \fIpayment_hash\fR [\fItimeout\fR]
+\fBwaitsendpay\fR \fIpayment_hash\fR [\fItimeout\fR] [\fIpartid\fR]

 .SH DESCRIPTION

@ -12,6 +12,9 @@ outgoing payment that was initiated by a previous \fBsendpay\fR
 invocation\.


+The \fIpartid\fR argument must match that of the \fBsendpay\fR command\.
+
+
 Optionally the client may provide a \fItimeout\fR, an integer in seconds,
 for this RPC command to return\. If the \fItimeout\fR is provided and the
 given amount of time passes without the payment definitely succeeding or
@ -43,6 +46,7 @@ route\.

 The following error codes may occur:

+.RS
 .IP \[bu]
 -1: Catchall nonspecific error\.
 .IP \[bu]
@ -65,9 +69,11 @@ nothing to wait for\.
 stored\. This should only occur when querying failed payments on very
 old databases\.

+.RE

 A routing failure object has the fields below:

+.RS
 .IP \[bu]
 \fIerring_index\fR: The index of the node along the route that reported
 the error\. 0 for the local node, 1 for the first hop, and so on\.
@ -86,6 +92,7 @@ error (or the final channel if the destination raised the error)\.
 \fIfailcodename\fR: The human-readable name corresponding to \fIfailcode\fR,
 if known\.

+.RE
 .SH AUTHOR

 ZmnSCPxj \fI<ZmnSCPxj@protonmail.com\fR> is mainly responsible\.
@ -98,7 +105,3 @@ ZmnSCPxj \fI<ZmnSCPxj@protonmail.com\fR> is mainly responsible\.

 Main web site: \fIhttps://github.com/ElementsProject/lightning\fR

-.HL
-
-Last updated 2019-05-22 16:46:09 CEST
-
--- a/doc/lightning-waitsendpay.7.md
+++ b/doc/lightning-waitsendpay.7.md
@ -4,7 +4,7 @@ lightning-waitsendpay -- Command for sending a payment via a route
 SYNOPSIS
 --------

-**waitsendpay** *payment\_hash* \[*timeout*\]
+**waitsendpay** *payment\_hash* \[*timeout*\] \[*partid*\]

 DESCRIPTION
 -----------
@ -13,6 +13,8 @@ The **waitsendpay** RPC command polls or waits for the status of an
 outgoing payment that was initiated by a previous **sendpay**
 invocation.

+The *partid* argument must match that of the **sendpay** command.
+
 Optionally the client may provide a *timeout*, an integer in seconds,
 for this RPC command to return. If the *timeout* is provided and the
 given amount of time passes without the payment definitely succeeding or
--- a/lightningd/htlc_set.c
+++ b/lightningd/htlc_set.c
@ -4,7 +4,6 @@
 #include <lightningd/lightningd.h>
 #include <lightningd/peer_htlcs.h>

-#if EXPERIMENTAL_FEATURES
 /* If an HTLC times out, we need to free entire set, since we could be processing
 * it in invoice.c right now. */
 static void htlc_set_hin_destroyed(struct htlc_in *hin,
@ -37,15 +36,12 @@ static void timeout_htlc_set(struct htlc_set *set)
 {
 	htlc_set_fail(set, WIRE_MPP_TIMEOUT);
 }
-#endif /* EXPERIMENTAL_FEATURES */

 void htlc_set_fail(struct htlc_set *set, enum onion_type failcode)
 {
 	for (size_t i = 0; i < tal_count(set->htlcs); i++) {
-#if EXPERIMENTAL_FEATURES
 		/* Don't remove from set */
 		tal_del_destructor2(set->htlcs[i], htlc_set_hin_destroyed, set);
-#endif
 		fail_htlc(set->htlcs[i], failcode);
 	}
 	tal_free(set);
@ -54,10 +50,8 @@ void htlc_set_fail(struct htlc_set *set, enum onion_type failcode)
 void htlc_set_fulfill(struct htlc_set *set, const struct preimage *preimage)
 {
 	for (size_t i = 0; i < tal_count(set->htlcs); i++) {
-#if EXPERIMENTAL_FEATURES
 		/* Don't remove from set */
 		tal_del_destructor2(set->htlcs[i], htlc_set_hin_destroyed, set);
-#endif
 		fulfill_htlc(set->htlcs[i], preimage);
 	}
 	tal_free(set);
@ -76,7 +70,6 @@ static struct htlc_set *new_htlc_set(struct lightningd *ld,
 	set->htlcs = tal_arr(set, struct htlc_in *, 1);
 	set->htlcs[0] = hin;

-#if EXPERIMENTAL_FEATURES
 	/* BOLT-9441a66faad63edc8cd89860b22fbf24a86f0dcd #4:
 	 * - MUST fail all HTLCs in the HTLC set after some reasonable
 	 *   timeout.
@ -87,9 +80,6 @@ static struct htlc_set *new_htlc_set(struct lightningd *ld,
 				    timeout_htlc_set, set);
 	htlc_set_map_add(&ld->htlc_sets, set);
 	tal_add_destructor2(set, destroy_htlc_set, &ld->htlc_sets);
-#else
-	set->timeout = NULL;
-#endif
 	return set;
 }

@ -114,20 +104,6 @@ void htlc_set_add(struct lightningd *ld,
 		return;
 	}

-#if !EXPERIMENTAL_FEATURES
-	/* BOLT-9441a66faad63edc8cd89860b22fbf24a86f0dcd #4:
-	 * - if it does not support `basic_mpp`:
-	 *    - MUST fail the HTLC if `total_msat` is not exactly equal to
-	 *    `amt_to_forward`.
-	 */
-	if (!amount_msat_eq(hin->msat, total_msat)) {
-		fail_htlc(hin, WIRE_FINAL_INCORRECT_HTLC_AMOUNT);
-		return;
-	}
-
-	/* We create a transient set which just has one entry. */
-	set = new_htlc_set(ld, hin, total_msat);
-#else
 	/* BOLT-9441a66faad63edc8cd89860b22fbf24a86f0dcd #4:
 	 *  - otherwise, if it supports `basic_mpp`:
 	 *    - MUST add it to the HTLC set corresponding to that `payment_hash`.
@ -175,7 +151,6 @@ void htlc_set_add(struct lightningd *ld,
 		htlc_set_fail(set, WIRE_FINAL_INCORRECT_HTLC_AMOUNT);
 		return;
 	}
-#endif /* EXPERIMENTAL_FEATURES */

 	/* BOLT-9441a66faad63edc8cd89860b22fbf24a86f0dcd #4:
 	 * - if the total `amount_msat` of this HTLC set equals `total_msat`:
--- a/lightningd/lightningd.c
+++ b/lightningd/lightningd.c
@ -161,11 +161,9 @@ static struct lightningd *new_lightningd(const tal_t *ctx)
 	htlc_in_map_init(&ld->htlcs_in);
 	htlc_out_map_init(&ld->htlcs_out);

-#if EXPERIMENTAL_FEATURES
 	/*~ For multi-part payments, we need to keep some incoming payments
 	 * in limbo until we get all the parts, or we time them out. */
 	htlc_set_map_init(&ld->htlc_sets);
-#endif /* EXPERIMENTAL_FEATURES */

 	/*~ We have a multi-entry log-book infrastructure: we define a 100MB log
 	 * book to hold all the entries (and trims as necessary), and multiple
--- a/lightningd/lightningd.h
+++ b/lightningd/lightningd.h
@ -163,10 +163,8 @@ struct lightningd {
 	struct htlc_in_map htlcs_in;
 	struct htlc_out_map htlcs_out;

-#if EXPERIMENTAL_FEATURES
 	/* Sets of HTLCs we are holding onto for MPP. */
 	struct htlc_set_map htlc_sets;
-#endif

 	struct wallet *wallet;

--- a/lightningd/pay.c
+++ b/lightningd/pay.c
@ -702,9 +702,7 @@ static bool should_use_tlv(enum route_hop_style style)
 {
 	switch (style) {
 	case ROUTE_HOP_TLV:
-#if EXPERIMENTAL_FEATURES
 		return true;
-#endif
 		/* Otherwise fall thru */
 	case ROUTE_HOP_LEGACY:
 		return false;
@ -1316,15 +1314,6 @@ static struct command_result *json_sendpay(struct command *cmd,
 							   msat));
 	}

-	/* It's easier to leave this in the API, then ignore it here. */
-#if !EXPERIMENTAL_FEATURES
-	if (payment_secret) {
-		log_unusual(cmd->ld->log,
-			    "sendpay: we don't support payment_secret yet, ignoring");
-		payment_secret = NULL;
-	}
-#endif
-
 	if (*partid && !payment_secret)
 		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
 				    "partid requires payment_secret");
--- a/tests/test_gossip.py
+++ b/tests/test_gossip.py
@ -2,7 +2,7 @@ from collections import Counter
 from fixtures import *  # noqa: F401,F403
 from fixtures import TEST_NETWORK
 from lightning import RpcError
-from utils import wait_for, TIMEOUT, only_one, sync_blockheight, expected_features, EXPERIMENTAL_FEATURES
+from utils import wait_for, TIMEOUT, only_one, sync_blockheight, expected_features

 import json
 import logging
@ -1450,10 +1450,7 @@ def test_gossip_store_compact_on_load(node_factory, bitcoind):
    l2.restart()

    wait_for(lambda: l2.daemon.is_in_log(r'gossip_store_compact_offline: [5-8] deleted, 9 copied'))
-    if EXPERIMENTAL_FEATURES:
-        wait_for(lambda: l2.daemon.is_in_log(r'gossip_store: Read 1/4/2/0 cannounce/cupdate/nannounce/cdelete from store \(0 deleted\) in 1452 bytes'))
-    else:
-        wait_for(lambda: l2.daemon.is_in_log(r'gossip_store: Read 1/4/2/0 cannounce/cupdate/nannounce/cdelete from store \(0 deleted\) in 1450 bytes'))
+    wait_for(lambda: l2.daemon.is_in_log(r'gossip_store: Read 1/4/2/0 cannounce/cupdate/nannounce/cdelete from store \(0 deleted\) in 1452 bytes'))


 def test_gossip_announce_invalid_block(node_factory, bitcoind):
--- a/tests/test_misc.py
+++ b/tests/test_misc.py
@ -5,7 +5,7 @@ from fixtures import TEST_NETWORK
 from flaky import flaky  # noqa: F401
 from lightning import RpcError
 from threading import Event
-from utils import DEVELOPER, TIMEOUT, VALGRIND, sync_blockheight, only_one, wait_for, TailableProc, EXPERIMENTAL_FEATURES, env
+from utils import DEVELOPER, TIMEOUT, VALGRIND, sync_blockheight, only_one, wait_for, TailableProc, env
 from ephemeral_port_reserve import reserve

 import json
@ -1715,22 +1715,15 @@ def test_dev_demux(node_factory):
 def test_list_features_only(node_factory):
    features = subprocess.check_output(['lightningd/lightningd',
                                        '--list-features-only']).decode('utf-8').splitlines()
-    if EXPERIMENTAL_FEATURES:
-        expected = ['option_data_loss_protect/odd',
-                    'option_upfront_shutdown_script/odd',
-                    'option_gossip_queries/odd',
-                    'option_var_onion_optin/odd',
-                    'option_payment_secret/odd',
-                    'option_basic_mpp/odd',
-                    'option_gossip_queries_ex/odd',
-                    'option_static_remotekey/odd',
-                    ]
-    else:
-        expected = ['option_data_loss_protect/odd',
-                    'option_upfront_shutdown_script/odd',
-                    'option_gossip_queries/odd',
-                    'option_gossip_queries_ex/odd',
-                    'option_static_remotekey/odd']
+    expected = ['option_data_loss_protect/odd',
+                'option_upfront_shutdown_script/odd',
+                'option_gossip_queries/odd',
+                'option_var_onion_optin/odd',
+                'option_payment_secret/odd',
+                'option_basic_mpp/odd',
+                'option_gossip_queries_ex/odd',
+                'option_static_remotekey/odd',
+                ]
    assert features == expected


--- a/tests/test_pay.py
+++ b/tests/test_pay.py
@ -3,7 +3,7 @@ from fixtures import *  # noqa: F401,F403
 from fixtures import TEST_NETWORK
 from flaky import flaky  # noqa: F401
 from lightning import RpcError, Millisatoshi
-from utils import DEVELOPER, wait_for, only_one, sync_blockheight, SLOW_MACHINE, TIMEOUT, VALGRIND, EXPERIMENTAL_FEATURES
+from utils import DEVELOPER, wait_for, only_one, sync_blockheight, SLOW_MACHINE, TIMEOUT, VALGRIND

 import concurrent.futures
 import copy
@ -2375,10 +2375,7 @@ def test_tlv_or_legacy(node_factory, bitcoind):

    # Since L1 hasn't seen broadcast, it doesn't know L2 is TLV, but invoice tells it about L3
    l2.daemon.wait_for_log("Got onion.*'type': 'legacy'")
-    if EXPERIMENTAL_FEATURES:
-        l3.daemon.wait_for_log("Got onion.*'type': 'tlv'")
-    else:
-        l3.daemon.wait_for_log("Got onion.*'type': 'legacy'")
+    l3.daemon.wait_for_log("Got onion.*'type': 'tlv'")

    # Turns out we only need 3 more blocks to announce l1->l2 channel.
    bitcoind.generate_block(3)
@ -2393,15 +2390,10 @@ def test_tlv_or_legacy(node_factory, bitcoind):
    inv = l3.rpc.invoice(10000, "test_tlv2", "test_tlv2")['bolt11']

    l1.rpc.pay(inv)
-    if EXPERIMENTAL_FEATURES:
-        l2.daemon.wait_for_log("Got onion.*'type': 'tlv'")
-        l3.daemon.wait_for_log("Got onion.*'type': 'tlv'")
-    else:
-        l2.daemon.wait_for_log("Got onion.*'type': 'legacy'")
-        l3.daemon.wait_for_log("Got onion.*'type': 'legacy'")
+    l2.daemon.wait_for_log("Got onion.*'type': 'tlv'")
+    l3.daemon.wait_for_log("Got onion.*'type': 'tlv'")


-@unittest.skipIf(not EXPERIMENTAL_FEATURES, 'Needs invoice secret support')
@unittest.skipIf(not DEVELOPER, 'Needs dev-routes')
@unittest.skipIf(TEST_NETWORK != 'regtest', "Invoice is network specific")
 def test_pay_no_secret(node_factory, bitcoind):
@ -2570,7 +2562,6 @@ def test_sendonion_rpc(node_factory):


@unittest.skipIf(not DEVELOPER, "needs dev-disconnect, dev-no-htlc-timeout")
-@unittest.skipIf(not EXPERIMENTAL_FEATURES, "needs partid support")
 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'], 'dev-no-htlc-timeout': None}] * 2 + [{'plugin': os.path.join(os.getcwd(), 'tests/plugins/print_htlc_onion.py')}])
@ -2597,29 +2588,34 @@ def test_partial_payment(node_factory, bitcoind, executor):
    r124 = l1.rpc.getroute(l4.info['id'], 499, 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])
+    l1.rpc.sendpay(route=r134, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=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})
+        l1.rpc.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])
+        l1.rpc.sendpay(route=r124, payment_hash=inv['payment_hash'],
+                       msatoshi=999, bolt11=inv['bolt11'],
+                       payment_secret=paysecret, partid=2)

    # This will work fine.
-    l1.rpc.call('sendpay', [r124, inv['payment_hash'], None, 1000, inv['bolt11'], paysecret, 2])
+    l1.rpc.sendpay(route=r124, payment_hash=inv['payment_hash'],
+                   msatoshi=1000, bolt11=inv['bolt11'],
+                   payment_secret=paysecret, partid=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])
+        l1.rpc.sendpay(route=r124, payment_hash=inv['payment_hash'],
+                       msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=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])
+    l1.rpc.sendpay(route=r124, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=1)
+    l1.rpc.sendpay(route=r134, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=2)

    # Make sure they've done the suppress-commitment thing before we unsuppress
    l2.daemon.wait_for_log(r'dev_disconnect')
@ -2629,10 +2625,9 @@ def test_partial_payment(node_factory, bitcoind, executor):
    l2.rpc.dev_reenable_commit(l4.info['id'])
    l3.rpc.dev_reenable_commit(l4.info['id'])

-    res = l1.rpc.call('waitsendpay', [inv['payment_hash'], None, 1])
+    res = l1.rpc.waitsendpay(payment_hash=inv['payment_hash'], partid=1)
    assert res['partid'] == 1
-    res = l1.rpc.call('waitsendpay', {'payment_hash': inv['payment_hash'],
-                                      'partid': 2})
+    res = l1.rpc.waitsendpay(payment_hash=inv['payment_hash'], partid=2)
    assert res['partid'] == 2

    for i in range(2):
@ -2649,7 +2644,6 @@ def test_partial_payment(node_factory, bitcoind, executor):
    assert pay['amount_sent_msat'] == Millisatoshi(1002)


-@unittest.skipIf(not EXPERIMENTAL_FEATURES, "needs partid support")
 def test_partial_payment_timeout(node_factory, bitcoind):
    l1, l2 = node_factory.line_graph(2)

@ -2657,19 +2651,18 @@ def test_partial_payment_timeout(node_factory, bitcoind):
    paysecret = l2.rpc.decodepay(inv['bolt11'])['payment_secret']

    route = l1.rpc.getroute(l2.info['id'], 500, 1)['route']
-    l1.rpc.call('sendpay', [route, inv['payment_hash'], None, 1000, inv['bolt11'], paysecret, 1])
+    l1.rpc.sendpay(route=route, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=1)

    with pytest.raises(RpcError, match=r'WIRE_MPP_TIMEOUT'):
-        l1.rpc.call('waitsendpay', [inv['payment_hash'], 70 + TIMEOUT // 4, 1])
+        l1.rpc.waitsendpay(payment_hash=inv['payment_hash'], timeout=70 + TIMEOUT // 4, partid=1)

    # We can still pay it normally.
-    l1.rpc.call('sendpay', [route, inv['payment_hash'], None, 1000, inv['bolt11'], paysecret, 1])
-    l1.rpc.call('sendpay', [route, inv['payment_hash'], None, 1000, inv['bolt11'], paysecret, 2])
-    l1.rpc.call('waitsendpay', [inv['payment_hash'], TIMEOUT, 1])
-    l1.rpc.call('waitsendpay', [inv['payment_hash'], TIMEOUT, 2])
+    l1.rpc.sendpay(route=route, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=1)
+    l1.rpc.sendpay(route=route, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=2)
+    l1.rpc.waitsendpay(payment_hash=inv['payment_hash'], timeout=TIMEOUT, partid=1)
+    l1.rpc.waitsendpay(payment_hash=inv['payment_hash'], timeout=TIMEOUT, partid=2)


-@unittest.skipIf(not EXPERIMENTAL_FEATURES, "needs partid support")
 def test_partial_payment_restart(node_factory, bitcoind):
    """Test that we recover a set when we restart"""
    l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True,
@ -2681,7 +2674,7 @@ def test_partial_payment_restart(node_factory, bitcoind):

    route = l1.rpc.getroute(l3.info['id'], 500, 1)['route']

-    l1.rpc.call('sendpay', [route, inv['payment_hash'], None, 1000, inv['bolt11'], paysecret, 1])
+    l1.rpc.sendpay(route=route, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=1)

    wait_for(lambda: [f['status'] for f in l2.rpc.listforwards()['forwards']] == ['offered'])

@ -2691,14 +2684,13 @@ def test_partial_payment_restart(node_factory, bitcoind):
    wait_for(lambda: [p['connected'] for p in l2.rpc.listpeers()['peers']] == [True, True])

    # Pay second part.
-    l1.rpc.call('sendpay', [route, inv['payment_hash'], None, 1000, inv['bolt11'], paysecret, 2])
+    l1.rpc.sendpay(route=route, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=2)

-    l1.rpc.call('waitsendpay', [inv['payment_hash'], TIMEOUT, 1])
-    l1.rpc.call('waitsendpay', [inv['payment_hash'], TIMEOUT, 2])
+    l1.rpc.waitsendpay(payment_hash=inv['payment_hash'], timeout=TIMEOUT, partid=1)
+    l1.rpc.waitsendpay(payment_hash=inv['payment_hash'], timeout=TIMEOUT, partid=2)


@unittest.skipIf(not DEVELOPER, "needs dev-fail")
-@unittest.skipIf(not EXPERIMENTAL_FEATURES, "needs partid support")
 def test_partial_payment_htlc_loss(node_factory, bitcoind):
    """Test that we discard a set when the HTLC is lost"""
    l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True)
@ -2708,7 +2700,7 @@ def test_partial_payment_htlc_loss(node_factory, bitcoind):

    route = l1.rpc.getroute(l3.info['id'], 500, 1)['route']

-    l1.rpc.call('sendpay', [route, inv['payment_hash'], None, 1000, inv['bolt11'], paysecret, 1])
+    l1.rpc.sendpay(route=route, payment_hash=inv['payment_hash'], msatoshi=1000, bolt11=inv['bolt11'], payment_secret=paysecret, partid=1)
    wait_for(lambda: [f['status'] for f in l2.rpc.listforwards()['forwards']] == ['offered'])

    l2.rpc.dev_fail(l3.info['id'])
@ -2719,4 +2711,4 @@ def test_partial_payment_htlc_loss(node_factory, bitcoind):

    with pytest.raises(RpcError,
                       match=r'WIRE_PERMANENT_CHANNEL_FAILURE \(reply from remote\)'):
-        l1.rpc.call('waitsendpay', [inv['payment_hash'], TIMEOUT, 1])
+        l1.rpc.waitsendpay(payment_hash=inv['payment_hash'], timeout=TIMEOUT, partid=1)
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@ -2,7 +2,7 @@ from collections import OrderedDict
 from fixtures import *  # noqa: F401,F403
 from flaky import flaky  # noqa: F401
 from lightning import RpcError, Millisatoshi
-from utils import DEVELOPER, only_one, sync_blockheight, TIMEOUT, wait_for, EXPERIMENTAL_FEATURES, TEST_NETWORK
+from utils import DEVELOPER, only_one, sync_blockheight, TIMEOUT, wait_for, TEST_NETWORK

 import json
 import os
@ -534,13 +534,8 @@ def test_htlc_accepted_hook_forward_restart(node_factory, executor):
    logline = l2.daemon.wait_for_log(r'Onion written to')
    fname = re.search(r'Onion written to (.*\.json)', logline).group(1)
    onion = json.load(open(fname))
-    if EXPERIMENTAL_FEATURES:
-        assert onion['type'] == 'tlv'
-        assert re.match(r'^11020203e80401..0608................$', onion['payload'])
-    else:
-        assert onion['type'] == 'legacy'
-        assert re.match(r'^0000006700000.000100000000000003e8000000..000000000000000000000000$', onion['payload'])
-        assert len(onion['payload']) == 66
+    assert onion['type'] == 'tlv'
+    assert re.match(r'^11020203e80401..0608................$', onion['payload'])
    assert len(onion['shared_secret']) == 64
    assert onion['forward_amount'] == '1000msat'
    assert len(onion['next_onion']) == 2 * (1300 + 32 + 33 + 1)
--- a/tests/utils.py
+++ b/tests/utils.py
@ -8,9 +8,5 @@ COMPAT = env("COMPAT", "1") == "1"

 def expected_features():
    """Return the expected features hexstring for this configuration"""
-    if EXPERIMENTAL_FEATURES:
-        # features 1, 3, 7, 9, 11, 13, 15 and 17 (0x02aaa2).
-        return "02aaa2"
-    else:
-        # features 1, 3, 7, 11 and 13 (0x28a2).
-        return "28a2"
+    # features 1, 3, 7, 9, 11, 13, 15 and 17 (0x02aaa2).
+    return "02aaa2"
--- a/wire/extracted_onion_experimental_e36f7b6517e1173dcbd49da3b516cfe1f48ae556
+++ b/wire/extracted_onion_experimental_e36f7b6517e1173dcbd49da3b516cfe1f48ae556
@ -1,17 +0,0 @@
--- wire/extracted_onion_wire_csv	2019-11-04 15:38:24.345401216 +1030
-+++ -	2019-11-06 14:40:16.145483573 +1030
-@@ -5,6 +5,9 @@
- tlvdata,tlv_payload,outgoing_cltv_value,outgoing_cltv_value,tu32,
- tlvtype,tlv_payload,short_channel_id,6
- tlvdata,tlv_payload,short_channel_id,short_channel_id,short_channel_id,
-+tlvtype,tlv_payload,payment_data,8
-+tlvdata,tlv_payload,payment_data,payment_secret,byte,32
-+tlvdata,tlv_payload,payment_data,total_msat,tu64,
- msgtype,invalid_realm,PERM|1
- msgtype,temporary_node_failure,NODE|2
- msgtype,permanent_node_failure,PERM|NODE|2
-@@ -48,3 +51,4 @@
- msgtype,invalid_onion_payload,PERM|22
- msgdata,invalid_onion_payload,type,varint,
- msgdata,invalid_onion_payload,offset,u16,
-+msgtype,mpp_timeout,23
--- a/wire/extracted_onion_wire_csv
+++ b/wire/extracted_onion_wire_csv
@ -5,6 +5,9 @@ tlvtype,tlv_payload,outgoing_cltv_value,4
 tlvdata,tlv_payload,outgoing_cltv_value,outgoing_cltv_value,tu32,
 tlvtype,tlv_payload,short_channel_id,6
 tlvdata,tlv_payload,short_channel_id,short_channel_id,short_channel_id,
+tlvtype,tlv_payload,payment_data,8
+tlvdata,tlv_payload,payment_data,payment_secret,byte,32
+tlvdata,tlv_payload,payment_data,total_msat,tu64,
 msgtype,invalid_realm,PERM|1
 msgtype,temporary_node_failure,NODE|2
 msgtype,permanent_node_failure,PERM|NODE|2
@ -48,3 +51,4 @@ msgtype,expiry_too_far,21
 msgtype,invalid_onion_payload,PERM|22
 msgdata,invalid_onion_payload,type,varint,
 msgdata,invalid_onion_payload,offset,u16,
+msgtype,mpp_timeout,23