feat: many many trezor pubKey skip derivation bits#11159
Merged
gomesalexandre merged 10 commits intodevelopfrom Nov 25, 2025
Merged
feat: many many trezor pubKey skip derivation bits#11159gomesalexandre merged 10 commits intodevelopfrom
gomesalexandre merged 10 commits intodevelopfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Hook as Hook (e.g., useTradeExecution)
participant Adapter as Adapter (UTXO/EVM)
participant Wallet
rect rgb(220,240,255)
Note right of User: Legacy flow (no pubKey)
User->>Hook: invoke send/approve (wallet, accountId)
Hook->>Adapter: buildSendTransaction(..., accountNumber, ...)
Adapter->>Wallet: getPublicKey(wallet, accountNumber)
Wallet-->>Adapter: xpub
Adapter->>Adapter: build & sign tx
Adapter-->>Hook: txResult
end
rect rgb(240,255,240)
Note right of User: New flow (pubKey provided for Trezor)
User->>Hook: invoke send/approve (wallet, accountId)
Hook->>Hook: if isTrezor(wallet) -> pubKey = fromAccountId(accountId).account
Hook->>Adapter: buildSendTransaction(..., accountNumber, pubKey)
alt pubKey provided
Adapter->>Adapter: use provided pubKey (bypass wallet)
else
Adapter->>Wallet: getPublicKey(...)
Wallet-->>Adapter: xpub
end
Adapter->>Adapter: build & sign tx
Adapter-->>Hook: txResult
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gomesalexandre
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Modals/Send/utils.ts (1)
198-223: Avoid logging UTXO pubKey/change address in production codeIn both the UTXO send path and
maybeFetchChangeAddress, we now log account‑level data:
console.log('UTXO buildSendTransaction pubKey:', { isTrezor: isTrezor(wallet), utxoPubKey })console.log('maybe fetch change addy')console.log('fetching change addy')console.log('fetched change addy:', changeAddress)Given
utxoPubKeyis an xpub derived fromfromAccountId(sendInput.accountId).accountandchangeAddressis a concrete receive address, these logs:
- Leak wallet‑level identifiers into the browser console unnecessarily.
- Add noise for end users and automated environments.
The functional behavior doesn’t depend on them, so it’s safer to drop them before merging:
- const utxoPubKey = isTrezor(wallet) ? fromAccountId(sendInput.accountId).account : undefined - console.log('UTXO buildSendTransaction pubKey:', { isTrezor: isTrezor(wallet), utxoPubKey }) + const utxoPubKey = isTrezor(wallet) ? fromAccountId(sendInput.accountId).account : undefined @@ - console.log('maybe fetch change addy') - // Only fetch for UTXO chains on Ledger/Trezor wallets - if (!isUtxoChainId(chainId) || !(isLedger(wallet) || isTrezor(wallet))) return undefined - console.log('fetching change addy') + // Only fetch for UTXO chains on Ledger/Trezor wallets + if (!isUtxoChainId(chainId) || !(isLedger(wallet) || isTrezor(wallet))) return undefined @@ - const changeAddress = await adapter.getAddress({ + const changeAddress = await adapter.getAddress({ accountNumber: accountMetadata.bip44Params.accountNumber, accountType: accountMetadata.accountType, wallet, isChange: true, pubKey: isTrezor(wallet) ? fromAccountId(sendInput.accountId).account : undefined, }) - console.log('fetched change addy:', changeAddress)The error path already uses
console.error, which aligns with existing logging preferences.Also applies to: 373-385
🧹 Nitpick comments (3)
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeNetworkFeeCryptoBaseUnit.tsx (1)
93-96: ReusepubKeyfor UTXO xpub and simplify Trezor checkThe UTXO block currently recomputes
fromAccountId(sellAssetAccountId).accountforxpubeven though you already derivedpubKeyabove, and thewallet &&guard is redundant inside thewallet && …-gated queryFn.You could tighten this without changing behavior:
const pubKey = fromAccountId(sellAssetAccountId).account // … const xpub = isTrezor(wallet) ? pubKey : (await adapter.getPublicKey(wallet, accountNumber, accountType)).xpub const _senderAddress = await adapter.getAddress({ accountNumber, accountType, wallet, ...(isLedger(wallet) || isTrezor(wallet) ? { pubKey } : {}), })This keeps the Trezor behavior the same while avoiding duplicate CAIP parsing and extra null checks.
Also applies to: 120-129, 133-134
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx (1)
294-301: TrezorpubKeypropagation across EVM/UTXO/Solana looks consistent; consider de‑duping CAIP parsingThe new Trezor handling in CowSwap, generic EVM, UTXO, and Solana branches all consistently:
- Use
fromAccountId(sellAssetAccountId).accountas the Trezor account key/xpub.- Bypass
getPublicKeyfor UTXO when on Trezor.- Pass
pubKeyintogetAddresswhere supported.This matches the PR’s intent and looks correct.
To reduce repetition and keep the Trezor behavior single-sourced, you could derive once after the
sellAssetAccountIdguard:if (!sellAssetAccountId) throw Error('missing sellAssetAccountId') const trezorPubKey = wallet && isTrezor(wallet) ? fromAccountId(sellAssetAccountId).account : undefined // … const from = await adapter.getAddress({ accountNumber, wallet, pubKey: trezorPubKey, }) // … const xpub = trezorPubKey ?? (await adapter.getPublicKey(wallet, accountNumber, accountType)).xpub const senderAddress = await adapter.getAddress({ accountNumber, accountType, wallet, pubKey: trezorPubKey, })Same idea could apply to the Solana and CowSwap EVM branches to avoid multiple
fromAccountIdcalls.Also applies to: 339-347, 375-389, 440-445
packages/chain-adapters/src/utxo/UtxoBaseAdapter.ts (1)
392-405: UTXObuildSendTransactionpubKey override looks correct; document expected formatThe new
pubKeyshort‑circuit cleanly preserves existing behavior for callers that don’t pass it, while allowing Trezor (and similar) flows to bypassgetPublicKey:const xpub = pubKey ?? (await this.getPublicKey(wallet, accountNumber, chainSpecific.accountType)).xpub const txToSign = await this.buildSendApiTransaction({ ...input, xpub })Given this method now trusts a caller‑supplied
pubKeyto be the canonical xpub used by unchained, it would be helpful to:
- Clarify in the
BuildSendTxInput<T>docs/type comment that, for UTXO chains,pubKeymust be the correctly versioned account xpub corresponding tochainSpecific.accountType.That should prevent accidental misuse (e.g., passing an address instead of an xpub) as this API gets reused elsewhere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
packages/chain-adapters/src/utxo/UtxoBaseAdapter.ts(1 hunks)src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsx(2 hunks)src/components/Modals/Send/utils.ts(3 hunks)src/components/Modals/Send/views/Confirm.tsx(2 hunks)src/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsx(3 hunks)src/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx(3 hunks)src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx(2 hunks)src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeNetworkFeeCryptoBaseUnit.tsx(1 hunks)src/features/defi/providers/fox-farming/hooks/useFoxFarming.ts(4 hunks)src/lib/utils/evm/approve.ts(2 hunks)src/lib/utils/evm/index.ts(2 hunks)src/lib/utils/evm/types.ts(1 hunks)src/react-queries/queries/mutations.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (32)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:27.004Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10569
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectModalSigningFooter.tsx:121-129
Timestamp: 2025-09-17T22:40:30.149Z
Learning: gomesalexandre maintains strict scope discipline even for style/UI PRs in shapeshift/web, declining functionally correct UX improvements (like keeping Cancel button enabled during gas simulation loading) when they fall outside the PR's stated styling objectives, demonstrating his consistent pattern of deferring valid but tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10566
File: src/hooks/useLedgerAccountGuard/useLedgerAccountGuard.tsx:4-4
Timestamp: 2025-09-23T10:36:13.916Z
Learning: gomesalexandre dismisses suggestions to expand PR scope beyond the current migration when the PR is already systematically implementing the desired pattern (like KeyManager.Ledger migration) within focused boundaries, preferring to keep scope contained rather than doing comprehensive codebase-wide cleanups.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:0-0
Timestamp: 2025-09-12T11:56:19.437Z
Learning: gomesalexandre rejected verbose try/catch error handling for address validation in Tenderly integration (PR #10461), calling the approach "ugly" but still implemented safety measures in commit ad7e424b89, preferring cleaner safety implementations over defensive programming patterns.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10418
File: src/Routes/RoutesCommon.tsx:231-267
Timestamp: 2025-09-03T21:17:27.699Z
Learning: gomesalexandre prefers to keep PR diffs focused and reasonable in size, deferring tangential improvements (like Mixpanel privacy enhancements) to separate efforts rather than expanding the scope of feature PRs.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10879
File: src/context/WalletProvider/WalletConnectV2/components/WalletConnectDirectRow.tsx:64-81
Timestamp: 2025-10-22T22:11:22.918Z
Learning: In early WalletConnect POC/features behind a flag, gomesalexandre prioritizes connection correctness/stability over UX polish; minimal safety guards (like preventing concurrent connects) are preferred, while visuals will be wired later by reallybeard.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
Learning: gomesalexandre dismissed an aria-label accessibility suggestion with "meh" in PR #10418 for WalletConnectToDappsHeaderButton.tsx, consistent with the team's pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10232
File: packages/unchained-client/openapitools.json:61-61
Timestamp: 2025-08-08T10:23:16.843Z
Learning: In shapeshift/web, for temporary “monkey patch” PRs (e.g., packages/unchained-client/openapitools.json using jsDelivr CDN refs like cosmos/mayachain), gomesalexandre is fine with branch-based URLs and does not want SHA pinning. Treat this as a scoped exception to their general preference for pinned dependencies/refs.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:55-55
Timestamp: 2025-09-10T15:35:46.223Z
Learning: gomesalexandre prefers fail-fast early returns over graceful degradation when critical data is missing in WalletConnect flows (like peer metadata in EIP155SignTypedDataConfirmation.tsx). He favors "safety first, always double-wrap" approach and believes missing peer metadata indicates bigger problems that should be surfaced explicitly rather than masked with partial UI rendering.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/ContractInteractionBreakdown.tsx:0-0
Timestamp: 2025-09-13T16:45:18.813Z
Learning: gomesalexandre prefers aggressively deleting unused/obsolete code files ("ramboing") rather than fixing technical issues in code that won't be used, demonstrating his preference for keeping codebases clean and PR scope focused.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/types.ts:7-7
Timestamp: 2025-09-10T15:34:29.604Z
Learning: gomesalexandre is comfortable relying on transitive dependencies (like abitype through ethers/viem) rather than explicitly declaring them in package.json, preferring to avoid package.json bloat when the transitive dependency approach works reliably in practice.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10503
File: .env:56-56
Timestamp: 2025-09-16T13:17:02.938Z
Learning: gomesalexandre prefers to enable feature flags globally in the base .env file when the intent is to activate features everywhere, even when there are known issues like crashes, demonstrating his preference for intentional global feature rollouts over cautious per-environment enablement.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10249
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:447-503
Timestamp: 2025-08-13T17:07:10.763Z
Learning: gomesalexandre prefers relying on TypeScript's type system for validation rather than adding defensive runtime null checks when types are properly defined. They favor a TypeScript-first approach over defensive programming with runtime validations.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpDepositActionSubscriber.tsx:61-66
Timestamp: 2025-08-14T17:51:47.556Z
Learning: gomesalexandre is not concerned about structured logging and prefers to keep console.error usage as-is rather than implementing structured logging patterns, even when project guidelines suggest otherwise.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10413
File: src/components/Modals/FiatRamps/fiatRampProviders/onramper/utils.ts:29-55
Timestamp: 2025-09-02T14:26:19.028Z
Learning: gomesalexandre prefers to keep preparatory/reference code simple until it's actively consumed, rather than implementing comprehensive error handling, validation, and robustness improvements upfront. They prefer to add these improvements when the code is actually being used in production.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:396-402
Timestamp: 2025-08-14T17:55:57.490Z
Learning: gomesalexandre is comfortable with functions/variables that return undefined or true (tri-state) when only the truthy case matters, preferring to rely on JavaScript's truthy/falsy behavior rather than explicitly returning boolean values.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10783
File: src/context/ModalStackProvider/useModalRegistration.ts:30-41
Timestamp: 2025-10-16T11:14:40.657Z
Learning: gomesalexandre prefers to add lint rules (like typescript-eslint/strict-boolean-expressions for truthiness checks on numbers) to catch common issues project-wide rather than relying on code review to catch them.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
📚 Learning: 2025-10-01T07:42:40.195Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10596
File: src/components/Layout/Header/NavBar/WalletConnectedMenu.tsx:77-99
Timestamp: 2025-10-01T07:42:40.195Z
Learning: In WalletConnectedMenu.tsx's handleReconnectWallet handler, gomesalexandre prefers throwing an error for unsupported wallet types in the default case rather than gracefully handling with a fallback. His reasoning: "if we have a problem here, we have bigger problems" - only supported wallets (KeepKey, Ledger, MetaMask, Coinbase, Phantom) should reach the reconnect flow when disconnected/locked, so encountering an unsupported type indicates a larger architectural issue that should be surfaced explicitly rather than masked with graceful degradation.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsxsrc/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-10-21T17:11:18.087Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:11:18.087Z
Learning: In src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx, within the handleInputChange function, use .toFixed() without arguments (not .toString()) when converting BigNumber amounts for input field synchronization. This avoids exponential notation in the input while preserving precision for presentational components like <Amount.Crypto /> and <Amount.Fiat /> to format appropriately.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsxsrc/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeNetworkFeeCryptoBaseUnit.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsxsrc/components/Modals/Send/views/Confirm.tsxsrc/components/Modals/Send/utils.ts
📚 Learning: 2025-11-19T16:59:50.569Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11012
File: src/context/WalletProvider/Vultisig/components/Connect.tsx:24-59
Timestamp: 2025-11-19T16:59:50.569Z
Learning: In src/context/WalletProvider/*/components/Connect.tsx files across the ShapeShift web codebase, the established pattern for handling null/undefined adapter from getAdapter() is to simply check `if (adapter) { ... }` without an else clause. All wallet Connect components (Coinbase, Keplr, Phantom, Ledger, MetaMask, WalletConnectV2, KeepKey, Vultisig) follow this pattern—they reset loading state after the if block but do not show error messages when adapter is null. This is an intentional design decision and should be maintained for consistency.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsxsrc/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeNetworkFeeCryptoBaseUnit.tsxpackages/chain-adapters/src/utxo/UtxoBaseAdapter.tssrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsxsrc/components/Modals/Send/views/Confirm.tsxsrc/components/Modals/Send/utils.ts
📚 Learning: 2025-08-29T07:07:49.332Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10386
File: src/components/MultiHopTrade/components/VerifyAddresses/VerifyAddresses.tsx:272-294
Timestamp: 2025-08-29T07:07:49.332Z
Learning: In UTXO sell address verification flow in VerifyAddresses.tsx, the user wants address verification to be marked as "verified/complete" before starting the change address fetch, not after. The verification step and change address fetch should be treated as separate sequential operations in the UI flow.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/Modals/Send/utils.ts
📚 Learning: 2025-11-20T12:00:44.965Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11078
File: src/setupVitest.ts:11-15
Timestamp: 2025-11-20T12:00:44.965Z
Learning: In shapeshift/web, src/setupVitest.ts must redirect 'ethers' to 'ethers5' for shapeshiftoss/hdwallet-trezor (and -trezor-connect), same as ledger and shapeshift-multichain. Removing 'trezor' from the regex causes CI/Vitest failures due to ethers v6 vs v5 API differences.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsxsrc/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsxsrc/components/Modals/Send/views/Confirm.tsxsrc/features/defi/providers/fox-farming/hooks/useFoxFarming.tssrc/components/Modals/Send/utils.ts
📚 Learning: 2025-09-12T12:00:33.924Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.924Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns in WalletConnect modals, including side-effects-during-render for error handling (showErrorToast + handleReject), rather than introducing isolated refactors that would make the codebase inconsistent.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsxsrc/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-09-12T12:00:33.924Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.924Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns across WalletConnect modal components, including side-effects-during-render for error handling (showErrorToast + handleReject calls before return null), rather than introducing isolated refactors that would create inconsistency in the codebase.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsxsrc/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-08-22T12:58:26.590Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/GenericTransactionActionCard.tsx:108-111
Timestamp: 2025-08-22T12:58:26.590Z
Learning: In the RFOX GenericTransactionDisplayType flow in src/components/Layout/Header/ActionCenter/components/GenericTransactionActionCard.tsx, the txHash is always guaranteed to be present according to NeOMakinG, so defensive null checks for txLink are not needed in this context.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsx
📚 Learning: 2025-09-12T11:52:39.280Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx:18-21
Timestamp: 2025-09-12T11:52:39.280Z
Learning: In WalletConnect dApps integration, gomesalexandre has implemented intentional routing logic where EIP155TransactionConfirmation is typed for EthSignTransactionCallRequest only, while a separate SendTransactionConfirmation component handles EthSendTransactionCallRequest. The WalletConnectModalManager contains conditional logic to route native send transactions to SendTransactionConfirmation and other transaction types to EIP155TransactionConfirmation, creating a clean separation of concerns between signing and sending flows.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsxsrc/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-08-05T23:36:13.214Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10187
File: src/state/slices/preferencesSlice/selectors.ts:21-25
Timestamp: 2025-08-05T23:36:13.214Z
Learning: The AssetId type from 'shapeshiftoss/caip' package is a string type alias, so it can be used directly as a return type for cache key resolvers in re-reselect selectors without needing explicit string conversion.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsxsrc/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-10-07T03:44:27.350Z
Learnt from: 0xApotheosis
Repo: shapeshift/web PR: 10760
File: src/components/ManageHiddenAssets/ManageHiddenAssetsList.tsx:78-84
Timestamp: 2025-10-07T03:44:27.350Z
Learning: In the ShapeShift web codebase, the following are stable references and do not need to be included in useCallback/useMemo dependency arrays:
- `navigate` from `useBrowserRouter()` hook
- Modal control objects (like `walletDrawer`) from `useModal()` hook (including their `isOpen`, `close`, and `open` methods)
- These are backed by stable context providers
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsxsrc/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsxsrc/components/Modals/Send/views/Confirm.tsxsrc/features/defi/providers/fox-farming/hooks/useFoxFarming.ts
📚 Learning: 2025-09-04T17:29:59.479Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx:28-33
Timestamp: 2025-09-04T17:29:59.479Z
Learning: In shapeshift/web, the useGetPopularAssetsQuery function in src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx intentionally uses primaryAssets[assetId] instead of falling back to assets[assetId]. The design distributes primary assets across chains by iterating through their related assets and adding the primary asset to each related asset's chain. This ensures primary assets appear in all chains where they have related assets, supporting the grouped asset system.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx
📚 Learning: 2025-08-05T22:41:35.473Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10187
File: src/pages/Assets/Asset.tsx:1-1
Timestamp: 2025-08-05T22:41:35.473Z
Learning: In the shapeshift/web codebase, component imports use direct file paths like '@/components/ComponentName/ComponentName' rather than barrel exports. The AssetAccountDetails component should be imported as '@/components/AssetAccountDetails/AssetAccountDetails', not from a directory index.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsx
📚 Learning: 2025-11-05T23:37:30.632Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10912
File: src/context/WalletProvider/NewWalletViews/NewWalletViewsSwitch.tsx:290-290
Timestamp: 2025-11-05T23:37:30.632Z
Learning: In the ShapeShift web codebase, `isMobile` imported from '@/lib/globals' is a module-level constant (defined as `export const isMobile = Boolean(window?.isShapeShiftMobile)`) that is evaluated once at module load time. It is a stable reference that does not need to be included in useCallback/useMemo/useEffect dependency arrays.
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsxsrc/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsxsrc/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-11-19T22:20:25.661Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10767
File: package.json:324-324
Timestamp: 2025-11-19T22:20:25.661Z
Learning: In shapeshift/web package.json, the resolution "gridplus-sdk/bs58check": "2.1.2" is intentional and must not be removed. It forces gridplus-sdk's transitive bs58check dependency from 4.0.0 down to 2.1.2 because bs58check 4.0.0 breaks legacy address validation (due to bs58 v6.0.0 and noble/hash vs 2.1.2's bs58 v4.0.0 and create-hash).
Applied to files:
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsx
📚 Learning: 2025-07-29T15:04:28.083Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10139
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx:109-115
Timestamp: 2025-07-29T15:04:28.083Z
Learning: In src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx, the component is used under an umbrella that 100% of the time contains the quote, making the type assertion `activeTradeQuote?.steps[currentHopIndex] as TradeQuoteStep` safe. Adding conditional returns before hooks would violate React's Rules of Hooks.
Applied to files:
src/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeNetworkFeeCryptoBaseUnit.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsxsrc/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-08-08T11:40:55.734Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx:41-41
Timestamp: 2025-08-08T11:40:55.734Z
Learning: In MultiHopTrade confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx and related hooks), there is only one active trade per flow. Because of this, persistent (module/Redux) dedupe for QuotesReceived in useTrackTradeQuotes is not necessary; the existing ref-based dedupe is acceptable.
Applied to files:
src/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeNetworkFeeCryptoBaseUnit.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx
📚 Learning: 2025-08-08T11:41:36.971Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:88-109
Timestamp: 2025-08-08T11:41:36.971Z
Learning: In MultiHopTrade Confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx), the Confirm route does not remount; navigating away goes to the swapper input page. Therefore, persistent deduplication across remounts for quote tracking is unnecessary; a ref-based single-mount dedupe is sufficient.
Applied to files:
src/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeNetworkFeeCryptoBaseUnit.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsxsrc/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-08-22T13:02:58.824Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/pages/RFOX/hooks/useRfoxRewardDistributionActionSubscriber.tsx:33-41
Timestamp: 2025-08-22T13:02:58.824Z
Learning: In src/pages/RFOX/hooks/useRfoxRewardDistributionActionSubscriber.tsx, NeOMakinG declined optimizing useMemo dependencies to depend on lifetimeRewardDistributionsQuery.data instead of the entire query object, indicating indifference toward this type of performance optimization.
Applied to files:
src/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsxsrc/features/defi/providers/fox-farming/hooks/useFoxFarming.ts
📚 Learning: 2025-11-03T22:31:30.786Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10985
File: packages/swapper/src/swappers/PortalsSwapper/getPortalsTradeQuote/getPortalsTradeQuote.ts:0-0
Timestamp: 2025-11-03T22:31:30.786Z
Learning: In packages/swapper/src/swappers/PortalsSwapper, the rate and quote files intentionally use different approaches for calculating buyAmountBeforeSlippageCryptoBaseUnit: getPortalsTradeRate.tsx uses minOutputAmount / (1 - buffer) for conservative estimates, while getPortalsTradeQuote.ts uses outputAmount / (1 - buffer) for final quote display. This difference is validated by on-chain simulation testing and is intentional.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeNetworkFeeCryptoBaseUnit.tsx
📚 Learning: 2025-08-04T16:02:27.360Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10171
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandedStepperSteps.tsx:458-458
Timestamp: 2025-08-04T16:02:27.360Z
Learning: In multi-hop swap transactions, last hop sell transactions might not be detected by the swapper (unlike buy transactions which are always known immediately). The conditional stepSource logic for last hop buy transactions (`isLastHopSellTxSeen ? stepSource : undefined`) serves as defensive programming for future multi-hop support with intermediate chains, even though multi-hop functionality is not currently supported in production.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx
📚 Learning: 2025-08-10T21:09:25.643Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10215
File: src/components/MultiHopTrade/hooks/useGetTradeRateInput.ts:65-67
Timestamp: 2025-08-10T21:09:25.643Z
Learning: In the MultiHopTrade components, `selectInputBuyAsset` and `selectInputSellAsset` selectors from `tradeInputSlice` always return defined values because they have default values in the initial state (BTC for buyAsset, ETH for sellAsset, with fallback to defaultAsset). Null checks for these assets are unnecessary when using these selectors.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx
📚 Learning: 2025-11-12T12:49:17.895Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11016
File: packages/swapper/src/swappers/NearIntentsSwapper/swapperApi/getTradeQuote.ts:109-125
Timestamp: 2025-11-12T12:49:17.895Z
Learning: In packages/chain-adapters/src/evm/utils.ts, the getErc20Data function already includes a guard that returns an empty string when contractAddress is undefined (line 8: `if (!contractAddress) return ''`). This built-in handling means callers don't need to conditionally invoke getErc20Data—it safely handles both ERC20 tokens and native assets.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/features/defi/providers/fox-farming/hooks/useFoxFarming.ts
📚 Learning: 2025-08-04T15:36:25.122Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10171
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandedStepperSteps.tsx:458-458
Timestamp: 2025-08-04T15:36:25.122Z
Learning: In swap transaction handling, buy transaction hashes should always use the swapper's explorer (stepSource) because they are known by the swapper immediately upon swap execution. The conditional logic for using default explorers applies primarily to sell transactions which need to be detected/indexed by external systems like Thorchain or ViewBlock.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx
📚 Learning: 2025-08-11T09:46:41.060Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10219
File: src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx:167-172
Timestamp: 2025-08-11T09:46:41.060Z
Learning: In the shapeshift/web repository, the display cache logic for trade quotes (using `selectUserAvailableTradeQuotes` and `selectUserUnavailableTradeQuotes`) is intentionally kept the same between `TradeInput.tsx` and `TradeQuotes.tsx` components. The `hasQuotes` computation in `TradeInput.tsx` uses these display cache selectors by design, matching the pattern used in `TradeQuotes.tsx`.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx
📚 Learning: 2025-09-04T12:16:47.748Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10430
File: src/components/Layout/Header/NavBar/PopoverWallet.tsx:72-94
Timestamp: 2025-09-04T12:16:47.748Z
Learning: gomesalexandre declined to add error boundaries to the PopoverWallet component in src/components/Layout/Header/NavBar/PopoverWallet.tsx, stating he didn't touch this component and preferring not to expand the scope of the PR with error boundary additions.
Applied to files:
src/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-08-21T22:15:25.918Z
Learnt from: kaladinlight
Repo: shapeshift/web PR: 10326
File: src/hooks/useActionCenterSubscribers/useFetchBasePortfolio.ts:31-59
Timestamp: 2025-08-21T22:15:25.918Z
Learning: In the ShapeShift web codebase, the maintainer prefers inferred return types for React hooks over explicit return types, particularly for complex hooks that can have intricate return signatures.
Applied to files:
src/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-07-29T10:27:23.424Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10128
File: .cursor/rules/react-best-practices.mdc:8-14
Timestamp: 2025-07-29T10:27:23.424Z
Learning: The ShapeShift team practices aggressive memoization in React components as documented in .cursor/rules/react-best-practices.mdc. They use useMemo for all transformations, derived values, and conditional values, and useCallback for all event handlers and functions that could be passed as props. This approach was adopted after experiencing performance issues ("had hard time") and is their current established practice, though they acknowledge it may evolve in the future.
Applied to files:
src/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-10-15T15:57:39.956Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10810
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:212-0
Timestamp: 2025-10-15T15:57:39.956Z
Learning: gomesalexandre uses discriminated union patterns (e.g., `isEIP1559 ? { max_fee_per_gas, max_priority_fee_per_gas } : { gas_price }`) in WalletConnect flows without additional validation guards, trusting that the runtime data structure ensures mutual exclusivity between EIP-1559 and legacy gas pricing fields.
Applied to files:
src/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-09-18T23:47:14.810Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10566
File: src/hooks/useWalletSupportsChain/useWalletSupportsChain.ts:55-66
Timestamp: 2025-09-18T23:47:14.810Z
Learning: In the useWalletSupportsChain architecture, checkWalletHasRuntimeSupport() determines if the app has runtime capability to interact with a chain type (not actual signing capabilities), while walletSupportsChain() does the actual capabilities detection by checking account IDs. For Ledger read-only mode, checkWalletHasRuntimeSupport should return true since the app can display balances/addresses, with KeyManager being the source of truth rather than wallet instance.
Applied to files:
src/components/Modals/Send/views/Confirm.tsx
📚 Learning: 2025-10-13T11:55:57.439Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10751
File: src/Routes/RoutesCommon.tsx:190-190
Timestamp: 2025-10-13T11:55:57.439Z
Learning: In the shapeshift/web codebase, there are multiple independent claim systems: Arbitrum bridge claims (removed in PR #10751), RFOX claims (in src/pages/RFOX/components/Claim/), and TCY claims (in src/pages/TCY/). Each has its own routes, components, and logic. When reviewing claim-related changes, distinguish which system is being modified and avoid suggesting changes to unrelated claim systems.
Applied to files:
src/features/defi/providers/fox-farming/hooks/useFoxFarming.ts
🧬 Code graph analysis (2)
src/features/defi/providers/fox-farming/hooks/useFoxFarming.ts (2)
src/lib/utils/evm/index.ts (1)
buildAndBroadcast(98-112)packages/chain-adapters/src/types.ts (1)
CONTRACT_INTERACTION(27-27)
src/components/Modals/Send/utils.ts (1)
src/lib/utils/utxo.ts (1)
isUtxoChainId(27-28)
🔇 Additional comments (10)
src/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsx (1)
1-1: Trezor pubKey derivation and approval wiring look correctpubKey is only derived for Trezor wallets with a defined accountId and cleanly passed into the approve mutation; dependencies and guards prevent undefined access and keep non-Trezor behavior unchanged.
Please sanity‑check with a Trezor by running a limit order approval and confirming there are no extra on‑device derivation prompts and that approvals still succeed.
Also applies to: 3-3, 91-95, 108-108
src/features/defi/providers/fox-farming/hooks/useFoxFarming.ts (1)
6-6: Unstake/claim Trezor pubKey handling and CONTRACT_INTERACTION receiver look consistentYou correctly gate pubKey on a Trezor wallet + farmingAccountId and pass it through createBuildCustomTxInput, while aligning unstake/claim receiverAddress with stake/approve via CONTRACT_INTERACTION; dependencies include farmingAccountId so callbacks update when the farming account changes.
Please verify on a Trezor that FOX Farming unstake/exit/claim flows no longer cause extra derivation prompts and that activity history/analytics still interpret these as contract interactions as expected.
Also applies to: 128-131, 137-138, 154-155, 312-314, 319-332
src/components/Modals/Send/views/Confirm.tsx (1)
21-21: Extending change-address display to Trezor is correctly scopedThe updated shouldShowChangeAddress condition cleanly adds Trezor alongside Ledger for UTXO sends, gated on assetId + wallet, without affecting non-UTXO or software wallets.
Please confirm on a Trezor UTXO send that the change address row appears and that its value matches the address returned by the underlying change-address logic.
Also applies to: 234-243
src/lib/utils/evm/types.ts (1)
6-13: ApproveInput pubKey extension is backward compatibleAdding an optional pubKey to ApproveInput cleanly exposes the new capability to approval flows while keeping existing callers valid through MaybeApproveInput/ApproveInputWithWallet.
src/components/Modals/Send/AddressBook/hooks/useInternalAccountReceiveAddress.tsx (1)
3-5: Hardware-wallet unchained receive-address logic is safely generalizedThe shouldFetchUnchainedAddress flag correctly includes Ledger, Trezor, and GridPlus, and you only derive pubKey from accountId when all prerequisites are met, keeping other wallets on the existing code path.
Please smoke-test receive-address lookups from the address book for Ledger, Trezor, and GridPlus to confirm they no longer trigger derivation prompts and still return the expected receive address.
Also applies to: 58-66
src/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx (1)
3-3: TradeConfirm Trezor pubKey plumbing mirrors the LimitOrder pattern correctlyYou derive pubKey from sellAssetAccountId only for Trezor wallets and thread it into the approve mutation; this matches the LimitOrderV2 approach and leaves non-Trezor behavior unchanged.
Please validate a multi‑hop trade with a Trezor where an approval is required to ensure no extra derivation prompts and successful approval completion.
Also applies to: 81-85, 93-96
src/lib/utils/evm/index.ts (1)
38-47: createBuildCustomTxInput now cleanly supports optional pubKeyExtending CreateBuildCustomTxInputArgs with pubKey and re-attaching it after fee calculation preserves the existing build shape while enabling hardware-wallet callers to pass a pre‑derived pubKey through to adapters.
Ensure the updated BuildCustomTxInput type in chain‑adapters accepts the optional pubKey and that EVM send/contract‑interaction flows with Trezor still sign and broadcast successfully.
Also applies to: 90-96
src/react-queries/queries/mutations.ts (1)
7-15: Approve mutation correctly passes through optional pubKeyDestructuring and forwarding pubKey from MaybeApproveInputWithWallet to approve() completes the new field’s propagation without altering validation or mutationKey semantics.
Also applies to: 25-33
src/lib/utils/evm/approve.ts (1)
12-21: Approve flow pubKey propagation is consistent with the rest of the EVM utilsPlumbing
pubKeyfromApproveInputWithWalletthrough tocreateBuildCustomTxInputis straightforward and matches the broader pattern in this PR. Call sites that don’t care about Trezor can omit it without behavior change.No changes requested here.
Also applies to: 32-41
src/components/Modals/Send/utils.ts (1)
258-282: Solana send pubKey extension to Trezor matches the adapter patternFor Solana, extending the existing Ledger path to also pass
pubKeywhenisTrezor(wallet)is true:pubKey: isLedger(wallet) || isTrezor(wallet) ? fromAccountId(sendInput.accountId).account : undefinedis consistent with how you’re handling Trezor across EVM/UTXO and should help avoid extra on-device derivation for SOL/SPL sends as well. No further changes needed here.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx (1)
293-301: CowSwap EVMgetAddressTrezor override looks consistent; optional small DRY improvementUsing
pubKey: wallet && isTrezor(wallet) ? fromAccountId(sellAssetAccountId).account : undefinedhere matches the PR goal of skipping on‑device derivation for Trezor while leaving non‑Trezor behavior intact, and it’s guarded by the earliersellAssetAccountIdnull check, so this looks safe.If you end up touching this again, you could optionally precompute a single
const trezorPubKey = wallet && isTrezor(wallet) ? fromAccountId(sellAssetAccountId).account : undefinednear the top ofexecuteTradeand reuse it across all branches to avoid re‑parsing the AccountId; not necessary for this PR though.Please have QA double‑check that CowSwap EIP‑712 signing with Trezor still uses the correct
fromaddress and no derivation prompts appear. Based on learnings, keeping this as an optional refactor avoids expanding scope.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsx(3 hunks)src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:27.004Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:0-0
Timestamp: 2025-09-12T11:56:19.437Z
Learning: gomesalexandre rejected verbose try/catch error handling for address validation in Tenderly integration (PR #10461), calling the approach "ugly" but still implemented safety measures in commit ad7e424b89, preferring cleaner safety implementations over defensive programming patterns.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10566
File: src/hooks/useLedgerAccountGuard/useLedgerAccountGuard.tsx:4-4
Timestamp: 2025-09-23T10:36:13.916Z
Learning: gomesalexandre dismisses suggestions to expand PR scope beyond the current migration when the PR is already systematically implementing the desired pattern (like KeyManager.Ledger migration) within focused boundaries, preferring to keep scope contained rather than doing comprehensive codebase-wide cleanups.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
Learning: gomesalexandre dismissed an aria-label accessibility suggestion with "meh" in PR #10418 for WalletConnectToDappsHeaderButton.tsx, consistent with the team's pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10418
File: src/Routes/RoutesCommon.tsx:231-267
Timestamp: 2025-09-03T21:17:27.699Z
Learning: gomesalexandre prefers to keep PR diffs focused and reasonable in size, deferring tangential improvements (like Mixpanel privacy enhancements) to separate efforts rather than expanding the scope of feature PRs.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10879
File: src/context/WalletProvider/WalletConnectV2/components/WalletConnectDirectRow.tsx:64-81
Timestamp: 2025-10-22T22:11:22.918Z
Learning: In early WalletConnect POC/features behind a flag, gomesalexandre prioritizes connection correctness/stability over UX polish; minimal safety guards (like preventing concurrent connects) are preferred, while visuals will be wired later by reallybeard.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10569
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectModalSigningFooter.tsx:121-129
Timestamp: 2025-09-17T22:40:30.149Z
Learning: gomesalexandre maintains strict scope discipline even for style/UI PRs in shapeshift/web, declining functionally correct UX improvements (like keeping Cancel button enabled during gas simulation loading) when they fall outside the PR's stated styling objectives, demonstrating his consistent pattern of deferring valid but tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:55-55
Timestamp: 2025-09-10T15:35:46.223Z
Learning: gomesalexandre prefers fail-fast early returns over graceful degradation when critical data is missing in WalletConnect flows (like peer metadata in EIP155SignTypedDataConfirmation.tsx). He favors "safety first, always double-wrap" approach and believes missing peer metadata indicates bigger problems that should be surfaced explicitly rather than masked with partial UI rendering.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:69-74
Timestamp: 2025-09-10T15:35:36.547Z
Learning: gomesalexandre dismissed alt text accessibility suggestion with "meh" in PR #10458 for EIP155SignTypedDataConfirmation.tsx Image component, consistent with team pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/SessionProposal.tsx:55-67
Timestamp: 2025-09-13T15:31:38.011Z
Learning: gomesalexandre tested WalletConnect method permissions with MetaMask and WalletConnect's official demo, finding that neither strictly honors method filtering by intersection of requested/supported methods. He prefers aligning with industry standard practices (granting broader EIP-155 method permissions) over theoretical security restrictions when real-world testing shows permissive behavior is the norm.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/ContractInteractionBreakdown.tsx:0-0
Timestamp: 2025-09-13T16:45:18.813Z
Learning: gomesalexandre prefers aggressively deleting unused/obsolete code files ("ramboing") rather than fixing technical issues in code that won't be used, demonstrating his preference for keeping codebases clean and PR scope focused.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/types.ts:7-7
Timestamp: 2025-09-10T15:34:29.604Z
Learning: gomesalexandre is comfortable relying on transitive dependencies (like abitype through ethers/viem) rather than explicitly declaring them in package.json, preferring to avoid package.json bloat when the transitive dependency approach works reliably in practice.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10503
File: .env:56-56
Timestamp: 2025-09-16T13:17:02.938Z
Learning: gomesalexandre prefers to enable feature flags globally in the base .env file when the intent is to activate features everywhere, even when there are known issues like crashes, demonstrating his preference for intentional global feature rollouts over cautious per-environment enablement.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10249
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:447-503
Timestamp: 2025-08-13T17:07:10.763Z
Learning: gomesalexandre prefers relying on TypeScript's type system for validation rather than adding defensive runtime null checks when types are properly defined. They favor a TypeScript-first approach over defensive programming with runtime validations.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpDepositActionSubscriber.tsx:61-66
Timestamp: 2025-08-14T17:51:47.556Z
Learning: gomesalexandre is not concerned about structured logging and prefers to keep console.error usage as-is rather than implementing structured logging patterns, even when project guidelines suggest otherwise.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10413
File: src/components/Modals/FiatRamps/fiatRampProviders/onramper/utils.ts:29-55
Timestamp: 2025-09-02T14:26:19.028Z
Learning: gomesalexandre prefers to keep preparatory/reference code simple until it's actively consumed, rather than implementing comprehensive error handling, validation, and robustness improvements upfront. They prefer to add these improvements when the code is actually being used in production.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:396-402
Timestamp: 2025-08-14T17:55:57.490Z
Learning: gomesalexandre is comfortable with functions/variables that return undefined or true (tri-state) when only the truthy case matters, preferring to rely on JavaScript's truthy/falsy behavior rather than explicitly returning boolean values.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10783
File: src/context/ModalStackProvider/useModalRegistration.ts:30-41
Timestamp: 2025-10-16T11:14:40.657Z
Learning: gomesalexandre prefers to add lint rules (like typescript-eslint/strict-boolean-expressions for truthiness checks on numbers) to catch common issues project-wide rather than relying on code review to catch them.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
📚 Learning: 2025-08-08T11:40:55.734Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx:41-41
Timestamp: 2025-08-08T11:40:55.734Z
Learning: In MultiHopTrade confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx and related hooks), there is only one active trade per flow. Because of this, persistent (module/Redux) dedupe for QuotesReceived in useTrackTradeQuotes is not necessary; the existing ref-based dedupe is acceptable.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsx
📚 Learning: 2025-08-08T11:41:36.971Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:88-109
Timestamp: 2025-08-08T11:41:36.971Z
Learning: In MultiHopTrade Confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx), the Confirm route does not remount; navigating away goes to the swapper input page. Therefore, persistent deduplication across remounts for quote tracking is unnecessary; a ref-based single-mount dedupe is sufficient.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsx
📚 Learning: 2025-09-12T11:52:39.280Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP155TransactionConfirmation.tsx:18-21
Timestamp: 2025-09-12T11:52:39.280Z
Learning: In WalletConnect dApps integration, gomesalexandre has implemented intentional routing logic where EIP155TransactionConfirmation is typed for EthSignTransactionCallRequest only, while a separate SendTransactionConfirmation component handles EthSendTransactionCallRequest. The WalletConnectModalManager contains conditional logic to route native send transactions to SendTransactionConfirmation and other transaction types to EIP155TransactionConfirmation, creating a clean separation of concerns between signing and sending flows.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx
📚 Learning: 2025-07-29T15:04:28.083Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10139
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx:109-115
Timestamp: 2025-07-29T15:04:28.083Z
Learning: In src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx, the component is used under an umbrella that 100% of the time contains the quote, making the type assertion `activeTradeQuote?.steps[currentHopIndex] as TradeQuoteStep` safe. Adding conditional returns before hooks would violate React's Rules of Hooks.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsx
📚 Learning: 2025-08-04T16:02:27.360Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10171
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandedStepperSteps.tsx:458-458
Timestamp: 2025-08-04T16:02:27.360Z
Learning: In multi-hop swap transactions, last hop sell transactions might not be detected by the swapper (unlike buy transactions which are always known immediately). The conditional stepSource logic for last hop buy transactions (`isLastHopSellTxSeen ? stepSource : undefined`) serves as defensive programming for future multi-hop support with intermediate chains, even though multi-hop functionality is not currently supported in production.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx
📚 Learning: 2025-08-29T07:07:49.332Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10386
File: src/components/MultiHopTrade/components/VerifyAddresses/VerifyAddresses.tsx:272-294
Timestamp: 2025-08-29T07:07:49.332Z
Learning: In UTXO sell address verification flow in VerifyAddresses.tsx, the user wants address verification to be marked as "verified/complete" before starting the change address fetch, not after. The verification step and change address fetch should be treated as separate sequential operations in the UI flow.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx
📚 Learning: 2025-11-20T12:00:45.005Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11078
File: src/setupVitest.ts:11-15
Timestamp: 2025-11-20T12:00:45.005Z
Learning: In shapeshift/web, src/setupVitest.ts must redirect 'ethers' to 'ethers5' for shapeshiftoss/hdwallet-trezor (and -trezor-connect), same as ledger and shapeshift-multichain. Removing 'trezor' from the regex causes CI/Vitest failures due to ethers v6 vs v5 API differences.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsxsrc/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsx
📚 Learning: 2025-09-04T17:29:59.479Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx:28-33
Timestamp: 2025-09-04T17:29:59.479Z
Learning: In shapeshift/web, the useGetPopularAssetsQuery function in src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx intentionally uses primaryAssets[assetId] instead of falling back to assets[assetId]. The design distributes primary assets across chains by iterating through their related assets and adding the primary asset to each related asset's chain. This ensures primary assets appear in all chains where they have related assets, supporting the grouped asset system.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx
📚 Learning: 2025-10-21T17:11:18.087Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:11:18.087Z
Learning: In src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx, within the handleInputChange function, use .toFixed() without arguments (not .toString()) when converting BigNumber amounts for input field synchronization. This avoids exponential notation in the input while preserving precision for presentational components like <Amount.Crypto /> and <Amount.Fiat /> to format appropriately.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx
📚 Learning: 2025-08-10T21:09:25.643Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10215
File: src/components/MultiHopTrade/hooks/useGetTradeRateInput.ts:65-67
Timestamp: 2025-08-10T21:09:25.643Z
Learning: In the MultiHopTrade components, `selectInputBuyAsset` and `selectInputSellAsset` selectors from `tradeInputSlice` always return defined values because they have default values in the initial state (BTC for buyAsset, ETH for sellAsset, with fallback to defaultAsset). Null checks for these assets are unnecessary when using these selectors.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx
📚 Learning: 2025-11-12T12:49:17.895Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11016
File: packages/swapper/src/swappers/NearIntentsSwapper/swapperApi/getTradeQuote.ts:109-125
Timestamp: 2025-11-12T12:49:17.895Z
Learning: In packages/chain-adapters/src/evm/utils.ts, the getErc20Data function already includes a guard that returns an empty string when contractAddress is undefined (line 8: `if (!contractAddress) return ''`). This built-in handling means callers don't need to conditionally invoke getErc20Data—it safely handles both ERC20 tokens and native assets.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx
📚 Learning: 2025-08-04T15:36:25.122Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10171
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandedStepperSteps.tsx:458-458
Timestamp: 2025-08-04T15:36:25.122Z
Learning: In swap transaction handling, buy transaction hashes should always use the swapper's explorer (stepSource) because they are known by the swapper immediately upon swap execution. The conditional logic for using default explorers applies primarily to sell transactions which need to be detected/indexed by external systems like Thorchain or ViewBlock.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx
📚 Learning: 2025-11-19T16:59:50.569Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11012
File: src/context/WalletProvider/Vultisig/components/Connect.tsx:24-59
Timestamp: 2025-11-19T16:59:50.569Z
Learning: In src/context/WalletProvider/*/components/Connect.tsx files across the ShapeShift web codebase, the established pattern for handling null/undefined adapter from getAdapter() is to simply check `if (adapter) { ... }` without an else clause. All wallet Connect components (Coinbase, Keplr, Phantom, Ledger, MetaMask, WalletConnectV2, KeepKey, Vultisig) follow this pattern—they reset loading state after the if block but do not show error messages when adapter is null. This is an intentional design decision and should be maintained for consistency.
Applied to files:
src/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsx
📚 Learning: 2025-10-07T03:44:27.350Z
Learnt from: 0xApotheosis
Repo: shapeshift/web PR: 10760
File: src/components/ManageHiddenAssets/ManageHiddenAssetsList.tsx:78-84
Timestamp: 2025-10-07T03:44:27.350Z
Learning: In the ShapeShift web codebase, the following are stable references and do not need to be included in useCallback/useMemo dependency arrays:
- `navigate` from `useBrowserRouter()` hook
- Modal control objects (like `walletDrawer`) from `useModal()` hook (including their `isOpen`, `close`, and `open` methods)
- These are backed by stable context providers
Applied to files:
src/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsx
📚 Learning: 2025-11-05T23:37:30.632Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10912
File: src/context/WalletProvider/NewWalletViews/NewWalletViewsSwitch.tsx:290-290
Timestamp: 2025-11-05T23:37:30.632Z
Learning: In the ShapeShift web codebase, `isMobile` imported from '@/lib/globals' is a module-level constant (defined as `export const isMobile = Boolean(window?.isShapeShiftMobile)`) that is evaluated once at module load time. It is a stable reference that does not need to be included in useCallback/useMemo/useEffect dependency arrays.
Applied to files:
src/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install and Cache
🔇 Additional comments (5)
src/components/MultiHopTrade/components/LimitOrderV2/hooks/useAllowanceApproval.tsx (2)
1-3: Imports for CAIP account parsing and Trezor type‑guard look appropriatePulling in
fromAccountIdandisTrezorhere is consistent with deriving an account specifier fromaccountIdand only applying the optimization to Trezor wallets, without touching other wallet types. No issues.
91-95: Trezor‑only pubKey derivation from accountId looks correct; double‑check pubKey’s expected shapeThe flow here makes sense:
- You gate on
wallet && isTrezor(wallet), so non‑Trezor behavior is unchanged.- You only derive
pubKeywhenactiveQuote?.params.accountIdis present, falling back toundefinedotherwise.fromAccountId(activeQuote.params.accountId).accountwill give the CAIP‑10 account component, which is a reasonable “account specifier” to pass down ifreactQueries.mutations.approveexpectspubKeyin that form.The only thing worth verifying is that
approve({ …, pubKey })indeed expects this CAIP‑10accountvalue (EVM address for limit orders) and not some other representation (e.g. xpub or BIP44 params). If the mutation is typed as “account specifier” across chains, this wiring is spot‑on; if it’s more wallet‑specific, we may need to adjust.If you want to be extra sure, you can grep the repo for the
pubKeyparameter onreactQueries.mutations.approveand confirm its JSDoc/types describe it as the CAIP‑10 account component (address/xpub/etc) rather than some other structure.Also applies to: 108-108
src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx (3)
341-346: EVMgetAddressTrezorpubKeywiring matches CowSwap pathThis EVM send path mirrors the CowSwap branch’s
getAddresscall, so Trezor now consistently reuses the CAIP account’saccountfield aspubKeyacross both EVM flows while leaving other wallets unchanged. That’s exactly what we want for skipping redundant derivation.Please verify with a Trezor EVM swap (multi‑hop if available) that:
- the
fromaddress matches the selected account, and- no additional Trezor address derivation prompts are shown beyond the expected signing prompts.
378-388: UTXO xpub/senderAddress Trezor fast‑path looks correct; confirm xpub/accountId invariantsFor UTXO, short‑circuiting to:
xpub = fromAccountId(sellAssetAccountId).accountfor Trezor, and- passing the same value as
pubKeyintogetAddress,avoids
getPublicKeyon device while preserving the previous code path for non‑Trezor wallets. Given the earliersellAssetAccountIdguard andaccountTypecheck, this is structurally sound.The subtle dependency is that the AccountId’s
accountportion must indeed be the xpub for UTXO Trezor accounts and stay in sync with what the adapter expects. I’d recommend explicitly sanity‑checking with a Trezor UTXO sell that:
- the displayed/send/change addresses line up with the imported account’s xpub, and
- no new derivation prompts occur during the swapper trade/fee‑polling flows mentioned in the PR description.
439-444: SolanagetAddressTrezorpubKeyusage aligns with other chainsPassing the CAIP account
accountfield aspubKeyfor Trezor in the SolanagetAddresscall follows the same pattern as EVM and UTXO, and keeps non‑Trezor behavior unchanged. That consistency is good given how many code paths this PR touches.Once Solana+Trezor flows are exercised, it’s worth confirming that the derived
fromaddress matches the portfolio account and no extra derivation prompts show up versus current behavior.
NeOMakinG
reviewed
Nov 25, 2025
NeOMakinG
approved these changes
Nov 25, 2025
Collaborator
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
https://jam.dev/c/fc36f299-c5e2-4d02-82b5-c2758aae996b
https://jam.dev/c/c81836f6-8a09-4c3e-823e-2443f63c733b
Seems to work as described!
1 task
gomesalexandre
added a commit
that referenced
this pull request
Nov 27, 2025
Extends PR #11159 to prevent Trezor Suite prompts in additional locations: - rFOX: stake, unstake, claim, change address, bridge approval (6 operations) - Uniswap V2 LP: add/remove liquidity, approve, getAddress (4 operations) - ThorChain LP: EVM deposits via depositWithExpiry (1 operation) - ThorChain Savers: approval (1 operation) - FOX Farming: stake (1 operation) Also renames RecipientAddressRow prop from `recipientAddress` to `receiveAddress` for consistency with the variable name. All changes follow the same pattern: pass `pubKey` parameter when Trezor wallet detected to skip on-device address derivation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
gomesalexandre
added a commit
that referenced
this pull request
Nov 27, 2025
Extends PR #11159 to prevent Trezor Suite prompts in additional locations: - rFOX: stake, unstake, claim, change address, bridge approval (6 operations) - Uniswap V2 LP: add/remove liquidity, approve, getAddress (4 operations) - ThorChain LP: EVM deposits via depositWithExpiry (1 operation) - ThorChain Savers: approval (1 operation) - FOX Farming: stake (1 operation) Also renames RecipientAddressRow prop from `recipientAddress` to `receiveAddress` for consistency with the variable name. All changes follow the same pattern: pass `pubKey` parameter when Trezor wallet detected to skip on-device address derivation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
gomesalexandre
added a commit
that referenced
this pull request
Nov 27, 2025
…ations Extends PR #11159 to prevent Trezor Suite prompts in additional locations: - rFOX: stake, unstake, claim, change address, bridge approval (5 operations) - Uniswap V2 LP: add/remove liquidity, approve, getAddress (4 operations) - ThorChain LP: EVM deposits via depositWithExpiry (1 operation) - ThorChain Savers: approval (1 operation) - FOX Farming: stake (1 operation) All changes follow the same pattern: pass `pubKey` parameter when Trezor wallet detected to skip on-device address derivation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Nov 27, 2025
Merged
1 task
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Makes Trezor actually usable by not spewing random address/xpub requests during normal app usage:
Also while at that:
displays change address for Trezor similar to Ledger in send flow (obviously, skipping on-device derivation for the precise reason above)Issue (if applicable)
closes N/A
Risk
Testing
Engineering
Operations
Screenshots (if applicable)
https://jam.dev/c/f43299fc-305c-434d-bfb0-928ed690f697
https://jam.dev/c/097ae6ba-adc7-49a0-84a7-d2395107ed0d
https://jam.dev/c/56c6748e-96a2-40c9-82fa-b80b36042032
https://jam.dev/c/69990f53-9846-470b-aab3-47a84dd6ee81
https://jam.dev/c/b9491418-8f8f-4052-91ef-2e19b4ab61a1
Summary by CodeRabbit
New Features
Performance Improvements
✏️ Tip: You can customize this high-level summary in your review settings.