Skip to content

Fix legacy P2PKH singlesig address verification#373

Open
etherbiswas wants to merge 5 commits into
3rdIteration:devfrom
etherbiswas:dev
Open

Fix legacy P2PKH singlesig address verification#373
etherbiswas wants to merge 5 commits into
3rdIteration:devfrom
etherbiswas:dev

Conversation

@etherbiswas

Copy link
Copy Markdown

Description

This PR fixes legacy P2PKH address verification when verifying addresses derived from singlesig descriptors.

Previously, get_multisig_address() only allowed legacy descriptors when descriptor.is_basic_multisig was true:

if descriptor.is_segwit or (descriptor.is_legacy and descriptor.is_basic_multisig):

This prevented valid legacy singlesig descriptors from being verified and resulted in the error:

p2pkh address verification not yet implemented!

The fix allows legacy descriptors to be derived regardless of whether they are multisig:

if descriptor.is_segwit or descriptor.is_legacy:

Testing

I reproduced the issue on a physical SeedSigner Pi0 Smartcard build using a Satochip card configured with legacy (P2PKH) addresses.

Before the change:

  • Address verification failed with:

    • p2pkh address verification not yet implemented!

After the change:

  • Legacy singlesig addresses verify successfully.
  • Existing SegWit verification behavior remains unchanged.

Screenshots

No UI changes were made.

[ ] Raspberry Pi OS Manual Build
[x] SeedSigner OS on a Pi0/Pi0W board
[ ] Other

[x] Bug fix
[ ] New feature
[ ] Code refactor
[ ] Documentation
[ ] Other

@etherbiswas

Copy link
Copy Markdown
Author

@3rdIteration please Review src/seedsigner/views/seed_views.py for the acutal patch.

Added detailed architecture flow diagrams and explanations for SeedSigner, including boot and runtime flow, main interaction loop, QR scan flow, PSBT signing flow, seed flow, and hardware flow.
Enhance README with architecture and flow diagrams
@etherbiswas

Copy link
Copy Markdown
Author

Address verification currently also only had multisig support only, when singlesig was being used the resultant behavior was skipping, refer commit 7431683

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.

1 participant