lnbits / lnurl

LNURL implementation for Python.
https://pypi.org/project/lnurl
MIT License
64 stars 19 forks source link

Add a server/service that can create LNURL responses #2

Closed eillarra closed 4 years ago

eillarra commented 4 years ago

It would be something like a LnurlService that will return the appropriate JSON responses. It makes sense to create/validate the responses because Python will be used mainly in the backend anyway (for what the spec calls LN SERVICE).

jogc commented 4 years ago

As a first step, couldn't it be useful to just be able to create the responses rather than a full-blown service thing? Sure, its easy enough to do on your own, but it would make sure all the required fields are there, the tag name is valid and spelled correctly, any HTTPS URLs in there are actually valid, etc.

Could be something like this:

For parsing:

response_instance_of_correct_type = LnurlResponse.from_json(json_data) # from json data
response_instance_of_correct_type = LnurlResponse.from_response(res) # directly from requests response object

For constructing:

outgoing_response = LnurlChannelResponse(uri, callback_url, k1)
outgoing_response = LnurlHostedChannelResponse(uri, k1, alias)
outgoing_response = LnurlAuthResponse(k1)
outgoing_response = LnurlWithdrawResponse(callback_url, query_param, max_withdrawable, default_description)
outgoing_response = LnurlPayResponse(callback_url, max_sendable, min_sendable, metadata)

str(outgoing_response) # gives json
outgoing_response.json() # also gives json
outgoing_response.dict() # gives the data in dict format if caller wants to do the json encoding himself
eillarra commented 4 years ago

Yes, I was thinking along the same lines.

I like using the general LnurlResponse.from_json() (from_dict()?) to parse and determine the correct response instance. I am not sure if we should add requests support directly or force users to always pass a dictionary (LnurlResponse.from_dict(res.json())).

+1 for constructors per type. We need to decide if we force **kwargs here :)

LnurlPayResponse('https://service.com/lnurl-pay', 40000, 30000, {})  # vs.
LnurlPayResponse(callback_url='https://service.com/lnurl-pay', max_sendable=40000, min_sendable=30000, metadata={})

Minor comments: LnurlWithdrawResponse in your example needs a fix and we might need a LnurlPaySuccessResponse:

out = LnurlWithdrawResponse(callback_url, k1, min_withdrawable, max_withdrawable, default_description)
out = LnurlPaySuccessResponse(pr, success_action, routes)
eillarra commented 4 years ago

Would it make sense to use pydantic for defining the different types?

eillarra commented 4 years ago

@jogc I have created a branch at eillarra/lnurl that starts integrating pydantic. This gives us validation and the .json() and .dict() helper methods out of the box.

The complete LNURL spec is not there yet, but it already covers more than v0.1.

Take a look and let me know what you think.

jogc commented 4 years ago

Yes, I was thinking along the same lines.

I like using the general LnurlResponse.from_json() (from_dict()?) to parse and determine the correct response instance. I am not sure if we should add requests support directly or force users to always pass a dictionary (LnurlResponse.from_dict(res.json())).

Lets be agnostic and just look for a json() method (like requests and aiohttp provides) or, if thats missing, a read() method (like urllib.request provides).

+1 for constructors per type. We need to decide if we force **kwargs here :)

Seems a bit harsh in this case. Is this being the current best practice in the Python community? Any examples of other libs that does this?

LnurlPayResponse('https://service.com/lnurl-pay', 40000, 30000, {})  # vs.
LnurlPayResponse(callback_url='https://service.com/lnurl-pay', max_sendable=40000, min_sendable=30000, metadata={})

Minor comments: LnurlWithdrawResponse in your example needs a fix and we might need a LnurlPaySuccessResponse:

out = LnurlWithdrawResponse(callback_url, k1, min_withdrawable, max_withdrawable, default_description)
out = LnurlPaySuccessResponse(pr, success_action, routes)

Yeah did't really pay to much attention when I made those examples.

jogc commented 4 years ago

@jogc I have created a branch at eillarra/lnurl that starts integrating pydantic. This gives us validation and the .json() and .dict() helper methods out of the box.

The complete LNURL spec is not there yet, but it already covers more than v0.1.

Take a look and let me know what you think.

Ill have a look

eillarra commented 4 years ago

The new structure is ready: https://github.com/eillarra/lnurl/tree/pydantic
Some responses need a little bit of work, but we are almost there.