Add missing Forge negative tests and TimeArena DRY/gas fixes #316

Closed
opened 2026-06-13 07:34:25 +00:00 by PlasticDigits · 12 comments
PlasticDigits commented 2026-06-13 07:34:25 +00:00 (Migrated from gitlab.com)

Parent

Gap analysis follow-up from #309.

Current codebase

  • TimeArena.t.sol covers happy-path routing, CRED, WarBow basics; weak on revert matrix.
  • Missing: pause blocks buy/WarBow/flag; steal-band reverts; revenge expiry; double finalize; rebasing token edge cases.
  • docs/operations/pause-and-final-signoff.md claims claimWarBowFlag works while paused — approved: docs wrong; code pause behavior is correct (fix docs in docs bundle, not here).
  • Duplicated charm bounds; up to 12 safeTransfers per buy; bubble-sort on 3 podium slots.

Why needed

Approved: Add missing negative tests; fix DRY/gas hotspots in TimeArena.

Constraints / guardrails

  • Negative tests must use exact revert strings already in code.
  • Gas optimizations must not change economics (#300, CRED pool).
  • NonStandardERC20 tests referenced in gap report — add if fee-on-transfer coverage gaps exist.

Relevant files

  • contracts/test/TimeArena.t.sol
  • contracts/test/TimeArenaBuyRouter.t.sol
  • contracts/src/arena/TimeArena.sol
  • contracts/test/NonStandardERC20.t.sol (create if missing)
  1. Add pause matrix: setPaused(true) blocks buy, buyWithCred, WarBow actions, claimWarBowFlag.
  2. WarBow reverts: steal band, daily cap, revenge expiry, wrong flag holder, silence not elapsed.
  3. finalizeWarbowPodium / autoroll guards (coordinate with autoroll issue).
  4. Consolidate duplicated charm bound checks; reduce transfer count if safe; keep bubble-sort (negligible) or document why kept.

Acceptance criteria

  • Pause negative tests for all user-facing mutating functions.
  • WarBow revert matrix covered.
  • DRY refactor for charm bounds without behavior change.
  • Gas snapshot or comment if transfer count reduced.

Test plan

  • cd contracts && forge test.
  • cd contracts && forge test --match-contract TimeArena -vv for new reverts.

Attack / abuse test plan

  • Each negative test documents an attack vector prevented (reentrancy, double finalize, out-of-band steal).

Verification criteria

  • New tests fail on pre-fix code (where applicable).
  • Full forge test green except known doub.csv fork skips.
## Parent Gap analysis follow-up from [#309](https://gitlab.com/PlasticDigits/yieldomega/-/issues/309). ## Current codebase - `TimeArena.t.sol` covers happy-path routing, CRED, WarBow basics; weak on revert matrix. - Missing: pause blocks buy/WarBow/flag; steal-band reverts; revenge expiry; double finalize; rebasing token edge cases. - `docs/operations/pause-and-final-signoff.md` claims `claimWarBowFlag` works while paused — **approved: docs wrong; code pause behavior is correct** (fix docs in docs bundle, not here). - Duplicated charm bounds; up to 12 `safeTransfer`s per buy; bubble-sort on 3 podium slots. ## Why needed Approved: Add missing negative tests; fix DRY/gas hotspots in `TimeArena`. ## Constraints / guardrails - Negative tests must use exact revert strings already in code. - Gas optimizations must not change economics (#300, CRED pool). - `NonStandardERC20` tests referenced in gap report — add if fee-on-transfer coverage gaps exist. ## Relevant files - `contracts/test/TimeArena.t.sol` - `contracts/test/TimeArenaBuyRouter.t.sol` - `contracts/src/arena/TimeArena.sol` - `contracts/test/NonStandardERC20.t.sol` (create if missing) ## Recommended direction 1. Add pause matrix: `setPaused(true)` blocks `buy`, `buyWithCred`, WarBow actions, `claimWarBowFlag`. 2. WarBow reverts: steal band, daily cap, revenge expiry, wrong flag holder, silence not elapsed. 3. `finalizeWarbowPodium` / autoroll guards (coordinate with autoroll issue). 4. Consolidate duplicated charm bound checks; reduce transfer count if safe; keep bubble-sort (negligible) or document why kept. ## Acceptance criteria - [ ] Pause negative tests for all user-facing mutating functions. - [ ] WarBow revert matrix covered. - [ ] DRY refactor for charm bounds without behavior change. - [ ] Gas snapshot or comment if transfer count reduced. ## Test plan - `cd contracts && forge test`. - `cd contracts && forge test --match-contract TimeArena -vv` for new reverts. ## Attack / abuse test plan - Each negative test documents an attack vector prevented (reentrancy, double finalize, out-of-band steal). ## Verification criteria - New tests fail on pre-fix code (where applicable). - Full `forge test` green except known `doub.csv` fork skips.
PlasticDigits commented 2026-06-13 07:34:43 +00:00 (Migrated from gitlab.com)

mentioned in issue #309

mentioned in issue #309
PlasticDigits commented 2026-06-13 08:08:32 +00:00 (Migrated from gitlab.com)

mentioned in commit 18a24e5101

mentioned in commit 18a24e5101e012f9e6c5f388439624d54bf850d8
PlasticDigits commented 2026-06-13 08:08:37 +00:00 (Migrated from gitlab.com)

mentioned in merge request !461

mentioned in merge request !461
PlasticDigits commented 2026-06-13 09:29:22 +00:00 (Migrated from gitlab.com)

mentioned in commit adb400d006

mentioned in commit adb400d0060f43151801aa57e7dac92533a6b4f3
PlasticDigits commented 2026-06-13 09:39:24 +00:00 (Migrated from gitlab.com)

mentioned in merge request !464

mentioned in merge request !464
PlasticDigits commented 2026-06-13 12:58:16 +00:00 (Migrated from gitlab.com)

mentioned in commit a80b465b34

mentioned in commit a80b465b34f0ba82622d1853026b6f5dcacf3be3
PlasticDigits commented 2026-06-13 12:58:19 +00:00 (Migrated from gitlab.com)

mentioned in merge request !463

mentioned in merge request !463
PlasticDigits commented 2026-06-13 12:58:34 +00:00 (Migrated from gitlab.com)

mentioned in commit 28fbbba370

mentioned in commit 28fbbba37045376e6af1aac1cc6700db0c483a28
PlasticDigits commented 2026-06-13 12:58:38 +00:00 (Migrated from gitlab.com)

mentioned in merge request !465

mentioned in merge request !465
PlasticDigits commented 2026-06-13 13:25:02 +00:00 (Migrated from gitlab.com)

Verification complete — GitLab #316

Verified merged implementation on main at 1a7b8ed5 (MR !461).

Acceptance criteria

Criterion Result How verified
Pause negative tests for all user-facing mutating functions PASS cd contracts && forge test --match-test test_pause_blocks -vv — 6/6 (buy, buyWithCred, warbowSteal, warbowActivateGuard, warbowRevenge, claimWarBowFlag) all revert TimeArena: paused
WarBow revert matrix covered PASS cd contracts && forge test --match-test "test_warbow_.*_reverts|test_finalize_warbow_podium_reverts" -vv — 10/10 (steal band, daily cap, revenge expiry/no-pending, flag holder/silence, double finalize, bad victim/epoch)
DRY refactor for charm bounds without behavior change PASS ArenaCharmBounds.sol shared by TimeArena, ArenaXp, ArenaCharmPriceTwap; existing test_buy_reverts_charm_* + fuzz charm tests green in TimeArenaTest (95 tests)
Gas snapshot or comment if transfer count reduced PASS NatSpec on _routeDoubPrizeSplit documents one safeTransfer when all pool slots map to address(vaults); test_buy_routes_doub_split still green
NonStandardERC20 / fee-on-transfer coverage PASS cd contracts && forge test --match-contract NonStandardERC20Test -vv — 4/4 (buy, WarBow guard/steal, top-up parity)
Full forge test PASS (known skips) cd contracts && forge test — 183 passed, 4 failed only in DoubAirdropMegaethForkTest (missing gitignored airdrop/doub.csv); DevStackIntegrationTest 5/5 PASS
Targeted TimeArena suite PASS cd contracts && forge test --match-contract "TimeArenaTest|NonStandardERC20Test" — 99/99
Attack-vector documentation in negative tests PASS Reviewed TimeArena.t.sol §316 comments (pause incident response, steal-band griefing, daily-cap bypass, revenge window, double finalize, etc.)
Invariants / docs cross-links PASS docs/testing/invariants-and-business-logic.md §316 (INV-TIME-ARENA-PAUSE-MUTATING, INV-TIME-ARENA-WARBOW-REVERT-MATRIX, INV-TIME-ARENA-CHARM-BOUNDS-DRY, INV-TIME-ARENA-BUY-ROUTING-GAS, INV-ERC20-123-NONSTANDARD)

Out of scope / not required for this issue

  • Rabby / Anvil E2E / indexer — contracts-only scope; no frontend/indexer changes in #316.
  • docs/operations/pause-and-final-signoff.md pause copy — explicitly deferred to docs bundle per issue body.

No repo changes from this verification pass; closing issue.

## Verification complete — GitLab #316 Verified merged implementation on `main` at `1a7b8ed5` (MR !461). ### Acceptance criteria | Criterion | Result | How verified | |-----------|--------|--------------| | Pause negative tests for all user-facing mutating functions | **PASS** | `cd contracts && forge test --match-test test_pause_blocks -vv` — 6/6 (`buy`, `buyWithCred`, `warbowSteal`, `warbowActivateGuard`, `warbowRevenge`, `claimWarBowFlag`) all revert `TimeArena: paused` | | WarBow revert matrix covered | **PASS** | `cd contracts && forge test --match-test "test_warbow_.*_reverts\|test_finalize_warbow_podium_reverts" -vv` — 10/10 (steal band, daily cap, revenge expiry/no-pending, flag holder/silence, double finalize, bad victim/epoch) | | DRY refactor for charm bounds without behavior change | **PASS** | `ArenaCharmBounds.sol` shared by `TimeArena`, `ArenaXp`, `ArenaCharmPriceTwap`; existing `test_buy_reverts_charm_*` + fuzz charm tests green in `TimeArenaTest` (95 tests) | | Gas snapshot or comment if transfer count reduced | **PASS** | NatSpec on `_routeDoubPrizeSplit` documents one `safeTransfer` when all pool slots map to `address(vaults)`; `test_buy_routes_doub_split` still green | | NonStandardERC20 / fee-on-transfer coverage | **PASS** | `cd contracts && forge test --match-contract NonStandardERC20Test -vv` — 4/4 (buy, WarBow guard/steal, top-up parity) | | Full `forge test` | **PASS** (known skips) | `cd contracts && forge test` — **183 passed**, **4 failed** only in `DoubAirdropMegaethForkTest` (missing gitignored `airdrop/doub.csv`); `DevStackIntegrationTest` **5/5 PASS** | | Targeted TimeArena suite | **PASS** | `cd contracts && forge test --match-contract "TimeArenaTest\|NonStandardERC20Test"` — **99/99** | | Attack-vector documentation in negative tests | **PASS** | Reviewed `TimeArena.t.sol` §316 comments (pause incident response, steal-band griefing, daily-cap bypass, revenge window, double finalize, etc.) | | Invariants / docs cross-links | **PASS** | `docs/testing/invariants-and-business-logic.md` §316 (`INV-TIME-ARENA-PAUSE-MUTATING`, `INV-TIME-ARENA-WARBOW-REVERT-MATRIX`, `INV-TIME-ARENA-CHARM-BOUNDS-DRY`, `INV-TIME-ARENA-BUY-ROUTING-GAS`, `INV-ERC20-123-NONSTANDARD`) | ### Out of scope / not required for this issue - Rabby / Anvil E2E / indexer — contracts-only scope; no frontend/indexer changes in #316. - `docs/operations/pause-and-final-signoff.md` pause copy — explicitly deferred to docs bundle per issue body. No repo changes from this verification pass; closing issue.
PlasticDigits (Migrated from gitlab.com) closed this issue 2026-06-13 13:25:03 +00:00
PlasticDigits commented 2026-06-25 22:05:46 +00:00 (Migrated from gitlab.com)

mentioned in merge request !520

mentioned in merge request !520
PlasticDigits commented 2026-07-07 12:41:28 +00:00 (Migrated from gitlab.com)

mentioned in issue #366

mentioned in issue #366
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#316
No description provided.