Skip to content

indexer: incremental supply counters, keyset pagination, holders, order book (closes #2129) - #2130

Merged
nullPointerEnjoyer merged 22 commits into
masterfrom
feature/indexer-supply-pagination
Oct 5, 2026
Merged

nullPointerEnjoyer merged 22 commits into
masterfrom
feature/indexer-supply-pagination

Conversation

@nullPointerEnjoyer

Copy link
Copy Markdown
Collaborator

Closes #2129.

Supply statistics

/statistics/coin and /statistics/token/{id} were slow because get_all_statistic resolved each counter with a window scan over the whole ml.statistics history (the primary key is (statistic, coin_or_token_id, block_height), which cannot serve a filter on coin_or_token_id alone). This PR rewrites it as an indexed per-statistic latest lookup (DISTINCT ON over the four counters), making both endpoints point queries.

The counters themselves were already maintained incrementally and reorg-safe by the scanner (height-versioned rows + del_statistics_above_height rollback), so no new supply tables are added — this deliberately deviates from the issue's sketch of ml.supply/ml.token_supply tables. Total minted stays derivable as circulating_supply + burned. The storage version is bumped 26 → 27, so existing deployments rescan from genesis, which rebuilds the new caches in the same pass (no separate backfill).

Keyset pagination for /pool and /transaction

  • ml.latest_pool_data_cache holds the newest state of each pool (creation height included), maintained same-pass by the scanner and rolled back on reorgs; /pool no longer runs a ROW_NUMBER window over the entire per-block pool history. Side effect: the pledge sort now orders by a numeric column instead of a textual byte comparison (which mis-ordered amounts of different widths).
  • Both /pool and /transaction accept an opaque URL-safe cursor (next_cursor in the response; an empty cursor= parameter starts the listing). The offset protocol keeps working unchanged; when both are sent the cursor takes precedence. A tx_global_index index backs the transaction cursor.

Holders endpoints

The scanner now maintains ml.address_amount (latest balance per address and asset, numeric column, reorg-rolled-back). New endpoints:

  • GET /api/v2/statistics/coin/holders
  • GET /api/v2/statistics/token/{token_id}/holders

Both list holders by balance, descending, with cursor pagination.

Order book endpoint

The scanner now maintains ml.latest_orders_cache (remaining ask/give balances per order, numeric columns, reorg-rolled-back; concluded orders leave the book via zero balances, frozen orders are excluded from reads). New endpoint:

  • GET /api/v2/order/pair/{pair}/book?side=ask|bid

It aggregates the remaining balances into price levels (remaining give/ask ratio, quoted in the quote currency per base unit) with cursor pagination. The existing /order/pair/{pair} endpoint is unchanged.

Tests

  • storage backend suite: pool keyset pagination, top holders, order book entries + reorg rollback, on both postgres and in-memory backends
  • scanner: statistics reorg rollback test
  • endpoint tests: order book paging and validation, coin holders paging, pool and transaction cursor pagination vs the offset listing

Deployment note

Version bump → one-time rescan from genesis on upgrade; postgres_max_connections unchanged (the rewrite strictly reduces query load; the new endpoints add only small indexed reads).

Supply statistics (closes the indexer part of #2129):

- get_all_statistic now resolves each counter (circulating supply, staked,
  burned, preminted) with an indexed per-statistic latest lookup instead of a
  window scan over the whole statistics history, making /statistics/coin and
  /statistics/token/{id} O(1) per counter. The counters themselves were
  already maintained incrementally and reorg-safe per block height, so no new
  supply tables are needed; total minted remains derivable as circulating
  supply + burned.
- storage version bumped to 27: existing deployments rescan from genesis,
  which rebuilds the new caches in the same pass.

Keyset pagination for /pool and /transaction:

- ml.latest_pool_data_cache holds the newest state of each pool (including its
  creation height), maintained by the scanner and rolled back on reorgs, so
  /pool no longer scans the whole per-block pool history; the pledge sort now
  orders by a numeric column instead of a textual byte comparison.
- both endpoints accept an opaque URL-safe cursor (next_cursor field in the
  response, empty cursor = first page); the offset protocol keeps working.
- an index on ml.transactions.tx_global_index backs the transaction cursor.

Holders endpoints:

- the scanner maintains ml.address_amount (latest balance per address and
  asset, numeric column, rolled back on reorgs).
- new endpoints /statistics/coin/holders and /statistics/token/{id}/holders
  list holders by balance with cursor pagination.

Order book endpoint:

- the scanner maintains ml.latest_orders_cache (remaining ask/give balances
  per order, rolled back on reorgs; concluded orders drop out via zero
  balances, frozen orders are excluded from reads).
- new endpoint /order/pair/{pair}/book?side=ask|bid aggregates remaining
  balances into price levels (remaining give/ask ratio) with cursor
  pagination; /order/pair/{pair} stays unchanged.
@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

🔍 OpenCodeReview found 11 issue(s) in this PR.

  • ✅ Successfully posted inline: 1 comment(s)
  • 📋 Routed to summary by policy: 9 comment(s)
  • ⏭️ Skipped (overlap with history): 1 comment(s)

⚠️ 1 warning(s) occurred during review.


maintainability · low

📄 api-server/stack-test-suite/tests/v2/statistics.rs (L514-L523)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category maintainability)

This test duplicates ~70 lines of server/state setup boilerplate (listener spawn, storage init, scan_genesis/scan_blocks, ApiServerWebServerState construction) already present in nearly identical form in the other new tests in this suite (orders.rs, pools.rs, transactions.rs) and in existing tests here. Consider extracting a shared test helper that builds the web server state from a chain-config plus produced blocks, so future changes to state construction (e.g. new CachedValues fields) only need one edit.


performance · low

📄 api-server/api-server-common/src/storage/impls/in_memory/mod.rs (L97-L99)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category performance)

