plugins/bcli: use the new feerate levels, and the floor.

Fixes: #4473
Changelog-Deprecated: Plugins: `estimatefees` returning feerates by name (e.g. "opening"); use `fee_floor` and `feerates`.
Changelog-Fixed: Plugins: `bcli` now tells us the minimal possible feerate, such as with mempool congestion, rather than assuming 1 sat/vbyte.
This commit is contained in:
Rusty Russell
2023-04-07 14:14:18 +09:30
parent 9e2d4240b1
commit 812a5a14c0
4 changed files with 89 additions and 62 deletions

View File

@@ -309,6 +309,11 @@ static void estimatefees_callback(const char *buf, const jsmntok_t *toks,
"feerates"), "feerates"),
&floor); &floor);
} else { } else {
if (!deprecated_apis)
bitcoin_plugin_error(call->bitcoind, buf, resulttok,
"estimatefees",
"missing fee_floor field");
feerates = parse_deprecated_feerates(call, call->bitcoind, feerates = parse_deprecated_feerates(call, call->bitcoind,
buf, resulttok); buf, resulttok);
floor = feerate_from_style(FEERATE_FLOOR, FEERATE_PER_KSIPA); floor = feerate_from_style(FEERATE_FLOOR, FEERATE_PER_KSIPA);

View File

@@ -456,20 +456,24 @@ static struct command_result *process_getblockchaininfo(struct bitcoin_cli *bcli
return command_finished(bcli->cmd, response); return command_finished(bcli->cmd, response);
} }
enum feerate_levels { struct estimatefee_params {
FEERATE_HIGHEST, u32 blocks;
FEERATE_URGENT, const char *style;
FEERATE_NORMAL, };
FEERATE_SLOW,
static const struct estimatefee_params estimatefee_params[] = {
{ 2, "CONSERVATIVE" },
{ 6, "ECONOMICAL" },
{ 12, "ECONOMICAL" },
{ 100, "ECONOMICAL" },
}; };
#define FEERATE_LEVEL_MAX (FEERATE_SLOW)
struct estimatefees_stash { struct estimatefees_stash {
/* This is max(mempoolminfee,minrelaytxfee) */ /* This is max(mempoolminfee,minrelaytxfee) */
u64 perkb_floor; u64 perkb_floor;
u32 cursor; u32 cursor;
/* FIXME: We use u64 but lightningd will store them as u32. */ /* FIXME: We use u64 but lightningd will store them as u32. */
u64 perkb[FEERATE_LEVEL_MAX+1]; u64 perkb[ARRAY_SIZE(estimatefee_params)];
}; };
static struct command_result * static struct command_result *
@@ -477,14 +481,21 @@ estimatefees_null_response(struct bitcoin_cli *bcli)
{ {
struct json_stream *response = jsonrpc_stream_success(bcli->cmd); struct json_stream *response = jsonrpc_stream_success(bcli->cmd);
json_add_null(response, "opening"); /* We give a floor, which is the standard minimum */
json_add_null(response, "mutual_close"); json_array_start(response, "feerates");
json_add_null(response, "unilateral_close"); json_array_end(response);
json_add_null(response, "delayed_to_us"); json_add_u32(response, "feerate_floor", 1000);
json_add_null(response, "htlc_resolution");
json_add_null(response, "penalty"); if (deprecated_apis) {
json_add_null(response, "min_acceptable"); json_add_null(response, "opening");
json_add_null(response, "max_acceptable"); json_add_null(response, "mutual_close");
json_add_null(response, "unilateral_close");
json_add_null(response, "delayed_to_us");
json_add_null(response, "htlc_resolution");
json_add_null(response, "penalty");
json_add_null(response, "min_acceptable");
json_add_null(response, "max_acceptable");
}
return command_finished(bcli->cmd, response); return command_finished(bcli->cmd, response);
} }
@@ -658,18 +669,6 @@ static struct command_result *getchaininfo(struct command *cmd,
/* Mutual recursion. */ /* Mutual recursion. */
static struct command_result *estimatefees_done(struct bitcoin_cli *bcli); 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" },
};
/* Add a feerate, but don't publish one that bitcoind won't accept. */ /* Add a feerate, but don't publish one that bitcoind won't accept. */
static void json_add_feerate(struct json_stream *result, const char *fieldname, static void json_add_feerate(struct json_stream *result, const char *fieldname,
struct command *cmd, struct command *cmd,
@@ -688,6 +687,16 @@ static void json_add_feerate(struct json_stream *result, const char *fieldname,
} }
} }
static u32 feerate_for_block(const struct estimatefees_stash *stash, u32 blocks)
{
for (size_t i = 0; i < ARRAY_SIZE(stash->perkb); i++) {
if (estimatefee_params[i].blocks != blocks)
continue;
return stash->perkb[i];
}
abort();
}
static struct command_result *estimatefees_next(struct command *cmd, static struct command_result *estimatefees_next(struct command *cmd,
struct estimatefees_stash *stash) struct estimatefees_stash *stash)
{ {
@@ -706,30 +715,45 @@ static struct command_result *estimatefees_next(struct command *cmd,
} }
response = jsonrpc_stream_success(cmd); response = jsonrpc_stream_success(cmd);
json_add_feerate(response, "opening", cmd, stash, if (deprecated_apis) {
stash->perkb[FEERATE_NORMAL]); json_add_feerate(response, "opening", cmd, stash,
json_add_feerate(response, "mutual_close", cmd, stash, feerate_for_block(stash, 12));
stash->perkb[FEERATE_SLOW]); json_add_feerate(response, "mutual_close", cmd, stash,
json_add_feerate(response, "unilateral_close", cmd, stash, feerate_for_block(stash, 100));
stash->perkb[FEERATE_URGENT]); json_add_feerate(response, "unilateral_close", cmd, stash,
json_add_feerate(response, "delayed_to_us", cmd, stash, feerate_for_block(stash, 6));
stash->perkb[FEERATE_NORMAL]); json_add_feerate(response, "delayed_to_us", cmd, stash,
json_add_feerate(response, "htlc_resolution", cmd, stash, feerate_for_block(stash, 12));
stash->perkb[FEERATE_URGENT]); json_add_feerate(response, "htlc_resolution", cmd, stash,
json_add_feerate(response, "penalty", cmd, stash, feerate_for_block(stash, 6));
stash->perkb[FEERATE_NORMAL]); json_add_feerate(response, "penalty", cmd, stash,
/* We divide the slow feerate for the minimum acceptable, lightningd feerate_for_block(stash, 12));
* will use floor if it's hit, though. */ /* We divide the slow feerate for the minimum acceptable, lightningd
json_add_feerate(response, "min_acceptable", cmd, stash, * will use floor if it's hit, though. */
stash->perkb[FEERATE_SLOW] / 2); json_add_feerate(response, "min_acceptable", cmd, stash,
/* BOLT #2: feerate_for_block(stash, 100) / 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 * Given the variance in fees, and the fact that the transaction may be
* margin (say 5x the expected fee requirement) * 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_feerate(response, "max_acceptable", cmd, stash, */
stash->perkb[FEERATE_HIGHEST] * 10); json_add_feerate(response, "max_acceptable", cmd, stash,
feerate_for_block(stash, 2) * 10);
}
/* Modern style: present an ordered array of block deadlines, and a floor. */
json_array_start(response, "feerates");
for (size_t i = 0; i < ARRAY_SIZE(stash->perkb); i++) {
if (!stash->perkb[i])
continue;
json_object_start(response, NULL);
json_add_u32(response, "blocks", estimatefee_params[i].blocks);
json_add_feerate(response, "feerate", cmd, stash, stash->perkb[i]);
json_object_end(response);
}
json_array_end(response);
json_add_u64(response, "feerate_floor", stash->perkb_floor);
return command_finished(cmd, response); return command_finished(cmd, response);
} }

