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: Check non witness utxo outpoint early #29855

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

achow101
Copy link
Member

A common issue that our fuzzers keep finding is that outpoints don't exist in the non witness utxos. Instead of trying to track this down and checking in various individual places, do the check early during deserialization. This also unifies the error message returned for this class of problems.

A common issue that our fuzzers keep finding is that outpoints don't
exist in the non witness utxos. Instead of trying to track this down and
checking in various individual places, do the check early during
deserialization.
@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 11, 2024

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK maflcko, dergoegge, S3RK

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #30212 (rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN by willcl-ark)
  • #21283 (Implement BIP 370 PSBTv2 by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@maflcko
Copy link
Member

maflcko commented Apr 12, 2024

lgtm ACK 9e13ccc

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Apr 24, 2024
A common issue that our fuzzers keep finding is that outpoints don't
exist in the non witness utxos. Instead of trying to track this down and
checking in various individual places, do the check early during
deserialization.

Github-Pull: bitcoin#29855
Rebased-From: 9e13ccc
@maflcko maflcko added this to the 28.0 milestone Jul 3, 2024
@maflcko
Copy link
Member

maflcko commented Jul 3, 2024

It would be nice to have this in 28.x

@maflcko maflcko requested a review from S3RK July 3, 2024 10:16
Copy link
Member

@dergoegge dergoegge left a comment

Choose a reason for hiding this comment

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

utACK 9e13ccc

@S3RK
Copy link
Contributor

S3RK commented Jul 8, 2024

tACK 9e13ccc

Reviewed code and verified new test vectors. Reproduced a crash on an older version with the new test vector.

@ryanofsky ryanofsky merged commit 1f9d307 into bitcoin:master Jul 8, 2024
16 checks passed
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jul 17, 2024
A common issue that our fuzzers keep finding is that outpoints don't
exist in the non witness utxos. Instead of trying to track this down and
checking in various individual places, do the check early during
deserialization.

Github-Pull: bitcoin#29855
Rebased-From: 9e13ccc
@fanquake fanquake mentioned this pull request Jul 17, 2024
@fanquake
Copy link
Member

Backported to 27.x in #30467.

fanquake added a commit that referenced this pull request Jul 24, 2024
4f23c86 [WIP] doc: update release notes for 27.x (fanquake)
54bb9b0 test: add test for modififed walletprocesspsbt calls (willcl-ark)
f22b9ca wallet: fix FillPSBT errantly showing as complete (willcl-ark)
05192ba init: change shutdown order of load block thread and scheduler (Martin Zumsande)
ab42206 Reapply "test: p2p: check that connecting to ourself leads to disconnect" (Sebastian Falbesoner)
064f214 net: prevent sending messages in `NetEventsInterface::InitializeNode` (Sebastian Falbesoner)
0933cf5 net: fix race condition in self-connect detection (Sebastian Falbesoner)
fa90989 psbt: Check non witness utxo outpoint early (Ava Chow)

Pull request description:

  Backports:
  * #29855
  * #30357
  * #30394 (modified test commit)
  * #30435

ACKs for top commit:
  stickies-v:
    ACK 4f23c86
  willcl-ark:
    ACK 4f23c86

Tree-SHA512: 5c26445f0855f9d14890369ce19873b0686804eeb659e7d6da36a6f404f64d019436e1e6471579eaa60a96ebf8f64311883b4aef9d0ed528a95bd610c101c079
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.

7 participants