Skip to content

Conversation

ekzyis
Copy link
Member

@ekzyis ekzyis commented Sep 23, 2025

Description

supersedes #2546 #2552 #2553 #2554

see description in individual PRs + commit messages

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

Refactors wallet architecture to a hook-based provider, extracts crypto utilities, removes v1 migration and old storage, and updates imports/components accordingly.

  • Wallets Architecture:
    • Migrate context/provider/reducer into hooks/global with WalletsProvider, state/actions, and always-on hooks (useServerWallets, automated retries, key init, old storage cleanup).
    • Remove legacy context files in wallets/client/context/* and v1→v2 migration code.
  • Crypto:
    • Extract key/crypto helpers to wallets/lib/crypto (deriveKey, encrypt, decrypt, generateRandomPassphrase).
    • Simplify crypto hooks and split passphrase flows into hooks/passphrase.
    • On key init, delete old IndexedDB and purge legacy localStorage wallets.
  • Queries/Hooks:
    • Update hooks/query to use new global/crypto/logger hooks; drop migration mutation; minor import/logic cleanup.
    • Adjust other hooks (indicator, wallet, payment, diagnostics) to new paths.
  • Components/Exports:
    • Rename components/draggable to components/dnd and re-export via components/index.
    • Update components/dnd.js to use useDndHandlers from new hooks.
  • Pages:
    • Update imports in pages/_app.js and pages/wallets/index.js to use wallets/client/hooks and new APIs.

Written by Cursor Bugbot for commit 983a638. 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 2 times, most recently from 99e452a to da6434d Compare September 28, 2025 14:43
* 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 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
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