fix(pair): Observe query panics on Decimal::from_ratio after #465 #1231
Labels
No labels
agent:fix_bugfix
agent:fix_conflicts
agent:fix_security
agent:gap_analysis
agent:implement
agent:implement
agent:implement
agent:open_issues
agent:ready
agent:research
agent:security_audit
agent:verify
architecture
backend
blocker:hybrid
blocker:launch
blocker:limit-orders
blocker:v2
block:log_only
block:security
bug
ci
contracts
correctness
deploy
dev
devops
docs
documentation
duplicate
e2e
enhancement
epic
feature
frontend
functional-completion
gas
good first issue
governance
help wanted
high-risk
hooks
hybrid
indexer
infra
infrastructure
integrators
invalid
launch-blocker
limit-orders
localnet
localterra
low priority
missing-implementation
needs-design
ops
performance
priority
high
priority
medium
product
qa
QA
question
ready
ready
research
scripts
security
security-hardening
smartcontracts
tech-debt
testing
ux
UX
v2
verification
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
code/cl8y-dex-terraclassic#1231
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
QueryMsg::Observestill panics on an extreme reserve ratio. Closed #465 replaced panickingDecimal::from_ratiowithDecimal::checked_from_ratioinoracle_updateonly (swap / provide / withdraw). The query extrapolation helperoracle_observe_singlestill uses the unwrapping constructor atsmartcontracts/contracts/pair/src/contract.rs:424–425.This is not a second LP brick. Observe is read-only; a panic cannot freeze
RESERVESor lock withdrawals. It is a CosmWasm VM abort on a public query: LCD returns a runtime error instead of TWAP cumulatives. After #465, a pair can keep executing with a lopsided ratio (observation skipped) while every live Observe poll that extrapolates from current reserves panics.Related but not this ticket:
from_ratiopanic / pair brick. AC verified; do not reopen.price_times_dtoverflow on execute after a representable ratio. That issue already marks query-onlyoracle_observe_singleas out of scope. Do not fold this into #1224.Bundle (do not split): checked ratio on the Observe extrapolation branch, non-panic degrade, unit tests next to
oracle_overflow_tests, no execute-path rewrite.Current codebase
oracle_observe_single(called fromquery_observefor eachseconds_ago):Decimal::from_ratioischecked_from_ratio(...).unwrap(). Ratio aboveDecimal::MAX(u128::MAX / 1e18) panics.QueryMsg::Observe { seconds_ago }mapsContractErrortoStdError::generic_err. A panic never becomes that error; the VM aborts.latest_obs) does not callfrom_ratio. The bug is the forward extrapolation used whenseconds_ago == 0ortargetis after the last stored observation — the path indexers use for “now.”oracle_update(execute) already:On
Err, itreturn Ok(())(skip sample).oracle_overflow_testscover onlyoracle_update.Why this is needed
A pair that hit the #465 skip still has live, extreme
RESERVES. The next Observe with a target afterlatest_obs.timestamprecomputes spot from those reserves and panics. Indexer TWAP / candles / listing adapters that poll Observe then look like an LCD outage for that pair, while swap/LP execute may still succeed.Expected vs actual
Decimal::MAXContractError::Oraclefrom_ratiooracle_updateextreme ratioprice_times_dtoverflowConstraints / guardrails
checked_from_ratioskip.price_times_dtexecute brick) in this ticket. If Observe’s existingprice_times_dt(...).map_err(ContractError::Oracle)already returnsErrinstead of panicking, leave that as a typed query error unless a one-line checked path is required to avoid panic. Do not widen the execute helper “while here.”Decimal::MAX(clamp can bias TWAP). For Observe, returning the last stored cumulatives (same as the zero-reserve branch) is the consistent fail-open for query liveness. A typedOracleerror is acceptable if docs say Observe may error on unrepresentable spot; a panic is not.seconds_agosemantics for representable ratios.MAX_PAIR_ASSET_DECIMALS.Decimalonly. No floats. Nounwrap/expecton ratio construction.readyvia labels.Relevant files
smartcontracts/contracts/pair/src/contract.rs(oracle_observe_single)from_ratio; zero-reserve sister branchsmartcontracts/contracts/pair/src/contract.rs(query_observe,QueryMsg::Observe)ContractErrortoStdErrorsmartcontracts/contracts/pair/src/contract.rs(oracle_overflow_tests)smartcontracts/testspair oracle query tests (if present)Observe { seconds_ago: [0] }does not VM-panic on lopsided reservesRecommended direction
oracle_observe_single’s forward-extrapolation branch, replace bothDecimal::from_ratiocalls withDecimal::checked_from_ratio(same operand order asoracle_update:price_a = reserve_b/reserve_a,price_b = reserve_a/reserve_b).Err: do not panic. Preferreturn Ok((latest_obs.price_a_cumulative, latest_obs.price_b_cumulative))(skip extrapolate). Alternative:Err(ContractError::Oracle { reason: ... })so LCD gets a contract error. Pick one and test it; skip-extrapolate matches #465 “missed sample.”price_times_dt+checked_addas typedOracleerrors (already non-panic). Do not switch them to unwrap.oracle_overflow_tests(or a siblingoracle_observe_overflow_tests):reserve_a = 1,reserve_b = Uint128::MAX,seconds_ago = 0,block_time > latest_obs.timestamp→ no panic; skip or typed error.reserve_a = MAX,reserve_b = 1) → same.dt, not frozen at last obs).oracle_updatetests unchanged and green.Acceptance criteria
Decimal. Whenoracle_observe_single/QueryMsg::Observeextrapolates (seconds_ago == 0or target after last observation). Then the call does not panic. It returns last cumulatives or a typedContractError::Oracle/StdError.oracle_updateruns. Then stillOkwith skip (#465). Observe hardening must not reintroduce execute panics.dt > 0. When Observe extrapolates. Then cumulatives still advance (not always equal tolatest_obs).seconds_agothat interpolates two stored observations (no spot ratio). When Observe runs. Then behavior unchanged.extreme_ratio_degrades_gracefully_instead_of_panickingandnormal_ratio_still_records_observationstay green.price_a_cumulatives/price_b_cumulatives) for the success path.Test plan (functional paths)
oracle_observe_single,reserve_a=1,reserve_b=Uint128::MAX,seconds_ago=0,block_time > last_tsreserve_a=MAX,reserve_b=1dt > 0,seconds_ago=0target == latest_obs.timestampcardinality_initialized >= 2QueryMsg::Observe { seconds_ago: [0] }throughqueryStdErroror success JSON; never VM panicoracle_updateextreme ratioTest plan (attack, hack, and abuse)
seconds_agolist with mixed 0 and in-window values, extreme reservesoracle_updateRESERVES/OBSERVATIONSseconds_agovec (existing gas/limits)Decimal::MAX“fix”from_ratioon execute “for consistency”checked_from_ratioDo not publish a mainnet or CW20-hook recipe. Unit tests with
Uint128::MAX/1are enough.Verification criteria
cd smartcontracts && cargo test -p cl8y-dex-pair oracle_overflow(existing #465 + new observe cases).cd smartcontracts && cargo test -p cl8y-dex-pair oracle_observe(if new module name).cd smartcontracts && cargo test -p cl8y-dex-tests oracleif that suite covers Observe queries.make test-contracts(or the repo’s documented contract gate) green.contract.rs: noDecimal::from_ratioon the Observe extrapolation path; execute comments for #465 remain accurate.Out of scope
price_times_dtexecute overflow / LP lock.kwidening (#464).First-pass model recommendation
Recommendation: grok-high
Rationale: Production CosmWasm pair TWAP query in
oracle_observe_single(contract.rs) plus oracle overflow tests. Founder-required contracts / wasm (model-policy invariant 5) even though the edit is local to one helper. Wrong degrade (clamp vs skip vs panic) changes integrator TWAP. Not Composer-eligible. Verify with pair unit tests for extreme-ratio Observe plus existing#465oracle_overflow_tests.cl8y-agent-control: queued
implementjob9e0548ce-fa2d-4688-9974-9c764475d7da(not executed; no Hetzner VM).Merged onto
origin/mainvia #1236.make verify-issue-1231was 14/14. Observe useschecked_from_ratioand skips (no clamp).Leftover: pair wasm store+migrate so listed columbus-5 pairs actually skip on overflow instead of panicking. Bundled with #1227 / #1230 in a new ops issue. Not #1232 (missing-key backfill).
This PR also landed
.woodpecker.yaml(gitleaks). Live Woodpecker still did not postci/woodpecker/pr/woodpecker; merges used a local gitleaks scan plus a Forgejo status. Follow-up CI issue filed separately.Follow-up ops ticket: #1246. Woodpecker enablement: #1247.
columbus-5 wasm for this ticket is not live. Ops tracker: #1246.
Live pairs are 11639 / 1.16.0 (#712). LCD
HybridSimulationbelief_price: "0"still 200 (same output as omitted belief) — #1230 / #1227 / #1231 execute/query wasm still needs a 1.17.0 store+migrate (gitCONTRACT_VERSIONis still 1.15.0). Tax listed pin is 11630 (not 11611/11619); ALPHAterra1x6e64…is 1.0.0 and needs a tax cw2 bump + CMM migrate for #1228 / #1237.columbus-5 pair wasm is live: 11664 / cw2 1.17.0 (store
DB35943A4925059E770A63C9ADDF35622696088A4DFFC8A91110466034995961). Factorypair_code_id=11664,GetPairCount=20. LCD UST1/cUSTCHybridSimulationbelief_price:"0"→Invalid belief_price 0. Ops #1246.