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: Relax GetInfo constraint for LNDhub connections #5083

Merged
merged 2 commits into from Jun 20, 2023

Conversation

dennisreimann
Copy link
Member

The LNDhub-compatible implementation by LNbits does not support the GetInfo call for all their funding sources — see lnbits/lnbits#1182. By catching that exception in combination with the LndHubLightningClient, we give people the ability to still use their LNbits-based LNDhub as a Lightning node.

Talked about this with @Kukks and @callebtc at BTCPrague, because people keep asking for a workaround. Fixes #4482.

The LNDhub-compatible implementation by LNbits does not support the `GetInfo` call for all their funding sources — see lnbits/lnbits#1182. By catching that exception in combination with the `LndHubLightningClient`, we give people the ability to still use their LNbits-based LNDhub as a Lightning node.

Fixes btcpayserver#4482.
@dennisreimann dennisreimann requested a review from Kukks June 17, 2023 06:22
@NicolasDorier
Copy link
Member

@dennisreimann A NullReferenceException is normally a bug, can you update the lightning lib to send a NotSupportedException instead?

@NicolasDorier
Copy link
Member

By NotSupportedException I mean, you need to catch where the NullReferenceException is thrown, and instead check for null and send the NotSupportedException and catch this.

Copy link
Member

@Kukks Kukks left a comment

Choose a reason for hiding this comment

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

I think this needs to be done on the Lightning.Lndhub lib instead of here.

@dennisreimann
Copy link
Member Author

Yup, I like the way that Nicolas suggested. Will update.

dennisreimann added a commit to btcpayserver/BTCPayServer.Lightning that referenced this pull request Jun 19, 2023
NicolasDorier pushed a commit to btcpayserver/BTCPayServer.Lightning that referenced this pull request Jun 19, 2023
@dennisreimann
Copy link
Member Author

dennisreimann commented Jun 19, 2023

Updated it. Depending in the LNbits version, the endpoint is either not supported or returning incomplete data — both cases are handled now and lead to the node getting registered, but without public addresses.

grafik

/cc @callebtc

@NicolasDorier NicolasDorier merged commit 0d0477d into btcpayserver:master Jun 20, 2023
4 checks passed
@dennisreimann dennisreimann deleted the lnbits-lndhub-fix branch June 20, 2023 08:31
@dennisreimann dennisreimann restored the lnbits-lndhub-fix branch June 22, 2023 08:57
@dennisreimann dennisreimann deleted the lnbits-lndhub-fix branch July 3, 2023 12:49
@massmux
Copy link

massmux commented Oct 25, 2023

Just to confirm that the setup that finally works is this one

BTCPay Server v1.11.7+a921504bc

LNBits ver 0.11.1 lndhub plugin 0.3.2

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

Successfully merging this pull request may close these issues.

[Bug]: LNBank and LNDHub (via lnbits extension) Stopped to work
4 participants