Post-#138 hygiene: WarBow constant rename + persist_decoded_log autocommit naming

Brouie commented

2026-05-05 02:21:45 +00:00

(Migrated from gitlab.com)

two low-priority hygiene items spotted while doing the #131/#138 children verification pass. neither is a correctness bug — both are naming/convention follow-ups that would help future reviewers. filed as one issue since they're both low and small.

Finding 1 — `WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY` constant name is stale post-#134

contracts/src/TimeCurve.sol:76 declares:

uint8 public constant WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY = 3;

post-#134 the constant is now used for BOTH the per-victim cap (L630) AND the per-attacker cap (L631):

bool victimLimitBefore = victimStealsToday >= WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY;
bool attackerLimitBefore = attackerStealsToday >= WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY;

behaviorally correct — both caps share threshold of 3. but the name says "PER_VICTIM" and reads as misleading on the attacker branch. anyone modifying the attacker cap independently in the future has to remember to either rename or duplicate the constant.

mitigation: rename to WARBOW_MAX_STEALS_PER_DAY (drops the misleading "PER_VICTIM"), or split into two constants WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY + WARBOW_MAX_STEALS_PER_ATTACKER_PER_DAY if the cap thresholds might diverge later.

note for context: the related event field bypassedVictimDailyLimit is similarly stale post-#134, but is already disclosed in NatSpec at L213 as "historical naming" with explicit cross-link to #134, so that's not in scope of this ticket.

Finding 2 — `persist_decoded_log` (public pool wrapper) is autocommit, not transactional

indexer/src/persist.rs:1356-1358 exposes:

pub async fn persist_decoded_log(pool: &PgPool, d: &DecodedLog) -> Result<()> {
    let mut conn = pool.acquire().await?;
    persist_decoded_log_conn(&mut *conn, d).await
}

acquires a connection from pool, calls _conn variant — but without wrapping in a transaction. each call is autocommit per-event.

production ingestion (indexer/src/ingestion.rs L150-180) uses _conn inside pool.begin() so production is fine. risk is forward-looking: a future caller (test, ad-hoc tool, batch script) might use persist_decoded_log for batch work and lose the atomic per-block invariant from #140 / INV-INDEXER-146.

mitigation: rename the public wrapper to persist_decoded_log_autocommit to make the semantic explicit, OR add a doc comment on the function declaring it single-event only and pointing at _conn for batch use.

cc @PlasticDigits

two low-priority hygiene items spotted while doing the #131/#138 children verification pass. neither is a correctness bug — both are naming/convention follow-ups that would help future reviewers. filed as one issue since they're both low and small. --- ## Finding 1 — `WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY` constant name is stale post-#134 `contracts/src/TimeCurve.sol:76` declares: ``` uint8 public constant WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY = 3; ``` post-#134 the constant is now used for BOTH the per-victim cap (L630) AND the per-attacker cap (L631): ``` bool victimLimitBefore = victimStealsToday >= WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY; bool attackerLimitBefore = attackerStealsToday >= WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY; ``` behaviorally correct — both caps share threshold of 3. but the name says "PER_VICTIM" and reads as misleading on the attacker branch. anyone modifying the attacker cap independently in the future has to remember to either rename or duplicate the constant. **mitigation**: rename to `WARBOW_MAX_STEALS_PER_DAY` (drops the misleading "PER_VICTIM"), or split into two constants `WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY` + `WARBOW_MAX_STEALS_PER_ATTACKER_PER_DAY` if the cap thresholds might diverge later. note for context: the related event field `bypassedVictimDailyLimit` is similarly stale post-#134, but is already disclosed in NatSpec at L213 as "historical naming" with explicit cross-link to #134, so that's not in scope of this ticket. --- ## Finding 2 — `persist_decoded_log` (public pool wrapper) is autocommit, not transactional `indexer/src/persist.rs:1356-1358` exposes: ```rust pub async fn persist_decoded_log(pool: &PgPool, d: &DecodedLog) -> Result<()> { let mut conn = pool.acquire().await?; persist_decoded_log_conn(&mut *conn, d).await } ``` acquires a connection from pool, calls `_conn` variant — but without wrapping in a transaction. each call is autocommit per-event. production ingestion (`indexer/src/ingestion.rs` L150-180) uses `_conn` inside `pool.begin()` so production is fine. risk is forward-looking: a future caller (test, ad-hoc tool, batch script) might use `persist_decoded_log` for batch work and lose the atomic per-block invariant from #140 / INV-INDEXER-146. **mitigation**: rename the public wrapper to `persist_decoded_log_autocommit` to make the semantic explicit, OR add a doc comment on the function declaring it single-event only and pointing at `_conn` for batch use. cc @PlasticDigits

