Repository navigation
Fix Chopsticks runtime-upgrade test panic (InvalidNumberOfDescendants) - #3800
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (5)
📝 WalkthroughWalkthroughThe fork-block selection script gains an anchored relay-chain state proof requirement enforced via ChangesAnchored proof gate and test suite adaptation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
InvalidNumberOfDescendants)
What does it do?
Fixes the
chopsticks-upgrade-testCI job (moonbeam / moonriver / moonbase) whichwas failing during block production after the runtime upgrade with:
Root cause. The upgraded runtime wires
parachain-system'sRelayParentOffsetto
pallet_async_backing, whoseRelayParentOffsetstorage value now defaults to1.With the offset
> 0,set_validation_dataverifies that the inherent carriesRelayParentOffset + 1 = 2relay-parent descendant headers. Chopsticks mocks therelay chain and builds the
setValidationDatainherent with0descendants, so everyblock built under the new runtime panics.
beforeAllpasses because those blocks stillrun under the old (live) runtime, which does not enforce the check; the first block
built under the new runtime (
C01T1) is the one that fails.Fix. After the upgrade is applied, force
AsyncBacking::RelayParentOffsetto0in the forked chain (
set_validation_dataskips the descendant check when the offsetis
0). This is a Chopsticks-only accommodation; the real network is unaffectedbecause real collators source the descendant headers from the live relay chain.
This also reverts the earlier, ineffective workarounds for the same panic:
test-upgrade-chain.ts(added in Reduce CI runner cost and make CI less flaky (pass 2) #3757).The panic is deterministic, so retrying never helped — it just exhausted attempts and
then failed the height assertion.
test/scripts/select-chopsticks-fork-block.ts, the CI step that ran it, and theblock: ${env.CHOPSTICKS_BLOCK}config entries (added in Make Chopsticks runtime upgrade tests less flaky in CI #3765). Fork-block selectionis irrelevant here: the failing inherent is fabricated fresh for each new block,
independent of the fork block. Chopsticks now forks from the latest finalized block again.
What important points should reviewers know?
disableRelayParentOffsetwrites the rawu32value0x00000000. Passing a falsyvalue (e.g.
0) to moonwall'ssetStoragewould delete the key and fall back to thedefault of
1, so the explicit little-endian hex is intentional.dev_newBlock({ count: 3 })so the new runtime's metadata(which contains
asyncBacking.relayParentOffset) is already active when the storage is set.RelayParentOffset = AsyncBacking.Is there something left for follow-up PRs?
the offset override could be dropped to exercise the feature end-to-end in the fork test.
What alternative implementations were considered?
state roots when descendants are already populated; it does not synthesize the missing
descendants, so it would not resolve
InvalidNumberOfDescendants { received: 0 }.currently carry
0descendants, and descendants of the fork block don't propagate into thefreshly built inherents anyway.
Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?
cumulus-pallet-parachain-systemRelayParentOffset/ relay-parent descendantverification.
relayParentDescendantsstate roots in oldsetValidationDataAcalaNetwork/chopsticks#1017 (descendant state-root handling) — related but does not cover theempty-descendants case hit here.
What value does it bring to the blockchain users?
runtime upgrades that change relay-parent-offset behavior are validated in CI before shipping.