Skip to content

Conversation

@jamillambert
Copy link
Collaborator

decodepsbt adds new musig fields in v30. These cannot be currently modelled with rust-bitcoin types and this PR does not change the model type. (rust-bitcoin/rust-bitcoin#4207).

The error types from v17 were not reexported and instead imported by referencing the v17 module in the import statement.

  • Tidy up the error exports and imports to reexport from v17 and then import from super instead of crate::v17.
  • In order to make the review easier first copy the previous version of decodepsbt and all the related sub types and errors from v24 to v30. This patch copies to code and updates the exports only.
  • Add the musig fields in a separate patch to make the changes from v24 clear.

@jamillambert
Copy link
Collaborator Author

This is a Draft to get feedback on the way it has been done, and the merge conflicts which will come after #409 goes in.
Specifically:

  • All of the error types have been redefined even though the model and into functions have not changed. This was to make it simpler in the future when the model will be updated and the errors also need to also be expanded to include the musig fields.
  • Not updating the model at all. The current model is just psbt: Psbt and fee: Option<Amount>. Adding extra fields didn't make sense.

@tcharding
Copy link
Member

This approach seems like a good one. Perhaps add a comment to the PsbtInput struct (in the last patch) that states something like "we cant' model this type because rust-bitcoin does not yet support Musig". Go separation of patches, obviously since you know we by now, I did not bother reading patch 2.

Reexport the errors from v17 for later versions instead of importing
from `crate::v17`.

Reexport the new version of the errors from v24 in later versions.

Reorginazation only, no functional changes.
Copy the raw_transactions folder from v24 to v30 and update exports.
No other changes.
Add the new MuSig2 fields in decodepsbt.
@jamillambert
Copy link
Collaborator Author

Added a comment to PsbtInput and the types table about musig. Rebased and fixed reexports causing merge conflict.

@jamillambert jamillambert marked this pull request as ready for review November 5, 2025 19:47
Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK 05af218

@tcharding tcharding merged commit 90f400c into rust-bitcoin:master Nov 5, 2025
30 checks passed
tcharding added a commit that referenced this pull request Nov 9, 2025
2343534 Update listwalletdir for v30 changes (Jamil Lambert, PhD)
cb73a83 Update getwalletinfo for v30 changes (Jamil Lambert, PhD)
86c40a4 Document that migratewallet is untested in v30 (Jamil Lambert, PhD)
4dbd863 Add deprecated flag to v30 settxfee test (Jamil Lambert, PhD)

Pull request description:

  There are four remaining RPCs that have changes in v30 that need to be implemented:

  - `settxfee` is deprecated in v30 and removed in v31. Feature gate the test to v30 and below and for v30 add the required `-deprecatedrpc=settxfee` flag. Remove TODO from the types table.
  - In v30 it is no longer possible to create a legacy wallet. This means that `migratewallet` cannot be tested using only v30. There are no changes to the RPC in v30 so the v29 test is still valid. Add a comment to the types table and test.
  - `getwalletinfo` had some return fields removed in v30 and a new flags field was added. Update the model to make the removed fields options and add the new flags. Update all the into functions. Redefine the type, error and into function for v30. Remove the TODO from the types table. Remove the `v29_and_below` feature gate from the test.
  - `listwalletdir` has a new `warnings` field was added in v30. Redefine the type for v30, there is no model. Remove the TODO from the types table. Remove the `v29_and_below` feature gate from the test.

  Together with #387, #388, #409 and #410 Closes #384

ACKs for top commit:
  tcharding:
    ACK 2343534

Tree-SHA512: 788e6150778dc722c7db79e63240d4c82d3ceca59ec859edd516d1ea8860671754be3072f4b5bdd391dac919bba6a61935c99df4bb3b4325cf981271ef4e87bf
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.

2 participants