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

improve(unwrapWeth): load WETH addresses from constants #1840

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

nicholaspai
Copy link
Member

@nicholaspai nicholaspai commented Sep 26, 2024

Simple refactor to re-use constants

@nicholaspai nicholaspai changed the title npai/redstone improve(unwrapWeth): Add Redstone to unwrap weth script Sep 26, 2024
@@ -23,6 +23,7 @@ const WETH_ADDRESSES = {
288: "0xDeadDeAddeAddEAddeadDEaDDEAdDeaDDeAD0000",
324: "0x5AEa5775959fBC2557Cc8789bC1bf90A239D9a91",
8453: "0x4200000000000000000000000000000000000006",
7777777: "0x4200000000000000000000000000000000000006",
Copy link
Contributor

@pxrl pxrl Sep 26, 2024

Choose a reason for hiding this comment

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

Should we just import { TOKEN_SYMBOLS_MAP } from ../src/utils and dereference TOKEN_SYMBOLS_MAP.WETH.addresses[chainId]? Then we get these addresses for free.

@nicholaspai nicholaspai changed the title improve(unwrapWeth): Add Redstone to unwrap weth script improve(unwrapWeth): load WETH addresses from constants Sep 27, 2024
Copy link
Contributor

@james-a-morris james-a-morris left a comment

Choose a reason for hiding this comment

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

one nit

@@ -36,10 +34,10 @@ export async function run(): Promise<void> {
const signerAddr = await baseSigner.getAddress();
const chainId = Number(args.chainId);
const connectedSigner = baseSigner.connect(await getProvider(chainId));
if (!isKeyOf(chainId, WETH_ADDRESSES)) {
if (!TOKEN_SYMBOLS_MAP.WETH.addresses[chainId]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we convert this to an assert( )

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.

3 participants