refactor(swap, gui): Move redeem/refund input page to the end#479
refactor(swap, gui): Move redeem/refund input page to the end#479
Conversation
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
swap/src/cli/api/request.rs (1)
1038-1040: Essential validation for security.The validation calls
assert_networkandassert_sum_to_oneare crucial for ensuring the Monero receive pool is properly configured and secure.
🧹 Nitpick comments (2)
src-gui/src/renderer/components/pages/swap/swap/SwapWidget.tsx (1)
87-87: Complex conditional logic for CancelButton - consider refactoringThe condition
!(isWaitingForBtcDeposit && !isSelectedSwapOffer)is logically equivalent to!isWaitingForBtcDeposit || isSelectedSwapOfferbut may be harder to understand. Consider extracting this to a well-named boolean variable for clarity.+ const shouldShowCancelButton = !isWaitingForBtcDeposit || isSelectedSwapOffer; + <Box sx={{ display: "flex", alignItems: "center", justifyContent: isWaitingForBtcDeposit ? "flex-end" : "space-between", }} > - {!(isWaitingForBtcDeposit && !isSelectedSwapOffer) && <CancelButton />} + {shouldShowCancelButton && <CancelButton />} <DebugPageSwitchBadge enabled={debug} setEnabled={setDebug} /> </Box>src-gui/src/renderer/rpc.ts (1)
250-253: Type casting is necessary but could be improved.The cast to
SelectOfferApprovalRequestis required for API integration, but usingas unknown assuggests a type mismatch. Consider ensuring the constructed object fully matches the expected interface.await resolveApproval(requestId, { bitcoin_change_address: btc_change_address, monero_receive_pool: address_pool, } satisfies SelectOfferApprovalRequest);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (33)
src-tauri/gen/apple/.gitignoreis excluded by!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]is excluded by!**/*.png,!**/gen/**src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/Contents.jsonis excluded by!**/gen/**src-tauri/gen/apple/Assets.xcassets/Contents.jsonis excluded by!**/gen/**src-tauri/gen/apple/ExportOptions.plistis excluded by!**/gen/**src-tauri/gen/apple/LaunchScreen.storyboardis excluded by!**/gen/**src-tauri/gen/apple/Podfileis excluded by!**/gen/**src-tauri/gen/apple/Sources/unstoppableswap-gui-rs/bindings/bindings.his excluded by!**/gen/**src-tauri/gen/apple/Sources/unstoppableswap-gui-rs/main.mmis excluded by!**/gen/**src-tauri/gen/apple/project.ymlis excluded by!**/gen/**src-tauri/gen/apple/unstoppableswap-gui-rs.xcodeproj/project.pbxprojis excluded by!**/gen/**src-tauri/gen/apple/unstoppableswap-gui-rs.xcodeproj/project.xcworkspace/contents.xcworkspacedatais excluded by!**/gen/**src-tauri/gen/apple/unstoppableswap-gui-rs.xcodeproj/project.xcworkspace/xcshareddata/WorkspaceSettings.xcsettingsis excluded by!**/gen/**src-tauri/gen/apple/unstoppableswap-gui-rs.xcodeproj/xcshareddata/xcschemes/unstoppableswap-gui-rs_iOS.xcschemeis excluded by!**/gen/**src-tauri/gen/apple/unstoppableswap-gui-rs_iOS/Info.plistis excluded by!**/gen/**src-tauri/gen/apple/unstoppableswap-gui-rs_iOS/unstoppableswap-gui-rs_iOS.entitlementsis excluded by!**/gen/**
📒 Files selected for processing (12)
src-gui/src/models/storeModel.ts(1 hunks)src-gui/src/renderer/components/pages/swap/swap/SwapStatePage.tsx(3 hunks)src-gui/src/renderer/components/pages/swap/swap/SwapWidget.tsx(2 hunks)src-gui/src/renderer/components/pages/swap/swap/init/AddressInputPage.tsx(3 hunks)src-gui/src/renderer/components/pages/swap/swap/init/deposit_and_choose_offer/DepositAndChooseOfferPage.tsx(2 hunks)src-gui/src/renderer/components/pages/swap/swap/init/deposit_and_choose_offer/MakerOfferItem.tsx(2 hunks)src-gui/src/renderer/rpc.ts(4 hunks)src-gui/src/store/features/swapSlice.ts(2 hunks)src-tauri/tauri.conf.json.backup(1 hunks)swap/src/cli/api/request.rs(9 hunks)swap/src/cli/api/tauri_bindings.rs(5 hunks)swap/src/cli/command.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src-tauri/tauri.conf.json.backup
🚧 Files skipped from review as they are similar to previous changes (2)
- swap/src/cli/command.rs
- swap/src/cli/api/tauri_bindings.rs
🧰 Additional context used
🧬 Code Graph Analysis (6)
src-gui/src/renderer/components/pages/swap/swap/SwapStatePage.tsx (4)
src-gui/src/store/hooks.ts (2)
useAppDispatch(34-34)useAppSelector(35-35)src-gui/src/renderer/components/pages/swap/swap/init/AddressInputPage.tsx (1)
AddressInputPage(10-139)src-gui/src/store/features/swapSlice.ts (1)
setSelectedOfferPeerId(41-43)src-gui/src/renderer/components/pages/swap/swap/init/deposit_and_choose_offer/DepositAndChooseOfferPage.tsx (1)
DepositAndChooseOfferPage(12-163)
src-gui/src/renderer/components/pages/swap/swap/init/deposit_and_choose_offer/DepositAndChooseOfferPage.tsx (1)
src-gui/src/models/tauriModelExt.ts (1)
TauriSwapProgressEventContent(16-18)
src-gui/src/renderer/components/pages/swap/swap/SwapWidget.tsx (6)
src-gui/src/store/hooks.ts (2)
useAppSelector(35-35)useActiveSwapInfo(133-136)src-gui/src/renderer/rpc.ts (1)
buyXmr(199-216)src-gui/src/renderer/components/pages/swap/swap/SwapStatePage.tsx (1)
SwapStatePage(29-146)src-gui/src/renderer/components/modal/swap/SwapStateStepper.tsx (1)
SwapStateStepper(176-195)src-gui/src/renderer/components/pages/swap/swap/CancelButton.tsx (1)
CancelButton(9-51)src-gui/src/renderer/components/modal/swap/pages/DebugPageSwitchBadge.tsx (1)
DebugPageSwitchBadge(5-27)
src-gui/src/renderer/components/pages/swap/swap/init/AddressInputPage.tsx (3)
src-gui/src/store/hooks.ts (1)
usePendingSelectMakerApproval(219-222)src-gui/src/renderer/rpc.ts (1)
resolveSelectMakerApproval(218-254)src-gui/src/store/features/swapSlice.ts (1)
setSelectedOfferPeerId(41-43)
swap/src/cli/api/request.rs (1)
swap/src/cli/api/tauri_bindings.rs (1)
request_approval(237-348)
src-gui/src/renderer/rpc.ts (2)
src-gui/src/models/rpcModel.ts (1)
BuyXmrResponse(64-66)src-gui/src/store/features/settingsSlice.ts (1)
DonateToDevelopmentTip(4-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (macos-latest)
- GitHub Check: build (x86_64-apple-darwin, macos-13)
- GitHub Check: build (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (29)
src-gui/src/models/storeModel.ts (1)
14-14: LGTM: Clean interface extensionThe addition of
selectedOfferPeerId: string | nullproperly extends the SwapSlice interface to track the selected offer peer ID. The nullable type correctly represents optional selection state.src-gui/src/store/features/swapSlice.ts (3)
11-11: LGTM: Proper Redux state initializationThe
selectedOfferPeerId: nullinitialization correctly establishes the default state for tracking selected offers.
41-43: LGTM: Well-implemented Redux reducerThe
setSelectedOfferPeerIdreducer correctly handles both setting a peer ID string and clearing the selection with null. The implementation follows Redux Toolkit best practices.
47-47: LGTM: Proper action exportThe
setSelectedOfferPeerIdaction is correctly included in the exported actions for use in components.src-gui/src/renderer/components/pages/swap/swap/init/deposit_and_choose_offer/DepositAndChooseOfferPage.tsx (2)
16-19: LGTM: Well-typed callback propThe
onSelectOfferprop is properly typed with a clear function signature(peerId: string) => voidand correctly extends the existing type interface.
129-129: LGTM: Proper callback propagationThe
onSelectOffercallback is correctly passed down to theMakerOfferItemcomponents, enabling proper offer selection handling.src-gui/src/renderer/components/pages/swap/swap/SwapWidget.tsx (3)
1-8: LGTM: Clean import additionsThe new imports are properly organized and include the necessary MUI Skeleton component and React hooks for the enhanced functionality.
25-30: LGTM: Clear boolean state derivationsThe
isWaitingForBtcDepositandisSelectedSwapOfferboolean constants clearly derive UI state from Redux, making the conditional logic more readable.
56-58: LGTM: Appropriate loading stateUsing
Skeletoncomponent whenswap.stateis null provides good UX feedback while the swap is initializing.src-gui/src/renderer/components/pages/swap/swap/SwapStatePage.tsx (5)
2-3: LGTM: Proper Redux hook importsThe Redux hooks
useAppDispatchanduseAppSelectorare correctly imported along with thesetSelectedOfferPeerIdaction.
31-32: LGTM: Clean Redux state selectionThe Redux state selection for
selectedOfferPeerIdfollows the proper pattern and uses typed selectors.
34-41: LGTM: Well-structured conditional renderingThe early return pattern for rendering
AddressInputPagewhen an offer is selected is clean and provides both the selected peer ID and a setter function for state management.
53-58: LGTM: Proper callback integrationThe
onSelectOffercallback correctly dispatches thesetSelectedOfferPeerIdaction, integrating the offer selection with Redux state management.
27-27: LGTM: Component import updateThe change from
InitPagetoAddressInputPagereflects the component's refactored functionality for handling address input after offer selection.src-gui/src/renderer/components/pages/swap/swap/init/deposit_and_choose_offer/MakerOfferItem.tsx (2)
14-18: LGTM! Clean separation of concerns.The refactoring successfully lifts the offer selection logic to higher-level components by introducing the
onSelectOffercallback. This improves component reusability and centralizes state management.
110-127: Well-implemented user feedback with proper accessibility.The Tooltip and Button implementation provides clear user feedback when insufficient Bitcoin is available, and the
spanwrapper ensures the tooltip works correctly with the disabled button.src-gui/src/renderer/components/pages/swap/swap/init/AddressInputPage.tsx (4)
10-16: Good component renaming and prop interface.The component rename from
InitPagetoAddressInputPagebetter reflects its purpose, and the new props clearly define the selected offer state management.
17-20: Proper integration with Redux state management.The use of
usePendingSelectMakerApprovalhook and finding the selected offer bypeer_idcorrectly integrates with the new state management flow.
36-45: Correct refactoring to new approval flow.The function rename to
confirmOfferand the call toresolveSelectMakerApprovalproperly align with the new architecture where offer confirmation is separated from the initial buy request. Clearing the selection after confirmation is the correct behavior.
131-135: Appropriate button text update.The button text change from "Continue" to "Confirm" better reflects the new action being performed - confirming a selected offer rather than continuing to the next step.
src-gui/src/renderer/rpc.ts (2)
199-216: Excellent simplification of buyXmr function.The function is now focused solely on initiating the buy request without address concerns, which aligns perfectly with the new architecture where address handling is deferred until after approval.
218-254: Well-implemented approval resolution function.The
resolveSelectMakerApprovalfunction correctly constructs the Monero receive pool with donation split logic and properly integrates with the backend API through theSelectOfferApprovalRequesttype.swap/src/cli/api/request.rs (7)
961-969: Good refactoring of determine_btc_to_swap return values.The function now returns the Bitcoin change address and Monero receive pool as part of its result tuple, properly integrating with the new approval flow architecture.
1025-1027: Correct update to approval callback signature.The callback now returns
Option<SelectOfferApprovalRequest>instead of a simple boolean, allowing it to provide rich address data when the user approves the offer.
1041-1056: Proper Bitcoin change address handling with fallback.The logic correctly handles both external change addresses and falls back to internal wallet addresses when none is provided. The network validation and logging are appropriate.
1650-1654: Correct function signature update.The
request_approvalparameter signature is properly updated to match the new approval flow that returns rich approval data instead of a simple boolean.
1661-1663: Proper return type updates.The function return type is correctly updated to include the optional Bitcoin change address and Monero address pool, maintaining type safety throughout the refactoring.
1749-1774: Well-structured approval response handling.The approval response handling correctly unwraps the
SelectOfferApprovalRequestto extract the Bitcoin change address and Monero receive pool, with proper error handling for rejected or failed approvals.
1819-1841: Consistent return value structure.The final return statement correctly includes all the new fields (Bitcoin change address and Monero receive pool) that were added to the function signature, maintaining consistency throughout the refactor.
...derer/components/pages/swap/swap/init/deposit_and_choose_offer/DepositAndChooseOfferPage.tsx
Show resolved
Hide resolved
|
We shouldn't save the peer id instead we should, split it into two approval requests:
This way we won't need any global state. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src-gui/src/renderer/components/pages/swap/swap/init/AddressInputPage.tsx (1)
16-18: Well-structured refactoring to support the new approval flow.The component has been successfully refactored to handle the
SpecifyRedeemRefundapproval request. The logic for constructing the address pool with donation support is clear and correct. The early return check forspecifyRedeemRefundApprovalproperly prevents undefined access issues.Also applies to: 34-67
🧹 Nitpick comments (1)
src-gui/src/renderer/components/pages/swap/swap/init/AddressInputPage.tsx (1)
12-14: Consider moving donation addresses to configuration.While the hardcoded donation addresses work, consider moving them to a configuration file for better maintainability and easier updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src-gui/src/models/tauriModelExt.ts(4 hunks)src-gui/src/renderer/components/pages/swap/SwapPage.tsx(1 hunks)src-gui/src/renderer/components/pages/swap/swap/SwapStatePage.tsx(1 hunks)src-gui/src/renderer/components/pages/swap/swap/SwapWidget.tsx(2 hunks)src-gui/src/renderer/components/pages/swap/swap/init/AddressInputPage.tsx(3 hunks)src-gui/src/renderer/components/pages/swap/swap/init/deposit_and_choose_offer/DepositAndChooseOfferPage.tsx(1 hunks)src-gui/src/renderer/components/pages/swap/swap/init/deposit_and_choose_offer/MakerOfferItem.tsx(1 hunks)src-gui/src/renderer/rpc.ts(2 hunks)src-gui/src/store/hooks.ts(2 hunks)src-gui/src/utils/sortUtils.ts(2 hunks)swap/src/cli/api/request.rs(2 hunks)swap/src/cli/api/tauri_bindings.rs(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- src-gui/src/renderer/components/pages/swap/swap/init/deposit_and_choose_offer/DepositAndChooseOfferPage.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src-gui/src/renderer/components/pages/swap/swap/init/deposit_and_choose_offer/MakerOfferItem.tsx
- src-gui/src/renderer/rpc.ts
- src-gui/src/renderer/components/pages/swap/swap/SwapWidget.tsx
- swap/src/cli/api/request.rs
- src-gui/src/renderer/components/pages/swap/swap/SwapStatePage.tsx
- swap/src/cli/api/tauri_bindings.rs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: eigenwallet/core#0
File: monero-sys/CLAUDE.md:0-0
Timestamp: 2025-07-23T20:01:33.031Z
Learning: Applies to monero-sys/src/lib.rs : In src/lib.rs, provide idiomatic Rust interfaces to the C++ code, use wrapper types (WalletManager, Wallet) with safer interfaces, and handle memory management and safety concerns (never expose raw pointers, implement Send and Sync, use Pin for C++ objects requiring stable memory addresses).
Learnt from: CR
PR: eigenwallet/core#0
File: monero-sys/CLAUDE.md:0-0
Timestamp: 2025-07-23T20:01:33.031Z
Learning: Applies to monero-sys/src/lib.rs : Implement proper deref for wrapper types and use the OnceLock pattern to ensure WalletManager is a singleton.
src-gui/src/renderer/components/pages/swap/SwapPage.tsx (1)
Learnt from: CR
PR: eigenwallet/core#0
File: AGENT.md:0-0
Timestamp: 2025-07-23T20:01:42.422Z
Learning: Applies to swap/Cargo.toml : The swap/Cargo.toml file is frequently edited and should be reviewed carefully for dependency and configuration changes
🧬 Code Graph Analysis (1)
src-gui/src/store/hooks.ts (1)
src-gui/src/models/tauriModelExt.ts (4)
PendingSelectOfferApprovalRequest(315-317)isPendingSelectOfferApprovalEvent(343-353)PendingSpecifyRedeemRefundApprovalRequest(319-321)isPendingSpecifyRedeemRefundApprovalEvent(355-365)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build (aarch64-apple-darwin, macos-latest)
- GitHub Check: test (macos-latest)
- GitHub Check: build (x86_64-apple-darwin, macos-13)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (5)
src-gui/src/store/hooks.ts (1)
13-16: LGTM! Hooks follow established patterns.The new hooks
usePendingSelectOfferApprovalandusePendingSpecifyRedeemRefundApprovalare implemented correctly, following the same pattern as existing approval hooks. They properly utilize the type guards and integrate seamlessly with the Redux store.Also applies to: 228-236
src-gui/src/utils/sortUtils.ts (1)
3-3: LGTM! Sorting function correctly handles the new approval type.The
sortSelectOfferApprovalsAndKnownQuotesfunction is well-implemented and follows the established pattern. The difference in spreading (...approval.request.contentinstead of...approval.request.content.maker) correctly reflects the different structure ofPendingSelectOfferApprovalRequest.Also applies to: 35-58
src-gui/src/renderer/components/pages/swap/SwapPage.tsx (2)
15-81: Well-implemented auto-suspension logic with proper cleanup.The implementation correctly handles both navigation and visibility-based suspension with appropriate timer management and cleanup. The 10-second grace period and the check for locked funds provide a good user experience while preventing data loss.
13-13: Consider potential race conditions between timers.Both effects can potentially set timers simultaneously (e.g., user navigates away while the page is also hidden). While the current implementation should work correctly due to proper cleanup, consider if you need additional logic to prevent multiple timers or to handle edge cases.
Also applies to: 30-37, 60-67
src-gui/src/models/tauriModelExt.ts (1)
9-9: Type definitions and guards correctly implement the new approval flow.All new types and type guards follow the established patterns in the codebase:
PendingSelectOfferApprovalRequestandPendingSpecifyRedeemRefundApprovalRequestproperly extend the base approval type- Type guards correctly check both the pending state and request type
SortableQuoteWithAddressappropriately extends the base quote type for sorting functionalityAlso applies to: 315-321, 343-365, 391-394
…ress, refundAddress) and remove params from buyXmr(...)
696ffca to
084fb90
Compare
|
We have to
|
e58cf0b to
8b2b28d
Compare
|
@Einliterflasche Do you think this is even necessary? Just moving the "where to redeem xmr to" / "where to refund bitcoin to" to the setting and defaulting to internal would be a lot easier actually... |
You're right, let's do that |
Note
Adds a new SelectOffer approval to collect redeem/refund addresses after maker selection, refactoring buy/start flow in backend and GUI to show an address input step and removing pre-swap address inputs.
ApprovalRequestType::SelectOfferwithSelectOfferDetailsandSelectOfferApprovalRequest;TauriEmitter.request_specify_redeem_refundadded.buy_xmrno longer takes addresses; after offer selection it requests redeem/refund addresses, validates/stores them, then starts swap.determine_btc_to_swapnow returnsQuoteWithAddress(single tuple element); consumers adjusted. CLI comment hints at future passing of change addr.AddressInputPageto input external Monero redeem and BTC refund addresses (with internal-wallet switches); shown when pendingSelectOfferapproval.InitPage;SwapStatePageprioritizes pendingSelectOfferby renderingAddressInputPage.SwapWidgetauto-startsbuyXmr()on null state, adds loadingSkeleton, and hides cancel during deposit/address input.usePendingSelectOfferApproval; sorting utilities extended for selectable offers (andSortableQuoteWithAddress).BitcoinAddressTextFieldandMoneroAddressTextFieldconverted toforwardRef; Monero field keeps recent addresses dialog.MakerOfferItemsupportsnoButton;SwapSetupInflightPagehides confirm while address selection pending.buyXmr()(no addresses); addedconfirmOfferWithAddresses()to resolveSelectOfferwith chosen addresses; improved init logging.Written by Cursor Bugbot for commit bf10c34. This will update automatically on new commits. Configure here.