Audit M-03: RabbitTreasury Burrow param bounds, finalizeEpoch liveness, and single-epochId doc clarity #119

Closed
opened 2026-05-04 04:17:44 +00:00 by PlasticDigits · 6 comments
PlasticDigits commented 2026-05-04 04:17:44 +00:00 (Migrated from gitlab.com)

Summary

This issue tracks audit finding M-03 from audits/audit_smartcontract_1777813071.md: RabbitTreasury Burrow parameter setters lack on-chain bounds for several math-critical WAD values. PARAMS_ROLE can set cStarWad, alphaWad, betaWad, lamWad, deltaMaxFracWad to arbitrary uint256 values. Some other setters already validate (e.g. setMBoundsWad enforces mMin < mMax, fee / min-efficiency setters enforce < WAD or (0, WAD]), but the unbounded group can still cause finalizeEpoch to revert, produce extreme repricing, or yield unintuitive redemption economics.

Privileged impact: finalizeEpoch is permissionless. After a params update, keepers or any caller rely on stable math; griefing or misconfiguration risks liveness breaks (finalize reverts) or sharp e/dynamics even without attacker control of roles.

Product / docs correction (maintainer input): Yieldomega v1 is not a multi-epoch product in the sense of distinct “Epoch 2 governance phases” or “defer changes until the next era.” The audit’s suggestion to use “delayed activation by next epoch” is not the right framing here. On-chain, finalizeEpoch still increments epochId for rolling snapshot / cooldown accounting only—that is not “a second product epoch.” Documentation and runbooks must state this clearly so readers do not assume a multi-epoch roadmap. Preferred mitigations are hard bounds on setters, regression tests, and, if governance wants delay, timelock / two-step admin—not “wait for the next epochId” as if that were a separate product stage.


Affected on-chain surface

Setter Current validation Risk
setCStarWad None Mis-set vs coverage / cMaxWad; interacts with multiplierWad
setAlphaWad None If alphaWad >= WAD, BurrowMath.multiplierWad can hit inner<=0 revert
setBetaWad None Numerical / curve steepness; extreme values
setLamWad None Smoothing; interacts with nextEWad step
setDeltaMaxFracWad None Cap on per-step e move; nextEWad e<=0 if step drives non-positive e

Reference: contracts/src/RabbitTreasury.sol (parameter setters ~L441–472), contracts/src/libraries/BurrowMath.sol (multiplierWad require(i > 0, "BurrowMath: inner<=0"), nextEWad require(en > 0, "BurrowMath: e<=0")).

Deployed defaults and intended envelopes are already listed in contracts/PARAMETERS.md (§ Rabbit Treasury (Burrow)), e.g. c_star 1.05, alpha 0.02, beta 2.0, lambda 0.5, delta_max_frac 0.02, m_min/m_max 0.98–1.02, c_max 2.0—setters should enforce a documented safe band consistent with simulations (and simulations/bounded_formulas/model.py / BurrowMath NatSpec cross-link).


  1. Add require guards on setCStarWad, setAlphaWad, setBetaWad, setLamWad, setDeltaMaxFracWad matching the simulation envelope in PARAMETERS.md (tight min/max WAD constants or shared internal library). Examples of constraints to encode in design review:

    • alphaWad < WAD (strict) so 1 + α·tanh(·) stays positive in multiplierWad.
    • 0 < lamWad ≤ WAD (or tighter band per sims).
    • 0 < deltaMaxFracWad ≤ WAD (or tighter, e.g. cap at a few percent per epoch step per product).
    • cStarWad in (0, cMaxWad] or consistent band vs initializer cMaxWad.
    • betaWad upper bound to limit exp/tanh path stress (coordinate with BurrowMath.t.sol fuzz envelope).
  2. Single source of truth: Either duplicate documented constants in Solidity with a comment “must match PARAMETERS.md table” or extract named constants in RabbitTreasury / BurrowMath and sync PARAMETERS.md + product docs to those names.

  3. Deferred activation: Do not implement “apply on next epoch” as the primary fix for v1 framing. If delayed effect is still desired operationally, use OpenZeppelin Timelock, two-step governance, or explicit “pending params + admin apply” pattern—documented as governance, not as “next epoch” product language.


Add to contracts/test/RabbitTreasury.t.sol / contracts/test/BurrowMath.t.sol as appropriate:

  1. Bounds rejection: For each setter, vm.expectRevert when val is just below min or just above max (per chosen constants).
  2. Finalize liveness smoke: With open epoch, non-trivial supply + backing, warp past epochEnd, assert finalizeEpoch succeeds after each boundary-legal update (can be a parametrized loop or small Cartesian sample).
  3. BurrowMath linkage: Reproduce pre-fix failure modes in unit tests: e.g. alphaWad = WAD would make multiplierWad revert—after fix, setAlphaWad(WAD) must revert at the treasury setter, not at finalize.
  4. Fuzz / invariant extension (optional): Constrain fuzz to legal params and assert finalizeEpoch + invariants still hold (extend contracts/test/RabbitTreasuryInvariant.t.sol only if it adds clear value).

Map new tests in docs/testing/invariants-and-business-logic.md § RabbitTreasury and BurrowMath if they introduce new named scenarios.


Files likely to change

Solidity / tests

  • contracts/src/RabbitTreasury.sol — bounds on setCStarWad, setAlphaWad, setBetaWad, setLamWad, setDeltaMaxFracWad; NatSpec referencing envelope.
  • contracts/src/libraries/BurrowMath.sol — optional shared constants or comments if bounds are centralized here.
  • contracts/test/RabbitTreasury.t.sol — bounds + finalize liveness tests.
  • contracts/test/BurrowMath.t.sol — numerical edge cases if needed.
  • contracts/test/RabbitTreasuryInvariant.t.sol — only if extending invariants.

Documentation

  • contracts/PARAMETERS.md — replace vague “Per simulations” with explicit min/max bounds columns aligned with on-chain requires.
  • docs/product/rabbit-treasury.md — clarify that epochId / finalizeEpoch are rolling accounting windows (snapshots, cooldown indexing), not a multi-phase “Epoch 2” product; avoid reader confusion with governance delay language.
  • docs/testing/invariants-and-business-logic.md — new/updated test map rows for param bounds + finalize liveness.
  • docs/onchain/fee-routing-and-governance.md (and any operator / Stage 3 runbook referencing Burrow params) — governance: bounds + optional timelock; not “defer to next epoch” as product semantics.
  • audits/audit_smartcontract_1777813071.md — optional maintainer note linking this issue and correcting the “next epoch” mitigation for v1 alignment.

Other

  • simulations/bounded_formulas/model.py (or notebooks) — verify stated parameter ranges match enforced bounds; update comments if needed.

No indexer / frontend change expected unless copy surfaces raw param names and needs a one-line clarification (unlikely).


Acceptance criteria

  • Each of setCStarWad, setAlphaWad, setBetaWad, setLamWad, setDeltaMaxFracWad rejects out-of-envelope values with a clear revert (prefer stable short reason strings consistent with existing "RT: …" style).
  • On-chain bounds are numerically consistent with contracts/PARAMETERS.md Rabbit Treasury table (single documented min/max or fixed allowed set).
  • alphaWad < WAD (or tighter) so BurrowMath: inner<=0 cannot be reached via governance alone from a previously valid state without violating a setter (document in NatSpec).
  • New Foundry tests cover revert at boundary and finalizeEpoch success after legal updates under representative reserve/supply conditions.
  • CI forge test passes for the touched packages.
  • Product/docs explicitly state: v1 is not a multi-epoch product roadmap; epochId rolling forward is accounting/snapshot/cooldown indexing only; governance delay should be described as timelock / process, not “next epoch” ambiguity.

Verification checklist (implementer)

  1. Run forge test --match-contract RabbitTreasury (or full forge test per docs/testing/strategy.md).
  2. Manually review diffs: setter bounds match PARAMETERS.md table.
  3. Grep docs for misleading “next epoch” / multi-epoch product phrasing; update to rolling window / epochId accounting language.
  4. If constants are duplicated, confirm one narrative in PARAMETERS + NatSpec (no drift).

