Skip to content

psbt: check that non-witness UTXOs' txids match the input txid#4637

Merged
apoelstra merged 3 commits into
rust-bitcoin:masterfrom
apoelstra:2025-06_fix-psbt
Jun 27, 2025
Merged

psbt: check that non-witness UTXOs' txids match the input txid#4637
apoelstra merged 3 commits into
rust-bitcoin:masterfrom
apoelstra:2025-06_fix-psbt

Conversation

@apoelstra

Copy link
Copy Markdown
Member

Fixes #4617

@github-actions github-actions Bot added the C-bitcoin PRs modifying the bitcoin crate label Jun 23, 2025
@github-actions github-actions Bot added the test label Jun 23, 2025
@coveralls

coveralls commented Jun 23, 2025

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15881559852

Details

  • 32 of 39 (82.05%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.004%) to 84.248%

Changes Missing Coverage Covered Lines Changed/Added Lines %
bitcoin/src/psbt/mod.rs 18 19 94.74%
bitcoin/src/psbt/error.rs 0 6 0.0%
Totals Coverage Status
Change from base Build 15857094248: -0.004%
Covered Lines: 23591
Relevant Lines: 28002

💛 - Coveralls

@apoelstra apoelstra left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

On 28e6952 successfully ran local tests

tcharding
tcharding previously approved these changes Jun 24, 2025

@tcharding tcharding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK 28e6952

@apoelstra

Copy link
Copy Markdown
Member Author

Gonna let this one sit for 48 hours since it's nontrivial (affects serde deserialization). Just in case Kix shows up and wants to comment.

Comment thread bitcoin/src/psbt/error.rs Outdated
NegativeFee,
/// Integer overflow in fee calculation
FeeOverflow,
/// Non-witness UTXO (which is a complete transaction) has txid that

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit - txid is lower case here while the acronym is upper cased elsewhere.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Its like that all over the place, we could have an issue to fix it though.

 gg txid | grep ///

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I had meant in the context of this PR: /// The TXID of the non-witness UTXO.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Anyway, I think the best thing would be rustdoc links [`Txid`] when possible

Putting this in its own commit so that the fix and the test can live in
separate commits which reviewers can swap.
This requires updating a couple serde test vectors -- we are tightening
validation so that some existing serde-serialized PSBTs no longer
deserialize. I believe this is the correct thing to do.

@tcharding tcharding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK 53b9332

@apoelstra apoelstra left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

On 53b9332 successfully ran local tests

@apoelstra apoelstra merged commit 2c18ec2 into rust-bitcoin:master Jun 27, 2025
24 checks passed
@apoelstra apoelstra deleted the 2025-06_fix-psbt branch June 27, 2025 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-bitcoin PRs modifying the bitcoin crate test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

psbt: check that non-witness UTXO matches outpoint hash

4 participants