Feat/prover db 2 producer#187
Conversation
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.
1.69.0Bug Fixes
Features |
- 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
force-pushed
the
feat/prover-db-2-producer
branch
from
7f05b5b to
4d8b6cb
Compare
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
force-pushed
the
feat/prover-db-2-producer
branch
from
4d8b6cb to
1acbe12
Compare
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.
rishitesh-snt
force-pushed
the
feat/prover-db-2-producer
branch
from
1acbe12 to
1a60cac
Compare
|
|
||
| Watches for these events from other pallets and relays them: | ||
| - `pallet_tables::SchemaUpdated` | ||
| - `pallet_tables::TablesCreatedWithCommitments` |
There was a problem hiding this comment.
FYI I believe this one is irrelevant now, we don't really support creating a table w/ an initial commitment and snapshot url. The event and extrinsic are still there for backwards compatibility but almost nobody has permission to use them
| As of commit `dd873ed`, the node boots with a loud `warning:` to stderr | ||
| if this flag is not set; running with `--prover-db-url` *without* the flag | ||
| is a hard startup error. |
There was a problem hiding this comment.
This commit isn't in main since it was squashed, but also I think we removed this warning since most nodes will not be indexing to provers
| impl pallet_prover_db_indexer::Config for Runtime { | ||
| type RuntimeEvent = RuntimeEvent; | ||
|
|
||
| // Used to resolve pallet_indexing's pallet index dynamically via | ||
| // PalletInfoAccess::index(), so reordering construct_runtime! never | ||
| // breaks the filter silently. | ||
| type IndexingPallet = Indexing; | ||
|
|
||
| // Variant index of `QuorumReached` within pallet_indexing::Event. | ||
| // The runtime supplies a resolver that looks the variant up by name | ||
| // at startup via scale_info::TypeInfo. See | ||
| // `DynamicQuorumReachedIndex` in runtime/src/lib.rs. | ||
| type QuorumReachedVariantIndex = DynamicQuorumReachedIndex; | ||
| } |
There was a problem hiding this comment.
This is outdated. but also, I think it's probably not worth including in this README? Anybody looking to implement this pallet in their runtime would learn how by just looking at the api reference for this trait
| with `--features indexer` (see the sxtdb repo), then launch | ||
| `sxt-node` with `--prover-db-url http://<prb-service-host>:<port>`. | ||
| The harness at `spaceandtimefdn/sxt-int-harness` (standalone crate) | ||
| drives table/data actions against the node via a TOML file. |
There was a problem hiding this comment.
This section doesn't seem necessary to include in the readme, especially the unit tests, seems destined to become outdated.
| --offchain-worker=always \ | ||
| --enable-offchain-indexing=true \ | ||
| --prover-db-url http://127.0.0.1:9999 | ||
| ``` |
There was a problem hiding this comment.
I think the docs of this pallet should be pretty agnostic of the CLI args. I think it would make sense to say, generally, that the complete functionality of this offchain worker requires enabling offchain indexing, offchain workers, and the prover db url storage to be set. And then maybe mention in the --prover-db-url help text that the other two CLI flags need to be enabled for complete functionality. The point being that this pallet's docs aren't making assumptions about the CLI defined by the node that uses this pallet.
| log::debug!( | ||
| target: "prover_db_indexer", | ||
| "on_finalize({}): writing {} events to offchain DB", | ||
| block_number, | ||
| index.events.len(), | ||
| ); |
There was a problem hiding this comment.
Correct me if I'm wrong, but I believe logs within the runtime should be performed with sp_tracing macros, not log
| this in `finalize_quorum`: it converts the indexer's Arrow IPC | ||
| submission to an `OnChainTable`, appends commitment meta columns, | ||
| and postcard-serializes the result. The prover-db-indexer never | ||
| decodes this payload — it's an opaque pass-through to the server. |
There was a problem hiding this comment.
NIT: I'm not super against this being mentioned here, but it does feel like it would be more appropriate as documentation of the Prover db api, not of this pallet.
| Arrow schema lacks a `META_ROW_NUMBER BIGINT NOT NULL` column. If your | ||
| on-chain table DDLs don't include this column, the first forward | ||
| attempt will fail at the server. Either add the column to the DDL or | ||
| patch the forwarder's `DEDUP_KEY_COLUMN` constant. |
There was a problem hiding this comment.
This doesn't really seem relevant to this pallet. It's relevant to the tables/indexing pallets that generate the META_ROW_NUMBER, and the PRB api that handles it in this special way, but I don't think this pallet does or will do anything with it
| // indexing quorum yields one, anything else yields nothing). | ||
| // The extractors are pure: they take an event and return what | ||
| // would be appended, leaving accumulation to the caller. | ||
| let events = polkadot_sdk::frame_system::Pallet::<T>::read_events_no_consensus() |
There was a problem hiding this comment.
If we go with this approach, I think we should use read_events_for_pallet instead. This one reads all system events so it has serious weight implications, I think
| // ─── PRODUCER ─────────────────────────────────────────────────── | ||
| // Runs during block execution (including sync). Captures events | ||
| // and persists them to the offchain DB for the OCW to consume. | ||
| fn on_finalize(n: BlockNumberFor<T>) { |
There was a problem hiding this comment.
Sorry I didn't realize this sooner, but I don't think we can use this hook for indexing. Any code that runs synchronously in the runtime needs to be benchmarked and "weights calculated" so that the chain can make sure not to schedule too much work into one block. So, the docs for the Hooks trait say that the weight for on_finalize needs to be accounted for and returned by on_initialize.
However, I don't think we can know the weight. The weight would depend greatly on the number of inserts and table creations in the block, which we have no way of knowing on_initialize. We may be able to know this about the previous block, and just use on_initialize to index all events from the previous block, but I'm not 100% sure about that.
This leans me back towards thinking we need to be collecting these events into some intermediate storage during the relevant extrinsics.
2 participants
Rationale for this change
Introduces pallet-prover-db-indexer with the producer side: on_finalize reads frame_system::Events, extracts pallet_tables::{SchemaUpdated, TablesCreatedWithCommitments, TableDropped} and
pallet_indexing::QuorumReached into a BlockIndex, and persists it to offchain storage. Wires the pallet into the runtime at pallet_index(111); offchain_worker is a no-op stub until PR 3.