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

Cross-Origin Request Blocked and JSON responses #12

Open
Reckless-Satoshi opened this issue Nov 22, 2022 · 6 comments
Open

Cross-Origin Request Blocked and JSON responses #12

Reckless-Satoshi opened this issue Nov 22, 2022 · 6 comments

Comments

@Reckless-Satoshi
Copy link

Reckless-Satoshi commented Nov 22, 2022

Hey Lnproxy :)

Background

We are adding an "Advanced Options" switch on RoboSats' payout submission, one of the new options is "Use Lnproxy". This is how it looks when enabled:

lnproxy cleaned

Draft PR: RoboSats/robosats#326
Selectable Lnproxy servers: https://github.com/Reckless-Satoshi/robosats/blob/9003448b0b3afc14297efd22cd847548afc83639/frontend/static/lnproxies.json

Cross-origin blocked

I am having issues using the response of the GET request, as it fails to get the data into the app:

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://rdq6tvulanl7aqtupmoboyk2z3suzkdwurejwyjyjf4itr3zhxrm2lad.onion/api/lnbc....?routing_msat=10000. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing).

As far as I read, it seems to be an issue that can be solved server-side. Seems like it could be as easy as adding these two params to the response header. But I am no expert :)

{"Access-Control-Allow-Origin":"*",
"Access-Control-Allow-Headers": "Origin, X-Requested-With, Content-Type, Accept"
}

(Of course "*" will allow all origins, maybe a more controlled solution is preferred by selectively allowing all RoboSats coordinator onion/clearnet urls)

More info on recommended solutions: https://stackoverflow.com/a/64641435

JSON response

I find it is a bit tough to work with the GET response since they are either HTML, plaintext or empty (they must be parsed and it is difficult to parse them). It would also be great if the response of the GET simply returns a JSON serialized object e.g.:

{"wrapped_invoice": "lnbt...",
"hash": "5a8df21....",
"error": "{'Budget too small' | 'CLTV_expiry too long'....}"
}

To make it compatible with how the Lnproxy page works now, maybe an extra GET parameter can be used e.g. /api/{invoice}?routing_msats={budget}&format={'json'}

Testnet

For testing and use in RoboSats Testnet it would be great if there is a Testnet Lnproxy server or if the same service can wrap both mainnnet and testnet invoices (by decoding the network first).

Thanks a lot for this cool tool, hoping to see more adoption of it soon!

@lnproxy
Copy link
Owner

lnproxy commented Nov 22, 2022

This is awesome!

cors

These commits should fix the issue:
lnproxy/lnproxy@423723b
lnproxy/lnproxy-webui@4fd3afc

It's live on my server now so let me know if it works.

json and testnet

These are excellent suggestions. I will implement ASAP.

@Reckless-Satoshi
Copy link
Author

These commits should fix the issue:
423723b

Tested and working! 🥳

lnproxy referenced this issue in lnproxy/lnproxy-webui Nov 27, 2022
Implements new query parameter discussed here:
https://github.com/lnproxy/lnproxy/issues/12

On success, returns the wrapped payment request:
    {"wpr":"lnbc..."}

On failure returns an error object that matches the lnurl spec:
    {"status":"ERROR","reason":"..."}
@lnproxy
Copy link
Owner

lnproxy commented Nov 27, 2022

OK, json formatting for the api is implemented in the latest web-ui code and live on my server.

Though I liked the idea of having the hash already pulled out of the invoice, I decided against it since it's really only safe if the client does it (otherwise the server could just lie and give clients an invoice that simply pays the server). So I just return the "wrapped payment request" (wpr):

{"wpr":"lnbc..."}

When there is an error, the return matches the lnurl spec:

{"status":"ERROR","reason":"..."}

@Reckless-Satoshi
Copy link
Author

Reckless-Satoshi commented Nov 27, 2022

{"wpr":"lnbc..."}
{"status":"ERROR","reason":"..."}

Amazing, thank you!

If I understand, this is the correct usage, right?

https://lnproxy.org/api/l{invoice_lowercase}?routing_msat={value}&format=json

Is it possible for response values to be "cleaned" server side? For example failure reasons are sometimes sent with linebreak characters at the end \n . Plaintext invoice had linebreaks too, but it seems not anymore in json format as far as I have tested.

The lnproxy shortcut panel already released on RoboSats v0.3.1, up and usable on the mainnet coordinator. 🎉

@lnproxy
Copy link
Owner

lnproxy commented Nov 27, 2022

Yes, except the l, so:

https://lnproxy.org/api/{invoice_lowercase}?routing_msat={value}&format=json

And yes, I've been meaning clean up the error messages. Right now it's quite messy.

@lnproxy
Copy link
Owner

lnproxy commented May 22, 2023

Error messages are fixed, finally. Every failure mode I can imagine might yield a useful error message has one now, and internal errors just return "internal error". JSON is working on the new api interface https://github.com/lnproxy/spec but I'll leave this open since still no testnet/signet

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

No branches or pull requests

2 participants