Verification checklist (reviewer / operator)

  1. Read NatSpec on each changed setter; confirm bounds cite the design envelope.
  2. Spot-check: attempt cast send / script with illegal value on testnet—revert at setter, not at finalizeEpoch.
  3. Confirm permissionless finalizeEpoch still succeeds on a healthy deployment after max-legal params (optional Anvil script).

References

  • Audit: M-03 — audits/audit_smartcontract_1777813071.md
  • Maintainer note: single-epoch product framing; reject audit’s “delayed activation by next epoch” as stated; align docs + mitigations accordingly.
## Summary This issue tracks **audit finding M-03** from `audits/audit_smartcontract_1777813071.md`: **`RabbitTreasury` Burrow parameter setters lack on-chain bounds** for several math-critical WAD values. `PARAMS_ROLE` can set **`cStarWad`, `alphaWad`, `betaWad`, `lamWad`, `deltaMaxFracWad`** to arbitrary `uint256` values. Some other setters already validate (e.g. `setMBoundsWad` enforces `mMin < mMax`, fee / min-efficiency setters enforce `< WAD` or `(0, WAD]`), but the unbounded group can still cause **`finalizeEpoch`** to revert, produce extreme repricing, or yield unintuitive redemption economics. **Privileged impact:** `finalizeEpoch` is **permissionless**. After a params update, **keepers or any caller** rely on stable math; griefing or misconfiguration risks **liveness breaks** (finalize reverts) or **sharp e/dynamics** even without attacker control of roles. **Product / docs correction (maintainer input):** Yieldomega v1 is **not** a **multi-epoch product** in the sense of distinct “Epoch 2 governance phases” or “defer changes until the next era.” The audit’s suggestion to use **“delayed activation by next epoch”** is **not** the right framing here. On-chain, `finalizeEpoch` still **increments `epochId`** for **rolling snapshot / cooldown accounting** only—that is **not** “a second product epoch.” **Documentation and runbooks** must state this clearly so readers do not assume a multi-epoch roadmap. Preferred mitigations are **hard bounds on setters**, **regression tests**, and, if governance wants delay, **timelock / two-step admin**—**not** “wait for the next `epochId`” as if that were a separate product stage. --- ## Affected on-chain surface | Setter | Current validation | Risk | |--------|-------------------|------| | `setCStarWad` | None | Mis-set vs coverage / `cMaxWad`; interacts with `multiplierWad` | | `setAlphaWad` | None | If `alphaWad >= WAD`, `BurrowMath.multiplierWad` can hit **`inner<=0`** revert | | `setBetaWad` | None | Numerical / curve steepness; extreme values | | `setLamWad` | None | Smoothing; interacts with `nextEWad` step | | `setDeltaMaxFracWad` | None | Cap on per-step `e` move; `nextEWad` **`e<=0`** if step drives non-positive `e` | Reference: `contracts/src/RabbitTreasury.sol` (parameter setters ~L441–472), `contracts/src/libraries/BurrowMath.sol` (`multiplierWad` **`require(i > 0, "BurrowMath: inner<=0")`**, `nextEWad` **`require(en > 0, "BurrowMath: e<=0")`**). **Deployed defaults** and intended envelopes are already listed in `contracts/PARAMETERS.md` (§ **Rabbit Treasury (Burrow)**), e.g. `c_star` 1.05, `alpha` 0.02, `beta` 2.0, `lambda` 0.5, `delta_max_frac` 0.02, `m_min`/`m_max` 0.98–1.02, `c_max` 2.0—**setters should enforce a documented safe band consistent with simulations** (and `simulations/bounded_formulas/model.py` / BurrowMath NatSpec cross-link). --- ## Recommended fix (implementation) 1. **Add `require` guards** on `setCStarWad`, `setAlphaWad`, `setBetaWad`, `setLamWad`, `setDeltaMaxFracWad` matching the **simulation envelope** in `PARAMETERS.md` (tight **min/max WAD** constants or shared internal library). Examples of constraints to encode in design review: - **`alphaWad < WAD`** (strict) so `1 + α·tanh(·)` stays positive in `multiplierWad`. - **`0 < lamWad ≤ WAD`** (or tighter band per sims). - **`0 < deltaMaxFracWad ≤ WAD`** (or tighter, e.g. cap at a few percent per epoch step per product). - **`cStarWad`** in `(0, cMaxWad]` or consistent band vs initializer `cMaxWad`. - **`betaWad`** upper bound to limit exp/tanh path stress (coordinate with `BurrowMath.t.sol` fuzz envelope). 2. **Single source of truth:** Either duplicate documented constants in Solidity with a comment “must match PARAMETERS.md table” or extract **named constants** in `RabbitTreasury` / `BurrowMath` and **sync** `PARAMETERS.md` + product docs to those names. 3. **Deferred activation:** **Do not** implement “apply on next epoch” as the primary fix for v1 framing. If delayed effect is still desired operationally, use **OpenZeppelin Timelock**, **two-step governance**, or explicit **“pending params + admin apply”** pattern—**documented as governance**, not as “next epoch” product language. --- ## Recommended regression tests (Foundry) Add to `contracts/test/RabbitTreasury.t.sol` / `contracts/test/BurrowMath.t.sol` as appropriate: 1. **Bounds rejection:** For each setter, **`vm.expectRevert`** when `val` is **just below min** or **just above max** (per chosen constants). 2. **Finalize liveness smoke:** With **open epoch**, **non-trivial supply + backing**, warp past `epochEnd`, assert **`finalizeEpoch` succeeds** after each **boundary-legal** update (can be a parametrized loop or small Cartesian sample). 3. **BurrowMath linkage:** Reproduce **pre-fix** failure modes in **unit** tests: e.g. **`alphaWad = WAD`** would make `multiplierWad` revert—after fix, **`setAlphaWad(WAD)` must revert** at the **treasury** setter, not at finalize. 4. **Fuzz / invariant extension (optional):** Constrain fuzz to legal params and assert **`finalizeEpoch` + invariants** still hold (extend `contracts/test/RabbitTreasuryInvariant.t.sol` only if it adds clear value). Map new tests in `docs/testing/invariants-and-business-logic.md` § **RabbitTreasury and BurrowMath** if they introduce new named scenarios. --- ## Files likely to change ### Solidity / tests - `contracts/src/RabbitTreasury.sol` — bounds on `setCStarWad`, `setAlphaWad`, `setBetaWad`, `setLamWad`, `setDeltaMaxFracWad`; NatSpec referencing envelope. - `contracts/src/libraries/BurrowMath.sol` — optional shared constants or comments if bounds are centralized here. - `contracts/test/RabbitTreasury.t.sol` — bounds + finalize liveness tests. - `contracts/test/BurrowMath.t.sol` — numerical edge cases if needed. - `contracts/test/RabbitTreasuryInvariant.t.sol` — only if extending invariants. ### Documentation - `contracts/PARAMETERS.md` — replace vague “Per simulations” with **explicit min/max bounds** columns aligned with on-chain `require`s. - `docs/product/rabbit-treasury.md` — clarify that **`epochId` / `finalizeEpoch`** are **rolling accounting windows** (snapshots, cooldown indexing), **not** a multi-phase “Epoch 2” product; avoid reader confusion with **governance delay** language. - `docs/testing/invariants-and-business-logic.md` — new/updated test map rows for param bounds + finalize liveness. - `docs/onchain/fee-routing-and-governance.md` (and any **operator / Stage 3 runbook** referencing Burrow params) — **governance**: bounds + optional timelock; **not** “defer to next epoch” as product semantics. - `audits/audit_smartcontract_1777813071.md` — optional maintainer note linking this issue and **correcting** the “next epoch” mitigation for v1 alignment. ### Other - `simulations/bounded_formulas/model.py` (or notebooks) — verify stated parameter ranges match enforced bounds; update comments if needed. **No indexer / frontend change** expected unless copy surfaces raw param names and needs a one-line clarification (unlikely). --- ## Acceptance criteria - [ ] Each of **`setCStarWad`, `setAlphaWad`, `setBetaWad`, `setLamWad`, `setDeltaMaxFracWad`** rejects out-of-envelope values with a **clear revert** (prefer stable short reason strings consistent with existing `"RT: …"` style). - [ ] On-chain bounds are **numerically consistent** with **`contracts/PARAMETERS.md`** Rabbit Treasury table (single documented min/max or fixed allowed set). - [ ] **`alphaWad < WAD`** (or tighter) so **`BurrowMath: inner<=0`** cannot be reached via governance alone from a previously valid state **without** violating a setter (document in NatSpec). - [ ] New **Foundry tests** cover **revert at boundary** and **`finalizeEpoch` success** after legal updates under representative reserve/supply conditions. - [ ] CI **`forge test`** passes for the touched packages. - [ ] Product/docs explicitly state: **v1 is not a multi-epoch product roadmap**; **`epochId` rolling forward** is **accounting/snapshot/cooldown indexing** only; governance delay should be described as **timelock / process**, not “next epoch” ambiguity. --- ## Verification checklist (implementer) 1. Run `forge test --match-contract RabbitTreasury` (or full `forge test` per `docs/testing/strategy.md`). 2. Manually review diffs: setter bounds match `PARAMETERS.md` table. 3. Grep docs for misleading **“next epoch”** / **multi-epoch product** phrasing; update to **rolling window / epochId accounting** language. 4. If constants are duplicated, confirm **one narrative** in PARAMETERS + NatSpec (no drift). ## Verification checklist (reviewer / operator) 1. Read NatSpec on each changed setter; confirm bounds cite the design envelope. 2. Spot-check: attempt `cast send` / script with illegal value on testnet—**revert at setter**, not at **`finalizeEpoch`**. 3. Confirm **permissionless `finalizeEpoch`** still succeeds on a healthy deployment after **max-legal** params (optional Anvil script). --- ## References - Audit: **M-03** — `audits/audit_smartcontract_1777813071.md` - Maintainer note: **single-epoch product framing**; **reject** audit’s “delayed activation by next epoch” **as stated**; align docs + mitigations accordingly.
PlasticDigits commented 2026-05-04 05:14:15 +00:00 (Migrated from gitlab.com)

