Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LSPS2: Handle all htlc_intercepted error scenarios #40

Open
johncantrell97 opened this issue Nov 6, 2023 · 3 comments
Open

LSPS2: Handle all htlc_intercepted error scenarios #40

johncantrell97 opened this issue Nov 6, 2023 · 3 comments

Comments

@johncantrell97
Copy link
Contributor

If the scid to peer map is empty for an scid we currently just do nothing and return Ok((). This means the intercepted HTLC will never be failed (or forwarded) until LDK detects it's close to expiry. The library should fail it for the user here.

If the peer is found but there's no peer_state then we return Err to the user but do not automatically fail it for them.

Finally, if we do have peer state but there's no outbound jit channel found, we will end up returning Ok(() and the HTLC will remain stuck yet again.

Need to properly fail and error all of these cases.

@johncantrell97 johncantrell97 changed the title LSPS2: Handle all htlc_intercepted error scenarios` LSPS2: Handle all htlc_intercepted error scenarios Nov 6, 2023
@ZmnSCPxj-jr
Copy link

You also want to consider composability, i.e. the intercepted-HTLC interface could also be used for non-LSPS services outside of ldk-lsp-client.

My suggestion is to create a composable interface with two primary methods: a "do you know this SCID?" call and a "you said you know this SCID so handle it now" call. Then a driver interfaces between the LDK intercepted-HTLC event and the composable interface, calls "do you know this SCID?" for each given interface and if that fails, definitely fails the interception event, otherwise it calls into "handle it now" of the first given interface instance that said it knows the SCID.

@johncantrell97
Copy link
Contributor Author

Yeah that's a good point. Originally I was thinking it would just return an error that differentiates between "IDontKnowThisSCID" and "IKnowThisSCIDAndItFailedForSomeReason" but your solution feels better.

@ZmnSCPxj-jr
Copy link

ZmnSCPxj-jr commented Nov 7, 2023

Properly speaking the "do you know this SCID" call should return a Option<dyn FnOnce<whatever>> or something rusty like that (the FnOnce is the function that is called for the "handle it now"), i.e. the "do you know this SCID" should only expose the "okay you said you know it so do it now" interface if it actually claims to know the SCID, otherwise returns None.

I proposed something similar for the. unrecognized message ID composable interface but that was rejected because of not being efficient due to the use of dyn though. On the other hand, correctness > efficiency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants