react-native-ldk icon indicating copy to clipboard operation
react-native-ldk copied to clipboard

Connecting with CLN peer fails with warning

Open tanx opened this issue 2 years ago • 26 comments

I ran into this issue while attempting to connect with a CLN based LSP peer on testnet (no channel open, just peering). I currently get this warning message when I try to connect to the peer. I spoke with the vendor and they have been able to connect from another LDK based node, so they suggested it might be an issue in the react-native module. This is the config I'm running: https://github.com/tanx/react-native-ldk/commit/a7b01bf78e2e01613f58faa5b1de5cf95d8b4ce5. Thank you 🙏

LDK: Finished noise handshake for connection with 025804d4431ad05b06a1a1ee41f22fefeb8ce800b0be3a92ff3b9f594a263da34e
 LOG  react-native-ldk: Success: add_peer_success
 LOG  addPeer Responses: ["add_peer_success"]
 LOG  Node ID: 02ebdcef4e8ce903da7555a00dd607b1d3aa77dbcd4eb3ec0b730c2bc6311cf4ce
 LOG  Backup updated for account wallet0
 LOG  LDK: Received peer Init message from 025804d4431ad05b06a1a1ee41f22fefeb8ce800b0be3a92ff3b9f594a263da34e: DataLossProtect: supported, InitialRoutingSync: not supported, UpfrontShutdownScript: supported, GossipQueries: supported, VariableLengthOnion: required, StaticRemoteKey: supported, PaymentSecret: required, BasicMPP: supported, Wumbo: not supported, ShutdownAnySegwit: supported, OnionMessages: not supported, ChannelType: supported, SCIDPrivacy: supported, ZeroConf: supported, unknown flags: supported
 LOG  LDK: Generating channel_reestablish events for 025804d4431ad05b06a1a1ee41f22fefeb8ce800b0be3a92ff3b9f594a263da34e
 LOG  react-native-ldk: Persisted channel manager to disk
 LOG  LDK: Got warning message from 025804d4431ad05b06a1a1ee41f22fefeb8ce800b0be3a92ff3b9f594a263da34e: gossip_timestamp_filter for bad chain: 0109000000000933ea01ad0ee984209779baaec3ced90fa3f408719526f8d77f494363b44b82ffffffff
 LOG  LDK: Marking channels with 025804d4431ad05b06a1a1ee41f22fefeb8ce800b0be3a92ff3b9f594a263da34e disconnected and generating channel_updates. We believe we can make future connections to this peer.

tanx avatar Jan 20 '23 16:01 tanx

Are you able to replicate this on regtest using something like Polar?

Jasonvdb avatar Jan 25 '23 10:01 Jasonvdb

Well with regtest on polar I can't connect to the CLN peer either, only the LND peer. Calling listpeers also only displays the pubkey of the LND peer.

tanx avatar Feb 06 '23 16:02 tanx

@Jasonvdb can you confirm that connecting to a CLN peer does not work for you either?

tanx avatar Feb 07 '23 09:02 tanx

LOG LDK: Got warning message from 025804d4431ad05b06a1a1ee41f22fefeb8ce800b0be3a92ff3b9f594a263da34e: gossip_timestamp_filter for bad chain: 0109000000000933ea01ad0ee984209779baaec3ced90fa3f408719526f8d77f494363b44b82ffffffff

This looks like it is the correct genesis hash for Testnet, however, it's likely in wrong endianess, as I don't see where the byte order is swapped. @Jasonvdb could you check whether you're handing the genesis hash in the right order to LDK?

tnull avatar Feb 08 '23 22:02 tnull

I'm testing this on a local regtest and I'm not able to get it to add CLN as a peer. I get a true result from peerHandler.connect(...) in swift but the peer is never added. Nothing in the logs also.

@tnull I don't get the bad chain warning though. I'm not changing the byte order anywhere except for funding_txid where I noticed I had to reverse bytes. Would that be a problem specifically for CLN though? Because we can add LND and Eclair as a peer fine.

Jasonvdb avatar Feb 09 '23 12:02 Jasonvdb

@tnull I don't get the bad chain warning though. I'm not changing the byte order anywhere except for funding_txid where I noticed I had to reverse bytes. Would that be a problem specifically for CLN though? Because we can add LND and Eclair as a peer fine.

Well, likely only CLN chooses to close the connection upon receiving a bogus chain hash. LDK does not, not sure about LND. Note this is in line with BOLT 1 as either is allowed:

  • upon receiving networks containing no common chains - MAY close the connection.

tnull avatar Feb 09 '23 15:02 tnull

I'm testing this on a local regtest and I'm not able to get it to add CLN as a peer. I get a true result from peerHandler.connect(...) in swift but the peer is never added. Nothing in the logs also.

@tnull I don't get the bad chain warning though. I'm not changing the byte order anywhere except for funding_txid where I noticed I had to reverse bytes. Would that be a problem specifically for CLN though? Because we can add LND and Eclair as a peer fine.

@Jasonvdb thanks for verifying. So it indeed looks like there is a reproducable issue in the react-native module that prevents connecting to a CLN node.

Well, likely only CLN chooses to close the connection upon receiving a bogus chain hash. LDK does not, not sure about LND. Note this is in line with BOLT 1 as either is allowed:

@tnull mmh, if CLN ist following the spec a little more strictly then perhaps the bug just isn't triggered with LND peers?

@Jasonvdb how can I help in debugging the root cause?

tanx avatar Feb 11 '23 13:02 tanx

@tnull mmh, if CLN ist following the spec a little more strictly then perhaps the bug just isn't triggered with LND peers?

Right, this is likely what happens.

@Jasonvdb how can I help in debugging the root cause?

I think you could try to find where the network graph is setup and byte-swap the genesis hash before it is passed in.

Note we'll also mitigate the issue upstream with the next release by replacing the genesis hash parameter with Network, which doesn't allow for any confusion over byte order (see https://github.com/lightningdevkit/rust-lightning/pull/2025)

tnull avatar Feb 11 '23 13:02 tnull

Tested this quickly and reversing bytes of genesis hash doesn't seem to help. Can still connect to other nodes but not CLN.

I also noticed I needed to reverse currentBlockchainTipHash when initialising a new node. I'll add these in https://github.com/synonymdev/react-native-ldk/pull/103 but I still wasn't able to connect to a CLN node.

Jasonvdb avatar Feb 13 '23 17:02 Jasonvdb

Tested this quickly and reversing bytes of genesis hash doesn't seem to help. Can still connect to other nodes but not CLN.

I also noticed I needed to reverse currentBlockchainTipHash when initialising a new node. I'll add these in #103 but I still wasn't able to connect to a CLN node.

Hm, does it always fail with above mentioned message?

tnull avatar Feb 13 '23 20:02 tnull

Hey, just wanted to follow up and ask if there are any updates on this? Thanks 🙏

tanx avatar Feb 23 '23 09:02 tanx

I know that Mutiny Wallet is able to peer with CLN just fine and it's based on LDK. Not sure if any of their code could be helpful https://github.com/MutinyWallet/mutiny-web https://github.com/MutinyWallet/mutiny-node

gkrizek avatar Feb 23 '23 14:02 gkrizek

Sorry for the delay, I'll dive into this properly next week

Jasonvdb avatar Feb 24 '23 07:02 Jasonvdb

@Jasonvdb Any progress on this? Happy to help debug anything on the LDK side.

tnull avatar Mar 14 '23 14:03 tnull

Just rebased my config on top of the current master in https://github.com/tanx/react-native-ldk/commit/10b2c373a1946a0e7432bdc7cbd484407d4fc989 and still having this issue.

tanx avatar Mar 28 '23 13:03 tanx

Mh, just confirmed again that I can indeed connect to the peer without issue from a Rust-based LDK peer.

Not sure if the endianness is indeed an issue here, but meanwhile 0.0.114 was released in which we remove any exposed genesis hashes from the public API to mitigate any confusion in that regard. Would be good to know if the issue still persist after the library has upgraded.

tnull avatar Mar 28 '23 15:03 tnull

Great, thanks for verifying @tnull. Good to know we can rule out LDK core 🙏

tanx avatar Mar 28 '23 17:03 tanx

Sorry for the delay here, I was away for a while. I did reverse bytes for genesis hash and it didn't fix, nothing actually seemed to change after doing that.

I'm busy updating to 114 now so I'll test again and get back to you.

Jasonvdb avatar Apr 01 '23 08:04 Jasonvdb

@Jasonvdb How is the update to 114 going?

gkrizek avatar Apr 12 '23 18:04 gkrizek

The bindings or other aspects have caused an increase in fatal bugs. We are working with the LDK team to resolve them, but there will be delays.

BitcoinErrorLog avatar Apr 12 '23 19:04 BitcoinErrorLog

Looks like there's a fix for the 114 update so hopefully this will land today https://github.com/synonymdev/react-native-ldk/pull/115

Jasonvdb avatar Apr 13 '23 07:04 Jasonvdb

Tested with 114 and still no luck connecting from Android or iOS. @tnull if it wasn't the genesis hashes do you have any other ideas of what we could try?

Jasonvdb avatar Apr 14 '23 06:04 Jasonvdb

So I just also confirmed that I'm able to connect the CLN-based peer on testnet without issue using the Swift bindings (see https://gist.github.com/tnull/ab5df1b4d889e62ae420e51837448f31), which means that the issue is very likely on the react-native-ldk side (which is even more plausible if the issue is also present on Android, which I wasn't aware of so far).

Will have another look at the codebase to see if I find something suspicious.

tnull avatar Apr 14 '23 11:04 tnull

Just got the same warning message with a Rust LDK client, in fact in the same setup I checked before. Will be investigating what's going on.

tnull avatar Apr 20 '23 19:04 tnull

Huh, interesting, just checked my LDK sample (with the network graph wiped and with one loaded from disk) and I don't see it :/.

TheBlueMatt avatar Apr 20 '23 20:04 TheBlueMatt

Huh, interesting, just checked my LDK sample (with the network graph wiped) and I don't see it :/.

Argh, nevermind, I had accidently set it to mainnet this time around, then the warning makes sense. So still works fine with the Rust client. Spoke too soon, excuse the noise! 🤦‍♂️

tnull avatar Apr 20 '23 20:04 tnull