mentioned in commit 547e9e1ae9

mentioned in commit 547e9e1ae9f943cb2948ce29eb60b1bc7923b24a
PlasticDigits commented 2026-05-04 05:14:19 +00:00 (Migrated from gitlab.com)

/cc @brouie

Merged to main (includes RabbitTreasury Burrow param envelopes on initialize + setCStarWad / setAlphaWad / setBetaWad / setLamWad / setDeltaMaxFracWad, Foundry coverage, docs ↔ invariants, PARAMETERS / simulation docstring, skills cross-links).

Verification checklist

Leaving the issue open for your sign-off on audit M-03.

/cc @brouie **Merged to `main`** (includes RabbitTreasury Burrow param envelopes on `initialize` + `setCStarWad` / `setAlphaWad` / `setBetaWad` / `setLamWad` / `setDeltaMaxFracWad`, Foundry coverage, docs ↔ invariants, PARAMETERS / simulation docstring, skills cross-links). **Verification checklist** - [ ] `cd contracts && forge test --match-contract 'RabbitTreasury|BurrowMath'` — expect **42** tests green (includes invariant fuzz). - [ ] Skim [`contracts/PARAMETERS.md`](contracts/PARAMETERS.md) Rabbit Treasury (Burrow) row vs on-chain `MAX_*` / revert strings. - [ ] [`docs/testing/invariants-and-business-logic.md#rabbit-treasury-burrow-param-bounds-gitlab-119`](docs/testing/invariants-and-business-logic.md#rabbit-treasury-burrow-param-bounds-gitlab-119) matches implementation. Leaving the issue **open** for your sign-off on audit **M-03**.
PlasticDigits commented 2026-05-04 05:16:46 +00:00 (Migrated from gitlab.com)

mentioned in commit 9905b25c9c

mentioned in commit 9905b25c9c11f63b798de6a4fc4ab3127fbd8a9e
PlasticDigits commented 2026-05-04 05:27:04 +00:00 (Migrated from gitlab.com)

mentioned in commit 717c7fd6a1

mentioned in commit 717c7fd6a1e34b66ec0fa8850b5250ddb6a37164
Brouie commented 2026-05-04 06:17:25 +00:00 (Migrated from gitlab.com)

verified on commit 547e9e1 (merged via d12bb99). all bound checks land + finalizeEpoch liveness covered.

setter bound rejections — your AC #1:

  • test_setAlphaWad_reverts_when_not_strictly_below_wad — alphaWad < WAD enforced (closes the BurrowMath inner<=0 revert path you flagged)
  • test_setBetaWad_reverts_out_of_envelope — beta upper bound enforced
  • test_setCStarWad_reverts_out_of_envelope — cStar in (0, cMaxWad] band
  • test_setDeltaMaxFracWad_reverts_out_of_envelope — deltaMax bounded
  • test_setLamWad_reverts_out_of_envelope — lambda bounded

finalizeEpoch liveness — your AC #2:

  • test_finalizeEpoch_succeeds_after_extreme_legal_params — finalize succeeds across boundary-legal updates

existing invariants intact:

  • invariant_rabbitTreasury_doubSupplyMatchesGhostMintBurn (256 runs, 6400 calls, 57 reverts) — supply ledger preserved
  • invariant_rabbitTreasury_reservesMatchTokenBalance (256 runs, 6400 calls, 43 reverts) — backing ledger preserved
  • test_protocolOwned_notExtracted_viaOrdinaryWithdraw — protocol-owned bucket isolation preserved
  • test_receiveFee_doesNotMintDoub_andDoesNotIncreaseRedeemable — fee accounting invariant preserved

setters now reject pre-fix BurrowMath revert modes at the treasury level, not at finalize. matches your spec — single source of truth in PARAMETERS.md documented bounds.

verified on commit `547e9e1` (merged via `d12bb99`). all bound checks land + finalizeEpoch liveness covered. **setter bound rejections — your AC #1:** - `test_setAlphaWad_reverts_when_not_strictly_below_wad` — alphaWad < WAD enforced (closes the BurrowMath `inner<=0` revert path you flagged) - `test_setBetaWad_reverts_out_of_envelope` — beta upper bound enforced - `test_setCStarWad_reverts_out_of_envelope` — cStar in (0, cMaxWad] band - `test_setDeltaMaxFracWad_reverts_out_of_envelope` — deltaMax bounded - `test_setLamWad_reverts_out_of_envelope` — lambda bounded **finalizeEpoch liveness — your AC #2:** - `test_finalizeEpoch_succeeds_after_extreme_legal_params` — finalize succeeds across boundary-legal updates **existing invariants intact:** - `invariant_rabbitTreasury_doubSupplyMatchesGhostMintBurn` (256 runs, 6400 calls, 57 reverts) — supply ledger preserved - `invariant_rabbitTreasury_reservesMatchTokenBalance` (256 runs, 6400 calls, 43 reverts) — backing ledger preserved - `test_protocolOwned_notExtracted_viaOrdinaryWithdraw` — protocol-owned bucket isolation preserved - `test_receiveFee_doesNotMintDoub_andDoesNotIncreaseRedeemable` — fee accounting invariant preserved setters now reject pre-fix BurrowMath revert modes at the treasury level, not at finalize. matches your spec — single source of truth in `PARAMETERS.md` documented bounds.
PlasticDigits commented 2026-05-04 08:12:53 +00:00 (Migrated from gitlab.com)

mentioned in commit 166fd6fa3a

mentioned in commit 166fd6fa3ab45ee699b8eb49c75c4bfb3c9a5bd5
PlasticDigits (Migrated from gitlab.com) closed this issue 2026-05-04 08:18:31 +00:00
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#119
No description provided.