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

Merged

PlasticDigits merged 1 commit from issue-316-forge-negative-tests-dry-gas into main 2026-06-13 09:29:22 +00:00

Conversation 5 Commits 1 Files changed 8 +363 -20

PlasticDigits commented 2026-06-13 08:08:35 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

Summary

GitLab #316 — gap follow-up from #309:

• Negative tests: pause matrix for all _requireLive mutators (buy, buyWithCred, WarBow steal/guard/revenge, claimWarBowFlag); WarBow revert matrix (steal band, daily cap, revenge expiry, flag holder/silence, double finalize, bad victim/epoch).
• DRY: new ArenaCharmBounds library shared by TimeArena, ArenaXp, and ArenaCharmPriceTwap (same TimeArena: charm bounds revert string).
• Gas: when default PodiumVaults maps every pool slot to address(vaults), buy and top-up routing batch DOUB into one safeTransfer per call; per-tranche funding events and economics unchanged (#300).
• Non-standard ERC-20: NonStandardERC20.t.sol covers buy, WarBow DOUB pulls, and topUpPodiumPools parity (INV-ERC20-123).
• Docs: invariants §316, WarBow play skill cross-link.

Acceptance checklist

Criterion	Command / step	Result
Pause negative tests for all user-facing mutating functions	cd contracts && forge test --match-test test_pause_blocks -vv	PASS
WarBow revert matrix covered	`cd contracts && forge test --match-test "test_warbow_.*_reverts	test_finalize_warbow_podium_reverts" -vv`
DRY refactor for charm bounds without behavior change	ArenaCharmBounds.sol + existing test_buy_reverts_charm_* / fuzz charm tests	PASS
Gas snapshot or comment if transfer count reduced	NatSpec on _routeDoubPrizeSplit; test_buy_routes_doub_split still green	PASS
Full forge test (except known doub.csv fork skips)	cd contracts && forge test	PASS (4 DoubAirdropMegaethFork + 1 DevStackIntegration setUp fail without doub.csv / env — pre-existing)
Targeted TimeArena suite	cd contracts && forge test --match-contract TimeArenaTest -vv	PASS (92 tests)
NonStandardERC20 coverage	cd contracts && forge test --match-contract NonStandardERC20Test -vv	PASS (4 tests)

Verification for third parties

1. Check out branch issue-316-forge-negative-tests-dry-gas.
2. cd contracts && forge test --match-contract "TimeArenaTest|NonStandardERC20Test" -vv — all tests green.
3. cd contracts && forge test — expect only known skips: DoubAirdropMegaethForkTest (missing gitignored airdrop/doub.csv) and DevStackIntegrationTest setUp (deploy env).
4. Review docs/testing/invariants-and-business-logic.md § TimeArena negative tests (#316).

Related: #316

---

Note

Medium Risk
Buy/top-up routing now uses a batched transfer path on the default vault layout; behavior is guarded by existing routing tests but it touches prize DOUB movement. Charm validation is refactored without intended logic change.

Overview
Adds Forge coverage for GitLab #316: pause blocks all _requireLive mutators (buy, CRED buy, WarBow steal/guard/revenge, flag claim), plus a WarBow revert matrix (steal band, daily cap, revenge window, flag rules, double finalize, bad victim/epoch).

Introduces ArenaCharmBounds so TimeArena, ArenaXp, and ArenaCharmPriceTwap share one 0.99–10 CHARM envelope and the same TimeArena: charm bounds revert. NonStandardERC20.t.sol asserts fee-on-transfer DOUB fails _pullDoubExact on buy, WarBow DOUB spends, and podium top-up.

Gas: when every PodiumVaults pool slot is the vault contract (default), buy and top-up routing send DOUB in one safeTransfer instead of many; per-tranche notify events and splits are unchanged. Invariants doc §316 and the WarBow play skill are updated.

^{Reviewed by Cursor Bugbot for commit 18a24e5101. Bugbot is set up for automated code reviews on this repo. Configure here.}

## Summary GitLab [#316](https://gitlab.com/PlasticDigits/yieldomega/-/issues/316) — gap follow-up from [#309](https://gitlab.com/PlasticDigits/yieldomega/-/issues/309): - **Negative tests:** pause matrix for all `_requireLive` mutators (`buy`, `buyWithCred`, WarBow steal/guard/revenge, `claimWarBowFlag`); WarBow revert matrix (steal band, daily cap, revenge expiry, flag holder/silence, double finalize, bad victim/epoch). - **DRY:** new `ArenaCharmBounds` library shared by `TimeArena`, `ArenaXp`, and `ArenaCharmPriceTwap` (same `TimeArena: charm bounds` revert string). - **Gas:** when default `PodiumVaults` maps every pool slot to `address(vaults)`, buy and top-up routing batch DOUB into **one** `safeTransfer` per call; per-tranche funding events and economics unchanged ([#300](https://gitlab.com/PlasticDigits/yieldomega/-/issues/300)). - **Non-standard ERC-20:** `NonStandardERC20.t.sol` covers buy, WarBow DOUB pulls, and `topUpPodiumPools` parity (`INV-ERC20-123`). - **Docs:** invariants §316, WarBow play skill cross-link. ## Acceptance checklist | Criterion | Command / step | Result | |-----------|----------------|--------| | Pause negative tests for all user-facing mutating functions | `cd contracts && forge test --match-test test_pause_blocks -vv` | PASS | | WarBow revert matrix covered | `cd contracts && forge test --match-test "test_warbow_.*_reverts|test_finalize_warbow_podium_reverts" -vv` | PASS | | DRY refactor for charm bounds without behavior change | `ArenaCharmBounds.sol` + existing `test_buy_reverts_charm_*` / fuzz charm tests | PASS | | Gas snapshot or comment if transfer count reduced | NatSpec on `_routeDoubPrizeSplit`; `test_buy_routes_doub_split` still green | PASS | | Full `forge test` (except known `doub.csv` fork skips) | `cd contracts && forge test` | PASS (4 `DoubAirdropMegaethFork` + 1 `DevStackIntegration` setUp fail without `doub.csv` / env — pre-existing) | | Targeted TimeArena suite | `cd contracts && forge test --match-contract TimeArenaTest -vv` | PASS (92 tests) | | NonStandardERC20 coverage | `cd contracts && forge test --match-contract NonStandardERC20Test -vv` | PASS (4 tests) | ## Verification for third parties 1. Check out branch `issue-316-forge-negative-tests-dry-gas`. 2. `cd contracts && forge test --match-contract "TimeArenaTest|NonStandardERC20Test" -vv` — all tests green. 3. `cd contracts && forge test` — expect only known skips: `DoubAirdropMegaethForkTest` (missing gitignored `airdrop/doub.csv`) and `DevStackIntegrationTest` setUp (deploy env). 4. Review `docs/testing/invariants-and-business-logic.md` § [TimeArena negative tests (#316)](docs/testing/invariants-and-business-logic.md#timearena-negative-tests-dry-gas-gitlab-316). Related: [#316](https://gitlab.com/PlasticDigits/yieldomega/-/issues/316) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Buy/top-up routing now uses a batched transfer path on the default vault layout; behavior is guarded by existing routing tests but it touches prize DOUB movement. Charm validation is refactored without intended logic change. > > **Overview** > Adds **Forge coverage** for GitLab #316: pause blocks all `_requireLive` mutators (buy, CRED buy, WarBow steal/guard/revenge, flag claim), plus a WarBow revert matrix (steal band, daily cap, revenge window, flag rules, double finalize, bad victim/epoch). > > Introduces **`ArenaCharmBounds`** so `TimeArena`, `ArenaXp`, and `ArenaCharmPriceTwap` share one 0.99–10 CHARM envelope and the same `TimeArena: charm bounds` revert. **`NonStandardERC20.t.sol`** asserts fee-on-transfer DOUB fails `_pullDoubExact` on buy, WarBow DOUB spends, and podium top-up. > > **Gas:** when every `PodiumVaults` pool slot is the vault contract (default), buy and top-up routing send DOUB in **one** `safeTransfer` instead of many; per-tranche notify events and splits are unchanged. Invariants doc §316 and the WarBow play skill are updated. > > ^{Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 18a24e5101e012f9e6c5f388439624d54bf850d8. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).} <!-- /CURSOR_SUMMARY -->

project_80561744_bot_5118c68a88bff95eacb6ab1c47c19cae commented 2026-06-13 08:08:45 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

changed the description

changed the description

project_80561744_bot_5118c68a88bff95eacb6ab1c47c19cae commented 2026-06-13 08:08:54 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

changed the description

changed the description

PlasticDigits commented 2026-06-13 08:10:58 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

Security review — MR !461

Commit reviewed: 18a24e5101e012f9e6c5f388439624d54bf850d8

Scope: Onchain changes in ArenaCharmBounds.sol, TimeArena.sol (_routeDoubPrizeSplit / _routeActiveAndSeedVaults batch routing), constant re-exports in ArenaXp.sol and ArenaCharmPriceTwap.sol; new Forge tests (TimeArena.t.sol, NonStandardERC20.t.sol); docs/skills only elsewhere.

Method: Diff review with sink tracing on batched DOUB routing (attacker-controlled buy/top-up amounts → _pullDoubExact → safeTransfer / notifyPodiumEpochFunded). Prior MR notes checked — no prior security-review threads to re-report.

Outcome: FINDINGS: 0 (no medium, high, or critical issues on this diff).

Inline threads: None (no qualifying findings).

Notes (informational, below reporting threshold)

• Batch routing gas path: Batching activates only when every relevant PodiumVaults pool slot resolves to address(podiumVaults) (default deploy layout). One safeTransfer replaces N transfers to the same recipient; per-tranche PodiumEpochFunded / PodiumFunded / SeedFunded emissions and split math are unchanged. notify* handlers are emit-only and only arena; buy/top-up remain nonReentrant. Indexer ingest uses PodiumEpochFunded (not ERC-20 Transfer count).
• ArenaCharmBounds: Pure DRY of existing 0.99–10 CHARM envelope; no new external surface.
• New tests: Negative/pause/WarBow matrix and fee-on-transfer coverage; no production behavior change beyond routing refactor above.

Security review: no medium+ findings on this diff.

## Security review — MR !461 **Commit reviewed:** `18a24e5101e012f9e6c5f388439624d54bf850d8` **Scope:** Onchain changes in `ArenaCharmBounds.sol`, `TimeArena.sol` (`_routeDoubPrizeSplit` / `_routeActiveAndSeedVaults` batch routing), constant re-exports in `ArenaXp.sol` and `ArenaCharmPriceTwap.sol`; new Forge tests (`TimeArena.t.sol`, `NonStandardERC20.t.sol`); docs/skills only elsewhere. **Method:** Diff review with sink tracing on batched DOUB routing (attacker-controlled buy/top-up amounts → `_pullDoubExact` → `safeTransfer` / `notifyPodiumEpochFunded`). Prior MR notes checked — no prior security-review threads to re-report. **Outcome:** **FINDINGS: 0** (no medium, high, or critical issues on this diff). **Inline threads:** None (no qualifying findings). ### Notes (informational, below reporting threshold) - **Batch routing gas path:** Batching activates only when every relevant `PodiumVaults` pool slot resolves to `address(podiumVaults)` (default deploy layout). One `safeTransfer` replaces N transfers to the same recipient; per-tranche `PodiumEpochFunded` / `PodiumFunded` / `SeedFunded` emissions and split math are unchanged. `notify*` handlers are emit-only and `only arena`; buy/top-up remain `nonReentrant`. Indexer ingest uses `PodiumEpochFunded` (not ERC-20 `Transfer` count). - **`ArenaCharmBounds`:** Pure DRY of existing 0.99–10 CHARM envelope; no new external surface. - **New tests:** Negative/pause/WarBow matrix and fee-on-transfer coverage; no production behavior change beyond routing refactor above. Security review: **no medium+ findings** on this diff.

PlasticDigits commented 2026-06-13 09:29:22 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

mentioned in commit adb400d006

mentioned in commit adb400d0060f43151801aa57e7dac92533a6b4f3

PlasticDigits (Migrated from gitlab.com) merged commit adb400d006 into main 2026-06-13 09:29:22 +00:00

PlasticDigits commented 2026-06-13 13:25:03 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

mentioned in issue #316

mentioned in issue #316

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

agent:fix_bugfix

  

agent:fix_conflicts

  

agent:fix_security

  

agent:gap_analysis

  

agent:implement

  

agent:open_issues

  

agents

  

agent:security_audit

  

agent:verify

  

area

timecurve

  

arena

  

arena-v2

  

audit

  

block:feature_review

  

block:security

  

bug

  

contracts

  

design

  

devops

  

docs

Documentation

  

documentation

  

enhancement

  

epic

Epic / umbrella issue

  

feature

  

frontend

  

indexer

  

low priority

Low priority, investigate other issues first

  

megaeth

  

operations

  

ops

Deploy and operations

  

performance

  

QA

  

qa checklist

  

referrals

  

removal

Legacy removal

  

security

  

smart-contracts

  

tech-debt

  

testing

  

time-arena

TimeArena v2 revamp

  

timecurve

  

visual-qa

No labels

agent:fix_bugfix

agent:fix_conflicts

agent:fix_security

agent:gap_analysis

agent:implement

agent:open_issues

agents

agent:security_audit

agent:verify

area

timecurve

arena

arena-v2

audit

block:feature_review

block:security

bug

contracts

design

devops

docs

documentation

enhancement

epic

feature

frontend

indexer

low priority

megaeth

operations

ops

performance

QA

qa checklist

referrals

removal

security

smart-contracts

tech-debt

testing

time-arena

timecurve

visual-qa

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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!461

No description provided.