PRO-3885-Max Button Fix #493
Conversation
📝 WalkthroughWalkthroughAdds MAX-selection support across Pulse buy flow: per-chain balances now include tokenAmount; HomeScreen tracks isMaxSelected and maxTokenAmount and passes them to Buy/PreviewBuy; Buy/UI updated for MAX display and behavior; useRelayBuy accepts maxTokenAmount to compute offers directly from token amounts. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant HomeScreen
participant Buy
participant PreviewBuy
participant RelayHook as useRelayBuy
participant API as QuoteAPI
User->>HomeScreen: Clicks MAX
HomeScreen->>HomeScreen: set isMaxSelected = true, set maxTokenAmount (from chain tokenAmount)
HomeScreen->>Buy: pass isMaxSelected, maxTokenAmount, setters
Buy->>Buy: render balance text (MAX mode)
Buy->>RelayHook: request best offer (include maxTokenAmount)
RelayHook->>RelayHook: convert maxTokenAmount -> wei (USDC decimals)
RelayHook->>API: fetch quote with wei amount
API-->>RelayHook: return quote
RelayHook-->>Buy: return best offer
Buy->>PreviewBuy: pass offer + isMaxSelected, maxTokenAmount
PreviewBuy->>PreviewBuy: display MAX-based offer
User->>Buy: manually edits USD
Buy->>HomeScreen: set isMaxSelected = false, clear maxTokenAmount
Buy->>RelayHook: trigger USD-based offer flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
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 |
Deploying pillarx-debug with
|
| Latest commit: |
a092f67
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c8426139.pillarx-debug.pages.dev |
| Branch Preview URL: | https://pro-3885-maxbuttonfix.pillarx-debug.pages.dev |
Deploying x with
|
| Latest commit: |
a092f67
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ed1db418.x-e62.pages.dev |
| Branch Preview URL: | https://pro-3885-maxbuttonfix.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/apps/pulse/components/Buy/Buy.tsx (1)
463-471: MissingmaxTokenAmountinfetchBuyOfferdependency array.
maxTokenAmountis used in the callback (lines 424-427) but is not listed in the dependency array. This causes stale closures whenmaxTokenAmountchanges.🔎 Proposed fix
}, [ debouncedUsdAmount, token, isRelayInitialized, isMaxSelected, getBestOffer, maxStableCoinBalance.chainId, usdcPrice, + maxTokenAmount, ]);src/apps/pulse/components/App/HomeScreen.tsx (1)
399-413: Remove debug console.log statements before merging.Lines 399 and 408-413 contain debug logging for stable balance calculations. These should be removed or converted to conditional debug logging for production.
🔎 Proposed fix
const stableBalance = getStableCurrencyBalanceOnEachChain(walletPortfolioData); - console.log('Stable balances on each chain:', stableBalance); const maxStableBalance = Math.max( ...Object.values(stableBalance).map((s) => s.balance) ); const chainIdOfMaxStableBalance = Number( Object.keys(stableBalance).find( (key) => stableBalance[Number(key)].balance === maxStableBalance ) || '1' ); - console.log( - 'Max stable coin balance:', - maxStableBalance, - 'on chainId:', - chainIdOfMaxStableBalance - );
🤖 Fix all issues with AI Agents
In @src/apps/pulse/components/Buy/PreviewBuy.tsx:
- Around line 622-629: The callback refreshPreviewBuyData currently uses
isMaxSelected and maxTokenAmount when calling getBestOffer but does not include
them in its dependency array; update the dependency array of
refreshPreviewBuyData to include isMaxSelected and maxTokenAmount so the hook
re-creates with current values and avoids stale closures (ensure any other
related variables used inside the callback are also present in the dependency
array).
In @src/apps/pulse/hooks/useRelayBuy.ts:
- Line 176: Remove the stray debug console.log calls in useRelayBuy hook: delete
the console.log('tokenAmount: ', maxTokenAmount) and the console.log statements
at the fee-calculation block (lines around 352-356) inside the useRelayBuy.ts
hook; if you need runtime togglable debugging, replace them with a guarded
logger (e.g., process.env.DEBUG or a passed-in logger) or use a debug utility so
production logs are not polluted, and ensure references to maxTokenAmount and
the fee variables remain unchanged in the surrounding logic.
- Around line 174-182: The NaN check is wrong because
Number.isNaN(maxTokenAmount) will always be false for a string; update the
validation in useRelayBuy (the maxTokenAmount branch) to coerce the string to a
number before checking (e.g., const numeric = Number(maxTokenAmount); if
(Number.isNaN(numeric)) throw new Error('Invalid maxTokenAmount'); ), then use
numeric (or keep the original string only after successful validation) when
calling parseUnits to set fromAmountInWei; reference maxTokenAmount, numeric (or
converted value), parseUnits, and fromAmountInWei to locate the change.
🧹 Nitpick comments (1)
src/apps/pulse/components/Buy/Buy.tsx (1)
307-316: Type safety concern withmaxTokenAmountcomparison.The condition
maxTokenAmount < 2on line 311 assumesmaxTokenAmountis a number, which matches the prop type. However, ifmaxTokenAmountisundefined(since it's optional), the comparisonundefined < 2evaluates tofalsedue to JavaScript coercion, which might not be the intended behavior. The guard on line 308 (maxTokenAmount && maxTokenAmount > 0) should prevent this, but consider using a more explicit type guard.🔎 Safer approach
- if (isMaxSelected && maxTokenAmount && maxTokenAmount > 0) { - const amount = maxTokenAmount; - - if (amount < 2) { + if (isMaxSelected && typeof maxTokenAmount === 'number' && maxTokenAmount > 0) { + if (maxTokenAmount < 2) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/apps/key-wallet/utils/blockchain.tssrc/apps/pulse/components/App/HomeScreen.tsxsrc/apps/pulse/components/Buy/Buy.tsxsrc/apps/pulse/components/Buy/PreviewBuy.tsxsrc/apps/pulse/components/Buy/tests/Buy.test.tsxsrc/apps/pulse/hooks/useRelayBuy.tssrc/apps/pulse/utils/utils.tsx
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Buy/PreviewBuy.tsxsrc/apps/pulse/components/Buy/Buy.tsxsrc/apps/pulse/hooks/useRelayBuy.tssrc/apps/pulse/utils/utils.tsxsrc/apps/pulse/components/App/HomeScreen.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Buy/PreviewBuy.tsxsrc/apps/pulse/components/Buy/Buy.tsxsrc/apps/pulse/hooks/useRelayBuy.tssrc/apps/pulse/components/Buy/tests/Buy.test.tsxsrc/apps/pulse/components/App/HomeScreen.tsx
📚 Learning: 2025-08-20T09:14:16.888Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
Applied to files:
src/apps/pulse/components/Buy/tests/Buy.test.tsx
📚 Learning: 2025-12-11T12:40:09.964Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 478
File: src/apps/pulse/components/App/AppWrapper.tsx:22-23
Timestamp: 2025-12-11T12:40:09.964Z
Learning: In the Pulse app's AppWrapper component (src/apps/pulse/components/App/AppWrapper.tsx), when users enter via `/pulse?searching=true` from PillarX home and select a token or market, they should continue into Pulse to complete their transaction, not navigate back to home. Only when closing the search without making a selection should they return to PillarX home.
Applied to files:
src/apps/pulse/components/App/HomeScreen.tsx
📚 Learning: 2025-11-21T13:10:33.422Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/App/HomeScreen.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/App/HomeScreen.tsx
🧬 Code graph analysis (3)
src/apps/pulse/components/Buy/Buy.tsx (1)
src/apps/pulse/utils/intent.ts (1)
getDispensableAssets(22-88)
src/apps/pulse/utils/utils.tsx (1)
src/apps/pulse/constants/tokens.ts (1)
STABLE_CURRENCIES(41-43)
src/apps/pulse/components/Buy/tests/Buy.test.tsx (2)
src/apps/pulse/components/Buy/Buy.tsx (1)
Buy(94-1015)src/test-utils/testUtils.tsx (1)
TestWrapper(116-134)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
- GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (6)
src/apps/key-wallet/utils/blockchain.ts (1)
311-311: LGTM!Adding explicit
'en-US'locale ensures consistent USD formatting regardless of the user's system locale settings.src/apps/pulse/components/Buy/Buy.tsx (1)
940-952: MAX button logic correctly handles fee deduction.The implementation properly:
- Uses
tokenAmount(actual balance) for display- Calculates fee-adjusted amount (
fullBalance * 0.99)- Rounds down conservatively with
Math.floorThis aligns with the PR objective to pass the whole token amount instead of USD-equivalent.
src/apps/pulse/components/Buy/tests/Buy.test.tsx (2)
331-367: Well-structured test harness for MAX state management.The
BuyWithStatewrapper component and conditionaluseStateWrapperparameter provide a clean approach to test stateful MAX behavior while keeping non-MAX tests simpler.
433-440: Test correctly verifies MAX button behavior.The test properly validates that clicking MAX displays the balance as text (
10050.00) rather than in an input field, matching the new UI behavior.src/apps/pulse/utils/utils.tsx (1)
218-264: LGTM! Correctly addstokenAmountto per-chain balance records.The change properly extends the balance structure to include
tokenAmount(raw balance) alongsidebalance(USD value). This enables the MAX button fix by providing the actual token amount for API calls.src/apps/pulse/components/App/HomeScreen.tsx (1)
130-134: State management and prop threading for MAX flow is well structured.The implementation correctly:
- Lifts
isMaxSelectedandmaxTokenAmountstate to HomeScreen- Extends
maxStableCoinBalancewithtokenAmount- Passes state and setters to Buy component
- Passes values to PreviewBuy for offer calculations
This enables the MAX button to use actual token amounts throughout the buy flow.
Also applies to: 171-172, 1217-1218, 1392-1393
github-actions
bot
temporarily deployed
to
Preview (PRO-3885-MaxButtonFix)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/apps/pulse/hooks/useRelayBuy.ts (1)
165-171: Clarify the dual-path comment structure.The comment block describes "Convert USD amount to USDC amount" but now there are two distinct paths: (1) use maxTokenAmount directly, or (2) convert USD to USDC. Consider restructuring the comments to clearly delineate both paths before the conditional logic.
🔎 Suggested comment structure
/** - * Step 2: Convert USD amount to USDC amount using actual USDC price - * If maxTokenAmount is provided, use it directly (for MAX selections) - * Otherwise, fromAmount is in USD, we need to convert to USDC amount - * Then convert to USDC's smallest unit (6 decimals) - * Example: $10 USD / $0.9998 USDC price = 10.002 USDC = 10002000 in wei + * Step 2: Determine USDC amount to use for the swap + * Path A: If maxTokenAmount is provided, use it directly (for MAX selections) + * Path B: Otherwise, convert USD to USDC amount using actual USDC price + * Then convert to USDC's smallest unit using decimals (e.g., 6 on Ethereum, 18 on BSC) + * Example (Path B): $10 USD / $0.9998 USDC price = 10.002 USDC = 10002000 in wei */src/apps/pulse/components/Buy/Buy.tsx (1)
941-953: Consider clarifying the MAX fee calculation logic.The MAX button calculates
maxAmount = fullBalance * 0.99to account for the 1% platform fee, which aligns with the fee calculation inbuildTransactions(useRelayBuy.ts lines 342-350). While the logic is correct, adding a brief inline comment explaining that this pre-deducts the fee sototalUsdcNeeded = (maxAmount * 100) / 99reconstructs the full balance would improve maintainability.🔎 Suggested clarification
// For API calls, calculate amount after 1% platform fee const maxAmount = fullBalance * 0.99; + // Pre-deduct 1% fee: buildTransactions will reverse this via (amount * 100) / 99 to get totalUsdcNeeded // Proper rounding: round down to be conservative with fee calculation const roundedAmount = Math.floor(maxAmount * 100) / 100;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/apps/pulse/components/App/HomeScreen.tsxsrc/apps/pulse/components/Buy/Buy.tsxsrc/apps/pulse/components/Buy/PreviewBuy.tsxsrc/apps/pulse/hooks/useRelayBuy.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/pulse/components/Buy/PreviewBuy.tsx
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/hooks/useRelayBuy.tssrc/apps/pulse/components/App/HomeScreen.tsxsrc/apps/pulse/components/Buy/Buy.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/hooks/useRelayBuy.tssrc/apps/pulse/components/App/HomeScreen.tsxsrc/apps/pulse/components/Buy/Buy.tsx
📚 Learning: 2025-12-11T12:40:09.964Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 478
File: src/apps/pulse/components/App/AppWrapper.tsx:22-23
Timestamp: 2025-12-11T12:40:09.964Z
Learning: In the Pulse app's AppWrapper component (src/apps/pulse/components/App/AppWrapper.tsx), when users enter via `/pulse?searching=true` from PillarX home and select a token or market, they should continue into Pulse to complete their transaction, not navigate back to home. Only when closing the search without making a selection should they return to PillarX home.
Applied to files:
src/apps/pulse/components/App/HomeScreen.tsx
📚 Learning: 2025-11-21T13:10:33.422Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/App/HomeScreen.tsxsrc/apps/pulse/components/Buy/Buy.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/App/HomeScreen.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Buy/Buy.tsx (1)
src/apps/pulse/utils/intent.ts (1)
getDispensableAssets(22-88)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (6)
src/apps/pulse/components/Buy/Buy.tsx (4)
68-92: LGTM!The type extensions for MAX support are well-structured. The tokenAmount field in maxStableCoinBalance and the new MAX-related props are properly typed and optional as expected.
275-286: LGTM!The manual input handler correctly resets MAX-related state when users type their own amount, ensuring the MAX and manual input paths don't conflict.
819-834: Verify the MAX display amount matches user expectations.When MAX is selected, line 821 displays the full balance (e.g., 100 USDC), but the actual
maxTokenAmountused internally is fee-adjusted (99 USDC, see line 952 in the MAX button handler). Users might be confused seeing "100 USD" displayed while only 99 is actually being used for the swap. Confirm this is the intended UX—showing the full balance in the input while internally deducting the 1% fee.
329-343: Validation bypass for MAX selection is safe—getBestOffer provides proper validation.The code correctly skips
dispensableAssetsvalidation when MAX is selected (lines 330–343) becausegetBestOffervalidates the full balance amount. The function validates themaxTokenAmountwith NaN checks, confirms the quote response contains valid output (currencyOut.amountFormatted > 0), and returns errors if validation fails. Errors bubble up asrelayErrorand preventPreviewBuyfrom opening. ThedispensableAssetsare not used in the Relay Buy flow, only in the Intent SDK path, so the bypass is appropriate for MAX selections.src/apps/pulse/components/App/HomeScreen.tsx (2)
130-134: LGTM!The MAX state management is well-integrated into HomeScreen. The tokenAmount field is properly initialized with safe fallbacks (
?? 0), and the new state variables (isMaxSelected, maxTokenAmount) are correctly typed and initialized.Also applies to: 171-172, 414-418
1210-1211: LGTM!The MAX-related props are correctly threaded through the component hierarchy. The default maxStableCoinBalance object includes the tokenAmount field, and the state update callbacks are properly passed to the Buy component.
Also applies to: 1368-1372, 1385-1386
| portfolioTokens={portfolioTokens} | ||
| maxStableCoinBalance={ | ||
| maxStableCoinBalance ?? { chainId: 1, balance: 2 } | ||
| maxStableCoinBalance ?? { |
There was a problem hiding this comment.
Is it a good idea to have default value for chainId and balance here?
There was a problem hiding this comment.
its a react render so as soon as the maxStableCoinBalance changes it will get reflected on the pulse buy/sell page its just a matter of milliseconds
RanaBug
left a comment
RanaBug
left a comment
There was a problem hiding this comment.
Minor comment, otherwise good to go
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.