feat: more idiomatic CLI interface for mock network #6897
Conversation
| client_start_height: Option<BlockHeight>, | ||
| /// If specified, the binary will set up client home dir before starting the | ||
| /// client node so head of the client chain will be the specified height | ||
| /// when the client starts. The given height must be the last block in an | ||
| /// epoch. | ||
| #[clap(long, default_value = "0")] | ||
| client_height: BlockHeight, | ||
| /// The height at which the mock network starts. The client would have to | ||
| /// catch up to this height before participating in new block production. | ||
| /// | ||
| /// Defaults to the largest height in history. | ||
| #[clap(long)] | ||
| network_height: Option<BlockHeight>, | ||
| /// Shortcut to set both `--client-height` and `--network-height`. | ||
| #[clap(long, conflicts_with_all(&["client-height", "network-height"]))] | ||
| start_height: Option<BlockHeight>, | ||
| /// Target height that the client should sync to before stopping. If not specified, | ||
| /// use the height of the last block in chain history | ||
| #[clap(short = 'h', long)] | ||
| #[clap(long)] |
There was a problem hiding this comment.
Note: I intentionally left out short options from here. They are an anti-pattern: for rarely used utilities, the reader can benefit a lot from seeing --target-height 92 rather than -h 92
tools/mock_node/src/setup.rs
Outdated
| @@ -340,8 +305,8 @@ pub fn setup_mock_node( | |||
| } | |||
|
|
|||
| #[cfg(test)] | |||
| mod test { | |||
| use crate::setup::{setup_mock_node, MockNetworkMode}; | |||
| mod tests { | |||
There was a problem hiding this comment.
@marcelo-gonzalez the idiomatic name here is tests, and one generally doesn't have to type it manually. There's tmod built-in snippet in both Clion and rust-analyzer which auto-comples to the whole stanza with cfg
tools/mock_node/README.md
Outdated
| your node and put the mock node's home dir there. See https://cloud.google.com/compute/docs/disks/add-persistent-disk | ||
| for how to attach and mount a new disk to an existing GCP node. | ||
| where the `node0` directory contains some pre-generated chain history in storage. | ||
| Ask [in Zulip](https://near.zulipchat.com/#narrow/stream/295558-pagoda.2Fcore) where to get example histories. |
There was a problem hiding this comment.
could also mention the gzipped archives in benches/ :)
There was a problem hiding this comment.
Right, discovered those only after submitting the PR
tools/mock_node/README.md
Outdated
| responding to the client's network requests by reading from a pre-generated chain history | ||
| in storage. | ||
| ```console | ||
| $ cargo run --release -p mock-net -- ~/.near/localnet/node0 |
There was a problem hiding this comment.
s/mock-net/mock-node/ and same for the other one below. also would be good to update the example commands in benches/README.md (s/mock_node/mock-node/)
matklad
force-pushed
the
m/mock-cli
branch
2 times, most recently
from
32b45f2
to
a2f33a0
Compare
tools/mock_node/README.md
Outdated
| Once you have the source dir already set up, you can run the command without `-s`, | ||
| ```bash | ||
| start_mock_node ~/.near ~/mock_node_home_dir -h 60926000 --mode "no_new_blocks" | ||
| Once you have the source dir already set up, you can run the command without `--`, |
There was a problem hiding this comment.
really? so you could run it without passing any options?
I guess you still have to pass both directories, right?
There was a problem hiding this comment.
Nope, it seems I just got distracted here, good catch!
tools/mock_node/README.md
Outdated
| of from genesis block. The following comment replays mainnet history from block height 60925880 to block height 60925900. | ||
|
|
||
| ```console | ||
| $ cargo r -r -p mock-node -- ~/.near ~/mock_node_home_dir --client-height 60925880 --target-height 60925900 |
There was a problem hiding this comment.
nit: I wonder what can we do so that people don't mix up the order of home_dirs
There was a problem hiding this comment.
We should just not have the second working dir. The binary should transparently cache the stuff in target/mock-node/<block-hash>.
Should I create an issue for that? (As in, do we view this tool as a beginning of a long-term support infra, or is it just a temporary exploration?)
There was a problem hiding this comment.
let's create an issue for that.
the target/mock-node/XX might work locally - but we will be using this tool also on GCP (where usually SSD is mapped under a different dir)
There was a problem hiding this comment.
- use kebab-case rather than snake-case - replace mode argument with custom syntax with a pair of * --client-height * --network-height - use convention-over-configuration for the binary name
These are mostly inconsequential cleanups which I am not feeling particularly strongly about specifically. I do feel moderately strongly about the overall point of sticking to conventions, defaults, and trying to leverage existing functionality instead of implementing custom logic :)