Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update fee hysteresis #6833

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions doc/getting-started/getting-started/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,11 @@ The [`lightning-listconfigs`](ref:lightning-listconfigs) command will output a v
The percentage of _estimatesmartfee 2/CONSERVATIVE_ to use for the commitment
transactions: default is 100.

- **commit-feerate-offset**=_INTEGER_

The additional feerate a channel opener adds to their preferred feerate to
lessen the odds of a disconnect due to feerate disagreement (default 5).

- **max-concurrent-htlcs**=_INTEGER_

Number of HTLCs one channel can handle concurrently in each direction.
Expand Down
6 changes: 5 additions & 1 deletion doc/lightning-listconfigs.7.md
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,9 @@ On success, an object is returned, containing:
- **commit-fee** (object, optional):
- **value\_int** (u64): field from config or cmdline, or default
- **source** (string): source of configuration setting
- **commit-feerate-offset** (object, optional):
- **value\_int** (u32): field from config or cmdline, or default
- **source** (string): source of configuration setting
- **# version** (string, optional): Special field indicating the current version **deprecated, removal in v24.05**
- **plugins** (array of objects, optional) **deprecated, removal in v24.05**:
- **path** (string): Full path of the plugin
Expand Down Expand Up @@ -359,6 +362,7 @@ On success, an object is returned, containing:
- **developer** (boolean, optional): Whether developer mode is enabled *(added v23.08)*
- **commit-fee** (u64, optional): The percentage of the 6-block fee estimate to use for commitment transactions **deprecated, removal in v24.05** *(added v23.05)*
- **min-emergency-msat** (msat, optional): field from config or cmdline, or default *(added v23.08)*
- **commit-feerate-offset** (u32, optional): additional commitment feerate applied by channel owner *(added v23.11)*

[comment]: # (GENERATE-FROM-SCHEMA-END)

Expand Down Expand Up @@ -476,4 +480,4 @@ RESOURCES

Main web site: <https://github.com/ElementsProject/lightning>

[comment]: # ( SHA256STAMP:cc7b6d10f93b9efb34ad76d0cc2273d29189a8dd7ef4acef2e5227755c279ea8)
[comment]: # ( SHA256STAMP:245e056bdda7c8015917c89e243a0fd3bdd1512ca760da5d7f0a284cb3214ef7)
5 changes: 5 additions & 0 deletions doc/lightningd-config.5.md
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,11 @@ opens a channel before the channel is usable.
The percentage of *estimatesmartfee 2/CONSERVATIVE* to use for the commitment
transactions: default is 100.

* **commit-feerate-offset**=*INTEGER*

The additional feerate a channel opener adds to their preferred feerate to
lessen the odds of a disconnect due to feerate disagreement (default 5).

* **max-concurrent-htlcs**=*INTEGER*

