Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: allow Ether in selectedToken #1662

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brtkx
Copy link
Contributor

@brtkx brtkx commented May 9, 2024

Refactor unblocking further work on #1652.

Context

To make ETH transfers to a custom gas token work, we need to let users select ETH even if they are on a chain with custom gas token. E.g. user has Xai selected, then if they were to select ETH (once enabled) it would still set selectedToken to null, which wouldn't change the currency at all (because null refers to the native currency on child chain).

To go around that, we allow NativeCurrencyEther as an additional type for selectedToken. Then selectedToken can be either an ERC20 token, Ether or null - null still defaulting to the native currency on the child chain as is (which could still be ETH!).

Testing

Everything should work the same.

Copy link

vercel bot commented May 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
arbitrum-token-bridge ✅ Ready (Inspect) Visit Preview May 9, 2024 0:44am

@cla-bot cla-bot bot added the cla-signed label May 9, 2024
@@ -16,6 +16,8 @@ export type NativeCurrencyBase = {

export type NativeCurrencyEther = NativeCurrencyBase & {
isCustom: false
address?: undefined
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These make typescript happy where it would be required to check for isERC20Bridge all the time.

return typeof (token as ERC20BridgeToken).address !== 'undefined'
}

export function isEther(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be confusing because if token is null then it could still be ether, but this method would then return false.

I'm open to renaming it if it's too confusing

@brtkx brtkx marked this pull request as ready for review May 9, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant