refactor(web): remove mock simulation methods from StellarService, route through SDK - #547
Conversation
…ute through SDK Removes all mock simulation methods (createPaymentStream, getAccountInfo, getWithdrawableAmount, withdrawFromStream, depositToStream, getStreamDetails) from apps/web/src/lib/stellar.ts that used Math.random() and setTimeout. All payment stream operations now exclusively route through @/services/stellar.service.ts and the SDK client wrappers in @/lib/api.ts. Adds tests for all remaining utility methods (validateStellarAddress, formatAmount, formatTokenAmount, calculateStreamProgress) covering success paths, failure paths, and edge cases.
|
@kathy-ai-art Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughPayment-stream creation, deposits, and withdrawals now use shared contract APIs with wallet signing. Stream records include contract identifiers. Legacy simulated stream operations were removed from ChangesPayment stream API migration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.6)apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsxFile contains syntax errors that prevent linting: Line 204: Expected an expression, or an assignment but instead found 'const'. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx (1)
190-203: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
formDatais now dead code.With
StellarService.createPaymentStream(formData)removed, this object (and thePaymentStreamFormDatatype import) is constructed and never read.🧹 Proposed cleanup
- // Convert form data to the format expected by StellarService - const formData: PaymentStreamFormData = { - recipientAddress: streamData.recipient, - token: streamData.token, - totalAmount: streamData.amount, - duration: streamData.durationValue, - durationUnit: streamData.duration === "hour" ? "hours" : "days", - cancelable: streamData.cancellability, - transferable: streamData.transferability, - }; - const tokenAddress = SUPPORTED_TOKENS.find(t => t.value === streamData.token)?.address;Drop the
PaymentStreamFormDataimport too if it has no other use in this file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx` around lines 190 - 203, Remove the unused formData construction in the payment stream creation flow and delete the PaymentStreamFormData import if it is not referenced elsewhere in CreatePaymentStream. Preserve the tokenAddress validation and subsequent logic unchanged.
🧹 Nitpick comments (5)
apps/web/src/lib/stellar.ts (1)
1-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
@deprecatedhere documents nothing and is misleading.A JSDoc block before an
importisn't attached to any symbol, so editors won't warn consumers. It also contradicts the sentence that follows: the retained utilities (validateStellarAddress,formatAmount,formatTokenAmount,calculateStreamProgress) are still the canonical implementations and are actively imported by the payment-stream modals. Consider dropping the tag and keeping only the explanatory note.♻️ Proposed doc fix
/** - * `@deprecated` Utility methods for Stellar address validation and amount formatting. - * All smart contract operations now route through {`@link` `@/services/stellar.service.ts`} - * or the SDK client wrappers in {`@link` `@/lib/api.ts`}. + * Pure utility helpers for Stellar address validation, amount formatting and + * stream progress calculation. + * + * NOTE: all smart-contract operations live in `@/services/stellar.service.ts` + * or the SDK client wrappers in `@/lib/api.ts` — do not add network calls here. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/stellar.ts` around lines 1 - 5, Remove the `@deprecated` tag from the file-level JSDoc in stellar.ts, retaining only an accurate explanatory note about smart contract operations and the still-canonical utility functions.apps/web/src/lib/api.ts (1)
230-239: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winValidate
streamIdbeforeBigInt().
BigInt(NaN)/BigInt(1.5)throws aRangeErrorfrom inside the SDK call, producing an opaque message in the caller's error toast. A guard here gives a clear failure for every caller.🛡️ Proposed guard
}): Promise<void> { + if (!Number.isInteger(params.streamId) || params.streamId < 0) { + throw new Error(`Invalid stream id: ${params.streamId}`); + } const client = createPaymentStreamClient(params.sender);Otherwise this mirrors
withdraw(Lines 133-152) cleanly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/api.ts` around lines 230 - 239, Update depositToStream to validate params.streamId before converting it with BigInt, rejecting NaN and non-integer values with a clear caller-facing error. Keep the existing client creation, deposit, and signAndSendTx flow unchanged for valid stream IDs, matching the validation behavior used by withdraw.apps/web/src/components/modules/payment-stream/WithdrawStreamModal.tsx (1)
134-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing success notification.
DepositStreamModal.tsx(Line 77) firesnotify.success("Deposit successful!"); the withdraw path closes silently. Add the matching toast for consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/modules/payment-stream/WithdrawStreamModal.tsx` at line 134, Add a success toast in the withdraw completion flow before or alongside the existing onSuccess callback, matching the notification pattern and message used by DepositStreamModal. Update the handler containing onSuccess?.(`withdraw_${Date.now()}`) so withdrawals display “Withdraw successful!” before closing.apps/web/src/components/modules/payment-stream/DepositStreamModal.tsx (1)
61-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the wallet guard above the amount conversion.
BigInt(Math.floor(parseFloat(...) * 1e7))throwsRangeErroron a non-numeric/NaN amount, so ordering the connectivity check first keeps the failure modes distinct and avoids doing work that will be discarded.♻️ Proposed reorder
- const amount = BigInt(Math.floor(parseFloat(data.amount) * 10000000)) - - // Use the real SDK-backed depositToStream from `@/lib/api` - // This routes through PaymentStreamClient.deposit() if (!isConnected || !signTransaction || !address) { notify.error('Wallet not connected'); return; } + + const amount = BigInt(Math.floor(parseFloat(data.amount) * 10000000))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/modules/payment-stream/DepositStreamModal.tsx` around lines 61 - 68, Move the wallet connectivity guard in the deposit submission flow above the amount conversion that computes amount with BigInt. Ensure !isConnected, !signTransaction, or !address returns with notify.error before parsing data.amount, while preserving the existing conversion and connected-wallet behavior.apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx (1)
205-211: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated duration-multiplier ladder.
The same
hour/day/week/month/year → secondsmapping and the same 1e7 scaling already exist inestimateFee(Lines 99-104). ExtractingDURATION_SECONDSand atoStroops(amount)helper keeps the fee estimate and the actual submission from drifting apart.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx` around lines 205 - 211, In CreatePaymentStream, extract the shared hour/day/week/month/year-to-seconds mapping into DURATION_SECONDS and the 1e7 amount conversion into a toStroops(amount) helper, then reuse both from estimateFee and the submission logic around durationMultiplier and amount. Remove the duplicated ladder and inline scaling while preserving the existing fallback and rounding behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx`:
- Around line 205-221: Update handleConfirmStream to revalidate the wallet
address immediately before calling createStream and return or surface an error
when it is unavailable; remove the unsafe address! assertion. Ensure cancelable
and transferable from the form state are either passed through createStream and
its contract call or disabled/clearly annotated in the UI until supported. Also
replace the parseFloat-based amount conversion in this handler with exact
decimal-string scaling before constructing the BigInt.
In `@apps/web/src/components/modules/payment-stream/DepositStreamModal.tsx`:
- Line 78: The payment-stream success callbacks fabricate timestamp IDs instead
of receiving real transaction hashes. Update depositToStream and withdraw in
apps/web/src/lib/api.ts to return the transaction hash, then pass those returned
hashes to onSuccess in
apps/web/src/components/modules/payment-stream/DepositStreamModal.tsx#L78-L78
and
apps/web/src/components/modules/payment-stream/WithdrawStreamModal.tsx#L134-L134,
removing the generated deposit_${Date.now()} and withdraw_${Date.now()} values.
- Around line 70-75: Remove the Number conversion in the deposit flow around
depositToStream and preserve the string StreamRecord.id when passing the stream
identifier. Ensure real stream operations receive a valid numeric contract ID
only after an explicit mapping step, or use the separate numeric ID field if
available; do not pass arbitrary record IDs such as stream_001_abc123def456 to
BigInt-based operations.
In `@apps/web/src/components/modules/payment-stream/WithdrawStreamModal.tsx`:
- Around line 72-87: Replace the placeholder timeout in the withdrawable-amount
effect with the real `getWithdrawableAmount` SDK/service flow used by the
payment-stream operations, preserving loading, abort, and error handling. Ensure
`withdrawableAmount` reflects the stream’s actual state so the Available display
and Max handler do not submit a fabricated zero; if the read API is unavailable,
disable Max and render an unavailable state instead. Remove the unused `onError`
effect dependency and inert cancellation logic as appropriate.
- Around line 118-132: Update the withdraw flow in WithdrawStreamModal,
including withdrawTo handling, so a selected “Other Address” is either passed
through to the SDK/contract as the withdrawal destination or the option is
removed/disabled with matching validation. Also require address alongside
isConnected and signTransaction before calling withdraw, and pass the validated
connected address without falling back to stream.recipient.
In `@apps/web/src/lib/stellar.test.ts`:
- Around line 121-137: Update StellarService.calculateStreamProgress to handle
streams whose startTime equals endTime without dividing by zero; return finite
progressPercentage and ratePerHour values, preserving the existing completed and
not-started behavior. Add a zero-duration test in the calculateStreamProgress
test suite that verifies both returned values are finite.
- Around line 58-78: Add a `decimals = 0` test for
`StellarService.formatTokenAmount` that preserves integer values such as “100”.
Update the trimming logic in `formatTokenAmount` so its trailing-zero regex only
removes zeros from an existing fractional portion, never integer trailing zeros
when `toFixed(0)` returns no decimal point.
---
Outside diff comments:
In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx`:
- Around line 190-203: Remove the unused formData construction in the payment
stream creation flow and delete the PaymentStreamFormData import if it is not
referenced elsewhere in CreatePaymentStream. Preserve the tokenAddress
validation and subsequent logic unchanged.
---
Nitpick comments:
In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx`:
- Around line 205-211: In CreatePaymentStream, extract the shared
hour/day/week/month/year-to-seconds mapping into DURATION_SECONDS and the 1e7
amount conversion into a toStroops(amount) helper, then reuse both from
estimateFee and the submission logic around durationMultiplier and amount.
Remove the duplicated ladder and inline scaling while preserving the existing
fallback and rounding behavior.
In `@apps/web/src/components/modules/payment-stream/DepositStreamModal.tsx`:
- Around line 61-68: Move the wallet connectivity guard in the deposit
submission flow above the amount conversion that computes amount with BigInt.
Ensure !isConnected, !signTransaction, or !address returns with notify.error
before parsing data.amount, while preserving the existing conversion and
connected-wallet behavior.
In `@apps/web/src/components/modules/payment-stream/WithdrawStreamModal.tsx`:
- Line 134: Add a success toast in the withdraw completion flow before or
alongside the existing onSuccess callback, matching the notification pattern and
message used by DepositStreamModal. Update the handler containing
onSuccess?.(`withdraw_${Date.now()}`) so withdrawals display “Withdraw
successful!” before closing.
In `@apps/web/src/lib/api.ts`:
- Around line 230-239: Update depositToStream to validate params.streamId before
converting it with BigInt, rejecting NaN and non-integer values with a clear
caller-facing error. Keep the existing client creation, deposit, and
signAndSendTx flow unchanged for valid stream IDs, matching the validation
behavior used by withdraw.
In `@apps/web/src/lib/stellar.ts`:
- Around line 1-5: Remove the `@deprecated` tag from the file-level JSDoc in
stellar.ts, retaining only an accurate explanatory note about smart contract
operations and the still-canonical utility functions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 51b06c83-1797-4125-af7b-76f609779228
📒 Files selected for processing (7)
apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsxapps/web/src/components/modules/payment-stream/DepositStreamModal.tsxapps/web/src/components/modules/payment-stream/WithdrawStreamModal.tsxapps/web/src/hooks/use-stream-delegation.tsapps/web/src/lib/api.tsapps/web/src/lib/stellar.test.tsapps/web/src/lib/stellar.ts
💤 Files with no reviewable changes (1)
- apps/web/src/hooks/use-stream-delegation.ts
…odal.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ream.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ream.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Change depositToStream/withdraw streamId type from number to string to avoid NaN from Number() on non-numeric stream IDs- Return real tx hash from signAndSendTx, depositToStream, withdraw; pass real hash to onSuccess in both modals- Add getWithdrawableAmount API wrapper in api.ts; replace setTimeout mock in WithdrawStreamModal- Fix formatTokenAmount to not strip integer trailing zeros when decimals=0; add test coverage
- Add contractStreamId: number field to StreamRecord interface (validations.ts)- Update mock data in StreamsHistory.tsx with contractStreamId- Revert api.ts depositToStream/withdraw/getWithdrawableAmount streamId to number- Update DepositStreamModal and WithdrawStreamModal to use stream.contractStreamId instead of stream.id- Update test mock stream to include contractStreamId
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
2 similar comments
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/src/components/modules/payment-stream/StreamsHistory.tsx (1)
40-55: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not use fixture stream IDs for live contract actions.
fetchStreamsreturns static records, but deposit and withdrawal now usecontractStreamIdfor real transactions. These fixture IDs can target unrelated on-chain streams when this view is used with a matching wallet.Replace this mock query with contract-backed stream data before enabling actions. Otherwise, disable stream actions for fixture records.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/modules/payment-stream/StreamsHistory.tsx` around lines 40 - 55, Replace the static stream records used by fetchStreams with contract-backed data before allowing deposit or withdrawal actions to use contractStreamId. Alternatively, identify fixture records in StreamsHistory and disable their stream actions so their IDs cannot trigger live transactions.apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx (1)
212-220: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRemove the duplicate
amountanddurationMultiplierdeclarations.Lines 212-216 declare these same block-scoped
constvariables twice in the same scope, which makesCreatePaymentStream.tsxfail TypeScript compilation.Proposed fix
- const amount = BigInt(Math.floor(parseFloat(streamData.amount) * 10000000)); - const durationMultiplier = streamData.duration === 'hour' ? 3600 : - streamData.duration === 'day' ? 86400 : - streamData.duration === 'week' ? 604800 : const amount = BigInt(Math.floor(parseFloat(streamData.amount) * 10000000)); const durationMultiplier = streamData.duration === 'hour' ? 3600 :🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx` around lines 212 - 220, Remove the duplicate amount and durationMultiplier const declarations in CreatePaymentStream, keeping one complete declaration pair with the existing hour/day/week/month duration mapping so the block compiles without changing behavior.apps/web/src/lib/api.ts (1)
132-137: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve
contractStreamIdprecision across the contract boundary.
createStreamreturns a decimal string from a bigint, but these APIs andStreamRecordconvert stream IDs tonumber. Values aboveNumber.MAX_SAFE_INTEGERlose precision beforeBigInt(params.streamId)runs. A later deposit, withdrawal, or query can then target the wrong contract stream.
apps/web/src/lib/api.ts#L132-L137: acceptstringorbigintforwithdraw.apps/web/src/lib/api.ts#L228-L233: acceptstringorbigintfordepositToStream.apps/web/src/lib/api.ts#L240-L242: acceptstringorbigintforgetWithdrawableAmount.apps/web/src/lib/validations.ts#L6-L9: storecontractStreamIdas a decimal string at the UI boundary.Convert the value directly with
BigInt(streamId)only when building the contract call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/api.ts` around lines 132 - 137, Preserve stream ID precision by updating withdraw, depositToStream, and getWithdrawableAmount in apps/web/src/lib/api.ts at lines 132-137, 228-233, and 240-242 to accept string or bigint, converting directly with BigInt(streamId) only when constructing contract calls. Update StreamRecord in apps/web/src/lib/validations.ts at lines 6-9 to store contractStreamId as a decimal string at the UI boundary.
♻️ Duplicate comments (2)
apps/web/src/lib/stellar.test.ts (1)
117-153: 🩺 Stability & Availability | 🟡 MinorHandle zero-duration streams in the utility and this test suite.
StellarService.calculateStreamProgressstill divides bytotalDurationandtotalHourswithout handlingstartTime === endTime. A zero-duration stream can returnNaNforprogressPercentageandInfinityforratePerHour. Add a regression test here and guard both divisors inapps/web/src/lib/stellar.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/stellar.test.ts` around lines 117 - 153, Update StellarService.calculateStreamProgress in stellar.ts to handle streams where startTime equals endTime, preventing division by zero in both progressPercentage and ratePerHour while preserving normal progress behavior. Add a regression test in the calculateStreamProgress test suite that creates a zero-duration stream and verifies finite, valid results for both fields.apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx (1)
228-236: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not silently drop stream options.
The form and confirmation modal display
cancelableandtransferable.createStreamaccepts neither value, and this call does not send them. The created stream therefore does not reflect the selected options.Pass both options through the API and contract call. If the contract does not support them, disable these controls and state that they are unavailable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx` around lines 228 - 236, Update the createStream flow in CreatePaymentStream to pass the selected cancelable and transferable values through the API and contract call, preserving the form and confirmation selections in the created stream. If createStream or the contract cannot accept these options, disable the corresponding controls and clearly mark them unavailable instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/modules/payment-stream/WithdrawStreamModal.tsx`:
- Around line 137-142: Update the withdrawal flow in WithdrawStreamModal around
withdraw so the validated data.withdrawTo destination is honored by passing it
through the API and contract call; alternatively remove or disable the “Other
Address” option until destination support exists. Ensure the selected address
cannot be accepted by validation while being ignored during withdrawal.
---
Outside diff comments:
In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx`:
- Around line 212-220: Remove the duplicate amount and durationMultiplier const
declarations in CreatePaymentStream, keeping one complete declaration pair with
the existing hour/day/week/month duration mapping so the block compiles without
changing behavior.
In `@apps/web/src/components/modules/payment-stream/StreamsHistory.tsx`:
- Around line 40-55: Replace the static stream records used by fetchStreams with
contract-backed data before allowing deposit or withdrawal actions to use
contractStreamId. Alternatively, identify fixture records in StreamsHistory and
disable their stream actions so their IDs cannot trigger live transactions.
In `@apps/web/src/lib/api.ts`:
- Around line 132-137: Preserve stream ID precision by updating withdraw,
depositToStream, and getWithdrawableAmount in apps/web/src/lib/api.ts at lines
132-137, 228-233, and 240-242 to accept string or bigint, converting directly
with BigInt(streamId) only when constructing contract calls. Update StreamRecord
in apps/web/src/lib/validations.ts at lines 6-9 to store contractStreamId as a
decimal string at the UI boundary.
---
Duplicate comments:
In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx`:
- Around line 228-236: Update the createStream flow in CreatePaymentStream to
pass the selected cancelable and transferable values through the API and
contract call, preserving the form and confirmation selections in the created
stream. If createStream or the contract cannot accept these options, disable the
corresponding controls and clearly mark them unavailable instead.
In `@apps/web/src/lib/stellar.test.ts`:
- Around line 117-153: Update StellarService.calculateStreamProgress in
stellar.ts to handle streams where startTime equals endTime, preventing division
by zero in both progressPercentage and ratePerHour while preserving normal
progress behavior. Add a regression test in the calculateStreamProgress test
suite that creates a zero-duration stream and verifies finite, valid results for
both fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2d443ce0-3256-43d6-aaf7-b7dfa5053667
📒 Files selected for processing (8)
apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsxapps/web/src/components/modules/payment-stream/DepositStreamModal.tsxapps/web/src/components/modules/payment-stream/StreamsHistory.tsxapps/web/src/components/modules/payment-stream/WithdrawStreamModal.tsxapps/web/src/lib/api.tsapps/web/src/lib/stellar.test.tsapps/web/src/lib/stellar.tsapps/web/src/lib/validations.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/lib/stellar.ts
- apps/web/src/components/modules/payment-stream/DepositStreamModal.tsx
| const hash = await withdraw({ | ||
| streamId: stream.contractStreamId, | ||
| amount, | ||
| sender: address, | ||
| signTransaction, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Send withdrawTo or remove the alternate-address option.
When the user selects “Other Address”, the form validates data.withdrawTo. This withdraw call does not send it, and the API accepts no destination parameter. The selected address has no effect on the withdrawal.
Disable this option until the contract supports it, or pass the destination through the API and contract call.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/modules/payment-stream/WithdrawStreamModal.tsx`
around lines 137 - 142, Update the withdrawal flow in WithdrawStreamModal around
withdraw so the validated data.withdrawTo destination is honored by passing it
through the API and contract call; alternatively remove or disable the “Other
Address” option until destination support exists. Ensure the selected address
cannot be accepted by validation while being ignored during withdrawal.
|
Hello @pragmaticAweds please review and merge PR |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx (1)
200-208: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRemove the broken duplicate declarations.
durationMultiplieris cut off after theweekbranch, thenamountanddurationMultiplierare declared again below. This leavesCreatePaymentStream.tsx:204with an unclosed ternary and duplicate declarations, so the file will not compile.Proposed fix
- const amount = BigInt(Math.floor(parseFloat(streamData.amount) * 10000000)); - const durationMultiplier = streamData.duration === 'hour' ? 3600 : - streamData.duration === 'day' ? 86400 : - streamData.duration === 'week' ? 604800 : const amount = BigInt(Math.floor(parseFloat(streamData.amount) * 10000000)); const durationMultiplier = streamData.duration === 'hour' ? 3600 : streamData.duration === 'day' ? 86400 : streamData.duration === 'week' ? 604800 : streamData.duration === 'month' ? 2592000 : 31536000;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx` around lines 200 - 208, Remove the first incomplete amount and durationMultiplier declarations in CreatePaymentStream, retaining the complete declarations that include the month and default-year duration branches so the function compiles without duplicate variables.
♻️ Duplicate comments (2)
apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx (2)
200-204: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse exact decimal scaling for the on-chain amount.
parseFloat(streamData.amount) * 10000000can round beforeMath.floor, socreateStreamcan receive a different stroop amount than the user entered. This previous review finding remains present at Lines [200] and [204]. Use an exact decimal-string scaler or decimal library in this path and the fee-estimation path at Line [94].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx` around lines 200 - 204, Replace floating-point amount conversion in CreatePaymentStream, including the createStream amount and the fee-estimation path around the fee calculation, with exact decimal-string scaling to seven decimal places before converting to BigInt. Preserve the existing floor/truncation behavior and ensure both paths use the same exact conversion logic.
216-224: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve
cancelableandtransferablein the contract request.The form still collects these options, but the
createStreamcall does not pass them. The API definition inapps/web/src/lib/api.tsLines [105]-[132] also does not accept or map these fields. The toggles therefore have no effect. Extend the API and contract call, or remove and disable the options until they are supported.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx` around lines 216 - 224, Update the CreatePaymentStream submission and createStream API contract to preserve the form’s cancelable and transferable values, forwarding both fields into the underlying contract request and mapping them as required. Ensure the toggles affect stream creation consistently; alternatively remove and disable these options throughout the form if support cannot be added.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx`:
- Around line 200-208: Remove the first incomplete amount and durationMultiplier
declarations in CreatePaymentStream, retaining the complete declarations that
include the month and default-year duration branches so the function compiles
without duplicate variables.
---
Duplicate comments:
In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsx`:
- Around line 200-204: Replace floating-point amount conversion in
CreatePaymentStream, including the createStream amount and the fee-estimation
path around the fee calculation, with exact decimal-string scaling to seven
decimal places before converting to BigInt. Preserve the existing
floor/truncation behavior and ensure both paths use the same exact conversion
logic.
- Around line 216-224: Update the CreatePaymentStream submission and
createStream API contract to preserve the form’s cancelable and transferable
values, forwarding both fields into the underlying contract request and mapping
them as required. Ensure the toggles affect stream creation consistently;
alternatively remove and disable these options throughout the form if support
cannot be added.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 64c78b7d-077e-4d6c-908a-e61441a2e44c
📒 Files selected for processing (5)
apps/web/src/components/modules/payment-stream/CreatePaymentStream.tsxapps/web/src/components/modules/payment-stream/StreamsHistory.tsxapps/web/src/lib/api.tsapps/web/src/lib/stellar.tsapps/web/src/lib/validations.ts
💤 Files with no reviewable changes (1)
- apps/web/src/components/modules/payment-stream/StreamsHistory.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/lib/validations.ts
- apps/web/src/lib/api.ts
- apps/web/src/lib/stellar.ts
2 participants
Closes #371
Summary
Removes all mock simulation methods from apps/web/src/lib/stellar.ts that used Math.random() and setTimeout instead of delegating to the real SDK service. All payment stream operations now exclusively route through @/services/stellar.service.ts and the SDK client wrappers in @/lib/api.ts.
What Changed
Key Design Decisions
Acceptance Criteria Checklist
Test Output
All 18 tests in src/lib/stellar.test.ts pass. 15 pre-existing failures in unrelated test files (sanitize-error, stellar.service edge/timeout) are unchanged.
Security Note
Follow-ups
Summary by CodeRabbit
New Features
Bug Fixes