fix: fix: protocol activity statistics stay zero — ponder watches wrong contract addresses (#4) #6

Closed
dev-bot wants to merge 0 commits from fix/issue-4 into master
Collaborator

Fixes #4

Changes

Fixes #4 ## Changes
dev-bot added 1 commit 2026-04-05 17:21:55 +00:00
1. Add LM_ADDRESS and POOL_ADDRESS to ponder .env.local (bootstrap.sh)
2. Discover pool address from Uniswap factory during bootstrap (bootstrap-common.sh)
3. Make ring buffer block threshold configurable via MINIMUM_BLOCKS_FOR_RINGBUFFER env var,
   set to 0 for local dev so early events populate the ring buffer

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator

AI Review

Summary

This PR fixes protocol activity stats staying zero by making ponder watch the correct contract addresses. It adds pool address discovery via the Uniswap factory to bootstrap, passes LM_ADDRESS, POOL_ADDRESS, and MINIMUM_BLOCKS_FOR_RINGBUFFER to the ponder env, and makes the ring-buffer minimum blocks threshold configurable via env so local dev doesn't have to wait 100 blocks.

The TypeScript change and the MINIMUM_BLOCKS_FOR_RINGBUFFER=0 override in bootstrap.sh are both correct. The pool address discovery logic in bootstrap-common.sh has one bug that will abort the bootstrap script in certain failure conditions.

Issues

  • bug scripts/bootstrap-common.sh:113: POOL_ADDRESS is never initialised before the discovery block. When factory is empty or 0x (factory call fails, returns empty, or returns a zero address), the if branch that sets POOL_ADDRESS is skipped, leaving the variable unbound. The immediately following check [[ -z "$POOL_ADDRESS" ... ]] then triggers a POOL_ADDRESS: unbound variable error and aborts the script under set -euo pipefail.

    Fix: initialise the variable to empty before the discovery block:

    POOL_ADDRESS=""
    # Discover Uniswap pool address from factory
    detect_swap_router
    

Non-issues

  • POOL_ADDRESS=$POOL_ADDRESS in the heredoc in containers/bootstrap.sh:108 looks like a self-reference but is correct: the unquoted heredoc terminator causes shell expansion, so the actual address value is written into the file.
  • Hardcoding fee tier 10000 in getPool call is consistent with the rest of bootstrap (seed swap uses the same tier).
  • parseInt(process.env.MINIMUM_BLOCKS_FOR_RINGBUFFER || '100') is fine; env var is always set to a valid integer by bootstrap.

Verdict

REQUEST_CHANGES — POOL_ADDRESS unbound variable will abort bootstrap when factory call returns empty


Reviewed at e20b451 | AGENTS.md

## AI Review <!-- reviewed: e20b4517fda4a7c1254598f8f84fe137a0dc97a4 --> ### Summary This PR fixes protocol activity stats staying zero by making ponder watch the correct contract addresses. It adds pool address discovery via the Uniswap factory to bootstrap, passes `LM_ADDRESS`, `POOL_ADDRESS`, and `MINIMUM_BLOCKS_FOR_RINGBUFFER` to the ponder env, and makes the ring-buffer minimum blocks threshold configurable via env so local dev doesn't have to wait 100 blocks. The TypeScript change and the `MINIMUM_BLOCKS_FOR_RINGBUFFER=0` override in `bootstrap.sh` are both correct. The pool address discovery logic in `bootstrap-common.sh` has one bug that will abort the bootstrap script in certain failure conditions. ### Issues - **bug** `scripts/bootstrap-common.sh:113`: `POOL_ADDRESS` is never initialised before the discovery block. When `factory` is empty or `0x` (factory call fails, returns empty, or returns a zero address), the `if` branch that sets `POOL_ADDRESS` is skipped, leaving the variable unbound. The immediately following check `[[ -z "$POOL_ADDRESS" ... ]]` then triggers a **`POOL_ADDRESS: unbound variable`** error and aborts the script under `set -euo pipefail`. Fix: initialise the variable to empty before the discovery block: ```bash POOL_ADDRESS="" # Discover Uniswap pool address from factory detect_swap_router ``` ### Non-issues - `POOL_ADDRESS=$POOL_ADDRESS` in the heredoc in `containers/bootstrap.sh:108` looks like a self-reference but is correct: the unquoted heredoc terminator causes shell expansion, so the actual address value is written into the file. - Hardcoding fee tier `10000` in `getPool` call is consistent with the rest of bootstrap (seed swap uses the same tier). - `parseInt(process.env.MINIMUM_BLOCKS_FOR_RINGBUFFER || '100')` is fine; env var is always set to a valid integer by bootstrap. ### Verdict **REQUEST_CHANGES** — POOL_ADDRESS unbound variable will abort bootstrap when factory call returns empty --- *Reviewed at `e20b451` | [AGENTS.md](AGENTS.md)*
review-bot requested changes 2026-04-05 17:22:56 +00:00
Dismissed
review-bot left a comment
Collaborator

AI Review: REQUEST_CHANGES — POOL_ADDRESS unbound variable will abort bootstrap when factory call returns empty

AI Review: **REQUEST_CHANGES** — POOL_ADDRESS unbound variable will abort bootstrap when factory call returns empty
Collaborator

AI Review

Summary

This PR correctly identifies and fixes the root cause of zero protocol activity statistics: ponder was not receiving the LM_ADDRESS or POOL_ADDRESS it needed to watch the right contracts, and the 100-block ring buffer guard was preventing stats from accumulating on fresh local chains. The three-part fix is well-scoped and the approach is sound.

Findings

  • bug scripts/bootstrap-common.sh:106-116: Unbound variable crash under set -u. POOL_ADDRESS is never initialized before the conditional block. If cast call for the factory fails (network hiccup, swap router not deployed), the if block on line 110 is skipped and POOL_ADDRESS remains unset. The subsequent check [[ -z "$POOL_ADDRESS" || ... ]] then triggers bash's nounset error and aborts the entire bootstrap — the opposite of the graceful fallback intended. Fix: add POOL_ADDRESS="" immediately before detect_swap_router on line 107.

Rest of the diff

  • containers/bootstrap.sh:107-109 — Writing POOL_ADDRESS=$POOL_ADDRESS in the heredoc is correct; the outer variable is expanded before the heredoc is written.
  • services/ponder/src/helpers/stats.ts:10parseInt without a radix is fine here; input is always "0" (local) or the string default "100".
  • MINIMUM_BLOCKS_FOR_RINGBUFFER=0 hardcoded in write_ponder_env is intentional and correct — fresh local chains have far fewer than 100 blocks at bootstrap time.

Verdict

REQUEST_CHANGES — POOL_ADDRESS unbound variable in bootstrap-common.sh will crash bootstrap under set -u when factory lookup fails


Reviewed at e20b451 | AGENTS.md

## AI Review <!-- reviewed: e20b4517fda4a7c1254598f8f84fe137a0dc97a4 --> ### Summary This PR correctly identifies and fixes the root cause of zero protocol activity statistics: ponder was not receiving the `LM_ADDRESS` or `POOL_ADDRESS` it needed to watch the right contracts, and the 100-block ring buffer guard was preventing stats from accumulating on fresh local chains. The three-part fix is well-scoped and the approach is sound. ### Findings - **bug** `scripts/bootstrap-common.sh:106-116`: **Unbound variable crash under `set -u`.** `POOL_ADDRESS` is never initialized before the conditional block. If `cast call` for the factory fails (network hiccup, swap router not deployed), the `if` block on line 110 is skipped and `POOL_ADDRESS` remains unset. The subsequent check `[[ -z "$POOL_ADDRESS" || ... ]]` then triggers bash's `nounset` error and aborts the entire bootstrap — the opposite of the graceful fallback intended. Fix: add `POOL_ADDRESS=""` immediately before `detect_swap_router` on line 107. ### Rest of the diff - `containers/bootstrap.sh:107-109` — Writing `POOL_ADDRESS=$POOL_ADDRESS` in the heredoc is correct; the outer variable is expanded before the heredoc is written. - `services/ponder/src/helpers/stats.ts:10` — `parseInt` without a radix is fine here; input is always `"0"` (local) or the string default `"100"`. - `MINIMUM_BLOCKS_FOR_RINGBUFFER=0` hardcoded in `write_ponder_env` is intentional and correct — fresh local chains have far fewer than 100 blocks at bootstrap time. ### Verdict **REQUEST_CHANGES** — POOL_ADDRESS unbound variable in bootstrap-common.sh will crash bootstrap under set -u when factory lookup fails --- *Reviewed at `e20b451` | [AGENTS.md](AGENTS.md)*
review-bot requested changes 2026-04-05 17:23:58 +00:00
Dismissed
review-bot left a comment
Collaborator

AI Review: REQUEST_CHANGES — POOL_ADDRESS unbound variable in bootstrap-common.sh will crash bootstrap under set -u when factory lookup fails

AI Review: **REQUEST_CHANGES** — POOL_ADDRESS unbound variable in bootstrap-common.sh will crash bootstrap under set -u when factory lookup fails
dev-qwen closed this pull request 2026-04-05 17:24:06 +00:00
johba reopened this pull request 2026-04-05 17:26:15 +00:00
Collaborator

AI Re-review (round 3)

Previous Findings

  • Unbound variable crash scripts/bootstrap-common.sh:106 — FIXED: POOL_ADDRESS="" added immediately before detect_swap_router, exactly as requested. The graceful fallback now works correctly under set -u.

New Issues

None.

Verdict

APPROVE — Previous finding fixed: POOL_ADDRESS initialized before conditional block


Reviewed at caabcde | Previous: e20b451 | AGENTS.md

## AI Re-review (round 3) <!-- reviewed: caabcde20c3501cf4ca9bef4e50284961d705631 --> ### Previous Findings - **Unbound variable crash** `scripts/bootstrap-common.sh:106` — FIXED: `POOL_ADDRESS=""` added immediately before `detect_swap_router`, exactly as requested. The graceful fallback now works correctly under `set -u`. ### New Issues None. ### Verdict **APPROVE** — Previous finding fixed: POOL_ADDRESS initialized before conditional block --- *Reviewed at `caabcde` | Previous: `e20b451` | [AGENTS.md](AGENTS.md)*
Collaborator

AI Re-review (round 3)

Previous Findings

  • Unbound POOL_ADDRESS under set -u (scripts/bootstrap-common.sh:107) → FIXED: POOL_ADDRESS="" added immediately before detect_swap_router, matching the suggested fix exactly.

New Issues

None.

Verdict

APPROVE — Previous finding fixed: POOL_ADDRESS initialised before conditional block


Reviewed at caabcde | Previous: e20b451 | AGENTS.md

## AI Re-review (round 3) <!-- reviewed: caabcde20c3501cf4ca9bef4e50284961d705631 --> ### Previous Findings - **Unbound `POOL_ADDRESS` under `set -u`** (`scripts/bootstrap-common.sh:107`) → **FIXED**: `POOL_ADDRESS=""` added immediately before `detect_swap_router`, matching the suggested fix exactly. ### New Issues None. ### Verdict **APPROVE** — Previous finding fixed: POOL_ADDRESS initialised before conditional block --- *Reviewed at `caabcde` | Previous: `e20b451` | [AGENTS.md](AGENTS.md)*
review-bot approved these changes 2026-04-05 17:27:15 +00:00
Dismissed
review-bot left a comment
Collaborator

AI Re-review (round 3): APPROVE — Previous finding fixed: POOL_ADDRESS initialized before conditional block

AI Re-review (round 3): **APPROVE** — Previous finding fixed: POOL_ADDRESS initialized before conditional block
review-bot approved these changes 2026-04-05 17:27:15 +00:00
review-bot left a comment
Collaborator

AI Re-review (round 3): APPROVE — Previous finding fixed: POOL_ADDRESS initialised before conditional block

AI Re-review (round 3): **APPROVE** — Previous finding fixed: POOL_ADDRESS initialised before conditional block
dev-qwen closed this pull request 2026-04-05 17:29:07 +00:00

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: johba/harb#6
No description provided.