feat: cancel stacks pending transaction #5501
Conversation
WalkthroughThe recent changes introduce a new feature for canceling Stacks transactions, including UI components, hooks, and routes. Enhancements include a new Changes
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: 1
Outside diff range and nitpick comments (4)
src/app/components/stacks-transaction-item/cancel-transaction-button.tsx (1)
19-21: Consider adding a comment explaining whye.stopPropagation()is used here, to help future maintainers understand its purpose.src/app/components/transaction-item/transaction-item.layout.tsx (1)
Line range hint
22-50: Consider using destructuring forpropsin the function signature to improve readability.- export function TransactionItemLayout({ + export function TransactionItemLayout({ actionButtonGroupElement, txCaption, txIcon, txStatus, txTitle, txValue, ...props }: TransactionItemLayoutProps) {src/app/features/dialogs/cancel-transaction-dialog/hooks/use-stx-cancel-transaction.ts (1)
25-89: Consider adding detailed comments explaining the transaction crafting and broadcasting process within theuseStxCancelTransactionhook to aid future maintainers.src/app/features/dialogs/cancel-transaction-dialog/cancel-stx-transaction-dialog.tsx (1)
63-63: Add a user-friendly message or UI element when waiting for transaction details.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- src/app/common/hooks/use-loading.ts (1 hunks)
- src/app/common/utils/get-burn-address.ts (1 hunks)
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (1 hunks)
- src/app/components/stacks-transaction-item/cancel-transaction-button.tsx (1 hunks)
- src/app/components/stacks-transaction-item/increase-fee-button.tsx (1 hunks)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (4 hunks)
- src/app/components/transaction-item/transaction-item.layout.tsx (3 hunks)
- src/app/features/dialogs/cancel-transaction-dialog/cancel-stx-transaction-dialog.tsx (1 hunks)
- src/app/features/dialogs/cancel-transaction-dialog/components/cancel-transaction-actions.tsx (1 hunks)
- src/app/features/dialogs/cancel-transaction-dialog/hooks/use-selected-tx.ts (1 hunks)
- src/app/features/dialogs/cancel-transaction-dialog/hooks/use-stx-cancel-transaction.ts (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)
Additional comments not posted (16)
src/app/features/dialogs/cancel-transaction-dialog/hooks/use-selected-tx.ts (1)
4-7: LGTM! The hookuseSelectedTxis well-implemented and correctly utilizes existing hooks to fetch transaction data.src/app/common/utils/get-burn-address.ts (1)
4-12: LGTM! The functiongetBurnAddressis well-implemented. Consider adding a comment for the default case to clarify that it returns the testnet address as a fallback.src/app/common/hooks/use-loading.ts (1)
5-5: LGTM! The addition ofCANCEL_TRANSACTION_DRAWERto theLoadingKeysenum is correctly implemented.src/app/components/stacks-transaction-item/increase-fee-button.tsx (1)
15-37: LGTM! The update to conditionally render theIncreaseFeeButtonbased on theisActivestate is well-implemented. Ensure that the use ofe.stopPropagation()in the onClick handler is intentional and necessary to prevent event bubbling.src/app/components/stacks-transaction-item/cancel-transaction-button.tsx (1)
8-37: The implementation ofCancelTransactionButtonlooks solid and follows best practices for conditional rendering and event handling.src/app/features/dialogs/cancel-transaction-dialog/components/cancel-transaction-actions.tsx (1)
12-39: TheCancelTransactionActionscomponent is well-implemented with proper use of hooks and accessibility attributes.src/app/components/transaction-item/transaction-item.layout.tsx (1)
Line range hint
22-50: The updates toTransactionItemLayoutare well-executed, enhancing the component's flexibility and maintainability.src/app/features/dialogs/cancel-transaction-dialog/hooks/use-stx-cancel-transaction.ts (1)
25-89: TheuseStxCancelTransactionhook is robust, correctly managing dependencies and state with comprehensive error handling and validation.src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (1)
89-89: RenamingrightElementtoactionButtonGroupElementimproves clarity. Ensure all references to the old prop name are updated across the project.Verification successful
Renaming
rightElementtoactionButtonGroupElementimproves clarity. Ensure all references to the old prop name are updated across the project.The search for
rightElementin.tsxand.tsfiles returned no results, indicating that all instances have been updated.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all instances of `rightElement` have been updated to `actionButtonGroupElement`. # Test: Search for the old prop name. Expect: No occurrences. rg --type typescript $'rightElement'Length of output: 70
Script:
#!/bin/bash # Description: Verify that all instances of `rightElement` have been updated to `actionButtonGroupElement`. # Test: Search for the old prop name in .tsx and .ts files. Expect: No occurrences. rg 'rightElement' --glob '**/*.tsx' --glob '**/*.ts'Length of output: 52
src/shared/route-urls.ts (1)
30-30: Addition ofCancelStxTransactionroute aligns with the new feature. Verify that this route is correctly handled in the routing logic.Verification successful
The new route
CancelStxTransactionis correctly handled in the routing logic and other relevant parts of the codebase.
src/app/routes/app-routes.tsx: The route is imported and used in the routing logic.src/app/features/dialogs/cancel-transaction-dialog/cancel-stx-transaction-dialog.tsx: The route is used in the dialog component.src/app/components/stacks-transaction-item/stacks-transaction-item.tsx: The route is used in multiple places within this component.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new route is correctly handled in the routing logic. # Test: Search for the new route in the routing logic. Expect: At least one occurrence. rg --type typescript $'CancelStxTransaction'Length of output: 78
Script:
#!/bin/bash # Description: Verify that the new route is correctly handled in the routing logic. # Test: Search for the new route in the routing logic. Expect: At least one occurrence. rg --type ts 'CancelStxTransaction'Length of output: 1457
src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (1)
24-24: Introduction ofCancelTransactionButtonand related logic for handling transaction cancellation. Ensure that the new components and methods are correctly implemented and tested.Also applies to: 70-84, 97-120
Verification successful
The verification confirms that the
CancelTransactionButtoncomponent and theonCancelTransactionmethod are correctly implemented and referenced in the codebase.
CancelTransactionButtonis imported and used insrc/app/components/stacks-transaction-item/stacks-transaction-item.tsxand defined insrc/app/components/stacks-transaction-item/cancel-transaction-button.tsx.onCancelTransactionis defined and used insrc/app/components/stacks-transaction-item/stacks-transaction-item.tsxand referenced insrc/app/components/stacks-transaction-item/cancel-transaction-button.tsx.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new components and methods for handling transaction cancellation are correctly implemented and tested. # Test: Search for the new components and methods in the codebase. Expect: At least one occurrence of each. rg --type typescript $'CancelTransactionButton' rg --type typescript $'onCancelTransaction'Length of output: 159
Script:
#!/bin/bash # Description: Verify that the new components and methods for handling transaction cancellation are correctly implemented and tested. # Test: Search for the new components and methods in the codebase. Expect: At least one occurrence of each. rg --glob '*.ts' --glob '*.tsx' 'CancelTransactionButton' rg --glob '*.ts' --glob '*.tsx' 'onCancelTransaction'Length of output: 1198
src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (1)
30-30: Update touseStacksBroadcastTransactionhook to handle transaction cancellation. Ensure that the updated hook is correctly implemented and tested.Also applies to: 37-37, 64-104, 144-144
Verification successful
The
useStacksBroadcastTransactionhook is correctly implemented and used in multiple files across the codebase. Here are the locations where it is imported and utilized:
src/app/pages/transaction-request/transaction-request.tsxsrc/app/pages/send/send-crypto-asset-form/form/stacks/stacks-send-form-confirmation.tsxsrc/app/features/dialogs/cancel-transaction-dialog/hooks/use-stx-cancel-transaction.tssrc/app/features/dialogs/increase-fee-dialog/increase-stx-fee-dialog.tsxThis confirms that the hook is integrated and used in various parts of the application.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the updated hook for handling transaction cancellation is correctly implemented and tested. # Test: Search for the updated hook in the codebase. Expect: At least one occurrence. rg --type typescript $'useStacksBroadcastTransaction'Length of output: 87
Script:
#!/bin/bash # Description: Verify that the updated hook for handling transaction cancellation is correctly implemented and tested. # Test: Search for the updated hook in the codebase. Expect: At least one occurrence. rg 'useStacksBroadcastTransaction' --glob '*.ts' --glob '*.tsx'Length of output: 1651
src/app/features/dialogs/cancel-transaction-dialog/cancel-stx-transaction-dialog.tsx (3)
26-134: The implementation ofCancelStxTransactionDialogappears robust and well-structured.
47-52: Ensure proper handling of transaction statuses other than 'pending'.
70-133: Review the form validation logic to ensure it aligns with the expected transaction cancellation requirements.Verification successful
The search results indicate that the
validationSchemafor theCancelStxTransactionDialogcomponent is defined in theuseStxCancelTransactionhook. The relevant snippet is:src/app/features/dialogs/cancel-transaction-dialog/hooks/use-stx-cancel-transaction.ts: validationSchema: yup.object({ fee: stxFeeValidator(availableUnlockedBalance) }),This schema validates the
feefield using thestxFeeValidatorfunction, which likely ensures that the fee is appropriate based on the available unlocked balance.Locations in the codebase:
src/app/features/dialogs/cancel-transaction-dialog/hooks/use-stx-cancel-transaction.tsConclusion:
The form validation logic for transaction cancellation appears to be in place and aligns with the expected requirements by validating the fee against the available unlocked balance.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the form validation logic for transaction cancellation. # Test: Search for form validation related to transaction cancellation. Expect: Comprehensive validation rules. rg --type typescript $'validationSchema' --context 10Length of output: 87
Script:
#!/bin/bash # Description: Verify the form validation logic for transaction cancellation. # Test: Search for form validation related to transaction cancellation. Expect: Comprehensive validation rules. rg --type ts,tsx 'validationSchema' --context 10Length of output: 79
Script:
#!/bin/bash # Description: Verify the form validation logic for transaction cancellation. # Test: Search for form validation related to transaction cancellation. Expect: Comprehensive validation rules. rg --type ts 'validationSchema' --context 10 rg --type tsx 'validationSchema' --context 10Length of output: 77600
src/app/routes/app-routes.tsx (1)
104-106: The new route forCancelStxTransactionDialogis correctly set up.
| useEffect(() => { | ||
| if (!rawTxId && txIdFromParams) { | ||
| setRawTxId(txIdFromParams); | ||
| } | ||
| if (isLoading && !rawTxId) { | ||
| setIsIdle(); | ||
| } | ||
| }, [isLoading, rawTxId, setIsIdle, setRawTxId, txIdFromParams]); |
There was a problem hiding this comment.
Tip
Codebase Verification
The search results indicate that txIdFromParams is being retrieved from the URL search parameters using searchParams.get('txId'), but there is no evidence of validation logic for txIdFromParams in the provided context.
- File:
src/app/features/dialogs/cancel-transaction-dialog/cancel-stx-transaction-dialog.tsx - Lines:
const txIdFromParams = searchParams.get('txId');if (!rawTxId && txIdFromParams) { setRawTxId(txIdFromParams); }
There is no validation to check if txIdFromParams is valid before it is used.
Analysis chain
Consider adding error handling for cases where txIdFromParams is invalid.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the validation of `txIdFromParams`.
# Test: Search for validation logic for `txIdFromParams`. Expect: Proper validation checks.
rg --type typescript $'txIdFromParams' --context 5
Length of output: 84
Script:
#!/bin/bash
# Description: Verify the validation of `txIdFromParams`.
# Test: Search for validation logic for `txIdFromParams`. Expect: Proper validation checks.
rg --type ts 'txIdFromParams' --context 5
Length of output: 6346
Aman-zishan
force-pushed
the
feat/cancel-pending-transaction
branch
from
e44e80c to
7f46856
Compare
Aman-zishan
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- src/app/common/hooks/use-loading.ts (1 hunks)
- src/app/common/utils/get-burn-address.ts (1 hunks)
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx (1 hunks)
- src/app/components/stacks-transaction-item/cancel-transaction-button.tsx (1 hunks)
- src/app/components/stacks-transaction-item/increase-fee-button.tsx (1 hunks)
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx (4 hunks)
- src/app/components/transaction-item/transaction-item.layout.tsx (3 hunks)
- src/app/features/dialogs/cancel-transaction-dialog/cancel-stx-transaction-dialog.tsx (1 hunks)
- src/app/features/dialogs/cancel-transaction-dialog/components/cancel-transaction-actions.tsx (1 hunks)
- src/app/features/dialogs/cancel-transaction-dialog/hooks/use-selected-tx.ts (1 hunks)
- src/app/features/dialogs/cancel-transaction-dialog/hooks/use-stx-cancel-transaction.ts (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 (14)
- src/app/common/hooks/use-loading.ts
- src/app/common/utils/get-burn-address.ts
- src/app/components/bitcoin-transaction-item/bitcoin-transaction-item.tsx
- src/app/components/stacks-transaction-item/cancel-transaction-button.tsx
- src/app/components/stacks-transaction-item/increase-fee-button.tsx
- src/app/components/stacks-transaction-item/stacks-transaction-item.tsx
- src/app/components/transaction-item/transaction-item.layout.tsx
- src/app/features/dialogs/cancel-transaction-dialog/cancel-stx-transaction-dialog.tsx
- src/app/features/dialogs/cancel-transaction-dialog/components/cancel-transaction-actions.tsx
- src/app/features/dialogs/cancel-transaction-dialog/hooks/use-selected-tx.ts
- src/app/features/dialogs/cancel-transaction-dialog/hooks/use-stx-cancel-transaction.ts
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx
- src/app/routes/app-routes.tsx
- src/shared/route-urls.ts
alter-eggo
requested review from
markmhendrickson,
314159265359879,
alter-eggo and
kyranjamie
|
Without a dev build, it is hard for me to test this. I assume it didn't build due to the failing tests. I think the "cancel transaction" text on the button could be shorter. Just "Cancel" is enough. That may leave enough room for a small symbol infront of it 🚫, 🛑 ? or simply an "x" in the same color as the >> in front of "increase fee" to match? |
Yeah the workflows are pending and i think approval is required for build process. Is there any way i can help? i can upload the build and share a link
I tried adding an icon but reverted it cause the longer text was resizing the icon, but making it "cancel" with an icon makes sense. I can try that and post a image here later and if it looks good i will commit the changes. |
There was a problem hiding this comment.
Thanks for the contribution @Aman-zishan—we'll try and review & QA this as soon as possible
| whenWallet({ | ||
| ledger: () => | ||
| whenPageMode({ | ||
| full: () => navigate(RouteUrls.CancelStxTransaction), | ||
| popup: () => openIndexPageInNewTab(RouteUrls.CancelStxTransaction, urlSearchParams), | ||
| })(), | ||
| software: () => navigate(RouteUrls.CancelStxTransaction), | ||
| })(); |
There was a problem hiding this comment.
I think we can always just navigate? Not sure we need to handle ledger/software differently here.
There was a problem hiding this comment.
I think we can always just navigate? Not sure we need to handle ledger/software differently here.
I coped this from increase fee logic not sure why it was in that way since increase fee was working fine i thought it's better to rely on already established code
There was a problem hiding this comment.
thanks for the contribution @Aman-zishan!
will test it a little bit later
| amount: 1, | ||
| memo: '_cancel transaction', | ||
| network: network, | ||
| } as any, |
There was a problem hiding this comment.
is it possible to avoid any here and above?
There was a problem hiding this comment.
is it possible to avoid
anyhere and above?
I didnt look much into type casting as i found this in the codebase and actually copied the options from here and did not want to cause any breaking changes
| isActive && ( | ||
| <styled.button | ||
| _hover={{ color: 'ink.text-subdued' }} | ||
| bg="ink.background-primary" | ||
| maxWidth="125px" | ||
| ml="auto" | ||
| onClick={e => { | ||
| onCancelTransaction(); | ||
| e.stopPropagation(); | ||
| }} | ||
| pointerEvents={!isActive ? 'none' : 'all'} | ||
| position="relative" | ||
| px="space.02" | ||
| py="space.01" | ||
| rounded="xs" | ||
| zIndex={999} | ||
| > | ||
| <HStack gap="space.01"> | ||
| <styled.span textStyle="label.03" color="yellow.action-primary-default"> | ||
| Cancel transaction | ||
| </styled.span> | ||
| </HStack> | ||
| </styled.button> |
There was a problem hiding this comment.
code here is pretty much similar to increase fee btn, can we create component to avoid code duplication?
There was a problem hiding this comment.
code here is pretty much similar to increase fee btn, can we create component to avoid code duplication?
Yes that makes sense, i can work on it
|
@kyranjamie Thanks for the review comments. I'm starting to address them. Is QA still pending? If so, I can continue working on the feedback alongside QA to streamline the process. Let me know if that works or if you'd prefer I wait until QA is complete. |
| export function useStxCancelTransaction() { | ||
| const [rawTxId, setRawTxId] = useRawTxIdState(); | ||
| const tx = useSelectedTx(); | ||
| const [, setTxId] = useRawTxIdState(); |
There was a problem hiding this comment.
Made a comment on the duplicated PR, but this code can't just be copied from the increase fee dialog. I have refactored it here bc there is code duplication causing bugs. See changes here: #5500
useRawTxIdState is an old atom being deprecated.
|
closing in favour of #5533 |
feature for stacks transactions Add option to cancel pending transaction #3406
New layout to have cancel transaction & increase fee without affecting tx caption
Attempts to cancel a transaction by crafting a new Tx that sends 1µSTX to burn address with a increase of 1µSTX to the previous fee.
Notifies the user once the transaction is fired
test case video (sending max to an address and trying to cancel that trasaction)
Screen.Recording.2024-06-07.at.11.02.00.AM.mov
https://explorer.hiro.so/txid/0x454c0b34dcaefd619766472652309dce44b95ad1b7795f3b9763d7403c28c2f1?chain=testnet
Summary by CodeRabbit
New Features
CancelTransactionButtoncomponent for canceling transactions.CancelStxTransactionDialogfor managing Stacks transaction cancellations.CancelTransactionActionscomponent with cancel and submit buttons.Enhancements
IncreaseFeeButtonto conditionally render based on theisActivestate.CancelStxTransaction.Refactor
rightElementprop toactionButtonGroupElementin various components for better clarity.Bug Fixes