Feat/send token confirm page #138
Conversation
WalkthroughThe recent updates enhance the wallet application by introducing a new confirmation page for token transfers, streamlining state management for token sending, and improving fee estimation across different blockchain wallets. These changes collectively enhance user experience, ensuring clearer workflows and greater usability when initiating and confirming transactions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SendTokenPage
participant SendTokenConfirmPage
participant sendTokenStore
participant EthereumChainWallet
User->>SendTokenPage: Initiates token transfer
SendTokenPage->>sendTokenStore: Updates toAddress and amount
SendTokenPage->>SendTokenConfirmPage: Navigates to confirmation
SendTokenConfirmPage->>EthereumChainWallet: Calls getEstimatedFee
EthereumChainWallet-->>SendTokenConfirmPage: Returns estimated fee
User->>SendTokenConfirmPage: Confirms transfer
SendTokenConfirmPage->>sendTokenStore: Triggers transfer mutation
sendTokenStore-->>User: Displays success message
Poem
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
Deploy preview for hibit-id ready! ✅ Preview Built with commit 045c13f. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (9)
apps/wallet/src/pages/send-token/store.ts (1)
1-2: Consider importing only necessary functions or types.If
RootAssetInfoandmakeAutoObservableare the only imports needed from their respective modules, this is fine. However, if there are more specific imports, consider importing only what is necessary to reduce bundle size and improve readability.apps/wallet/src/pages/send-token/confirm-page.tsx (5)
1-18: Review Imports for Optimization and ConsistencyThe imports are well-structured, but consider the following:
- Ensure all imported assets are used in the component.
- Group similar imports together for better readability, such as React hooks and components.
- Verify if all imported modules are necessary for this file.
19-27: Initialize State with Type AnnotationsThe
useStatehooks forerrMsgandtransferResultare correctly used, but consider adding explicit type annotations for better clarity and maintainability.const [errMsg, setErrMsg] = useState<string>(''); const [transferResult, setTransferResult] = useState<{ state: 'pending' | 'done'; txId: string; }>({ state: 'pending', txId: '', });
59-68: Enhance Error Message LogicThe
useEffectfor setting error messages is well-implemented. Ensure that all possible states are covered, and consider logging or handling unexpected states for debugging.
105-114: Consider Loading State EnhancementsThe loading state UI for pending transfers is clear. Consider adding more detailed loading indicators or messages if the process takes a long time.
140-209: Review UI Elements for AccessibilityThe UI elements such as buttons and labels are well-structured. Ensure that all elements are accessible, with appropriate ARIA attributes and keyboard navigation.
apps/wallet/src/utils/chain/chain-wallets/ton/index.ts (3)
Line range hint
1-14: Review Imports for Optimization and ConsistencyThe imports are well-structured, but consider the following:
- Ensure all imported modules are used in the file.
- Group similar imports together for better readability, such as external libraries and internal utilities.
Line range hint
232-250: Review Private Methods for EfficiencyThe private methods
initWallet,getIsTestNet, andgetJettonWalletare well-implemented. Ensure that they are efficient and handle all edge cases, especially in asynchronous operations.
188-231: Enhance Error Handling ingetEstimatedFeeMethodThe
getEstimatedFeemethod should include a try-catch block around theestimateExternalMessageFeecall to handle any exceptions that may be thrown by this external API. This will ensure more robust error handling and prevent unhandled exceptions from propagating up the call stack.
- Consider wrapping the
estimateExternalMessageFeecall in a try-catch block to handle potential exceptions.Analysis chain
Ensure Robust Error Handling in getEstimatedFee
The
getEstimatedFeemethod is well-implemented, but ensure robust error handling for all possible scenarios, especially when dealing with external API calls.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify robust error handling in `getEstimatedFee`. # Test: Check for error handling in external API calls within the method. rg --type ts "estimateExternalMessageFee"Length of output: 165
Script:
#!/bin/bash # Description: Check for error handling mechanisms around the `estimateExternalMessageFee` call. # Search for try-catch blocks or error handling around the `estimateExternalMessageFee` call in the file. rg --type ts -A 10 "estimateExternalMessageFee" apps/wallet/src/utils/chain/chain-wallets/ton/index.tsLength of output: 414
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (4)
apps/wallet/src/assets/copy.svgis excluded by!**/*.svgapps/wallet/src/assets/external.svgis excluded by!**/*.svgapps/wallet/src/assets/transfer-loading.svgis excluded by!**/*.svgapps/wallet/src/assets/transfer-success.svgis excluded by!**/*.svg
Files selected for processing (10)
- apps/wallet/src/App.tsx (2 hunks)
- apps/wallet/src/pages/send-token/confirm-page.tsx (1 hunks)
- apps/wallet/src/pages/send-token/index.tsx (4 hunks)
- apps/wallet/src/pages/send-token/store.ts (1 hunks)
- apps/wallet/src/utils/chain/chain-wallets/ethereum/index.ts (1 hunks)
- apps/wallet/src/utils/chain/chain-wallets/index.ts (1 hunks)
- apps/wallet/src/utils/chain/chain-wallets/ton/index.ts (1 hunks)
- apps/wallet/src/utils/chain/chain-wallets/types.ts (1 hunks)
- apps/wallet/src/utils/formatter/numberFormatter.ts (1 hunks)
- apps/wallet/src/utils/link.ts (1 hunks)
Additional comments not posted (16)
apps/wallet/src/pages/send-token/store.ts (3)
4-8: Ensure type completeness forSendTokenState.The
SendTokenStateinterface is straightforward, but make sure that the types fortoken,toAddress, andamountare sufficient for all expected use cases. For instance, ifamountmight need to handle large numbers or decimals, ensure it is handled correctly.
10-32: Appropriate use of MobX for state management.The
SendTokenStoreclass correctly usesmakeAutoObservableto manage state changes automatically. The methodssetStateandresetare clear and concise, providing necessary functionality for state management.
34-34: Consider exporting the store as a singleton.Exporting
sendTokenStoreas a singleton is a good practice for ensuring a single source of truth in the application state. Make sure this aligns with the overall architecture and usage patterns in your application.apps/wallet/src/utils/link.ts (2)
4-14: Ensure robustness in URL construction.The function
getChainTxLinkconstructs URLs for transaction links. Ensure that theexplorerfield inchainInfois always a valid URL string to prevent runtime errors. Consider adding validation or error handling if necessary.
16-29: Ensure robustness in URL construction.Similarly, the function
getChainAddressLinkconstructs URLs for address links. Verify that theexplorerfield inchainInfois consistently formatted to prevent issues. Consider adding validation or error handling if necessary.apps/wallet/src/utils/chain/chain-wallets/types.ts (1)
24-24: Ensure all subclasses implementgetEstimatedFee.The addition of the
getEstimatedFeemethod to theChainWalletclass requires all subclasses to implement this method. Ensure that all existing subclasses are updated accordingly to prevent runtime errors.apps/wallet/src/utils/chain/chain-wallets/index.ts (1)
55-58: Ensure proper error handling for unsupported chains.The
getEstimatedFeemethod relies on thegetmethod to retrieve a wallet instance. Ensure that thegetmethod handles cases where a chain is not supported, as this could lead to unhandled errors when callinggetEstimatedFee.apps/wallet/src/utils/formatter/numberFormatter.ts (1)
22-28: LGTM! Verify usage offormatNumber.The update to handle
undefinedandnullinputs by returning'--'is a good improvement for robustness. Ensure that all usages offormatNumberin the codebase expect this behavior.apps/wallet/src/App.tsx (1)
24-24: LGTM!The addition of the
SendTokenConfirmPageroute is a great enhancement for the user flow. Lazy loading the component is a good practice for performance.Also applies to: 107-107
apps/wallet/src/pages/send-token/index.tsx (1)
17-17: LGTM! VerifysendTokenStoreintegration.The refactor to use
sendTokenStorefor state management simplifies the component and improves maintainability. Ensure thatsendTokenStoreis correctly integrated and functioning as expected.Also applies to: 22-22, 63-64, 80-85
Verification successful
Integration of
sendTokenStoreis verified and correct.The
sendTokenStoreis properly integrated and used in theconfirm-page.tsxandindex.tsxfiles. Itsstateis accessed and updated appropriately, and theresetmethod is utilized as expected. No issues found with the integration.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `sendTokenStore` in the codebase. # Test: Search for `sendTokenStore` usage. Expect: The store is correctly used and updated. rg --type ts -A 5 $'sendTokenStore'Length of output: 2670
apps/wallet/src/utils/chain/chain-wallets/ethereum/index.ts (1)
106-142: LGTM! VerifygetEstimatedFeeintegration.The
getEstimatedFeemethod is a valuable addition for estimating transaction fees. Ensure that this method is correctly integrated and utilized within the application.Verification successful
Integration of
getEstimatedFeeVerifiedThe
getEstimatedFeemethod is well-integrated across the codebase. It is implemented for different chains, such as Ethereum and Ton, and is used in key components like the transaction confirmation page. The method's integration is consistent with its intended purpose for estimating transaction fees.
- Usage Locations:
- Implemented in:
ethereum/index.ts,ton/index.ts- Abstract definition:
types.ts- Utilized in:
send-token/confirm-page.tsx,chain-wallets/index.tsThe method is correctly utilized and integrated within the application.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `getEstimatedFee` in the codebase. # Test: Search for `getEstimatedFee` usage. Expect: The method is correctly used and integrated. rg --type ts -A 5 $'getEstimatedFee'Length of output: 2597
apps/wallet/src/pages/send-token/confirm-page.tsx (4)
212-212: LGTMThe component is well-structured and follows good practices for React and TypeScript.
31-46: Check Query Dependencies and Error HandlingThe queries for
nativeTokenQuery,nativeBalanceQuery, andfeeQueryare correctly set up. However, ensure that:
- Error handling is in place for failed queries.
- Dependencies in
useQueryhooks are correctly specified to avoid unnecessary re-renders.
28-30: Ensure Navigation SafetyThe
navigatefunction fromuseNavigateis used correctly, but ensure that all navigation paths are valid and handled properly in the application.Verification successful
Navigation Paths Are Valid
The navigation paths used with
useNavigateare properly defined and handled in the application's routing configuration.
- Routes such as
/create-password,/verify-password,/account-manage,/send/confirm, and others are correctly set up inapps/wallet/src/App.tsx.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all navigation paths used with `useNavigate` are valid. # Test: Search for all instances of `useNavigate` and `navigate` to ensure paths are correct. rg --type tsx "useNavigate|navigate"Length of output: 66
Script:
#!/bin/bash # Description: Verify that all navigation paths used with `useNavigate` are valid. # Test: Search for all instances of `useNavigate` and `navigate` in TypeScript files. rg --type-add 'ts:*.{ts,tsx}' --type ts "useNavigate|navigate"Length of output: 5407
Script:
#!/bin/bash # Description: Verify that all navigation paths used with `useNavigate` are defined in the routing configuration. # Test: Search for route definitions in TypeScript files to ensure all navigation paths are valid. rg --type ts "Route|path" -A 3Length of output: 6694
116-138: Ensure Transaction Link SafetyThe transaction link is generated and displayed correctly. Ensure that the link is safe and that user data is not exposed in the URL.
Verification successful
Transaction Link Safety Verified
The
getChainTxLinkfunction constructs transaction URLs using only thechainIdandtxId, without exposing any sensitive user data in the URL. This implementation appears safe.
- Function implementation in
apps/wallet/src/utils/link.tsconfirms the safety.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the safety of transaction links. # Test: Ensure transaction links do not expose sensitive user data. rg --type tsx "getChainTxLink"Length of output: 58
Script:
#!/bin/bash # Description: Verify the safety of transaction links. # Test: Ensure transaction links do not expose sensitive user data. rg --type ts "getChainTxLink"Length of output: 367
Script:
#!/bin/bash # Description: Retrieve the implementation of getChainTxLink to verify URL safety. # Extract the implementation of getChainTxLink function. ast-grep --lang typescript --pattern 'export const getChainTxLink = $_' apps/wallet/src/utils/link.tsLength of output: 807
apps/wallet/src/utils/chain/chain-wallets/ton/index.ts (1)
188-231: LGTMThe
getEstimatedFeemethod is correctly implemented and follows best practices for asynchronous operations.
| const minNativeBalance = useMemo(() => { | ||
| if (!feeQuery.data || !state.token) { | ||
| return null | ||
| } | ||
| if (state.token.chainAssetType.equals(ChainAssetType.Native)) { | ||
| return new BigNumber(state.amount).plus(feeQuery.data) | ||
| } else { | ||
| return feeQuery.data | ||
| } | ||
| }, [state, feeQuery.data]) |
There was a problem hiding this comment.
Optimize useMemo Calculation
The useMemo hook is used to calculate minNativeBalance. Ensure that the dependencies array is complete and that the calculation is necessary to optimize performance.
const minNativeBalance = useMemo(() => {
if (!feeQuery.data || !state.token) {
return null;
}
return state.token.chainAssetType.equals(ChainAssetType.Native)
? new BigNumber(state.amount).plus(feeQuery.data)
: feeQuery.data;
}, [state.amount, state.token, feeQuery.data]);| const transferMutation = useMutation({ | ||
| mutationFn: async ({ address, amount }: { | ||
| address: string | ||
| amount: string | ||
| }) => { | ||
| if (!hibitIdSession.walletPool || !state.token) { | ||
| throw new Error('Wallet or token not ready') | ||
| } | ||
| return await hibitIdSession.walletPool.transfer( | ||
| address, | ||
| new BigNumber(amount), | ||
| state.token | ||
| ) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Consider Mutation Error Handling
The useMutation hook for transferMutation lacks specific error handling. Consider adding onError and onSuccess callbacks for better control over mutation outcomes.
const transferMutation = useMutation({
mutationFn: async ({ address, amount }: { address: string; amount: string }) => {
if (!hibitIdSession.walletPool || !state.token) {
throw new Error('Wallet or token not ready');
}
return await hibitIdSession.walletPool.transfer(address, new BigNumber(amount), state.token);
},
onError: (error) => {
console.error('Transfer failed:', error);
toaster.error(error instanceof Error ? error.message : JSON.stringify(error));
},
onSuccess: (data) => {
console.log('Transfer successful:', data);
},
});| const handleSend = async () => { | ||
| if (!hibitIdSession.walletPool || !state.token || errMsg) { | ||
| return | ||
| } | ||
| try { | ||
| const txId = await transferMutation.mutateAsync({ | ||
| address: state.toAddress, | ||
| amount: state.amount, | ||
| }) | ||
| console.debug('[txId]', txId) | ||
| setTransferResult({ state: 'done', txId }) | ||
| sendTokenStore.reset() | ||
| } catch (e) { | ||
| console.error(e) | ||
| setTransferResult({ state: 'pending', txId: '' }) | ||
| toaster.error(e instanceof Error ? e.message : JSON.stringify(e)) | ||
| } |
There was a problem hiding this comment.
Improve Error Handling in handleSend Function
The handleSend function has basic error handling. Consider enhancing it by categorizing errors and providing user-friendly messages.
const handleSend = async () => {
if (!hibitIdSession.walletPool || !state.token || errMsg) {
return;
}
try {
const txId = await transferMutation.mutateAsync({
address: state.toAddress,
amount: state.amount,
});
console.debug('[txId]', txId);
setTransferResult({ state: 'done', txId });
sendTokenStore.reset();
} catch (e: unknown) {
console.error('Error during transfer:', e);
setTransferResult({ state: 'pending', txId: '' });
const errorMessage = e instanceof Error ? e.message : 'An unknown error occurred';
toaster.error(errorMessage);
}
};
Summary by CodeRabbit
New Features
Bug Fixes
Refactor