Skip to content

Commit

Permalink
bcli: become less aggressive with onchain fee levels.
Browse files Browse the repository at this point in the history 
Users are more upset recently with the cost of unilateral closes
than they are the risk of being cheated.  While we complete our
anchor implementation so we can use low fees there, let's
get less aggressive (we already have 34 or 18 blocks to close
in the worst case).

The changes are:

- Commit transactions were "2 CONSERVATIVE" now "6 ECONOMICAL".
- HTLC resolution txs were "3 CONSERVATIVE" now "6 ECONOMICAL".
- Penalty txs were "3 CONSERVATIVE" now "12 ECONOMICAL".
- Normal txs were "4 ECONOMICAL" now "12 ECONOMICAL".

There can be no perfect levels, but we have had understandable
complaints recently about how high our default fee levels are.

Changelog-Changed: Protocol: channel feerates reduced to bitcoind's "6 block ECONOMICAL" rate.
Signed-off-by: Rusty Russell <[email protected]>
  • Loading branch information
@rustyrussell
rustyrussell committed May 11, 2021
1 parent 9a22e7b commit d8e6889
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 37 deletions.
4 changes: 2 additions & 2 deletions contrib/pyln-testing/pyln/testing/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1014,9 +1014,9 @@ def mock_estimatesmartfee(r):
params = r['params']
if params == [2, 'CONSERVATIVE']:
feerate = feerates[0] * 4
elif params == [3, 'CONSERVATIVE']:
elif params == [6, 'ECONOMICAL']:
feerate = feerates[1] * 4
elif params == [4, 'ECONOMICAL']:
elif params == [12, 'ECONOMICAL']:
feerate = feerates[2] * 4
elif params == [100, 'ECONOMICAL']:
feerate = feerates[3] * 4
Expand Down
18 changes: 7 additions & 11 deletions plugins/bcli.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ static struct command_result *process_getblockchaininfo(struct bitcoin_cli *bcli
}

enum feerate_levels {
FEERATE_VERY_URGENT,
FEERATE_HIGHEST,
FEERATE_URGENT,
FEERATE_NORMAL,
FEERATE_SLOW,
Expand Down Expand Up @@ -604,19 +604,15 @@ static struct command_result *getchaininfo(struct command *cmd,
/* Mutual recursion. */
static struct command_result *estimatefees_done(struct bitcoin_cli *bcli);

/*
* Calls `estimatesmartfee` with targets 2/CONSERVATIVE (very urgent),
* 3/CONSERVATIVE (urgent), 4/ECONOMICAL (normal), and 100/ECONOMICAL (slow)
*/
struct estimatefee_params {
u32 blocks;
const char *style;
};

static const struct estimatefee_params estimatefee_params[] = {
[FEERATE_VERY_URGENT] = { 2, "CONSERVATIVE" },
[FEERATE_URGENT] = { 3, "CONSERVATIVE" },
[FEERATE_NORMAL] = { 4, "ECONOMICAL" },
[FEERATE_HIGHEST] = { 2, "CONSERVATIVE" },
[FEERATE_URGENT] = { 6, "ECONOMICAL" },
[FEERATE_NORMAL] = { 12, "ECONOMICAL" },
[FEERATE_SLOW] = { 100, "ECONOMICAL" },
};

Expand All @@ -640,10 +636,10 @@ static struct command_result *estimatefees_next(struct command *cmd,
json_add_u64(response, "opening", stash->perkb[FEERATE_NORMAL]);
json_add_u64(response, "mutual_close", stash->perkb[FEERATE_SLOW]);
json_add_u64(response, "unilateral_close",
stash->perkb[FEERATE_VERY_URGENT] * bitcoind->commit_fee_percent / 100);
stash->perkb[FEERATE_URGENT] * bitcoind->commit_fee_percent / 100);
json_add_u64(response, "delayed_to_us", stash->perkb[FEERATE_NORMAL]);
json_add_u64(response, "htlc_resolution", stash->perkb[FEERATE_URGENT]);
json_add_u64(response, "penalty", stash->perkb[FEERATE_URGENT]);
json_add_u64(response, "penalty", stash->perkb[FEERATE_NORMAL]);
/* We divide the slow feerate for the minimum acceptable, lightningd
* will use floor if it's hit, though. */
json_add_u64(response, "min_acceptable",
Expand All @@ -655,7 +651,7 @@ static struct command_result *estimatefees_next(struct command *cmd,
* margin (say 5x the expected fee requirement)
*/
json_add_u64(response, "max_acceptable",
stash->perkb[FEERATE_VERY_URGENT]
stash->perkb[FEERATE_HIGHEST]
* bitcoind->max_fee_multiplier);
return command_finished(cmd, response);
}
Expand Down
8 changes: 4 additions & 4 deletions tests/test_closing.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams):

# 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 15000, 0 added, 1 fulfilled, 0 failed, 0 changed')
l2.daemon.wait_for_log('got commitsig .*: feerate 11000, 0 added, 1 fulfilled, 0 failed, 0 changed')
l2.daemon.wait_for_log('coins payment_hash: {}'.format(sticky_inv['payment_hash']))

# l2 moves on for closed l3
Expand Down Expand Up @@ -970,7 +970,7 @@ def test_penalty_htlc_tx_timeout(node_factory, bitcoind, chainparams):

# 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 15000, 0 added, 1 fulfilled, 0 failed, 0 changed')
l2.daemon.wait_for_log('got commitsig .*: feerate 11000, 0 added, 1 fulfilled, 0 failed, 0 changed')
l2.daemon.wait_for_log('coins payment_hash: {}'.format(sticky_inv_2['payment_hash']))

# l2 moves on for closed l3
Expand Down Expand Up @@ -2000,11 +2000,11 @@ 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': 16000
'max_possible_feerate': 11000
}]
assert l2.db_query('SELECT min_possible_feerate, max_possible_feerate FROM channels;') == [{
'min_possible_feerate': 5000,
'max_possible_feerate': 16000
'max_possible_feerate': 11000
}]

