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

lightning-liquidity alpha release #1050

Merged
merged 4 commits into from Feb 26, 2024

Conversation

johncantrell97
Copy link
Contributor

This PR:

  • fixes usage of liquidity library to pass payment_size_msat to select_opening_params
  • upgrades to the alpha release of the liquidity library.

There is no longer the need (or ability) to use a user_channel_id with the LSPS flow. It actually introduces a library generated RequestId that is returned when a request is made. This can be used exactly like the voltage flow fee_id. Allowed for some simplification by removing all usage of user_channel_id and remove the Option on both FeeResponse id and InvoiceRequest fee_id. Both flows use this id now.

@@ -355,7 +344,7 @@ impl<S: MutinyStorage> Lsp for LspsClient<S> {
// this is only here to keep both voltage + lsps logic symmetric
if inbound_capacity_msat >= fee_request.amount_msat {
return Ok(FeeResponse {
id: None,
id: String::from("deadbeef"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty sure this is wrong lol

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not actually :)

we aren't using the flow in this case. if they have liquidity we want to just generate a regular invoice and not use lsps.

so instead of returning some kind of Error here, to keep the flows symmetric it just returns nonsense response that won't get used

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but does have me wondering why we even have to enter the flow, let me review and remind myself

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like it can be refactored out, let me try

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, removed the fee check when user has enough inbound liquidity on lsps flow

@benthecarman
Copy link
Collaborator

benthecarman commented Feb 22, 2024

Not able to create an invoice on mutinynet with this branch, just hangs

@johncantrell97
Copy link
Contributor Author

Not able to create an invoice on mutinynet with this branch, just hangs

what token are you using, lspstoken doesn't exist anymore :(

try with: 4GH1W3YW

though we are also having issues testing with ldk-node where the channel opens but htlc is never forwarded.. looking into that separately. but you should be able to get an invoice

@benthecarman
Copy link
Collaborator

Is there anything we can do to log that the token is invalid?

@benthecarman
Copy link
Collaborator

try with: 4GH1W3YW

that doesn't seem to work either :/

@johncantrell97
Copy link
Contributor Author

Is there anything we can do to log that the token is invalid?

I need to run for a bit but yes there should be. we return (and the spec specifies) an error code for this. just need to double check how the client surfaces it (if it does)

@johncantrell97
Copy link
Contributor Author

try with: 4GH1W3YW

that doesn't seem to work either :/

Can you try again we just deployed a couple fixes. What payment size are you trying? Anything helpful in logs?

@benthecarman
Copy link
Collaborator

Trying on signet for 100k sats and still failing, nothing in the logs

Copy link
Collaborator

@benthecarman benthecarman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested and looks good to me

@benthecarman benthecarman merged commit 13e5bf9 into MutinyWallet:master Feb 26, 2024
9 checks passed
benthecarman pushed a commit that referenced this pull request Feb 26, 2024
* specify payment size with buy request

* upgrade to liquidity alpha release crate

* remove fee check when user liquidity

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

Successfully merging this pull request may close these issues.

None yet

2 participants