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

Reduce default fees #4507

Merged
merged 3 commits into from
May 11, 2021
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
1 change: 1 addition & 0 deletions connectd/connectd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1607,6 +1607,7 @@ static struct io_plan *peer_disconnected(struct io_conn *conn,
"peer_disconnected unknown peer: %s",
type_to_string(tmpctx, struct node_id, &id));
node_set_del(&daemon->peers, node);
status_peer_debug(&id, "disconnect");

/* Wake up in case there's a reconnecting peer waiting in io_wait. */
io_wake(node);
Expand Down
4 changes: 2 additions & 2 deletions contrib/pyln-testing/pyln/testing/utils.py
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
205 changes: 83 additions & 122 deletions plugins/bcli.c
Original file line number Diff line number Diff line change
Expand Up @@ -400,10 +400,18 @@ static struct command_result *process_getblockchaininfo(struct bitcoin_cli *bcli
return command_finished(bcli->cmd, response);
}

enum feerate_levels {
FEERATE_HIGHEST,
FEERATE_URGENT,
FEERATE_NORMAL,
FEERATE_SLOW,
};
#define FEERATE_LEVEL_MAX (FEERATE_SLOW)

struct estimatefees_stash {
u32 cursor;
/* FIXME: We use u64 but lightningd will store them as u32. */
u64 very_urgent, urgent, normal, slow;
u64 perkb[FEERATE_LEVEL_MAX+1];
};

static struct command_result *
Expand Down Expand Up @@ -452,114 +460,6 @@ estimatefees_parse_feerate(struct bitcoin_cli *bcli, u64 *feerate)
return NULL;
}

/* We got all the feerates, give them to lightningd. */
static struct command_result *estimatefees_final_step(struct bitcoin_cli *bcli)
{
struct command_result *err;
struct json_stream *response;
struct estimatefees_stash *stash = bcli->stash;

/* bitcoind could theoretically fail to estimate for a higher target. */
if (*bcli->exitstatus != 0)
return estimatefees_null_response(bcli);

err = estimatefees_parse_feerate(bcli, &stash->slow);
if (err)
return err;

response = jsonrpc_stream_success(bcli->cmd);
json_add_u64(response, "opening", stash->normal);
json_add_u64(response, "mutual_close", stash->slow);
json_add_u64(response, "unilateral_close",
stash->very_urgent * bitcoind->commit_fee_percent / 100);
json_add_u64(response, "delayed_to_us", stash->normal);
json_add_u64(response, "htlc_resolution", stash->urgent);
json_add_u64(response, "penalty", stash->urgent);
/* We divide the slow feerate for the minimum acceptable, lightningd
* will use floor if it's hit, though. */
json_add_u64(response, "min_acceptable", stash->slow / 2);
/* BOLT #2:
*
* Given the variance in fees, and the fact that the transaction may be
* spent in the future, it's a good idea for the fee payer to keep a good
* margin (say 5x the expected fee requirement)
*/
json_add_u64(response, "max_acceptable",
stash->very_urgent * bitcoind->max_fee_multiplier);

return command_finished(bcli->cmd, response);
}

/* We got the response for the normal feerate, now treat the slow one. */
static struct command_result *estimatefees_fourth_step(struct bitcoin_cli *bcli)
{
struct command_result *err;
struct estimatefees_stash *stash = bcli->stash;
const char **params = tal_arr(bcli->cmd, const char *, 2);

/* bitcoind could theoretically fail to estimate for a higher target. */
if (*bcli->exitstatus != 0)
return estimatefees_null_response(bcli);

err = estimatefees_parse_feerate(bcli, &stash->normal);
if (err)
return err;

params[0] = "100";
params[1] = "ECONOMICAL";
start_bitcoin_cli(NULL, bcli->cmd, estimatefees_final_step, true,
BITCOIND_LOW_PRIO, "estimatesmartfee", params, stash);

return command_still_pending(bcli->cmd);
}

/* We got the response for the urgent feerate, now treat the normal one. */
static struct command_result *estimatefees_third_step(struct bitcoin_cli *bcli)
{
struct command_result *err;
struct estimatefees_stash *stash = bcli->stash;
const char **params = tal_arr(bcli->cmd, const char *, 2);

/* If we cannot estimate fees, no need to continue bothering bitcoind. */
if (*bcli->exitstatus != 0)
return estimatefees_null_response(bcli);

err = estimatefees_parse_feerate(bcli, &stash->urgent);
if (err)
return err;

params[0] = "4";
params[1] = "ECONOMICAL";
start_bitcoin_cli(NULL, bcli->cmd, estimatefees_fourth_step, true,
BITCOIND_LOW_PRIO, "estimatesmartfee", params, stash);

return command_still_pending(bcli->cmd);
}

/* We got the response for the very urgent feerate, now treat the urgent one. */
static struct command_result *estimatefees_second_step(struct bitcoin_cli *bcli)
{
struct command_result *err;
struct estimatefees_stash *stash = bcli->stash;
const char **params = tal_arr(bcli->cmd, const char *, 2);

/* If we cannot estimate fees, no need to continue bothering bitcoind. */
if (*bcli->exitstatus != 0)
return estimatefees_null_response(bcli);

err = estimatefees_parse_feerate(bcli, &stash->very_urgent);
if (err)
return err;

params[0] = "3";
params[1] = "CONSERVATIVE";
start_bitcoin_cli(NULL, bcli->cmd, estimatefees_third_step, true,
BITCOIND_LOW_PRIO, "estimatesmartfee", params, stash);

return command_still_pending(bcli->cmd);
}


static struct command_result *process_sendrawtransaction(struct bitcoin_cli *bcli)
{
struct json_stream *response;
Expand Down Expand Up @@ -694,32 +594,93 @@ static struct command_result *getchaininfo(struct command *cmd,
return command_still_pending(cmd);
}

/* Mutual recursion. */
static struct command_result *estimatefees_done(struct bitcoin_cli *bcli);

struct estimatefee_params {
u32 blocks;
const char *style;
};

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

static struct command_result *estimatefees_next(struct command *cmd,
struct estimatefees_stash *stash)
{
struct json_stream *response;

if (stash->cursor < ARRAY_SIZE(stash->perkb)) {
const char **params = tal_arr(cmd, const char *, 2);

params[0] = tal_fmt(params, "%u", estimatefee_params[stash->cursor].blocks);
params[1] = estimatefee_params[stash->cursor].style;
start_bitcoin_cli(NULL, cmd, estimatefees_done, true,
BITCOIND_LOW_PRIO, "estimatesmartfee", params, stash);

return command_still_pending(cmd);
}

response = jsonrpc_stream_success(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_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_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",
stash->perkb[FEERATE_SLOW] / 2);
/* BOLT #2:
*
* Given the variance in fees, and the fact that the transaction may be
* spent in the future, it's a good idea for the fee payer to keep a good
* margin (say 5x the expected fee requirement)
*/
json_add_u64(response, "max_acceptable",
stash->perkb[FEERATE_HIGHEST]
* bitcoind->max_fee_multiplier);
return command_finished(cmd, response);
}

/* Get the current feerates. We use an urgent feerate for unilateral_close and max,
* a slightly less urgent feerate for htlc_resolution and penalty transactions,
* a slow feerate for min, and a normal one for all others.
*
* Calls `estimatesmartfee` with targets 2/CONSERVATIVE (very urgent),
* 3/CONSERVATIVE (urgent), 4/ECONOMICAL (normal), and 100/ECONOMICAL (slow)
* then returns the feerates as sat/kVB.
*/
static struct command_result *estimatefees(struct command *cmd,
const char *buf UNUSED,
const jsmntok_t *toks UNUSED)
{
struct estimatefees_stash *stash = tal(cmd, struct estimatefees_stash);
const char **params = tal_arr(cmd, const char *, 2);

if (!param(cmd, buf, toks, NULL))
return command_param_failed();
return command_param_failed();

/* First call to estimatesmartfee, for very urgent estimation (unilateral
* and max_acceptable feerates). */
params[0] = "2";
params[1] = "CONSERVATIVE";
start_bitcoin_cli(NULL, cmd, estimatefees_second_step, true,
BITCOIND_LOW_PRIO, "estimatesmartfee", params, stash);
stash->cursor = 0;
return estimatefees_next(cmd, stash);
}

return command_still_pending(cmd);
static struct command_result *estimatefees_done(struct bitcoin_cli *bcli)
{
struct command_result *err;
struct estimatefees_stash *stash = bcli->stash;

/* If we cannot estimate fees, no need to continue bothering bitcoind. */
if (*bcli->exitstatus != 0)
return estimatefees_null_response(bcli);

err = estimatefees_parse_feerate(bcli, &stash->perkb[stash->cursor]);
if (err)
return err;

stash->cursor++;
return estimatefees_next(bcli->cmd, stash);
}

/* Send a transaction to the Bitcoin network.
Expand Down
8 changes: 4 additions & 4 deletions tests/test_closing.py
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
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
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
Loading