Light refactor of Electrum seed support #571
Conversation
src/seedsigner/views/psbt_views.py
Outdated
| status_headline=None, | ||
| text=f"Some features disabled for Electrum seeds", | ||
| show_back_button=False, | ||
| ) | ||
| from seedsigner.views.seed_views import SeedMnemonicEntryView | ||
| self.controller.storage.init_pending_mnemonic(num_words=12, is_electrum=True) | ||
| return Destination(SeedMnemonicEntryView) |
There was a problem hiding this comment.
I think you meant this to direct to the new SeedElectrumMnemonicStartView since removed the warning screen above
There was a problem hiding this comment.
from seedsigner.views.seed_views import SeedElectrumMnemonicStartView
return Destination(SeedElectrumMnemonicStartView)
src/seedsigner/views/tools_views.py
Outdated
| self.controller.storage.init_pending_mnemonic(num_words=12, is_electrum=True) | ||
| return Destination(SeedMnemonicEntryView) | ||
| from seedsigner.views.seed_views import SeedElectrumWarningView | ||
| return Destination(SeedElectrumWarningView) |
There was a problem hiding this comment.
I believe you meant to call SeedElectrumMnemonicStartView here, perhaps renamed it in the middle of the refactor
There was a problem hiding this comment.
This is what I get for being too dependent on VSCode to just magically get things right...
(...okay, and for not having enough FlowTest scenarios to catch things!)
|
As of f06666e ACK tested Everything is working for me:
|
Ooh, I should add the "custom extension" wrinkle to the FlowTest. |
courtesy of @newtonick!
kdmukai
force-pushed
the
electrum_seed_cleanup
branch
from
c43d4c6
to
134476f
Compare
|
I've retested this w/ a modified version on top of 6d114f7:
w/ that said, I repeat my ACK tested from earlier today w/ same manual tests performed. |
src/seedsigner/views/psbt_views.py
Outdated
| status_headline=None, | ||
| text=f"Some features disabled for Electrum seeds", | ||
| show_back_button=False, | ||
| ) | ||
| from seedsigner.views.seed_views import SeedMnemonicEntryView | ||
| self.controller.storage.init_pending_mnemonic(num_words=12, is_electrum=True) | ||
| return Destination(SeedMnemonicEntryView) |
There was a problem hiding this comment.
from seedsigner.views.seed_views import SeedElectrumMnemonicStartView
return Destination(SeedElectrumMnemonicStartView)
|
tACK |
Description
Light refactor of #513:
Viewshould only ever invoke a singleScreen. Pulled the Electrum seed warning screen into its ownView. Also gain some modest DRY efficiency and easier future maintenance this way.SettingsEntryfor the Electrum seed option used adisplay_namevalue that was way too long to fit in the button list (Settings -> Advanced) that would leave a rendering artifact on the far right edge (scroll past the Electrum option to the bottom of the list).ElectrumSeedmnemonic validation.Housekeeping, more robust testing:
ElectrumSeed.override_script.ElectrumSeed:Reversions:
SeedQREncoderthat were probably added at an earlier phase when Electrum seed + SeedQR was being considered. Downstream effect of this reversion includesseed_views.py,test_encodepsbtqr.py,test_seedqr.py.Misc:
boolreturn fromSeed._generate_seedas it is never used.Seed:wallet_type->sig_type; the term "wallet" is just too overused and poorly specified in bitcoin. Note:embit_utilsdoes still usewallet_typebut in order to keep this ever-larger PR in check and since it's older, existing code, I left that refactor for another day.Settings list issue before the fix in this PR:
After:
New/Updated screenshots:
After moving part of the long settings name into
help_text:This pull request is categorized as a:
Checklist
pytestand made sure all unit tests pass before sumbitting the PRIf you modified or added functionality/workflow, did you add new unit tests?
I have tested this PR on the following platforms/os:
Note: Keep your changes limited in scope; if you uncover other issues or improvements along the way, ideally submit those as a separate PR. The more complicated the PR the harder to review, test, and merge.