Skip to content

Conversation

@gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Sep 24, 2025

Description

Removes assumptions that all BIP-21 QR codes for EVM chains are ERC-681. ERC-681 is a superset of the latter, but sometimes, EVM QR codes may be just that, BIP-21. Few differences:

  • No notion of token for BIP-21. This spec was originally created for BTC, which doesn't support multiple assets.
    So in case of a BIP-21 QR, we always assume native asset, even if you generated it while requesting a token receive (this aligns with Trust scan capabilities)
  • Amounts are always precision for BIP-21, vs. ERC-681 specifying base units
  • No chain_id parameter for BIP-21, but a chain scheme prefix (ERC-681 always has ethereum:, and then uses the @<chain_id> param to specify chain

Issue (if applicable)

closes #10638

Risk

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

Low - doesn't really touch the ERC-681 logic, so isolated to new QR codes we didn't handle before, or handled wrong.

Testing

Test the below with MM mobile QR codes:

  • Sanity test an EIP-681 ERC-20 receive with amount, e.g from MM mobile, go to "Receive", then "Request Payment", select an ERC-20 and amount. Notice everything is prefilled nice and well and we end up on amount screen
  • Sanity test an EIP-681 EVM native asset receive with amount, e.g from MM mobile, go to "Receive", then "Request Payment", select an ERC-20 and amount. Notice everything is prefilled nice and well and we end up on amount screen

Test the below with Trust Mobile:

  • Test a BIP-21 non-Ethereum-chain (i.e any EVM asset on a chain other than Ethereum) EVM asset receive with amount. Notice you always end up on confirm screen with amount, with native asset selected (:warning: note: since there is no notion
    of token for this spec, you will always see the native asset selected, regardless of whether you generated a receive QR for an ERC-200 token. That aligns with how Trust handles that when scanning - there is no information of token available)
  • Test a BIP-21 ETH on Ethereum (i.e the native asset) and notice it works exactly the same as the above

Engineering

  • ^

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)
  • ^

Screenshots (if applicable)

  • EIP-681 ERC-20 receive with amount

https://jam.dev/c/4dd13ed3-015d-414d-bb04-9051a151fe6d

  • EIP-681 native EVM asset receive with amount

https://jam.dev/c/af83d4fa-b52d-425d-be75-9302ecccda60

  • BIP-21 non-ETH EVM native asset receive with amount (broken in develop/prod)

https://jam.dev/c/a08ac77c-9e31-4773-b674-f23413e8de39

  • BIP-21 ETH receive with amount (working-ish in develop, happy here)

https://jam.dev/c/5d6886f1-c89f-4bcf-a167-a081c6bf62c9

Summary by CodeRabbit

  • New Features

    • Expanded QR/address parsing: supports ERC‑681, BIP‑21, multiple EVM chains, BTC/DOGE, Solana, ENS, token transfers, and amounts.
    • Smarter send flow after QR scan: detects ambiguous transfers and routes to the appropriate step.
    • Captures and uses vanity addresses when available.
  • Bug Fixes

    • Safer handling of dangerous parameters and mismatched schemes.
    • Improved compatibility with unsupported addresses and precise amount parsing.
  • Tests

    • Extensive new coverage across popular wallet QR formats and edge cases.

gomesalexandre and others added 28 commits September 22, 2025 19:15
Enhanced EIP-681 QR code support with smart asset validation and navigation.

- Replace security blanket with intelligent asset validation
- Add support for ERC-20 token QRs with proper asset lookup
- Implement smart step skipping based on available QR data
- Use viem utilities for proper hex/chain ID conversion
- Add chain pre-selection for all QR types
- Handle both token and native asset QRs appropriately

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…dation

- Replace blanket security blocking with intelligent asset validation
- Support MetaMask-style QR codes with proper token contract validation
- Add smart step skipping based on available QR data (asset, recipient, amount)
- Convert amounts from base units using asset precision
- Set default accountId for asset when QR is scanned
- Navigate to Details screen for review instead of skipping to Confirm
- Clean up debug logging statements

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Restored helpful comments that explain control flow and error handling
in address parsing logic, including URL vs address error handling
and ChainId validation flow explanations.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add console log for QR scanning debugging
- Add float amount detection using BigNumber.js to skip Trust's imprecise amounts
- Add comprehensive test coverage for 8 major wallets:
  - Trust Mobile: Stellar (unsupported), float detection, raw amounts
  - MetaMask Mobile: Full EIP-681 compliance, scientific notation, ERC-20
  - Rabby Mobile: Basic addresses, Zerion format
  - BASE Mobile: Simple ethereum: prefix
  - Ledger Live: Plain addresses (ETH + BTC)
  - Keplr: Chain ID parsing, unsupported schemas (Cosmos/Thor)
  - Phantom: Plain Solana addresses
  - Solflare: Plain Solana addresses
