Skip to content

Conversation

@Aharonee
Copy link
Contributor

@Aharonee Aharonee commented May 19, 2025

Change Description

PSBT module dependencies are out of date. This PR updates them and fixes a unit test that relies on old implementation.

The bug in the unit test was mostly due to the choice to use many sub-modules in this repo, which results in different packages using different versions of the same code, in a way that is less than obvious to contributors.
I will try to address this issue in a future PR.

Steps to Test

To verify the change to the test, simply cherrypick this change only on master branch and run the unit test.

Pull Request Checklist

Testing

  • Your PR passes all CI checks.
  • Tests covering the positive and negative (error paths) are included.
  • Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

📝 Please see our Contribution Guidelines for further guidance.

Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks for the update/fix.

@Roasbeef Roasbeef added this to the v0.25 milestone Jul 2, 2025
@Roasbeef
Copy link
Member

CI isn't running on this for some reason. I also don't see a button to approve the run fro first time contributors.

@Aharonee can you force push?

Upgrade several package versions and fix a test case for transaction serialization.
@Aharonee Aharonee force-pushed the chore/update_psbt_deps branch from 1a684be to 4727f32 Compare July 11, 2025 20:31
@Aharonee
Copy link
Contributor Author

Aharonee commented Jul 11, 2025

CI isn't running on this for some reason. I also don't see a button to approve the run fro first time contributors.

@Aharonee can you force push?

@Roasbeef could you try requesting a review from yourself? It tends to fix this issue in my experience

@yyforyongyu
Copy link
Collaborator

Approved CI run

@coveralls
Copy link

Pull Request Test Coverage Report for Build 16229198423

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 276 unchanged lines in 19 files lost coverage.
  • Overall coverage decreased (-0.3%) to 54.868%

Files with Coverage Reduction New Missed Lines %
btcutil/block.go 1 88.57%
wire/msgblock.go 2 96.53%
wire/msggetcfcheckpt.go 2 88.24%
txscript/opcode.go 4 98.49%
wire/msggetcfheaders.go 4 81.82%
wire/msggetcfilters.go 4 81.82%
btcec/v2/schnorr/signature.go 5 79.92%
txscript/script.go 8 89.51%
wire/msgcfilter.go 11 68.42%
wire/msgtx.go 13 98.02%
Totals Coverage Status
Change from base Build 16228326782: -0.3%
Covered Lines: 31096
Relevant Lines: 56674

💛 - Coveralls

Copy link
Collaborator

@yyforyongyu yyforyongyu left a comment

Choose a reason for hiding this comment

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

🙏

@yyforyongyu yyforyongyu merged commit 9adca74 into btcsuite:master Jul 14, 2025
3 checks passed
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.

5 participants