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 <ccan/asort/asort.h> #include <ccan/io/io.h> #include <ccan/structeq/structeq.h> +#include <inttypes.h> 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=<percent>", 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=<percent>", 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=<percent>", 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. */