-
Notifications
You must be signed in to change notification settings - Fork 160
[#985] Create a ZIP Draft to document Regtest mode #986
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
base: main
Are you sure you want to change the base?
Conversation
8a33bbb
to
e4b17fc
Compare
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.
nit: it's difficult to read this on a wide monitor, please hard-wrap lines.
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.
Partial review; I have not yet reviewed beyond the section Behavior for ZIP-200 Network Upgrade Mechanism
.
In general, specification language needs to be somewhat more precise than most of what is presented in this draft. In particular, the affected components and actors should be clearly identified in each portion of the specification, and particular care should be taken with respect to any conditionals. Focus on what has to happen more than the actor in each section.
Thank you very much for your comments @nuttycom. I've included your feedback and in-line suggestions. |
have a consensus node on a local network with a private state that a developer | ||
can control in order to reproduce certain situations deterministically for | ||
testing purposes. Regtest, as it was implemented on Zcashd, is similar to a | ||
“testnet” but without miners and remote peers. |
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.
If I recall correctly many of the qa/rpc-tests in zcash acually have more than local "peer".. I wonder whether the "remote-vs-local" distinction here, might be somewhat orthogonal to the purpose of regtest mode, which though frequently used with multiple nodes in the same ip:tcp space, could be used more "remotely" with the same regtest properties.
|
||
## Motivation | ||
It is necessary to define Regtest mode so that different implementations of Zcash | ||
Full Nodes provide the same capabilities so that testing infrastructure can be |
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.
Again it seems to me that "regtest" has utility for relatively low-capability "nodes". That is, less than "full".
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.
what is a "low-capability" node?
specifically designed to avoid requirement 1 in Testnet and Mainnet | ||
1. Developers MUST specify the activation heights of the different Network | ||
Upgrades on launch via parameters or configuration file. | ||
1. Remote Peer-to-Peer connections MUST NOT be allowed. All peers MUST be local |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
||
## Requirements | ||
1. Nodes launched in Regtest mode MUST be able to generate deterministic sequences | ||
of blocks and transactions. |
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.
How about:
"Finalizers, launched in regtest mode, MUST be able to efficiently generate deterministic sequences of valid blocks of transactions."
It seems like efficiency is a requirement. I also wonder if "of transactions" might be redundant, or conversely if "blocks of" might be incompatible with future architectures.
``` | ||
ZIP: Unassigned | ||
Title: Regtest: Definition of a Local Consensus test mode for Zcash Full-Nodes | ||
Owners: ZIP Editors |
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.
Owners must be specific named people who have each explicitly consented to be an Owner. I haven't, so this can't just be replaced by the current ZIP Editors (nor do the ZIP Editors have time or capacity to be assigned the role of maintaining a ZIP by others). Normally the Owners of the ZIP are the people who wrote it.
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.
NACK due to a ZIP 0 violation in the Owners field.
License: MIT | ||
Pull-Request: <https://github.com/zcash/zips/pull/986> | ||
``` | ||
# Regtest: Definition of a Local Consensus test mode for Zcash Full-Nodes |
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.
Delete this and unindent every other heading by one level.
(I'm aware that semantically, this is the title which is usually at the first heading level in Markdown. However, no other Markdown ZIP uses that convention, and in practice it's necessary to have all heading levels available. The stylesheet is also designed under the assumption that major sections are at the first heading level.)
of blocks and transactions can be generated as defined in requirement 1. | ||
1. Changes on Human-Readable Parts (HRP) of address string encodings (if applicable) | ||
are defined with their respective prefix to signal a regtest variant. | ||
1. Regtest functionality SHOULD allow developers to ensure test coverage of their |
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.
1. Regtest functionality SHOULD allow developers to ensure test coverage of their | |
1. Regtest functionality should allow developers to ensure test coverage of their |
This is not a conformance requirement.
closes #985