From 02cb7abd9df2332c26aa9ca2ac3ebb376dd27856 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 18 Aug 2016 14:23:46 +0930 Subject: [PATCH] bitcoind: keep running fee estimate. This avoids us having to query it when we create anchor transaction, and lets us always use dynamic fee information. The config options for max and min are now percentages, rather than absolute. Signed-off-by: Rusty Russell --- daemon/bitcoind.c | 5 ++-- daemon/chaintopology.c | 23 +++++++++++++++++ daemon/chaintopology.h | 3 +++ daemon/lightningd.c | 58 +++++++++++++++++------------------------- daemon/lightningd.h | 15 ++++++----- daemon/packets.c | 8 +++++- daemon/peer.c | 55 ++++++++++++++------------------------- daemon/test/test.sh | 13 +++++----- state.c | 13 ++-------- state.h | 2 +- state_types.h | 3 --- 11 files changed, 97 insertions(+), 101 deletions(-) diff --git a/daemon/bitcoind.c b/daemon/bitcoind.c index a21f63109..7184dc2ed 100644 --- a/daemon/bitcoind.c +++ b/daemon/bitcoind.c @@ -191,9 +191,8 @@ static void process_estimatefee_6(struct bitcoin_cli *bcli) if (fee < 0) { log_unusual(bcli->dstate->base_log, - "Unable to estimate fee, using %"PRIu64, - bcli->dstate->config.closing_fee_rate); - fee_rate = bcli->dstate->config.closing_fee_rate; + "Unable to estimate fee"); + fee_rate = 0; } else { /* Since we used 6 as an estimate, double it. */ fee *= 2; diff --git a/daemon/chaintopology.c b/daemon/chaintopology.c index c189bdc01..7ba5041d6 100644 --- a/daemon/chaintopology.c +++ b/daemon/chaintopology.c @@ -12,6 +12,7 @@ #include #include #include +#include struct block { int height; @@ -62,6 +63,7 @@ struct topology { struct block *root; struct block *tip; struct block_map block_map; + u64 feerate; }; static void start_poll_chaintip(struct lightningd_state *dstate); @@ -304,6 +306,13 @@ static void free_blocks(struct lightningd_state *dstate, struct block *b) } } +static void update_fee(struct lightningd_state *dstate, u64 rate, u64 *feerate) +{ + log_debug(dstate->base_log, "Feerate %"PRIu64" -> %"PRIu64, + rate, *feerate); + *feerate = rate; +} + /* B is the new chain (linked by ->next); update topology */ static void topology_changed(struct lightningd_state *dstate, struct block *prev, @@ -325,6 +334,9 @@ static void topology_changed(struct lightningd_state *dstate, /* Maybe need to rebroadcast. */ rebroadcast_txs(dstate); + + /* Once per new block head, update fee estimate. */ + bitcoind_estimate_fee(dstate, update_fee, &dstate->topology->feerate); } static struct block *new_block(struct lightningd_state *dstate, @@ -460,11 +472,22 @@ u32 get_block_height(struct lightningd_state *dstate) return dstate->topology->tip->height; } +u64 get_feerate(struct lightningd_state *dstate) +{ + if (dstate->topology->feerate == 0) { + log_info(dstate->base_log, + "No fee estimate: using default fee rate"); + return dstate->config.default_fee_rate; + } + return dstate->topology->feerate; +} + void setup_topology(struct lightningd_state *dstate) { dstate->topology = tal(dstate, struct topology); block_map_init(&dstate->topology->block_map); + dstate->topology->feerate = 0; bitcoind_getblockcount(dstate, get_init_blockhash, NULL); /* Once it gets first block, it calls io_break() and we return. */ diff --git a/daemon/chaintopology.h b/daemon/chaintopology.h index ea6ff13f1..815c8a110 100644 --- a/daemon/chaintopology.h +++ b/daemon/chaintopology.h @@ -25,6 +25,9 @@ u32 get_tip_mediantime(struct lightningd_state *dstate); /* Get highest block number. */ u32 get_block_height(struct lightningd_state *dstate); +/* Get fee rate. */ +u64 get_feerate(struct lightningd_state *dstate); + /* Broadcast a single tx, and rebroadcast as reqd (takes ownership of tx) */ void broadcast_tx(struct peer *peer, const struct bitcoin_tx *tx); diff --git a/daemon/lightningd.c b/daemon/lightningd.c index 3c43a070a..a05f7d90d 100644 --- a/daemon/lightningd.c +++ b/daemon/lightningd.c @@ -107,15 +107,18 @@ static void config_register_opts(struct lightningd_state *dstate) opt_register_arg("--forever-confirms", opt_set_u32, opt_show_u32, &dstate->config.forever_confirms, "Confirmations after which we consider a reorg impossible"); - opt_register_arg("--commit-fee-rate", opt_set_u64, opt_show_u64, - &dstate->config.commitment_fee_rate, - "Satoshis to offer for commitment transaction fee (per kb)"); - opt_register_arg("--min-commit-fee-rate", opt_set_u64, opt_show_u64, - &dstate->config.commitment_fee_rate_min, - "Minimum satoshis to accept for commitment transaction fee (per kb)"); - opt_register_arg("--closing-fee-rate", opt_set_u64, opt_show_u64, - &dstate->config.closing_fee_rate, - "Satoshis to use for mutual close transaction fee (per kb)"); + opt_register_arg("--commit-fee-min=", opt_set_u32, opt_show_u32, + &dstate->config.commitment_fee_min_percent, + "Minimum percentage of fee to accept for commitment"); + opt_register_arg("--commit-fee-max=", opt_set_u32, opt_show_u32, + &dstate->config.commitment_fee_max_percent, + "Maximum percentage of fee to accept for commitment"); + opt_register_arg("--commit-fee=", opt_set_u32, opt_show_u32, + &dstate->config.commitment_fee_percent, + "Percentage of fee to request for their commitment"); + opt_register_arg("--default-fee-rate", opt_set_u64, opt_show_u64, + &dstate->config.default_fee_rate, + "Satoshis per kb if can't estimate fees"); opt_register_arg("--min-htlc-expiry", opt_set_u32, opt_show_u32, &dstate->config.min_htlc_expiry, "Minimum number of blocks to accept an HTLC before expiry"); @@ -168,16 +171,15 @@ static void default_config(struct config *config) */ config->forever_confirms = 100; - /* FIXME: These should float with bitcoind's recommendations! */ + /* Insist between 2 and 20 times the 2-block fee. */ + config->commitment_fee_min_percent = 200; + config->commitment_fee_max_percent = 2000; - /* Pay hefty fee (double historic high of ~100k). */ - config->commitment_fee_rate = 200000; + /* We offer to pay 5 times 2-block fee */ + config->commitment_fee_percent = 500; - /* Don't accept less than double the average 2-block fee. */ - config->commitment_fee_rate_min = 50000; - - /* Use this for mutual close. */ - config->closing_fee_rate = 20000; + /* Use this rate by default if estimatefee doesn't estimate. */ + config->default_fee_rate = 40000; /* Don't bother me unless I have 6 hours to collect. */ config->min_htlc_expiry = 6 * 6; @@ -201,24 +203,12 @@ static void default_config(struct config *config) static void check_config(struct lightningd_state *dstate) { - /* BOLT #2: - * The sender MUST set `close_fee` lower than or equal to the - * fee of the final commitment transaction - */ - /* We do this by ensuring it's less than the minimum we would accept. */ - if (dstate->config.closing_fee_rate > dstate->config.commitment_fee_rate_min) - fatal("Closing fee rate %"PRIu64 - " can't exceed minimum commitment fee rate %"PRIu64, - dstate->config.closing_fee_rate, - dstate->config.commitment_fee_rate_min); - - if (dstate->config.commitment_fee_rate_min - > dstate->config.commitment_fee_rate) - fatal("Minumum fee rate %"PRIu64 - " can't exceed commitment fee rate %"PRIu64, - dstate->config.commitment_fee_rate_min, - dstate->config.commitment_fee_rate); + if (dstate->config.commitment_fee_max_percent + < dstate->config.commitment_fee_min_percent) + fatal("Commitment fee invalid min-max %u-%u", + dstate->config.commitment_fee_min_percent, + dstate->config.commitment_fee_max_percent); if (dstate->config.forever_confirms < 100) log_unusual(dstate->base_log, diff --git a/daemon/lightningd.h b/daemon/lightningd.h index 78fd393e0..6823be8a5 100644 --- a/daemon/lightningd.h +++ b/daemon/lightningd.h @@ -29,14 +29,17 @@ struct config { /* How many blocks until we stop watching a close commit? */ u32 forever_confirms; - /* What are we prepared to pay in commitment fee (satoshis/kb). */ - u64 commitment_fee_rate; + /* Maximum percent of fee rate we'll accept. */ + u32 commitment_fee_max_percent; - /* How little are we prepared to have them pay? */ - u64 commitment_fee_rate_min; + /* Minimum percent of fee rate we'll accept. */ + u32 commitment_fee_min_percent; - /* What fee we use for the closing transaction (satoshis/kb) */ - u64 closing_fee_rate; + /* Percent of fee rate we'll use. */ + u32 commitment_fee_percent; + + /* What fee we use if estimatefee fails (satoshis/kb) */ + u64 default_fee_rate; /* Minimum/maximum time for an expiring HTLC (blocks). */ u32 min_htlc_expiry, max_htlc_expiry; diff --git a/daemon/packets.c b/daemon/packets.c index 57f4f622e..dc7fb158d 100644 --- a/daemon/packets.c +++ b/daemon/packets.c @@ -1,5 +1,6 @@ #include "bitcoin/script.h" #include "bitcoin/tx.h" +#include "chaintopology.h" #include "close_tx.h" #include "commit_tx.h" #include "controlled_time.h" @@ -275,6 +276,7 @@ Pkt *accept_pkt_open(struct peer *peer, const Pkt *pkt, { struct rel_locktime locktime; const OpenChannel *o = pkt->open; + u64 feerate = get_feerate(peer->dstate); if (!proto_to_rel_locktime(o->delay, &locktime)) return pkt_err(peer, "Invalid delay"); @@ -284,7 +286,11 @@ Pkt *accept_pkt_open(struct peer *peer, const Pkt *pkt, return pkt_err(peer, "Delay too great"); if (o->min_depth > peer->dstate->config.anchor_confirms_max) return pkt_err(peer, "min_depth too great"); - if (o->initial_fee_rate < peer->dstate->config.commitment_fee_rate_min) + if (o->initial_fee_rate + < feerate * peer->dstate->config.commitment_fee_min_percent / 100) + return pkt_err(peer, "Commitment fee rate too low"); + if (o->initial_fee_rate + > feerate * peer->dstate->config.commitment_fee_max_percent / 100) return pkt_err(peer, "Commitment fee rate too low"); if (o->anch == OPEN_CHANNEL__ANCHOR_OFFER__WILL_CREATE_ANCHOR) peer->remote.offer_anchor = CMD_OPEN_WITH_ANCHOR; diff --git a/daemon/peer.c b/daemon/peer.c index 00fac1cec..16680eac6 100644 --- a/daemon/peer.c +++ b/daemon/peer.c @@ -126,9 +126,7 @@ static const struct bitcoin_tx *bitcoin_spend_ours(struct peer *peer) * use 176 from an example run. */ assert(measure_tx_cost(tx) == 83 * 4); - /* FIXME: Dynamic fees! */ - fee = fee_by_feerate(83 + 176 / 4, - peer->dstate->config.closing_fee_rate); + fee = fee_by_feerate(83 + 176 / 4, get_feerate(peer->dstate)); /* FIXME: Fail gracefully in these cases (not worth collecting) */ if (fee > commit->output[p2wsh_out].amount @@ -727,7 +725,6 @@ static bool closing_pkt_in(struct peer *peer, const Pkt *pkt) return false; } - /* FIXME: Dynamic fee! */ return true; } @@ -921,9 +918,8 @@ static void peer_calculate_close_fee(struct peer *peer) const uint64_t txsize = 41 + 221 + 10 + 32 + 32; uint64_t maxfee; - /* FIXME: Dynamic fee */ peer->closing.our_fee - = fee_by_feerate(txsize, peer->dstate->config.closing_fee_rate); + = fee_by_feerate(txsize, get_feerate(peer->dstate)); /* BOLT #2: * The sender MUST set `close_fee` lower than or equal to the @@ -932,10 +928,7 @@ static void peer_calculate_close_fee(struct peer *peer) */ maxfee = commit_tx_fee(peer->local.commit->tx, peer->anchor.satoshis); if (peer->closing.our_fee > maxfee) { - /* This shouldn't happen: we never accept a commit fee - * less than the min_rate, which is greater than the - * closing_fee_rate. Also, our txsize estimate for - * the closing tx is 2 bytes smaller than the commitment tx. */ + /* This could only happen if the fee rate dramatically */ log_unusual(peer->log, "Closing fee %"PRIu64" exceeded commit fee %"PRIu64", reducing.", peer->closing.our_fee, maxfee); @@ -1198,9 +1191,7 @@ static const struct bitcoin_tx *htlc_fulfill_tx(const struct peer *peer, /* Witness length can vary, due to DER encoding of sigs, but we * use 539 from an example run. */ - /* FIXME: Dynamic fees! */ - fee = fee_by_feerate(83 + 539 / 4, - peer->dstate->config.closing_fee_rate); + fee = fee_by_feerate(83 + 539 / 4, get_feerate(peer->dstate)); /* FIXME: Fail gracefully in these cases (not worth collecting) */ if (fee > satoshis || is_dust(satoshis - fee)) @@ -1637,8 +1628,10 @@ static struct peer *new_peer(struct lightningd_state *dstate, &peer->local.locktime)) fatal("Could not convert locktime_blocks"); peer->local.mindepth = dstate->config.anchor_confirms; - peer->local.commit_fee_rate = dstate->config.commitment_fee_rate; - + /* FIXME: Make this dynamic! */ + peer->local.commit_fee_rate + = get_feerate(peer->dstate) + * peer->dstate->config.commitment_fee_percent / 100; peer->local.commit = peer->remote.commit = NULL; peer->local.staging_cstate = peer->remote.staging_cstate = NULL; @@ -1660,6 +1653,8 @@ static struct peer *new_peer(struct lightningd_state *dstate, log_prefix(dstate->base_log), in_or_out, netaddr_name(peer, &peer->addr)); + log_debug(peer->log, "Using fee rate %"PRIu64, + peer->local.commit_fee_rate); return peer; } @@ -2181,9 +2176,7 @@ static const struct bitcoin_tx *htlc_timeout_tx(const struct peer *peer, /* Witness length can vary, due to DER encoding of sigs, but we * use 539 from an example run. */ - /* FIXME: Dynamic fees! */ - fee = fee_by_feerate(83 + 539 / 4, - peer->dstate->config.closing_fee_rate); + fee = fee_by_feerate(83 + 539 / 4, get_feerate(peer->dstate)); /* FIXME: Fail gracefully in these cases (not worth collecting) */ if (fee > satoshis || is_dust(satoshis - fee)) @@ -2720,8 +2713,7 @@ static void resolve_their_steal(struct peer *peer, } assert(n == steal_tx->input_count); - /* FIXME: Dynamic fees! */ - fee = peer->dstate->config.closing_fee_rate + fee = get_feerate(peer->dstate) * (measure_tx_cost(steal_tx) + wsize) / 1000; if (fee > input_total || is_dust(input_total - fee)) { @@ -2992,19 +2984,22 @@ struct bitcoin_tx *peer_create_close_tx(struct peer *peer, u64 fee) cstate.side[THEIRS].pay_msat / 1000); } -/* Now we can create anchor tx. */ -static void got_feerate(struct lightningd_state *dstate, - u64 rate, struct peer *peer) +/* Creation the bitcoin anchor tx, spending output user provided. */ +void bitcoin_create_anchor(struct peer *peer) { u64 fee; struct bitcoin_tx *tx = bitcoin_tx(peer, 1, 1); size_t i; + /* We must be offering anchor for us to try creating it */ + assert(peer->local.offer_anchor); + tx->output[0].script = scriptpubkey_p2wsh(tx, peer->anchor.witnessscript); tx->output[0].script_length = tal_count(tx->output[0].script); /* Add input script length. FIXME: This is normal case, not exact. */ - fee = fee_by_feerate(measure_tx_cost(tx)/4 + 1+73 + 1+33 + 1, rate); + fee = fee_by_feerate(measure_tx_cost(tx)/4 + 1+73 + 1+33 + 1, + get_feerate(peer->dstate)); if (fee >= peer->anchor.input->amount) /* FIXME: Report an error here! * We really should set this when they do command, but @@ -3031,18 +3026,6 @@ static void got_feerate(struct lightningd_state *dstate, /* To avoid malleation, all inputs must be segwit! */ for (i = 0; i < tx->input_count; i++) assert(tx->input[i].witness); - - state_event(peer, BITCOIN_ANCHOR_CREATED, NULL); -} - -/* Creation the bitcoin anchor tx, spending output user provided. */ -void bitcoin_create_anchor(struct peer *peer, enum state_input done) -{ - /* We must be offering anchor for us to try creating it */ - assert(peer->local.offer_anchor); - - assert(done == BITCOIN_ANCHOR_CREATED); - bitcoind_estimate_fee(peer->dstate, got_feerate, peer); } /* We didn't end up broadcasting the anchor: we don't need to do anything diff --git a/daemon/test/test.sh b/daemon/test/test.sh index d3f18a6fb..a8acb2d20 100755 --- a/daemon/test/test.sh +++ b/daemon/test/test.sh @@ -18,8 +18,8 @@ REDIRERR2="$DIR2/errors" FGREP="fgrep -q" # We inject 0.01 bitcoin, but then fees (estimatefee fails and we use a -# fee rate as per the close tx). -AMOUNT=995940000 +# fee rate as per the default). +AMOUNT=991880000 # Default fee rate per kb. FEE_RATE=200000 @@ -292,10 +292,11 @@ EOF cp $DIR2/config $DIR3/config if [ -n "$DIFFERENT_FEES" ]; then - FEE_RATE2=300000 - CLOSE_FEE_RATE2=30000 - echo "commit-fee-rate=$FEE_RATE2" >> $DIR2/config - echo "closing-fee-rate=$CLOSE_FEE_RATE2" >> $DIR2/config + # Simply override default fee (estimatefee fails on regtest anyway) + CLOSE_FEE_RATE2=50000 + # We use 5x fee rate for commits, by defailt. + FEE_RATE2=$(($CLOSE_FEE_RATE2 * 5)) + echo "default-fee-rate=$CLOSE_FEE_RATE2" >> $DIR2/config fi if [ -n "$GDB1" ]; then diff --git a/state.c b/state.c index 05f6cc8ac..9efeb6ac2 100644 --- a/state.c +++ b/state.c @@ -103,16 +103,8 @@ enum state state(struct peer *peer, peer_open_complete(peer, err->error->problem); goto err_breakdown; } - bitcoin_create_anchor(peer, BITCOIN_ANCHOR_CREATED); - return next_state(peer, input, - STATE_OPEN_WAIT_FOR_ANCHOR_CREATE); - } else if (input_is_pkt(input)) { - peer_open_complete(peer, "unexpected packet"); - goto unexpected_pkt; - } - break; - case STATE_OPEN_WAIT_FOR_ANCHOR_CREATE: - if (input_is(input, BITCOIN_ANCHOR_CREATED)) { + bitcoin_create_anchor(peer); + /* This shouldn't happen! */ if (!setup_first_commit(peer)) { err = pkt_err(peer, @@ -124,7 +116,6 @@ enum state state(struct peer *peer, return next_state(peer, input, STATE_OPEN_WAIT_FOR_COMMIT_SIG); } else if (input_is_pkt(input)) { - bitcoin_release_anchor(peer, BITCOIN_ANCHOR_CREATED); peer_open_complete(peer, "unexpected packet"); goto unexpected_pkt; } diff --git a/state.h b/state.h index a268bb9fe..a540efd94 100644 --- a/state.h +++ b/state.h @@ -107,7 +107,7 @@ void peer_watch_anchor(struct peer *peer, enum state_input timeout); /* Start creation of the bitcoin anchor tx. */ -void bitcoin_create_anchor(struct peer *peer, enum state_input done); +void bitcoin_create_anchor(struct peer *peer); /* Get the bitcoin anchor tx. */ const struct bitcoin_tx *bitcoin_anchor(struct peer *peer); diff --git a/state_types.h b/state_types.h index ab646a7f8..558b9a1ee 100644 --- a/state_types.h +++ b/state_types.h @@ -12,7 +12,6 @@ enum state { */ STATE_OPEN_WAIT_FOR_OPEN_NOANCHOR, STATE_OPEN_WAIT_FOR_OPEN_WITHANCHOR, - STATE_OPEN_WAIT_FOR_ANCHOR_CREATE, STATE_OPEN_WAIT_FOR_ANCHOR, STATE_OPEN_WAIT_FOR_COMMIT_SIG, STATE_OPEN_WAITING_OURANCHOR, @@ -93,8 +92,6 @@ enum state_input { /* * Bitcoin events */ - /* Bitcoin anchor tx created. */ - BITCOIN_ANCHOR_CREATED, /* It reached the required depth. */ BITCOIN_ANCHOR_DEPTHOK, /* It didn't reach the required depth in time. */