code/cl8y-dex-terraclassic

Fork 0

pair oracle: price_a_cumulative can saturate u128, permanently bricking observe() (and likely swaps) on a live pair #1322

New issue

Closed

opened 2026-09-22 13:48:48 +00:00 by LeonardoLUNC · 6 comments

LeonardoLUNC commented

2026-09-22 13:48:48 +00:00

On one live economic pair, observe{seconds_ago:[0,N]} now reverts for every N with "Oracle: Cannot Add with 340144359629112943994362291128760055446 and ". The first operand is price_a_cumulative at 99.96% of u128::MAX; the second grows with wall-clock, so the sum overflows and the condition is unrecoverable — the accumulator is monotonic. The last stored observation is timestamped ~2h before the query began failing, which matches the remaining headroom divided by the accrual rate. Nine other pairs on the same factory are below 0.001% of u128.

Two consequences worth separating: (1) any consumer using observe() as a TWAP reference loses it silently — a client that treats a query error as "no data" will fall through to an unguarded path; (2) if the swap handler performs the same checked add when writing the ring buffer, the pair becomes untradeable through the pool, permanently, with LP still deposited.

Suggested directions: saturating or wrapping accumulation with delta-based reads (the Uniswap V2 convention), a wider accumulator, or normalising the price by token decimals before accumulating so that pairs with a large decimal asymmetry do not accrue orders of magnitude faster than others.

On one live economic pair, `observe{seconds_ago:[0,N]}` now reverts for every N with "Oracle: Cannot Add with 340144359629112943994362291128760055446 and <accrue-to-now term>". The first operand is `price_a_cumulative` at 99.96% of `u128::MAX`; the second grows with wall-clock, so the sum overflows and the condition is unrecoverable — the accumulator is monotonic. The last stored observation is timestamped ~2h before the query began failing, which matches the remaining headroom divided by the accrual rate. Nine other pairs on the same factory are below 0.001% of u128. Two consequences worth separating: (1) any consumer using `observe()` as a TWAP reference loses it silently — a client that treats a query error as "no data" will fall through to an unguarded path; (2) if the swap handler performs the same checked add when writing the ring buffer, the pair becomes untradeable through the pool, permanently, with LP still deposited. Suggested directions: saturating or wrapping accumulation with delta-based reads (the Uniswap V2 convention), a wider accumulator, or normalising the price by token decimals before accumulating so that pairs with a large decimal asymmetry do not accrue orders of magnitude faster than others.

PlasticDigits commented

2026-09-22 15:17:16 +00:00

Owner

Approved for fix

PlasticDigits commented

2026-09-22 15:24:08 +00:00

Owner

Repair note

One issue. The query failure and the execute lock are the same checked_add on price_*_cumulative. Do not split them.

Not a duplicate, and not already implemented. wrapping_add / wrapping_sub are not used on this oracle. Sibling tickets stay separate:

Ticket	State	Why it is not this bug
#465	Closed	`Decimal::from_ratio` panic when the reserve ratio cannot be a `Decimal`. Execute already skips.
#1231	Closed	Same unrepresentable ratio on Observe forward-extrapolation. Query already returns the last stored cumulatives.
#1224	Open	A single `price × dt` does not fit in `u128` (`price_times_dt`) even though the ratio is a `Decimal`. Wrapping the cumulative does not fix that. Do not fold #1224 into this ticket or mark it done from this work.

No other open issue tracks cumulative saturation.

Current codebase

Every pair stores an arithmetic-mean TWAP as Uint128 cumulatives of CosmWasm Decimal spot (reserve_b / reserve_a and the reciprocal), scaled by 1e18. oracle_update runs before reserve writes on the only three paths that save RESERVES: swap (pool and hybrid, including a book take), provide, and withdraw (smartcontracts/contracts/pair/src/contract.rs). Limit place, cancel, claim, and reprice do not call oracle_update and do not write RESERVES.

After the #465 / #1231 ratio skip, both execute and Observe still do:

price_times_dt (smartcontracts/packages/dex-common/src/oracle.rs) — price.atomics() * dt via checked_mul.
last_cumulative.checked_add(delta) — execute prefixes the error price_a overflow: / price_b overflow:; Observe stores e.to_string() with no prefix.

