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

Add balance command #79

Closed
wants to merge 4 commits into from

Conversation

jrchatruc
Copy link
Contributor

@jrchatruc jrchatruc commented Nov 17, 2023

What πŸ’»

  • Add npx zksync-cli wallet balance command to check ETH balance of the specified address

Why βœ‹

  • Seeing ETH balance might be useful during the development

Evidence πŸ“·

image

@jrchatruc jrchatruc requested a review from a team as a code owner November 17, 2023 15:47
Copy link
Member

@JackHamer09 JackHamer09 left a comment

Choose a reason for hiding this comment

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

Should we also ask for which token to get balance to? ETH or ERC20
And then if ERC20 is selected we also ask for address of the token.
Would be useful to have something like that :)
Not required for this PR tho..

src/commands/wallet/command.ts Outdated Show resolved Hide resolved
src/commands/wallet/balance.ts Outdated Show resolved Hide resolved
src/commands/wallet/balance.ts Outdated Show resolved Hide resolved
src/commands/wallet/balance.ts Outdated Show resolved Hide resolved
src/commands/wallet/balance.ts Outdated Show resolved Hide resolved
src/common/options.ts Outdated Show resolved Hide resolved
@jrchatruc
Copy link
Contributor Author

Should we also ask for which token to get balance to? ETH or ERC20 And then if ERC20 is selected we also ask for address of the token. Would be useful to have something like that :) Not required for this PR tho..

That would be very useful indeed, we'll add it on a subsquent PR!

@jrchatruc
Copy link
Contributor Author

Addressed all the commends above here aef66ef and here 2e88921. In regards to this

For now there is no L1 features.
So we should either not mention L1 and just say Manage zkSync wallet related features
or
Add possibility to choose L1 network in the list.

For now I just changed the description to not mention L1, we might add L1 features in the future.

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.

2 participants