From 33168fc733b6bab26327983149a18c70e4aed775 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 23 Sep 2021 12:12:47 +0930 Subject: [PATCH] lightningd: provide 10 minutes for channel fee increases to propagate. This was measured as a 95th percentile in our rough testing, thanks to all the volunteers who monitored my channels. Fixes: #4761 Signed-off-by: Rusty Russell Changelog-Added: JSON-RPC: `setchannelfee` gives a grace period (`enforcedelay`) before rejecting old-fee payments: default 10 minutes. --- contrib/pyln-client/pyln/client/lightning.py | 7 ++- doc/lightning-setchannelfee.7.md | 11 +++- lightningd/channel.c | 4 ++ lightningd/channel.h | 3 + lightningd/peer_control.c | 21 +++++-- lightningd/peer_htlcs.c | 15 ++++- tests/test_pay.py | 61 ++++++++++++++++++++ 7 files changed, 111 insertions(+), 11 deletions(-) diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py index ae9660e79..c128ff4f0 100644 --- a/contrib/pyln-client/pyln/client/lightning.py +++ b/contrib/pyln-client/pyln/client/lightning.py @@ -1153,16 +1153,17 @@ class LightningRpc(UnixDomainSocketRpc): } 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 index bd780c060..da2b8c161 100644 --- 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 index 9c4c3a509..3e6076d39 100644 --- 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 index 7726fa880..ba0075f31 100644 --- 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 index 72b4c8f18..0e3d28d2c 100644 --- 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 index ad827a495..d7ec61315 100644 --- 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 index 10a444551..244314dea 100644 --- 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