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

ImOnline Pallet Migration Causing Storage Root Mismatch #136

Open
jimjbrettj opened this issue Oct 3, 2024 · 2 comments
Open

ImOnline Pallet Migration Causing Storage Root Mismatch #136

jimjbrettj opened this issue Oct 3, 2024 · 2 comments
Labels
runtime-upgrade Runtime Upgrade

Comments

@jimjbrettj
Copy link

Hello, my name is Jimmy and I am a member of the Gossamer team at ChainSafe Systems. We are building a Golang implementation of the Polkadot host and currently are using Paseo as one of the networks to test our node.

When syncing Paseo, I have run into an interesting situation at block 1789153 where the root we calculate differs from the expected root. This is associated with upgrading the spec to version 1002004. From my investigation, it appears our issue has to do with the ImOnline Pallet migration.

When I execute this block against the previous state I get the following logs:
image

If you compare my logs to the reference logs found here, you will notice that in our execution we find one key associated with the ImOnline pallet and remove it, while your logs have the "No ImOnline keys found post-removal 🎉" log.

I check which host calls use a key from this pallet, as you can see in the logs in my output above. In order, there is a call to NextKey(), then Set(), and then ClearPrefix(). The interesting thing is that if I block the Set() call of the key associated with the ImOnline pallet, the root is calculated correctly. Given that a) your logs don't indicate that you have this insertion (since no keys are removed after) and b) removing the insertion fixes our issue, this seems to point to the issue being the Set call. I am trying to figure out the context around this problem and determine where it is coming from.

I have spent time digging through this runtime upgrade, the ImOnline Migration logic, and the migration logic for other pallets that get migrated in this block, but I have had no luck finding where this insertion might be coming from in the runtime.

I am opening this issue with the hope that you might be able to give some context around the migration or shed some light on where this insertion could potentially be coming from. Any help would be greatly appreciated 🙏

@dimartiro
Copy link

Hi, I'm taking over Jimmy's issue on the Gossamer side.

I've been taking a look and, based on my findings, every time we add a new pallet we are adding the storage_version key in the storage.

This ends up adding the key 0x2b06af9719ac64d755623cda8ddd9b944e7b9012096b41c4eb3aaf947f6ea429 in the storage with the value 0x0000 given that it is: twox_128("ImOnline") + twox_128(":__STORAGE_VERSION__:").

Then, I am still trying to figure out why the runtime is removing that pallet and cleaning up the keys with the pallet name prefix 0x2b06af9719ac64d755623cda8ddd9b94.

I need to understand why you are removing 0 keys and we are removing 1 at that point.

Aren't you inserting the 0x2b06af9719ac64d755623cda8ddd9b944e7b9012096b41c4eb3aaf947f6ea429 key? Or, for some reason, are you ignoring the keys with the :__STORAGE_VERSION__: postfix?

@dimartiro
Copy link

Fixed! was related with a wrong implemented runtime function on our side.
@jimjbrettj could you close it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
runtime-upgrade Runtime Upgrade
Projects
Status: Planned
Development

No branches or pull requests

2 participants