lightningd: provide 10 minutes for channel fee increases to propagate.

rustyrussell · cdecker · commit 33168fc733b6 · 2021-09-23T15:05:09.000+02:00
This was measured as a 95th percentile in our rough testing, thanks to all the volunteers who monitored my channels. Fixes: ElementsProject#4761 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Added: JSON-RPC: `setchannelfee` gives a grace period (`enforcedelay`) before rejecting old-fee payments: default 10 minutes.
diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py
@@ -1153,16 +1153,17 @@ def sendpay(self, route, payment_hash, label=None, msatoshi=None, bolt11=None, p
         }
         return self.call("sendpay", payload)
 
-    def setchannelfee(self, id, base=None, ppm=None):
+    def setchannelfee(self, id, base=None, ppm=None, enforcedelay=None):
         """
         Set routing fees for a channel/peer {id} (or 'all'). {base} is a value in millisatoshi
         that is added as base fee to any routed payment. {ppm} is a value added proportionally
-        per-millionths to any routed payment volume in satoshi.
+        per-millionths to any routed payment volume in satoshi. {enforcedelay} is the number of seconds before enforcing this change.
         """
         payload = {
             "id": id,
             "base": base,
-            "ppm": ppm
+            "ppm": ppm,
+            "enforcedelay": enforcedelay,
         }
         return self.call("setchannelfee", payload)
 
diff --git a/doc/lightning-setchannelfee.7.md b/doc/lightning-setchannelfee.7.md
@@ -4,7 +4,7 @@ lightning-setchannelfee -- Command for setting specific routing fees on a lightn
 SYNOPSIS
 --------
 
-**setchannelfee** *id* \[*base*\] \[*ppm*\]
+**setchannelfee** *id* \[*base*\] \[*ppm*\] \[*enforcedelay*\]
 
 DESCRIPTION
 -----------
@@ -32,6 +32,15 @@ and 1,000,000 satoshi is being routed through the channel, an
 proportional fee of 1,000 satoshi is added, resulting in a 0.1% fee. If
 the parameter is left out, the global config value will be used again.
 
+*enforcedelay* is the number of seconds to delay before enforcing the
+new fees (default 600, which is ten minutes).  This gives the network
+a chance to catch up with the new rates and avoids rejecting HTLCs
+before they do.  This only has an effect if rates are increased (we
+always allow users to overpay fees), only applies to a single rate
+increase per channel (we don't remember an arbitrary number of prior
+feerates) and if the node is restarted the updated fees are enforced
+immediately.
+
 RETURN VALUE
 ------------
 
diff --git a/lightningd/channel.c b/lightningd/channel.c
@@ -264,6 +264,8 @@ struct channel *new_unsaved_channel(struct peer *peer,
 
 	channel->feerate_base = feerate_base;
 	channel->feerate_ppm = feerate_ppm;
+	channel->old_feerate_timeout.ts.tv_sec = 0;
+	channel->old_feerate_timeout.ts.tv_nsec = 0;
 	/* closer not yet known */
 	channel->closer = NUM_SIDES;
 
@@ -440,6 +442,8 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
 		= tal_steal(channel, future_per_commitment_point);
 	channel->feerate_base = feerate_base;
 	channel->feerate_ppm = feerate_ppm;
+	channel->old_feerate_timeout.ts.tv_sec = 0;
+	channel->old_feerate_timeout.ts.tv_nsec = 0;
 	channel->remote_upfront_shutdown_script
 		= tal_steal(channel, remote_upfront_shutdown_script);
 	channel->static_remotekey_start[LOCAL] = local_static_remotekey_start;
diff --git a/lightningd/channel.h b/lightningd/channel.h
@@ -196,6 +196,9 @@ struct channel {
 
 	/* Feerate per channel */
 	u32 feerate_base, feerate_ppm;
+	/* But allow these feerates up until this time. */
+	struct timeabs old_feerate_timeout;
+	u32 old_feerate_base, old_feerate_ppm;
 
 	/* If they used option_upfront_shutdown_script. */
 	const u8 *remote_upfront_shutdown_script;
diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c
@@ -1944,8 +1944,18 @@ static struct command_result *param_msat_u32(struct command *cmd,
 }
 
 static void set_channel_fees(struct command *cmd, struct channel *channel,
-		u32 base, u32 ppm, struct json_stream *response)
+			     u32 base, u32 ppm, u32 delaysecs,
+			     struct json_stream *response)
 {
+	/* We only need to defer values if we *increase* them; we always
+	 * allow users to overpay fees. */
+	if (base > channel->feerate_base || ppm > channel->feerate_ppm) {
+		channel->old_feerate_timeout
+			= timeabs_add(time_now(), time_from_sec(delaysecs));
+		channel->old_feerate_base = channel->feerate_base;
+		channel->old_feerate_ppm = channel->feerate_ppm;
+	}
+
 	/* set new values */
 	channel->feerate_base = base;
 	channel->feerate_ppm = ppm;
@@ -1976,7 +1986,7 @@ static struct command_result *json_setchannelfee(struct command *cmd,
 	struct json_stream *response;
 	struct peer *peer;
 	struct channel *channel;
-	u32 *base, *ppm;
+	u32 *base, *ppm, *delaysecs;
 
 	/* Parse the JSON command */
 	if (!param(cmd, buffer, params,
@@ -1985,6 +1995,7 @@ static struct command_result *json_setchannelfee(struct command *cmd,
 			     &base, cmd->ld->config.fee_base),
 		   p_opt_def("ppm", param_number, &ppm,
 			     cmd->ld->config.fee_per_satoshi),
+		   p_opt_def("enforcedelay", param_number, &delaysecs, 600),
 		   NULL))
 		return command_param_failed();
 
@@ -2011,12 +2022,14 @@ static struct command_result *json_setchannelfee(struct command *cmd,
 			    channel->state != CHANNELD_AWAITING_LOCKIN &&
 			    channel->state != DUALOPEND_AWAITING_LOCKIN)
 				continue;
-			set_channel_fees(cmd, channel, *base, *ppm, response);
+			set_channel_fees(cmd, channel, *base, *ppm, *delaysecs,
+					 response);
 		}
 
 	/* single channel should be updated */
 	} else {
-		set_channel_fees(cmd, channel, *base, *ppm, response);
+		set_channel_fees(cmd, channel, *base, *ppm, *delaysecs,
+				 response);
 	}
 
 	/* Close and return response */
diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c
@@ -706,9 +706,18 @@ static void forward_htlc(struct htlc_in *hin,
 	if (!check_fwd_amount(hin, amt_to_forward, hin->msat,
 			      next->feerate_base,
 			      next->feerate_ppm)) {
-		needs_update_appended = true;
-		failmsg = towire_fee_insufficient(tmpctx, hin->msat, NULL);
-		goto fail;
+		/* Are we in old-fee grace-period? */
+		if (!time_before(time_now(), next->old_feerate_timeout)
+		    || !check_fwd_amount(hin, amt_to_forward, hin->msat,
+					 next->old_feerate_base,
+					 next->old_feerate_ppm)) {
+			needs_update_appended = true;
+			failmsg = towire_fee_insufficient(tmpctx, hin->msat,
+							  NULL);
+			goto fail;
+		}
+		log_info(hin->key.channel->log,
+			 "Allowing payment using older feerate");
 	}
 
 	if (!check_cltv(hin, cltv_expiry, outgoing_cltv_value,
diff --git a/tests/test_pay.py b/tests/test_pay.py
@@ -4615,6 +4615,67 @@ def test_pay_low_max_htlcs(node_factory):
     )
 
 
+def test_setchannelfee_enforcement_delay(node_factory, bitcoind):
+    # Fees start at 1msat + 1%
+    l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True,
+                                         opts={'fee-base': 1,
+                                               'fee-per-satoshi': 10000})
+
+    chanid1 = only_one(l1.rpc.getpeer(l2.info['id'])['channels'])['short_channel_id']
+    chanid2 = only_one(l2.rpc.getpeer(l3.info['id'])['channels'])['short_channel_id']
+
+    route = [{'msatoshi': 1011,
+              'id': l2.info['id'],
+              'delay': 20,
+              'channel': chanid1},
+             {'msatoshi': 1000,
+              'id': l3.info['id'],
+              'delay': 10,
+              'channel': chanid2}]
+
+    # This works.
+    inv = l3.rpc.invoice(1000, "test1", "test1")
+    l1.rpc.sendpay(route,
+                   payment_hash=inv['payment_hash'],
+                   payment_secret=inv['payment_secret'])
+    l1.rpc.waitsendpay(inv['payment_hash'])
+
+    # Increase fee immediately; l1 payment rejected.
+    l2.rpc.setchannelfee("all", 2, 10000, 0)
+
+    inv = l3.rpc.invoice(1000, "test2", "test2")
+    l1.rpc.sendpay(route,
+                   payment_hash=inv['payment_hash'],
+                   payment_secret=inv['payment_secret'])
+    with pytest.raises(RpcError, match=r'WIRE_FEE_INSUFFICIENT'):
+        l1.rpc.waitsendpay(inv['payment_hash'])
+
+    # Test increased amount.
+    route[0]['msatoshi'] += 1
+    inv = l3.rpc.invoice(1000, "test3", "test3")
+    l1.rpc.sendpay(route,
+                   payment_hash=inv['payment_hash'],
+                   payment_secret=inv['payment_secret'])
+    l1.rpc.waitsendpay(inv['payment_hash'])
+
+    # Now, give us 30 seconds please.
+    l2.rpc.setchannelfee("all", 3, 10000, 30)
+    inv = l3.rpc.invoice(1000, "test4", "test4")
+    l1.rpc.sendpay(route,
+                   payment_hash=inv['payment_hash'],
+                   payment_secret=inv['payment_secret'])
+    l1.rpc.waitsendpay(inv['payment_hash'])
+    l2.daemon.wait_for_log("Allowing payment using older feerate")
+
+    time.sleep(30)
+    inv = l3.rpc.invoice(1000, "test5", "test5")
+    l1.rpc.sendpay(route,
+                   payment_hash=inv['payment_hash'],
+                   payment_secret=inv['payment_secret'])
+    with pytest.raises(RpcError, match=r'WIRE_FEE_INSUFFICIENT'):
+        l1.rpc.waitsendpay(inv['payment_hash'])
+
+
 def test_listpays_with_filter_by_status(node_factory, bitcoind):
     """
     This test check if the filtering by status of the command listpays
