Cursor rule

1---
2description: Shell Script Best Practices
3globs: ["**/*.sh", "**/Makefile", "**/make/*.mk"]
4alwaysApply: false
5---
6 
7 
8# Shell Script Best Practices
9 
10## Overview
11 
12Rules for writing robust, maintainable shell scripts in go-crypto-wallet.
13These rules are based on code review feedback and industry best practices.
14 
15## Critical Shell Options
16 
17### Always Use Strict Mode
18 
19```bash
20set -euo pipefail
21```
22 
23**Explanation:**
24 
25- `-e`: Exit immediately if any command fails
26- `-u`: Treat unset variables as errors
27- `-o pipefail`: Fail if any command in a pipeline fails (not just the last one)
28 
29**Why pipefail matters:**
30 
31```bash
32# WITHOUT pipefail (BAD)
33set -eu
34cat nonexistent.txt | grep "pattern"  # Only grep's exit code matters
35 
36# WITH pipefail (GOOD)
37set -euo pipefail
38cat nonexistent.txt | grep "pattern"  # Fails immediately if cat fails
39```
40 
41## Variable Configuration
42 
43### Make Hardcoded Values Configurable
44 
45**Bad:**
46 
47```bash
48# Hardcoded volume name - brittle if project name changes
49docker volume rm "go-crypto-wallet_wallet-mysql"
50```
51 
52**Good:**
53 
54```bash
55# Configurable with default
56DOCKER_VOLUME_NAME="${DOCKER_VOLUME_NAME:-go-crypto-wallet_wallet-mysql}"
57docker volume rm "$DOCKER_VOLUME_NAME"
58```
59 
60**Benefits:**
61 
62- Flexibility for different environments
63- No breakage if project structure changes
64- Easy to override in CI/CD
65 
66### Environment Variable Naming
67 
68Use descriptive, uppercase names with underscores:
69 
70```bash
71RPC_USER="${RPC_USER:-xyz}"
72RPC_PASSWORD="${RPC_PASSWORD:-xyz}"
73WALLET_PASSPHRASE="${WALLET_PASSPHRASE:-test}"
74DOCKER_VOLUME_NAME="${DOCKER_VOLUME_NAME:-go-crypto-wallet_wallet-mysql}"
75```
76 
77### Use Environment Variables Instead of Modifying Config Files
78 
79**Bad (creates backups, modifies files):**
80 
81```bash
82# Backup and modify config file
83sed -i.bak 's|host: "127.0.0.1:18332"|host: "127.0.0.1:18332/wallet/watch"|' config.yaml
84 
85# ... operations ...
86 
87# Restore backup
88if [ -f "config.yaml.bak" ]; then
89    mv "config.yaml.bak" "config.yaml"
90fi
91```
92 
93**Good (use environment variables):**
94 
95```bash
96# Create wrapper function to set environment variable per-command
97watch_with_wallet() {
98    WALLET_BITCOIN_HOST="127.0.0.1:18332/wallet/watch" watch "$@"
99}
100 
101# Use wrapper function
102watch_with_wallet -c config.yaml create payment
103```
104 
105**Benefits:**
106 
107- No risk of leaving modified configs if script fails
108- No backup files to manage
109- Cleaner and more robust
110- Easy to override in CI/CD
111- No file permission issues
112 
113**When to use this pattern:**
114 
115- Applications that support environment variable overrides (check config documentation)
116- Temporary config changes for scripts or tests
117- Multi-environment setups (dev, staging, prod)
118 
119## Error Handling
120 
121### Refactor Duplicate Error Handling
122 
123**Bad (Duplicated):**
124 
125```bash
126if echo "$output" | grep -q "error"; then
127    log_error "Operation failed"
128    log_error "This could indicate:"
129    log_error "  - Reason 1"
130    log_error "  - Reason 2"
131    return 1
132fi
133 
134# ... later in code ...
135
136if echo "$output2" | grep -q "error"; then
137    log_error "Operation failed"
138    log_error "This could indicate:"
139    log_error "  - Reason 1"
140    log_error "  - Reason 2"
141    return 1
142fi
143```
144 
145**Good (DRY with helper function):**
146 
147```bash
148# Create helper function
149log_operation_error() {
150    log_error "Operation failed"
151    log_error "This could indicate:"
152    log_error "  - Reason 1"
153    log_error "  - Reason 2"
154    return 1
155}
156 
157# Use it
158if echo "$output" | grep -q "error"; then
159    log_operation_error
160fi
161
162if echo "$output2" | grep -q "error"; then
163    log_operation_error
164fi
165```
166 
167### Error Handling in Command Substitution
168 
169Always handle errors in command substitution:
170 
171```bash
172# Get output with error handling
173balance_json=$(bitcoin-cli getbalances 2>&1 || true)
174 
175# Parse with fallback
176balance=$(echo "$balance_json" | jq -r '.amount // 0' 2>/dev/null || echo "0")
177```
178 
179## Robust Comparisons
180 
181### Variable Quoting in Test Conditions (SC2086)
182 
183**Always quote variables in `[ ]` test conditions** to prevent globbing and word splitting.
184 
185**Bad (Unquoted - can fail if variable is empty or contains spaces):**
186 
187```bash
188# These can fail unexpectedly
189while [ $counter -lt $max ]; do
190if [ $removal_attempts -lt $max_attempts ]; then
191```
192 
193**Good (Properly quoted):**
194 
195```bash
196# Always quote variables in test conditions
197while [ "$counter" -lt "$max" ]; do
198if [ "$removal_attempts" -lt "$max_attempts" ]; then
199```
200 
201**Why this matters:**
202 
203- If `$counter` is empty, `[ -lt 5 ]` fails with "unary operator expected"
204- If `$counter` contains spaces, it splits into multiple arguments
205- Quoted variables ensure reliable behavior in all cases
206 
207**This applies to all test operators:**
208 
209```bash
210# Numeric comparisons - quote both sides
211[ "$a" -lt "$b" ]
212[ "$a" -gt "$b" ]
213[ "$a" -eq "$b" ]
214 
215# String comparisons - quote both sides
216[ "$str" = "value" ]
217[ "$str" != "value" ]
218[ -z "$str" ]
219[ -n "$str" ]
220```
221 
222### Floating Point Comparisons with bc
223 
224**Bad (Fragile):**
225 
226```bash
227if (($(echo "$balance > 0" | bc -l))); then
228    # Fails if bc is not installed or input is malformed
229fi
230```
231 
232**Good (Robust):**
233 
234```bash
235if [ -n "$balance" ] && [ "$(echo "$balance > 0" | bc -l 2>/dev/null || echo 0)" -eq 1 ]; then
236    # Handles bc failures gracefully
237fi
238```
239 
240**Why this is better:**
241 
242- Checks variable is not empty first
243- Redirects bc errors to /dev/null
244- Defaults to 0 on failure
245- Uses `-eq 1` for reliable comparison
246 
247### String Comparisons
248 
249Always quote variables:
250 
251```bash
252# BAD
253if [ $var = "value" ]; then
254 
255# GOOD
256if [ "$var" = "value" ]; then
257 
258# EVEN BETTER (handles empty/unset)
259if [ "${var:-}" = "value" ]; then
260```
261 
262## Script Structure
263 
264### Standard Script Template
265 
266```bash
267#!/usr/bin/env bash
268 
269# Script Description
270# Usage: ./script.sh [OPTIONS]
271# Options:
272#   --option  Description
273
274set -euo pipefail
275 
276# Script directory for relative paths
277SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
278PROJECT_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)"
279 
280# Source common utilities
281# shellcheck source=path/to/common.sh
282source "${SCRIPT_DIR}/common.sh"
283 
284# Configuration with defaults
285VERBOSE="${VERBOSE:-false}"
286CONFIG_FILE="${CONFIG_FILE:-config.yaml}"
287 
288###############################################################################
289# Functions
290###############################################################################
291 
292show_help() {
293    cat <<EOF
294Usage: $0 [OPTIONS]
295 
296Options:
297    --help      Show this help message
298    --verbose   Enable verbose output
299EOF
300}
301 
302main() {
303    # Parse arguments
304    while [ $# -gt 0 ]; do
305        case "$1" in
306            --help|-h)
307                show_help
308                exit 0
309                ;;
310            --verbose)
311                VERBOSE=true
312                shift
313                ;;
314            *)
315                log_error "Unknown option: $1"
316                show_help
317                exit 1
318                ;;
319        esac
320    done
321 
322    # Main logic here
323}
324 
325# Run main
326main "$@"
327```
328 
329## Common Patterns
330 
331### Directory Operations
332 
333```bash
334# Create directory if needed
335mkdir -p "${TARGET_DIR}"
336 
337# Clean directory except .gitkeep
338find "$dir" -type f ! -name '.gitkeep' -delete 2>/dev/null || true
339 
340# Remove directory contents except specific file
341find "$wallet_dir" -mindepth 1 ! -name 'bitcoin.conf' -exec rm -rf {} + 2>/dev/null || true
342```
343 
344### File Path Handling
345 
346```bash
347# Always quote paths with spaces
348cd "/path/with spaces"  # GOOD
349cd /path/with spaces    # BAD - will fail
350 
351# Use double quotes for variables
352cp "${SOURCE_FILE}" "${DEST_FILE}"
353 
354# Extract filename from path
355filename="${path##*/}"
356 
357# Extract directory from path
358dirname="${path%/*}"
359```
360 
361### Loop Patterns
362 
363```bash
364# Loop over arguments
365for arg in "$@"; do
366    echo "$arg"
367done
368 
369# Loop with counter
370for i in $(seq 1 10); do
371    echo "$i"
372done
373 
374# Loop over array
375wallets=(watch keygen sign1 sign2)
376for wallet in "${wallets[@]}"; do
377    echo "$wallet"
378done
379```
380 
381## Docker Operations
382 
383### Safe Docker Volume Removal
384 
385```bash
386# Try multiple times with backoff
387removal_attempts=0
388max_removal_attempts=5
389 
390# Note: Always quote variables in test conditions
391while [ "$removal_attempts" -lt "$max_removal_attempts" ]; do
392    if docker volume rm "$volume_name" 2>/dev/null; then
393        log_info "Volume removed successfully"
394        break
395    fi
396    removal_attempts=$((removal_attempts + 1))
397    if [ "$removal_attempts" -lt "$max_removal_attempts" ]; then
398        log_warn "Retrying in 2 seconds... (attempt $removal_attempts/$max_removal_attempts)"
399        sleep 2
400    fi
401done
402```
403 
404### Wait for Container Health
405 
406```bash
407wait_for_healthy() {
408    local container_name=$1
409    local max_wait=${2:-60}
410    local counter=0
411 
412    # Note: Always quote variables in test conditions
413    while [ "$counter" -lt "$max_wait" ]; do
414        status=$(docker inspect --format='{{.State.Health.Status}}' "$container_name" 2>/dev/null || echo "not_found")
415
416        if [ "$status" = "healthy" ]; then
417            return 0
418        fi
419 
420        counter=$((counter + 1))
421        sleep 1
422    done
423
424    return 1
425}
426```
427 
428## Logging Best Practices
429 
430### Use Descriptive Log Levels
431 
432```bash
433log_info "Starting operation..."
434log_warn "Retrying due to temporary failure"
435log_error "Critical error occurred"
436log_debug "Variable value: $var"  # Only shown if VERBOSE=true
437```
438 
439### Log Context for Debugging
440 
441```bash
442# BAD
443log_error "Failed"
444 
445# GOOD
446log_error "Failed to create wallet"
447log_error "Container: $container_name"
448log_error "Output: $output"
449log_error "Exit code: $?"
450```
451 
452## Security Considerations
453 
454### Never Log Sensitive Information
455 
456```bash
457# BAD
458log_info "Password: $PASSWORD"
459log_info "Private key: $PRIVATE_KEY"
460 
461# GOOD
462log_info "Credentials loaded"
463log_info "Private key file: ${KEY_FILE} (not displaying contents)"
464```
465 
466### Validate User Input
467 
468```bash
469# Validate before use
470if [ -z "$user_input" ]; then
471    log_error "Input cannot be empty"
472    exit 1
473fi
474 
475# Sanitize paths
476if [[ "$path" =~ \.\. ]]; then
477    log_error "Path traversal detected"
478    exit 1
479fi
480```
481 
482## Testing and Validation
483 
484### Check Prerequisites
485 
486```bash
487check_prerequisites() {
488    # Check required commands
489    for cmd in docker jq bc; do
490        if ! command -v "$cmd" >/dev/null 2>&1; then
491            log_error "$cmd is not installed"
492            exit 1
493        fi
494    done
495 
496    # Check required files
497    for file in "$CONFIG_FILE" "$KEY_FILE"; do
498        if [ ! -f "$file" ]; then
499            log_error "Required file not found: $file"
500            exit 1
501        fi
502    done
503}
504```
505 
506### Dry Run Mode
507 
508```bash
509DRY_RUN="${DRY_RUN:-false}"
510 
511execute_command() {
512    local cmd="$1"
513
514    if [ "$DRY_RUN" = "true" ]; then
515        log_info "[DRY RUN] Would execute: $cmd"
516    else
517        eval "$cmd"
518    fi
519}
520```
521 
522## Common Utilities from common.sh
523 
524This project provides common utilities in `scripts/operation/common.sh`:
525 
526```bash
527# Source it
528source "${SCRIPT_DIR}/../../common.sh"
529 
530# Available functions
531log_info "message"
532log_warn "message"
533log_error "message"
534log_step "Major Section"
535log_substep "Minor Section"
536
537check_docker
538wait_for_healthy "container-name" 60
539btc_cli "btc-watch" "getblockcount"
540clean_dir_except_gitkeep "data/address/btc"
541```
542 
543## ShellCheck Integration
544 
545**Always run Makefile targets after modifying shell scripts:**
546 
547```bash
548# Format all shell scripts
549make shfmt
550 
551# Lint all shell scripts
552make shellcheck
553```
554 
555These targets automatically process all `.sh` files in the `scripts/` directory.
556 
557For individual file testing (optional):
558 
559```bash
560# Format single file
561shfmt -l -w script.sh
562 
563# Check single file
564shellcheck script.sh
565```
566 
567### Common ShellCheck Directives
568 
569```bash
570# Disable specific warning
571# shellcheck disable=SC2086
572echo $var
573 
574# Mark sourced file
575# shellcheck source=path/to/file.sh
576source "${SCRIPT_DIR}/common.sh"
577 
578# Disable for entire file (use sparingly)
579# shellcheck disable=SC1090,SC2034
580```
581 
582## File Naming Conventions
583 
584- Use lowercase with hyphens: `e2e-p2pkh-2of3.sh`
585- Descriptive names: `setup-bitcoin-nodes.sh` not `setup.sh`
586- Prefix with purpose: `test-`, `e2e-`, `setup-`, `cleanup-`
587 
588## Documentation
589 
590### Comprehensive Header Comments
591 
592```bash
593#!/usr/bin/env bash
594 
595# Bitcoin E2E Workflow Script - Pattern 2: P2PKH 2-of-3 Multisig
596# This script automates the complete Bitcoin workflow
597#
598# Usage: ./script.sh [OPTIONS]
599#
600# Options:
601#   --reset    Full reset and run from scratch
602#   --cleanup  Stop containers and cleanup state
603#   --verbose  Enable verbose output
604#   -h, --help Display help message
605#
606# Environment Variables:
607#   RPC_USER          Bitcoin RPC username (default: xyz)
608#   RPC_PASSWORD      Bitcoin RPC password (default: xyz)
609#
610# Reference Documentation:
611#   docs/chains/btc/operations/e2e-transaction-patterns.md
612```
613 
614### Function Documentation
615 
616```bash
617# Wait for Docker container to be healthy
618# Usage: wait_for_healthy "container-name" [max_wait_seconds]
619# Arguments:
620#   $1 - Container name
621#   $2 - Maximum wait time in seconds (default: 60)
622# Returns:
623#   0 on success, 1 on timeout
624wait_for_healthy() {
625    local container_name=$1
626    local max_wait=${2:-60}
627    # ... implementation ...
628}
629```
630 
631## Language Requirements
632 
633### Write All Comments and Messages in English
634 
635All shell scripts must use English for:
636 
637- **Header comments** - Script description, usage, options
638- **Inline comments** - Code explanations
639- **Function documentation** - Usage, arguments, returns
640- **Log messages** - Info, warning, error messages
641- **Help text** - Command-line help output
642 
643**Bad (Japanese):**
644 
645```bash
646# 変換完了
647log_success "変換完了!"
648log_info "DRY-RUN モードでした"
649```
650 
651**Good (English):**
652 
653```bash
654# Conversion complete
655log_success "Conversion complete!"
656log_info "DRY-RUN mode was enabled"
657```
658 
659**Rationale:**
660 
661- Ensures accessibility for international contributors
662- Maintains consistency across the codebase
663- Facilitates collaboration with global developer community
664- Aligns with project documentation standards (see `documentation-language` rule)
665 
666## Makefile Integration
667 
668Shell scripts are often called from Makefiles:
669 
670```makefile
671.PHONY: btc-e2e-reset
672btc-e2e-reset:
673 ./scripts/operation/btc/e2e/e2e-workflow.sh --reset
674 
675.PHONY: btc-e2e-verbose
676btc-e2e-verbose:
677 ./scripts/operation/btc/e2e/e2e-workflow.sh --verbose
678```
679 
680## References
681 
682- [ShellCheck](https://www.shellcheck.net/) - Shell script analysis tool
683- [Google Shell Style Guide](https://google.github.io/styleguide/shellguide.html)
684- [Bash Hackers Wiki](https://wiki.bash-hackers.org/)
685- Project: @scripts/operation/common.sh - Common utilities
686- Project: @scripts/operation/btc/e2e/ - E2E test examples
687 
688## Quick Checklist
689 
690Before committing shell scripts:
691 
692- [ ] `set -euo pipefail` at the top
693- [ ] All hardcoded values are configurable via environment variables
694- [ ] Floating point comparisons use robust bc patterns
695- [ ] No duplicate error handling code (use helper functions)
696- [ ] All variables are quoted: `"$var"`
697- [ ] Error messages include context
698- [ ] No sensitive information in logs
699- [ ] Prerequisites are checked
700- [ ] Help message is comprehensive
701- [ ] **All comments and messages are in English**
702- [ ] Run `make shfmt` (format all shell scripts)
703- [ ] Run `make shellcheck` (lint all shell scripts)
704- [ ] File is executable: `chmod +x script.sh`
705