Tracking pending transactions#30
Conversation
…nsactions at once from single user
WalkthroughThe changes introduce explicit tracking and management of pending bridge transactions and processing state within the account store and propagate this state to UI components. New methods and properties are added to manage transaction processing, and UI elements are updated to reflect ongoing transaction status, disabling actions as appropriate during processing. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainComponent
participant HomePage
participant useTariAccount (store)
User->>MainComponent: Click "Bridge to Ethereum"
MainComponent->>HomePage: handleBridgeToEthereum()
HomePage->>useTariAccount: addPendingTransaction(txId)
HomePage->>useTariAccount: bridgeToEthereum()
alt Success
HomePage->>MainComponent: Advance modal step
HomePage->>useTariAccount: removePendingTransaction(txId)
else Error
HomePage->>useTariAccount: removePendingTransaction(txId)
HomePage->>Console: Log error
end
useTariAccount-->>MainComponent: Update isProcessingTransaction
MainComponent->>User: Disable/enable UI based on isProcessingTransaction
Possibly related PRs
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (2)
store/account.ts (2)
56-61: Guard against duplicate transaction IDs.The
addPendingTransactionfunction adds transaction IDs to the array without checking for duplicates. While unlikely with the current timestamp-based ID generation, adding a check would make the code more robust.addPendingTransaction: (txId: string) => { set((state) => ({ - pendingBridgeTx: [...state.pendingBridgeTx, txId], + pendingBridgeTx: state.pendingBridgeTx.includes(txId) + ? state.pendingBridgeTx + : [...state.pendingBridgeTx, txId], isProcessingTransaction: true, })) },
71-73: Potential inconsistency with direct state setting.The
setProcessingTransactionmethod allows directly setting the processing state, which could potentially get out of sync with the actual pending transactions. Consider adding a warning comment to document this potential issue or adding validation that prevents settingisProcessingTransactiontofalsewhen there are still pending transactions.setProcessingTransaction: (isProcessing: boolean) => { - set({ isProcessingTransaction: isProcessing }) + set((state) => { + // Prevent setting isProcessingTransaction to false if there are pending transactions + if (!isProcessing && state.pendingBridgeTx.length > 0) { + console.warn('Attempting to set isProcessingTransaction to false while pending transactions exist') + return { isProcessingTransaction: true } + } + return { isProcessingTransaction: isProcessing } + }) },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/page.tsx(3 hunks)components/main/main.component.tsx(1 hunks)components/main/main.types.ts(1 hunks)store/account.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/page.tsx (1)
store/account.ts (1)
useTariAccount(30-74)
🔇 Additional comments (9)
components/main/main.types.ts (1)
12-12: LGTM: AddingisProcessingTransactionproperty is appropriate.Adding the
isProcessingTransactionboolean property toMainComponentPropsallows the component to reflect the transaction processing state in the UI, which is essential for providing feedback to users during bridging operations.store/account.ts (4)
7-8: LGTM: State properties for tracking pending transactions.The changes to the
Stateinterface appropriately add support for tracking pending transactions with thependingBridgeTxarray and the processing status with theisProcessingTransactionflag.
13-15: LGTM: New action methods for transaction management.These methods provide a complete API for managing pending transactions and their processing state, which aligns well with the PR objectives.
27-27: LGTM: Initial state initialization.Setting the initial processing state to
falseis appropriate, as no transactions would be in progress when the application first loads.
62-70: LGTM: Effective removal of pending transactions.The
removePendingTransactionimplementation correctly removes the specified transaction ID and updates the processing state based on whether any transactions remain. This ensures the UI state accurately reflects the actual processing state.components/main/main.component.tsx (2)
25-25: LGTM: AddingisProcessingTransactionto component props.Properly destructuring the new prop that was added to the component's type definition.
32-32: LGTM: Updating the disabled state logic.The updated
isDisabledlogic correctly incorporates the transaction processing state, which prevents users from initiating new transactions while one is already in progress. This is a good UX improvement.app/page.tsx (2)
33-38: LGTM: Destructuring transaction-related properties.Properly accessing the transaction management functionality from the account store.
135-135: LGTM: Combining processing state indicators.Using a logical OR between
isProcessingTransactionandisBridgingensures that the UI remains disabled during any kind of transaction processing, whether it's tracked in the account store or by the bridge hook.
| const txId = `bridge-${Date.now()}` | ||
|
|
||
| addPendingTransaction(txId) | ||
|
|
||
| bridgeToEthereum({ amount, ethAddress: address }) | ||
| .then(() => { | ||
| setModalStep(2) | ||
| }) | ||
| .catch((error) => { | ||
| console.error('Bridge operation failed:', error) | ||
|
|
||
| removePendingTransaction(txId) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling feedback for the user.
While the code correctly adds and removes pending transactions, it doesn't provide any user feedback when a bridge operation fails. The transaction is removed from pending, but the UI doesn't reflect the error.
bridgeToEthereum({ amount, ethAddress: address })
.then(() => {
setModalStep(2)
})
.catch((error) => {
console.error('Bridge operation failed:', error)
+ // Provide user feedback about the error
+ // For example, showing an error message or resetting the modal state
+ setModalStep(1) // Go back to previous step or create a new error step
+ // Consider using a toast notification or modal to display the error
removePendingTransaction(txId)
})📝 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.
| const txId = `bridge-${Date.now()}` | |
| addPendingTransaction(txId) | |
| bridgeToEthereum({ amount, ethAddress: address }) | |
| .then(() => { | |
| setModalStep(2) | |
| }) | |
| .catch((error) => { | |
| console.error('Bridge operation failed:', error) | |
| removePendingTransaction(txId) | |
| }) | |
| const txId = `bridge-${Date.now()}` | |
| addPendingTransaction(txId) | |
| bridgeToEthereum({ amount, ethAddress: address }) | |
| .then(() => { | |
| setModalStep(2) | |
| }) | |
| .catch((error) => { | |
| console.error('Bridge operation failed:', error) | |
| // Provide user feedback about the error | |
| // For example, showing an error message or resetting the modal state | |
| setModalStep(1) // Go back to previous step or create a new error step | |
| // Consider using a toast notification or modal to display the error | |
| removePendingTransaction(txId) | |
| }) |
1 participant
Implemented processed transaction tracking to store to prevent multiple transactions at once from single user
Summary by CodeRabbit
New Features
Improvements