Brouie commented

2026-05-05 02:49:23 +00:00

(Migrated from gitlab.com)

mentioned in issue #149

PlasticDigits commented

2026-05-05 03:03:06 +00:00

(Migrated from gitlab.com)

mentioned in commit 3f204678ba

mentioned in commit 3f204678ba970ae96d1de822a2f69b762badd534

PlasticDigits commented

2026-05-05 03:03:13 +00:00

(Migrated from gitlab.com)

GitLab #148 — implemented on main (`245b70b`)

Summary

TimeCurve: Renamed WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY → WARBOW_MAX_STEALS_PER_DAY (shared UTC-day cap semantics unchanged from #134/#138; naming aligned with behavior).
Indexer: Renamed pool helper persist_decoded_log → persist_decoded_log_autocommit with explicit autocommit semantics; updated ingestion/reorg call sites and integration tests.
Frontend: ABI / multicall / protocol copy updated for the new getter.
Docs & agent surfaces: docs/testing/invariants-and-business-logic.md (INV-WARBOW-148-CONST, INV-INDEXER-148-AUTOCOMMIT, anchor #post-138-hygiene-naming-gitlab-148), docs/indexer/design.md, .cursor/skills/yieldomega-guardrails/SKILL.md, skills/README.md, skills/play-timecurve-warbow/SKILL.md.

Verification (please confirm)

contracts: FOUNDRY_PROFILE=ci forge test --match-contract TimeCurveTest passes; on-chain reads use WARBOW_MAX_STEALS_PER_DAY().
indexer: cargo clippy --all-targets -- -D warnings and cargo test pass; no remaining references to the old helper name in ingestion paths.
frontend: npm ci && npm run typecheck && npm test pass; arena/protocol UI still shows the steal cap label correctly.
Docs: invariants page cross-links match code; WarBow play skill mentions the renamed getter.

/cc @brouie — please verify when you have a moment. Leaving this issue open until you sign off.

Agent handoff: submodule init may be needed in fresh worktrees (git submodule update --init contracts/lib/prb-math) before forge test.

## GitLab #148 — implemented on main (245b70b) **Summary** - **TimeCurve:** Renamed `WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY` → `WARBOW_MAX_STEALS_PER_DAY` (shared UTC-day cap semantics unchanged from #134/#138; naming aligned with behavior). - **Indexer:** Renamed pool helper `persist_decoded_log` → `persist_decoded_log_autocommit` with explicit autocommit semantics; updated ingestion/reorg call sites and integration tests. - **Frontend:** ABI / multicall / protocol copy updated for the new getter. - **Docs & agent surfaces:** `docs/testing/invariants-and-business-logic.md` (`INV-WARBOW-148-CONST`, `INV-INDEXER-148-AUTOCOMMIT`, anchor `#post-138-hygiene-naming-gitlab-148`), `docs/indexer/design.md`, `.cursor/skills/yieldomega-guardrails/SKILL.md`, `skills/README.md`, `skills/play-timecurve-warbow/SKILL.md`. **Verification (please confirm)** - [ ] `contracts`: `FOUNDRY_PROFILE=ci forge test --match-contract TimeCurveTest` passes; on-chain reads use `WARBOW_MAX_STEALS_PER_DAY()`. - [ ] `indexer`: `cargo clippy --all-targets -- -D warnings` and `cargo test` pass; no remaining references to the old helper name in ingestion paths. - [ ] `frontend`: `npm ci && npm run typecheck && npm test` pass; arena/protocol UI still shows the steal cap label correctly. - [ ] Docs: invariants page cross-links match code; WarBow play skill mentions the renamed getter. /cc @brouie — please verify when you have a moment. Leaving this issue **open** until you sign off. --- _Agent handoff: submodule init may be needed in fresh worktrees (`git submodule update --init contracts/lib/prb-math`) before `forge test`._

PlasticDigits commented

2026-05-05 03:03:57 +00:00

(Migrated from gitlab.com)

mentioned in commit e12ddab4bc

mentioned in commit e12ddab4bc771206d54d5d8284d0c5e827e43898

Brouie commented

2026-05-05 04:12:29 +00:00

(Migrated from gitlab.com)

@PlasticDigits — verification PASS on e12ddab (245b70b + the e12ddab doc follow-up). walked your checklist:

forge test --match-contract TimeCurveTest: 116/116 PASS, 0 failures, 0 skipped — includes test_warbow_steal_attacker_daily_limit_fourth_needs_bypass exercising the renamed WARBOW_MAX_STEALS_PER_DAY on both victim AND attacker cap branches at TimeCurve.sol L630-631.
cargo test indexer: 31/31 PASS (29 unit + 2 integration). integration_stage2 postgres tests pass clean — postgres_gitlab146_block_transaction_all_or_nothing_for_shared_tx_hash + postgres_stage2_persist_all_events_and_rollback_after. no remaining persist_decoded_log (sans suffix) call sites in ingestion/reorg paths.
cargo clippy --all-targets -- -D warnings: clean.
frontend tsc -b: clean.
frontend vitest run: 281/281 PASS across 46 files including walletBuySessionGuard (7 tests), all timeCurve / warbow / referral suites.
docs anchor #post-138-hygiene-naming-gitlab-148 + INV-WARBOW-148-CONST + INV-INDEXER-148-AUTOCOMMIT cross-links resolve. arena/protocol UI label paths in TimeCurveProtocolPage.tsx and useTimeCurveArenaModel.tsx updated to the new getter.

constant rename from WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY → WARBOW_MAX_STEALS_PER_DAY matches behavior across both #134 cap branches. autocommit suffix on persist_decoded_log_autocommit makes the per-event semantic explicit at the type/name level — production ingestion still uses _conn inside pool.begin() per #140/#146 invariant.

ready for close on your side.

@PlasticDigits — verification PASS on `e12ddab` (`245b70b` + the `e12ddab` doc follow-up). walked your checklist: - `forge test --match-contract TimeCurveTest`: 116/116 PASS, 0 failures, 0 skipped — includes `test_warbow_steal_attacker_daily_limit_fourth_needs_bypass` exercising the renamed `WARBOW_MAX_STEALS_PER_DAY` on both victim AND attacker cap branches at TimeCurve.sol L630-631. - `cargo test` indexer: 31/31 PASS (29 unit + 2 integration). `integration_stage2` postgres tests pass clean — `postgres_gitlab146_block_transaction_all_or_nothing_for_shared_tx_hash` + `postgres_stage2_persist_all_events_and_rollback_after`. no remaining `persist_decoded_log` (sans suffix) call sites in ingestion/reorg paths. - `cargo clippy --all-targets -- -D warnings`: clean. - frontend `tsc -b`: clean. - frontend `vitest run`: 281/281 PASS across 46 files including `walletBuySessionGuard` (7 tests), all timeCurve / warbow / referral suites. - docs anchor `#post-138-hygiene-naming-gitlab-148` + `INV-WARBOW-148-CONST` + `INV-INDEXER-148-AUTOCOMMIT` cross-links resolve. arena/protocol UI label paths in `TimeCurveProtocolPage.tsx` and `useTimeCurveArenaModel.tsx` updated to the new getter. constant rename from `WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY` → `WARBOW_MAX_STEALS_PER_DAY` matches behavior across both #134 cap branches. autocommit suffix on `persist_decoded_log_autocommit` makes the per-event semantic explicit at the type/name level — production ingestion still uses `_conn` inside `pool.begin()` per #140/#146 invariant. ready for close on your side.

Brouie commented

2026-05-05 04:35:55 +00:00

(Migrated from gitlab.com)

mentioned in issue #129

Brouie commented

2026-05-05 04:39:46 +00:00

(Migrated from gitlab.com)

mentioned in issue #113

Brouie commented

2026-05-05 04:59:21 +00:00

(Migrated from gitlab.com)

mentioned in issue #151

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-05-05 05:39:06 +00:00

PlasticDigits commented

2026-05-13 15:13:34 +00:00

(Migrated from gitlab.com)

mentioned in commit 00114ed338

mentioned in commit 00114ed338c9379e22c9c4c91a898c69b16ba31c

PlasticDigits commented

2026-05-13 15:13:34 +00:00

(Migrated from gitlab.com)

mentioned in commit 1fbe68a17a

mentioned in commit 1fbe68a17a50c2672088ee269b7a22d76c312cac

Rows
Columns

Post-#138 hygiene: WarBow constant rename + persist_decoded_log autocommit naming #148

Finding 1 — WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY constant name is stale post-#134

Finding 2 — persist_decoded_log (public pool wrapper) is autocommit, not transactional

GitLab #148 — implemented on main (245b70b)

Finding 1 — `WARBOW_MAX_STEALS_PER_VICTIM_PER_DAY` constant name is stale post-#134

Finding 2 — `persist_decoded_log` (public pool wrapper) is autocommit, not transactional

GitLab #148 — implemented on main (`245b70b`)