From 9138ebf8071367cb0c0824a269782a756515225a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 8 Oct 2021 11:17:49 +1030 Subject: [PATCH] channeld: remove liveness logic pre-commitment. We're going to continuously ping, so this is redundant. Signed-off-by: Rusty Russell --- channeld/channeld.c | 39 ------------------------------------ tests/test_misc.py | 49 --------------------------------------------- 2 files changed, 88 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 846ad2daa..aa90df5ca 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -157,9 +157,6 @@ struct peer { /* Make sure timestamps move forward. */ u32 last_update_timestamp; - /* Make sure peer is live. */ - struct timeabs last_recv; - /* Additional confirmations need for local lockin. */ u32 depth_togo; @@ -1087,27 +1084,6 @@ static struct bitcoin_signature *calc_commitsigs(const tal_t *ctx, return htlc_sigs; } -/* Have we received something from peer recently? */ -static bool peer_recently_active(struct peer *peer) -{ - return time_less(time_between(time_now(), peer->last_recv), - time_from_sec(30)); -} - -static void maybe_send_ping(struct peer *peer) -{ - /* Already have a ping in flight? */ - if (peer->expecting_pong) - return; - - if (peer_recently_active(peer)) - return; - - /* Send a ping to try to elicit a receive. */ - sync_crypto_write_no_delay(peer->pps, take(make_ping(NULL, 1, 0))); - peer->expecting_pong = true; -} - /* Peer protocol doesn't want sighash flags. */ static secp256k1_ecdsa_signature *raw_sigs(const tal_t *ctx, const struct bitcoin_signature *sigs) @@ -1268,14 +1244,6 @@ static void send_commit(struct peer *peer) return; } - /* If we haven't received a packet for > 30 seconds, delay. */ - if (!peer_recently_active(peer)) { - /* Mark this as done and try again. */ - peer->commit_timer = NULL; - start_commit_timer(peer); - return; - } - /* If we wanted to update fees, do it now. */ if (want_fee_update(peer, &feerate_target)) { /* FIXME: We occasionally desynchronize with LND here, so @@ -1385,9 +1353,6 @@ static void send_commit(struct peer *peer) static void start_commit_timer(struct peer *peer) { - /* We should send a ping now if we need a liveness check. */ - maybe_send_ping(peer); - /* Already armed? */ if (peer->commit_timer) return; @@ -2156,8 +2121,6 @@ static void peer_in(struct peer *peer, const u8 *msg) */ bool soft_error = peer->funding_locked[REMOTE] || peer->funding_locked[LOCAL]; - peer->last_recv = time_now(); - /* Catch our own ping replies. */ if (type == WIRE_PONG && peer->expecting_pong) { peer->expecting_pong = false; @@ -3900,8 +3863,6 @@ int main(int argc, char *argv[]) peer->shutdown_sent[LOCAL] = false; peer->shutdown_wrong_funding = NULL; peer->last_update_timestamp = 0; - /* We actually received it in the previous daemon, but near enough */ - peer->last_recv = time_now(); peer->last_empty_commitment = 0; #if EXPERIMENTAL_FEATURES peer->stfu = false; diff --git a/tests/test_misc.py b/tests/test_misc.py index 923be4844..547e270e1 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1388,55 +1388,6 @@ def test_reserve_enforcement(node_factory, executor): assert only_one(l1.rpc.listpeers()['peers'])['connected'] is False -@pytest.mark.developer("needs dev_disconnect") -def test_htlc_send_timeout(node_factory, bitcoind, compat): - """Test that we don't commit an HTLC to an unreachable node.""" - # Feerates identical so we don't get gratuitous commit to update them - l1, l2, l3 = node_factory.line_graph(3, opts=[{'log-level': 'io', - 'feerates': (7500, 7500, 7500, 7500)}, - # Blackhole it after it sends HTLC_ADD to l3. - {'log-level': 'io', - 'feerates': (7500, 7500, 7500, 7500), - 'disconnect': ['0WIRE_UPDATE_ADD_HTLC']}, - {}], - wait_for_announce=True) - - chanid2 = l2.get_channel_scid(l3) - - # Make sure we have 30 seconds without any incoming traffic from l3 to l2 - # so it tries to ping before sending WIRE_COMMITMENT_SIGNED. - timedout = False - while not timedout: - try: - l2.daemon.wait_for_log(r'channeld-chan#[0-9]*: \[IN\] ', timeout=30) - except TimeoutError: - timedout = True - - inv = l3.rpc.invoice(123000, 'test_htlc_send_timeout', 'description') - with pytest.raises(RpcError, match=r'Ran out of routes to try after [0-9]+ attempt[s]?') as excinfo: - l1.rpc.pay(inv['bolt11']) - - err = excinfo.value - # Complains it stopped after several attempts. - # FIXME: include in pylightning - PAY_STOPPED_RETRYING = 210 - assert err.error['code'] == PAY_STOPPED_RETRYING - - status = only_one(l1.rpc.call('paystatus')['pay']) - - # Temporary channel failure - assert status['attempts'][0]['failure']['data']['failcode'] == 0x1007 - assert status['attempts'][0]['failure']['data']['erring_node'] == l2.info['id'] - assert status['attempts'][0]['failure']['data']['erring_channel'] == chanid2 - - # L2 should send ping, but never receive pong so never send commitment. - l2.daemon.wait_for_log(r'{}-.*channeld.*: \[OUT\] 0012'.format(l3.info['id'])) - assert not l2.daemon.is_in_log(r'{}-.*channeld.*: \[IN\] 0013'.format(l3.info['id'])) - assert not l2.daemon.is_in_log(r'{}-.*channeld.*: \[OUT\] 0084'.format(l3.info['id'])) - # L2 killed the channel with l3 because it was too slow. - l2.daemon.wait_for_log('{}-.*channeld-.*Adding HTLC 0 too slow: killing connection'.format(l3.info['id'])) - - def test_ipv4_and_ipv6(node_factory): """Test we can bind to both IPv4 and IPv6 addresses (if supported)""" port = reserve()