fix: use second routing hint in MRH #975
Conversation
The probing logic of LND assumes that the first node of the routing hint is a LSP (and probes that node) if all route hints have the same first node. We trick that heuristic by adding a second routing hint with a different public key when we add a magic routing hint to the invoice.
WalkthroughThe Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/swap/ReverseRoutingHints.ts (1)
143-154: Implementation correctly addresses LND probing issue.The addition of a second routing hint with different parameters effectively prevents LND from assuming all route hints belong to the same LSP. The hard-coded values serve their purpose as a "decoy" hint.
Consider documenting the specific hard-coded values used (nodeId, chanId, fee parameters) to ensure they remain appropriate over time and don't accidentally conflict with real network nodes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/swap/ReverseRoutingHints.ts(1 hunks)test/unit/swap/ReverseRoutingHints.spec.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build (ubuntu-latest, 20, stable)
- GitHub Check: build-rust (ubuntu-latest, 20, stable)
- GitHub Check: build-rust (ubuntu-latest, 20, stable)
- GitHub Check: build (ubuntu-latest, 20, stable)
🔇 Additional comments (3)
test/unit/swap/ReverseRoutingHints.spec.ts (3)
6-6: Import correctly added for test dependencies.The import of
transactionToLndScidis needed for thefakeRoutingHintconstant definition.
20-29: Test constant matches implementation values.The
fakeRoutingHintconstant correctly mirrors the second routing hint added in the implementation, ensuring test accuracy.
150-150: Tests properly updated for dual routing hints.The test expectations correctly include both the original routing hint and the new
fakeRoutingHint, ensuring comprehensive coverage of the updated functionality.Also applies to: 223-223
| @@ -140,6 +140,17 @@ class ReverseRoutingHints { | |||
| feeProportionalMillionths: 21, | |||
| }, | |||
| ], | |||
| // Useless hint to trick LND into thinking the MRH is not an LSP | |||
There was a problem hiding this comment.
In case the return value is changed in wherever it being used, it would change the values in the const object. Not sure if our gRPC transformations are destructive, so I create a new object every time
The probing logic of LND assumes that the first node of the routing hint is a LSP (and probes that node) if all route hints have the same first node. We trick that heuristic by adding a second routing hint with a different public key when we add a magic routing hint to the invoice.
Summary by CodeRabbit