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

Use the TXID as the initial file name for PSBT #10515

Merged
merged 2 commits into from Apr 20, 2023

Conversation

adamPetho
Copy link
Collaborator

@adamPetho adamPetho commented Apr 17, 2023

Fixes: #10513

From the top of my head I went with the first 5 character of the txid as the initial name for the PSBT file.

Feel free to suggest otherwise.

MarnixCroes
MarnixCroes previously approved these changes Apr 17, 2023
Copy link
Collaborator

@MarnixCroes MarnixCroes left a comment

Choose a reason for hiding this comment

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

nice

MaxHillebrand
MaxHillebrand previously approved these changes Apr 17, 2023
Copy link
Member

@MaxHillebrand MaxHillebrand left a comment

Choose a reason for hiding this comment

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

Super useful.
cACK

@adamPetho adamPetho changed the title Use the TXID's first characters as an initial file name for PSBT Use the TXID as the initial file name for PSBT Apr 18, 2023
Copy link
Collaborator

@kiminuo kiminuo left a comment

Choose a reason for hiding this comment

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

LGTM, not tested

Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

This is how it looks now I guess it is too long 2dd31fbf6731d4dbf2b0a419a2d8cd83628a7f7d78998b3a5d81ded09cdff6c0.psbt.

@adamPetho
Copy link
Collaborator Author

This is how it looks now I guess it is too long 2dd31fbf6731d4dbf2b0a419a2d8cd83628a7f7d78998b3a5d81ded09cdff6c0.psbt.

There is nothing human-readable inside a psbt file.
So IMO it's a nice addition to have the whole TXID as its name.
User can easily check the status of the tx on a block explorer. (don't need to open Wasabi)

Also, as Kimi said:

the user is free to make it shorter

@kiminuo
Copy link
Collaborator

kiminuo commented Apr 18, 2023

This is how it looks now I guess it is too long 2dd31fbf6731d4dbf2b0a419a2d8cd83628a7f7d78998b3a5d81ded09cdff6c0.psbt.

I opened a random block explorer. I can type there 9d3f7d892f702bdfbcc1f76ebd18fe5611d7b66cb799dc76be50e938371e8f3a and it works. If I query 9d3f7d892 (shorter name), it returns nothing.

Obviously, the question is whether this is what people do, but mostly if I have a transaction, I want to know what it is about so I use a tool to inspect it (block explorer, etc.) So to me it makes sense.

@adamPetho adamPetho requested a review from molnard April 18, 2023 14:08
@molnard
Copy link
Collaborator

molnard commented Apr 18, 2023

Can you test if CC can handle that size of filename on the SD card?

@adamPetho
Copy link
Collaborator Author

adamPetho commented Apr 18, 2023

Can you test if CC can handle that size of filename on the SD card?

CC has no problem with the long file name.

I was able to sign the psbt with CC and broadcasted the signed psbt with Wasabi.

@yahiheb
Copy link
Collaborator

yahiheb commented Apr 18, 2023

User can easily check the status of the tx on a block explorer. (don't need to open Wasabi)

I guess the main reason to use the PSBT workflow and save the PSBT file is to import it to a hardware wallet to sign it and then broadcast it.
So the user cannot check the transaction if they didn't even broadcast it yet.

Obviously, the question is whether this is what people do, but mostly if I have a transaction, I want to know what it is about so I use a tool to inspect it (block explorer, etc.) So to me it makes sense.

If what I said above is true then I doubt that people will be getting their txid from a file saved on an MicroSD card or something.

@MaxHillebrand
Copy link
Member

Short identifier are fine, imo.

Is the full txid inside the file?

@molnard molnard requested a review from brizik April 19, 2023 08:34
@adamPetho
Copy link
Collaborator Author

Is the full txid inside the file?

No, there is nothing human-readable inside the PSBT.

@kiminuo
Copy link
Collaborator

kiminuo commented Apr 19, 2023

So the user cannot check the transaction if they didn't even broadcast it yet.

You can still check it after you broadcast it, or not? Hm, or does this change txid?

Btw: Can I download PSBT of wasabi transactions? I mean those that are not to be used in a hardware wallet.

Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

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

TXID as filename - ACK

Copy link

@brizik brizik left a comment

Choose a reason for hiding this comment

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

tACK
TXID set as the initial file name for PSBT (FYI - works with hard wallet ONLY)

TXID as initial file name for PSBT_tACK

Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

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

tACK

@molnard
Copy link
Collaborator

molnard commented Apr 19, 2023

Any objective reason to change this?

@yahiheb
Copy link
Collaborator

yahiheb commented Apr 19, 2023

Any objective reason to change this?

No objections it is just unnecessarily a long file name.

@yahiheb
Copy link
Collaborator

yahiheb commented Apr 19, 2023

So the user cannot check the transaction if they didn't even broadcast it yet.

You can still check it after you broadcast it, or not? Hm, or does this change txid?

AFAIK no it doesn't change the txid, and yes of course you can check it after you broadcast the tx, but honestly what is the likelihood that a user would copy the txid from that file stored on some MicroSD Card, that is very weird UX, you can simply copy the txid from the history list.

Btw: Can I download PSBT of wasabi transactions? I mean those that are not to be used in a hardware wallet.

No currently there is no way to that in Wasabi.

@molnard molnard merged commit ab64b97 into zkSNACKs:master Apr 20, 2023
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically give the file a name when saving PSBT
7 participants