search

lightningdevkit / lightning-liquidity

Other
27 stars 17 forks source link

error on htlc_intercepted for unknown scid #138

Open johncantrell97 opened 3 months ago

johncantrell97 commented 3 months ago

Currently we return Ok(()) if the intercept_scid is unknown which means upstream users won't know that we didn't know about this SCID and the intercepted htlc will be stuck until expiry.

We should signal Error for unknown scid's so upstream users can fail back if there's no other reason for the intercepted htlc.

johncantrell97 commented 3 months ago

Hm, wasn't this kind of expected behavior as we wouldn't exactly fail if the user decided not to hand us every single intercept SCID?

See the comment:

  /// Will do nothing if the intercept scid does not match any of the ones we gave out.

I think we need to error or maybe we return some kind of enum/bool. The LSP needs some way to know whether or not that this handler of intercepted HTLCs was interested in the HTLC but failed to process or does not care about that htlc.

In theory the lsp could have a chain of handlers for intercepted htlcs that it tries in succession until it finds the one that cares about it.

More importantly, if we intercept an HTLC and the liquidity library returns Ok(()) then our code assumes it was or will be handled by liquidity crate but in this case nothing will ever happen and the HTLC will be held until expiry.

We need some way to know that we should fail the htlc backwards if our handlers don't know about it or do but fail to process it.

tnull commented 3 months ago

Hm, wasn't this kind of expected behavior as we wouldn't exactly fail if the user decided not to hand us every single intercept SCID? See the comment:

    /// Will do nothing if the intercept scid does not match any of the ones we gave out.

I think we need to error or maybe we return some kind of enum/bool. The LSP needs some way to know whether or not that this handler of intercepted HTLCs was interested in the HTLC but failed to process or does not care about that htlc.

In theory the lsp could have a chain of handlers for intercepted htlcs that it tries in succession until it finds the one that cares about it.

More importantly, if we intercept an HTLC and the liquidity library returns Ok(()) then our code assumes it was or will be handled by liquidity crate but in this case nothing will ever happen and the HTLC will be held until expiry.

We need some way to know that we should fail the htlc backwards if our handlers don't know about it or do but fail to process it.

Alright, mind adjusting said comment to reflect the new behavior then?

tnull commented 2 months ago

Gentle ping @johncantrell97