Test gap: #63 opt-in semantics not covered for referral path or buyFor router path

Brouie commented

2026-04-27 04:05:25 +00:00

(Migrated from gitlab.com)

Found Monday 2026-04-27 during #63 source-level verification.

Summary

#63 added an opt-in plantWarBowFlag parameter to four buy entry points:

buy(charmWad) — plain (default false)
buy(charmWad, plantWarBowFlag)
buy(charmWad, codeHash, plantWarBowFlag) — referral
buyFor(buyer, charmWad, plantWarBowFlag) — router path
buyFor(buyer, charmWad, codeHash, plantWarBowFlag) — referral + router

contracts/test/TimeCurve.t.sol covers items 1, 2, and 4 (plain buy unchanged, opt-in plant works, same holder plain follow-up preserves silence). But no test asserts opt-in semantics specifically on:

The referral path (codeHash variant)
The buyFor router path (either variant)

Why it matters

The contract logic is shared via _buy / _buyForExternal so the L250/L259 tests indirectly cover the parameter wiring. But explicit assertions would close the gap for future regressions where someone refactors _buy or one of its callers and breaks one entry point's parameter passthrough silently.

Suggested tests

test_referral_buy_with_plant_false_does_not_plant — buy(charmWad, codeHash, false) → flagPlanted=false, warbowPendingFlagOwner unchanged
test_referral_buy_with_plant_true_plants — buy(charmWad, codeHash, true) → flagPlanted=true, warbowPendingFlagOwner=msg.sender
test_buy_for_with_plant_false_does_not_plant — buyFor called from a router → flagPlanted=false, warbowPendingFlagOwner unchanged (also asserts buyer attribution remains correct, since msg.sender on TimeCurve is router)
test_buy_for_with_plant_true_plants — same shape, plant=true, owner=buyer (NOT router)

Severity

LOW. Not a current bug, just a coverage gap. Worth adding pre-mainnet for regression safety.

Out of scope

#63 itself (closed), #76 browser walkthrough.

cc @PlasticDigits

Found Monday 2026-04-27 during #63 source-level verification. ## Summary #63 added an opt-in plantWarBowFlag parameter to four buy entry points: - buy(charmWad) — plain (default false) - buy(charmWad, plantWarBowFlag) - buy(charmWad, codeHash, plantWarBowFlag) — referral - buyFor(buyer, charmWad, plantWarBowFlag) — router path - buyFor(buyer, charmWad, codeHash, plantWarBowFlag) — referral + router contracts/test/TimeCurve.t.sol covers items 1, 2, and 4 (plain buy unchanged, opt-in plant works, same holder plain follow-up preserves silence). But no test asserts opt-in semantics specifically on: - The referral path (codeHash variant) - The buyFor router path (either variant) ## Why it matters The contract logic is shared via _buy / _buyForExternal so the L250/L259 tests indirectly cover the parameter wiring. But explicit assertions would close the gap for future regressions where someone refactors _buy or one of its callers and breaks one entry point's parameter passthrough silently. ## Suggested tests - test_referral_buy_with_plant_false_does_not_plant — buy(charmWad, codeHash, false) → flagPlanted=false, warbowPendingFlagOwner unchanged - test_referral_buy_with_plant_true_plants — buy(charmWad, codeHash, true) → flagPlanted=true, warbowPendingFlagOwner=msg.sender - test_buy_for_with_plant_false_does_not_plant — buyFor called from a router → flagPlanted=false, warbowPendingFlagOwner unchanged (also asserts buyer attribution remains correct, since msg.sender on TimeCurve is router) - test_buy_for_with_plant_true_plants — same shape, plant=true, owner=buyer (NOT router) ## Severity LOW. Not a current bug, just a coverage gap. Worth adding pre-mainnet for regression safety. ## Out of scope #63 itself (closed), #76 browser walkthrough. cc @PlasticDigits

Brouie commented

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

(Migrated from gitlab.com)

mentioned in issue #63

PlasticDigits commented

2026-04-27 04:19:35 +00:00

(Migrated from gitlab.com)

mentioned in commit a1f286d000

mentioned in commit a1f286d00034ed2fffefd70179ec099195514b61

PlasticDigits commented

2026-04-27 04:19:42 +00:00

(Migrated from gitlab.com)

Fixed on `main` (commit `a1f286d`)

Implemented the four suggested Forge tests from this issue so referral buy(charmWad, codeHash, plantWarBowFlag) and router buyFor(buyer, charmWad, plantWarBowFlag) cannot regress silently if _buy / callers are refactored:

Test	File
`test_referral_buy_with_plant_false_does_not_plant`	`contracts/test/TimeCurveReferral.t.sol`
`test_referral_buy_with_plant_true_plants`	same
`test_buy_for_with_plant_false_does_not_plant`	`contracts/test/TimeCurve.t.sol`
`test_buy_for_with_plant_true_plants`	same

Docs / cross-links: docs/testing/invariants-and-business-logic.md (Referral CHARM row + WarBow ladder bullet), skills/README.md, skills/play-timecurve-warbow/SKILL.md, .cursor/skills/yieldomega-guardrails/SKILL.md (item 16).

CI-style verification run: cd contracts && FOUNDRY_PROFILE=ci forge test — 196 passed.

Checklist for @brouie

