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

Add an 'id' field to commando requests #22

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

niftynei
Copy link
Contributor

@niftynei niftynei commented Jul 4, 2023

Tested for the golang one, the commando one probably needs some more test runs.

Fixes the error where commando requires an id field at the top level for calls.

@jb55 jb55 closed this Jul 4, 2023
@jb55 jb55 reopened this Jul 4, 2023
commando.c Outdated
@@ -27,6 +27,8 @@ int EXPORT commando_make_rpc_msg(const char *method, const char *params,
cursor_push_str(&msgbuf, params) &&
cursor_push_str(&msgbuf, ",\"rune\":\"") &&
cursor_push_str(&msgbuf, rune) &&
cursor_push_str(&msgbuf, "\",\"id\":\"") &&
cursor_push_u64(&msgbuf, (u64)req_id) &&
Copy link
Owner

Choose a reason for hiding this comment

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

this won't push a string, this pushes a raw u64

@niftynei
Copy link
Contributor Author

niftynei commented Jul 5, 2023 via email

@jb55
Copy link
Owner

jb55 commented Jul 5, 2023 via email

@niftynei niftynei force-pushed the nifty/go_id branch 2 times, most recently from dd1ccf9 to 88fe3d5 Compare November 10, 2023 22:34
@niftynei
Copy link
Contributor Author

Updated: now the c code uses char * for ids (which matches underlying update)

@jb55
Copy link
Owner

jb55 commented Nov 27, 2023 via email

@niftynei
Copy link
Contributor Author

Is this still a number?

I think any string is valid now? Yeah it definitely breaks the API for users downstream of the C library.

@jb55
Copy link
Owner

jb55 commented Nov 30, 2023 via email

@niftynei niftynei force-pushed the nifty/go_id branch 2 times, most recently from 0a672a7 to 2396322 Compare November 30, 2023 21:23
turn the type from int to string -> print directly to CLN
Lets us send a bunch of messages at once (but currently incredibly
not thread safe lol)
@jb55
Copy link
Owner

jb55 commented Dec 1, 2023 via email

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.

2 participants