Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rococo fails executing block 2848657 #1416

Closed
kamilsa opened this issue Nov 21, 2022 · 3 comments
Closed

Rococo fails executing block 2848657 #1416

kamilsa opened this issue Nov 21, 2022 · 3 comments
Assignees
Labels
bug Something isn't working W3F approved

Comments

@kamilsa
Copy link
Contributor

kamilsa commented Nov 21, 2022

After executing rococo's block 2848657 resulting and expected merkle roots do not match

22.11.21 13:35:00.611471  kagome           Info      BlockExecutor  Applying block #2848657 (0xc7fe�3ab0) (Secondary VRF in slot 278115133, epoch 4887, authority #41)
22.11.21 13:35:00.611650  kagome           Info      BlockStorage  Added block #2848657 (0xc7fe�3ab0) as child of #2848656 (0x8ca6�ec02)
22.11.21 13:35:00.856911  kagome           Info      MiscExtension  utf8: Hash not equal
22.11.21 13:35:00.856954  kagome           Info      MiscExtension  hex: 5d63d717e29f7209d3c76446179a5253ced5e3066e60e76a3db6df8fd72ace3e
22.11.21 13:35:00.856959  kagome           Info      MiscExtension  hex: 68a00e4565410772c83ec89b0b548d7511ac6689a6b299551db344982289efef
22.11.21 13:35:00.857157  kagome           Error     IoExtension  target: runtime, message: panicked at 'Storage root must match that calculated.', /home/builder/cargo/git/checkouts/substrate-7e08433d4c370a21/7a4e516/frame/executive/src/lib.rs:525:9
22.11.21 13:35:00.857164  kagome           Error     RuntimeExternalInterface  Trap: unreachable
22.11.21 13:35:00.857453  kagome           Info      BlockStorage  Removed block #2848657 (0xc7fe�3ab0)
@kamilsa kamilsa added this to the Kusama treasury 1 milestone Nov 21, 2022
@kamilsa kamilsa added the bug Something isn't working label Nov 21, 2022
@kamilsa
Copy link
Contributor Author

kamilsa commented Nov 25, 2022

Likely issue is in one of these methods
ext_crypto_ecdsa_sign_prehashed_version_1
ext_crypto_ecdsa_verify_prehashed_version_1

@turuslan
Copy link
Contributor

Next block after runtime upgrade is executed on old (wrong, must be new) code.

@turuslan
Copy link
Contributor

turuslan commented Nov 28, 2022

  • RuntimeUpgradeTracker must always read :code from state
  • RuntimeUpgradeTracker may use (custom) BlockTree for caching (e.g. parent/child block has same :code if it didn't change it with digest)
  • RuntimeUpgradeTracker must not wait/expect until block with digest is added, but it must check digests by itself during cache lookup/traversal.
  • remove "state_root" from RuntimeUpgradeTracker and CodeProvider, must be cached by :code hash
  • RuntimeUpgradeTracker caching must limit search distance and fallback to reading from state (e.g. last update was on genesis, search will go too far)
  • RuntimeUpgradeTracker cache should not be persistent
  • Did :code change without digest in old networks? Should these blocks be hardcoded?

  • trie encode/decode LeafContainingHashes and BranchContainingHashes value as fixed hash, not length prefixed bytes.
  • trie get/put must work with LeafContainingHashes and BranchContainingHashes
    • "polkatot_trie_impl.cpp", value check must check value_hash too, sync
  • trie commit/hash/serialize/store/encode for version=1 must transform dirty(?) values ≥33 into hashed variant (Leaf -> LeafContainingHashes, BranchWithValue -> BranchContainingHashes)
  • should read trie nodes be transformed? should host api storage be private to prevent non-wasm reads?
    • no
  • does trie restructuring/reshaping make node value "dirty"/"written"?

  • [untested] child storage commits without version (executeOnChildStorage)
  • [untested] child storage calls clearChildBatches
  • [untested] should persistentCallAt commit storage without version or preserve in-memory trie between calls (e.g. Core_initialize_block + BlockBuilder_apply_extrinsic + BlockBuilder_finalize)
  • [untested] should trie batch cursor see new keys from batch?

  • (RuntimeUpgradeTracker must use code from chainspec codeSubstitutes map value, not from state).
    • RuntimeUpgradeTracker returned state, and that state was handled by RuntimeCodeProvider
  • chainspec codeSubstitutes key is block number (substrate)

  • [refactor] remove TrieBatch(Impl), PersistentTrieBatch(Impl), EphemeralTrieBatch(Impl)
  • [refactor] simplify trie nodes from virtual to variant/optional/...
  • [refactor] split trie decoder (db reader) and encoder (version, db writer)

This was referenced Dec 2, 2022
@turuslan turuslan moved this from Todo to Review in progress in KAGOME Dec 9, 2022
@turuslan turuslan moved this from Review in progress to In Progress in KAGOME Dec 9, 2022
@kamilsa kamilsa moved this from In Progress to Review in progress in KAGOME Dec 15, 2022
This was referenced Jan 9, 2023
@github-project-automation github-project-automation bot moved this from Review in progress to Done in KAGOME Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working W3F approved
Projects
None yet
Development

No branches or pull requests

2 participants