Refactor: Cancel STX transactions #5533
Conversation
WalkthroughThe update introduces several modifications and new features to handle Stacks (STX) blockchain transactions more efficiently. It includes functions and components for generating burn addresses, managing transaction actions, and increasing or canceling transaction fees. The changes encompass new React components, hooks, and utility functions to streamline transaction handling, with updated UI elements and route configurations to support these functionalities. Changes
Sequence Diagram(s)sequenceDiagram
actor User
User ->> App: Click "Increase Fee"
App ->> TransactionActionIconButton: Render Button
User ->> TransactionActionIconButton: Click Button
TransactionActionIconButton ->> Dialog: Open Increase Fee Dialog
Dialog ->> useStxIncreaseFee: Call Hook
useStxIncreaseFee ->> Network: Fetch Transaction Data
Network ->> useStxIncreaseFee: Return Data
useStxIncreaseFee ->> Dialog: Update UI with Data
User ->> Dialog: Submit New Fee
Dialog ->> useStxIncreaseFee: Submit Fee Update
useStxIncreaseFee ->> Network: Broadcast Transaction
Network ->> useStxIncreaseFee: Confirmation
useStxIncreaseFee ->> Dialog: Close Dialog
sequenceDiagram
actor User
User ->> App: Click "Cancel Transaction"
App ->> TransactionActionIconButton: Render Button
User ->> TransactionActionIconButton: Click Button
TransactionActionIconButton ->> Dialog: Open Cancel Transaction Dialog
Dialog ->> useStxCancelTransaction: Call Hook
useStxCancelTransaction ->> Network: Fetch Transaction Data
Network ->> useStxCancelTransaction: Return Data
useStxCancelTransaction ->> Dialog: Update UI with Data
User ->> Dialog: Confirm Cancellation
Dialog ->> useStxCancelTransaction: Submit Cancellation
useStxCancelTransaction ->> Network: Broadcast Cancellation
Network ->> useStxCancelTransaction: Confirmation
useStxCancelTransaction ->> Dialog: Close Dialog
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 Configration 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!pnpm-lock.yaml
Files selected for processing (16)
- src/app/common/utils/get-burn-address.ts (1 hunks)
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (2 hunks)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (5 hunks)
- src/app/components/stacks-transaction-item/transaction-action-button.tsx (1 hunks)
- src/app/components/transaction-item/transaction-item.layout.tsx (3 hunks)
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx (2 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (1 hunks)
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4 hunks)
- src/app/routes/app-routes.tsx (2 hunks)
- src/shared/route-urls.ts (1 hunks)
Files not summarized due to errors (5)
- src/app/components/stacks-transaction-item/transaction-action-button.tsx: Error: Server error. Please try again later.
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx: Error: Server error. Please try again later.
- src/app/routes/app-routes.tsx: Error: Server error. Please try again later.
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx: Error: Server error. Please try again later.
- src/shared/route-urls.ts: Error: Server error. Please try again later.
Files not reviewed due to errors (8)
- src/app/components/stacks-transaction-item/transaction-action-button.tsx (Error: Server error. Please try again later.)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (Error: Server error. Please try again later.)
- src/app/components/transaction-item/transaction-item.layout.tsx (Error: Server error. Please try again later.)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (Error: Server error. Please try again later.)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (Error: Server error. Please try again later.)
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (Error: Server error. Please try again later.)
- src/shared/route-urls.ts (Error: Server error. Please try again later.)
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (Error: Server error. Please try again later.)
Additional comments not posted (16)
src/app/common/utils/get-burn-address.ts (1)
4-12: LGTM! ThegetBurnAddressfunction correctly handles different network chain IDs.src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1)
7-16: LGTM! TheIncreaseStxFeeDialogcomponent is well-structured and uses appropriate hooks and components for its functionality.src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1)
7-16: LGTM! TheCancelStxTransactionDialogcomponent is well-structured and appropriately uses hooks and components for handling transaction cancellations.src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (1)
6-8: LGTM! TheTransactionActionscomponent is well-adapted to handle various transaction states and actions efficiently.Also applies to: 12-12
src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx (3)
Line range hint
24-73: The structure and logic of theIncreaseBtcFeeDialogcomponent are well-implemented, making good use of React patterns and hooks.
24-24: Ensure that theuseBtcIncreaseFeehook is appropriately implemented to handle the logic required for increasing Bitcoin transaction fees.Verification successful
The
useBtcIncreaseFeehook is indeed present in the codebase and is defined insrc/app/features/dialogs/transaction-action-dialog/hooks/use-btc-increase-fee.ts. This confirms that the hook is implemented and used in theIncreaseBtcFeeDialogcomponent.
src/app/features/dialogs/transaction-action-dialog/hooks/use-btc-increase-fee.ts: Defines theuseBtcIncreaseFeehook.src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx: Imports and uses theuseBtcIncreaseFeehook.Given this context, the implementation of the
useBtcIncreaseFeehook appears to be in place and used correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `useBtcIncreaseFee` hook. # Test: Search for the hook implementation. Expect: Proper handling of Bitcoin transaction fee logic. ast-grep --lang typescript --pattern $'function useBtcIncreaseFee($_, $_) { $$$ }'Length of output: 83
Script:
#!/bin/bash # Description: Search for any function or hook related to increasing BTC fee. # Test: Search for functions or hooks that might be related to increasing BTC fee. rg -i 'increase.*fee' --type=tsLength of output: 6186
73-73: TheTransactionActionscomponent is used here to provide UI for action buttons. Ensure that the props passed to this component are handled correctly in the component's definition.src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (3)
31-31: Adding theisCancelTransactionparameter is a good extension to handle different types of transaction scenarios. Ensure that this parameter is used effectively in the logic.
Line range hint
31-145: The structure and logic of theuseStacksBroadcastTransactionhook are well-implemented, making good use of React patterns and hooks.
65-65: The logic to navigate based on the type of transaction (increase fee or cancel) is a good use of conditional rendering based on state. Ensure that these conditions are thoroughly tested to handle edge cases.Also applies to: 104-105
src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (3)
1-1: The use of routing hooks likeuseMatchanduseNavigateis appropriate for components that need to handle navigation based on the application's state or routes.
63-66: The use ofuseMatchto determine if the current route matches specific transaction actions is a clever way to conditionally render UI elements based on the route.
Line range hint
1-139: The structure and logic of theStacksTransactionItemcomponent are well-implemented, making good use of React patterns and hooks.src/app/routes/app-routes.tsx (3)
108-114: The setup of nested routes for handling transaction broadcast errors is a good practice. It ensures that the user is directed to an appropriate error page if something goes wrong during the transaction process.
Line range hint
19-114: The structure and logic of the routing setup insrc/app/routes/app-routes.tsxare well-implemented, making good use of React Router patterns and error handling.
19-21: The integration of new dialog components for handling Stacks and Bitcoin transactions is well done. Ensure that these components are tested within the routing context to handle edge cases.
|
Thanks so much for taking this on @Aman-zishan. I see you also addressed some other minor issues from the first version, such as missing space and shortening the text. I like how you load the minimum fee that can replace the transaction by default and then allow the user to change that. I think the "x" in front of cancel could be a bit smaller, the same height as the >> symbol in front of "increase fee." I would also prefer to see it in the same color as the >> symbol. The red color is reserved for errors; we like to use red sparingly. |
Done, i had set both icons to be the same variant "small", i think the SVG of |
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (5 hunks)
Additional comments not posted (5)
src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (5)
1-1: Update imports to reflect the new routing and action handling functionalities.
29-39: Define new types and constants to manage transaction actions and routing.
77-83: Review the logic for handling transaction actions based on the action type. Ensure that the routing keys are correctly mapped and used.
139-139: Ensure that the conditional rendering logic foractionButtonGroupElementis clear and concise.
63-65: Ensure thatuseMatchis used correctly for route matching in new transaction action routes.
| const actionButtonGroup = ( | ||
| <HStack alignItems="start" gap="space.01"> | ||
| {[ | ||
| { | ||
| label: 'Cancel', | ||
| onClick: onCancelTransaction, | ||
| icon: <CloseIcon color="stacks" variant="small" height={12} />, | ||
| maxWidth: '110px', | ||
| }, | ||
| { | ||
| label: 'Increase Fee', | ||
| onClick: onIncreaseFee, | ||
| icon: <ChevronsRightIcon color="stacks" variant="small" />, | ||
| maxWidth: '110px', | ||
| }, | ||
| ].map(({ label, onClick, icon, maxWidth }) => ( | ||
| <TransactionActionButton | ||
| key={label} | ||
| icon={icon} | ||
| maxWidth={maxWidth} | ||
| isEnabled={isOriginator && isPending} | ||
| isSelected={isTransactionActionRoute} | ||
| onClick={onClick} | ||
| label={label} | ||
| /> | ||
| ))} | ||
| </HStack> |
There was a problem hiding this comment.
The construction of the actionButtonGroup is verbose. Consider refactoring to improve readability and reduce redundancy.
- const actionButtonGroup = (
- <HStack alignItems="start" gap="space.01">
- {[
- {
- label: 'Cancel',
- onClick: onCancelTransaction,
- icon: <CloseIcon color="stacks" variant="small" height={12} />,
- maxWidth: '110px',
- },
- {
- label: 'Increase Fee',
- onClick: onIncreaseFee,
- icon: <ChevronsRightIcon color="stacks" variant="small" />,
- maxWidth: '110px',
- },
- ].map(({ label, onClick, icon, maxWidth }) => (
- <TransactionActionButton
- key={label}
- icon={icon}
- maxWidth={maxWidth}
- isEnabled={isOriginator && isPending}
- isSelected={isTransactionActionRoute}
- onClick={onClick}
- label={label}
- />
- ))}
- </HStack>
- );
+ const actionButtons = [
+ { label: 'Cancel', action: 'cancel' },
+ { label: 'Increase Fee', action: 'increaseFee' },
+ ].map(({ label, action }) => (
+ <TransactionActionButton
+ key={label}
+ icon={action === 'cancel' ? <CloseIcon color="stacks" variant="small" height={12} /> : <ChevronsRightIcon color="stacks" variant="small" />}
+ maxWidth="110px"
+ isEnabled={isOriginator && isPending}
+ isSelected={isTransactionActionRoute}
+ onClick={() => handleTransactionAction(action)}
+ label={label}
+ />
+ ));Committable suggestion was skipped due to low confidence.
| const onIncreaseFee = (): void => handleTransactionAction('increaseFee'); | ||
| const onCancelTransaction = (): void => handleTransactionAction('cancel'); | ||
|
|
There was a problem hiding this comment.
Simplify the function calls for transaction actions to enhance readability and maintainability.
- const onIncreaseFee = (): void => handleTransactionAction('increaseFee');
- const onCancelTransaction = (): void => handleTransactionAction('cancel');
+ const onIncreaseFee = () => handleTransactionAction('increaseFee');
+ const onCancelTransaction = () => handleTransactionAction('cancel');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 onIncreaseFee = (): void => handleTransactionAction('increaseFee'); | |
| const onCancelTransaction = (): void => handleTransactionAction('cancel'); | |
| const onIncreaseFee = () => handleTransactionAction('increaseFee'); | |
| const onCancelTransaction = () => handleTransactionAction('cancel'); |
Aman-zishan
force-pushed
the
fix/5501/rework
branch
from
d031e11 to
f43e50a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx
alter-eggo
requested review from
kyranjamie,
pete-watters,
fbwoolf and
alter-eggo
|
Thanks @Aman-zishan . We value your work and will check it soon and provide feedback |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (2 hunks)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (5 hunks)
- src/app/components/stacks-transaction-item/transaction-action-button.tsx (1 hunks)
- src/app/components/transaction-item/transaction-item.layout.tsx (3 hunks)
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (8)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx
- src/app/components/transaction-item/transaction-item.layout.tsx
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx
Additional comments not posted (5)
src/app/components/stacks-transaction-item/transaction-action-button.tsx (2)
3-10: Ensure consistent naming and usage of theActionButtonPropsinterface.The interface
ActionButtonPropsis well-defined with optional and required properties. This approach is good for flexibility and clarity in component usage.
13-39: Review theTransactionActionButtoncomponent for logic and syntax correctness.The component correctly handles the
isActivestate based onisEnabledandisSelected. The conditional rendering and event handling are implemented properly. The use of styled components for styling is consistent with modern React practices.src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (3)
26-26: Properly integrated theTransactionActionButtoncomponent.The import statement is correct and follows the project's structure for importing sibling components.
80-84: Check the usage of theTransactionActionButtonwithin theBitcoinTransactionItemcomponent.The
TransactionActionButtonis used effectively to conditionally render the "Increase fee" button based on the transaction's status and originator properties. TheisEnabledandisSelectedproperties are well-utilized to control the button's state.
91-91: Ensure that thebottomElementprop ofTransactionItemLayoutis used correctly.The conditional rendering logic for
bottomElementis correctly implemented. It only renders theTransactionActionButtonwhen it is enabled, thus maintaining UI consistency and user expectations.
Aman-zishan
force-pushed
the
fix/5501/rework
branch
2 times, most recently
from
7dacb81 to
52a9798
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
src/app/components/stacks-transaction-item/transaction-action-button.tsx (1)
3-10: Ensure that all properties ofActionButtonPropsare documented for clarity, especially sinceisEnabledandisSelectedcan have significant effects on the component's behavior.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- src/app/common/utils/get-burn-address.ts (1 hunks)
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (2 hunks)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (5 hunks)
- src/app/components/stacks-transaction-item/transaction-action-button.tsx (1 hunks)
- src/app/components/transaction-item/transaction-item.layout.tsx (3 hunks)
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx (2 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (1 hunks)
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4 hunks)
- src/app/routes/app-routes.tsx (2 hunks)
- src/shared/route-urls.ts (1 hunks)
Files skipped from review as they are similar to previous changes (13)
- src/app/common/utils/get-burn-address.ts
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx
- src/app/components/transaction-item/transaction-item.layout.tsx
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx
- src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx
Additional comments not posted (2)
src/app/routes/app-routes.tsx (2)
19-21: The integration of new dialog components for STX and BTC transactions is well done. Ensure they are correctly used in the UI and that their states are managed appropriately.
108-114: The routing setup forCancelStxTransactionDialogis correctly nested within transaction signing routes, which ensures that the necessary context is maintained. However, verify that theBroadcastErrorcomponent is appropriately used and styled for this context.Verification successful
The
rgcommand successfully found instances ofBroadcastErrorin.tsfiles, but failed to recognize thetsxfile type. The results show multiple usages and imports ofBroadcastErroracross the codebase, including in thesrc/app/routes/app-routes.tsxfile. However, the output does not provide specific information about the styling of theBroadcastErrorcomponent.To ensure thorough verification, we need to gather more detailed information about the implementation and styling of the
BroadcastErrorcomponent, specifically in thesrc/app/pages/send/broadcast-error/broadcast-error.tsxfile.
The
BroadcastErrorcomponent is implemented insrc/app/pages/send/broadcast-error/broadcast-error.tsxand utilizes theBroadcastErrorLayoutfromsrc/app/pages/send/broadcast-error/components/broadcast-error.layout.tsx. The layout includes various styling elements ensuring a consistent and appropriate design.The integration of
BroadcastErrorwithin theCancelStxTransactionDialogroute appears correct, and the styling withinBroadcastErrorLayoutis consistent with the application's design language.
src/app/pages/send/broadcast-error/broadcast-error.tsx: Implementation ofBroadcastError.src/app/pages/send/broadcast-error/components/broadcast-error.layout.tsx: Implementation ofBroadcastErrorLayout.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration and styling of `BroadcastError` within `CancelStxTransactionDialog`. # Test: Search for the component usage and styling. Expect: Consistent integration and appropriate styling. rg --type typescript $'BroadcastError'Length of output: 72
Script:
#!/bin/bash # Description: Verify the integration and styling of `BroadcastError` within `CancelStxTransactionDialog`. # Test: Search for the component usage and styling. Expect: Consistent integration and appropriate styling. rg --type ts,tsx 'BroadcastError'Length of output: 64
Script:
#!/bin/bash # Description: Verify the integration and styling of `BroadcastError` within `CancelStxTransactionDialog`. # Test: Search for the component usage and styling. Expect: Consistent integration and appropriate styling. rg --type ts --type tsx 'BroadcastError'Length of output: 68
Script:
#!/bin/bash # Description: Verify the integration and styling of `BroadcastError` within `CancelStxTransactionDialog`. # Test: Search for the component usage and styling. Expect: Consistent integration and appropriate styling. rg --type ts 'BroadcastError' rg --type tsx 'BroadcastError'Length of output: 6270
Script:
#!/bin/bash # Description: Gather detailed information about the implementation and styling of `BroadcastError`. # Locate the `BroadcastError` component implementation. ast-grep --lang tsx --pattern 'export function BroadcastError($_) { $$$ }' # Locate the `BroadcastErrorLayout` component implementation, as it is used within `BroadcastError`. ast-grep --lang tsx --pattern 'export function BroadcastErrorLayout($_) { $$$ }'Length of output: 4733
| if (!isActive) return null; | ||
|
|
||
| return ( | ||
| <styled.button | ||
| _hover={{ color: 'ink.text-subdued' }} | ||
| bg="ink.background-primary" | ||
| maxWidth="110px" | ||
| maxWidth={maxWidth} | ||
| ml="auto" | ||
| onClick={e => { | ||
| onIncreaseFee(); | ||
| onButtonClick(); | ||
| e.stopPropagation(); | ||
| }} | ||
| opacity={!isActive ? 0 : 1} | ||
| pointerEvents={!isActive ? 'none' : 'all'} | ||
| position="relative" | ||
| px="space.02" | ||
| py="space.01" | ||
| rounded="xs" | ||
| zIndex={999} | ||
| > | ||
| <HStack gap="space.01"> | ||
| <ChevronsRightIcon color="stacks" variant="small" /> | ||
| <styled.span textStyle="label.03">Increase fee</styled.span> | ||
| {icon} | ||
| <styled.span textStyle="label.03" color={textColor}> | ||
| {label} | ||
| </styled.span> |
There was a problem hiding this comment.
The implementation of TransactionActionButton is clean and functional. However, consider handling cases where onButtonClick might be undefined to prevent runtime errors.
- onButtonClick();
+ onButtonClick?.();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.
| if (!isActive) return null; | |
| return ( | |
| <styled.button | |
| _hover={{ color: 'ink.text-subdued' }} | |
| bg="ink.background-primary" | |
| maxWidth="110px" | |
| maxWidth={maxWidth} | |
| ml="auto" | |
| onClick={e => { | |
| onIncreaseFee(); | |
| onButtonClick(); | |
| e.stopPropagation(); | |
| }} | |
| opacity={!isActive ? 0 : 1} | |
| pointerEvents={!isActive ? 'none' : 'all'} | |
| position="relative" | |
| px="space.02" | |
| py="space.01" | |
| rounded="xs" | |
| zIndex={999} | |
| > | |
| <HStack gap="space.01"> | |
| <ChevronsRightIcon color="stacks" variant="small" /> | |
| <styled.span textStyle="label.03">Increase fee</styled.span> | |
| {icon} | |
| <styled.span textStyle="label.03" color={textColor}> | |
| {label} | |
| </styled.span> | |
| if (!isActive) return null; | |
| return ( | |
| <styled.button | |
| _hover={{ color: 'ink.text-subdued' }} | |
| bg="ink.background-primary" | |
| maxWidth={maxWidth} | |
| ml="auto" | |
| onClick={e => { | |
| onButtonClick?.(); | |
| e.stopPropagation(); | |
| }} | |
| position="relative" | |
| px="space.02" | |
| py="space.01" | |
| rounded="xs" | |
| zIndex={999} | |
| > | |
| <HStack gap="space.01"> | |
| {icon} | |
| <styled.span textStyle="label.03" color={textColor}> | |
| {label} | |
| </styled.span> |
| @@ -27,6 +27,7 @@ export enum RouteUrls { | |||
| FundChooseCurrency = '/fund-choose-currency', | |||
| IncreaseStxFee = '/increase-fee/stx/:txid', | |||
| IncreaseBtcFee = '/increase-fee/btc', | |||
| CancelStxTransaction = '/cancel-transaction/stx/:txid', | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The new route CancelStxTransaction is defined but not used anywhere else in the codebase. Please ensure this route is integrated into the application's routing and related components.
src/shared/route-urls.ts:CancelStxTransaction = '/cancel-transaction/stx/:txid'
Analysis chain
The addition of CancelStxTransaction to RouteUrls is consistent with the naming conventions and structure of existing routes. Ensure it is correctly integrated in all relevant parts of the application.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of `CancelStxTransaction` in the application's routing and related components.
# Test: Search for the route usage. Expect: Consistent usage across the application.
rg --type typescript $'CancelStxTransaction'
Length of output: 78
Script:
#!/bin/bash
# Description: Verify the usage of `CancelStxTransaction` in the application's routing and related components.
# Test: Search for the route usage. Expect: Consistent usage across the application.
rg --glob '*.ts' 'CancelStxTransaction'
Length of output: 122
Script:
#!/bin/bash
# Description: Verify the usage of `CancelStxTransaction` in the application's routing and related components.
# Test: Search for the route usage in routing and component files. Expect: Consistent usage across the application.
rg --glob '{src/routes/*.ts,src/components/*.ts}' 'CancelStxTransaction'
Length of output: 221
Script:
#!/bin/bash
# Description: Verify the usage of `CancelStxTransaction` in the application's routing and related components.
# Test: Search for the route usage in all TypeScript files. Expect: Consistent usage across the application.
rg --glob '**/*.ts' 'CancelStxTransaction'
Length of output: 125
|
@pete-watters Rebased and addressed the review comments. |
Aman-zishan
force-pushed
the
fix/5501/rework
branch
from
52a9798 to
1582c87
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- src/app/common/utils/get-burn-address.ts (1 hunks)
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (2 hunks)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (5 hunks)
- src/app/components/stacks-transaction-item/transaction-action-button.tsx (1 hunks)
- src/app/components/transaction-item/transaction-item.layout.tsx (3 hunks)
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx (2 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (1 hunks)
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4 hunks)
- src/app/routes/app-routes.tsx (2 hunks)
- src/shared/route-urls.ts (1 hunks)
Files skipped from review as they are similar to previous changes (16)
- src/app/common/utils/get-burn-address.ts
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx
- src/app/components/stacks-transaction-item/transaction-action-button.tsx
- src/app/components/transaction-item/transaction-item.layout.tsx
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx
- src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx
- src/app/routes/app-routes.tsx
- src/shared/route-urls.ts
Aman-zishan
force-pushed
the
fix/5501/rework
branch
from
1582c87 to
e69db87
Compare
Aman-zishan
force-pushed
the
fix/5501/rework
branch
from
8029708 to
5971f04
Compare
Aman-zishan
force-pushed
the
fix/5501/rework
branch
from
5971f04 to
85cc745
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- src/app/common/utils/get-burn-address.ts (1 hunks)
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (2 hunks)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (5 hunks)
- src/app/components/stacks-transaction-item/transaction-action-button.tsx (1 hunks)
- src/app/components/transaction-item/transaction-item.layout.tsx (2 hunks)
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx (2 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (1 hunks)
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4 hunks)
Files skipped from review due to trivial changes (1)
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx
Additional comments not posted (13)
src/app/common/utils/get-burn-address.ts (2)
8-9: Ensure accuracy of the burn address for Testnet.Similarly, the hardcoded burn address for Testnet should be verified to ensure it is correct and up-to-date.
6-7: Ensure accuracy of the burn address for Mainnet.The hardcoded burn address for Mainnet should be verified for correctness to avoid any critical issues in transaction processing.
src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1)
7-16: Review the structure and props ofIncreaseStxFeeDialog.The component is well-structured and uses clear, descriptive props. Ensure that the
useStxIncreaseFeehook andIncreaseFeeFieldcomponent are implemented correctly and handle all edge cases.src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1)
7-17: Review the structure and props ofCancelStxTransactionDialog.The component is well-structured and appropriately uses the
useStxCancelTransactionhook. TheisCancelprop is a good addition for conditional rendering within the dialog. Ensure that the hook handles the cancelation logic robustly.src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (1)
7-7: Review the props and functionality ofTransactionActions.The
TransactionActionscomponent is flexible and handles different states likeisLoadingandisBroadcasting. The use of Formik'shandleSubmitis appropriate here. Ensure that theonCancelfunction is implemented correctly and that it does not lead to any side effects.Also applies to: 13-13
src/app/components/stacks-transaction-item/transaction-action-button.tsx (1)
17-39: JSX structure and styling are well implemented.The conditional rendering and the structured layout using
HStackfor the icon and label are correctly implemented. The usage of styled components for dynamic styling based on props liketextColorandmaxWidthis also commendable.src/app/components/transaction-item/transaction-item.layout.tsx (1)
33-44: Layout changes enhance UI clarity.Switching from
HStacktoVStackfor the transaction caption improves the organization and readability of the content. The use of conditional rendering for the status and the handling of overflow and text truncation are well implemented.src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (1)
9-42: Logic for increasing transaction fee is correctly implemented.The hook uses a well-structured callback to handle the transaction fee increase. It correctly checks for the presence of the transaction and raw transaction data before proceeding. The use of
useCallbackwith appropriate dependencies ensures that the function is not unnecessarily recreated.src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (1)
18-53: Hook structure and integration of functionalities are commendable.The hook effectively integrates data fetching, transaction broadcasting, and validation schema setup. The conditional handling of transaction actions based on the
actionTypeand the use ofuseEffectfor user notifications are well implemented.src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (1)
18-76: Ensure error handling and validation inuseStxCancelTransaction.The
onSubmitfunction withinuseStxCancelTransactiondoes not handle potential errors that might occur during the transaction generation or broadcasting processes. Also, the function lacks validation for thefeevalue which could lead to incorrect transaction fees being set. Consider adding error handling and validation to improve robustness and user experience.async (values: any) => { + if (!values.fee || isNaN(values.fee)) { + throw new Error("Invalid fee value"); + } if (!tx || !rawTx || !account) return; + try { const newTx = await generateUnsignedTransaction(options); const txId = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); await refreshAccountData(); submittedTransactionsActions.transactionReplacedByFee(txId); await stacksBroadcastTransaction(newTx); + } catch (error) { + console.error("Failed to submit transaction:", error); + throw error; // Or handle the error based on your application's error handling policy + } },src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (2)
77-81: Review logic forisEnabledproperty inTransactionActionButton.The
isEnabledproperty is determined by several conditions including transaction confirmation and input types. Ensure that these conditions are correctly encapsulating all scenarios where the button should be active. If there are more conditions affecting the button's state, they should be explicitly handled here.Verification successful
Review logic for
isEnabledproperty inTransactionActionButton.The
isEnabledproperty inbitcoin-transaction-item.tsxis determined by the conditions:isOriginator && !transaction.status.confirmed && !containsTaprootInput(transaction). These conditions ensure that the button is enabled only if the user is the originator of the transaction, the transaction is not confirmed, and the transaction does not contain a Taproot input. This logic appears to encapsulate the necessary scenarios for enabling the button.
src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx:const isEnabled = isOriginator && !transaction.status.confirmed && !containsTaprootInput(transaction);Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the conditions for enabling the TransactionActionButton in bitcoin-transaction-item.tsx. # Test: Search for the logic that determines the `isEnabled` property. Expect: Comprehensive conditions covering all scenarios. rg --type typescript --multiline $'const isEnabled =\n\s*isOriginator && !transaction.status.confirmed && !containsTaprootInput\(transaction\);'Length of output: 179
Script:
#!/bin/bash # Correcting the file type and searching for the logic that determines the `isEnabled` property in the relevant file. rg --multiline --type ts $'const isEnabled =\n\s*isOriginator && !transaction.status.confirmed && !containsTaprootInput\(transaction\);'Length of output: 391
24-24: Correct import path forTransactionActionButton.The import path for
TransactionActionButtonseems incorrect as it references a component from 'stacks-transaction-item' which might not be appropriate for a Bitcoin transaction context. Verify if this is the intended usage or if a more specific button component should be used for Bitcoin transactions.- import { TransactionActionButton } from '../stacks-transaction-item/transaction-action-button'; + import { BitcoinTransactionActionButton } from './bitcoin-transaction-action-button'; // Assuming a separate component for Bitcoin transactions existssrc/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (1)
31-31: Ensure robust handling of transaction types inuseStacksBroadcastTransaction.The logic for handling different types of transactions (increase fee, cancel) has been expanded. It's crucial to ensure that this logic robustly handles scenarios where both flags might be mistakenly set or neither is set. Consider adding assertions or warnings if unexpected combinations of flags are used.
+ if (isIncreaseFeeTransaction && isCancelTransaction) { + console.warn("Both increase fee and cancel transaction flags are set. Please check the logic."); + } if (isIncreaseFeeTransaction || isCancelTransaction) { navigate(RouteUrls.Activity); return; }Also applies to: 38-38, 65-105
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range and nitpick comments (1)
src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (1)
Line range hint
24-81: Refactor to improve readability and maintainability.The function
BitcoinTransactionItemis well-structured, but the logic to determine theisEnabledstate (lines 68-69) could be encapsulated into a separate function or hook to improve readability and maintainability. This separation would make the component easier to test and modify in the future.+ const determineIfTransactionCanBeIncreased = (transaction, bitcoinAddress) => { + return !isBitcoinTxInbound(bitcoinAddress, transaction) && + !transaction.status.confirmed && + !containsTaprootInput(transaction); + }; + const isEnabled = - isOriginator && !transaction.status.confirmed && !containsTaprootInput(transaction); + determineIfTransactionCanBeIncreased(transaction, bitcoinAddress);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- src/app/common/utils/get-burn-address.ts (1 hunks)
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (2 hunks)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (5 hunks)
- src/app/components/stacks-transaction-item/transaction-action-button.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx (2 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (1 hunks)
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4 hunks)
Files skipped from review due to trivial changes (1)
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx
Additional comments not posted (10)
src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1)
7-17: Well-structured and clear implementation ofIncreaseStxFeeDialog.The component is cleanly implemented, making effective use of props and context to render the
StxTransactionActionDialogwith appropriate configurations. The use of hooks and component composition adheres to React best practices.src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1)
7-18: Well-implementedCancelStxTransactionDialogcomponent.The component is effectively structured, leveraging the
StxTransactionActionDialogwith appropriate props and hooks. The addition of theisCancelprop is a thoughtful detail that enhances the component's specificity and clarity in its role.src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx (2)
23-23: Updated imports and hook usageThe addition of
TransactionActionsanduseBtcIncreaseFeehooks aligns with the PR's objective to refactor transaction actions. This change should help in managing transaction-related actions more effectively.
72-72: Refactored transaction action handlingThe integration of
TransactionActionswithin theFootercomponent is a good use of the newly imported component, providing a centralized way to manage transaction actions. This should enhance the modularity and maintainability of the code.src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4)
31-31: AddedisCancelTransactionflagAdding the
isCancelTransactionflag to the hook's interface is crucial for supporting the new functionality of canceling transactions. This change is well-aligned with the PR's objective to handle transaction cancellations.
38-38: Updated function signature to include cancellation flagThe inclusion of
isCancelTransactionin the function signature allows the hook to conditionally handle transaction cancellations, which is a significant enhancement in functionality.
65-65: Conditional navigation logic for transaction actionsThe conditional logic to navigate to the appropriate route based on whether the transaction is an increase fee or cancellation is a good implementation detail that enhances user experience by directing them to relevant pages after actions are performed.
103-105: Toast messages for transaction cancellationImplementing specific toast messages for transaction cancellation provides clear user feedback, which is essential for good user experience. This change ensures users are well-informed about the state of their transactions.
src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (1)
1-1: Refactored transaction item with improved routing and action handlingThe changes in this file introduce improved routing with
useMatchfor specific transaction actions and enhance the transaction item component's capability to handle actions like fee increase and cancellation. This is well-aligned with the PR's objectives to enhance transaction handling and user interface interactions.Also applies to: 27-27, 36-39, 63-66, 77-81, 101-101
src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (1)
1-1: Comprehensive dialog implementation for STX transaction actionsThe implementation of
StxTransactionActionDialogis comprehensive, handling various aspects such as form initialization, user interactions, and displaying transaction details. The use of hooks for action handling and dynamic fee adjustments is particularly noteworthy, aligning well with the PR's objectives to provide a robust user interface for transaction management.Also applies to: 26-26, 50-138
| export function TransactionActionButton(props: ActionButtonProps) { | ||
| const { isEnabled, isSelected, onButtonClick, label, icon, textColor, maxWidth } = props; | ||
| const isActive = isEnabled && !isSelected; | ||
|
|
||
| if (!isActive) return null; | ||
|
|
||
| return ( | ||
| <styled.button | ||
| _hover={{ color: 'ink.text-subdued' }} | ||
| bg="ink.background-primary" | ||
| maxWidth="110px" | ||
| maxWidth={maxWidth} | ||
| ml="auto" | ||
| onClick={e => { | ||
| onIncreaseFee(); | ||
| onButtonClick(); | ||
| e.stopPropagation(); | ||
| }} | ||
| opacity={!isActive ? 0 : 1} | ||
| pointerEvents={!isActive ? 'none' : 'all'} | ||
| position="relative" | ||
| px="space.02" | ||
| py="space.01" | ||
| rounded="xs" | ||
| zIndex={999} | ||
| > | ||
| <HStack gap="space.01"> | ||
| <ChevronsRightIcon color="stacks" variant="small" /> | ||
| <styled.span textStyle="label.03">Increase fee</styled.span> | ||
| {icon} | ||
| <styled.span textStyle="label.03" color={textColor}> | ||
| {label} | ||
| </styled.span> |
There was a problem hiding this comment.
Ensure robust handling of potentially undefined onButtonClick.
The implementation of TransactionActionButton is clean and functional. However, consider handling cases where onButtonClick might be undefined to prevent runtime errors. This was previously mentioned and should be addressed to enhance the component's robustness.
- onButtonClick();
+ onButtonClick?.();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.
| export function TransactionActionButton(props: ActionButtonProps) { | |
| const { isEnabled, isSelected, onButtonClick, label, icon, textColor, maxWidth } = props; | |
| const isActive = isEnabled && !isSelected; | |
| if (!isActive) return null; | |
| return ( | |
| <styled.button | |
| _hover={{ color: 'ink.text-subdued' }} | |
| bg="ink.background-primary" | |
| maxWidth="110px" | |
| maxWidth={maxWidth} | |
| ml="auto" | |
| onClick={e => { | |
| onIncreaseFee(); | |
| onButtonClick(); | |
| e.stopPropagation(); | |
| }} | |
| opacity={!isActive ? 0 : 1} | |
| pointerEvents={!isActive ? 'none' : 'all'} | |
| position="relative" | |
| px="space.02" | |
| py="space.01" | |
| rounded="xs" | |
| zIndex={999} | |
| > | |
| <HStack gap="space.01"> | |
| <ChevronsRightIcon color="stacks" variant="small" /> | |
| <styled.span textStyle="label.03">Increase fee</styled.span> | |
| {icon} | |
| <styled.span textStyle="label.03" color={textColor}> | |
| {label} | |
| </styled.span> | |
| export function TransactionActionButton(props: ActionButtonProps) { | |
| const { isEnabled, isSelected, onButtonClick, label, icon, textColor, maxWidth } = props; | |
| const isActive = isEnabled && !isSelected; | |
| if (!isActive) return null; | |
| return ( | |
| <styled.button | |
| _hover={{ color: 'ink.text-subdued' }} | |
| bg="ink.background-primary" | |
| maxWidth={maxWidth} | |
| ml="auto" | |
| onClick={e => { | |
| onButtonClick?.(); | |
| e.stopPropagation(); | |
| }} | |
| position="relative" | |
| px="space.02" | |
| py="space.01" | |
| rounded="xs" | |
| zIndex={999} | |
| > | |
| <HStack gap="space.01"> | |
| {icon} | |
| <styled.span textStyle="label.03" color={textColor}> | |
| {label} | |
| </styled.span> |
| export function useStxIncreaseFee(txid: string | undefined) { | ||
| const { | ||
| tx, | ||
| rawTx, | ||
| isLoadingTx, | ||
| isLoadingRawTx, | ||
| refreshAccountData, | ||
| submittedTransactionsActions, | ||
| stacksBroadcastTransaction, | ||
| availableUnlockedBalance, | ||
| validationSchema, | ||
| } = useStxTransactionActions(txid, 'increaseFee'); | ||
|
|
||
| const onSubmit = useCallback( | ||
| async (values: any, rawTx?: StacksTransaction) => { | ||
| if (!tx || !rawTx) return; | ||
| rawTx.setFee(stxToMicroStx(values.fee).toString()); | ||
| const txid = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); | ||
| await refreshAccountData(); | ||
| submittedTransactionsActions.transactionReplacedByFee(txid); | ||
| await stacksBroadcastTransaction(rawTx); | ||
| }, | ||
| [tx, refreshAccountData, submittedTransactionsActions, stacksBroadcastTransaction] | ||
| ); | ||
|
|
||
| return { | ||
| availableUnlockedBalance, | ||
| isLoadingRawTx, | ||
| isLoadingTx, | ||
| onSubmit, | ||
| rawTx, | ||
| tx, | ||
| validationSchema, | ||
| }; |
There was a problem hiding this comment.
Consider adding error handling and type safety.
The function useStxIncreaseFee handles transaction fee updates but lacks error handling for asynchronous operations (lines 24-29). Consider wrapping these operations in a try-catch block to handle potential exceptions gracefully.
Additionally, the parameter values in the onSubmit function lacks a specific type definition, which could lead to runtime errors if unexpected data types are passed. Define a more specific type than any to improve type safety and maintainability.
- async (values: any, rawTx?: StacksTransaction) => {
+ async (values: { fee: number }, rawTx?: StacksTransaction) => {
+ try {
if (!tx || !rawTx) return;
rawTx.setFee(stxToMicroStx(values.fee).toString());
const txid = tx.tx_id || safelyFormatHexTxid(rawTx.txid());
await refreshAccountData();
submittedTransactionsActions.transactionReplacedByFee(txid);
await stacksBroadcastTransaction(rawTx);
+ } catch (error) {
+ // Handle errors appropriately
+ }
},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.
| export function useStxIncreaseFee(txid: string | undefined) { | |
| const { | |
| tx, | |
| rawTx, | |
| isLoadingTx, | |
| isLoadingRawTx, | |
| refreshAccountData, | |
| submittedTransactionsActions, | |
| stacksBroadcastTransaction, | |
| availableUnlockedBalance, | |
| validationSchema, | |
| } = useStxTransactionActions(txid, 'increaseFee'); | |
| const onSubmit = useCallback( | |
| async (values: any, rawTx?: StacksTransaction) => { | |
| if (!tx || !rawTx) return; | |
| rawTx.setFee(stxToMicroStx(values.fee).toString()); | |
| const txid = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); | |
| await refreshAccountData(); | |
| submittedTransactionsActions.transactionReplacedByFee(txid); | |
| await stacksBroadcastTransaction(rawTx); | |
| }, | |
| [tx, refreshAccountData, submittedTransactionsActions, stacksBroadcastTransaction] | |
| ); | |
| return { | |
| availableUnlockedBalance, | |
| isLoadingRawTx, | |
| isLoadingTx, | |
| onSubmit, | |
| rawTx, | |
| tx, | |
| validationSchema, | |
| }; | |
| export function useStxIncreaseFee(txid: string | undefined) { | |
| const { | |
| tx, | |
| rawTx, | |
| isLoadingTx, | |
| isLoadingRawTx, | |
| refreshAccountData, | |
| submittedTransactionsActions, | |
| stacksBroadcastTransaction, | |
| availableUnlockedBalance, | |
| validationSchema, | |
| } = useStxTransactionActions(txid, 'increaseFee'); | |
| const onSubmit = useCallback( | |
| async (values: { fee: number }, rawTx?: StacksTransaction) => { | |
| try { | |
| if (!tx || !rawTx) return; | |
| rawTx.setFee(stxToMicroStx(values.fee).toString()); | |
| const txid = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); | |
| await refreshAccountData(); | |
| submittedTransactionsActions.transactionReplacedByFee(txid); | |
| await stacksBroadcastTransaction(rawTx); | |
| } catch (error) { | |
| // Handle errors appropriately | |
| } | |
| }, | |
| [tx, refreshAccountData, submittedTransactionsActions, stacksBroadcastTransaction] | |
| ); | |
| return { | |
| availableUnlockedBalance, | |
| isLoadingRawTx, | |
| isLoadingTx, | |
| onSubmit, | |
| rawTx, | |
| tx, | |
| validationSchema, | |
| }; | |
| } |
| export function useStxTransactionActions( | ||
| txid: string | undefined, | ||
| actionType: 'cancel' | 'increaseFee' | ||
| ) { | ||
| const { data: tx, isLoading: isLoadingTx } = useTransactionById(txid || ''); | ||
| const toast = useToast(); | ||
| const refreshAccountData = useRefreshAllAccountData(); | ||
| const stxAddress = useCurrentStacksAccountAddress(); | ||
| const availableUnlockedBalance = useStxAvailableUnlockedBalance(stxAddress); | ||
| const submittedTransactionsActions = useSubmittedTransactionsActions(); | ||
| const { isLoadingRawTx, rawTx } = useStacksRawTransaction(txid || ''); | ||
| const { stacksBroadcastTransaction } = useStacksBroadcastTransaction({ | ||
| token: 'STX', | ||
| isCancelTransaction: actionType === 'cancel', | ||
| isIncreaseFeeTransaction: actionType === 'increaseFee', | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| if (tx && tx.tx_status !== 'pending') { | ||
| toast.info('Your transaction went through!.'); | ||
| } | ||
| }, [toast, tx, tx?.tx_status]); | ||
|
|
||
| const validationSchema = yup.object({ fee: stxFeeValidator(availableUnlockedBalance) }); | ||
|
|
||
| return { | ||
| tx, | ||
| rawTx, | ||
| isLoadingTx, | ||
| isLoadingRawTx, | ||
| refreshAccountData, | ||
| submittedTransactionsActions, | ||
| stacksBroadcastTransaction, | ||
| availableUnlockedBalance, | ||
| validationSchema, | ||
| }; |
There was a problem hiding this comment.
Enhance the use of useEffect for transaction status updates.
The useEffect hook in lines 35-39 is used to display a toast message when a transaction status changes. However, the dependency array includes tx?.tx_status which might cause unnecessary re-renders if other properties of tx change. It's more efficient to track only the tx_status in a separate state and update it conditionally to minimize re-renders.
+ const [status, setStatus] = useState(tx?.tx_status);
+
useEffect(() => {
- if (tx && tx.tx_status !== 'pending') {
+ if (status !== 'pending') {
toast.info('Your transaction went through!.');
}
- }, [toast, tx, tx?.tx_status]);
+ }, [toast, status]);
+
+ useEffect(() => {
+ if (tx?.tx_status !== status) {
+ setStatus(tx?.tx_status);
+ }
+ }, [tx?.tx_status]);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.
| export function useStxTransactionActions( | |
| txid: string | undefined, | |
| actionType: 'cancel' | 'increaseFee' | |
| ) { | |
| const { data: tx, isLoading: isLoadingTx } = useTransactionById(txid || ''); | |
| const toast = useToast(); | |
| const refreshAccountData = useRefreshAllAccountData(); | |
| const stxAddress = useCurrentStacksAccountAddress(); | |
| const availableUnlockedBalance = useStxAvailableUnlockedBalance(stxAddress); | |
| const submittedTransactionsActions = useSubmittedTransactionsActions(); | |
| const { isLoadingRawTx, rawTx } = useStacksRawTransaction(txid || ''); | |
| const { stacksBroadcastTransaction } = useStacksBroadcastTransaction({ | |
| token: 'STX', | |
| isCancelTransaction: actionType === 'cancel', | |
| isIncreaseFeeTransaction: actionType === 'increaseFee', | |
| }); | |
| useEffect(() => { | |
| if (tx && tx.tx_status !== 'pending') { | |
| toast.info('Your transaction went through!.'); | |
| } | |
| }, [toast, tx, tx?.tx_status]); | |
| const validationSchema = yup.object({ fee: stxFeeValidator(availableUnlockedBalance) }); | |
| return { | |
| tx, | |
| rawTx, | |
| isLoadingTx, | |
| isLoadingRawTx, | |
| refreshAccountData, | |
| submittedTransactionsActions, | |
| stacksBroadcastTransaction, | |
| availableUnlockedBalance, | |
| validationSchema, | |
| }; | |
| export function useStxTransactionActions( | |
| txid: string | undefined, | |
| actionType: 'cancel' | 'increaseFee' | |
| ) { | |
| const { data: tx, isLoading: isLoadingTx } = useTransactionById(txid || ''); | |
| const toast = useToast(); | |
| const refreshAccountData = useRefreshAllAccountData(); | |
| const stxAddress = useCurrentStacksAccountAddress(); | |
| const availableUnlockedBalance = useStxAvailableUnlockedBalance(stxAddress); | |
| const submittedTransactionsActions = useSubmittedTransactionsActions(); | |
| const { isLoadingRawTx, rawTx } = useStacksRawTransaction(txid || ''); | |
| const { stacksBroadcastTransaction } = useStacksBroadcastTransaction({ | |
| token: 'STX', | |
| isCancelTransaction: actionType === 'cancel', | |
| isIncreaseFeeTransaction: actionType === 'increaseFee', | |
| }); | |
| const [status, setStatus] = useState(tx?.tx_status); | |
| useEffect(() => { | |
| if (status !== 'pending') { | |
| toast.info('Your transaction went through!.'); | |
| } | |
| }, [toast, status]); | |
| useEffect(() => { | |
| if (tx?.tx_status !== status) { | |
| setStatus(tx?.tx_status); | |
| } | |
| }, [tx?.tx_status]); | |
| const validationSchema = yup.object({ fee: stxFeeValidator(availableUnlockedBalance) }); | |
| return { | |
| tx, | |
| rawTx, | |
| isLoadingTx, | |
| isLoadingRawTx, | |
| refreshAccountData, | |
| submittedTransactionsActions, | |
| stacksBroadcastTransaction, | |
| availableUnlockedBalance, | |
| validationSchema, | |
| }; | |
| } |
| export function useStxCancelTransaction(txid: string | undefined) { | ||
| const { | ||
| tx, | ||
| rawTx, | ||
| isLoadingTx, | ||
| isLoadingRawTx, | ||
| refreshAccountData, | ||
| submittedTransactionsActions, | ||
| stacksBroadcastTransaction, | ||
| availableUnlockedBalance, | ||
| validationSchema, | ||
| } = useStxTransactionActions(txid, 'cancel'); | ||
|
|
||
| const network = useCurrentStacksNetworkState(); | ||
| const account = useCurrentStacksAccount(); | ||
|
|
||
| const onSubmit = useCallback( | ||
| async (values: any) => { | ||
| if (!tx || !rawTx || !account) return; | ||
|
|
||
| const options: GenerateUnsignedTransactionOptions = { | ||
| publicKey: account.stxPublicKey, | ||
| nonce: tx.nonce, | ||
| fee: stxToMicroStx(values?.fee || 0).toNumber(), | ||
| txData: { | ||
| txType: TransactionTypes.STXTransfer, | ||
| recipient: getBurnAddress(network), | ||
| amount: 1, | ||
| memo: '_cancel transaction', | ||
| network: network, | ||
| } as any, | ||
| }; | ||
| const newTx = await generateUnsignedTransaction(options); | ||
| const txId = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); | ||
| await refreshAccountData(); | ||
| submittedTransactionsActions.transactionReplacedByFee(txId); | ||
| await stacksBroadcastTransaction(newTx); | ||
| }, | ||
| [ | ||
| tx, | ||
| rawTx, | ||
| account, | ||
| network, | ||
| refreshAccountData, | ||
| submittedTransactionsActions, | ||
| stacksBroadcastTransaction, | ||
| ] | ||
| ); | ||
|
|
||
| return { | ||
| availableUnlockedBalance, | ||
| isLoadingRawTx, | ||
| isLoadingTx, | ||
| onSubmit, | ||
| rawTx, | ||
| tx, | ||
| validationSchema, | ||
| }; |
There was a problem hiding this comment.
Improve type safety and error handling in the cancellation process.
The function useStxCancelTransaction performs a critical operation but lacks robust type safety and error handling. The values parameter in the onSubmit function uses any, which is unsafe. Also, consider handling potential exceptions in asynchronous operations to ensure the application's stability and provide feedback to the user.
Additionally, the transaction type is hardcoded as TransactionTypes.STXTransfer in line 43, which might not always be appropriate for a cancellation. Ensure this aligns with the intended transaction types for cancellations.
- async (values: any) => {
+ async (values: { fee?: number }) => {
+ try {
if (!tx || !rawTx || !account) return;
const options: GenerateUnsignedTransactionOptions = {
publicKey: account.stxPublicKey,
nonce: tx.nonce,
fee: stxToMicroStx(values?.fee || 0).toNumber(),
txData: {
txType: TransactionTypes.STXTransfer, // Verify if this should be dynamic
recipient: getBurnAddress(network),
amount: 1,
memo: '_cancel transaction',
network: network,
} as any,
};
const newTx = await generateUnsignedTransaction(options);
const txId = tx.tx_id || safelyFormatHexTxid(rawTx.txid());
await refreshAccountData();
submittedTransactionsActions.transactionReplacedByFee(txId);
await stacksBroadcastTransaction(newTx);
+ } catch (error) {
+ // Handle errors appropriately
+ }
},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.
| export function useStxCancelTransaction(txid: string | undefined) { | |
| const { | |
| tx, | |
| rawTx, | |
| isLoadingTx, | |
| isLoadingRawTx, | |
| refreshAccountData, | |
| submittedTransactionsActions, | |
| stacksBroadcastTransaction, | |
| availableUnlockedBalance, | |
| validationSchema, | |
| } = useStxTransactionActions(txid, 'cancel'); | |
| const network = useCurrentStacksNetworkState(); | |
| const account = useCurrentStacksAccount(); | |
| const onSubmit = useCallback( | |
| async (values: any) => { | |
| if (!tx || !rawTx || !account) return; | |
| const options: GenerateUnsignedTransactionOptions = { | |
| publicKey: account.stxPublicKey, | |
| nonce: tx.nonce, | |
| fee: stxToMicroStx(values?.fee || 0).toNumber(), | |
| txData: { | |
| txType: TransactionTypes.STXTransfer, | |
| recipient: getBurnAddress(network), | |
| amount: 1, | |
| memo: '_cancel transaction', | |
| network: network, | |
| } as any, | |
| }; | |
| const newTx = await generateUnsignedTransaction(options); | |
| const txId = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); | |
| await refreshAccountData(); | |
| submittedTransactionsActions.transactionReplacedByFee(txId); | |
| await stacksBroadcastTransaction(newTx); | |
| }, | |
| [ | |
| tx, | |
| rawTx, | |
| account, | |
| network, | |
| refreshAccountData, | |
| submittedTransactionsActions, | |
| stacksBroadcastTransaction, | |
| ] | |
| ); | |
| return { | |
| availableUnlockedBalance, | |
| isLoadingRawTx, | |
| isLoadingTx, | |
| onSubmit, | |
| rawTx, | |
| tx, | |
| validationSchema, | |
| }; | |
| export function useStxCancelTransaction(txid: string | undefined) { | |
| const { | |
| tx, | |
| rawTx, | |
| isLoadingTx, | |
| isLoadingRawTx, | |
| refreshAccountData, | |
| submittedTransactionsActions, | |
| stacksBroadcastTransaction, | |
| availableUnlockedBalance, | |
| validationSchema, | |
| } = useStxTransactionActions(txid, 'cancel'); | |
| const network = useCurrentStacksNetworkState(); | |
| const account = useCurrentStacksAccount(); | |
| const onSubmit = useCallback( | |
| async (values: { fee?: number }) => { | |
| try { | |
| if (!tx || !rawTx || !account) return; | |
| const options: GenerateUnsignedTransactionOptions = { | |
| publicKey: account.stxPublicKey, | |
| nonce: tx.nonce, | |
| fee: stxToMicroStx(values?.fee || 0).toNumber(), | |
| txData: { | |
| txType: TransactionTypes.STXTransfer, // Verify if this should be dynamic | |
| recipient: getBurnAddress(network), | |
| amount: 1, | |
| memo: '_cancel transaction', | |
| network: network, | |
| } as any, | |
| }; | |
| const newTx = await generateUnsignedTransaction(options); | |
| const txId = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); | |
| await refreshAccountData(); | |
| submittedTransactionsActions.transactionReplacedByFee(txId); | |
| await stacksBroadcastTransaction(newTx); | |
| } catch (error) { | |
| // Handle errors appropriately | |
| } | |
| }, | |
| [ | |
| tx, | |
| rawTx, | |
| account, | |
| network, | |
| refreshAccountData, | |
| submittedTransactionsActions, | |
| stacksBroadcastTransaction, | |
| ] | |
| ); | |
| return { | |
| availableUnlockedBalance, | |
| isLoadingRawTx, | |
| isLoadingTx, | |
| onSubmit, | |
| rawTx, | |
| tx, | |
| validationSchema, | |
| }; | |
| } |
Aman-zishan
force-pushed
the
fix/5501/rework
branch
from
85cc745 to
0830428
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- src/app/common/utils/get-burn-address.ts (1 hunks)
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (2 hunks)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (5 hunks)
- src/app/components/stacks-transaction-item/transaction-action-button.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx (2 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (1 hunks)
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4 hunks)
- src/app/routes/app-routes.tsx (2 hunks)
- src/shared/route-urls.ts (1 hunks)
Files skipped from review as they are similar to previous changes (12)
- src/app/common/utils/get-burn-address.ts
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx
- src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx
Additional comments not posted (3)
src/app/components/stacks-transaction-item/transaction-action-button.tsx (1)
13-26: Ensure robust handling of potentially undefinedonButtonClick.The implementation of
TransactionActionButtonis clean and functional. However, consider handling cases whereonButtonClickmight be undefined to prevent runtime errors. This was previously mentioned and should be addressed to enhance the component's robustness.- onButtonClick(); + onButtonClick?.();src/shared/route-urls.ts (1)
30-30: Ensure integration of the new route.The new route
CancelStxTransactionis defined but please ensure it is integrated into the application's routing and related components.Verification successful
The new route
CancelStxTransactionis properly integrated into the application's routing and related components.The route is used in the following locations:
src/app/routes/app-routes.tsx:
- Imported in the routes file.
- Defined as a path for a
Routecomponent.- Used in a nested path definition.
src/app/components/stacks-transaction-item/stacks-transaction-item.tsx:
- Used as a key in an object.
src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx:
- Used in the
CancelStxTransactionDialogcomponent.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `CancelStxTransaction` in the application's routing and related components. # Test: Search for the route usage. Expect: Consistent usage across the application. rg --type typescript $'CancelStxTransaction'Length of output: 78
Script:
#!/bin/bash # Description: Verify the usage of `CancelStxTransaction` in the application's routing and related components. # Test: Search for the route usage. Expect: Consistent usage across the application. rg --type ts 'CancelStxTransaction'Length of output: 893
src/app/routes/app-routes.tsx (1)
108-114: Correct implementation of new routes for STX transaction cancellation.The new routes for handling STX transaction cancellation are correctly implemented and integrated into the app's routing system. This includes the main route for the cancellation dialog and a nested route for handling broadcast errors. Good job on maintaining consistency with the existing routing structure.
|
@Aman-zishan can you add integration tests as well? 🙏 @kyranjamie can provide guidance |
Sure, i will work on that I was wondering if there are some mock functions for transaction RBF handling existing in the codebase so that i can write the integration test on a controlled environment, i think integration test for increase fee is very similar to cancel so i will try to include both in this PR |
Aman-zishan
force-pushed
the
fix/5501/rework
branch
from
0830428 to
be433c4
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (1)
src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (1)
Line range hint
24-81: Review theBitcoinTransactionItemcomponent for usability and logic.The
BitcoinTransactionItemcomponent handles navigation and transaction details effectively. However, consider adding error handling for asynchronous operations likeopenTxLink. Also, ensure that the component gracefully handles cases wheretransactionmight be null or undefined at the start.The use of
isEnabledfor controlling the display of theincreaseFeeButtonis a good practice, ensuring that the button is only shown when appropriate.+ if (!transaction) return <div>No transaction data available.</div>; if (!transaction) return null; const onIncreaseFee = () => { navigate(RouteUrls.IncreaseBtcFee, { state: { btcTx: transaction } }); }; const openTxLink = () => { + try { void analytics.track('view_bitcoin_transaction'); if (inscriptionData) { openInNewTab(inscriptionData.id); return; } handleOpenTxLink({ txid: transaction?.txid || '' }); + } catch (error) { + // Handle errors appropriately + } };
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- src/app/common/utils/get-burn-address.ts (1 hunks)
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (2 hunks)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (5 hunks)
- src/app/components/stacks-transaction-item/transaction-action-button.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx (2 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (1 hunks)
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4 hunks)
- src/app/routes/app-routes.tsx (2 hunks)
- src/shared/route-urls.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx
Additional comments not posted (10)
src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1)
7-16: Good use of reusable components inIncreaseStxFeeDialog.The implementation uses well-defined hooks and components, promoting reusability and maintainability. The dialog component is structured correctly and uses props effectively.
src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1)
7-16: Well-implementedCancelStxTransactionDialog.The component is cleanly implemented with clear, purpose-specific properties like
isCancel. It effectively reuses existing components and hooks, which enhances code maintainability and consistency.src/app/components/stacks-transaction-item/transaction-action-button.tsx (1)
26-26: Ensure robust handling of potentially undefinedonButtonClick.The implementation of
TransactionActionButtonis clean and functional. However, consider handling cases whereonButtonClickmight be undefined to prevent runtime errors. This was previously mentioned and should be addressed to enhance the component's robustness.- onButtonClick(); + onButtonClick?.();src/shared/route-urls.ts (1)
30-30: Integration of new routeCancelStxTransaction.The route
CancelStxTransactionhas been added correctly following the existing naming and structuring conventions. Ensure that this new route is integrated and utilized in the relevant parts of the application to enable the cancellation functionality.Verification successful
Integration of new route
CancelStxTransaction.The route
CancelStxTransactionhas been integrated and utilized consistently across the application:
src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx
- Function
CancelStxTransactionDialoguses the route.- Route URL is correctly referenced.
src/app/components/stacks-transaction-item/stacks-transaction-item.tsx
- The route is used in the
cancelproperty.src/app/routes/app-routes.tsx
- The
CancelStxTransactionDialogcomponent is imported and used in routing.- Path is correctly set for the route.
The new route appears to be well-integrated into the application's routing and related components.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `CancelStxTransaction` in the application's routing and related components. # Test: Search for the route usage. Expect: Consistent usage across the application. rg --glob '**/*.tsx' 'CancelStxTransaction'Length of output: 818
src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (1)
31-31: Enhancements to handle transaction cancellation in broadcasting logic.The integration of
isCancelTransactioninto theuseStacksBroadcastTransactionhook is well-implemented. It correctly modifies the behavior based on transaction type (increase fee or cancel). Ensure that the additional logic does not introduce side effects, especially in the transaction flow.Also applies to: 38-38, 65-65, 103-105, 145-145
src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (1)
1-1: Refinement of transaction item component to support action handling.The addition of
TransactionActionandactionRouteMapto handle transaction actions like cancellation and fee increase is well-structured. It enhances modularity and maintainability of the code. Ensure that the UI elements and interactions are tested to confirm they behave as expected under different conditions.Also applies to: 6-6, 27-27, 36-39, 63-66, 77-81, 101-101, 123-123
src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (1)
44-44: Enhancements to transaction action dialog for cancellation functionality.The modifications to include cancellation functionality in the
StxTransactionActionDialogare correctly implemented. The use ofisCancelto adjust the initial fee and the integration with form handling are particularly noteworthy. Ensure thorough testing to verify the form behaves correctly, especially in edge cases.Also applies to: 56-56, 77-77, 101-101
src/app/routes/app-routes.tsx (3)
19-21: Well-organized import structure for transaction dialogs.The import statements for transaction-related dialogs (
CancelStxTransactionDialog,IncreaseBtcFeeDialog,IncreaseStxFeeDialog) are neatly organized, which enhances maintainability and readability.
108-114: Correct implementation of the new STX transaction cancellation route.The routing for
CancelStxTransactionDialogis appropriately set up with nested routes for handling broadcast errors. This is a good use of React Router's capabilities to manage complex routing needs efficiently.
108-114: Verify integration with other transaction-related routes.While the routing setup for the
CancelStxTransactionDialogis correctly implemented, it's important to ensure that this new route does not conflict with existing transaction handling routes and that it integrates seamlessly with the broader routing logic.Verification successful
Verify integration with other transaction-related routes.
While the routing setup for the
CancelStxTransactionDialogis correctly implemented, it's important to ensure that this new route does not conflict with existing transaction handling routes and that it integrates seamlessly with the broader routing logic.
Verified integration with other transaction-related routes.
The routing setup for the
CancelStxTransactionDialoghas been correctly implemented and does not conflict with existing transaction handling routes. The new route integrates seamlessly with the broader routing logic.
- No conflicts found with
RouteUrls.CancelStxTransactionin thesrc/app/routes/directory.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential route conflicts with the new CancelStxTransaction route. # Test: Search for similar route patterns to identify potential conflicts. rg --type typescript 'RouteUrls.CancelStxTransaction' src/app/routes/Length of output: 104
Script:
#!/bin/bash # Description: Check for potential route conflicts with the new CancelStxTransaction route. # Test: Search for similar route patterns to identify potential conflicts. rg 'RouteUrls.CancelStxTransaction' src/app/routes/Length of output: 302
| default: | ||
| return 'ST000000000000000000002AMW42H'; |
There was a problem hiding this comment.
Clarify the default case in getBurnAddress.
The function returns the Testnet burn address as the default. It might be clearer to either handle this explicitly or add a comment explaining why this is the default behavior, to avoid confusion in environments where the chain ID might not be explicitly set.
- return 'ST000000000000000000002AMW42H';
+ // Default to Testnet burn address unless specified otherwise
+ return 'ST000000000000000000002AMW42H'; 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.
| default: | |
| return 'ST000000000000000000002AMW42H'; | |
| default: | |
| // Default to Testnet burn address unless specified otherwise | |
| return 'ST000000000000000000002AMW42H'; |
| useEffect(() => { | ||
| if (tx && tx.tx_status !== 'pending') { | ||
| toast.info('Your transaction went through!.'); | ||
| } | ||
| }, [toast, tx, tx?.tx_status]); |
There was a problem hiding this comment.
Optimize the useEffect for transaction status updates.
The useEffect hook is triggered by changes in the tx?.tx_status. However, to avoid unnecessary re-renders, consider using a separate state to track the transaction status. This approach can help in reducing the impact on performance and improving the clarity of state changes.
+ const [status, setStatus] = useState(tx?.tx_status);
+
useEffect(() => {
- if (tx && tx.tx_status !== 'pending') {
+ if (status !== 'pending') {
toast.info('Your transaction went through!.');
}
- }, [toast, tx, tx?.tx_status]);
+ }, [toast, status]);
+
+ useEffect(() => {
+ if (tx?.tx_status !== status) {
+ setStatus(tx?.tx_status);
+ }
+ }, [tx?.tx_status]);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.
| useEffect(() => { | |
| if (tx && tx.tx_status !== 'pending') { | |
| toast.info('Your transaction went through!.'); | |
| } | |
| }, [toast, tx, tx?.tx_status]); | |
| const [status, setStatus] = useState(tx?.tx_status); | |
| useEffect(() => { | |
| if (status !== 'pending') { | |
| toast.info('Your transaction went through!.'); | |
| } | |
| }, [toast, status]); | |
| useEffect(() => { | |
| if (tx?.tx_status !== status) { | |
| setStatus(tx?.tx_status); | |
| } | |
| }, [tx?.tx_status]); |
Aman-zishan
force-pushed
the
fix/5501/rework
branch
2 times, most recently
from
85f293a to
ebee2b2
Compare
Aman-zishan
force-pushed
the
fix/5501/rework
branch
2 times, most recently
from
6fe42df to
e401e9b
Compare
Aman-zishan
force-pushed
the
fix/5501/rework
branch
from
e401e9b to
4cfd655
Compare
|
@markmhendrickson Addressed the change requests from #5536 discussions and rebased to latest. There is an issue with the icons where the close icon appears to be bigger compared to others and i couldn't find the exact icons as seen in the mock ups by @mica000 ; i have asked @pete-watters for guidance on the same. |
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range, codebase verification and nitpick comments (7)
src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (1)
1-2: Remove unnecessary blank line.The blank line after the import statement is unnecessary.
- import { useCallback } from 'react'; - + import { useCallback } from 'react';src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (1)
1-2: Remove unnecessary blank line.The blank line after the import statement is unnecessary.
- import { useCallback } from 'react'; - + import { useCallback } from 'react';src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (1)
1-2: Remove unnecessary blank line.The blank line after the import statement is unnecessary.
- import { useEffect } from 'react'; - + import { useEffect } from 'react';src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (4)
46-56: Consider adding type annotations fornavigate,location, andtxid.Adding explicit type annotations can improve readability and maintainability.
const navigate: ReturnType<typeof useNavigate> = useNavigate(); const location: ReturnType<typeof useLocation> = useLocation(); const { txid }: { txid?: string } = useParams();
68-69: Handle loading states more gracefully.Consider showing different messages or components for loading raw transaction and loading transaction states.
- if (isLoadingRawTx || isLoadingTx) return <LoadingSpinner />; + if (isLoadingRawTx) return <LoadingSpinner message="Loading raw transaction..." />; + if (isLoadingTx) return <LoadingSpinner message="Loading transaction..." />;
78-83: Consider adding validation for theinitialValuesin Formik.Adding validation ensures that the initial values are correct and within expected ranges.
- initialValues={{ fee: new BigNumber(microStxToStx(initialFee)).toNumber() }} + initialValues={{ fee: new BigNumber(microStxToStx(initialFee)).toNumber() }} + validate={values => { + const errors: { fee?: string } = {}; + if (values.fee <= 0) { + errors.fee = 'Fee must be greater than zero'; + } + return errors; + }}
93-97: Ensure thatTransactionActionscomponent handles all possible states.The
TransactionActionscomponent handles disabled and loading states through its props (isDisabled,isLoading, andisBroadcasting). However, there is no explicit handling of an error state in the provided code. Consider adding error state management if applicable.
src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsxAnalysis chain
Ensure that
TransactionActionscomponent handles all possible states.Verify that the
TransactionActionscomponent handles all possible states, such as disabled, loading, and error states.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `TransactionActions` component handles all possible states. # Test: Search for the `TransactionActions` component usage. Expect: Proper handling of all states. rg --type tsx 'TransactionActions'Length of output: 62
Script:
#!/bin/bash # Description: Verify that the `TransactionActions` component handles all possible states. # Step 1: Locate the definition of the `TransactionActions` component. ast-grep --lang typescript --pattern 'function TransactionActions($_$) { $$$ }' > transaction_actions_definition.txt # Step 2: Search for the usage of `TransactionActions` in the codebase. rg 'TransactionActions' --type ts --type tsx > transaction_actions_usage.txt # Combine results for easier review. cat transaction_actions_definition.txt transaction_actions_usage.txtLength of output: 225
Script:
#!/bin/bash # Description: Verify that the `TransactionActions` component handles all possible states. # Step 1: Locate the definition of the `TransactionActions` component. ast-grep --lang typescript --pattern 'const TransactionActions: React.FC' > transaction_actions_definition.txt # Step 2: Search for the usage of `TransactionActions` in TypeScript and TypeScript React files. rg 'TransactionActions' --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' > transaction_actions_usage.txt # Combine results for easier review. cat transaction_actions_definition.txt transaction_actions_usage.txtLength of output: 2043
Script:
#!/bin/bash # Description: Extract the contents of the `TransactionActions` component to verify state handling. # Extract the contents of the `transaction-actions.tsx` file. cat src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsxLength of output: 1097
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- src/app/common/utils/get-burn-address.ts (1 hunks)
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (3 hunks)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (5 hunks)
- src/app/components/stacks-transaction-item/transaction-action-button.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (2 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx (2 hunks)
- src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1 hunks)
- src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (1 hunks)
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4 hunks)
- src/app/routes/app-routes.tsx (2 hunks)
- src/shared/route-urls.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- src/app/features/dialogs/transaction-action-dialog/increase-btc-fee-dialog.tsx
Additional context used
Learnings (7)
src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (1)
Learnt from: pete-watters PR: leather-io/extension#5536 File: src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx:40-40 Timestamp: 2024-07-02T12:35:48.552Z Learning: The `FeeComponent` prop in `stx-transaction-action-dialog.tsx` should be typed as `React.ComponentType<{ currentFee: number }>` to ensure type safety, based on the `IncreaseFeeField` component's expected props.src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (1)
Learnt from: pete-watters PR: leather-io/extension#5536 File: src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx:40-40 Timestamp: 2024-07-02T12:35:48.552Z Learning: The `FeeComponent` prop in `stx-transaction-action-dialog.tsx` should be typed as `React.ComponentType<{ currentFee: number }>` to ensure type safety, based on the `IncreaseFeeField` component's expected props.src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-increase-fee.ts (2)
Learnt from: pete-watters PR: leather-io/extension#5536 File: src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx:40-40 Timestamp: 2024-07-02T12:35:48.552Z Learning: The `FeeComponent` prop in `stx-transaction-action-dialog.tsx` should be typed as `React.ComponentType<{ currentFee: number }>` to ensure type safety, based on the `IncreaseFeeField` component's expected props.Learnt from: pete-watters PR: leather-io/extension#5536 File: src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts:18-53 Timestamp: 2024-07-02T12:33:54.211Z Learning: When refactoring hooks, it's beneficial to split them into smaller, more focused hooks to improve clarity and reusability. For example, the `useStxTransactionActions` hook can be split into `useTransactionData`, `useTransactionStatusNotifier`, and `useTransactionActions`.src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-cancel-transaction.ts (1)
Learnt from: pete-watters PR: leather-io/extension#5536 File: src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts:18-53 Timestamp: 2024-07-02T12:33:54.211Z Learning: When refactoring hooks, it's beneficial to split them into smaller, more focused hooks to improve clarity and reusability. For example, the `useStxTransactionActions` hook can be split into `useTransactionData`, `useTransactionStatusNotifier`, and `useTransactionActions`.src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (1)
Learnt from: pete-watters PR: leather-io/extension#5536 File: src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts:18-53 Timestamp: 2024-07-02T12:33:54.211Z Learning: When refactoring hooks, it's beneficial to split them into smaller, more focused hooks to improve clarity and reusability. For example, the `useStxTransactionActions` hook can be split into `useTransactionData`, `useTransactionStatusNotifier`, and `useTransactionActions`.src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (1)
Learnt from: pete-watters PR: leather-io/extension#5536 File: src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx:31-31 Timestamp: 2024-07-02T12:37:09.196Z Learning: When adding new boolean arguments to functions, ensure they have default values to maintain consistent behavior when not provided.src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (2)
Learnt from: pete-watters PR: leather-io/extension#5536 File: src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx:40-40 Timestamp: 2024-07-02T12:35:48.552Z Learning: The `FeeComponent` prop in `stx-transaction-action-dialog.tsx` should be typed as `React.ComponentType<{ currentFee: number }>` to ensure type safety, based on the `IncreaseFeeField` component's expected props.Learnt from: pete-watters PR: leather-io/extension#5536 File: src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts:18-53 Timestamp: 2024-07-02T12:33:54.211Z Learning: When refactoring hooks, it's beneficial to split them into smaller, more focused hooks to improve clarity and reusability. For example, the `useStxTransactionActions` hook can be split into `useTransactionData`, `useTransactionStatusNotifier`, and `useTransactionActions`.
Additional comments not posted (33)
src/app/common/utils/get-burn-address.ts (2)
1-2: Imports look good.The imports for
StacksNetworkandChainIDare appropriate and necessary for the function.
4-12: Clarify the default case ingetBurnAddress.The function returns the Testnet burn address as the default. It might be clearer to either handle this explicitly or add a comment explaining why this is the default behavior, to avoid confusion in environments where the chain ID might not be explicitly set.
- return 'ST000000000000000000002AMW42H'; + // Default to Testnet burn address unless specified otherwise + return 'ST000000000000000000002AMW42H';src/app/features/dialogs/transaction-action-dialog/increase-stx-fee-dialog.tsx (2)
1-5: Imports and component usage look good.The imports for
RouteUrls,IncreaseFeeField,useStxIncreaseFee, andStxTransactionActionDialogare appropriate and necessary for the component.
7-17: Component structure is sound.The
IncreaseStxFeeDialogcomponent correctly usesStxTransactionActionDialogand passes the necessary props. This ensures that the dialog is correctly configured for increasing the transaction fee.src/app/features/dialogs/transaction-action-dialog/cancel-stx-transaction-dialog.tsx (2)
1-5: Imports and component usage look good.The imports for
RouteUrls,IncreaseFeeField,useStxCancelTransaction, andStxTransactionActionDialogare appropriate and necessary for the component.
7-18: Component structure is sound.The
CancelStxTransactionDialogcomponent correctly usesStxTransactionActionDialogand passes the necessary props, includingisCancel, ensuring that the dialog is correctly configured for canceling the transaction.src/app/components/stacks-transaction-item/transaction-action-button.tsx (3)
1-4: Imports look good.The imports for
IconButtonandBasicTooltipare appropriate and necessary for the component.
5-11: Props interface is well-defined.The
ActionButtonPropsinterface is clearly defined and includes all necessary properties for the component.
13-31: Ensure robust handling of potentially undefinedonButtonClick.The implementation of
TransactionActionIconButtonis clean and functional. However, consider handling cases whereonButtonClickmight be undefined to prevent runtime errors. This was previously mentioned and should be addressed to enhance the component's robustness.- onButtonClick(); + onButtonClick?.();src/app/features/dialogs/transaction-action-dialog/components/transaction-actions.tsx (1)
7-12: Ensure that optional props are explicitly marked.The
isBroadcastingandisLoadingprops are optional, but the type definition does not make this explicit.- interface TransactionActionsProps { - isBroadcasting?: boolean; - isDisabled: boolean; - isLoading?: boolean; - onCancel(): void; - } + interface TransactionActionsProps { + isBroadcasting?: boolean; + isDisabled: boolean; + isLoading?: boolean; + onCancel(): void; + }Likely invalid or redundant comment.
src/app/features/dialogs/transaction-action-dialog/hooks/use-stx-transaction-actions.ts (2)
33-49: Ensure actionType is correctly handled.The
useTransactionActionshook handles different action types. Ensure that the actionType is correctly handled and validated.
51-73: Enhance the use ofuseEffectfor transaction status updates.The
useEffecthook in theuseTransactionStatusNotifierfunction should be optimized as suggested earlier.src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (3)
8-8: Import Statement UpdateThe new import statement for
TransactionActionIconButtonis correctly added to replace theIncreaseFeeButton. This improves modularity and reusability.
24-24: Import Statement UpdateThe import statement for
TransactionActionIconButtonis correctly added to replace theIncreaseFeeButton. This aligns with the objective of refactoring to improve code reuse.
77-82: Refactor: Using TransactionActionIconButtonThe refactoring to use
TransactionActionIconButtonfor the increase fee action is well done. This component-based approach improves reusability and maintainability. Ensure thatTransactionActionIconButtonhandles all necessary props correctly.src/shared/route-urls.ts (1)
30-30: Add Route: CancelStxTransactionThe new route
CancelStxTransactionis correctly added. Ensure this route is integrated into the application's routing and related components.src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (8)
1-2: New ImportsThe new imports for
useMatch,useNavigate,HStack,ChevronsRightIcon, andCloseIconare correctly added. These are necessary for the new functionality introduced.
27-27: Import Statement UpdateThe import statement for
TransactionActionIconButtonis correctly added. This aligns with the refactoring objective to improve modularity and reusability.
29-34: New Types and InterfacesThe new type
TransactionActionand interfaceActionRouteMapare well-defined. These additions enhance the readability and maintainability of the code.
36-39: Action Route MapThe
actionRouteMapobject is correctly defined to map transaction actions to route URLs. This enhances the modularity and readability of the code.
63-66: UseMatch for Route MatchingThe use of
useMatchto check the current route for cancel and increase fee actions is appropriately added. This helps in determining the current transaction action route.
77-81: Handle Transaction ActionThe
handleTransactionActionfunction is well-implemented to navigate to the appropriate route based on the action. This enhances the modularity and reusability of the code.
101-117: Action Button GroupThe refactored
actionButtonGroupusingTransactionActionIconButtonfor cancel and increase fee actions is well done. This improves the readability and maintainability of the code.
126-126: Conditional Rendering of Action Button GroupThe conditional rendering of
actionButtonGroupbased on whether the transaction is pending and the current account is the originator is correctly implemented.src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4)
38-38: Handling New ArgumentThe new argument
isCancelTransactionis correctly included in the function arguments. Ensure that all call sites of this function are updated accordingly.
65-65: Navigation on Transaction ActionThe navigation logic correctly handles the
isCancelTransactioncase to navigate to the activity page. This ensures a consistent user experience.
103-105: Success Toast for CancellationThe success toast for transaction cancellation is appropriately added. This provides feedback to the user upon successful transaction cancellation.
145-145: Dependency Array UpdateThe dependency array is correctly updated to include
isCancelTransaction. This ensures the memoized function is updated when the argument changes.src/app/features/dialogs/transaction-action-dialog/stx-transaction-action-dialog.tsx (2)
1-23: Imports look good.The imported modules are relevant and correctly used in the file.
25-44: Interfaces are well-defined.The interfaces
IStxTransactionActionHookReturnandStxTransactionActionDialogPropsare well-defined and appropriately used.src/app/routes/app-routes.tsx (3)
19-21: Imports look good.The imported modules are relevant and correctly used in the file.
112-114: Ensure that theBroadcastErrorroute works as expected.Verify that the
BroadcastErrorroute forCancelStxTransactionworks as expected and handles all possible states.Verification successful
The
BroadcastErrorroute forCancelStxTransactionworks as expected.The
BroadcastErrorcomponent effectively handles different states by extracting and displaying error-related information from the location state. TheBroadcastErrorLayoutcomponent properly renders this information, ensuring that all possible states are addressed.
src/app/routes/app-routes.tsx:112-114src/app/pages/send/broadcast-error/broadcast-error.tsxsrc/app/pages/send/broadcast-error/components/broadcast-error.layout.tsxScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `BroadcastError` route for `CancelStxTransaction` works as expected. # Test: Search for the `BroadcastError` component usage. Expect: Proper handling of all states. rg --type tsx 'BroadcastError'Length of output: 58
Script:
#!/bin/bash # Description: Verify that the `BroadcastError` route for `CancelStxTransaction` works as expected. # Test: Search for the `BroadcastError` component usage in TypeScript and TSX files. Expect: Proper handling of all states. rg 'BroadcastError' --type-add 'ts:*.{ts,tsx}' --type tsLength of output: 6238
Script:
#!/bin/bash # Description: Inspect the implementation of the BroadcastError component to ensure it handles all possible states. # Extract the implementation of the BroadcastError component. ast-grep --lang tsx --pattern 'export function BroadcastError() { $$$ }'Length of output: 1751
Script:
#!/bin/bash # Description: Inspect the implementation of the BroadcastErrorLayout component to ensure it handles all possible states. # Extract the implementation of the BroadcastErrorLayout component. ast-grep --lang tsx --pattern 'export function BroadcastErrorLayout($_) { $$$ }'Length of output: 4656
108-110: Ensure that theCancelStxTransactionDialogroute works as expected.Verify that the
CancelStxTransactionDialogroute works as expected and handles all possible states.
| @@ -28,12 +28,14 @@ interface UseStacksBroadcastTransactionArgs { | |||
| token: CryptoCurrencies; | |||
| decimals?: number; | |||
| isIncreaseFeeTransaction?: boolean; | |||
| isCancelTransaction?: boolean; | |||
There was a problem hiding this comment.
Default Value for New Argument
It's good to add a default value for the new boolean argument isCancelTransaction to maintain consistent behavior when not provided.
- isCancelTransaction?: boolean;
+ isCancelTransaction?: boolean = false;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.
| isCancelTransaction?: boolean; | |
| isCancelTransaction?: boolean = false; |
| export function TransactionActions(props: TransactionActionsProps) { | ||
| const { onCancel, isDisabled, isLoading, isBroadcasting } = props; | ||
|
|
||
| const { handleSubmit } = useFormikContext(); | ||
|
|
There was a problem hiding this comment.
Avoid unnecessary destructuring of props.
Destructuring props in the function signature can be more concise.
- export function TransactionActions(props: TransactionActionsProps) {
- const { onCancel, isDisabled, isLoading, isBroadcasting } = props;
+ export function TransactionActions({ onCancel, isDisabled, isLoading, isBroadcasting }: TransactionActionsProps) {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.
| export function TransactionActions(props: TransactionActionsProps) { | |
| const { onCancel, isDisabled, isLoading, isBroadcasting } = props; | |
| const { handleSubmit } = useFormikContext(); | |
| export function TransactionActions({ onCancel, isDisabled, isLoading, isBroadcasting }: TransactionActionsProps) { | |
| const { handleSubmit } = useFormikContext(); |
| onClick={() => handleSubmit()} | ||
| aria-busy={isLoading || isBroadcasting} |
There was a problem hiding this comment.
Avoid redundant onClick handler in submit button.
The onClick handler for the submit button is redundant since the button's type is already set to "submit".
- <Button
- type="submit"
- onClick={() => handleSubmit()}
- aria-busy={isLoading || isBroadcasting}
- borderRadius="sm"
- aria-disabled={isDisabled}
- disabled={isDisabled}
- flexGrow={1}
- >
- {actionText}
- </Button>
+ <Button
+ type="submit"
+ aria-busy={isLoading || isBroadcasting}
+ borderRadius="sm"
+ aria-disabled={isDisabled}
+ disabled={isDisabled}
+ flexGrow={1}
+ >
+ {actionText}
+ </Button>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.
| onClick={() => handleSubmit()} | |
| aria-busy={isLoading || isBroadcasting} | |
| <Button | |
| type="submit" | |
| aria-busy={isLoading || isBroadcasting} | |
| borderRadius="sm" | |
| aria-disabled={isDisabled} | |
| disabled={isDisabled} | |
| flexGrow={1} | |
| > | |
| {actionText} | |
| </Button> |
| const onSubmit = useCallback( | ||
| async (values: { fee: number }, rawTx?: StacksTransaction) => { | ||
| if (!tx || !rawTx) return; | ||
| rawTx.setFee(stxToMicroStx(values.fee).toString()); | ||
| const txid = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); | ||
| await refreshAccountData(); | ||
| submittedTransactionsActions.transactionReplacedByFee(txid); | ||
| await stacksBroadcastTransaction(rawTx); | ||
| }, | ||
| [tx, refreshAccountData, submittedTransactionsActions, stacksBroadcastTransaction] | ||
| ); |
There was a problem hiding this comment.
Add error handling to the onSubmit function.
The onSubmit function lacks error handling for asynchronous operations. Consider wrapping these operations in a try-catch block to handle potential exceptions gracefully.
- async (values: { fee: number }, rawTx?: StacksTransaction) => {
- if (!tx || !rawTx) return;
- rawTx.setFee(stxToMicroStx(values.fee).toString());
- const txid = tx.tx_id || safelyFormatHexTxid(rawTx.txid());
- await refreshAccountData();
- submittedTransactionsActions.transactionReplacedByFee(txid);
- await stacksBroadcastTransaction(rawTx);
- },
+ async (values: { fee: number }, rawTx?: StacksTransaction) => {
+ try {
+ if (!tx || !rawTx) return;
+ rawTx.setFee(stxToMicroStx(values.fee).toString());
+ const txid = tx.tx_id || safelyFormatHexTxid(rawTx.txid());
+ await refreshAccountData();
+ submittedTransactionsActions.transactionReplacedByFee(txid);
+ await stacksBroadcastTransaction(rawTx);
+ } catch (error) {
+ // Handle errors appropriately
+ }
+ },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 onSubmit = useCallback( | |
| async (values: { fee: number }, rawTx?: StacksTransaction) => { | |
| if (!tx || !rawTx) return; | |
| rawTx.setFee(stxToMicroStx(values.fee).toString()); | |
| const txid = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); | |
| await refreshAccountData(); | |
| submittedTransactionsActions.transactionReplacedByFee(txid); | |
| await stacksBroadcastTransaction(rawTx); | |
| }, | |
| [tx, refreshAccountData, submittedTransactionsActions, stacksBroadcastTransaction] | |
| ); | |
| const onSubmit = useCallback( | |
| async (values: { fee: number }, rawTx?: StacksTransaction) => { | |
| try { | |
| if (!tx || !rawTx) return; | |
| rawTx.setFee(stxToMicroStx(values.fee).toString()); | |
| const txid = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); | |
| await refreshAccountData(); | |
| submittedTransactionsActions.transactionReplacedByFee(txid); | |
| await stacksBroadcastTransaction(rawTx); | |
| } catch (error) { | |
| // Handle errors appropriately | |
| } | |
| }, | |
| [tx, refreshAccountData, submittedTransactionsActions, stacksBroadcastTransaction] | |
| ); |
| const onSubmit = useCallback( | ||
| async (values: { fee: number }) => { | ||
| if (!tx || !rawTx || !account) return; | ||
|
|
||
| const options: GenerateUnsignedTransactionOptions = { | ||
| publicKey: account.stxPublicKey, | ||
| nonce: tx.nonce, | ||
| fee: stxToMicroStx(values?.fee || 0).toNumber(), | ||
| txData: { | ||
| txType: TransactionTypes.STXTransfer, | ||
| recipient: getBurnAddress(network), | ||
| amount: 1, | ||
| memo: '_cancel transaction', | ||
| network: network, | ||
| } as any, | ||
| }; | ||
| const newTx = await generateUnsignedTransaction(options); | ||
| const txId = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); | ||
| await refreshAccountData(); | ||
| submittedTransactionsActions.transactionReplacedByFee(txId); | ||
| await stacksBroadcastTransaction(newTx); | ||
| }, |
There was a problem hiding this comment.
Add error handling to the onSubmit function.
The onSubmit function lacks error handling for asynchronous operations. Consider wrapping these operations in a try-catch block to handle potential exceptions gracefully.
- async (values: { fee: number }) => {
- if (!tx || !rawTx || !account) return;
- const options: GenerateUnsignedTransactionOptions = {
- publicKey: account.stxPublicKey,
- nonce: tx.nonce,
- fee: stxToMicroStx(values?.fee || 0).toNumber(),
- txData: {
- txType: TransactionTypes.STXTransfer,
- recipient: getBurnAddress(network),
- amount: 1,
- memo: '_cancel transaction',
- network: network,
- } as any,
- };
- const newTx = await generateUnsignedTransaction(options);
- const txId = tx.tx_id || safelyFormatHexTxid(rawTx.txid());
- await refreshAccountData();
- submittedTransactionsActions.transactionReplacedByFee(txId);
- await stacksBroadcastTransaction(newTx);
- },
+ async (values: { fee: number }) => {
+ try {
+ if (!tx || !rawTx || !account) return;
+ const options: GenerateUnsignedTransactionOptions = {
+ publicKey: account.stxPublicKey,
+ nonce: tx.nonce,
+ fee: stxToMicroStx(values?.fee || 0).toNumber(),
+ txData: {
+ txType: TransactionTypes.STXTransfer,
+ recipient: getBurnAddress(network),
+ amount: 1,
+ memo: '_cancel transaction',
+ network: network,
+ } as any,
+ };
+ const newTx = await generateUnsignedTransaction(options);
+ const txId = tx.tx_id || safelyFormatHexTxid(rawTx.txid());
+ await refreshAccountData();
+ submittedTransactionsActions.transactionReplacedByFee(txId);
+ await stacksBroadcastTransaction(newTx);
+ } catch (error) {
+ // Handle errors appropriately
+ }
+ },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 onSubmit = useCallback( | |
| async (values: { fee: number }) => { | |
| if (!tx || !rawTx || !account) return; | |
| const options: GenerateUnsignedTransactionOptions = { | |
| publicKey: account.stxPublicKey, | |
| nonce: tx.nonce, | |
| fee: stxToMicroStx(values?.fee || 0).toNumber(), | |
| txData: { | |
| txType: TransactionTypes.STXTransfer, | |
| recipient: getBurnAddress(network), | |
| amount: 1, | |
| memo: '_cancel transaction', | |
| network: network, | |
| } as any, | |
| }; | |
| const newTx = await generateUnsignedTransaction(options); | |
| const txId = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); | |
| await refreshAccountData(); | |
| submittedTransactionsActions.transactionReplacedByFee(txId); | |
| await stacksBroadcastTransaction(newTx); | |
| }, | |
| const onSubmit = useCallback( | |
| async (values: { fee: number }) => { | |
| try { | |
| if (!tx || !rawTx || !account) return; | |
| const options: GenerateUnsignedTransactionOptions = { | |
| publicKey: account.stxPublicKey, | |
| nonce: tx.nonce, | |
| fee: stxToMicroStx(values?.fee || 0).toNumber(), | |
| txData: { | |
| txType: TransactionTypes.STXTransfer, | |
| recipient: getBurnAddress(network), | |
| amount: 1, | |
| memo: '_cancel transaction', | |
| network: network, | |
| } as any, | |
| }; | |
| const newTx = await generateUnsignedTransaction(options); | |
| const txId = tx.tx_id || safelyFormatHexTxid(rawTx.txid()); | |
| await refreshAccountData(); | |
| submittedTransactionsActions.transactionReplacedByFee(txId); | |
| await stacksBroadcastTransaction(newTx); | |
| } catch (error) { | |
| // Handle errors appropriately | |
| } | |
| }, |
| export function useTransactionStatusNotifier(tx: any) { | ||
| const toast = useToast(); | ||
| useEffect(() => { | ||
| if (tx && tx.tx_status !== 'pending') { | ||
| toast.info('Your transaction went through!'); | ||
| } | ||
| }, [toast, tx, tx?.tx_status]); | ||
| } |
There was a problem hiding this comment.
Optimize the useEffect for transaction status updates.
The useEffect hook is triggered by changes in the tx?.tx_status. However, to avoid unnecessary re-renders, consider using a separate state to track the transaction status. This approach can help in reducing the impact on performance and improving the clarity of state changes.
+ const [status, setStatus] = useState(tx?.tx_status);
+
useEffect(() => {
- if (tx && tx.tx_status !== 'pending') {
+ if (status !== 'pending') {
toast.info('Your transaction went through!');
}
- }, [toast, tx, tx?.tx_status]);
+ }, [toast, status]);
+
+ useEffect(() => {
+ if (tx?.tx_status !== status) {
+ setStatus(tx?.tx_status);
+ }
+ }, [tx?.tx_status]);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.
| export function useTransactionStatusNotifier(tx: any) { | |
| const toast = useToast(); | |
| useEffect(() => { | |
| if (tx && tx.tx_status !== 'pending') { | |
| toast.info('Your transaction went through!'); | |
| } | |
| }, [toast, tx, tx?.tx_status]); | |
| } | |
| export function useTransactionStatusNotifier(tx: any) { | |
| const toast = useToast(); | |
| const [status, setStatus] = useState(tx?.tx_status); | |
| useEffect(() => { | |
| if (status !== 'pending') { | |
| toast.info('Your transaction went through!'); | |
| } | |
| }, [toast, status]); | |
| useEffect(() => { | |
| if (tx?.tx_status !== status) { | |
| setStatus(tx?.tx_status); | |
| } | |
| }, [tx?.tx_status]); | |
| } |
markmhendrickson
removed request for
kyranjamie,
pete-watters,
fabric-8,
fbwoolf,
alter-eggo and
mica000
|
close in favor of #5920 |
This PR :
(the layout in this video is outdated)
cancel-stx-tx.mp4
Dropped Tx: https://explorer.hiro.so/txid/0x39dfa74f30d600cd82754030851a72763be3cdda0be9d04f6b777719400841fa?chain=testnet&api=https%3A%2F%2Fapi.nakamoto.testnet.hiro.so
Burn Tx:
https://explorer.hiro.so/txid/0x3095aa2f935c74e858caeb948c8fd20f84a5d0e9d4f4713685cb0d0c8ed2cecb?chain=testnet&api=https%3A%2F%2Fapi.nakamoto.testnet.hiro.so
Summary by CodeRabbit
New Features
Improvements
Bug Fixes