- Use proper chain ID constants (bscChainId) instead of hard-coded strings
- Randomize personal wallet addresses while keeping real contract addresses

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add Bitcoin BIP-21 with float amount test case
- Add plain DOGE address parsing test
- Add DOGE BIP-21 with amount test case
- Import dogeAssetId and dogeChainId constants

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Remove direct navigation to Details screen from QR code parsing to honor intentional ambiguity for safety
- Update test case names to be more descriptive and contextual
- Change "asset registry" terminology to "asset slice"

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…ip ci]

- QR codes with both asset and amount should go to Details (amount confirmation screen)
- Preserve existing behavior for amount confirmation while maintaining safety

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Extract parseMaybeAmountCryptoPrecision to top-level with destructured args
- Add EVM float handling for non-compliant wallets (Trust, etc)
- Improve QR navigation logic to only show Select for ambiguous multi-token chains
- Clean up parseHexChainId by inlining usage and improve variable naming
- Update test to reflect new EVM float parsing behavior
- Replace let reassignments with const ternaries for cleaner code
- Add proper ChainReference typing instead of any casts

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@gomesalexandre gomesalexandre requested a review from a team as a code owner September 24, 2025 11:46
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 24, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

Adds comprehensive BIP-21 and ERC-681 parsing across multiple chains, updates QR form handling to store vanity addresses and adjust navigation based on parsed data, and expands test coverage with many provider-specific QR scenarios and edge cases.

Changes

Cohort / File(s) Summary
QR Form flow updates
src/components/Modals/QrCode/Form.tsx
Stores both address and vanityAddress from parsing; logs decoded text; derives chain namespace from assetId; introduces isAmbiguousTransfer to decide between Select vs Details routes; adjusts handling for ERC-681/Solana Pay and token vs native transfers.
Address parsing and URL handling
src/lib/address/address.ts
Extends parseMaybeUrlWithChainId to support BIP-21 and ERC-681 across EVM and other chains; adds scheme-to-chain mapping, helpers (isBip21Url/isErc681Url), ERC-20 handling, precision vs base-unit amounts, and refined error handling. Parameter names internally renamed (no external signature change).
Expanded test coverage
src/lib/address/address.test.ts
Adds grouped suites for Trust, MetaMask, Rabby, BASE, Ledger Live, Keplr, Phantom, etc.; covers multiple schemes, chainId formats, token/native transfers, dangerous parameter stripping, precision handling, and USDC asset mapping.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant User as User
  participant QR as QR Scanner
  participant Form as QR Form
  participant Addr as parseMaybeUrlWithChainId
  participant Nav as Navigator

  User->>QR: Scan QR
  QR-->>Form: decodedText
  Form->>Addr: parse(decodedText, { assetId, chainId })
  Addr-->>Form: { address, vanityAddress, assetId, chainId, amount? }

  Note over Form: Set To=address, VanityAddress=vanityAddress

  alt isAmbiguousTransfer<br/>(ERC-681/SolanaPay, native, no amount)
    Form->>Nav: Go to SendRoutes.Select
  else
    Form->>Nav: Go to SendRoutes.Details
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested reviewers

  • NeOMakinG
  • 0xApotheosis

Poem

