fix: 🐛 post-resync defects found testing redesign/07 against real chain data - #355
Open
prashantasdeveloper wants to merge 4 commits into
Open
prashantasdeveloper wants to merge 4 commits into
prashantasdeveloper wants to merge 4 commits into
Conversation
…g proposal `resolveMetadataKey` only inspected the outer, signed extrinsic's method, so a `setAssetMetadata(Details)` call reached via a `utility.batch*` extrinsic (the common shape for "create an asset and set its metadata inline") or executed by `multiSig.approve` of a previously created proposal never resolved — the row was silently dropped with a `MissingReferencedEntity` anomaly instead of being written. Root-caused against 12 real occurrences (testnet genesis-to-tip resync, specs 7002000/7004000/ 7004001/8000000): most were `utility.batch*` wrapping a direct `setAssetMetadata` call alongside `createAsset`/`registerUniqueTicker`/etc; the rest were `multiSig.approve` executing an `asset.set_asset_metadata` proposal, whose call the indexer already captured in `MultiSigProposal.params.proposals` at `ProposalAdded` time (`multiSig.proposals` on-chain storage is cleared once a proposal executes, so that's the only place left to read it from). Both paths pick their call by ordinal position among same-extrinsic `SetAssetMetadataValue(Details)` siblings, then check it against the already-decoded `assetId` as a consistency guard — matching by `assetId` alone (an earlier version of this fix, caught in review) picks the wrong call whenever a batch or proposal sets more than one key, including twice for the same asset. `parseMetadataKey` also strips the thousands separators `toHuman()` puts in a `u64` (`"1,234"`), another review catch that would otherwise have split a key's rows depending on which path resolved it.
Crashed the redesign/07 resync in a genuine, non-transient loop: block 25,557,789's `asset.Approval` — "has no field \"assetId\"; it carries [owner, spender, asset_id, amount]" — repeating on every retry, `rc` climbing (11 restarts and counting) with no way to get past it. The decode layer's own doc comment says Polymesh pallets emit tuple-style events (no field names at any spec version) and only upstream Substrate pallets emit named ones — the registered shape table and every handler's destructuring are camelCase-only on that assumption. A later v8 testnet runtime breaks it: `asset.Approval`'s metadata now names its fields, in Substrate's own idiomatic Rust snake_case (`asset_id`), not the camelCase every handler reads. Once metadata names a field, `decodeEvent` uses those names as-is and never falls back to the (correctly camelCase) shape table, so the mismatch is a guaranteed `FieldNotFound` — thrown by design, but nothing catches it at the handler call site, so it takes the worker down instead of just recording the anomaly it already writes on the way out. Fixed at the root, in `metadataFieldNames`, rather than in the one handler that happened to hit it: normalise a field name to camelCase only when it contains an underscore, so a name that is already camelCase (everything decoded successfully so far) passes through unchanged.
prashantasdeveloper
force-pushed
the
fix/redesign-post-resync-defects
branch
from
September 11, 2026 18:25
40de904 to
f0c7a3b
Compare
`getOrCreateAccount` defaulted `createdEventId` to the block's first event (`${blockId}/0000000000`)
whenever a caller had no real one to give — every call reached through the asset-holder resolution
chain (`rawAssetHolderToAssetHolder` -> `extractAssetHolder` -> `meshAssetHolderToAssetHolder`), the
path that lazily discovers an Account/IdentityKey while resolving a transfer's from/to holder.
Accepted as a bounded follow-up during phase 7 rather than an in-phase cascade (see
docs/implementation/13-entity-provenance.md and the phase-7 decisions log).
`meshAssetHolderToAssetHolder` / `extractAssetHolder` / `rawAssetHolderToAssetHolder` now take an
optional `blockEventId`, threaded from every real call site that already extracts one — asset
transfers and allowances (mapAsset.ts), NFT transfers (mapNfts.ts), settlement legs and funds
transfers (mapSettlement.ts, settlements.ts) — so a lazily-created Account's `createdEventId`
records the event that actually caused the discovery. A caller with no event to give still gets
the old block-event-0 default; `getOrCreateAccount`'s signature is unchanged.
The portfolio-only paths (`meshPortfolioToAssetHolder` / `rawPortfolioToAssetHolder`, used across
mapPortfolio.ts, mapIdentities.ts, stos.ts, distributions.ts, the seed) never call
`getOrCreateAccount` and are untouched — they build a `PortfolioDetails` from already-decoded
fields, no account resolution involved.
…t:S7776) `BATCH_METHODS.includes(...)` is an O(n) scan of a 6-element array on every batched SetAssetMetadataValue(Details); a Set's `.has()` is O(1) and is what Sonar's own rule asks for.
prashantasdeveloper
force-pushed
the
fix/redesign-post-resync-defects
branch
from
September 11, 2026 18:36
f0c7a3b to
fa27b92
Compare
|
prashantasdeveloper
added this pull request to stack #358
September 15, 2026 08:23
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
1 participant
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
Three fixes found by resync-testing
redesign/07-schema-invariantsfrom genesis againsttestnet — the first full resync of anything past Phase 4, so the first time several code paths
ever ran against real chain data. Branched off
redesign/07-schema-invariants; base this PR onit, not
master.1.
SetAssetMetadataValuecouldn't resolve its key through a batch or multisig proposalresolveMetadataKeyonly inspected the outer, signed extrinsic. AsetAssetMetadata(Details)call reached via
utility.batch*(the common "create an asset and set its metadata inline"shape) or executed by
multiSig.approveof a previously-created proposal never resolved — therow was silently dropped with a
MissingReferencedEntityanomaly instead of being written.Root-caused against 12 real occurrences on testnet (specs
7002000/7004000/7004001/8000000). Both new paths pick their call by ordinal position among same-extrinsicSetAssetMetadataValue(Details)siblings (not byasset_idalone — a batch/proposal can set morethan one key, including twice for the same asset) and verify against the already-decoded
assetIdas a consistency guard.parseMetadataKeyalso strips the thousands separatorstoHuman()puts in au64("1,234"), which would otherwise split a key's rows depending onwhich path resolved it. Both of those were caught in an Opus review of the first version of this
fix, along with several test-coverage gaps — see the commit body for the full review trail.
Pre-existing since Phase 5 (
redesign/05-holdings-nfts,6149410); not introduced by phase 7.2. A later v8 runtime names an event's fields in snake_case, crashing the indexer
Found live, mid-resync, as a genuine crash loop (not a candidate item — this blocked the resync
from ever completing).
asset.Approval's metadata names its fields in Substrate's idiomatic Rustsnake_case (
asset_id) on a later v8 testnet spec, not the camelCase (assetId) every shapetable and handler in this codebase reads. The decode layer's own doc comment asserted Polymesh
pallets are tuple-style (no field names) at every spec version — true until this runtime — so
there was no shape-table fallback once metadata named a field at all, and nothing caught the
resulting
FieldNotFoundat the handler call site.Fixed at the root in
metadataFieldNames, not as a one-event patch: normalise a metadata fieldname to camelCase only when it contains an underscore, a no-op for the (previously universal)
already-camelCase case. Deployed directly to the resync server mid-run (code-only change, no
schema drop) to unblock it; confirmed it sailed through the stuck block cleanly.
3. A17: lazily-discovered accounts got approximate provenance
getOrCreateAccountdefaultedcreatedEventIdto the block's first event(
${blockId}/0000000000) for every account discovered while resolving an asset transfer'sfrom/to holder — accepted as a bounded phase-7 follow-up rather than an in-phase cascade (see
docs/implementation/13-entity-provenance.md).Threaded the real
blockEventIdthrough the whole asset-holder resolution chain(
rawAssetHolderToAssetHolder→extractAssetHolder→meshAssetHolderToAssetHolder→getOrCreateAccount) from every real call site: asset transfers/allowances, NFT transfers,settlement legs and funds transfers. The portfolio-only paths never call
getOrCreateAccountandneeded no change. A caller with no event to give still gets the old default.
Testing
yarn codegen && yarn typecheck && yarn lint && yarn test:unit && yarn check-handlers && yarn build— all green throughout (final: 0 type errors, 0 lint warnings, 512/512 unit tests).
redesign/07-schema-invariants+these fixes): reached chain head cleanly. Final
indexer_anomaliestable has exactly the 12rows item 1 explains — nothing else — and zero
BalanceReconciliationDriftrows (betterthan expected; the phase-7 reconcile-hygiene fix left no drift at all across the whole run).