From c6af2a8cb242612393354c5ae64661440aa4f949 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 27 Apr 2018 11:46:03 +0930 Subject: [PATCH] lightningd: loosen feerate minimum. We're getting spurious closures, even on mainnet. Using --ignore-fee-limits is dangerous; it's slightly less so to lower the minimum (which is the usual cause of problems). So let's halve it, but beware the floor. This is a workaround, until we get independent feerates in the spec. Fixes: #613 Signed-off-by: Rusty Russell --- lightningd/chaintopology.c | 2 +- lightningd/chaintopology.h | 3 +++ lightningd/opening_control.c | 9 +-------- lightningd/peer_control.c | 14 ++++++++++---- tests/test_lightningd.py | 4 ++-- wallet/test/run-wallet.c | 3 +++ 6 files changed, 20 insertions(+), 15 deletions(-) diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index 41af56038..ea7be3271 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -253,7 +253,7 @@ static void next_updatefee_timer(struct chain_topology *topo); * This formula is satisfied by a feerate of 4030 (hand-search). */ #define FEERATE_FLOOR 253 -static u32 feerate_floor(void) +u32 feerate_floor(void) { /* Assert that bitcoind will see this as above minRelayTxFee */ BUILD_ASSERT(FEERATE_BITCOIND_SEES(FEERATE_FLOOR, MINIMUM_TX_WEIGHT) diff --git a/lightningd/chaintopology.h b/lightningd/chaintopology.h index d0030ff13..4f7d11f63 100644 --- a/lightningd/chaintopology.h +++ b/lightningd/chaintopology.h @@ -136,6 +136,9 @@ 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 the minimum possible fee to allow relaying by bitcoind */ +u32 feerate_floor(void); + /* Broadcast a single tx, and rebroadcast as reqd (copies tx). * If failed is non-NULL, call that and don't rebroadcast. */ void broadcast_tx(struct chain_topology *topo, diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index dec49908e..4f12e7661 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -744,15 +744,8 @@ u8 *peer_accept_channel(const tal_t *ctx, subd_send_msg(uc->openingd, take(msg)); - /* BOLT #2: - * - * Given the variance in fees, and the fact that the transaction may - * be spent in the future, it's a good idea for the fee payer to keep - * a good margin, say 5x the expected fee requirement */ msg = towire_opening_fundee(uc, uc->minimum_depth, - get_feerate(ld->topology, FEERATE_SLOW), - get_feerate(ld->topology, FEERATE_IMMEDIATE) - * 5, + feerate_min(ld), feerate_max(ld), open_msg); subd_req(uc, uc->openingd, take(msg), -1, 2, diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 0c114d5f7..d91f8011f 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -162,12 +162,18 @@ u8 *p2wpkh_for_keyidx(const tal_t *ctx, struct lightningd *ld, u64 keyidx) u32 feerate_min(struct lightningd *ld) { + u32 min; + + /* We can't allow less than feerate_floor, since that won't relay */ if (ld->config.ignore_fee_limits) - return 1; + min = 1; + else + /* Set this to half of slow rate.*/ + min = get_feerate(ld->topology, FEERATE_SLOW) / 2; - /* Set this to average of slow and normal.*/ - return (get_feerate(ld->topology, FEERATE_SLOW) - + get_feerate(ld->topology, FEERATE_NORMAL)) / 2; + if (min < feerate_floor()) + return feerate_floor(); + return min; } /* BOLT #2: diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 30e5df060..f1b810658 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -4135,7 +4135,7 @@ class LightningDTests(BaseLightningDTests): # L1 asks for stupid low fees l1.rpc.dev_setfees(15) - l1.daemon.wait_for_log('Peer permanent failure in CHANNELD_NORMAL: lightning_channeld: received ERROR channel .*: update_fee 15 outside range 4250-75000') + l1.daemon.wait_for_log('Peer permanent failure in CHANNELD_NORMAL: lightning_channeld: received ERROR channel .*: update_fee 15 outside range 500-75000') # Make sure the resolution of this one doesn't interfere with the next! # Note: may succeed, may fail with insufficient fee, depending on how # bitcoind feels! @@ -4277,7 +4277,7 @@ class LightningDTests(BaseLightningDTests): # Make l2 upset by asking for crazy fee. l1.rpc.dev_setfees('150000') # Wait for l1 notice - l1.daemon.wait_for_log(r'Peer permanent failure in CHANNELD_NORMAL: lightning_channeld: received ERROR channel .*: update_fee 150000 outside range 4250-75000') + l1.daemon.wait_for_log(r'Peer permanent failure in CHANNELD_NORMAL: lightning_channeld: received ERROR channel .*: update_fee 150000 outside range 500-75000') # Can't pay while its offline. self.assertRaises(ValueError, l1.rpc.sendpay, to_json(route), rhash) diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 152108346..de3090561 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -71,6 +71,9 @@ bool extract_channel_id(const u8 *in_pkt UNNEEDED, struct channel_id *channel_id /* Generated stub for features_supported */ bool features_supported(const u8 *gfeatures UNNEEDED, const u8 *lfeatures UNNEEDED) { fprintf(stderr, "features_supported called!\n"); abort(); } +/* Generated stub for feerate_floor */ +u32 feerate_floor(void) +{ fprintf(stderr, "feerate_floor called!\n"); abort(); } /* Generated stub for fromwire_gossipctl_peer_disconnect_reply */ bool fromwire_gossipctl_peer_disconnect_reply(const void *p UNNEEDED) { fprintf(stderr, "fromwire_gossipctl_peer_disconnect_reply called!\n"); abort(); }