I nuzzle the codes where the prefixes play,
From bip to 681, I hop the right way.
Chains in a meadow, amounts in the breeze,
Vanity blossoms on parsing trees.
Select or Details—two trails in sight,
I twitch, then leap—the route parsed right. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning Most production changes align with the linked issue, but I found an out-of-scope debug logging addition: a console.log for QR decode text in src/components/Modals/QrCode/Form.tsx that is not required by the feature and should not be in production code. Other modifications appear to be parsing logic, tests, or internal renames that are within scope. Remove the stray console.log from src/components/Modals/QrCode/Form.tsx (or gate it behind a dev-only flag), then re-run tests/CI and confirm no other debug/logging was accidentally introduced before merging.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues Check ✅ Passed The changes implement explicit BIP-21 vs ERC-681 detection (isBip21Url / isErc681Url), add URN-scheme-to-chain mappings for multiple EVM chains, extend parseMaybeUrlWithChainId to derive chainId/assetId correctly for BIP-21/ERC-681 cases, and update Form navigation to handle ambiguous transfers; the expanded test suite covers many BIP-21/ERC-681 scenarios and amount/value parsing, which together satisfy the linked issue's acceptance criteria. The PR therefore meets the coding objectives of detecting and handling BIP-21 for EVMs, interpreting BIP-21 amounts as precision values, and accepting non-ethereum: scheme prefixes where appropriate. Based on the provided summaries and tests, the implementation aligns with issue #10638.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed The title succinctly encapsulates the primary change of adding BIP-21 support for EVM chains, matching the pull request’s main objective. It is concise and free of extraneous details.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (5)
  • BIP-21: Entity not found: Issue - Could not find referenced Issue.
  • ERC-681: Entity not found: Issue - Could not find referenced Issue.
  • EIP-681: Entity not found: Issue - Could not find referenced Issue.
  • ERC-20: Entity not found: Issue - Could not find referenced Issue.
  • ERC-200: Entity not found: Issue - Could not find referenced Issue.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gomesalexandre gomesalexandre changed the base branch from develop to feat_erc_681_1_out_of_maybe_2 September 24, 2025 11:47
@gomesalexandre gomesalexandre changed the title [skip ci] wip: vibe feat: bip-21 full support for EVM chains Sep 24, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/QrCode/Form.tsx (1)

162-165: Avoid any in catch; use unknown and guard

Aligns with TS best practices and repo guidelines.

Apply this diff:

-        } catch (e: any) {
-          setAddressError(e.message)
+        } catch (e: unknown) {
+          const message = e instanceof Error ? e.message : 'Unknown QR parse error'
+          setAddressError(message)
         }
🧹 Nitpick comments (5)
src/lib/address/address.ts (4)

107-127: Preserve BIP‑21 amount precision (avoid float coercion)

bip21.decode often returns options.amount as a number, causing precision loss (e.g., …5678 → …5677). Prefer extracting the raw amount string from the query when present, falling back to decoded value as needed.

Apply this diff near the BIP‑21 return to prefer the raw string:

-          const parsedUrl = bip21.decode(urlOrAddress, scheme)
+          const parsedUrl = bip21.decode(urlOrAddress, scheme)
+          const amountFromQuery = (() => {
+            try {
+              const qs = urlOrAddress.split('?')[1]
+              if (!qs) return undefined
+              return new URLSearchParams(qs).get('amount') ?? undefined
+            } catch {
+              return undefined
+            }
+          })()

           const chainId = detectedChainId === inputChainId ? inputChainId : detectedChainId
           const assetId =
             detectedChainId === inputChainId && inputAssetId
               ? inputAssetId
               : getChainAdapterManager().get(detectedChainId)?.getFeeAssetId() ||
                 toAssetId({
                   chainId: detectedChainId,
                   assetNamespace: ASSET_NAMESPACE.slip44,
                   assetReference: ASSET_REFERENCE.Ethereum,
                 })

           return {
             assetId,
             maybeAddress: parsedUrl.address,
             chainId,
-            ...(parsedUrl.options?.amount && {
-              amountCryptoPrecision: bnOrZero(parsedUrl.options.amount).toFixed(),
-            }),
+            ...((amountFromQuery ?? parsedUrl.options?.amount) && {
+              amountCryptoPrecision: bnOrZero(
+                amountFromQuery ?? parsedUrl.options?.amount,
+              ).toFixed(),
+            }),
           }

197-205: Use structured logging instead of console.error

Console logging in library code hinders observability and can leak details to user consoles. Prefer the project’s structured logger with context (function, chainId, scheme, input size).

If a logger is available (e.g., lib/logger), replace console.error with logger.error({ err, context }, 'parseMaybeUrlWithChainId failed') and include sanitized context.

Also applies to: 226-227


61-79: Clarify EIP‑681 detection criteria

isErc681Url treats bare ethereum:address as non‑ERC‑681 (handled as BIP‑21), which is fine. Consider a short comment noting that this is intentional to support wallets emitting BIP‑21‑style ethereum: without ERC‑681 features.


