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

Add BIP 370: PSBT Version 2 #1059

Merged
merged 2 commits into from Mar 15, 2021
Merged

Add BIP 370: PSBT Version 2 #1059

merged 2 commits into from Mar 15, 2021

Conversation

achow101
Copy link
Member

Adds a BIP specifying PSBT version 2, as discussed on the mailing list: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2020-December/018300.html

This requires the formatting changes to BIP 174 to be merged first: #1055

@luke-jr
Copy link
Member

luke-jr commented Feb 3, 2021

Needs a backward compatibility section

@luke-jr luke-jr added the New BIP label Feb 3, 2021
@achow101
Copy link
Member Author

achow101 commented Feb 4, 2021

Needs a backward compatibility section

There's a Compatibility section.

@luke-jr
Copy link
Member

luke-jr commented Feb 4, 2021

Would be nice to have it named per BIP 2

Let's use BIP number 370

@luke-jr luke-jr changed the title Add bip-psbt2 Add BIP 370: PSBT2 Feb 4, 2021
@luke-jr luke-jr changed the title Add BIP 370: PSBT2 Add BIP 370: PSBT Version 2 Feb 4, 2021
@achow101
Copy link
Member Author

achow101 commented Feb 4, 2021

Would be nice to have it named per BIP 2

Let's use BIP number 370

Done.

bip-0370.mediawiki Outdated Show resolved Hide resolved
bip-0370.mediawiki Show resolved Hide resolved
bip-0370.mediawiki Outdated Show resolved Hide resolved
bip-0370.mediawiki Outdated Show resolved Hide resolved
If the newly added input has an incompatible time lock, then it must not be added.
If it changes the transaction's locktime when there are existing signatures, it must not be added.

If the Has SIGHASH_SINGLE flag is True, then the Constructor must iterate through the inputs and find the inputs which have signatures that use SIGHASH_SINGLE.
Copy link
Contributor

Choose a reason for hiding this comment

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

Incomplete sentence, If the PSBT_GLOBAL_TX_MODIFIABLE field has

Copy link
Member Author

Choose a reason for hiding this comment

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

Has SIGHASH_SINGLE flag is the noun in this sentence.

bip-0174.mediawiki Show resolved Hide resolved
Copy link
Contributor

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

ACK b3d224f.

We will probably implement this in rust-bitcoin and cross check once we have test-vectors.

Previously these tests were using 0x0f as the unknown field number. As
these have now been defined, change them to use 0xf0 instead as it is
unlikely we will use those anything soon.
The PSBT version number is set to 2. The transaction version number must be set to at least 2. <ref>'''Why does the transaction version number need to be at least 2?''' The transaction version number is part of the validation rules for some features such as OP_CHECKSEQUENCEVERIFY. Since it is backwards compatible, and there are other ways to disable those features (e.g. through sequence numbers), it is easier to require transactions be able to support these features than to try to negotiate the transaction version number.</ref>
The Creator should also set PSBT_GLOBAL_PREFERRED_LOCKTIME.
If the Creator is not also a Constructor and will be giving the PSBT to others to add inputs and outputs, the PSBT_GLOBAL_TX_MODIFIABLE field must be present and and the Inputs Modifiable and Outputs Modifiable flags set appropriately.
If the Creator is a Constructor and no inputs and outputs will be added by other entities, PSBT_GLOBAL_TX_MODIFIABLE may be omitted.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this also explicitly specify that if the no inputs/outputs will be modified(instead of added), then PSBT_GLOBAL_TX_MODIFIABLE can be omitted.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because things like signatures will be added to the inputs, so you would expect them to be modified.

Copy link
Contributor

Choose a reason for hiding this comment

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

What does "input modification" mean in the PSBT_GLOBAL_TX_MODIFIABLE flag? Does this mean don't touch any data apart from witness data? Is changing something like nSequence allowed?

Copy link
Member Author

Choose a reason for hiding this comment

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

It just means that the inputs vector can be modified, i.e. new inputs added. In terms of an input or output being modified, anything can be changed except for the things that uniquely identify the input or output. This means that you can't change the previous txid and output index in an input, and you can't change the scriptpubkey or the amount in an output.

@luke-jr luke-jr merged commit eaaf376 into bitcoin:master Mar 15, 2021
| [[bip-psb2.mediawiki|psbt2]]
|-
| SIGHASH_SINGLE Inputs
| <tt>PSBT_GLOBAL_SIGHASH_SINGLE_INPUTS = 0x07</tt>
Copy link
Contributor

Choose a reason for hiding this comment

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

Post-merge coment: This field is not present in BIP370. Was it left here by mistake ? Or not added to BIP370 by mistake ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, that should have been removed from 174.

Copy link
Contributor

@dgpv dgpv Dec 17, 2021

Choose a reason for hiding this comment

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

btw, if the BIP would require signers to add PSBT_IN_SIGHASH_TYPE if they used SIGHASH_SINGLE for the signature, and finalization to preserve PSBT_IN_SIGHASH_TYPE, then constructors could rely on this instead, and they would not need an interpreter. The problem would be older finalizers that can still remove this field, though. Was this bitmap planned to be used essentially for the same purpose ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants