fix: read a block's event metadata from its parent state - #1506
Merged
Conversation
Weight Diff Report
35 extrinsic(s) changed across 2 pallet(s). New: 4. Removed: 2. pallet_omnipool_liquidity_mining
pallet_stableswap
New extrinsics (4)
Removed extrinsics (2)
Threshold: ±10%. Base |
|
Crate versions that have not been updated:
Runtime version has not been increased. |
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
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.
Follow-up to #1503, which is already merged and tagged
v49.2.1. Fixes a cache-poisoning bug found in a security review of that PR.The bug
layout()keyed the event-layout cache on the spec version fromat's post-state, but fetched metadata by calling the runtime API atat— which executes whatever:codesits inat's post-state.At block N the metadata call returns the new runtime's layout while the spec key is still old, so the new layout gets cached under the old spec version. Every later old-spec block then decodes with the wrong metadata — and since record spans are measured from that metadata, a wrong layout can mis-measure and swallow neighbouring records, which is the exact failure #1503 exists to eliminate. It persists until restart.
The fix
Keep the spec key from
at—LastRuntimeUpgradein a block's post-state exactly names the runtime that executed it, which is the layout needed to decode its events — and take the metadata from the parent, whose post-state:codeis that runtime:Note taking the spec key from the parent instead would mismatch in the other direction: at N+1 the parent's post-state already has the new
:codebutLastRuntimeUpgradestill reads old.Falls back to
atat genesis and when the parent header is unknown, so it is never worse than before.Tests
metadata_comes_from_the_parent_state— the regression itselfgenesis_and_unknown_blocks_fall_back_to_themselves— no reads at the zero hash18 node synth tests pass; formatting clean. The same fix is on #1504 as
5eb4e9d7f.Once this lands,
v49.2.1moves onto it — no GitHub release or Docker image was published against the old commit, so nothing downstream is pinned.