-
Notifications
You must be signed in to change notification settings - Fork 7
SHARD-2738: Fix erc20 transaction information #94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
| (item: { contractType: ContractType; contractInfo: { decimals: string } | null; balance: BigNumberish }) => { | ||
| if (item.contractType === ContractType.ERC_20) { | ||
| const decimalsValue = item.contractInfo.decimals ? parseInt(item.contractInfo.decimals) : 18 | ||
| item.balance = utils.formatUnits(item.balance, decimalsValue) | ||
| const decimalsValue = item.contractInfo?.decimals ? parseInt(item.contractInfo.decimals) : 18 | ||
| item.balance = formatUnits(item.balance, decimalsValue) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Modifying item.balance directly mutates the original token object, which can cause unintended side effects if the tokens array is reused elsewhere. Instead, create a new array with updated balances and avoid mutating the original objects. [general, importance: 7]
| (item: { contractType: ContractType; contractInfo: { decimals: string } | null; balance: BigNumberish }) => { | |
| if (item.contractType === ContractType.ERC_20) { | |
| const decimalsValue = item.contractInfo.decimals ? parseInt(item.contractInfo.decimals) : 18 | |
| item.balance = utils.formatUnits(item.balance, decimalsValue) | |
| const decimalsValue = item.contractInfo?.decimals ? parseInt(item.contractInfo.decimals) : 18 | |
| item.balance = formatUnits(item.balance, decimalsValue) | |
| } | |
| tokens = tokens.map( | |
| (item: { contractType: ContractType; contractInfo: { decimals: string } | null; balance: BigNumberish }) => { | |
| if (item.contractType === ContractType.ERC_20) { | |
| const decimalsValue = item.contractInfo?.decimals ? parseInt(item.contractInfo.decimals) : 18 | |
| return { ...item, balance: formatUnits(item.balance, decimalsValue) } | |
| } | |
| return item | |
| } | |
| ) |
| <div> | ||
| {calculateTokenValue(primaryERC20Transfer, primaryERC20Transfer.tokenType, undefined, true)}{' '} | ||
| {primaryERC20Transfer.contractInfo?.symbol || primaryERC20Transfer.contractAddress} | ||
| {transaction?.wrappedEVMAccount?.readableReceipt?.value !== '0x0' && ( | ||
| <div style={{ fontSize: '0.8em', color: '#666', marginTop: '4px' }}> | ||
| + {calculateFullValue(`${transaction?.wrappedEVMAccount?.readableReceipt?.value ?? 0}`)} SHM | ||
| </div> | ||
| )} | ||
| </div> | ||
| ) : ( | ||
| `${calculateFullValue(`${transaction?.wrappedEVMAccount?.readableReceipt?.value ?? 0}`)} SHM` | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The check for transaction?.wrappedEVMAccount?.readableReceipt?.value !== '0x0' may fail if the value is undefined or in a different format (e.g., '0'). Use a more robust check to ensure correct display of the SHM value. [general, importance: 6]
| <div> | |
| {calculateTokenValue(primaryERC20Transfer, primaryERC20Transfer.tokenType, undefined, true)}{' '} | |
| {primaryERC20Transfer.contractInfo?.symbol || primaryERC20Transfer.contractAddress} | |
| {transaction?.wrappedEVMAccount?.readableReceipt?.value !== '0x0' && ( | |
| <div style={{ fontSize: '0.8em', color: '#666', marginTop: '4px' }}> | |
| + {calculateFullValue(`${transaction?.wrappedEVMAccount?.readableReceipt?.value ?? 0}`)} SHM | |
| </div> | |
| )} | |
| </div> | |
| ) : ( | |
| `${calculateFullValue(`${transaction?.wrappedEVMAccount?.readableReceipt?.value ?? 0}`)} SHM` | |
| )} | |
| {isERC20Transfer && primaryERC20Transfer ? ( | |
| <div> | |
| {calculateTokenValue(primaryERC20Transfer, primaryERC20Transfer.tokenType, undefined, true)}{' '} | |
| {primaryERC20Transfer.contractInfo?.symbol || primaryERC20Transfer.contractAddress} | |
| {transaction?.wrappedEVMAccount?.readableReceipt?.value && transaction?.wrappedEVMAccount?.readableReceipt?.value !== '0x0' && transaction?.wrappedEVMAccount?.readableReceipt?.value !== '0' && ( | |
| <div style={{ fontSize: '0.8em', color: '#666', marginTop: '4px' }}> | |
| + {calculateFullValue(`${transaction?.wrappedEVMAccount?.readableReceipt?.value ?? 0}`)} SHM | |
| </div> | |
| )} | |
| </div> | |
| ) : ( | |
| `${calculateFullValue(`${transaction?.wrappedEVMAccount?.readableReceipt?.value ?? 0}`)} SHM` | |
| )} |
PR Type
Bug fix, Enhancement
Description
Improved ERC-20 transaction and token display logic
Fixed null/undefined access for contract info fields
Standardized usage of ethers
formatUnitsfor value formattingEnhanced transaction detail view for ERC-20 transfers
Changes walkthrough 📝
useAccountDetailHook.ts
Fix null access and standardize ERC-20 balance formattingsrc/frontend/account/AccountDetail/useAccountDetailHook.ts
contractInfoin token processingformatUnitsfrom ethers for ERC-20 balance formattingTokenDropdown.tsx
Fix ERC-20 token count and improve dropdown displaysrc/frontend/account/TokenDropdown/TokenDropdown.tsx
contractInfofieldsaccount.ts
Fix null access in token query by addresssrc/storage/account.ts
contractInfoandcontractTypeToken.tsx
Standardize token value formatting and null safetysrc/frontend/token/Token.tsx
utils.formatUnitswithformatUnitsfor valueformatting
Ovewview.tsx
Enhance ERC-20 transfer display in transaction overviewsrc/frontend/transaction/TransactionDetail/Overview/Ovewview.tsx
calculateValue.ts
Standardize ERC-20 value calculation and formattingsrc/frontend/utils/calculateValue.ts
utils.formatUnitswithformatUnits