From c059b8d5688d518fba9741dac0ab6c719f696989 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 20 Mar 2018 14:00:21 +1030 Subject: [PATCH 1/2] chaintopology: set a fee floor to avoid creating unrelayable txs. Naively, this would be 250 satoshi per sipa, but it's not since bitcoind's fee calculation was not rewritten to deal with weight, but instead bolted on using vbytes. The resulting calculations made me cry; I dried my tears on the thorns of BUILD_ASSERT (I know that makes no sense, but bear with me here as I'm trying not to swear at my bitcoind colleagues right now). Fixes: #1194 Signed-off-by: Rusty Russell --- lightningd/chaintopology.c | 53 +++++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index c41a73fa1b75..c53fbc069594 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -268,6 +268,48 @@ static const char *feerate_name(enum feerate feerate) /* Mutual recursion via timer. */ static void next_updatefee_timer(struct chain_topology *topo); +/* bitcoind considers 250 satoshi per kw to be the minimum acceptable fee: + * less than this won't even relay. + */ +#define BITCOIND_MINRELAYTXFEE_PER_KW 250 +/* + * But bitcoind uses vbytes (ie. (weight + 3) / 4) for this + * calculation, rather than weight, meaning we can disagree since we do + * it sanely (as specified in BOLT #3). + */ +#define FEERATE_BITCOIND_SEES(feerate, weight) \ + (((feerate) * (weight)) / 1000 * 1000 / ((weight) + 3)) +/* ie. fee = (feerate * weight) // 1000 + * bitcoind needs (worst-case): fee * 1000 / (weight + 3) >= 4000 + * + * (feerate * weight) // 1000 * 1000 // (weight + 3) >= 4000 + * + * The feerate needs to be higher for lower weight, and our minimum tx weight + * is 464 (version (4) + count_tx_in (1) + tx_in (32 + 4 + 1 + 4) + + * count_tx_out (1) + amount (8) + P2WSH (1 + 1 + 32) + witness 1 + 1 + + * + 1 + ). Assume it's 400 to give a significant safety margin (it + * only makes 1 difference in the result anyway). + */ +#define MINIMUM_TX_WEIGHT 400 +/* + * This formula is satisfied by a feerate of 4030 (hand-search). + */ +#define FEERATE_FLOOR 253 +static u32 feerate_floor(void) +{ + /* Assert that bitcoind will see this as above minRelayTxFee */ + BUILD_ASSERT(FEERATE_BITCOIND_SEES(FEERATE_FLOOR, MINIMUM_TX_WEIGHT) + >= BITCOIND_MINRELAYTXFEE_PER_KW); + /* And a lesser value won't do */ + BUILD_ASSERT(FEERATE_BITCOIND_SEES(FEERATE_FLOOR-1, MINIMUM_TX_WEIGHT) + < BITCOIND_MINRELAYTXFEE_PER_KW); + /* And I'm right about it being OK for larger txs, too */ + BUILD_ASSERT(FEERATE_BITCOIND_SEES(FEERATE_FLOOR, (MINIMUM_TX_WEIGHT*2)) + >= BITCOIND_MINRELAYTXFEE_PER_KW); + + return FEERATE_FLOOR; +} + /* We sanitize feerates if necessary to put them in descending order. */ static void update_feerates(struct bitcoind *bitcoind, const u32 *satoshi_per_kw, @@ -277,12 +319,17 @@ static void update_feerates(struct bitcoind *bitcoind, bool changed = false; for (size_t i = 0; i < NUM_FEERATES; i++) { - if (satoshi_per_kw[i] != topo->feerate[i]) + u32 feerate = satoshi_per_kw[i]; + + if (feerate < feerate_floor()) + feerate = feerate_floor(); + + if (feerate != topo->feerate[i]) log_debug(topo->log, "%s feerate %u (was %u)", feerate_name(i), - satoshi_per_kw[i], topo->feerate[i]); + feerate, topo->feerate[i]); old_feerates[i] = topo->feerate[i]; - topo->feerate[i] = satoshi_per_kw[i]; + topo->feerate[i] = feerate; } for (size_t i = 0; i < NUM_FEERATES; i++) { From 1d3b05661df45ba214b6beca2ac2702f497c8454 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 20 Mar 2018 14:37:30 +1030 Subject: [PATCH 2/2] chaintopology: add debug message when we're adjusting due to feerate floor. But only if we're actually going to change the feerate, otherwise we'd log every time. Suggested-by: @ZmnSCPxj Signed-off-by: Rusty Russell --- lightningd/chaintopology.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index c53fbc069594..4b8be5b72d57 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -324,10 +324,15 @@ static void update_feerates(struct bitcoind *bitcoind, if (feerate < feerate_floor()) feerate = feerate_floor(); - if (feerate != topo->feerate[i]) + if (feerate != topo->feerate[i]) { log_debug(topo->log, "%s feerate %u (was %u)", feerate_name(i), feerate, topo->feerate[i]); + if (feerate != satoshi_per_kw[i]) + log_debug(topo->log, + "...feerate %u hit floor %u", + satoshi_per_kw[i], feerate); + } old_feerates[i] = topo->feerate[i]; topo->feerate[i] = feerate; }