bitcoind.generate_block(5)
Expand Down
6 changes: 3 additions & 3 deletions tests/test_connection.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1969,7 +1969,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 14000')
l2.daemon.wait_for_log('peer updated fee to 11000')
l2.pay(l1, 100000000)

# Now shutdown cleanly.
Expand Down Expand Up @@ -2049,7 +2049,7 @@ def test_fee_limits(node_factory, bitcoind):

# Try stupid high fees
l1.stop()
l1.set_feerates((15000 * 10, 11000, 7500, 3750), False)
l1.set_feerates((15000, 11000 * 10, 7500, 3750), False)
l1.start()

l3.daemon.wait_for_log('peer_in WIRE_UPDATE_FEE')
Expand Down Expand Up @@ -2892,7 +2892,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 15000'))
wait_for(lambda: l1.daemon.is_in_log('Setting REMOTE feerate to 11000'))
wait_for(lambda: l1.daemon.is_in_log('peer_out WIRE_UPDATE_FEE'))

# Now change feerates to something l1 can't afford.
Expand Down
28 changes: 12 additions & 16 deletions tests/test_misc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1470,54 +1470,50 @@ def test_feerates(node_factory):
assert t not in feerates['perkb']

# Now try setting them, one at a time.
# Set CONSERVATIVE/2 feerate, for max and unilateral_close
# Set CONSERVATIVE/2 feerate, for max
l1.set_feerates((15000, 0, 0, 0), True)
wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) == 3)
wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) == 2)
feerates = l1.rpc.feerates('perkw')
assert feerates['perkw']['unilateral_close'] == 15000
assert feerates['warning_missing_feerates'] == 'Some fee estimates unavailable: bitcoind startup?'
assert 'perkb' not in feerates
assert feerates['perkw']['max_acceptable'] == 15000 * 10
assert feerates['perkw']['min_acceptable'] == 253

