-
Notifications
You must be signed in to change notification settings - Fork 902
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
msggen: add missing-grpc
command
#7209
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 3b64d42
Building tooling around a temporary thing may appear weird, and I thought so too initially, but "temporary" had been the status quo for too long not to do this 😅
@jackstar12 Could you please rebase the PR on origin/master and resolve the conflict? |
ecd47d2
to
f252233
Compare
Just rebased:
|
@jackstar12 Thanks for the PR and adding the missing commands list code too :). Also I would suggest that only six RPC commands are missing now:
Below is the justification for others to be removed from the list:
|
@jackstar12 Also please add a change-log entry including the list of rpc methods added with this PR. |
I would also.add that the two methods Since there is no concept of a connection as such in grpc (it multiplexes commands over several connections so which connection you're instrumenting is not clear) and cln-rpc opens a new FD for every call as well, these methods have absolutely no effect and should not be mapped. This leaves us with 4 methods to translate. Shall we maybe just mao them and skip adding a separate tool to identify unmapped methods? |
Keeping the logic to identify un-mapped methods might not be a bad idea. It will be helpful in identifying future new methods (eg. listaddresses) as well. We can also confirm it with
|
Note that some commands are either experimental or internal use only. We haven't documented this well in the past, so they get baked in: we should consider this in future. |
We should also move this list of supported commands to the schema, where documentation can access it. |
This PR adds a
missing-grpc
command to msggen which can be used to track down missing grpc methods: #7167 (comment)It currently outputs: