perf(server): stop re-reading metadata and rings on every allowance call - #366
Open
TarikGul wants to merge 11 commits into
Open
perf(server): stop re-reading metadata and rings on every allowance call#366TarikGul wants to merge 11 commits into
TarikGul wants to merge 11 commits into
Conversation
`allocate_statement_store_allowance` resolved a LitePeople ring before scanning for a slot, so every `statement_create_proof_authorized` paid a `Members.RingKeys` page walk plus `chain_getBlockHash`, `state_getRuntimeVersion` and `CurrentRingIndex` — even when the account already held a slot for the period and nothing needed submitting. The ring is only ever used to build a proof for an extrinsic. Scan for an existing slot first under the `Ignore` policy and return the allowance secret when one is held, resolving the ring only when a submission is actually required. This matches what `allocate_bulletin_allowance` already does: it reads `TransactionStorage.Authorizations` and returns before opening a People-chain connection at all. `slot::find_allocated_slot` is the query form of `scan_slot_excluding` — it ignores free slots, so a table fully occupied by other accounts answers "no slot held" rather than erroring with `NoFreeStatementStoreSlot`. The `Increase` policy is untouched: it always wants an additional slot, so it goes straight to ring resolution as before. One semantic change worth calling out: an account that already holds a slot no longer has to prove ring membership to receive its own allowance key. Membership is what earns a slot, not what makes an already-granted slot usable, so a user whose membership lapses mid-period keeps working until the period rolls over instead of failing with `MissingLitePeopleMembership`.
Both native allowance paths downloaded the full `state_getMetadata` response on every call. For statement-store that download exists only to read one constant — `Resources.LiteStmtStoreSlotsPerPeriod`, the slot count the scan needs — so the steady state pulled the entire runtime metadata to learn a single `u32`. `MetadataCache` keys decoded metadata by genesis hash and revalidates it with `state_getRuntimeVersion`, a small request that only misses across a runtime upgrade. It lives on `RuntimeServices` next to the existing preimage and statement caches, so it is shared by every product runtime built from one host role, and is keyed rather than per-chain-wrapper so the Asset Hub PGAS path can reuse it without a second cache. Combined with scanning before ring resolution, an already-allocated statement-store product call now costs one `state_getRuntimeVersion` plus the slot scan, where it previously cost a metadata download, two chain-state reads, a ring-index read and a `Members.RingKeys` page walk on top of the same scan. `StatementStoreRpc::genesis_hash` and the `RuntimeServices` field are both gated to non-wasm targets: `statement_allowance` is native-only, so on wasm the accessor would be dead code.
…hash
The metadata cache validated its entry with `state_getRuntimeVersion` and
then `fetch_chain_state` asked for the same runtime version again, so the
submission path made two identical requests. Metadata and `ChainState` are
both fixed for a given runtime, so one entry holds both: `MetadataCache`
becomes `ChainContextCache` returning `ChainContext { metadata, state }`,
and the allowance paths no longer call `fetch_chain_state` at all.
`fetch_chain_state` keeps its signature for `truapi-host-cli` and is now
composed from `fetch_genesis_hash` and `fetch_runtime_version`, the same two
requests in the same order.
Filling `ChainState` from the cache key also closes a gap: the genesis hash
baked into every allowance extrinsic came from `chain_getBlockHash(0)` on
whatever connection the host supplied, and nothing checked it against the
chain the caller asked for. A host that wires `connect()` to the wrong chain
produced extrinsics signed for that chain, failing with an opaque validity
error. `ChainContextCache::get` now compares the two and reports
`GenesisHashMismatch` before downloading metadata.
Per allowance-gated product call, the steady state is one
`state_getRuntimeVersion` plus the slot scan; a submission adds the ring
walk and the extrinsic. Neither re-reads metadata or the genesis hash while
the runtime is unchanged.
Records why the cache needs no eviction policy (one entry per configured chain, not per call) and corrects the crate README, which said the allowance paths need no metadata.
Testing against the live paseo-next-v2 People chain showed the previous
commit's genesis check was wrong in both directions.
`network.rs` configures the People genesis as `c5af1826…`, but the chain
reports `89a63b11…`; Asset Hub has likewise diverged (`bf0488db…`
configured, `23e730eb…` live). Only Bulletin still matches. The testnet was
wiped and the constants were never refreshed — the exact failure RFC-0026
exists to remove.
So a divergence does not mean the host connected to the wrong chain, and
rejecting it broke the allowance path on a network where it had been
working. It also had the polarity backwards: `CheckGenesis` is signed over
`ChainState.genesis_hash`, so the value must come from the chain. Using the
configured constant would have produced extrinsics no node accepts.
`ChainContextCache::get` now keys entries by the caller's constant, since
that is the identity it routes connections by, and fills `ChainState` from
what the chain reports. A divergence is logged rather than fatal.
That split also exposed a cache bug the live run confirmed: the insert keyed
by the reported hash while the lookup used the configured one, so on any
network with a stale constant the cache missed on every call — silently
undoing the previous commit. `a_stale_configured_genesis_still_keys_the_cache`
covers it.
`tests/live_people_chain.rs` holds the checks that found this, `#[ignore]`d
so `cargo test` stays offline:
cargo +nightly test -p truapi-host-cli --test live_people_chain \
-- --ignored --nocapture
They confirm the reported genesis reaches `ChainState`, that a second read
hits the cache (`Arc::ptr_eq` on the metadata), that `find_allocated_slot`
scans a live period without erroring, and that live spec 1000032 still
exposes the `AsResources` variant indices the offline fixture pins at spec
1000000 — (2, 1) and (3, 1) in both.
TarikGul
force-pushed
the
tg/allowances-in-core
branch
from
August 11, 2026 19:29
fc8b9b5 to
9ab10b6
Compare
Imod7
reviewed
Aug 12, 2026
| .lock() | ||
| .expect("chain context cache mutex poisoned") | ||
| .get(&configured_genesis_hash) | ||
| .filter(|cached| cached.state.spec_version == spec_version) |
Contributor
There was a problem hiding this comment.
This predicate accepts a cache entry on spec_version alone, but the entry also carries state.genesis_hash, and that is what gets signed into CheckGenesis. A testnet wipe redeploys the same runtime - same spec version, new genesis - which is the one case this filter cannot see, so the cache keeps signing with the dead chain's hash until the process restarts. main re-read the genesis every call and self-healed. Fetching it via futures::try_join! alongside the runtime version costs no extra latency.
| ) | ||
| .await? | ||
| { | ||
| debug!( |
Contributor
There was a problem hiding this comment.
When the product does not yet hold an allowance for the current period - which, since a period is one UTC day, happens at least once per product per day - this call and the one at line 907 scan the same slots. find_allocated_slot reads all ten seq slots for the period (slot.rs:302-310) and returns None; then register_statement_account → scan_slot_excluding re-derives the same ten aliases and re-reads the same ten keys (slot.rs:266-268). About 430 ms wasted on the first allocation per product per UTC day. Adding a SlotSelection::Full variant would let the one existing scan serve both callers. Watch two things: register_statement_account must map Full back to NoFreeStatementStoreSlot, and truapi-host-cli has a second match that needs the new arm.
| seq, | ||
| "statement-store allowance already allocated" | ||
| ); | ||
| return Ok(allowance.secret.to_bytes().to_vec()); |
Contributor
There was a problem hiding this comment.
Would you add a test for this early return? The helper and the cache are tested separately, but nothing asserts the composition - so a refactor could remove the optimisation entirely with every test still green. The crate already has the idiom: statement_store.rs:677, :919 and :959 assert platform.sent_rpc…is_empty() to prove an early return never reached the chain, and runtime.rs:2464 has a recorded_rpc_method_count helper. Here it needs the counting form rather than is_empty(), since this path does legitimately make the version read and the slot reads - asserting recorded_rpc_method_count(&platform.sent_rpc, "author_submitAndWatchExtrinsic") == 0 plus the same for the RingKeys read would pin it. signing_fixture in this module already wires up the StubPlatform you'd need.
| pub async fn get( | ||
| &self, | ||
| rpc: &RpcClient, | ||
| configured_genesis_hash: [u8; 32], |
Contributor
There was a problem hiding this comment.
This parameter takes the chain's identity separately from the rpc on line 190, so the type cannot check that the two describe the same chain - the caller has to get the pairing right by hand. Could StatementStoreRpc return an already-scoped (RpcClient, ChainContext) instead, so the key never crosses this boundary? That would also make the new genesis_hash() accessor unnecessary - it exists only so a caller can re-pair the key manually.
| configured_genesis_hash: [u8; 32], | ||
| ) -> Result<ChainContext, StatementAllowanceError> { | ||
| let (spec_version, transaction_version) = fetch_runtime_version(rpc).await?; | ||
| if let Some(cached) = self.cached(configured_genesis_hash, spec_version) { |
Contributor
There was a problem hiding this comment.
Two calls arriving at the same time while the cache is still empty will both start the same metadata download, rather than the second one waiting for the first to finish - so you pay ~811 KiB twice, which is the cost this cache exists to avoid. It is reachable: statement_store_allowance_key and bulletin_allowance_key are separate entry points that both land here with the same key, so a product calling both at startup hits it. chain_runtime.rs already handles this by keeping the in-progress fetch around so later callers await it instead of starting their own - reusing that would be nicer than hand-rolling something here.
An entry was reused whenever the spec version matched, but the entry also carries the genesis hash that `CheckGenesis` is signed over. The one case the spec version cannot see is a chain wiped and redeployed from the same runtime: same spec version, new genesis. The entry stayed "valid" and every allowance extrinsic was signed for a chain that no longer exists, until the process restarted. Reading the genesis on every call is what made `main` self-heal through exactly that. Both validation reads are now issued together with `try_join!`, so checking the pair costs no extra round trip, and the entry is reused only when both still match. Reported by Imod7 on #366.
The steady-state check and the registration each ran their own full scan, so the first allocation of a period read all ten slots, derived all ten aliases, then did it again. A period is one UTC day, so every product paid that once a day — in the path this PR exists to make cheap. `scan_slot_excluding` now reports `SlotSelection::Full` instead of raising `NoFreeStatementStoreSlot`, which lets one scan answer both questions: whether an allowance is already recorded, and which slot to claim otherwise. Its result is handed to `register_statement_account` through `RegistrationParams::preselected`, used for the first attempt only — the duplicate-submit retry still rescans, and `register_statement_account` maps `Full` back to the error for callers that scan nothing themselves. `find_allocated_slot` is gone; it existed only because the scan used to error on a full table. Two things fall out. The policy no longer gates the scan: `Increase` wants an additional slot and `Ignore` wants an existing one, and one scan serves both, so `allocate_statement_store_allowance` reads the same regardless. And `Full` is the variant slot eviction will hang off — replacing the oldest replaceable slot is a decision about what to do when the scan reports `Full`. `a_preselected_slot_is_not_rescanned` scripts zero free slots, so ignoring the preselection makes the transport run dry rather than quietly rescanning. Reported by Imod7 on #366.
…ubmit The cache and the slot scan were covered separately, so nothing asserted the thing this PR is for: that an allowance already recorded on chain is served without resolving a ring or submitting anything. A refactor could have deleted the early return with every test still green. `an_existing_allowance_is_served_without_touching_the_ring` drives `allocate_statement_store_allowance` over the `StubPlatform` chain provider and asserts on the requests it made: no `chain_getFinalizedHead`, which is what `find_including_ring` opens with, so no ring was resolved; nothing starting `author_submit`; and exactly one `state_getStorage`, the slot read that answered it. The call is bounded at five seconds. Losing the early return does not produce a wrong answer, it produces a wait on a chain read the stub deliberately leaves unanswered — so without the bound the regression hangs instead of reporting. With it, removing the early return fails in five seconds with the reason. Reported by Imod7 on #366.
Two callers arriving on an empty entry each started their own metadata download, paying the cost the cache exists to avoid. It is reachable without contriving anything: `statement_store_allowance_key` and `bulletin_allowance_key` are separate entry points that land on the same key, so a product using both at startup hits it. A miss now takes a lock and re-checks the entry, so the second caller waits for the first download and then finds it. `chain_runtime` shares one in-flight future for the same purpose, which reads better, but `Shared` requires a `Clone` output and `StatementAllowanceError` is not — erasing it to a string to fit would cost every caller its typed error. `concurrent_misses_download_the_metadata_once` polls two `get` calls through `join!` over a transport that answers by method and yields inside every request, so the misses genuinely interleave rather than serialising by luck. It asserts one download and that both callers come away with the same entry. Reported by Imod7 on #366.
`ChainContextCache::get` took the chain's identity separately from the client reading it, so nothing could check the two described the same chain and the caller had to pair them by hand. That hash keys the cache and is what a reported divergence is measured against, so a mismatch attributes one chain's metadata to another with nothing at the call site to show it. It has already gone wrong once here, between the insert and the lookup. `ChainClient` carries a client together with the genesis hash the host routes it by, and `StatementStoreRpc::chain_client` is the only way to build one for the People chain — so the pairing happens where the hash lives. `get` takes the scoped client and reads both from it. That removes the `genesis_hash()` accessor, which existed only so a caller could re-pair the key manually, and drops two statements from each allocation path. Reported by Imod7 on #366.
Two conflicts, both in `allocate_statement_store_allowance` and `allocate_bulletin_allowance`, where #360 replaced the compiled-in ring-VRF derivation with a registry lookup: let bandersnatch = derive_lite_person_ring_vrf_entropy(&entropy); becomes a reservation read off the active session, let bandersnatch = *signing_host.reserved_lite_person_entropy(&session)?; Resolved by taking main's entropy acquisition and keeping this branch's chain context: the scoped `chain_client`, the cached metadata and chain state, and the single slot scan. The two changes touch adjacent lines for unrelated reasons and neither depends on the other. `fetch_metadata` and `fetch_chain_state` are no longer called on either path — `ChainContextCache` supplies both — so main's calls are dropped rather than merged. Verified on the result: fmt, clippy --all-features -D warnings, 697 workspace tests, wasm32, and the three read-only live People-chain tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Steady state for an allowance-gated product call (
statement_create_proof_authorized,preimage_submit) is onestate_getRuntimeVersionplus the slot scan. A submission addsthe ring walk and the extrinsic. Neither re-reads metadata or the genesis hash while the
runtime is unchanged.
This matches
allocate_bulletin_allowance, which already returns before opening aPeople-chain connection when allowance is in place.
Changes
slot::find_allocated_slot— query form ofscan_slot_excluding; ignores free slots, soa table occupied by other accounts reports "no slot held" rather than erroring. The
Ignorepolicy scans with it first and resolves a ring only when a submission isrequired.
Increaseis unchanged.statement_allowance::ChainContextCacheonRuntimeServices— holdsChainContext { metadata, state }keyed by the caller's configured genesis hash,revalidated with one
state_getRuntimeVersion. One entry per configured chain, so noeviction policy.
ChainState.genesis_hashcomes from the chain, not the caller's constant:CheckGenesisis signed over it, and configured constants drift (paseo-next-v2's People and Asset Hub
hashes are both stale today). Divergence is logged, not fatal.
tests/live_people_chain.rs— read-only live-chain checks,#[ignore]d socargo teststays offline.
sso_responder.rsstatement_allowance.rs(mortalityfield onChainState)sso_responder.rs