possible bug in annoucement_signatures construction?

[joemphilips]

Issue and Steps to Reproduce

While I've been trying to open the public channel from "c-lightning -> my rust-lightning node" , the rl fails to verify the annoucement_signatures sent from c-lightning.
I first thought this is a bug in rl because doing the same for "c-lightning -> lnd" seems ok.

But by taking a closer look, it seems that when c-lightning signs to channel_announcement , it uses a wrong node_id key for the remote node.
I don't know where it came from.

reproducing the bug.

git clone --recursive https://github.com/joemphilips/NRustLightning
git checkout 78dbc74fb597b6a71f5086326abb72cc392cf254
cd tests/NRustLightning.Server.Tests
docker-compose build && dotnet test --filter="FullyQualifiedName~CanOpenCloseChannels"

I can reproduce the failure by running above code, but it is probably not helpful for debugging.
(By looking into the error message rl just tells that rl failed to verify the signature. and force-close the channel.)

Investigation done in my side

What I have done to check the above fact is

1. printf the hex encoded channel_announcement in both sides.
2. deserialize as UnsignedChannelAnnonuncement (which is a msg for actually performing signing) with my own LN library
3. pritty print the UnsignedChannelAnnouncement and compare the equality.

Everything besides nodeid deserializes to the same object, but in the c-lightning side, node_id for the remote node is completely different.

Actual pretty printed messages.

Not sure this helps but I will put pretty-printed UnsignedChannelAnnouncement object here.

The one printed in c-lightning side is

{ Features =
            basic_mpp is non supported. gossip_queries is non supported. gossip_queries_ex is non supported. initial_routing_sync is non supported. option_data_loss_protect is non supported. option_static_remotekey is non supported. option_support_large_channel is non supported. option_upfront_shutdown_script is non supported. payment_secret is non supported. var_onion_optin is non supported. 
  ChainHash = 06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f
  ShortChannelId = 60416x256x0
  NodeId1 =
           NodeId
             02a4f894bdf8005260bcca913f531a4cbcda2a780fdafab43724b20e09dcf64af5
  NodeId2 =
           NodeId
             037d62e9651e9ee4c67b1adf93e9f017caaa3b30dba13629fcca2e96d367acaa95
  BitcoinKey1 =
               ComparablePubKey
                 023ae70127ed8da26e32af63716f7a88bd6f5647264ccc46c77e2d604f95850b8c
  BitcoinKey2 =
               ComparablePubKey
                 027586d1bab7c71bfe125c342400e088acf77b987c77efa063c20562c98c613d35
  ExcessData = [||] }

And the one printed in rl side is

{ Features =
            basic_mpp is non supported. gossip_queries is non supported. gossip_queries_ex is non supported. initial_routing_sync is non supported. option_data_loss_protect is non supported. option_static_remotekey is non supported. option_support_large_channel is non supported. option_upfront_shutdown_script is non supported. payment_secret is non supported. var_onion_optin is non supported. 
  ChainHash = 06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f
  ShortChannelId = 60416x256x0
  NodeId1 =
           NodeId
             025383792642c752a6cfe786b5ef8edce1dc9c6ba93c644139d4cd1a1aff487874
  NodeId2 =
           NodeId
             02a4f894bdf8005260bcca913f531a4cbcda2a780fdafab43724b20e09dcf64af5
  BitcoinKey1 =
               ComparablePubKey
                 027586d1bab7c71bfe125c342400e088acf77b987c77efa063c20562c98c613d35
  BitcoinKey2 =
               ComparablePubKey
                 023ae70127ed8da26e32af63716f7a88bd6f5647264ccc46c77e2d604f95850b8c
  ExcessData = [||] }

Here, nodeid for my rl node is 025383792642c752a6cfe786b5ef8edce1dc9c6ba93c644139d4cd1a1aff487874
and the nodeid for c-lightning is 02a4f894bdf8005260bcca913f531a4cbcda2a780fdafab43724b20e09dcf64af5

You can see that 037d62e9651e9ee4c67b1adf93e9f017caaa3b30dba13629fcca2e96d367acaa95 in c-lightning came out of no where.

NOTE: The ordering of the pubkey is also different, but I think this is a side effect fo the c-lightning is using wrong nodeid. rl uses node_id they exchanged at first for determining the ordering.

getinfo output

node id is different because I restarted the node when the test finished. Not sure this will help either but I will put here anyway.

{
   "id": "02854958c8ba0f2f4cf6f7c8998e656b7c2b1f5fe9d0a1df2551538c7a6d752a29",
   "alias": "YELLOWSEAGULL-7981f4c",
   "color": "028549",
   "num_peers": 0,
   "num_pending_channels": 0,
   "num_active_channels": 0,
   "num_inactive_channels": 0,
   "address": [
      {
         "type": "ipv4",
         "address": "172.25.0.4",
         "port": 9735
      }
   ],
   "binding": [
      {
         "type": "ipv4",
         "address": "172.25.0.4",
         "port": 9735
      }
   ],
   "version": "7981f4c",
   "blockheight": 558,
   "network": "regtest",
   "msatoshi_fees_collected": 0,
   "fees_collected_msat": "0msat",
   "lightning-dir": "/root/.lightning/regtest",
   "warning_lightningd_sync": "Still loading latest blocks from bitcoind."
}

