channeld: don't commit until we've seen recent incoming msg, ping if required.

Now sending a ping makes sense: it should force the other end to send
a reply, unblocking the commitment process.

Note that rather than waiting for a reply, we're actually spinning on
a 100ms loop in this case.  But it's simple and it works.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

CHANGELOG.md

@@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - JSON API: `listnodes` has new field `global_features`.
 - JSON API: `ping` command to send a ping to a connected peer.
 - Protocol: gossipd now deliberately delays spamming with `channel_update`.
+- Protocol: liveness ping when we commit changes but peer is idle: speeds up
+  failures and reduces forced closures.
 - Config: `--conf` option to set config file.
 - JSON API: Added description to invoices and payments (#1740).
 - pylightning: RpcError now has `method` and `payload` fields.

channeld/channel.c

@@ -155,6 +155,9 @@ struct peer {
 
 	/* Make sure timestamps move forward. */
 	u32 last_update_timestamp;
+
+	/* Make sure peer is live. */
+	struct timeabs last_recv;
 };
 
 static u8 *create_channel_announcement(const tal_t *ctx, struct peer *peer);
@@ -928,12 +931,23 @@ static struct commit_sigs *calc_commitsigs(const tal_t *ctx,
 	return commit_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->cs, PEER_FD,
 				   take(make_ping(NULL, 1, 0)));
 	peer->expecting_pong = true;
@@ -982,6 +996,14 @@ 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 (peer->channel->funder == LOCAL
 	    && peer->desired_feerate != channel_feerate(peer->channel, REMOTE)) {
@@ -1587,6 +1609,8 @@ static void peer_in(struct peer *peer, const u8 *msg)
 {
 	enum wire_type type = fromwire_peektype(msg);
 
+	peer->last_recv = time_now();
+
 	/* Catch our own ping replies. */
 	if (type == WIRE_PONG && peer->expecting_pong) {
 		peer->expecting_pong = false;
@@ -2455,6 +2479,8 @@ int main(int argc, char *argv[])
 	peer->next_commit_sigs = NULL;
 	peer->shutdown_sent[LOCAL] = false;
 	peer->last_update_timestamp = 0;
+	/* We actually received it in the previous daemon, but near enough */
+	peer->last_recv = time_now();
 
 	/* We send these to HSM to get real signatures; don't have valgrind
 	 * complain. */

tests/test_misc.py

@@ -778,7 +778,6 @@ def test_reserve_enforcement(node_factory, executor):
     )
 
 
-@pytest.mark.xfail(strict=True)
 @unittest.skipIf(not DEVELOPER, "needs dev_disconnect")
 def test_htlc_send_timeout(node_factory, bitcoind):
     """Test that we don't commit an HTLC to an unreachable node."""