L-05: Balance-delta ERC20 accounting (standard ERC20 covenant)

PlasticDigits commented

2026-05-04 04:20:50 +00:00

(Migrated from gitlab.com)

Source

Tracked from internal smart-contract audit excerpt L-05 in audits/audit_smartcontract_1777813071.md:

Standard-ERC20 assumptions are security-critical — contracts use SafeERC20, but accounting generally uses requested amounts rather than received balance deltas. Deployment invariant must remain “standard ERC20 only”; accounting should match tokens actually credited.

Problem statement

SafeERC20 correctly handles missing/false bool returns but does not make transferFrom move exactly the caller-supplied amount in all real ERC20 deployments. Categories of tokens that violate the “canonical ERC20 accounting model” include fee-on-transfer, rebasing, rebate-on-transfer hooks (ERC-777-adjacent), pausable/blacklist, upgradeable/token-with-fees wrappers, etc.

Across core contracts, totalRaised, treasury totalReserves, routed fee splits, burns, vesting allocations, events, and invariants implicitly assume balance change == requestedAmount.

That assumption is accurate only for standard ERC-20 implementations (constant balance on transfer excluding mint/burn semantics the token documents). For other assets it becomes a silent or revert-prone correctness bug: internal ledgers drift from balanceOf, permissionless routers can fail mid-flow, payouts can under-transfer, indexer “volume” ghosts misalign with treasury reality, etc.

Operational docs already warn operators; audit L-05 calls out this is policy + implementation smell, not purely documentation.

Goals (dual requirement)

Onchain bookkeeping: Prefer balance deltas (before/after balanceOf snapshots around each pull/push cohort) wherever value enters or leaves protocol-controlled ERC20 custody, rather than trusting nominated request sizes for += ledger updates, distributeFees arguments, burns, emitted economic fields, etc.
For canonical standard ERC20, received == requested anyway; deltas add correctness parity between storage and balanceOf.
Documentation & deployment posture: Strengthen explicit, discoverable docs that acceptedAsset / CL8Y / reserve / vested funding tokens are expected to be standard ERC20. Balance-delta accounting is not consent to ship fee-on-transfer or rebasing “supported assets”: it aligns ledgers when possible and makes violations observeable/testable.

Recommended implementation approach

Shared pattern

For each safeTransferFrom(payer, this, nominal):

uint256 b0 = token.balanceOf(address(this));
token.safeTransferFrom(payer, address(this), nominal);
uint256 received = token.balanceOf(address(this)) - b0;
Use received (not nominal) for:
- Stateful counters (totalRaised, totalReserves, etc.).
- Downstream safeTransfer / distributeFees / vesting allocations driven by “what actually arrived”.
- Event payloads intended to mirror economically realized ingress/egress (unless intentionally documenting “priced quote”; see below).

Edge cases:

received == 0: generally revert (no-op steals gas / breaks invariants).
Strict parity (optional complementary guardrail): if product requires pricing math ingress == physical ingress, add require(received == nominal, "...") after snapshot — standard tokens pass; malformed tokens fail early and explicitly. Decide per path whether economic rule is received must equal priced amount (preserves participant-facing CHARM/price linkage) vs best-effort received (usually wrong for auctions with fixed charmWad). Default recommendation for TimeCurve._buy:** require(received == amount) + ledger from received so CHARM sizing stays coherent and storage uses delta by construction even if someone later tweaks code paths.

`TimeCurve` (priority)

contracts/src/TimeCurve.sol: _buy today does acceptedAsset.safeTransferFrom(..., amount) then totalRaised += amount and feeRouter.distributeFees(acceptedAsset, amount).

Migrate to received from acceptedAsset balance delta on address(this) for distributeFees, totalRaised, and Buy / ReferralApplied payloads (use received everywhere the audit calls “accounting”; align NatSpec/event comments).
Apply the same discipline to warbowSteal, warbowRevenge, warbowGuard multi-pull paths ( totalBurn accounting vs actual aggregate delta before burn sink transfer).
Any other acceptedAsset.safeTransferFrom ingress in this file — same checklist.

`TimeCurveBuyRouter`

contracts/src/TimeCurveBuyRouter.sol already computes cl8yGain from CL8Y balance delta post-swap. Extend consistency:

Stable path: amountInMaximum pull should record received delta on address(this) (not amountInMaximum) before approving router; refunds already use leftovers — verify no ledger uses amountInMaximum as “spent”.
Document that grossCl8y emitted is still priced target; buyFor ultimately depends on TimeCurve ingress delta after approval.

`RabbitTreasury`

contracts/src/RabbitTreasury.sol: deposits that safeTransferFrom then mutate totalReserves — switch reserves bookkeeping to received deltas (+ optional require(received == amount)).

`ReferralRegistry`

contracts/src/ReferralRegistry.sol: burn uses transferFrom → dead address. Measure delta at BURN_ADDRESS or delta leaving caller consistently; emit/docs should match actually burned.

`FeeRouter`, `FeeSink`, `PodiumPool`

contracts/src/FeeRouter.sol, contracts/src/sinks/FeeSink.sol, contracts/src/sinks/PodiumPool.sol: callers must pass exact spendable balance delta (not optimistic request). Confirm distributeFees and withdrawals never assume requested > balanceOf.

`DoubPresaleVesting`

contracts/src/vesting/DoubPresaleVesting.sol: funding / claim paths using ERC20 — same delta discipline for any accounting fields.

Regression / new tests (`forge`)

Area	Suggested coverage
Standard ERC20 parity	Property tests: `received == nominal` paths unchanged vs pre-refactor (`totalRaised`, fee splits).
`NonStandardERC20.t.sol`	Extend `contracts/test/NonStandardERC20.t.sol`: fee-on-transfer / rebasing stubs assert ledger == balance deltas, not nominal; assert `require(received == nominal)` (if adopted) triggers with clear selectors/revert strings.
TimeCurve invariant	`contracts/test/TimeCurveInvariant.t.sol`: ghost buy volume tracks summed deltas aligned with `totalRaised`.
Router fork / Anvil	`contracts/test/TimeCurveBuyRouter.t.sol`, fork tests — regress CL8Y surplus handling + stable refunds.
Treasury / registry	Add focused tests for `RabbitTreasury` deposit deltas; `ReferralRegistry` burn measurably matches `registrationBurnAmount` only for standard tokens.

Files likely to touch

Solidity

Tests

contracts/test/NonStandardERC20.t.sol
contracts/test/TimeCurveInvariant.t.sol
contracts/test/TimeCurve.t.sol and related (WarBow / Fork suites as impacted)
New small helper internal pure/view wrappers only if duplication is high (keep diff focused).

Docs

docs/onchain/security-and-threat-model.md — reconcile “Implementation notes” with delta-first wording and reiterated standard-ERC20-only deployment covenant.
docs/testing/invariants-and-business-logic.md — update invariant text: totalRaised tracks sum of received deltas under standard ERC20 (equals nominal); link tests.
Product/rail docs as relevant: docs/product/primitives.md, docs/product/referrals.md, docs/product/rabbit-treasury.md, docs/integrations/kumbaya.md — single paragraph each: accepted assets must be vanilla ERC20; delta accounting ≠ multi-asset carte blanche.

Indexer / frontend (conditional)

If event field semantics or names change (Buy, ReferralApplied, BuyViaKumbaya): indexer/, frontend/src ABI bindings and aggregates.
If only runtime values converge to same numbers on canonical tokens → likely no schema change.

Acceptance criteria

Every production ERC20 ingress into TimeCurve, RabbitTreasury, ReferralRegistry, DoubPresaleVesting, TimeCurveBuyRouter (ingress legs), FeeRouter/sinks callers reviewed; those that update += ledger state derive from measured deltas, not uninstrumented amount parameters.
Fork/invariant regressions: forge test green; TimeCurveInvariant handler ghost vs totalRaised coherent under fuzz (standard mock ERC20 unchanged numerically vs baseline).
NonStandardERC20 mocks exercise either corrected accounting or explicit require(received == nominal) failure — tests document which policy each path chooses.
Docs: clear deployment covenant (“standard ERC20 only — no FoT/rebasing/777/pausable/third-party wrappers unless explicitly waived”) and clarify why balance deltas exist (ledger–balanceOf parity, not generalized multi-asset).
No silent semantic drift on canonical deployments: numerical outcomes for mocks with 18-dec standard ERC20 match pre-change snapshots (golden assertions or pinned expected values in touched tests).

Verification checklist (implementation)

git diff sanity: only erc20 bookkeeping + tests + documented surfaces; no unrelated refactors.
forge fmt on touched Solidity.
forge test full contracts suite (or document CI-equivalent subset if partial locally).
Grep residual pattern: totalRaised += amount without preceding delta — must be justified (amount == received by construction immediately above).
NatSpec / dev comments above acceptedAsset and reserveAsset setters emphasize canonical ERC20 expectations.
If events change ABI: regenerate Rust/TS ABI artifacts per repo convention and run cargo test / npm run typecheck as applicable.

Verification checklist (reviewer QA)

Read updated security/threat-model subsection — covenant is unambiguous for operators.
Spot-check TimeCurve._buy trace: balanceOf[this] delta feeds distributeFees + totalRaised + events.
Spot-check RabbitTreasury deposits: totalReserves vs reserveAsset.balanceOf(this) after synthetic deposit sequences.
Confirm BuyViaKumbaya analytics still coherent: grossCl8y meaning documented (priced vs measured if both appear).
Confirm reject-list narrative (FoT/rebasing) still prominent — delta math does not read as feature support.

Out of scope (explicit)

General multi-asset safe composition (allowlist adapters per asset) — future work referenced in audit only.
Replacing SafeERC20 or supporting FoT‑native treasury logic without governance decision.

## Source Tracked from internal smart-contract audit excerpt **L-05** in [`audits/audit_smartcontract_1777813071.md`](/PlasticDigits/yieldomega/-/blob/main/audits/audit_smartcontract_1777813071.md): > Standard-ERC20 assumptions are security-critical — contracts use `SafeERC20`, but accounting generally uses **requested amounts** rather than **received balance deltas**. Deployment invariant must remain “standard ERC20 only”; accounting should match tokens actually credited. --- ## Problem statement `SafeERC20` correctly handles missing/false **`bool` returns** but does **not** make **`transferFrom`** move exactly the caller-supplied **`amount`** in all real ERC20 deployments. Categories of tokens that violate the “canonical ERC20 accounting model” include **fee-on-transfer**, **rebasing**, **rebate-on-transfer hooks** (ERC-777-adjacent), **pausable/blacklist**, **upgradeable/token-with-fees** wrappers, etc. Across core contracts, **`totalRaised`**, treasury **`totalReserves`**, routed fee splits, burns, vesting allocations, events, and invariants implicitly assume **`balance change == requestedAmount`**. That assumption is accurate **only** for **standard** ERC-20 implementations (constant balance on transfer excluding mint/burn semantics the token documents). For other assets it becomes a **silent or revert-prone correctness bug**: internal ledgers drift from **`balanceOf`**, permissionless routers can fail mid-flow, payouts can under-transfer, indexer “volume” ghosts misalign with treasury reality, etc. Operational docs already warn operators; audit L-05 calls out this is **policy + implementation smell**, not purely documentation. --- ## Goals (dual requirement) 1. **Onchain bookkeeping:** Prefer **balance deltas** (before/after `balanceOf` snapshots around each pull/push cohort) wherever value enters or leaves protocol-controlled ERC20 custody, rather than trusting nominated request sizes for **`+=` ledger updates**, **`distributeFees` arguments**, burns, emitted economic fields, etc. For **canonical standard ERC20**, **`received == requested`** anyway; deltas add **correctness parity** between storage and **`balanceOf`**. 2. **Documentation & deployment posture:** Strengthen explicit, discoverable docs that **`acceptedAsset` / CL8Y / reserve / vested funding tokens are expected to be standard ERC20.** Balance-delta accounting is **not** consent to ship fee-on-transfer or rebasing “supported assets”: it aligns ledgers when possible and makes violations **observeable/testable**. --- ## Recommended implementation approach ### Shared pattern For each **`safeTransferFrom(payer, this, nominal)`**: 1. **`uint256 b0 = token.balanceOf(address(this));`** 2. **`token.safeTransferFrom(payer, address(this), nominal);`** 3. **`uint256 received = token.balanceOf(address(this)) - b0;`** 4. Use **`received`** (not **`nominal`**) for: - Stateful counters (**`totalRaised`**, **`totalReserves`**, etc.). - Downstream **`safeTransfer`** / **`distributeFees`** / vesting allocations driven by “what actually arrived”. - Event payloads intended to mirror **economically realized** ingress/egress (**unless** intentionally documenting “priced quote”; see below). **Edge cases:** - **`received == 0`:** generally **`revert`** (no-op steals gas / breaks invariants). - **Strict parity (optional complementary guardrail):** if product requires **pricing math ingress == physical ingress**, add **`require(received == nominal, "...")`** *after* snapshot — standard tokens pass; malformed tokens fail **early and explicitly**. Decide per path whether economic rule is **`received` must equal priced `amount`** (preserves participant-facing CHARM/price linkage) vs **best-effort `received`** (usually wrong for auctions with fixed **`charmWad`**). Default recommendation for **`TimeCurve._buy`**:** **`require(received == amount)` + ledger from `received`** so CHARM sizing stays coherent *and* storage uses delta **by construction** even if someone later tweaks code paths. ### `TimeCurve` (priority) [`contracts/src/TimeCurve.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/TimeCurve.sol): **`_buy`** today does **`acceptedAsset.safeTransferFrom(..., amount)`** then **`totalRaised += amount`** and **`feeRouter.distributeFees(acceptedAsset, amount)`**. - Migrate to **`received`** from **`acceptedAsset`** balance delta on **`address(this)`** for **`distributeFees`**, **`totalRaised`**, and **`Buy`** / **`ReferralApplied`** payloads (use **`received`** everywhere the audit calls “accounting”; align NatSpec/event comments). - Apply the same discipline to **`warbowSteal`**, **`warbowRevenge`**, **`warbowGuard`** multi-pull paths ( **`totalBurn`** accounting vs actual aggregate delta before burn sink transfer). - Any other **`acceptedAsset.safeTransferFrom`** ingress in this file — same checklist. ### `TimeCurveBuyRouter` [`contracts/src/TimeCurveBuyRouter.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/TimeCurveBuyRouter.sol) already computes **`cl8yGain`** from CL8Y **balance delta** post-swap. Extend **consistency**: - Stable path: **`amountInMaximum`** pull should record **received** delta on **`address(this)`** (not **`amountInMaximum`**) before approving router; refunds already use leftovers — verify no ledger uses **`amountInMaximum`** as “spent”. - Document that **`grossCl8y`** emitted is still priced target; **`buyFor`** ultimately depends on **`TimeCurve`** ingress delta after approval. ### `RabbitTreasury` [`contracts/src/RabbitTreasury.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/RabbitTreasury.sol): deposits that **`safeTransferFrom`** then mutate **`totalReserves`** — switch reserves bookkeeping to **`received`** deltas (+ optional **`require(received == amount)`**). ### `ReferralRegistry` [`contracts/src/ReferralRegistry.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/ReferralRegistry.sol): burn uses **`transferFrom` → dead address**. Measure **delta at `BURN_ADDRESS`** or **delta leaving caller** consistently; emit/docs should match **actually burned**. ### `FeeRouter`, `FeeSink`, `PodiumPool` [`contracts/src/FeeRouter.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/FeeRouter.sol), [`contracts/src/sinks/FeeSink.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/sinks/FeeSink.sol), [`contracts/src/sinks/PodiumPool.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/sinks/PodiumPool.sol): callers must pass **`exact spendable balance delta`** (not optimistic request). Confirm **`distributeFees`** and withdrawals never assume **`requested > balanceOf`**. ### `DoubPresaleVesting` [`contracts/src/vesting/DoubPresaleVesting.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/vesting/DoubPresaleVesting.sol): funding / claim paths using ERC20 — same delta discipline for any accounting fields. --- ## Regression / new tests (`forge`) | Area | Suggested coverage | |------|---------------------| | **Standard ERC20 parity** | Property tests: **`received == nominal`** paths unchanged vs pre-refactor (**`totalRaised`**, fee splits). | | **`NonStandardERC20.t.sol`** | Extend [`contracts/test/NonStandardERC20.t.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/test/NonStandardERC20.t.sol): fee-on-transfer / rebasing stubs assert **ledger == balance deltas**, not nominal; assert **`require(received == nominal)`** (if adopted) triggers with clear selectors/revert strings. | | **TimeCurve invariant** | [`contracts/test/TimeCurveInvariant.t.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/test/TimeCurveInvariant.t.sol): ghost buy volume tracks **summed deltas** aligned with **`totalRaised`**. | | **Router fork / Anvil** | [`contracts/test/TimeCurveBuyRouter.t.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/test/TimeCurveBuyRouter.t.sol), fork tests — regress CL8Y surplus handling + stable refunds. | | **Treasury / registry** | Add focused tests for **`RabbitTreasury`** deposit deltas; **`ReferralRegistry`** burn measurably matches **`registrationBurnAmount`** only for standard tokens. | --- ## Files likely to touch **Solidity** - [`contracts/src/TimeCurve.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/TimeCurve.sol) - [`contracts/src/TimeCurveBuyRouter.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/TimeCurveBuyRouter.sol) - [`contracts/src/RabbitTreasury.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/RabbitTreasury.sol) - [`contracts/src/ReferralRegistry.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/ReferralRegistry.sol) - [`contracts/src/FeeRouter.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/FeeRouter.sol) - [`contracts/src/sinks/FeeSink.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/sinks/FeeSink.sol) - [`contracts/src/sinks/PodiumPool.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/sinks/PodiumPool.sol) - [`contracts/src/vesting/DoubPresaleVesting.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/src/vesting/DoubPresaleVesting.sol) **Tests** - [`contracts/test/NonStandardERC20.t.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/test/NonStandardERC20.t.sol) - [`contracts/test/TimeCurveInvariant.t.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/test/TimeCurveInvariant.t.sol) - [`contracts/test/TimeCurve.t.sol`](/PlasticDigits/yieldomega/-/blob/main/contracts/test/TimeCurve.t.sol) and related (**`WarBow`** / **`Fork`** suites as impacted) - New small helper **`internal pure/view`** wrappers only if duplication is high (keep diff focused). **Docs** - [`docs/onchain/security-and-threat-model.md`](/PlasticDigits/yieldomega/-/blob/main/docs/onchain/security-and-threat-model.md) — reconcile “Implementation notes” with **delta-first** wording and reiterated **standard-ERC20-only** deployment covenant. - [`docs/testing/invariants-and-business-logic.md`](/PlasticDigits/yieldomega/-/blob/main/docs/testing/invariants-and-business-logic.md) — update invariant text: **`totalRaised`** tracks **sum of received deltas** under standard ERC20 (**equals** nominal); link tests. - Product/rail docs as relevant: **[`docs/product/primitives.md`](/PlasticDigits/yieldomega/-/blob/main/docs/product/primitives.md)**, **[`docs/product/referrals.md`](/PlasticDigits/yieldomega/-/blob/main/docs/product/referrals.md)**, **[`docs/product/rabbit-treasury.md`](/PlasticDigits/yieldomega/-/blob/main/docs/product/rabbit-treasury.md)**, **[`docs/integrations/kumbaya.md`](/PlasticDigits/yieldomega/-/blob/main/docs/integrations/kumbaya.md)** — single paragraph each: **accepted assets must be vanilla ERC20;** delta accounting ≠ multi-asset carte blanche. **Indexer / frontend (conditional)** - If event field **semantics** or names change (**`Buy`**, **`ReferralApplied`**, **`BuyViaKumbaya`**): [`indexer/`](/PlasticDigits/yieldomega/-/blob/main/indexer/), [`frontend/src`](/PlasticDigits/yieldomega/-/blob/main/frontend/src/) ABI bindings and aggregates. - If only **runtime values** converge to same numbers on canonical tokens → likely **no schema change**. --- ## Acceptance criteria 1. **Every production ERC20 ingress** into **`TimeCurve`**, **`RabbitTreasury`**, **`ReferralRegistry`**, **`DoubPresaleVesting`**, **`TimeCurveBuyRouter`** (ingress legs), **`FeeRouter`/sinks callers** reviewed; those that update **`+=` ledger state** derive from **measured deltas**, not uninstrumented **`amount` parameters**. 2. **Fork/invariant regressions**: **`forge test`** green; **`TimeCurveInvariant`** handler ghost vs **`totalRaised`** coherent under fuzz (standard mock ERC20 unchanged numerically vs baseline). 3. **`NonStandardERC20`** mocks exercise **either** corrected accounting **or** explicit **`require(received == nominal)`** failure — tests document which policy each path chooses. 4. **Docs**: clear **deployment covenant** (“**standard ERC20 only** — no FoT/rebasing/777/pausable/third-party wrappers unless explicitly waived”) and clarify **why balance deltas exist** (**ledger–`balanceOf` parity**, not generalized multi-asset). 5. **No silent semantic drift** on canonical deployments: numerical outcomes for mocks with **18-dec standard ERC20** match **pre-change** snapshots (golden assertions or pinned expected values in touched tests). --- ## Verification checklist (implementation) 1. **`git diff` sanity:** only erc20 bookkeeping + tests + documented surfaces; no unrelated refactors. 2. **`forge fmt`** on touched Solidity. 3. **`forge test`** full contracts suite (or document CI-equivalent subset if partial locally). 4. Grep residual pattern: **`totalRaised += amount`** without preceding delta — **must be justified** (`amount` == `received` by construction immediately above). 5. NatSpec / dev comments above **`acceptedAsset`** and **`reserveAsset`** setters emphasize **canonical ERC20** expectations. 6. If events change ABI: regenerate Rust/TS ABI artifacts per repo convention and run **`cargo test`** / **`npm run typecheck`** as applicable. --- ## Verification checklist (reviewer QA) 1. Read updated **security/threat-model** subsection — covenant is unambiguous for operators. 2. Spot-check **`TimeCurve._buy`** trace: **`balanceOf[this]` delta** feeds **`distributeFees`** + **`totalRaised`** + events. 3. Spot-check **`RabbitTreasury`** deposits: **`totalReserves`** vs **`reserveAsset.balanceOf(this)`** after synthetic deposit sequences. 4. Confirm **`BuyViaKumbaya`** analytics still coherent: **`grossCl8y`** meaning documented (priced vs measured if both appear). 5. Confirm **reject-list** narrative (FoT/rebasing) still prominent — **delta math does not read as feature support.** --- ## Out of scope (explicit) - **General multi-asset safe composition** (`allowlist adapters per asset`) — future work referenced in audit **only**. - Replacing **`SafeERC20`** or supporting **FoT‑native** treasury logic without governance decision.