[ZmnSCPxj]

This is the code that generates the channel announcements:

lightning/channeld/channeld.c

Lines 440 to 459 in fc2561f

	if (peer->channel_direction == 0) {
	first = LOCAL;
	second = REMOTE;
	} else {
	first = REMOTE;
	second = LOCAL;
	}
	cannounce = towire_channel_announcement(
	ctx, &peer->announcement_node_sigs[first],
	&peer->announcement_node_sigs[second],
	&peer->announcement_bitcoin_sigs[first],
	&peer->announcement_bitcoin_sigs[second],
	features,
	&chainparams->genesis_blockhash,
	&peer->short_channel_ids[LOCAL],
	&peer->node_ids[first],
	&peer->node_ids[second],
	&peer->channel->funding_pubkey[first],
	&peer->channel->funding_pubkey[second]);

The peer->node_ids[REMOTE] is loaded here by the fromwire_channel_init:

lightning/channeld/channeld.c

Line 3145 in fc2561f

&peer->node_ids[LOCAL],

The fromwire_channel_init gets its data from the towire_channel_init over in lightningd/channel_control.c here:

lightning/lightningd/channel_control.c

Line 550 in fc2561f

&channel->peer->id,

...which should be correct. Huh. Can you show debug-level logs from C-Lightning? That should show the peer->id that C-Lightning thinks the peer is.

[joemphilips]

@ZmnSCPxj Does this help? I deleted volumes so the seeds and keys has changed. I don't know how to fix a seed.

lightningd_log.txt

Also, I added some scripts for easy reproduction to my repo (Which probably does not need any dependency besides docker-compose and jq)

git clone --recursive https://github.com/joemphilips/NRustLightning
git checkout payment_integration_tests
cd tests/NRustLightning.Server.Tests
source env.sh
  && docker-compose up --build

./cliutils/connect-all.sh && ./cliutils/prepare_funds.sh && ./cliutils/prepare_tx_for_fee.sh && ./cliutils/open_inbound_channels.sh

[ZmnSCPxj]

Thank you. Looks like you added a new debug print. What is req here?

channel_annonucement is 010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000006226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f0000d2000001000002550ede68f6503e783cdb00a0f9e1b5b192c77b9077f180a445245ff6845cc7d50339bf81f00556bbd9ae471553718b14dd72d142acca5f0da95dab50f5ae667cec02c7830f699bcb1639741f61f7d197863f837d005dc2226c642fa1e65ddc1765f703b121e60eeb380a5e145e3508af2cfc727653a9e23b7ff74668352bfb607ec43d
req is 000201b0010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000006226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f0000d2000001000002550ede68f6503e783cdb00a0f9e1b5b192c77b9077f180a445245ff6845cc7d50339bf81f00556bbd9ae471553718b14dd72d142acca5f0da95dab50f5ae667cec02c7830f699bcb1639741f61f7d197863f837d005dc2226c642fa1e65ddc1765f703b121e60eeb380a5e145e3508af2cfc727653a9e23b7ff74668352bfb607ec43d

It looks like the channel_announcement this time is correct though?

010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000006226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f0000d20000010000

02550ede68f6503e783cdb00a0f9e1b5b192c77b9077f180a445245ff6845cc7d5 <- local pubkey, correctly sorted since it is lexicographically less than remote pubkey
0339bf81f00556bbd9ae471553718b14dd72d142acca5f0da95dab50f5ae667cec <- remote pubkey
02c7830f699bcb1639741f61f7d197863f837d005dc2226c642fa1e65ddc1765f7 <- local channel funding pubkey
03b121e60eeb380a5e145e3508af2cfc727653a9e23b7ff74668352bfb607ec43d <- remote channel funding pubkey

Is the rust-lightning pubkey 0339bf81f00556bbd9ae471553718b14dd72d142acca5f0da95dab50f5ae667cec? That is what we think the peer is, and that "should" be correct since the noiseXK protocol is going to use that pubkey, and that is the link-level encryption protocol, so mismatch here would lead to C-Lightning and the rust-lightning node not communicating at all.

[joemphilips]

Thanks for the feedback, let me double check.

[joemphilips]

Shoot I was printing rust-lightning's value as c-lightning's and vise versa.
So this must be a bug in rust-lightning ... which makes more sense because no one have ever tried opening public channel before.

Sorry for the confusion, I will probably open another issue in rl side ...

[joemphilips]

Ah, it was not even a bug in rl. It is just I was misusing its api and using different our_network_key for transport layer and for channel.... 😓

[ZmnSCPxj]

Ok, good to know no problem on our side!