client-cmds/nlean: use fork digest in gossip topic (leanSpec #622)#165
client-cmds/nlean: use fork digest in gossip topic (leanSpec #622)#165
Conversation
There was a problem hiding this comment.
Pull request overview
Updates lean-quickstart devnet tooling to align nlean’s gossip topic network identifier with the fork-digest format and refreshes related devnet4/ansible/subnet-gen workflows.
Changes:
- Switch nlean’s default
--networkfromdevnet0to fork digest12345678and bump multiple client Docker defaults to devnet4 tags. - Rework subnet expansion to write a single
validator-config-expanded.yamland improve aggregator selection behavior across subnets. - Improve Ansible workflows by generating a deduped prepare inventory and making validator-config path handling more robust.
Reviewed changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
spin-node.sh |
Enforces --network for Ansible, changes subnet expansion output to validator-config-expanded.yaml, refines aggregator selection. |
run-ansible.sh |
Uses prepare inventory for prepare, updates inventory mutation for SSH settings, passes absolute validator-config path to Ansible. |
parse-vc.sh |
Adds dual-key (proposer/attester) hash-sig key layout support and exports additional env vars. |
parse-env.sh |
Removes defaulting of networkName (now handled in spin-node.sh). |
local-devnet/genesis/validator-config.yaml |
Makes attestation_committee_count explicit and updates inline comments. |
generate-subnet-config.py |
Adds shared-host mode (duplicate IP templates) and refactors replicate-mode expansion. |
generate-genesis.sh |
Updates hash-sig-cli image to devnet4 and adds dual-key manifest handling + committee count emission. |
generate-ansible-inventory.sh |
Generates hosts-prepare.yml (one host per unique IP) in addition to hosts.yml. |
docs/adding-a-new-client.md |
Documents devnet4 dual-key genesis outputs and updated config fields. |
client-cmds/zeam-cmd.sh |
Bumps default zeam Docker image tag to devnet4. |
client-cmds/ream-cmd.sh |
Bumps default ream Docker image tag to latest-devnet4. |
client-cmds/qlean-cmd.sh |
Bumps qlean images to devnet-4 and adds dual-key hash-sig path selection for Docker. |
client-cmds/peam-cmd.sh |
Bumps default peam Docker image tag to devnet4. |
client-cmds/nlean-cmd.sh |
Sets default nlean network name to 12345678 and bumps image to devnet4. |
client-cmds/grandine-cmd.sh |
Bumps default grandine Docker image tag to devnet-4. |
client-cmds/gean-cmd.sh |
Bumps default gean Docker image tag to devnet4. |
client-cmds/ethlambda-cmd.sh |
Bumps default ethlambda Docker image tag to devnet4. |
ansible/roles/zeam/tasks/main.yml |
Updates fallback default image to devnet4. |
ansible/roles/zeam/defaults/main.yml |
Updates fallback default image to devnet4. |
ansible/roles/ream/tasks/main.yml |
Updates fallback default image to latest-devnet4. |
ansible/roles/ream/defaults/main.yml |
Updates fallback default image to latest-devnet4. |
ansible/roles/qlean/tasks/main.yml |
Updates fallback default image to devnet-4-amd64. |
ansible/roles/qlean/defaults/main.yml |
Updates fallback default image to devnet-4-amd64. |
ansible/roles/peam/tasks/main.yml |
Updates fallback default image to devnet4. |
ansible/roles/peam/defaults/main.yml |
Updates fallback default image to devnet4. |
ansible/roles/nlean/tasks/main.yml |
Updates fallback default image to devnet4. |
ansible/roles/nlean/defaults/main.yml |
Updates fallback default image to devnet4 (network fallback still devnet0). |
ansible/roles/grandine/tasks/main.yml |
Updates fallback default image to devnet-4. |
ansible/roles/grandine/defaults/main.yml |
Updates fallback default image to devnet-4. |
ansible/roles/genesis/tasks/main.yml |
Adds ATTESTATION_COMMITTEE_COUNT to generated config (currently hardcoded). |
ansible/roles/gean/tasks/main.yml |
Updates fallback default image to devnet4. |
ansible/roles/gean/defaults/main.yml |
Updates fallback default image to devnet4. |
ansible/roles/ethlambda/tasks/main.yml |
Updates fallback default image to devnet4. |
ansible/roles/ethlambda/defaults/main.yml |
Updates fallback default image to devnet4. |
ansible/playbooks/prepare.yml |
Documents deduped prepare inventory and updated expanded-config naming. |
ansible-devnet/genesis/validator-config.yaml |
Updates ansible-devnet validator config for multi-subnet/shared-host layout. |
ansible-devnet/genesis/validator-config-expanded.yaml |
Adds a generated reference expanded config (new file). |
ansible-devnet/genesis/test-validator-config.yaml |
Adds a test template for subnet expansion (new file). |
TESTING_DEVNET3.md |
Updates committee-count guidance and clarifies behavior expectations. |
README.md |
Updates docs for prepare constraints, subnet expansion modes, and devnet4 dual-key genesis format. |
.gitignore |
Ignores hosts-prepare.yml and adds logs/ + monitoring/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Generate config.yaml | ||
| copy: | ||
| content: | | ||
| # Genesis Settings | ||
| GENESIS_TIME: {{ genesis_time }} | ||
| # Chain Settings | ||
| ATTESTATION_COMMITTEE_COUNT: 1 | ||
| # Validator Settings | ||
| VALIDATOR_COUNT: {{ total_validators }} | ||
| dest: "{{ genesis_dir }}/config.yaml" |
There was a problem hiding this comment.
ATTESTATION_COMMITTEE_COUNT is hardcoded to 1 here, which will generate a config.yaml that disagrees with validator-config templates using config.attestation_committee_count (e.g. when --subnets N sets it to N). Read the value from validator_config_file (defaulting to 1 when absent/null) and write that into config.yaml instead of a constant.
| nlean_docker_image: "ghcr.io/nleaneth/nlean:latest" | ||
| nlean_docker_image: "ghcr.io/nleaneth/nlean:devnet4" | ||
| deployment_mode: docker # docker or binary | ||
| nlean_network_name: "devnet0" |
There was a problem hiding this comment.
nlean_network_name fallback default is still devnet0, but client-cmds/nlean-cmd.sh now defaults to the fork digest 12345678. Update this role default to 12345678 to keep Ansible deployments consistent if extraction from client-cmds fails or is bypassed.
| nlean_network_name: "devnet0" | |
| nlean_network_name: "12345678" |
| # Pass the absolute path of the active validator config. ansible-playbook runs | ||
| # with cwd ansible/; lookup('file', ...) treats relative paths as relative to | ||
| # that directory, so a path like ansible-devnet/genesis/foo.yaml would break. | ||
| _local_vc_path="$validator_config_file" | ||
| if [[ "$_local_vc_path" != /* ]]; then | ||
| _local_vc_path="$scriptDir/$_local_vc_path" | ||
| fi | ||
| EXTRA_VARS="$EXTRA_VARS local_validator_config_path=$_local_vc_path" |
There was a problem hiding this comment.
local_validator_config_path is appended to EXTRA_VARS without shell/Ansible quoting. If the path contains spaces or characters interpreted by the shell, the resulting ansible-playbook ... -e "..." (executed via eval) can break or be unsafe. Prefer building the command as a bash array (no eval) and pass extra-vars via --extra-vars with proper quoting (or --extra-vars @file.json).
| # Append genesis_validators to config.yaml | ||
| if [ -s "$GENESIS_VALIDATORS_TMP" ]; then | ||
| # Append directly to config.yaml (simpler and more reliable than yq merge) | ||
| echo "" >> "$CONFIG_FILE" | ||
| echo "# Genesis Validator Pubkeys" >> "$CONFIG_FILE" | ||
| echo "# List of Genesis Validators' Public Keys (attestation + proposal)" >> "$CONFIG_FILE" | ||
| echo "GENESIS_VALIDATORS:" >> "$CONFIG_FILE" |
There was a problem hiding this comment.
The comment # List of Genesis Validators' Public Keys (attestation + proposal) is written unconditionally, but in legacy (single-key) mode this script appends GENESIS_VALIDATORS as a list of strings (not {attestation_pubkey, proposal_pubkey} objects). Either normalize legacy output to the same object schema, or make the header/comment and docs reflect that legacy manifests produce a different config.yaml schema.
| # --subnets-expanded file (e.g. validator-config-subnets-3.yaml) when | ||
| # validator-config-expanded.yaml (from --subnets N) when | ||
| # --subnets N was passed, or validator-config.yaml otherwise. | ||
| # validator_config_basename is injected by run-ansible.sh. |
There was a problem hiding this comment.
This comment says validator_config_basename is injected by run-ansible.sh, but that variable doesn't appear to be set/used anywhere (and isn't referenced in this playbook). Consider removing or correcting this note to avoid misleading future edits.
| # validator_config_basename is injected by run-ansible.sh. |
| expanded_config="${configDir}/validator-config-expanded.yaml" | ||
| [ "$dryRun" == "true" ] && echo "[DRY RUN] Generating subnet config preview (no deployment will occur)" | ||
| echo "Generating subnet config ($subnets subnet(s) per client) → $expanded_config" | ||
| echo "Generating subnet config ($subnets subnet(s)) → $expanded_config" |
There was a problem hiding this comment.
The PR description focuses on changing nlean’s default --network to the fork digest, but this PR also changes subnet expansion semantics (validator-config-expanded.yaml), aggregator selection logic, Ansible inventory generation (adds hosts-prepare.yml), and hash-sig dual-key handling / devnet4 image tags. Please update the PR description (and test plan) to reflect these additional behavioral changes so reviewers know what to validate.
5e08ec2 to
497150a
Compare
Change nlean's default --network value from "devnet0" to "12345678"
to match the fork digest gossipsub topic format defined in leanSpec
PR #622. All lean clients are adopting this change.
The --network flag is passed to nlean's GossipTopicProvider which
builds topics like /leanconsensus/{fork_digest}/block/ssz_snappy.
497150a to
48e3b71
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| nlean_repo="${NLEAN_REPO:-$scriptDir/../nlean}" | ||
| nlean_docker_image="${NLEAN_DOCKER_IMAGE:-ghcr.io/nleaneth/nlean:devnet3}" | ||
| nlean_network_name="${NLEAN_NETWORK_NAME:-devnet0}" | ||
| nlean_docker_image="${NLEAN_DOCKER_IMAGE:-ghcr.io/nleaneth/nlean:devnet4}" |
There was a problem hiding this comment.
Bumping the default image to ghcr.io/nleaneth/nlean:devnet4 while also switching this script to use --fork-digest creates an inconsistency with the repo’s Ansible nlean role, which extracts this default image from client-cmds/nlean-cmd.sh but still starts the container with --network (see ansible/roles/nlean/tasks/main.yml). If devnet4 no longer supports --network, Ansible deployments will fail. Consider either updating the Ansible role in the same PR to pass --fork-digest, or keeping the default image pinned to a version that still accepts --network until the Ansible change lands.
| nlean_docker_image="${NLEAN_DOCKER_IMAGE:-ghcr.io/nleaneth/nlean:devnet4}" | |
| nlean_docker_image="${NLEAN_DOCKER_IMAGE:-ghcr.io/nleaneth/nlean:devnet3}" |
| if [[ -z "${nlean_fork_digest// }" ]]; then | ||
| nlean_fork_digest="12345678" |
There was a problem hiding this comment.
nlean_fork_digest is taken from NLEAN_FORK_DIGEST and later interpolated into a command string that is executed via eval (see spin-node.sh). As-is, leading/trailing whitespace is preserved and non-hex characters (or shell metacharacters like ;) could break the CLI invocation or enable shell injection. Suggest trimming the value and validating it matches exactly 8 hex chars (e.g., ^[0-9a-fA-F]{8}$), failing fast with a clear error if invalid.
| if [[ -z "${nlean_fork_digest// }" ]]; then | |
| nlean_fork_digest="12345678" | |
| nlean_fork_digest="${nlean_fork_digest#${nlean_fork_digest%%[![:space:]]*}}" | |
| nlean_fork_digest="${nlean_fork_digest%${nlean_fork_digest##*[![:space:]]}}" | |
| if [[ -z "$nlean_fork_digest" ]]; then | |
| nlean_fork_digest="12345678" | |
| elif [[ ! "$nlean_fork_digest" =~ ^[0-9a-fA-F]{8}$ ]]; then | |
| echo "Error: NLEAN_FORK_DIGEST must be exactly 8 hexadecimal characters" >&2 | |
| exit 1 |
Summary
--networkfrom"devnet0"to"12345678"(fork digest)fork-digest-gossipsub-topicsbranchContext
Gossip topic format changing from:
to:
The
"12345678"is a dummy fork digest shared across all clients, will eventually be derived from fork version + genesis validators root.Test plan