diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index 3db25e2ce..e4bbb1eba 100644 --- a/lightningd/bitcoind.c +++ b/lightningd/bitcoind.c @@ -309,6 +309,11 @@ static void estimatefees_callback(const char *buf, const jsmntok_t *toks, "feerates"), &floor); } else { + if (!deprecated_apis) + bitcoin_plugin_error(call->bitcoind, buf, resulttok, + "estimatefees", + "missing fee_floor field"); + feerates = parse_deprecated_feerates(call, call->bitcoind, buf, resulttok); floor = feerate_from_style(FEERATE_FLOOR, FEERATE_PER_KSIPA); diff --git a/plugins/bcli.c b/plugins/bcli.c index 57ab9edf4..a13a939bd 100644 --- a/plugins/bcli.c +++ b/plugins/bcli.c @@ -456,20 +456,24 @@ 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, +struct estimatefee_params { + u32 blocks; + const char *style; +}; + +static const struct estimatefee_params estimatefee_params[] = { + { 2, "CONSERVATIVE" }, + { 6, "ECONOMICAL" }, + { 12, "ECONOMICAL" }, + { 100, "ECONOMICAL" }, }; -#define FEERATE_LEVEL_MAX (FEERATE_SLOW) struct estimatefees_stash { /* This is max(mempoolminfee,minrelaytxfee) */ u64 perkb_floor; u32 cursor; /* 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 * @@ -477,14 +481,21 @@ estimatefees_null_response(struct bitcoin_cli *bcli) { struct json_stream *response = jsonrpc_stream_success(bcli->cmd); - json_add_null(response, "opening"); - 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"); + /* We give a floor, which is the standard minimum */ + json_array_start(response, "feerates"); + json_array_end(response); + json_add_u32(response, "feerate_floor", 1000); + + if (deprecated_apis) { + json_add_null(response, "opening"); + 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); } @@ -658,18 +669,6 @@ static struct command_result *getchaininfo(struct command *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" }, -}; - /* 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, 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, struct estimatefees_stash *stash) { @@ -706,30 +715,45 @@ static struct command_result *estimatefees_next(struct command *cmd, } response = jsonrpc_stream_success(cmd); - json_add_feerate(response, "opening", cmd, stash, - stash->perkb[FEERATE_NORMAL]); - json_add_feerate(response, "mutual_close", cmd, stash, - stash->perkb[FEERATE_SLOW]); - json_add_feerate(response, "unilateral_close", cmd, stash, - stash->perkb[FEERATE_URGENT]); - json_add_feerate(response, "delayed_to_us", cmd, stash, - stash->perkb[FEERATE_NORMAL]); - json_add_feerate(response, "htlc_resolution", cmd, stash, - stash->perkb[FEERATE_URGENT]); - json_add_feerate(response, "penalty", cmd, stash, - stash->perkb[FEERATE_NORMAL]); - /* We divide the slow feerate for the minimum acceptable, lightningd - * will use floor if it's hit, though. */ - json_add_feerate(response, "min_acceptable", cmd, stash, - 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_feerate(response, "max_acceptable", cmd, stash, - stash->perkb[FEERATE_HIGHEST] * 10); + if (deprecated_apis) { + json_add_feerate(response, "opening", cmd, stash, + feerate_for_block(stash, 12)); + json_add_feerate(response, "mutual_close", cmd, stash, + feerate_for_block(stash, 100)); + json_add_feerate(response, "unilateral_close", cmd, stash, + feerate_for_block(stash, 6)); + json_add_feerate(response, "delayed_to_us", cmd, stash, + feerate_for_block(stash, 12)); + json_add_feerate(response, "htlc_resolution", cmd, stash, + feerate_for_block(stash, 6)); + json_add_feerate(response, "penalty", cmd, stash, + feerate_for_block(stash, 12)); + /* We divide the slow feerate for the minimum acceptable, lightningd + * will use floor if it's hit, though. */ + json_add_feerate(response, "min_acceptable", cmd, stash, + 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 + * margin (say 5x the expected fee requirement) + */ + 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); } diff --git a/tests/test_misc.py b/tests/test_misc.py index 5a995f960..b28cd8fcb 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -86,11 +86,11 @@ def test_bitcoin_failure(node_factory, bitcoind): l1.daemon.rpcproxy.mock_rpc('getblockhash', crash_bitcoincli) # 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']) # 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']) # 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.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)]) # Will exit with failure code. assert l1.daemon.wait() == 1 @@ -1990,8 +1990,8 @@ def test_bitcoind_feerate_floor(node_factory, bitcoind): "mutual_close": 20004, "unilateral_close": 44000, "penalty": 30000, - # FIXME: this should increase: - "min_acceptable": 10000, + # This has increased (rounded up) + "min_acceptable": 20004, "max_acceptable": 600000, "estimates": [{"blockcount": 2, "feerate": 60000, @@ -2031,8 +2031,8 @@ def test_bitcoind_feerate_floor(node_factory, bitcoind): "unilateral_close": 44000, # This has increased (rounded up!) "penalty": 30004, - # FIXME: this should increase to 30004! - "min_acceptable": 15000, + # This has increased (rounded up) + "min_acceptable": 30004, "max_acceptable": 600000, "estimates": [{"blockcount": 2, "feerate": 60000, diff --git a/tests/test_plugin.py b/tests/test_plugin.py index c6e4f1999..981abb178 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -1743,10 +1743,8 @@ def test_bcli(node_factory, bitcoind, chainparams): # Failure case of feerate is tested in test_misc.py estimates = l1.rpc.call("estimatefees") - for est in ["opening", "mutual_close", "unilateral_close", "delayed_to_us", - "htlc_resolution", "penalty", "min_acceptable", - "max_acceptable"]: - assert est in estimates + assert 'feerate_floor' in estimates + assert [f['blocks'] for f in estimates['feerates']] == [2, 6, 12, 100] resp = l1.rpc.call("getchaininfo") assert resp["chain"] == chainparams['name']