[#138] Finding 8: Tests — regression coverage (#131 + indexer crash) batch #146

Closed
opened 2026-05-04 08:52:43 +00:00 by PlasticDigits · 12 comments
PlasticDigits commented 2026-05-04 08:52:43 +00:00 (Migrated from gitlab.com)

Parent: Split from #138 — Pre-deploy review pass 2 (Finding 8).

Severity: Low (aggregate coverage gap)

Problem

Several security-relevant behaviors called out in GitLab #131 and the #138 review lack negative / regression tests pinning current behavior. Without tests, future “fixes” may silently change economics or safety.

Proposed test list (from #138):

ID Test Notes
a test_burn_drains_arbitrary_holder_via_secondary_minter Non–RabbitTreasury MINTER_ROLE burns another wallet’s DOUB — #131 finding 1
b test_warbow_steal_fresh_wallet_drains_high_bp_victim_for_one_cl8y Document / label existing bypass when victim abp=0
c test_warbow_revenge_overwritten_by_second_stealer_shields_first A→V→B→V then revenge succeeds while A unrevengable — #131 finding 5
d test_buy_fee_routing_freeze_after_buy_blocks_warbow_defense Owner flips setBuyFeeRoutingEnabled mid-sale — #131 finding 2
e test_excess_doub_above_total_allocated_is_unrecoverable Extra DOUB sent to vesting post-startVesting — #131 finding 7
f Indexer integration: crash mid-block Align with Finding 2 transactional ingestion — partial persist then restart → no orphan rows

References

Kind Path / link
Contracts tests contracts/test/ — add beside existing WarBow / Doubloon / vesting suites
Indexer tests indexer/tests/integration_stage2.rs
Testing strategy docs/testing/strategy.md
Invariants map docs/testing/invariants-and-business-logic.md
Parent issues #131, #138
Skill .cursor/skills/yieldomega-guardrails/SKILL.md

Implement the six tests (a–f). Use descriptive revert/assert messages referencing intended economic or safety invariant so future readers know whether a failure is “bug” vs “documented behavior.”

For (f), coordinate with the transactional-ingestion issue: test may be skipped until the fix lands, or implemented first as expected-fail / feature-flagged — prefer implementing after Finding 2 fix to avoid flaky CI.

Acceptance criteria

  • Tests (a–e) in Forge CI with clear names and comments.
  • Test (f) implemented or explicitly tracked with dependency on indexer transaction scope.
  • forge test green in CI; indexer test passes when (f) included.

Verification checklist

  • cd contracts && forge test --match-test <each> locally.
  • CI job logs show new tests executing.
  • For adversarial tests, code review confirms they match current contract semantics (no “wishful” assertions).
**Parent:** Split from [#138 — Pre-deploy review pass 2](https://gitlab.com/PlasticDigits/yieldomega/-/issues/138) (Finding 8). **Severity:** Low (aggregate coverage gap) ## Problem Several **security-relevant behaviors** called out in GitLab [#131](https://gitlab.com/PlasticDigits/yieldomega/-/issues/131) and the **#138** review lack **negative / regression tests** pinning current behavior. Without tests, future “fixes” may silently change economics or safety. Proposed test list (from #138): | ID | Test | Notes | |----|------|--------| | a | `test_burn_drains_arbitrary_holder_via_secondary_minter` | Non–`RabbitTreasury` `MINTER_ROLE` burns another wallet’s DOUB — [#131 finding 1](https://gitlab.com/PlasticDigits/yieldomega/-/issues/131) | | b | `test_warbow_steal_fresh_wallet_drains_high_bp_victim_for_one_cl8y` | Document / label existing bypass when victim `abp=0` | | c | `test_warbow_revenge_overwritten_by_second_stealer_shields_first` | A→V→B→V then revenge succeeds while A unrevengable — [#131 finding 5](https://gitlab.com/PlasticDigits/yieldomega/-/issues/131) | | d | `test_buy_fee_routing_freeze_after_buy_blocks_warbow_defense` | Owner flips `setBuyFeeRoutingEnabled` mid-sale — [#131 finding 2](https://gitlab.com/PlasticDigits/yieldomega/-/issues/131) | | e | `test_excess_doub_above_total_allocated_is_unrecoverable` | Extra DOUB sent to vesting post-`startVesting` — [#131 finding 7](https://gitlab.com/PlasticDigits/yieldomega/-/issues/131) | | f | Indexer integration: crash mid-block | Align with **Finding 2** transactional ingestion — partial persist then restart → **no orphan rows** | ## References | Kind | Path / link | |------|-------------| | Contracts tests | [`contracts/test/`](https://gitlab.com/PlasticDigits/yieldomega/-/tree/main/contracts/test) — add beside existing WarBow / Doubloon / vesting suites | | Indexer tests | [`indexer/tests/integration_stage2.rs`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/indexer/tests/integration_stage2.rs) | | Testing strategy | [`docs/testing/strategy.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/docs/testing/strategy.md) | | Invariants map | [`docs/testing/invariants-and-business-logic.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/docs/testing/invariants-and-business-logic.md) | | Parent issues | [#131](https://gitlab.com/PlasticDigits/yieldomega/-/issues/131), [#138](https://gitlab.com/PlasticDigits/yieldomega/-/issues/138) | | Skill | [`.cursor/skills/yieldomega-guardrails/SKILL.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/.cursor/skills/yieldomega-guardrails/SKILL.md) | ## Recommended fix Implement the six tests (a–f). Use descriptive revert/assert messages referencing **intended economic or safety invariant** so future readers know whether a failure is “bug” vs “documented behavior.” For (f), coordinate with the transactional-ingestion issue: test may be **skipped** until the fix lands, or implemented first as **expected-fail** / feature-flagged — prefer implementing after Finding 2 fix to avoid flaky CI. ## Acceptance criteria - [ ] Tests (a–e) in Forge CI with clear names and comments. - [ ] Test (f) implemented or explicitly tracked with dependency on indexer transaction scope. - [ ] `forge test` green in CI; indexer test passes when (f) included. ## Verification checklist - [ ] `cd contracts && forge test --match-test <each>` locally. - [ ] CI job logs show new tests executing. - [ ] For adversarial tests, code review confirms they match current contract semantics (no “wishful” assertions).
PlasticDigits commented 2026-05-04 08:53:13 +00:00 (Migrated from gitlab.com)

mentioned in issue #138

mentioned in issue #138
PlasticDigits commented 2026-05-04 14:16:28 +00:00 (Migrated from gitlab.com)

mentioned in commit d45b0069c1

mentioned in commit d45b0069c128daf8b34b1c7a353a0c9f1535b599
PlasticDigits commented 2026-05-04 14:16:35 +00:00 (Migrated from gitlab.com)

Landed on main (d45b006) — @brouie please verify

Summary

  • Indexer: Block ingestion now runs one Postgres transaction per block: all persist_decoded_log_conn calls for that block plus indexed_blocks upsert and chain_pointer save commit together or roll back together; failures bubble up (no silent log-only errors on that path). Helpers: persist_decoded_log_conn, save_chain_pointer_conn, upsert_indexed_block_conn.
  • Contracts (Forge): Regression tests for #131/#138 review items — Doubloon secondary-minter burn, TimeCurve fee-routing freeze + WarBow paths, DoubPresaleVesting excess DOUB after startVesting.
  • Docs / agents: INV-INDEXER-146 in docs/testing/invariants-and-business-logic.md (anchor #indexer-block-ingest-transaction-gitlab-146), docs/indexer/design.md, docs/testing/strategy.md, skills/README.md, .cursor/skills/yieldomega-guardrails/SKILL.md item 49.

Checklist for verification

  1. cd contracts && forge test --match-test 'test_burn_drains|test_buy_fee_routing_freeze|test_warbow_steal|test_warbow_revenge|test_excess_doub' — all pass.
  2. cd indexer && cargo test — unit tests pass; with YIELDOMEGA_PG_TEST_URL set, postgres_gitlab146_block_transaction_all_or_nothing_for_shared_tx_hash validates rollback vs commit on shared tx_hash.
  3. Code review indexer/src/ingestion.rs — single pool.begin(), persist loop uses _conn APIs, commit/rollback matches described behavior.
  4. Skim docs/testing/invariants-and-business-logic.md § INV-INDEXER-146 and cross-links from skills/README.md / guardrails.

Issue left open for your sign-off.

## Landed on `main` (d45b006) — @brouie please verify **Summary** - **Indexer:** Block ingestion now runs **one Postgres transaction per block**: all `persist_decoded_log_conn` calls for that block plus `indexed_blocks` upsert and `chain_pointer` save commit together or **roll back** together; failures **bubble up** (no silent log-only errors on that path). Helpers: `persist_decoded_log_conn`, `save_chain_pointer_conn`, `upsert_indexed_block_conn`. - **Contracts (Forge):** Regression tests for #131/#138 review items — Doubloon secondary-minter burn, TimeCurve fee-routing freeze + WarBow paths, DoubPresaleVesting excess DOUB after `startVesting`. - **Docs / agents:** `INV-INDEXER-146` in `docs/testing/invariants-and-business-logic.md` (anchor `#indexer-block-ingest-transaction-gitlab-146`), `docs/indexer/design.md`, `docs/testing/strategy.md`, `skills/README.md`, `.cursor/skills/yieldomega-guardrails/SKILL.md` item **49**. **Checklist for verification** 1. `cd contracts && forge test --match-test 'test_burn_drains|test_buy_fee_routing_freeze|test_warbow_steal|test_warbow_revenge|test_excess_doub'` — all pass. 2. `cd indexer && cargo test` — unit tests pass; with `YIELDOMEGA_PG_TEST_URL` set, `postgres_gitlab146_block_transaction_all_or_nothing_for_shared_tx_hash` validates rollback vs commit on shared `tx_hash`. 3. Code review `indexer/src/ingestion.rs` — single `pool.begin()`, persist loop uses `_conn` APIs, `commit`/`rollback` matches described behavior. 4. Skim `docs/testing/invariants-and-business-logic.md` § **INV-INDEXER-146** and cross-links from `skills/README.md` / guardrails. Issue left **open** for your sign-off.
PlasticDigits commented 2026-05-04 14:34:03 +00:00 (Migrated from gitlab.com)

mentioned in commit 0f7b9a4e34

mentioned in commit 0f7b9a4e3428a61b87530717ec32e780f3c27b28
PlasticDigits commented 2026-05-04 15:08:09 +00:00 (Migrated from gitlab.com)

mentioned in commit 15fc07617b

mentioned in commit 15fc07617b61dd6bf4529b2b4117039f19e9de6b
PlasticDigits commented 2026-05-04 15:12:19 +00:00 (Migrated from gitlab.com)

mentioned in issue #139

mentioned in issue #139
PlasticDigits commented 2026-05-04 15:21:31 +00:00 (Migrated from gitlab.com)

mentioned in commit a421ba1303

mentioned in commit a421ba130364ea4dcf638b9a6e2d31d1e7cdbadf
PlasticDigits commented 2026-05-04 15:24:13 +00:00 (Migrated from gitlab.com)

mentioned in issue #140

mentioned in issue #140
Brouie commented 2026-05-05 02:18:03 +00:00 (Migrated from gitlab.com)

@PlasticDigits — verification PASS on d45b006. walked your checklist:

  1. forge test --match-test 'test_burn_drains|test_buy_fee_routing_freeze|test_warbow_steal|test_warbow_revenge|test_excess_doub' green: 14/14 PASS across 4 suites. all six regression items shipped:
  • a. test_burn_drains_arbitrary_holder_via_secondary_minter ✓ (Doubloon)
  • b. test_warbow_steal_fresh_wallet_drains_high_bp_victim_for_one_cl8y ✓
  • c. test_warbow_revenge_overwritten_by_second_stealer_shields_first ✓
  • d. test_buy_fee_routing_freeze_after_buy_blocks_warbow_defense ✓
  • e. test_excess_doub_above_total_allocated_is_unrecoverable ✓ (verified in #137, 32/32 PASS in DoubPresaleVestingTest)
  • f. indexer crash mid-block — covered by postgres_gitlab146_block_transaction_all_or_nothing_for_shared_tx_hash at integration_stage2.rs:1264-1341 (source-verified, Postgres-gated path skipped this round per #140 note).
  1. cargo test green from #140 run — 31/31 PASS, 0 failures. block-level transaction behavior verified at source.
  2. indexer/src/ingestion.rs L150-180 reviewed under #140: single pool.begin(), per-event persist_decoded_log_conn(&mut *tx, ...), upsert_indexed_block_conn, save_chain_pointer_conn, all on same tx, commit/rollback matches described behavior.
  3. #indexer-block-ingest-transaction-gitlab-146 anchor + INV-INDEXER-146 invariant in invariants-and-business-logic.md read cleanly. docs/indexer/design.md ingestion bullet matches code paths. cross-links in skills/README.md + guardrails item 49.
    the adversarial coverage matrix is now regression-pinned in CI. future 'fixes' can't silently change the economics or safety properties without one of these tests going red.
    ready for close on your side.
@PlasticDigits — verification PASS on `d45b006`. walked your checklist: 1. `forge test --match-test 'test_burn_drains|test_buy_fee_routing_freeze|test_warbow_steal|test_warbow_revenge|test_excess_doub'` green: 14/14 PASS across 4 suites. all six regression items shipped: - a. `test_burn_drains_arbitrary_holder_via_secondary_minter` ✓ (Doubloon) - b. `test_warbow_steal_fresh_wallet_drains_high_bp_victim_for_one_cl8y` ✓ - c. `test_warbow_revenge_overwritten_by_second_stealer_shields_first` ✓ - d. `test_buy_fee_routing_freeze_after_buy_blocks_warbow_defense` ✓ - e. `test_excess_doub_above_total_allocated_is_unrecoverable` ✓ (verified in #137, 32/32 PASS in DoubPresaleVestingTest) - f. indexer crash mid-block — covered by `postgres_gitlab146_block_transaction_all_or_nothing_for_shared_tx_hash` at `integration_stage2.rs:1264-1341` (source-verified, Postgres-gated path skipped this round per #140 note). 2. `cargo test` green from #140 run — 31/31 PASS, 0 failures. block-level transaction behavior verified at source. 3. `indexer/src/ingestion.rs` L150-180 reviewed under #140: single `pool.begin()`, per-event `persist_decoded_log_conn(&mut *tx, ...)`, `upsert_indexed_block_conn`, `save_chain_pointer_conn`, all on same tx, commit/rollback matches described behavior. 4. `#indexer-block-ingest-transaction-gitlab-146` anchor + `INV-INDEXER-146` invariant in `invariants-and-business-logic.md` read cleanly. `docs/indexer/design.md` ingestion bullet matches code paths. cross-links in `skills/README.md` + guardrails item 49. the adversarial coverage matrix is now regression-pinned in CI. future 'fixes' can't silently change the economics or safety properties without one of these tests going red. ready for close on your side.
PlasticDigits (Migrated from gitlab.com) closed this issue 2026-05-05 02:37:15 +00:00
Brouie commented 2026-05-05 02:49:23 +00:00 (Migrated from gitlab.com)

mentioned in issue #149

mentioned in issue #149
Brouie commented 2026-05-05 04:12:29 +00:00 (Migrated from gitlab.com)

mentioned in issue #148

mentioned in issue #148
PlasticDigits commented 2026-05-13 15:13:33 +00:00 (Migrated from gitlab.com)

mentioned in commit d74e6e75b1

mentioned in commit d74e6e75b12afcc07a8550cb40ccd2bdaf067e79
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
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
code/yieldomega#146
No description provided.