diff --git a/.claude/bootstrap.sh b/.claude/bootstrap.sh index 021f5270f3fb..078ddd1c22a2 100755 --- a/.claude/bootstrap.sh +++ b/.claude/bootstrap.sh @@ -4,7 +4,9 @@ # `test`. Keeps hook scripts and their tests as a self-contained component. source $(git rev-parse --show-toplevel)/ci3/source_bootstrap -hash=$(cache_content_hash ^.claude) +# Hash everything agents_symlink_test inspects, not just .claude/: the .codex mirrors and the root +# AGENTS.md/CLAUDE.md symlinks live outside .claude/, so a change there must still rerun the test. +hash=$(cache_content_hash "^.*.claude" "^.*.codex" "^AGENTS.md" "^CLAUDE.md") function test_cmds { # source_base cd's us into .claude/, so glob relative-to-here, but emit paths diff --git a/.claude/tests/agents_symlink_test b/.claude/tests/agents_symlink_test index 48a13ff77f16..4c3f2244bcf1 100755 --- a/.claude/tests/agents_symlink_test +++ b/.claude/tests/agents_symlink_test @@ -82,17 +82,43 @@ while IFS= read -r dir; do "$ROOT"/noir/noir-repo/*) continue;; esac codex="$(dirname "$dir")/.codex" - if [[ ! -L "$codex" ]]; then - fail "${codex#$ROOT/} is missing or is not a symlink to .claude" + rel=${codex#$ROOT/} + # A .codex may either be a symlink to its sibling .claude (the repo root keeps this form), or a + # real directory whose immediate children symlink into .claude (the per-package form, required + # because a sandbox cannot bind-mount a path that is itself a symlink). Either way Codex must see + # exactly the same contents as .claude. + if [[ -L "$codex" ]]; then + resolved=$(cd "$(dirname "$codex")" && cd "$(readlink "$codex")" 2>/dev/null && pwd -P) || resolved="" + claude_resolved=$(cd "$dir" && pwd -P) + if [[ "$resolved" != "$claude_resolved" ]]; then + fail "$rel is a symlink to ${resolved:-?}, expected its sibling .claude ($claude_resolved)" + else + pass "$rel -> .claude" + fi continue fi - resolved=$(cd "$(dirname "$codex")" && cd "$(readlink "$codex")" 2>/dev/null && pwd -P) || resolved="" - claude_resolved=$(cd "$dir" && pwd -P) - if [[ "$resolved" != "$claude_resolved" ]]; then - fail "${codex#$ROOT/} resolves to $resolved, expected $claude_resolved" + if [[ ! -d "$codex" ]]; then + fail "$rel is missing (expected a directory mirroring .claude via child symlinks)" continue fi - pass "${codex#$ROOT/}" + codex_ok=1 + # Forward: every entry in .claude must have a matching symlink in .codex. + while IFS= read -r child; do + name=$(basename "$child") + if [[ ! -L "$codex/$name" || ! "$codex/$name" -ef "$child" ]]; then + fail "$rel/$name should be a symlink to ../.claude/$name" + codex_ok=0 + fi + done < <(find "$dir" -mindepth 1 -maxdepth 1) + # Reverse: every entry in .codex must correspond to a .claude entry (no stale or dangling links). + while IFS= read -r entry; do + name=$(basename "$entry") + if [[ ! -e "$dir/$name" && ! -L "$dir/$name" ]]; then + fail "$rel/$name is stale; no matching .claude/$name" + codex_ok=0 + fi + done < <(find "$codex" -mindepth 1 -maxdepth 1) + (( codex_ok )) && pass "$rel" done < <(find "$ROOT" -type d -name .claude -not -path "$ROOT/noir/*" -not -path "$ROOT/**/node_modules/*") echo diff --git a/.github/workflows/deploy-network.yml b/.github/workflows/deploy-network.yml index 83f40bc4ad7c..353edc65eb44 100644 --- a/.github/workflows/deploy-network.yml +++ b/.github/workflows/deploy-network.yml @@ -111,7 +111,9 @@ jobs: uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd with: ref: ${{ steps.checkout-ref.outputs.ref }} - fetch-depth: 1 + # Full history so recursive submodules can checkout pinned commits (not only branch tips). + fetch-depth: 0 + lfs: true persist-credentials: false submodules: recursive # Initialize git submodules for l1-contracts dependencies diff --git a/.github/workflows/nightly-bench-10tps.yml b/.github/workflows/nightly-bench-10tps.yml index 68d79cb190cb..47a4df7db8e0 100644 --- a/.github/workflows/nightly-bench-10tps.yml +++ b/.github/workflows/nightly-bench-10tps.yml @@ -80,7 +80,7 @@ jobs: - name: Checkout uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 with: - ref: next + ref: ${{ github.sha }} - name: Authenticate to Google Cloud uses: google-github-actions/auth@6fc4af4b145ae7821d527454aa9bd537d1f2dc5f @@ -108,7 +108,7 @@ jobs: - name: Checkout uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 with: - ref: next + ref: ${{ github.sha }} - name: Run 10 TPS benchmark timeout-minutes: 240 diff --git a/.github/workflows/weekly-proving-bench.yml b/.github/workflows/weekly-proving-bench.yml index 654a38c610a5..ff5b060e82ff 100644 --- a/.github/workflows/weekly-proving-bench.yml +++ b/.github/workflows/weekly-proving-bench.yml @@ -71,7 +71,7 @@ jobs: - name: Checkout uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 with: - ref: next + ref: ${{ github.sha }} - name: Authenticate to Google Cloud uses: google-github-actions/auth@6fc4af4b145ae7821d527454aa9bd537d1f2dc5f diff --git a/.test_patterns.yml b/.test_patterns.yml index 299706444e55..7652dfda2060 100644 --- a/.test_patterns.yml +++ b/.test_patterns.yml @@ -91,6 +91,13 @@ tests: owners: - *nico + # Fails in CI: "No execution steps in ivc-inputs.msgpack" — pinned Chonk IVC + # inputs missing/stale. http://ci.aztec-labs.com/d7647a841ee811a0 + - regex: "bbapi/chonk_pinned_inputs.test" + skip: true + owners: + - *palla + # e2e tests skipped - regex: "testbench/port_change.test.ts" skip: true @@ -342,6 +349,15 @@ tests: owners: - *adam + # http://ci.aztec-labs.com/93754bdf832ef9bd + # P2P receipt race in the compose CLI-wallet flow: the account deployment tx + # can be reported as dropped after the 5s grace window before inclusion is + # visible to the queried node. + - regex: "yarn-project/end-to-end/scripts/run_test.sh compose ../cli-wallet/test/flows/public_authwit_transfer.sh" + error_regex: "Transaction .* was dropped\\. Reason: Tx dropped by P2P node" + owners: + - *palla + - regex: "cd l1-contracts && forge test" error_regex: "Encountered 1 failing test in test/staking_asset_handler/claim.t.sol:ClaimTest" owners: @@ -379,20 +395,6 @@ tests: owners: - *spyros - # Multi-validator web3signer suite is unstable under proposer pipelining: - # two distinct failure modes have been seen in the same "should build blocks - # & attest with multiple validator keys" case — Promise.all index-0 - # waitForTx aborting with "Tx dropped by P2P node" - # (ci.aztec-labs.com/9a5fa90aa18f62e7), and the proposer missing the slot's - # 5-attestation deadline ("AttestationTimeoutError" / "Block .* not found .* - # reorg") on PR #23344 run 26370196367 (ci.aztec-labs.com/b91d3218b5e88ae4). - # Error-regex flake matches still let ci3 retry-and-fail both attempts. Skip - # outright on merge-train/spartan until proposer pipelining stabilises. - - regex: "yarn-project/end-to-end/scripts/run_test.sh web3signer src/composed/web3signer/e2e_multi_validator_node_key_store.test.ts" - skip: true - owners: - - *palla - # http://ci.aztec-labs.com/98d59d04f85223f8 # Build-cache flake: module not found during Jest startup - regex: "src/e2e_sequencer/gov_proposal.parallel.test.ts" diff --git a/barretenberg/.codex b/barretenberg/.codex deleted file mode 120000 index c8161850a43d..000000000000 --- a/barretenberg/.codex +++ /dev/null @@ -1 +0,0 @@ -.claude \ No newline at end of file diff --git a/barretenberg/.codex/agents b/barretenberg/.codex/agents new file mode 120000 index 000000000000..0efb85ec44f4 --- /dev/null +++ b/barretenberg/.codex/agents @@ -0,0 +1 @@ +../.claude/agents \ No newline at end of file diff --git a/barretenberg/.codex/skills b/barretenberg/.codex/skills new file mode 120000 index 000000000000..454b8427cd75 --- /dev/null +++ b/barretenberg/.codex/skills @@ -0,0 +1 @@ +../.claude/skills \ No newline at end of file diff --git a/barretenberg/cpp/.codex b/barretenberg/cpp/.codex deleted file mode 120000 index c8161850a43d..000000000000 --- a/barretenberg/cpp/.codex +++ /dev/null @@ -1 +0,0 @@ -.claude \ No newline at end of file diff --git a/barretenberg/cpp/.codex/agents b/barretenberg/cpp/.codex/agents new file mode 120000 index 000000000000..0efb85ec44f4 --- /dev/null +++ b/barretenberg/cpp/.codex/agents @@ -0,0 +1 @@ +../.claude/agents \ No newline at end of file diff --git a/barretenberg/ts/src/bb_backends/node/platform.ts b/barretenberg/ts/src/bb_backends/node/platform.ts index adc92efff1bc..d312cc43ad15 100644 --- a/barretenberg/ts/src/bb_backends/node/platform.ts +++ b/barretenberg/ts/src/bb_backends/node/platform.ts @@ -83,7 +83,8 @@ export function detectPlatform(): Platform | null { * * Search order: * 1. If customPath is provided and exists, return it - * 2. Otherwise search in /build//bb + * 2. If BB_BINARY_PATH is set and exists, return it + * 3. Otherwise search in /build//bb */ export function findBbBinary(customPath?: string): string | null { // Check custom path first if provided @@ -95,6 +96,14 @@ export function findBbBinary(customPath?: string): string | null { return null; } + const envPath = process.env.BB_BINARY_PATH; + if (envPath) { + if (fs.existsSync(envPath)) { + return path.resolve(envPath); + } + return null; + } + // Automatic detection const platform = detectPlatform(); if (!platform) { diff --git a/ci3/bootstrap_ec2 b/ci3/bootstrap_ec2 index 39fb276d0f2c..2ebc53569961 100755 --- a/ci3/bootstrap_ec2 +++ b/ci3/bootstrap_ec2 @@ -83,6 +83,7 @@ else fi [ -n "${INSTANCE_POSTFIX:-}" ] && instance_name+="_$INSTANCE_POSTFIX" +docker_hostname=$(echo -n "$instance_name" | tr '_' '-' | cut -c 1-63) if [ "$use_ssh" -eq 1 ]; then echo_header "request build instance (SSH)" @@ -298,7 +299,7 @@ start_build() { docker run --privileged --rm \${docker_args:-} \ --name aztec_build \ - --hostname $instance_name \ + --hostname $docker_hostname \ -v bootstrap_ci_local_docker:/var/lib/docker \ -v bootstrap_ci_repo:/home/aztec-dev/aztec-packages \ -v \$HOME/.ssh:/home/aztec-dev/.ssh:ro \ diff --git a/docs/.codex b/docs/.codex deleted file mode 120000 index c8161850a43d..000000000000 --- a/docs/.codex +++ /dev/null @@ -1 +0,0 @@ -.claude \ No newline at end of file diff --git a/docs/.codex/agents b/docs/.codex/agents new file mode 120000 index 000000000000..0efb85ec44f4 --- /dev/null +++ b/docs/.codex/agents @@ -0,0 +1 @@ +../.claude/agents \ No newline at end of file diff --git a/docs/.codex/commands b/docs/.codex/commands new file mode 120000 index 000000000000..0b631dd5b721 --- /dev/null +++ b/docs/.codex/commands @@ -0,0 +1 @@ +../.claude/commands \ No newline at end of file diff --git a/iac/network/scripts/destroy_bootnodes.sh b/iac/network/scripts/destroy_bootnodes.sh index b4598f634b36..58a715f1e5cf 100755 --- a/iac/network/scripts/destroy_bootnodes.sh +++ b/iac/network/scripts/destroy_bootnodes.sh @@ -3,20 +3,86 @@ set -e # This script should be run from the root of iac/network. It will destroy all VMs for the provided network -# Only the VMs are destroyed. No other infrastructure is modified - -# Usage: ./scripts/destroy_bootnodes.sh - -NETWORK_NAME=${1:-} -PROJECT_ID=${2:-} +# By default, only the VMs are destroyed. Use the optional flags to delete other per-network bootnode resources. + +# Usage: ./scripts/destroy_bootnodes.sh [--destroy-ip] [--destroy-private-key] [--dry-run] + +usage() { + echo "Usage: ./scripts/destroy_bootnodes.sh [--destroy-ip] [--destroy-private-key] [--dry-run]" + echo "" + echo "Options:" + echo " --destroy-ip Delete the reserved static IP addresses and remove them from Terraform state" + echo " --destroy-private-key Delete the per-region bootnode private-key secrets" + echo " --dry-run Show the planned VM destroy and print other destructive commands without running them" +} + +print_command() { + printf '+' + printf ' %q' "$@" + printf '\n' +} + +run_destructive() { + print_command "$@" + + if [[ "$DRY_RUN" != "true" ]]; then + "$@" + fi +} + +DESTROY_IP=false +DESTROY_PRIVATE_KEY=false +DRY_RUN=false +POSITIONAL_ARGS=() + +while [[ $# -gt 0 ]]; do + case "$1" in + --destroy-ip) + DESTROY_IP=true + shift + ;; + --destroy-private-key) + DESTROY_PRIVATE_KEY=true + shift + ;; + --dry-run) + DRY_RUN=true + shift + ;; + -h | --help) + usage + exit 0 + ;; + *) + POSITIONAL_ARGS+=("$1") + shift + ;; + esac +done + +NETWORK_NAME=${POSITIONAL_ARGS[0]:-} +PROJECT_ID=${POSITIONAL_ARGS[1]:-} + +if [[ ${#POSITIONAL_ARGS[@]} -gt 2 ]]; then + echo "Unexpected arguments: ${POSITIONAL_ARGS[*]:2}" + usage + exit 1 +fi if [[ -z "$NETWORK_NAME" ]]; then echo "NETWORK_NAME is required" + usage exit 1 fi if [[ -z "$PROJECT_ID" ]]; then echo "PROJECT_ID is required" + usage + exit 1 +fi + +if [[ "$DESTROY_IP" == "true" || "$DESTROY_PRIVATE_KEY" == "true" ]] && ! command -v gcloud >/dev/null 2>&1; then + echo "gcloud is required when --destroy-ip or --destroy-private-key is used" exit 1 fi @@ -27,20 +93,22 @@ TAG=latest ROOT=$(git rev-parse --show-toplevel)/iac/network -cd $ROOT/bootnode/ip/gcp +cd "$ROOT/bootnode/ip/gcp" -terraform init -backend-config="prefix=network/$NETWORK_NAME/bootnode/ip/gcp" +terraform init -reconfigure -backend-config="prefix=network/$NETWORK_NAME/bootnode/ip/gcp" >/dev/null OUTPUT=$(terraform output -json ip_addresses) echo "IP Addresses output: $OUTPUT" GCP_REGIONS_ARRAY=() +GCP_IPS_ARRAY=() while read -r REGION IP; do echo "IP: $IP is in region $REGION" GCP_REGIONS_ARRAY+=("$REGION") + GCP_IPS_ARRAY+=("$IP") done < <(echo "$OUTPUT" | jq -r 'to_entries | .[] | "\(.key) \(.value)"') @@ -52,22 +120,80 @@ PRIVATE_KEYS_TF_ARG=$GCP_REGIONS_TF_ARG echo "GCP_REGIONS: $GCP_REGIONS_TF_ARG" -cd $ROOT +cd "$ROOT" BOOTNODE_START_SCRIPT="$ROOT/scripts/bootnode_startup.sh" -cd $ROOT/bootnode/vm/gcp - -terraform init -backend-config="prefix=network/$NETWORK_NAME/bootnode/vm/gcp" - -terraform apply \ - -var="regions=$GCP_REGIONS_TF_ARG" \ - -var="start_script=$BOOTNODE_START_SCRIPT" \ - -var="network_name=$NETWORK_NAME" \ - -var="peer_id_private_keys=$PRIVATE_KEYS_TF_ARG" \ - -var="machine_type=" \ - -var="project_id=$PROJECT_ID" \ - -var="p2p_port=$P2P_PORT" \ - -var="l1_chain_id=$L1_CHAIN_ID" \ - -var="image_tag=$TAG" \ - --destroy +cd "$ROOT/bootnode/vm/gcp" + +terraform init -reconfigure -backend-config="prefix=network/$NETWORK_NAME/bootnode/vm/gcp" >/dev/null + +VM_TERRAFORM_ARGS=( + -var="regions=$GCP_REGIONS_TF_ARG" + -var="start_script=$BOOTNODE_START_SCRIPT" + -var="network_name=$NETWORK_NAME" + -var="peer_id_private_keys=$PRIVATE_KEYS_TF_ARG" + -var="machine_type=" + -var="project_id=$PROJECT_ID" + -var="p2p_port=$P2P_PORT" + -var="l1_chain_id=$L1_CHAIN_ID" + -var="image_tag=$TAG" +) + +if [[ "$DRY_RUN" == "true" ]]; then + terraform plan "${VM_TERRAFORM_ARGS[@]}" -destroy +else + terraform apply "${VM_TERRAFORM_ARGS[@]}" --destroy +fi + +if [[ "$DESTROY_IP" == "true" ]]; then + cd "$ROOT/bootnode/ip/gcp" + + terraform init -reconfigure -backend-config="prefix=network/$NETWORK_NAME/bootnode/ip/gcp" >/dev/null + + for INDEX in "${!GCP_REGIONS_ARRAY[@]}"; do + REGION=${GCP_REGIONS_ARRAY[$INDEX]} + IP=${GCP_IPS_ARRAY[$INDEX]} + ADDRESS_NAME="$NETWORK_NAME-bootnodes-$REGION" + STATE_ADDRESS="module.ip.google_compute_address.static_ip[\"$REGION\"]" + + echo "Deleting static IP $ADDRESS_NAME ($IP) in $REGION" + + if DESCRIBE_OUTPUT=$(gcloud compute addresses describe "$ADDRESS_NAME" --region "$REGION" --project "$PROJECT_ID" 2>&1); then + run_destructive gcloud compute addresses delete "$ADDRESS_NAME" --region "$REGION" --project "$PROJECT_ID" --quiet + elif [[ "${DESCRIBE_OUTPUT,,}" == *"not found"* ]]; then + echo "Static IP $ADDRESS_NAME was not found; removing Terraform state entry if present." + else + echo "$DESCRIBE_OUTPUT" + exit 1 + fi + + if terraform state show "$STATE_ADDRESS" >/dev/null 2>&1; then + run_destructive terraform state rm "$STATE_ADDRESS" + else + echo "Terraform state entry $STATE_ADDRESS not found; skipping state removal." + fi + done + + run_destructive terraform apply -auto-approve \ + -var="regions=[]" \ + -var="name=$NETWORK_NAME-bootnodes" \ + -var="project_id=$PROJECT_ID" +fi + +if [[ "$DESTROY_PRIVATE_KEY" == "true" ]]; then + for REGION in "${GCP_REGIONS_ARRAY[@]}"; do + SECRET_NAME="$NETWORK_NAME-$REGION-bootnode-private-key" + + echo "Deleting secret $SECRET_NAME" + + if DESCRIBE_OUTPUT=$(gcloud secrets describe "$SECRET_NAME" --project "$PROJECT_ID" 2>&1); then + run_destructive gcloud secrets delete "$SECRET_NAME" --project "$PROJECT_ID" --quiet + elif [[ "${DESCRIBE_OUTPUT,,}" == *"not found"* ]]; then + echo "Secret $SECRET_NAME was not found; skipping." + else + echo "$DESCRIBE_OUTPUT" + exit 1 + fi + done +fi diff --git a/spartan/aztec-postgres/templates/statefulset.yaml b/spartan/aztec-postgres/templates/statefulset.yaml index bd8974ced5bb..8f4f7e680ac8 100644 --- a/spartan/aztec-postgres/templates/statefulset.yaml +++ b/spartan/aztec-postgres/templates/statefulset.yaml @@ -18,6 +18,10 @@ spec: app.kubernetes.io/instance: {{ .Release.Name }} app.kubernetes.io/component: {{ .Values.component | default .Chart.Name }} spec: + {{- with .Values.nodeSelector }} + nodeSelector: + {{- toYaml . | nindent 8 }} + {{- end }} containers: - name: postgres image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}" diff --git a/spartan/aztec-postgres/values.yaml b/spartan/aztec-postgres/values.yaml index 349a60ac0cc3..f6187304a373 100644 --- a/spartan/aztec-postgres/values.yaml +++ b/spartan/aztec-postgres/values.yaml @@ -28,3 +28,5 @@ persistence: service: port: 5432 + +nodeSelector: {} diff --git a/spartan/aztec-prover-stack/templates/agent-scaledobject.yaml b/spartan/aztec-prover-stack/templates/agent-scaledobject.yaml new file mode 100644 index 000000000000..98f55a7cf5b4 --- /dev/null +++ b/spartan/aztec-prover-stack/templates/agent-scaledobject.yaml @@ -0,0 +1,40 @@ +{{- if .Values.agent.autoscaling.keda.enabled }} +{{- $agentChartName := default "agent" .Values.agent.nameOverride }} +{{- $agentDefaultName := ternary .Release.Name (printf "%s-%s" .Release.Name $agentChartName) (contains $agentChartName .Release.Name) }} +{{- $agentName := default $agentDefaultName .Values.agent.fullnameOverride | trunc 63 | trimSuffix "-" }} +{{- $queueQuery := printf "sum(aztec_proving_queue_size{k8s_namespace_name=%q})" .Release.Namespace }} +apiVersion: keda.sh/v1alpha1 +kind: ScaledObject +metadata: + name: {{ $agentName }} + labels: + {{- include "chart.labels" . | nindent 4 }} +spec: + scaleTargetRef: + kind: Deployment + name: {{ $agentName }} + pollingInterval: {{ .Values.agent.autoscaling.keda.pollingInterval }} + cooldownPeriod: {{ .Values.agent.autoscaling.keda.cooldownPeriod }} + minReplicaCount: {{ .Values.agent.autoscaling.keda.minReplicaCount }} + maxReplicaCount: {{ .Values.agent.autoscaling.keda.maxReplicaCount }} + triggers: + {{- if .Values.agent.autoscaling.keda.scalingBands }} + {{- range $band := .Values.agent.autoscaling.keda.scalingBands }} + - type: prometheus + metadata: + serverAddress: {{ $.Values.agent.autoscaling.keda.prometheus.serverAddress | quote }} + metricName: {{ printf "aztec_proving_queue_size_agents_%v_over_%v" $band.replicas $band.queueSize | replace "." "_" | quote }} + query: {{ printf "((%s or vector(0)) > bool %v) * %v" $queueQuery $band.queueSize $band.replicas | quote }} + threshold: "1" + activationThreshold: "0" + {{- end }} + {{- else }} + - type: prometheus + metadata: + serverAddress: {{ .Values.agent.autoscaling.keda.prometheus.serverAddress | quote }} + metricName: "aztec_proving_queue_size" + query: {{ $queueQuery | quote }} + threshold: "1" + activationThreshold: "0" + {{- end }} +{{- end }} diff --git a/spartan/aztec-prover-stack/values.yaml b/spartan/aztec-prover-stack/values.yaml index 2e8a433e5eb0..ae014973bcd7 100644 --- a/spartan/aztec-prover-stack/values.yaml +++ b/spartan/aztec-prover-stack/values.yaml @@ -84,6 +84,17 @@ agent: nodeType: "prover-agent" replicaCount: 1 + autoscaling: + keda: + enabled: false + pollingInterval: 30 + cooldownPeriod: 300 + minReplicaCount: 0 + maxReplicaCount: 1 + scalingBands: [] + prometheus: + serverAddress: "" + persistence: enabled: false diff --git a/spartan/environments/mainnet.env b/spartan/environments/mainnet.env index 97d6b69748f5..e8b74d9b3efe 100644 --- a/spartan/environments/mainnet.env +++ b/spartan/environments/mainnet.env @@ -12,7 +12,8 @@ VERIFY_CONTRACTS=false DEPLOY_INTERNAL_BOOTNODE=false VALIDATOR_REPLICAS=0 RPC_REPLICAS=1 -PROVER_REPLICAS=4 +PROVER_REPLICAS=0 +PROVER_ENABLED=false CREATE_RPC_INGRESS=true CREATE_RPC_DNS=true diff --git a/spartan/environments/next-net.env b/spartan/environments/next-net.env index f8541c8b5229..efd92c9f1d64 100644 --- a/spartan/environments/next-net.env +++ b/spartan/environments/next-net.env @@ -44,13 +44,23 @@ AZTEC_LAG_IN_EPOCHS_FOR_VALIDATOR_SET=2 AZTEC_LAG_IN_EPOCHS_FOR_RANDAO=2 AZTEC_INBOX_LAG=2 -VALIDATOR_REPLICAS=4 -VALIDATORS_PER_NODE=12 +VALIDATOR_REPLICAS=2 +VALIDATORS_PER_NODE=24 VALIDATOR_PUBLISHERS_PER_REPLICA=4 VALIDATOR_PUBLISHER_MNEMONIC_START_INDEX=5000 PUBLISHERS_PER_PROVER=2 PROVER_PUBLISHER_MNEMONIC_START_INDEX=8000 +PROVER_AGENT_KEDA_ENABLED=true +PROVER_AGENT_KEDA_PROMETHEUS_SERVER_ADDRESS=REPLACE_WITH_GCP_SECRET +PROVER_AGENT_KEDA_MIN_REPLICAS=0 +PROVER_AGENT_KEDA_MAX_REPLICAS=4 +PROVER_AGENT_KEDA_SCALING_BANDS='[ + { + queueSize = 0 + replicas = 4 + } +]' BOT_TRANSFERS_REPLICAS=1 BOT_TRANSFERS_TX_INTERVAL_SECONDS=250 diff --git a/spartan/environments/prove-n-tps-fake.env b/spartan/environments/prove-n-tps-fake.env index 9aa4fc5f6359..46ea329e2582 100644 --- a/spartan/environments/prove-n-tps-fake.env +++ b/spartan/environments/prove-n-tps-fake.env @@ -22,8 +22,8 @@ FUNDING_PRIVATE_KEY="0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf OTEL_COLLECTOR_ENDPOINT=REPLACE_WITH_GCP_SECRET -VALIDATOR_REPLICAS=4 -VALIDATORS_PER_NODE=12 +VALIDATOR_REPLICAS=2 +VALIDATORS_PER_NODE=24 VALIDATOR_PUBLISHERS_PER_REPLICA=4 VALIDATOR_PUBLISHER_MNEMONIC_START_INDEX=5000 diff --git a/spartan/environments/prove-n-tps-real.env b/spartan/environments/prove-n-tps-real.env index 16e0548e91e9..0713b0e4eef2 100644 --- a/spartan/environments/prove-n-tps-real.env +++ b/spartan/environments/prove-n-tps-real.env @@ -22,8 +22,8 @@ FUNDING_PRIVATE_KEY="0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf OTEL_COLLECTOR_ENDPOINT=REPLACE_WITH_GCP_SECRET -VALIDATOR_REPLICAS=4 -VALIDATORS_PER_NODE=12 +VALIDATOR_REPLICAS=2 +VALIDATORS_PER_NODE=24 VALIDATOR_PUBLISHERS_PER_REPLICA=4 VALIDATOR_PUBLISHER_MNEMONIC_START_INDEX=5000 diff --git a/spartan/environments/staging-public.env b/spartan/environments/staging-public.env index b3f8b318c4a4..ef03a277c131 100644 --- a/spartan/environments/staging-public.env +++ b/spartan/environments/staging-public.env @@ -47,11 +47,11 @@ CREATE_ROLLUP_CONTRACTS=${CREATE_ROLLUP_CONTRACTS:-false} P2P_TX_POOL_DELETE_TXS_AFTER_REORG=true VALIDATOR_REPLICAS=2 -VALIDATORS_PER_NODE=64 +VALIDATORS_PER_NODE=128 VALIDATOR_PUBLISHERS_PER_REPLICA=4 VALIDATOR_PUBLISHER_MNEMONIC_START_INDEX=5000 VALIDATOR_HA_REPLICAS=1 -VALIDATOR_HA_REPLICA_COUNT=4 +VALIDATOR_HA_REPLICA_COUNT=1 VALIDATOR_RESOURCE_PROFILE="prod" PROVER_FAILED_PROOF_STORE=gs://aztec-develop/staging-public/failed-proofs diff --git a/spartan/environments/testnet.env b/spartan/environments/testnet.env index 10cf468f1341..8f0e5c30e7ac 100644 --- a/spartan/environments/testnet.env +++ b/spartan/environments/testnet.env @@ -76,8 +76,8 @@ RPC_INGRESS_STATIC_IP_NAME=testnet-rpc-ip RPC_INGRESS_SSL_CERT_NAMES='["testnet-rpc-cert"]' -VALIDATOR_REPLICAS=4 -VALIDATORS_PER_NODE=64 +VALIDATOR_REPLICAS=2 +VALIDATORS_PER_NODE=128 VALIDATOR_PUBLISHERS_PER_REPLICA=8 VALIDATOR_PUBLISHER_MNEMONIC_START_INDEX=5000 VALIDATOR_HA_REPLICAS=1 @@ -89,3 +89,17 @@ PROVER_FAILED_PROOF_STORE=gs://aztec-develop/testnet/failed-proofs L1_TX_FAILED_STORE=gs://aztec-develop/testnet/failed-l1-txs PROVER_REPLICAS=4 PROVER_RESOURCE_PROFILE="prod" +PROVER_AGENT_KEDA_ENABLED=false +PROVER_AGENT_KEDA_PROMETHEUS_SERVER_ADDRESS= +PROVER_AGENT_KEDA_MIN_REPLICAS=0 +PROVER_AGENT_KEDA_MAX_REPLICAS=8 +PROVER_AGENT_KEDA_SCALING_BANDS='[ + { + queueSize = 0 + replicas = 4 + }, + { + queueSize = 50 + replicas = 8 + } +]' diff --git a/spartan/scripts/deploy_network.sh b/spartan/scripts/deploy_network.sh index 9ef173dfdacd..bc7e9e346cdd 100755 --- a/spartan/scripts/deploy_network.sh +++ b/spartan/scripts/deploy_network.sh @@ -131,6 +131,16 @@ AZTEC_EPOCHS_LAG=${AZTEC_EPOCHS_LAG:-} SEQ_ENFORCE_TIME_TABLE=${SEQ_ENFORCE_TIME_TABLE:-} SEQ_SKIP_CHECKPOINT_PUBLISH_PERCENT=${SEQ_SKIP_CHECKPOINT_PUBLISH_PERCENT:-0} PROVER_REPLICAS=${PROVER_REPLICAS:-4} +PROVER_ENABLED=${PROVER_ENABLED:-true} +PROVER_AGENT_KEDA_ENABLED=${PROVER_AGENT_KEDA_ENABLED:-false} +PROVER_AGENT_KEDA_MIN_REPLICAS=${PROVER_AGENT_KEDA_MIN_REPLICAS:-0} +PROVER_AGENT_KEDA_MAX_REPLICAS=${PROVER_AGENT_KEDA_MAX_REPLICAS:-$PROVER_REPLICAS} +PROVER_AGENT_KEDA_SCALING_BANDS=${PROVER_AGENT_KEDA_SCALING_BANDS:-[]} +PROVER_AGENT_KEDA_PROMETHEUS_SERVER_ADDRESS=${PROVER_AGENT_KEDA_PROMETHEUS_SERVER_ADDRESS:-} +if [[ "$PROVER_ENABLED" == "true" && "$PROVER_AGENT_KEDA_ENABLED" == "true" && -z "$PROVER_AGENT_KEDA_PROMETHEUS_SERVER_ADDRESS" ]]; then + die "PROVER_AGENT_KEDA_ENABLED=true requires PROVER_AGENT_KEDA_PROMETHEUS_SERVER_ADDRESS. Set it explicitly, for example via GCP secret replacement." +fi +PROVER_AGENT_REPLICA_CAPACITY=$([[ "$PROVER_ENABLED" == "true" ]] && echo "$PROVER_AGENT_KEDA_MAX_REPLICAS" || echo 0) PROVER_AGENTS_PER_PROVER=${PROVER_AGENTS_PER_PROVER:-1} R2_ACCESS_KEY_ID=${R2_ACCESS_KEY_ID:-} R2_SECRET_ACCESS_KEY=${R2_SECRET_ACCESS_KEY:-} @@ -252,7 +262,7 @@ if (( TOTAL_VALIDATOR_PUBLISHERS > 0 )); then fi # Add prover publishers to prefunding list -TOTAL_PROVER_PUBLISHERS=$((PROVER_REPLICAS * PUBLISHERS_PER_PROVER)) +TOTAL_PROVER_PUBLISHERS=$((PROVER_AGENT_REPLICA_CAPACITY * PUBLISHERS_PER_PROVER)) if (( TOTAL_PROVER_PUBLISHERS > 0 )); then PROVER_PUBLISHER_RANGE=$(seq "$PROVER_PUBLISHER_MNEMONIC_START_INDEX" $((PROVER_PUBLISHER_MNEMONIC_START_INDEX + TOTAL_PROVER_PUBLISHERS - 1)) | tr '\n' ',' | sed 's/,$//') @@ -629,6 +639,13 @@ BOT_CROSS_CHAIN_L2_PRIVATE_KEY = "${BOT_CROSS_CHAIN_L2_PRIVATE_KEY:-0xcafe03}" PROVER_AGENTS_PER_PROVER = ${PROVER_AGENTS_PER_PROVER} PROVER_AGENT_POLL_INTERVAL_MS = ${PROVER_AGENT_POLL_INTERVAL_MS} +PROVER_AGENT_KEDA_ENABLED = ${PROVER_AGENT_KEDA_ENABLED:-false} +PROVER_AGENT_KEDA_PROMETHEUS_SERVER_ADDRESS = "${PROVER_AGENT_KEDA_PROMETHEUS_SERVER_ADDRESS:-}" +PROVER_AGENT_KEDA_MIN_REPLICAS = ${PROVER_AGENT_KEDA_MIN_REPLICAS:-0} +PROVER_AGENT_KEDA_MAX_REPLICAS = ${PROVER_AGENT_KEDA_MAX_REPLICAS:-$PROVER_REPLICAS} +PROVER_AGENT_KEDA_SCALING_BANDS = ${PROVER_AGENT_KEDA_SCALING_BANDS:-[]} +PROVER_AGENT_KEDA_POLLING_INTERVAL_SECONDS = ${PROVER_AGENT_KEDA_POLLING_INTERVAL_SECONDS:-30} +PROVER_AGENT_KEDA_COOLDOWN_PERIOD_SECONDS = ${PROVER_AGENT_KEDA_COOLDOWN_PERIOD_SECONDS:-300} RPC_INGRESS_ENABLED = ${RPC_INGRESS_ENABLED} RPC_INGRESS_HOSTS = ${RPC_INGRESS_HOSTS} @@ -649,6 +666,7 @@ PROVER_PROOF_STORE = "${PROVER_PROOF_STORE:-}" PROVER_BROKER_DEBUG_REPLAY_ENABLED = ${PROVER_BROKER_DEBUG_REPLAY_ENABLED:-false} DEPLOY_ARCHIVAL_NODE = ${DEPLOY_ARCHIVAL_NODE} PROVER_REPLICAS = ${PROVER_REPLICAS} +PROVER_ENABLED = ${PROVER_ENABLED} PROVER_TEST_DELAY_TYPE = "${PROVER_TEST_DELAY_TYPE}" PROVER_TEST_VERIFICATION_DELAY_MS = ${PROVER_TEST_VERIFICATION_DELAY_MS} diff --git a/spartan/scripts/install_deps.sh b/spartan/scripts/install_deps.sh index f1fac04c4de9..536ccffd820a 100755 --- a/spartan/scripts/install_deps.sh +++ b/spartan/scripts/install_deps.sh @@ -134,6 +134,19 @@ if ! command -v cast &> /dev/null; then sudo chmod +x /usr/local/bin/cast fi +TERRAFORM_VERSION="1.7.5" +if ! command -v terraform &> /dev/null; then + log "Installing terraform ${TERRAFORM_VERSION}..." + tf_os="$(os)" + if [ "$tf_os" = "macos" ]; then + tf_os="darwin" + fi + curl -fsSL "https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_${tf_os}_$(arch).zip" -o terraform.zip + unzip -qo terraform.zip + sudo mv terraform /usr/local/bin/terraform + rm terraform.zip +fi + require_cmd git require_cmd kubectl require_cmd terraform diff --git a/spartan/scripts/network_deploy.sh b/spartan/scripts/network_deploy.sh index eab3b12962f7..8fa7fcaf3e12 100755 --- a/spartan/scripts/network_deploy.sh +++ b/spartan/scripts/network_deploy.sh @@ -28,6 +28,7 @@ gcp_auth # Second pass: source environment with GCP secret processing source_network_env "$env_file" + # Optional: provision per-network IP + managed cert (+ DNS record in the delegated # rpc.aztec-labs.com zone) via the network-frontend terraform module. The module's # outputs are exported as env vars that deploy_network.sh already consumes. diff --git a/spartan/scripts/setup_gcp_secrets.sh b/spartan/scripts/setup_gcp_secrets.sh index 9eadb39ecdc1..10b2cb30e05c 100755 --- a/spartan/scripts/setup_gcp_secrets.sh +++ b/spartan/scripts/setup_gcp_secrets.sh @@ -88,6 +88,7 @@ declare -A SECRET_MAPPINGS=( ["FUNDING_PRIVATE_KEY"]="${L1_NETWORK}-funding-private-key" ["ROLLUP_DEPLOYMENT_PRIVATE_KEY"]="${L1_NETWORK}-labs-rollup-private-key" ["OTEL_COLLECTOR_ENDPOINT"]="otel-collector-url" + ["PROVER_AGENT_KEDA_PROMETHEUS_SERVER_ADDRESS"]="prometheus-internal-read-url" ["ETHERSCAN_API_KEY"]="etherscan-api-key" ["LABS_INFRA_MNEMONIC"]="${MNEMONIC_SECRET}" ["STORE_SNAPSHOT_URL"]="r2-account-id" diff --git a/spartan/scripts/source_network_env.sh b/spartan/scripts/source_network_env.sh index 9a99c22c5481..6257490e8149 100755 --- a/spartan/scripts/source_network_env.sh +++ b/spartan/scripts/source_network_env.sh @@ -24,6 +24,15 @@ function source_network_env { if grep -q "REPLACE_WITH_GCP_SECRET" "$env_file" && command -v gcloud &> /dev/null; then echo "Environment file contains GCP secret placeholders. Processing secrets..." + # Activate GCP credentials before Secret Manager reads (same order as network_deploy.sh). + if declare -f gcp_auth >/dev/null 2>&1; then + gcp_auth + else + # shellcheck source=scripts/gcp_auth.sh + source "$spartan/scripts/gcp_auth.sh" + gcp_auth + fi + # Process GCP secrets source $spartan/scripts/setup_gcp_secrets.sh "$env_file" diff --git a/spartan/terraform/deploy-aztec-infra/main.tf b/spartan/terraform/deploy-aztec-infra/main.tf index 46583be5f6fc..d14fd6d69d8a 100644 --- a/spartan/terraform/deploy-aztec-infra/main.tf +++ b/spartan/terraform/deploy-aztec-infra/main.tf @@ -52,7 +52,7 @@ module "web3signer" { VALIDATOR_MNEMONIC_START_INDEX = tonumber(var.VALIDATOR_MNEMONIC_START_INDEX) VALIDATOR_PUBLISHER_MNEMONIC_START_INDEX = tonumber(var.VALIDATOR_PUBLISHER_MNEMONIC_START_INDEX) VALIDATOR_PUBLISHERS_PER_REPLICA = var.VALIDATOR_PUBLISHERS_PER_REPLICA - PROVER_COUNT = tonumber(var.PROVER_REPLICAS) + PROVER_COUNT = local.prover_agent_replica_capacity PUBLISHERS_PER_PROVER = tonumber(var.PROVER_PUBLISHERS_PER_PROVER) PROVER_PUBLISHER_MNEMONIC_START_INDEX = tonumber(var.PROVER_PUBLISHER_MNEMONIC_START_INDEX) @@ -94,6 +94,8 @@ locals { tag = split(":", var.VALIDATOR_HA_DOCKER_IMAGE)[1] } : local.aztec_image + prover_agent_replica_capacity = var.PROVER_ENABLED ? (var.PROVER_AGENT_KEDA_ENABLED ? var.PROVER_AGENT_KEDA_MAX_REPLICAS : tonumber(var.PROVER_REPLICAS)) : 0 + # Max node count: max of primary (VALIDATOR_REPLICAS) and HA pod counts # Determines how many attester keystores and publisher key ranges to generate effective_ha_count = var.VALIDATOR_HA_REPLICAS > 0 ? coalesce(var.VALIDATOR_HA_REPLICA_COUNT, tonumber(var.VALIDATOR_REPLICAS)) : 0 @@ -317,7 +319,7 @@ locals { wait = true } : null - prover = { + prover = var.PROVER_ENABLED ? { name = "${var.RELEASE_PREFIX}-prover" chart = "aztec-prover-stack" values = [ @@ -343,6 +345,19 @@ locals { node = { logLevel = var.LOG_LEVEL } + autoscaling = { + keda = { + enabled = var.PROVER_AGENT_KEDA_ENABLED && var.PROVER_ENABLED + pollingInterval = var.PROVER_AGENT_KEDA_POLLING_INTERVAL_SECONDS + cooldownPeriod = var.PROVER_AGENT_KEDA_COOLDOWN_PERIOD_SECONDS + minReplicaCount = var.PROVER_AGENT_KEDA_MIN_REPLICAS + maxReplicaCount = var.PROVER_AGENT_KEDA_MAX_REPLICAS + scalingBands = var.PROVER_AGENT_KEDA_SCALING_BANDS + prometheus = { + serverAddress = var.PROVER_AGENT_KEDA_PROMETHEUS_SERVER_ADDRESS + } + } + } } })], local.is_kind ? [yamlencode({ agent = { @@ -377,7 +392,7 @@ locals { "agent.node.env.CRS_PATH" = "/usr/src/crs" "agent.node.proverRealProofs" = var.PROVER_REAL_PROOFS "agent.node.env.PROVER_AGENT_POLL_INTERVAL_MS" = var.PROVER_AGENT_POLL_INTERVAL_MS - "agent.replicaCount" = var.PROVER_REPLICAS + "agent.replicaCount" = var.PROVER_AGENT_KEDA_ENABLED ? "0" : var.PROVER_REPLICAS "agent.node.env.BOOTSTRAP_NODES" = "asdf" "agent.node.env.PROVER_AGENT_COUNT" = var.PROVER_AGENTS_PER_PROVER "agent.node.env.PROVER_TEST_DELAY_TYPE" = var.PROVER_TEST_DELAY_TYPE @@ -410,7 +425,7 @@ locals { boot_node_host_path = "node.node.env.BOOT_NODE_HOST" bootstrap_nodes_path = "node.node.env.BOOTSTRAP_NODES" wait = var.WAIT_FOR_PROVER_DEPLOY - } + } : null rpc = { name = "${var.RELEASE_PREFIX}-rpc" diff --git a/spartan/terraform/deploy-aztec-infra/variables.tf b/spartan/terraform/deploy-aztec-infra/variables.tf index 189ec5309838..5b8c00682983 100644 --- a/spartan/terraform/deploy-aztec-infra/variables.tf +++ b/spartan/terraform/deploy-aztec-infra/variables.tf @@ -264,6 +264,12 @@ variable "PROVER_REPLICAS" { default = 4 } +variable "PROVER_ENABLED" { + description = "Whether to deploy the prover stack" + type = bool + default = true +} + variable "PROVER_TEST_DELAY_TYPE" { description = "The type of test delay to introduce in the prover (fixed, realistic)" type = string @@ -845,6 +851,51 @@ variable "PROVER_AGENT_POLL_INTERVAL_MS" { default = 1000 } +variable "PROVER_AGENT_KEDA_ENABLED" { + description = "Whether KEDA should scale prover agent pods from proving queue depth" + type = bool + default = false +} + +variable "PROVER_AGENT_KEDA_MIN_REPLICAS" { + description = "Minimum prover agent pods managed by KEDA" + type = number + default = 0 +} + +variable "PROVER_AGENT_KEDA_MAX_REPLICAS" { + description = "Maximum prover agent pods managed by KEDA" + type = number + default = 1 +} + +variable "PROVER_AGENT_KEDA_SCALING_BANDS" { + description = "Step scaling bands for prover agents. Each band scales to replicas when total proving queue size is greater than queueSize." + type = list(object({ + queueSize = number + replicas = number + })) + default = [] +} + +variable "PROVER_AGENT_KEDA_PROMETHEUS_SERVER_ADDRESS" { + description = "Prometheus server URL queried by KEDA for prover queue depth" + type = string + default = "" +} + +variable "PROVER_AGENT_KEDA_POLLING_INTERVAL_SECONDS" { + description = "KEDA polling interval for prover agent queue-depth scaling" + type = number + default = 30 +} + +variable "PROVER_AGENT_KEDA_COOLDOWN_PERIOD_SECONDS" { + description = "KEDA cooldown period before scaling prover agents back down" + type = number + default = 300 +} + variable "PROVER_AGENT_INCLUDE_METRICS" { description = "Metrics whitelist in the prover agent" type = string diff --git a/spartan/terraform/deploy-ethereum-nodes/main.tf b/spartan/terraform/deploy-ethereum-nodes/main.tf index 5383738cc6e5..2da78bf96ef3 100644 --- a/spartan/terraform/deploy-ethereum-nodes/main.tf +++ b/spartan/terraform/deploy-ethereum-nodes/main.tf @@ -49,13 +49,6 @@ locals { # we have to mark it as non-sensitive otherwise we can't run for_each on helm_releases :( jwt = nonsensitive(random_bytes.jwt.hex) - resources = { - requests = { - cpu = 2 - memory = "60Gi" - } - } - nodeSelector = { "node-type" = "infra" } @@ -75,12 +68,25 @@ locals { repository = split(":", var.reth_image)[0] tag = split(":", var.reth_image)[1] } + + resources = { + requests = { + cpu = var.reth_cpu_request + memory = var.reth_memory_request + } + limits = { + cpu = var.reth_cpu_limit + memory = var.reth_memory_limit + } + } + p2pNodePort = { enabled = true port = var.reth_p2p_port } extraArgs = [ "--chain=${var.chain}", + # "--storage.v2=true", ] persistence = { @@ -88,6 +94,33 @@ locals { size = var.reth_storage storageClassName = "premium-rwo" } + + # disabled because sepolia db has already been migrated, mainnet has not been migrated yet + #initContainers = [ + # { + # name = "migrate-storage-v2" + # image = var.reth_image + # imagePullPolicy = "Always" + # command = [ + # "sh", + # "-ac", + # <<-EOT + # if [ ! -f /data/db/database.version ]; then + # echo "No existing Reth database found, skipping storage v2 migration." + # exit 0 + # fi + + # reth db --datadir=/data --chain=${var.chain} migrate-v2 + # EOT + # ] + # volumeMounts = [ + # { + # name = "storage" + # mountPath = "/data" + # } + # ] + # } + #] }) ] } @@ -105,6 +138,18 @@ locals { repository = split(":", var.lighthouse_image)[0] tag = split(":", var.lighthouse_image)[1] } + + resources = { + requests = { + cpu = var.lighthouse_cpu_request + memory = var.lighthouse_memory_request + } + limits = { + cpu = var.lighthouse_cpu_limit + memory = var.lighthouse_memory_limit + } + } + checkpointSync = { enabled = true url = var.checkpoint_sync_url diff --git a/spartan/terraform/deploy-ethereum-nodes/mainnet.tfvars b/spartan/terraform/deploy-ethereum-nodes/mainnet.tfvars index 089b015f251a..50f72d6568e3 100644 --- a/spartan/terraform/deploy-ethereum-nodes/mainnet.tfvars +++ b/spartan/terraform/deploy-ethereum-nodes/mainnet.tfvars @@ -1,7 +1,17 @@ namespace = "eth-mainnet" chain = "mainnet" checkpoint_sync_url = "https://mainnet.checkpoint.sigp.io" + reth_p2p_port = 32100 reth_storage = "4Ti" -lighthouse_p2p_port = 32101 -lighthouse_storage = "2Ti" +reth_cpu_request = "1" +reth_cpu_limit = "8" +reth_memory_request = "64Gi" +reth_memory_limit = "112Gi" + +lighthouse_p2p_port = 32101 +lighthouse_storage = "2Ti" +lighthouse_cpu_request = "1" +lighthouse_cpu_limit = "8" +lighthouse_memory_request = "24Gi" +lighthouse_memory_limit = "48Gi" diff --git a/spartan/terraform/deploy-ethereum-nodes/sepolia.tfvars b/spartan/terraform/deploy-ethereum-nodes/sepolia.tfvars index 3d049919ea69..bcae9e1c8389 100644 --- a/spartan/terraform/deploy-ethereum-nodes/sepolia.tfvars +++ b/spartan/terraform/deploy-ethereum-nodes/sepolia.tfvars @@ -1,7 +1,17 @@ namespace = "sepolia" chain = "sepolia" checkpoint_sync_url = "https://checkpoint-sync.sepolia.ethpandaops.io" + reth_p2p_port = 32000 reth_storage = "2Ti" -lighthouse_p2p_port = 32001 -lighthouse_storage = "2Ti" +reth_cpu_request = "1" +reth_cpu_limit = "4" +reth_memory_request = "12Gi" +reth_memory_limit = "24Gi" + +lighthouse_p2p_port = 32001 +lighthouse_storage = "2Ti" +lighthouse_cpu_request = "1" +lighthouse_cpu_limit = "4" +lighthouse_memory_request = "24Gi" +lighthouse_memory_limit = "48Gi" diff --git a/spartan/terraform/deploy-ethereum-nodes/variables.tf b/spartan/terraform/deploy-ethereum-nodes/variables.tf index 691cbbf8255b..7af9f3e7dec5 100644 --- a/spartan/terraform/deploy-ethereum-nodes/variables.tf +++ b/spartan/terraform/deploy-ethereum-nodes/variables.tf @@ -48,7 +48,7 @@ variable "lighthouse_p2p_port" { variable "reth_image" { description = "Reth Docker image" type = string - default = "ghcr.io/paradigmxyz/reth:v2.0.0" + default = "ghcr.io/paradigmxyz/reth:v2.2.0" } variable "reth_chart_version" { @@ -63,16 +63,40 @@ variable "reth_storage" { default = "4Ti" } +variable "reth_cpu_request" { + description = "Reth CPU requests" + type = string + default = "2" +} + +variable "reth_cpu_limit" { + description = "Reth CPU limit" + type = string + default = "4" +} + +variable "reth_memory_request" { + description = "Reth RAM requests" + type = string + default = "16Gi" +} + +variable "reth_memory_limit" { + description = "Reth RAM limit" + type = string + default = "32Gi" +} + variable "lighthouse_image" { description = "Lighthouse Docker image" type = string - default = "sigp/lighthouse:v8.0.1" + default = "sigp/lighthouse:v8.1.3" } variable "lighthouse_chart_version" { description = "Lighthouse Helm chart version" type = string - default = "1.1.7" + default = "1.1.8" } variable "lighthouse_storage" { @@ -80,3 +104,27 @@ variable "lighthouse_storage" { type = string default = "1Ti" } + +variable "lighthouse_cpu_request" { + description = "Lighthouse CPU requests" + type = string + default = "2" +} + +variable "lighthouse_cpu_limit" { + description = "Lighthouse CPU limit" + type = string + default = "4" +} + +variable "lighthouse_memory_request" { + description = "Lighthouse RAM requests" + type = string + default = "16Gi" +} + +variable "lighthouse_memory_limit" { + description = "Lighthouse RAM limit" + type = string + default = "32Gi" +} diff --git a/spartan/terraform/deploy-keda/keda-2.19.0.tgz b/spartan/terraform/deploy-keda/keda-2.19.0.tgz new file mode 100644 index 000000000000..419c21ab4168 Binary files /dev/null and b/spartan/terraform/deploy-keda/keda-2.19.0.tgz differ diff --git a/spartan/terraform/deploy-keda/main.tf b/spartan/terraform/deploy-keda/main.tf new file mode 100644 index 000000000000..934029338508 --- /dev/null +++ b/spartan/terraform/deploy-keda/main.tf @@ -0,0 +1,39 @@ +terraform { + backend "gcs" { + } + required_providers { + helm = { + source = "hashicorp/helm" + version = "~> 2.16.1" + } + } +} + +provider "helm" { + kubernetes { + config_path = "~/.kube/config" + config_context = var.GKE_CLUSTER_CONTEXT + } +} + +resource "helm_release" "keda" { + name = var.RELEASE_NAME + chart = "${path.module}/keda-2.19.0.tgz" + namespace = var.KEDA_NAMESPACE + create_namespace = true + upgrade_install = true + + values = [ + yamlencode({ + crds = { + install = true + } + nodeSelector = { + node-type = "infra" + } + }) + ] + + timeout = 300 + wait = true +} diff --git a/spartan/terraform/deploy-keda/private.tfbackend b/spartan/terraform/deploy-keda/private.tfbackend new file mode 100644 index 000000000000..e6c561284cb4 --- /dev/null +++ b/spartan/terraform/deploy-keda/private.tfbackend @@ -0,0 +1,2 @@ +bucket="aztec-terraform" +prefix="terraform/state/deploy-keda/aztec-gke-private" diff --git a/spartan/terraform/deploy-keda/private.tfvars b/spartan/terraform/deploy-keda/private.tfvars new file mode 100644 index 000000000000..a3dcd6d09193 --- /dev/null +++ b/spartan/terraform/deploy-keda/private.tfvars @@ -0,0 +1 @@ +GKE_CLUSTER_CONTEXT = "gke_testnet-440309_us-west1-a_aztec-gke-private" diff --git a/spartan/terraform/deploy-keda/public.tfbackend b/spartan/terraform/deploy-keda/public.tfbackend new file mode 100644 index 000000000000..a165529713b9 --- /dev/null +++ b/spartan/terraform/deploy-keda/public.tfbackend @@ -0,0 +1,2 @@ +bucket="aztec-terraform" +prefix="terraform/state/deploy-keda/aztec-gke-public" diff --git a/spartan/terraform/deploy-keda/public.tfvars b/spartan/terraform/deploy-keda/public.tfvars new file mode 100644 index 000000000000..46c320b3772f --- /dev/null +++ b/spartan/terraform/deploy-keda/public.tfvars @@ -0,0 +1 @@ +GKE_CLUSTER_CONTEXT = "gke_testnet-440309_us-west1-a_aztec-gke-public" diff --git a/spartan/terraform/deploy-keda/update-chart.sh b/spartan/terraform/deploy-keda/update-chart.sh new file mode 100755 index 000000000000..743ed88e029b --- /dev/null +++ b/spartan/terraform/deploy-keda/update-chart.sh @@ -0,0 +1,14 @@ +#!/bin/bash +set -euo pipefail + +VERSION="${1:?Usage: $0 }" +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +rm "$DIR"/keda-*.tgz + +helm repo add kedacore https://kedacore.github.io/charts --force-update >/dev/null +helm pull kedacore/keda --version "$VERSION" --destination "$DIR" + +sed -i "s/keda-.*\.tgz/keda-${VERSION}.tgz/" "$DIR/main.tf" + +echo "Updated to KEDA $VERSION" diff --git a/spartan/terraform/deploy-keda/variables.tf b/spartan/terraform/deploy-keda/variables.tf new file mode 100644 index 000000000000..c6ac69101569 --- /dev/null +++ b/spartan/terraform/deploy-keda/variables.tf @@ -0,0 +1,18 @@ +variable "GKE_CLUSTER_CONTEXT" { + description = "GKE cluster context" + type = string + default = "gke_testnet-440309_us-west1-a_aztec-gke-private" +} + +variable "RELEASE_NAME" { + description = "Helm release name for KEDA" + type = string + default = "keda" +} + +variable "KEDA_NAMESPACE" { + description = "Namespace to install KEDA into" + type = string + default = "keda" +} + diff --git a/spartan/terraform/deploy-metrics/main.tf b/spartan/terraform/deploy-metrics/main.tf index bf30ce3d59ac..d1a98c03d1a5 100644 --- a/spartan/terraform/deploy-metrics/main.tf +++ b/spartan/terraform/deploy-metrics/main.tf @@ -29,6 +29,11 @@ data "terraform_remote_state" "ssl" { } } +data "google_compute_subnetwork" "default" { + name = "default" + region = var.region +} + resource "google_compute_address" "grafana_ip" { provider = google name = "grafana-ip" @@ -51,6 +56,18 @@ resource "google_compute_address" "otel_collector_ip" { } } +resource "google_compute_address" "prometheus_ip" { + provider = google + name = "prometheus-ip" + address_type = "INTERNAL" + region = var.region + subnetwork = data.google_compute_subnetwork.default.id + + lifecycle { + prevent_destroy = true + } +} + provider "kubernetes" { alias = "gke-cluster" config_path = "~/.kube/config" @@ -209,6 +226,21 @@ resource "helm_release" "aztec-gke-cluster" { value = google_compute_address.otel_collector_ip.address } + set { + name = "prometheus.server.service.type" + value = "LoadBalancer" + } + + set { + name = "prometheus.server.service.annotations.networking\\.gke\\.io\\/load-balancer-type" + value = "Internal" + } + + set { + name = "prometheus.server.service.loadBalancerIP" + value = google_compute_address.prometheus_ip.address + } + set { name = "prometheus.serverFiles.prometheus\\.yml.scrape_configs[0].job_name" value = "prometheus" diff --git a/spartan/terraform/deploy-telemetry/main.tf b/spartan/terraform/deploy-telemetry/main.tf deleted file mode 100644 index b44f1bc0cc28..000000000000 --- a/spartan/terraform/deploy-telemetry/main.tf +++ /dev/null @@ -1,186 +0,0 @@ -terraform { - backend "gcs" { - bucket = "aztec-terraform" - prefix = "metrics-deploy/us-west1-a/aztec-gke-private/telemetry/terraform.tfstate" - } - required_providers { - helm = { - source = "hashicorp/helm" - version = "~> 2.16.1" - } - kubernetes = { - source = "hashicorp/kubernetes" - version = "~> 2.24.0" - } - } -} - -provider "google" { - project = var.project - region = var.region -} - -provider "kubernetes" { - alias = "gke-cluster" - config_path = "~/.kube/config" - config_context = var.cluster -} - -provider "helm" { - alias = "gke-cluster" - kubernetes { - config_path = "~/.kube/config" - config_context = var.cluster - } -} - -resource "google_compute_global_address" "otel_collector_ingress" { - provider = google - name = "${var.RELEASE_NAME}-otel-collector-ingress" - address_type = "EXTERNAL" - - lifecycle { - prevent_destroy = true - } -} - -resource "kubernetes_namespace" "ns" { - provider = kubernetes.gke-cluster - metadata { - name = var.RELEASE_NAME - } -} - -resource "kubernetes_manifest" "otel_ingress_certificate" { - provider = kubernetes.gke-cluster - - manifest = { - "apiVersion" = "networking.gke.io/v1" - "kind" = "ManagedCertificate" - "metadata" = { - "name" = "otel-ingress-cert" - "namespace" = kubernetes_namespace.ns.metadata[0].name - } - "spec" = { - "domains" = var.HOSTS - } - } -} - -resource "kubernetes_manifest" "otel_ingress_backend" { - provider = kubernetes.gke-cluster - - manifest = { - "apiVersion" = "cloud.google.com/v1" - "kind" = "BackendConfig" - "metadata" = { - "name" = "otel-ingress-backend" - "namespace" = kubernetes_namespace.ns.metadata[0].name - } - "spec" = { - "healthCheck" = { - "checkIntervalSec" = 15 - "timeoutSec" = 5 - "type" = "HTTP" - "port" = 13133 - "requestPath" = "/" - } - } - } -} - -locals { - prefixes = jsondecode(file("../../../yarn-project/cli/public_include_metric_prefixes.json")) - registries = ["0xec4156431d0f3df66d4e24ba3d30dcb4c85fa309", "0xf299347e765cfb27f913bde8e4983fd0f195676f", "0x2e48addca360da61e4d6c21ff2b1961af56eb83b", "0xc2f24280f5c7f4897370dfdeb30f79ded14f1c81"] - roles = ["sequencer"] - - otel_metric_allowlist = join(" or ", formatlist("HasPrefix(name, %q)", local.prefixes)) - otel_registry_allowlist = join(" or ", formatlist("resource.attributes[\"aztec.registry_address\"] == %q", local.registries)) - otel_role_allowlist = join(" or ", formatlist("resource.attributes[\"aztec.node_role\"] == %q", local.roles)) -} - -resource "helm_release" "otel_collector" { - provider = helm.gke-cluster - name = "otel" - namespace = kubernetes_namespace.ns.metadata[0].name - repository = "https://open-telemetry.github.io/opentelemetry-helm-charts" - chart = "opentelemetry-collector" - version = "0.127.2" - create_namespace = false - upgrade_install = true - dependency_update = true - force_update = true - reuse_values = false - reset_values = true - - # base values file - values = [ - file("./values/public-otel-collector.yaml"), - yamlencode({ - ingress = { - hosts = [ - for index, host in var.HOSTS : ({ - host = host - paths = [ - { - path = "/" - pathType = "Prefix" - port = 4318 - } - ] - }) - ] - } - }), - # have to use a heredoc because of quotation issues with OTTL - <<-EOF -config: - processors: - filter: - metrics: - metric: - - 'not (${local.otel_registry_allowlist})' - - 'not (${local.otel_role_allowlist})' - - 'not (${local.otel_metric_allowlist})' -EOF - ] - - set { - name = "ingress.annotations.kubernetes\\.io\\/ingress\\.global-static-ip-name" - value = google_compute_global_address.otel_collector_ingress.name - } - - set { - name = "ingress.annotations.networking\\.gke\\.io\\/managed-certificates" - value = "otel-ingress-cert" - } - - timeout = 300 - wait = true - wait_for_jobs = true - atomic = true - cleanup_on_fail = true -} - -resource "helm_release" "public_prometheus" { - provider = helm.gke-cluster - name = "prometheus" - namespace = kubernetes_namespace.ns.metadata[0].name - repository = "https://prometheus-community.github.io/helm-charts" - chart = "prometheus" - version = "25.27.0" - create_namespace = false - upgrade_install = true - dependency_update = true - force_update = true - reuse_values = false - reset_values = true - - values = [file("./values/public-prometheus.yaml")] - - timeout = 300 - wait = true - wait_for_jobs = true - atomic = true - cleanup_on_fail = true -} diff --git a/spartan/terraform/deploy-telemetry/outputs.tf b/spartan/terraform/deploy-telemetry/outputs.tf deleted file mode 100644 index 693227f05012..000000000000 --- a/spartan/terraform/deploy-telemetry/outputs.tf +++ /dev/null @@ -1,9 +0,0 @@ -output "otel_ingress_hostname" { - description = "Public otel ingress" - value = "https://${var.HOSTS[0]}" -} - -output "otel_ingress_ip" { - description = "Public otel ingress IP address" - value = google_compute_global_address.otel_collector_ingress.address -} diff --git a/spartan/terraform/deploy-telemetry/values/public-otel-collector.yaml b/spartan/terraform/deploy-telemetry/values/public-otel-collector.yaml deleted file mode 100644 index 2e18a2d16667..000000000000 --- a/spartan/terraform/deploy-telemetry/values/public-otel-collector.yaml +++ /dev/null @@ -1,155 +0,0 @@ -mode: statefulset -replicaCount: 1 - -nodeSelector: - node-type: infra - -image: - repository: ghcr.io/open-telemetry/opentelemetry-collector-releases/opentelemetry-collector-contrib - tag: "0.128.0" - -resources: - requests: - memory: 12Gi - cpu: "2" - limits: - memory: 60Gi - cpu: "7" - -ports: - otlp: - enabled: false - otlp-http: - enabled: true - containerPort: 4318 - servicePort: 4318 - hostPort: 4318 - protocol: TCP - jaeger-compact: - enabled: false - jaeger-thrift: - enabled: false - jaeger-grpc: - enabled: false - zipkin: - enabled: false - -config: - extensions: - health_check: - endpoint: ${env:MY_POD_IP}:13133 - - receivers: - jaeger: {} - prometheus: {} - zipkin: {} - otlp: - protocols: - grpc: {} - http: - endpoint: ${env:MY_POD_IP}:4318 - - processors: - memory_limiter: - check_interval: 1s - limit_mib: 12000 - spike_limit_mib: 2000 - - filter: - metrics: - metric: [] # placeholder - datapoint: - - 'metric.type == METRIC_DATA_TYPE_HISTOGRAM and Len(explicit_bounds) > 20' - - resource: - attributes: - - pattern: "(k8s|os|telemetry|service|exported).*" - action: delete - - transform: - metric_statements: - - context: datapoint - statements: - - set(attributes["aztec.node_role"], resource.attributes["aztec.node_role"]) - - set(attributes["aztec.registry_address"], resource.attributes["aztec.registry_address"]) - - batch: {} - - exporters: - prometheus: - endpoint: ${env:MY_POD_IP}:8889 - namespace: external - metric_expiration: 5m - resource_to_telemetry_conversion: - enabled: false - - service: - telemetry: - metrics: - address: ${env:MY_POD_IP}:8888 - - pipelines: - logs: null - traces: null - - metrics: - receivers: - - otlp - processors: - - memory_limiter - - resource - - filter - - transform - - batch - exporters: - - prometheus - -ports: - otlp: - enabled: false - otlp-http: - enabled: true - jaeger-compact: - enabled: false - jaeger-thrift: - enabled: false - jaeger-grpc: - enabled: false - zipkin: - enabled: false - metrics: - enabled: false - healthcheck: - enabled: true - containerPort: 13133 - servicePort: 13133 - hostPort: 13133 - protocol: TCP - prom-otel: - enabled: true - containerPort: 8888 - servicePort: 8888 - hostPort: 8888 - protocol: TCP - prom-aztec: - enabled: true - containerPort: 8889 - servicePort: 8889 - hostPort: 8889 - protocol: TCP -service: - enabled: true - annotations: - cloud.google.com/backend-config: "{\"default\":\"otel-ingress-backend\"}" - cloud.google.com/neg: "{\"ingress\": true}" - -ingress: - enabled: true - annotations: - kubernetes.io/ingress.allow-http: "true" - kubernetes.io/ingress.class: gce - kubernetes.io/ingress.global-static-ip-name: "" - cloud.google.com/healthcheck-port: "13133" - cloud.google.com/healthcheck-path: "/" - - # networking.gke.io/managed-certificates: null diff --git a/spartan/terraform/deploy-telemetry/values/public-prometheus.yaml b/spartan/terraform/deploy-telemetry/values/public-prometheus.yaml deleted file mode 100644 index c25335185818..000000000000 --- a/spartan/terraform/deploy-telemetry/values/public-prometheus.yaml +++ /dev/null @@ -1,39 +0,0 @@ -server: - global: - evaluation_interval: 30s - scrape_interval: 1m - scrape_timeout: 20s - resources: - requests: - memory: 40Gi - cpu: "4" - limits: - memory: 60Gi - cpu: "7" - nodeSelector: - node-type: infra - persistentVolume: - enabled: true - size: 100Gi - replicaCount: 1 - statefulSet: - enabled: true - -serverFiles: - prometheus.yml: - scrape_configs: - - job_name: public_telemetry - kubernetes_sd_configs: - - role: pod - namespaces: - own_namespace: true - names: [] - -alertmanager: - enabled: false -prometheus-node-exporter: - enabled: false -prometheus-pushgateway: - enabled: false -kube-state-metrics: - enabled: false diff --git a/spartan/terraform/deploy-telemetry/variables.tf b/spartan/terraform/deploy-telemetry/variables.tf deleted file mode 100644 index 55f7f8c4cca4..000000000000 --- a/spartan/terraform/deploy-telemetry/variables.tf +++ /dev/null @@ -1,27 +0,0 @@ -variable "cluster" { - description = "GKE cluster context" - type = string - default = "gke_testnet-440309_us-west1-a_aztec-gke-private" -} - -variable "project" { - default = "testnet-440309" - type = string -} - -variable "region" { - default = "us-west1" - type = string -} - -variable "RELEASE_NAME" { - description = "Name of helm deployment and k8s namespace" - type = string - default = "public-telemetry" -} - -variable "HOSTS" { - description = "The public hostname for the ingress" - type = list(string) - default = ["telemetry.alpha-testnet.aztec-labs.com"] -} diff --git a/spartan/terraform/modules/validator-ha-postgres/main.tf b/spartan/terraform/modules/validator-ha-postgres/main.tf index 73e63981bd94..e7f4ba3e4b3a 100644 --- a/spartan/terraform/modules/validator-ha-postgres/main.tf +++ b/spartan/terraform/modules/validator-ha-postgres/main.tf @@ -51,6 +51,9 @@ resource "helm_release" "postgres" { enabled = true size = var.STORAGE_SIZE } + nodeSelector = { + "node-type" = "network" + } })] timeout = 300 @@ -68,6 +71,9 @@ resource "kubernetes_job_v1" "migrations" { template { metadata {} spec { + node_selector = { + "node-type" = "network" + } container { name = "migrate" image = var.AZTEC_DOCKER_IMAGE diff --git a/spartan/terraform/modules/web3signer/main.tf b/spartan/terraform/modules/web3signer/main.tf index af5ce6ff56f4..7cf255f87410 100644 --- a/spartan/terraform/modules/web3signer/main.tf +++ b/spartan/terraform/modules/web3signer/main.tf @@ -77,6 +77,19 @@ resource "helm_release" "web3signer" { repository = split(":", var.WEB3SIGNER_DOCKER_IMAGE)[0] tag = split(":", var.WEB3SIGNER_DOCKER_IMAGE)[1] } + nodeSelector = { + "node-type" = "network" + } + resources = { + requests = { + cpu = "100m" + memory = "512Mi" + } + limits = { + cpu = "1" + memory = "2Gi" + } + } extraVolumes = [ { name = "keystores" diff --git a/yarn-project/.codex b/yarn-project/.codex deleted file mode 120000 index c8161850a43d..000000000000 --- a/yarn-project/.codex +++ /dev/null @@ -1 +0,0 @@ -.claude \ No newline at end of file diff --git a/yarn-project/.codex/agents b/yarn-project/.codex/agents new file mode 120000 index 000000000000..0efb85ec44f4 --- /dev/null +++ b/yarn-project/.codex/agents @@ -0,0 +1 @@ +../.claude/agents \ No newline at end of file diff --git a/yarn-project/.codex/scripts b/yarn-project/.codex/scripts new file mode 120000 index 000000000000..26e538548009 --- /dev/null +++ b/yarn-project/.codex/scripts @@ -0,0 +1 @@ +../.claude/scripts \ No newline at end of file diff --git a/yarn-project/.codex/settings.json b/yarn-project/.codex/settings.json new file mode 120000 index 000000000000..11a726369cef --- /dev/null +++ b/yarn-project/.codex/settings.json @@ -0,0 +1 @@ +../.claude/settings.json \ No newline at end of file diff --git a/yarn-project/.codex/skills b/yarn-project/.codex/skills new file mode 120000 index 000000000000..454b8427cd75 --- /dev/null +++ b/yarn-project/.codex/skills @@ -0,0 +1 @@ +../.claude/skills \ No newline at end of file diff --git a/yarn-project/archiver/src/archiver-sync.test.ts b/yarn-project/archiver/src/archiver-sync.test.ts index f4898e6791f8..626bae272e39 100644 --- a/yarn-project/archiver/src/archiver-sync.test.ts +++ b/yarn-project/archiver/src/archiver-sync.test.ts @@ -689,6 +689,13 @@ describe('Archiver Sync', () => { const invalidCheckpointDetectedSpy = jest.fn(); archiver.events.on(L2BlockSourceEvents.InvalidAttestationsCheckpointDetected, invalidCheckpointDetectedSpy); + // And another spy for DescendentOfInvalidAttestationsCheckpointDetected, which fires only for a + // checkpoint with VALID attestations that builds on a rejected ancestor. CP3 here has invalid + // attestations of its own, so it is caught by the attestation check first and should never + // reach the descendant path — this spy must not fire in this test. + const descendantOfInvalidSpy = jest.fn(); + archiver.events.on(L2BlockSourceEvents.DescendentOfInvalidAttestationsCheckpointDetected, descendantOfInvalidSpy); + // Add valid checkpoint 1 with correct attestations const { checkpoint: cp1 } = await fake.addCheckpoint(CheckpointNumber(1), { l1BlockNumber: 70n, @@ -780,21 +787,24 @@ describe('Archiver Sync', () => { expect(validationStatus.checkpoint.checkpointNumber).toEqual(2); expect(validationStatus.checkpoint.archive.toString()).toEqual(badCp2b.archive.root.toString()); - // Check that event was also emitted for bad CP3 + // CP3 has invalid attestations of its own, so it is caught by the attestation check (which + // runs before the descendant-of-invalid check) and surfaced as an + // InvalidAttestationsCheckpointDetected event — NOT a descendant event — even though it also + // builds on the rejected bad CP2b. + expect(descendantOfInvalidSpy).not.toHaveBeenCalled(); + + // Should have been called 3 times for invalid attestations: bad CP2, bad CP2b, bad CP3 + expect(invalidCheckpointDetectedSpy).toHaveBeenCalledTimes(3); expect(invalidCheckpointDetectedSpy).toHaveBeenCalledWith( expect.objectContaining({ type: L2BlockSourceEvents.InvalidAttestationsCheckpointDetected, validationResult: expect.objectContaining({ valid: false, - reason: 'invalid-attestation', checkpoint: expect.objectContaining({ checkpointNumber: 3 }), }), }), ); - // Should have been called 3 times: bad CP2, bad CP2b, bad CP3 - expect(invalidCheckpointDetectedSpy).toHaveBeenCalledTimes(3); - // Now recover: remove bad checkpoints and add good CP2 and CP3 with valid attestations // Good checkpoints have messages that the archiver will validate logger.warn('Fourth sync: adding good CP2 and CP3 with correct attestations'); @@ -831,6 +841,76 @@ describe('Archiver Sync', () => { // With a valid pending chain validation status expect(await archiver.getPendingChainValidationStatus()).toEqual(expect.objectContaining({ valid: true })); }, 15_000); + + it('skips a valid-attestations checkpoint that builds on a rejected ancestor', async () => { + // Regression for the archiver "non-consecutive checkpoint" retry loop: when a checkpoint + // with insufficient/invalid attestations is followed by a valid-attestations descendant, + // addCheckpoints used to throw InitialCheckpointNumberNotSequentialError and loop on the + // catch handler's L1-sync-point rollback. Now the descendant is detected, skipped, and + // surfaced via DescendentOfInvalidAttestationsCheckpointDetected. + expect(await archiver.getCheckpointNumber()).toEqual(CheckpointNumber(0)); + + fake.setTargetCommitteeSize(3); + const signers = times(3, Secp256k1Signer.random); + const committee = signers.map(s => s.address); + epochCache.getCommitteeForEpoch.mockResolvedValue({ committee, seed: 0n } as EpochCommitteeInfo); + + const descendantOfInvalidSpy = jest.fn(); + archiver.events.on(L2BlockSourceEvents.DescendentOfInvalidAttestationsCheckpointDetected, descendantOfInvalidSpy); + + // Valid CP1 + const { checkpoint: cp1 } = await fake.addCheckpoint(CheckpointNumber(1), { + l1BlockNumber: 70n, + messagesL1BlockNumber: 50n, + numL1ToL2Messages: 3, + signers, + }); + const cp1Archive = cp1.blocks.at(-1)!.archive; + + // Bad CP2 (insufficient attestations — random signers not in committee) + const badSigners = times(3, Secp256k1Signer.random); + const { checkpoint: badCp2 } = await fake.addCheckpoint(CheckpointNumber(2), { + l1BlockNumber: 80n, + numL1ToL2Messages: 0, + signers: badSigners, + previousArchive: cp1Archive, + }); + + // Valid-attestations CP3 chained from bad CP2: this is the case that used to wedge the + // synchronizer. + const { checkpoint: validCp3 } = await fake.addCheckpoint(CheckpointNumber(3), { + l1BlockNumber: 82n, + numL1ToL2Messages: 0, + signers, + previousArchive: badCp2.blocks.at(-1)!.archive, + }); + + fake.setL1BlockNumber(85n); + await archiver.syncImmediate(); + + // Archiver should have stayed at CP1 (skipped both CP2 and CP3) without throwing. + expect(await archiver.getCheckpointNumber()).toEqual(CheckpointNumber(1)); + + // The descendant event should have fired for CP3 with the bad CP2 ancestor. + expect(descendantOfInvalidSpy).toHaveBeenCalledWith( + expect.objectContaining({ + type: L2BlockSourceEvents.DescendentOfInvalidAttestationsCheckpointDetected, + checkpoint: expect.objectContaining({ + checkpointNumber: 3, + archive: validCp3.archive.root, + }), + ancestorArchiveRoot: badCp2.archive.root, + ancestorCheckpointNumber: 2, + }), + ); + expect(descendantOfInvalidSpy).toHaveBeenCalledTimes(1); + + // The rejected entries should persist in the store, keyed by their own archive roots. + const rejectedBad = await archiverStore.blocks.getRejectedCheckpointByArchiveRoot(badCp2.archive.root); + const rejectedValid = await archiverStore.blocks.getRejectedCheckpointByArchiveRoot(validCp3.archive.root); + expect(rejectedBad).toBeDefined(); + expect(rejectedValid).toBeDefined(); + }, 15_000); }); describe('reorg handling', () => { diff --git a/yarn-project/archiver/src/errors.ts b/yarn-project/archiver/src/errors.ts index 64fae2aa69dd..6de0234d9585 100644 --- a/yarn-project/archiver/src/errors.ts +++ b/yarn-project/archiver/src/errors.ts @@ -31,13 +31,13 @@ export class InitialCheckpointNumberNotSequentialError extends Error { export class CheckpointNumberNotSequentialError extends Error { constructor( - newCheckpointNumber: CheckpointNumber, - previous: CheckpointNumber | undefined, + public readonly newCheckpointNumber: CheckpointNumber, + public readonly previousCheckpointNumber: CheckpointNumber | undefined, source?: 'proposed' | 'confirmed', ) { const qualifier = source ? `${source} ` : ''; super( - `Cannot insert new checkpoint ${newCheckpointNumber} given previous ${qualifier}checkpoint number is ${previous ?? 'undefined'}`, + `Cannot insert new checkpoint ${newCheckpointNumber} given previous ${qualifier}checkpoint number is ${previousCheckpointNumber ?? 'undefined'}`, ); this.name = 'CheckpointNumberNotSequentialError'; } diff --git a/yarn-project/archiver/src/modules/l1_synchronizer.ts b/yarn-project/archiver/src/modules/l1_synchronizer.ts index a0daa21d7c83..3d12a9d58a4a 100644 --- a/yarn-project/archiver/src/modules/l1_synchronizer.ts +++ b/yarn-project/archiver/src/modules/l1_synchronizer.ts @@ -17,7 +17,7 @@ import { count } from '@aztec/foundation/string'; import { DateProvider, Timer, elapsed } from '@aztec/foundation/timer'; import { isDefined, isErrorClass } from '@aztec/foundation/types'; import { type ArchiverEmitter, L2BlockSourceEvents, type ValidateCheckpointResult } from '@aztec/stdlib/block'; -import { Checkpoint, PublishedCheckpoint } from '@aztec/stdlib/checkpoint'; +import { Checkpoint, type CheckpointData, PublishedCheckpoint } from '@aztec/stdlib/checkpoint'; import { type L1RollupConstants, getEpochAtSlot, getSlotAtNextL1Block } from '@aztec/stdlib/epoch-helpers'; import { computeInHashFromL1ToL2Messages } from '@aztec/stdlib/messaging'; import type { CoordinationSignatureContext } from '@aztec/stdlib/p2p'; @@ -32,6 +32,7 @@ import { retrieveL1ToL2Messages, retrievedToPublishedCheckpoint, } from '../l1/data_retrieval.js'; +import type { RejectedCheckpoint } from '../store/block_store.js'; import { type ArchiverDataStores, getArchiverSynchPoint } from '../store/data_stores.js'; import type { L2TipsCache } from '../store/l2_tips_cache.js'; import { MessageStoreError } from '../store/message_store.js'; @@ -46,13 +47,15 @@ type RollupStatus = { pendingCheckpointNumber: CheckpointNumber; pendingArchive: string; validationResult: ValidateCheckpointResult | undefined; + /** Last valid checkpoint observed on L1 and synced on this iteration */ lastRetrievedCheckpoint?: PublishedCheckpoint; - lastL1BlockWithCheckpoint?: bigint; + /** Last checkpoint observed on L1 across both valid and rejected entries on this iteration */ + lastSeenCheckpoint?: PublishedCheckpoint; }; /** * Handles L1 synchronization for the archiver. - * Responsible for fetching checkpoints, L1→L2 messages, and handling L1 reorgs. + * Responsible for fetching checkpoints, L1 to L2 messages, and handling L1 reorgs. */ export class ArchiverL1Synchronizer implements Traceable { private l1BlockNumber: bigint | undefined; @@ -202,18 +205,10 @@ export class ArchiverL1Synchronizer implements Traceable { currentL1Timestamp, ); - // If the last checkpoint we processed had an invalid attestation, we manually advance the L1 syncpoint - // past it, since otherwise we'll keep downloading it and reprocessing it on every iteration until - // we get a valid checkpoint to advance the syncpoint. - if (!rollupStatus.validationResult?.valid && rollupStatus.lastL1BlockWithCheckpoint !== undefined) { - await this.stores.blocks.setSynchedL1BlockNumber(rollupStatus.lastL1BlockWithCheckpoint); - } - // And lastly we check if we are missing any checkpoints behind us due to a possible L1 reorg. // We only do this if rollup cant prune on the next submission. Otherwise we will end up - // re-syncing the checkpoints we have just unwound above. We also dont do this if the last checkpoint is invalid, - // since the archiver will rightfully refuse to sync up to it. - if (!rollupCanPrune && rollupStatus.validationResult?.valid) { + // re-syncing the checkpoints we have just unwound above. + if (!rollupCanPrune) { await this.checkForNewCheckpointsBeforeL1SyncPoint(rollupStatus, blocksSynchedTo, currentL1BlockNumber); } @@ -796,7 +791,7 @@ export class ArchiverL1Synchronizer implements Traceable { let searchStartBlock: bigint = blocksSynchedTo; let searchEndBlock: bigint = blocksSynchedTo; let lastRetrievedCheckpoint: PublishedCheckpoint | undefined; - let lastL1BlockWithCheckpoint: bigint | undefined = undefined; + let lastSeenCheckpoint: PublishedCheckpoint | undefined; do { [searchStartBlock, searchEndBlock] = this.nextRange(searchEndBlock, currentL1BlockNumber); @@ -865,6 +860,9 @@ export class ArchiverL1Synchronizer implements Traceable { // Now loop through all checkpoints and validate their attestations for (const published of publishedCheckpoints) { + // Check the attestations uploaded by the publisher to L1 are correct + // Rollup contract does not validate attestations to save on gas, so this + // falls on the nodes to verify offchain and skip those checkpoints. const validationResult = this.config.skipValidateCheckpointAttestations ? { valid: true as const } : await validateCheckpointAttestations( @@ -875,17 +873,29 @@ export class ArchiverL1Synchronizer implements Traceable { this.log, ); - // Only update the validation result if it has changed, so we can keep track of the first invalid checkpoint + // Also skip the checkpoint if it builds on a previously-rejected ancestor. Without + // this, addCheckpoints would throw InitialCheckpointNumberNotSequentialError when the + // ancestor was skipped earlier (e.g. due to invalid attestations), the catch handler + // would roll back the L1 sync point, and the next iteration would re-fetch and re-throw. + const rejectedAncestor = await this.stores.blocks.getRejectedCheckpointByArchiveRoot( + published.checkpoint.header.lastArchiveRoot, + ); + + // Update the validation result if it has changed, so we can keep track of the first invalid checkpoint // in case there is a sequence of more than one invalid checkpoint, as we need to invalidate the first one. // There is an exception though: if a checkpoint is invalidated and replaced with another invalid checkpoint, // we need to update the validation result, since we need to be able to invalidate the new one. // See test 'chain progresses if an invalid checkpoint is invalidated with an invalid one' for more info. - if ( - rollupStatus.validationResult?.valid !== validationResult.valid || - (!rollupStatus.validationResult.valid && - !validationResult.valid && - rollupStatus.validationResult.checkpoint.checkpointNumber === validationResult.checkpoint.checkpointNumber) - ) { + // Do not update the validation result if there is a rejected ancestor, since in that case we want to keep the + // original invalidation, as the new checkpoint is extending from a previous invalid one. + const validStatusChanged = rollupStatus.validationResult?.valid !== validationResult.valid; + const invalidStatusWithSameCheckpointNumber = + !validationResult.valid && + rollupStatus.validationResult && + !rollupStatus.validationResult.valid && + rollupStatus.validationResult.checkpoint.checkpointNumber === validationResult.checkpoint.checkpointNumber; + + if (!rejectedAncestor && (validStatusChanged || invalidStatusWithSameCheckpointNumber)) { rollupStatus.validationResult = validationResult; } @@ -902,9 +912,55 @@ export class ArchiverL1Synchronizer implements Traceable { validationResult, }); - // We keep consuming checkpoints if we find an invalid one, since we do not listen for CheckpointInvalidated events - // We just pretend the invalid ones are not there and keep consuming the next checkpoints - // Note that this breaks if the committee ever attests to a descendant of an invalid checkpoint + // Persist a rejected-ancestor entry so any later checkpoint that builds on this one + // is detected and skipped (rather than tripping the addCheckpoints consecutive-number + // check and causing the sync point to roll back in a loop). + await this.stores.blocks.addRejectedCheckpoint({ + checkpointNumber: published.checkpoint.number, + archiveRoot: published.checkpoint.archive.root, + parentArchiveRoot: published.checkpoint.header.lastArchiveRoot, + slotNumber: published.checkpoint.header.slotNumber, + l1: published.l1, + reason: 'invalid-attestations' as const, + }); + + continue; + } + + if (rejectedAncestor) { + const descendantInfo = published.checkpoint.toCheckpointInfo(); + this.log.warn( + `Skipping checkpoint ${published.checkpoint.number} as it is a descendant of ` + + `rejected checkpoint ${rejectedAncestor.checkpointNumber} (${rejectedAncestor.reason})`, + { + checkpointNumber: published.checkpoint.number, + checkpointHash: published.checkpoint.hash(), + l1BlockNumber: published.l1.blockNumber, + l1BlockHash: published.l1.blockHash, + ancestorCheckpointNumber: rejectedAncestor.checkpointNumber, + ancestorArchiveRoot: rejectedAncestor.archiveRoot.toString(), + ancestorReason: rejectedAncestor.reason, + }, + ); + + this.events.emit(L2BlockSourceEvents.DescendentOfInvalidAttestationsCheckpointDetected, { + type: L2BlockSourceEvents.DescendentOfInvalidAttestationsCheckpointDetected, + checkpoint: descendantInfo, + ancestorArchiveRoot: rejectedAncestor.archiveRoot, + ancestorCheckpointNumber: rejectedAncestor.checkpointNumber, + }); + + // Persist this chainpoint as rejected as well, so we can construct a chain of + // skipped checkpoints starting from the first one with invalid attestations. + await this.stores.blocks.addRejectedCheckpoint({ + checkpointNumber: published.checkpoint.number, + archiveRoot: published.checkpoint.archive.root, + parentArchiveRoot: published.checkpoint.header.lastArchiveRoot, + slotNumber: published.checkpoint.header.slotNumber, + l1: published.l1, + reason: 'descends-from-invalid-attestations' as const, + }); + continue; } @@ -1005,12 +1061,21 @@ export class ArchiverL1Synchronizer implements Traceable { const previousCheckpoint = previousCheckpointNumber ? await this.stores.blocks.getCheckpointData(CheckpointNumber(previousCheckpointNumber)) : undefined; - const updatedL1SyncPoint = previousCheckpoint?.l1.blockNumber ?? this.l1Constants.l1StartBlock; + const lastFinalizedCheckpoint = await this.stores.blocks.getCheckpointData( + await this.stores.blocks.getFinalizedCheckpointNumber(), + ); + const updatedL1SyncPoint = + previousCheckpoint?.l1.blockNumber ?? + lastFinalizedCheckpoint?.l1.blockNumber ?? + this.l1Constants.l1StartBlock; await this.stores.blocks.setSynchedL1BlockNumber(updatedL1SyncPoint); this.log.warn( `Attempting to insert checkpoint ${newCheckpointNumber} with previous block ${previousCheckpointNumber}. Rolling back L1 sync point to ${updatedL1SyncPoint} to try and fetch the missing blocks.`, { previousCheckpointNumber, + previousCheckpoint: previousCheckpoint?.header.toInspect(), + lastFinalizedCheckpoint: lastFinalizedCheckpoint?.header.toInspect(), + l1StartBlock: this.l1Constants.l1StartBlock, newCheckpointNumber, updatedL1SyncPoint, }, @@ -1031,13 +1096,13 @@ export class ArchiverL1Synchronizer implements Traceable { }); } lastRetrievedCheckpoint = validCheckpoints.at(-1) ?? lastRetrievedCheckpoint; - lastL1BlockWithCheckpoint = calldataCheckpoints.at(-1)?.l1.blockNumber ?? lastL1BlockWithCheckpoint; + lastSeenCheckpoint = publishedCheckpoints.at(-1) ?? lastSeenCheckpoint; } while (searchEndBlock < currentL1BlockNumber); // Important that we update AFTER inserting the blocks. await updateProvenCheckpoint(); - return { ...rollupStatus, lastRetrievedCheckpoint, lastL1BlockWithCheckpoint }; + return { ...rollupStatus, lastRetrievedCheckpoint, lastSeenCheckpoint }; } /** @@ -1137,35 +1202,38 @@ export class ArchiverL1Synchronizer implements Traceable { blocksSynchedTo: bigint, currentL1BlockNumber: bigint, ): Promise { - const { lastRetrievedCheckpoint, pendingCheckpointNumber } = status; - // Compare the last checkpoint we have (either retrieved in this round or loaded from store) with what the - // rollup contract told us was the latest one (pinned at the currentL1BlockNumber). + const { lastSeenCheckpoint, pendingCheckpointNumber } = status; + // Compare the last checkpoint (valid or not) we have (either retrieved in this round or loaded from store) + // with what the rollup contract told us was the latest one (pinned at the currentL1BlockNumber). const latestLocalCheckpointNumber = - lastRetrievedCheckpoint?.checkpoint.number ?? (await this.stores.blocks.getLatestCheckpointNumber()); + lastSeenCheckpoint?.checkpoint.number ?? + CheckpointNumber.max( + await this.stores.blocks.getLatestCheckpointNumber(), + await this.stores.blocks.getLatestRejectedCheckpointNumber(), + ) ?? + CheckpointNumber.ZERO; + if (latestLocalCheckpointNumber < pendingCheckpointNumber) { // Here we have consumed all logs until the `currentL1Block` we pinned at the beginning of the archiver loop, // but still haven't reached the pending checkpoint according to the call to the rollup contract. // We suspect an L1 reorg that added checkpoints *behind* us. If that is the case, it must have happened between // the last checkpoint we saw and the current one, so we reset the last synched L1 block number. In the edge case // we don't have one, we go back 2 L1 epochs, which is the deepest possible reorg (assuming Casper is working). - let latestLocalCheckpointArchive: string | undefined = undefined; - let targetL1BlockNumber = maxBigint(currentL1BlockNumber - 64n, 0n); - if (lastRetrievedCheckpoint) { - latestLocalCheckpointArchive = lastRetrievedCheckpoint.checkpoint.archive.root.toString(); - targetL1BlockNumber = lastRetrievedCheckpoint.l1.blockNumber; - } else if (latestLocalCheckpointNumber > 0) { - const checkpoint = await this.stores.blocks - .getRangeOfCheckpoints(latestLocalCheckpointNumber, 1) - .then(([c]) => c); - latestLocalCheckpointArchive = checkpoint.archive.root.toString(); - targetL1BlockNumber = checkpoint.l1.blockNumber; - } + const latestLocalCheckpoint: PublishedCheckpoint | CheckpointData | RejectedCheckpoint | undefined = + lastSeenCheckpoint ?? + (await this.stores.blocks.getCheckpointData(latestLocalCheckpointNumber)) ?? + (await this.stores.blocks.getRejectedCheckpointByNumber(latestLocalCheckpointNumber)); + + const targetL1BlockNumber = + latestLocalCheckpoint?.l1.blockNumber ?? + maxBigint(currentL1BlockNumber - 64n, this.l1Constants.l1StartBlock, 0n); + this.log.warn( `Failed to reach checkpoint ${pendingCheckpointNumber} at ${currentL1BlockNumber} (latest is ${latestLocalCheckpointNumber}). ` + `Rolling back last synched L1 block number to ${targetL1BlockNumber}.`, { latestLocalCheckpointNumber, - latestLocalCheckpointArchive, + latestLocalCheckpointL1: latestLocalCheckpoint?.l1, blocksSynchedTo, currentL1BlockNumber, ...status, diff --git a/yarn-project/archiver/src/store/block_store.test.ts b/yarn-project/archiver/src/store/block_store.test.ts index 29c6126a1ec0..d093bbf30784 100644 --- a/yarn-project/archiver/src/store/block_store.test.ts +++ b/yarn-project/archiver/src/store/block_store.test.ts @@ -2833,4 +2833,87 @@ describe('BlockStore', () => { expect(await blockStore.getBlock({ number: BlockNumber(2) })).toBeUndefined(); }); }); + + describe('rejected checkpoints', () => { + const makeEntry = (overrides: { archiveRoot?: Fr; l1BlockNumber?: number; checkpointNumber?: number } = {}) => ({ + checkpointNumber: CheckpointNumber(overrides.checkpointNumber ?? 1), + archiveRoot: overrides.archiveRoot ?? Fr.random(), + parentArchiveRoot: Fr.random(), + slotNumber: SlotNumber(1), + l1: makeL1PublishedData(overrides.l1BlockNumber ?? 100), + reason: 'invalid-attestations' as const, + }); + + it('returns an empty result when no rejected checkpoints have been recorded', async () => { + expect(await blockStore.getRejectedCheckpointByArchiveRoot(Fr.random())).toBeUndefined(); + expect(await blockStore.getLatestRejectedCheckpointNumber()).toEqual( + CheckpointNumber(INITIAL_CHECKPOINT_NUMBER - 1), + ); + }); + + it('round-trips an added rejected entry', async () => { + const entry = makeEntry(); + await blockStore.addRejectedCheckpoint(entry); + + const stored = await blockStore.getRejectedCheckpointByArchiveRoot(entry.archiveRoot); + expect(stored).toBeDefined(); + expect(stored!.checkpointNumber).toEqual(entry.checkpointNumber); + expect(stored!.archiveRoot.toString()).toEqual(entry.archiveRoot.toString()); + expect(stored!.parentArchiveRoot.toString()).toEqual(entry.parentArchiveRoot.toString()); + expect(stored!.slotNumber).toEqual(entry.slotNumber); + expect(stored!.l1.blockNumber).toEqual(entry.l1.blockNumber); + expect(stored!.l1.blockHash).toEqual(entry.l1.blockHash); + expect(stored!.reason).toEqual(entry.reason); + }); + + it('updates an existing entry when re-added with the same archive root', async () => { + const archiveRoot = Fr.random(); + await blockStore.addRejectedCheckpoint(makeEntry({ archiveRoot, l1BlockNumber: 100 })); + await blockStore.addRejectedCheckpoint(makeEntry({ archiveRoot, l1BlockNumber: 110 })); + + const stored = await blockStore.getRejectedCheckpointByArchiveRoot(archiveRoot); + expect(stored).toBeDefined(); + expect(stored!.l1.blockNumber).toEqual(110n); + }); + + it('preserves the descends-from-invalid-attestations reason', async () => { + const entry = { + ...makeEntry(), + reason: 'descends-from-invalid-attestations' as const, + }; + await blockStore.addRejectedCheckpoint(entry); + const stored = await blockStore.getRejectedCheckpointByArchiveRoot(entry.archiveRoot); + expect(stored!.reason).toEqual('descends-from-invalid-attestations'); + }); + + it('returns the latest rejected checkpoint number across all entries', async () => { + await blockStore.addRejectedCheckpoint(makeEntry({ checkpointNumber: 1 })); + await blockStore.addRejectedCheckpoint(makeEntry({ checkpointNumber: 5 })); + await blockStore.addRejectedCheckpoint(makeEntry({ checkpointNumber: 3 })); + + expect(await blockStore.getLatestRejectedCheckpointNumber()).toEqual(CheckpointNumber(5)); + }); + + it('looks up a rejected entry by checkpoint number', async () => { + const entry = makeEntry({ checkpointNumber: 7 }); + await blockStore.addRejectedCheckpoint(entry); + + const stored = await blockStore.getRejectedCheckpointByNumber(CheckpointNumber(7)); + expect(stored?.archiveRoot.toString()).toEqual(entry.archiveRoot.toString()); + expect(await blockStore.getRejectedCheckpointByNumber(CheckpointNumber(8))).toBeUndefined(); + }); + + it('removes a rejected entry by archive root', async () => { + const entry = makeEntry({ checkpointNumber: 4 }); + await blockStore.addRejectedCheckpoint(entry); + expect(await blockStore.getRejectedCheckpointByArchiveRoot(entry.archiveRoot)).toBeDefined(); + + await blockStore.removeRejectedCheckpointByArchiveRoot(entry.archiveRoot); + expect(await blockStore.getRejectedCheckpointByArchiveRoot(entry.archiveRoot)).toBeUndefined(); + expect(await blockStore.getRejectedCheckpointByNumber(CheckpointNumber(4))).toBeUndefined(); + expect(await blockStore.getLatestRejectedCheckpointNumber()).toEqual( + CheckpointNumber(INITIAL_CHECKPOINT_NUMBER - 1), + ); + }); + }); }); diff --git a/yarn-project/archiver/src/store/block_store.ts b/yarn-project/archiver/src/store/block_store.ts index 6e68cd2e41c8..0db47a55eafb 100644 --- a/yarn-project/archiver/src/store/block_store.ts +++ b/yarn-project/archiver/src/store/block_store.ts @@ -73,6 +73,41 @@ type BlockStorage = { indexWithinCheckpoint: number; }; +/** Reason a checkpoint was rejected during sync. */ +export type RejectedCheckpointReason = 'invalid-attestations' | 'descends-from-invalid-attestations'; + +/** + * A checkpoint observed on L1 that the archiver decided not to ingest, recorded so that + * any descendant that builds on top of it can also be skipped (rather than throwing + * `InitialCheckpointNumberNotSequentialError` and looping). An entry is dropped via + * {@link BlockStore.removeRejectedCheckpointByArchiveRoot} once a checkpoint with the same + * archive root is later ingested as valid (e.g. it gathered enough attestations), which + * re-enables its descendants. + */ +export type RejectedCheckpoint = { + /** Checkpoint number this entry represents. */ + checkpointNumber: CheckpointNumber; + /** Archive root produced by this rejected checkpoint (matched against descendants' `lastArchiveRoot`). */ + archiveRoot: Fr; + /** `lastArchiveRoot` from this checkpoint's header (the ancestor it built on). */ + parentArchiveRoot: Fr; + /** Slot number of the rejected checkpoint. */ + slotNumber: SlotNumber; + /** L1 publication data for the rejected checkpoint (block number, hash, timestamp). */ + l1: L1PublishedData; + /** Why the entry was recorded. */ + reason: RejectedCheckpointReason; +}; + +type RejectedCheckpointStorage = { + checkpointNumber: number; + archiveRoot: Buffer; + parentArchiveRoot: Buffer; + slotNumber: number; + l1: Buffer; + reason: RejectedCheckpointReason; +}; + /** Checkpoint Storage shared between Checkpoints + Proposed Checkpoints */ type CommonCheckpointStorage = { header: Buffer; @@ -153,6 +188,12 @@ export class BlockStore { /** Index mapping block archive to block number */ #blockArchiveIndex: AztecAsyncMap; + /** Map rejected checkpoints (due to invalid attestations) by archive root */ + #rejectedCheckpoints: AztecAsyncMap; + + /** Index mapping a rejected checkpoint's number to its archive root, so the latest can be read in reverse order */ + #rejectedCheckpointsByNumber: AztecAsyncMap; + #log = createLogger('archiver:block_store'); constructor(private db: AztecAsyncKVStore) { @@ -169,6 +210,8 @@ export class BlockStore { this.#checkpoints = db.openMap('archiver_checkpoints'); this.#slotToCheckpoint = db.openMap('archiver_slot_to_checkpoint'); this.#proposedCheckpoints = db.openMap('archiver_proposed_checkpoints'); + this.#rejectedCheckpoints = db.openMap('archiver_rejected_checkpoints'); + this.#rejectedCheckpointsByNumber = db.openMap('archiver_rejected_checkpoints_by_number'); } /** @@ -340,9 +383,13 @@ export class BlockStore { // Remove proposed checkpoint if it exists, since L1 is authoritative await this.#proposedCheckpoints.delete(checkpoint.checkpoint.number); + + // Drop any rejected entry for this archive root: a checkpoint that was previously rejected + // (e.g. invalid attestations) is now being ingested as valid, so its descendants are allowed. + await this.removeRejectedCheckpointByArchiveRoot(checkpoint.checkpoint.archive.root); } - await this.#lastSynchedL1Block.set(checkpoints[checkpoints.length - 1].l1.blockNumber); + await this.advanceSynchedL1BlockNumber(checkpoints[checkpoints.length - 1].l1.blockNumber); return true; }); } @@ -387,7 +434,7 @@ export class BlockStore { feeAssetPriceModifier: incoming.checkpoint.feeAssetPriceModifier.toString(), }); // Update the sync point to reflect the new L1 block - await this.#lastSynchedL1Block.set(incoming.l1.blockNumber); + await this.advanceSynchedL1BlockNumber(incoming.l1.blockNumber); } return checkpoints.slice(i); } @@ -776,8 +823,12 @@ export class BlockStore { // Remove only this pending entry — remaining entries N+1, N+2, ... stay valid await this.#proposedCheckpoints.delete(proposed.checkpointNumber); + // Drop any rejected entry for this archive root: a checkpoint that was previously rejected + // (e.g. invalid attestations) is now being promoted as valid, so its descendants are allowed. + await this.removeRejectedCheckpointByArchiveRoot(proposed.archive.root); + // Update the last synced L1 block - await this.#lastSynchedL1Block.set(l1.blockNumber); + await this.advanceSynchedL1BlockNumber(l1.blockNumber); }); } @@ -1427,4 +1478,80 @@ export class BlockStore { await this.#pendingChainValidationStatus.delete(); } } + + /** Records a rejected-checkpoint entry, keyed by its own archive root. */ + async addRejectedCheckpoint(entry: RejectedCheckpoint): Promise { + const archiveRootHex = entry.archiveRoot.toString(); + await this.#rejectedCheckpoints.set(archiveRootHex, { + checkpointNumber: entry.checkpointNumber, + archiveRoot: entry.archiveRoot.toBuffer(), + parentArchiveRoot: entry.parentArchiveRoot.toBuffer(), + slotNumber: entry.slotNumber, + l1: entry.l1.toBuffer(), + reason: entry.reason, + }); + await this.#rejectedCheckpointsByNumber.set(entry.checkpointNumber, archiveRootHex); + await this.advanceSynchedL1BlockNumber(entry.l1.blockNumber); + } + + /** Returns the rejected-checkpoint entry with the given archive root, or undefined if not present. */ + async getRejectedCheckpointByArchiveRoot(archiveRoot: Fr): Promise { + const stored = await this.#rejectedCheckpoints.getAsync(archiveRoot.toString()); + return stored ? this.rejectedCheckpointFromStorage(stored) : undefined; + } + + /** Returns the rejected-checkpoint entry recorded for the given checkpoint number, or undefined if none. */ + async getRejectedCheckpointByNumber(checkpointNumber: CheckpointNumber): Promise { + const archiveRootHex = await this.#rejectedCheckpointsByNumber.getAsync(checkpointNumber); + if (archiveRootHex === undefined) { + return undefined; + } + const stored = await this.#rejectedCheckpoints.getAsync(archiveRootHex); + return stored ? this.rejectedCheckpointFromStorage(stored) : undefined; + } + + /** Returns the highest checkpoint number recorded across all rejected entries, or `INITIAL_CHECKPOINT_NUMBER - 1` if none. */ + async getLatestRejectedCheckpointNumber(): Promise { + const [latest] = await toArray(this.#rejectedCheckpointsByNumber.keysAsync({ reverse: true, limit: 1 })); + return CheckpointNumber(latest ?? INITIAL_CHECKPOINT_NUMBER - 1); + } + + /** Removes a rejected-checkpoint entry by its archive root (used when an entry no longer matches L1). */ + async removeRejectedCheckpointByArchiveRoot(archiveRoot: Fr): Promise { + const archiveRootHex = archiveRoot.toString(); + const stored = await this.#rejectedCheckpoints.getAsync(archiveRootHex); + await this.#rejectedCheckpoints.delete(archiveRootHex); + if (stored) { + // Only clear the by-number index if it still points at this archive root, so a distinct + // entry that shares the checkpoint number (e.g. an L1 reorg replacement) is not dropped. + const indexed = await this.#rejectedCheckpointsByNumber.getAsync(stored.checkpointNumber); + if (indexed === archiveRootHex) { + await this.#rejectedCheckpointsByNumber.delete(stored.checkpointNumber); + } + } + } + + /** + * Advances the stored last-synched L1 block number to `l1BlockNumber` only if it is strictly + * greater than the current value. Use this whenever ingesting checkpoint-shaped data so the + * sync pointer never walks backwards on out-of-order writes (e.g. an invalid checkpoint + * advance followed by a valid-checkpoint commit landing at an earlier L1 block). + */ + private async advanceSynchedL1BlockNumber(l1BlockNumber: bigint): Promise { + const current = await this.#lastSynchedL1Block.getAsync(); + if (current === undefined || l1BlockNumber > current) { + await this.#lastSynchedL1Block.set(l1BlockNumber); + } + } + + private rejectedCheckpointFromStorage(stored: RejectedCheckpointStorage): RejectedCheckpoint { + return { + checkpointNumber: CheckpointNumber(stored.checkpointNumber), + archiveRoot: Fr.fromBuffer(stored.archiveRoot), + parentArchiveRoot: Fr.fromBuffer(stored.parentArchiveRoot), + slotNumber: SlotNumber(stored.slotNumber), + l1: L1PublishedData.fromBuffer(stored.l1), + reason: stored.reason, + }; + } } diff --git a/yarn-project/aztec-node/src/aztec-node/server.test.ts b/yarn-project/aztec-node/src/aztec-node/server.test.ts index 462fa2cba60e..6c35abec8370 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.test.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.test.ts @@ -222,8 +222,7 @@ describe('aztec node', () => { undefined, undefined, undefined, - undefined, - undefined, + async () => {}, 12345, rollupVersion.toNumber(), globalVariablesBuilder, @@ -739,8 +738,7 @@ describe('aztec node', () => { undefined, slasherClient, undefined, - undefined, - undefined, + async () => {}, 12345, rollupVersion.toNumber(), globalVariablesBuilder, @@ -930,8 +928,7 @@ describe('aztec node', () => { undefined, slasherClient, undefined, - undefined, - undefined, + async () => {}, 12345, rollupVersion.toNumber(), globalVariablesBuilder, @@ -1002,8 +999,7 @@ describe('aztec node', () => { undefined, undefined, undefined, - undefined, - undefined, + async () => {}, 12345, rollupVersion.toNumber(), globalVariablesBuilder, @@ -1056,8 +1052,7 @@ describe('aztec node', () => { undefined, undefined, undefined, - undefined, - undefined, + async () => {}, 12345, rollupVersion.toNumber(), globalVariablesBuilder, diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index edba020f1e27..f4be06db0b6e 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -45,6 +45,7 @@ import { import { PublicContractsDB, PublicProcessorFactory } from '@aztec/simulator/server'; import { AttestationsBlockWatcher, + AttestedInvalidProposalWatcher, BroadcastedInvalidCheckpointProposalWatcher, CheckpointEquivocationWatcher, DataWithholdingWatcher, @@ -186,8 +187,7 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb protected readonly proverNode: ProverNode | undefined, protected readonly slasherClient: SlasherClientInterface | undefined, protected readonly validatorsSentinel: Sentinel | undefined, - protected readonly dataWithholdingWatcher: DataWithholdingWatcher | undefined, - protected readonly attestationsBlockWatcher: AttestationsBlockWatcher | undefined, + private readonly stopStartedWatchers: () => Promise, protected readonly l1ChainId: number, protected readonly version: number, protected readonly globalVariableBuilder: GlobalVariableBuilderInterface, @@ -737,80 +737,81 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb let validatorsSentinel: Awaited> | undefined; let dataWithholdingWatcher: DataWithholdingWatcher | undefined; let attestationsBlockWatcher: AttestationsBlockWatcher | undefined; + let attestedInvalidProposalWatcher: AttestedInvalidProposalWatcher | undefined; let broadcastedInvalidCheckpointProposalWatcher: BroadcastedInvalidCheckpointProposalWatcher | undefined; let checkpointEquivocationWatcher: CheckpointEquivocationWatcher | undefined; if (!proverOnly) { validatorsSentinel = await createSentinel(epochCache, archiver, p2pClient, reexecutionTracker, config); - if (validatorsSentinel && config.slashInactivityPenalty > 0n) { + if (validatorsSentinel) { watchers.push(validatorsSentinel); } - if (config.slashDataWithholdingPenalty > 0n) { - dataWithholdingWatcher = new DataWithholdingWatcher( - epochCache, - archiver, - p2pClient.getTxProvider(), - p2pClient, - reexecutionTracker, - { chainId: config.l1ChainId, rollupAddress: config.rollupAddress }, - config, - ); - watchers.push(dataWithholdingWatcher); - } + dataWithholdingWatcher = new DataWithholdingWatcher( + epochCache, + archiver, + p2pClient.getTxProvider(), + p2pClient, + reexecutionTracker, + { chainId: config.l1ChainId, rollupAddress: config.rollupAddress }, + config, + ); + watchers.push(dataWithholdingWatcher); + + broadcastedInvalidCheckpointProposalWatcher = new BroadcastedInvalidCheckpointProposalWatcher( + p2pClient, + archiver, + epochCache, + config, + ); + watchers.push(broadcastedInvalidCheckpointProposalWatcher); - if (config.slashBroadcastedInvalidCheckpointProposalPenalty > 0n) { - broadcastedInvalidCheckpointProposalWatcher = new BroadcastedInvalidCheckpointProposalWatcher( + if (validatorClient) { + attestedInvalidProposalWatcher = new AttestedInvalidProposalWatcher( p2pClient, + validatorClient, archiver, epochCache, config, + { log: log.createChild('attested-invalid-proposal-watcher') }, ); - watchers.push(broadcastedInvalidCheckpointProposalWatcher); + watchers.push(attestedInvalidProposalWatcher); } - if (config.slashDuplicateProposalPenalty > 0n) { - checkpointEquivocationWatcher = new CheckpointEquivocationWatcher(archiver, epochCache, config); - watchers.push(checkpointEquivocationWatcher); - } + checkpointEquivocationWatcher = new CheckpointEquivocationWatcher(archiver, epochCache, config); + watchers.push(checkpointEquivocationWatcher); - // We assume we want to slash for invalid attestations unless all max penalties are set to 0 - if ( - config.slashProposeInvalidAttestationsPenalty > 0n || - config.slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty > 0n - ) { - attestationsBlockWatcher = new AttestationsBlockWatcher(archiver, epochCache, config); - watchers.push(attestationsBlockWatcher); - } + attestationsBlockWatcher = new AttestationsBlockWatcher(archiver, epochCache, config, log.getBindings()); + watchers.push(attestationsBlockWatcher); } + const watchersToStart = compactArray([ + validatorsSentinel, + dataWithholdingWatcher, + attestationsBlockWatcher, + broadcastedInvalidCheckpointProposalWatcher, + attestedInvalidProposalWatcher, + checkpointEquivocationWatcher, + ]); + const startedWatchers: Watcher[] = []; + const stopStartedWatchers = async () => { + for (const watcher of startedWatchers) { + await tryStop(watcher); + } + }; + // Start p2p-related services once the archiver has completed sync void archiver .waitForInitialSync() .then(async () => { - if (validatorsSentinel) { - await validatorsSentinel.start(); - started.push(validatorsSentinel); - } - if (dataWithholdingWatcher) { - await dataWithholdingWatcher.start(); - started.push(dataWithholdingWatcher); - } - if (attestationsBlockWatcher) { - await attestationsBlockWatcher.start(); - started.push(attestationsBlockWatcher); - } - if (broadcastedInvalidCheckpointProposalWatcher) { - await broadcastedInvalidCheckpointProposalWatcher.start(); - started.push(broadcastedInvalidCheckpointProposalWatcher); - } - if (checkpointEquivocationWatcher) { - await checkpointEquivocationWatcher.start(); - started.push(checkpointEquivocationWatcher); + for (const watcher of watchersToStart) { + await watcher.start(); + startedWatchers.push(watcher); } log.info(`All p2p services started`); }) .catch(err => log.error('Failed to start p2p services after archiver sync', err)); + started.push({ stop: stopStartedWatchers }); // Validator enabled, create/start relevant service let sequencer: SequencerClient | undefined; @@ -976,8 +977,7 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb proverNode, slasherClient, validatorsSentinel, - dataWithholdingWatcher, - attestationsBlockWatcher, + stopStartedWatchers, ethereumChain.chainInfo.id, config.rollupVersion, globalVariableBuilder, @@ -1259,9 +1259,7 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb */ public async stop() { this.log.info(`Stopping Aztec Node`); - await tryStop(this.attestationsBlockWatcher); - await tryStop(this.validatorsSentinel); - await tryStop(this.dataWithholdingWatcher); + await this.stopStartedWatchers(); await tryStop(this.slasherClient); await Promise.all([tryStop(this.peerProofVerifier), tryStop(this.rpcProofVerifier)]); await tryStop(this.sequencer); diff --git a/yarn-project/aztec-node/src/sentinel/sentinel.test.ts b/yarn-project/aztec-node/src/sentinel/sentinel.test.ts index d28d49bc81a8..966a72b34fb9 100644 --- a/yarn-project/aztec-node/src/sentinel/sentinel.test.ts +++ b/yarn-project/aztec-node/src/sentinel/sentinel.test.ts @@ -986,6 +986,24 @@ describe('sentinel', () => { ]); }); + it('emits zero-amount inactivity offenses when the penalty is zero', async () => { + sentinel.updateConfig({ slashInactivityPenalty: 0n, slashInactivityConsecutiveEpochThreshold: 1 }); + const emitSpy = jest.spyOn(sentinel, 'emit'); + + await sentinel.handleEpochPerformance(EpochNumber(5), { + [validator1.toString()]: { missed: 8, total: 10 }, + }); + + expect(emitSpy).toHaveBeenCalledWith(WANT_TO_SLASH_EVENT, [ + { + validator: validator1, + amount: 0n, + offenseType: OffenseType.INACTIVITY, + epochOrSlot: 5n, + }, + ]); + }); + it('should not slash when no validators meet consecutive threshold', async () => { // Update config to require 3 consecutive epochs sentinel.updateConfig({ slashInactivityConsecutiveEpochThreshold: 3 }); diff --git a/yarn-project/aztec-node/src/sentinel/sentinel.ts b/yarn-project/aztec-node/src/sentinel/sentinel.ts index 53d1dcc70099..78a50ff8c075 100644 --- a/yarn-project/aztec-node/src/sentinel/sentinel.ts +++ b/yarn-project/aztec-node/src/sentinel/sentinel.ts @@ -18,6 +18,7 @@ import { type WantToSlashArgs, type Watcher, type WatcherEmitter, + getOffenseTypeName, } from '@aztec/slasher'; import type { SlasherConfig } from '@aztec/slasher/config'; import { @@ -335,10 +336,6 @@ export class Sentinel extends (EventEmitter as new () => WatcherEmitter) impleme } protected async handleEpochPerformance(epoch: EpochNumber, performance: ValidatorsEpochPerformance) { - if (this.config.slashInactivityPenalty === 0n) { - return; - } - const inactiveValidators = getEntries(performance) .filter(([_, { missed, total }]) => total > 0 && missed / total >= this.config.slashInactivityTargetPercentage) .map(([address]) => address); @@ -362,9 +359,17 @@ export class Sentinel extends (EventEmitter as new () => WatcherEmitter) impleme })); if (criminals.length > 0) { - this.logger.verbose( - `Identified ${criminals.length} validators to slash due to inactivity in at least ${epochThreshold} consecutive epochs`, - { ...args, epochThreshold }, + this.logger.info( + `Identified ${criminals.length} inactivity offenses in at least ${epochThreshold} consecutive epochs`, + { + offenses: args.map(arg => ({ + validator: arg.validator.toString(), + amount: arg.amount, + offenseType: getOffenseTypeName(arg.offenseType), + epochOrSlot: arg.epochOrSlot, + })), + epochThreshold, + }, ); this.emit(WANT_TO_SLASH_EVENT, args); } diff --git a/yarn-project/end-to-end/src/composed/web3signer/e2e_multi_validator_node_key_store.test.ts b/yarn-project/end-to-end/src/composed/web3signer/e2e_multi_validator_node_key_store.test.ts index 018c4a8e2f15..af674e9dcdad 100644 --- a/yarn-project/end-to-end/src/composed/web3signer/e2e_multi_validator_node_key_store.test.ts +++ b/yarn-project/end-to-end/src/composed/web3signer/e2e_multi_validator_node_key_store.test.ts @@ -284,21 +284,24 @@ describe('e2e_multi_validator_node', () => { deployL1ContractsValues, aztecNode, sequencer: sequencerClient, - } = await setup(1, { - ...PIPELINING_SETUP_OPTS, - initialValidators, - aztecTargetCommitteeSize: COMMITTEE_SIZE, - keyStoreDirectory, - minTxsPerBlock: 1, - maxTxsPerBlock: 1, - archiverPollingIntervalMS: 200, - sequencerPollingIntervalMS: 200, - worldStateBlockCheckIntervalMS: 200, - blockCheckIntervalMS: 200, - startProverNode: true, - aztecEpochDuration: 8, - aztecProofSubmissionEpochs: 4, - })); + } = await setup( + 1, + { + ...PIPELINING_SETUP_OPTS, + initialValidators, + aztecTargetCommitteeSize: COMMITTEE_SIZE, + keyStoreDirectory, + maxTxsPerBlock: 1, + archiverPollingIntervalMS: 200, + sequencerPollingIntervalMS: 200, + worldStateBlockCheckIntervalMS: 200, + blockCheckIntervalMS: 200, + startProverNode: true, + aztecEpochDuration: 8, + aztecProofSubmissionEpochs: 4, + }, + { syncChainTip: 'checkpointed' }, + )); sequencer = (sequencerClient! as TestSequencerClient).getSequencer(); publisherFactory = (sequencer as TestSequencer).publisherFactory; diff --git a/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.parallel.test.ts b/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.parallel.test.ts index 431632c757b2..2d8323251021 100644 --- a/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.parallel.test.ts +++ b/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.parallel.test.ts @@ -1,4 +1,4 @@ -import { CalldataRetriever } from '@aztec/archiver'; +import { type Archiver, CalldataRetriever } from '@aztec/archiver'; import type { AztecNodeService } from '@aztec/aztec-node'; import type { AztecAddress } from '@aztec/aztec.js/addresses'; import { NO_WAIT } from '@aztec/aztec.js/contracts'; @@ -8,6 +8,7 @@ import { waitForTx } from '@aztec/aztec.js/node'; import { RollupContract } from '@aztec/ethereum/contracts'; import type { Operator } from '@aztec/ethereum/deploy-aztec-l1-contracts'; import type { ExtendedViemWalletClient, ViemPublicClient, ViemPublicDebugClient } from '@aztec/ethereum/types'; +import { range } from '@aztec/foundation/array'; import { asyncMap } from '@aztec/foundation/async-map'; import { CheckpointNumber, SlotNumber } from '@aztec/foundation/branded-types'; import { times, timesAsync } from '@aztec/foundation/collection'; @@ -17,9 +18,10 @@ import { createLogger } from '@aztec/foundation/log'; import { promiseWithResolvers } from '@aztec/foundation/promise'; import { retryUntil } from '@aztec/foundation/retry'; import { bufferToHex } from '@aztec/foundation/string'; -import { timeoutPromise } from '@aztec/foundation/timer'; +import { executeTimeout, timeoutPromise } from '@aztec/foundation/timer'; import type { TestContract } from '@aztec/noir-test-contracts.js/Test'; import { OffenseType } from '@aztec/slasher'; +import { L2BlockSourceEvents } from '@aztec/stdlib/block'; import { computeQuorum, getTimestampForSlot } from '@aztec/stdlib/epoch-helpers'; import { jest } from '@jest/globals'; @@ -388,17 +390,55 @@ describe('e2e_epochs/epochs_invalidate_block', () => { const { l2SlotNumber: currentSlot } = await test.monitor.run(); logger.warn(`First checkpoint mined, current slot is ${currentSlot}`); - // Pick the next two slots with a 2-slot gap to account for pipelining plus a margin - const badSlot1 = SlotNumber.add(currentSlot, 3); - const badSlot2 = SlotNumber.add(currentSlot, 4); + // The bad config is applied while sequencers are already running; skip pairs where a pipelined + // pre-bad target slot could snapshot that config before the intended bad slots. + let badSlot1: SlotNumber | undefined; + let badSlot2: SlotNumber | undefined; + let badProposers: EthAddress[] = []; + const firstCandidateSlot = Number(currentSlot) + 3; + const firstUnsnapshottedTargetSlot = SlotNumber.add(currentSlot, 2); + const maxBadSlotSearchAttempts = 20; + for (let attempt = 0; attempt < maxBadSlotSearchAttempts && badSlot1 === undefined; attempt++) { + const candidateSlot1 = SlotNumber(firstCandidateSlot + attempt); + const candidateSlot2 = SlotNumber.add(candidateSlot1, 1); + const preBadTargetSlots = range( + Math.max(0, Number(candidateSlot1) - Number(firstUnsnapshottedTargetSlot)), + Number(firstUnsnapshottedTargetSlot), + ).map(SlotNumber); + const [preBadProposers, p1, p2] = await Promise.all([ + Promise.all(preBadTargetSlots.map(slot => test.epochCache.getProposerAttesterAddressInSlot(slot))), + test.epochCache.getProposerAttesterAddressInSlot(candidateSlot1), + test.epochCache.getProposerAttesterAddressInSlot(candidateSlot2), + ]); + + logger.warn(`Checking bad checkpoint slots ${candidateSlot1} and ${candidateSlot2}`, { + preBadTargetSlots, + preBadProposers: preBadProposers.map(proposer => proposer?.toString()), + p1: p1?.toString(), + p2: p2?.toString(), + }); + + const badProposerHasUnsnapshottedPreBadSlot = + p1 !== undefined && + p2 !== undefined && + preBadProposers.some(proposer => proposer !== undefined && (proposer.equals(p1) || proposer.equals(p2))); + + if (p1 && p2 && !badProposerHasUnsnapshottedPreBadSlot) { + badSlot1 = candidateSlot1; + badSlot2 = candidateSlot2; + badProposers = [p1, p2]; + } + } + if (badSlot1 === undefined || badSlot2 === undefined) { + throw new Error(`Could not find bad checkpoint slots after ${maxBadSlotSearchAttempts} attempts`); + } const badSlots = [badSlot1, badSlot2]; - const badProposers = await Promise.all(badSlots.map(s => test.epochCache.getProposerAttesterAddressInSlot(s))); const badNodes = []; for (let badProposerIndex = 0; badProposerIndex < badProposers.length; badProposerIndex++) { const badProposer = badProposers[badProposerIndex]; logger.warn(`Disabling invalidation checks and attestation gathering for proposer ${badProposer}`); - const nodeIndex = nodes.findIndex(n => n.getSequencer()!.validatorAddresses!.some(a => a.equals(badProposer!))); + const nodeIndex = nodes.findIndex(n => n.getSequencer()!.validatorAddresses!.some(a => a.equals(badProposer))); if (nodeIndex === -1) { throw new Error(`Could not find node for proposer ${badProposer}`); } @@ -433,7 +473,7 @@ describe('e2e_epochs/epochs_invalidate_block', () => { // Wait for both checkpoints to be mined logger.warn(`Waiting for two checkpoints to be mined on slots ${expectedFirstSlot} and ${expectedSecondSlot}`); const [firstCheckpoint, secondCheckpoint] = await Promise.race([ - await Promise.all([firstCheckpointPromise.promise, secondCheckpointPromise.promise]), + Promise.all([firstCheckpointPromise.promise, secondCheckpointPromise.promise]), timeoutPromise(test.L2_SLOT_DURATION_IN_S * 8 * 1000).then(() => [CheckpointNumber(0), CheckpointNumber(0)]), ]); @@ -472,6 +512,210 @@ describe('e2e_epochs/epochs_invalidate_block', () => { logger.warn(`Test succeeded '${expect.getState().currentTestName}'`); }); + // P1 publishes a checkpoint with insufficient attestations; the next proposer P2 publishes a + // valid descendant without first invalidating P1. Before the fix, the archiver tripped its + // `InitialCheckpointNumberNotSequentialError` consecutive-number guard, rolled back the L1 + // sync point, and looped indefinitely. The fix records P1 as a rejected ancestor and skips P2 + // (its valid descendant) outright, emitting `DescendentOfInvalidAttestationsCheckpointDetected` + // so the slasher can target P2's proposer. This test verifies the chain advances past P2 and + // that both proposers end up flagged for slashing. + it('archiver skips a descendant of an invalid-attestations checkpoint', async () => { + const sequencers = nodes.map(node => node.getSequencer()!); + + // The committee invalidation fallback is already disabled by the fixture-level + // `secondsBeforeInvalidatingBlockAsCommitteeMember`. We also need to disable the non-committee + // fallback (`considerInvalidatingCheckpoint` at sequencer.ts:950, called from L345) on every + // node, otherwise any sequencer whose pending chain is invalid will eventually invalidate P1 + // and break the loop we're trying to reproduce. + sequencers.forEach(s => + s.updateConfig({ + secondsBeforeInvalidatingBlockAsNonCommitteeMember: Number.MAX_SAFE_INTEGER, + minTxsPerBlock: 0, + }), + ); + let badSlot1: SlotNumber | undefined; + let p1Proposer: EthAddress | undefined; + let p2Proposer: EthAddress | undefined; + let candidate = Number(test.epochCache.getEpochAndSlotNow().slot) + 4; + const maxAttempts = 200; + for (let attempt = 0; attempt < maxAttempts && badSlot1 === undefined; attempt++) { + try { + const [p1, p2] = await Promise.all([ + test.epochCache.getProposerAttesterAddressInSlot(SlotNumber(candidate)), + test.epochCache.getProposerAttesterAddressInSlot(SlotNumber(candidate + 1)), + ]); + if (p1 && p2 && !p1.equals(p2)) { + badSlot1 = SlotNumber(candidate); + p1Proposer = p1; + p2Proposer = p2; + break; + } + candidate++; + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + if (!msg.includes('EpochNotStable')) { + throw err; + } + const block = await test.l1Client.getBlock({ includeTransactions: false }); + const warpBy = test.epochDuration * test.L2_SLOT_DURATION_IN_S; + const newTs = Number(block.timestamp) + warpBy; + logger.warn(`Hit EpochNotStable at candidate ${candidate}, warping L1 forward by ${warpBy}s to ${newTs}`); + await test.context.cheatCodes.eth.warp(newTs, { resetBlockInterval: true }); + const newCurrentSlot = Number(test.epochCache.getEpochAndSlotNow().slot); + if (candidate < newCurrentSlot + 4) { + candidate = newCurrentSlot + 4; + } + } + } + if (badSlot1 === undefined || !p1Proposer || !p2Proposer) { + throw new Error(`Could not find two consecutive slots with different proposers after ${maxAttempts} attempts`); + } + const badSlot2 = SlotNumber.add(badSlot1, 1); + + const p1NodeIndex = nodes.findIndex(n => n.getSequencer()!.validatorAddresses!.some(a => a.equals(p1Proposer!))); + const p2NodeIndex = nodes.findIndex(n => n.getSequencer()!.validatorAddresses!.some(a => a.equals(p2Proposer!))); + if (p1NodeIndex === -1 || p2NodeIndex === -1) { + throw new Error(`Could not find nodes for proposers P1=${p1Proposer} P2=${p2Proposer}`); + } + const p1Node = nodes[p1NodeIndex]; + const p2Node = nodes[p2NodeIndex]; + logger.warn(`Applying malicious configs`, { + p1NodeIndex, + p1Proposer: p1Proposer.toString(), + badSlot1, + p2NodeIndex, + p2Proposer: p2Proposer.toString(), + badSlot2, + }); + + // P1 publishes its checkpoint with only its own self-attestation (insufficient) and skips + // any invalidation of earlier checkpoints. + await p1Node.setConfig({ + skipCollectingAttestations: true, + skipInvalidateBlockAsProposer: true, + minTxsPerBlock: 0, + }); + + // P2 collects attestations normally so its checkpoint lands valid, but bypasses the + // parent-validity gate, so it ends up pushing a valid checkpoint with valid attestations + // that descends from the invalid P1, which is the scenario we want to test. + await p2Node.setConfig({ + skipWaitForValidParentCheckpointOnL1: true, + skipInvalidateBlockAsProposer: true, + minTxsPerBlock: 0, + }); + + // Subscribe to the new archiver event so we can assert P2 was surfaced through it. + const observerIndex = range(nodes.length).find(i => i !== p1NodeIndex && i !== p2NodeIndex)!; + const observerArchiver = nodes[observerIndex].getBlockSource() as Archiver; + const descendantEvents: { checkpointNumber: CheckpointNumber; ancestorCheckpointNumber: CheckpointNumber }[] = []; + const onDescendant = (event: { + checkpoint: { checkpointNumber: CheckpointNumber }; + ancestorCheckpointNumber: CheckpointNumber; + }) => { + descendantEvents.push({ + checkpointNumber: event.checkpoint.checkpointNumber, + ancestorCheckpointNumber: event.ancestorCheckpointNumber, + }); + }; + + observerArchiver.events.on(L2BlockSourceEvents.DescendentOfInvalidAttestationsCheckpointDetected, onDescendant); + + // Watch for both CheckpointProposed events at the targeted slots. + const p1CheckpointPromise = promiseWithResolvers(); + const p2CheckpointPromise = promiseWithResolvers(); + test.monitor.on('checkpoint', ({ checkpointNumber, l2SlotNumber }) => { + if (l2SlotNumber === badSlot1) { + p1CheckpointPromise.resolve(checkpointNumber); + } + if (l2SlotNumber === badSlot2) { + p2CheckpointPromise.resolve(checkpointNumber); + } + }); + + // Send a couple of txs so there's content for both checkpoints. + logger.warn('Sending transactions to fill the bad checkpoints'); + await Promise.all(times(4, i => testContract.methods.emit_nullifier(BigInt(i + 1)).send({ from, wait: NO_WAIT }))); + + // Sequencers are still stopped. Warp to the L1 block immediately before the pipelined build + // window for P1, so the first proposer job that can observe the malicious config is the + // intended checkpoint, not an earlier slot owned by the same validator. + const buildSlot = SlotNumber.add(badSlot1, -1); + const buildSlotStart = getTimestampForSlot(buildSlot, test.constants); + const warpTo = buildSlotStart - BigInt(test.L1_BLOCK_TIME_IN_S); + logger.warn(`Warping L1 to timestamp ${warpTo} (one L1 block before build slot ${buildSlot})`); + await test.context.cheatCodes.eth.warp(Number(warpTo), { resetBlockInterval: true }); + + await Promise.all(sequencers.map(s => s.start())); + logger.warn(`Started all sequencers after warping to the target build window`); + + logger.warn(`Waiting for two checkpoints to be mined on slots ${badSlot1} and ${badSlot2}`); + const [p1Checkpoint, p2Checkpoint] = await executeTimeout( + () => Promise.all([p1CheckpointPromise.promise, p2CheckpointPromise.promise]), + test.L2_SLOT_DURATION_IN_S * 8 * 1000, + 'Waiting for both checkpoints', + ); + logger.warn(`Observed checkpoints`, { p1Checkpoint, p2Checkpoint, badSlot1, badSlot2 }); + expect(p2Checkpoint).toEqual(CheckpointNumber(p1Checkpoint + 1)); + + // P1 must have landed with insufficient attestations (the trigger for the archiver skip). + await assertCheckpointInsufficientAttestations(p1Checkpoint); + + // Restore P2 to a healthy config so a later proposer (or P2 in a future slot) can resume + // the chain by invalidating P1 and posting fresh checkpoints. + await p2Node.setConfig({ skipWaitForValidParentCheckpointOnL1: false, skipInvalidateBlockAsProposer: false }); + + // The archiver should no longer stall: wait for the chain to advance past P2 within a + // handful of slots. Note we wait on local checkpoint progress here (i.e. for the chain to + // get unstuck), not specifically on observing P2 in the checkpointed tip — P1 and P2 will + // both be skipped, the chain will be invalidated, and progress comes from later slots. + const targetCheckpoint = CheckpointNumber(p2Checkpoint + 1); + logger.warn(`Waiting for node ${observerIndex} to advance past checkpoint ${p2Checkpoint}`); + await retryUntil( + async () => { + const tips = await nodes[observerIndex].getChainTips(); + return tips.checkpointed.checkpoint.number >= targetCheckpoint; + }, + 'archiver advances past P2', + test.L2_SLOT_DURATION_IN_S * 8, + 0.5, + ); + + // Confirm the descendant-of-invalid event fired for P2 at least once. + logger.warn(`Observed ${descendantEvents.length} DescendentOfInvalidAttestationsCheckpointDetected events`); + expect(descendantEvents.some(e => e.checkpointNumber === p2Checkpoint)).toBe(true); + + // Both proposers should be flagged for slashing: P1 under PROPOSED_INSUFFICIENT_ATTESTATIONS + // and P2 under PROPOSED_DESCENDANT_OF_CHECKPOINT_WITH_INVALID_ATTESTATIONS. + const offenses = await nodes[observerIndex].getSlashOffenses('all'); + logger.warn(`Collected ${offenses.length} offenses`, { + offenses: offenses.map(o => ({ + offenseType: o.offenseType, + validator: o.validator.toString(), + slot: o.epochOrSlot, + })), + }); + const insufficient = offenses.find( + o => o.offenseType === OffenseType.PROPOSED_INSUFFICIENT_ATTESTATIONS && o.epochOrSlot === BigInt(badSlot1), + ); + expect(insufficient).toBeDefined(); + expect(insufficient!.validator.equals(p1Proposer!)).toBeTrue(); + + const descendant = offenses.find( + o => + o.offenseType === OffenseType.PROPOSED_DESCENDANT_OF_CHECKPOINT_WITH_INVALID_ATTESTATIONS && + o.epochOrSlot === BigInt(badSlot2), + ); + expect(descendant).toBeDefined(); + expect(descendant!.validator.equals(p2Proposer!)).toBeTrue(); + + logger.warn(`Test succeeded '${expect.getState().currentTestName}'`); + observerArchiver.events.removeListener( + L2BlockSourceEvents.DescendentOfInvalidAttestationsCheckpointDetected, + onDescendant, + ); + }); + // All tests but this one disable invalidation by committee. This test disables invalidation by proposer and // instead waits for a committee member to invalidate the block after several proposers not doing so. it('committee member invalidates a block if proposer does not come through', async () => { diff --git a/yarn-project/end-to-end/src/e2e_p2p/broadcasted_invalid_block_proposal_slash.test.ts b/yarn-project/end-to-end/src/e2e_p2p/broadcasted_invalid_block_proposal_slash.test.ts index 2915bca2ce86..335ecc5aec4e 100644 --- a/yarn-project/end-to-end/src/e2e_p2p/broadcasted_invalid_block_proposal_slash.test.ts +++ b/yarn-project/end-to-end/src/e2e_p2p/broadcasted_invalid_block_proposal_slash.test.ts @@ -14,7 +14,7 @@ import path from 'path'; import { shouldCollectMetrics } from '../fixtures/fixtures.js'; import { createNodes } from '../fixtures/setup_p2p_test.js'; import { P2PNetworkTest } from './p2p_network.js'; -import { advanceToEpochBeforeProposer, awaitCommitteeExists, awaitOffenseDetected } from './shared.js'; +import { advanceToEpochBeforeProposer, awaitCommitteeExists } from './shared.js'; const TEST_TIMEOUT = 1_000_000; @@ -141,7 +141,7 @@ describe('e2e_p2p_broadcasted_invalid_block_proposal_slash', () => { // Create remaining honest nodes, also with sequencers stopped, for the same reason. const honestNodes = await createNodes( - { ...t.ctx.aztecNodeConfig, dontStartSequencer: true }, + { ...t.ctx.aztecNodeConfig, dontStartSequencer: true, skipBroadcastProposals: true }, t.ctx.dateProvider, t.bootstrapNodeEnr, NUM_VALIDATORS - 1, @@ -183,18 +183,8 @@ describe('e2e_p2p_broadcasted_invalid_block_proposal_slash', () => { // Wait for offense to be detected. Under proposer pipelining, the invalid block proposal is // broadcast at the slot boundary while a receiver's wall clock may have already advanced - // past the build slot — when that happens, the honest node rejects the gossip with "invalid - // slot number" before slashing logic runs. Collect offenses from every node so we catch - // whichever node managed to process the proposal while still in the build slot. - await awaitOffenseDetected({ - epochDuration: t.ctx.aztecNodeConfig.aztecEpochDuration, - logger: t.logger, - nodeAdmin: nodes[1], // Use honest node to check for offenses - slashingRoundSize, - waitUntilOffenseCount: 1, - timeoutSeconds: AZTEC_SLOT_DURATION * 16, - }); - + // past the build slot. Honest sequencers are running so their validator clients emit offenses, + // but they do not broadcast proposals until after the offense is detected. const invalidBlockOffenses = await retryUntil( async () => { const allOffenses = (await Promise.all(nodes.map(n => n.getSlashOffenses('all')))).flat(); @@ -204,7 +194,7 @@ describe('e2e_p2p_broadcasted_invalid_block_proposal_slash', () => { } }, 'broadcasted invalid block proposal offense', - AZTEC_SLOT_DURATION * 4, + AZTEC_SLOT_DURATION * 16, ); t.logger.warn(`Collected broadcasted invalid block proposal offenses`, { invalidBlockOffenses }); @@ -219,6 +209,10 @@ describe('e2e_p2p_broadcasted_invalid_block_proposal_slash', () => { t.logger.warn(`Slashed ${args.attester.toString()}`); slashPromise.resolve(args); }); + + t.logger.warn('Re-enabling honest proposal broadcasts'); + await Promise.all(honestNodes.map(n => n.setConfig({ skipBroadcastProposals: false }))); + const { amount, attester } = await slashPromise.promise; expect(invalidProposerAddress.toString()).toEqual(attester.toString()); expect(amount).toEqual(slashingAmount); diff --git a/yarn-project/end-to-end/src/e2e_p2p/multiple_validators_sentinel.parallel.test.ts b/yarn-project/end-to-end/src/e2e_p2p/multiple_validators_sentinel.parallel.test.ts index eb907ce5571e..b833d3ac6fc3 100644 --- a/yarn-project/end-to-end/src/e2e_p2p/multiple_validators_sentinel.parallel.test.ts +++ b/yarn-project/end-to-end/src/e2e_p2p/multiple_validators_sentinel.parallel.test.ts @@ -104,21 +104,33 @@ describe('e2e_p2p_multiple_validators_sentinel', () => { } }); - it('collects attestations for all validators on a node', async () => { - // Ensure all nodes see each other, especially the sentinel, before starting slot counting - await t.waitForP2PMeshConnectivity([...nodes, sentinel]); - - // Wait until validator nodes have advanced past their first proposed slot so that the - // pipelining warm-up period (where some attestations may be missed) is behind us. + const waitForPostWarmupCheckpoint = async (action: string): Promise => { await t.monitor.run(); const warmupSlot = Number(t.monitor.l2SlotNumber) + 1; - t.logger.info(`Waiting for warmup slot ${warmupSlot} before establishing initial slot`); + t.logger.info(`Waiting for warmup slot ${warmupSlot} before ${action}`); await retryUntil( async () => (await t.monitor.run()).l2SlotNumber >= warmupSlot, 'warmup slot', AZTEC_SLOT_DURATION * 3, ); + const warmupCheckpoint = t.monitor.checkpointNumber; + t.logger.info(`Waiting for checkpoint after warmup before ${action}`, { warmupCheckpoint }); + await retryUntil( + async () => (await t.monitor.run()).checkpointNumber > warmupCheckpoint, + 'post-warmup checkpoint', + AZTEC_SLOT_DURATION * (SLOT_COUNT + 1) * 3, + ); + }; + + it('collects attestations for all validators on a node', async () => { + // Ensure all nodes see each other, especially the sentinel, before starting slot counting + await t.waitForP2PMeshConnectivity([...nodes, sentinel]); + + // Wait until validator nodes have advanced past their first proposed slot and landed a checkpoint so that the + // pipelining warm-up period (where some attestations may be missed) is behind us. + await waitForPostWarmupCheckpoint('establishing initial slot'); + const { checkpointNumber: initialBlock, l2SlotNumber: initialSlot } = t.monitor; const timeout = AZTEC_SLOT_DURATION * SLOT_COUNT * 4; @@ -156,6 +168,8 @@ describe('e2e_p2p_multiple_validators_sentinel', () => { // Ensure all nodes see each other, especially the sentinel await t.waitForP2PMeshConnectivity([...nodes, sentinel]); + await waitForPostWarmupCheckpoint('stopping a validator node and establishing initial slot'); + // Stop the second node, this means the first node won't be able to propose since won't achieve quorum await tryStop(nodes[1]); @@ -200,11 +214,14 @@ describe('e2e_p2p_multiple_validators_sentinel', () => { const stats = await sentinel.getValidatorsStats(); t.logger.info(`Collected validator stats at slot ${t.monitor.l2SlotNumber}`, { stats }); - // Check that all of the first node validators have attestations recorded + const historyForSlot = (validator: (typeof firstNodeValidators)[number]) => + stats.stats[validator.toString().toLowerCase()]?.history.filter(h => h.slot === slotForSentinel) ?? []; + + // Check that all of the first node validators have attestations recorded for the selected proposer slot. for (const validator of firstNodeValidators) { - const validatorStats = stats.stats[validator.toString().toLowerCase()]; - const history = validatorStats?.history.filter(h => h.slot > initialSlot && h.slot <= slotForSentinel) ?? []; + const history = historyForSlot(validator); t.logger.info(`Asserting stats for online validator ${validator}`, { history }); + expect(history).not.toBeEmpty(); expect( history.filter( h => h.status === 'attestation-missed' || h.status === 'blocks-missed' || h.status === 'checkpoint-missed', @@ -215,14 +232,13 @@ describe('e2e_p2p_multiple_validators_sentinel', () => { // At least one of the first node validators must have been seen as proposer const firstNodeBlockProposedHistory = firstNodeValidators .flatMap(v => stats.stats[v.toString().toLowerCase()].history) - .filter(h => h.slot > initialSlot && h.slot <= slotForSentinel) + .filter(h => h.slot === slotForSentinel) .filter(h => h.status === 'checkpoint-valid' || h.status === 'checkpoint-mined'); expect(firstNodeBlockProposedHistory).not.toBeEmpty(); - // And all of the proposers for the offline node must be seen as missed attestation or proposal + // And all of the validators for the offline node must be seen as missed attestation or proposal. for (const validator of offlineValidators) { - const validatorStats = stats.stats[validator.toString().toLowerCase()]; - const history = validatorStats.history?.filter(h => h.slot > initialSlot && h.slot <= slotForSentinel) ?? []; + const history = historyForSlot(validator); t.logger.info(`Asserting stats for offline validator ${validator}`, { history }); expect( history.filter( diff --git a/yarn-project/end-to-end/src/e2e_slashing/attested_invalid_proposal.test.ts b/yarn-project/end-to-end/src/e2e_slashing/attested_invalid_proposal.test.ts index 3dc0f67ad3e5..ae15e396f0fd 100644 --- a/yarn-project/end-to-end/src/e2e_slashing/attested_invalid_proposal.test.ts +++ b/yarn-project/end-to-end/src/e2e_slashing/attested_invalid_proposal.test.ts @@ -135,15 +135,18 @@ async function advanceToEpochBeforePipelinedTargetSlot({ const currentEpoch = await cheatCodes.getEpoch(); const nextEpoch = Number(currentEpoch) + 1; const firstSlotOfNextEpoch = nextEpoch * Number(epochDuration); + // The prior pipelined target can start first after the epoch warp and consume the bad proposer config. + const priorPipelinedTargetSlot = SlotNumber(firstSlotOfNextEpoch); const pipelinedTargetSlot = SlotNumber(firstSlotOfNextEpoch + 1); + const priorProposer = await epochCache.getProposerAttesterAddressInSlot(priorPipelinedTargetSlot); const proposer = await epochCache.getProposerAttesterAddressInSlot(pipelinedTargetSlot); logger.info( `Checking pipelined target slot ${pipelinedTargetSlot} in epoch ${nextEpoch} for proposer ${targetProposer}`, - { proposer: proposer?.toString() }, + { proposer: proposer?.toString(), priorPipelinedTargetSlot, priorProposer: priorProposer?.toString() }, ); - if (proposer?.equals(targetProposer)) { + if (proposer?.equals(targetProposer) && !priorProposer?.equals(targetProposer)) { return { targetEpoch: EpochNumber(nextEpoch), targetSlot: pipelinedTargetSlot }; } @@ -202,7 +205,15 @@ describe('e2e_slashing_attested_invalid_proposal', () => { async function createInvalidProposalSlashingScenario({ badProposerConfig = {}, - }: { badProposerConfig?: Partial[0]> } = {}) { + corruptBlockProposal = true, + expectBadProposerOffense = true, + expectedBadProposerOffenseType = OffenseType.BROADCASTED_INVALID_BLOCK_PROPOSAL, + }: { + badProposerConfig?: Partial[0]>; + corruptBlockProposal?: boolean; + expectBadProposerOffense?: boolean; + expectedBadProposerOffenseType?: OffenseType; + } = {}) { const { rollup } = await t.getContracts(); await t.ctx.cheatCodes.rollup.advanceToEpoch(EpochNumber(4)); @@ -212,7 +223,9 @@ describe('e2e_slashing_attested_invalid_proposal', () => { { ...t.ctx.aztecNodeConfig, dontStartSequencer: true, - invalidBlockProposalIndexWithinCheckpoint: BAD_BLOCK_INDEX_WITHIN_CHECKPOINT, + ...(corruptBlockProposal + ? { invalidBlockProposalIndexWithinCheckpoint: BAD_BLOCK_INDEX_WITHIN_CHECKPOINT } + : {}), ...badProposerConfig, }, t.ctx.dateProvider!, @@ -389,11 +402,18 @@ describe('e2e_slashing_attested_invalid_proposal', () => { }); const expectedSlashOffenses = [ - { - description: 'bad proposer broadcasted invalid block proposal', - validator: badProposer, - offenseType: OffenseType.BROADCASTED_INVALID_BLOCK_PROPOSAL, - }, + ...(expectBadProposerOffense + ? [ + { + description: + expectedBadProposerOffenseType === OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL + ? 'bad proposer broadcasted invalid checkpoint proposal' + : 'bad proposer broadcasted invalid block proposal', + validator: badProposer, + offenseType: expectedBadProposerOffenseType, + }, + ] + : []), { description: 'lazy validator attested to invalid checkpoint proposal', validator: lazyValidator, @@ -439,6 +459,16 @@ describe('e2e_slashing_attested_invalid_proposal', () => { }; } + it('slashes a lazy attester for an invalid checkpoint proposal', async () => { + await createInvalidProposalSlashingScenario({ + badProposerConfig: { + broadcastInvalidCheckpointProposalOnly: true, + }, + corruptBlockProposal: false, + expectBadProposerOffense: false, + }); + }); + it('slashes a lazy attester for an invalid checkpoint and clears it on delayed equivocation', async () => { const { rollup, diff --git a/yarn-project/foundation/src/bigint-buffer/bigint-buffer.test.ts b/yarn-project/foundation/src/bigint-buffer/bigint-buffer.test.ts index 474c26fffb7b..baabd3d20381 100644 --- a/yarn-project/foundation/src/bigint-buffer/bigint-buffer.test.ts +++ b/yarn-project/foundation/src/bigint-buffer/bigint-buffer.test.ts @@ -1,6 +1,25 @@ -import { fromHex, toHex } from './index.js'; +import { fromHex, toBufferBE, toHex } from './index.js'; describe('bigint-buffer', () => { + describe('toBufferBE', () => { + it('serializes zero to a zeroed buffer of the requested width', () => { + expect(toBufferBE(0n, 32)).toEqual(Buffer.alloc(32)); + }); + + it('big-endian pads small values on the left', () => { + expect(toBufferBE(1n, 4)).toEqual(Buffer.from([0, 0, 0, 1])); + expect(toBufferBE(0x0102n, 4)).toEqual(Buffer.from([0, 0, 1, 2])); + }); + + it('serializes a value that exactly fills the width', () => { + expect(toBufferBE(0xdeadbeefn, 4)).toEqual(Buffer.from('deadbeef', 'hex')); + }); + + it('throws on negative values', () => { + expect(() => toBufferBE(-1n, 32)).toThrow('negative'); + }); + }); + describe('toHex', () => { it('does not pad even length', () => { expect(toHex(16n)).toEqual('0x10'); diff --git a/yarn-project/foundation/src/bigint-buffer/index.ts b/yarn-project/foundation/src/bigint-buffer/index.ts index a97045ac4bec..69a11323cab8 100644 --- a/yarn-project/foundation/src/bigint-buffer/index.ts +++ b/yarn-project/foundation/src/bigint-buffer/index.ts @@ -49,9 +49,14 @@ export function toBufferLE(num: bigint, width: number): Buffer { * @returns A big-endian buffer representation of num. */ export function toBufferBE(num: bigint, width: number): Buffer { - if (num < BigInt(0)) { + if (num < 0n) { throw new Error(`Cannot convert negative bigint ${num.toString()} to buffer with toBufferBE.`); } + // The values serialized in the hot paths are overwhelmingly zero (field elements are mostly + // zero-padding in protocol structs), and the hex round-trip is wasteful for them. + if (num === 0n) { + return Buffer.alloc(width); + } const hex = num.toString(16); const buffer = Buffer.from(hex.padStart(width * 2, '0').slice(0, width * 2), 'hex'); if (buffer.length > width) { diff --git a/yarn-project/foundation/src/branded-types/checkpoint_number.ts b/yarn-project/foundation/src/branded-types/checkpoint_number.ts index 97c1f1f14b95..194b3062cf8a 100644 --- a/yarn-project/foundation/src/branded-types/checkpoint_number.ts +++ b/yarn-project/foundation/src/branded-types/checkpoint_number.ts @@ -1,5 +1,6 @@ import { z } from 'zod'; +import { isDefined } from '../types/index.js'; import type { BlockNumber } from './block_number.js'; import type { Branded } from './types.js'; @@ -90,6 +91,15 @@ CheckpointNumber.add = function (n: CheckpointNumber, increment: number): Checkp return CheckpointNumber(n + increment); }; +/** Computes max of a set of checkpoint numbers, ignoring undefined values. */ +CheckpointNumber.max = function (...values: (CheckpointNumber | undefined)[]): CheckpointNumber | undefined { + const filtered = values.filter(isDefined); + if (filtered.length === 0) { + return undefined; + } + return CheckpointNumber(Math.max(...filtered)); +}; + /** The zero checkpoint value. */ CheckpointNumber.ZERO = CheckpointNumber(0); diff --git a/yarn-project/foundation/src/collection/index.ts b/yarn-project/foundation/src/collection/index.ts index 64ebf402e3ba..b62ed9c1f586 100644 --- a/yarn-project/foundation/src/collection/index.ts +++ b/yarn-project/foundation/src/collection/index.ts @@ -1,3 +1,4 @@ export * from './array.js'; +export * from './lru_map.js'; export * from './lru_set.js'; export * from './object.js'; diff --git a/yarn-project/foundation/src/collection/lru_map.test.ts b/yarn-project/foundation/src/collection/lru_map.test.ts new file mode 100644 index 000000000000..9c25853c07f6 --- /dev/null +++ b/yarn-project/foundation/src/collection/lru_map.test.ts @@ -0,0 +1,163 @@ +import { LruMap } from './lru_map.js'; + +describe('LruMap', () => { + it('stores and retrieves values', () => { + const map = new LruMap(3); + map.set('a', 1); + map.set('b', 2); + expect(map.get('a')).toBe(1); + expect(map.get('b')).toBe(2); + expect(map.get('c')).toBeUndefined(); + }); + + it('reports correct size', () => { + const map = new LruMap(5); + expect(map.size).toBe(0); + map.set(1, 'a'); + expect(map.size).toBe(1); + map.set(2, 'b'); + map.set(3, 'c'); + expect(map.size).toBe(3); + }); + + it('overwrites the value of an existing key without growing', () => { + const map = new LruMap(5); + map.set('a', 1); + map.set('a', 2); + expect(map.get('a')).toBe(2); + expect(map.size).toBe(1); + }); + + it('does not grow beyond maxSize', () => { + const map = new LruMap(3); + map.set(1, 1); + map.set(2, 2); + map.set(3, 3); + map.set(4, 4); + expect(map.size).toBe(3); + expect(map.get(1)).toBeUndefined(); // evicted (least recent) + expect(map.get(2)).toBe(2); + expect(map.get(3)).toBe(3); + expect(map.get(4)).toBe(4); + }); + + it('evicts least recently used, not least recently added', () => { + const map = new LruMap(3); + map.set('a', 1); + map.set('b', 2); + map.set('c', 3); + + // Access 'a' via get(), making it the most recently used + expect(map.get('a')).toBe(1); + + // Now 'b' is the least recently used. Adding 'd' should evict 'b'. + map.set('d', 4); + expect(map.get('b')).toBeUndefined(); // evicted + expect(map.get('a')).toBe(1); // kept (was refreshed) + expect(map.get('c')).toBe(3); + expect(map.get('d')).toBe(4); + }); + + it('refreshes recency on set() of existing key', () => { + const map = new LruMap(3); + map.set('a', 1); + map.set('b', 2); + map.set('c', 3); + + // Re-set 'a', refreshing its recency + map.set('a', 10); + + // 'b' is now least recent. Adding 'd' should evict 'b'. + map.set('d', 4); + expect(map.get('b')).toBeUndefined(); // evicted + expect(map.get('a')).toBe(10); + expect(map.size).toBe(3); + }); + + it('has() does not refresh recency', () => { + const map = new LruMap(2); + map.set('a', 1); + map.set('b', 2); + + // has('a') must not refresh recency, so 'a' stays the LRU entry + expect(map.has('a')).toBe(true); + map.set('c', 3); + expect(map.has('a')).toBe(false); // evicted + expect(map.has('b')).toBe(true); + expect(map.has('c')).toBe(true); + }); + + it('deletes entries', () => { + const map = new LruMap(5); + map.set('a', 1); + map.set('b', 2); + expect(map.delete('a')).toBe(true); + expect(map.delete('a')).toBe(false); + expect(map.get('a')).toBeUndefined(); + expect(map.get('b')).toBe(2); + expect(map.size).toBe(1); + }); + + it('reuses freed capacity after delete', () => { + const map = new LruMap(2); + map.set(1, 1); + map.set(2, 2); + map.delete(1); + map.set(3, 3); + expect(map.size).toBe(2); + expect(map.get(2)).toBe(2); + expect(map.get(3)).toBe(3); + }); + + it('clears all entries', () => { + const map = new LruMap(5); + map.set(1, 1); + map.set(2, 2); + map.set(3, 3); + map.clear(); + expect(map.size).toBe(0); + expect(map.get(1)).toBeUndefined(); + }); + + it('works correctly after clear and re-add', () => { + const map = new LruMap(2); + map.set('a', 1); + map.set('b', 2); + map.clear(); + map.set('c', 3); + map.set('d', 4); + expect(map.size).toBe(2); + expect(map.get('a')).toBeUndefined(); + expect(map.get('c')).toBe(3); + expect(map.get('d')).toBe(4); + }); + + it('works with maxSize of 1', () => { + const map = new LruMap(1); + map.set(1, 1); + expect(map.get(1)).toBe(1); + map.set(2, 2); + expect(map.get(1)).toBeUndefined(); + expect(map.get(2)).toBe(2); + expect(map.size).toBe(1); + }); + + it('throws on invalid maxSize', () => { + expect(() => new LruMap(0)).toThrow('LruMap maxSize must be at least 1'); + expect(() => new LruMap(-1)).toThrow('LruMap maxSize must be at least 1'); + }); + + it('handles sequential evictions correctly', () => { + const map = new LruMap(3); + // Fill to capacity + for (let i = 0; i < 3; i++) { + map.set(i, i); + } + // Evict each one in FIFO order (no access refreshes) + for (let i = 3; i < 10; i++) { + map.set(i, i); + expect(map.size).toBe(3); + expect(map.get(i - 3)).toBeUndefined(); // oldest was evicted + } + }); +}); diff --git a/yarn-project/foundation/src/collection/lru_map.ts b/yarn-project/foundation/src/collection/lru_map.ts new file mode 100644 index 000000000000..0ae66c6bb57b --- /dev/null +++ b/yarn-project/foundation/src/collection/lru_map.ts @@ -0,0 +1,143 @@ +/** Node in a doubly-linked list used by {@link LruMap}. */ +type LruNode = { + key: K; + value: V; + prev: LruNode | undefined; + next: LruNode | undefined; +}; + +/** + * A bounded key-value map with Least Recently Used (LRU) eviction. + * Both {@link get} and {@link set} count as an access and refresh the entry's + * recency, so entries that are actively used stay in the map longest. + * + * Uses a doubly-linked list for O(1) ordering and a Map for O(1) lookup. + * Head = least recent, tail = most recent. + */ +export class LruMap { + /** Map from key to its linked-list node for O(1) lookup. */ + private readonly map = new Map>(); + private head: LruNode | undefined; + private tail: LruNode | undefined; + + constructor(private readonly maxSize: number) { + if (maxSize < 1) { + throw new Error('LruMap maxSize must be at least 1'); + } + } + + /** Number of entries in the map. */ + get size(): number { + return this.map.size; + } + + /** Returns true if the key is present, without refreshing its recency. */ + has(key: K): boolean { + return this.map.has(key); + } + + /** + * Returns the value for the key, or undefined if absent. + * Refreshes the entry's recency so it becomes the most recently used. + */ + get(key: K): V | undefined { + const node = this.map.get(key); + if (!node) { + return undefined; + } + this.moveToTail(node); + return node.value; + } + + /** + * Stores a value for the key, refreshing its recency. If the key already exists, overwrites the value. + * If the map is at capacity, evicts the least recently used entry. + */ + set(key: K, value: V): void { + const existing = this.map.get(key); + if (existing) { + existing.value = value; + this.moveToTail(existing); + return; + } + + if (this.map.size >= this.maxSize) { + this.evictHead(); + } + + const node: LruNode = { key, value, prev: this.tail, next: undefined }; + if (this.tail) { + this.tail.next = node; + } else { + this.head = node; + } + this.tail = node; + this.map.set(key, node); + } + + /** Removes the entry for the key, returning true if it was present. */ + delete(key: K): boolean { + const node = this.map.get(key); + if (!node) { + return false; + } + this.unlink(node); + this.map.delete(key); + return true; + } + + /** Removes all entries from the map. */ + clear(): void { + this.map.clear(); + this.head = undefined; + this.tail = undefined; + } + + /** Unlinks a node from its current position and relinks it at the tail. */ + private moveToTail(node: LruNode): void { + if (node === this.tail) { + return; + } + this.unlink(node); + + node.prev = this.tail; + node.next = undefined; + if (this.tail) { + this.tail.next = node; + } else { + this.head = node; + } + this.tail = node; + } + + /** Detaches a node from the linked list, fixing up its neighbours and the head/tail pointers. */ + private unlink(node: LruNode): void { + if (node.prev) { + node.prev.next = node.next; + } else { + this.head = node.next; + } + if (node.next) { + node.next.prev = node.prev; + } else { + this.tail = node.prev; + } + node.prev = undefined; + node.next = undefined; + } + + /** Evicts the head (least recently used) node. */ + private evictHead(): void { + const oldHead = this.head; + if (!oldHead) { + return; + } + this.head = oldHead.next; + if (this.head) { + this.head.prev = undefined; + } else { + this.tail = undefined; + } + this.map.delete(oldHead.key); + } +} diff --git a/yarn-project/foundation/src/config/env_var.ts b/yarn-project/foundation/src/config/env_var.ts index c62136c29ca7..99869e33e6b4 100644 --- a/yarn-project/foundation/src/config/env_var.ts +++ b/yarn-project/foundation/src/config/env_var.ts @@ -164,6 +164,7 @@ export type EnvVar = | 'P2P_MIN_TX_POOL_AGE_MS' | 'P2P_MISSING_TX_COLLECTION_DEADLINE_SLOTS' | 'P2P_RPC_PRICE_BUMP_PERCENTAGE' + | 'P2P_TX_VALIDATION_CACHE_SIZE' | 'DEBUG_P2P_INSTRUMENT_MESSAGES' | 'PEER_ID_PRIVATE_KEY' | 'PEER_ID_PRIVATE_KEY_PATH' diff --git a/yarn-project/p2p/src/client/factory.ts b/yarn-project/p2p/src/client/factory.ts index 721ed3129afb..3f271e43744f 100644 --- a/yarn-project/p2p/src/client/factory.ts +++ b/yarn-project/p2p/src/client/factory.ts @@ -24,6 +24,7 @@ import { createTxValidatorForTransactionsEnteringPendingTxPool, getDefaultAllowedSetupFunctions, } from '../msg_validators/index.js'; +import { TxValidationCache } from '../msg_validators/tx_validator/tx_validation_cache.js'; import { DummyP2PService } from '../services/dummy_service.js'; import { LibP2PService } from '../services/index.js'; import { createFileStoreTxSources } from '../services/tx_collection/file_store_tx_source.js'; @@ -137,6 +138,9 @@ export async function createP2PClient( attestationPool: deps.attestationPool ?? new AttestationPool(attestationStore, telemetry), }; + const txValidationCache = + config.txValidationCacheSize > 0 ? new TxValidationCache(config.txValidationCacheSize) : undefined; + const p2pService = await createP2PService( config, archiver, @@ -151,9 +155,15 @@ export async function createP2PClient( packageVersion, logger.createChild('libp2p_service'), telemetry, + txValidationCache, ); - const txValidatorForTxCollection = createTxValidatorForOnDemandReceivedTxs(proofVerifier, config); + const txValidatorForTxCollection = createTxValidatorForOnDemandReceivedTxs( + proofVerifier, + config, + /*bindings=*/ undefined, + txValidationCache, + ); const nodeSources = [ ...createNodeRpcTxSources(config.txCollectionNodeRpcUrls, txValidatorForTxCollection, config), ...(deps.rpcTxProviders ?? []).map( @@ -230,6 +240,7 @@ async function createP2PService( packageVersion: string, logger: Logger, telemetry: TelemetryClient, + txValidationCache?: TxValidationCache, ) { if (!config.p2pEnabled) { logger.verbose('P2P is disabled. Using dummy P2P service.'); @@ -253,6 +264,7 @@ async function createP2PService( blockMinFeesProvider, telemetry, logger: logger.createChild(`libp2p_service`), + txValidationCache, }); return p2pService; diff --git a/yarn-project/p2p/src/config.ts b/yarn-project/p2p/src/config.ts index b8e388b98747..e9206ebbc705 100644 --- a/yarn-project/p2p/src/config.ts +++ b/yarn-project/p2p/src/config.ts @@ -189,6 +189,9 @@ export interface P2PConfig /** The node's seen message ID cache size */ seenMessageCacheSize: number; + /** Maximum number of (validator, tx) pairs to keep in the tx validation LRU cache. */ + txValidationCacheSize: number; + /** True to disable the status handshake on peer connected. */ p2pDisableStatusHandshake?: boolean; @@ -512,6 +515,11 @@ export const p2pConfigMappings: ConfigMappingsType = { description: 'The number of messages to keep in the seen message cache', ...numberConfigHelper(100_000), // 100K }, + txValidationCacheSize: { + env: 'P2P_TX_VALIDATION_CACHE_SIZE', + description: 'Maximum number of items to keep in the tx validation LRU cache.', + ...numberConfigHelper(5_000), + }, p2pDisableStatusHandshake: { env: 'P2P_DISABLE_STATUS_HANDSHAKE', description: 'True to disable the status handshake on peer connected.', diff --git a/yarn-project/p2p/src/msg_validators/tx_validator/cached_tx_validator.test.ts b/yarn-project/p2p/src/msg_validators/tx_validator/cached_tx_validator.test.ts new file mode 100644 index 000000000000..0c0b44bb97df --- /dev/null +++ b/yarn-project/p2p/src/msg_validators/tx_validator/cached_tx_validator.test.ts @@ -0,0 +1,71 @@ +import { mockTx } from '@aztec/stdlib/testing'; +import type { Tx, TxValidationResult, TxValidator } from '@aztec/stdlib/tx'; + +import { jest } from '@jest/globals'; + +import { CachedTxValidator } from './cached_tx_validator.js'; +import type { ITxValidationCache } from './tx_validation_cache.js'; + +describe('CachedTxValidator', () => { + class TestValidator implements TxValidator { + public readonly identifier = Symbol('TestValidator'); + + constructor(private readonly validateImpl: (tx: Tx) => Promise) {} + + public validateTx(tx: Tx): Promise { + return this.validateImpl(tx); + } + } + + class TestTxValidatorCache implements ITxValidationCache { + public readonly getOrValidate: jest.MockedFunction; + + constructor(impl?: ITxValidationCache['getOrValidate']) { + this.getOrValidate = jest.fn(impl ?? ((_s, _h, validate) => validate())); + } + } + + it('returns inner validator unchanged when cache is not provided', () => { + const inner = new TestValidator(() => Promise.resolve({ result: 'valid' })); + + const wrapped = CachedTxValidator.new(inner, undefined); + + expect(wrapped).toBe(inner); + }); + + it('delegates validation to cache.getOrValidate using validator identifier and tx hash', async () => { + const tx = await mockTx(1); + const validate = jest.fn<(tx: Tx) => Promise>().mockResolvedValue({ result: 'valid' }); + const inner = new TestValidator(txArg => validate(txArg)); + const cache = new TestTxValidatorCache(); + + const wrapped = CachedTxValidator.new(inner, cache); + await wrapped.validateTx(tx); + + expect(cache.getOrValidate).toHaveBeenCalledTimes(1); + expect(cache.getOrValidate).toHaveBeenCalledWith(inner.identifier, tx, expect.any(Function)); + expect(validate).toHaveBeenCalledTimes(1); + }); + + it('returns the value produced by cache.getOrValidate', async () => { + const tx = await mockTx(2); + const result: TxValidationResult = { result: 'invalid', reason: ['cache-hit'] }; + const validate = jest.fn<(tx: Tx) => Promise>().mockResolvedValue({ result: 'valid' }); + const inner = new TestValidator(txArg => validate(txArg)); + const cache = new TestTxValidatorCache(() => Promise.resolve(result)); + + const wrapped = CachedTxValidator.new(inner, cache); + + await expect(wrapped.validateTx(tx)).resolves.toEqual(result); + expect(validate).not.toHaveBeenCalled(); + }); + + it('propagates rejections from cache.getOrValidate', async () => { + const tx = await mockTx(3); + const error = new Error('cache failed'); + const cache = new TestTxValidatorCache(() => Promise.reject(error)); + const wrapped = CachedTxValidator.new(new TestValidator(() => Promise.resolve({ result: 'valid' })), cache); + + await expect(wrapped.validateTx(tx)).rejects.toThrow(error.message); + }); +}); diff --git a/yarn-project/p2p/src/msg_validators/tx_validator/cached_tx_validator.ts b/yarn-project/p2p/src/msg_validators/tx_validator/cached_tx_validator.ts new file mode 100644 index 000000000000..226faad79bb3 --- /dev/null +++ b/yarn-project/p2p/src/msg_validators/tx_validator/cached_tx_validator.ts @@ -0,0 +1,31 @@ +import type { Tx, TxValidationResult, TxValidator } from '@aztec/stdlib/tx'; + +import type { ITxValidationCache } from './tx_validation_cache.js'; + +/** Wraps a {@link TxValidator} to cache its results in a shared {@link ITxValidationCache}. */ +export class CachedTxValidator implements TxValidator { + constructor( + private readonly inner: TxValidator, + private readonly validatorSymbol: symbol, + private readonly cache: ITxValidationCache, + ) {} + + public static new( + inner: TxValidator & { identifier: symbol }, + cache?: ITxValidationCache, + ): TxValidator { + return CachedTxValidator.newWithIdentifier(inner, inner.identifier, cache); + } + + public static newWithIdentifier( + inner: TxValidator, + identifier: symbol, + cache?: ITxValidationCache, + ): TxValidator { + return cache ? new CachedTxValidator(inner, identifier, cache) : inner; + } + + public validateTx(tx: T): Promise { + return this.cache.getOrValidate(this.validatorSymbol, tx, () => this.inner.validateTx(tx)); + } +} diff --git a/yarn-project/p2p/src/msg_validators/tx_validator/data_validator.ts b/yarn-project/p2p/src/msg_validators/tx_validator/data_validator.ts index 7c284b6d0ce3..d1dcd7c75ead 100644 --- a/yarn-project/p2p/src/msg_validators/tx_validator/data_validator.ts +++ b/yarn-project/p2p/src/msg_validators/tx_validator/data_validator.ts @@ -20,6 +20,8 @@ import { } from '@aztec/stdlib/tx'; export class DataTxValidator implements TxValidator { + public readonly identifier: symbol = Symbol('DataTxValidator'); + #log: Logger; constructor(bindings?: LoggerBindings) { diff --git a/yarn-project/p2p/src/msg_validators/tx_validator/factory.ts b/yarn-project/p2p/src/msg_validators/tx_validator/factory.ts index 33ba7a6ffb59..e15078e1bb6d 100644 --- a/yarn-project/p2p/src/msg_validators/tx_validator/factory.ts +++ b/yarn-project/p2p/src/msg_validators/tx_validator/factory.ts @@ -53,6 +53,7 @@ import type { TxMetaData } from '../../mem_pools/tx_pool_v2/tx_metadata.js'; import { AggregateTxValidator } from './aggregate_tx_validator.js'; import { ArchiveCache } from './archive_cache.js'; import { type ArchiveSource, BlockHeaderTxValidator } from './block_header_validator.js'; +import { CachedTxValidator } from './cached_tx_validator.js'; import { ContractInstanceTxValidator } from './contract_instance_validator.js'; import { DataTxValidator } from './data_validator.js'; import { DoubleSpendTxValidator, type NullifierSource } from './double_spend_validator.js'; @@ -64,6 +65,7 @@ import { SizeTxValidator } from './size_validator.js'; import { TimestampTxValidator } from './timestamp_validator.js'; import { TxPermittedValidator } from './tx_permitted_validator.js'; import { TxProofValidator } from './tx_proof_validator.js'; +import { TxValidationCache } from './tx_validation_cache.js'; /** * A validator paired with a peer penalty severity. @@ -210,8 +212,9 @@ function createTxValidatorForMinimumTxIntegrityChecks( rollupVersion: number; }, bindings?: LoggerBindings, + cache?: TxValidationCache, ): TxValidator { - return new AggregateTxValidator( + const aggregate = new AggregateTxValidator( new MetadataTxValidator( { l1ChainId: new Fr(l1ChainId), @@ -222,10 +225,14 @@ function createTxValidatorForMinimumTxIntegrityChecks( bindings, ), new SizeTxValidator(bindings), - new DataTxValidator(bindings), + CachedTxValidator.new(new DataTxValidator(bindings), cache), new ContractInstanceTxValidator(bindings), - new TxProofValidator(verifier, bindings), + CachedTxValidator.new(new TxProofValidator(verifier, bindings), cache), ); + + // This validator is not state-dependent so we can cache it. + const identifier = Symbol('TxValidatorForMinimumTxIntegrityChecks'); + return CachedTxValidator.newWithIdentifier(aggregate, identifier, cache); } /** @@ -244,8 +251,9 @@ export function createTxValidatorForOnDemandReceivedTxs( rollupVersion: number; }, bindings?: LoggerBindings, + cache?: TxValidationCache, ): TxValidator { - return createTxValidatorForMinimumTxIntegrityChecks(verifier, { l1ChainId, rollupVersion }, bindings); + return createTxValidatorForMinimumTxIntegrityChecks(verifier, { l1ChainId, rollupVersion }, bindings, cache); } /** @@ -263,8 +271,9 @@ export function createTxValidatorForBlockProposalReceivedTxs( rollupVersion: number; }, bindings?: LoggerBindings, + cache?: TxValidationCache, ): TxValidator { - return createTxValidatorForMinimumTxIntegrityChecks(verifier, { l1ChainId, rollupVersion }, bindings); + return createTxValidatorForMinimumTxIntegrityChecks(verifier, { l1ChainId, rollupVersion }, bindings, cache); } /** diff --git a/yarn-project/p2p/src/msg_validators/tx_validator/index.ts b/yarn-project/p2p/src/msg_validators/tx_validator/index.ts index 893796772a3b..4a21303b725e 100644 --- a/yarn-project/p2p/src/msg_validators/tx_validator/index.ts +++ b/yarn-project/p2p/src/msg_validators/tx_validator/index.ts @@ -14,3 +14,5 @@ export * from './tx_permitted_validator.js'; export * from './timestamp_validator.js'; export * from './size_validator.js'; export * from './factory.js'; +export * from './tx_validation_cache.js'; +export * from './cached_tx_validator.js'; diff --git a/yarn-project/p2p/src/msg_validators/tx_validator/tx_proof_validator.ts b/yarn-project/p2p/src/msg_validators/tx_validator/tx_proof_validator.ts index eae4cbf33c3c..d461097a738e 100644 --- a/yarn-project/p2p/src/msg_validators/tx_validator/tx_proof_validator.ts +++ b/yarn-project/p2p/src/msg_validators/tx_validator/tx_proof_validator.ts @@ -3,6 +3,8 @@ import type { ClientProtocolCircuitVerifier } from '@aztec/stdlib/interfaces/ser import { TX_ERROR_INVALID_PROOF, Tx, type TxValidationResult, type TxValidator } from '@aztec/stdlib/tx'; export class TxProofValidator implements TxValidator { + public readonly identifier: symbol = Symbol('TxProofValidator'); + #log: Logger; constructor( diff --git a/yarn-project/p2p/src/msg_validators/tx_validator/tx_validation_cache.test.ts b/yarn-project/p2p/src/msg_validators/tx_validator/tx_validation_cache.test.ts new file mode 100644 index 000000000000..4402ba4abaa9 --- /dev/null +++ b/yarn-project/p2p/src/msg_validators/tx_validator/tx_validation_cache.test.ts @@ -0,0 +1,190 @@ +import { sleep } from '@aztec/foundation/sleep'; +import { mockTx } from '@aztec/stdlib/testing'; +import type { Tx, TxValidationResult } from '@aztec/stdlib/tx'; + +import { jest } from '@jest/globals'; + +import { TxValidationCache } from './tx_validation_cache.js'; + +describe('TxValidationCache', () => { + const validatorA = Symbol('validatorA'); + const validatorB = Symbol('validatorB'); + + let cache: TxValidationCache; + let tx: Tx; + + beforeEach(async () => { + cache = new TxValidationCache(100); + tx = await mockTx(1); + }); + + // The cache key is derived asynchronously, so a call's promise is only registered after its key + // resolves. This waits until a call has been registered before issuing a follow-up, making the + // coalescing assertions deterministic rather than dependent on the order in which key digests resolve. + const waitUntilCached = async (validatorSymbol: symbol, forTx: Tx) => { + const key = await cache.key(validatorSymbol, forTx); + while (cache.get(key) === undefined) { + await sleep(1); + } + }; + + describe('get / set', () => { + it('returns undefined on a cache miss', async () => { + expect(cache.get(await cache.key(validatorA, tx))).toBeUndefined(); + }); + + it('returns the stored promise on a cache hit', async () => { + const result: TxValidationResult = { result: 'valid' }; + cache.set(await cache.key(validatorA, tx), Promise.resolve(result)); + + await expect(cache.get(await cache.key(validatorA, tx))).resolves.toEqual(result); + }); + + it('does not share entries across different validator symbols', async () => { + cache.set(await cache.key(validatorA, tx), Promise.resolve({ result: 'valid' })); + + expect(cache.get(await cache.key(validatorB, tx))).toBeUndefined(); + }); + + it('does not share entries across different txs', async () => { + const otherTx = await mockTx(2); + cache.set(await cache.key(validatorA, tx), Promise.resolve({ result: 'valid' })); + + expect(cache.get(await cache.key(validatorA, otherTx))).toBeUndefined(); + }); + }); + + describe('LRU eviction', () => { + it('evicts the least-recently-used entry when the cache is full', async () => { + const smallCache = new TxValidationCache(2); + const tx1 = await mockTx(10); + const tx2 = await mockTx(11); + const tx3 = await mockTx(12); + const result: TxValidationResult = { result: 'valid' }; + + smallCache.set(await smallCache.key(validatorA, tx1), Promise.resolve(result)); + smallCache.set(await smallCache.key(validatorA, tx2), Promise.resolve(result)); + // tx1 is now the LRU entry; adding tx3 should evict it + smallCache.set(await smallCache.key(validatorA, tx3), Promise.resolve(result)); + + expect(smallCache.get(await smallCache.key(validatorA, tx1))).toBeUndefined(); + expect(smallCache.get(await smallCache.key(validatorA, tx2))).toBeDefined(); + expect(smallCache.get(await smallCache.key(validatorA, tx3))).toBeDefined(); + }); + + it('refreshes recency on get so that accessed entries are not evicted first', async () => { + const smallCache = new TxValidationCache(2); + const tx1 = await mockTx(20); + const tx2 = await mockTx(21); + const tx3 = await mockTx(22); + const result: TxValidationResult = { result: 'valid' }; + + smallCache.set(await smallCache.key(validatorA, tx1), Promise.resolve(result)); + smallCache.set(await smallCache.key(validatorA, tx2), Promise.resolve(result)); + // Access tx1 so tx2 becomes the LRU entry + void smallCache.get(await smallCache.key(validatorA, tx1)); + smallCache.set(await smallCache.key(validatorA, tx3), Promise.resolve(result)); + + expect(smallCache.get(await smallCache.key(validatorA, tx1))).toBeDefined(); + expect(smallCache.get(await smallCache.key(validatorA, tx2))).toBeUndefined(); + expect(smallCache.get(await smallCache.key(validatorA, tx3))).toBeDefined(); + }); + + it('throws when constructed with maxSize < 1', () => { + expect(() => new TxValidationCache(0)).toThrow(); + }); + }); + + describe('getOrValidate', () => { + it('calls validate and caches the result on a miss', async () => { + const expected: TxValidationResult = { result: 'invalid', reason: ['bad'] }; + const validate = jest.fn<() => Promise>().mockResolvedValue(expected); + + await expect(cache.getOrValidate(validatorA, tx, validate)).resolves.toEqual(expected); + expect(validate).toHaveBeenCalledTimes(1); + }); + + it('returns the cached promise on a hit without calling validate again', async () => { + const expected: TxValidationResult = { result: 'valid' }; + const validate = jest.fn<() => Promise>().mockResolvedValue(expected); + + await cache.getOrValidate(validatorA, tx, validate); + await expect(cache.getOrValidate(validatorA, tx, validate)).resolves.toEqual(expected); + expect(validate).toHaveBeenCalledTimes(1); + }); + + it('shares an in-flight validation so concurrent calls for the same key validate once', async () => { + const expected: TxValidationResult = { result: 'invalid', reason: ['bad proof'] }; + + let resolveValidation!: (v: TxValidationResult) => void; + const inFlight = new Promise(resolve => { + resolveValidation = resolve; + }); + const validate = jest.fn<() => Promise>().mockReturnValue(inFlight); + + const first = cache.getOrValidate(validatorA, tx, validate); + await waitUntilCached(validatorA, tx); + const second = cache.getOrValidate(validatorA, tx, validate); + const third = cache.getOrValidate(validatorA, tx, validate); + + resolveValidation(expected); + + await expect(first).resolves.toEqual(expected); + await expect(second).resolves.toEqual(expected); + await expect(third).resolves.toEqual(expected); + + expect(validate).toHaveBeenCalledTimes(1); + }); + + it('scopes validation results by validator symbol', async () => { + const resultA: TxValidationResult = { result: 'valid' }; + const resultB: TxValidationResult = { result: 'invalid', reason: ['nope'] }; + + const validateA = jest.fn<() => Promise>().mockResolvedValue(resultA); + const validateB = jest.fn<() => Promise>().mockResolvedValue(resultB); + + await expect(cache.getOrValidate(validatorA, tx, validateA)).resolves.toEqual(resultA); + await expect(cache.getOrValidate(validatorB, tx, validateB)).resolves.toEqual(resultB); + + expect(validateA).toHaveBeenCalledTimes(1); + expect(validateB).toHaveBeenCalledTimes(1); + }); + + it('caches a rejected validation so a later call reuses the failure without retrying', async () => { + const error = new Error('temporary failure'); + const success: TxValidationResult = { result: 'valid' }; + const validate = jest + .fn<() => Promise>() + .mockRejectedValueOnce(error) + .mockResolvedValueOnce(success); + + await expect(cache.getOrValidate(validatorA, tx, validate)).rejects.toThrow(error.message); + await expect(cache.getOrValidate(validatorA, tx, validate)).rejects.toThrow(error.message); + expect(validate).toHaveBeenCalledTimes(1); + }); + + it('caches a rejected in-flight validation so a later call reuses the failure', async () => { + const error = new Error('downstream unavailable'); + const success: TxValidationResult = { result: 'invalid', reason: ['bad tx'] }; + + let rejectValidation!: (err: Error) => void; + const firstInFlight = new Promise((_, reject) => { + rejectValidation = reject; + }); + + const validate = jest + .fn<() => Promise>() + .mockReturnValueOnce(firstInFlight) + .mockResolvedValueOnce(success); + + const first = cache.getOrValidate(validatorA, tx, validate); + await waitUntilCached(validatorA, tx); + + rejectValidation(error); + await expect(first).rejects.toThrow(error.message); + + await expect(cache.getOrValidate(validatorA, tx, validate)).rejects.toThrow(error.message); + expect(validate).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/yarn-project/p2p/src/msg_validators/tx_validator/tx_validation_cache.ts b/yarn-project/p2p/src/msg_validators/tx_validator/tx_validation_cache.ts new file mode 100644 index 000000000000..f4d89590689d --- /dev/null +++ b/yarn-project/p2p/src/msg_validators/tx_validator/tx_validation_cache.ts @@ -0,0 +1,102 @@ +import { LruMap } from '@aztec/foundation/collection'; +import { type Logger, createLogger } from '@aztec/foundation/log'; +import type { Tx, TxValidationResult } from '@aztec/stdlib/tx'; + +import { webcrypto } from 'node:crypto'; + +/** + * Minimal interface consumed by {@link CachedTxValidator}. + * Keeping the dependency on an interface lets callers (and tests) substitute any cache implementation. + */ +export interface ITxValidationCache { + /** Returns the cached promise if present, otherwise calls `validate`, caches its promise, and returns it. */ + getOrValidate( + validatorSymbol: symbol, + tx: Tx, + validate: () => Promise, + ): Promise; +} + +/** + * Caches per-validator tx validation results to avoid redundant work across repeated validation calls. + * + * The cache key is composed from the validator symbol and tx hash, ensuring results are + * scoped to the specific validator that produced them. + * + * Promises are stored before they are awaited, so concurrent calls for the same pair share + * a single in-flight validation rather than launching duplicate work. + * + * Entries are evicted in least-recently-used order once the cache reaches `maxSize`. + */ +export class TxValidationCache implements ITxValidationCache { + #log: Logger; + + private readonly entries: LruMap>; + // Remember hashes for known Tx object references to skip rehashing on subsequent lookups. + // WeakMap holds keys weakly, so an entry doesn't keep the Tx alive once nothing else references it. + private readonly txHashesCache: WeakMap = new WeakMap(); + + constructor(maxSize: number) { + this.entries = new LruMap(maxSize); + this.#log = createLogger('p2p:tx_validation_cache'); + } + + /** + * Computes the cache key scoping a validation result to a specific validator and tx. + * + * @param validatorSymbol - The symbol of the validator. + * @param tx - The tx to compute the key for. + * @returns The cache key. + * + * Note: the key should NOT use the tx.hash because it can't be trusted at this point. + */ + public async key(validatorSymbol: symbol, tx: Tx): Promise { + // Hashing the whole tx takes a few milliseconds. So if we have already hashed + // this particular object, we avoid rehashing it. + let hash = this.txHashesCache.get(tx); + if (hash === undefined) { + hash = Buffer.from(await webcrypto.subtle.digest('SHA-256', tx.toBuffer())).toString('hex'); + this.txHashesCache.set(tx, hash); + } + return `${Symbol.keyFor(validatorSymbol) ?? validatorSymbol.toString()}:${hash}`; + } + + /** Returns the cached promise for the given key, or undefined if not cached. Refreshes recency. */ + public get(key: string): Promise | undefined { + return this.entries.get(key); + } + + /** Stores a validation promise under the given key, evicting the LRU entry if at capacity. */ + public set(key: string, result: Promise): void { + this.entries.set(key, result); + } + + /** Removes the cached validation promise for the given key. */ + public delete(key: string): void { + this.entries.delete(key); + } + + /** + * Returns the cached promise if present, otherwise calls `validate`, stores its promise + * immediately (before awaiting), and returns it. + */ + public async getOrValidate( + validatorSymbol: symbol, + tx: Tx, + validate: () => Promise, + ): Promise { + const key = await this.key(validatorSymbol, tx); + const cached = this.get(key); + if (cached !== undefined) { + this.#log.debug('Returning cached tx validation result', { + validator: validatorSymbol.toString(), + txHash: tx.txHash.toString(), + key: key, + }); + return cached; + } + const promise = validate(); + this.set(key, promise); + return promise; + } +} diff --git a/yarn-project/p2p/src/services/libp2p/libp2p_service.ts b/yarn-project/p2p/src/services/libp2p/libp2p_service.ts index 9c00dc517ba6..2da0cdecebea 100644 --- a/yarn-project/p2p/src/services/libp2p/libp2p_service.ts +++ b/yarn-project/p2p/src/services/libp2p/libp2p_service.ts @@ -75,6 +75,7 @@ import { createSecondStageTxValidationsForGossipedTransactions, createTxValidatorForBlockProposalReceivedTxs, } from '../../msg_validators/tx_validator/factory.js'; +import { TxValidationCache } from '../../msg_validators/tx_validator/tx_validation_cache.js'; import { GossipSubEvent } from '../../types/index.js'; import { type PubSubLibp2p, convertToMultiaddr } from '../../util.js'; import { getVersions } from '../../versioning.js'; @@ -202,6 +203,7 @@ export class LibP2PService extends WithTracer implements P2PService { private blockMinFeesProvider: BlockMinFeesProvider, telemetry: TelemetryClient, logger: Logger = createLogger('p2p:libp2p_service'), + private txValidationCache?: TxValidationCache, ) { super(telemetry, 'LibP2PService'); this.telemetry = telemetry; @@ -302,6 +304,7 @@ export class LibP2PService extends WithTracer implements P2PService { telemetry: TelemetryClient; logger: Logger; packageVersion: string; + txValidationCache?: TxValidationCache; }, ) { const { @@ -315,6 +318,7 @@ export class LibP2PService extends WithTracer implements P2PService { telemetry, logger, packageVersion, + txValidationCache, } = deps; const { p2pPort, maxPeerCount, listenAddress } = config; const bindAddrTcp = convertToMultiaddr(listenAddress, p2pPort, 'tcp'); @@ -522,6 +526,7 @@ export class LibP2PService extends WithTracer implements P2PService { blockMinFeesProvider, telemetry, logger, + txValidationCache, ); } @@ -1608,6 +1613,7 @@ export class LibP2PService extends WithTracer implements P2PService { l1ChainId: this.config.l1ChainId, rollupVersion: this.config.rollupVersion, proofVerifier: this.proofVerifier, + txValidationCache: this.txValidationCache, }, peerScoring: this.peerManager, validateRequestedBlockTxsConsistency: this.validateRequestedBlockTxsConsistency.bind(this), @@ -1619,6 +1625,7 @@ export class LibP2PService extends WithTracer implements P2PService { this.proofVerifier, { l1ChainId: this.config.l1ChainId, rollupVersion: this.config.rollupVersion }, this.logger.getBindings(), + this.txValidationCache, ); const results = await Promise.all( diff --git a/yarn-project/p2p/src/services/reqresp/batch-tx-requester/tx_validator.ts b/yarn-project/p2p/src/services/reqresp/batch-tx-requester/tx_validator.ts index dd8015a76a8a..e603482e051d 100644 --- a/yarn-project/p2p/src/services/reqresp/batch-tx-requester/tx_validator.ts +++ b/yarn-project/p2p/src/services/reqresp/batch-tx-requester/tx_validator.ts @@ -2,16 +2,23 @@ import type { ClientProtocolCircuitVerifier } from '@aztec/stdlib/interfaces/ser import type { TxValidator } from '@aztec/stdlib/tx'; import { createTxValidatorForOnDemandReceivedTxs } from '../../../msg_validators/index.js'; +import type { TxValidationCache } from '../../../msg_validators/tx_validator/tx_validation_cache.js'; export interface BatchRequestTxValidatorConfig { l1ChainId: number; rollupVersion: number; proofVerifier: ClientProtocolCircuitVerifier; + txValidationCache?: TxValidationCache; } export function createBatchRequestTxValidator(config: BatchRequestTxValidatorConfig): TxValidator { - return createTxValidatorForOnDemandReceivedTxs(config.proofVerifier, { - l1ChainId: config.l1ChainId, - rollupVersion: config.rollupVersion, - }); + return createTxValidatorForOnDemandReceivedTxs( + config.proofVerifier, + { + l1ChainId: config.l1ChainId, + rollupVersion: config.rollupVersion, + }, + /*bindings=*/ undefined, + config.txValidationCache, + ); } diff --git a/yarn-project/precommit.sh b/yarn-project/precommit.sh index a4863d268296..4407f3045225 100755 --- a/yarn-project/precommit.sh +++ b/yarn-project/precommit.sh @@ -18,6 +18,11 @@ staged_files=$(git diff-index --diff-filter=d --relative --cached --name-only HE # Filter for formattable files staged_format_files=$(echo "$staged_files" | grep -E '\.(json|js|mjs|cjs|ts)$' || true) +# Drop symlinks; prettier errors when handed a symbolic link (e.g. .codex/settings.json). +if [[ -n "$staged_format_files" ]]; then + staged_format_files=$(echo "$staged_format_files" | while IFS= read -r f; do [ -L "$f" ] || printf '%s\n' "$f"; done) +fi + # Get unstaged changes for formattable files unstaged_format_files=$(git diff --relative --name-only --diff-filter=d | grep -E '\.(json|js|mjs|cjs|ts)$' || true) diff --git a/yarn-project/sequencer-client/src/config.ts b/yarn-project/sequencer-client/src/config.ts index 16f703bccc9c..7801bc99f309 100644 --- a/yarn-project/sequencer-client/src/config.ts +++ b/yarn-project/sequencer-client/src/config.ts @@ -52,6 +52,7 @@ export const DefaultSequencerConfig = { secondsBeforeInvalidatingBlockAsNonCommitteeMember: 432, // 36 L1 blocks skipCollectingAttestations: false, skipInvalidateBlockAsProposer: false, + skipWaitForValidParentCheckpointOnL1: false, broadcastInvalidBlockProposal: false, broadcastInvalidCheckpointProposalOnly: false, injectFakeAttestation: false, @@ -188,6 +189,12 @@ export const sequencerConfigMappings: ConfigMappingsType = { description: 'Do not invalidate the previous block if invalid when we are the proposer (for testing only)', ...booleanConfigHelper(DefaultSequencerConfig.skipInvalidateBlockAsProposer), }, + skipWaitForValidParentCheckpointOnL1: { + description: + 'Bypass the parent checkpoint validity check before submitting a pipelined checkpoint, ' + + 'allowing the proposer to publish even when the parent landed on L1 with invalid attestations (for testing only)', + ...booleanConfigHelper(DefaultSequencerConfig.skipWaitForValidParentCheckpointOnL1), + }, broadcastInvalidBlockProposal: { description: 'Broadcast invalid block proposals with corrupted state (for testing only)', ...booleanConfigHelper(DefaultSequencerConfig.broadcastInvalidBlockProposal), diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts index 209e04b4856b..e86b5533edcc 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts @@ -396,6 +396,13 @@ export class CheckpointProposalJob implements Traceable { * If the parent has invalid attestations, enqueues an invalidation. Returns whether to proceed with the proposal. */ protected async waitForValidParentCheckpointOnL1(): Promise { + if (this.config.skipWaitForValidParentCheckpointOnL1) { + this.log.warn(`Skipping waitForValidParentCheckpointOnL1 due to test configuration`, { + checkpointNumber: this.checkpointNumber, + }); + return true; + } + const parentCheckpointNumber = CheckpointNumber(this.checkpointNumber - 1); // Wait until archiver has synced L1 past the parent's slot (slotNow) diff --git a/yarn-project/slasher/src/config.ts b/yarn-project/slasher/src/config.ts index ddc934f02b62..64ca1e97c6c1 100644 --- a/yarn-project/slasher/src/config.ts +++ b/yarn-project/slasher/src/config.ts @@ -71,7 +71,7 @@ export const slasherConfigMappings: ConfigMappingsType = { }, slashDataWithholdingPenalty: { env: 'SLASH_DATA_WITHHOLDING_PENALTY', - description: 'Penalty amount for slashing validators for data withholding (set to 0 to disable).', + description: 'Penalty for data withholding (0 records offenses without slash votes).', ...bigintConfigHelper(DefaultSlasherConfig.slashDataWithholdingPenalty), }, slashDataWithholdingToleranceSlots: { @@ -125,29 +125,28 @@ export const slasherConfigMappings: ConfigMappingsType = { }, slashInactivityPenalty: { env: 'SLASH_INACTIVITY_PENALTY', - description: 'Penalty amount for slashing an inactive validator (set to 0 to disable).', + description: 'Penalty for an inactive validator (0 records offenses without slash votes).', ...bigintConfigHelper(DefaultSlasherConfig.slashInactivityPenalty), }, slashProposeInvalidAttestationsPenalty: { env: 'SLASH_PROPOSE_INVALID_ATTESTATIONS_PENALTY', - description: 'Penalty amount for slashing a proposer that proposed invalid attestations (set to 0 to disable).', + description: 'Penalty for proposing invalid attestations (0 records offenses without slash votes).', ...bigintConfigHelper(DefaultSlasherConfig.slashProposeInvalidAttestationsPenalty), }, slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty: { env: 'SLASH_PROPOSE_DESCENDANT_OF_CHECKPOINT_WITH_INVALID_ATTESTATIONS_PENALTY', description: - 'Penalty amount for slashing a proposer that published a checkpoint building on an invalid checkpoint (set to 0 to disable).', + 'Penalty for publishing a checkpoint building on an invalid checkpoint (0 records offenses without slash votes).', ...bigintConfigHelper(DefaultSlasherConfig.slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty), }, slashAttestInvalidCheckpointProposalPenalty: { env: 'SLASH_ATTEST_INVALID_CHECKPOINT_PROPOSAL_PENALTY', - description: - 'Penalty amount for slashing a validator that attested to an invalid checkpoint proposal (set to 0 to disable).', + description: 'Penalty for attesting to an invalid checkpoint proposal (0 records offenses without slash votes).', ...bigintConfigHelper(DefaultSlasherConfig.slashAttestInvalidCheckpointProposalPenalty), }, slashUnknownPenalty: { env: 'SLASH_UNKNOWN_PENALTY', - description: 'Penalty amount for slashing a validator for an unknown offense (set to 0 to disable).', + description: 'Penalty for an unknown offense (0 records offenses without slash votes).', ...bigintConfigHelper(DefaultSlasherConfig.slashUnknownPenalty), }, slashOffenseExpirationRounds: { diff --git a/yarn-project/slasher/src/index.ts b/yarn-project/slasher/src/index.ts index d947aad82568..d383a639287f 100644 --- a/yarn-project/slasher/src/index.ts +++ b/yarn-project/slasher/src/index.ts @@ -1,6 +1,7 @@ export * from './config.js'; export * from './watchers/data_withholding_watcher.js'; export * from './watchers/attestations_block_watcher.js'; +export * from './watchers/attested_invalid_proposal_watcher.js'; export * from './watchers/broadcasted_invalid_checkpoint_proposal_watcher.js'; export * from './watchers/checkpoint_equivocation_watcher.js'; export * from './slasher_client.js'; diff --git a/yarn-project/slasher/src/slash_offenses_collector.ts b/yarn-project/slasher/src/slash_offenses_collector.ts index f92e5dd790ad..22833112fe0a 100644 --- a/yarn-project/slasher/src/slash_offenses_collector.ts +++ b/yarn-project/slasher/src/slash_offenses_collector.ts @@ -4,7 +4,7 @@ import { SerialQueue } from '@aztec/foundation/queue'; import type { Prettify } from '@aztec/foundation/types'; import type { L1RollupConstants } from '@aztec/stdlib/epoch-helpers'; import type { SlasherConfig } from '@aztec/stdlib/interfaces/server'; -import { type Offense, getSlotForOffense } from '@aztec/stdlib/slashing'; +import { type Offense, getOffenseTypeName, getSlotForOffense } from '@aztec/stdlib/slashing'; import type { SlasherOffensesStore } from './stores/offenses_store.js'; import { @@ -89,7 +89,7 @@ export class SlashOffensesCollector { }; if (this.shouldSkipOffense(offense)) { - this.log.verbose('Skipping offense during grace period', offense); + this.log.verbose('Skipping offense during grace period', this.getOffenseLogData(offense)); continue; } @@ -98,13 +98,16 @@ export class SlashOffensesCollector { if (this.settings.slashingAmounts) { const minSlash = this.settings.slashingAmounts[0]; if (arg.amount < minSlash) { - this.log.warn(`Offense amount ${arg.amount} is below minimum slashing amount ${minSlash}`); + this.log.warn( + `Offense amount ${arg.amount} is below minimum slashing amount ${minSlash}`, + this.getOffenseLogData(offense), + ); } } - this.log.info(`Adding pending offense for validator ${arg.validator}`, offense); + this.log.info(`Adding pending offense for validator ${arg.validator}`, this.getOffenseLogData(offense)); } else { - this.log.debug('Skipping repeated offense', offense); + this.log.debug('Skipping repeated offense', this.getOffenseLogData(offense)); } } } @@ -114,7 +117,7 @@ export class SlashOffensesCollector { const cleared = await this.offensesStore.clearOffenses(arg); if (cleared > 0) { this.log.info(`Cleared ${cleared} pending offenses`, { - offenseType: arg.offenseType, + offenseType: getOffenseTypeName(arg.offenseType), epochOrSlot: arg.epochOrSlot, validators: arg.validators?.map(validator => validator.toString()), }); @@ -139,6 +142,14 @@ export class SlashOffensesCollector { return offenseSlot < this.settings.rollupRegisteredAtL2Slot + this.config.slashGracePeriodL2Slots; } + private getOffenseLogData(offense: Offense) { + return { + ...offense, + validator: offense.validator.toString(), + offenseType: getOffenseTypeName(offense.offenseType), + }; + } + private enqueueStoreMutation(label: string, callback: () => Promise) { void this.storeMutationQueue.put(callback).catch(err => this.log.error(`Error handling ${label}`, err)); } diff --git a/yarn-project/slasher/src/slasher_client.test.ts b/yarn-project/slasher/src/slasher_client.test.ts index fa657c55bdc5..beb78674a5f7 100644 --- a/yarn-project/slasher/src/slasher_client.test.ts +++ b/yarn-project/slasher/src/slasher_client.test.ts @@ -301,6 +301,24 @@ describe('SlasherClient', () => { const actions = await slasherClient.getProposerActions(SlotNumber.fromBigInt(currentSlot)); expect(actions).toHaveLength(0); }); + + it('should not return any action for zero-amount offenses', async () => { + const currentRound = 5n; + const currentSlot = currentRound * BigInt(roundSize); + const targetRound = 3n; + + await offensesStore.addOffense( + createOffense({ + validator: committee[0], + epochOrSlot: targetRound * BigInt(roundSize), + offenseType: OffenseType.PROPOSED_INSUFFICIENT_ATTESTATIONS, + amount: 0n, + }), + ); + + const actions = await slasherClient.getProposerActions(SlotNumber.fromBigInt(currentSlot)); + expect(actions).toHaveLength(0); + }); }); describe('execute-slash', () => { diff --git a/yarn-project/slasher/src/slasher_client.ts b/yarn-project/slasher/src/slasher_client.ts index 4ec78bb55b79..f6dce62add58 100644 --- a/yarn-project/slasher/src/slasher_client.ts +++ b/yarn-project/slasher/src/slasher_client.ts @@ -14,6 +14,7 @@ import { type ProposerSlashAction, type ProposerSlashActionProvider, getEpochsForRound, + getOffenseTypeName, getSlashConsensusVotesFromOffenses, } from '@aztec/stdlib/slashing'; @@ -50,6 +51,14 @@ export type SlasherClientConfig = SlashOffensesCollectorConfig & 'slashValidatorsAlways' | 'slashValidatorsNever' | 'slashExecuteRoundsLookBack' | 'slashMaxPayloadSize' >; +type AlwaysSlashOffense = { + validator: EthAddress; + amount: bigint; + offenseType: OffenseType.UNKNOWN; +}; + +type SlashVoteOffense = Offense | AlwaysSlashOffense; + /** * The Slasher client is responsible for managing slashable offenses using * the consensus-based slashing model where proposers vote on individual validator offenses. @@ -309,7 +318,7 @@ export class SlasherClient implements ProposerSlashActionProvider, SlasherClient // Compute offenses to slash, by loading the offenses for this round, adding synthetic offenses // for validators that should always be slashed, and removing the ones that should never be slashed. const offensesForRound = await this.gatherOffensesForRound(currentRound); - const offensesFromAlwaysSlash = (this.config.slashValidatorsAlways ?? []).map(validator => ({ + const offensesFromAlwaysSlash: AlwaysSlashOffense[] = (this.config.slashValidatorsAlways ?? []).map(validator => ({ validator, amount: this.settings.slashingAmounts[2], offenseType: OffenseType.UNKNOWN, @@ -323,7 +332,7 @@ export class SlasherClient implements ProposerSlashActionProvider, SlasherClient slotNumber, currentRound, slashedRound, - offensesToForgive, + offensesFromAlwaysSlash: offensesFromAlwaysSlash.map(getOffenseLogData), slashValidatorsAlways: this.config.slashValidatorsAlways, }); } @@ -333,7 +342,7 @@ export class SlasherClient implements ProposerSlashActionProvider, SlasherClient slotNumber, currentRound, slashedRound, - offensesToForgive, + offensesToForgive: offensesToForgive.map(getOffenseLogData), slashValidatorsNever: this.config.slashValidatorsNever, }); } @@ -343,11 +352,11 @@ export class SlasherClient implements ProposerSlashActionProvider, SlasherClient return undefined; } - this.log.info(`Voting to slash ${offensesToSlash.length} offenses`, { + this.log.debug(`Computing slash votes for ${offensesToSlash.length} offenses`, { slotNumber, currentRound, slashedRound, - offensesToSlash, + offensesToSlash: offensesToSlash.map(getOffenseLogData), }); const committees = await this.collectCommitteesActiveDuringRound(slashedRound); @@ -365,12 +374,20 @@ export class SlasherClient implements ProposerSlashActionProvider, SlasherClient slotNumber, currentRound, slashedRound, - offensesToSlash, + offensesToSlash: offensesToSlash.map(getOffenseLogData), committees, }); return undefined; } + this.log.info(`Voting to slash ${offensesToSlash.length} offenses`, { + slotNumber, + slashedRound, + currentRound, + votes, + offensesToSlash: offensesToSlash.map(getOffenseLogData), + }); + this.log.debug(`Computed votes for slashing ${offensesToSlash.length} offenses`, { slashedRound, currentRound, @@ -429,3 +446,11 @@ export class SlasherClient implements ProposerSlashActionProvider, SlasherClient return round - BigInt(this.settings.slashingOffsetInRounds); } } + +function getOffenseLogData(offense: SlashVoteOffense) { + return { + ...offense, + validator: offense.validator.toString(), + offenseType: getOffenseTypeName(offense.offenseType), + }; +} diff --git a/yarn-project/slasher/src/watchers/attestations_block_watcher.test.ts b/yarn-project/slasher/src/watchers/attestations_block_watcher.test.ts index 933362d00cde..54a26f58d642 100644 --- a/yarn-project/slasher/src/watchers/attestations_block_watcher.test.ts +++ b/yarn-project/slasher/src/watchers/attestations_block_watcher.test.ts @@ -1,13 +1,15 @@ -import type { EpochCache } from '@aztec/epoch-cache'; +import type { EpochCache, EpochCommitteeInfo } from '@aztec/epoch-cache'; import { CheckpointNumber, EpochNumber, SlotNumber } from '@aztec/foundation/branded-types'; import { Fr } from '@aztec/foundation/curves/bn254'; import { EthAddress } from '@aztec/foundation/eth-address'; import type { + DescendentOfInvalidAttestationsCheckpointEvent, InvalidCheckpointDetectedEvent, L2BlockSourceEventEmitter, ValidateCheckpointNegativeResult, } from '@aztec/stdlib/block'; import type { CheckpointInfo } from '@aztec/stdlib/checkpoint'; +import type { L1RollupConstants } from '@aztec/stdlib/epoch-helpers'; import { OffenseType } from '@aztec/stdlib/slashing'; import { jest } from '@jest/globals'; @@ -45,8 +47,15 @@ describe('AttestationsBlockWatcher', () => { proposer = EthAddress.fromString('0x0000000000000000000000000000000000000abc'); committee = [proposer, EthAddress.fromString('0x0000000000000000000000000000000000000def')]; - // Default mock return value + // Default mock return values epochCache.getProposerFromEpochCommittee.mockReturnValue(proposer); + epochCache.getL1Constants.mockReturnValue({ epochDuration: 32 } as L1RollupConstants); + epochCache.getCommitteeForEpoch.mockResolvedValue({ + committee, + seed: 0n, + epoch: EpochNumber(0), + isEscapeHatchOpen: false, + } as EpochCommitteeInfo); }); it('should emit WANT_TO_SLASH_EVENT for proposer when invalid checkpoint detected due to insufficient attestations', () => { @@ -110,36 +119,19 @@ describe('AttestationsBlockWatcher', () => { expect(handler).toHaveBeenCalledTimes(1); }); - it('should emit WANT_TO_SLASH_EVENT for attestors when checkpoint built on invalid parent', () => { - // First, handle an invalid checkpoint using the pre-configured data - const invalidCheckpointValidationResult: ValidateCheckpointNegativeResult = { - valid: false, - reason: 'insufficient-attestations', - checkpoint: checkpointInfo, - committee, - epoch: EpochNumber(1), - seed: 0n, - attestors: [], - attestations: [], - }; - - const invalidCheckpointEvent: InvalidCheckpointDetectedEvent = { - type: 'invalidCheckpointDetected', - validationResult: invalidCheckpointValidationResult, - }; - - watcher.handleInvalidCheckpoint(invalidCheckpointEvent); - - // Now handle a checkpoint that builds on the invalid checkpoint + it('emits both an invalid-attestations slash and a descendant slash for a checkpoint that has invalid attestations and builds on an invalid ancestor', async () => { + // A checkpoint that both has invalid attestations of its own and extends a previously-rejected + // ancestor produces two offenses. The archiver reports each via its own event, so the watcher sees + // an invalidCheckpointDetected (own attestations) and a descendentOfInvalidAttestationsCheckpointDetected + // (extends a rejected ancestor) for the same checkpoint. const childCheckpointInfo: CheckpointInfo = { archive: Fr.random(), - lastArchive: checkpointInfo.archive, // Parent archive + lastArchive: checkpointInfo.archive, // Parent archive (the rejected ancestor) slotNumber: SlotNumber(2), checkpointNumber: CheckpointNumber(2), timestamp: BigInt(Math.floor(Date.now() / 1000)), }; const proposer2 = EthAddress.fromString('0x0000000000000000000000000000000000000def'); - epochCache.getProposerFromEpochCommittee.mockReturnValue(proposer2); const attestor1 = EthAddress.fromString('0x0000000000000000000000000000000000000111'); @@ -156,13 +148,11 @@ describe('AttestationsBlockWatcher', () => { attestations: [], }; - const childEvent: InvalidCheckpointDetectedEvent = { + // First event: slash the proposer for the invalid attestations on its own checkpoint. + watcher.handleInvalidCheckpoint({ type: 'invalidCheckpointDetected', validationResult: childValidationResult, - }; - - handler.mockClear(); - watcher.handleInvalidCheckpoint(childEvent); + }); expect(handler).toHaveBeenCalledWith([ { @@ -173,25 +163,28 @@ describe('AttestationsBlockWatcher', () => { } satisfies WantToSlashArgs, ]); + // Second event: slash the same proposer for building on the rejected ancestor. + await watcher.handleDescendantOfInvalid({ + type: 'descendentOfInvalidAttestationsCheckpointDetected', + checkpoint: childCheckpointInfo, + ancestorArchiveRoot: checkpointInfo.archive, + ancestorCheckpointNumber: checkpointInfo.checkpointNumber, + }); + expect(handler).toHaveBeenCalledWith([ { - validator: attestor1, - amount: config.slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty, - offenseType: OffenseType.PROPOSED_DESCENDANT_OF_CHECKPOINT_WITH_INVALID_ATTESTATIONS, - epochOrSlot: 2n, - }, - { - validator: attestor2, + validator: proposer2, amount: config.slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty, offenseType: OffenseType.PROPOSED_DESCENDANT_OF_CHECKPOINT_WITH_INVALID_ATTESTATIONS, epochOrSlot: 2n, - }, - ] satisfies WantToSlashArgs[]); + } satisfies WantToSlashArgs, + ]); expect(handler).toHaveBeenCalledTimes(2); }); - it('should not process the same invalid checkpoint twice', () => { - const validationResult: ValidateCheckpointNegativeResult = { + it('emits WANT_TO_SLASH_EVENT for proposer when a valid-attestations descendant of an invalid checkpoint is detected', async () => { + // Seed the watcher with one invalid ancestor so the descendant event hits the cache. + const invalidValidationResult: ValidateCheckpointNegativeResult = { valid: false, reason: 'insufficient-attestations', checkpoint: checkpointInfo, @@ -201,20 +194,120 @@ describe('AttestationsBlockWatcher', () => { attestors: [], attestations: [], }; - - const event: InvalidCheckpointDetectedEvent = { + watcher.handleInvalidCheckpoint({ type: 'invalidCheckpointDetected', - validationResult, + validationResult: invalidValidationResult, + }); + + handler.mockClear(); + + const descendantCheckpointInfo: CheckpointInfo = { + archive: Fr.random(), + lastArchive: checkpointInfo.archive, + slotNumber: SlotNumber(2), + checkpointNumber: CheckpointNumber(2), + timestamp: BigInt(Math.floor(Date.now() / 1000)), + }; + const descendantProposer = EthAddress.fromString('0x0000000000000000000000000000000000000def'); + epochCache.getProposerFromEpochCommittee.mockReturnValue(descendantProposer); + + const event: DescendentOfInvalidAttestationsCheckpointEvent = { + type: 'descendentOfInvalidAttestationsCheckpointDetected', + checkpoint: descendantCheckpointInfo, + ancestorArchiveRoot: checkpointInfo.archive, + ancestorCheckpointNumber: checkpointInfo.checkpointNumber, }; - // Handle the same event twice - watcher.handleInvalidCheckpoint(event); - watcher.handleInvalidCheckpoint(event); + await watcher.handleDescendantOfInvalid(event); - // Should only emit once (duplicate was skipped) + expect(handler).toHaveBeenCalledWith([ + { + validator: descendantProposer, + amount: config.slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty, + offenseType: OffenseType.PROPOSED_DESCENDANT_OF_CHECKPOINT_WITH_INVALID_ATTESTATIONS, + epochOrSlot: 2n, + } satisfies WantToSlashArgs, + ]); expect(handler).toHaveBeenCalledTimes(1); }); + it('slashes a further descendant that both has invalid attestations and extends another descendant', async () => { + // The archiver tracks the rejected chain and reports each descendant via its own event. A further + // descendant (D2) that both has its own invalid attestations and extends an earlier descendant (D1) + // is reported through two events; the watcher slashes its proposer for each offense independently. + const d2: CheckpointInfo = { + archive: Fr.random(), + lastArchive: Fr.random(), + slotNumber: SlotNumber(3), + checkpointNumber: CheckpointNumber(3), + timestamp: BigInt(Math.floor(Date.now() / 1000)), + }; + const d2Proposer = EthAddress.fromString('0x0000000000000000000000000000000000000bbb'); + epochCache.getProposerFromEpochCommittee.mockReturnValue(d2Proposer); + + // D2 has invalid attestations of its own. + watcher.handleInvalidCheckpoint({ + type: 'invalidCheckpointDetected', + validationResult: { + valid: false, + reason: 'insufficient-attestations', + checkpoint: d2, + committee, + epoch: EpochNumber(1), + seed: 0n, + attestors: [], + attestations: [], + }, + }); + + expect(handler).toHaveBeenCalledWith([ + { + validator: d2Proposer, + amount: config.slashProposeInvalidAttestationsPenalty, + offenseType: OffenseType.PROPOSED_INSUFFICIENT_ATTESTATIONS, + epochOrSlot: 3n, + } satisfies WantToSlashArgs, + ]); + + // D2 also extends an earlier descendant of an invalid checkpoint. + await watcher.handleDescendantOfInvalid({ + type: 'descendentOfInvalidAttestationsCheckpointDetected', + checkpoint: d2, + ancestorArchiveRoot: Fr.random(), + ancestorCheckpointNumber: CheckpointNumber(1), + }); + + expect(handler).toHaveBeenCalledWith([ + { + validator: d2Proposer, + amount: config.slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty, + offenseType: OffenseType.PROPOSED_DESCENDANT_OF_CHECKPOINT_WITH_INVALID_ATTESTATIONS, + epochOrSlot: 3n, + } satisfies WantToSlashArgs, + ]); + expect(handler).toHaveBeenCalledTimes(2); + }); + + it('handles descendant-of-invalid event when no proposer is found', async () => { + epochCache.getProposerFromEpochCommittee.mockReturnValue(undefined); + + const descendant: CheckpointInfo = { + archive: Fr.random(), + lastArchive: Fr.random(), + slotNumber: SlotNumber(2), + checkpointNumber: CheckpointNumber(2), + timestamp: BigInt(Math.floor(Date.now() / 1000)), + }; + await watcher.handleDescendantOfInvalid({ + type: 'descendentOfInvalidAttestationsCheckpointDetected', + checkpoint: descendant, + ancestorArchiveRoot: Fr.random(), + ancestorCheckpointNumber: CheckpointNumber(1), + }); + + expect(handler).not.toHaveBeenCalled(); + }); + it('should handle case when no proposer is found', () => { epochCache.getProposerFromEpochCommittee.mockReturnValue(undefined); diff --git a/yarn-project/slasher/src/watchers/attestations_block_watcher.ts b/yarn-project/slasher/src/watchers/attestations_block_watcher.ts index cd02e7312c5b..886797b2f445 100644 --- a/yarn-project/slasher/src/watchers/attestations_block_watcher.ts +++ b/yarn-project/slasher/src/watchers/attestations_block_watcher.ts @@ -1,15 +1,16 @@ import { EpochCache } from '@aztec/epoch-cache'; -import { SlotNumber } from '@aztec/foundation/branded-types'; +import { EpochNumber } from '@aztec/foundation/branded-types'; import { merge, pick } from '@aztec/foundation/collection'; -import { FifoSet } from '@aztec/foundation/fifo-set'; -import { type Logger, createLogger } from '@aztec/foundation/log'; +import { type Logger, type LoggerBindings, createLogger } from '@aztec/foundation/log'; import { + type DescendentOfInvalidAttestationsCheckpointEvent, type InvalidCheckpointDetectedEvent, type L2BlockSourceEventEmitter, L2BlockSourceEvents, type ValidateCheckpointNegativeResult, } from '@aztec/stdlib/block'; -import { OffenseType } from '@aztec/stdlib/slashing'; +import { getEpochAtSlot } from '@aztec/stdlib/epoch-helpers'; +import { OffenseType, getOffenseTypeName } from '@aztec/stdlib/slashing'; import EventEmitter from 'node:events'; @@ -20,22 +21,28 @@ const AttestationsBlockWatcherConfigKeys = [ 'slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty', 'slashProposeInvalidAttestationsPenalty', ] as const; -const MAX_INVALID_CHECKPOINTS = 100; type AttestationsBlockWatcherConfig = Pick; /** - * This watcher is responsible for detecting invalid blocks and creating slashing arguments for offenders. - * An invalid block is one that doesn't have enough attestations or has incorrect attestations. - * The proposer of an invalid block should be slashed. - * If there's another block consecutive to the invalid one, its proposer and attestors should also be slashed. + * Watches the archiver for checkpoints whose publication is itself a slashable offense. + * + * Two cases are handled, both targeting the proposer of the offending checkpoint: + * + * - Invalid-attestations checkpoint: the proposer published a checkpoint to L1 whose + * attestations are either insufficient (below quorum) or incorrect (signature from a + * non-committee member, malformed signature, etc.). Slashed via + * {@link OffenseType.PROPOSED_INSUFFICIENT_ATTESTATIONS} or + * {@link OffenseType.PROPOSED_INCORRECT_ATTESTATIONS}. + * + * - Descendant of an invalid checkpoint: the proposer published a checkpoint that extends a + * previously-rejected one. The descendant may itself have valid attestations, but it is still + * unusable. Triggered by the archiver's `CheckpointBuiltOnInvalidAncestorDetected` event + * when the descendant has valid attestations (skipped before ingestion). Slashes the descendant's + * proposer via {@link OffenseType.PROPOSED_DESCENDANT_OF_CHECKPOINT_WITH_INVALID_ATTESTATIONS}. */ export class AttestationsBlockWatcher extends (EventEmitter as new () => WatcherEmitter) implements Watcher { - private log: Logger = createLogger('attestations-block-watcher'); - - // Recently seen invalid archive roots. - private invalidArchiveRoots = FifoSet.withLimit(MAX_INVALID_CHECKPOINTS); - + private log: Logger; private config: AttestationsBlockWatcherConfig; private boundHandleInvalidCheckpoint = (event: InvalidCheckpointDetectedEvent) => { @@ -49,12 +56,23 @@ export class AttestationsBlockWatcher extends (EventEmitter as new () => Watcher } }; + private boundHandleDescendantOfInvalid = (event: DescendentOfInvalidAttestationsCheckpointEvent) => { + this.handleDescendantOfInvalid(event).catch(err => { + this.log.error('Error handling descendant of invalid checkpoint', err, { + checkpointNumber: event.checkpoint.checkpointNumber, + ancestorCheckpointNumber: event.ancestorCheckpointNumber, + }); + }); + }; + constructor( private l2BlockSource: L2BlockSourceEventEmitter, private epochCache: EpochCache, config: AttestationsBlockWatcherConfig, + bindings?: LoggerBindings, ) { super(); + this.log = createLogger('slasher:attestations-block-watcher', bindings); this.config = pick(config, ...AttestationsBlockWatcherConfigKeys); this.log.info('AttestationsBlockWatcher initialized'); } @@ -69,6 +87,10 @@ export class AttestationsBlockWatcher extends (EventEmitter as new () => Watcher L2BlockSourceEvents.InvalidAttestationsCheckpointDetected, this.boundHandleInvalidCheckpoint, ); + this.l2BlockSource.events.on( + L2BlockSourceEvents.DescendentOfInvalidAttestationsCheckpointDetected, + this.boundHandleDescendantOfInvalid, + ); return Promise.resolve(); } @@ -77,90 +99,87 @@ export class AttestationsBlockWatcher extends (EventEmitter as new () => Watcher L2BlockSourceEvents.InvalidAttestationsCheckpointDetected, this.boundHandleInvalidCheckpoint, ); + this.l2BlockSource.events.removeListener( + L2BlockSourceEvents.DescendentOfInvalidAttestationsCheckpointDetected, + this.boundHandleDescendantOfInvalid, + ); return Promise.resolve(); } /** Event handler for invalid checkpoints as reported by the archiver. Public for testing purposes. */ public handleInvalidCheckpoint(event: InvalidCheckpointDetectedEvent): void { const { validationResult } = event; - const checkpoint = validationResult.checkpoint; - - // Check if we already have processed this checkpoint, archiver may emit the same event multiple times - if (this.invalidArchiveRoots.has(checkpoint.archive.toString())) { - this.log.trace(`Already processed invalid checkpoint ${checkpoint.checkpointNumber}`); - return; - } + const { reason, checkpoint } = validationResult; this.log.verbose(`Detected invalid checkpoint ${checkpoint.checkpointNumber}`, { ...checkpoint, reason: validationResult.valid === false ? validationResult.reason : 'unknown', }); - this.invalidArchiveRoots.add(checkpoint.archive.toString()); + const { checkpointNumber, slotNumber: slot } = checkpoint; + const epochCommitteeInfo = { ...validationResult, isEscapeHatchOpen: false }; + const proposer = this.epochCache.getProposerFromEpochCommittee(epochCommitteeInfo, slot); - // Slash the proposer of the invalid checkpoint - this.slashProposer(event.validationResult); + if (!proposer) { + this.log.warn(`No proposer found for checkpoint ${checkpointNumber} at slot ${slot}`); + return; + } - // Check if the parent of this checkpoint is invalid as well, if so, we will slash its attestors as well - this.slashAttestorsOnAncestorInvalid(event.validationResult); - } + const offense = this.getOffenseFromInvalidationReason(reason); + const amount = this.config.slashProposeInvalidAttestationsPenalty; + const args: WantToSlashArgs = { + validator: proposer, + amount, + offenseType: offense, + epochOrSlot: BigInt(slot), + }; - private slashAttestorsOnAncestorInvalid(validationResult: ValidateCheckpointNegativeResult) { - const checkpoint = validationResult.checkpoint; - - const parentArchive = checkpoint.lastArchive.toString(); - if (this.invalidArchiveRoots.has(parentArchive)) { - const attestors = validationResult.attestors; - this.log.info( - `Want to slash attestors of checkpoint ${checkpoint.checkpointNumber} built on invalid checkpoint`, - { - ...checkpoint, - ...attestors, - parentArchive, - }, - ); + this.log.info(`Detected invalid attestations checkpoint proposer offense`, { + ...checkpoint, + reason, + validator: args.validator.toString(), + amount: args.amount, + offenseType: getOffenseTypeName(args.offenseType), + epochOrSlot: args.epochOrSlot, + }); - this.emit( - WANT_TO_SLASH_EVENT, - attestors.map(attestor => ({ - validator: attestor, - amount: this.config.slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty, - offenseType: OffenseType.PROPOSED_DESCENDANT_OF_CHECKPOINT_WITH_INVALID_ATTESTATIONS, - epochOrSlot: BigInt(SlotNumber(checkpoint.slotNumber)), - })), - ); - } + this.emit(WANT_TO_SLASH_EVENT, [args]); } - private slashProposer(validationResult: ValidateCheckpointNegativeResult) { - const { reason, checkpoint } = validationResult; - const checkpointNumber = checkpoint.checkpointNumber; + /** + * Event handler for valid-attestations checkpoints that build on a previously-rejected ancestor. + * The archiver emits this when ingesting the descendant, and we slash its proposer. + */ + public async handleDescendantOfInvalid(event: DescendentOfInvalidAttestationsCheckpointEvent): Promise { + const { checkpoint, ancestorCheckpointNumber, ancestorArchiveRoot } = event; + const slot = checkpoint.slotNumber; - const epochCommitteeInfo = { - committee: validationResult.committee, - seed: validationResult.seed, - epoch: validationResult.epoch, - isEscapeHatchOpen: false, - }; - const proposer = this.epochCache.getProposerFromEpochCommittee(epochCommitteeInfo, slot); + const epoch = EpochNumber(getEpochAtSlot(slot, this.epochCache.getL1Constants())); + const epochCommitteeInfo = await this.epochCache.getCommitteeForEpoch(epoch); + const proposer = this.epochCache.getProposerFromEpochCommittee({ ...epochCommitteeInfo, epoch }, slot); if (!proposer) { - this.log.warn(`No proposer found for checkpoint ${checkpointNumber} at slot ${slot}`); + this.log.warn( + `No proposer found for invalid descendant checkpoint ${checkpoint.checkpointNumber} at slot ${slot}`, + ); return; } - const offense = this.getOffenseFromInvalidationReason(reason); - const amount = this.config.slashProposeInvalidAttestationsPenalty; const args: WantToSlashArgs = { validator: proposer, - amount, - offenseType: offense, + amount: this.config.slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty, + offenseType: OffenseType.PROPOSED_DESCENDANT_OF_CHECKPOINT_WITH_INVALID_ATTESTATIONS, epochOrSlot: BigInt(slot), }; - this.log.info(`Want to slash proposer of checkpoint ${checkpointNumber} due to ${reason}`, { + this.log.info(`Detected invalid descendant checkpoint proposer offense`, { ...checkpoint, - ...args, + ancestorCheckpointNumber, + ancestorArchiveRoot: ancestorArchiveRoot.toString(), + validator: args.validator.toString(), + amount: args.amount, + offenseType: getOffenseTypeName(args.offenseType), + epochOrSlot: args.epochOrSlot, }); this.emit(WANT_TO_SLASH_EVENT, [args]); diff --git a/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.test.ts b/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.test.ts new file mode 100644 index 000000000000..db2696a3b0a0 --- /dev/null +++ b/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.test.ts @@ -0,0 +1,173 @@ +import type { EpochCacheInterface } from '@aztec/epoch-cache'; +import { SlotNumber } from '@aztec/foundation/branded-types'; +import { Secp256k1Signer } from '@aztec/foundation/crypto/secp256k1-signer'; +import type { L2BlockSource } from '@aztec/stdlib/block'; +import type { P2PClient } from '@aztec/stdlib/interfaces/server'; +import type { CheckpointAttestation } from '@aztec/stdlib/p2p'; +import { OffenseType } from '@aztec/stdlib/slashing'; +import { + makeCheckpointAttestationFromProposal, + makeCheckpointHeader, + makeCheckpointProposal, +} from '@aztec/stdlib/testing'; + +import { jest } from '@jest/globals'; +import { type MockProxy, mock } from 'jest-mock-extended'; + +import { DefaultSlasherConfig, type SlasherConfig } from '../config.js'; +import { WANT_TO_SLASH_EVENT, type WantToSlashArgs } from '../watcher.js'; +import { AttestedInvalidProposalWatcher, type InvalidProposalSlotSource } from './attested_invalid_proposal_watcher.js'; + +describe('AttestedInvalidProposalWatcher', () => { + let p2pClient: MockProxy>; + let l2BlockSource: MockProxy>; + let epochCache: MockProxy>; + let invalidProposalSlots: Set; + let proposalEquivocationSlots: Set; + let invalidProposalSlotSource: InvalidProposalSlotSource; + let config: SlasherConfig; + let watcher: AttestedInvalidProposalWatcher; + let handler: jest.MockedFunction<(args: WantToSlashArgs[]) => void>; + + beforeEach(() => { + p2pClient = mock>(); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([]); + l2BlockSource = mock>(); + l2BlockSource.getSyncedL2SlotNumber.mockResolvedValue(SlotNumber(11)); + epochCache = mock>(); + epochCache.getSlotNow.mockReturnValue(SlotNumber(11)); + epochCache.getL1Constants.mockReturnValue({ ethereumSlotDuration: 4 } as ReturnType< + EpochCacheInterface['getL1Constants'] + >); + invalidProposalSlots = new Set(); + proposalEquivocationSlots = new Set(); + invalidProposalSlotSource = { + hasInvalidProposals: slot => invalidProposalSlots.has(slot), + hasProposalEquivocation: slot => proposalEquivocationSlots.has(slot), + }; + config = { + ...DefaultSlasherConfig, + slashAttestInvalidCheckpointProposalPenalty: 13n, + }; + watcher = new AttestedInvalidProposalWatcher( + p2pClient, + invalidProposalSlotSource, + l2BlockSource, + epochCache, + config, + ); + handler = jest.fn(); + watcher.on(WANT_TO_SLASH_EVENT, handler); + }); + + const makeAttestation = async ( + slot: SlotNumber, + attesterSigner = Secp256k1Signer.random(), + ): Promise => { + const checkpointProposal = await makeCheckpointProposal({ + checkpointHeader: makeCheckpointHeader(1, { slotNumber: slot }), + }); + return makeCheckpointAttestationFromProposal(checkpointProposal, attesterSigner); + }; + + it('slashes checkpoint attestations already in the pool for a marked invalid proposal slot', async () => { + const slot = SlotNumber(10); + const attesterSigner = Secp256k1Signer.random(); + invalidProposalSlots.add(slot); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([await makeAttestation(slot, attesterSigner)]); + + await watcher.scanSlot(slot); + + expect(handler).toHaveBeenCalledWith([ + { + validator: attesterSigner.address, + amount: 13n, + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: 10n, + }, + ]); + }); + + it('emits zero-amount offenses when the penalty is zero', async () => { + const slot = SlotNumber(10); + const attesterSigner = Secp256k1Signer.random(); + invalidProposalSlots.add(slot); + watcher.updateConfig({ slashAttestInvalidCheckpointProposalPenalty: 0n }); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([await makeAttestation(slot, attesterSigner)]); + + await watcher.scanSlot(slot); + + expect(handler).toHaveBeenCalledWith([ + { + validator: attesterSigner.address, + amount: 0n, + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: 10n, + }, + ]); + }); + + it('deduplicates repeated scans for the same attester and slot', async () => { + const slot = SlotNumber(10); + invalidProposalSlots.add(slot); + const attestation = await makeAttestation(slot); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([attestation]); + + await watcher.scanSlot(slot); + await watcher.scanSlot(slot); + + expect(handler).toHaveBeenCalledTimes(1); + }); + + it('scans only marked invalid proposal slots once they are past the scan lag', async () => { + watcher = new AttestedInvalidProposalWatcher( + p2pClient, + invalidProposalSlotSource, + l2BlockSource, + epochCache, + config, + { scanSlotLookback: 2 }, + ); + invalidProposalSlots = new Set([SlotNumber(8), SlotNumber(9), SlotNumber(10)]); + + await watcher.scan(); + + expect(p2pClient.getCheckpointAttestationsForSlot.mock.calls.map(([slot]) => slot)).toEqual([ + SlotNumber(9), + SlotNumber(10), + ]); + }); + + it('does not rescan completed slots', async () => { + invalidProposalSlots = new Set([SlotNumber(9), SlotNumber(10)]); + + await watcher.scan(); + await watcher.scan(); + + expect(p2pClient.getCheckpointAttestationsForSlot.mock.calls.map(([slot]) => slot)).toEqual([ + SlotNumber(9), + SlotNumber(10), + ]); + + l2BlockSource.getSyncedL2SlotNumber.mockResolvedValue(SlotNumber(12)); + invalidProposalSlots = new Set([SlotNumber(9), SlotNumber(10), SlotNumber(11)]); + await watcher.scan(); + + expect(p2pClient.getCheckpointAttestationsForSlot.mock.calls.map(([slot]) => slot)).toEqual([ + SlotNumber(9), + SlotNumber(10), + SlotNumber(11), + ]); + }); + + it('does not slash attestations once proposal equivocation has been detected for the slot', async () => { + const slot = SlotNumber(10); + invalidProposalSlots.add(slot); + proposalEquivocationSlots.add(slot); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([await makeAttestation(slot)]); + + await watcher.scanSlot(slot); + + expect(handler).not.toHaveBeenCalled(); + }); +}); diff --git a/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.ts b/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.ts new file mode 100644 index 000000000000..440b23413f34 --- /dev/null +++ b/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.ts @@ -0,0 +1,168 @@ +import type { EpochCacheInterface } from '@aztec/epoch-cache'; +import { SlotNumber } from '@aztec/foundation/branded-types'; +import { merge, pick } from '@aztec/foundation/collection'; +import type { EthAddress } from '@aztec/foundation/eth-address'; +import { FifoSet } from '@aztec/foundation/fifo-set'; +import { type Logger, createLogger } from '@aztec/foundation/log'; +import { RunningPromise } from '@aztec/foundation/running-promise'; +import type { L2BlockSource } from '@aztec/stdlib/block'; +import type { P2PClient, SlasherConfig } from '@aztec/stdlib/interfaces/server'; +import type { CheckpointAttestation } from '@aztec/stdlib/p2p'; +import { OffenseType, getOffenseTypeName } from '@aztec/stdlib/slashing'; + +import EventEmitter from 'node:events'; + +import { WANT_TO_SLASH_EVENT, type WantToSlashArgs, type Watcher, type WatcherEmitter } from '../watcher.js'; + +const AttestedInvalidProposalWatcherConfigKeys = ['slashAttestInvalidCheckpointProposalPenalty'] as const; + +const SCAN_SLOT_LAG = 1; +const DEFAULT_SCAN_SLOT_LOOKBACK = 4; +const MAX_TRACKED_BAD_ATTESTATIONS = 10_000; + +type AttestedInvalidProposalWatcherConfig = Pick< + SlasherConfig, + (typeof AttestedInvalidProposalWatcherConfigKeys)[number] +>; + +type P2PCheckpointAttestationSource = Pick; + +type AttestedInvalidProposalWatcherOptions = { + scanSlotLookback?: number; + log?: Logger; +}; + +export type InvalidProposalSlotSource = { + hasInvalidProposals(slot: SlotNumber): boolean; + hasProposalEquivocation(slot: SlotNumber): boolean; +}; + +export class AttestedInvalidProposalWatcher extends (EventEmitter as new () => WatcherEmitter) implements Watcher { + private readonly log: Logger; + private readonly runningPromise: RunningPromise; + private readonly emittedOffenses = FifoSet.withLimit(MAX_TRACKED_BAD_ATTESTATIONS); + private readonly scanSlotLookback: number; + private config: AttestedInvalidProposalWatcherConfig; + private lastScannedSlot: SlotNumber | undefined; + + constructor( + private readonly p2pClient: P2PCheckpointAttestationSource, + private readonly invalidProposalSlotSource: InvalidProposalSlotSource, + private readonly l2BlockSource: Pick, + private readonly epochCache: Pick, + config: AttestedInvalidProposalWatcherConfig, + options: AttestedInvalidProposalWatcherOptions = {}, + ) { + super(); + const constants = epochCache.getL1Constants(); + this.log = options.log ?? createLogger('attested-invalid-proposal-watcher'); + this.config = pick(config, ...AttestedInvalidProposalWatcherConfigKeys); + this.scanSlotLookback = Math.max(1, options.scanSlotLookback ?? DEFAULT_SCAN_SLOT_LOOKBACK); + + const intervalMs = Math.max(1000, (constants.ethereumSlotDuration * 1000) / 4); + this.runningPromise = new RunningPromise(() => this.scan(), this.log, intervalMs); + this.log.info('AttestedInvalidProposalWatcher initialized', { scanSlotLookback: this.scanSlotLookback }); + } + + public updateConfig(config: Partial): void { + this.config = merge(this.config, pick(config, ...AttestedInvalidProposalWatcherConfigKeys)); + this.log.verbose('AttestedInvalidProposalWatcher config updated', this.config); + } + + public start(): Promise { + this.runningPromise.start(); + return Promise.resolve(); + } + + public stop(): Promise { + return this.runningPromise.stop(); + } + + public async scan(): Promise { + const currentSlot = (await this.l2BlockSource.getSyncedL2SlotNumber()) ?? this.epochCache.getSlotNow(); + // genesis + if (currentSlot <= SlotNumber(SCAN_SLOT_LAG)) { + return; + } + + const newestSlotToConsider = SlotNumber(currentSlot - SCAN_SLOT_LAG); + const oldestSlot = + this.lastScannedSlot === undefined + ? SlotNumber(Math.max(0, newestSlotToConsider - this.scanSlotLookback + 1)) + : SlotNumber(this.lastScannedSlot + 1); + if (oldestSlot > newestSlotToConsider) { + return; + } + + for (let slot = oldestSlot; slot <= newestSlotToConsider; slot++) { + await this.scanSlot(slot); + } + + this.lastScannedSlot = newestSlotToConsider; + } + + /** Scans a single invalid-proposal slot. */ + public async scanSlot(slot: SlotNumber): Promise { + if ( + this.invalidProposalSlotSource.hasProposalEquivocation(slot) || + !this.invalidProposalSlotSource.hasInvalidProposals(slot) + ) { + return; + } + + let attestations: CheckpointAttestation[]; + try { + attestations = await this.p2pClient.getCheckpointAttestationsForSlot(slot); + } catch (err) { + this.log.warn('Error getting checkpoint attestations for invalid proposal slot', { err, slot }); + return; + } + + const slashArgs = attestations + .map(attestation => this.getSlashArgs(slot, attestation)) + .filter((args): args is WantToSlashArgs => args !== undefined) + .filter(args => this.markAsNewOffense(args)); + + if (slashArgs.length === 0) { + return; + } + + this.log.info('Detected attestations to invalid checkpoint proposal', { + slot, + offenses: slashArgs.map(args => ({ + validator: args.validator.toString(), + amount: args.amount, + offenseType: getOffenseTypeName(args.offenseType), + epochOrSlot: args.epochOrSlot, + })), + }); + this.emit(WANT_TO_SLASH_EVENT, slashArgs); + } + + private getSlashArgs(slot: SlotNumber, attestation: CheckpointAttestation): WantToSlashArgs | undefined { + const attester = attestation.getSender(); + if (!attester) { + this.log.warn('Cannot slash checkpoint attestation with invalid signature', { + slot, + archive: attestation.archive.toString(), + }); + return undefined; + } + + return this.getSlashArgsForAttester(slot, attester); + } + + private getSlashArgsForAttester(slot: SlotNumber, attester: EthAddress): WantToSlashArgs { + return { + validator: attester, + amount: this.config.slashAttestInvalidCheckpointProposalPenalty, + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(slot), + }; + } + + private markAsNewOffense(args: WantToSlashArgs): boolean { + const key = `${args.validator.toString()}-${args.offenseType}-${args.epochOrSlot}`; + return this.emittedOffenses.addIfAbsent(key); + } +} diff --git a/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.test.ts b/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.test.ts index 5d8432b9d3d2..f9f5b25ca669 100644 --- a/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.test.ts +++ b/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.test.ts @@ -195,6 +195,26 @@ describe('BroadcastedInvalidCheckpointProposalWatcher', () => { expect(handler).not.toHaveBeenCalled(); }); + it('emits zero-amount offenses when the penalty is zero', async () => { + const signer = Secp256k1Signer.random(); + const slot = SlotNumber(10); + const blocks = await makeBlocks(signer, slot, 4); + const checkpoint = await makeCheckpointCore(signer, slot, blocks[1]); + watcher.updateConfig({ slashBroadcastedInvalidCheckpointProposalPenalty: 0n }); + mockProposals(slot, blocks, [checkpoint]); + + await watcher.scanSlot(slot); + + expect(handler).toHaveBeenCalledWith([ + { + validator: signer.address, + amount: 0n, + offenseType: OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: 10n, + }, + ]); + }); + it('does not emit duplicate offenses on repeated scans', async () => { const signer = Secp256k1Signer.random(); const slot = SlotNumber(10); diff --git a/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.ts b/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.ts index 1e9249b2cdf4..0ac35019774b 100644 --- a/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.ts +++ b/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.ts @@ -8,7 +8,7 @@ import { RunningPromise } from '@aztec/foundation/running-promise'; import type { L2BlockSource } from '@aztec/stdlib/block'; import type { P2PClient, SlasherConfig } from '@aztec/stdlib/interfaces/server'; import type { BlockProposal, CheckpointProposalCore } from '@aztec/stdlib/p2p'; -import { OffenseType } from '@aztec/stdlib/slashing'; +import { OffenseType, getOffenseTypeName } from '@aztec/stdlib/slashing'; import EventEmitter from 'node:events'; @@ -89,10 +89,6 @@ export class BroadcastedInvalidCheckpointProposalWatcher * `currentSlot` at the archiver's last synced L2 slot. */ public async scan(): Promise { - if (this.config.slashBroadcastedInvalidCheckpointProposalPenalty <= 0n) { - return; - } - const currentSlot = (await this.l2BlockSource.getSyncedL2SlotNumber()) ?? this.epochCache.getSlotNow(); if (currentSlot <= SlotNumber(SCAN_SLOT_LAG)) { return; @@ -111,10 +107,6 @@ export class BroadcastedInvalidCheckpointProposalWatcher /** Scans a single slot. Public for tests. */ public async scanSlot(slot: SlotNumber): Promise { - if (this.config.slashBroadcastedInvalidCheckpointProposalPenalty <= 0n) { - return; - } - const proposals = await this.p2pClient.getProposalsForSlot(slot); const slashArgs = this.getSlashArgsForProposals(slot, proposals).filter(args => this.markAsNewOffense(args)); if (slashArgs.length === 0) { @@ -125,7 +117,8 @@ export class BroadcastedInvalidCheckpointProposalWatcher slot, offenses: slashArgs.map(args => ({ validator: args.validator.toString(), - offenseType: args.offenseType, + amount: args.amount, + offenseType: getOffenseTypeName(args.offenseType), epochOrSlot: args.epochOrSlot, })), }); diff --git a/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.test.ts b/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.test.ts index 8b6fe48f2fde..11c00a2d271b 100644 --- a/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.test.ts +++ b/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.test.ts @@ -86,9 +86,11 @@ describe('CheckpointEquivocationWatcher', () => { expect(handler).not.toHaveBeenCalled(); }); - it('does not emit when the penalty is zero', async () => { + it('emits a zero-amount offense when the penalty is zero', async () => { await watcher.stop(); + const proposer = EthAddress.random(); config = { ...config, slashDuplicateProposalPenalty: 0n }; + epochCache.getProposerAttesterAddressInSlot.mockResolvedValueOnce(proposer); watcher = new CheckpointEquivocationWatcher(l2BlockSource, epochCache, config); handler = jest.fn(); watcher.on(WANT_TO_SLASH_EVENT, handler); @@ -96,7 +98,14 @@ describe('CheckpointEquivocationWatcher', () => { await emitAndFlush(makeEvent()); - expect(handler).not.toHaveBeenCalled(); + expect(handler).toHaveBeenCalledWith([ + { + validator: proposer, + amount: 0n, + offenseType: OffenseType.DUPLICATE_PROPOSAL, + epochOrSlot: 10n, + }, + ]); }); it('emits separately for distinct slots', async () => { diff --git a/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.ts b/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.ts index 887986a8ba7e..0070de630f98 100644 --- a/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.ts +++ b/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.ts @@ -7,7 +7,7 @@ import { L2BlockSourceEvents, } from '@aztec/stdlib/block'; import type { SlasherConfig } from '@aztec/stdlib/interfaces/server'; -import { OffenseType } from '@aztec/stdlib/slashing'; +import { OffenseType, getOffenseTypeName } from '@aztec/stdlib/slashing'; import EventEmitter from 'node:events'; @@ -66,10 +66,6 @@ export class CheckpointEquivocationWatcher extends (EventEmitter as new () => Wa /** Public for tests. */ public async onEquivocationDetected(event: CheckpointEquivocationDetectedEvent): Promise { - if (this.config.slashDuplicateProposalPenalty <= 0n) { - return; - } - const proposer = await this.epochCache.getProposerAttesterAddressInSlot(event.slotNumber); if (!proposer) { this.log.warn(`Cannot attribute checkpoint equivocation: no proposer for slot ${event.slotNumber}`, { @@ -89,6 +85,8 @@ export class CheckpointEquivocationWatcher extends (EventEmitter as new () => Wa this.log.info(`Detected checkpoint equivocation offense`, { slotNumber: event.slotNumber, checkpointNumber: event.checkpointNumber, + amount: slashArgs.amount, + offenseType: getOffenseTypeName(slashArgs.offenseType), l1ArchiveRoot: event.l1ArchiveRoot.toString(), proposedArchiveRoot: event.proposedArchiveRoot.toString(), validator: proposer.toString(), diff --git a/yarn-project/slasher/src/watchers/data_withholding_watcher.test.ts b/yarn-project/slasher/src/watchers/data_withholding_watcher.test.ts index 310eaf3c3b62..5db77003fa20 100644 --- a/yarn-project/slasher/src/watchers/data_withholding_watcher.test.ts +++ b/yarn-project/slasher/src/watchers/data_withholding_watcher.test.ts @@ -291,16 +291,32 @@ describe('DataWithholdingWatcher', () => { expect(l2BlockSource.getCheckpoint).toHaveBeenCalledTimes(1); }); - it('respects penalty=0 as a disable switch', async () => { + it('emits zero-amount offenses when the penalty is zero', async () => { watcher.updateConfig({ slashDataWithholdingPenalty: 0n }); await startAtSlot(10); - setSyncedSlot(10 + TOLERANCE + 5); + setSyncedSlot(11 + TOLERANCE + 1); + + const slot = 11; + const published = makePublished(slot, 1); + const missing = published.checkpoint.blocks[0].body.txEffects[0].txHash; + const attester = EthAddress.random(); + l2BlockSource.getCheckpoint.mockResolvedValue(published); + mockMissing([missing]); + watcher.attestersBySlot.set(slot, [attester]); const captured = captureEmits(); await watcher.work(); - expect(l2BlockSource.getCheckpoint).not.toHaveBeenCalled(); - expect(captured).toHaveLength(0); + expect(captured).toEqual([ + [ + { + validator: attester, + amount: 0n, + offenseType: OffenseType.DATA_WITHHOLDING, + epochOrSlot: BigInt(slot), + }, + ], + ]); }); it('does not slash a checkpoint with no recoverable attesters even if txs are missing', async () => { diff --git a/yarn-project/slasher/src/watchers/data_withholding_watcher.ts b/yarn-project/slasher/src/watchers/data_withholding_watcher.ts index 091c2be823f6..65f578f9007c 100644 --- a/yarn-project/slasher/src/watchers/data_withholding_watcher.ts +++ b/yarn-project/slasher/src/watchers/data_withholding_watcher.ts @@ -9,7 +9,7 @@ import { getAttestationInfoFromPublishedCheckpoint } from '@aztec/stdlib/block'; import type { CheckpointReexecutionTracker, PublishedCheckpoint } from '@aztec/stdlib/checkpoint'; import type { ITxProvider, P2PApi, SlasherConfig } from '@aztec/stdlib/interfaces/server'; import { ConsensusPayload, type CoordinationSignatureContext } from '@aztec/stdlib/p2p'; -import { OffenseType } from '@aztec/stdlib/slashing'; +import { OffenseType, getOffenseTypeName } from '@aztec/stdlib/slashing'; import type { TxHash } from '@aztec/stdlib/tx'; import EventEmitter from 'node:events'; @@ -85,10 +85,6 @@ export class DataWithholdingWatcher extends (EventEmitter as new () => WatcherEm return; } - if (this.config.slashDataWithholdingPenalty === 0n) { - return; // disabled - } - // tolerance is the number of full slots that must elapse after the checkpoint's slot // before we declare its data missing. For checkpoint slot S, we therefore process S // only once we are in slot `S + tolerance + 1` or later. Drive this off the archiver's @@ -175,9 +171,11 @@ export class DataWithholdingWatcher extends (EventEmitter as new () => WatcherEm return; } - this.log.warn(`Detected data withholding at slot ${slot}. Slashing ${attesters.length} attesters.`, { + this.log.info(`Detected data withholding offense at slot ${slot}`, { slot, checkpointNumber, + amount: this.config.slashDataWithholdingPenalty, + offenseType: getOffenseTypeName(OffenseType.DATA_WITHHOLDING), missingTxs: missingTxs.map(h => h.toString()), records: collectionRecords, attesters: attesters.map(a => a.toString()), diff --git a/yarn-project/stdlib/src/block/l2_block_source.ts b/yarn-project/stdlib/src/block/l2_block_source.ts index 429e36da59d7..9d6d46323a0d 100644 --- a/yarn-project/stdlib/src/block/l2_block_source.ts +++ b/yarn-project/stdlib/src/block/l2_block_source.ts @@ -16,6 +16,7 @@ import type { TypedEventEmitter } from '@aztec/foundation/types'; import { z } from 'zod'; import type { CheckpointData, ProposedCheckpointData } from '../checkpoint/checkpoint_data.js'; +import type { CheckpointInfo } from '../checkpoint/checkpoint_info.js'; import type { PublishedCheckpoint } from '../checkpoint/published_checkpoint.js'; import type { L1RollupConstants } from '../epoch-helpers/index.js'; import { CheckpointHeader } from '../rollup/checkpoint_header.js'; @@ -294,6 +295,9 @@ export type ArchiverEmitter = TypedEventEmitter<{ [L2BlockSourceEvents.InvalidAttestationsCheckpointDetected]: (args: InvalidCheckpointDetectedEvent) => void; [L2BlockSourceEvents.L2BlocksCheckpointed]: (args: L2CheckpointEvent) => void; [L2BlockSourceEvents.CheckpointEquivocationDetected]: (args: CheckpointEquivocationDetectedEvent) => void; + [L2BlockSourceEvents.DescendentOfInvalidAttestationsCheckpointDetected]: ( + args: DescendentOfInvalidAttestationsCheckpointEvent, + ) => void; }>; export interface L2BlockSourceEventEmitter extends L2BlockSource { events: ArchiverEmitter; @@ -376,6 +380,7 @@ export enum L2BlockSourceEvents { L2BlocksCheckpointed = 'l2BlocksCheckpointed', InvalidAttestationsCheckpointDetected = 'invalidCheckpointDetected', CheckpointEquivocationDetected = 'checkpointEquivocationDetected', + DescendentOfInvalidAttestationsCheckpointDetected = 'descendentOfInvalidAttestationsCheckpointDetected', } export type L2BlockProvenEvent = { @@ -418,3 +423,19 @@ export type CheckpointEquivocationDetectedEvent = { l1ArchiveRoot: Fr; proposedArchiveRoot: Fr; }; + +/** + * Emitted when the archiver observes a checkpoint that builds on a previously-rejected + * ancestor (typically one with insufficient/invalid attestations). The descendant itself may + * have valid attestations, but it cannot be ingested because the chain it extends was + * skipped. The slasher uses this to slash the proposer of the descendant. + */ +export type DescendentOfInvalidAttestationsCheckpointEvent = { + type: 'descendentOfInvalidAttestationsCheckpointDetected'; + /** The descendant checkpoint being rejected. */ + checkpoint: CheckpointInfo; + /** Archive root of the rejected ancestor this descendant builds on. */ + ancestorArchiveRoot: Fr; + /** Checkpoint number of the rejected ancestor. */ + ancestorCheckpointNumber: CheckpointNumber; +}; diff --git a/yarn-project/stdlib/src/interfaces/configs.ts b/yarn-project/stdlib/src/interfaces/configs.ts index 83453f405fe4..27de324a3630 100644 --- a/yarn-project/stdlib/src/interfaces/configs.ts +++ b/yarn-project/stdlib/src/interfaces/configs.ts @@ -62,6 +62,11 @@ export interface SequencerConfig { skipCollectingAttestations?: boolean; /** Do not invalidate the previous block if invalid when we are the proposer (for testing only) */ skipInvalidateBlockAsProposer?: boolean; + /** + * Bypass the parent checkpoint validity check before submitting a pipelined checkpoint, allowing + * the proposer to publish even when the parent landed on L1 with invalid attestations (for testing only). + */ + skipWaitForValidParentCheckpointOnL1?: boolean; /** Broadcast invalid block proposals with corrupted state (for testing only) */ broadcastInvalidBlockProposal?: boolean; /** Broadcast an invalid block proposal only at this indexWithinCheckpoint (for testing only) */ @@ -126,6 +131,7 @@ export const SequencerConfigSchema = zodFor()( attestationPropagationTime: z.number().optional(), skipCollectingAttestations: z.boolean().optional(), skipInvalidateBlockAsProposer: z.boolean().optional(), + skipWaitForValidParentCheckpointOnL1: z.boolean().optional(), secondsBeforeInvalidatingBlockAsCommitteeMember: z.number(), secondsBeforeInvalidatingBlockAsNonCommitteeMember: z.number(), broadcastInvalidBlockProposal: z.boolean().optional(), diff --git a/yarn-project/stdlib/src/tx/tx_bench.test.ts b/yarn-project/stdlib/src/tx/tx_bench.test.ts index 9f5f02840376..d8d0133809a3 100644 --- a/yarn-project/stdlib/src/tx/tx_bench.test.ts +++ b/yarn-project/stdlib/src/tx/tx_bench.test.ts @@ -1,5 +1,6 @@ import { Timer } from '@aztec/foundation/timer'; +import { webcrypto } from 'node:crypto'; import fs from 'node:fs/promises'; import path from 'node:path'; import { type RecordableHistogram, createHistogram } from 'perf_hooks'; @@ -11,46 +12,29 @@ const RUNS = 100; describe('Tx', () => { let privateTxHistogram: RecordableHistogram; let publicTxHistogram: RecordableHistogram; + let privateSha256Histogram: RecordableHistogram; + let publicSha256Histogram: RecordableHistogram; beforeAll(() => { privateTxHistogram = createHistogram(); publicTxHistogram = createHistogram(); + privateSha256Histogram = createHistogram(); + publicSha256Histogram = createHistogram(); }); afterAll(async () => { if (process.env.BENCH_OUTPUT) { const data: any[] = []; - data.push({ - name: `Tx/private/getTxHash/avg`, - value: privateTxHistogram.mean, - unit: 'ms', - }); - data.push({ - name: `Tx/private/getTxHash/p50`, - value: privateTxHistogram.percentile(50), - unit: 'ms', - }); - data.push({ - name: `Tx/private/getTxHash/p95`, - value: privateTxHistogram.percentile(95), - unit: 'ms', - }); + const recordHistogram = (name: string, histogram: RecordableHistogram) => { + data.push({ name: `${name}/avg`, value: histogram.mean, unit: 'ms' }); + data.push({ name: `${name}/p50`, value: histogram.percentile(50), unit: 'ms' }); + data.push({ name: `${name}/p95`, value: histogram.percentile(95), unit: 'ms' }); + }; - data.push({ - name: `Tx/public/getTxHash/avg`, - value: publicTxHistogram.mean, - unit: 'ms', - }); - data.push({ - name: `Tx/public/getTxHash/p50`, - value: publicTxHistogram.percentile(50), - unit: 'ms', - }); - data.push({ - name: `Tx/public/getTxHash/p95`, - value: publicTxHistogram.percentile(95), - unit: 'ms', - }); + recordHistogram('Tx/private/getTxHash', privateTxHistogram); + recordHistogram('Tx/public/getTxHash', publicTxHistogram); + recordHistogram('Tx/private/sha256', privateSha256Histogram); + recordHistogram('Tx/public/sha256', publicSha256Histogram); await fs.mkdir(path.dirname(process.env.BENCH_OUTPUT), { recursive: true }); await fs.writeFile(process.env.BENCH_OUTPUT, JSON.stringify(data, null, 2)); @@ -59,12 +43,15 @@ describe('Tx', () => { await using f = await fs.open(process.env.BENCH_OUTPUT_MD!, 'w'); await f.write('|TYPE|MIN|AVG|P50|P90|MAX|\n'); await f.write('|----|---|---|---|---|---|\n'); - await f.write( - `|PRV|${privateTxHistogram.min}|${privateTxHistogram.mean}|${privateTxHistogram.percentile(50)}|${privateTxHistogram.percentile(90)}|${privateTxHistogram.max}|\n`, - ); - await f.write( - `|PUB|${publicTxHistogram.min}|${publicTxHistogram.mean}|${publicTxHistogram.percentile(50)}|${publicTxHistogram.percentile(90)}|${publicTxHistogram.max}|\n`, - ); + const writeRow = async (type: string, histogram: RecordableHistogram) => { + await f.write( + `|${type}|${histogram.min}|${histogram.mean}|${histogram.percentile(50)}|${histogram.percentile(90)}|${histogram.max}|\n`, + ); + }; + await writeRow('PRV', privateTxHistogram); + await writeRow('PUB', publicTxHistogram); + await writeRow('PRV-SHA256', privateSha256Histogram); + await writeRow('PUB-SHA256', publicSha256Histogram); } }); @@ -85,4 +72,22 @@ describe('Tx', () => { publicTxHistogram.record(Math.max(1, Math.ceil(timer.ms()))); } }); + + it('calculates SHA-256 of a private-only tx buffer', async () => { + const tx = await mockTxForRollup(42); + for (let i = 0; i < RUNS; i++) { + const timer = new Timer(); + await webcrypto.subtle.digest('SHA-256', tx.toBuffer()); + privateSha256Histogram.record(Math.max(1, Math.ceil(timer.ms()))); + } + }); + + it('calculates SHA-256 of a tx buffer with enqueued public calls', async () => { + const tx = await mockTx(42); + for (let i = 0; i < RUNS; i++) { + const timer = new Timer(); + await webcrypto.subtle.digest('SHA-256', tx.toBuffer()); + publicSha256Histogram.record(Math.max(1, Math.ceil(timer.ms()))); + } + }); }); diff --git a/yarn-project/txe/src/state_machine/index.ts b/yarn-project/txe/src/state_machine/index.ts index 746f79f0b9dd..9aeb59c21046 100644 --- a/yarn-project/txe/src/state_machine/index.ts +++ b/yarn-project/txe/src/state_machine/index.ts @@ -52,8 +52,7 @@ export class TXEStateMachine { undefined, undefined, undefined, - undefined, - undefined, + async () => {}, VERSION, CHAIN_ID, new TXEGlobalVariablesBuilder(), diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index d582a73c3575..a4af734ccc5b 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -417,7 +417,15 @@ describe('ValidatorClient', () => { Array.isArray(args) && args[0]?.offenseType === OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL, ); - + const getAttestedToInvalidCheckpointProposalSlashEvents = ( + emitSpy: jest.SpiedFunction, + ) => + emitSpy.mock.calls.filter( + ([event, args]) => + event === WANT_TO_SLASH_EVENT && + Array.isArray(args) && + args[0]?.offenseType === OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + ); beforeEach(async () => { const emptyInHash = computeInHashFromL1ToL2Messages([]); const blockHeader = makeBlockHeader(1, { blockNumber: BlockNumber(100), slotNumber: SlotNumber(100) }); @@ -898,53 +906,37 @@ describe('ValidatorClient', () => { expect(blockSource.getBlockData).toHaveBeenCalledWith({ number: blockNumber }); }); - it('should not emit WANT_TO_SLASH_EVENT if slashing is disabled', async () => { + it('emits zero-amount invalid block proposal offenses when the penalty is zero', async () => { validatorClient.updateConfig({ slashBroadcastedInvalidBlockPenalty: 0n }); const emitSpy = jest.spyOn(validatorClient, 'emit'); blockBuildResult.block.archive.root = Fr.random(); const isValid = await validatorClient.validateBlockProposal(proposal, sender); + const proposer = proposal.getSender(); expect(isValid).toBe(false); - expect(emitSpy).not.toHaveBeenCalled(); + expect(proposer).toBeDefined(); + expect(emitSpy).toHaveBeenCalledWith(WANT_TO_SLASH_EVENT, [ + { + validator: proposer!, + amount: 0n, + offenseType: OffenseType.BROADCASTED_INVALID_BLOCK_PROPOSAL, + epochOrSlot: expect.any(BigInt), + }, + ]); }); - it('slashes checkpoint attestations received after an invalid proposal slot is marked only once', async () => { - await validatorClient.registerHandlers(); - const attestationCallback = p2pClient.registerCheckpointAttestationCallback.mock.calls[0][0]; - const emitSpy = jest.spyOn(validatorClient, 'emit'); + it('marks invalid block proposal slots for delayed attestation slashing', async () => { blockBuildResult.block.archive.root = Fr.random(); - await validatorClient.validateBlockProposal(proposal, sender); - - const attesterSigner = Secp256k1Signer.random(); - const attestation = makeCheckpointAttestation({ - header: makeCheckpointHeader(1, { slotNumber: proposal.slotNumber }), - attesterSigner, - }); - attestationCallback(attestation); - attestationCallback(attestation); + const isValid = await validatorClient.validateBlockProposal(proposal, sender); - const badAttestationEvents = emitSpy.mock.calls.filter( - ([event, args]) => - event === WANT_TO_SLASH_EVENT && - Array.isArray(args) && - args[0]?.offenseType === OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, - ); - expect(badAttestationEvents).toHaveLength(1); - expect(badAttestationEvents[0][1]).toEqual([ - { - validator: attesterSigner.address, - amount: config.slashAttestInvalidCheckpointProposalPenalty, - offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, - epochOrSlot: BigInt(proposal.slotNumber), - }, - ]); + expect(isValid).toBe(false); + expect(validatorClient.hasInvalidProposals(proposal.slotNumber)).toBe(true); }); - it('clears and suppresses bad attestation offenses when proposal equivocation is detected', async () => { + it('records proposal equivocation and emits clear event', async () => { await validatorClient.registerHandlers(); - const attestationCallback = p2pClient.registerCheckpointAttestationCallback.mock.calls[0][0]; const duplicateProposalCallback = p2pClient.registerDuplicateProposalCallback.mock.calls[0][0]; const emitSpy = jest.spyOn(validatorClient, 'emit'); blockBuildResult.block.archive.root = Fr.random(); @@ -956,26 +948,26 @@ describe('ValidatorClient', () => { type: 'block', }); - const attestation = makeCheckpointAttestation({ - header: makeCheckpointHeader(1, { slotNumber: proposal.slotNumber }), - attesterSigner: Secp256k1Signer.random(), - }); - attestationCallback(attestation); - + expect(validatorClient.hasProposalEquivocation(proposal.slotNumber)).toBe(true); expect(emitSpy).toHaveBeenCalledWith(WANT_TO_CLEAR_SLASH_EVENT, [ { offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, epochOrSlot: BigInt(proposal.slotNumber), }, ]); - expect( - emitSpy.mock.calls.some( - ([event, args]) => - event === WANT_TO_SLASH_EVENT && - Array.isArray(args) && - args[0]?.offenseType === OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, - ), - ).toBe(false); + }); + + it('marks invalid proposal slots when the bad attestation penalty is zero', async () => { + validatorClient.updateConfig({ + slashBroadcastedInvalidBlockPenalty: 0n, + slashAttestInvalidCheckpointProposalPenalty: 0n, + }); + blockBuildResult.block.archive.root = Fr.random(); + + const isValid = await validatorClient.validateBlockProposal(proposal, sender); + + expect(isValid).toBe(false); + expect(validatorClient.hasInvalidProposals(proposal.slotNumber)).toBe(true); }); it('reexecutes for bad attestation slashing when invalid block proposer slashing is disabled', async () => { @@ -989,7 +981,7 @@ describe('ValidatorClient', () => { expect(checkpointsBuilder.openCheckpoint).toHaveBeenCalled(); }); - it('does not emit bad attestation offenses when the bad attestation penalty is disabled', async () => { + it('emits zero-amount bad attestation offenses when the bad attestation penalty is zero', async () => { await validatorClient.registerHandlers(); const attestationCallback = p2pClient.registerCheckpointAttestationCallback.mock.calls[0][0]; validatorClient.updateConfig({ @@ -997,9 +989,10 @@ describe('ValidatorClient', () => { slashAttestInvalidCheckpointProposalPenalty: 0n, }); const emitSpy = jest.spyOn(validatorClient, 'emit'); + const attesterSigner = Secp256k1Signer.random(); const attestation = makeCheckpointAttestation({ header: makeCheckpointHeader(1, { slotNumber: proposal.slotNumber }), - attesterSigner: Secp256k1Signer.random(), + attesterSigner, }); blockBuildResult.block.archive.root = Fr.random(); @@ -1007,7 +1000,19 @@ describe('ValidatorClient', () => { attestationCallback(attestation); expect(isValid).toBe(false); - expect(emitSpy).not.toHaveBeenCalled(); + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)).toEqual([ + [ + WANT_TO_SLASH_EVENT, + [ + { + validator: attesterSigner.address, + amount: 0n, + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(proposal.slotNumber), + }, + ], + ], + ]); }); it('emits WANT_TO_SLASH_EVENT for checkpoint_header_mismatch checkpoint proposals', async () => { @@ -1086,7 +1091,7 @@ describe('ValidatorClient', () => { ]); }); - it('does not emit checkpoint proposal slash event when the penalty is disabled', async () => { + it('emits zero-amount checkpoint proposal offenses when the penalty is zero', async () => { validatorClient.updateConfig({ slashBroadcastedInvalidCheckpointProposalPenalty: 0n }); const checkpointHandler = registerAllNodesCheckpointHandler(); const { checkpointProposal } = await makeCheckpointProposalWithHeaderMismatch(); @@ -1096,7 +1101,19 @@ describe('ValidatorClient', () => { const attestations = await validatorClient.attestToCheckpointProposal(checkpointProposal, sender); expect(attestations).toBeUndefined(); - expect(getBroadcastedInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(0); + expect(getBroadcastedInvalidCheckpointProposalSlashEvents(emitSpy)).toEqual([ + [ + WANT_TO_SLASH_EVENT, + [ + { + validator: checkpointProposal.getSender()!, + amount: 0n, + offenseType: OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(checkpointProposal.slotNumber), + }, + ], + ], + ]); }); it.each(['last_block_not_found', 'checkpoint_already_published'])( @@ -1127,6 +1144,75 @@ describe('ValidatorClient', () => { expect(getBroadcastedInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(1); }); + it('marks invalid checkpoint proposal slots for delayed attestation slashing', async () => { + const checkpointHandler = registerAllNodesCheckpointHandler(); + const { checkpointProposal } = await makeCheckpointProposalWithHeaderMismatch(); + + await checkpointHandler(checkpointProposal, sender); + + expect(validatorClient.hasInvalidProposals(checkpointProposal.slotNumber)).toBe(true); + }); + + it('marks invalid checkpoint proposal slots when proposer slashing is disabled', async () => { + validatorClient.updateConfig({ slashBroadcastedInvalidCheckpointProposalPenalty: 0n }); + const checkpointHandler = registerAllNodesCheckpointHandler(); + const { checkpointProposal } = await makeCheckpointProposalWithHeaderMismatch(); + const emitSpy = jest.spyOn(validatorClient, 'emit'); + + await checkpointHandler(checkpointProposal, sender); + + expect(getBroadcastedInvalidCheckpointProposalSlashEvents(emitSpy)).toEqual([ + [ + WANT_TO_SLASH_EVENT, + [ + { + validator: checkpointProposal.getSender()!, + amount: 0n, + offenseType: OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(checkpointProposal.slotNumber), + }, + ], + ], + ]); + expect(validatorClient.hasInvalidProposals(checkpointProposal.slotNumber)).toBe(true); + }); + + it('records checkpoint proposal equivocation and emits clear event', async () => { + await validatorClient.registerHandlers(); + const checkpointHandler = registerAllNodesCheckpointHandler(); + const duplicateProposalCallback = p2pClient.registerDuplicateProposalCallback.mock.calls[0][0]; + const { checkpointProposal } = await makeCheckpointProposalWithHeaderMismatch(); + const emitSpy = jest.spyOn(validatorClient, 'emit'); + + await checkpointHandler(checkpointProposal, sender); + duplicateProposalCallback({ + slot: checkpointProposal.slotNumber, + proposer: checkpointProposal.getSender()!, + type: 'checkpoint', + }); + + expect(validatorClient.hasProposalEquivocation(checkpointProposal.slotNumber)).toBe(true); + expect(emitSpy).toHaveBeenCalledWith(WANT_TO_CLEAR_SLASH_EVENT, [ + { + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(checkpointProposal.slotNumber), + }, + ]); + }); + + it('does not mark invalid proposal slots after a non-slashable invalid checkpoint proposal', async () => { + const checkpointHandler = registerAllNodesCheckpointHandler(); + const checkpointProposal = await makeCheckpointProposalForSlot(); + jest.spyOn(validatorClient.getProposalHandler(), 'handleCheckpointProposal').mockResolvedValue({ + isValid: false, + reason: 'last_block_not_found', + }); + + await checkpointHandler(checkpointProposal, sender); + + expect(validatorClient.hasInvalidProposals(checkpointProposal.slotNumber)).toBe(false); + }); + it('emits slash event even if validator is not in the current committee', async () => { epochCache.filterInCommittee.mockResolvedValue([]); const checkpointHandler = registerAllNodesCheckpointHandler(); diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index c7721e1ea66f..7b155518933a 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -19,6 +19,7 @@ import { WANT_TO_SLASH_EVENT, type Watcher, type WatcherEmitter, + getOffenseTypeName, } from '@aztec/slasher'; import type { AztecAddress } from '@aztec/stdlib/aztec-address'; import type { CommitteeAttestationsAndSigners, L2BlockSink, L2BlockSource } from '@aztec/stdlib/block'; @@ -128,10 +129,10 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) private lastAttestedEpochByAttester: Map = new Map(); private proposersOfInvalidBlocks = FifoSet.withLimit(MAX_PROPOSERS_OF_INVALID_BLOCKS); - private slotsWithInvalidBlockProposals = FifoSet.withLimit(MAX_TRACKED_INVALID_PROPOSAL_SLOTS); + private slotsWithInvalidProposals = FifoSet.withLimit(MAX_TRACKED_INVALID_PROPOSAL_SLOTS); private invalidCheckpointProposalOffenseKeys = FifoSet.withLimit(MAX_TRACKED_INVALID_CHECKPOINT_PROPOSALS); - private slotsWithProposalEquivocation = FifoSet.withLimit(MAX_TRACKED_INVALID_PROPOSAL_SLOTS); private badAttestationOffenseKeys = FifoSet.withLimit(MAX_TRACKED_BAD_ATTESTATIONS); + private slotsWithProposalEquivocation = FifoSet.withLimit(MAX_TRACKED_INVALID_PROPOSAL_SLOTS); /** Tracks the last checkpoint proposal we attested to, to prevent equivocation. */ private lastAttestedProposal?: CheckpointProposalCore; @@ -171,7 +172,6 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) // Refresh epoch cache every second to trigger alert if participation in committee changes this.epochCacheUpdateLoop = new RunningPromise(this.handleEpochCommitteeUpdate.bind(this), this.log, 1000); - const myAddresses = this.getValidatorAddresses(); this.log.verbose(`Initialized validator with addresses: ${myAddresses.map(a => a.toString()).join(', ')}`); } @@ -354,6 +354,14 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) return this.config; } + public hasProposalEquivocation(slotNumber: SlotNumber): boolean { + return this.slotsWithProposalEquivocation.has(slotNumber); + } + + public hasInvalidProposals(slotNumber: SlotNumber): boolean { + return this.slotsWithInvalidProposals.has(slotNumber); + } + public updateConfig(config: Partial) { this.config = { ...this.config, ...config }; this.proposalHandler.updateConfig(config); @@ -475,27 +483,8 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) fishermanMode: this.config.fishermanMode || false, }); - // Reexecute txs if we are part of the committee, or if slashing is enabled, or if we are configured to always reexecute. - // In fisherman mode, we always reexecute to validate proposals. - const { - slashBroadcastedInvalidBlockPenalty, - slashAttestInvalidCheckpointProposalPenalty, - alwaysReexecuteBlockProposals, - fishermanMode, - } = this.config; - const shouldReexecute = - fishermanMode || - slashBroadcastedInvalidBlockPenalty > 0n || - slashAttestInvalidCheckpointProposalPenalty > 0n || - partOfCommittee || - alwaysReexecuteBlockProposals || - this.blobClient.canUpload(); - - const validationResult = await this.proposalHandler.handleBlockProposal( - proposal, - proposalSender, - !!shouldReexecute && !escapeHatchOpen, - ); + // Reexecute outside the escape hatch so slashing observers can detect invalid proposals even when penalties are 0. + const validationResult = await this.proposalHandler.handleBlockProposal(proposal, proposalSender, !escapeHatchOpen); if (!validationResult.isValid) { const reason = validationResult.reason || 'unknown'; @@ -523,13 +512,13 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) validationResult.reason && SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT.includes(validationResult.reason) ) { - if (slashBroadcastedInvalidBlockPenalty > 0n) { - this.log.warn(`Slashing proposer for invalid block proposal`, proposalInfo); - this.slashInvalidBlock(proposal); - } - if (slashAttestInvalidCheckpointProposalPenalty > 0n) { - this.markInvalidProposalSlot(proposal.slotNumber); - } + this.log.info(`Detected invalid block proposal offense`, { + ...proposalInfo, + amount: this.config.slashBroadcastedInvalidBlockPenalty, + offenseType: getOffenseTypeName(OffenseType.BROADCASTED_INVALID_BLOCK_PROPOSAL), + }); + this.slashInvalidBlock(proposal); + this.markInvalidProposalSlot(proposal.slotNumber); } return false; } @@ -768,19 +757,19 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) return; } + this.markInvalidProposalSlot(proposal.slotNumber); + if (this.slashInvalidCheckpointProposal(proposal)) { - this.log.warn(`Slashing proposer for invalid checkpoint proposal`, { + this.log.info(`Detected invalid checkpoint proposal offense`, { ...proposalInfo, reason: result.reason, + amount: this.config.slashBroadcastedInvalidCheckpointProposalPenalty, + offenseType: getOffenseTypeName(OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL), }); } } private slashInvalidCheckpointProposal(proposal: CheckpointProposalCore): boolean { - if (this.config.slashBroadcastedInvalidCheckpointProposalPenalty <= 0n) { - return false; - } - const proposer = proposal.getSender(); if (!proposer) { this.log.warn(`Cannot slash checkpoint proposal with invalid signature`, { @@ -791,7 +780,7 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) } const offenseType = OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL; - const offenseKey = `${proposer.toString()}:${offenseType}:${this.getSlotKey(proposal.slotNumber)}`; + const offenseKey = `${proposer.toString()}:${offenseType}:${proposal.slotNumber}`; if (!this.invalidCheckpointProposalOffenseKeys.addIfAbsent(offenseKey)) { return false; } @@ -808,14 +797,12 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) } private markInvalidProposalSlot(slotNumber: SlotNumber): void { - const slotKey = this.getSlotKey(slotNumber); - this.slotsWithInvalidBlockProposals.add(slotKey); + this.slotsWithInvalidProposals.add(slotNumber); } private handleCheckpointAttestation(attestation: CheckpointAttestation): void { const slotNumber = attestation.slotNumber; - const slotKey = this.getSlotKey(slotNumber); - if (!this.slotsWithInvalidBlockProposals.has(slotKey) || this.slotsWithProposalEquivocation.has(slotKey)) { + if (!this.slotsWithInvalidProposals.has(slotNumber) || this.slotsWithProposalEquivocation.has(slotNumber)) { return; } @@ -832,18 +819,16 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) } private slashAttestedToInvalidCheckpointProposal(slotNumber: SlotNumber, attester: EthAddress): void { - if (this.config.slashAttestInvalidCheckpointProposalPenalty <= 0n) { - return; - } - - const offenseKey = `${this.getSlotKey(slotNumber)}:${attester.toString()}`; + const offenseKey = `${attester.toString()}:${OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL}:${slotNumber}`; if (!this.badAttestationOffenseKeys.addIfAbsent(offenseKey)) { return; } - this.log.warn(`Slashing attester for attesting to invalid checkpoint proposal`, { + this.log.info(`Detected attestation to invalid checkpoint proposal offense`, { attester: attester.toString(), slotNumber, + amount: this.config.slashAttestInvalidCheckpointProposalPenalty, + offenseType: getOffenseTypeName(OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL), }); this.emit(WANT_TO_SLASH_EVENT, [ @@ -862,16 +847,16 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) */ private handleDuplicateProposal(info: DuplicateProposalInfo): void { const { slot, proposer, type } = info; - const slotKey = this.getSlotKey(slot); - this.slotsWithProposalEquivocation.add(slotKey); + this.slotsWithProposalEquivocation.add(slot); - this.log.warn(`Triggering slash event for duplicate ${type} proposal from ${proposer.toString()} at slot ${slot}`, { + this.log.info(`Detected duplicate ${type} proposal offense from ${proposer.toString()} at slot ${slot}`, { proposer: proposer.toString(), slot, type, + amount: this.config.slashDuplicateProposalPenalty, + offenseType: getOffenseTypeName(OffenseType.DUPLICATE_PROPOSAL), }); - // Emit slash event this.emit(WANT_TO_SLASH_EVENT, [ { validator: proposer, @@ -896,9 +881,11 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) private handleDuplicateAttestation(info: DuplicateAttestationInfo): void { const { slot, attester } = info; - this.log.warn(`Triggering slash event for duplicate attestation from ${attester.toString()} at slot ${slot}`, { + this.log.info(`Detected duplicate attestation offense from ${attester.toString()} at slot ${slot}`, { attester: attester.toString(), slot, + amount: this.config.slashDuplicateAttestationPenalty, + offenseType: getOffenseTypeName(OffenseType.DUPLICATE_ATTESTATION), }); this.emit(WANT_TO_SLASH_EVENT, [ @@ -911,10 +898,6 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) ]); } - private getSlotKey(slot: SlotNumber): string { - return slot.toString(); - } - async createBlockProposal( blockHeader: BlockHeader, checkpointNumber: CheckpointNumber,