Skip to content

Feat/prover db 3 consumer#188

Open
rishitesh-snt wants to merge 18 commits intomainfrom
feat/prover-db-3-consumer
Open

Feat/prover db 3 consumer#188
rishitesh-snt wants to merge 18 commits intomainfrom
feat/prover-db-3-consumer

Conversation

@rishitesh-snt
Copy link
Copy Markdown
Contributor

@rishitesh-snt rishitesh-snt commented Apr 28, 2026

Rationale for this change

Replaces the offchain_worker stub with the OCW HTTP-forwarding consumer that drains the producer queue, asks the server for its last checkpoint, and posts protobuf payloads (create_table / drop_table /
put_batches / checkpoint). Adds http_client.rs, the prover-db.proto wire format generated via prost-build, and 4 integration tests covering the OCW path.

Adds the operator-facing wiring for the upcoming prover-db-indexer pallet:
a `--prover-db-url` CLI flag that, at startup, seeds the URL into the OCW
persistent local-storage key `prover_db_indexer::prover_db_url` before the
first block is authored. Also surfaces a clear error if `--prover-db-url`
is set without `--enable-offchain-indexing=true`, since the pallet's
on_finalize writes would otherwise be silently dropped.

The storage key is kept as a local literal in `node/src/service.rs` so
this change is independent of the pallet, which is added in a follow-up
PR. The literal must stay in sync with `pallet_prover_db_indexer::PROVER_DB_URL_KEY`
once that pallet lands; the next PR refactors the node to import it.

The flag is inert until the pallet ships, so this PR cannot affect the
running system.
@rishitesh-snt rishitesh-snt requested review from a team as code owners April 28, 2026 14:30
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 28, 2026

1.69.0

