feat(deployment): redesign deposit modal with ACT auto-mint support#3048
Merged
ygrishajev merged 1 commit intomainfrom Apr 13, 2026
Merged
feat(deployment): redesign deposit modal with ACT auto-mint support#3048ygrishajev merged 1 commit intomainfrom
ygrishajev merged 1 commit intomainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces two new custom hooks ( Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Modal as DeploymentDepositModal
participant Component as ManifestEdit/<br/>DetailTopBar
participant DepositHook as useDepositDeployment
participant MintHook as useMintACT
participant Wallet as Wallet/Signer
participant Service as TransactionService
participant Ledger as Blockchain Ledger
User->>Modal: Select ACT denom & enter amount
User->>Modal: Click Continue (onSubmit)
Modal->>Component: Call onSubmit(depositUdenom)
Component->>Component: Convert USD→AKT if needed
alt ACT balance insufficient
Component->>MintHook: Call mint(deficitUdenom)
MintHook->>MintHook: Validate wallet & price
MintHook->>MintHook: Calculate AKT to burn
MintHook->>Service: Create mint transaction
Service->>Wallet: Sign transaction
Wallet->>Ledger: Broadcast mint
Ledger->>Service: Confirm mint
Service->>Ledger: Poll for settlement
Ledger->>MintHook: Settlement complete
MintHook->>Component: Mint success
end
Component->>DepositHook: Call deposit(depositUdenom)
DepositHook->>Service: Create deposit transaction
Service->>Wallet: Sign transaction
Wallet->>Ledger: Broadcast deposit
Ledger->>DepositHook: Confirm success
DepositHook->>Component: onSuccess callback
Component->>User: Update UI / show success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/http-sdk/src/bme/bme-http.service.ts (1)
35-46: Consider using an abortable delay for faster signal response.The abort signal is only checked at the start of each iteration. If an abort occurs during the
setTimeoutdelay, it won't be detected until the next iteration begins, potentially adding up to 5 seconds of unnecessary waiting.♻️ Optional improvement for faster abort response
+ private abortableDelay(ms: number, signal?: AbortSignal): Promise<boolean> { + return new Promise(resolve => { + if (signal?.aborted) { + resolve(false); + return; + } + const timeout = setTimeout(() => resolve(true), ms); + signal?.addEventListener("abort", () => { + clearTimeout(timeout); + resolve(false); + }, { once: true }); + }); + } async waitForLedgerRecordsSettlement(address: string, options?: { pollIntervalMs?: number; maxAttempts?: number; signal?: AbortSignal }): Promise<boolean> { const pollInterval = options?.pollIntervalMs ?? DEFAULT_POLL_INTERVAL_MS; const maxAttempts = options?.maxAttempts ?? DEFAULT_MAX_POLL_ATTEMPTS; for (let attempt = 0; attempt < maxAttempts; attempt++) { if (options?.signal?.aborted) return false; const { records } = await this.getLedgerRecords({ source: address, status: "ledger_record_status_pending" }); if (records.length === 0) return true; - await new Promise(resolve => setTimeout(resolve, pollInterval)); + const continued = await this.abortableDelay(pollInterval, options?.signal); + if (!continued) return false; } return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/http-sdk/src/bme/bme-http.service.ts` around lines 35 - 46, The polling loop in bme-http.service.ts checks options?.signal?.aborted only at iteration start, so an abort during the setTimeout delay can wait the full pollInterval; change the delay to be abortable by replacing the plain new Promise(resolve => setTimeout(...)) with an abort-aware wait (e.g., Promise.race between a timeout promise and a promise that rejects or resolves when options.signal fires), wire up and clean up the signal event listener, and ensure the loop returns false immediately when the signal is triggered; keep this behavior around the for-loop that calls this.getLedgerRecords({ source: address, status: "ledger_record_status_pending" }) so aborts are handled promptly during polling.apps/deploy-web/src/hooks/useMintACT/useMintACT.spec.ts (1)
1-14: Move fake-timer setup intosetup()instead of suite hooks.This repo’s unit-test convention is to opt into test state from
setup(), notbeforeEach. Keeping fake timers global to the suite also makes timer leakage easier once more cases get added.As per coding guidelines,
Use setup function instead of beforeEach in unit and service level tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/useMintACT/useMintACT.spec.ts` around lines 1 - 14, The suite-level vi.useFakeTimers()/vi.useRealTimers() calls should be moved into a local setup() function so tests opt into fake timers per test instead of via beforeEach/afterEach; remove the beforeEach and afterEach hooks in the spec and add a setup() helper (e.g., function setup(){ vi.useFakeTimers(); return () => vi.useRealTimers(); }) and call its teardown inside each test (or have tests call setup() at start and call the returned cleanup in after-test logic) so the fake-timer state is controlled via setup() rather than globally for the describe block; update tests to invoke setup() in their bodies around useMintACT tests accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx`:
- Around line 180-224: When minting is in flight (isMinting true) disable all
close paths: set the "Cancel" action in actions to disabled: isMinting and make
the Popup non-closeable by passing onClose only when not isMinting (e.g.
onClose={isMinting ? undefined : trackAndClose}) and toggling
enableCloseOnBackdropClick to !isMinting; update references to actions,
trackAndClose, isMinting, submitForm and Popup props so the modal cannot be
dismissed while mint() is pending.
In `@apps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.ts`:
- Around line 43-48: Replace the unsafe "as any" cast on the mocked useWallet
return with a properly typed partial/mock: update the test to return a Partial
or a vitest-mock-extended mock of the wallet type instead of casting to any,
ensuring the object implements address and signAndBroadcastTx; e.g., target the
real wallet interface used by useWallet and provide a typed mock for address and
signAndBroadcastTx so the test keeps type-safety.
In `@apps/deploy-web/src/hooks/useMintACT/useMintACT.spec.ts`:
- Around line 125-154: Replace the ad-hoc "as any" casts in the dependencies
object with typed mocks from vitest-mock-extended: import mock and create mocks
for the return types of useWallet, useServices, useWalletBalance, usePricing,
and useBmeParams (e.g., mock<ReturnType<typeof useWallet>>(),
mock<ReturnType<typeof useServices>>(), etc.), then assign the mocked properties
(address, signAndBroadcastTx, bmeHttpService.waitForLedgerRecordsSettlement,
errorHandler.reportError, balance, refetch, price, data.minMintAct) on those
mock objects instead of casting, ensuring DEPENDENCIES' shape matches the real
hook return types and removing all "as any" usages.
---
Nitpick comments:
In `@apps/deploy-web/src/hooks/useMintACT/useMintACT.spec.ts`:
- Around line 1-14: The suite-level vi.useFakeTimers()/vi.useRealTimers() calls
should be moved into a local setup() function so tests opt into fake timers per
test instead of via beforeEach/afterEach; remove the beforeEach and afterEach
hooks in the spec and add a setup() helper (e.g., function setup(){
vi.useFakeTimers(); return () => vi.useRealTimers(); }) and call its teardown
inside each test (or have tests call setup() at start and call the returned
cleanup in after-test logic) so the fake-timer state is controlled via setup()
rather than globally for the describe block; update tests to invoke setup() in
their bodies around useMintACT tests accordingly.
In `@packages/http-sdk/src/bme/bme-http.service.ts`:
- Around line 35-46: The polling loop in bme-http.service.ts checks
options?.signal?.aborted only at iteration start, so an abort during the
setTimeout delay can wait the full pollInterval; change the delay to be
abortable by replacing the plain new Promise(resolve => setTimeout(...)) with an
abort-aware wait (e.g., Promise.race between a timeout promise and a promise
that rejects or resolves when options.signal fires), wire up and clean up the
signal event listener, and ensure the loop returns false immediately when the
signal is triggered; keep this behavior around the for-loop that calls
this.getLedgerRecords({ source: address, status: "ledger_record_status_pending"
}) so aborts are handled promptly during polling.
🪄 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
Run ID: 1d3c6c48-0bce-4a18-ad33-886b69461131
📒 Files selected for processing (14)
apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.tsxapps/deploy-web/src/components/deployments/DeploymentListRow.tsxapps/deploy-web/src/components/layout/TransactionModal.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.spec.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.tsxapps/deploy-web/src/components/sdl/DeploymentMinimumEscrowAlertText.tsxapps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.tsapps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.tsapps/deploy-web/src/hooks/useMintACT/useMintACT.spec.tsapps/deploy-web/src/hooks/useMintACT/useMintACT.tspackages/http-sdk/src/bme/bme-http.service.ts
apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.ts
Show resolved
Hide resolved
ygrishajev
force-pushed
the
feature/deposit-modal
branch
from
836c1f4 to
9359b05
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3048 +/- ##
==========================================
+ Coverage 59.71% 59.94% +0.23%
==========================================
Files 1034 1036 +2
Lines 24273 24398 +125
Branches 6003 6043 +40
==========================================
+ Hits 14494 14626 +132
+ Misses 8528 8523 -5
+ Partials 1251 1249 -2
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx (1)
162-165:⚠️ Potential issue | 🟠 MajorBlock every close path while ACT minting is in flight.
Cancel,onClose, and backdrop-close are still live duringmint(). If the modal unmounts after the burn tx is sent, settlement polling is aborted and the deferred deposit never runs, leaving the user with minted ACT but no deposit applied.Guard all close paths
- const trackAndClose = useCallback(() => { + const trackAndClose = useCallback(() => { + if (isMinting) return; analyticsService.track("close_deposit_modal", "Amplitude"); onCancel(); - }, [analyticsService, onCancel]); + }, [analyticsService, onCancel, isMinting]); ... { label: "Cancel", color: "primary", variant: "outline", side: "right", + disabled: isMinting, onClick: trackAndClose }, ... - onClose={trackAndClose} - enableCloseOnBackdropClick + onClose={isMinting ? undefined : trackAndClose} + enableCloseOnBackdropClick={!isMinting}Also applies to: 195-224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx` around lines 162 - 165, Prevent modal from closing while minting is in progress by guarding all close paths (trackAndClose, onCancel, any onClose/backdrop handlers and the Cancel button) with the mint-in-flight flag used by mint(); e.g., add an isMinting (or useRef isMintingRef) check at the start of trackAndClose and any close callbacks and return early if true, disable backdrop click and interaction when isMinting is true, and ensure the code that starts mint() awaits settlement/deferred deposit completion (or moves the settlement polling outside component unmount) so unmounting does not abort the deferred deposit; update references in DeploymentDepositModal (trackAndClose, mint, onCancel and the other close handlers around lines ~195-224) accordingly.apps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.ts (1)
42-47:⚠️ Potential issue | 🟡 MinorReplace the
as anywallet mock.This cast drops type-safety in the test harness and violates the repo TS rule. Prefer a typed mock/partial for the
useWalletreturn instead.Typed mock sketch
+import { mock } from "vitest-mock-extended"; +import { useWallet } from "@src/context/WalletProvider"; ... dependencies: { - useWallet: () => - ({ - address: "akash1abc", - signAndBroadcastTx - }) as any + useWallet: () => + mock<ReturnType<typeof useWallet>>({ + address: "akash1abc", + signAndBroadcastTx + }) }#!/bin/bash rg -n '\bas any\b' apps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.tsAs per coding guidelines: "
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.ts` around lines 42 - 47, Replace the "as any" cast on the wallet mock with a properly typed mock: declare a mockWallet with type Partial<ReturnType<typeof useWallet>> containing address and signAndBroadcastTx, then return that from the useWallet dependency (casting only from Partial to ReturnType<typeof useWallet> if necessary) so the spec uses a typed mock instead of any; update the mock variable referenced in useDepositDeployment.spec to use mockWallet and ensure signAndBroadcastTx matches the expected signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx`:
- Around line 162-165: Prevent modal from closing while minting is in progress
by guarding all close paths (trackAndClose, onCancel, any onClose/backdrop
handlers and the Cancel button) with the mint-in-flight flag used by mint();
e.g., add an isMinting (or useRef isMintingRef) check at the start of
trackAndClose and any close callbacks and return early if true, disable backdrop
click and interaction when isMinting is true, and ensure the code that starts
mint() awaits settlement/deferred deposit completion (or moves the settlement
polling outside component unmount) so unmounting does not abort the deferred
deposit; update references in DeploymentDepositModal (trackAndClose, mint,
onCancel and the other close handlers around lines ~195-224) accordingly.
In `@apps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.ts`:
- Around line 42-47: Replace the "as any" cast on the wallet mock with a
properly typed mock: declare a mockWallet with type Partial<ReturnType<typeof
useWallet>> containing address and signAndBroadcastTx, then return that from the
useWallet dependency (casting only from Partial to ReturnType<typeof useWallet>
if necessary) so the spec uses a typed mock instead of any; update the mock
variable referenced in useDepositDeployment.spec to use mockWallet and ensure
signAndBroadcastTx matches the expected signature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1135a91-9810-41b2-92f3-2b40e512c955
📒 Files selected for processing (14)
apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.tsxapps/deploy-web/src/components/deployments/DeploymentListRow.tsxapps/deploy-web/src/components/layout/TransactionModal.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.spec.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.tsxapps/deploy-web/src/components/sdl/DeploymentMinimumEscrowAlertText.tsxapps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.tsapps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.tsapps/deploy-web/src/hooks/useMintACT/useMintACT.spec.tsapps/deploy-web/src/hooks/useMintACT/useMintACT.tspackages/http-sdk/src/bme/bme-http.service.ts
✅ Files skipped from review due to trivial changes (4)
- apps/deploy-web/src/components/sdl/DeploymentMinimumEscrowAlertText.tsx
- apps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.spec.tsx
- apps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.ts
- apps/deploy-web/src/hooks/useMintACT/useMintACT.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.tsx
- apps/deploy-web/src/components/layout/TransactionModal.tsx
- apps/deploy-web/src/components/deployments/DeploymentListRow.tsx
- packages/http-sdk/src/bme/bme-http.service.ts
ygrishajev
force-pushed
the
feature/deposit-modal
branch
2 times, most recently
from
6fb3bdf to
3a0aafc
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/deploy-web/src/hooks/useMintACT/useMintACT.ts (1)
83-86: Clarify distinct error conditions with specific messages.The error message "Wallet not connected or price unavailable" is used for both wallet disconnection (line 84) and price unavailability (line 54). Using the same message for different conditions makes debugging harder for users and developers.
♻️ Proposed fix for clearer error messages
if (!address) { - setError("Wallet not connected or price unavailable"); + setError("Wallet not connected"); return; }And for the price check in
calculateAktToBurn:if (!price || price <= 0) { - setError("Wallet not connected or price unavailable"); + setError("Price unavailable. Please try again later."); return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/hooks/useMintACT/useMintACT.ts` around lines 83 - 86, The single error message conflates two distinct failure modes; update the address check in the useMintACT hook (where setError is called when !address) to set a specific message like "Wallet not connected" and change the price-related error path in calculateAktToBurn to set a distinct message such as "Price unavailable" (or propagate/throw a specific PriceUnavailable error) so consumers and logs can distinguish wallet vs price failures; adjust any callers or tests of setError accordingly.packages/http-sdk/src/bme/bme-http.service.ts (1)
35-46: Consider adding error handling for transient network failures.If
getLedgerRecordsthrows (e.g., due to a transient network error), the exception propagates and aborts the entire polling operation. For a settlement-wait utility, it may be more resilient to catch and retry on transient errors rather than failing immediately.♻️ Proposed fix to handle transient errors
for (let attempt = 0; attempt < maxAttempts; attempt++) { if (options?.signal?.aborted) return false; - const { records } = await this.getLedgerRecords({ - source: address, - status: "ledger_record_status_pending" - }); - - if (records.length === 0) return true; + try { + const { records } = await this.getLedgerRecords({ + source: address, + status: "ledger_record_status_pending" + }); + + if (records.length === 0) return true; + } catch { + // Retry on transient errors; will fail after maxAttempts + } await new Promise(resolve => setTimeout(resolve, pollInterval)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/http-sdk/src/bme/bme-http.service.ts` around lines 35 - 46, The polling loop currently lets exceptions from getLedgerRecords abort the whole operation; wrap the call to this.getLedgerRecords in a try/catch, detect transient/network errors (e.g., network/FetchError, errno ENETUNREACH/ECONNRESET/ETIMEDOUT or status 502/503/504) and on those errors await the same pollInterval and continue the loop instead of throwing, while for non-transient errors rethrow; if you want to avoid consuming a retry attempt for transient errors, decrement attempt (or switch to a while/retry-count) inside the catch so transient failures don’t prematurely exhaust maxAttempts, and still honor options?.signal?.aborted and return false when aborted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx`:
- Around line 138-152: Guard the convertAndSubmitDeposit submit handler against
non-button submits by early-returning when the form shouldn't submit: at the
start of convertAndSubmitDeposit check the same boolean used for the action
button (isSubmitDisabled) and if true simply return (or call form.reset/notify)
so Enter-key or programmatic submits can't bypass validation; apply the same
guard to any other submit handlers mentioned (lines 260-261) and ensure you
reference the existing symbols convertAndSubmitDeposit, isSubmitDisabled,
willAutoMint, mint and onSubmit so the early-return sits before any
denomToUdenom, setPendingDeposit, mint or onSubmit calls.
- Around line 143-147: The code currently calls mint(deficit) whenever
willAutoMint is true even if deficit <= 0; change the logic in the branch that
computes deficit (using deposit, denomToUdenom and depositData?.balance) to
check if deficit > 0 before calling setPendingDeposit and mint — if deficit <=
0, skip minting and proceed with the normal submission flow instead of invoking
mint; update the block around willAutoMint, setPendingDeposit and mint to only
set pending and call mint when deficit is strictly positive.
---
Nitpick comments:
In `@apps/deploy-web/src/hooks/useMintACT/useMintACT.ts`:
- Around line 83-86: The single error message conflates two distinct failure
modes; update the address check in the useMintACT hook (where setError is called
when !address) to set a specific message like "Wallet not connected" and change
the price-related error path in calculateAktToBurn to set a distinct message
such as "Price unavailable" (or propagate/throw a specific PriceUnavailable
error) so consumers and logs can distinguish wallet vs price failures; adjust
any callers or tests of setError accordingly.
In `@packages/http-sdk/src/bme/bme-http.service.ts`:
- Around line 35-46: The polling loop currently lets exceptions from
getLedgerRecords abort the whole operation; wrap the call to
this.getLedgerRecords in a try/catch, detect transient/network errors (e.g.,
network/FetchError, errno ENETUNREACH/ECONNRESET/ETIMEDOUT or status
502/503/504) and on those errors await the same pollInterval and continue the
loop instead of throwing, while for non-transient errors rethrow; if you want to
avoid consuming a retry attempt for transient errors, decrement attempt (or
switch to a while/retry-count) inside the catch so transient failures don’t
prematurely exhaust maxAttempts, and still honor options?.signal?.aborted and
return false when aborted.
🪄 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
Run ID: 70ff96ea-00a4-46bc-8e06-577a0dcd9992
📒 Files selected for processing (15)
apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.tsxapps/deploy-web/src/components/deployments/DeploymentListRow.tsxapps/deploy-web/src/components/layout/TransactionModal.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.spec.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.tsxapps/deploy-web/src/components/sdl/DeploymentMinimumEscrowAlertText.tsxapps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.tsapps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.tsapps/deploy-web/src/hooks/useMintACT/useMintACT.spec.tsapps/deploy-web/src/hooks/useMintACT/useMintACT.tsapps/deploy-web/tests/unit/mocks.tsxpackages/http-sdk/src/bme/bme-http.service.ts
✅ Files skipped from review due to trivial changes (3)
- apps/deploy-web/src/components/sdl/DeploymentMinimumEscrowAlertText.tsx
- apps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.ts
- apps/deploy-web/src/hooks/useMintACT/useMintACT.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.spec.tsx
- apps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.tsx
- apps/deploy-web/src/components/deployments/DeploymentListRow.tsx
apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx
Show resolved
Hide resolved
apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx (2)
143-147:⚠️ Potential issue | 🟡 MinorSkip minting when rounding removes the ACT deficit.
willAutoMintis decided beforesubmittedAmountis rounded to 6 decimals. Near the balance boundary, that can producedeficit <= 0here, and we still callmint(deficit). In that case we should fall through to the normal deposit path instead of minting zero/negative ACT.Suggested fix
if (willAutoMint) { const deficit = deposit - denomToUdenom(depositData?.balance ?? 0); + if (deficit <= 0) { + onSubmit(deposit); + return; + } setPendingDeposit(deposit); mint(deficit); } else { onSubmit(deposit); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx` around lines 143 - 147, willAutoMint is computed before submittedAmount is rounded, so after rounding the computed deficit can be <= 0 and you still call mint(deficit); change the flow to recompute the deficit using the rounded deposit amount (the same value used for the actual deposit) and only call mint when that recomputed deficit > 0; otherwise skip minting and proceed with the normal deposit path. Specifically, after rounding the amount used for deposit, calculate const deficit = deposit - denomToUdenom(depositData?.balance ?? 0), call setPendingDeposit(deposit) as before, but only call mint(deficit) if deficit > 0; if deficit <= 0 do not call mint and continue to the non-auto-mint branch behavior.
138-152:⚠️ Potential issue | 🟠 MajorBlock non-button submits when the form is logically disabled.
Continueis disabled withisSubmitDisabled, buthandleSubmit(convertAndSubmitDeposit)can still run on Enter or any programmatic form submit. That bypasses the min/balance/minting checks and can still callmint()/onSubmit()while the UI says submission is blocked.Suggested fix
const convertAndSubmitDeposit = useCallback( ({ amount: submittedAmount }: z.infer<typeof formSchema>) => { + if (isSubmitDisabled) return; + const amountInDenom = roundDecimal((isManaged && denom === UAKT_DENOM ? pricing.usdToAkt(submittedAmount) : submittedAmount) || 0, 6); const deposit = denomToUdenom(amountInDenom); if (willAutoMint) { const deficit = deposit - denomToUdenom(depositData?.balance ?? 0); setPendingDeposit(deposit); mint(deficit); } else { onSubmit(deposit); } }, - [isManaged, denom, pricing, onSubmit, willAutoMint, depositData?.balance, mint] + [isSubmitDisabled, isManaged, denom, pricing, onSubmit, willAutoMint, depositData?.balance, mint] );Also applies to: 260-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx` around lines 138 - 152, convertAndSubmitDeposit can still run when the form is logically disabled (isSubmitDisabled) because pressing Enter or programmatic submits call handleSubmit(convertAndSubmitDeposit); fix this by adding an early guard at the top of convertAndSubmitDeposit that returns immediately if isSubmitDisabled is true (so mint() / onSubmit() never run), and add isSubmitDisabled to the useCallback dependency array; also apply the same guard to any other submit wrapper that calls convertAndSubmitDeposit (e.g., the form submit handler bound via handleSubmit) to ensure programmatic/Enter submits are blocked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.tsx`:
- Around line 140-145: onDeploymentDeposit currently fire-and-forgets the
deposit tx so setAutoTopUpEnabled may be toggled even if the deposit is rejected
or fails; change the flow so depositDeployment (from
useDepositDeployment().deposit) returns the signAndBroadcastTx result (a Promise
resolving success/failure) and have onDeploymentDeposit await that result before
calling setIsDepositingDeployment(false) and before invoking
setAutoTopUpEnabled; update callers (including the toggle handler that calls
onDeploymentDeposit and setAutoTopUpEnabled) to await
onDeploymentDeposit(deposit) and only enable auto top-up when the awaited
deposit promise indicates success.
---
Duplicate comments:
In
`@apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx`:
- Around line 143-147: willAutoMint is computed before submittedAmount is
rounded, so after rounding the computed deficit can be <= 0 and you still call
mint(deficit); change the flow to recompute the deficit using the rounded
deposit amount (the same value used for the actual deposit) and only call mint
when that recomputed deficit > 0; otherwise skip minting and proceed with the
normal deposit path. Specifically, after rounding the amount used for deposit,
calculate const deficit = deposit - denomToUdenom(depositData?.balance ?? 0),
call setPendingDeposit(deposit) as before, but only call mint(deficit) if
deficit > 0; if deficit <= 0 do not call mint and continue to the non-auto-mint
branch behavior.
- Around line 138-152: convertAndSubmitDeposit can still run when the form is
logically disabled (isSubmitDisabled) because pressing Enter or programmatic
submits call handleSubmit(convertAndSubmitDeposit); fix this by adding an early
guard at the top of convertAndSubmitDeposit that returns immediately if
isSubmitDisabled is true (so mint() / onSubmit() never run), and add
isSubmitDisabled to the useCallback dependency array; also apply the same guard
to any other submit wrapper that calls convertAndSubmitDeposit (e.g., the form
submit handler bound via handleSubmit) to ensure programmatic/Enter submits are
blocked.
🪄 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
Run ID: 348c0db9-dd9f-40a3-bf45-ed6fefa8c323
📒 Files selected for processing (15)
apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.tsxapps/deploy-web/src/components/deployments/DeploymentListRow.tsxapps/deploy-web/src/components/layout/TransactionModal.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.spec.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.tsxapps/deploy-web/src/components/sdl/DeploymentMinimumEscrowAlertText.tsxapps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.tsapps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.tsapps/deploy-web/src/hooks/useMintACT/useMintACT.spec.tsapps/deploy-web/src/hooks/useMintACT/useMintACT.tsapps/deploy-web/tests/unit/mocks.tsxpackages/http-sdk/src/bme/bme-http.service.ts
✅ Files skipped from review due to trivial changes (4)
- apps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.spec.tsx
- apps/deploy-web/src/components/sdl/DeploymentMinimumEscrowAlertText.tsx
- apps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.ts
- apps/deploy-web/src/hooks/useMintACT/useMintACT.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/deploy-web/tests/unit/mocks.tsx
- packages/http-sdk/src/bme/bme-http.service.ts
- apps/deploy-web/src/components/deployments/DeploymentListRow.tsx
- apps/deploy-web/src/hooks/useMintACT/useMintACT.ts
apps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.tsx
Show resolved
Hide resolved
ygrishajev
force-pushed
the
feature/deposit-modal
branch
2 times, most recently
from
a8b98ee to
e02d3ad
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.tsx (1)
140-144:⚠️ Potential issue | 🟠 MajorOnly close the deposit flow after the deposit promise succeeds.
Line 142 still clears
isDepositingDeploymentbeforedepositDeployment()settles. The auto-top-up toggle is now gated correctly, but the modal path still drops the user out of the flow if the post-mint deposit is rejected or fails.Suggested fix
const onDeploymentDeposit = useCallback( async (deposit: number) => { - setIsDepositingDeployment(false); - return depositDeployment(deposit); + const deposited = await depositDeployment(deposit); + if (deposited) { + setIsDepositingDeployment(false); + } + return deposited; }, [depositDeployment] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.tsx` around lines 140 - 144, The handler onDeploymentDeposit clears isDepositingDeployment before depositDeployment settles; change it so it awaits depositDeployment(deposit) first and only calls setIsDepositingDeployment(false) after the deposit promise resolves successfully (i.e., await result = depositDeployment(deposit); setIsDepositingDeployment(false); return result), and ensure errors from depositDeployment are propagated (do not swallow them with a premature state reset).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/deploy-web/src/hooks/useMintACT/useMintACT.ts`:
- Around line 101-104: The current mint flow in useMintACT calls
bmeHttpService.waitForLedgerRecordsSettlement which returns true only when there
are zero pending records for the whole address, causing unrelated
pending-records to trigger "Mint settlement timed out." Change the polling to
scope to this mint's record(s): before broadcasting, snapshot existing pending
records (via bmeHttpService.getPendingLedgerRecords or similar), then after
broadcast identify the new record/tx id(s) produced by this mint and poll only
those ids (or call a new waitForLedgerRecordsSettlement variant that accepts an
array of record/tx ids to ignore/target). Update the useMintACT logic (around
the abortController.current.signal usage and the call site of
waitForLedgerRecordsSettlement) to use the snapshot-and-poll-by-id approach so
pre-existing pending records are ignored.
---
Duplicate comments:
In
`@apps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.tsx`:
- Around line 140-144: The handler onDeploymentDeposit clears
isDepositingDeployment before depositDeployment settles; change it so it awaits
depositDeployment(deposit) first and only calls setIsDepositingDeployment(false)
after the deposit promise resolves successfully (i.e., await result =
depositDeployment(deposit); setIsDepositingDeployment(false); return result),
and ensure errors from depositDeployment are propagated (do not swallow them
with a premature state reset).
🪄 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
Run ID: 51972666-9264-44cd-b4ac-96544d9fdee0
📒 Files selected for processing (14)
apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.spec.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.tsxapps/deploy-web/src/components/deployments/DeploymentListRow.tsxapps/deploy-web/src/components/layout/TransactionModal.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.spec.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.tsxapps/deploy-web/src/components/sdl/DeploymentMinimumEscrowAlertText.tsxapps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.tsapps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.tsapps/deploy-web/src/hooks/useMintACT/useMintACT.spec.tsapps/deploy-web/src/hooks/useMintACT/useMintACT.tspackages/http-sdk/src/bme/bme-http.service.ts
✅ Files skipped from review due to trivial changes (6)
- apps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.spec.tsx
- apps/deploy-web/src/components/sdl/DeploymentMinimumEscrowAlertText.tsx
- apps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.spec.ts
- apps/deploy-web/src/hooks/useDepositDeployment/useDepositDeployment.ts
- apps/deploy-web/src/hooks/useMintACT/useMintACT.spec.ts
- apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.spec.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.spec.tsx
- apps/deploy-web/src/components/deployments/DeploymentListRow.tsx
- apps/deploy-web/src/components/layout/TransactionModal.tsx
- apps/deploy-web/src/components/new-deployment/ManifestEdit/ManifestEdit.tsx
- packages/http-sdk/src/bme/bme-http.service.ts
ygrishajev
force-pushed
the
feature/deposit-modal
branch
from
e02d3ad to
ff7195c
Compare
baktun14
reviewed
Apr 9, 2026
apps/deploy-web/src/components/deployments/DeploymentDepositModal/DeploymentDepositModal.tsx
Outdated
Show resolved
Hide resolved
Contributor
|
For managed wallets, users aren't aware what ACT is, we should show "Credits" or just "$" instead "Select the $ amount" "Minimum 0.5$" "Current balance: 10.00$" 
|
ygrishajev
force-pushed
the
feature/deposit-modal
branch
from
ff7195c to
34a895c
Compare
baktun14
approved these changes
Apr 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Resolves CON-173
The deposit modal needed a redesign to support ACT deposits with auto-minting. Previously, users with insufficient ACT balance had to manually navigate to the Mint/Burn page before depositing. This change streamlines the flow by auto-minting ACT when the user's balance is insufficient.
What
useMintACThook — encapsulates AKT→ACT minting: price calculation with slippage, min-mint enforcement, tx broadcast, settlement polling, balance refreshuseDepositDeploymenthook — thin wrapper for deposit-to-existing-deployment tx (build message, broadcast, call onSuccess)DeploymentDepositModalredesign — ACT-specific UI with radio presets (25/50/100), custom amount input, balance display, auto-mint orchestration when ACT balance is insufficientBmeHttpService.waitForLedgerRecordsSettlement— polling utility for mint settlement confirmationTransactionModal— addedmintingACTstate, refactored description display to use a mapDeploymentMinimumEscrowAlertText— changed<b>to<span>for styling consistencyDeploymentDetailTopBar,DeploymentListRow,ManifestEdit) updated to use new hooks and modal APIScreen.Recording.2026-04-09.at.11.05.26.comp.mov
Screen.Recording.2026-04-09.at.13.02.31.mov
Summary by CodeRabbit