refactor: better handling signer type #17
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the wallet signer architecture to provide better type safety and separation between browser and ledger wallet types. The refactoring moves signer-related domain types to a dedicated src/domain/signer.ts file and updates the type system to distinguish between BrowserWalletAccount (address-only) and LedgerWalletAccount (address + id).
Changes:
- Introduced separate
BrowserSignerandLedgerSignertypes with corresponding wallet account types that have different fields - Moved error classes (
SignTransactionError,SwitchAccountError,SwitchChainError) fromdomain/wallettodomain/signer - Refactored
signTransactionto no longer require an explicitaccountparameter, retrieving it from internal state instead
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/domain/signer.ts | New file containing signer types, wallet account schemas, and error classes with discriminated unions for browser vs ledger |
| src/domain/wallet.ts | Updated to use signer types from domain/signer, refined wallet type definitions with proper type narrowing helpers |
| src/domain/transactions.ts | Removed optional fields (maxFeePerGas, maxPriorityFeePerGas, nonce) from EvmTx schema |
| src/domain/tokens.ts | New file with token utilities (unrelated to signer refactoring) |
| src/services/wallet/signer.ts | Simplified to a thin Context.Tag wrapper for the Signer type |
| src/services/wallet/browser-signer.ts | Updated to use BrowserWalletAccount and new SignerService, removed account parameter from signTransaction |
| src/services/wallet/ledger-signer/index.ts | Updated to use LedgerWalletAccount and new SignerService, signTransaction now retrieves account from state |
| src/services/wallet/wallet-service.ts | Refactored wallet state matching to use discriminated union types with Match.exhaustive |
| src/atoms/wallet-atom.ts | Split switchAccount atom into browser and ledger variants, updated wallet comparison logic |
| src/components/molecules/address-switcher.tsx | Refactored to handle browser and ledger accounts separately with dedicated list components |
| src/components/modules/Account/Deposit/state.tsx | Updated to use new token utilities and improved percentage calculation handling |
| src/components/modules/Account/Withdraw/state.tsx | Improved percentage calculation with proper 100% handling and precision control |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Philippoes
approved these changes
Jan 30, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.