Skip to content

fix(0329): scale soroban amm legs by each token's decimals - #395

Merged
stkrolikiewicz merged 13 commits into
developfrom
fix/0329_soroban-token-amounts-assume-seven-decimals
Oct 7, 2026
Merged

stkrolikiewicz merged 13 commits into
developfrom
fix/0329_soroban-token-amounts-assume-seven-decimals

Conversation

@stkrolikiewicz

@stkrolikiewicz stkrolikiewicz commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Soroban AMM legs are now scaled by each token's own decimals instead of a fixed 7. Classic assets and their SACs stay at 7; a pure Soroban token uses its SEP-41 decimals(). This fixes contract-token prices that were off by 10^(7 − decimals): SolvBTC at a tenth of its price, the 18-decimal tokens 10^11 too low. A price past Decimal::MAX drops the trade instead of panicking.
  • A trade whose token has unknown decimals is dropped, never guessed. One decode_resolving step resolves the token over Soroban RPC, persists it to the new prices.asset_decimals, records it, and decodes the ledger again. It is shared by the live ledger processor, events-backfill and sdex-backfill (combined mode). Lookups run concurrently. Absent is terminal per process. A node behind the decoded ledger is retried on the next trade. Transient parks the token for 10 min live and retries inline in backfills.
  • Dropped trades are never silent: the live processor publishes TradesMissingDecimals with a new alarm, and both backfills print trades dropped (decimals): N. The simulateTransaction envelope and its classification move from asset-discovery into prices_ingest_core::soroban_rpc, shared by symbol() and decimals().
  • reingest_0286.py compares volumes only on candles without a contract-token leg, because their volume moves by design. It marks a month with dropped decimals trades DEFECT, refuses an events-backfill binary without the new line, and takes amm-done --decimals in --amm wait. Docs updated: schema §3.10b, the amm-trades-schema decimals question closed, and the 0286 runbook.
  • Deploy order:
    • Apply init.sql (creates prices.asset_decimals) before the ingest; the live processor's init fails without it.
    • Deploy the observability stack for the new alarm.
    • Rebuild events-backfill on the CH host before phase 3 uses the new script.
    • The history repair and the optional seed are in lore 0329.

The ingest needs decimals() from the same simulateTransaction call the
asset-discovery symbol stage makes. Move the envelope builder and the
Absent/Transient classification into prices_ingest_core::soroban_rpc so
both readers share one boundary. symbols.rs re-exports the old names.
amm_trade_to_tick scaled both legs at a fixed 7, so a contract token
with d decimals priced at 10^(7 - d) of market. Classic identities and
their SACs stay at 7; a pure Soroban token uses its decimals() from the
asset registry, and a token whose decimals are unknown yields no tick
and is reported in LedgerSoroban::missing_decimals instead of being
guessed.
Add prices.asset_decimals and a DecimalsResolver that asks a token's
SEP-41 decimals() by simulation, rejects anything above rust_decimal's
scale, and leaves a failed contract alone for ten minutes. The writer
loads the table into the asset registry and writes new answers.
The live processor, events-backfill and sdex-backfill (combined mode)
now resolve the tokens a decode reported missing, persist them, and
decode that ledger again. Cold starts load prices.asset_decimals first.
events-backfill prints the trades it still could not reprice.
Schema overview gains section 3.10b, amm-trades-schema closes its open
decimals question, and the 0286 re-ingest runbook explains the new
trades dropped (decimals) line events-backfill prints.
Comment thread docs/runbooks/0286-reingest-history.md
Comment thread packages/prices-ledger-processor/src/reconcile.rs Outdated
Comment thread packages/prices-ingest-core/src/decimals.rs Outdated
Comment thread packages/prices-ingest-core/src/decimals.rs
Comment thread packages/sdex-backfill/src/ingest.rs Outdated
Comment thread docs/runbooks/0286-reingest-history.md
Comment thread packages/events-backfill/src/run.rs Outdated
Comment thread packages/prices-clickhouse/schema/init.sql Outdated
Comment thread packages/prices-ingest-core/src/canonical.rs Outdated
The decode now counts trades (not legs) dropped for unknown decimals.
The live processor puts them on RunStats, WARNs with the contracts and
publishes TradesMissingDecimals, alarmed like UnregisteredPoolEvents;
sdex-backfill prints them in its summary like events-backfill.
A contract that answers with no usable scale is not asked again by the
process; a Transient failure still retries after ten minutes. The calls
for one ledger run concurrently, so N new tokens cost one RPC timeout.
Per-token decimals move contract-token volumes by design, so the
reconcile compares volumes on the other candles and trades on all. The
script now requires and gates on events-backfill's decimals line.
@adamkoot

adamkoot commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Review at 2b70318. These are new findings; none of them repeats Oskar's comments. Items 2 and 4 are new defects in the fix commits for his points (de3ae5a, c1e0fb1/34152397).

1. packages/prices-ingest-core/src/soroban.rs:894 — CRITICAL

With per-token scales the AMM price quotient can exceed Decimal::MAX, and the plain / at :900-904 panics ("Division overflowed", rust_decimal 1.42). Anyone can halt live ingest on one ledger.

Before this PR both legs were scale 7, so the quotient was raw_out / raw_in and always fit. Now it is (raw_num / raw_den) × 10^(d_den − d_num), and the gap can reach 10^28 (MAX_DECIMALS = 28, Known(0) accepted). Example: a token whose decimals() returns 28, a Soroswap pair with XLM holding 10 raw of it and 100 XLM, swap 1 raw in → ≈9.07e7 stroops out → quotient ≈9.07e28 > 7.92e28. The panic is outside the Err arm, the cursor does not move, and the same ledger panics on every redelivery. Both backfills crash on it too. price_forming is only a flag, so it does not gate the division. Fix: checked_div, drop the trade on None, and add a (28, 7) / (28, 0) regression test.

2. packages/prices-ingest-core/src/decimals.rs:136 — WARNING

Since de3ae5a, "contract not found on the RPC node" is parked as Absent for the life of the process, so a just-deployed token's trades are dropped until a cold start.

result.error maps to Absent, and decimals_rpc_it::a_contract_that_was_never_deployed_is_absent pins that for a missing contract. The live processor sees a token's first swap seconds after the ledger closes. If the RPC backend is a ledger behind, decimals() simulates against a contract that does not exist yet → Absent → parked. Before de3ae5a this healed after 600 s. Fix: read latestLedger from the simulate response, and treat the error as Transient when latestLedger is below the ledger being decoded.

3. infra/src/lib/stacks/observability-stack.ts:1482 — WARNING

The alarm's remedy, "insert the row into prices.asset_decimals by hand", does nothing on a warm processor.

The table is read only at cold start (load_registry). After that, a missing token goes only to DecimalsResolver, which never asks again for a token parked as Absent. The operator inserts the row and reprices, but the warm Lambda keeps dropping the token's trades, so every minute after the repair is lost again. Fix: check asset_decimals for the missing contracts before calling RPC, or at least say in the alarm text that a forced cold start is needed.

4. tools/scripts/reingest_0286.py:909 — WARNING

The trades dropped (decimals) DEFECT gate works only in ssh/mtls mode. --amm wait + amm-done never records the count.

Only amm_summary sets ms["decimals"]. The wait branch (:946-955) reads just --fallbacks from amm.done, and the prompt at :950 does not mention the new line. An operator who runs events-backfill by hand, sees trades dropped (decimals): 40 and runs amm-done … --fallbacks 0 gets the month marked done with those trades missing. Fix: add --decimals to amm-done (or have it parse the log through amm_summary), store it in amm.done, and set ms["decimals"] in the wait branch.

5. packages/prices-ingest-core/src/decimals.rs:31 — WARNING

The 600 s wall-clock parking of Transient failures also applies to events-backfill and sdex-backfill, where 10 minutes covers days of ledgers.

A single 429/5xx when a token is first met parks it, and every trade of that token for the next 10 minutes of run time is dropped. The month gets a non-zero trades dropped (decimals), the orchestrator marks it DEFECT, and it has to be re-run. Fix: in backfill mode, retry Transient inline with a short backoff (or park by ledger count instead of Instant). Keep the 600 s window for the live Lambda.

6. packages/prices-ledger-processor/src/reconcile.rs:171 — WARNING

The live resolve → persist → re-decode → TradesMissingDecimals path has no test, and the resolver is hard-wired to from_env() (mainnet RPC).

None of these has a test: the re-decode, the write_decimals → record order, the flushed-minute filter, the metric, or the sdex-backfill count. Any reconcile test whose fixtures contain a pure-Soroban swap (reconcile_e2e, reconcile_drain) now sends real simulateTransaction calls to mainnet.sorobanrpc.com. Fix: inject the resolver (a trait, or rpc_url in Reconciler::new), and add one test with a fake that returns Known(8) and one with a fake that returns Transient.

7. packages/prices-ingest-core/src/soroban.rs:2411 — nit

The mixed-decimals test only sells the token, so the inverted branch (amount_in / amount_out, volume_base = amount_out) is never covered with mixed scales. A swapped decimals_in/decimals_out would hide there. Add the mirror case that buys the token with USDC.

8. packages/prices-ingest-core/src/decimals.rs:217 — nit

Instant::now() - RETRY_AFTER panics when the host has been up for less than 600 s (on Linux, Instant counts CLOCK_MONOTONIC from boot), which is possible on a freshly provisioned CI VM. Use checked_sub, or inject a clock.

- checked_div on the AMM quotient: per-token scales can push it past
  Decimal::MAX, where the plain division panicked and stalled ingest.
- A contract error from a node behind the decoded ledger (latestLedger)
  is Behind, asked again next trade, not parked as Absent.
- Backfills retry a Transient failure inline (1/5/20 s) instead of
  parking it for ten minutes of wall clock.
- One decode_resolving for all three callers, tested with a fake
  resolver; reconcile tests inject an offline resolver.
- The alarm says a hand-inserted row needs a cold start.
--amm wait recorded only the fallbacks, so a hand run with dropped
trades could be marked done. amm-done now requires --decimals and the
script refuses an amm.done without it.
@stkrolikiewicz

stkrolikiewicz commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

Thanks, all eight addressed in 0731c6e / a2bdd1a / f44ced6:

  1. Overflow panic. The quotient now uses checked_div and the trade is dropped on None. Regression test a_price_past_decimal_max_drops_the_trade_instead_of_panicking covers (28, 7), your 9.07e28 case, which now returns None, and checks (28, 0) in both directions for no panic. In that pair the canonical order puts the 28-decimal leg in the numerator, so the quotient is tiny there.
  2. Absent on a lagging node. simulate takes as_of. A contract error from a node whose latestLedger is below the decoded ledger is Behind, which is never parked and is asked again on the token's next trade. Pinned by a_contract_error_from_a_node_behind_the_ledger_is_not_a_fact.
  3. Alarm remedy. The text now says a hand-inserted row needs a forced cold start (any function configuration update), and that the reprice comes after it.
  4. --amm wait. amm-done requires --decimals, writes it to amm.done, and the wait branch sets ms["decimals"]. An amm.done without the count is refused. The prompt and runbook §6 mention it.
  5. Backfill parking. DecimalsResolver::backfill retries a Transient failure inline (1/5/20 s) and does not park. live keeps the 600 s window.
  6. Tests. The resolve → persist → record → re-decode step is now one decode_resolving, shared by all three callers and generic over a ResolveDecimals trait. It is tested with a fake for Known (persisted, then recorded, then priced), unresolved (counted, nothing persisted), and a failed persist (nothing recorded). Reconciler::with_decimals_resolver points every reconcile test at a refusing address, so none can reach mainnet. A plausible case was the static Comet pool's BLND SAC in an empty registry. The Galexie fixtures are gitignored, so I could not prove it either way.
  7. Inverted branch. Added the_inverted_branch_scales_each_leg_by_its_own_decimals (8/18/6).
  8. Instant underflow. The test no longer subtracts from Instant; it sets the resolver's retry window to zero instead.

@stkrolikiewicz
stkrolikiewicz merged commit 24e07cc into develop Oct 7, 2026
4 checks passed
stkrolikiewicz added a commit that referenced this pull request Oct 7, 2026
First post-deploy prices match the market. All 46 historical contract
tokens are seeded and Hetzner's events-backfill is the #395 build, so
0286 phase 3 needs no redo; Oskar restarts on the new script before 2025-07.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants