fix: normalize token address casing in market data retrieval#809
fix: normalize token address casing in market data retrieval#809kvhnuke wants to merge 1 commit into
Conversation
|
💼 Build Files |
WalkthroughThis change normalizes Ethereum contract addresses to lowercase in the transaction decoder. Market data lookups now use a lowercase key for the token contract address, and the decoded transaction's ChangesAddress normalization in transaction decoder
Estimated code review effort: 1 (Trivial) | ~5 minutes Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/extension/src/providers/ethereum/libs/transaction/decoder.ts (1)
62-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLGTM on lowercase market-data lookup, minor duplication.
Lowercasing the contract address for
getMarketInfoByContractsand the subsequentmarketInfolookups correctly addresses the casing mismatch.tx.to!.toLowerCase()is computed 4 times though; extracting it once would avoid the repeated work and improve readability.♻️ Optional refactor
+ const lowerTo = tx.to!.toLowerCase(); await marketData .getMarketInfoByContracts( - [tx.to!.toLowerCase()], + [lowerTo], network.coingeckoPlatform!, ) .then(marketInfo => { - if (marketInfo[tx.to!.toLowerCase()]) { + if (marketInfo[lowerTo]) { currentPriceUSD = - marketInfo[tx.to!.toLowerCase()]!.current_price ?? 0; - CGToken = marketInfo[tx.to!.toLowerCase()]!.id; + marketInfo[lowerTo]!.current_price ?? 0; + CGToken = marketInfo[lowerTo]!.id; } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/extension/src/providers/ethereum/libs/transaction/decoder.ts` around lines 62 - 73, In the transaction decoder logic, the lowercase contract address is computed repeatedly via tx.to!.toLowerCase() inside the market-data lookup and response access, which adds unnecessary duplication. Refactor the flow in the decoder.ts handler by extracting that value once into a local variable and reusing it for getMarketInfoByContracts and the marketInfo indexing, keeping the existing behavior in the decoder logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/extension/src/providers/ethereum/libs/transaction/decoder.ts`:
- Line 89: The DecodedTx mapping in decoder.ts is still calling toLowerCase on
tx.to via a non-null assertion, which will crash for contract-creation
transactions where tx.to is absent. Update the decoding logic around the tx.to
assignment so it safely handles undefined/null values, preserving the existing
behavior for contract creation by leaving toAddress unset or passing through the
missing value without invoking string methods.
---
Nitpick comments:
In `@packages/extension/src/providers/ethereum/libs/transaction/decoder.ts`:
- Around line 62-73: In the transaction decoder logic, the lowercase contract
address is computed repeatedly via tx.to!.toLowerCase() inside the market-data
lookup and response access, which adds unnecessary duplication. Refactor the
flow in the decoder.ts handler by extracting that value once into a local
variable and reusing it for getMarketInfoByContracts and the marketInfo
indexing, keeping the existing behavior in the decoder logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b55cd4ad-d20e-41c1-94a3-81e9491efee2
📒 Files selected for processing (1)
packages/extension/src/providers/ethereum/libs/transaction/decoder.ts
| isContractCreation, | ||
| dataHex: bufferToHex(dataDecoder.data), | ||
| toAddress: tx.to!, | ||
| toAddress: tx.to!.toLowerCase(), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Crash on contract-creation transactions.
tx.to can be undefined/null (see Line 17: isContractCreation = tx.to ? false : true, and the optional toAddress?: string in DecodedTx). The ! non-null assertion is compile-time only and has no runtime effect, so calling .toLowerCase() unconditionally will throw a TypeError for every contract-creation transaction, where previously tx.to was simply passed through as-is without any method call.
🐛 Proposed fix
- toAddress: tx.to!.toLowerCase(),
+ toAddress: tx.to?.toLowerCase(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| toAddress: tx.to!.toLowerCase(), | |
| toAddress: tx.to?.toLowerCase(), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/extension/src/providers/ethereum/libs/transaction/decoder.ts` at
line 89, The DecodedTx mapping in decoder.ts is still calling toLowerCase on
tx.to via a non-null assertion, which will crash for contract-creation
transactions where tx.to is absent. Update the decoding logic around the tx.to
assignment so it safely handles undefined/null values, preserving the existing
behavior for contract creation by leaving toAddress unset or passing through the
missing value without invoking string methods.
1 participant
Summary by CodeRabbit