Skip to content

fix: second class chains dedupe internal/value - #11527

Merged
gomesalexandre merged 1 commit into
developfrom
fix_second_class_chain_fixes
Dec 24, 2025
Merged

fix: second class chains dedupe internal/value#11527
gomesalexandre merged 1 commit into
developfrom
fix_second_class_chain_fixes

Conversation

@gomesalexandre

@gomesalexandre gomesalexandre commented Dec 24, 2025

Copy link
Copy Markdown
Contributor

Description

Native amounts are currently doubled for same-chain second-class EVM Txs (sends/swaps) in upserted Txs, when the Tx is a same-account Tx. The reason here is we parse both the value and the internal transfer value out of debug RPC call. This ensures deduping, so Tx value is not doubled in Tx history.

Issue (if applicable)

Risk

High Risk PRs Require 2 approvals

Low

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

Second-class EVM chains (XPL, Monad, Plasma, HyperEVM) - fixes duplicate transfer amounts in transaction history for same-account sends and swaps.

Testing

  • make a self-send of native asset for second class EVM chain
  • ensure amounts are not doubled in upserted Tx
  • do a same-account swap out of native asset for second-class EVM chain
  • ditto ensure amount not doubled in Tx history

Engineering

Operations

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

Screenshots (if applicable)

develop

Screenshot 2025-12-24 at 13 08 48

this diff

Screenshot 2025-12-24 at 13 12 35 Screenshot 2025-12-24 at 13 10 45

Native amounts are currently doubled for same-chain second-class EVM Txs (sends/swaps) in upserted Txs, when the Tx is a same-account Tx. The reason here is we parse both the `value` and the internal transfer value out of debug RPC call. This ensures deduping, so Tx value is not doubled in Tx history.

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

Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
@gomesalexandre
gomesalexandre requested a review from a team as a code owner December 24, 2025 12:19
@coderabbitai

coderabbitai Bot commented Dec 24, 2025

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds a guard condition in SecondClassEvmAdapter.ts to skip internal transactions that exactly match the native transaction's from, to, and value fields, preventing duplicate processing of native transfers as both native and internal transactions.

Changes

Cohort / File(s) Summary
Internal transaction deduplication
packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts
Adds a continue guard in the internal transaction processing loop to skip transactions where internalFrom === txFrom, internalTo === txTo, and internalTx.value === tx.value, preventing double-counting of native transfers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested labels

capy

Suggested reviewers

  • NeOMakinG

Poem

🐰 A hop, skip, and guard so fine,
No more doubles on the line,
Native transfers, clear and true,
Second-class chains fixed anew! 🎉

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: deduplicating internal and value transactions on second-class chains, which directly addresses the PR objective of fixing doubled native amounts.
Linked Issues check ✅ Passed The code change directly implements the fix for issue #11524 by adding deduplication logic to prevent double-counting of native transfers in transaction parsing, matching the acceptance criteria.
Out of Scope Changes check ✅ Passed The change is narrowly scoped to the SecondClassEvmAdapter's parse method with a single guard clause, containing only logic directly related to fixing the duplicate native amount issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_second_class_chain_fixes

📜 Recent review details

Configuration used: Organization 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 bfcc0df and 84e25d0.

📒 Files selected for processing (1)
  • packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Never assume a library is available - always check imports/package.json first
Prefer composition over inheritance
Write self-documenting code with clear variable and function names
Keep functions small and focused on a single responsibility
Avoid deep nesting - use early returns instead
Prefer procedural and easy to understand code
Never expose, log, or commit secrets, API keys, or credentials
Validate all inputs, especially user inputs
Handle errors gracefully with meaningful messages
Don't silently catch and ignore exceptions
Log errors appropriately for debugging
Provide fallback behavior when possible
Use appropriate data structures for the task
Never add code comments unless explicitly requested
When modifying code, do not add comments that reference previous implementations or explain what changed. Comments should only describe the current logic and functionality.
Use meaningful names for branches, variables, and functions
Always run yarn lint --fix and yarn type-check after making changes
Avoid let variable assignments - prefer const with inline IIFE switch statements or extract to functions for conditional logic