Number of HTLCs one channel can handle concurrently in each direction.
Expand Down
23 changes: 23 additions & 0 deletions doc/schemas/listconfigs.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1351,6 +1351,24 @@
"description": "source of configuration setting"
}
}
},
"commit-feerate-offset": {
"type": "object",
"additionalProperties": false,
"required": [
"value_int",
"source"
],
"properties": {
"value_int": {
"type": "u32",
"description": "field from config or cmdline, or default"
},
"source": {
"type": "string",
"description": "source of configuration setting"
}
}
}
}
},
Expand Down Expand Up @@ -1770,6 +1788,11 @@
"type": "msat",
"added": "v23.08",
"description": "field from config or cmdline, or default"
},
"commit-feerate-offset": {
"type": "u32",
"added": "v23.11",
"description": "additional commitment feerate applied by channel owner"
}
}
}
7 changes: 7 additions & 0 deletions lightningd/channel_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ void channel_update_feerates(struct lightningd *ld, const struct channel *channe
else
min_feerate = feerate_min(ld, NULL);
max_feerate = feerate_max(ld, NULL);
/* The channel opener should use a slightly higher than minimal feerate
* in order to avoid excessive feerate disagreements */
if (channel->opener == LOCAL) {
feerate += ld->config.feerate_offset;
if (feerate > max_feerate)
feerate = max_feerate;
}

if (channel->ignore_fee_limits || ld->config.ignore_fee_limits) {
min_feerate = 1;
Expand Down
3 changes: 3 additions & 0 deletions lightningd/lightningd.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ struct config {

/* Percent of CONSERVATIVE/2 feerate we'll use for commitment txs. */
u64 commit_fee_percent;

/* Commit feerate offset above min_feerate to use as a channel opener */
u32 feerate_offset;
};

typedef STRMAP(const char *) alt_subdaemon_map;
Expand Down
6 changes: 6 additions & 0 deletions lightningd/options.c
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,7 @@ static const struct config testnet_config = {

.max_fee_multiplier = 10,
.commit_fee_percent = 100,
.feerate_offset = 5,
};

/* aka. "Dude, where's my coins?" */
Expand Down Expand Up @@ -1022,6 +1023,7 @@ static const struct config mainnet_config = {

.max_fee_multiplier = 10,
.commit_fee_percent = 100,
.feerate_offset = 5,
};

static void check_config(struct lightningd *ld)
Expand Down Expand Up @@ -1576,6 +1578,10 @@ static void register_opts(struct lightningd *ld)
clnopt_witharg("--commit-fee", OPT_SHOWINT,
opt_set_u64, opt_show_u64, &ld->config.commit_fee_percent,
"Percentage of fee to request for their commitment");
clnopt_witharg("--commit-feerate-offset", OPT_SHOWINT,
opt_set_u32, opt_show_u32, &ld->config.feerate_offset,
"Additional feerate per kw to apply to feerate updates "
"as the channel opener");
clnopt_witharg("--min-emergency-msat", OPT_SHOWMSATS,
opt_set_sat_nondust, opt_show_sat, &ld->emergency_sat,
"Amount to leave in wallet for spending anchor closes");
Expand Down
12 changes: 6 additions & 6 deletions tests/test_closing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1251,7 +1251,7 @@ def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams, anchors):

# reconnect with l1, which will fulfill the payment
l2.rpc.connect(l1.info['id'], 'localhost', l1.port)
l2.daemon.wait_for_log('got commitsig .*: feerate 11000, blockheight: 0, 0 added, 1 fulfilled, 0 failed, 0 changed')
l2.daemon.wait_for_log('got commitsig .*: feerate 11005, blockheight: 0, 0 added, 1 fulfilled, 0 failed, 0 changed')

# l2 moves on for closed l3
bitcoind.generate_block(1)
Expand Down Expand Up @@ -1463,7 +1463,7 @@ def test_penalty_htlc_tx_timeout(node_factory, bitcoind, chainparams, anchors):

# reconnect with l1, which will fulfill the payment
l2.rpc.connect(l1.info['id'], 'localhost', l1.port)
l2.daemon.wait_for_log('got commitsig .*: feerate {}, blockheight: 0, 0 added, 1 fulfilled, 0 failed, 0 changed'.format(3750 if anchors else 11000))
l2.daemon.wait_for_log('got commitsig .*: feerate {}, blockheight: 0, 0 added, 1 fulfilled, 0 failed, 0 changed'.format(3755 if anchors else 11005))

# l2 moves on for closed l3
bitcoind.generate_block(1, wait_for_mempool=1)
Expand Down Expand Up @@ -2650,12 +2650,12 @@ def test_onchain_different_fees(node_factory, bitcoind, executor):

# Both sides should have correct feerate
assert l1.db_query('SELECT min_possible_feerate, max_possible_feerate FROM channels;') == [{
'min_possible_feerate': 5000,
'max_possible_feerate': 11000
'min_possible_feerate': 5005,
'max_possible_feerate': 11005
}]
assert l2.db_query('SELECT min_possible_feerate, max_possible_feerate FROM channels;') == [{
'min_possible_feerate': 5000,
'max_possible_feerate': 11000
'min_possible_feerate': 5005,
'max_possible_feerate': 11005
}]

bitcoind.generate_block(5)
Expand Down
22 changes: 11 additions & 11 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -2412,7 +2412,7 @@ def test_update_fee(node_factory, bitcoind):
# Make payments.
l1.pay(l2, 200000000)
# First payment causes fee update.
l2.daemon.wait_for_log('peer updated fee to 11000')
l2.daemon.wait_for_log('peer updated fee to 11005')
l2.pay(l1, 100000000)

# Now shutdown cleanly.
Expand Down Expand Up @@ -2454,22 +2454,22 @@ def test_fee_limits(node_factory, bitcoind):
l1.set_feerates((15, 15, 15, 15), False)
l1.start()

l1.daemon.wait_for_log('Received WARNING .*: update_fee 253 outside range 1875-75000')
l1.daemon.wait_for_log('Received WARNING .*: update_fee 258 outside range 1875-75000')
# They hang up on *us*
l1.daemon.wait_for_log('Peer transient failure in CHANNELD_NORMAL: channeld: Owning subdaemon channeld died')

# Disconnects, but does not error. Make sure it's noted in their status though.
# FIXME: does not happen for l1!
# assert 'update_fee 253 outside range 1875-75000' in only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status'][0]
assert 'update_fee 253 outside range 1875-75000' in only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['status'][0]
assert 'update_fee 258 outside range 1875-75000' in only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['status'][0]

assert only_one(l2.rpc.listpeerchannels()['channels'])['feerate']['perkw'] != 253
assert only_one(l2.rpc.listpeerchannels()['channels'])['feerate']['perkw'] != 258
# Make l2 accept those fees, and it should recover.
assert only_one(l2.rpc.setchannel(l1.get_channel_scid(l2), ignorefeelimits=True)['channels'])['ignore_fee_limits'] is True
assert only_one(l2.rpc.listpeerchannels()['channels'])['ignore_fee_limits'] is True

# Now we stay happy (and connected!)
wait_for(lambda: only_one(l2.rpc.listpeerchannels()['channels'])['feerate']['perkw'] == 253)
wait_for(lambda: only_one(l2.rpc.listpeerchannels()['channels'])['feerate']['perkw'] == 258)
assert only_one(l2.rpc.listpeerchannels()['channels'])['peer_connected'] is True

l1.rpc.close(l2.info['id'])
Expand Down Expand Up @@ -2583,19 +2583,19 @@ def test_update_fee_reconnect(node_factory, bitcoind):
# Make l1 send out feechange; triggers disconnect/reconnect.
# (Note: < 10% change, so no smoothing here!)
l1.set_feerates((14000, 14000, 14000, 3750))
l1.daemon.wait_for_log('Setting REMOTE feerate to 14000')
l2.daemon.wait_for_log('Setting LOCAL feerate to 14000')
l1.daemon.wait_for_log('Setting REMOTE feerate to 14005')
l2.daemon.wait_for_log('Setting LOCAL feerate to 14005')
l1.daemon.wait_for_log(r'dev_disconnect: \+WIRE_COMMITMENT_SIGNED')

# Wait for reconnect....
l1.daemon.wait_for_log('Feerate:.*LOCAL now 14000')
l1.daemon.wait_for_log('Feerate:.*LOCAL now 14005')

l1.pay(l2, 200000000)
l2.pay(l1, 100000000)

# They should both have gotten commits with correct feerate.
assert l1.daemon.is_in_log('got commitsig [0-9]*: feerate 14000')
assert l2.daemon.is_in_log('got commitsig [0-9]*: feerate 14000')
assert l1.daemon.is_in_log('got commitsig [0-9]*: feerate 14005')
assert l2.daemon.is_in_log('got commitsig [0-9]*: feerate 14005')

# Now shutdown cleanly.
l1.rpc.close(chan)
Expand Down Expand Up @@ -3347,7 +3347,7 @@ def test_feerate_spam(node_factory, chainparams):
l1.pay(l2, 10**9 - slack)

# It will send this once (may have happened before line_graph's wait)
wait_for(lambda: l1.daemon.is_in_log('Setting REMOTE feerate to 11000'))
wait_for(lambda: l1.daemon.is_in_log('Setting REMOTE feerate to 11005'))
wait_for(lambda: l1.daemon.is_in_log('peer_out WIRE_UPDATE_FEE'))

# Now change feerates to something l1 can't afford.
Expand Down
27 changes: 27 additions & 0 deletions tests/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -3831,3 +3831,30 @@ def test_even_sendcustommsg(node_factory):
l2.daemon.wait_for_log(r'\[IN\] {}'.format(msg))
l1.daemon.wait_for_log('Invalid unknown even msg')
wait_for(lambda: l1.rpc.listpeers(l2.info['id'])['peers'] == [])


def test_set_feerate_offset(node_factory, bitcoind):
opts = [{'commit-feerate-offset': 100}, {}]
l1, l2 = node_factory.get_nodes(2, opts=opts)
assert l1.daemon.is_in_log('Server started with public key')
configs = l1.rpc.listconfigs()['configs']
assert configs['commit-feerate-offset'] == {'source': 'cmdline',
'value_int': 100}
scid12 = l1.fundchannel(l2)[0]
# chanid = l1.get_channel_scid(l2)

# node 1 sets fees.
l1.set_feerates((14000, 11000, 7500, 3750))

l1.pay(l2, 200000000)
# First payment causes fee update, which should reflect the feerate offset.
l1.daemon.wait_for_log('lightningd: update_feerates: feerate = 11100, '
'min=1875, max=150000, penalty=7500')
l2.daemon.wait_for_log('peer updated fee to 11100')
l2.pay(l1, 100000000)

# Now shutdown cleanly.
l1.rpc.close(scid12)

l1.daemon.wait_for_log(' to CLOSINGD_COMPLETE')
l2.daemon.wait_for_log(' to CLOSINGD_COMPLETE')
Loading