L-01: TimeCurveBuyRouter — fail fast when sale scheduled but not yet live (audit_smartcontract_1777813071) #118

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

Source

Internal smart contract audit finding L-01 in audits/audit_smartcontract_1777813071.md (2026-05-03).

Summary

TimeCurveBuyRouter.buyViaKumbaya validates that the TimeCurve sale is “scheduled” (saleStart() != 0), not ended, and before deadline(), but it does not enforce that the sale is actually live (block.timestamp >= saleStart()). With startSaleAt(epoch) (scheduled sale start — GitLab #114), epoch can be in the future. The router may still run path validation, WETH wrap / stable pull, kumbayaRouter.exactOutput, and refunds before TimeCurve.buyFor reverts with "TimeCurve: sale not live".

Impact

  • No net loss of user funds in the normal case: the failing buyFor reverts and the whole transaction rolls back (including the swap and refunds).
  • Wasted gas and unnecessary external calls (Kumbaya router) when a user or buggy client submits buyViaKumbaya before saleStart.
  • Inconsistent fail-fast behavior vs TimeCurve.buy / buy, which already require block.timestamp >= saleStart early.

Severity aligns with audit Low: operational / UX / gas, not direct theft.

Current behavior (evidence)

In contracts/src/TimeCurveBuyRouter.sol, buyViaKumbaya gates sale phase as:

if (tc.saleStart() == 0 || tc.ended() || block.timestamp >= tc.deadline()) revert TimeCurveBuyRouter__BadSalePhase();

There is no block.timestamp >= tc.saleStart() check. TimeCurve enforces live window on buys, e.g. require(block.timestamp >= saleStart, "TimeCurve: sale not live"); in _buy.

Reference direct-buy regression: test_startSaleAt_future_epoch_buys_revert_until_live in contracts/test/TimeCurve.t.sol (GitLab #114).

  1. Onchain (primary): Extend the router’s sale-phase guard so pre-open scheduled sales revert before any swap:

    • After confirming saleStart() > 0, add block.timestamp < tc.saleStart() to the revert condition (same predicate family as TimeCurve), reusing TimeCurveBuyRouter__BadSalePhase so selectors and UX remain stable — or introduce a dedicated error (e.g. TimeCurveBuyRouter__SaleNotLive) only if product wants distinct decoding.

    Suggested combined guard (conceptual):

    • Revert BadSalePhase when: saleStart == 0, ended, block.timestamp < saleStart, or block.timestamp >= deadline.
  2. NatSpec: Update the buyViaKumbaya / contract dev notes to state the router fails closed for not-yet-live scheduled sales at entry (not only at TimeCurve).

  3. Documentation: Briefly note in Kumbaya / buy-router docs that buyViaKumbaya aligns with TimeCurve live-window rules for fail-fast gas behavior.

  1. Unit / integration (Forge) — add a test in contracts/test/TimeCurveBuyRouter.t.sol (or a focused test file) that:

    • Deploys the same stack as existing router tests but calls TimeCurve.startSaleAt(block.timestamp + delta) with delta > 0 (sale scheduled, not live).
    • Pranks a funded buyer with approved USDM / ETH as needed.
    • vm.expectRevert(TimeCurveBuyRouter.TimeCurveBuyRouter__BadSalePhase.selector) (or the chosen error) on buyViaKumbaya without expecting "TimeCurve: sale not live" — proving the router bails before buyFor.
    • vm.warp(saleStart) and assert a successful buyViaKumbaya credits CHARM (mirror success path of test_buyViaKumbaya_stable_creditsAlice / ETH variant).
  2. Optional fork / Anvil script path: If contracts/test/VerifyTimeCurveBuyRouterAnvil.t.sol is extended, ensure SKIP_ANVIL_RICH_STATE / timing assumptions still leave block.timestamp >= saleStart where success is intended; add a negative case only if the harness can warp cheaply without breaking other assumptions.

  3. CI: cd contracts && FOUNDRY_PROFILE=ci forge test (per docs/testing/strategy.md).

Files likely to change

Area Path Change
Contract contracts/src/TimeCurveBuyRouter.sol Add block.timestamp < saleStart() (or equivalent) to sale-phase check; optional NatSpec.
Tests contracts/test/TimeCurveBuyRouter.t.sol New test: future saleStart → early revert; warp → success.
Docs docs/integrations/kumbaya.md Mention router fail-fast for pre-open sales (single-tx path).
Test map docs/testing/invariants-and-business-logic.md Short bullet under buyFor / companion router or #65/#66 linking this fix + test name / invariant id.
Optional audits/audit_smartcontract_1777813071.md Add “Addressed in GitLab #…” under L-01 when merged (use this issue number).
Frontend Usually no change required: pre-open UX should already block buys; this hardens the onchain path for power users / integrators. If any code decodes BadSalePhase only, confirm messaging still makes sense.

Out of scope unless regression appears: indexer (BuyViaKumbaya schema unchanged), bots.


Acceptance criteria

  • buyViaKumbaya reverts with the documented router error when saleStart > 0, !ended, block.timestamp < saleStart, and block.timestamp < deadline (i.e. scheduled but not yet live). No exactOutput call is required to observe the revert (gas stays bounded vs current “swap then revert” path).
  • buyViaKumbaya still succeeds for block.timestamp >= saleStart under existing test conditions (stable + ETH paths unchanged).
  • Forge regression test covers future saleStart + warp-to-success.
  • NatSpec (and at least one Kumbaya doc touchpoint) describes the live-window check on the router.
  • docs/testing/invariants-and-business-logic.md (or equivalent test map) references the new coverage.

Verification checklist — implementer

  1. Read TimeCurve._buy / buyFor sale live checks for parity.
  2. Implement guard in TimeCurveBuyRouter.buyViaKumbaya immediately after loading TimeCurve tc (before currentCharmBoundsWad / pricing / path / swap).
  3. Run forge test --match-contract TimeCurveBuyRouter then full forge test in contracts/.
  4. Run forge coverage or CI profile if required by team for router edits.

Verification checklist — reviewer

  1. Confirm revert path does not touch kumbayaRouter when block.timestamp < saleStart (static read of ordering in diff).
  2. Confirm no loosening of deadline or ended semantics.
  3. Confirm new test fails on main without fix (optional bisect) and passes with fix.
  4. Skim docs/integrations/kumbaya.md for consistency with TimeCurve “sale not live” language.

Verification checklist — release / ops

  1. No migration or proxy upgrade strictly required if deploying new TimeCurveBuyRouter bytecode; if router is immutable per deployment, document behavior change for integrators (“pre-open buyViaKumbaya now reverts early with BadSalePhase”).
  2. Optional: note in release notes / operator runbook that estimated gas for invalid pre-open buyViaKumbaya calls drops.
## Source Internal smart contract audit finding **L-01** in [`audits/audit_smartcontract_1777813071.md`](audits/audit_smartcontract_1777813071.md) (2026-05-03). ## Summary **`TimeCurveBuyRouter.buyViaKumbaya`** validates that the TimeCurve sale is “scheduled” (`saleStart() != 0`), not ended, and before `deadline()`, but it does **not** enforce that the sale is **actually live** (`block.timestamp >= saleStart()`). With **`startSaleAt(epoch)`** ([scheduled sale start — GitLab #114](https://gitlab.com/PlasticDigits/yieldomega/-/issues/114)), `epoch` can be in the future. The router may still run **path validation**, **WETH wrap / stable pull**, **`kumbayaRouter.exactOutput`**, and refunds before **`TimeCurve.buyFor`** reverts with **`"TimeCurve: sale not live"`**. ## Impact - **No net loss of user funds** in the normal case: the failing `buyFor` reverts and the whole transaction rolls back (including the swap and refunds). - **Wasted gas** and **unnecessary external calls** (Kumbaya router) when a user or buggy client submits **`buyViaKumbaya`** before **`saleStart`**. - **Inconsistent fail-fast behavior** vs **`TimeCurve.buy` / `buy`**, which already require **`block.timestamp >= saleStart`** early. Severity aligns with audit **Low**: operational / UX / gas, not direct theft. ## Current behavior (evidence) In `contracts/src/TimeCurveBuyRouter.sol`, **`buyViaKumbaya`** gates sale phase as: ```solidity if (tc.saleStart() == 0 || tc.ended() || block.timestamp >= tc.deadline()) revert TimeCurveBuyRouter__BadSalePhase(); ``` There is **no** `block.timestamp >= tc.saleStart()` check. **`TimeCurve`** enforces live window on buys, e.g. **`require(block.timestamp >= saleStart, "TimeCurve: sale not live");`** in **`_buy`**. Reference direct-buy regression: **`test_startSaleAt_future_epoch_buys_revert_until_live`** in [`contracts/test/TimeCurve.t.sol`](contracts/test/TimeCurve.t.sol) (GitLab #114). ## Recommended fix 1. **Onchain (primary):** Extend the router’s sale-phase guard so pre-open scheduled sales revert **before** any swap: - After confirming **`saleStart() > 0`**, add **`block.timestamp < tc.saleStart()`** to the revert condition (same predicate family as **`TimeCurve`**), reusing **`TimeCurveBuyRouter__BadSalePhase`** so selectors and UX remain stable — or introduce a dedicated error (e.g. **`TimeCurveBuyRouter__SaleNotLive`**) only if product wants distinct decoding. Suggested combined guard (conceptual): - Revert **`BadSalePhase`** when: **`saleStart == 0`**, **`ended`**, **`block.timestamp < saleStart`**, or **`block.timestamp >= deadline`**. 2. **NatSpec:** Update the **`buyViaKumbaya`** / contract dev notes to state the router fails closed for **not-yet-live** scheduled sales at entry (not only at `TimeCurve`). 3. **Documentation:** Briefly note in Kumbaya / buy-router docs that **`buyViaKumbaya`** aligns with **`TimeCurve`** live-window rules for **fail-fast** gas behavior. ## Regression tests (recommended) 1. **Unit / integration (Forge)** — add a test in [`contracts/test/TimeCurveBuyRouter.t.sol`](contracts/test/TimeCurveBuyRouter.t.sol) (or a focused test file) that: - Deploys the same stack as existing router tests but calls **`TimeCurve.startSaleAt(block.timestamp + delta)`** with **`delta > 0`** (sale scheduled, not live). - **Pranks** a funded buyer with approved USDM / ETH as needed. - **`vm.expectRevert(TimeCurveBuyRouter.TimeCurveBuyRouter__BadSalePhase.selector)`** (or the chosen error) on **`buyViaKumbaya`** **without** expecting **`"TimeCurve: sale not live"`** — proving the router bails **before** `buyFor`. - **`vm.warp(saleStart)`** and assert a **successful** **`buyViaKumbaya`** credits CHARM (mirror success path of **`test_buyViaKumbaya_stable_creditsAlice`** / ETH variant). 2. **Optional fork / Anvil script path:** If [`contracts/test/VerifyTimeCurveBuyRouterAnvil.t.sol`](contracts/test/VerifyTimeCurveBuyRouterAnvil.t.sol) is extended, ensure **`SKIP_ANVIL_RICH_STATE`** / timing assumptions still leave **`block.timestamp >= saleStart`** where success is intended; add a **negative** case only if the harness can warp cheaply without breaking other assumptions. 3. **CI:** `cd contracts && FOUNDRY_PROFILE=ci forge test` (per [`docs/testing/strategy.md`](docs/testing/strategy.md)). ## Files likely to change | Area | Path | Change | |------|------|--------| | **Contract** | [`contracts/src/TimeCurveBuyRouter.sol`](contracts/src/TimeCurveBuyRouter.sol) | Add **`block.timestamp < saleStart()`** (or equivalent) to sale-phase check; optional NatSpec. | | **Tests** | [`contracts/test/TimeCurveBuyRouter.t.sol`](contracts/test/TimeCurveBuyRouter.t.sol) | New test: future **`saleStart`** → early revert; warp → success. | | **Docs** | [`docs/integrations/kumbaya.md`](docs/integrations/kumbaya.md) | Mention router fail-fast for pre-open sales (single-tx path). | | **Test map** | [`docs/testing/invariants-and-business-logic.md`](docs/testing/invariants-and-business-logic.md) | Short bullet under **`buyFor` / companion router** or **#65/#66** linking this fix + test name / invariant id. | | **Optional** | [`audits/audit_smartcontract_1777813071.md`](audits/audit_smartcontract_1777813071.md) | Add “Addressed in GitLab #…” under L-01 when merged (use this issue number). | | **Frontend** | Usually **no** change required: pre-open UX should already block buys; this hardens the **onchain** path for power users / integrators. If any code decodes **`BadSalePhase`** only, confirm messaging still makes sense. | **Out of scope unless regression appears:** indexer (`BuyViaKumbaya` schema unchanged), bots. --- ## Acceptance criteria - [ ] **`buyViaKumbaya`** reverts with the **documented router error** when **`saleStart > 0`**, **`!ended`**, **`block.timestamp < saleStart`**, and **`block.timestamp < deadline`** (i.e. scheduled but not yet live). No **`exactOutput`** call is required to observe the revert (gas stays bounded vs current “swap then revert” path). - [ ] **`buyViaKumbaya`** still succeeds for **`block.timestamp >= saleStart`** under existing test conditions (stable + ETH paths unchanged). - [ ] **Forge** regression test covers future **`saleStart`** + warp-to-success. - [ ] **NatSpec** (and at least one **Kumbaya** doc touchpoint) describes the live-window check on the router. - [ ] **`docs/testing/invariants-and-business-logic.md`** (or equivalent test map) references the new coverage. --- ## Verification checklist — implementer 1. Read **`TimeCurve._buy`** / **`buyFor`** sale live checks for parity. 2. Implement guard in **`TimeCurveBuyRouter.buyViaKumbaya`** immediately after loading **`TimeCurve tc`** (before **`currentCharmBoundsWad`** / pricing / path / swap). 3. Run **`forge test --match-contract TimeCurveBuyRouter`** then full **`forge test`** in **`contracts/`**. 4. Run **`forge coverage`** or CI profile if required by team for router edits. ## Verification checklist — reviewer 1. Confirm revert path does **not** touch **`kumbayaRouter`** when **`block.timestamp < saleStart`** (static read of ordering in diff). 2. Confirm **no** loosening of **`deadline`** or **`ended`** semantics. 3. Confirm new test **fails** on **main without fix** (optional bisect) and **passes** with fix. 4. Skim **`docs/integrations/kumbaya.md`** for consistency with **`TimeCurve`** “sale not live” language. ## Verification checklist — release / ops 1. No migration or proxy upgrade strictly required if deploying **new** **`TimeCurveBuyRouter`** bytecode; if router is **immutable** per deployment, document behavior change for integrators (“pre-open `buyViaKumbaya` now reverts early with `BadSalePhase`”). 2. Optional: note in release notes / operator runbook that **estimated gas** for invalid pre-open **`buyViaKumbaya`** calls drops.
PlasticDigits commented 2026-05-04 05:10:46 +00:00 (Migrated from gitlab.com)

mentioned in commit e9bbcc7181

mentioned in commit e9bbcc71814a1b1b3cfa8840aa20333782cc2673
PlasticDigits commented 2026-05-04 05:11:07 +00:00 (Migrated from gitlab.com)

Merged to main (66098c9 — merge fix/gl118-buy-router-fail-fast-sale-live). Leaving issue open for verification.

/cc @brouie

Summary

  • TimeCurveBuyRouter.buyViaKumbaya now treats block.timestamp < saleStart() (scheduled sale not yet live per startSaleAt) like TimeCurve buy: revert TimeCurveBuyRouter__BadSalePhase before exactOutput / refunds / buyFor — aligns with audit L-01 / #118.
  • Forge: TimeCurveBuyRouterScheduledNotLive118Test — pre-live BadSalePhase (stable + ETH); vm.warp(saleStart) then success. Refactored TimeCurveBuyRouterFixture so #118 tests do not inherit unrelated happy-path tests.
  • Docs: INV-BUYROUTER-118, docs/integrations/kumbaya.md, skills/README.md, audit § L-01 addressed note; guardrail §13 merged with #117 rescue/refund wording.

Verification checklist for @brouie

  1. contracts/src/TimeCurveBuyRouter.sol: sale-phase check loads saleStart once and includes block.timestamp < saleStart_ ahead of currentCharmBoundsWad / swap (ordering: no kumbayaRouter until live).
  2. forge test --match-contract 'TimeCurveBuyRouter(Test|ScheduledNotLive118Test)|VerifyTimeCurveBuyRouterAnvil' passes (13 tests including #117 router tests).
  3. BadSalePhase remains the revert for “not yet live”; selectors unchanged for clients decoding BadSalePhase only.
  4. Skim docs/integrations/kumbaya.md “scheduled sale / fail-fast” + INV-BUYROUTER-118 table matches TimeCurve sale not live semantics.

Note: local contracts/out owned by root caused Permission denied on this runner when compiling under yieldomega/contracts; CI / normal checkout should use a writable out/ — verification above ran via full compile in a clean worktree.

Merged to **main** (66098c9 — merge `fix/gl118-buy-router-fail-fast-sale-live`). Leaving issue **open** for verification. /cc @brouie ### Summary - **`TimeCurveBuyRouter.buyViaKumbaya`** now treats **`block.timestamp < saleStart()`** (scheduled sale not yet live per **`startSaleAt`**) like **`TimeCurve`** **`buy`**: revert **`TimeCurveBuyRouter__BadSalePhase`** **before** **`exactOutput`** / refunds / **`buyFor`** — aligns with audit **L-01** / [#118](https://gitlab.com/PlasticDigits/yieldomega/-/issues/118). - **Forge:** [`TimeCurveBuyRouterScheduledNotLive118Test`](contracts/test/TimeCurveBuyRouter.t.sol) — pre-live **`BadSalePhase`** (stable + ETH); **`vm.warp(saleStart)`** then success. Refactored **`TimeCurveBuyRouterFixture`** so #118 tests do not inherit unrelated happy-path tests. - **Docs:** [`INV-BUYROUTER-118`](docs/testing/invariants-and-business-logic.md#timecurve-buy-router-buyviakumbaya-sale-live-fail-fast-gitlab-118), [`docs/integrations/kumbaya.md`](docs/integrations/kumbaya.md#issue-65-single-tx-router), [`skills/README.md`](skills/README.md), audit § L-01 addressed note; guardrail **§13** merged with **#117** rescue/refund wording. ### Verification checklist for @brouie 1. **`contracts/src/TimeCurveBuyRouter.sol`**: sale-phase check loads **`saleStart`** once and includes **`block.timestamp < saleStart_`** ahead of **`currentCharmBoundsWad`** / swap (**ordering**: no **`kumbayaRouter`** until live). 2. **`forge test --match-contract 'TimeCurveBuyRouter(Test|ScheduledNotLive118Test)|VerifyTimeCurveBuyRouterAnvil'`** passes (**13 tests** including **#117** router tests). 3. **`BadSalePhase`** remains the revert for “not yet live”; selectors unchanged for clients decoding **`BadSalePhase`** only. 4. Skim **`docs/integrations/kumbaya.md`** “scheduled sale / fail-fast” + **`INV-BUYROUTER-118`** table matches **`TimeCurve`** **sale not live** semantics. **Note:** local **`contracts/out`** owned by root caused **`Permission denied`** on this runner when compiling under `yieldomega/contracts`; CI / normal checkout should use a writable **`out/`** — verification above ran via full compile in a clean worktree.
PlasticDigits commented 2026-05-04 05:16:46 +00:00 (Migrated from gitlab.com)

mentioned in commit 9905b25c9c

mentioned in commit 9905b25c9c11f63b798de6a4fc4ab3127fbd8a9e
Brouie commented 2026-05-04 06:16:23 +00:00 (Migrated from gitlab.com)

verified on commit e9bbcc7. fix landed clean.

source ordering (TimeCurveBuyRouter.sol):

  • line 110-114: saleStart_ read + 4-condition guard (saleStart == 0 || ended || block.timestamp < saleStart) → reverts TimeCurveBuyRouter__BadSalePhase
  • line 142: kumbayaRouter.exactOutput(...) swap call

guard fires before pricing or swap. spec confirmed: revert path does not touch kumbayaRouter when block.timestamp < saleStart.

forge tests all pass:

  • test_buyViaKumbaya_badSalePhase_whenScheduledNotLive_eth — pre-open ETH path reverts
  • test_buyViaKumbaya_badSalePhase_whenScheduledNotLive_stable — pre-open stable path reverts
  • test_buyViaKumbaya_eth_succeeds_afterWarpToSaleStart — warp-then-success ETH path
  • test_buyViaKumbaya_stable_succeeds_afterWarpToSaleStart — warp-then-success stable path

implementer + reviewer + release-ops checklist all pass at the source/test level.

verified on commit `e9bbcc7`. fix landed clean. **source ordering** (TimeCurveBuyRouter.sol): - line 110-114: `saleStart_` read + 4-condition guard (`saleStart == 0 || ended || block.timestamp < saleStart`) → reverts `TimeCurveBuyRouter__BadSalePhase` - line 142: `kumbayaRouter.exactOutput(...)` swap call guard fires before pricing or swap. spec confirmed: revert path does not touch kumbayaRouter when block.timestamp < saleStart. **forge tests** all pass: - `test_buyViaKumbaya_badSalePhase_whenScheduledNotLive_eth` — pre-open ETH path reverts - `test_buyViaKumbaya_badSalePhase_whenScheduledNotLive_stable` — pre-open stable path reverts - `test_buyViaKumbaya_eth_succeeds_afterWarpToSaleStart` — warp-then-success ETH path - `test_buyViaKumbaya_stable_succeeds_afterWarpToSaleStart` — warp-then-success stable path implementer + reviewer + release-ops checklist all pass at the source/test level.
PlasticDigits (Migrated from gitlab.com) closed this issue 2026-05-04 08:18:37 +00:00
PlasticDigits commented 2026-05-13 16:07:18 +00:00 (Migrated from gitlab.com)

mentioned in issue #207

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