Storing a full ChainConfig clone just to encode bech32 sort keys duplicates a potentially large struct (genesis block included). The struct already uses Arc for genesis_block; wrapping the config in Arc<ChainConfig> (or storing only the pieces Address::new needs) avoids the unnecessary deep copy while keeping the same API.

💡 Suggested Change

Before:

    // kept so that order/pool identifiers can be encoded exactly like the postgres
    // backend does (bech32 address strings), keeping tie-break orders identical
    chain_config: ChainConfig,

After:

    // kept so that order/pool identifiers can be encoded exactly like the postgres
    // backend does (bech32 address strings), keeping tie-break orders identical
    chain_config: Arc<ChainConfig>,

test · low

📄 api-server/storage-test-suite/src/basic.rs (L2579-L2580)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category test)

The ordering assertion only validates the first tuple element (ask_balance). If the documented ordering contract includes a secondary sort key (e.g. give_balance or order id) for entries with equal ask_balance, it is not verified here — random_order makes collisions unlikely, so this stays untested. Consider asserting the full tuple ordering, or seeding two orders with identical ask_balance but different give_balance to pin the tie-break behavior.

💡 Suggested Change

Before:

    // the entries come back with the deepest orders first (the ordering contract)
    assert!(entries.windows(2).all(|pair| pair[0].0 >= pair[1].0));

After:

    // the entries come back with the deepest orders first (the ordering contract)
    assert!(entries.windows(2).all(|pair| (pair[0].0, &pair[0].1) >= (pair[1].0, &pair[1].1)));

test · low

📄 api-server/stack-test-suite/tests/v2/transactions.rs (L385-L391)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category test)

The cursor-walk baseline here is fetched with the legacy offset mode (/api/v2/transaction?offset=0&items=100), while the walk uses the new cursor mode. The comparison assert_eq!(paged_txs, expected_txs) therefore implicitly asserts that both modes produce the same (newest-first) order. That holds today, but if the legacy offset mode ever changes its ordering this test will fail with a confusing mismatch far from the cause. Consider fetching the baseline through the cursor path itself (or adding a brief comment noting the order-equivalence assumption), similar to how the orderings are cross-checked in the transactions handler.


maintainability · low

📄 api-server/web-server/src/api/v2.rs (L1504-L1505)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category maintainability)

The pools path duplicates the address canonicalization (parse → into_object → re-encode) that canonical_address_string implements for the holders endpoints, just with inline typed errors. Two independent copies of a security-relevant normalization are prone to divergence; reuse the helper (mapping None to InvalidCursor) or have canonical_address_string return the same error type.

💡 Suggested Change

Before:

                        let pool_address =
                            Address::from_string(&state.chain_config, &id).map_err(|_| {

After:

                        let pool_id = canonical_address_string(&state.chain_config, &id)
                            .ok_or(ApiServerWebServerError::ClientError(
                                ApiServerWebServerClientError::InvalidCursor,
                            ))?;

bug · low

📄 api-server/web-server/src/api/v2.rs (L2709-L2709)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category bug)

Several expect() calls in this request-handler path (checked_div/checked_rem on the scaled price, u128::try_from(rem)) panic on broken invariants rather than returning a server error, unlike the rest of this function which carefully maps overflow to an internal server error. The invariants hold today (denominator validated at cursor decode, digits < 10), but a panic inside an async handler takes down the task; a Result-based fallback would be consistent with the surrounding defensive style.

💡 Suggested Change

Before:

    let scaled = scaled.checked_div(&Uint256::from(denom)).expect("denominator is not zero");

After:

    let scaled = scaled.checked_div(&Uint256::from(denom)).ok_or_else(overflow)?;

maintainability · low

📄 api-server/api-server-common/src/storage/impls/postgres/queries.rs (L264-L266)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category maintainability)

The fabricated fragments ("$4;", "$2;") exist only to push the const-computed parameter count, and placeholder_count_1 parses single-digit $n indices only. If any real fragment ever gains a placeholder index >= 10 (e.g. $10), the parse silently reads 1, the array length no longer matches the query text, and the intended compile-time guard degrades into a runtime error. This is documented in comments, but a cheap safeguard would be a const assertion that no fragment contains a two-digit placeholder (e.g. scanning for $ followed by two digits and failing via a const panic), so the single-digit assumption is enforced rather than assumed.

💡 Suggested Change

Before:

const CURSOR_PARAM_COUNT: usize =
            placeholder_count(QUERY_HEAD, CURSOR_PREDICATE, QUERY_TAIL, "$4;");
        const NO_CURSOR_PARAM_COUNT: usize = placeholder_count(QUERY_HEAD, QUERY_TAIL, "$2;", "");

After:

// const-panic if a two-digit placeholder ever appears, keeping the
        // single-digit assumption of placeholder_count_1 enforced:
        const _: () = assert!(placeholder_count_1(QUERY_HEAD) < 10);
        const CURSOR_PARAM_COUNT: usize =
            placeholder_count(QUERY_HEAD, CURSOR_PREDICATE, QUERY_TAIL, "$4;");
        const NO_CURSOR_PARAM_COUNT: usize = placeholder_count(QUERY_HEAD, QUERY_TAIL, "$2;", "");

maintainability · low

📄 api-server/api-server-common/src/storage/impls/postgres/queries.rs (L59-L62)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category maintainability)

placeholder_count and placeholder_count_1 are both actively used (in const contexts) by get_top_address_amounts below, so #[allow(dead_code)] is stale on the usage sites' helpers; it suppresses future legitimate dead-code warnings if the only callers are ever removed. Unless required by a known const-eval lint false positive, drop the attributes.


performance · low

📄 api-server/api-server-common/src/storage/impls/postgres/queries.rs (L2247-L2258)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category performance)