ContractError::Oracle renders as Oracle: {reason}. The reported string Oracle: Cannot Add with 340144359629112943994362291128760055446 and <accrue-to-now term> matches the Observe mapping, not the execute prefix. That cumulative is about 99.96% of u128::MAX (340282366920938463463374607431768211455); headroom is about 1.38e35. query_observe uses ? per seconds_ago entry, so one overflowing “now” point fails the whole query, including historical points that would have fit. That matches “every N reverts.”

u128::MAX is about 3.40e38. A raw ratio whose atomics accrue near 1e30–1e31 per second (18-vs-6 decimal asymmetry, plus a premium or imbalance, still below Decimal::MAX) fills a Uint128 in months, not geological time. Nine calmer pairs staying under 0.001% of u128 fits that. Docs currently call this overflow “handled gracefully with errors” (dex-common oracle module comment and docs/twap-oracle.md). The error is the brick.

Charts is the in-repo consumer. getTwapPrices (frontend-dapp/src/services/terraclassic/oracle.ts) catches a failed observe and returns null prices, so /charts shows TWAP building… rather than a hard error. computeTwapPriceDecimalString returns null when cumEnd < cumStart. TWAP is display-only (quote per base, not a swap belief price). compute_twap_price in dex-common has the same end-before-start reject. Historical interpolation uses plain after - before and before + diff * dt / span.

There is no storage migration that rewrites cumulatives. Pair migrate leaves OBSERVATIONS in place.

Why a new implementation is needed

The accumulator is monotonic and the add is checked, so once last + price×dt exceeds u128::MAX the condition does not heal. Wall-clock only grows.

Observe. Integrators and Charts lose the TWAP. A client that treats the query error as “no data” (Charts does) shows an empty oracle while the pool is still the live price source.
Execute. The same add sits at the top of swap, provide, and withdraw. The next block with dt > 0 and non-zero reserves reverts those messages with Oracle: price_a overflow: … or price_b overflow: …. LP cannot exit. Router hops that call the pair fail with it. Limit escrow can still be cancelled or claimed, because those messages never touch the oracle. This is a permanent liveness lock of pool reserves until a governance pair wasm migrate, not a drain of other users’ tokens.

Skipping the sample forever (return Ok and freeze the cumulative) would unbrick trading and still publish a stale integral. Saturating at MAX drops every later second and makes TWAP read as zero across the clamp. Neither is the Uniswap-style fix. Rescaling by token decimals, or widening the stored type to u256, changes the public Uint128 Observe ABI and makes pre-change observations incomparable. Those are different products.

Constraints and guardrails

Keep #465 and #1231 behavior: an unrepresentable reserve ratio skips the sample (execute Ok, Observe returns the last stored cumulatives). Do not clamp to Decimal::MAX. Do not bring back panicking Decimal::from_ratio.
Do not implement #1224 here. If price_times_dt returns Err, do not wrap a truncated product into the cumulative. A single delta that does not fit in u128 must stay out of this ticket.
Sample pre-op reserves. This bug is overflow of the running sum, not when the spot is read.
Wrapping add/sub is modulo 2^128 only. The window integral is meaningful when that integral itself fits in u128 (true for Charts-length windows at the live pair’s accrual; false for a #1224-sized single step).
Do not add an admin or migrate message that resets or rewrites cumulatives. A keyholder must not be able to set TWAP.
Do not change Observe JSON field names, ring cardinality rules, or seconds_ago semantics.
Do not rescale accumulation by 10^(decimals0 − decimals1) in this ticket. Human scaling stays in the dApp (rawLimitPriceToHuman / #564).
Do not widen Observation to u256 in this ticket.
Query stays read-only: no RESERVES or OBSERVATIONS writes from Observe.
Columbus-5 code-id migrate that unbricks the live pair is a follow-up ops ticket after this wasm is merged. This ticket does not include a production migrate script or a mainnet broadcast.
Floats are forbidden. No unwrap on the cumulative add.

Relevant files

smartcontracts/contracts/pair/src/contract.rs — oracle_update, oracle_observe_single, query_observe; call sites in execute_swap, execute_provide_liquidity, execute_withdraw_liquidity
smartcontracts/packages/dex-common/src/oracle.rs — price_times_dt, compute_twap_price, overflow comment
smartcontracts/contracts/pair/src/error.rs — ContractError::Oracle
smartcontracts/tests/src/lib.rs — oracle_tests (plain subtraction of Observe results)
frontend-dapp/src/services/terraclassic/oracle.ts — computeTwapPriceDecimalString, getTwapPrices
frontend-dapp/src/services/terraclassic/__tests__/oracle.test.ts
frontend-dapp/src/pages/ChartsPage.tsx — TWAP chips; nulls render as “TWAP building…”
docs/twap-oracle.md
docs/contracts-security-audit.md (O1231 row must stay true; add this ticket beside it, do not rewrite O1231)
skills/AGENTS_TWAP_OBSERVE_RATIO.md — pointer only, so later work does not “fix” this by weakening #1231

Recommended direction

Use wrapping accumulation, the Uniswap V2 convention, on both cumulatives:

oracle_update and Observe forward-extrapolation: wrapping_add of a delta that price_times_dt already accepted. Advance the ring and timestamp the same way as today.
Historical interpolation and compute_twap_price: wrapping_sub(end, start) so a window that crosses the modulus still yields the in-window integral. Then divide by elapsed time and Decimal::from_atomics(..., 18) as today. Remove the hard error that treats end < start as corruption; that branch is the wrap, not a corrupt store.
computeTwapPriceDecimalString: same wrapping sub on the u128 modulus, so a Charts window that crosses the modulus still shows the pair TWAP. Keep null for a non-positive elapsed time and a zero average.

Reject, for this ticket: saturating add, skip-and-freeze once near MAX, decimal normalization, and a wider stored integer.

After the pair is migrated, the already-stored cumulative near MAX does not need a rewrite. The next swap’s delta wraps and the reserve write commits.

Acceptance criteria

Seeding price_a_cumulative at 340144359629112943994362291128760055446 (and the symmetric price_b case) plus a representable spot and a dt whose price × dt fits in u128 but whose sum does not: oracle_update returns Ok, stores wrapping_add, and advances the ring timestamp. It must not return Oracle: price_a overflow / price_b overflow.
The same seed: QueryMsg::Observe with seconds_ago that includes 0 and an in-buffer historical offset returns JSON for every offset. The “now” cumulative is the wrapped sum. Historical points that do not add a new delta stay on the stored curve.
Swap, provide, and withdraw on that seeded state still move RESERVES (multitest). They must not revert with ContractError::Oracle for this add.
A window whose true integral crosses 2^128 once: compute_twap_price and computeTwapPriceDecimalString return that integral divided by elapsed time, not null and not a value near 2^128 / dt.
Interpolation between two stored observations that straddle one wrap does not panic and matches the same wrapping integral.
#465 and #1231 tests stay green: unrepresentable ratio still skips; balanced dt > 0 still advances cumulatives; Observe JSON keys stay price_a_cumulatives / price_b_cumulatives.
A price_times_dt overflow (delta itself does not fit) is unchanged by this ticket and is still not written as a wrapped truncated delta. #1224 remains open.
Same-block (block_time <= last_ts), zero reserves, and the first zero-cumulative seed behave as they do now.
Docs stop saying this overflow is handled by returning an error. They describe modulo 2^128 and the “integral must fit in u128” window rule. O1231 text is not weakened.

Test plan

Unit (pair + dex-common)

Near-max cumulative, modest fitting delta, one side only and then the other side only: wrap, ring index advances, the other cumulative still exact-adds.
Delta that lands exactly on 2^128 (sum ≡ 0): stored cumulative is zero; the next observation can add again.
Two observations straddle the modulus; target timestamp between them: interpolation matches wrapping math.
seconds_ago = 0 at target == latest.timestamp: returns stored cumulatives and does not add.
Zero reserve and #465 ratio (1 vs u128::MAX): still skip, cumulatives unchanged.
Balanced reserves far from the ceiling: cumulatives identical to today’s checked add (regression).
compute_twap_price for a non-wrapping window matches existing tests; wrapping window matches (end - start) mod 2^128; time_elapsed == 0 still errors.
price_times_dt overflow still errors and oracle_update does not persist a new observation in that case (pin current #1224 behavior so this change cannot wrap the truncated product).

Multitest (smartcontracts/tests oracle module)

Provide, advance time, swap: observations still record on the happy path (existing test).
Seed or drive a cumulative to MAX - small with a fitting price, advance one block, swap, provide, and withdraw: each succeeds and reserves change.
Observe [0, window] across the wrap: both cumulatives present; client-side wrapping sub reconstructs the price.

Frontend

computeTwapPriceDecimalString: existing non-wrap cases; cumEnd < cumStart where the wrapping diff is the real window integral returns that price; elapsed ≤ 0 and zero average stay null.
getTwapPrices still returns null prices when observe throws, and still returns a price when Observe succeeds across a wrap (mock the response).

Paths that must keep working without an oracle write

Limit place, cancel, claim, and reprice on a pair whose cumulative is already near MAX (they do not call oracle_update). Do not require them to start updating the oracle.

Attack, hack, and abuse

These are lock / stale-oracle / bad-integral risks, not a pool drain. Tests seed storage or use the unit harness. Do not add a mainnet reserve-skew walkthrough.

Accelerate the ceiling. A representable but large spot (still a successful checked_from_ratio) increases atomics per second. On a thin pool that is cheap relative to TVL; on a deep pool it is the existing TWAP-manipulation cost. After the wrap fix, the same trade must not freeze swap or LP exit. Assert execute Ok and reserves moved. Assert the recorded delta is the full price × dt, not a saturated stub.
Truncated-delta wrap (#1224 conflation). A test where atomics * dt exceeds u128 must not store wrapping_mul’s low bits. That would understate TWAP and look like a successful observation. Expected: no new observation from this ticket’s math (current error or, later, #1224’s skip — not a wrapped lie).
Saturating or skip-and-freeze regression. A test that the cumulative is not stuck at MAX and that a later second still changes the integral. Frozen TWAP while spot moves is a stale-price bug for any future consumer that treats Observe as a mark.
False wrap on bad inputs. compute_twap_price is only defined for two snapshots of the same counter. A unit test documents that a single modulus crossing reconstructs the small integral. Do not add a public setter. Migrate must not accept a caller-supplied cumulative.
Interpolation panic as a query DoS. Stored observations that straddle the modulus must return Ok. A panic or a whole-vector ? failure would again blank every seconds_ago list.
Argument-swapped subtraction. Wrapping sub of a reversed window is a huge value. Decimal::from_atomics may still succeed. Keep this helper on pair-oracle snapshots in time order. Charts must not feed the result into swap belief_price (it does not today; add no such wiring).
Admin rewrite. Grep-level test or review note: no new execute arm writes price_*_cumulative except oracle_update’s wrapping add and the existing first-observation zero seed.
Observe gas. The query remains a pure read. Wrapping add must not loop or binary-search more than today’s ring walk.
Limit-book confusion. Document in the test name that cancel/claim still succeed when pool swap reverts before the fix, so nobody “repairs” the lock by blocking cancels. After the fix, swap and cancel both succeed; escrow accounting is unchanged.
Decimal-rescale discontinuity. No test setup should mix pre-scale and post-scale cumulatives. This ticket must not change units.

Verification

cd smartcontracts && cargo test -p cl8y-dex-pair oracle_overflow -- --nocapture
cd smartcontracts && cargo test -p cl8y-dex-pair oracle_observe -- --nocapture
cd smartcontracts && cargo test -p dex-common --lib oracle -- --nocapture
cd smartcontracts && cargo test -p cl8y-dex-tests oracle -- --test-threads=1
make test-contracts
Frontend: make test-frontend scoped to oracle.test.ts and Charts TWAP if those tests change
make verify-issue-1231 still passes (it already runs the #465 oracle_overflow tests and the #1231 Observe tests)
#1224’s price_times_dt execute brick stays open; this issue does not close it
Docs: docs/twap-oracle.md describes modulo 2^128; the “handled with errors” sentence is gone
Live pair: code merge does not by itself unbrick columbus-5. Recovery is a later pair wasm migrate that preserves OBSERVATIONS. Out of scope to execute here.

## Repair note One issue. The query failure and the execute lock are the same `checked_add` on `price_*_cumulative`. Do not split them. Not a duplicate, and not already implemented. `wrapping_add` / `wrapping_sub` are not used on this oracle. Sibling tickets stay separate: | Ticket | State | Why it is not this bug | | --- | --- | --- | | [#465](https://git.cl8y.com/code/cl8y-dex-terraclassic/issues/465) | Closed | `Decimal::from_ratio` panic when the reserve ratio cannot be a `Decimal`. Execute already skips. | | [#1231](https://git.cl8y.com/code/cl8y-dex-terraclassic/issues/1231) | Closed | Same unrepresentable ratio on Observe forward-extrapolation. Query already returns the last stored cumulatives. | | [#1224](https://git.cl8y.com/code/cl8y-dex-terraclassic/issues/1224) | Open | A **single** `price × dt` does not fit in `u128` (`price_times_dt`) even though the ratio is a `Decimal`. Wrapping the cumulative does not fix that. Do not fold #1224 into this ticket or mark it done from this work. | No other open issue tracks cumulative saturation. ## Current codebase Every pair stores an arithmetic-mean TWAP as `Uint128` cumulatives of CosmWasm `Decimal` spot (`reserve_b / reserve_a` and the reciprocal), scaled by 1e18. `oracle_update` runs **before** reserve writes on the only three paths that save `RESERVES`: swap (pool and hybrid, including a book take), provide, and withdraw (`smartcontracts/contracts/pair/src/contract.rs`). Limit place, cancel, claim, and reprice do not call `oracle_update` and do not write `RESERVES`. After the #465 / #1231 ratio skip, both execute and Observe still do: 1. `price_times_dt` (`smartcontracts/packages/dex-common/src/oracle.rs`) — `price.atomics() * dt` via `checked_mul`. 2. `last_cumulative.checked_add(delta)` — execute prefixes the error `price_a overflow:` / `price_b overflow:`; Observe stores `e.to_string()` with no prefix. `ContractError::Oracle` renders as `Oracle: {reason}`. The reported string `Oracle: Cannot Add with 340144359629112943994362291128760055446 and <accrue-to-now term>` matches the **Observe** mapping, not the execute prefix. That cumulative is about 99.96% of `u128::MAX` (`340282366920938463463374607431768211455`); headroom is about `1.38e35`. `query_observe` uses `?` per `seconds_ago` entry, so one overflowing “now” point fails the whole query, including historical points that would have fit. That matches “every N reverts.” `u128::MAX` is about `3.40e38`. A raw ratio whose atomics accrue near `1e30`–`1e31` per second (18-vs-6 decimal asymmetry, plus a premium or imbalance, still **below** `Decimal::MAX`) fills a `Uint128` in months, not geological time. Nine calmer pairs staying under 0.001% of `u128` fits that. Docs currently call this overflow “handled gracefully with errors” (`dex-common` oracle module comment and `docs/twap-oracle.md`). The error is the brick. Charts is the in-repo consumer. `getTwapPrices` (`frontend-dapp/src/services/terraclassic/oracle.ts`) catches a failed `observe` and returns null prices, so `/charts` shows **TWAP building…** rather than a hard error. `computeTwapPriceDecimalString` returns null when `cumEnd < cumStart`. TWAP is display-only (quote per base, not a swap belief price). `compute_twap_price` in `dex-common` has the same end-before-start reject. Historical interpolation uses plain `after - before` and `before + diff * dt / span`. There is no storage migration that rewrites cumulatives. Pair `migrate` leaves `OBSERVATIONS` in place. ## Why a new implementation is needed The accumulator is monotonic and the add is checked, so once `last + price×dt` exceeds `u128::MAX` the condition does not heal. Wall-clock only grows. - **Observe.** Integrators and Charts lose the TWAP. A client that treats the query error as “no data” (Charts does) shows an empty oracle while the pool is still the live price source. - **Execute.** The same add sits at the top of swap, provide, and withdraw. The next block with `dt > 0` and non-zero reserves reverts those messages with `Oracle: price_a overflow: …` or `price_b overflow: …`. LP cannot exit. Router hops that call the pair fail with it. Limit escrow can still be cancelled or claimed, because those messages never touch the oracle. This is a permanent liveness lock of pool reserves until a governance pair wasm migrate, not a drain of other users’ tokens. Skipping the sample forever (return `Ok` and freeze the cumulative) would unbrick trading and still publish a stale integral. Saturating at `MAX` drops every later second and makes TWAP read as zero across the clamp. Neither is the Uniswap-style fix. Rescaling by token decimals, or widening the stored type to `u256`, changes the public `Uint128` Observe ABI and makes pre-change observations incomparable. Those are different products. ## Constraints and guardrails - Keep #465 and #1231 behavior: an unrepresentable reserve ratio skips the sample (execute `Ok`, Observe returns the last stored cumulatives). Do not clamp to `Decimal::MAX`. Do not bring back panicking `Decimal::from_ratio`. - Do not implement #1224 here. If `price_times_dt` returns `Err`, do **not** wrap a truncated product into the cumulative. A single delta that does not fit in `u128` must stay out of this ticket. - Sample **pre-op** reserves. This bug is overflow of the running sum, not when the spot is read. - Wrapping add/sub is modulo `2^128` only. The window integral is meaningful when that integral itself fits in `u128` (true for Charts-length windows at the live pair’s accrual; false for a #1224-sized single step). - Do not add an admin or migrate message that resets or rewrites cumulatives. A keyholder must not be able to set TWAP. - Do not change Observe JSON field names, ring cardinality rules, or `seconds_ago` semantics. - Do not rescale accumulation by `10^(decimals0 − decimals1)` in this ticket. Human scaling stays in the dApp (`rawLimitPriceToHuman` / #564). - Do not widen `Observation` to `u256` in this ticket. - Query stays read-only: no `RESERVES` or `OBSERVATIONS` writes from Observe. - Columbus-5 code-id migrate that unbricks the live pair is a follow-up ops ticket after this wasm is merged. This ticket does not include a production migrate script or a mainnet broadcast. - Floats are forbidden. No `unwrap` on the cumulative add. ## Relevant files - `smartcontracts/contracts/pair/src/contract.rs` — `oracle_update`, `oracle_observe_single`, `query_observe`; call sites in `execute_swap`, `execute_provide_liquidity`, `execute_withdraw_liquidity` - `smartcontracts/packages/dex-common/src/oracle.rs` — `price_times_dt`, `compute_twap_price`, overflow comment - `smartcontracts/contracts/pair/src/error.rs` — `ContractError::Oracle` - `smartcontracts/tests/src/lib.rs` — `oracle_tests` (plain subtraction of Observe results) - `frontend-dapp/src/services/terraclassic/oracle.ts` — `computeTwapPriceDecimalString`, `getTwapPrices` - `frontend-dapp/src/services/terraclassic/__tests__/oracle.test.ts` - `frontend-dapp/src/pages/ChartsPage.tsx` — TWAP chips; nulls render as “TWAP building…” - `docs/twap-oracle.md` - `docs/contracts-security-audit.md` (O1231 row must stay true; add this ticket beside it, do not rewrite O1231) - `skills/AGENTS_TWAP_OBSERVE_RATIO.md` — pointer only, so later work does not “fix” this by weakening #1231 ## Recommended direction Use **wrapping accumulation**, the Uniswap V2 convention, on both cumulatives: - `oracle_update` and Observe forward-extrapolation: `wrapping_add` of a delta that `price_times_dt` already accepted. Advance the ring and timestamp the same way as today. - Historical interpolation and `compute_twap_price`: `wrapping_sub(end, start)` so a window that crosses the modulus still yields the in-window integral. Then divide by elapsed time and `Decimal::from_atomics(..., 18)` as today. Remove the hard error that treats `end < start` as corruption; that branch is the wrap, not a corrupt store. - `computeTwapPriceDecimalString`: same wrapping sub on the `u128` modulus, so a Charts window that crosses the modulus still shows the pair TWAP. Keep null for a non-positive elapsed time and a zero average. Reject, for this ticket: saturating add, skip-and-freeze once near `MAX`, decimal normalization, and a wider stored integer. After the pair is migrated, the already-stored cumulative near `MAX` does not need a rewrite. The next swap’s delta wraps and the reserve write commits. ## Acceptance criteria 1. Seeding `price_a_cumulative` at `340144359629112943994362291128760055446` (and the symmetric `price_b` case) plus a **representable** spot and a `dt` whose `price × dt` fits in `u128` but whose sum does not: `oracle_update` returns `Ok`, stores `wrapping_add`, and advances the ring timestamp. It must not return `Oracle: price_a overflow` / `price_b overflow`. 2. The same seed: `QueryMsg::Observe` with `seconds_ago` that includes `0` and an in-buffer historical offset returns JSON for **every** offset. The “now” cumulative is the wrapped sum. Historical points that do not add a new delta stay on the stored curve. 3. Swap, provide, and withdraw on that seeded state still move `RESERVES` (multitest). They must not revert with `ContractError::Oracle` for this add. 4. A window whose true integral crosses `2^128` once: `compute_twap_price` and `computeTwapPriceDecimalString` return that integral divided by elapsed time, not null and not a value near `2^128 / dt`. 5. Interpolation between two stored observations that straddle one wrap does not panic and matches the same wrapping integral. 6. #465 and #1231 tests stay green: unrepresentable ratio still skips; balanced `dt > 0` still advances cumulatives; Observe JSON keys stay `price_a_cumulatives` / `price_b_cumulatives`. 7. A `price_times_dt` overflow (delta itself does not fit) is unchanged by this ticket and is still not written as a wrapped truncated delta. #1224 remains open. 8. Same-block (`block_time <= last_ts`), zero reserves, and the first zero-cumulative seed behave as they do now. 9. Docs stop saying this overflow is handled by returning an error. They describe modulo `2^128` and the “integral must fit in `u128`” window rule. O1231 text is not weakened. ## Test plan **Unit (pair + dex-common)** - Near-max cumulative, modest fitting delta, one side only and then the other side only: wrap, ring index advances, the other cumulative still exact-adds. - Delta that lands exactly on `2^128` (sum ≡ 0): stored cumulative is zero; the next observation can add again. - Two observations straddle the modulus; target timestamp between them: interpolation matches wrapping math. - `seconds_ago = 0` at `target == latest.timestamp`: returns stored cumulatives and does not add. - Zero reserve and #465 ratio (`1` vs `u128::MAX`): still skip, cumulatives unchanged. - Balanced reserves far from the ceiling: cumulatives identical to today’s checked add (regression). - `compute_twap_price` for a non-wrapping window matches existing tests; wrapping window matches `(end - start) mod 2^128`; `time_elapsed == 0` still errors. - `price_times_dt` overflow still errors and `oracle_update` does not persist a new observation in that case (pin current #1224 behavior so this change cannot wrap the truncated product). **Multitest (`smartcontracts/tests` oracle module)** - Provide, advance time, swap: observations still record on the happy path (existing test). - Seed or drive a cumulative to `MAX - small` with a fitting price, advance one block, swap, provide, and withdraw: each succeeds and reserves change. - Observe `[0, window]` across the wrap: both cumulatives present; client-side wrapping sub reconstructs the price. **Frontend** - `computeTwapPriceDecimalString`: existing non-wrap cases; `cumEnd < cumStart` where the wrapping diff is the real window integral returns that price; elapsed ≤ 0 and zero average stay null. - `getTwapPrices` still returns null prices when `observe` throws, and still returns a price when Observe succeeds across a wrap (mock the response). **Paths that must keep working without an oracle write** - Limit place, cancel, claim, and reprice on a pair whose cumulative is already near `MAX` (they do not call `oracle_update`). Do not require them to start updating the oracle. ## Attack, hack, and abuse These are lock / stale-oracle / bad-integral risks, not a pool drain. Tests seed storage or use the unit harness. Do not add a mainnet reserve-skew walkthrough. - **Accelerate the ceiling.** A representable but large spot (still a successful `checked_from_ratio`) increases atomics per second. On a thin pool that is cheap relative to TVL; on a deep pool it is the existing TWAP-manipulation cost. After the wrap fix, the same trade must **not** freeze swap or LP exit. Assert execute `Ok` and reserves moved. Assert the recorded delta is the full `price × dt`, not a saturated stub. - **Truncated-delta wrap (#1224 conflation).** A test where `atomics * dt` exceeds `u128` must not store `wrapping_mul`’s low bits. That would understate TWAP and look like a successful observation. Expected: no new observation from this ticket’s math (current error or, later, #1224’s skip — not a wrapped lie). - **Saturating or skip-and-freeze regression.** A test that the cumulative is not stuck at `MAX` and that a later second still changes the integral. Frozen TWAP while spot moves is a stale-price bug for any future consumer that treats Observe as a mark. - **False wrap on bad inputs.** `compute_twap_price` is only defined for two snapshots of the **same** counter. A unit test documents that a single modulus crossing reconstructs the small integral. Do not add a public setter. Migrate must not accept a caller-supplied cumulative. - **Interpolation panic as a query DoS.** Stored observations that straddle the modulus must return Ok. A panic or a whole-vector `?` failure would again blank every `seconds_ago` list. - **Argument-swapped subtraction.** Wrapping sub of a reversed window is a huge value. `Decimal::from_atomics` may still succeed. Keep this helper on pair-oracle snapshots in time order. Charts must not feed the result into swap `belief_price` (it does not today; add no such wiring). - **Admin rewrite.** Grep-level test or review note: no new execute arm writes `price_*_cumulative` except `oracle_update`’s wrapping add and the existing first-observation zero seed. - **Observe gas.** The query remains a pure read. Wrapping add must not loop or binary-search more than today’s ring walk. - **Limit-book confusion.** Document in the test name that cancel/claim still succeed when pool swap reverts **before** the fix, so nobody “repairs” the lock by blocking cancels. After the fix, swap and cancel both succeed; escrow accounting is unchanged. - **Decimal-rescale discontinuity.** No test setup should mix pre-scale and post-scale cumulatives. This ticket must not change units. ## Verification - `cd smartcontracts && cargo test -p cl8y-dex-pair oracle_overflow -- --nocapture` - `cd smartcontracts && cargo test -p cl8y-dex-pair oracle_observe -- --nocapture` - `cd smartcontracts && cargo test -p dex-common --lib oracle -- --nocapture` - `cd smartcontracts && cargo test -p cl8y-dex-tests oracle -- --test-threads=1` - `make test-contracts` - Frontend: `make test-frontend` scoped to `oracle.test.ts` and Charts TWAP if those tests change - `make verify-issue-1231` still passes (it already runs the #465 `oracle_overflow` tests and the #1231 Observe tests) - #1224’s `price_times_dt` execute brick stays open; this issue does not close it - Docs: `docs/twap-oracle.md` describes modulo `2^128`; the “handled with errors” sentence is gone - Live pair: code merge does not by itself unbrick columbus-5. Recovery is a later pair wasm migrate that preserves `OBSERVATIONS`. Out of scope to execute here.

PlasticDigits commented

2026-09-22 15:27:36 +00:00

Owner

Instead zero extend u128 into u256

PlasticDigits referenced this issue

2026-09-22 15:55:08 +00:00

fix(pair): widen TWAP cumulative to Uint256 (#1224 #1322) #1323

PlasticDigits referenced this issue from a commit

2026-09-22 17:06:58 +00:00

Merge pull request 'fix(pair): widen TWAP cumulative to Uint256 (#1224 #1322)' (#1323) from fix/oracle-cumulative-u256 into main

PlasticDigits commented

2026-09-22 17:09:44 +00:00

Owner

Wasm from #1323 is on main (c17e71d3). make verify-issue-1322 passed before merge, and Woodpecker ci/woodpecker/pr/woodpecker succeeded on the updated head. The live pair is still the old code. Columbus-5 migrate to cw2 1.18.0, keeping OBSERVATIONS, is #1324.

Wasm from #1323 is on main (`c17e71d3`). `make verify-issue-1322` passed before merge, and Woodpecker `ci/woodpecker/pr/woodpecker` succeeded on the updated head. The live pair is still the old code. Columbus-5 migrate to cw2 1.18.0, keeping `OBSERVATIONS`, is #1324.

PlasticDigits commented

2026-09-23 10:42:53 +00:00

Owner

+1 wasm execute still reverts on the pair oracle price_a cumulative checked-add when the running sum is a very large integer; same defect as this ticket, including the execute path already tracked here. No new constraint.

PlasticDigits referenced this issue

2026-09-24 09:15:36 +00:00

security(oracle): price_times_dt overflow still bricks pair after #465 skip #1224

PlasticDigits commented

2026-09-24 09:28:43 +00:00

Owner

Verified the merged Uint256 fix (#1323) on origin/main at 54c4868e: make verify-issue-1322 passed all 18 checks. O1322-1–O1322-8 are documented and cross-linked with pair/oracle and Charts code, tests, TWAP docs, audit docs, and the third-party agent skill.

Remaining operational follow-ups:

Migrate the affected Columbus-5 pair to cw2 1.18.0 while preserving OBSERVATIONS: #1324.
Resolve or explicitly verify missing-key migrate backfill before broadcast: #1232.

No live-chain migration was run here.

Verified the merged Uint256 fix (#1323) on origin/main at 54c4868e: make verify-issue-1322 passed all 18 checks. O1322-1–O1322-8 are documented and cross-linked with pair/oracle and Charts code, tests, TWAP docs, audit docs, and the third-party agent skill. Remaining operational follow-ups: - Migrate the affected Columbus-5 pair to cw2 1.18.0 while preserving OBSERVATIONS: #1324. - Resolve or explicitly verify missing-key migrate backfill before broadcast: #1232. No live-chain migration was run here.

PlasticDigits closed this issue

2026-09-24 09:28:43 +00:00

PlasticDigits referenced this issue

2026-09-24 09:39:31 +00:00

docs(oracle): clarify TWAP rollout and migration preflight #1339

PlasticDigits referenced this issue

2026-09-24 09:49:16 +00:00

ops: migrate columbus-5 pairs to cw2 1.18.0 and keep OBSERVATIONS #1324

PlasticDigits referenced this issue

2026-09-24 10:28:07 +00:00

ops: verify Columbus-5 pair TWAP migration #1349