chore: 🤖 address the reviews of the holdings, identity, schema and post-resync branches - #361
Open
prashantasdeveloper wants to merge 46 commits into
Open
prashantasdeveloper wants to merge 46 commits into
prashantasdeveloper wants to merge 46 commits into
Conversation
`KeyRole` and `KeyRoleEnum` were told apart only by a suffix that carries no information, and they spelled the same concept two different ways (`Primary`/`Secondary` against `PrimaryKey`/`SecondaryKey`), which is why a mapping existed between them at all. They are now `IdentityKeyRole` (the role held within one membership interval) and `AccountKeyRole` (the key's current role, a superset that can also say the key sits outside the identity system). Both spell the shared values the same way. Each enum's docstring now states which it is and how the two relate. Nothing downstream depends on the old names: both enums are new in this unmerged stack.
Eight entities carried an `eventId` column with no stated rule, and the reason for keeping any of them was recorded in one place only. The column is not simply redundant — filters and aggregates are generated over an entity's own columns, so `createdEvent.eventId` can be selected but not filtered or grouped on — but that argument only holds where event type is a query axis. Kept, each with the same docstring: `PolyxEntry`, `StakingEvent`, `AssetTransaction`, `AssetAgentAction`. Dropped from `Account`, which restated `updatedEvent`, and from `DistributionPayment`, which restated `reclaimed`. The rule is recorded in the entity-provenance plan so the next column is decided rather than copied.
`AssetTransaction`, `InstructionEvent`, `Funding`, `DistributionPayment`, `BridgeEvent` and `StakingEvent` are never read back anywhere in `src/`, so no handler could ever update one and `updatedEvent` always equalled `createdEvent` — a redundant non-null relation, plus its index, on some of the largest tables in the database. Two entities that the design table also listed as append-only turned out not to be: a claim's revocation sets `revokeDate`, and a confidential leg affirmation's approval sets `status`. Both keep `updatedEvent`, and the claim revocation now stamps it, which it had not been doing.
A metadata event says that a value was set on an asset but never which key, so the key has to come from the originating call. The previous approach counted metadata events and filtered metadata calls, then zipped the two lists by position — which only holds if every filtered call emits exactly one counted event, and that failed both ways: a register-and-set call emits a counted event without being in the call filter (two values in one batch came out swapped), and a call that fails under a non-atomic batch emits nothing (every later value landed under the failed call's key). Matching is structural now. The extrinsic's calls are normalised into a tree and walked alongside the extrinsic's own events, using the per-call markers the batch and multisig pallets emit as the call boundaries. A failed call is explicit rather than inferred, and anything unrecognised aborts the extrinsic to an anomaly rather than guessing a key. Both batch eras are handled: the per-call markers, and the older per-call event-count vector — which is load-bearing, since metadata events predate the markers by several million blocks. Two further defects go with it: a `None` detail means "unchanged" on chain and was clearing real locks, and a key that could not be resolved returned silently. The test codec mock gained `toHex()`, without which code that writes an id one way and reads it back another passed every test and would have failed on real data. Based on a patch from the review of the post-resync defect branch.
Three call sites passed a bare block id into a parameter that goes straight into `createdEvent` / `updatedEvent`. Both are strings, so nothing caught it, and the result was rows whose event relation resolves to nothing: every `StatType`, the balance row created on a ledger miss, and every genesis-seeded balance. The seeded rows now carry the seed event marker, which is what the holdings seeder already used and what tells a seeded row from an event-caused one.
The identity and multisig handlers had a cluster of related defects around an account's lifetime. Removing a secondary key deleted the `Account` row outright, orphaning the membership interval the same handler had just closed, the account's balance and every ledger entry — all of which point at it through non-null relations. It also broke primary-key rotation, where the chain announces the incoming key as removed immediately before promoting it. The row is kept and unlinked instead. Re-announcing a key that is already indexed rewrote its row from scratch and moved its provenance forward; account writes now update in place and only a genuinely new address takes the current event as its `createdEvent`. Three unguarded reads that would have killed the block are guarded and record an anomaly instead, as does a rotation that finds no membership to carry forward — which previously returned silently after the close had already been committed. A rotation also reopens one interval per interval it closed, rather than closing all and reopening the first. A multisig signer's role now comes from the chain's key record rather than from the event: creating a multisig and authorising a signer are offers the chain only records on the key once the signer accepts, so taking the role from the offer relabelled keys that were an identity's primary or secondary key, with nothing to put them back. The role is re-read when a signer is removed. A multisig's admin DID is resolved before a relation points at it — from the index, then from chain state, and recorded as an anomaly if neither knows it — rather than writing a reference that only fails at query time. The primary-key rotation handler now documents the three-event sequence it depends on, which reads as a leaked interval without it.
…s block back Writing an `EventIdEnum` into the narrow instruction-event enum through a double cast discarded the only thing that enum is for. The handled events are mapped explicitly now, and an event that reaches the writer without a mapping is recorded rather than written as a value the schema never declared. The column is also indexed, since filtering without joining the events table is why it is kept locally at all. `EvmTransaction` regains `block`, matching how events and extrinsics keep theirs — there is no other direct link from one to the block it was included in.
…ma's derived state The block handler has to run on every block: under historical tracking a row's validity starts at the block it is saved in, so the holder buffer is only safe because the very next block flushes it. `modulo: 1` expressed that but was strictly worse than no filter — it kept issuing a dictionary query per batch whose every height was then unioned straight back in. The filter is gone and the handler documents why it is unfiltered, and what the lever would be if the cost has to come down. Three fields that summarise state held authoritatively elsewhere now say so in their docstrings, an identity key's id format is documented, and the permissions json field records that its contents are not filterable. The status an authorization can never hold says that it cannot. The timestamp decisions are recorded rather than left open. The schema sync does not revert a manual column conversion — checked against the migration service and the sync path — so the case against converting is cost, which is now stated as such; and the epoch-integer question is declined rather than left to win by default, with the narrower version kept on the table.
prashantasdeveloper
marked this pull request as ready for review
September 24, 2026 14:27
… handler The `NftHolder` rollup buffer and the POLYX reconcile queue were both flushed from a block handler on the following block. That handler had no filter, and an unfiltered block handler produces no dictionary query conditions — so the node stops skipping and scans the whole chain instead of the ~2% of heights that carry a subscribed event. Measured on the range being indexed at the time: 1,175 events across blocks 4,000,000–4,050,000. A genesis resync ran at ~120 blocks/s with a two-day estimate on an otherwise idle box, with the work queued behind block fetching rather than the database, which sat at 0.12% CPU. `modulo: 1` had identical coverage and additionally paid a dictionary query per batch whose every height was unioned back in. Both flushes now hang off the handlers that fill them, and the block handler is gone. The holder rollup is written by the block's last holdings event, found in the block's own event list — no extra chain read, and the write stays inside the block that made the change, which is what historical mode requires of it: a row's validity begins where it is saved, so writing from a later block would date the change to that block and leave every query in between reading the old array. The reconcile queue flushes from the ledger's read path, before the current block applies any movement of its own. Any later block will do, not only the next height: a block the index skipped moved no balance, so the derived side has not left the queued block, and the comparison already re-checked that against the row before trusting it. Sampling is decided once per block on first ask, because the gap test moves its own marker when it passes and would otherwise answer "yes" and then "no" within one block.
prashantasdeveloper
force-pushed
the
redesign/12-review-fixes
branch
from
September 25, 2026 04:12
362ba03 to
911bf7f
Compare
…state Found by the genesis resync, which it killed. A worker crashed on block 24,730,189 with `staking.Rewarded has no field "dest"`, the container restarted, and the block was re-indexed correctly on the retry — so it cost a restart rather than data, but only by luck. Two things were wrong. Upstream Substrate's `Rewarded` carries `dest`; Polymesh keeps its own `(identity, stash, amount)` shape, and testnet metadata declares no `dest` at spec 8000000 any more than at 7004001 — so the v8 branch was asking for a field that never exists. And reading it as a required field made that fatal instead of merely wrong: the node can attribute a block near an upgrade boundary to the later runtime, which is how a pre-v8 reward reached the v8 branch at all. The payee is now read as optional and resolved from `staking.payee` when absent, which is where it has always come from for the pre-v8 era. Correct in both eras, and no longer sensitive to which runtime the node thinks a boundary block belongs to. `optionalField` moves to the decode layer, where the rest of the field-resolution helpers live, rather than staying private to the POLYX ledger — it is the primitive for "absence is the answer", and this is its second caller.
The transfer-manager concept is gone from the chain, and what was left here was half of a translation onto the statistics model that replaced it: `TransferManagerAdded` wrote a `StatType` for a percentage restriction and nothing for a count one, `TransferManagerRemoved` was unregistered so even that row was never cleared, and both exemption handlers ran for either kind — leaving a pre-v5 asset able to hold an exemption against a restriction that was never written. The exemption path also collided: pre-v5 it keyed rows on the exempted entity alone, where the v5+ path keys them on asset, operation, claim type and entity. Two assets exempting the same identity shared one row, and a pre-v5 row could never match its v5+ equivalent. So the events are recorded as unhandled rather than half-translated, and the handlers, `getTransferManagerValue` and `TransferRestrictionTypeEnum` go with them. Neither consumer reads this era — the SDK reads transfer restrictions from chain state — so nothing downstream loses a source. The one thing it does close off is a future analytics view over pre-v5 restrictions, which would now need its own resync.
Corporate actions, checkpoints, ballots, agents and the relayer. **Relations that could not be traversed.** A distribution and a ballot are each a corporate action plus extra data, keyed on the same `CAId`, but only the ballot could reach its action. Both directions are wired now. `AssetAgent` was indexed on both sides and reachable from neither, so "this asset's agents" and "the assets this identity is an agent of" each needed a top-level query; both are derived fields, which cost no column and no index. **Two relations that were declared and never written.** A corporate action's checkpoint is set when the chain names one that already exists — the `Existing` variant carries the id, so it needs nothing but decoding. A checkpoint's schedule is paired from the index: the event names the moment but not the schedule, and two schedules can fall due at the same moment, so the timestamp alone is ambiguous. The chain emits them in ascending schedule order, which is what makes the pairing exact — the lowest-numbered schedule that declared this moment and has not yet been claimed. A pairing that does not line up is recorded and left null rather than guessed at. **A field whose name stopped being true.** `pendingCheckpoints` was captured when a schedule was created and never decremented as its checkpoints fired, so "pending" described something that stopped being the case after the first one. It is `scheduledCheckpoints` now, with the live view as a derived relation — still-pending is the declared set minus the checkpoints that exist. Not decrementing is deliberate: it is a JSON array, and under historical tracking every save rewrites the whole thing. **A log of a value the chain replaces.** Ballot votes were keyed per event, but on chain `Votes` is keyed `(CAId, IdentityId)` and voting again replaces the previous vote — so a voter who changed their mind got two rows with nothing marking which counted, and any tally double-counted them. One row per voter now, upserted, with the superseded votes still readable as the events themselves. **Content stored as hex.** A ballot's motions — the titles, the info links, the choices, which is the readable content of a ballot — were passed through as `0x…` while only the outer title was decoded. All of it is decoded now; the stored shape is unchanged. **Eight silent returns.** A handler that could not find the row its event referred to returned without a trace, which hides both of the things that cause it: the creating event was missed, or the two handlers disagree about the id. They record a missing-referenced-entity anomaly now, through one shared helper that the relayer's own version is folded into. **Agent actions that were not recorded.** The whole `nft` pallet was absent, so an agent minting a fungible token was recorded and one minting an NFT was not; `statistics.SetAssetTransferCompliance` and `capitaldistribution.Reclaimed` were the remaining agent-only events in their pallets. `BenefitClaimed` goes the other way — it is emitted by an agent's `push_benefit` *and* by any holder's `claim`, so recording it attributed holders' claims to agents, which is the over-inclusion `asset.Transfer` is excluded for. `IssuedNFT` stays out with its reason written down: pre-6.0 it names the collection, never the asset, so it cannot be resolved by parameter position. **Relayer field names that did not match the runtime.** Checked against testnet metadata at spec 8001020: `RemovedSubsidy` carries `remaining`, `RemovedPendingSubsidy` carries `initial_polyx_limit`, and only `SubsidyDebited` carries `amount`. The shape table said `amount` for all three. It is the record of what an event looks like, so the runtime's names win over a convenient shared one — and `RemovedSubsidy.remaining` is now kept, being the allowance left when the subsidy ended. **Values that stood in for the unknown.** `Subsidy.totalDebited` was zero across the pre-v8 range, where the chain emitted nothing to accumulate, making "nothing was drawn" and "not knowable" the same stored answer; it is nullable. An unrecognised `targetTreatment` fell back to `Exclude`, which is not a catch-all but the opposite of `Include` — so an unknown value silently inverted who a corporate action applied to. `toEnum` takes no fallback there, and its signature now says that omitting one is a real choice. **Units and derivations, stated.** The three `Permill` columns say they are parts per million. The checkpoint timestamp no longer falls back to the block's: a schedule only advances when balances change, so a checkpoint's moment can predate its block. `AssetAgent` loses `group` and `permissions`, which were always null and would have gone stale the first time an agent changed group — permissions belong to the `AgentGroup`, and the timeline to `AssetAgentHistory`. `relayer.RelayedTx` is recorded as deliberately unhandled: the standing relationship and every fee drawn against it are already on `Subsidy`, and the relayed call is an `Extrinsic` like any other.
`CAPTURED_MODULES` still listed the six pallets that mattered when the arity contract test was written, so `checkpoint`, `corporateAction`, `corporateBallot` and `relayer` had their shapes checked by nobody: an arity change in a future runtime would pass CI and surface as a worker crash mid-index. The list tracks `src/decode/shapes/` now. Running the sync against testnet at spec 8001020 with the wider list adds the six events and six calls the schema enums were behind on — the 8.1.x per-account asset freezing (`FrozenBalanceSet`, `SetAccountFreeze`), the split-off `ControllerTransferTo`, and the NFT approvals — plus the arity fixture for that spec version. Until they were in the enum they could only decode as `Unknown` and raise an anomaly per occurrence; the catch-all event handler resolves them properly now, whether or not a domain handler is registered for them later. The integration queries stop asserting against snapshots that are not there. Four snapshot files were deleted with the entity rename while their `toMatchSnapshot()` calls stayed, and a missing snapshot makes Jest write one instead of comparing — so those cases passed whatever came back, in the suite meant to evidence the rename preserving the data. They assert field by field instead. Regenerating the snapshots would not have been enough: two of those files still query columns the provenance rework removed, so the wider suite needs its own pass against the redesigned schema.
Five smells and one duplication, all in code this branch added. The two cognitive-complexity findings are in the metadata key resolver. `toCallNode` was one if/else chain over every section and method it knows; it dispatches per section now, with the three wrappers that differ only in which argument holds the call reduced to a table. `walkBatch` carried both batch eras inline; the pre-v7 path — find the closer whose per-call counts account for the events seen, then walk each call inside its own bound — is its own function, and so is the marker handling after each call of a v7+ batch. Same logic throughout, no behaviour change, and the tests that pin all three wrong-key cases still pass. `sameAsset` reads as an explicit guard rather than a chain of `!== undefined` checks. Two absent ids still count as not the same asset, which is the point: nothing was compared, so nothing matched. The nested ternary in `rotateIdentityKey` becomes two named steps. The duplication was three cases in the metadata tests building the same registration and set events from literals. Two builders replace them, which also makes what each case is actually varying visible.
Found by the resync, which raised 23 of these between blocks 5.87M and 6.58M — all of them noise. A pre-v6 `CheckpointSchedule` carries a period, a start and a count; it never enumerates the moments it will fire at, which is what the pairing matches against. So every scheduled checkpoint in that era failed to pair and recorded an anomaly for it, at one row per checkpoint for the whole pre-v6 range. Unpairable by construction is not the same as unexpected. A checkpoint whose asset has no schedule declaring moments at all is now left unlinked in silence, and the anomaly is kept for the case it was meant for: a schedule that does declare moments, none of which is this one.
The `!counts || counts.reduce(...) !== offset` guard the batch-walk refactor left behind reads as two unrelated checks. It is one: an event with no count vector sums to nothing, and nothing never equals a real offset — so the optional chain does both jobs, and a comment says which is which.
The metadata snapshot now captures thirteen pallets and records which events the runtime names, with a fixture for every era from 3001 to 8001020. The contract test asserts that each emitted tuple event has a decoder and that no registered decoder is stale, so runtime drift in any pallet the decode layer registers fails CI rather than a resync. Pre-v7 multiSig shapes and the NFT shapes of every era are registered, and ClassicTickerClaimed carries its third field.
Each block is indexed under the runtime that ran it. The dictionary starts each runtime late, so about 2% of blocks carried the previous spec's label; the spec is now read from the executing runtime once per block, before any handler runs. Fees follow the chain end to end. Every runtime pays fees to the block author, found from the BABE digest. Before v5.4.0 no fee event exists, so the fee is priced with payment.queryInfo at the parent block. Up to v5.4.0 the treasury took 80% of each fee, announced as TreasuryReimbursement, so the author is credited only the rest. Which path applies is decided by what the extrinsic emitted, not by the spec label. Busy early blocks no longer crawl: a block's fee quotes are requested together, and an extrinsic's events are found through a per-block index. New coverage: NFT approvals and operator approvals, balance freezes, legacy IssuedNFT and RedeemedNFT, ControllerTransferTo, set_controller, and the PIP lifecycle events. Checkpoints pair by ordinal instead of a store read, and a value that cannot be read is recorded as an anomaly instead of skipped. A paying key's re-offer no longer hides its live subsidy, so the fees it covers are not charged to the user. An index can start after genesis. The first block seeds balances, holdings, multisigs and EVM mappings from chain state, and an IndexOrigin row records where the index began and which domains were seeded. An asset created before the start block is read from chain when first referenced. Block timestamps are read through one helper that fails loudly when a block has none.
CI now counts strictNullChecks errors against a checked-in baseline and fails if the count rises, so the codebase moves toward strict mode one file at a time. Comments no longer cite review or decision ids; each explains its reason in place.
…call declared Before v5.4.0 a fee had no event of its own, and it was priced with payment.queryInfo. That prices the weight a call declared, but a call that used less is refunded after it runs: staking.rebond and contracts.instantiate routinely are, and sudo calls are refunded in full. Payers were charged for weight they never paid for, and block authors were credited with it. Every runtime in that range gave the treasury floor(80%) of each fee and announced it just before the extrinsic closed, so the fee is now read back from that cut. The cut names a single fee three times in four; otherwise the quote picks between the two it allows. No cut means nothing was charged. Across sampled testnet blocks, every quote for a call that was not refunded falls inside the range the cut allows.
…ounce Before v8, applying a deferred slash paid the offence's reporters their share of it through resolve_creating, which emits no event. The rest went to the treasury as a TreasuryReimbursement. The ledger debited the validator and credited the treasury, so the reporters' share left the validator and arrived nowhere. On testnet the one reporter of the era-2159 slash ended 637.5 POLYX short from block 7,751,343. The validator's own Slash now reads staking.unappliedSlashes at the parent block, where the slash is still held, and credits each reporter an even share of its payout, capped at what was slashed. The slashes of the offence, less what the reporters were paid, must match the treasury's receipt; a mismatch is recorded as an anomaly.
Every reconstructed pre-v5.4 fee was charged to its signer, or the signer's relayer subsidiser. The runtime chose the payer in CddHandler::get_valid_payer, the same from spec 3000 to v5.4.x: accepting an invitation is paid by the primary key of the identity that issued it, removing one can be, and a multisig or bridge proposal by the primary key of the identity the multisig belongs to. The subsidy applied is the payer's, not the signer's. So signers ran low, some into balances the chain can't hold, and the real payers high. resolveFeePayer mirrors that rule, reading the state the block started from, since the payer is chosen before the call runs and the call can change it. Storage is read at the parent hash as raw bytes, decoded with the block's registry, and only when the runtime that wrote the parent's state is the one the registry describes; on the first block after an upgrade the layouts differ. Before metadata v14 decoded storage keeps snake_case field names, so fields are read in either form. A lookup that fails records an anomaly and charges the signer rather than stopping the indexer. The pre-v8 slash reporter lookup now uses the same parent-state reader, so it gains the same runtime check, and records an anomaly when the deferred slashes can't be read. scripts/verify-fee-payer.ts runs the resolver over real blocks beside how the accounts' balances moved. On testnet blocks 466,634, 469,641, 801,695, 1,056,229, 1,056,451, 4,400,125 and 8,479,343 every resolved payer moved by exactly its fees and every signer by nothing.
Before v5.4.0 no transaction fee was announced, and reconstructing them (fee quotes, the treasury's cut, the fee candidates, posting the fee) is the ledger's one large piece of machinery that applies to a single span of runtimes. It now lives in preV54Fees.ts beside feePayer.ts, so the ledger reads as the movements every runtime shares. The moved code is unchanged. The ledger exports the four helpers the module shares with the evented fee path: extrinsicEmits, treasuryShareAt, creditBlockAuthor and activeSubsidiser.
A pre-v5.4 fee was read back from the treasury's 80% cut, and a cut that is a multiple of 4 allows two fees a unit apart. The tie was broken with a fee quote, but a quote prices the weight a call declared: for a refunded call (contracts.call, contracts.instantiate, staking.rebond) it matched neither fee and the lower one was taken, a unit short on the payer and the block author for every such call. From spec 3000 to 5003001 the fee multiplier never moved, so the runtime charged exactly the base weight's fee, 100 units a byte, the fee for the weight the call used (from its ExtrinsicSuccess/Failed event, after any refund) and the tip, at Perbill(30,000 / 650,000,000) = 46,153 ppb rounded to nearest. The fee is now computed that way and checked against the cut; one the cut doesn't allow is recorded and the lowest allowed is charged. Checked on testnet over 1,365 extrinsics in all 13 pre-v5.4 specs, spam blocks, failed and refunded calls: the formula equals the chain's quote at the declared weight and fits the cut at the used weight every time. The per-extrinsic fee quotes, and the round trips that held up spam blocks, are gone. scripts/verify-fee-payer.ts now prints the computed fee against the cut.
deferredSlashesBefore read a deferred slash's era from its
staking.unappliedSlashes key with a DataView over key.buffer. Inside the
SubQuery sandbox the key's bytes reach the mapping behind a proxy, and so
does their buffer, which DataView rejects ("First argument to DataView
constructor must be an ArrayBuffer"). The testnet genesis resync crashed
69 times on the slash at block 7,737,503 and stalled until the read was
patched on the server.
storageEntriesAtParent now returns each key's arguments decoded by the
block's registry against the map's metadata (StorageKey.setMeta), as it
already decodes the value, instead of the key's bytes for each caller to
slice. The key goes in as hex: Bytes reads a Uint8Array as length-prefixed.
Checked against the testnet archive at block 7,737,503's parent: the two
deferred slashes decode to eras 2155 and 2159, as the bytes say.
A key whose arguments don't decode makes the deferred slashes unreadable,
recorded as an anomaly, rather than a slash of era 0.
From 5.0.0 treasury's unsafe_disbursement pays by Currency::transfer, which
emits balances.Transfer{treasury -> recipient}, and emits
TreasuryDisbursement straight after it. The handler re-filed that transfer
instead of posting a second movement, but looked for it only within the
extrinsic. disbursement is root-only, so it runs from a PIP's enactment,
usually as the block initialises, with no extrinsic, and the lookup found
nothing: every such payment was counted twice, the recipient credited and
the treasury debited once more than the chain did.
On testnet PIP 26 paid 11 x 1 unit at block 6,527,686 (spec 5001020), all
in the Initialization phase, each Transfer directly before its
TreasuryDisbursement; a full ledger check showed the recipient +11 and the
treasury -11 from there on. Too small for the reconciler's threshold; a
real-size disbursement would drift by its whole amount.
The transfer is now matched on the block, written by the immediately
preceding event, from the treasury, to the recipient, for the amount, so
equal payments to one recipient each take their own transfer.
From v5.4.1 (spec 5004001) both fee events, protocolFee.FeeCharged and transactionPayment.TransactionFeePaid, name the account the fee was taken from, subsidiser included (fee_key). Before it each named someone else, and the ledger handled every runtime as if it still did. - From v5.4.1, applying the subsidy again to the named account charged a subsidised paying key's own subsidiser. Testnet block 14,872,581 (v6.3): register_ticker by a user 5DvWtd... subsidises, itself subsidised by 5EUqMb...; both events named 5DvWtd..., who paid, and the 25 POLYX protocol fee and the transaction fee were moved on to 5EUqMb.... At 18,277,893 (v7.2) a multisig accepted a paying key earlier in the same extrinsic, and its fee went to the new paying key, not the multisig. - On v5.4.0 (spec 5004000) TransactionFeePaid named the signer, while the fee was charged as before v5.4, to the payer get_valid_payer chose or that payer's subsidiser. So a call someone else pays for (relayer.accept_paying_key, identity.join_identity_as_key, the multisig *_as_key calls) left its signer low, some below zero, and its payer high: block 8,524,136, accept_paying_key signed by 5Ckx2..., whose balance did not move; the issuer's primary key paid the 127,547. From v5.4.1 the named account is now charged as is. On v5.4.0 the transaction fee goes where resolveFeeAccount says, the payer and subsidy rules the unannounced pre-v5.4 fee uses, now shared; a protocol fee before v5.4.1 still takes the named payer's subsidy. Checked on testnet at 5004000, 5004001, 5004002, 6002010, 7000005 and 7003003; v8 emits fee_key too. Testnet ran 5004000 from block 8,479,610 to 8,981,211.
A transfer that creates its recipient emits Endowed, which is credited as the account's endowment, and then Transfer, which posts only the sender's debit once it finds that endowment. It was looked for only within the extrinsic, so a transfer outside one credited the recipient twice. Testnet block 10,036,148 (spec 6000001) ran scheduled settlement instructions as it initialised, each paying a new account, and 29 recipients were credited twice, 97.46 POLYX in all. Outside an extrinsic the endowment is now matched on the block, and only the one the immediately preceding event wrote: the balances pallet emits Endowed straight before its Transfer, all 29 times in that block, and a looser match would swallow a later same-amount transfer to the account.
Every save of a historical entity closes its current row with UPDATE ... WHERE id = $1 AND _block_range @> $2, and the only index SubQuery gives id is a plain btree: addHistoricalIdIndex runs after _block_range is appended to the declared indexes, so the id index never gets it. An account touched every block builds up tens of thousands of row versions, and each close-out scanned them all. On a testnet genesis resync account_balances and staking_positions held ~27,000 versions for busy ids; the update took 3-12 s per batch with Postgres at 100% CPU. A GiST (id, _block_range) index took the lookup from 157 ms to 0.8 ms and the sync from ~180 to ~1,280 heights/s. compat.sql now creates it on both tables, unless the table already has one under any name. On an existing database it builds holding writes back, so the node pauses until it is done.
Before v8 every dropped positive imbalance (a staking reward, a bridge mint) is funded from the block reward reserve, which try_mutate_account touches whether or not it holds anything; empty, it is recreated, and emits Endowed(brr, 0). A testnet resync wrote 210,000 of them, each an entry and a new balance version, 27,000 versions for the reserve alone, for no movement. They are now ignored.
On a drift the live reconciler recorded a BalanceReconciliationDrift anomaly and set the indexed balance to chain state, with no entry for the difference. The account's entries then no longer added up to its balance, and a full ledger check stopped seeing it: on a testnet resync several fee payers dropped out of the check while their signers stayed, the cause visible only in the anomalies table. The correction is now posted as a BalanceSetAdjustment per changed pool, filed under the event that queued the check (its ids are passed along with its index), so the entries sum to the balance again and every corrected unit is traceable to the anomaly that explains it. The entry writer is shared with handleBalanceSet, which already recorded a balance set to a value that way. A forced check with no provoking event still corrects, and its anomaly says no entry was written.
Before v8 a deposit not offset by a withdrawal leaves a positive imbalance, and drop_positive_imbalance takes what it can from the block reward reserve's free balance and mints only the rest, with no event (balances pallet, v3.0.0 to v7.4.0; v8 has no reserve). The ledger recorded every such deposit as minted. On testnet the reserve held 1 POLYX and paid the first reward of block 9,259,823, so the index held the reserve 1 POLYX high from there; on mainnet it held nothing at genesis or at any of 104 samples to the head, so every reward there was minted. The deposits are staking rewards, bridge mints, the testnet identity grant and, before 5.0.0, a treasury disbursement, which withdrew from the treasury, burned that, and deposited to the recipient. Each is now credited from the reserve for what its balance in the index covers and minted for the rest; a deposit already credited by the Endowed of the account it created gets the reserve's debit beside it; a pre-5.0 disbursement burns the reserve's share. Two movements under one event shared entry ids, which carry no account, so postTransition takes an id tag. It also lets several slash reporters each keep an entry, which until now overwrote each other. A lifetime total such as totalRewards is now added only on the side it is about: the reserve paid a reward, it did not receive one.
ensureTrueSpecVersion read the runtime version at every block's parent hash, because the dictionary labels blocks near an upgrade with the neighbouring runtime's spec, late and sometimes early. That read cost two round trips per block: the sandbox downloads the whole parent block to check an explicit hash. The index already records every upgrade as a ChainUpgrade, from the system.CodeUpdated the dictionary always delivers, and blocks are processed strictly in order even with --workers: workers fetch in parallel, but the dispatcher resolves fetches in order and processes them through a queue of concurrency 1, so an upgrade's row is written before any later block is handled. So a block's spec is now that of the latest upgrade recorded in an earlier block, with no chain read. Before the first row the chain is read once per worker. The rows are read in full, through the paging helper: getByFields merges its cache with the database without ordering across the two, so a one-row latest read could miss the newest row. The test store mock gains an in-memory getByFields, which the real store always answers with an array.
A v8 Ethereum transaction (revive.eth_transact) can withdraw 0 from its payer before the fee estimate. Every Withdraw was posted as a Burn, the empty one included, and refileFeeWithdrawal takes the payer's first burn in the extrinsic for the fee's own withdrawal. So it took the 0, which failed the size check, and the fee was posted a second time beside the real withdrawal and its refund. Testnet block 25,118,132: Withdraw 0, Withdraw 242,500, Withdraw 71, refund 44,371, fee 198,129; the payer was left 198,129 low. A full check found 8 such accounts. postTransition now writes nothing for an amount of 0, which also stops every other empty event writing an entry and a balance version.
Both are development instrumentation in the production schema, and the redesign marked them for removal (docs/implementation/09-infrastructure.md §9.7); no consumer reads either. FoundType cost a write per argument of every event: the serializer called logFoundType for each one, which saved the same id again, without awaiting it. A testnet genesis resync left 1.4M row versions for 403 type names, ~4,400 per id, and closing each version scanned the whole table, 166 ms a time, which made it the slowest of the range-closing updates. Nothing has written Debug for some time. The serializer loses its logFoundType parameter, and its test checks the Vec-of-structs serialization it used to check alongside the logging.
Before AssetBalanceUpdated, asset.Transfer named no instruction, and handleAssetTransfer took the instruction from the block's first InstructionExecuted. In a block executing several instructions every transfer was filed under the first: testnet block 5,091,763 executed 27 as it initialised, and all 270 of its transfers went to one. Settlement executes an instruction's legs one transfer at a time, then emits InstructionExecuted, in the same dispatch (v4.1.0 to v6.x; a failed instruction rolls back with its events), and a leg's transfer can be preceded by the CheckpointCreated of a schedule it advances. So a transfer's instruction is now the next InstructionExecuted in its phase, reached past only transfers and checkpoints; anything else in between means the transfer was not a leg and it gets none. Checked on testnet blocks 5,091,763 and 5,091,764: all 270 and 260 transfers matched, each executed instruction to its own legs.
The ledger pairs events with entries written earlier in the same block or extrinsic: a transfer with the endowment it created, a fee with its withdrawal and refund, a deposit with its endowment, a reward with its mint. Each lookup was a store getByFields on PolyxEntry, and SubQuery's getByFields sorts every cached row of the entity and sends their ids to Postgres as a NOT IN list, so a block of N transfers cost about N^2: a busy block's cache is mostly its own entries. Every entry is now registered, as it is written, in an index held in the block context, by extrinsic, account and movement, and every pairing lookup reads it, with no store search. It is complete: pairing only matches entries of the same block or extrinsic, a block is indexed whole and in order by one thread, and both writers (writeMovementSide and recordAdjustment) register. The lookups hand back the live entries, so a relabel is seen by every later lookup.
updateLegs, run on every affirmation and execution, found an instruction's legs with getByFields, which sorts every cached Leg and sends their ids to Postgres as a NOT IN list: quadratic in a block of affirmations, such as testnet block 5,091,762's 498. It searched even on the scheduled and unsigned execution paths, which have no signer to add and so discarded what it found. A leg's id is instructionId/legIndex, with indices from 0 and no gaps, so the legs are now read by id until one is missing, and nothing is read when there is no signer.
Each key change read every IdentityKey interval the account had ever had, and each nomination every Nomination the stash had ever made, open and closed, then picked out the open ones. The cost of a change grew with the history: testnet block 1,056,718 re-permissioned one key 100 times after 1,400 earlier changes, each change paging through ~1,500 rows. Both now filter on the closing field being null, through the (account, validToBlock) index IdentityKey already had and a new (position, validToEvent) one on Nomination. An open row is written with an explicit null: the store's cache matches a filter with isEqual, which does not take an unset field for null, so a row opened earlier in the same batch would otherwise be missed.
Every fee queried the payer's Subsidy rows, and on v8 tried the paying key's withdrawal when the payer's was not found. From v5.4.1 both fee events name the account the fee was taken from, subsidiser included, and v8 withdraws the fee from that same account (fee_key, transaction-payment v8.0.0 and v8.1.2), so on every current runtime the query found nothing the event had not already said and the fallback could only match what the first attempt had. The subsidy is now read only where the event names someone else: a protocol fee before v5.4.1, and through resolveFeeAccount a v5.4.0 transaction fee. The v8 fallback is gone, and its test now models what v8 emits, the paying key in both the withdrawal and the event.
The payee and controller were already read once per stash per block, but not the ledger behind them. A payout block restakes many rewards, and for each one both the balance ledger (resyncing the staking lock) and StakingPosition read the same staking.ledger. api.query returns the block's end state, so every read in the block gives the same answer: the ledger is now kept per block beside the other two, as they are, a failed read excepted. A StakingPosition test put Bonded, Unbonded and Withdrawn in one block with a different ledger behind each, which no chain can produce; they now sit in successive blocks.
…rows Before reading a block's parent state, storageAtParent checked that the runtime which wrote it is the one the block's registry describes, with two chain reads: the parent's header, then the runtime version at its parent. That was once per block that read parent state, in fee-payer and slash handling. A runtime changes only after the block carrying its system.CodeUpdated, which the index records as a ChainUpgrade. So unless an upgrade came in the parent, the parent ran the same runtime as the block, whose spec ensureTrueSpecVersion has already set; only on the block after an upgrade is the writer still read from the chain. A block mislabelled by the dictionary is caught the same way, its registry not matching its spec. The slash tests' registry now describes the block's own runtime, which the old double read had hidden behind a mock that answered both reads with the same number.
A pre-v6 asset.Transfer that reached no InstructionExecuted was left with no instruction, and an affirmation or execution of an instruction with no legs in the index changed nothing, both silently: a gap in the reasoning would show only as missing links. asset.Transfer exists up to v5.x and is emitted by unsafe_transfer, which only three paths reach (v3.0.0, v4.1.0, v5.4.0): settlement, ending in InstructionExecuted, with only further transfers and CheckpointCreated between the legs (an NFT leg emits nothing; STO investments settle through it too); asset.controller_transfer, followed straight away by ControllerTransfer; and a capital distribution claim, followed straight away by BenefitClaimed. transferInstruction now tells the three apart, and anything else is recorded as an UnreadableValue anomaly naming what followed the transfer. Every one of testnet's 16,488 pre-v6 transfers classified: 13,563 settlement legs, 9 controller transfers, 128 claims and 2,788 issuances or redemptions, none unexplained. An instruction always has a leg, written when it is created, so finding none for a signed affirmation or execution is now recorded as a MissingReferencedEntity anomaly.
Every redemption and transfer rebuilt the holder's whole nftIds array, filtering out the tokens that left, so each event cost as much as the holder held. Testnet blocks from 15,391,560 redeemed 400 tokens a block, one per event, from one portfolio holding tens of thousands, and slowed the sync more than any other blocks. While a holder is buffered for its block, its tokens are now kept as a set, changed per token at a cost independent of how many it holds, and written back to nftIds once, when the block's buffer is flushed.
transferInstruction read block.events, typed through @subql/types, as @polkadot/types' EventRecord. tsc accepts the two as one type, but the webpack/ts-loader build the image runs treats them as distinct and failed with four TS2345 errors, so c436035 onward did not build. The records are now cast as the rest of the file already casts them.
A v8 transaction fee was paired with the payer's first withdrawal in the extrinsic, which had to be at least the fee. Testnet block 25,473,580 withdrew 3,132 first, deposited it back around a relayer subsidy, and withdrew the 155,668 fee between: the 3,132 failed the check and the fee was posted a second time. Two other accounts were left low the same way at the tip of a full testnet resync. The fee is withdrawn as an estimate and what the chain did not charge is deposited back to the payer in the same extrinsic, so its withdrawal is now the payer's first that accounts for it: equal to the fee, or exceeding it by exactly a refund deposited to the payer. A protocol fee is unchanged: the exact withdrawal immediately before FeeCharged.
|
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.
Addresses the review comments on the holdings (#352), identity/key (#353), schema-invariants (#354),
post-resync-defects (#355) and coverage (#356) branches. Based on #360 so the whole stack stays
linear.
Everything was verified against this branch's tip before being fixed — two threads were already
resolved upstream and are listed at the bottom.
Thread → change
Holdings (#352)
identityIdonly set at creation, so it goes staleIdentity, keys and multisig (#353)
createAccountclobbers an existing row's provenancecreatedEvent. Moved to the account utils, as suggested.Account.removedeletes the row the new history points atkeyRole = Unlinked), inSecondaryKeysRemoved,SignerLeftand the rotation path.Account.getcrashes the blockhandlePrimaryKeyUpdatedis correct only because of its siblingsrotateIdentityKeybails after the close has committedkeyRolewritten from a pending offerresolveKeyRoleits first caller.linkSignerAccountdocstring's two claims don't holdMultiSigAdmin.adminpoints at an unchecked DIDMultiSigAddedAdminand the pre-7.x creator path.resolveKeyRoleunused, and its comment overstates the invariantKeyRole/KeyRoleEnumnames are confusingIdentityKeyRole/AccountKeyRolewith the shared values spelled the same way, as proposed.IdentityKeyid format undocumented; nodatetimedatetimedeliberately left off.transactionGroupsis no longer filterable inside a jsonFieldAuthorizationRetryLimitReacheddeclared but never emittedSchema invariants (#354)
EvmTransaction.blockEventandExtrinsicdeclare theirs.eventId as unknown as InstructionEventEnumInstructionEvent.eventshould be indexedeventIdcolumns with no stated rulePolyxEntry,StakingEvent,AssetTransaction,AssetAgentAction), dropped fromAccountandDistributionPayment.updatedEventfrom the append-only familyreconcileBlockearly-returns on every invocationcompared=1090 drifted=4).modulo: 1disables dictionary block-skippingmapStatisticspasses a block id as an event idmapPolyxLedger/accountBalancelikewisemapIdentitiespasses a block id intorotateIdentityKeyPost-resync defects (#355)
The metadata-key patch is applied as sent, on top of this branch's
LockedUntilfix — it appliedcleanly and keeps that field. The root cause, the three wrong-key cases, the
None-detail lockwipe, the silent unresolved-key returns and the
toHex()test-mock gap are all in.Coverage (#356)
AssetAgentunreachable from either sideAsset.agentsandIdentity.agentOfadded as derived fields — no column, no index.Distribution/CorporateAction/CorporateBallotshare aCAIdbut cannot traverseDistribution.corporateActionadded, withCorporateAction.distribution/.ballotderiving back.VoteCastevents.Permillcolumns stored baretotalDebiteduses0for an unindexable pastpendingCheckpointsnames something that stops being truescheduledCheckpoints, not decremented, withCheckpointSchedule.checkpointsderiving the live view.relayer.RelayedTxunhandledAssetAgent.group/.permissionsalways nullAgentGroup; the timeline isAssetAgentHistory.GroupChangedreplacesAgentPermissionsChangedCheckpoint.schedulenever set(scheduleId, moment)ordering, which is exact where the timestamp alone is ambiguous. Unpairable → anomaly, left null.CorporateAction.checkpointnever setExistingvariant, which carries the id. TheScheduledvariant links itself fromCheckpointCreated.CAPTURED_MODULESmisses four palletssrc/decode/shapes/; the sync added the enum members it was behind on and the spec-8001020 arity fixture.RemovedSubsidy.remainingis now kept.mapCheckpointfalls back toblock.timestamptargetTreatmentfalls back toExcludetoEnum's signature now says an omitted fallback is a choice.EventIdEnumand the agent-action table behind the runtimenftpallet,statistics.SetAssetTransferComplianceandcapitaldistribution.Reclaimed.toMatchSnapshot()callsFour places this diverges from the review
ClaimandConfidentialLegAffirmationkeepupdatedEvent. Both are read back and mutated —a revocation sets
revokeDate, an approval setsstatus— so they fail the append-only test. Theclaim revocation was in fact not stamping
updatedEvent, which is fixed here. Also,TickerExternalAgentHistoryno longer exists in the schema, so the family is six entities, notnine.
The block handler is removed outright, which is a bigger fix than the thread asked for. The
first version of this branch only dropped the
modulo: 1filter and documented why the handler hadto stay. Then the resync run against that version measured ~120 blocks/s and a ~2 day ETA, on a
box that was idle (load average 3 of 32 cores, Postgres at 0.12% CPU, 247 blocks sitting in
Awaiting process). The cause was exactly the one this thread identified: blocks 4,000,000–4,050,000carry 1,175 events across 50,000 blocks, so ~98% of the chain was being fetched and processed for
nothing. An unconditional block handler produces no dictionary query conditions, so the node stops
skipping;
modulo: 1had identical coverage and additionally paid a dictionary query per batch.masterhas no such handler, which is why resyncs were fast before this stack.So both flushes the handler existed for were moved onto event handlers. The
NftHolderrollup iswritten by the block's last holdings event, identified from the block's own event list — no
extra chain read, and the write stays inside the block that made the change, which is what
historical mode requires. The POLYX reconcile queue flushes from the ledger's read path, before a
later block applies any movement of its own: a block the index skipped moved no balance, so the
snapshot stays comparable however many heights later the flush lands, and
reconcileOnealreadyre-checked that against the row. Sampling is now decided once per block on first ask, since the
gap test moves its marker when it passes.
The metadata resolver does not depend on
ItemCompletedeverywhere — checked against the chain.asset.SetAssetMetadataValuefirst appears at block 4,397,819 (spec 5000002);utility.ItemCompleted/ItemFailedonly at block 10,036,148 (spec 6000001). For the ~5.6M blocksin between, the per-call event-count vector on
BatchCompleted/BatchInterrupted/BatchOptimisticFailedis the only call boundary — which is exactly the path the patch alreadycarries, so the assumption holds by a different route than the one assumed. That is now recorded in
the resolver.
The schema sync does not revert a
timestamptzconversion.SchemaMigrationServicecomparesthe previous GraphQL schema against the next one and emits DDL only for fields that changed in the
schema — it never inspects the live column type — and the only other DDL path is
sequelize.sync()with no options, which is
CREATE TABLE IF NOT EXISTS. So the conversion was never unworkable; itis declined on cost, and the recurring part of that cost (every newly added
Datefield needs theALTERextended, with nothing in CI to catch a miss) is now stated.Three more places this diverges, on #356
Six missing
EventIdEnummembers, not seven.balances.Restoredis in the runtime andalready declared in the schema, which is why the drift report does not list it. The other six are
exactly as described.
IssuedNFTcannot join the agent-action table. Pre-6.0 it is(IdentityId, NFTCollectionId, NFTId)— it names the collection, never the asset or ticker — so itcannot be resolved by parameter position, and the table has no shape for a collection lookup. Its
siblings
NftCollectionCreatedandRedeemedNFTdo name the asset and are in.The omission is written down rather than left looking like an oversight.
BenefitClaimedis removed rather than confirmed. It is emitted by an agent'spush_benefitand by any holder's
claim, so recording it attributed holders' own claims to the agents — theover-inclusion
asset.Transferis excluded for.Reclaimed, which only an agent can call, takesits place.
One defect the resync found
A worker died on block 24,730,189 with
staking.Rewarded has no field "dest", restarting thecontainer. Chain metadata says Polymesh keeps its own
(identity, stash, amount)shape at spec8000000 as well as 7004001 — there is no
destin either — so the v8 branch was asking for a fieldthat never exists. Reading it as required is what made that fatal rather than merely wrong: the node
can attribute a block near an upgrade to the later runtime, which is how a pre-v8 reward reached
that branch. It is optional now, with the payee resolved from
staking.payeewhen absent.Commits
Thirteen, each typechecked on its own tree. All
chore, because everything corrected here wasintroduced inside this unmerged stack — typing them
fix/featwould make the release notes read"added X" then "fixed X" for what ships as one change.
Verification
yarn codegen,yarn typecheck,yarn lint,yarn test:unit(708 tests),yarn buildandyarn check-handlersall pass. A clean genesis resync from this branch is still required forsign-off — the schema changed again with the #356 fixes, so it cannot reuse the completed run.
That run did finish, against the first nine commits: caught up at 26,027,249, one restart (the
staking.Rewardeddefect above, fixed here), 646BalanceReconciliationDriftrows and 3MissingReferencedEntity. All 646 are onfree— the pre-v8 block-author fee share that has noevent, which remains an open question — and none on
frozen, where the previous run had 10. Themetadata resolver produced no anomalies at all across the whole replay, including the spec-6000001
boundary where the per-call batch markers arrive. The reconciler compared ~29,000 balances by block
10M against 1,090 for the entire previous run, because sampling now lands only on blocks that
actually moved a balance.