feat: credits UI

[gmolki]

No description provided.

[foxpatch-aleph]

Here is the review result:

{
  "verdict": "REQUEST_CHANGES",
  "summary": "This PR introduces a comprehensive credit system UI including top-up, transfer, payment status, and reporting modals, along with credit-aware payment display and dashboard charts. The overall architecture is solid with good separation of concerns. However, there are several substantive issues that need to be addressed: (1) dead code in the top-up checkout flow where `nSteps` is computed but never used, making the checkout notification display incorrect; (2) the `TopUpCreditsModal` does not expose `handleOpen`, making the modal impossible to open programmatically; (3) floating-point precision issues in credit transfer max-fill logic that could cause rounding errors; (4) a `JSON.stringify` debug statement left in the codebase; (5) an unsafe `as any` type assertion in the payment status modal; and (6) the `TopUpCreditsModal` input does not guard against `NaN` when the user clears the field."
}

Key findings:

Bugs / Logic errors:

1. TopUpCreditsModal/hook.ts:118 — Dead code, broken checkout notifications. nSteps is computed by mapping iSteps through stepsCatalog, but the while loop at line 133 calls next(nSteps) with the variable nSteps which holds the correct notification steps. However, the generator steps from creditManager.addSteps(state) is what's actually iterated. The issue is that nSteps is computed from iSteps (the API step types) but steps is the generator from creditManager.addSteps(). These may not correspond 1:1, and the notification steps shown to the user may not match the actual transaction steps.

2. TopUpCreditsModal/cmp.tsx:36 — handleOpen not exposed. The hook returns handleOpen but it's never destructured or exported, making the modal impossible to open from outside.

3. TopUpCreditsModal/cmp.tsx:170 — NaN input guard missing. Number("") produces NaN, and NaN <= 0 is false, so the submit button could remain enabled with a NaN amount.

4. CreditTransferModal/hook.ts:116 — Floating-point precision. Math.floor(x * CREDITS_PER_USD) / CREDITS_PER_USD can produce values like 0.33333333333333337. Should round to 6 decimal places.

Type safety:

5. PaymentStatusModal/cmp.tsx:179 — as any type assertion. Bypasses TypeScript checking on explorer URL parameters.

6. TopUpCreditsModal/cmp.tsx:269,285,296 — as unknown as string casts. Indicates StyledRadio types are wrong; should be fixed at the source.

Code quality:

7. TopUpCreditsModal/cmp.tsx:142 — Debug code. {/* {JSON.stringify(values, null, 2)} */} should be removed.

8. CreditCharts/hook.ts:71 — Unverified assumption. r.cost_credit * 3600 * 24 assumes per-second rate; needs verification against API.

9. CreditCharts/hook.ts:130 — Date parsing assumption. new Date(oldestEntry.message_timestamp) may not handle Unix timestamps correctly.

[foxpatch-aleph]

⚠️ autodev could not generate a review for this PR (empty output from the model).

Click Re-request review to retry.

[foxpatch-aleph]

This PR introduces a comprehensive credit payment system with top-up, transfer, status tracking, and issue reporting. The state management follows existing patterns well and the reducer-based UI state is clean. However, several issues should be addressed: the disabled ExternalLink in ManageEntityHeader is still clickable due to commented-out pointer-events, the CreditTransferModal lacks balance validation against the total transfer amount, ReportIssueModal sends unsanitized user input to an external webhook without length limits, EntityProxyUrl hardcodes an API URL instead of using getApiServer(), and TransferStatusModal hardcodes an explorer URL instead of reusing getExplorerURL. None of these are blocking, but the balance validation and hardcoded URLs should be addressed before merge.

src/components/common/ExternalLink/styles.ts (line 20): The pointer-events: none is commented out, so a disabled ExternalLink is still clickable. In ManageEntityHeader the "Top-up" link uses <ExternalLink disabled /> with no href, so clicking it does nothing but still shows a pointer cursor. Uncomment pointer-events: none to properly disable interaction.

src/components/modals/CreditTransferModal/hook.ts (line 62): The onSubmit converts recipient amounts to credits via Math.round(r.amount * CREDITS_PER_USD) but never validates that the total across all recipients doesn't exceed the user's balance. Consider adding a .refine() to creditTransferSchema that checks data.recipients.reduce((sum, r) => sum + r.amount, 0) <= balanceDollars, or validate before calling transfer().

src/components/modals/ReportIssueModal/hook.ts (line 65): The report issue payload sends user-typed description directly to an external webhook with no length limit or sanitization. Consider adding a max length check (e.g., 5000 chars) on the message before submission to prevent abuse.

src/components/common/entityData/EntityProxyUrl/hook.ts (line 4): PROXY_API_BASE is hardcoded to 'https://api.2n6.me'. The rest of the codebase uses getApiServer() for environment-based API configuration. Consider using the same pattern for consistency and to support different environments.

src/components/modals/TransferStatusModal/cmp.tsx (line 163): The Aleph explorer URL is hardcoded as https://explorer.aleph.im/address/ETH/... instead of using the existing getExplorerURL helper that PaymentStatusModal uses. This creates inconsistency and won't adapt to environment changes.

src/components/modals/TopUpCreditsModal/cmp.tsx (line 328): isEthereumNetwork is included in TopUpCreditsModalFooterProps but is not destructured or used in the component. The tooltip logic is handled via getEthereumNetworkTooltip() which internally checks this. Remove the unused prop from the type to avoid confusion.

src/hooks/common/useTransferStatus.ts (line 79): The initial poll() call in startPolling is not guarded against component unmount. If the component unmounts between poll() being called and the async checkStatus resolving, state updates will fire on an unmounted component. Consider using an isMountedRef or an AbortController.

src/components/common/entityData/ManageEntityHeader/cmp.tsx (line 237): The "Top-up" link text says to top-up your wallet but the link is disabled with no href. Users see a link they can't click to resolve the issue. Consider making this an actual button that opens the TopUpCreditsModal instead of a disabled link.

[foxpatch-aleph]

The PR introduces a comprehensive credit payment system with well-structured modal components, proper Zod validation schemas, and clean domain/manager separation. The payment flow logic (TopUpCreditsModal, CreditTransferModal) is generally correct with proper async generator handling and notification integration. However, there are a few issues worth addressing: the disabled ExternalLink in ManageEntityHeader is still clickable (pointer-events: none is commented out), the EntityProxyUrl constructs href from unvalidated API data (potential XSS/open-redirect if the API is compromised), and no tests were added for the new payment functionality. These are non-blocking but should be addressed before production deployment.

src/components/common/ExternalLink/styles.ts (line 20): The pointer-events: none is commented out, so a disabled ExternalLink is still clickable. In ManageEntityHeader (line 237), the disabled "Top-up" link has no href and will navigate to the current page when clicked. Uncomment pointer-events: none or use a non-anchor element when disabled.

src/components/common/entityData/EntityProxyUrl/cmp.tsx (line 28): The href is constructed from unvalidated API response data (data.url). If the API returns a malicious value (e.g., starting with javascript:), it could lead to XSS. Consider validating the URL format before using it, e.g., ensure it only contains expected domain characters.

src/components/modals/PaymentStatusModal/cmp.tsx (line 179): Using as any to bypass the type system on the getExplorerURL argument. This could mask type mismatches between the object shape and what getExplorerURL expects (the Message type). Consider constructing a properly typed object instead.

src/components/modals/TopUpCreditsModal/cmp.tsx (line 336): The isEthereumNetwork prop is included in the TopUpCreditsModalFooterProps Pick type but is not destructured or used in the component. Either remove it from the type or use it (e.g., to conditionally style the button).

src/domain/credit.ts (line 403): The payment history API URL is constructed with ?address=${address} without URL-encoding the address. While Ethereum addresses are safe (hex + 0x prefix), this is a defensive practice. Consider using encodeURIComponent(address).

src/components/modals/ReportIssueModal/hook.ts (line 12): The report issue webhook endpoint URL is hardcoded. Consider moving this to a configuration file or environment variable for consistency with other API endpoints (e.g., getApiServer() pattern used elsewhere).

