-
Notifications
You must be signed in to change notification settings - Fork 913
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
newaddr: various fixes for msggen and docs #7217
Conversation
01aa02b
to
b7d1b14
Compare
b7d1b14
to
79c68e9
Compare
So previously when generating cln-rpc's |
79c68e9
to
85b2ab2
Compare
this also found another wrong assigment in |
Yeah, we haven't removed fields so far, hence we never had a confirmation it'd be assigning stable field numbers. It seems you are hitting a couple of new and untested scenarios. Apologies for not having tried some of these before, and thanks for the incredible help 🙏 |
85b2ab2
to
0e6f72d
Compare
had to add another check for an edge case that would cause problems in #7218 |
0e6f72d
to
11ad463
Compare
added explicit numbering of rust enums |
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.
I think I found one place where we are still just iterating numbers rather than picking them from the .mssggen.json
file in grpc.py
, otherwise this is very much good to go 👍
11ad463
to
7c69cf6
Compare
rebased and fixed |
7c69cf6
to
240cb06
Compare
Where in |
I meant the |
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 a227b279ba26e3ff0d81a22247a7bdb2f7ea8aae
@@ -330,7 +330,6 @@ | |||
"NewaddrAddresstype": { | |||
"all": 2, | |||
"bech32": 0, | |||
"p2sh-segwit": 1, |
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.
Technically we could leave these in, as a trace for future reference, so we don't end up reusing the same number for a new enum variant. But I'll merge this as is, since that's out of scope for this PR.
There were various occurences of
p2sh-segwit
still left in msggen, tests and documentation. So i removed them. I also updated some documentation files to reflect the current state ofnewaddr
.Is it problematic that the values in
message NewaddrResponse
changed?Fixes: #6996
Replaces: #7146