Conversation
…and the ramp/deposit feature flow
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive private receive functionality for the Moonlight wallet, allowing users to generate receiving addresses (UTXOs) for confidential transactions. The implementation follows established patterns from the deposit flow and includes both backend handlers and frontend UI components.
Changes:
- Added private receive backend handler with UTXO reservation and random amount partitioning
- Implemented deposit backend handler with bundle submission to privacy providers
- Created receive and deposit frontend flows with form and confirmation pages
- Added cryptographically secure random partitioning utility for privacy enhancement
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/background/handlers/private/receive.ts | New handler for generating receive addresses with 5 UTXOs |
| src/background/handlers/private/receive.types.ts | Type definitions for receive requests and responses |
| src/background/handlers/private/deposit.ts | New handler for depositing funds to private channels |
| src/background/handlers/private/deposit.types.ts | Type definitions for deposit requests and responses |
| src/background/utils/random-partition.ts | Cryptographic random partitioning utility for privacy |
| src/background/services/privacy-provider-client.ts | Enhanced token validation and bundle submission |
| src/background/messages.ts | Added Receive and Deposit message types |
| src/background/handler.ts | Registered new handlers |
| src/popup/pages/receive-page.tsx | Receive form page component |
| src/popup/pages/receive-confirmation-page.tsx | Receive confirmation and address display page |
| src/popup/pages/deposit-page.tsx | Deposit form page component |
| src/popup/pages/deposit-review-page.tsx | Deposit review page component |
| src/popup/templates/receive-form-template.tsx | Reusable receive form template |
| src/popup/templates/deposit-form-template.tsx | Reusable deposit form template |
| src/popup/templates/deposit-review-template.tsx | Deposit review template |
| src/popup/templates/home-template.tsx | Integrated receive and deposit buttons in private view |
| src/popup/api/receive.ts | API wrapper for receive handler |
| src/popup/api/deposit.ts | API wrapper for deposit handler |
| src/popup/hooks/state.tsx | Added receive and deposit state management |
| src/popup/app.tsx | Added routing for new pages |
| deno.json | Removed blank lines between script groups |
| deno.lock | Updated dependency versions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </Text> | ||
| </div> | ||
| <Text className="text-sm font-medium whitespace-nowrap"> | ||
| {parseFloat(utxo.amount) / 10_000_000} XLM |
There was a problem hiding this comment.
Manual division by 10_000_000 is used here to convert stroops to XLM, but the codebase uses the toDecimals utility function from @colibri/core for this purpose elsewhere (see home-template.tsx). Consider using toDecimals(BigInt(utxo.amount), 7) instead for consistency and to avoid potential floating-point precision issues.
| return { | ||
| type: MessageType.Receive, | ||
| ok: false, | ||
| error: { code: "INVALID_AMOUNT", message: "Invalid deposit amount" }, |
There was a problem hiding this comment.
The error message says "Invalid deposit amount" but this is the receive handler, not deposit. The message should say "Invalid receive amount" for consistency and to avoid confusion.
| error: { code: "INVALID_AMOUNT", message: "Invalid deposit amount" }, | |
| error: { code: "INVALID_AMOUNT", message: "Invalid receive amount" }, |
| // Handle leftover from integer division | ||
| const remainderExtra = remaining - assignedExtra; | ||
| if (remainderExtra > 0n) { | ||
| // Distribute the leftover randomly among parts | ||
| const remainderCount = Number(remainderExtra); | ||
| const indices = new Set<number>(); | ||
|
|
||
| // Generate unique random indices to receive +1 | ||
| while (indices.size < remainderCount && indices.size < partsCount) { | ||
| const idx = Number( | ||
| randomIntInRangeBigInt(0n, BigInt(partsCount - 1)), | ||
| ); | ||
| indices.add(idx); | ||
| } | ||
|
|
||
| // Add +1 to the selected indices | ||
| for (const idx of indices) { | ||
| amounts[idx] += 1n; | ||
| } | ||
| } |
There was a problem hiding this comment.
The code only handles the case when remainderExtra > 0n but doesn't handle the case when it might be negative (which could theoretically happen due to rounding in integer division). While the mathematical logic suggests this shouldn't occur, defensive programming would suggest either adding an assertion that remainderExtra >= 0n or handling the negative case to prevent potential silent bugs.
| sumWeights += weight; | ||
| } | ||
|
|
||
| // Distribuir o remaining proporcionalmente aos pesos |
There was a problem hiding this comment.
Comment is in Portuguese instead of English. The comment should be translated to maintain consistency with the rest of the codebase, which uses English for all comments and documentation.
| // Distribuir o remaining proporcionalmente aos pesos | |
| // Distribute the remaining amount proportionally to the weights |
| > | ||
| {copiedIndex === index | ||
| ? <IconInfoCircle className="h-4 w-4 text-green-400" /> | ||
| : <IconExternalLink className="h-4 w-4" />} |
There was a problem hiding this comment.
The icon used for the copy button is misleading. IconExternalLink suggests opening an external link, but the button actually copies the address to clipboard. Consider using IconCopy instead (which is already imported) to match the user's expectation and the MLXDR copy button pattern above.
| // Handle leftover from integer division | ||
| const remainderExtra = remaining - assignedExtra; | ||
| if (remainderExtra > 0n) { | ||
| // Distribute the leftover randomly among parts |
There was a problem hiding this comment.
Converting a potentially large bigint value to Number on line 113 could cause issues if remainderExtra exceeds Number.MAX_SAFE_INTEGER (2^53 - 1). While this is unlikely in practice given the typical transaction amounts, it would be safer to add a check or use a different approach. Consider adding a guard: if (remainderExtra > BigInt(Number.MAX_SAFE_INTEGER)) throw new Error("Remainder too large to distribute") before the conversion.
| // Distribute the leftover randomly among parts | |
| // Distribute the leftover randomly among parts | |
| if (remainderExtra > BigInt(Number.MAX_SAFE_INTEGER)) { | |
| throw new Error("Remainder too large to distribute"); | |
| } |
| const canStartDeposit = Boolean(selectedPrivateChannel?.id) && | ||
| Boolean(selectedPrivateChannel?.selectedProviderId) && | ||
| Boolean(props.isConnected) && | ||
| Boolean(props.onStartDeposit); |
There was a problem hiding this comment.
The variable canStartDeposit is misleadingly named as it's used to enable/disable both the Receive and Ramp (deposit) buttons, not just deposit. Consider renaming it to something more generic like canStartPrivateAction or canUsePrivateFeatures to better reflect its actual usage across multiple private operations.
| const canStartDeposit = Boolean(selectedPrivateChannel?.id) && | |
| Boolean(selectedPrivateChannel?.selectedProviderId) && | |
| Boolean(props.isConnected) && | |
| Boolean(props.onStartDeposit); | |
| const canUsePrivateFeatures = Boolean(selectedPrivateChannel?.id) && | |
| Boolean(selectedPrivateChannel?.selectedProviderId) && | |
| Boolean(props.isConnected) && | |
| Boolean(props.onStartDeposit); | |
| // Legacy alias: used to gate multiple private actions (e.g. Receive and Ramp) | |
| const canStartDeposit = canUsePrivateFeatures; |
| useMemo(() => { | ||
| if (!formData) return; | ||
| getPrivateChannels({ network }) | ||
| .then((res) => { | ||
| if (res.ok) { | ||
| setPrivateChannels({ | ||
| channels: res.channels, | ||
| selectedChannelId: res.selectedChannelId, | ||
| }); | ||
| } | ||
| }) | ||
| .catch((err) => { | ||
| console.error("Failed to load private channels", err); | ||
| }); | ||
| }, [network, formData]); |
There was a problem hiding this comment.
useMemo is being misused here for side effects. The useMemo hook is intended for memoizing computed values, not for executing side effects like API calls. This should be changed to useEffect instead. Using useMemo for side effects can lead to unpredictable behavior since React may choose to discard memoized values at any time.
| provider={selectedProvider} | ||
| amount={amount} | ||
| setAmount={setAmount} | ||
| maxAmount="1,000" |
There was a problem hiding this comment.
The hardcoded maxAmount="1,000" appears arbitrary and doesn't seem to be based on any actual validation or constraint. Consider either removing this prop if there's no real maximum limit for receiving, or implementing proper validation based on actual system constraints. The comma formatting is also inconsistent with how amounts are typically represented in the codebase.
| maxAmount="1,000" |
| "pre-commit": "deno run --allow-run --allow-read src/scripts/pre-commit.ts", | ||
| "prepare": "deno run --allow-read --allow-write src/scripts/install-hooks.ts", | ||
|
|
||
| "fmt": "deno fmt", |
There was a problem hiding this comment.
Blank lines were removed between script groups. While this doesn't affect functionality, it reduces readability by removing visual separation between different groups of scripts (build scripts, git hooks, and formatting/linting tools).
1 participant
This PR implements the private receive flow, allowing users to generate receiving addresses (UTXOs) for receiving confidential transactions. The implementation follows the same patterns as the deposit flow and aligns with the sandbox reference implementation.
Overview
Users can now generate receiving addresses by specifying an amount they wish to receive. The system will:
Features
Receive Form: Input screen for specifying the amount to receive
Receive Confirmation: Display screen showing generated receiving addresses
Home Integration: Receive button in private view now navigates to the receive flow
Technical Changes
Backend
New handler:
src/background/handlers/private/receive.tsUtxoBasedStellarAccountpartitionAmountRandomutilityMoonlightOperation.createoperationsNew types:
src/background/handlers/private/receive.types.tsReceiveRequest: network, channelId, providerId, accountId, amountReceiveResponse: operationsMLXDR, utxos array, metadataMessage system: Added
MessageType.Receiveto message routingFrontend
New API service:
src/popup/api/receive.tsNew pages:
src/popup/pages/receive-page.tsx: Main receive form pagesrc/popup/pages/receive-confirmation-page.tsx: Confirmation and address displayNew template:
src/popup/templates/receive-form-template.tsxState management: Added receive routes and state handling
receive,receive-confirmationgoReceive,goReceiveConfirmation,setReceiveFormData,setReceiveResult,clearReceiveDataNavigation: Integrated receive button in
HomeTemplateprivate viewImplementation Details
partitionAmountRandomFiles Changed
New Files:
src/background/handlers/private/receive.tssrc/background/handlers/private/receive.types.tssrc/popup/api/receive.tssrc/popup/pages/receive-page.tsxsrc/popup/pages/receive-confirmation-page.tsxsrc/popup/templates/receive-form-template.tsxModified Files:
src/background/messages.ts- Added Receive message typesrc/background/handler.ts- Registered receive handlersrc/popup/hooks/state.tsx- Added receive routes and statesrc/popup/app.tsx- Added receive route handlingsrc/popup/pages/home-page.tsx- Connected receive buttonsrc/popup/templates/home-template.tsx- Added receive action propsrc/popup/pages/deposit-page.tsx- Fixed chain state response handlingTesting Notes
Future Enhancements