Skip to content

clnrest: add more request and response types #8383

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daywalker90
Copy link
Collaborator

@daywalker90 daywalker90 commented Jun 27, 2025

Changelog-Added: clnrest can now return successful responses as xml, yaml, html or form-encoded in addition to json defined in the 'Accept' header. The same goes for request types except for html defined in the 'Content-type' header.

Important

25.09 FREEZE July 28TH: Non-bugfix PRs not ready by this date will wait for 25.12.

RC1 is scheduled on August 11th

The final release is scheduled for September 1st.

Fixes #7164

Right now the html response type is just the json wrapped as text in an html body and for requests i did not add html.

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed.
  • Related issues have been listed and linked, including any that this PR closes.

@ShahanaFarooqui
Copy link
Collaborator

BTW, the PR is clearly WIP but thought to post my test cases here for future review. Feel free to use them in test suite if they seem helpful.

-----------------------
Request:
curl -k -X 'POST' 'https://127.0.0.1:3010/v1/getinfo' -H 'rune: MyRune' -d '{}'

Response:
{"code":null,"data":null,"message":"Unsupported accept header: */*"}

Expectation:
Should respond getinfo response in json (default to application/json).

-----------------------
Request:
curl -k -X 'POST' 'https://127.0.0.1:3010/v1/getinfo' -H 'rune: MyRune' -d '{}' -H 'accept: application/json' -H 'Content-Type: application/json'

Response:
Successful getinfo json response.

Expectation:
Successful getinfo json response.

-----------------------
Request:
curl -k -X 'POST' 'https://127.0.0.1:3010/v1/newaddr' -H 'rune: MyRune' -d '{"addresstype": "p2tr"}' -H 'accept: application/json' -H 'Content-Type: application/json'

Response:
Successful json response with p2tr address.

Expectation:
Successful json response with p2tr address.


-----------------------
Request:
curl -k -X 'POST' 'https://127.0.0.1:3010/v1/newaddr' -H 'rune: MyRune' -d '{"addresstype": "p2tr"}' -H 'accept: application/xml' -H 'Content-Type: application/json'

Response:
Successful xml response with p2tr address.

Expectation:
Successful xml response with p2tr address.


-----------------------
Request:
curl -k -X 'POST' 'https://127.0.0.1:3010/v1/newaddr' -H 'rune: MyRune' -d '<addresstype>p2tr</addresstype>' -H 'accept: application/xml' -H 'Content-Type: application/xml'

Response:
Responded with bech32 address in xml format.

Expectation:
It should respond with p2tr address in xml format but `Content-Type: application/xml` doesn't work and the request generates default bech32 addres instead.

@daywalker90 daywalker90 force-pushed the clnrest-return-types branch from 3c27f6d to 733601e Compare June 28, 2025 19:30
@daywalker90 daywalker90 changed the title clnrest: add xml, yaml, html and form-encoded response types clnrest: add more request and response types Jun 28, 2025
@daywalker90
Copy link
Collaborator Author

I've added support for different request types (except html, how would that work?). I also added some tests and improved the documentation/compatibility with the swagger ui.

@daywalker90
Copy link
Collaborator Author

haha what is the CI smoking:

newaddr = l1.rpc.newaddr(addresstype='p2tr')['p2tr']
pyln.client.lightning.RpcError: RPC call failed: method: newaddr, payload: {'addresstype': 'p2tr'}, error: {'code': -32602, 'message': "'addresstype' should be 'p2tr', 'bech32', or 'all', not 'p2tr'"}

@daywalker90 daywalker90 force-pushed the clnrest-return-types branch from 733601e to 277730d Compare June 28, 2025 21:32
@ShahanaFarooqui
Copy link
Collaborator

I've added support for different request types (except html, how would that work?). I also added some tests and improved the documentation/compatibility with the swagger ui.

For now, we can proceed without adding HTML type support and will reconsider in future if a compelling use case arises.

Changelog-Added: clnrest can now return successful responses as xml, yaml, or form-encoded in addition to json defined in the 'Accept' header. The same goes for request types defined in the 'Content-type' header.
@daywalker90 daywalker90 force-pushed the clnrest-return-types branch from 277730d to 7e44466 Compare June 30, 2025 13:32
@daywalker90
Copy link
Collaborator Author

pyln.client.lightning.RpcError: RPC call failed: method: newaddr, payload: {'addresstype': 'p2tr'}, error: {'code': -32602, 'message': "'addresstype' should be 'p2tr', 'bech32', or 'all', not 'p2tr'"}

Apparently this isn't a weird CI bug but liquid just doesn't allow p2tr addresses (Is this known?). I've added a skip on liquid for those tests.

For now, we can proceed without adding HTML type support and will reconsider in future if a compelling use case arises.

Yes, i removed the html type from responses.

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.

clnrest: dynamic content-type and accept headers
2 participants