Skip to content

Conversation

bumi
Copy link
Owner

@bumi bumi commented Oct 24, 2021

This checks for errors when creating the invoice and logs the error and returns an LNURL error response.
Before it would return a blank pr.

[fixes #21]

This checks for errors when creating the invoice and logs the error and returns an LNURL error response.
Before it would return a blank pr.
@bumi
Copy link
Owner Author

bumi commented Oct 24, 2021

Do you think this is this enough? @joostjager

this also registers a shorter lnurlp that can be used additionally to the lightning address
return c.JSON(http.StatusOK, lnurl.LNURLErrorResponse{Status: "ERROR", Reason: "Invalid Amount"})
}
sats := msats / 1000 // we need sats
metadataHash := sha256.Sum256([]byte(lnurlMetadata))
invoice, err := lnClient.AddInvoice(sats, lightningAddress, metadataHash[:])
if err != nil {
stdOutLogger.Printf("Error creating invoice: %s", err)
return c.JSON(http.StatusOK, lnurl.LNURLErrorResponse{Status: "ERROR", Reason: "Server Error"})

Choose a reason for hiding this comment

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

Is StatusOK appropriate here?

Copy link
Owner Author

@bumi bumi Oct 24, 2021

Choose a reason for hiding this comment

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

IMO not, but I think that's how LNURL wants the response?

edit: I only found a comment now that LNURL does not care about the HTTP response. But I've only seen other services returning a HTTP 200

Choose a reason for hiding this comment

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

Interesting, didn't know that

Copy link
Owner Author

Choose a reason for hiding this comment

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

as described in the spec it is not the best HTTP citizen. :)

@bumi bumi merged commit 36d4dec into master Oct 25, 2021
@bumi bumi deleted the lnurl-logging branch October 25, 2021 09:00
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.

No checking of err result of AddInvoice in the lnurl block
2 participants