Skip to content

Add legacy P2SH multisig support - #131

Merged
Jacksper13 merged 4 commits into
mainfrom
agent/legacy-p2sh-sortedmulti
Sep 17, 2026
Merged

Jacksper13 merged 4 commits into
mainfrom
agent/legacy-p2sh-sortedmulti

Conversation

@Jacksper13

@Jacksper13 Jacksper13 commented Aug 11, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

Adds focused support for legacy sh(sortedmulti(...)) multisig accounts:

  • imports and serializes legacy P2SH descriptors, including crypto-output
  • preserves per-cosigner BIP-45 derivation prefixes
  • reconstructs legacy account descriptors from PSBT inputs and requires non_witness_utxo
  • routes registered legacy inputs and outputs through the shared registered-multisig path and script validation
  • enforces the 15-key legacy script limit
  • adds P2SH-specific regression coverage

Dependencies

This branch is temporarily stacked on:

Once those merge, this branch can be rebased onto main without changing its P2SH behavior.

Scope

The general multisig PSBT hardening was moved to and merged through #134. This PR now contains only the remaining bare-P2SH compatibility layer on top of the shared path handling.

@Jacksper13
Jacksper13 marked this pull request as ready for review August 11, 2026 09:22
@Jacksper13 Jacksper13 changed the title Support legacy P2SH sortedmulti multisig Add legacy P2SH multisig support and harden PSBT validation Aug 11, 2026
@nicoburniske

Copy link
Copy Markdown
Contributor

this adds a lot of legacy p2sh/bip-45 and general multisig code that isn't needed for the security fix, which makes it much harder to review. i think the fix should stay focused on validating against the stored MultiSigDetails, with legacy support handled separately.

#134

@Jacksper13
Jacksper13 force-pushed the agent/legacy-p2sh-sortedmulti branch from 148f862 to 1675c54 Compare August 12, 2026 16:18
@Jacksper13 Jacksper13 changed the title Add legacy P2SH multisig support and harden PSBT validation Add legacy P2SH multisig support Aug 12, 2026
@Jacksper13
Jacksper13 force-pushed the agent/legacy-p2sh-sortedmulti branch from 1675c54 to aeec2af Compare August 13, 2026 11:48
@mjg-foundation

Copy link
Copy Markdown
Contributor

this work is required for signing unchained and future casa multisig PSBTs, needs to ship alongside those exports in 1.4.0. Make sure to tag ngwallet after merging, and make a PR to KeyOS to bump the ngwallet version to that tag (or latest).

Comment thread src/config.rs
Comment thread src/config.rs
@mjg-foundation

Copy link
Copy Markdown
Contributor

Review found 1 urgent and 1 high issue in the legacy P2SH compatibility layer.

Comment thread src/config.rs
@Jacksper13
Jacksper13 force-pushed the agent/legacy-p2sh-sortedmulti branch 2 times, most recently from 9ee8c37 to dacff2b Compare August 14, 2026 08:17
@Jacksper13

Copy link
Copy Markdown
Collaborator Author

@mjg-foundation Follow-up is now split by the code that owns each invariant: #139 preserves and round-trips fixed crypto-output derivation prefixes, #140 provides the shared registered-multisig path/script validation, and #131 remains focused on the legacy P2SH wrapper. The remaining KeyOS P2SH export mapping is tracked internally for the 1.5.0 integration; P2SH is intentionally not being exposed in KeyOS 1.4.0. I have left the review threads open for your verification.

@Jacksper13
Jacksper13 force-pushed the agent/legacy-p2sh-sortedmulti branch 2 times, most recently from fe9e7db to edbd873 Compare August 14, 2026 08:22

@mjg-foundation mjg-foundation left a comment

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.

Re-reviewed the new commits — no new issues.

@Jacksper13
Jacksper13 force-pushed the agent/legacy-p2sh-sortedmulti branch from edbd873 to 2558051 Compare August 18, 2026 16:36
@Jacksper13
Jacksper13 merged commit 307ace7 into main Sep 17, 2026
1 check 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.

3 participants