Skip to content

fix(wallet): added the validation of previous tx outputs for get_psbt_input #1911

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ItshMoh
Copy link

@ItshMoh ItshMoh commented Mar 24, 2025

Description

It is fixing the issue bitcoindevkit/bdk_wallet#50 . This PR is adding validation of previous tx outputs. we would be checking the size of the prev_tx.output vector.

Notes to the reviewers

Here i have checked the size of the prev_tx.output vector. I have also added an error type of "InvalidVoutIndex"

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@ValuedMammal ValuedMammal added the audit Suggested as result of external code audit label Mar 24, 2025
@ValuedMammal ValuedMammal moved this to Needs Review in BDK Wallet Mar 24, 2025
@ValuedMammal ValuedMammal added this to the 1.3.0 milestone Mar 26, 2025
@ValuedMammal
Copy link
Collaborator

Note that adding the enum variant would constitute a major semver change, but there should be a way to reuse an error from rust-miniscript, for example UtxoUpdateError.

@ItshMoh
Copy link
Author

ItshMoh commented Mar 27, 2025

Note that adding the enum variant would constitute a major semver change, but there should be a way to reuse an error from rust-miniscript, for example UtxoUpdateError.

hey @ValuedMammal As per your suggestion i have updated the error variant . I have found IndexOutOfBounds error variant in UtxoUpdateError i have used that.
Now it is ready for review.

@ItshMoh
Copy link
Author

ItshMoh commented Mar 28, 2025

Hey @ValuedMammal updated as you have suggested.

@luisschwab
Copy link
Member

Hey @ItshMoh, this PR needs to be closed and moved to the new bdk_wallet repo. Here's how:

  1. From your Github account, fork https://github.com/bitcoindevkit/bdk_wallet
  2. From your local system bdk repo folder, push your PR changes to your bdk_wallet fork on Github:
    git checkout <PR branch path>
    git push [email protected]:<username>/bdk_wallet.git
    
  3. Clone to your forked bdk_wallet repo to your local system and switch to your PR branch:
    git clone [email protected]:<username>/bdk_wallet.git
    cd bdk_wallet
    git fetch origin <PR branch path>
    git checkout -b <PR branch path> FETCH_HEAD
    git rebase master
    
  4. Resolve rebase conflicts (if any)
  5. Force push your PR branch: git push origin -f
  6. Create new PR in bitcoindevkit/bdk_wallet repo from your fork branch
  7. Copy/paste title and description from your original bdk PR and add a link back to it in your new PR.
  8. Close original PR with comment that includes link to new PR.

@ValuedMammal ValuedMammal removed this from the 2.0.0 milestone Apr 22, 2025
@notmandatory notmandatory moved this to In Progress in BDK Chain Apr 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit Suggested as result of external code audit
Projects
Status: In Progress
Status: Needs Review
Development

Successfully merging this pull request may close these issues.

3 participants