Skip to content

Comments

BNB Fix on Pulse#475

Merged
vignesha22 merged 1 commit intostagingfrom
BNB_Bug_Fix
Dec 10, 2025
Merged

BNB Fix on Pulse#475
vignesha22 merged 1 commit intostagingfrom
BNB_Bug_Fix

Conversation

@vignesha22
Copy link
Contributor

@vignesha22 vignesha22 commented Dec 10, 2025

Description

  • Changed usdc decimals fetch to dynamic instead of hardcoded 6 since BNB chain USDC has 18 decimals

How Has This Been Tested?

  • Tested locally

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved stablecoin decimal precision handling across trading operations to ensure accurate token conversions and balance calculations.
  • Refactor

    • Enhanced token data structures to include decimal information, enabling more reliable and consistent handling of different token types across the application.

✏️ Tip: You can customize this high-level summary in your review settings.

@vignesha22 vignesha22 requested review from IAmKio and RanaBug December 10, 2025 12:36
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Walkthrough

This PR refactors the USDC token API to return a rich token object with decimal precision instead of a simple address string. The getUSDCAddress method is renamed to getUSDCToken across the Relay buy and sell hooks, with implementations and tests updated accordingly. Token constants are expanded to include a decimals field.

Changes

Cohort / File(s) Summary
Hook API Implementation
src/apps/pulse/hooks/useRelayBuy.ts, src/apps/pulse/hooks/useRelaySell.ts
Renamed getUSDCAddress(chainId)getUSDCToken(chainId) returning { chainId, address, decimals } object. Updated all internal call sites to destructure address and decimals from the token object, and use decimals in place of hardcoded values for conversion logic.
Hook Tests
src/apps/pulse/hooks/tests/useRelaySell.test.tsx
Updated test mocks to return token object structure { chainId, address, decimals } from getUSDCToken. Adjusted assertions to validate token properties instead of raw address strings. Extended executeSell calls to include additional parameters.
Component Usage
src/apps/pulse/components/Sell/PreviewSell.tsx
Updated hook usage from getUSDCAddress(chainId) to getUSDCToken(chainId), extracting address from the returned token object. Fallback to empty string if token is undefined.
Component Tests
src/apps/pulse/components/Sell/tests/PreviewSell.test.tsx, src/apps/pulse/components/Buy/tests/Buy.test.tsx
Updated all mock definitions of useRelaySell and useRelayBuy to expose getUSDCToken returning token object with { chainId, address, decimals } instead of getUSDCAddress returning address string.
Token Constants
src/apps/pulse/constants/tokens.ts
Expanded allStableCurrencies entries from { chainId, address } to { chainId, address, decimals }, adding numeric decimals field to each currency definition.
Hook Cleanup
src/apps/pulse/hooks/useTopUp.ts
Removed dependency on getUSDCAddress from useRelaySell usage; simplified transaction assembly by dropping USDC address resolution logic and updated dependency arrays.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify decimal threading: Confirm that usdcDecimals extracted from the token object is correctly propagated through all conversion, balance, and quote calculation code paths (particularly in useRelayBuy.ts and useRelaySell.ts).
  • Mock consistency: Ensure all test mocks across components and hooks consistently return the new token object structure with all required fields.
  • Call-site completeness: Verify no orphaned references to getUSDCAddress remain; confirm all destructuring patterns properly handle the new return type.
  • Fallback handling: Review edge cases where getUSDCToken returns null or undefined to ensure address fallbacks (empty string) are handled appropriately.

Possibly related PRs

  • PR #446: Touches the same Relay sell APIs, updating getUSDCAddressgetUSDCToken (token object with decimals) and modifying executeSell/getBestSellOffer signatures.
  • PR #394: Directly affects the same useRelaySell/useRelayBuy API and token-decimals changes.
  • PR #391: Introduces the initial getUSDCAddress/getBestSellOffer/executeSell surface that this PR refactors to include token decimals.

Suggested reviewers

  • IAmKio
  • RanaBug

Poem

