Skip to content

Conversation

notTanveer
Copy link
Contributor

@notTanveer notTanveer commented Aug 27, 2025

Description

eliminates conditional logic throughout the codebase. instead of checking boolean flags and then deciding which class to instantiate, we directly store the class.

this follows the same pattern as Destination class adnd makes the code cleaner and less error prone.

This pull request is categorized as a:

  • Code refactor

Checklist

  • I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

  • N/A

I have tested this PR on the following platforms/os:

@kdmukai
Copy link
Contributor

kdmukai commented Aug 27, 2025

PR description should document the fact that validate_mnemonic() was removed and the rationale.

@notTanveer
Copy link
Contributor Author

updated the PR: init_pending_mnemonic method now takes seed_class as input instead of the is_electrum flag. this makes the code more extensible and future-proof for supporting additional seed formats.

@notTanveer notTanveer requested a review from alvroble August 29, 2025 15:09
notTanveer added a commit to notTanveer/seedsigner that referenced this pull request Aug 29, 2025
notTanveer added a commit to notTanveer/seedsigner that referenced this pull request Aug 29, 2025
seed = ElectrumSeed(self._pending_mnemonic)
else:
seed = Seed(self._pending_mnemonic)
seed = self._pending_Seed_cls(self._pending_mnemonic)
return seed.get_fingerprint(network)
except InvalidSeedException:
return None
Copy link
Contributor

Choose a reason for hiding this comment

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

This new approach LGTM now

Copy link
Contributor

@alvroble alvroble left a comment

Choose a reason for hiding this comment

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

I like the flexibility the seed_class parameter permits now. Tested ACK on Raspberry Pi OS Manual Build as of 0198399 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: SoB In Progress
Development

Successfully merging this pull request may close these issues.

3 participants