PlasticDigits commented

2026-05-04 05:16:47 +00:00

(Migrated from gitlab.com)

mentioned in commit 9905b25c9c

mentioned in commit 9905b25c9c11f63b798de6a4fc4ab3127fbd8a9e

PlasticDigits commented

2026-05-04 05:38:31 +00:00

(Migrated from gitlab.com)

mentioned in commit ebbf58ed93

mentioned in commit ebbf58ed9379559e94cc4855ca2e2b343ae18c70

PlasticDigits commented

2026-05-04 05:53:54 +00:00

(Migrated from gitlab.com)

mentioned in commit a574c6337a

mentioned in commit a574c6337a2276a816e30d8d7c28e0d93bb13c39

PlasticDigits commented

2026-05-04 05:53:58 +00:00

(Migrated from gitlab.com)

mentioned in issue #122

Brouie commented

2026-05-04 06:08:20 +00:00

(Migrated from gitlab.com)

mentioned in issue #117

Brouie commented

2026-05-04 06:35:33 +00:00

(Migrated from gitlab.com)

verified on commit a574c63 (the ingress parity work you mentioned on #122). all 4 fee-on-transfer parity tests now pass clean — these were the failures I'd flagged earlier on #117.

ingress balance-delta enforcement (your AC):

test_feeOnTransfer_rabbitTreasury_deposit_reverts_erc20Parity — RabbitTreasury.deposit reverts TimeCurve: ERC20 parity (or analogue) on FoT
test_feeOnTransfer_rabbitTreasury_receiveFee_reverts_erc20Parity — receiveFee path same
test_feeOnTransfer_referralRegistry_register_reverts_erc20Parity — registry burn path same
test_feeOnTransfer_timeCurve_buyReverts_erc20Parity — TimeCurve.buy reverts with the new parity message instead of opaque OZ ERC20InsufficientBalance

existing non-standard token coverage intact:

test_alwaysRevert_feeRouter_distributeReverts — pre-existing always-revert behavior preserved
test_alwaysRevert_rabbitTreasury_depositReverts
test_blockedSink_feeRouter_distributeReverts
test_rebasing_treasury_balanceCanDesyncFromTotalReserves — rebasing desync warning preserved

totalRaised + FeeRouter.distributeFees now use the measured segment per your spec. accidental FoT/rebasing token deployment now fails at the ingress check rather than producing silently wrong accounting downstream. complements #122's allowlist + governed rescue.

verified on commit `a574c63` (the ingress parity work you mentioned on #122). all 4 fee-on-transfer parity tests now pass clean — these were the failures I'd flagged earlier on #117. **ingress balance-delta enforcement (your AC):** - `test_feeOnTransfer_rabbitTreasury_deposit_reverts_erc20Parity` — RabbitTreasury.deposit reverts `TimeCurve: ERC20 parity` (or analogue) on FoT - `test_feeOnTransfer_rabbitTreasury_receiveFee_reverts_erc20Parity` — receiveFee path same - `test_feeOnTransfer_referralRegistry_register_reverts_erc20Parity` — registry burn path same - `test_feeOnTransfer_timeCurve_buyReverts_erc20Parity` — TimeCurve.buy reverts with the new parity message instead of opaque OZ `ERC20InsufficientBalance` **existing non-standard token coverage intact:** - `test_alwaysRevert_feeRouter_distributeReverts` — pre-existing always-revert behavior preserved - `test_alwaysRevert_rabbitTreasury_depositReverts` - `test_blockedSink_feeRouter_distributeReverts` - `test_rebasing_treasury_balanceCanDesyncFromTotalReserves` — rebasing desync warning preserved **totalRaised + FeeRouter.distributeFees** now use the measured segment per your spec. accidental FoT/rebasing token deployment now fails at the ingress check rather than producing silently wrong accounting downstream. complements #122's allowlist + governed rescue.

Brouie commented

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

(Migrated from gitlab.com)

mentioned in issue #124

PlasticDigits commented

2026-05-04 08:03:31 +00:00

(Migrated from gitlab.com)

mentioned in commit 53dab468ff

mentioned in commit 53dab468ffd7003aebb0d24426f14f2548cbf453

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-05-04 08:19:31 +00:00

Brouie commented

2026-05-04 08:28:14 +00:00

(Migrated from gitlab.com)

@PlasticDigits — testFuzz_warbow_steal_burn_exact_to_sink still fails after 53dab46. counterexample: args=[21118, 57005]. second arg decodes to 0x000000000000000000000000000000000000dEaD = BURN_SINK.

walks through:

_pullAcceptedExact(BURN_SINK, 1e18) → transferFrom(BURN_SINK, tc, 1e18) ← BURN_SINK was pre-funded by test setup
safeTransfer(BURN_SINK, 1e18) → tc returns 1e18 to BURN_SINK
net BURN_SINK balance change: 0
assertion balanceOf(BURN_SINK) - sinkBefore == 1e18 fails because actual delta is 0

your vm.assume(... != address(tc)) patch caught the address(tc)==payer round-trip but not the BURN_SINK==msg.sender round-trip. they're the same shape — _pullAcceptedExact pulls into tc, then tc transfers back out to BURN_SINK, net zero on the assertion target.

two paths:

test fix (small): add vm.assume(victim != BURN_SINK && stealer != BURN_SINK) alongside the existing address(tc) checks in the steal test (and also revenge and guard tests for consistency, though those happen to pass currently).
contract fix (defensive): add require(msg.sender != BURN_SINK) in warbowSteal/warbowRevenge/warbowActivateGuard. closes the no-op-burn edge for any actor that controls 0xdead (theoretical only, no real wallet does).

option 1 is the immediate CI green. option 2 is the defense-in-depth. up to you.

@PlasticDigits — `testFuzz_warbow_steal_burn_exact_to_sink` still fails after `53dab46`. counterexample: `args=[21118, 57005]`. second arg decodes to `0x000000000000000000000000000000000000dEaD` = BURN_SINK. walks through: 1. `_pullAcceptedExact(BURN_SINK, 1e18)` → `transferFrom(BURN_SINK, tc, 1e18)` ← BURN_SINK was pre-funded by test setup 2. `safeTransfer(BURN_SINK, 1e18)` → tc returns 1e18 to BURN_SINK 3. net BURN_SINK balance change: 0 4. assertion `balanceOf(BURN_SINK) - sinkBefore == 1e18` fails because actual delta is 0 your `vm.assume(... != address(tc))` patch caught the address(tc)==payer round-trip but not the BURN_SINK==msg.sender round-trip. they're the same shape — `_pullAcceptedExact` pulls into tc, then tc transfers back out to BURN_SINK, net zero on the assertion target. two paths: 1. **test fix (small)**: add `vm.assume(victim != BURN_SINK && stealer != BURN_SINK)` alongside the existing address(tc) checks in the steal test (and also revenge and guard tests for consistency, though those happen to pass currently). 2. **contract fix (defensive)**: add `require(msg.sender != BURN_SINK)` in `warbowSteal`/`warbowRevenge`/`warbowActivateGuard`. closes the no-op-burn edge for any actor that controls 0xdead (theoretical only, no real wallet does). option 1 is the immediate CI green. option 2 is the defense-in-depth. up to you.

PlasticDigits (Migrated from gitlab.com) reopened this issue

2026-05-04 13:13:52 +00:00

PlasticDigits commented

2026-05-04 13:14:08 +00:00

(Migrated from gitlab.com)

Option 2 is the correct choice

PlasticDigits commented

2026-05-04 13:45:45 +00:00

(Migrated from gitlab.com)

mentioned in commit daed936003

mentioned in commit daed93600365869558c98c239051cc146881b50d

PlasticDigits commented

2026-05-04 13:45:45 +00:00

(Migrated from gitlab.com)

mentioned in commit a4d2a50ccb

mentioned in commit a4d2a50ccb65cecdbb8e313bbe4ad28a7dae9b3f

PlasticDigits commented

2026-05-04 13:48:06 +00:00

(Migrated from gitlab.com)

Fix merged (option 2 — onchain guard, GitLab #123)

Pushed to main as merge commit daed936.

What changed

TimeCurve: added _requireWarbowParticipantCaller() — require(msg.sender != BURN_SINK, "TimeCurve: burn sink caller") — and call it from warbowSteal, warbowRevenge, and warbowActivateGuard (after sale-interaction gates). Closes the #123 / balance-delta _pullAcceptedExact round-trip where msg.sender == 0x…dEaD could net zero CL8Y into the burn sink while WarBow BP logic could still move.
Tests: TimeCurveWarBowCl8yBurns.t.sol — test_warbow_steal_reverts_when_caller_is_burn_sink, test_warbow_revenge_reverts_when_caller_is_burn_sink, test_warbow_guard_reverts_when_caller_is_burn_sink; fuzz cases also assume away BURN_SINK for the sink-delta invariants.
Docs / agent cross-links: INV-WARBOW-123-BURN-CALLER in docs/testing/invariants-and-business-logic.md, row in docs/onchain/security-and-threat-model.md, skills/README.md, skills/play-timecurve-warbow/SKILL.md, contributor .cursor/skills/yieldomega-guardrails/SKILL.md item #41.

Verification checklist for @brouie

git pull and confirm daed936 on main; git show --stat daed936.
cd contracts && forge test --match-path test/TimeCurveWarBowCl8yBurns.t.sol -vvv — 6 tests green (3 fuzz + 3 explicit revert).
forge test full suite locally or CI green on the pushed commit.
Read-through _requireWarbowParticipantCaller call sites vs BURN_SINK relay in contracts/src/TimeCurve.sol; confirm no other CL8Y WarBow entrypoint bypasses this pattern.
Skim invariant anchor #inv-warbow-123-burn-caller still reads as defense-in-depth (not “support FoT tokens”).

Leaving this issue open for your sign-off.

## Fix merged (option 2 — onchain guard, [GitLab #123](https://gitlab.com/PlasticDigits/yieldomega/-/issues/123)) Pushed to `main` as merge commit **`daed936`**. ### What changed - **`TimeCurve`:** added `_requireWarbowParticipantCaller()` — `require(msg.sender != BURN_SINK, "TimeCurve: burn sink caller")` — and call it from `warbowSteal`, `warbowRevenge`, and `warbowActivateGuard` (after sale-interaction gates). Closes the **`#123` / balance-delta `_pullAcceptedExact`** round-trip where `msg.sender == 0x…dEaD` could net **zero** CL8Y into the burn sink while WarBow BP logic could still move. - **Tests:** `TimeCurveWarBowCl8yBurns.t.sol` — `test_warbow_steal_reverts_when_caller_is_burn_sink`, `test_warbow_revenge_reverts_when_caller_is_burn_sink`, `test_warbow_guard_reverts_when_caller_is_burn_sink`; fuzz cases also `assume` away `BURN_SINK` for the sink-delta invariants. - **Docs / agent cross-links:** `INV-WARBOW-123-BURN-CALLER` in [`docs/testing/invariants-and-business-logic.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/docs/testing/invariants-and-business-logic.md), row in [`docs/onchain/security-and-threat-model.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/docs/onchain/security-and-threat-model.md), [`skills/README.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/skills/README.md), [`skills/play-timecurve-warbow/SKILL.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/skills/play-timecurve-warbow/SKILL.md), contributor [`.cursor/skills/yieldomega-guardrails/SKILL.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/.cursor/skills/yieldomega-guardrails/SKILL.md) item **#41**. ### Verification checklist for @brouie 1. `git pull` and confirm `daed936` on `main`; `git show --stat daed936`. 2. `cd contracts && forge test --match-path test/TimeCurveWarBowCl8yBurns.t.sol -vvv` — **6** tests green (3 fuzz + 3 explicit revert). 3. `forge test` full suite locally or CI green on the pushed commit. 4. Read-through `_requireWarbowParticipantCaller` call sites vs `BURN_SINK` relay in [`contracts/src/TimeCurve.sol`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/contracts/src/TimeCurve.sol); confirm **no** other CL8Y WarBow entrypoint bypasses this pattern. 5. Skim invariant anchor [`#inv-warbow-123-burn-caller`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/docs/testing/invariants-and-business-logic.md) still reads as **defense-in-depth** (not “support FoT tokens”). Leaving this issue **open** for your sign-off.

Brouie commented

2026-05-05 01:48:43 +00:00

(Migrated from gitlab.com)

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

pulled, daed936 on main. git show --stat daed936 confirms.
forge test --match-path test/TimeCurveWarBowCl8yBurns.t.sol -vvv green. testFuzz_warbow_steal_burn_exact_to_sink clean at 256 runs after the BURN_SINK assume — yesterday's red is now green.
full forge suite green locally on a421ba1 (HEAD post-merge).
_requireWarbowParticipantCaller call sites read clean — warbowSteal (L624), warbowRevenge (L695), warbowActivateGuard (L724). claimWarBowFlag correctly skipped (no _pullAcceptedExact call, just flag plant/claim, no CL8Y movement). no other WarBow entrypoint pulls CL8Y.
#inv-warbow-123-burn-caller reads as defense-in-depth — frames the round-trip parity as a WarBow-state-mutation gap, not asset support.
option 2 was the right call. test-side vm.assume would have left a real ingress shape for any future actor that controls 0xdead.
ready for close on your side.

@PlasticDigits — verification PASS on `daed936`. walked your checklist: 1. pulled, `daed936` on main. `git show --stat daed936` confirms. 2. `forge test --match-path test/TimeCurveWarBowCl8yBurns.t.sol -vvv` green. `testFuzz_warbow_steal_burn_exact_to_sink` clean at 256 runs after the BURN_SINK assume — yesterday's red is now green. 3. full forge suite green locally on `a421ba1` (HEAD post-merge). 4. `_requireWarbowParticipantCaller` call sites read clean — `warbowSteal` (L624), `warbowRevenge` (L695), `warbowActivateGuard` (L724). `claimWarBowFlag` correctly skipped (no `_pullAcceptedExact` call, just flag plant/claim, no CL8Y movement). no other WarBow entrypoint pulls CL8Y. 5. `#inv-warbow-123-burn-caller` reads as defense-in-depth — frames the round-trip parity as a WarBow-state-mutation gap, not asset support. option 2 was the right call. test-side `vm.assume` would have left a real ingress shape for any future actor that controls 0xdead. ready for close on your side.

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-05-05 02:31:43 +00:00

PlasticDigits commented

2026-05-13 16:07:18 +00:00

(Migrated from gitlab.com)