-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(zk_toolbox): Make chain create independent from ecosystem #3210
base: matias/refactor-chain-commands
Are you sure you want to change the base?
feat(zk_toolbox): Make chain create independent from ecosystem #3210
Conversation
…/zksync-era into matias/separate-chain-create
…bs/zksync-era into matias/separate-chain-create
…bs/zksync-era into matias/separate-chain-create
…bs/zksync-era into matias/separate-chain-create
} else { | ||
(L2ChainId::from(args.chain_id), None) | ||
}; | ||
let internal_id = ecosystem_config.list_of_chains().len() as u32; | ||
|
||
let internal_id = args.number_of_chains + 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's always zero in a case we don't have ecosystem config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally looks good, please have a look closely to method visibility.
Looks like we have a lot of unnecessary public methods
…bs/zksync-era into matias/separate-chain-create
…bs/zksync-era into matias/separate-chain-create
let l1_network = l1_network.unwrap_or_else(|| { | ||
self.l1_network.unwrap_or_else(|| { | ||
PromptSelect::new(MSG_L1_NETWORK_PROMPT, L1Network::iter()).ask() | ||
}) | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks a bit odd. Why do we have two places for l1 network?
What ❔
Why ❔
Checklist
zkstack dev fmt
andzkstack dev lint
.