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

cli: fix transfer print psbt version #181

Closed
wants to merge 1 commit into from

Conversation

eauxxs
Copy link

@eauxxs eauxxs commented Apr 29, 2024

Description:
The display implementation of Psbt does not use {:#}(formatter.alternate()) to determine the psbt version. All printing here is the V2 version of psbt.

@dr-orlovsky
Copy link
Member

Sorry, this doesn't make any sense: neither description nor code changes. Please update or this will be closed.

Use google translate if necessary and read documentation on how rust Display work

@eauxxs
Copy link
Author

eauxxs commented Apr 30, 2024

In the transfer code, the psbt version returned by Runtime.pay is always V2, regardless of whether the parameter of transfer command -v2 is passed in or not.
If I do not specify the psbt_file parameter in the parameter, then psbt will be displayed and printed to the terminal. But in the impl Display for Psbt code, println("{psbt}"), f.width() == None, f.sign_aware_zero_pad() == false, so it will match the _ branch and the returned ver is 2.

image

Sorry, my English writing is terrible. I tried my best to explain this matter. If there are errors in my ideas or code implementation, you can close this pull request. Sorry to take up your time.

@dr-orlovsky
Copy link
Member

Thank you very much for reporting the issue, you are right!

But this PR doesn't fixes it and do not make any sense - it is simply wrong, so I close it and open an issue to address the bug you have discovered.

@eauxxs
Copy link
Author

eauxxs commented May 2, 2024

Thank you very much for your reply. It seems that I don't have to learn rust again😄. But I have some doubts about the point where you say the fixed code do not make any sense. Because I only made changes based on the corresponding documentation, and I have tested it locally and it is runnable. Maybe your solution is that the Runtime::pay method should have a parameter to control the version of psbt returned? Or some other plan? @dr-orlovsky

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