manage_validator_keys.sh (7)

31-43: Improve version-check error handling and cleanup
The version check is solid but can be hardened:

• Use rm -f to avoid failures if the old binary is missing.
• Handle a failing deposit --version gracefully to avoid an unbound variable.
• Consolidate error output to stderr.

 function downloadEthstakerDepositCli(){
-    if [ -d "$DEPOSIT_CLI_PATH" ]; then
-        edc_version_installed=$("$DEPOSIT_CLI_PATH"/deposit --version)
+    if [[ -d "$DEPOSIT_CLI_PATH" ]]; then
+        edc_version_installed=$("$DEPOSIT_CLI_PATH"/deposit --version 2>/dev/null || echo "")
         if [[ "${edc_version_installed}" =~ .*"${edc_version}".* ]]; then
             echo "ethstaker_deposit-cli is up-to-date"
             return
         else
-            rm "$DEPOSIT_CLI_PATH"/deposit
+            rm -f "${DEPOSIT_CLI_PATH}/deposit"
             echo "ethstaker_deposit-cli update available: ${edc_version_installed}"
             echo "Updating to v${edc_version}"
         fi
     fi

---

71-71: Simplify argument parsing and locality
The &&/|| chain for ARGUMENT is terse but can be hard to follow. An explicit local with a default and an if block is clearer:

-function generateNewValidatorKeys(){
-    [[ $# -eq 1 ]] && local ARGUMENT=$1 && checkLido "$1" || ARGUMENT="default"
+function generateNewValidatorKeys(){
+    local ARGUMENT="default"
+    if [[ $# -eq 1 ]]; then
+        ARGUMENT=$1
+        checkLido "$ARGUMENT"
+    fi

---

93-93: Handle cd failures explicitly
Avoid || true so you catch misconfigurations. If the directory is wrong, bail out:

-    cd "$DEPOSIT_CLI_PATH" || true
+    cd "$DEPOSIT_CLI_PATH" || { echo "ERROR: Cannot cd to $DEPOSIT_CLI_PATH" >&2; exit 1; }

---

166-166: Align importValidatorKeys argument logic
As with generateNewValidatorKeys, prefer initializing ARGUMENT with a default via local ARGUMENT="default" and then overriding inside an if block instead of chaining &&/||.

---

186-186: Refine addRestoreValidatorKeys plugin detection
Same pattern: initialize ARGUMENT locally, then call checkLido if $# -eq 1. This keeps consistency across commands.

---

207-207: Validate directory change in addRestoreValidatorKeys
Remove || true and explicitly error out if cd fails:

-    cd "$DEPOSIT_CLI_PATH" || true
+    cd "$DEPOSIT_CLI_PATH" || { echo "ERROR: $DEPOSIT_CLI_PATH not found" >&2; exit 1; }

---

443-482: Enhance queue query reliability and portability
The refactored queryEntryQueue is a strong improvement. A few further enhancements:

1. Install bc alongside jq/curl since it’s used for churn calculations.
2. Declare all function-local variables (json, entering, wait_time, etc.) with local.
3. Implement retry logic around the curl call to handle transient API failures.
4. Consider leveraging bash arithmetic if sub-minute precision suffices, to remove the bc dependency.

Example:

 function queryEntryQueue(){
-    #Variables
+    # Variables
+    local json entering wait_time retries=3

     BEACONCHAIN_VALIDATOR_QUEUE_API_URL="/api/v1/validators/queue"
@@
-    if ! json=$(curl -fsSL "${BEACONCHAIN_URLS["${NETWORK}"]}"${BEACONCHAIN_VALIDATOR_QUEUE_API_URL}); then
+    # Retry on failure
+    for ((i=1; i<=retries; i++)); do
+        if json=$(curl -fsSL "${BEACONCHAIN_URLS["$NETWORK"]}$BEACONCHAIN_VALIDATOR_QUEUE_API_URL"); then
+            break
+        elif [[ $i -eq retries ]]; then
+            echo "ERROR: Entry Queue API failed after $retries attempts." >&2
+            return 1
+        fi
+        sleep $((i * 2))
+    done

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro