From 7ca4422d7dcf511c4b8e32bb827ca42b1679481d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 12 Apr 2018 12:33:54 +0930 Subject: [PATCH] closing_control: always prefer lower fee, not closest to ideal. We had an intermittant test failure, where the fee we negotiated was further from our ideal than the final commitment transaction. It worked fine if the other side sent the mutual close first, but not if we sent our unilateral close first. ERROR: test_closing_different_fees (__main__.LightningDTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "tests/test_lightningd.py", line 1319, in test_closing_different_fees wait_for(lambda: p.rpc.listpeers(l1.info['id'])['peers'][0]['channels'][0]['status'][1] == 'ONCHAIN:Tracking mutual close transaction') File "tests/test_lightningd.py", line 74, in wait_for raise ValueError("Error waiting for {}", success) ValueError: ('Error waiting for {}', . at 0x7f4b43e31a60>) Really, if we're prepared to negotiate it, we should be prepared to accept it ourselves. Simply take the cheapest tx which is above our minimum. Signed-off-by: Rusty Russell --- lightningd/closing_control.c | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index 8712d7067..d0f9fa113 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -21,8 +21,7 @@ static bool better_closing_fee(struct lightningd *ld, struct channel *channel, const struct bitcoin_tx *tx) { - u64 weight, fee, last_fee, ideal_fee, min_fee; - s64 old_diff, new_diff; + u64 weight, fee, last_fee, min_fee; size_t i; /* Calculate actual fee (adds in eliminated outputs) */ @@ -49,23 +48,9 @@ static bool better_closing_fee(struct lightningd *ld, return false; } - ideal_fee = get_feerate(ld->topology, FEERATE_NORMAL) * weight / 1000; - - /* We prefer fee which is closest to our ideal. */ - old_diff = imaxabs((s64)ideal_fee - (s64)last_fee); - new_diff = imaxabs((s64)ideal_fee - (s64)fee); - - /* In case of a tie, prefer new over old: this covers the preference - * for a mutual close over a unilateral one. */ - log_debug(channel->log, "... That's %s our ideal %"PRIu64, - new_diff < old_diff - ? "closer to" - : new_diff > old_diff - ? "further from" - : "same distance to", - ideal_fee); - - return new_diff <= old_diff; + /* Prefer lower fee: in case of a tie, prefer new over old: this + * covers the preference for a mutual close over a unilateral one. */ + return fee <= last_fee; } static void peer_received_closing_signature(struct channel *channel,