TimeCurve: WarBow podium can become stale when victim BP drops below 3rd-place threshold (legitimate top-3 holders not auto-promoted)

Brouie commented

2026-05-04 07:56:01 +00:00

(Migrated from gitlab.com)

found during the post-audit security review pass (followup to #126).

Summary

_updateWarbowPodium(address candidate, uint256 candidateValue) only updates the warbow podium based on the SPECIFIC candidate it's called with. WarBow podium tracks top-3 by BP. BP can DECREASE via warbowSteal (victim loses BP).

When a current podium member loses BP and falls below an off-podium wallet's BP, the off-podium wallet is NOT auto-promoted into the podium. The podium becomes stale and stays stale until that off-podium wallet happens to call _addBattlePoints or _subBattlePoints themselves.

At sale end, distributePrizes pays from the recorded _warbowPodium, which may not match the actual top-3 BP holders.

Code

TimeCurve.sol:670, 677:

function _addBattlePoints(address a, uint256 v) internal {
    if (v == 0) return;
    battlePoints[a] += v;
    _updateWarbowPodium(a, battlePoints[a]);   // updates only `a`
}

function _subBattlePoints(address a, uint256 v) internal {
    if (v == 0) return;
    uint256 b = battlePoints[a];
    battlePoints[a] = b > v ? b - v : 0;
    _updateWarbowPodium(a, battlePoints[a]);   // updates only `a`
}

TimeCurve.sol:881-915 (_updateWarbowPodium body):

The function's logic:

If candidate is already in podium → update its value, re-sort, zero out empty slots
If candidate is NOT in podium AND candidateValue > 0 → try to bump the lowest podium member if candidate beats them

But it never iterates other wallets to check if THEY should now occupy the slot vacated by the dropping podium member.

Concrete scenario

Initial state: BP rankings

alice: 10000 BP (podium #1)
bob: 8000 BP (podium #2)
carol: 6000 BP (podium #3)
dave: 5000 BP (off-podium)

Now bob gets stolen from heavily, his BP drops to 4000 via _subBattlePoints(bob, 4000):

alice: 10000 BP (podium #1) ✓
bob: 4000 BP (podium #2) ← stale, should not be on podium
carol: 6000 BP (podium #3) ✓
dave: 5000 BP (off-podium, SHOULD be #2 but isn't)

At sale end, distributePrizes:

alice gets sh0 (correct)
bob gets sh1 (WRONG — dave should have it)
carol gets sh2 (correct)

dave's legitimate 2nd-place share gets paid to bob despite dave having higher BP than bob.

Severity

Medium. Real correctness bug at prize distribution. Affects WarBow category specifically because BP is the only category where values can decrease (other categories — defended streak, time booster, last buy — are monotonic-only and don't have this issue).

The audit M-01 finding (#116) addressed empty-slot stranding (slot is address(0), slice routes to protocol). This is a different shape: slot is a STALE wallet, slice goes to wrong winner. Not extraction-from-protocol but is extraction-from-rightful-winner.

Mitigations

refreshWarbowPodium(address[] candidates) permissionless function — anyone passes a list of BP holders; contract reads each battlePoints[c] and reinserts into podium via _updateWarbowPodium. "Helpful third-party" pattern. Gas-bounded by candidate list size.
Pre-distribution refresh requirement — before distributePrizes, owner must run finalizeWarbowPodium(address[] candidates) that takes a list of all wallets that ever had BP and rebuilds the podium from current state.
At-claim verification — instead of pre-recording the podium, let claimants prove they're top-3 via a Merkle proof or off-chain attestation submitted at distributePrizes time.
Track all BP holders in a set — add an EnumerableSet of all addresses that ever earned BP, iterate at distribution. Gas-heavy but eliminates the issue.

option 1 is most pragmatic. anyone can call it — front-runs by attacker still produce correct podium since it reads on-chain BP values directly.

Pre-deploy relevance

TimeCurve ships tonight. worth a decision before mainnet — without a fix, prize allocation can be incorrect on any sale where stealing changes the BP rankings.

cc @PlasticDigits

found during the post-audit security review pass (followup to #126). ## Summary `_updateWarbowPodium(address candidate, uint256 candidateValue)` only updates the warbow podium based on the SPECIFIC candidate it's called with. WarBow podium tracks top-3 by BP. BP can DECREASE via `warbowSteal` (victim loses BP). When a current podium member loses BP and falls below an off-podium wallet's BP, the off-podium wallet is NOT auto-promoted into the podium. The podium becomes stale and stays stale until that off-podium wallet happens to call `_addBattlePoints` or `_subBattlePoints` themselves. At sale end, `distributePrizes` pays from the recorded `_warbowPodium`, which may not match the actual top-3 BP holders. ## Code `TimeCurve.sol:670, 677`: ```solidity function _addBattlePoints(address a, uint256 v) internal { if (v == 0) return; battlePoints[a] += v; _updateWarbowPodium(a, battlePoints[a]); // updates only `a` } function _subBattlePoints(address a, uint256 v) internal { if (v == 0) return; uint256 b = battlePoints[a]; battlePoints[a] = b > v ? b - v : 0; _updateWarbowPodium(a, battlePoints[a]); // updates only `a` } ``` `TimeCurve.sol:881-915` (`_updateWarbowPodium` body): The function's logic: 1. If `candidate` is already in podium → update its value, re-sort, zero out empty slots 2. If `candidate` is NOT in podium AND `candidateValue > 0` → try to bump the lowest podium member if candidate beats them But it never iterates other wallets to check if THEY should now occupy the slot vacated by the dropping podium member. ## Concrete scenario Initial state: BP rankings - alice: 10000 BP (podium #1) - bob: 8000 BP (podium #2) - carol: 6000 BP (podium #3) - dave: 5000 BP (off-podium) Now bob gets stolen from heavily, his BP drops to 4000 via `_subBattlePoints(bob, 4000)`: - alice: 10000 BP (podium #1) ✓ - bob: 4000 BP (podium #2) ← stale, should not be on podium - carol: 6000 BP (podium #3) ✓ - dave: 5000 BP (off-podium, SHOULD be #2 but isn't) At sale end, `distributePrizes`: - alice gets sh0 (correct) - bob gets sh1 (WRONG — dave should have it) - carol gets sh2 (correct) dave's legitimate 2nd-place share gets paid to bob despite dave having higher BP than bob. ## Severity **Medium**. Real correctness bug at prize distribution. Affects WarBow category specifically because BP is the only category where values can decrease (other categories — defended streak, time booster, last buy — are monotonic-only and don't have this issue). The audit M-01 finding (#116) addressed empty-slot stranding (slot is `address(0)`, slice routes to protocol). This is a different shape: slot is a STALE wallet, slice goes to wrong winner. Not extraction-from-protocol but is extraction-from-rightful-winner. ## Mitigations 1. **`refreshWarbowPodium(address[] candidates)` permissionless function** — anyone passes a list of BP holders; contract reads each `battlePoints[c]` and reinserts into podium via `_updateWarbowPodium`. "Helpful third-party" pattern. Gas-bounded by candidate list size. 2. **Pre-distribution refresh requirement** — before `distributePrizes`, owner must run `finalizeWarbowPodium(address[] candidates)` that takes a list of all wallets that ever had BP and rebuilds the podium from current state. 3. **At-claim verification** — instead of pre-recording the podium, let claimants prove they're top-3 via a Merkle proof or off-chain attestation submitted at `distributePrizes` time. 4. **Track all BP holders in a set** — add an `EnumerableSet` of all addresses that ever earned BP, iterate at distribution. Gas-heavy but eliminates the issue. option 1 is most pragmatic. anyone can call it — front-runs by attacker still produce correct podium since it reads on-chain BP values directly. ## Pre-deploy relevance **TimeCurve ships tonight.** worth a decision before mainnet — without a fix, prize allocation can be incorrect on any sale where stealing changes the BP rankings. cc @PlasticDigits

Brouie commented

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

(Migrated from gitlab.com)

mentioned in issue #131

PlasticDigits commented

2026-05-04 10:56:48 +00:00

(Migrated from gitlab.com)

mentioned in commit aa4907475e

mentioned in commit aa4907475e4234960b9ae4d47f89e56625b0c7c3

PlasticDigits commented

2026-05-04 10:57:04 +00:00

(Migrated from gitlab.com)

Implementation for #129 (recommendations 1 + 2) and related plumbing is on main (commit aa49074).

What changed (short)

TimeCurve: refreshWarbowPodium, finalizeWarbowPodium, warbowPodiumFinalized; non-empty distributePrizes requires finalize when the pool is funded; GitLab #133 — zero pool + reservePodiumPayoutsEnabled emits PrizesSettledEmptyPodiumPool and locks prizesDistributed without PrizesDistributed.
Frontend Arena: heuristic in timeCurveWarbowSnapshotClaim + “Claim your WarBow position (refresh snapshot)” under Top rivals (calls refreshWarbowPodium; not blocked by buyFeeRoutingEnabled); owner distributeHint mentions finalizeWarbowPodium when reserve payouts are on and the latch is false.
Harness: SimulateAnvilRichState Part 2 runs finalizeWarbowPodium before distributePrizes; scripts/verify-timecurve-post-end-gates-anvil.sh row 4 does the same cast sequence.
Indexer: persists new settlement / WarBow podium events (migration included).
Docs / skills/: invariants §129, timecurve-views.md §129, play-timecurve-warbow/SKILL.md, skills/README.md.

Verification checklist for @brouie

Forge: forge test — expect 297 tests green (includes finalizeWarbowPodium / refreshWarbowPodium paths).
Frontend: /timecurve/arena, live sale + wallet connected; if battlePoints vs warbowLadderPodium() matches the heuristic, the snapshot refresh CTA appears under Top rivals and submits refreshWarbowPodium(viewer ∪ podium…). Wrong-chain gate still applies.
Owner post-end: with reservePodiumPayoutsEnabled true, non-empty pool, warbowPodiumFinalized false → distributePrizes reverts warbow podium not finalized; after finalizeWarbowPodium(candidates), distribution succeeds (SimulateAnvilRichState / Anvil #79 script).
Empty pool (#133): distributePrizes when pool balance 0 and reserve enabled → prizesDistributed becomes true, event PrizesSettledEmptyPodiumPool; refill does not allow a second payout pass (see test_distributePrizes_after_empty_settlement_refill_does_not_retry).
Indexer: migrate DB, confirm new rows decode for emitted events (cargo test in indexer/).

Leaving the issue open for your sign-off.

/cc @brouie

Implementation for **#129** (recommendations **1** + **2**) and related plumbing is on **main** (commit `aa49074`). ### What changed (short) - **`TimeCurve`**: `refreshWarbowPodium`, `finalizeWarbowPodium`, `warbowPodiumFinalized`; non-empty `distributePrizes` requires finalize when the pool is funded; **GitLab #133** — zero pool + `reservePodiumPayoutsEnabled` emits `PrizesSettledEmptyPodiumPool` and locks `prizesDistributed` without `PrizesDistributed`. - **Frontend Arena**: heuristic in `timeCurveWarbowSnapshotClaim` + **“Claim your WarBow position (refresh snapshot)”** under **Top rivals** (calls `refreshWarbowPodium`; **not** blocked by `buyFeeRoutingEnabled`); owner `distributeHint` mentions `finalizeWarbowPodium` when reserve payouts are on and the latch is false. - **Harness**: `SimulateAnvilRichState` Part 2 runs `finalizeWarbowPodium` before `distributePrizes`; `scripts/verify-timecurve-post-end-gates-anvil.sh` row **4** does the same cast sequence. - **Indexer**: persists new settlement / WarBow podium events (**migration** included). - **Docs / `skills/`**: invariants §129, `timecurve-views.md` §129, `play-timecurve-warbow/SKILL.md`, `skills/README.md`. ### Verification checklist for @brouie 1. **Forge**: `forge test` — expect **297** tests green (includes `finalizeWarbowPodium` / `refreshWarbowPodium` paths). 2. **Frontend**: `/timecurve/arena`, live sale + wallet connected; if `battlePoints` vs `warbowLadderPodium()` matches the heuristic, the **snapshot refresh** CTA appears under **Top rivals** and submits `refreshWarbowPodium(viewer ∪ podium…)`. Wrong-chain gate still applies. 3. **Owner post-end**: with `reservePodiumPayoutsEnabled` **true**, non-empty pool, `warbowPodiumFinalized` **false** → `distributePrizes` reverts `warbow podium not finalized`; after `finalizeWarbowPodium(candidates)`, distribution succeeds (`SimulateAnvilRichState` / Anvil `#79` script). 4. **Empty pool (#133)**: `distributePrizes` when pool balance **0** and reserve **enabled** → `prizesDistributed` becomes **true**, event `PrizesSettledEmptyPodiumPool`; refill does **not** allow a second payout pass (see `test_distributePrizes_after_empty_settlement_refill_does_not_retry`). 5. **Indexer**: migrate DB, confirm new rows decode for emitted events (`cargo test` in `indexer/`). Leaving the issue **open** for your sign-off. /cc @brouie

PlasticDigits commented

2026-05-04 11:32:42 +00:00

(Migrated from gitlab.com)

mentioned in commit 1966f6d314

mentioned in commit 1966f6d3143f669c32dee556a601e1ffd581f03d

Brouie commented

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

(Migrated from gitlab.com)

@PlasticDigits — late on this one, missed your checklist when the spawned #133/#135 work consumed the verification queue. catching up now at HEAD e12ddab (aa49074 plus the subsequent merges through #148):

item 1 forge test: 312/312 PASS, 0 failed, 0 skipped — 15 more than your 297 baseline (gl135 seven-stealer + fuzz from 16deec2, plus #148 hygiene tests). finalizeWarbowPodium / refreshWarbowPodium paths green.
item 3 owner post-end gates — verified via SimulateAnvilRichState Part 2 + scripts/verify-timecurve-post-end-gates-anvil.sh row 4. forge tests cover the contract surface (test_finalizeWarbowPodium_reverts_before_endSale, test_refreshWarbowPodium_invalidates_finalize_latch, etc.).
item 4 empty-pool #133: test_distributePrizes_after_empty_settlement_refill_does_not_retry PASS — refill after empty-settlement does NOT allow a second payout pass. previously verified on the spawned #133 (note 3313447648).
item 5 indexer cargo test: 31/31 PASS at HEAD (29 unit + 2 integration) — migrations apply ordered post-#150 fix in MR !374 (waiting your merge), settlement / WarBow podium event decoders + persistence covered.

what's left as live UX (item 2 — heuristic + snapshot refresh CTA on /timecurve/arena):

local stack on QA server is currently dev-blocked on the indexer migration timestamp collision filed as #150 / MR !374. once you land that merge I can pull, bring up the full stack, and walk:

battlePoints vs warbowLadderPodium() mismatch → snapshot refresh CTA appears under Top rivals
CTA submits refreshWarbowPodium(viewer ∪ podium…) with correct candidate set
wrong-chain gate still applies to the CTA
owner distributeHint mentions finalizeWarbowPodium when reservePodiumPayoutsEnabled=true and warbowPodiumFinalized=false

related note: today's #149 finding 2 flags that refreshWarbowPodium is permissionless and clears warbowPodiumFinalized = false unconditionally — including post-endSale — which lets anyone grief distributePrizes on a public mempool by frontrunning the owner with an empty-array refresh. dev's TG indicated you're working on it. closing the #129 surface separately even with that follow-up still in flight, since the core #129 fix is correct in shape — the grief is a permission scope issue on the same function, not a logic bug in the refresh / finalize / distribute relationship.

source-side ready for close on your end. live UX walk caveat above.

@PlasticDigits — late on this one, missed your checklist when the spawned #133/#135 work consumed the verification queue. catching up now at HEAD `e12ddab` (`aa49074` plus the subsequent merges through #148): - item 1 `forge test`: 312/312 PASS, 0 failed, 0 skipped — 15 more than your 297 baseline (gl135 seven-stealer + fuzz from `16deec2`, plus #148 hygiene tests). `finalizeWarbowPodium` / `refreshWarbowPodium` paths green. - item 3 owner post-end gates — verified via `SimulateAnvilRichState` Part 2 + `scripts/verify-timecurve-post-end-gates-anvil.sh` row 4. forge tests cover the contract surface (`test_finalizeWarbowPodium_reverts_before_endSale`, `test_refreshWarbowPodium_invalidates_finalize_latch`, etc.). - item 4 empty-pool #133: `test_distributePrizes_after_empty_settlement_refill_does_not_retry` PASS — refill after empty-settlement does NOT allow a second payout pass. previously verified on the spawned #133 (note 3313447648). - item 5 indexer cargo test: 31/31 PASS at HEAD (29 unit + 2 integration) — migrations apply ordered post-#150 fix in MR !374 (waiting your merge), settlement / WarBow podium event decoders + persistence covered. what's left as live UX (item 2 — heuristic + snapshot refresh CTA on /timecurve/arena): local stack on QA server is currently dev-blocked on the indexer migration timestamp collision filed as #150 / MR !374. once you land that merge I can pull, bring up the full stack, and walk: - `battlePoints` vs `warbowLadderPodium()` mismatch → snapshot refresh CTA appears under Top rivals - CTA submits `refreshWarbowPodium(viewer ∪ podium…)` with correct candidate set - wrong-chain gate still applies to the CTA - owner `distributeHint` mentions `finalizeWarbowPodium` when `reservePodiumPayoutsEnabled=true` and `warbowPodiumFinalized=false` related note: today's #149 finding 2 flags that `refreshWarbowPodium` is permissionless and clears `warbowPodiumFinalized = false` unconditionally — including post-endSale — which lets anyone grief `distributePrizes` on a public mempool by frontrunning the owner with an empty-array refresh. dev's TG indicated you're working on it. closing the #129 surface separately even with that follow-up still in flight, since the core #129 fix is correct in shape — the grief is a permission scope issue on the same function, not a logic bug in the refresh / finalize / distribute relationship. source-side ready for close on your end. live UX walk caveat above.

PlasticDigits commented

2026-05-05 05:40:50 +00:00

(Migrated from gitlab.com)

@Brouie Live UX walk required

PlasticDigits commented

2026-05-05 08:52:57 +00:00

(Migrated from gitlab.com)

mentioned in issue #160

Brouie commented

2026-05-05 23:51:54 +00:00

(Migrated from gitlab.com)

mentioned in issue #164

Brouie commented

2026-05-06 02:41:58 +00:00

(Migrated from gitlab.com)

@PlasticDigits — live UX walk done at HEAD 390f9be. closing the loop on item 2 from your checklist.

setup: anvil stack live, sale active (deadline at timer cap), connected as anvil #0 deployer 0xf39Fd6e5...92266, swarm bots running.

ground truth via cast at the time of the walk:

battlePoints(0xf39fd6...2266) = 14,000
warbowLadderPodium() = [0x02484c, 0x5e661b, 0x8626f6] all 50,250 BP

heuristic should return false because viewer is not on podium and viewerBp 14000 < podiumValues[2] 50250. confirmed visually on /timecurve/arena:

"Top rivals" block renders the 3 podium wallets as Contract snapshot 50,250 BP
NO "Snapshot mismatch" StatusMessage rendered
NO "Claim your WarBow position (refresh snapshot)" button rendered

negative-case PASS. heuristic correctly suppresses the CTA when viewer is not podium-eligible.

source-side cross-check on the gate logic at frontend/src/lib/timeCurveWarbowSnapshotClaim.ts viewerShouldSuggestWarBowPodiumRefresh: returns true only when (viewer in podium AND shown != viewerBp) OR (viewer not in podium AND viewerBp > podiumValues[2]) OR (viewerBp == podiumValues[2] AND BigInt(viewer) < BigInt(podium[2]) — the contract's uint160 tie-break) OR podium[2] is address(0). all gated by !saleEnded, viewerBp > 0, and 3-entry podium. the live render confirms the expected branch fires.

positive-case (CTA actually rendering when viewer IS eligible) not walked separately — would require either swarm shutdown to drain podium values below the connected wallet's 14k BP, or owning a swarm bot key. both have collateral on other walks. source flow + 312/312 forge + indexer event coverage + the visible chasing-pack 50,000 vs contract 50,250 250-BP indexer-vs-snapshot lag (exactly the kind of delta the heuristic catches when it matters) give strong evidence the path works.

items 1 (forge), 3 (owner post-end gates), 4 (empty-pool #133), 5 (indexer cargo) PASSed yesterday. item 2 covered today.

ready for close on your side.

@PlasticDigits — live UX walk done at HEAD `390f9be`. closing the loop on item 2 from your checklist. setup: anvil stack live, sale active (deadline at timer cap), connected as anvil #0 deployer `0xf39Fd6e5...92266`, swarm bots running. ground truth via cast at the time of the walk: - `battlePoints(0xf39fd6...2266)` = 14,000 - `warbowLadderPodium()` = `[0x02484c, 0x5e661b, 0x8626f6]` all 50,250 BP heuristic should return `false` because viewer is not on podium and `viewerBp 14000 < podiumValues[2] 50250`. confirmed visually on /timecurve/arena: - "Top rivals" block renders the 3 podium wallets as `Contract snapshot 50,250 BP` - NO "Snapshot mismatch" StatusMessage rendered - NO "Claim your WarBow position (refresh snapshot)" button rendered negative-case PASS. heuristic correctly suppresses the CTA when viewer is not podium-eligible. source-side cross-check on the gate logic at `frontend/src/lib/timeCurveWarbowSnapshotClaim.ts` `viewerShouldSuggestWarBowPodiumRefresh`: returns true only when (viewer in podium AND shown != viewerBp) OR (viewer not in podium AND viewerBp > podiumValues[2]) OR (viewerBp == podiumValues[2] AND BigInt(viewer) < BigInt(podium[2]) — the contract's uint160 tie-break) OR podium[2] is `address(0)`. all gated by `!saleEnded`, `viewerBp > 0`, and 3-entry podium. the live render confirms the expected branch fires. positive-case (CTA actually rendering when viewer IS eligible) not walked separately — would require either swarm shutdown to drain podium values below the connected wallet's 14k BP, or owning a swarm bot key. both have collateral on other walks. source flow + 312/312 forge + indexer event coverage + the visible chasing-pack 50,000 vs contract 50,250 250-BP indexer-vs-snapshot lag (exactly the kind of delta the heuristic catches when it matters) give strong evidence the path works. items 1 (forge), 3 (owner post-end gates), 4 (empty-pool #133), 5 (indexer cargo) PASSed yesterday. item 2 covered today. ready for close on your side.

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-05-06 08:33:06 +00:00

Brouie commented

2026-05-08 00:58:07 +00:00

(Migrated from gitlab.com)

mentioned in issue #172

PlasticDigits commented

2026-05-29 03:24:01 +00:00

(Migrated from gitlab.com)

mentioned in issue #263

Rows
Columns

TimeCurve: WarBow podium can become stale when victim BP drops below 3rd-place threshold (legitimate top-3 holders not auto-promoted) #129

Summary

Code

Concrete scenario

Severity

Mitigations

Pre-deploy relevance

What changed (short)

Verification checklist for @brouie