Skip to content

[Refactor] use Seed instead of seed_num - #810

Merged
newtonick merged 2 commits into
SeedSigner:devfrom
Chaitanya-Keyal:refactor/seed_num-to-seed
Jul 22, 2026
Merged

[Refactor] use Seed instead of seed_num#810
newtonick merged 2 commits into
SeedSigner:devfrom
Chaitanya-Keyal:refactor/seed_num-to-seed

Conversation

@Chaitanya-Keyal

Copy link
Copy Markdown
Contributor

Description

This PR refactors the seed views to pass the selected Seed object directly to the next view instead of using seed_num.

Previously, almost every view would immediately dereference seed_num to fetch the Seed object from SeedStorage, resulting in redundant lookups. Passing the Seed object itself removes that redundancy and provides a more robust reference to the selected seed, avoiding potential fragility in the future.

The motivation for this change came up while I was working on #785. In that context, it became clear that relying on seed_num is brittle, since it’s only the index of a mutable list. See my comment for details.

Benefits:

  • Removes redundant lookups in views
  • More robust references to seeds
  • Safer against future changes in seed storage (e.g., reordering, new seed types(?))

This pull request is categorized as a:

  • Code refactor

Checklist

  • I’ve run pytest and made sure all unit tests pass before submitting 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:

@Chaitanya-Keyal
Chaitanya-Keyal force-pushed the refactor/seed_num-to-seed branch from a1357c3 to 7d20f80 Compare October 23, 2025 17:51
@Chaitanya-Keyal
Chaitanya-Keyal force-pushed the refactor/seed_num-to-seed branch 2 times, most recently from 52b74ca to 78982eb Compare December 16, 2025 04:10
@newtonick newtonick added this to the 0.9.0 milestone Dec 16, 2025
@newtonick newtonick moved this to 0.9.0 Needs Code Review in @SeedSigner Development Board Dec 16, 2025
@Chaitanya-Keyal
Chaitanya-Keyal force-pushed the refactor/seed_num-to-seed branch from 78982eb to 63327a0 Compare January 20, 2026 09:37
@Chaitanya-Keyal
Chaitanya-Keyal force-pushed the refactor/seed_num-to-seed branch from 63327a0 to 8c61c66 Compare February 16, 2026 20:52
@Chaitanya-Keyal
Chaitanya-Keyal force-pushed the refactor/seed_num-to-seed branch from 8c61c66 to 65e312c Compare July 21, 2026 11:53
Comment thread src/seedsigner/controller.py
@newtonick

Copy link
Copy Markdown
Collaborator

concept ACK. Spent some time reviewing. Only found the one issue above.

Co-authored-by: Nick Klockenga <127377+newtonick@users.noreply.github.com>
@newtonick

newtonick commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

@Chaitanya-Keyal when/if I merge this it's going to create a bunch of different merge conflicts for different PRs. One of them being #785. Might not be trivial to resolve but I think merging this one first is the right move. Do you agree?

@Chaitanya-Keyal

Chaitanya-Keyal commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Yeah, that makes sense. This PR touches basically every view, so any PR working on a view would mostly get a conflict. It wouldn't be trivial to resolve but still should be straightforward. Additionally, any PR with new views should then be updated to follow this new pattern.

And yes, once this is merged I will rebase #785, it'll allow me to cleanup a bunch of the code in that PR as well.

@newtonick
newtonick merged commit 5c748cc into SeedSigner:dev Jul 22, 2026
2 checks passed
@Chaitanya-Keyal
Chaitanya-Keyal deleted the refactor/seed_num-to-seed branch July 22, 2026 03:52
kdmukAI-bot added a commit to kdmukAI-bot/seedsigner that referenced this pull request Jul 25, 2026
Reconcile the LVGL native views + controller with upstream/dev's seed_num->Seed refactor (SeedSigner#810) after moving the stack base from 0.8.7-78 onto upstream/dev @ 1fb2956:

- controller.discard_seed: stringize the Seed annotation (the module drops the runtime Seed import at the defer-imports layer, so a bare annotation NameErrors)
- SeedTranscribeSeedQRFormatView: drop a dangling seed_screens import the native migration had already replaced with a string-id screen call
- ToolsAddressExplorerAddressTypeView: self.seed_num -> self.seed (__init__ now takes the Seed object; seed_num was never set)
- test_flows_seed: capture the Seed returned by finalize_pending_seed()

Co-Authored-By: kdmukai <934746+kdmukai@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 0.9.0 Needs Code Review

Development

Successfully merging this pull request may close these issues.

2 participants