Conversation
pinebit
changed the title
| exit 1 | ||
| fi | ||
|
|
||
| if [ ! -f .charon/cluster-lock.json ]; then |
There was a problem hiding this comment.
Did we add the option to specify custom directory for those in the edit commands? We currently support it for run, dkg, etc.
But I suppose those scripts are made only for the general use case?
There was a problem hiding this comment.
It's done via WORK_DIR env var, this is not documented though (it was added for testing mocking)..
|
|
||
| # VC container must be stopped before export (Lodestar locks the database while running) | ||
| if [ "$DRY_RUN" = false ]; then | ||
| if docker compose ps "$VC" 2>/dev/null | grep -q Up; then |
There was a problem hiding this comment.
This greps the first line, but... there might be some warnings that are first. i.e.: I get the following
➜ charon-distributed-validator-node git:(main) docker compose ps vc-teku
WARN[0000] The "MEVBOOST_RELAYS" variable is not set. Defaulting to a blank string.
NAME IMAGE COMMAND SERVICE CREATED STATUS PORTS
charon-distributed-validator-node-vc-teku-1 consensys/teku:25.11.0 "/opt/teku/bin/teku …" vc-teku 23 seconds ago Up 22 seconds 5051/tcp, 8008/tcp, 9000/tcp, 9000/udp
➜ charon-distributed-validator-node git:(main) docker compose ps vc-teku | grep -q Up
WARN[0000] The "MEVBOOST_RELAYS" variable is not set. Defaulting to a blank string.
There was a problem hiding this comment.
Oh, good finding, thank you, I will improve this.
| docker run --rm $DOCKER_FLAGS \ | ||
| -v "$REPO_ROOT/.charon:/opt/charon/.charon" \ | ||
| -v "$REPO_ROOT/$OUTPUT_DIR:/opt/charon/output" \ | ||
| "obolnetwork/charon:${CHARON_VERSION:-v1.9.0-rc3}" \ |
There was a problem hiding this comment.
Shall we put v1.9.0 here and merge this PR after v1.9.0 is merged?
There was a problem hiding this comment.
Yes I think I will wait for 1.9.0, then update the tag and merge.
| # Verify ceremony output | ||
| if [ -f "$OUTPUT_DIR/cluster-lock.json" ]; then | ||
| log_info "Ceremony completed successfully!" | ||
| NEW_VALIDATORS=$(jq '.distributed_validators | length' "$OUTPUT_DIR/cluster-lock.json" 2>/dev/null || echo "?") |
There was a problem hiding this comment.
Does the new lock contain only new validators? It's all validators, right?
Same for operators below.
There was a problem hiding this comment.
This comment belongs to add-operators script, not add-validators. But in any case, the new lock contains ALL validators.
| - .env file with NETWORK and VC variables set | ||
| - For --generate-enr: Docker installed | ||
| - For ceremony: .charon/charon-enr-private-key must exist | ||
| - For ceremony: Cluster-lock.json received from existing operators |
There was a problem hiding this comment.
| - For ceremony: Cluster-lock.json received from existing operators | |
| - For ceremony: cluster-lock.json received from existing operators |
| log_info "ENR private key generated" | ||
| fi | ||
|
|
||
| if [ -f .charon/charon-enr-private-key ]; then |
There was a problem hiding this comment.
Shouldn't we have an else for this condition as well? There should always be .charon/charon-enr-private-key, otherwise something went wrong?
There was a problem hiding this comment.
Yes, the private key is a prerequisite to all edits. Otherwise the ceremony would just fail. I will see what else to improve here.
| log_error "Invalid --num-validators: must be a positive integer" | ||
| exit 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
Shall we validate that the other 2 fields are 0x<HEX>? And the length of all 3 is the same?
There was a problem hiding this comment.
That is delegated to the charon command, which does much precise testing. Here, it is more checking for "presence".
| # Remove operator with custom threshold | ||
| ./scripts/edit/remove-operators/remaining-operator.sh \ | ||
| --operator-enrs-to-remove "enr:-..." \ | ||
| --new-threshold 3 |
There was a problem hiding this comment.
Not related to the scripts.
Thinking about it more now, probably it would've made sense to allow that in add and replace operators commands as well
There was a problem hiding this comment.
In replace, we only replace single operator by design, the threshold remains unchanged anyway.
In add-operators, we cannot increase threshold because of how re-DKG crypto math works. We can probably reduce the threshold while adding operators, but this looks an odd wish.
3 participants
Cluster Edit Scripts
script/edit/)scripts/edit/vc/)Testing
./scripts/edit/test/e2e_test.shthat runs complete E2E test using docker compose, by running real Lodestar VC, checking ASDB export/import and all edit commands in a sequence.