Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR adds first-class PYUSD handling to the token bridge UI, including support for Arbitrum One’s PYUSD OFT vs canonical representations and better integration with LiFi routing/overrides.
Changes:
- Introduces PYUSD utilities + common addresses, and adds token override logic for PYUSD (Ethereum ↔ Arbitrum One OFT) across the UI and LiFi APIs.
- Extends token metadata with
importLookupAddress,sourceBalanceAddress, anddestinationBalanceAddressto support multi-address tokens and correct balance lookups. - Updates token selection, destination-token resolution, routing eligibility, and token search/import flows; adds/updates targeted tests.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/arb-token-bridge-ui/src/util/PyusdUtils.ts | Adds PYUSD token constructors + helper predicates for Ethereum / Arb One canonical / Arb One OFT. |
| packages/arb-token-bridge-ui/src/util/CommonAddressUtils.ts | Adds PYUSD addresses for Ethereum and Arbitrum One (canonical + OFT). |
| packages/arb-token-bridge-ui/src/hooks/useSelectedToken.ts | Special-cases PYUSD selection/mapping with stable metadata (price/listIds) to prevent rerender churn. |
| packages/arb-token-bridge-ui/src/hooks/useDestinationToken.ts | Improves destination token resolution for OFT-style tokens and non-swap flows. |
| packages/arb-token-bridge-ui/src/hooks/useBalanceOnSourceChain.ts | Adds sourceBalanceAddress support for correct balance sourcing. |
| packages/arb-token-bridge-ui/src/hooks/useBalanceOnDestinationChain.ts | Adds destinationBalanceAddress support for correct balance sourcing. |
| packages/arb-token-bridge-ui/src/hooks/arbTokenBridge.types.ts | Extends BridgeToken with import/balance lookup address fields. |
| packages/arb-token-bridge-ui/src/hooks/tests/useSelectedToken.test.ts | Adds regression test ensuring stable PYUSD token object reference. |
| packages/arb-token-bridge-ui/src/hooks/tests/useDestinationToken.test.ts | Adds PYUSD deposit/withdraw destination-token mapping tests. |
| packages/arb-token-bridge-ui/src/components/common/NetworkSelectionContainer.tsx | Adjusts network switching behavior to keep token selection aligned with resolved destination token. |
| packages/arb-token-bridge-ui/src/components/TransferPanel/hooks/useRoutesUpdater.ts | Refactors “fromToken” handling and delays LiFi fetch until ERC20 selection resolves. |
| packages/arb-token-bridge-ui/src/components/TransferPanel/hooks/useIsSwapTransfer.test.ts | Adds tests ensuring PYUSD canonical/OFT cases are not misclassified as swaps. |
| packages/arb-token-bridge-ui/src/components/TransferPanel/TransferPanelMain.tsx | Keeps selected token aligned when switching networks in swap/resolved-destination scenarios. |
| packages/arb-token-bridge-ui/src/components/TransferPanel/TransferPanel.tsx | Improves “token already imported” detection and fixes ternary precedence when choosing LiFi token addresses. |
| packages/arb-token-bridge-ui/src/components/TransferPanel/TransferDisabledDialog.tsx | Disables canonical transfer for PYUSD-specific cases and avoids dialog before token lists load. |
| packages/arb-token-bridge-ui/src/components/TransferPanel/TokenSearchUtils.ts | Sanitizes/parses priceUSD extension values from token lists. |
| packages/arb-token-bridge-ui/src/components/TransferPanel/TokenSearch.tsx | Adds PYUSD OFT visibility/balance handling for Arb One → Ethereum withdrawals; improves error typing and normalization. |
| packages/arb-token-bridge-ui/src/components/TransferPanel/TokenRow.tsx | Uses override logo only when present; checks importLookupAddress when deciding if token is added to bridge. |
| packages/arb-token-bridge-ui/src/components/TransferPanel/TokenImportDialog.tsx | Normalizes addresses for lookups and token-add flows. |
| packages/arb-token-bridge-ui/src/components/TransferPanel/TokenButton.tsx | Avoids token loader when selection has already resolved. |
| packages/arb-token-bridge-ui/src/app/api/crosschain-transfers/utils.ts | Adds PYUSD-specific LiFi eligibility rules + token overrides. |
| packages/arb-token-bridge-ui/src/app/api/crosschain-transfers/utils.test.ts | Adds tests for PYUSD LiFi eligibility and override mapping. |
| packages/arb-token-bridge-ui/src/app/api/crosschain-transfers/lifi.ts | Normalizes PYUSD (and USDT) name/symbol overrides in LiFi token metadata. |
| packages/app/src/app/api/crosschain-transfers/lifi/tokens/registry.ts | Registers PYUSD in the LiFi token registry and normalizes PYUSD metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (token.destinationBalanceAddress) { | ||
| return ( | ||
| erc20DestinationChainBalances[token.destinationBalanceAddress.toLowerCase()] ?? constants.Zero | ||
| ); | ||
| } |
There was a problem hiding this comment.
destinationBalanceAddress is applied before the deposit-mode l2Address branch. For tokens that set destinationBalanceAddress to an L1 address (e.g. PYUSD withdrawal metadata), this makes deposit-mode destination balance lookups use an L1 address on the L2 chain and return 0. Consider only using destinationBalanceAddress when !isDepositMode (or otherwise gating it so deposit-mode continues to use l2Address).
| const isWaitingForSelectedErc20Token = !!tokenFromSearchParams && !selectedToken; | ||
|
|
||
| const lifiParameters = { | ||
| enabled: eligibleRouteTypes.includes('lifi'), // only fetch lifi routes if lifi is eligible | ||
| enabled: eligibleRouteTypes.includes('lifi') && !isWaitingForSelectedErc20Token, // only fetch LiFi routes once the ERC20 selection has resolved | ||
| fromAddress: address, |
There was a problem hiding this comment.
isWaitingForSelectedErc20Token treats any truthy tokenFromSearchParams with selectedToken === null as “waiting”. But the token query param can legitimately be AddressZero for native-token flows (e.g. via sanitizeNullSelectedToken), where selectedToken is expected to stay null. This will disable LiFi route fetching indefinitely for those cases; consider excluding AddressZero/native-token representations from the “waiting for ERC20” condition.
a6981ed to
3102ba9
Compare
| const normalizedTokenAddress = tokenFromSearchParams?.toLowerCase(); | ||
| const ethereumPyusdAddress = CommonAddress.Ethereum.PYUSD.toLowerCase(); | ||
| const listSelectedToken = normalizedTokenAddress | ||
| ? tokensFromLists[normalizedTokenAddress] | ||
| : undefined; | ||
| const userSelectedToken = normalizedTokenAddress | ||
| ? tokensFromUser[normalizedTokenAddress] | ||
| : undefined; | ||
| const pyusdListEntry = listSelectedToken || tokensFromLists[ethereumPyusdAddress]; | ||
| const stablePyusdListIdsRef = useRef<Set<string> | undefined>(undefined); | ||
|
|
||
| if (!areSetsEqual(stablePyusdListIdsRef.current, pyusdListEntry?.listIds)) { | ||
| stablePyusdListIdsRef.current = pyusdListEntry?.listIds | ||
| ? new Set(pyusdListEntry.listIds) | ||
| : undefined; | ||
| } | ||
|
|
||
| const stablePyusdListIds = stablePyusdListIdsRef.current; | ||
| const selectedPyusdToken = useMemo(() => { | ||
| return getSelectedPyusdToken({ | ||
| tokenAddress: tokenFromSearchParams, | ||
| isDepositMode, | ||
| sourceChainId: networks.sourceChain.id, | ||
| destinationChainId: networks.destinationChain.id, | ||
| pyusdPriceUSD: pyusdListEntry?.priceUSD, | ||
| pyusdL2Address: pyusdListEntry?.l2Address, | ||
| pyusdListIds: stablePyusdListIds, | ||
| }); | ||
| }, [ | ||
| isDepositMode, | ||
| networks.destinationChain.id, | ||
| networks.sourceChain.id, | ||
| pyusdListEntry?.l2Address, | ||
| pyusdListEntry?.priceUSD, | ||
| stablePyusdListIds, | ||
| tokenFromSearchParams, | ||
| ]); |
There was a problem hiding this comment.
This changes enforce the ref to be the same over time, many hook depends on selectedToken and we don't want to cause hooks to reexecute all the time
7527071 to
5ae886b
Compare
54b5354 to
1487a42
Compare
| const [networks, setNetworks] = useNetworks(); | ||
| const [, setSelectedToken] = useSelectedToken(); | ||
| const isSwapTransfer = useIsSwapTransfer(); | ||
| const nextSelectedTokenAddress = useNetworkSwitchSelectedTokenAddress(); |
There was a problem hiding this comment.
the name nextSelectedTokenAddress is hard to understand
| function parsePriceUSD(price: unknown): number | undefined { | ||
| const parsedPrice = Number(price); | ||
|
|
||
| if (!Number.isFinite(parsedPrice) || parsedPrice <= 0) { |
There was a problem hiding this comment.
do we need to use isFinite here?
| }); | ||
| }); | ||
|
|
||
| describe('shouldBlockWithdrawOnlyDeposit', () => { |
There was a problem hiding this comment.
also add tests for when isSelectedTokenWithdrawOnlyLoading is true?
| ].includes(token.toLowerCase()); | ||
| } | ||
|
|
||
| export function shouldBlockWithdrawOnlyDeposit({ |
There was a problem hiding this comment.
naming is hard and this one is a confusing name because withdraw-only and deposit are contradictory.
it should just be called shouldBlockDeposit
| !destinationToken || | ||
| addressesEqual(destinationToken, selectedToken?.address) || | ||
| addressesEqual(destinationToken, selectedToken?.importLookupAddress) || | ||
| addressesEqual(destinationToken, selectedToken?.l2Address) |
There was a problem hiding this comment.
is there a case when the destinationToken and selectedToken?.l2Address are the same and we want it to be a Swap transfer? maybe only when we enable cross-chain swaps?
just want to make sure this doesn't cause bugs for other tokens
There was a problem hiding this comment.
For cross-chain swaps, I think there can be an early return if source+destination chains don't match and we have both source+destination addresses non-empty.
My concern is if we really want this condition at all since it has served us well without it till now?
There was a problem hiding this comment.
let's add some tests for non PYUSD tokens like ETH<->USDC, ETH<->ARB, USDC<->USDT etc to ensure it works well for other tokens
| // The token's L1 address is already on the list. | ||
| // LiFi bridgeInfo carries the canonical parent-chain branding, so prefer it. | ||
| acc[addressOnL1]!.name = parentTokenName; | ||
| acc[addressOnL1]!.symbol = parentTokenSymbol; | ||
| acc[addressOnL1]!.decimals = parentTokenDecimals; | ||
| acc[addressOnL1]!.logoURI = parentTokenLogoURI; |
There was a problem hiding this comment.
doesn't this change it for all tokens and not only LiFi?
maybe add a condition like if (tokenList.bridgeTokenListId === LIFI_TRANSFER_LIST_ID) {?
There was a problem hiding this comment.
please check this override change because it doesn't only change PYUSD
for example with ApeChain:
Prod:
https://portal.arbitrum.io/bridge?amount=0.1&destinationChain=apechain&destinationToken=0x0000000000000000000000000000000000000000&sanitized=true&sourceChain=arbitrum-one&token=0x0000000000000000000000000000000000000000

This PR: 😱 the "selected token" i guess is APE????
https://arbitrum-portal-git-add-pyusd-support-offchain-labs.vercel.app/bridge?amount=0.1&destinationChain=apechain&destinationToken=0x0000000000000000000000000000000000000000&sanitized=true&sourceChain=arbitrum-one&token=0x0000000000000000000000000000000000000000

There was a problem hiding this comment.
When testing this, we realised that there's another issue (already on production), the link redirect to APE to WETH (and it update to ETH to WETH when we select the source token). We will fix that later once integration tests are merged
| }; | ||
| } | ||
|
|
||
| if (isTokenArbitrumOnePyusdCanonical(tokenAddress)) { |
There was a problem hiding this comment.
should also check for isDepositMode?
1487a42 to
55077cb
Compare
b53798a to
271e895
Compare
| 'sUSDC': '0x940098b108fb7d0a7e374f6eded7760787464609', | ||
| 'sUSDe': '0x211cc4dd073734da055fbf44a2b4667d5e5fe5d2', | ||
| 'PYUSDCanonical': '0x327006c8712fe0abdbbd55b7999db39b0967342e', | ||
| 'PYUSDOFT': '0x46850ad61c2b7d64d08c9c754f45254596696984', |
There was a problem hiding this comment.
I'd name these as PYUSDCanonical and PYUSD.
As PYUSD (OFT version) is the official token, it should be THE representation of the token in code without any suffixes.
| } | ||
|
|
||
| if (isTokenArbitrumOnePyusdOft(token.address) && chainId === ChainId.ArbitrumOne) { | ||
| return withOverriddenNameAndSymbol(token, { symbol: 'PYUSD', name: 'PayPal USD OFT' }); |
There was a problem hiding this comment.
Why are we calling it "PayPal USD OFT" for end users? It should be called PayPal USD since it's the official version on ArbOne.
| if (isTokenArbitrumOnePyusdOft(token.address) && chainId === ChainId.ArbitrumOne) { | ||
| return { | ||
| ...token, | ||
| logoURI: ETHEREUM_PYUSD_LOGO_URI, |
There was a problem hiding this comment.
Shouldn't this be ARBITRUM_ONE_PYUSD_OFT_LOGO_URI?
| } | ||
|
|
||
| // Canonical PYUSD is only supported through canonical withdraw | ||
| if (isTokenArbitrumOnePyusdCanonical(fromToken)) { |
There was a problem hiding this comment.
Should also add a check for sourceChainId being Arbitrum one? extremely rare case but if a valid token has the same address as ArbitrumOnePyusdCanonical on another chain, Lifi would be blocked for it.
| return undefined; | ||
| } | ||
|
|
||
| return token.l2Address ?? token.address; |
There was a problem hiding this comment.
Wouldn't falling back to token.address be incorrect here? Ideally child chain address is ONLY token.l2Address.
| isL2Native?: boolean; | ||
| priceUSD?: number; | ||
| importLookupAddress?: string; | ||
| sourceBalanceAddress?: string; |
There was a problem hiding this comment.
This one is questionable. In every PYUSD variant, sourceBalanceAddress === address:
getEthereumPyusdToken(): address = 0x6c3e..., sourceBalanceAddress = 0x6c3e...
getArbitrumOnePyusdOftToken(): address = 0x4685..., sourceBalanceAddress = 0x4685...
getArbitrumOnePyusdCanonicalToken(): address = 0x3270..., sourceBalanceAddress = 0x3270...
| } | ||
|
|
||
| // In withdrawal mode: destination = parent chain, use parent address | ||
| if (token.destinationBalanceAddress) { |
There was a problem hiding this comment.
Pls move this code snippet above the comment present in L73, as it relates to different code.
| !destinationToken || | ||
| addressesEqual(destinationToken, selectedToken?.address) || | ||
| addressesEqual(destinationToken, selectedToken?.importLookupAddress) || | ||
| addressesEqual(destinationToken, selectedToken?.l2Address) |
There was a problem hiding this comment.
For cross-chain swaps, I think there can be an early return if source+destination chains don't match and we have both source+destination addresses non-empty.
My concern is if we really want this condition at all since it has served us well without it till now?
d77b9e4 to
edd1564
Compare
8671c5c to
25a5061
Compare
7087bec to
6d56cb2
Compare
6d56cb2 to
6378a3e
Compare
3912738 to
7f94679
Compare
Summary
Steps to test
Eth to ArbOne
Search for PYUSD:
Select PYUSD:
Switch network (source: Arb1) and back (source: Ethereum again).
Reload:
Arb1 to Eth (OFT)
Search for PYUSD:
Select PYUSD (OFT version):
Switch network (source: Ethereum) and back (source: Arb1 again)
Reload:
Arb1 to Eth (Canonical)
Search for PYUSD:
Select PYUSD (Canonical version, blue logo):
Switch network (source: Ethereum) and back (source: Arb1 again)
Reload: