pytest: add test store announcement_signatures, cleanups and a small bugfix

channeld/channel_wire.csv

@@ -192,4 +192,4 @@ channel_specific_feerates,,feerate_ppm,u32
 # When we receive announcement_signatures for channel announce
 channel_got_announcement,1017
 channel_got_announcement,,remote_ann_node_sig,secp256k1_ecdsa_signature
-channel_got_announcement,,remote_ann_bitcoin_sig,secp256k1_ecdsa_signature
+channel_got_announcement,,remote_ann_bitcoin_sig,secp256k1_ecdsa_signature

lightningd/channel_control.c

@@ -126,7 +126,7 @@ static void peer_got_announcement(struct channel *channel, const u8 *msg)
 					       &remote_ann_node_sig,
 					       &remote_ann_bitcoin_sig)) {
 		channel_internal_error(channel,
-				       "bad channel_got_funding_locked %s",
+				       "bad channel_got_announcement %s",
 				       tal_hex(tmpctx, msg));
 		return;
 	}
@@ -286,7 +286,7 @@ void peer_start_channeld(struct channel *channel,
 	enum side *fulfilled_sides;
 	const struct failed_htlc **failed_htlcs;
 	enum side *failed_sides;
-	struct short_channel_id funding_channel_id;
+	struct short_channel_id scid;
 	u64 num_revocations;
 	struct lightningd *ld = channel->peer->ld;
 	const struct config *cfg = &ld->config;
@@ -326,16 +326,16 @@ void peer_start_channeld(struct channel *channel,
 		   &fulfilled_sides, &failed_htlcs, &failed_sides);
 
 	if (channel->scid) {
-		funding_channel_id = *channel->scid;
-		reached_announce_depth
-			= (short_channel_id_blocknum(&funding_channel_id)
-			   + ANNOUNCE_MIN_DEPTH <= get_block_height(ld->topology));
+		scid = *channel->scid;
+		/* Subtle: depth=1 at funding height. */
+		reached_announce_depth = get_block_height(ld->topology) + 1 >=
+				       short_channel_id_blocknum(&scid) + ANNOUNCE_MIN_DEPTH;
 		log_debug(channel->log, "Already have funding locked in%s",
 			  reached_announce_depth
 			  ? " (and ready to announce)" : "");
 	} else {
 		log_debug(channel->log, "Waiting for funding confirmations");
-		memset(&funding_channel_id, 0, sizeof(funding_channel_id));
+		memset(&scid, 0, sizeof(scid));
 		reached_announce_depth = false;
 	}
 
@@ -411,7 +411,7 @@ void peer_start_channeld(struct channel *channel,
 				      failed_htlcs, failed_sides,
 				      channel->scid != NULL,
 				      channel->remote_funding_locked,
-				      &funding_channel_id,
+				      &scid,
 				      reconnected,
 				      channel->state == CHANNELD_SHUTTING_DOWN,
 				      channel->remote_shutdown_scriptpubkey != NULL,

tests/test_connection.py

@@ -601,6 +601,39 @@ def test_shutdown_reconnect(node_factory):
     assert l1.bitcoin.rpc.getmempoolinfo()['size'] == 1
 
 
+@flaky
+@unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1")
+def test_reconnect_remote_sends_no_sigs(node_factory):
+    """We re-announce, even when remote node doesn't send its announcement_signatures on reconnect.
+    """
+    l1, l2 = node_factory.line_graph(2, wait_for_announce=True, opts={'may_reconnect': True})
+
+    # When l1 restarts (with rescan=1), make it think it hasn't
+    # reached announce_depth, so it wont re-send announcement_signatures
+    def no_blocks_above(req):
+        if req['params'][0] > 107:
+            return {"result": None,
+                    "error": {"code": -8, "message": "Block height out of range"}, "id": req['id']}
+        else:
+            return {'result': l1.bitcoin.rpc.getblockhash(req['params'][0]),
+                    "error": None, 'id': req['id']}
+
+    l1.daemon.rpcproxy.mock_rpc('getblockhash', no_blocks_above)
+    l1.restart()
+
+    # l2 will now uses (REMOTE's) announcement_signatures it has stored
+    wait_for(lambda: only_one(l2.rpc.listpeers()['peers'][0]['channels'])['status'] == [
+        'CHANNELD_NORMAL:Reconnected, and reestablished.',
+        'CHANNELD_NORMAL:Funding transaction locked. Channel announced.'])
+
+    # But l2 still sends its own sigs on reconnect
+    l2.daemon.wait_for_logs([r'peer_out WIRE_ANNOUNCEMENT_SIGNATURES',
+                             r'peer_out WIRE_ANNOUNCEMENT_SIGNATURES'])
+
+    # l1 only did send them the first time
+    assert(''.join(l1.daemon.logs).count(r'peer_out WIRE_ANNOUNCEMENT_SIGNATURES') == 1)
+
+
 def test_shutdown_awaiting_lockin(node_factory, bitcoind):
     l1 = node_factory.get_node()
     l2 = node_factory.get_node(options={'funding-confirms': 3})

tests/test_misc.py

@@ -931,7 +931,6 @@ def test_funding_reorg_private(node_factory, bitcoind):
     # Create a fork that changes short_channel_id from 106x1x0 to 108x1x0
     bitcoind.simple_reorg(106, 2)                   # heights 106-108
     bitcoind.generate_block(1)                      # height 109 (to reach minimum_depth=2 again)
-    l1.daemon.rpcproxy = bitcoind.get_proxy()       # otherwise complains `address already in use`
     l1.start()
 
     # l2 was running, sees last stale block being removed
@@ -955,7 +954,6 @@ def test_funding_reorg_remote_lags(node_factory, bitcoind):
     # may_reconnect so channeld will restart
     opts = {'funding-confirms': 1, 'may_reconnect': True}
     l1, l2 = node_factory.line_graph(2, fundchannel=False, opts=opts)
-    l2.may_fail = True                              # mock_rpc causes dev_memleak
     l1.fundwallet(10000000)
     sync_blockheight(bitcoind, [l1])                # height 102
 
@@ -964,19 +962,16 @@ def test_funding_reorg_remote_lags(node_factory, bitcoind):
     l1.wait_channel_active('103x1x0')
 
     # Make l2 temporary blind for blocks > 107
-    def no_more_blocks():          # although the mock doesn't imitate exitstatus=8, it suffices
-            return {'code': -8, 'message': 'Block height out of range'}
+    def no_more_blocks(req):
+            return {"result": None,
+                    "error": {"code": -8, "message": "Block height out of range"}, "id": req['id']}
 
     l2.daemon.rpcproxy.mock_rpc('getblockhash', no_more_blocks)
 
     # Reorg changes short_channel_id 103x1x0 to 103x2x0, l1 sees it, restarts channeld
     bitcoind.simple_reorg(102, 1)                   # heights 102 - 108
     l1.daemon.wait_for_log(r'Peer transient failure .* short_channel_id changed to 103x2x0 \(was 103x1x0\)')
 
-    # l1 watches at least one more funding confirmation
-    # to depth=7 and sends its announce signature
-    bitcoind.generate_block(1)                      # height 109
-
     wait_for(lambda: only_one(l2.rpc.listpeers()['peers'][0]['channels'])['status'] == [
         'CHANNELD_NORMAL:Reconnected, and reestablished.',
         'CHANNELD_NORMAL:Funding transaction locked. They need our announcement signatures.'])