From 6338ae8a441ede743abefce73356cbb38eed338f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 22 Aug 2018 12:03:32 +0930 Subject: [PATCH] channeld: update fees if we're restarting. This is a noop if we're opening a new channel (channel_fees_can_change(channel) is false until funding locked in), but important if we're restarting. Signed-off-by: Rusty Russell --- lightningd/channel_control.c | 4 ++++ lightningd/peer_htlcs.c | 36 +++++++++++++++++++++++++----------- lightningd/peer_htlcs.h | 2 ++ tests/test_closing.py | 1 - tests/test_connection.py | 18 ++++++++++-------- 5 files changed, 41 insertions(+), 20 deletions(-) diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index 8cdc7ca85..d27bf7400 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -333,6 +333,10 @@ void peer_start_channeld(struct channel *channel, /* We don't expect a response: we are triggered by funding_depth_cb. */ subd_send_msg(channel->owner, take(initmsg)); + + /* On restart, feerate might not be what we expect: adjust now. */ + if (channel->funder == LOCAL) + try_update_feerates(ld, channel); } bool channel_tell_funding_locked(struct lightningd *ld, diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 8bbf32295..c9d733ea1 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -1671,6 +1671,29 @@ void htlcs_notify_new_block(struct lightningd *ld, u32 height) } while (removed); } +static void update_feerates(struct lightningd *ld, struct channel *channel) +{ + u8 *msg = towire_channel_feerates(NULL, + get_feerate(ld->topology, + FEERATE_IMMEDIATE), + feerate_min(ld), + feerate_max(ld)); + subd_send_msg(channel->owner, take(msg)); +} + +void try_update_feerates(struct lightningd *ld, struct channel *channel) +{ + /* No point until funding locked in */ + if (!channel_fees_can_change(channel)) + return; + + /* Can't if no daemon listening. */ + if (!channel->owner) + return; + + update_feerates(ld, channel); +} + void notify_feerate_change(struct lightningd *ld) { struct peer *peer; @@ -1679,22 +1702,13 @@ void notify_feerate_change(struct lightningd *ld) * it's going to generate more txs. */ list_for_each(&ld->peers, peer, list) { struct channel *channel = peer_active_channel(peer); - u8 *msg; - if (!channel || !channel_fees_can_change(channel)) + if (!channel) continue; /* FIXME: We choose not to drop to chain if we can't contact * peer. We *could* do so, however. */ - if (!channel->owner) - continue; - - msg = towire_channel_feerates(channel, - get_feerate(ld->topology, - FEERATE_IMMEDIATE), - feerate_min(ld), - feerate_max(ld)); - subd_send_msg(channel->owner, take(msg)); + try_update_feerates(ld, channel); } } diff --git a/lightningd/peer_htlcs.h b/lightningd/peer_htlcs.h index 3bc84b01b..6386ffa67 100644 --- a/lightningd/peer_htlcs.h +++ b/lightningd/peer_htlcs.h @@ -59,4 +59,6 @@ void onchain_fulfilled_htlc(struct channel *channel, const struct preimage *preimage); void htlcs_notify_new_block(struct lightningd *ld, u32 height); + +void try_update_feerates(struct lightningd *ld, struct channel *channel); #endif /* LIGHTNING_LIGHTNINGD_PEER_HTLCS_H */ diff --git a/tests/test_closing.py b/tests/test_closing.py index be581f5ff..75bab6f3a 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -917,7 +917,6 @@ def test_onchain_all_dust(node_factory, bitcoind, executor): l1.daemon.wait_for_log('onchaind complete, forgetting peer') -@pytest.mark.xfail(strict=True) @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1 for dev_fail") def test_onchain_different_fees(node_factory, bitcoind, executor): """Onchain handling when we've had a range of fees""" diff --git a/tests/test_connection.py b/tests/test_connection.py index 8fef75224..ef4fffdd8 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1168,11 +1168,13 @@ def test_dataloss_protection(node_factory, bitcoind): # channel_id "[0-9a-f]{64}" # next_local_commitment_number - "0000000000000001" + "000000000000000[1-9]" # next_remote_revocation_number - "0000000000000000" - # your_last_per_commitment_secret (unknown == zeroes) - "0{64}" + "000000000000000[0-9]" + # your_last_per_commitment_secret (funding_depth may + # trigger a fee-update and commit, hence this may not + # be zero) + "[0-9a-f]{64}" # my_current_per_commitment_point "0[23][0-9a-f]{64}") # After an htlc, we should get different results (two more commits) @@ -1190,9 +1192,9 @@ def test_dataloss_protection(node_factory, bitcoind): # channel_id "[0-9a-f]{64}" # next_local_commitment_number - "0000000000000003" + "000000000000000[1-9]" # next_remote_revocation_number - "0000000000000002" + "000000000000000[1-9]" # your_last_per_commitment_secret "[0-9a-f]{64}" # my_current_per_commitment_point @@ -1219,12 +1221,12 @@ def test_dataloss_protection(node_factory, bitcoind): # l2 should still recover something! bitcoind.generate_block(1) - l2.daemon.wait_for_log("ERROR: Unknown commitment #2, recovering our funds!") + l2.daemon.wait_for_log("ERROR: Unknown commitment #[0-9], recovering our funds!") # Restarting l2, and it should remember from db. l2.restart() - l2.daemon.wait_for_log("ERROR: Unknown commitment #2, recovering our funds!") + l2.daemon.wait_for_log("ERROR: Unknown commitment #[0-9], recovering our funds!") bitcoind.generate_block(100) l2.daemon.wait_for_log('WIRE_ONCHAIN_ALL_IRREVOCABLY_RESOLVED')