55-57: Type the reverse mapping for safety

URN_SCHEME_TO_CHAIN_ID is inferred as Record<string, unknown>. Adding an explicit type helps prevent accidental key/value swaps during refactors.

Example:

const URN_SCHEME_TO_CHAIN_ID: Record<string, ChainId> = Object.fromEntries(
  Object.entries(CHAIN_ID_TO_URN_SCHEME).map(([chainId, scheme]) => [scheme, chainId as ChainId]),
)
src/components/Modals/QrCode/Form.tsx (1)

100-100: Remove console.log of decoded QR content

Scanning can include addresses or URIs we shouldn’t log in production.

Apply this diff:

-          console.log('QR decode text:', decodedText)
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 49afcd3 and 1d23290.

📒 Files selected for processing (3)
  • src/components/Modals/QrCode/Form.tsx (4 hunks)
  • src/lib/address/address.test.ts (1 hunks)
  • src/lib/address/address.ts (5 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)

**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern

**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...

Files:

  • src/components/Modals/QrCode/Form.tsx
  • src/lib/address/address.ts
  • src/lib/address/address.test.ts
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)

**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern

**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components

Files:

  • src/components/Modals/QrCode/Form.tsx
**/*

📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)

**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case

Files:

  • src/components/Modals/QrCode/Form.tsx
  • src/lib/address/address.ts
  • src/lib/address/address.test.ts
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)

**/*.{tsx,jsx}: ALWAYS use useMemo for expensive computations, object/array creations, and filtered data
ALWAYS use useMemo for derived values and computed properties
ALWAYS use useMemo for conditional values and simple transformations
ALWAYS use useCallback for event handlers and functions passed as props
ALWAYS use useCallback for any function that could be passed as a prop or dependency
ALWAYS include all dependencies in useEffect, useMemo, useCallback dependency arrays
NEVER use // eslint-disable-next-line react-hooks/exhaustive-deps unless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped with memo

Files:

  • src/components/Modals/QrCode/Form.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)

USE Redux only for global state shared across multiple places

Files:

  • src/components/Modals/QrCode/Form.tsx
  • src/lib/address/address.ts
  • src/lib/address/address.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: gomesalexandre
PR: shapeshift/web#10458
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:46-59
Timestamp: 2025-09-10T15:34:54.593Z
Learning: After extensive testing by gomesalexandre in PR #10458, dApps do not send EIP-712 domain.chainId as hex or bigint values in practice. The simple String(domain.chainId) conversion is sufficient for real-world usage in WalletConnect dApps structured signing.
Learnt from: gomesalexandre
PR: shapeshift/web#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
PR: shapeshift/web#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
PR: shapeshift/web#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
PR: shapeshift/web#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
PR: shapeshift/web#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
PR: shapeshift/web#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
PR: shapeshift/web#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
PR: shapeshift/web#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
PR: shapeshift/web#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.
Learnt from: gomesalexandre
PR: shapeshift/web#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.
📚 Learning: 2025-08-05T23:36:13.214Z
Learnt from: premiumjibles
PR: shapeshift/web#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/QrCode/Form.tsx
📚 Learning: 2025-09-08T15:53:09.362Z
Learnt from: gomesalexandre
PR: shapeshift/web#10442
File: src/components/TradeAssetSearch/components/GroupedAssetList/GroupedAssetList.tsx:34-35
Timestamp: 2025-09-08T15:53:09.362Z
Learning: In DefaultAssetList.tsx, the GroupedAssetList component already receives the activeChainId prop correctly on line ~58, contrary to automated analysis that may flag it as missing.

Applied to files:

  • src/components/Modals/QrCode/Form.tsx
📚 Learning: 2025-09-04T17:29:59.479Z
Learnt from: NeOMakinG
PR: shapeshift/web#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/QrCode/Form.tsx
🧬 Code graph analysis (3)
src/components/Modals/QrCode/Form.tsx (4)
src/lib/address/address.ts (1)
  • parseAddressInputWithChainId (441-471)
packages/caip/src/assetId/assetId.ts (1)
  • fromAssetId (143-178)
packages/caip/src/constants.ts (1)
  • CHAIN_NAMESPACE (77-82)
packages/utils/src/index.ts (1)
  • isToken (28-41)