The no-cursor variant repeats the exact query text that could be derived from QUERY_HEAD/QUERY_TAIL (as the cursor branch does via format!). Duplicating it as a literal here means an edit to the shared constants (e.g. adding a filter to QUERY_HEAD) would not apply to this branch, partially defeating the stated goal that the two variants cannot drift apart. Consider assembling both variants from the same constants.


⚠️ Warnings:

  • api-server/api-server-common/src/storage/impls/in_memory/mod.rs (token_budget_reached): skipped round 2 of group "api-server/api-server-common/src/storage/impls/in_memory/mod.rs,api-server/api-server-common/src/storage/impls/in_memory/transactional/read.rs,api-server/api-server-common/src/storage/impls/in_memory/transactional/write.rs": used 584641 tokens exceeds budget 500000

Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs
Comment thread api-server/web-server/src/api/v2.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs Outdated
Addresses the first automated review pass:

- items=0 is now rejected by the shared pagination parameter parser: the
  cursor endpoints fetch items+1 rows and read the last returned row to build
  the next cursor, so an accepted zero page size made all four cursor
  endpoints panic remotely (requests return 400 instead).
- the in-memory pool listings now filter pools by a non-zero staker balance,
  matching the postgres implementation (decommissioned is not equivalent:
  pledge/rewards can be zero while the staker balance is not, and vice versa).
- the holders' address and the pool id columns are declared with the C
  collation so ordering and cursor comparisons are plain byte order,
  deterministic across database clusters and identical to the in-memory
  backend's ordering.
- the order book price decimal field is computed with exact integer
  arithmetic (u128 mantissa scaled in a u256) instead of f64; the fraction
  string is built by the shared decimal formatter. An f64 remains only as an
  unreachable-in-practice fallback if the exact mantissa overflows u128.
- endpoint tests: zero page size is rejected on /pool and /transaction.
@nullPointerEnjoyer

nullPointerEnjoyer commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator Author

OCR cycle log

Cycle Commit Run Findings Disposition
1 e36cc98 37070978644 7 (4 high, 3 medium) All fixed in de27544: items=0 now rejected by the shared pagination parser (remote panic on the four cursor endpoints); in-memory pool filters aligned with postgres (staker_balance != 0); COLLATE "C" on address/pool_id columns for deterministic byte-order tie-breaks; exact integer price decimals (u256-scaled mantissa). Threads replied + resolved.
2 de27544 37072083099 9 (1 high, 8 medium) 8 fixed in c1de28d (set-based holders rollback with constant round trips; order-book aggregation capped at 10k orders/query; holders cursor validates the tie-break address; zero-denominator guard in ratio reduction; exact integer formatting on the price fallback path; stronger endpoint tests: exact page content, baseline truncation assert, items=0 cases). 1 answered without code change (fresh-schema concern): the storage version bump 26 -> 27 makes the scanner drop and rebuild the schema from genesis on upgrade — the established indexer upgrade mechanism, noted in the PR description. Threads replied + resolved.
3 c1de28d 37073683181 7 (2 high, 5 medium) All fixed in 95dcaf9: order-book reads now return (entries, has_more) with the 10k cap enforced identically in both backends (in-memory got the same cap, ordering, and bech32 order-id tie-breaks; it now keeps the chain config for exact encoding parity) and the endpoint surfaces truncation as an additive truncated field; explicit byte-order collation on every ordering-relevant ORDER BY plus trait docs; deterministic pool-listing tie-breaks in both backends; tests assert the raw backend order; the one expect in a handler maps to an internal server error. Threads replied + resolved.
4 95dcaf9 37076092374 6 medium (+1 leftover c3) All fixed in 9c39d5e: sort keys (bech32-encoded order/pool ids) are encoded once per entry instead of once per comparison; the pledge listing decodes the balance once and treats an unparseable balance as absent (no expect on a read path); the order-book handler defensively skips zero-balance entries (zero-denominator price would corrupt the level map ordering); cursor-walk tests are bounded by the known item count so a broken cursor fails instead of hanging. Leftover cycle-3 tie-break thread also resolved.
5 9c39d5e 37077636512 9 (2 high, 7 medium) Fixed in eeb1503: the holders and pools keyset predicates now expand the row comparison with an explicit COLLATE "C" on the tie-break (no reliance on default-collation resolution), and every pool ordering pins it; latest_orders_currencies_index covers the book read's sort columns so the pair scan needs no separate sort; the order-book price decimal is rendered with exact u256 digit extraction — the floating-point fallback is gone (an impossible scaling overflow now returns an internal server error instead of a lossy value); the storage suite pins the equal-balance tie-break and its cursor boundary. Two findings answered as deliberate design: the in-memory pool filter matches the deployed postgres semantics (cycle-1 parity fix), and per-page re-aggregation is bounded by the 10k cap with the truncated flag.
6 eeb1503 37079275419 5 medium Fixed in 4c25c29: the orders-cache reorg restore names its columns instead of SELECT *; CoinOrTokenStatistic::VARIANTS carries a compile-time exhaustiveness guard (wildcard-less match in the const initializer); a transactions cursor index that does not fit an i64 is rejected as a 400 InvalidCursor instead of a 500; the holders test baseline asserts itself against the page limit; the order-book handler documents that a paginated walk is not a consistent snapshot of a moving book.
7 4c25c29 37080851629 4 (1 high, 3 medium) The high was based on a misread: a v26 database IS dropped and rescanned on the version mismatch (scanner-daemon compares the stored version with the current one and calls reinitialize_and_rescan), so no in-place migration exists or is needed; is_initialized now documents that true does not mean the version is current. Fixed in 674fab6: cursors carry an endpoint tag with deny_unknown_fields and foreign cursors are rejected as InvalidCursor (with a cross-endpoint 400 test); reduce_rational pins its zero-denominator invariant with a debug assertion. The creation-height premise is answered by the same rescan path (plus MIN-height self-healing on rollback).
8 674fab6 37082386049 3 (1 high, 2 medium) Fixed in 165afb7: a pools cursor height that does not fit an i64 is rejected as a 400 InvalidCursor before the storage layer (the panic path was remotely reachable via a forged cursor); the in-memory sort-key helpers propagate AddressableError instead of expecting (matching postgres); the decorated sorts compare references so no string key is cloned.
9 165afb7 37083963842 0 Clean pass.
10 2c74e88 (trigger) 37085199288 0 Second consecutive clean pass — review gate satisfied (0 findings twice in a row). Security review of the full diff posted as comment: no new remotely exploitable issues (parameterized SQL, read-only endpoints, validated/capped inputs, forged-cursor panic paths fixed during review, reorg-rollback coverage).
11 6ee00f2 37104084771 0 Third consecutive clean pass. This push fixed the failing 'Static code checks' workflow: codecheck.py requires the exact repo license header and cursor.rs line 8 pointed at opensource.org instead of the canonical LICENSE URL. All CI green on 6ee00f2 (build, static checks, OCR, permutation, wasm).

Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs Outdated
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Comment thread api-server/stack-test-suite/tests/v2/transactions.rs
Comment thread api-server/storage-test-suite/src/basic.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs Outdated
- set-based reorg rollback for the holders table (constant round trips instead of per-row queries)
- cap order book aggregation at 10k deepest orders per query
- validate the holders cursor tie-break address; guard reduce_rational against zero denominators
- exact integer price formatting via 256-bit truncation (no float fallback inconsistency)
- tests: items=0 rejected on transaction/pool listings, exact holder page content, baseline truncation assert
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs Outdated
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs Outdated
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Comment thread api-server/api-server-common/src/storage/storage_api/mod.rs Outdated
Comment thread api-server/api-server-common/src/storage/storage_api/mod.rs Outdated
Comment thread api-server/storage-test-suite/src/basic.rs
Comment thread api-server/web-server/src/api/v2.rs Outdated
- order-book reads return (entries, has_more) with the 10k cap enforced in both
  backends; the endpoint surfaces truncation as an additive "truncated" field
- deterministic tie-breaks everywhere: order ids and pool ids sort by their encoded
  (bech32) address strings in byte order in both backends (in-memory storage now
  keeps the chain config for that); explicit COLLATE "C" in the holders ORDER BY
- in-memory pool listings sort by the same keys as postgres (creation height then
  encoded address; pledge then encoded address)
- tests assert the raw backend order instead of re-sorting
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs Outdated
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs Outdated
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs Outdated
Comment thread api-server/stack-test-suite/tests/v2/statistics.rs
Comment thread api-server/web-server/src/api/v2.rs
Comment thread api-server/web-server/src/api/v2.rs
- encode order-id/pool-id sort keys once per entry in the in-memory backend
  (decorate-sort-undecorate) instead of once per comparison
- the pledge listing decodes the balance once and treats an unparseable balance
  as absent instead of expecting in a production sort
- the order-book handler defensively skips zero-balance entries so a future
  storage change cannot corrupt the level ordering
- cursor-walk endpoint tests are bounded by the known item count
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs Outdated
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs Outdated
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs Outdated
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs Outdated
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs Outdated
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs
Comment thread api-server/web-server/src/api/v2.rs Outdated
- the holders and pools keyset predicates expand the row comparison so the
  byte-order (C) collation of the tie-break is explicit rather than relying on
  the column default; the pool ORDER BYs pin it too
- latest_orders_currencies_index now covers the book read's ordering so the pair
  scan needs no separate sort
- the order-book price decimal is rendered exactly (u256 digit extraction) with
  no floating-point fallback; an impossible scaling overflow becomes an internal
  server error
- the storage test pins the equal-balance tie-break and its cursor boundary
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Comment thread api-server/api-server-common/src/storage/storage_api/mod.rs
Comment thread api-server/stack-test-suite/tests/v2/statistics.rs
Comment thread api-server/web-server/src/api/v2.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs
- VARIANTS gets a compile-time exhaustiveness guard so a new statistic variant
  cannot be silently omitted from the latest-state lookup
- the orders-cache reorg restore names its columns instead of SELECT *
- a transactions cursor index that does not fit an i64 is rejected as an invalid
  cursor (client error) before reaching the storage layer
- the holders test baseline checks itself against the page limit; the order book
  handler documents that a walk is not a consistent snapshot of a moving book
Comment thread api-server/api-server-common/src/storage/impls/mod.rs
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Comment thread api-server/web-server/src/api/cursor.rs
Comment thread api-server/web-server/src/api/v2.rs
- cursors carry an endpoint tag and reject unknown fields, so a cursor minted
  by one listing can no longer be decoded by another (with a test)
- reduce_rational pins its zero-denominator invariant with a debug assertion
- is_initialized documents that true does not mean the storage version is
  current; the caller must compare versions and reinitialize on mismatch
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs Outdated
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs Outdated
- a pools cursor height that does not fit an i64 is rejected as an invalid
  cursor (client error) before reaching the storage layer
- the in-memory sort-key helpers propagate an encoding error instead of
  panicking on a mismatched chain config, matching the postgres backend
- the decorated sorts compare references instead of cloning string keys
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs
Comment thread api-server/web-server/src/api/v2.rs
Comment thread api-server/web-server/src/api/v2.rs
@nullPointerEnjoyer

Copy link
Copy Markdown
Collaborator Author

Security review of the PR diff

Scope: the full diff against master (20 files, ~3,000 insertions): storage schema additions (holder amounts, pool/order caches), keyset cursor pagination, four new read endpoints, scanner counter/reorg paths, and their tests. This was a focused source-first review of the changed surface, not a full-repo audit.

Verdict: no new remotely exploitable vulnerabilities found. Basis:

  • Injection: every added SQL statement (25 query/execute call sites) is fully parameterized ($1..$n binds); no user-controlled string is interpolated into SQL. format! usage in the diff is confined to test URL construction and log/JSON formatting.
  • Attack surface: the four new endpoints (/statistics/coin/holders, /statistics/token/:id/holders, /order/pair/:pair/book, plus cursor modes on /pool and /transaction) are read-only GET handlers on the existing public API; no new write paths are exposed remotely. Storage writes happen only inside the scanner, which is not remotely reachable.
  • Unauthenticated input handling: all remote inputs are validated and capped — page sizes bounded (1..=100, zero rejected), cursors are base64 JSON with an endpoint tag, deny_unknown_fields, strict key parsing (u64/u128/address validation) and i64 range checks before values reach the storage layer (previously panics on forged cursors — fixed during review, commits 4c25c29, 165afb7). All remotely reachable expect/unwrap panic paths found by the review process (empty page size, zero-balance book entries, out-of-range cursor indices, sort-key encoding) were eliminated or converted to typed client/server errors.
  • Resource exhaustion: every listing is keyset-paginated with server-side LIMIT; the order-book read is capped at 10,000 orders with truncation surfaced to clients; aggregation is O(capped rows).
  • Data integrity: the height-versioned supply counters and the new cache tables roll back on reorgs via del_*_above_height paths that are exercised against a real Postgres instance in the storage test suite (including the data-modifying-CTE snapshot pitfall), and the scanner storage version bump forces a from-genesis rescan, so counter/cache invariants are rebuilt rather than migrated in place.
  • Error handling: storage failures in the new handlers are logged server-side and returned as generic internal errors; database error details are not echoed to clients.

Residual notes (informational, no action required): a paginated walk over a moving book is not a consistent snapshot — documented on the handler; ORDER_BOOK_MAX_ORDERS bounds re-aggregation cost per request; balances/holders data is already public on the indexer (per-address endpoints predate this PR).

This review covers the diff only; it does not claim coverage of pre-existing indexer code, deployment configuration, or the upstream dependency stack.

@nullPointerEnjoyer
nullPointerEnjoyer marked this pull request as ready for review October 3, 2026 01:48
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs Outdated
- Canonicalize the address id when minting and decoding holders cursors:
  Address::from_string keeps the input verbatim and bech32 accepts an
  all-uppercase encoding, which would corrupt the byte-order keyset
  comparison. Re-encode via Address::new like the pools listing does.
- Extract sorted_live_pools() shared by the offset and keyset pool
  listings, removing the duplicated decorate/filter/sort block, and
  document the in-memory backend's full-scan cost characteristics.
- Comment the intentional <= conflict guard on the
  latest_pool_data_cache upsert (same-height reorg replacement).
- Document the items=0 -> 400 behavior change across paginated v2
  endpoints in the changelog, and that the order book truncated flag is
  informational (levels beyond the cap are unreachable by cursor walk).
