lndk-org / lndk

MIT License
76 stars 19 forks source link

Fix peer connect race #121

Closed orbitalturtle closed 1 week ago

orbitalturtle commented 1 month ago

This PR tackles an issue that has popped up in bolt12-playground when paying Eclair offers.

If we need to connect directly to the introduction node, LNDK's onion messenger receives a PeerConnected event to process this new connection. When we first look at that new peer, the features map is empty, and LNDK decides onion support must be "false". This messes up future payment attempts, because the onion messenger thinks that this peer doesn't support onion messaging, and shouldn't be considered in future payment paths. To fix this, the first commit adds a little wait loop that looks for a non-empty map.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 0.00%. Comparing base (dcb7f38) to head (3eeb40f).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #121 +/- ## ====================================== Coverage 0.00% 0.00% ====================================== Files 1 1 Lines 97 97 ====================================== Misses 97 97 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

orbitalturtle commented 1 month ago

Looks like the cltv expiry change breaks our itests. But that might be because our fork of ldk-sample, which we use for the integration tests, is woefully behind

EDIT: I confirmed this is because our fork of ldk-sample is behind. I'm removing the CLTV expiry commit and moving it to the transient-keys branch instead. Once #119 is in, we can merge that bug fix as well.