# Set CONSERVATIVE/3 feerate, for htlc_resolution and penalty
# Set ECONOMICAL/6 feerate, for unilateral_close and htlc_resolution
l1.set_feerates((15000, 11000, 0, 0), True)
wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) == 5)
wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) == 4)
feerates = l1.rpc.feerates('perkw')
assert feerates['perkw']['unilateral_close'] == 15000
assert feerates['perkw']['unilateral_close'] == 11000
assert feerates['perkw']['htlc_resolution'] == 11000
assert feerates['perkw']['penalty'] == 11000
assert feerates['warning_missing_feerates'] == 'Some fee estimates unavailable: bitcoind startup?'
assert 'perkb' not in feerates
assert feerates['perkw']['max_acceptable'] == 15000 * 10
assert feerates['perkw']['min_acceptable'] == 253

# Set ECONOMICAL/4 feerate, for all but min (so, no mutual_close feerate)
# Set ECONOMICAL/12 feerate, for all but min (so, no mutual_close feerate)
l1.set_feerates((15000, 11000, 6250, 0), True)
wait_for(lambda: len(l1.rpc.feerates('perkb')['perkb']) == len(types) - 1 + 2)
feerates = l1.rpc.feerates('perkb')
assert feerates['perkb']['unilateral_close'] == 15000 * 4
assert feerates['perkb']['unilateral_close'] == 11000 * 4
assert feerates['perkb']['htlc_resolution'] == 11000 * 4
assert feerates['perkb']['penalty'] == 11000 * 4
assert 'mutual_close' not in feerates['perkb']
for t in types:
if t not in ("unilateral_close", "htlc_resolution", "penalty", "mutual_close"):
if t not in ("unilateral_close", "htlc_resolution", "mutual_close"):
assert feerates['perkb'][t] == 25000
assert feerates['warning_missing_feerates'] == 'Some fee estimates unavailable: bitcoind startup?'
assert 'perkw' not in feerates
assert feerates['perkb']['max_acceptable'] == 15000 * 4 * 10
assert feerates['perkb']['min_acceptable'] == 253 * 4

# Set ECONOMICAL/100 feerate for min
# Set ECONOMICAL/100 feerate for min and mutual_close
l1.set_feerates((15000, 11000, 6250, 5000), True)
wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) >= len(types) + 2)
feerates = l1.rpc.feerates('perkw')
assert feerates['perkw']['unilateral_close'] == 15000
assert feerates['perkw']['unilateral_close'] == 11000
assert feerates['perkw']['htlc_resolution'] == 11000
assert feerates['perkw']['penalty'] == 11000
assert feerates['perkw']['mutual_close'] == 5000
for t in types:
if t not in ("unilateral_close", "htlc_resolution", "penalty", "mutual_close"):
if t not in ("unilateral_close", "htlc_resolution", "mutual_close"):
assert feerates['perkw'][t] == 25000 // 4
assert 'warning' not in feerates
assert 'perkb' not in feerates
Expand Down Expand Up @@ -2402,7 +2398,7 @@ def test_commitfee_option(node_factory):

mock_wu = 5000
for l in [l1, l2]:
l.set_feerates((mock_wu, 0, 0, 0), True)
l.set_feerates((0, mock_wu, 0, 0), True)
l1_commit_fees = l1.rpc.call("estimatefees")["unilateral_close"]
l2_commit_fees = l2.rpc.call("estimatefees")["unilateral_close"]

Expand Down
2 changes: 1 addition & 1 deletion tests/test_pay.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ def test_pay_disconnect(node_factory, bitcoind):
l1.daemon.wait_for_log('peer_out WIRE_CHANNEL_REESTABLISH')

# Make l2 upset by asking for crazy fee.
l1.set_feerates((10**6, 1000**6, 1000**6, 1000**6), False)
l1.set_feerates((10**6, 10**6, 10**6, 10**6), False)

# Wait for l1 notice
l1.daemon.wait_for_log(r'Peer transient failure in CHANNELD_NORMAL: channeld WARNING: .*: update_fee \d+ outside range 1875-75000')
Expand Down

0 comments on commit d8e6889

Please sign in to comment.