src/lib/address/address.ts (5)
packages/caip/src/constants.ts (15)
  • ethChainId (60-60)
  • arbitrumChainId (66-66)
  • optimismChainId (62-62)
  • polygonChainId (64-64)
  • bscChainId (63-63)
  • avalancheChainId (61-61)
  • baseChainId (68-68)
  • gnosisChainId (65-65)
  • btcChainId (55-55)
  • bchChainId (56-56)
  • dogeChainId (57-57)
  • ltcChainId (58-58)
  • ASSET_NAMESPACE (109-118)
  • ASSET_REFERENCE (120-139)
  • CHAIN_NAMESPACE (77-82)
packages/caip/src/assetId/assetId.ts (1)
  • toAssetId (59-130)
src/state/slices/assetsSlice/selectors.ts (1)
  • selectAssetById (16-20)
src/state/store.ts (1)
  • store (137-137)
src/lib/math.ts (1)
  • fromBaseUnit (10-24)
src/lib/address/address.test.ts (2)
packages/caip/src/constants.ts (16)
  • ethAssetId (9-9)
  • ethChainId (60-60)
  • baseAssetId (17-17)
  • baseChainId (68-68)
  • avalancheAssetId (10-10)
  • avalancheChainId (61-61)
  • gnosisChainId (65-65)
  • btcAssetId (4-4)
  • btcChainId (55-55)
  • dogeAssetId (6-6)
  • dogeChainId (57-57)
  • ltcAssetId (7-7)
  • ltcChainId (58-58)
  • bscChainId (63-63)
  • solAssetId (18-18)
  • solanaChainId (75-75)
src/lib/address/address.ts (2)
  • ParseAddressByChainIdOutput (411-416)
  • parseMaybeUrlWithChainId (81-234)
🔇 Additional comments (7)
src/lib/address/address.test.ts (5)

29-166: BIP-21 coverage looks solid; one precision edge case merits a note

Great scenarios and expectations, including cross‑chain BIP‑21 handling and scheme → chain mapping. For the avalanchec case (Lines 82-98), the expected '1.1234567812345677' likely results from bip21.decode coercing query params to numbers, then stringifying. If we later switch to parsing the amount as a raw string, this assertion will need updating.

Consider adding a brief comment in this test explaining the rounding artifact to future readers.


168-349: EIP-681 tests are comprehensive and align with the parser behavior

  • ENS passthrough, parameter stripping, hex chain_id, native value/amount handling, and the “dangerous ERC‑20” path all look correct.
  • The USDC transfer case validating asset slice lookup is especially valuable.

334-348: Mismatch between URN scheme and input chainId handled as expected

Good negative coverage; this ensures we don’t accept cross‑scheme UTXO URLs under the wrong chainId.


437-501: Non‑EVM unsupported addresses gracefully fall back

Nice to see Cosmos and THOR handled as plain input without crashing. This keeps UX predictable.


503-535: Solana plain address passthrough looks good

Coverage for Phantom/Solflare QRs is a good addition ahead of any Solana Pay work.

src/lib/address/address.ts (1)

152-181: ERC‑20 guard is correct; consider extending to ERC‑721/1155 if needed later

The transfer path strictly requires token presence in the asset slice, which is the right safety default. If future wallets emit ERC‑721/1155 requests, we may need analogous handling.

src/components/Modals/QrCode/Form.tsx (1)

144-161: Ambiguous transfer routing heuristic looks good

The EVM/Solana ambiguity check that routes to Select when no amount and no token is a sensible UX choice.

@gomesalexandre gomesalexandre changed the title feat: bip-21 full support for EVM chains feat: bip-21 support for EVM chains Sep 24, 2025
Copy link
Collaborator

@NeOMakinG NeOMakinG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base automatically changed from feat_erc_681_1_out_of_maybe_2 to develop September 25, 2025 14:02
gomesalexandre and others added 2 commits September 25, 2025 16:14
Add comprehensive BIP-21 QR code support for multiple blockchain networks.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@gomesalexandre gomesalexandre enabled auto-merge (squash) September 25, 2025 14:23
@gomesalexandre gomesalexandre merged commit 1b22f99 into develop Sep 25, 2025
4 checks passed
@gomesalexandre gomesalexandre deleted the feat_bip21_full_support branch September 25, 2025 14:31
@coderabbitai coderabbitai bot mentioned this pull request Sep 25, 2025
1 task
@coderabbitai coderabbitai bot mentioned this pull request Oct 30, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BIP-21 scan full support

3 participants