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.

Sign up for GitHub

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

Jump to bottom

Still some msat suffixes #6244

Closed
hMsats opened this issue May 9, 2023 · 3 comments · Fixed by #6245
Closed

Still some msat suffixes #6244

hMsats opened this issue May 9, 2023 · 3 comments · Fixed by #6245
Assignees
@vincenzopalazzo
Labels
bug
Milestone
v23.05

Comments

@hMsats
Copy link
Contributor

hMsats commented May 9, 2023

Issue and Steps to Reproduce

local_funds_msat etc. still have suffix msat in lightning-cli listpeers

Example:

          "funding": {
                  "local_funds_msat": "500000000msat",
                  "remote_funds_msat": "0msat",
                  "pushed_msat": 0
               },

getinfo output

"version": "v23.05rc3"
@vincenzopalazzo vincenzopalazzo self-assigned this May 9, 2023
@vincenzopalazzo vincenzopalazzo added this to the v23.05 milestone May 9, 2023
@vincenzopalazzo
Copy link
Contributor

Oh that is a really nice catch! thanks

vincenzopalazzo added a commit to vincenzopalazzo/lightning that referenced this issue May 9, 2023
@vincenzopalazzo
jsonrpc: Remove the old "_msat" prefix in the listpeerchannels command
f501371 
This is a regression that we introduced in this release
due to some dirty parts of our codebase.

For historical reasons (I think), we were using a `json_add_sat_only`
procedure defined in `peer_control.c`. So when @rustyrussell removed the _msat,
we thought that all the fields were reflecting the new behavior, but
we were wrong.

This PR fixes this bug and also removes the additional function
from `peer_control.c`. This way, we can be sure that there is no other part
of our codebase that uses this method (except for other `json_add` methods).

Link: ElementsProject#6244
Reported-by: @hMsats
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
vincenzopalazzo added a commit to vincenzopalazzo/lightning that referenced this issue May 9, 2023
@vincenzopalazzo
jsonrpc: Remove the old "_msat" prefix in the listpeerchannels command
f0544cf 
This is a regression that we introduced in this release
due to some dirty parts of our codebase.

For historical reasons (I think), we were using a `json_add_sat_only`
procedure defined in `peer_control.c`. So when @rustyrussell removed the _msat,
we thought that all the fields were reflecting the new behavior, but
we were wrong.

This PR fixes this bug and also removes the additional function
from `peer_control.c`. This way, we can be sure that there is no other part
of our codebase that uses this method (except for other `json_add` methods).

Link: ElementsProject#6244
Reported-by: @hMsats
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
vincenzopalazzo added a commit to vincenzopalazzo/lightning that referenced this issue May 9, 2023
@vincenzopalazzo
jsonrpc: Remove the old "_msat" prefix in the listpeerchannels command
6f9d0ce 
This is a regression that we introduced in this release
due to some dirty parts of our codebase.

For historical reasons (I think), we were using a `json_add_sat_only`
procedure defined in `peer_control.c`. So when @rustyrussell removed the _msat,
we thought that all the fields were reflecting the new behavior, but
we were wrong.

This PR fixes this bug and also removes the additional function
from `peer_control.c`. This way, we can be sure that there is no other part
of our codebase that uses this method (except for other `json_add` methods).

Link: ElementsProject#6244
Reported-by: @hMsats
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@vincenzopalazzo vincenzopalazzo mentioned this issue May 9, 2023
Merged
@vincenzopalazzo
Copy link
Contributor

Patch up #6245

Thanks @hMsats <3

@hMsats
Copy link
Contributor Author

hMsats commented May 9, 2023

@vincenzopalazzo thanks Vincenzo! Better avoid a "Great Msat Migration" part 2 :-)

ShahanaFarooqui pushed a commit that referenced this issue May 9, 2023
@vincenzopalazzo @ShahanaFarooqui
jsonrpc: Remove the old "_msat" prefix in the listpeerchannels command
3a2b703 
This is a regression that we introduced in this release
due to some dirty parts of our codebase.

For historical reasons (I think), we were using a `json_add_sat_only`
procedure defined in `peer_control.c`. So when @rustyrussell removed the _msat,
we thought that all the fields were reflecting the new behavior, but
we were wrong.

This PR fixes this bug and also removes the additional function
from `peer_control.c`. This way, we can be sure that there is no other part
of our codebase that uses this method (except for other `json_add` methods).

Link: #6244
Reported-by: @hMsats
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vincenzopalazzo vincenzopalazzo

Labels
bug
Projects
None yet
Milestone
v23.05
Development

Successfully merging a pull request may close this issue.

jsonrpc: Remove the old "_msat" prefix in the listpeerchannels command vincenzopalazzo/lightning
2 participants
@vincenzopalazzo @hMsats