From e6db0eafc2843b79a1d535fb388e7e30821701de Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 23 Mar 2023 16:18:09 +1030 Subject: [PATCH] plugins/bcli: use getmempoolinfo to determine minimum possible fee. Fixes: #4473 Signed-off-by: Rusty Russell Changelog-Fixed: wallet: we no longer make txs below minrelaytxfee or mempoolminfee. --- plugins/bcli.c | 86 ++++++++++++++++++++++++++++++++++++-------- tests/test_misc.py | 90 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 14 deletions(-) diff --git a/plugins/bcli.c b/plugins/bcli.c index 47bc121f5d30..a79b67b1907a 100644 --- a/plugins/bcli.c +++ b/plugins/bcli.c @@ -472,6 +472,8 @@ enum feerate_levels { #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]; @@ -675,6 +677,24 @@ static const struct estimatefee_params estimatefee_params[] = { [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, + const struct estimatefees_stash *stash, + uint64_t value) +{ + /* 0 is special, it means "unknown" */ + if (value && value < stash->perkb_floor) { + plugin_log(cmd->plugin, LOG_DBG, + "Feerate %s raised from %"PRIu64 + " perkb to floor of %"PRIu64, + fieldname, value, stash->perkb_floor); + json_add_u64(result, fieldname, stash->perkb_floor); + } else { + json_add_u64(result, fieldname, value); + } +} + static struct command_result *estimatefees_next(struct command *cmd, struct estimatefees_stash *stash) { @@ -693,29 +713,64 @@ static struct command_result *estimatefees_next(struct command *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]); + 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] * bitcoind->commit_fee_percent / 100); + 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_u64(response, "min_acceptable", - stash->perkb[FEERATE_SLOW] / 2); + 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_u64(response, "max_acceptable", - stash->perkb[FEERATE_HIGHEST] - * bitcoind->max_fee_multiplier); + json_add_feerate(response, "max_acceptable", cmd, stash, + stash->perkb[FEERATE_HIGHEST] + * bitcoind->max_fee_multiplier); return command_finished(cmd, response); } +static struct command_result *getminfees_done(struct bitcoin_cli *bcli) +{ + const jsmntok_t *tokens; + const char *err; + u64 mempoolfee, relayfee; + struct estimatefees_stash *stash = bcli->stash; + + if (*bcli->exitstatus != 0) + return estimatefees_null_response(bcli); + + tokens = json_parse_simple(bcli->output, + bcli->output, bcli->output_bytes); + if (!tokens) + return command_err_bcli_badjson(bcli, + "cannot parse getmempoolinfo"); + + /* Look at minrelaytxfee they configured, and current min fee to get + * into mempool. */ + err = json_scan(tmpctx, bcli->output, tokens, + "{mempoolminfee:%,minrelaytxfee:%}", + JSON_SCAN(json_to_bitcoin_amount, &mempoolfee), + JSON_SCAN(json_to_bitcoin_amount, &relayfee)); + if (err) + return command_err_bcli_badjson(bcli, err); + + stash->perkb_floor = max_u64(mempoolfee, relayfee); + stash->cursor = 0; + return estimatefees_next(bcli->cmd, stash); +} + /* 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. @@ -729,8 +784,11 @@ static struct command_result *estimatefees(struct command *cmd, if (!param(cmd, buf, toks, NULL)) return command_param_failed(); - stash->cursor = 0; - return estimatefees_next(cmd, stash); + start_bitcoin_cli(NULL, cmd, getminfees_done, true, + BITCOIND_LOW_PRIO, stash, + "getmempoolinfo", + NULL); + return command_still_pending(cmd); } static struct command_result *estimatefees_done(struct bitcoin_cli *bcli) diff --git a/tests/test_misc.py b/tests/test_misc.py index 517102a63043..c14e4815e02b 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1883,8 +1883,10 @@ def test_bitcoind_fail_first(node_factory, bitcoind): def mock_fail(*args): raise ValueError() + # If any of these succeed, they reset fail timeout. l1.daemon.rpcproxy.mock_rpc('getblockhash', mock_fail) l1.daemon.rpcproxy.mock_rpc('estimatesmartfee', mock_fail) + l1.daemon.rpcproxy.mock_rpc('getmempoolinfo', mock_fail) l1.daemon.start(wait_for_initialized=False, stderr_redir=True) l1.daemon.wait_for_logs([r'getblockhash [a-z0-9]* exited with status 1', @@ -1898,6 +1900,94 @@ def mock_fail(*args): l1.daemon.rpcproxy.mock_rpc('estimatesmartfee', None) +@unittest.skipIf(TEST_NETWORK == 'liquid-regtest', "Fees on elements are different") +def test_bitcoind_feerate_floor(node_factory, bitcoind): + """Don't return a feerate less than minrelaytxfee/mempoolnifee.""" + l1 = node_factory.get_node() + + anchors = EXPERIMENTAL_FEATURES + assert l1.rpc.feerates('perkb') == { + "perkb": { + "opening": 30000, + "mutual_close": 15000, + "unilateral_close": 44000, + "delayed_to_us": 30000, + "htlc_resolution": 44000, + "penalty": 30000, + "min_acceptable": 7500, + "max_acceptable": 600000 + }, + "onchain_fee_estimates": { + "opening_channel_satoshis": 5265, + "mutual_close_satoshis": 2523, + "unilateral_close_satoshis": 6578, + "htlc_timeout_satoshis": 7326 if anchors else 7293, + "htlc_success_satoshis": 7766 if anchors else 7733, + } + } + + l1.daemon.rpcproxy.mock_rpc('getmempoolinfo', + { + "mempoolminfee": 0.00010001, + "minrelaytxfee": 0.00020001 + }) + l1.restart() + assert l1.rpc.feerates('perkb') == { + "perkb": { + "opening": 30000, + # This has increased (rounded up) + "mutual_close": 20004, + "unilateral_close": 44000, + "delayed_to_us": 30000, + "htlc_resolution": 44000, + "penalty": 30000, + # This has increased (rounded up!) + "min_acceptable": 20004, + "max_acceptable": 600000 + }, + "onchain_fee_estimates": { + "opening_channel_satoshis": 5265, + # This increases too + "mutual_close_satoshis": 3365, + "unilateral_close_satoshis": 6578, + "htlc_timeout_satoshis": 7326 if anchors else 7293, + "htlc_success_satoshis": 7766 if anchors else 7733, + } + } + + l1.daemon.rpcproxy.mock_rpc('getmempoolinfo', + { + "mempoolminfee": 0.00030001, + "minrelaytxfee": 0.00010001 + }) + l1.restart() + assert l1.rpc.feerates('perkb') == { + "perkb": { + # This has increased (rounded up!) + "opening": 30004, + # This has increased (rounded up!) + "mutual_close": 30004, + "unilateral_close": 44000, + # This has increased (rounded up!) + "delayed_to_us": 30004, + "htlc_resolution": 44000, + # This has increased (rounded up!) + "penalty": 30004, + # This has increased (rounded up!) + "min_acceptable": 30004, + "max_acceptable": 600000 + }, + "onchain_fee_estimates": { + "opening_channel_satoshis": 5265, + # This increases too + "mutual_close_satoshis": 5048, + "unilateral_close_satoshis": 6578, + "htlc_timeout_satoshis": 7326 if anchors else 7293, + "htlc_success_satoshis": 7766 if anchors else 7733, + } + } + + @pytest.mark.developer("needs --dev-force-bip32-seed") @unittest.skipIf(TEST_NETWORK != 'regtest', "Addresses are network specific") def test_dev_force_bip32_seed(node_factory):