Files:

  • packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Avoid useEffect where practical - use it only when necessary and following best practices
Avoid 'any' types - use specific type annotations instead
For default values with user overrides, use computed values (useMemo) instead of useEffect - pattern: userSelected ?? smartDefault ?? fallback
When function parameters are unused due to interface requirements, refactor the interface or implementation to remove them rather than prefixing with underscore
Sanitize data before displaying to prevent XSS
Memoize aggressively - wrap component variables in useMemo and callbacks in useCallback where possible
For static JSX icon elements (e.g., <TbCopy />) that don't depend on state/props, define them as constants outside the component to avoid re-renders instead of using useMemo
Account for light/dark mode using useColorModeValue hook
Account for responsive mobile designs in all UI components
When applying styles, use the existing standards and conventions of the codebase
Use Chakra UI components and conventions
All copy/text must use translation keys - never hardcode strings
Use the translation hook: useTranslate() from react-polyglot
Use useFeatureFlag('FlagName') hook to access feature flag values in components
Prefer type over interface for type definitions
Use strict typing - avoid any
Use Nominal types for domain identifiers (e.g., WalletId, AccountId)
Import types from @shapeshiftoss/caip for chain/account/asset IDs
Use useAppSelector for Redux state
Use useAppDispatch for Redux actions
Memoize expensive computations with useMemo
Memoize callbacks with useCallback

**/*.{ts,tsx}: Use Result<T, E> pattern for error handling in swappers and APIs; ALWAYS use Ok() and Err() from @sniptt/monads; AVOID throwing within swapper API implementations
ALWAYS use custom error classes from @shapeshiftoss/errors with meaningful error codes for internationalization and relevant details in error objects
ALWAYS wrap async op...

Files:

  • packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts
**/*.{js,jsx,ts,tsx}

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

**/*.{js,jsx,ts,tsx}: Use camelCase for variables, functions, and methods with descriptive names that explain the purpose
Use verb prefixes for functions that perform actions (e.g., fetch, validate, execute, update, calculate)
Use UPPER_SNAKE_CASE for constants and configuration values with descriptive names
Use handle prefix for event handlers with descriptive names in camelCase
Use descriptive boolean variable names with is, has, can, should prefixes
Use named exports for components, functions, and utilities instead of default exports
Use descriptive import names and avoid renaming imports unless necessary
Avoid non-descriptive variable names like data, item, obj, and single-letter variable names except in loops
Avoid abbreviations in names unless they are widely understood
Avoid generic function names like fn, func, or callback

Files:

  • packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11170
File: patches/@shapeshiftoss+bitcoinjs-lib+7.0.0-shapeshift.0.patch:9-19
Timestamp: 2025-11-25T21:43:10.838Z
Learning: In shapeshift/web, gomesalexandre will not expand PR scope to fix latent bugs in unused API surface (like bitcoinjs-lib patch validation methods) when comprehensive testing proves the actual used code paths work correctly, preferring to avoid costly hdwallet/web verdaccio publish cycles and full regression testing for conceptual issues with zero runtime impact.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 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
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:133-137
Timestamp: 2025-09-12T10:15:10.389Z
Learning: gomesalexandre has identified that EIP-712 domain chainId should be preferred over request context chainId for accuracy in WalletConnect dApps structured signing flows. The domain chainId from the parsed message is more specific and accurate than the general request context, especially for asset resolution and network-specific operations.
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: 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: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/*.ts : Reuse executeEvmTransaction utility for EVM-based swappers instead of implementing custom transaction execution
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.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/hooks/useSimulateEvmTransaction.ts:0-0
Timestamp: 2025-09-12T13:44:17.019Z
Learning: gomesalexandre prefers letting chain adapter errors throw naturally in useSimulateEvmTransaction rather than adding explicit error handling for missing adapters, consistent with his fail-fast approach and dismissal of defensive validation as "stale" in WalletConnect transaction simulation flows.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11449
File: packages/chain-adapters/src/tron/TronChainAdapter.ts:570-596
Timestamp: 2025-12-17T14:50:01.629Z
Learning: In packages/chain-adapters/src/tron/TronChainAdapter.ts, the parseTx method uses `unknown` type for the txHashOrTx parameter intentionally. TRON is a "second-class chain" that works differently from other chains - it accepts either a string hash (to fetch TronTx via unchained client) or a TronTx object directly. The base chain-adapter interface is strongly typed and doesn't accommodate this flexible signature, so `unknown` is used as an appropriate escape hatch rather than a type safety issue.
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-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/*.ts : Reuse executeEvmTransaction utility for EVM-based swappers instead of implementing custom transaction execution

Applied to files:

  • packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts
📚 Learning: 2025-09-12T13:44:17.019Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/hooks/useSimulateEvmTransaction.ts:0-0
Timestamp: 2025-09-12T13:44:17.019Z
Learning: gomesalexandre prefers letting chain adapter errors throw naturally in useSimulateEvmTransaction rather than adding explicit error handling for missing adapters, consistent with his fail-fast approach and dismissal of defensive validation as "stale" in WalletConnect transaction simulation flows.

Applied to files:

  • packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts
📚 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:

  • packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts
📚 Learning: 2025-12-17T14:50:01.629Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11449
File: packages/chain-adapters/src/tron/TronChainAdapter.ts:570-596
Timestamp: 2025-12-17T14:50:01.629Z
Learning: In packages/chain-adapters/src/tron/TronChainAdapter.ts, the parseTx method uses `unknown` type for the txHashOrTx parameter intentionally. TRON is a "second-class chain" that works differently from other chains - it accepts either a string hash (to fetch TronTx via unchained client) or a TronTx object directly. The base chain-adapter interface is strongly typed and doesn't accommodate this flexible signature, so `unknown` is used as an appropriate escape hatch rather than a type safety issue.

Applied to files:

  • packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/**/*.ts : Avoid side effects in swap logic; ensure swap methods are deterministic and stateless

Applied to files:

  • packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/endpoints.ts : Reuse checkEvmSwapStatus utility for checking EVM swap status instead of implementing custom status checks

Applied to files:

  • packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts
📚 Learning: 2025-12-04T22:57:50.850Z
Learnt from: kaladinlight
Repo: shapeshift/web PR: 11290
File: packages/chain-adapters/src/utxo/zcash/ZcashChainAdapter.ts:48-51
Timestamp: 2025-12-04T22:57:50.850Z
Learning: In packages/chain-adapters/src/**/*ChainAdapter.ts files, the getName() method uses the pattern `const enumIndex = Object.values(ChainAdapterDisplayName).indexOf(ChainAdapterDisplayName.XXX); return Object.keys(ChainAdapterDisplayName)[enumIndex]` to reverse-lookup the enum key from its value. This is the established pattern used consistently across almost all chain adapters (Bitcoin, Ethereum, Litecoin, Dogecoin, Polygon, Arbitrum, Cosmos, etc.) and should be preserved for consistency when adding new chain adapters.

Applied to files:

  • packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts
⏰ 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: Call / Static
🔇 Additional comments (1)
packages/chain-adapters/src/evm/SecondClassEvmAdapter.ts (1)

503-509: LGTM! Deduplication logic correctly prevents double-counting.

The guard effectively skips internal transactions that exactly match the native transaction (same from/to addresses and value), which solves the reported issue of doubled native amounts in transaction history for same-account transactions on second-class EVM chains.

The logic is sound:

  • Native transfers are added first (lines 476-494)
  • This guard prevents processing duplicate internal transfers from the debug RPC call
  • Legitimate distinct internal transfers are still processed correctly

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

Copy link
Copy Markdown
Contributor Author

Yeeting as obvious/blatant mistake once CI finishes.

@gomesalexandre
gomesalexandre merged commit 5ad85c5 into develop Dec 24, 2025
4 checks passed
@gomesalexandre
gomesalexandre deleted the fix_second_class_chain_fixes branch December 24, 2025 12:33
This was referenced Dec 24, 2025
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.

Second-class chains wrong Tx amounts

1 participant