View File

@@ -86,11 +86,11 @@ def test_bitcoin_failure(node_factory, bitcoind):
l1.daemon.rpcproxy.mock_rpc('getblockhash', crash_bitcoincli) l1.daemon.rpcproxy.mock_rpc('getblockhash', crash_bitcoincli)
# This should cause both estimatefee and getblockhash fail # This should cause both estimatefee and getblockhash fail
l1.daemon.wait_for_logs(['Unable to estimate .* fee', l1.daemon.wait_for_logs(['Unable to estimate any fees',
'getblockhash .* exited with status 1']) 'getblockhash .* exited with status 1'])
# And they should retry! # And they should retry!
l1.daemon.wait_for_logs(['Unable to estimate .* fee', l1.daemon.wait_for_logs(['Unable to estimate any fees',
'getblockhash .* exited with status 1']) 'getblockhash .* exited with status 1'])
# Restore, then it should recover and get blockheight. # Restore, then it should recover and get blockheight.
@@ -1931,7 +1931,7 @@ def test_bitcoind_fail_first(node_factory, bitcoind):
l1.daemon.start(wait_for_initialized=False, stderr_redir=True) l1.daemon.start(wait_for_initialized=False, stderr_redir=True)
l1.daemon.wait_for_logs([r'getblockhash [a-z0-9]* exited with status 1', l1.daemon.wait_for_logs([r'getblockhash [a-z0-9]* exited with status 1',
r'Unable to estimate opening fees', r'Unable to estimate any fees',
r'BROKEN.*we have been retrying command for --bitcoin-retry-timeout={} seconds'.format(timeout)]) r'BROKEN.*we have been retrying command for --bitcoin-retry-timeout={} seconds'.format(timeout)])
# Will exit with failure code. # Will exit with failure code.
assert l1.daemon.wait() == 1 assert l1.daemon.wait() == 1
@@ -1990,8 +1990,8 @@ def test_bitcoind_feerate_floor(node_factory, bitcoind):
"mutual_close": 20004, "mutual_close": 20004,
"unilateral_close": 44000, "unilateral_close": 44000,
"penalty": 30000, "penalty": 30000,
# FIXME: this should increase: # This has increased (rounded up)
"min_acceptable": 10000, "min_acceptable": 20004,
"max_acceptable": 600000, "max_acceptable": 600000,
"estimates": [{"blockcount": 2, "estimates": [{"blockcount": 2,
"feerate": 60000, "feerate": 60000,
@@ -2031,8 +2031,8 @@ def test_bitcoind_feerate_floor(node_factory, bitcoind):
"unilateral_close": 44000, "unilateral_close": 44000,
# This has increased (rounded up!) # This has increased (rounded up!)
"penalty": 30004, "penalty": 30004,
# FIXME: this should increase to 30004! # This has increased (rounded up)
"min_acceptable": 15000, "min_acceptable": 30004,
"max_acceptable": 600000, "max_acceptable": 600000,
"estimates": [{"blockcount": 2, "estimates": [{"blockcount": 2,
"feerate": 60000, "feerate": 60000,

View File

@@ -1743,10 +1743,8 @@ def test_bcli(node_factory, bitcoind, chainparams):
# Failure case of feerate is tested in test_misc.py # Failure case of feerate is tested in test_misc.py
estimates = l1.rpc.call("estimatefees") estimates = l1.rpc.call("estimatefees")
for est in ["opening", "mutual_close", "unilateral_close", "delayed_to_us", assert 'feerate_floor' in estimates
"htlc_resolution", "penalty", "min_acceptable", assert [f['blocks'] for f in estimates['feerates']] == [2, 6, 12, 100]
"max_acceptable"]:
assert est in estimates
resp = l1.rpc.call("getchaininfo") resp = l1.rpc.call("getchaininfo")
assert resp["chain"] == chainparams['name'] assert resp["chain"] == chainparams['name']