src/components/common/entityData/EntityProxyUrl/hook.ts (line 4): The proxy API base URL (https://api.2n6.me) is hardcoded and doesn't match the aleph.im domain pattern used elsewhere in the codebase. This should be configurable or at least use a consistent domain.

src/components/modals/CreditTransferModal/hook.ts (line 69): The expiration field from the form is an empty string by default. When converting with new Date(r.expiration).getTime(), an empty string will produce NaN. The r.expiration && guard catches this, but consider explicitly checking for a non-empty string to be safe.

[foxpatch-aleph]

Well-structured credits UI feature with proper Zod validation and clean modal/state architecture. The main concern is the hardcoded third-party API endpoint (api.2n6.me) in EntityProxyUrl which needs verification. There are also minor UX bugs (empty input → 0, dead Top-up link) and a typo that should be fixed. No security-critical issues found.

src/components/common/entityData/EntityProxyUrl/hook.ts (line 4): Hardcoded third-party API endpoint https://api.2n6.me is not an Aleph domain. Verify this is an official service and consider moving to a config/environment variable for maintainability.

src/components/common/entityData/ManageEntityHeader/cmp.tsx (line 237): The "Top-up" ExternalLink has disabled and no href — users see a non-clickable link with no way to top up from this insufficient-credits warning. Wire it to open the TopUpCreditsModal or remove it.

src/components/common/entityData/EntityStatus/cmp.tsx (line 38): Typo: NOT ALLLOCATED should be NOT ALLOCATED.

src/components/modals/TopUpCreditsModal/cmp.tsx (line 171): Number(e.target.value) converts an empty input to 0 instead of allowing an empty/undefined state. Consider checking for empty string before converting, e.g. e.target.value === '' ? '' : Number(e.target.value).

src/components/modals/TopUpCreditsModal/hook.ts (line 117): Both getAddSteps and addSteps are called on consecutive lines producing iSteps/nSteps and steps. The naming is confusing — consider adding a comment explaining the difference (e.g., one returns step names for UI, the other returns a generator for execution).

src/components/modals/CreditTransferModal/hook.ts (line 68): Math.round(r.amount * CREDITS_PER_USD) could allow transferring slightly more credits than the user's balance due to floating-point rounding. Consider using Math.floor to be safe, consistent with handleFillMax which already uses Math.floor.

src/components/modals/TopUpCreditsModal/cmp.tsx (line 328): isEthereumNetwork is picked in TopUpCreditsModalFooterProps but never destructured or used in the footer component. Remove it from the Pick type or use it.

src/components/modals/ReportIssueModal/hook.ts (line 77): The POST to https://n8n.aleph.im/webhook/report-issue has no authentication or rate limiting. Consider adding a basic debounce/throttle or captcha to prevent abuse.

[foxpatch-aleph]

The credits UI PR introduces substantial new functionality (top-up, transfer, payment status, issue reporting modals) with good adherence to project conventions. The code is generally well-structured with proper hook/component separation and consistent patterns. However, there are a few issues: a disabled link that still navigates, an import placed after an export, an unused destructured prop, a time-based warning that won't update without re-render, and a hardcoded API URL. None of these are blocking, but the disabled-link bug and the showWarning timing issue should be addressed before release.

src/components/common/ExternalLink/styles.ts (line 20): The pointer-events: none is commented out, so disabled ExternalLink components still navigate when clicked. In ManageEntityHeader/cmp.tsx:237, the "Top-up" link uses disabled but users can still click through. Uncomment this line or prevent href from being set when $disabled is true.

src/components/modals/TopUpCreditsModal/hook.ts (line 32): Import statement import { useAppState } is placed after export const defaultValues on line 26. Move all imports to the top of the file before any exports, per standard conventions.

src/components/modals/TopUpCreditsModal/cmp.tsx (line 337): isEthereumNetwork is destructured in TopUpCreditsModalFooter but never used. Remove it from the Pick type (line 328) and the destructuring.

src/components/modals/PaymentStatusModal/cmp.tsx (line 125): showWarning is computed at render time using Date.now(). If the modal stays open without a re-render, this warning will never appear after the 5-minute threshold. Consider adding a timer or moving this logic to the hook with an interval.

src/components/common/entityData/EntityProxyUrl/hook.ts (line 4): PROXY_API_BASE is hardcoded to https://api.2n6.me. Consider using a centralized config or environment variable, consistent with how getApiServer() is used elsewhere in the codebase.

src/helpers/schemas/credit.ts (line 64): expiration: z.string().optional() doesn't validate that the value is a valid date. In CreditTransferModal/hook.ts:70, new Date(r.expiration).getTime() could produce NaN. Add a .refine() to validate date strings.

src/components/modals/CreditTransferModal/hook.ts (line 116): Math.floor(max * CREDITS_PER_USD) / CREDITS_PER_USD can produce floating point artifacts like 99.99999999. Consider rounding to a fixed decimal place (e.g., 6) after the floor to avoid display issues.