Pull latest main and confirm the four tests exist and pass locally (forge test --match-test 'test_referral_buy_with_plant|test_buy_for_with_plant').
Spot-check invariants-and-business-logic.md: WarBow bullet lists all four test names; Referral row cites #77.
Optional: confirm buyFor test asserts charmWeight(router)==0 and warbowPendingFlagOwner == buyer (not router) when plant=true.
Close this issue when satisfied (left open per implementer instruction).

Issue #63 behavior unchanged — coverage only.

## Fixed on `main` (commit a1f286d) Implemented the four suggested Forge tests from this issue so **referral `buy(charmWad, codeHash, plantWarBowFlag)`** and **router `buyFor(buyer, charmWad, plantWarBowFlag)`** cannot regress silently if `_buy` / callers are refactored: | Test | File | |------|------| | `test_referral_buy_with_plant_false_does_not_plant` | `contracts/test/TimeCurveReferral.t.sol` | | `test_referral_buy_with_plant_true_plants` | same | | `test_buy_for_with_plant_false_does_not_plant` | `contracts/test/TimeCurve.t.sol` | | `test_buy_for_with_plant_true_plants` | same | **Docs / cross-links:** `docs/testing/invariants-and-business-logic.md` (Referral CHARM row + WarBow ladder bullet), `skills/README.md`, `skills/play-timecurve-warbow/SKILL.md`, `.cursor/skills/yieldomega-guardrails/SKILL.md` (item 16). **CI-style verification run:** `cd contracts && FOUNDRY_PROFILE=ci forge test` — **196 passed**. --- ### Checklist for @brouie - [ ] Pull latest `main` and confirm the four tests exist and pass locally (`forge test --match-test 'test_referral_buy_with_plant|test_buy_for_with_plant'`). - [ ] Spot-check `invariants-and-business-logic.md`: WarBow bullet lists all four test names; Referral row cites #77. - [ ] Optional: confirm `buyFor` test asserts `charmWeight(router)==0` and `warbowPendingFlagOwner == buyer` (not router) when `plant=true`. - [ ] Close this issue when satisfied (left **open** per implementer instruction). Issue #63 behavior unchanged — coverage only.

Brouie commented

2026-04-27 04:43:16 +00:00

(Migrated from gitlab.com)

@PlasticDigits verified on a1f286d.

Tests

cd contracts && FOUNDRY_PROFILE=ci forge test --match-contract 'TimeCurveTest|TimeCurveReferralTest':

80/80 passing across both suites (75 TimeCurve + 5 TimeCurveReferral)

All 4 suggested test names landed exactly

contracts/test/TimeCurve.t.sol:

L291 test_buy_for_with_plant_false_does_not_plant
L305 test_buy_for_with_plant_true_plants

contracts/test/TimeCurveReferral.t.sol:

L127 test_referral_buy_with_plant_false_does_not_plant
L139 test_referral_buy_with_plant_true_plants

Test shape spot-check (TimeCurve.t.sol L291, L305)

The buyFor tests cover the most important regression risk — buyer-vs-router attribution on the router path:

plant=false: charmWeight(alice) = 1e18 (buyer attribution), charmWeight(companion) = 0 (NOT router), pendingFlagOwner = address(0) (NOT planted)
plant=true: charmWeight(alice) = 1e18, pendingFlagOwner = alice (NOT companion), plantAt > 0

That's exactly the invariant I called out — would catch any future refactor of _buyForExternal that silently breaks pass-through OR attribution.

Closing #77.

@PlasticDigits verified on a1f286d. ## Tests cd contracts && FOUNDRY_PROFILE=ci forge test --match-contract 'TimeCurveTest|TimeCurveReferralTest': - 80/80 passing across both suites (75 TimeCurve + 5 TimeCurveReferral) ## All 4 suggested test names landed exactly contracts/test/TimeCurve.t.sol: - L291 test_buy_for_with_plant_false_does_not_plant - L305 test_buy_for_with_plant_true_plants contracts/test/TimeCurveReferral.t.sol: - L127 test_referral_buy_with_plant_false_does_not_plant - L139 test_referral_buy_with_plant_true_plants ## Test shape spot-check (TimeCurve.t.sol L291, L305) The buyFor tests cover the most important regression risk — buyer-vs-router attribution on the router path: - plant=false: charmWeight(alice) = 1e18 (buyer attribution), charmWeight(companion) = 0 (NOT router), pendingFlagOwner = address(0) (NOT planted) - plant=true: charmWeight(alice) = 1e18, pendingFlagOwner = alice (NOT companion), plantAt > 0 That's exactly the invariant I called out — would catch any future refactor of _buyForExternal that silently breaks pass-through OR attribution. Closing #77.

Brouie (Migrated from gitlab.com) closed this issue

2026-04-27 04:43:18 +00:00

Brouie commented

2026-04-27 06:43:55 +00:00

(Migrated from gitlab.com)

mentioned in issue #76

Rows
Columns

Test gap: #63 opt-in semantics not covered for referral path or buyFor router path #77

Summary

Why it matters

Suggested tests

Severity

Out of scope

Fixed on main (commit a1f286d)

Checklist for @brouie

Tests

All 4 suggested test names landed exactly

Test shape spot-check (TimeCurve.t.sol L291, L305)

Fixed on `main` (commit `a1f286d`)