Comment thread api-server/web-server/src/api/v2.rs Outdated
Make the order book cursor tag side-specific so a cursor minted for one side is rejected on the other, guard next-cursor minting against an empty page instead of expecting, log on the zero-denominator price fallback, document the floored decimal price and the ignored transaction cursor tie-break, and use expect with context in tests. The postgres pool cache upsert becomes a monotone envelope update (min creation height, max latest state) so late lower-height writes correct the creation height like the in-memory backend and the rollback path; covered by a new shared pool_data_keyset block and a rollback-coupling note in top_address_amounts.
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Comment thread api-server/web-server/src/api/v2.rs Outdated
Cap the encoded cursor length before the base64 decode, filter zero-balance pools before the bech32 sort-key encoding, compute the order book price scale by exponentiation, fail the holders listing with a server error when a stored address cannot be re-encoded canonically, collate the pool listing tie-break explicitly, and document the offset-mode items guard and the coin ticker case-insensitivity. No behavioral change for well-formed requests.
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs Outdated
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Mirror the holders cursor tuple to the returned row shape (address, amount) across the trait, both backends and call sites, reuse canonical_address_string on the decode side, guard the reduce against a zero gcd, reject empty trading-pair segments with the accurate error, extend the pledge index with the tie-break column, skip empty versioned maps in the in-memory pool listing, and drop unused test bindings. No behavioral change for well-formed requests.
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Comment thread api-server/web-server/src/api/v2.rs
Propagate an undecodable pool staker balance as a storage error instead of silently shrinking the in-memory listing, fold the versioned balance delete and the holders re-derivation into one helper so the ordering dependency is structural, include the failing address/order in balance parse errors, check the creation block height cast, note the DISTINCT ON omission semantics, document pool listing page stability and the canonical address encoding contract, and expect instead of unwrap in the orders test setup.
Share one constant query text between the cursor and no-cursor top-addresses variants so the ordering and the cursor predicate cannot drift apart, include the offending identifier and the underlying error in address-decode failures instead of the bare AddressableError, skip empty versioned maps without expect in the in-memory pledge listing, document the transaction listing ordering contract, and simplify the statistics test bootstrap. No behavioral change for well-formed storage.
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Comment thread api-server/web-server/src/api/v2.rs Outdated
Re-encode pool-id cursors through Address::new so keyset boundaries use canonical bech32; document orders-cache rollback invariant; propagate staker-balance decode errors in sorted_live_pools; validate cursor key counts; add context to decode errors.
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Comment thread api-server/stack-test-suite/tests/v2/pools.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs Outdated
Reject holders cursors above the max representable amount; make reduce_rational return Option so a zero denominator surfaces as an internal error; validate pools cursor key count; add debug asserts to the transactions pagination bounds; document holders upsert staleness policy; compile-time parameter-count check for the top-holders queries; expect the pools test web server.
Comment thread api-server/stack-test-suite/tests/v2/statistics.rs Outdated
Comment thread api-server/stack-test-suite/tests/v2/statistics.rs Outdated
Comment thread api-server/web-server/src/api/v2.rs
Comment thread api-server/web-server/src/api/v2.rs
Compile-time placeholder checks now read the actual query constants instead of a duplicated literal; balance-write cache upsert rationale documented; in-memory sort-key errors carry context; statistics VARIANTS ordering contract documented; holders cursor doc specifies atom-string encoding; order book aggregation cost documented; test web servers expected consistently.
Comment thread api-server/api-server-common/src/storage/impls/postgres/queries.rs
Cursor decode rejects key vectors above a small constant bound; the order book cursor walk stops (no next_cursor) when the storage cap truncated the aggregation, so a partial book is never continued; the book cursor rejects a non-empty tie-break id; the transactions pagination debug assert checks the fetch bound instead of restating the page condition; scanner statistics tests fail loudly on a missing key; the in-memory id sort keys share one helper.
Comment thread api-server/api-server-common/src/storage/impls/in_memory/mod.rs
Comment thread api-server/storage-test-suite/src/basic.rs
Comment thread api-server/storage-test-suite/src/basic.rs
Move query-text constants before the executable statements in the holders query (items_after_statements is denied on production code); restore the reorg statistics helper to treat a missing statistic as zero, which is the intended meaning after a rollback, with the rationale documented.
Comment thread api-server/web-server/src/api/v2.rs
@nullPointerEnjoyer
nullPointerEnjoyer merged commit cb1a748 into master Oct 5, 2026
21 checks passed
@nullPointerEnjoyer
nullPointerEnjoyer deleted the feature/indexer-supply-pagination branch October 5, 2026 06:49
nullPointerEnjoyer added a commit that referenced this pull request Oct 6, 2026
…tion

indexer: incremental supply counters, keyset pagination, holders, order book (closes #2129)
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.

Indexer: incremental supply statistics (scanner counters) + keyset pagination for /api/v2 listings

2 participants