🐰 USDC tokens now wear their decimals with pride,
No more bare addresses left to hide!
From string to object, so rich and complete,
This relay refactor makes precision so sweet. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'BNB Fix on Pulse' is vague and doesn't clearly convey the main technical change (USDC decimals handling). Consider a more specific title like 'Make USDC decimals dynamic based on chain' to better describe the primary change.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description covers the main change and testing approach, with the bug fix type selected appropriately.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch BNB_Bug_Fix

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 116b02c and 86b579a.

📒 Files selected for processing (8)
  • src/apps/pulse/components/Buy/tests/Buy.test.tsx (1 hunks)
  • src/apps/pulse/components/Sell/PreviewSell.tsx (2 hunks)
  • src/apps/pulse/components/Sell/tests/PreviewSell.test.tsx (4 hunks)
  • src/apps/pulse/constants/tokens.ts (1 hunks)
  • src/apps/pulse/hooks/tests/useRelaySell.test.tsx (6 hunks)
  • src/apps/pulse/hooks/useRelayBuy.ts (6 hunks)
  • src/apps/pulse/hooks/useRelaySell.ts (12 hunks)
  • src/apps/pulse/hooks/useTopUp.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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/tests/useRelaySell.test.tsx
  • src/apps/pulse/components/Sell/tests/PreviewSell.test.tsx
  • src/apps/pulse/components/Sell/PreviewSell.tsx
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.

Applied to files:

  • src/apps/pulse/hooks/tests/useRelaySell.test.tsx
  • src/apps/pulse/hooks/useRelaySell.ts
  • src/apps/pulse/hooks/useRelayBuy.ts
  • src/apps/pulse/hooks/useTopUp.ts
📚 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/Sell/tests/PreviewSell.test.tsx
📚 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.ts
  • src/apps/pulse/constants/tokens.ts
🧬 Code graph analysis (2)
src/apps/pulse/hooks/useRelaySell.ts (1)
src/apps/pulse/constants/tokens.ts (1)
  • STABLE_CURRENCIES (41-43)
src/apps/pulse/hooks/useRelayBuy.ts (2)
src/apps/pulse/constants/tokens.ts (1)
  • STABLE_CURRENCIES (41-43)
src/apps/the-exchange/utils/blockchain.ts (1)
  • toWei (130-132)
⏰ 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). (3)
  • GitHub Check: lint
  • GitHub Check: unit-tests
  • GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (26)
src/apps/pulse/components/Buy/tests/Buy.test.tsx (1)

97-101: LGTM!

The mock correctly reflects the new getUSDCToken API, returning a token object with chainId, address, and decimals. The structure is consistent with the implementation changes.

src/apps/pulse/components/Sell/tests/PreviewSell.test.tsx (2)

165-169: LGTM!

The mock correctly implements the new getUSDCToken API with proper token object shape. All test cases are consistently updated.


348-349: LGTM!

Edge case properly tests null return from getUSDCToken, ensuring the component handles missing USDC token gracefully.

src/apps/pulse/constants/tokens.ts (2)

3-38: LGTM! All stable currency entries now include decimals.

The expansion of the token data structure to include decimals enables dynamic decimal handling across all chains, which is the right approach for supporting chains with non-standard USDC implementations.


29-33: Core fix correctly addresses BNB chain USDC with 18 decimals.

The USDC on BNB Smart Chain (address 0x8AC76a51cc950d9822D68b83fE1Ad97B32Cd580d) uses 18 decimals, unlike standard USDC implementations on other chains which use 6 decimals. This change fixes the BNB-specific handling in the token configuration.

src/apps/pulse/hooks/useTopUp.ts (1)

35-36: LGTM!

Clean removal of the getUSDCAddress dependency. The buildSellTransactions function now internally handles USDC token lookup via getUSDCToken, so useTopUp no longer needs direct access.

src/apps/pulse/components/Sell/PreviewSell.tsx (2)

72-72: LGTM!

Correctly updated to use the new getUSDCToken API.


235-236: LGTM!

Proper null handling when extracting the USDC address from the token object. Falls back to empty string when token is not found, which correctly triggers the conditional rendering logic downstream (lines 721-742).

src/apps/pulse/hooks/useRelaySell.ts (11)

114-127: LGTM! Core API change implemented correctly.

The getUSDCToken function now returns a complete token object with chainId, address, and decimals, enabling dynamic decimal handling across different chains. The null return for unknown chains is handled correctly in all call sites.


148-156: LGTM!

Correct extraction of both address and decimals from the token object, with proper null check before proceeding.


216-224: LGTM! Dynamic decimal conversion.

The USDC amount conversion now correctly uses usdcDecimals instead of a hardcoded value. This is the key fix that enables proper handling of BNB chain's 18-decimal USDC.


341-347: LGTM!

Proper token lookup and null handling in buildSellTransactions.


1019-1032: LGTM!

getBestSellOfferWithBridge correctly uses getUSDCToken to get both address and decimals for the bridge operation.


1150-1157: LGTM!

buildSellTransactionWithBridge correctly extracts both address and decimals from the USDC token object.


1417-1419: LGTM!

The USDC amount formatting for the bridge now correctly uses dynamic usdcDecimals instead of a hardcoded value.


1472-1475: LGTM!

parseUnits correctly uses dynamic usdcDecimals for converting the bridge amount to BigInt.


1082-1082: LGTM!

Dependency array correctly updated to include getUSDCToken.


1604-1611: LGTM!

Dependency array correctly includes getUSDCToken for buildSellTransactionWithBridge.


1618-1619: LGTM!

Export correctly renamed from getUSDCAddress to getUSDCToken to reflect the new API.

src/apps/pulse/hooks/tests/useRelaySell.test.tsx (3)

52-59: LGTM! Mock data aligns with new token object structure.

The mock correctly includes the decimals field, matching the new token object shape returned by getUSDCToken.


185-185: LGTM! Tests properly validate the new token object API.

The tests correctly verify:

  • getUSDCToken returns an object with address, decimals, and chainId for supported chains
  • getUSDCToken returns null for unsupported chains

Also applies to: 252-298, 300-342


640-644: The executeSell signature is consistent across all call sites. The function takes four parameters—token, amount, toChainId, and an optional userPortfolio—and all invocations correctly pass the required parameters. The test call at lines 640-644 passes three arguments, which is valid since userPortfolio is optional; the PreviewSell.tsx call passes all four arguments. No updates are needed.

src/apps/pulse/hooks/useRelayBuy.ts (4)

111-120: LGTM! Function correctly returns rich token object.

The refactored getUSDCToken function now returns a complete token descriptor including chainId, address, and decimals, enabling dynamic decimal handling as intended by this PR.


140-148: LGTM! Proper extraction of token details.

The code correctly retrieves the token object and extracts both usdcAddress and usdcDecimals for subsequent use.


319-326: LGTM! Consistent token retrieval pattern.

The code follows the same pattern as getBestOffer, correctly retrieving the token object and extracting both address and decimals.


371-371: No action required — USDC decimals in STABLE_CURRENCIES are correctly configured.

The configuration in src/apps/pulse/constants/tokens.ts accurately reflects on-chain values:

  • Ethereum mainnet (0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48): 6 decimals ✓
  • BNB Chain (0x8AC76a51cc950d9822D68b83fE1Ad97B32Cd580d): 18 decimals ✓
  • All other chains (Optimism, Polygon, Base, Arbitrum, Gnosis): 6 decimals ✓

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link

Deploying x with  Cloudflare Pages  Cloudflare Pages

Latest commit: 86b579a
Status: ✅  Deploy successful!
Preview URL: https://599f9f59.x-e62.pages.dev
Branch Preview URL: https://bnb-bug-fix.x-e62.pages.dev

View logs

@cloudflare-workers-and-pages
Copy link

Deploying pillarx-debug with  Cloudflare Pages  Cloudflare Pages

Latest commit: 86b579a
Status: ✅  Deploy successful!
Preview URL: https://e14e1c74.pillarx-debug.pages.dev
Branch Preview URL: https://bnb-bug-fix.pillarx-debug.pages.dev

View logs

@vignesha22 vignesha22 merged commit 3a0a9fa into staging Dec 10, 2025
7 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Jan 6, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants