Skip to content

Conversation

@ekzyis
Copy link
Member

@ekzyis ekzyis commented Sep 23, 2025

Description

supersedes #2546 (merged into master already in #2585) #2553 (merged already in #2584) #2552 #2554

#2552 (comment):

It has been 2 months since we released #2169.

I think we can now remove the code to migrate wallets that are still stored locally.

Instead, we just delete them now and also delete the old IndexedDB.

#2554 (comment):

This PR is limited to moving files or functions around to make more sense:

  • moved wallets/client/context into ../hooks
    • distinction didn't make sense
  • hooks/ only contain hooks (function name must starts with use)
    • moved pure functions to wallets/lib/crypto
    • context providers are the exception
    • moved passphrase hooks into own file

Additional context

For me personally, it is definitely better to just have one big PR because I know what I did and then I can resolve conflicts between commits immediately. If I want to use the web interface to review and document my changes, I can also push to my own repository and do it there.

The small PRs were mostly meant to make review easier, but since that is apparently not really the case, I now squashed every individual PR into one commit that I added here.

I hope that is better.

Checklist

Are your changes backward compatible? Please answer below:

yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:

tested PRs individually before

For frontend changes: Tested on mobile, light and dark mode? Please answer below:

n/a

Did you introduce any new environment variables? If so, call them out explicitly here:

no

Did you use AI for this? If so, how much did it assist you?

no


Note

Reorganizes wallets by moving provider/reducer into hooks, removing v1→v2 client migration, extracting crypto utilities, renaming DnD component, and updating imports/usages across the app.

  • Wallets Architecture:
    • Global Hooks: Move WalletsProvider, state, and actions into wallets/client/hooks/global.js; remove wallets/client/context/* (provider/reducer) and update all imports to @/wallets/client/hooks.
    • Migration Removal: Delete client-side migration logic; add cleanup of old localStorage wallets and old IndexedDB via useDeleteLocalWallets and useDeleteOldDb.
    • Key Handling: Initialize and validate keys in global.js; emit KEY_STORAGE_UNAVAILABLE, WRONG_KEY, KEY_MATCH actions.
  • Crypto:
    • Extract deriveKey, encrypt, decrypt, generateRandomPassphrase to wallets/lib/crypto.
    • Split passphrase flows into wallets/client/hooks/passphrase.js; keep encryption/decryption/key APIs in wallets/client/hooks/crypto.js.
  • Components & Hooks:
    • Rename components/draggable to components/dnd and update re-exports/consumers.
    • Update various hooks (indicator, diagnostics, payment, query, wallet) to use new global/crypto/logger/query modules and simplified imports.
  • Pages:
    • Use WalletsProvider from @/wallets/client/hooks in pages/_app.js and pages/wallets/index.js; wire passphrase prompt and DnD reorder via new hooks.

Written by Cursor Bugbot for commit e56a914. This will update automatically on new commits. Configure here.

@huumn
Copy link
Member

huumn commented Sep 23, 2025

Just to be clear: not preferring 5 one-line PRs modifying the same 10 lines of code, all so closely related to each other that they practically conflict on all lines when merged out of order, does not mean we want big vague PRs. We're looking for goldilocks PRs - not as small as possible but also not a PR that's like here's everything I did this week.

Given that you "own" the wallet code, this PR is fine by me, but you didn't need to do this retroactively.

I know you understand this, but I feel like you're being kind of rigid, trying to treat PR size like a bunch of fixed rules, an algorithm, rather than an also vibes thing. It's as much an art as it is a science.

@ekzyis ekzyis force-pushed the wallets branch 2 times, most recently from e299459 to 5e122a6 Compare September 28, 2025 09:14
cursor[bot]

This comment was marked as outdated.

@ekzyis ekzyis force-pushed the wallets branch 3 times, most recently from da6434d to 983a638 Compare September 28, 2025 15:04
@ekzyis ekzyis changed the title Wallet improvements Remove client-side migration and reorganize wall Sep 29, 2025
@ekzyis ekzyis changed the title Remove client-side migration and reorganize wall Remove client-side migration and reorganize wallets Sep 29, 2025
ekzyis added 2 commits October 3, 2025 19:07
* Move passphrase hooks into own file
* Rename to draggable.js to dnd.js
* Create wallets/lib/crypto.js
* Move wallets/client/context stuff into ../hooks

The distinction between context and hooks did not make sense.

* Fix circular imports within wallets/client/hooks
@ekzyis
Copy link
Member Author

ekzyis commented Oct 11, 2025

(I updated the description.)

@huumn can you review or just merge this?

I moved files around in this PR and this creates conflicts with basically any wallet change that I want to make in a separate PR that is not new code in new files like in #2575

@huumn huumn merged commit 9e43efc into master Oct 11, 2025
7 checks passed
@huumn huumn deleted the wallets branch October 11, 2025 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants