fix: modified logic for checking the native token's remaining balance #1429
Conversation
|
Visit the preview URL for this PR (updated for commit a60c78c): https://astar-apps--pr1429-fix-vastr-send-j38cl0iw.web.app (expires Mon, 06 Jan 2025 08:47:58 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: dd76fe72958fe2910fef9d53f0b4539b82b849db |
impelcrypto
changed the title
| const isNativeToken = param.assetId === idAstarNativeToken; | ||
| // Memo: Check if the native token's remaining balance is enough to pay the transaction fee | ||
| if (isNativeToken) { | ||
| const useableBalance = await this.AssetsRepository.getNativeBalance(param.senderAddress); |
There was a problem hiding this comment.
AssetsRepository should be renamed to assetsRepository since it is a private variable
There was a problem hiding this comment.
Fixed in this commit
| if (isNativeToken) { | ||
| const useableBalance = await this.AssetsRepository.getNativeBalance(param.senderAddress); | ||
| const isBalanceEnough = | ||
| Number(ethers.utils.formatEther(useableBalance)) - |
There was a problem hiding this comment.
There are unnecessary conversions here. The main reason for that is useableBalance and param.amount are strings. IMO using string for balances is not natural (more natural is bigint) and leads to increased code complexity.
You don't need to fix this now, but we should have this in mind for the future
There was a problem hiding this comment.
Noted, I've modified like this but we need to do some refactoring in the future.
Pull Request Summary
Check list