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

psbt: don't serialize witness data for input non-witness utxos #452

Merged
merged 3 commits into from
Jul 5, 2024

Conversation

jgriffiths
Copy link
Contributor

Some test vectors in BIP 0174 are incorrect - they include witness data in non-witness UTXOs, which is not output (at least now) by core.

Accept witness data if provided, but do not output it. This matches cores behaviour as can be seen by running utxoupdatepsbt on the test samples that have been fixed here. We keep some old test cases from the BIP to prove wallys re-serialization matches core.

Some test vectors in BIP 0174 are incorrect - they include witness
data in non-witness UTXOs, which is not output (at least now) by core.

Accept witness data if provided, but do not output it. This matches
cores behaviour as can be seen by running utxoupdatepsbt on the test
samples that have been fixed here. We keep some old test cases from
the BIP to prove wallys re-serialization matches core.
@jgriffiths jgriffiths force-pushed the psbt_non_witness_utxo branch from b041664 to 1eb727c Compare July 5, 2024 10:47
@JamieDriver
Copy link
Contributor

JamieDriver commented Jul 5, 2024

I agree from my own testing w/ core that 'p2wpkh' and 'p2sh-p2wpkh' inputs have both witness_utxo and non_witness_utxo sections, whereas 'p2pkh' inputs only have a non_witness_utxo section.

I agree also that non_witness_utxo sections do not themselves contain txinwitness data.

On this basis, utack 1eb727c

@jgriffiths jgriffiths merged commit 1eb727c into master Jul 5, 2024
4 checks passed
@jgriffiths jgriffiths deleted the psbt_non_witness_utxo branch July 5, 2024 12:26
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