Skip to content

Commit

Permalink
plugins/bcli: use getmempoolinfo to determine minimum possible fee.
Browse files Browse the repository at this point in the history 
Fixes: #4473
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: wallet: we no longer make txs below minrelaytxfee or mempoolminfee.
  • Loading branch information
@rustyrussell
rustyrussell committed Apr 5, 2023
1 parent 38bc049 commit e6db0ea
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 14 deletions.
86 changes: 72 additions & 14 deletions plugins/bcli.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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)
{
Expand All @@ -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.
Expand All @@ -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)
Expand Down
90 changes: 90 additions & 0 deletions tests/test_misc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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):
Expand Down

0 comments on commit e6db0ea

Please sign in to comment.