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: store Orbit chains as json #1665

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

refactor: store Orbit chains as json #1665

wants to merge 35 commits into from

Conversation

brtkx
Copy link
Contributor

@brtkx brtkx commented May 9, 2024

A requirement for #1664. Need to store Orbit chains as json so that the workflow can easily update it.

@cla-bot cla-bot bot added the cla-signed label May 9, 2024
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 Aug 5, 2024 10:00am

Copy link
Contributor Author

@brtkx brtkx May 9, 2024

Choose a reason for hiding this comment

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

This file is the removed orbitChainsList.ts file (and renamed) and Orbit chains are removed from here. These chains are now part of json configs.

@fionnachan fionnachan self-requested a review May 9, 2024 17:00
@brtkx brtkx marked this pull request as ready for review May 21, 2024 16:24
@brtkx brtkx marked this pull request as draft May 21, 2024 16:26
@brtkx brtkx marked this pull request as ready for review May 21, 2024 17:16
@brtkx brtkx marked this pull request as draft May 24, 2024 10:43
@@ -0,0 +1,304 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please double check that I didn't miss an Orbit chain while migrating, because I was resolving some conflicts too.

Also, pls compare old data against the new.

.refine(bool => bool === true, 'Invalid input, must be true')

function validateOrbitChain(chain: OrbitChainConfig) {
try {
Copy link
Member

Choose a reason for hiding this comment

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

now that we're validating this in prebuild, we want it to throw when there's an error, so let's remove try/catch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we throw in the catch block, we just want to show info about the chain name/ID first

typeof getOrbitChains({
mainnet: true,
testnet: false
}).find(chain => chain.chainId === chainId) !== '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.

This is less performant than the previous implementation but I feel like we shouldn't export both orbit objects and a getter function and have less consistent codebase.

We work with small lists so using find here should be fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally our ArbitrumNetwork should have a testnet flag. That would simplify isNetwork itself as well

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.

2 participants