diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index 3fa083ee3..3b5cd5617 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -302,14 +302,14 @@ static void update_feerates(struct bitcoind *bitcoind, u32 feerate = satoshi_per_kw[i]; /* Takes into account override_fee_rate */ - old_feerates[i] = get_feerate(topo, i); + old_feerates[i] = try_get_feerate(topo, i); /* If estimatefee failed, don't do anything. */ if (!feerate) continue; /* Initial smoothed feerate is the polled feerate */ - if (!topo->feerate[i]) { + if (!old_feerates[i]) { old_feerates[i] = feerate; log_debug(topo->log, "Smoothed feerate estimate for %s initialized to polled estimate %u", @@ -361,7 +361,7 @@ static void update_feerates(struct bitcoind *bitcoind, topo->feerate[j] = topo->feerate[i]; } } - if (get_feerate(topo, i) != old_feerates[i]) + if (try_get_feerate(topo, i) != old_feerates[i]) changed = true; } @@ -618,39 +618,8 @@ u32 get_block_height(const struct chain_topology *topo) return topo->tip->height; } -/* We may only have estimate for 2 blocks, for example. Extrapolate. */ -static u32 guess_feerate(const struct chain_topology *topo, enum feerate feerate) +u32 try_get_feerate(const struct chain_topology *topo, enum feerate feerate) { - size_t i = 0; - u32 rate = 0; - - /* We assume each one is half the previous. */ - for (i = 0; i < feerate; i++) { - if (topo->feerate[i]) { - log_info(topo->log, - "No fee estimate for %s: basing on %s rate", - feerate_name(feerate), - feerate_name(i)); - rate = topo->feerate[i]; - } - rate /= 2; - } - - if (rate == 0) { - rate = topo->default_fee_rate >> feerate; - log_info(topo->log, - "No fee estimate for %s: basing on default fee rate", - feerate_name(feerate)); - } - - return rate; -} - -u32 get_feerate(const struct chain_topology *topo, enum feerate feerate) -{ - if (topo->feerate[feerate] == 0) { - return guess_feerate(topo, feerate); - } return topo->feerate[feerate]; } diff --git a/lightningd/chaintopology.h b/lightningd/chaintopology.h index 32e91cace..eb9573b7f 100644 --- a/lightningd/chaintopology.h +++ b/lightningd/chaintopology.h @@ -131,8 +131,8 @@ size_t get_tx_depth(const struct chain_topology *topo, /* Get highest block number. */ u32 get_block_height(const struct chain_topology *topo); -/* Get fee rate in satoshi per kiloweight. */ -u32 get_feerate(const struct chain_topology *topo, enum feerate feerate); +/* Get fee rate in satoshi per kiloweight, or 0 if unavailable! */ +u32 try_get_feerate(const struct chain_topology *topo, enum feerate feerate); /* Broadcast a single tx, and rebroadcast as reqd (copies tx). * If failed is non-NULL, call that and don't rebroadcast. */ diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index c3302dd0f..409a1c8fc 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -20,11 +20,16 @@ static void update_feerates(struct lightningd *ld, struct channel *channel) { - u8 *msg = towire_channel_feerates(NULL, - get_feerate(ld->topology, - FEERATE_IMMEDIATE), - feerate_min(ld), - feerate_max(ld)); + u8 *msg; + u32 feerate = try_get_feerate(ld->topology, FEERATE_IMMEDIATE); + + /* Nothing to do if we don't know feerate. */ + if (!feerate) + return; + + msg = towire_channel_feerates(NULL, feerate, + feerate_min(ld, NULL), + feerate_max(ld, NULL)); subd_send_msg(channel->owner, take(msg)); } @@ -336,8 +341,8 @@ void peer_start_channeld(struct channel *channel, &channel->our_config, &channel->channel_info.their_config, channel->channel_info.feerate_per_kw, - feerate_min(ld), - feerate_max(ld), + feerate_min(ld, NULL), + feerate_max(ld, NULL), &channel->last_sig, cs, &channel->channel_info.remote_fundingkey, diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index b29054cef..f48f0e5f5 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -24,7 +25,9 @@ static bool better_closing_fee(struct lightningd *ld, const struct bitcoin_tx *tx) { u64 weight, fee, last_fee, min_fee; + u32 min_feerate; size_t i; + bool feerate_unknown; /* Calculate actual fee (adds in eliminated outputs) */ fee = channel->funding_satoshi; @@ -41,17 +44,25 @@ static bool better_closing_fee(struct lightningd *ld, /* Weight once we add in sigs. */ weight = measure_tx_weight(tx) + 74 * 2; - min_fee = get_feerate(ld->topology, FEERATE_SLOW) * weight / 1000; + /* If we don't have a feerate estimate, this gives feerate_floor */ + min_feerate = feerate_min(ld, &feerate_unknown); + + min_fee = min_feerate * weight / 1000; if (fee < min_fee) { log_debug(channel->log, "... That's below our min %"PRIu64 " for weight %"PRIu64" at feerate %u", - min_fee, weight, - get_feerate(ld->topology, FEERATE_SLOW)); + min_fee, weight, min_feerate); return false; } - /* Prefer lower fee: in case of a tie, prefer new over old: this - * covers the preference for a mutual close over a unilateral one. */ + /* In case of a tie, prefer new over old: this covers the preference + * for a mutual close over a unilateral one. */ + + /* If we don't know the feerate, prefer higher fee. */ + if (feerate_unknown) + return fee >= last_fee; + + /* Otherwise prefer lower fee. */ return fee <= last_fee; } @@ -131,6 +142,7 @@ void peer_start_closingd(struct channel *channel, const u8 *channel_reestablish) { u8 *initmsg; + u32 feerate; u64 minfee, startfee, feelimit; u64 num_revocations; u64 funding_msatoshi, our_msatoshi, their_msatoshi; @@ -175,9 +187,17 @@ void peer_start_closingd(struct channel *channel, feelimit = commit_tx_base_fee(channel->channel_info.feerate_per_kw[LOCAL], 0); - minfee = commit_tx_base_fee(get_feerate(ld->topology, FEERATE_SLOW), 0); - startfee = commit_tx_base_fee(get_feerate(ld->topology, FEERATE_NORMAL), - 0); + /* Pick some value above slow feerate (or min possible if unknown) */ + minfee = commit_tx_base_fee(feerate_min(ld, NULL), 0); + + /* If we can't determine feerate, start at half unilateral feerate. */ + feerate = try_get_feerate(ld->topology, FEERATE_NORMAL); + if (!feerate) { + feerate = channel->channel_info.feerate_per_kw[LOCAL] / 2; + if (feerate < feerate_floor()) + feerate = feerate_floor(); + } + startfee = commit_tx_base_fee(feerate, 0); if (startfee > feelimit) startfee = feelimit; diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 0c277e759..3f29f4b45 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -393,6 +394,7 @@ enum watch_result onchaind_funding_spent(struct channel *channel, struct lightningd *ld = channel->peer->ld; struct pubkey final_key; int hsmfd; + u32 feerate; channel_fail_permanent(channel, "Funding transaction spent"); @@ -437,6 +439,19 @@ enum watch_result onchaind_funding_spent(struct channel *channel, /* This could be a mutual close, but it doesn't matter. */ 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. */ + u64 fee = channel->funding_satoshi; + for (size_t i = 0; i < tal_count(channel->last_tx->output); i++) + fee -= channel->last_tx->output[i].amount; + + feerate = fee / measure_tx_weight(tx); + if (feerate < feerate_floor()) + feerate = feerate_floor(); + } + msg = towire_onchain_init(channel, &channel->their_shachain.chain, channel->funding_satoshi, @@ -450,7 +465,7 @@ 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, - get_feerate(ld->topology, FEERATE_NORMAL), + feerate, channel->our_config.dust_limit_satoshis, &our_last_txid, p2wpkh_for_keyidx(tmpctx, ld, diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index f643deed7..a507dc4a9 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -737,7 +737,8 @@ void peer_start_openingd(struct peer *peer, cs, &uc->local_basepoints, &uc->local_funding_pubkey, uc->minimum_depth, - feerate_min(peer->ld), feerate_max(peer->ld), + feerate_min(peer->ld, NULL), + feerate_max(peer->ld, NULL), !peer_active_channel(peer), send_msg); subd_send_msg(uc->openingd, take(msg)); @@ -763,7 +764,7 @@ static void json_fund_channel(struct command *cmd, struct pubkey *id; struct peer *peer; struct channel *channel; - u32 feerate_per_kw = get_feerate(cmd->ld->topology, FEERATE_NORMAL); + u32 feerate_per_kw = try_get_feerate(cmd->ld->topology, FEERATE_NORMAL); u8 *msg; fc->cmd = cmd; @@ -778,6 +779,11 @@ static void json_fund_channel(struct command *cmd, if (!json_tok_wtx(&fc->wtx, buffer, sattok, MAX_FUNDING_SATOSHI)) return; + if (!feerate_per_kw) { + command_fail(cmd, LIGHTNINGD, "Cannot estimate fees"); + return; + } + peer = peer_by_id(cmd->ld, id); if (!peer) { command_fail(cmd, LIGHTNINGD, "Unknown peer"); @@ -817,8 +823,7 @@ static void json_fund_channel(struct command *cmd, msg = towire_opening_funder(NULL, fc->wtx.amount, fc->push_msat, - get_feerate(cmd->ld->topology, - FEERATE_NORMAL), + feerate_per_kw, fc->wtx.change, fc->wtx.change_key_index, fc->channel_flags, diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 4fdd8f3a5..327918c15 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -189,16 +189,24 @@ u8 *p2wpkh_for_keyidx(const tal_t *ctx, struct lightningd *ld, u64 keyidx) return scriptpubkey_p2wpkh(ctx, &shutdownkey); } -u32 feerate_min(struct lightningd *ld) +u32 feerate_min(struct lightningd *ld, bool *unknown) { u32 min; + if (unknown) + *unknown = false; + /* We can't allow less than feerate_floor, since that won't relay */ if (ld->config.ignore_fee_limits) min = 1; - else - /* Set this to half of slow rate.*/ - min = get_feerate(ld->topology, FEERATE_SLOW) / 2; + else { + u32 feerate = try_get_feerate(ld->topology, FEERATE_SLOW); + if (!feerate && unknown) + *unknown = true; + + /* Set this to half of slow rate (if unknown, will be floor) */ + min = feerate / 2; + } if (min < feerate_floor()) return feerate_floor(); @@ -211,13 +219,25 @@ u32 feerate_min(struct lightningd *ld) * 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) +u32 feerate_max(struct lightningd *ld, bool *unknown) { + u32 feerate; + + if (unknown) + *unknown = false; + if (ld->config.ignore_fee_limits) return UINT_MAX; - return get_feerate(ld->topology, FEERATE_IMMEDIATE) * - ld->config.max_fee_multiplier; + /* If we don't know feerate, don't limit other side. */ + feerate = try_get_feerate(ld->topology, FEERATE_IMMEDIATE); + if (!feerate) { + if (unknown) + *unknown = true; + return UINT_MAX; + } + + return feerate * ld->config.max_fee_multiplier; } static void sign_last_tx(struct channel *channel) diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index 5fce0b0d5..b1c78527a 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -88,9 +88,10 @@ void activate_peers(struct lightningd *ld); void drop_to_chain(struct lightningd *ld, struct channel *channel, bool cooperative); -/* Get range of feerates to insist other side abide by for normal channels. */ -u32 feerate_min(struct lightningd *ld); -u32 feerate_max(struct lightningd *ld); +/* Get range of feerates to insist other side abide by for normal channels. + * If we have to guess, sets *unknown to true, otherwise false. */ +u32 feerate_min(struct lightningd *ld, bool *unknown); +u32 feerate_max(struct lightningd *ld, bool *unknown); void channel_watch_funding(struct lightningd *ld, struct channel *channel); #endif /* LIGHTNING_LIGHTNINGD_PEER_CONTROL_H */ diff --git a/tests/test_connection.py b/tests/test_connection.py index 47b40ed47..1107d7c0f 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1105,6 +1105,57 @@ def test_fundee_forget_funding_tx_unconfirmed(node_factory, bitcoind): assert len(l2.rpc.listpeers(l1.info['id'])['peers']) == 0 +@unittest.skipIf(not DEVELOPER, "needs dev_fail") +def test_no_fee_estimate(node_factory, bitcoind, executor): + l1 = node_factory.get_node(start=False) + l1.bitcoind_cmd_override(cmd='estimatesmartfee', + failscript="""echo '{ "errors": [ "Insufficient data or no feerate found" ], "blocks": 0 }'; exit 0""") + l1.start() + + l2 = node_factory.get_node() + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + + # Can't fund a channel. + l1.fundwallet(10**7) + with pytest.raises(RpcError, match=r'Cannot estimate fees'): + l1.rpc.fundchannel(l2.info['id'], 10**6) + + # Can't withdraw either. + with pytest.raises(RpcError, match=r'Cannot estimate fees'): + l1.rpc.withdraw(l2.rpc.newaddr()['address'], 'all') + + # But can accept incoming connections. + l2.fund_channel(l1, 10**6) + + # Can do HTLCs. + l2.pay(l1, 10**5) + + # Can do mutual close. + l1.rpc.close(l2.info['id']) + bitcoind.generate_block(100) + + # Can do unilateral close. + l2.rpc.connect(l1.info['id'], 'localhost', l1.port) + l2.fund_channel(l1, 10**6) + l2.pay(l1, 10**9 // 2) + l1.rpc.dev_fail(l2.info['id']) + l1.daemon.wait_for_log('sendrawtx exit 0') + bitcoind.generate_block(5) + l1.daemon.wait_for_log('sendrawtx exit 0') + bitcoind.generate_block(100) + + # Restart estimatesmartfee, wait for it to pass 5000 + l1.set_feerates((15000, 7500, 3750), True) + l1.daemon.wait_for_log('Feerate estimate for Normal set to [567][0-9]{3}') + + # Can now fund a channel. + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + l1.rpc.fundchannel(l2.info['id'], 10**6) + + # Can withdraw. + l1.rpc.withdraw(l2.rpc.newaddr()['address'], 'all') + + @unittest.skipIf(not DEVELOPER, "needs --dev-disconnect") def test_funder_feerate_reconnect(node_factory, bitcoind): # l1 updates fees, then reconnect so l2 retransmits commitment_signed. diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 9c8cbf5c7..5752a33f2 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -75,9 +75,6 @@ bool fromwire_connect_peer_connected(const tal_t *ctx UNNEEDED, const void *p UN /* Generated stub for fromwire_hsm_sign_commitment_tx_reply */ bool fromwire_hsm_sign_commitment_tx_reply(const void *p UNNEEDED, secp256k1_ecdsa_signature *sig UNNEEDED) { fprintf(stderr, "fromwire_hsm_sign_commitment_tx_reply called!\n"); abort(); } -/* Generated stub for get_feerate */ -u32 get_feerate(const struct chain_topology *topo UNNEEDED, enum feerate feerate UNNEEDED) -{ fprintf(stderr, "get_feerate called!\n"); abort(); } /* Generated stub for invoices_autoclean_set */ void invoices_autoclean_set(struct invoices *invoices UNNEEDED, u64 cycle_seconds UNNEEDED, @@ -357,6 +354,9 @@ u8 *towire_errorfmt(const tal_t *ctx UNNEEDED, /* Generated stub for towire_hsm_sign_commitment_tx */ u8 *towire_hsm_sign_commitment_tx(const tal_t *ctx UNNEEDED, const struct pubkey *peer_id UNNEEDED, u64 channel_dbid UNNEEDED, const struct bitcoin_tx *tx UNNEEDED, const struct pubkey *remote_funding_key UNNEEDED, u64 funding_amount UNNEEDED) { fprintf(stderr, "towire_hsm_sign_commitment_tx called!\n"); abort(); } +/* Generated stub for try_get_feerate */ +u32 try_get_feerate(const struct chain_topology *topo UNNEEDED, enum feerate feerate UNNEEDED) +{ fprintf(stderr, "try_get_feerate called!\n"); abort(); } /* Generated stub for watch_txid */ struct txwatch *watch_txid(const tal_t *ctx UNNEEDED, struct chain_topology *topo UNNEEDED, diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 92da7c10f..8bbf969ce 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -90,7 +90,7 @@ static void json_withdraw(struct command *cmd, const jsmntok_t *desttok, *sattok; struct withdrawal *withdraw = tal(cmd, struct withdrawal); - u32 feerate_per_kw = get_feerate(cmd->ld->topology, FEERATE_NORMAL); + u32 feerate_per_kw = try_get_feerate(cmd->ld->topology, FEERATE_NORMAL); struct bitcoin_tx *tx; enum address_parse_result addr_parse; @@ -107,6 +107,11 @@ static void json_withdraw(struct command *cmd, if (!json_tok_wtx(&withdraw->wtx, buffer, sattok, -1ULL)) return; + if (!feerate_per_kw) { + command_fail(cmd, LIGHTNINGD, "Cannot estimate fees"); + return; + } + /* Parse address. */ addr_parse = json_tok_address_scriptpubkey(cmd, get_chainparams(cmd->ld),