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

chore: handle events before lnclient is initialized #236

Closed
wants to merge 1 commit into from

Conversation

im-adithya
Copy link
Member

Title

@im-adithya im-adithya requested a review from rolznz January 25, 2024 12:23
@bumi
Copy link
Contributor

bumi commented Jan 25, 2024

can this happen?

service.go Show resolved Hide resolved
@rolznz
Copy link
Contributor

rolznz commented Jan 29, 2024

can this happen?

It could happen if your NWC is restarted and in a locked state, and a connected app tries to make payments. I think it might be good to return a specific error here so it's easier to debug when something goes wrong.

@bumi
Copy link
Contributor

bumi commented Jan 29, 2024

it could happen if your NWC is restarted and in a locked state, and a connected app tries to make payments. I think it might be good to return a specific error here so it's easier to debug when something goes wrong.

then the NWC app is also not listening for that messages and will not be able to reply.

and I am not sure if such internals should be publicly exposed. the app will likely not even handle that error well.

@rolznz
Copy link
Contributor

rolznz commented Jan 29, 2024

@bumi yeah, you're right. It's impossible for this to work with the new setup. (the nostr key is also encrypted so we couldn't reply even if we started the server)

@rolznz
Copy link
Contributor

rolznz commented Jan 29, 2024

@im-adithya I will close this for now - see above

@rolznz rolznz closed this Jan 29, 2024
@im-adithya im-adithya deleted the task-handle-req branch February 1, 2024 07:14
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.

3 participants