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

Lightnet sub-commands implementation (start/stop/status). #510

Merged
merged 31 commits into from
Nov 7, 2023

Conversation

shimkiv
Copy link
Member

@shimkiv shimkiv commented Oct 27, 2023

Corresponding RFC implementation with slightly extended API.
FYI: boolean options with default value of true can be negated by adding --no- prefix.
For example:

  • The --archive option of zk lightnet start sub-command is true by default. And to set it to false you can do zk lightnet start --no-archive.

Tests will be added later as part of the work on #509.


Closes #500


image

image

image

image

image

image

image

image

src/bin/index.js Outdated Show resolved Hide resolved
src/bin/index.js Outdated Show resolved Hide resolved
@shimkiv
Copy link
Member Author

shimkiv commented Oct 30, 2023

I have forgotten to check it on Windows and of course there are some issues. It should not impact review process though. Apologies for inconvenience.

shimkiv and others added 2 commits October 30, 2023 16:01
Co-authored-by: Barrie Byron <[email protected]>
Co-authored-by: Barrie Byron <[email protected]>
@shimkiv
Copy link
Member Author

shimkiv commented Oct 30, 2023

I have forgotten to check it on Windows and of course there are some issues. It should not impact review process though. Apologies for inconvenience.

Seems to be false alarm and the issue was caused by Docker Engine not being started. I'll add additional check.

src/bin/index.js Outdated
hidden: false,
choices: Constants.lightnetProofLevels,
default: 'none',
description: '"none" value will make the network faster.',
Copy link
Contributor

Choose a reason for hiding this comment

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

@shimkiv what is the disadvantage of none proof-level? none is default, but does not test other proofs, right? maybe more description to help user choose the appropriate proof level?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is the protocol configuration. One of the expensive parts of the protocol is creating blockchain snark proofs, so mocking that is important for faster everything. The proof_level Mina constant can be used to disable proving (set it to be check) or disable block calculation altogether (set it to none). None is default because for zkApps development with o1js we don't really care about protocol proofs, assuming that it is well tested. Sometimes though one might want to run zkApps against network that has closer to "real world" properties (say, some of the Nightly CI tests). That is when zk lightnet start --mode multi-node --type real --proof-level full might be useful.

hidden: false,
default: true,
description:
'Whether to pull the latest version of the Docker image from the Docker Hub.',
Copy link
Contributor

Choose a reason for hiding this comment

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

@shimkiv advantages and disadvantages? default is true, so when would we want to use false?

Copy link
Member Author

Choose a reason for hiding this comment

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

Docker images are updated each night and pulling the new image will takes resources and time should you start the network next day from scratch. It is useful to set false when you develop/test zkapp functionality using same version of o1js and Mina dependencies. The reason it is not set to false by default is because I was thinking to encourage people always use "latest and greatest" but not sure already.

src/bin/index.js Outdated Show resolved Hide resolved
hidden: false,
default: true,
description:
'Whether to save the Docker container processes logs to the host file system.',
Copy link
Contributor

Choose a reason for hiding this comment

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

default is true, what happens if false? where else would Docker container processes logs be saved? (or not saved at all?)

Copy link
Member Author

Choose a reason for hiding this comment

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

  • zk lightnet stop - saves logs from container to host file-system, stops and removes the container.
  • zk lightnet stop --no-save-logs - stops and removes the container, logs produced by different apps inside the container are gone. Useful for short lived networks when users will just check something zkapp related and don't care about logs.
  • zk lightnet stop --no-save-logs --no-clean-up - stops the container, it is not removed so container still can be accessible by users in order to get logs manually.

src/bin/index.js Outdated Show resolved Hide resolved
src/lib/constants.js Show resolved Hide resolved
src/lib/constants.js Outdated Show resolved Hide resolved
src/lib/lightnet.js Outdated Show resolved Hide resolved
src/lib/lightnet.js Outdated Show resolved Hide resolved
src/lib/lightnet.js Outdated Show resolved Hide resolved
src/lib/lightnet.test.js Outdated Show resolved Hide resolved
src/lib/lightnet.js Outdated Show resolved Hide resolved
@MartinMinkov
Copy link
Contributor

Seems to be false alarm and the issue was caused by Docker Engine not being started. I'll add additional check.

Ah, so Windows does work? Even the logs store mechanism? I'm surprised since it's using non friendly windows seperators in paths 😅

Copy link
Collaborator

@ymekuria ymekuria left a comment

Choose a reason for hiding this comment

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

Great work @shimkiv! I left some comments questions and suggestions. I may have some more comments after I use all the lightnet commands more thoroughly.

src/lib/lightnet.js Outdated Show resolved Hide resolved
src/lib/lightnet.js Outdated Show resolved Hide resolved
src/lib/lightnet.js Outdated Show resolved Hide resolved
src/lib/lightnet.js Outdated Show resolved Hide resolved
src/lib/lightnet.js Outdated Show resolved Hide resolved
src/lib/lightnet.js Show resolved Hide resolved
src/lib/lightnet.js Outdated Show resolved Hide resolved
src/lib/lightnet.js Show resolved Hide resolved
src/lib/lightnet.js Show resolved Hide resolved
src/lib/lightnet.js Outdated Show resolved Hide resolved
@shimkiv shimkiv requested a review from ymekuria October 31, 2023 12:06
src/lib/lightnet.js Show resolved Hide resolved
@shimkiv shimkiv merged commit 59b5b2d into main Nov 7, 2023
12 checks passed
@shimkiv shimkiv deleted the lightnet-start-stop-status branch November 7, 2023 08:22
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.

zk lightnet start/stop/status
4 participants