diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index c24f874328cc..c9d4a30a76b1 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -887,7 +887,7 @@ def force_feerates(self, rate): self.set_feerates([rate] * 3, False) self.restart() self.daemon.wait_for_log('peer_out WIRE_UPDATE_FEE') - assert(self.rpc.feerates('perkw')['perkw']['normal'] == rate) + assert(self.rpc.feerates('perkw')['perkw']['opening'] == rate) def wait_for_onchaind_broadcast(self, name, resolve=None): """Wait for onchaind to drop tx name to resolve (if any)""" diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index 2d1ef51f0a84..cc631c3b3fe8 100644 --- a/lightningd/bitcoind.c +++ b/lightningd/bitcoind.c @@ -38,7 +38,7 @@ /* The names of the requests we can make to our Bitcoin backend. */ static const char *methods[] = {"getchaininfo", "getrawblockbyheight", "sendrawtransaction", "getutxout", - "getfeerate"}; + "estimatefees"}; static void plugin_config_cb(const char *buffer, const jsmntok_t *toks, @@ -113,9 +113,17 @@ void bitcoind_check_commands(struct bitcoind *bitcoind) /* Our Bitcoin backend plugin gave us a bad response. We can't recover. */ static void bitcoin_plugin_error(struct bitcoind *bitcoind, const char *buf, const jsmntok_t *toks, const char *method, - const char *reason) + const char *fmt, ...) { - struct plugin *p = strmap_get(&bitcoind->pluginsmap, method); + va_list ap; + char *reason; + struct plugin *p; + + va_start(ap, fmt); + reason = tal_vfmt(NULL, fmt, ap); + va_end(ap); + + p = strmap_get(&bitcoind->pluginsmap, method); fatal("%s error: bad response to %s (%s), response was %.*s", p->cmd, method, reason, toks->end - toks->start, buf + toks->start); @@ -133,118 +141,110 @@ static void bitcoin_plugin_send(struct bitcoind *bitcoind, plugin_request_send(plugin, req); } -/* `getfeerate` +/* `estimatefees` * * Gather feerate from our Bitcoin backend. Will set the feerate to `null` * if estimation failed. * + * - `opening` is used for funding and also misc transactions + * - `mutual_close` is used for the mutual close transaction + * - `unilateral_close` is used for unilateral close (commitment transactions) + * - `delayed_to_us` is used for resolving our output from our unilateral close + * - `htlc_resolution` is used for resolving onchain HTLCs + * - `penalty` is used for resolving revoked transactions + * - `min` is the minimum acceptable feerate + * - `max` is the maximum acceptable feerate + * * Plugin response: * { - * "feerate": , + * "opening": , + * "mutual_close": , + * "unilateral_close": , + * "delayed_to_us": , + * "htlc_resolution": , + * "penalty": , + * "min_acceptable": , + * "max_acceptable": , * } */ struct estimatefee_call { struct bitcoind *bitcoind; - size_t i; - const u32 *blocks; - const char **estmode; - void (*cb)(struct bitcoind *bitcoind, const u32 satoshi_per_kw[], void *); void *arg; - u32 *satoshi_per_kw; }; -static void do_one_estimatefee(struct bitcoind *bitcoind, - struct estimatefee_call *call); - -static void getfeerate_callback(const char *buf, const jsmntok_t *toks, - const jsmntok_t *idtok, - struct estimatefee_call *call) +static void estimatefees_callback(const char *buf, const jsmntok_t *toks, + const jsmntok_t *idtok, + struct estimatefee_call *call) { const jsmntok_t *resulttok, *feeratetok; - u64 feerate; + u32 *feerates = tal_arr(call, u32, NUM_FEERATES); resulttok = json_get_member(buf, toks, "result"); if (!resulttok) bitcoin_plugin_error(call->bitcoind, buf, toks, - "getfeerate", + "estimatefees", "bad 'result' field"); - feeratetok = json_get_member(buf, resulttok, "feerate"); - if (!feeratetok) - bitcoin_plugin_error(call->bitcoind, buf, toks, - "getfeerate", - "bad 'feerate' field"); + for (enum feerate f = 0; f < NUM_FEERATES; f++) { + feeratetok = json_get_member(buf, resulttok, feerate_name(f)); + if (!feeratetok) + bitcoin_plugin_error(call->bitcoind, buf, toks, + "estimatefees", + "missing '%s' field", feerate_name(f)); - /* FIXME: We could trawl recent blocks for median fee... */ - if (!json_to_u64(buf, feeratetok, &feerate)) { - log_unusual(call->bitcoind->log, "Unable to estimate %s/%u fee", - call->estmode[call->i], call->blocks[call->i]); + /* FIXME: We could trawl recent blocks for median fee... */ + if (!json_to_u32(buf, feeratetok, &feerates[f])) { + log_unusual(call->bitcoind->log, + "Unable to estimate %s fees", + feerate_name(f)); #if DEVELOPER - /* This is needed to test for failed feerate estimates - * in DEVELOPER mode */ - call->satoshi_per_kw[call->i] = 0; + /* This is needed to test for failed feerate estimates + * in DEVELOPER mode */ + feerates[f] = 0; #else - /* If we are in testnet mode we want to allow payments - * with the minimal fee even if the estimate didn't - * work out. This is less disruptive than erring out - * all the time. */ - if (chainparams->testnet) - call->satoshi_per_kw[call->i] = FEERATE_FLOOR; - else - call->satoshi_per_kw[call->i] = 0; + /* If we are in testnet mode we want to allow payments + * with the minimal fee even if the estimate didn't + * work out. This is less disruptive than erring out + * all the time. */ + if (chainparams->testnet) + feerates[f] = FEERATE_FLOOR; + else + feerates[f] = 0; #endif - } else - /* Rate in satoshi per kw. */ - call->satoshi_per_kw[call->i] - = feerate_from_style(feerate, FEERATE_PER_KBYTE); - - call->i++; - if (call->i == tal_count(call->satoshi_per_kw)) { - call->cb(call->bitcoind, call->satoshi_per_kw, call->arg); - tal_free(call); - } else { - /* Next */ - do_one_estimatefee(call->bitcoind, call); + } else + /* Rate in satoshi per kw. */ + feerates[f] = feerate_from_style(feerates[f], + FEERATE_PER_KBYTE); } -} -static void do_one_estimatefee(struct bitcoind *bitcoind, - struct estimatefee_call *call) -{ - struct jsonrpc_request *req; - - req = jsonrpc_request_start(bitcoind, "getfeerate", - bitcoind->log, getfeerate_callback, - call); - json_add_num(req->stream, "blocks", call->blocks[call->i]); - json_add_string(req->stream, "mode", call->estmode[call->i]); - jsonrpc_request_end(req); - bitcoin_plugin_send(bitcoind, req); + call->cb(call->bitcoind, feerates, call->arg); + tal_free(call); } void bitcoind_estimate_fees_(struct bitcoind *bitcoind, - const u32 blocks[], const char *estmode[], size_t num_estimates, void (*cb)(struct bitcoind *bitcoind, const u32 satoshi_per_kw[], void *), void *arg) { - struct estimatefee_call *efee = tal(bitcoind, struct estimatefee_call); - - efee->bitcoind = bitcoind; - efee->i = 0; - efee->blocks = tal_dup_arr(efee, u32, blocks, num_estimates, 0); - efee->estmode = tal_dup_arr(efee, const char *, estmode, num_estimates, - 0); - efee->cb = cb; - efee->arg = arg; - efee->satoshi_per_kw = tal_arr(efee, u32, num_estimates); - - do_one_estimatefee(bitcoind, efee); + struct jsonrpc_request *req; + struct estimatefee_call *call = tal(bitcoind, struct estimatefee_call); + + call->bitcoind = bitcoind; + call->cb = cb; + call->arg = arg; + + /* No parameter needed, we always want an urgent, normal and slow + * feerate. This gives computation flexibility to the plugin. */ + req = jsonrpc_request_start(bitcoind, "estimatefees", bitcoind->log, + estimatefees_callback, call); + jsonrpc_request_end(req); + plugin_request_send(strmap_get(&bitcoind->pluginsmap, + "estimatefees"), req); } /* `sendrawtransaction` diff --git a/lightningd/bitcoind.h b/lightningd/bitcoind.h index 0afbbe110411..1bc105120b5e 100644 --- a/lightningd/bitcoind.h +++ b/lightningd/bitcoind.h @@ -61,14 +61,13 @@ struct bitcoind *new_bitcoind(const tal_t *ctx, struct log *log); void bitcoind_estimate_fees_(struct bitcoind *bitcoind, - const u32 blocks[], const char *estmode[], size_t num_estimates, void (*cb)(struct bitcoind *bitcoind, const u32 satoshi_per_kw[], void *), void *arg); -#define bitcoind_estimate_fees(bitcoind_, blocks, estmode, num, cb, arg) \ - bitcoind_estimate_fees_((bitcoind_), (blocks), (estmode), (num), \ +#define bitcoind_estimate_fees(bitcoind_, num, cb, arg) \ + bitcoind_estimate_fees_((bitcoind_), (num), \ typesafe_cb_preargs(void, void *, \ (cb), (arg), \ struct bitcoind *, \ diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index 9c86e9efd388..c0ca7c58ccf0 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -300,9 +301,14 @@ static void watch_for_utxo_reconfirmation(struct chain_topology *topo, const char *feerate_name(enum feerate feerate) { switch (feerate) { - case FEERATE_URGENT: return "urgent"; - case FEERATE_NORMAL: return "normal"; - case FEERATE_SLOW: return "slow"; + case FEERATE_OPENING: return "opening"; + case FEERATE_MUTUAL_CLOSE: return "mutual_close"; + case FEERATE_UNILATERAL_CLOSE: return "unilateral_close"; + case FEERATE_DELAYED_TO_US: return "delayed_to_us"; + case FEERATE_HTLC_RESOLUTION: return "htlc_resolution"; + case FEERATE_PENALTY: return "penalty"; + case FEERATE_MIN: return "min_acceptable"; + case FEERATE_MAX: return "max_acceptable"; } abort(); } @@ -337,13 +343,23 @@ static void add_feerate_history(struct chain_topology *topo, topo->feehistory[feerate][0] = val; } +/* Did the the feerate change since we last estimated it ? */ +static bool feerate_changed(struct chain_topology *topo, u32 old_feerates[]) +{ + for (enum feerate f = 0; f < NUM_FEERATES; f++) { + if (try_get_feerate(topo, f) != old_feerates[f]) + return true; + } + + return false; +} + /* We sanitize feerates if necessary to put them in descending order. */ static void update_feerates(struct bitcoind *bitcoind, const u32 *satoshi_per_kw, struct chain_topology *topo) { u32 old_feerates[NUM_FEERATES]; - bool changed = false; /* Smoothing factor alpha for simple exponential smoothing. The goal is to * have the feerate account for 90 percent of the values polled in the last * 2 minutes. The following will do that in a polling interval @@ -405,26 +421,7 @@ static void update_feerates(struct bitcoind *bitcoind, maybe_completed_init(topo); } - /* Make sure (known) fee rates are in order. */ - for (size_t i = 0; i < NUM_FEERATES; i++) { - if (!topo->feerate[i]) - continue; - for (size_t j = 0; j < i; j++) { - if (!topo->feerate[j]) - continue; - if (topo->feerate[j] < topo->feerate[i]) { - log_unusual(topo->log, - "Feerate estimate for %s (%u) above %s (%u)", - feerate_name(i), topo->feerate[i], - feerate_name(j), topo->feerate[j]); - topo->feerate[j] = topo->feerate[i]; - } - } - if (try_get_feerate(topo, i) != old_feerates[i]) - changed = true; - } - - if (changed) + if (feerate_changed(topo, old_feerates)) notify_feerate_change(bitcoind->ld); next_updatefee_timer(topo); @@ -432,30 +429,39 @@ static void update_feerates(struct bitcoind *bitcoind, static void start_fee_estimate(struct chain_topology *topo) { - /* FEERATE_IMMEDIATE, FEERATE_NORMAL, FEERATE_SLOW */ - const char *estmodes[] = { "CONSERVATIVE", "ECONOMICAL", "ECONOMICAL" }; - const u32 blocks[] = { 2, 4, 100 }; - - BUILD_ASSERT(ARRAY_SIZE(blocks) == NUM_FEERATES); - /* Once per new block head, update fee estimates. */ - bitcoind_estimate_fees(topo->bitcoind, blocks, estmodes, NUM_FEERATES, - update_feerates, topo); + bitcoind_estimate_fees(topo->bitcoind, NUM_FEERATES, update_feerates, + topo); } -u32 mutual_close_feerate(struct chain_topology *topo) +u32 opening_feerate(struct chain_topology *topo) { - return try_get_feerate(topo, FEERATE_NORMAL); + return try_get_feerate(topo, FEERATE_OPENING); } -u32 opening_feerate(struct chain_topology *topo) +u32 mutual_close_feerate(struct chain_topology *topo) { - return try_get_feerate(topo, FEERATE_NORMAL); + return try_get_feerate(topo, FEERATE_MUTUAL_CLOSE); } u32 unilateral_feerate(struct chain_topology *topo) { - return try_get_feerate(topo, FEERATE_URGENT); + return try_get_feerate(topo, FEERATE_UNILATERAL_CLOSE); +} + +u32 delayed_to_us_feerate(struct chain_topology *topo) +{ + return try_get_feerate(topo, FEERATE_DELAYED_TO_US); +} + +u32 htlc_resolution_feerate(struct chain_topology *topo) +{ + return try_get_feerate(topo, FEERATE_HTLC_RESOLUTION); +} + +u32 penalty_feerate(struct chain_topology *topo) +{ + return try_get_feerate(topo, FEERATE_PENALTY); } u32 feerate_from_style(u32 feerate, enum feerate_style style) @@ -510,7 +516,8 @@ static struct command_result *json_feerates(struct command *cmd, response = json_stream_success(cmd); json_object_start(response, json_feerate_style_name(*style)); for (size_t i = 0; i < ARRAY_SIZE(feerates); i++) { - if (!feerates[i]) + if (!feerates[i] || feerates[i] == FEERATE_MIN + || feerates[i] == FEERATE_MAX) continue; json_add_num(response, feerate_name(i), feerate_to_style(feerates[i], *style)); @@ -519,6 +526,23 @@ static struct command_result *json_feerates(struct command *cmd, feerate_to_style(feerate_min(cmd->ld, NULL), *style)); json_add_u64(response, "max_acceptable", feerate_to_style(feerate_max(cmd->ld, NULL), *style)); + + if (deprecated_apis) { + /* urgent feerate was CONSERVATIVE/2, i.e. what bcli gives us + * now for unilateral close feerate */ + json_add_u64(response, "urgent", + feerate_to_style(unilateral_feerate(cmd->ld->topology), *style)); + /* normal feerate was ECONOMICAL/4, i.e. what bcli gives us + * now for opening feerate */ + json_add_u64(response, "normal", + feerate_to_style(opening_feerate(cmd->ld->topology), *style)); + /* slow feerate was ECONOMICAL/100, i.e. what bcli gives us + * now for min feerate, but doubled (the min was slow/2 but now + * the Bitcoin plugin directly gives the real min acceptable). */ + json_add_u64(response, "slow", + feerate_to_style(feerate_min(cmd->ld, NULL) * 2, *style)); + } + json_object_end(response); if (missing) @@ -535,6 +559,20 @@ static struct command_result *json_feerates(struct command *cmd, /* eg. 02000000000101c4fecaae1ea940c15ec502de732c4c386d51f981317605bbe5ad2c59165690ab00000000009db0e280010a2d0f00000000002200208d290003cedb0dd00cd5004c2d565d55fc70227bf5711186f4fa9392f8f32b4a0400483045022100952fcf8c730c91cf66bcb742cd52f046c0db3694dc461e7599be330a22466d790220740738a6f9d9e1ae5c86452fa07b0d8dddc90f8bee4ded24a88fe4b7400089eb01483045022100db3002a93390fc15c193da57d6ce1020e82705e760a3aa935ebe864bd66dd8e8022062ee9c6aa7b88ff4580e2671900a339754116371d8f40eba15b798136a76cd150147522102324266de8403b3ab157a09f1f784d587af61831c998c151bcc21bb74c2b2314b2102e3bd38009866c9da8ec4aa99cc4ea9c6c0dd46df15c61ef0ce1f271291714e5752ae9a3ed620 == weight 598 */ json_add_u64(response, "unilateral_close_satoshis", unilateral_feerate(cmd->ld->topology) * 598 / 1000); + /* BOLT #3: + * + * The *expected weight* of an HTLC transaction is calculated as follows: + * ... + * results in weights of: + * + * 663 (HTLC-timeout) + * 703 (HTLC-success) + * + */ + json_add_u64(response, "htlc_timeout_satoshis", + htlc_resolution_feerate(cmd->ld->topology) * 663 / 1000); + json_add_u64(response, "htlc_success_satoshis", + htlc_resolution_feerate(cmd->ld->topology) * 703 / 1000); json_object_end(response); } @@ -812,20 +850,18 @@ u32 feerate_min(struct lightningd *ld, bool *unknown) if (ld->config.ignore_fee_limits) min = 1; else { - min = try_get_feerate(ld->topology, FEERATE_SLOW); + min = try_get_feerate(ld->topology, FEERATE_MIN); if (!min) { if (unknown) *unknown = true; } else { - const u32 *hist = ld->topology->feehistory[FEERATE_SLOW]; + const u32 *hist = ld->topology->feehistory[FEERATE_MIN]; /* If one of last three was an outlier, use that. */ for (size_t i = 0; i < FEE_HISTORY_NUM; i++) { if (hist[i] < min) min = hist[i]; } - /* Normally, we use half of slow rate. */ - min /= 2; } } @@ -834,16 +870,10 @@ u32 feerate_min(struct lightningd *ld, bool *unknown) return min; } -/* 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) - */ u32 feerate_max(struct lightningd *ld, bool *unknown) { u32 feerate; - const u32 *feehistory = ld->topology->feehistory[FEERATE_URGENT]; + const u32 *feehistory = ld->topology->feehistory[FEERATE_MAX]; if (unknown) *unknown = false; @@ -852,7 +882,7 @@ u32 feerate_max(struct lightningd *ld, bool *unknown) return UINT_MAX; /* If we don't know feerate, don't limit other side. */ - feerate = try_get_feerate(ld->topology, FEERATE_URGENT); + feerate = try_get_feerate(ld->topology, FEERATE_MAX); if (!feerate) { if (unknown) *unknown = true; @@ -864,7 +894,7 @@ u32 feerate_max(struct lightningd *ld, bool *unknown) if (feehistory[i] > feerate) feerate = feehistory[i]; } - return feerate * ld->config.max_fee_multiplier; + return feerate; } /* On shutdown, channels get deleted last. That frees from our list, so diff --git a/lightningd/chaintopology.h b/lightningd/chaintopology.h index 03b25c0b4ae7..456296a09500 100644 --- a/lightningd/chaintopology.h +++ b/lightningd/chaintopology.h @@ -22,11 +22,16 @@ struct txwatch; /* FIXME: move all feerate stuff out to new lightningd/feerate.[ch] files */ enum feerate { - FEERATE_URGENT, /* Aka: aim for next block. */ - FEERATE_NORMAL, /* Aka: next 4 blocks or so. */ - FEERATE_SLOW, /* Aka: next 100 blocks or so. */ + FEERATE_OPENING, + FEERATE_MUTUAL_CLOSE, + FEERATE_UNILATERAL_CLOSE, + FEERATE_DELAYED_TO_US, + FEERATE_HTLC_RESOLUTION, + FEERATE_PENALTY, + FEERATE_MIN, + FEERATE_MAX, }; -#define NUM_FEERATES (FEERATE_SLOW+1) +#define NUM_FEERATES (FEERATE_MAX+1) /* We keep the last three in case there are outliers (for min/max) */ #define FEE_HISTORY_NUM 3 @@ -146,9 +151,13 @@ u32 try_get_feerate(const struct chain_topology *topo, enum feerate feerate); u32 feerate_min(struct lightningd *ld, bool *unknown); u32 feerate_max(struct lightningd *ld, bool *unknown); -u32 mutual_close_feerate(struct chain_topology *topo); u32 opening_feerate(struct chain_topology *topo); +u32 mutual_close_feerate(struct chain_topology *topo); u32 unilateral_feerate(struct chain_topology *topo); +/* For onchain resolution. */ +u32 delayed_to_us_feerate(struct chain_topology *topo); +u32 htlc_resolution_feerate(struct chain_topology *topo); +u32 penalty_feerate(struct chain_topology *topo); /* We always use feerate-per-ksipa, ie. perkw */ u32 feerate_from_style(u32 feerate, enum feerate_style style); diff --git a/lightningd/json.c b/lightningd/json.c index 9d1146fcb792..c1752556e9f6 100644 --- a/lightningd/json.c +++ b/lightningd/json.c @@ -115,6 +115,15 @@ struct command_result *param_feerate(struct command *cmd, const char *name, if (json_tok_streq(buffer, tok, feerate_name(i))) return param_feerate_estimate(cmd, feerate, i); } + /* We used SLOW, NORMAL, and URGENT as feerate targets previously, + * and many commands rely on this syntax now. + * It's also really more natural for an user interface. */ + if (json_tok_streq(buffer, tok, "slow")) + return param_feerate_estimate(cmd, feerate, FEERATE_MIN); + else if (json_tok_streq(buffer, tok, "normal")) + return param_feerate_estimate(cmd, feerate, FEERATE_OPENING); + else if (json_tok_streq(buffer, tok, "urgent")) + return param_feerate_estimate(cmd, feerate, FEERATE_UNILATERAL_CLOSE); /* We have to split the number and suffix. */ suffix.start = suffix.end; diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index d42c8441f457..a2dfc5540010 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -61,10 +61,6 @@ struct config { /* ipv6 bind disable */ bool no_ipv6_bind; - /* Accept fee changes only if they are in the range our_fee - - * our_fee*multiplier */ - u32 max_fee_multiplier; - /* Are we allowed to use DNS lookup for peers. */ bool use_dns; diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 499b7ec13576..d6f0d03364f9 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -457,7 +457,7 @@ enum watch_result onchaind_funding_spent(struct channel *channel, struct lightningd *ld = channel->peer->ld; struct pubkey final_key; int hsmfd; - u32 feerate; + u32 feerates[3]; channel_fail_permanent(channel, "Funding transaction spent"); @@ -502,33 +502,41 @@ enum watch_result onchaind_funding_spent(struct channel *channel, bitcoin_txid(tx, &txid); bitcoin_txid(channel->last_tx, &our_last_txid); - /* We try to use normal feerate for onchaind spends. */ - feerate = try_get_feerate(ld->topology, FEERATE_NORMAL); - if (!feerate) { - /* We have at least one data point: the last tx's feerate. */ - struct amount_sat fee = channel->funding; - for (size_t i = 0; i < channel->last_tx->wtx->num_outputs; i++) { - struct amount_asset asset = - bitcoin_tx_output_get_amount(channel->last_tx, i); - struct amount_sat amt; - assert(amount_asset_is_main(&asset)); - amt = amount_asset_to_sat(&asset); - if (!amount_sat_sub(&fee, fee, amt)) { - log_broken(channel->log, "Could not get fee" - " funding %s tx %s", - type_to_string(tmpctx, - struct amount_sat, - &channel->funding), - type_to_string(tmpctx, - struct bitcoin_tx, - channel->last_tx)); - return KEEP_WATCHING; + /* We try to get the feerate for each transaction type, 0 if estimation + * failed. */ + feerates[0] = delayed_to_us_feerate(ld->topology); + feerates[1] = htlc_resolution_feerate(ld->topology); + feerates[2] = penalty_feerate(ld->topology); + /* We check them separately but there is a high chance that if estimation + * failed for one, it failed for all.. */ + for (size_t i = 0; i < 3; i++) { + if (!feerates[i]) { + /* We have at least one data point: the last tx's feerate. */ + struct amount_sat fee = channel->funding; + for (size_t i = 0; + i < channel->last_tx->wtx->num_outputs; i++) { + struct amount_asset asset = + bitcoin_tx_output_get_amount(channel->last_tx, i); + struct amount_sat amt; + assert(amount_asset_is_main(&asset)); + amt = amount_asset_to_sat(&asset); + if (!amount_sat_sub(&fee, fee, amt)) { + log_broken(channel->log, "Could not get fee" + " funding %s tx %s", + type_to_string(tmpctx, + struct amount_sat, + &channel->funding), + type_to_string(tmpctx, + struct bitcoin_tx, + channel->last_tx)); + return KEEP_WATCHING; + } } - } - feerate = fee.satoshis / bitcoin_tx_weight(tx); /* Raw: reverse feerate extraction */ - if (feerate < feerate_floor()) - feerate = feerate_floor(); + feerates[i] = fee.satoshis / bitcoin_tx_weight(tx); /* Raw: reverse feerate extraction */ + if (feerates[i] < feerate_floor()) + feerates[i] = feerate_floor(); + } } msg = towire_onchain_init(channel, @@ -545,7 +553,8 @@ enum watch_result onchaind_funding_spent(struct channel *channel, * we specify theirs. */ channel->channel_info.their_config.to_self_delay, channel->our_config.to_self_delay, - feerate, + /* delayed_to_us, htlc, and penalty. */ + feerates[0], feerates[1], feerates[2], channel->our_config.dust_limit, &our_last_txid, channel->shutdown_scriptpubkey[LOCAL], diff --git a/lightningd/options.c b/lightningd/options.c index b9631b3b175f..544749bae891 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -518,13 +518,6 @@ static void dev_register_opts(struct lightningd *ld) opt_register_arg("--dev-bitcoind-poll", opt_set_u32, opt_show_u32, &ld->topology->poll_seconds, "Time between polling for new transactions"); - opt_register_arg("--dev-max-fee-multiplier", opt_set_u32, opt_show_u32, - &ld->config.max_fee_multiplier, - "Allow the fee proposed by the remote end to be up to " - "multiplier times higher than our own. Small values " - "will cause channels to be closed more often due to " - "fee fluctuations, large values may result in large " - "fees."); opt_register_noarg("--dev-fast-gossip", opt_set_bool, &ld->dev_fast_gossip, @@ -598,9 +591,6 @@ static const struct config testnet_config = { /* Rescan 5 hours of blocks on testnet, it's reorg happy */ .rescan = 30, - /* Fees may be in the range our_fee - 10*our_fee */ - .max_fee_multiplier = 10, - .use_dns = true, /* Sets min_effective_htlc_capacity - at 1000$/BTC this is 10ct */ @@ -659,9 +649,6 @@ static const struct config mainnet_config = { /* Rescan 2.5 hours of blocks on startup, it's not so reorg happy */ .rescan = 15, - /* Fees may be in the range our_fee - 10*our_fee */ - .max_fee_multiplier = 10, - .use_dns = true, /* Sets min_effective_htlc_capacity - at 1000$/BTC this is 10ct */ diff --git a/onchaind/onchain_wire.csv b/onchaind/onchain_wire.csv index c4d9edf7ca61..ebff07fe6b5e 100644 --- a/onchaind/onchain_wire.csv +++ b/onchaind/onchain_wire.csv @@ -15,7 +15,9 @@ msgdata,onchain_init,old_remote_per_commitment_point,pubkey, msgdata,onchain_init,remote_per_commitment_point,pubkey, msgdata,onchain_init,local_to_self_delay,u32, msgdata,onchain_init,remote_to_self_delay,u32, -msgdata,onchain_init,feerate_per_kw,u32, +msgdata,onchain_init,delayed_to_us_feerate,u32, +msgdata,onchain_init,htlc_feerate,u32, +msgdata,onchain_init,penalty_feerate,u32, msgdata,onchain_init,local_dust_limit_satoshi,amount_sat, # Gives an easy way to tell if it's our unilateral close or theirs... msgdata,onchain_init,our_broadcast_txid,bitcoin_txid, diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c index 9209721e0fc9..c2522ad6737f 100644 --- a/onchaind/onchaind.c +++ b/onchaind/onchaind.c @@ -41,8 +41,14 @@ static const struct pubkey *remote_per_commitment_point; /* The commitment number we're dealing with (if not mutual close) */ static u64 commit_num; -/* The feerate to use when we generate transactions. */ -static u32 feerate_per_kw; +/* The feerate for the transaction spending our delayed output. */ +static u32 delayed_to_us_feerate; + +/* The feerate for transactions spending HTLC outputs. */ +static u32 htlc_feerate; + +/* The feerate for transactions spending from revoked transactions. */ +static u32 penalty_feerate; /* Min and max feerates we ever used */ static u32 min_possible_feerate, max_possible_feerate; @@ -321,7 +327,8 @@ static struct bitcoin_tx *tx_to_us(const tal_t *ctx, u32 locktime, const void *elem, size_t elemsize, const u8 *wscript, - enum tx_type *tx_type) + enum tx_type *tx_type, + u32 feerate) { struct bitcoin_tx *tx; struct amount_sat fee, min_out, amt; @@ -340,7 +347,7 @@ static struct bitcoin_tx *tx_to_us(const tal_t *ctx, /* Worst-case sig is 73 bytes */ weight = bitcoin_tx_weight(tx) + 1 + 3 + 73 + 0 + tal_count(wscript); weight = elements_add_overhead(weight, 1, 1); - fee = amount_tx_fee(feerate_per_kw, weight); + fee = amount_tx_fee(feerate, weight); /* Result is trivial? Spend with small feerate, but don't wait * around for it as it might not confirm. */ @@ -977,10 +984,8 @@ static void resolve_htlc_tx(const struct chainparams *chainparams, * nSequence `to_self_delay` and a witness stack ` * 0` */ - tx = tx_to_us(*outs, delayed_payment_to_us, - out, to_self_delay[LOCAL], 0, NULL, 0, - wscript, - &tx_type); + tx = tx_to_us(*outs, delayed_payment_to_us, out, to_self_delay[LOCAL], + 0, NULL, 0, wscript, &tx_type, htlc_feerate); propose_resolution(out, tx, to_self_delay[LOCAL], tx_type); } @@ -1002,10 +1007,8 @@ static void steal_htlc_tx(struct tracked_output *out) * To spend this via penalty, the remote node uses a witness stack * ` 1` */ - tx = tx_to_us(out, penalty_to_us, out, 0xFFFFFFFF, 0, - &ONE, sizeof(ONE), - out->wscript, - &tx_type); + tx = tx_to_us(out, penalty_to_us, out, 0xFFFFFFFF, 0, &ONE, + sizeof(ONE), out->wscript, &tx_type, penalty_feerate); propose_resolution(out, tx, 0, tx_type); } @@ -1326,11 +1329,10 @@ static void handle_preimage(const struct chainparams *chainparams, * - MUST *resolve* the output by spending it to a * convenient address. */ - tx = tx_to_us(outs[i], remote_htlc_to_us, - outs[i], 0, 0, - preimage, sizeof(*preimage), - outs[i]->wscript, - &tx_type); + tx = tx_to_us(outs[i], remote_htlc_to_us, outs[i], 0, + 0, preimage, sizeof(*preimage), + outs[i]->wscript, &tx_type, + htlc_feerate); propose_resolution(outs[i], tx, 0, tx_type); } } @@ -1606,10 +1608,8 @@ static size_t resolve_our_htlc_theircommit(struct tracked_output *out, * - MUST *resolve* the output, by spending it to a convenient * address. */ - tx = tx_to_us(out, remote_htlc_to_us, - out, 0, cltv_expiry, NULL, 0, - htlc_scripts[matches[0]], - &tx_type); + tx = tx_to_us(out, remote_htlc_to_us, out, 0, cltv_expiry, NULL, 0, + htlc_scripts[matches[0]], &tx_type, htlc_feerate); propose_resolution_at_block(out, tx, cltv_expiry, tx_type); @@ -1868,11 +1868,10 @@ static void handle_our_unilateral(const struct bitcoin_tx *tx, * * 0 */ - to_us = tx_to_us(out, delayed_payment_to_us, - out, to_self_delay[LOCAL], 0, - NULL, 0, - local_wscript, - &tx_type); + to_us = tx_to_us(out, delayed_payment_to_us, out, + to_self_delay[LOCAL], 0, NULL, 0, + local_wscript, &tx_type, + delayed_to_us_feerate); /* BOLT #5: * @@ -1988,12 +1987,8 @@ static void steal_to_them_output(struct tracked_output *out) &keyset->self_revocation_key, &keyset->self_delayed_payment_key); - tx = tx_to_us(tmpctx, - penalty_to_us, - out, 0xFFFFFFFF, 0, - &ONE, sizeof(ONE), - wscript, - &tx_type); + tx = tx_to_us(tmpctx, penalty_to_us, out, 0xFFFFFFFF, 0, &ONE, + sizeof(ONE), wscript, &tx_type, penalty_feerate); propose_resolution(out, tx, 0, tx_type); } @@ -2012,12 +2007,8 @@ static void steal_htlc(struct tracked_output *out) * */ pubkey_to_der(der, &keyset->self_revocation_key); - tx = tx_to_us(out, - penalty_to_us, - out, 0xFFFFFFFF, 0, - der, sizeof(der), - out->wscript, - &tx_type); + tx = tx_to_us(out, penalty_to_us, out, 0xFFFFFFFF, 0, der, sizeof(der), + out->wscript, &tx_type, penalty_feerate); propose_resolution(out, tx, 0, tx_type); } @@ -2687,7 +2678,9 @@ int main(int argc, char *argv[]) &remote_per_commit_point, &to_self_delay[LOCAL], &to_self_delay[REMOTE], - &feerate_per_kw, + &delayed_to_us_feerate, + &htlc_feerate, + &penalty_feerate, &dust_limit, &our_broadcast_txid, &scriptpubkey[LOCAL], @@ -2710,7 +2703,9 @@ int main(int argc, char *argv[]) tx->chainparams = chainparams; - status_debug("feerate_per_kw = %u", feerate_per_kw); + status_debug("delayed_to_us_feerate = %u, htlc_feerate = %u, " + "penalty_feerate = %u", delayed_to_us_feerate, + htlc_feerate, penalty_feerate); bitcoin_txid(tx, &txid); /* We need to keep tx around, but there's only one: not really a leak */ tal_steal(ctx, notleak(tx)); diff --git a/onchaind/test/run-grind_feerate-bug.c b/onchaind/test/run-grind_feerate-bug.c index 129ad2bad88c..00cb83e8a771 100644 --- a/onchaind/test/run-grind_feerate-bug.c +++ b/onchaind/test/run-grind_feerate-bug.c @@ -43,7 +43,7 @@ bool fromwire_onchain_dev_memleak(const void *p UNNEEDED) bool fromwire_onchain_htlc(const void *p UNNEEDED, struct htlc_stub *htlc UNNEEDED, bool *tell_if_missing UNNEEDED, bool *tell_immediately UNNEEDED) { fprintf(stderr, "fromwire_onchain_htlc called!\n"); abort(); } /* Generated stub for fromwire_onchain_init */ -bool fromwire_onchain_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *feerate_per_kw UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *funder UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct bitcoin_tx **tx UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, secp256k1_ecdsa_signature **htlc_signature UNNEEDED, u64 *num_htlcs UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED, bool *option_static_remotekey UNNEEDED) +bool fromwire_onchain_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *delayed_to_us_feerate UNNEEDED, u32 *htlc_feerate UNNEEDED, u32 *penalty_feerate UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *funder UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct bitcoin_tx **tx UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, secp256k1_ecdsa_signature **htlc_signature UNNEEDED, u64 *num_htlcs UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED, bool *option_static_remotekey UNNEEDED) { fprintf(stderr, "fromwire_onchain_init called!\n"); abort(); } /* Generated stub for fromwire_onchain_known_preimage */ bool fromwire_onchain_known_preimage(const void *p UNNEEDED, struct preimage *preimage UNNEEDED) diff --git a/onchaind/test/run-grind_feerate.c b/onchaind/test/run-grind_feerate.c index 332d9bafdf88..c6fa8337248a 100644 --- a/onchaind/test/run-grind_feerate.c +++ b/onchaind/test/run-grind_feerate.c @@ -47,7 +47,7 @@ bool fromwire_onchain_dev_memleak(const void *p UNNEEDED) bool fromwire_onchain_htlc(const void *p UNNEEDED, struct htlc_stub *htlc UNNEEDED, bool *tell_if_missing UNNEEDED, bool *tell_immediately UNNEEDED) { fprintf(stderr, "fromwire_onchain_htlc called!\n"); abort(); } /* Generated stub for fromwire_onchain_init */ -bool fromwire_onchain_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *feerate_per_kw UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *funder UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct bitcoin_tx **tx UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, secp256k1_ecdsa_signature **htlc_signature UNNEEDED, u64 *num_htlcs UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED, bool *option_static_remotekey UNNEEDED) +bool fromwire_onchain_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *delayed_to_us_feerate UNNEEDED, u32 *htlc_feerate UNNEEDED, u32 *penalty_feerate UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *funder UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct bitcoin_tx **tx UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, secp256k1_ecdsa_signature **htlc_signature UNNEEDED, u64 *num_htlcs UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED, bool *option_static_remotekey UNNEEDED) { fprintf(stderr, "fromwire_onchain_init called!\n"); abort(); } /* Generated stub for fromwire_onchain_known_preimage */ bool fromwire_onchain_known_preimage(const void *p UNNEEDED, struct preimage *preimage UNNEEDED) diff --git a/plugins/bcli.c b/plugins/bcli.c index 440b1ef70a18..53520641a7d3 100644 --- a/plugins/bcli.c +++ b/plugins/bcli.c @@ -59,6 +59,10 @@ struct bitcoind { /* Passthrough parameters for bitcoin-cli */ char *rpcuser, *rpcpass, *rpcconnect, *rpcport; + + /* The factor to time the urgent feerate by to get the maximum + * acceptable feerate. */ + u32 max_fee_multiplier; }; static struct bitcoind *bitcoind; @@ -449,17 +453,36 @@ static struct command_result *process_getblockchaininfo(struct bitcoin_cli *bcli return command_finished(bcli->cmd, response); } -static struct command_result *process_estimatefee(struct bitcoin_cli *bcli) + +struct estimatefees_stash { + /* FIXME: We use u64 but lightningd will store them as u32. */ + u64 urgent, normal, slow; +}; + +static struct command_result * +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"); + + return command_finished(bcli->cmd, response); +} + +static struct command_result * +estimatefees_parse_feerate(struct bitcoin_cli *bcli, u64 *feerate) { const jsmntok_t *tokens, *feeratetok = NULL; - struct json_stream *response; bool valid; - u64 feerate; char *err; - if (*bcli->exitstatus != 0) - goto end; - tokens = json_parse_input(bcli->output, bcli->output, (int)bcli->output_bytes, &valid); if (!tokens) { @@ -478,23 +501,103 @@ static struct command_result *process_estimatefee(struct bitcoin_cli *bcli) feeratetok = json_get_member(bcli->output, tokens, "feerate"); if (feeratetok && - !json_to_bitcoin_amount(bcli->output, feeratetok, &feerate)) { + !json_to_bitcoin_amount(bcli->output, feeratetok, feerate)) { err = tal_fmt(bcli->cmd, "%s: bad 'feerate' field (%.*s)", bcli_args(bcli), (int)bcli->output_bytes, bcli->output); return command_done_err(bcli->cmd, BCLI_ERROR, err, NULL); - } + } else if (!feeratetok) + /* We return null if estimation failed, and bitcoin-cli will + * exit with 0 but no feerate field on failure. */ + return estimatefees_null_response(bcli); + + 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; -end: response = jsonrpc_stream_success(bcli->cmd); - if (feeratetok) - json_add_u64(response, "feerate", feerate); - else - json_add_null(response, "feerate"); + json_add_u64(response, "opening", stash->normal); + json_add_u64(response, "mutual_close", stash->normal); + json_add_u64(response, "unilateral_close", stash->urgent); + json_add_u64(response, "delayed_to_us", stash->normal); + json_add_u64(response, "htlc_resolution", stash->normal); + json_add_u64(response, "penalty", stash->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->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->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_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); + + /* 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_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 estimatefees, 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_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; @@ -623,25 +726,28 @@ static struct command_result *getchaininfo(struct command *cmd, return command_still_pending(cmd); } -/* Get current feerate. - * Calls `estimatesmartfee` and returns the feerate as btc/k*VBYTE*. +/* Get the current feerates. We use an urgent feerate for unilateral_close and max, + * a slow feerate for min, and a normal for all others. + * + * Calls `estimatesmartfee` with targets 2/CONSERVATIVE (urgent), + * 4/ECONOMICAL (normal), and 100/ECONOMICAL (slow) then returns the + * feerates as sat/kVB. */ -static struct command_result *getfeerate(struct command *cmd, - const char *buf UNUSED, - const jsmntok_t *toks UNUSED) +static struct command_result *estimatefees(struct command *cmd, + const char *buf UNUSED, + const jsmntok_t *toks UNUSED) { - u32 *blocks; + struct estimatefees_stash *stash = tal(cmd, struct estimatefees_stash); const char **params = tal_arr(cmd, const char *, 2); - if (!param(cmd, buf, toks, - p_req("blocks", param_number, &blocks), - p_req("mode", param_string, ¶ms[1]), - NULL)) + if (!param(cmd, buf, toks, NULL)) return command_param_failed(); - params[0] = tal_fmt(params, "%u", *blocks); - start_bitcoin_cli(NULL, cmd, process_estimatefee, true, - BITCOIND_LOW_PRIO, "estimatesmartfee", params, NULL); + /* First call to estimatesmartfee, for urgent estimation. */ + params[0] = "2"; + params[1] = "CONSERVATIVE"; + start_bitcoin_cli(NULL, cmd, estimatefees_second_step, true, + BITCOIND_LOW_PRIO, "estimatesmartfee", params, stash); return command_still_pending(cmd); } @@ -773,11 +879,12 @@ static const struct plugin_command commands[] = { getchaininfo }, { - "getfeerate", + "estimatefees", "bitcoin", - "Get the Bitcoin feerate in btc/kilo-vbyte.", + "Get the urgent, normal and slow Bitcoin feerates as" + " sat/kVB.", "", - getfeerate + estimatefees }, { "sendrawtransaction", @@ -814,6 +921,7 @@ int main(int argc, char *argv[]) bitcoind->rpcpass = NULL; bitcoind->rpcconnect = NULL; bitcoind->rpcport = NULL; + bitcoind->max_fee_multiplier = 10; plugin_main(argv, init, PLUGIN_STATIC, commands, ARRAY_SIZE(commands), NULL, 0, NULL, 0, @@ -846,5 +954,15 @@ int main(int argc, char *argv[]) "how long to keep retrying to contact bitcoind" " before fatally exiting", u64_option, &bitcoind->retry_timeout), +#if DEVELOPER + plugin_option("dev-max-fee-multiplier", + "string", + "Allow the fee proposed by the remote end to" + " be up to multiplier times higher than our " + "own. Small values will cause channels to be" + " closed more often due to fee fluctuations," + " large values may result in large fees.", + u32_option, &bitcoind->max_fee_multiplier), +#endif /* DEVELOPER */ NULL); } diff --git a/plugins/libplugin.c b/plugins/libplugin.c index f589b2b7e1b1..7653a40cde39 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -797,6 +797,25 @@ char *u64_option(const char *arg, u64 *i) return NULL; } +char *u32_option(const char *arg, u32 *i) +{ + char *endp; + u64 n; + + errno = 0; + n = strtoul(arg, &endp, 0); + if (*endp || !arg[0]) + return tal_fmt(NULL, "'%s' is not a number", arg); + if (errno) + return tal_fmt(NULL, "'%s' is out of range", arg); + + *i = n; + if (*i != n) + return tal_fmt(NULL, "'%s' is too large (overflow)", arg); + + return NULL; +} + char *charp_option(const char *arg, char **p) { *p = tal_strdup(NULL, arg); diff --git a/plugins/libplugin.h b/plugins/libplugin.h index 1d6bdb3b5deb..18da74066bca 100644 --- a/plugins/libplugin.h +++ b/plugins/libplugin.h @@ -234,6 +234,7 @@ void plugin_log(struct plugin *p, enum log_level l, const char *fmt, ...) PRINTF /* Standard helpers */ char *u64_option(const char *arg, u64 *i); +char *u32_option(const char *arg, u32 *i); char *charp_option(const char *arg, char **p); /* The main plugin runner: append with 0 or more plugin_option(), then NULL. */ diff --git a/tests/plugins/bitcoin/part1.py b/tests/plugins/bitcoin/part1.py index 930b786ca975..24cd61fb2d21 100755 --- a/tests/plugins/bitcoin/part1.py +++ b/tests/plugins/bitcoin/part1.py @@ -11,7 +11,7 @@ plugin = Plugin() -@plugin.method("getfeerate") +@plugin.method("estimatefees") def getfeerate(plugin, **kwargs): time.sleep(1) return {} diff --git a/tests/test_closing.py b/tests/test_closing.py index 13ab6b1441e5..129574a691d6 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -1094,7 +1094,7 @@ def test_onchain_all_dust(node_factory, bitcoind, executor): # Make l1's fees really high (and wait for it to exceed 50000) l1.set_feerates((100000, 100000, 100000)) - l1.daemon.wait_for_log('Feerate estimate for normal set to [56789][0-9]{4}') + l1.daemon.wait_for_log('Feerate estimate for unilateral_close set to [56789][0-9]{4}') bitcoind.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') diff --git a/tests/test_misc.py b/tests/test_misc.py index e82935ff0e03..b88408d909cb 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1359,6 +1359,10 @@ def test_feerates(node_factory): }) l1.start() + # All estimation types + types = ["opening", "mutual_close", "unilateral_close", "delayed_to_us", + "htlc_resolution", "penalty"] + # Query feerates (shouldn't give any!) wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) == 2) feerates = l1.rpc.feerates('perkw') @@ -1366,6 +1370,8 @@ def test_feerates(node_factory): assert 'perkb' not in feerates assert feerates['perkw']['max_acceptable'] == 2**32 - 1 assert feerates['perkw']['min_acceptable'] == 253 + for t in types: + assert t not in feerates['perkw'] wait_for(lambda: len(l1.rpc.feerates('perkb')['perkb']) == 2) feerates = l1.rpc.feerates('perkb') @@ -1373,42 +1379,55 @@ def test_feerates(node_factory): assert 'perkw' not in feerates assert feerates['perkb']['max_acceptable'] == (2**32 - 1) assert feerates['perkb']['min_acceptable'] == 253 * 4 + for t in types: + assert t not in feerates['perkb'] # Now try setting them, one at a time. + # Set CONSERVATIVE/2 feerate, for max and unilateral_close l1.set_feerates((15000, 0, 0), True) wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) == 3) feerates = l1.rpc.feerates('perkw') - assert feerates['perkw']['urgent'] == 15000 + assert feerates['perkw']['unilateral_close'] == 15000 assert feerates['warning'] == '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 l1.set_feerates((15000, 6250, 0), True) - wait_for(lambda: len(l1.rpc.feerates('perkb')['perkb']) == 4) + wait_for(lambda: len(l1.rpc.feerates('perkb')['perkb']) == len(types) + 2) feerates = l1.rpc.feerates('perkb') - assert feerates['perkb']['urgent'] == 15000 * 4 - assert feerates['perkb']['normal'] == 25000 + assert feerates['perkb']['unilateral_close'] == 15000 * 4 + for t in types: + if t != "unilateral_close": + assert feerates['perkb'][t] == 25000 assert feerates['warning'] == '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 l1.set_feerates((15000, 6250, 5000), True) - wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) == 5) + wait_for(lambda: len(l1.rpc.feerates('perkw')['perkw']) >= len(types) + 2) feerates = l1.rpc.feerates('perkw') - assert feerates['perkw']['urgent'] == 15000 - assert feerates['perkw']['normal'] == 25000 // 4 - assert feerates['perkw']['slow'] == 5000 + assert feerates['perkw']['unilateral_close'] == 15000 + for t in types: + if t != "unilateral_close": + assert feerates['perkw'][t] == 25000 // 4 assert 'warning' not in feerates assert 'perkb' not in feerates assert feerates['perkw']['max_acceptable'] == 15000 * 10 assert feerates['perkw']['min_acceptable'] == 5000 // 2 - assert len(feerates['onchain_fee_estimates']) == 3 - assert feerates['onchain_fee_estimates']['opening_channel_satoshis'] == feerates['perkw']['normal'] * 702 // 1000 - assert feerates['onchain_fee_estimates']['mutual_close_satoshis'] == feerates['perkw']['normal'] * 673 // 1000 - assert feerates['onchain_fee_estimates']['unilateral_close_satoshis'] == feerates['perkw']['urgent'] * 598 // 1000 + assert len(feerates['onchain_fee_estimates']) == 5 + assert feerates['onchain_fee_estimates']['opening_channel_satoshis'] == feerates['perkw']['opening'] * 702 // 1000 + assert feerates['onchain_fee_estimates']['mutual_close_satoshis'] == feerates['perkw']['mutual_close'] * 673 // 1000 + assert feerates['onchain_fee_estimates']['unilateral_close_satoshis'] == feerates['perkw']['unilateral_close'] * 598 // 1000 + htlc_feerate = feerates["perkw"]["htlc_resolution"] + htlc_timeout_cost = feerates["onchain_fee_estimates"]["htlc_timeout_satoshis"] + htlc_success_cost = feerates["onchain_fee_estimates"]["htlc_success_satoshis"] + assert htlc_timeout_cost == htlc_feerate * 663 // 1000 + assert htlc_success_cost == htlc_feerate * 703 // 1000 def test_logging(node_factory): @@ -1667,7 +1686,7 @@ def mock_fail(*args): wait_for(lambda: l1.daemon.is_in_log( r'getblockhash [a-z0-9]* exited with status 1')) wait_for(lambda: l1.daemon.is_in_log( - r'Unable to estimate CONSERVATIVE/2 fee')) + r'Unable to estimate opening fees')) # Now unset the mock, so calls go through again l1.daemon.rpcproxy.mock_rpc('getblockhash', None) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 2397f8d6e958..c32bacdc6341 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -1071,8 +1071,11 @@ def test_bcli(node_factory, bitcoind, chainparams): l1.rpc.plugin_stop("bcli") # Failure case of feerate is tested in test_misc.py - assert "feerate" in l1.rpc.call("getfeerate", {"blocks": 3, - "mode": "CONSERVATIVE"}) + 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 resp = l1.rpc.call("getchaininfo") assert resp["chain"] == chainparams['name'] diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index d5b75dcfbc75..6ec384873a58 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -297,8 +297,10 @@ static struct command_result *json_prepare_tx(struct command *cmd, } if (!feerate_per_kw) { + /* We mainly use `txprepare` for opening transactions, and FEERATE_OPENING + * is kind of the new FEERATE_NORMAL so it fits well `withdraw` too. */ result = param_feerate_estimate(cmd, &feerate_per_kw, - FEERATE_NORMAL); + FEERATE_OPENING); if (result) return result; }