Bug Fixes

  • add basic documentation to runtime crate (30c26e7
  • add missing docs for runtime module items (52a2905
  • add missing docs to chain-utils (c8235c6
  • address warnings in on-chain-table (095a856
  • address warnings in pallet-rewards mock module (886bc36
  • allow deprecated CurrencyAdapter usage (cfe0517
  • allow missing docs in private items in test_end_row_limits binary (7700cd6
  • allow missing_docs_in_private_items on event forwarder contract (b2d6f8b
  • allow unused private_key variables in pallet-keystore benchmarks (c218f0d
  • declare anonymous lifetime in commitment-sql proptest helper (63b3fc0
  • declare anonymous lifetimes in canaries parse functions (f81e9de
  • document all items in watcher main module (2fe60db
  • document memory_commitment_map module in commitment-map (32a38e3
  • document missing items in canaries (72c5f33
  • document missing items in event-forwarder (5408075
  • document modules in chain-utils (3eb6545
  • document modules in rpc crate (19cf1af
  • don't hide elided lifetimes in backwards compatibility test case generator (4e6968e
  • don't import frame_benchmarking if runtime-benchmarks is disabled in node (32cfca3
  • expect AttestationInfo::block_number to be dead code in canaries (7753c44
  • expect some dead code in NewFullBase in node (fe06f5e
  • ignore unused error variable in watcher main module (d330fe5
  • ignore unused variable in attestation-tree test (ce8adf3
  • ignore unused variable in pallet-attestation benchmarks (b0ef4a7
  • implement benchmark configurations for runtime outside runtime api implementation (d1a3db8
  • only define migrations with runtime-benchmarks disabled (26e8804
  • pallet: silence clippy::manual_inspect from FRAME macro expansion (5ec8511
  • privatize deposit_event in pallet-rewards (6c1caaf
  • privatize deposit_event in pallet-smartcontracts (fe43a19
  • privatize deposit_event in pallet-system-tables (8bf7c21
  • privatize deposit_event in pallet-tables (da951a5
  • privatize pallet-keystore deposit_event (e878e37
  • privatize pallet-permissions deposit_event (099a7ba
  • privatize system-contracts deposit_event (305dfc9
  • privatize zkpay deposit_event (9d5993c
  • propagate tui error in update_ui in watcher (fe943eb
  • remove dead code in chain-utils (0070600
  • remove dead code in node (6958ec6
  • remove unused block_number parameter from verify_attestations in watcher (6225426
  • remove unused block_number parameter from verify_signature in watcher (08f3bff
  • remove unused const in pallet-tables test (e7bf9f5
  • remove unused const in runtime test (c7e0add
  • remove unused dependencies from sxt-core (361f8a0
  • remove unused destructure variables in verify_attestation in watcher (dc04d24
  • remove unused import in memory_commitment_map test (4c2300a
  • remove unused import in pallet-attestation benchmarks (5fa662e
  • remove unused import in pallet-commitments migrations (ecf6a05
  • remove unused imports and format imports in chain-utils (5cd269b
  • remove unused imports in attestation-tree (91f5d61
  • remove unused imports in node (1f4c7a8
  • remove unused imports in pallet-smartcontracts (b7fa723
  • remove unused imports in pallet-tables (a08ccfd
  • remove unused imports in runtime (614a1e3
  • remove unused imports in test_end_row_limits (0328143
  • remove unused imports in watcher (340d27b
  • remove unused substrate key path from watcher client (2bd4d02
  • switch hex to being a dev dependency in rpc crate (7b01208
  • warn unused_crate_dependencies in sxt-core (62c85b7

Features

Reverts

  • node: restore cli.rs to main version (a32605e

- Hoist `PROVER_DB_URL_KEY` into `sxt-core` so node and the
  prover-db-indexer pallet share one canonical key (with `/` as the
  domain separator, matching substrate convention).
- Tighten the CLI doc since it surfaces in `--help`; drop the internals.
- Enable clap `env` feature so `PROVER_DB_URL` env var also configures it.
- Drop the eprintln! warning when offchain indexing is off without
  `--prover-db-url` — that's the typical case, not worth shouting.
- Pass `&Cli` into `new_full_base` instead of an owned URL, so future
  options can be threaded through without further signature changes.
- Hoist `use codec::Encode` to the file's import block.
@rishitesh-snt rishitesh-snt force-pushed the feat/prover-db-3-consumer branch from 18e54e3 to 172ab5e Compare April 29, 2026 05:41
Standardize on "prover-db indexer URL" across `sxt_core`, the node CLI,
and the service-side seeding helper. Drop the pallet-internal references
(`on_finalize`, `sp_io::offchain_index::set`) from the runtime check —
that scaffolding lands in follow-up PRs and shouldn't leak into PR1.
@rishitesh-snt rishitesh-snt force-pushed the feat/prover-db-3-consumer branch from 172ab5e to 5fdbcb6 Compare April 29, 2026 08:01
Per review: sxt-core groups items by the pallet they belong to (with the
exception of `IdentLength`/`ByteString`). Move `PROVER_DB_URL_KEY` out of
the top-level into a new `prover_db_indexer` module to follow that
pattern. The node now imports it as
`sxt_core::prover_db_indexer::PROVER_DB_URL_KEY`.
Introduces `pallet-prover-db-indexer`, an offchain worker that will
forward table lifecycle and `pallet_indexing` quorum events to an
external prover-db indexer over HTTP.

This PR ships only the **producer** side (`on_finalize`):

- Reads `frame_system::Events`, downcasts via `RuntimeEvent: TryInto<...>`
  (no SCALE re-encoding), extracts `pallet_tables::{SchemaUpdated,
  TablesCreatedWithCommitments, TableDropped}` and
  `pallet_indexing::QuorumReached` into a `BlockIndex` of `BlockEvent`s.
- Persists non-empty `BlockIndex`es to offchain storage keyed by block
  number using `sp_io::offchain_index::set` (a no-op unless the node
  was started with `--enable-offchain-indexing=true`).

The **consumer** half (HTTP forwarding via `offchain_worker`) lands in
the follow-up PR; until then `offchain_worker` is a no-op stub. Because
producer writes are gated by `--enable-offchain-indexing` at the node
level — and the operator-facing flag added in the previous PR refuses
to seed `--prover-db-url` without it — landing this PR alone cannot
affect a running system.

Wiring:
- Workspace member + workspace dep entry in root `Cargo.toml`.
- Runtime registers the pallet at `pallet_index(111)` with
  `Config<native_api::Api>`.
- `node/src/service.rs` switches its URL-key literal over to
  `sxt_runtime::pallet_prover_db_indexer::PROVER_DB_URL_KEY`, replacing
  the local placeholder introduced in the previous PR.
`#[pallet::hooks]` expands into code that uses `map_err` over `inspect_err`,
which `clippy --all-features` flags under `clippy::manual_inspect`. The
hit is in macro-generated code, not in our hook bodies, so allow it on
the pallet module's existing allow list.
Replaces the no-op `offchain_worker` stub with the OCW HTTP-forwarding
consumer that drains the producer's offchain DB queue and posts
protobuf payloads to the configured prover-db indexer.

The OCW path:
  1. Reads the indexer URL from offchain local storage
     (`PROVER_DB_URL_KEY`); skips the round if unset.
  2. Asks the server for its last checkpoint via `/v1/get_last_checkpoint`
     — server is the sole source of truth, so we never diverge from
     what's actually been committed.
  3. Walks blocks `cursor+1..=tip` (capped at `MAX_BLOCKS_PER_INVOCATION`),
     forwards each `BlockEvent` (`create_table` / `drop_table` /
     `put_batches`), checkpoints on the server, deletes consumed
     entries from the offchain DB.

Adds:
- `src/http_client.rs` — `sp_io::offchain::http` POST helpers and
  `/v1` endpoint wrappers (create_table, drop_table, put_batches,
  checkpoint, get_last_checkpoint).
- `proto/prover-db.proto` + `build.rs` — wire-format definitions
  generated via `prost-build`; included in `lib.rs` as `mod proto`.
- `offchain_index::{read, clear}` — consumer-side helpers to load
  and delete entries the producer wrote.
- `src/{mock.rs, tests.rs}` — mock runtime + 4 integration tests
  covering the OCW path: skip-when-unconfigured, single-block forward
  with deletion, empty-block checkpointing, server-checkpoint resume,
  multi-block in-order processing.
- Cargo deps: `prost`, `prost-build`, and the dev-deps the mock
  runtime needs (`pallet-permissions`, `pallet-commitments`, etc.,
  `parking_lot`, `proof-of-sql-commitment-map`).
Surface OCW HTTP traffic and failures so operators can diagnose issues
against a real prb-service without rebuilding the node:

- New `endpoint()` helper trims a trailing `/` on the base URL before
  joining a path, so a `url::Url`-derived base (which normalizes to
  end in `/`) no longer produces `host:port//v1/...` and a 404.
- Capture the response body on non-200 and surface it inline in the
  warn log + the `Status` error variant, so 4xx/5xx reasons (route not
  found, bad payload) are visible.
- Add warn logs on Request::send / try_wait / response errors,
  carrying the URL and the underlying offchain http error variant.
- Add debug logs at consumer-round start, per POST request, and per
  POST response (status + body size).
- Include URL in the existing get_last_checkpoint warn so the operator
  can see exactly which endpoint failed.
@rishitesh-snt rishitesh-snt force-pushed the feat/prover-db-3-consumer branch from d6690b1 to d8585ca Compare April 30, 2026 12:20
Comment on lines +69 to +81
/// `host:port//v1/...` when callers pass a `url::Url` (which normalizes
/// to include a trailing `/` on the path component).
fn endpoint(base_url: &str, path: &str) -> String {
format!("{}{}", base_url.trim_end_matches('/'), path)
}

/// Fetch the last checkpoint from the server.
pub fn get_last_checkpoint(base_url: &str) -> Result<Option<u64>, Error> {
let url = endpoint(base_url, "/v1/get_last_checkpoint");
let body = post(&url, &[])?;
let resp = proto::GetLastCheckpointResponse::decode(body.as_slice())?;
Ok(resp.has_checkpoint.then_some(resp.sequence_number))
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have thought using url.join("v1/get_last_checkpoint") would work as you'd expect, based off the docs

arrow_schema,
key,
};
let url = endpoint(base_url, "/v1/create_table");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like these extra &'static str should just be constants in this file.

}

Ok(response_body)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is still kinda big. Would you be opposed to splitting it into one that adds the http client and one that uses it in the OCW?

Comment on lines +211 to +215
log::warn!(
target: "prover_db_indexer",
"POST {} -> status {}; body ({} bytes): {}",
url, status, response_body.len(), snippet,
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I mentioned this on the last PR already, but in this one too the logging should probably be done with sp_tracing

/// Fetch the last checkpoint from the server.
pub fn get_last_checkpoint(base_url: &str) -> Result<Option<u64>, Error> {
let url = endpoint(base_url, "/v1/get_last_checkpoint");
let body = post(&url, &[])?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these really all POST requests? I'm somewhat surprised this one isn't a GET but maybe it's because there's some HTTP -> GRPC translation going on that assumes post?

);

// 2. Current block number.
let current_block: u64 = polkadot_sdk::frame_system::Pallet::<T>::block_number()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just get this passed in by offchain_worker? It is called with a block number

record_batch: entry.data.clone(),
}],
)
.map_err(|_| "put_batches failed")?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer the errors of these two functions to be expressed with a Snafu enum, and to .to_string() them as needed

start, end, cursor, current_block,
);

for block_num in start..=end {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So here's a question - Theoretically an offchain workers keep getting spawned per block-ish. If a previous offchain worker is not finished forwarding, and a new one begins, does this logic do anything to prevent them from submitting out of order? Does it need to (i.e., does the PRB reject anything below MAX(META_ROW_NUMBER), or does it just reject duplicates)?

…trait

- sxt-core: add EventCapture trait, types, offchain key helpers
- pallet-prover-db-indexer: drop on_finalize, impl EventCapture with on-chain manifest
- pallet-tables: Config::EventCapture; capture at SchemaUpdated/TableDropped
- pallet-indexing: Config::EventCapture; capture at QuorumReached
- runtime: wire EventCapture, add MaxEventsPerBlock=2048
- mocks: type EventCapture = () in tables/indexing/rewards/system_tables/smartcontracts
- README: drop on_finalize/CLI-specific/server-side sections
- node/cli: --prover-db-url help notes offchain-indexing/worker flags
…ucer

# Conflicts:
#	node/src/cli.rs
#	sxt-core/src/lib.rs
#	sxt-core/src/prover_db_indexer.rs
…ain high-water-mark

- sxt-core: rename key_for_manifest -> key_for_high_water; value semantics shift from Vec<u32> to u32
- pallet-prover-db-indexer: drop CurrentBlockManifest storage, MaxEventsPerBlock Config type, and on_initialize hook; capture_events writes event blob + hwm only
- runtime: drop MaxEventsPerBlock parameter_types and Config wiring
- pallet Cargo.toml: drop log dep (overflow warn path is gone)
…allet wrapper

- pallet: Pallet<T, I> -> Pallet<T>, Config<I> -> Config; instance was vestigial since the supertrait bound on pallet_indexing::Config<I> was dropped
- delete native_pallet.rs: no instance to pin, runtime can reference Pallet<Runtime> directly
- Cargo.toml: drop native-api dep
- runtime: drop ::native_pallet:: path segment in three places
Drop consumer/OCW, HTTP wire contract, OCW URL, and OCW scheduling
sections — those land with the consumer PR.
…e capture, hwm probe

Merge feat/prover-db-2-producer (extrinsic-time capture via EventCapture trait) into PR 3, then rewrite the consumer to read the new offchain key shape.

- lib.rs: drop on_finalize/extract_block_index/try_extract_*; keep offchain_worker hook calling run_consumer; rewrite forward_block to read key_for_high_water then probe key_for_event(block, 0..=hwm); split forwarding loop into forward_block (per-block) + forward_events (per-extrinsic)
- offchain_index.rs deleted (BlockIndex/key_for_block scheme retired)
- offchain_consumer.rs added: OCW-only read_high_water/read_events/clear_high_water/clear_events helpers
- Cargo.toml: drop pallet-tables/pallet-indexing/native-api main deps; keep prost/log/build-deps and dev-deps
- mock.rs: minimal frame_system + pallet only (Config no longer pulls in tables/indexing/balances/staking/etc.)
- tests.rs: seed offchain DB via key_for_event + key_for_high_water; add a test exercising the multiple-extrinsics-in-one-block probe path
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.

2 participants