fix(vm,erc20): decode pre-migration EVM state for historical queries#14
Merged
Merged
Conversation
Historical eth_call (and estimateGas/traces) below the store-migration height failed or returned wrong results, because pre-migration state is decoded with the current, incompatible layout: - x/vm Params: proto field numbers shifted (old field 10 active_static_precompiles vs new history_serve_window), so decoding panics with "wrong wireType = 2 for field HistoryServeWindow". - x/erc20 precompiles: the native/dynamic lists moved from a single concatenated-blob key to per-address keys, so precompiles appear unregistered at historical heights and calls execute the decoy ERC20 bytecode (returning 0 or reverting) instead of the precompile. Add a lazy, height-gated shim on the read path: - utils.LazyUpgradeHeight resolves and caches the migration height from a caller-supplied resolver (nil = disabled, keeping the modules generic). - x/vm GetParams decodes the legacy Params layout below that height via types.DecodeLegacyParams. - x/erc20 IsNative/IsDynamicPrecompileAvailable fall back to the legacy concatenated-blob keys below that height. Query-path only: on current heights the code path is byte-for-byte unchanged, so this is not consensus-affecting and ships as a plain binary update. The resolver is wired from the applied x/upgrade done-height on the app side.
The migration height was resolved lazily from the query context. For a historical eth_call below the migration height, that context predates the upgrade, so GetDoneHeight returned 0 and the legacy-decode paths were skipped (params happened to work only because GetParams warms its cache every block via BeginBlock; erc20 precompile-availability is not hit every block, so its cache stayed cold and gTAC.balanceOf at old heights returned 0 instead of the real balance). Add PrimeLegacyParamsHeight / PrimeLegacyPrecompilesHeight to warm both gates from a current-height context; the app calls them in BeginBlocker. Once the upgrade is applied the height is cached and reused by historical reads. Verified end-to-end (v1.0.4 -> v1.6.0) on real pre-upgrade state.
Move the per-block cache warming into the x/vm module's BeginBlock instead of relying on the host app's BeginBlocker. BeginBlock now primes its own legacy Params height and, via the Erc20Keeper interface, the erc20 precompile-availability height. Keeps the whole historical-decode fix inside the fork; the host app no longer needs a warming hook.
erc20Keeper is optional in the vm keeper (GetPrecompileInstance guards nil), so keep BeginBlock consistent and nil-safe for chains that wire the vm keeper without x/erc20.
AkKoks
force-pushed
the
fix/historical-eth-call-legacy-decode
branch
from
327931e to
5796b41
Compare
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.
Historical eth_call (and estimateGas/traces) below the store-migration height failed or returned wrong results, because pre-migration state is decoded with the current, incompatible layout:
Add a lazy, height-gated shim on the read path:
Query-path only: on current heights the code path is byte-for-byte unchanged, so this is not consensus-affecting and ships as a plain binary update. The resolver is wired from the applied x/upgrade done-height on the app side.
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
mainbranch