Skip to content

Commit

Permalink
lightningd: add a feerate offset when updating feerates as opener
Browse files Browse the repository at this point in the history
Adding a fee offset as the channel opener reduces the likelihood of a
disconnect by the peer do to slight variation in feerate calculation
between nodes.

Changelog-Fixed: Some peer disconnects due to update_fee disagreements are avoided.
  • Loading branch information
endothermicdev authored and rustyrussell committed Nov 1, 2023
1 parent 577075c commit 4265699
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 17 deletions.
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
2 changes: 2 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
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

0 comments on commit 4265699

Please sign in to comment.