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

An entry in the Proxy.proxies storage map cannot be decoded #453

Open
jsdw opened this issue Sep 6, 2024 · 3 comments
Open

An entry in the Proxy.proxies storage map cannot be decoded #453

jsdw opened this issue Sep 6, 2024 · 3 comments

Comments

@jsdw
Copy link

jsdw commented Sep 6, 2024

Hello!

While working on decoding historic storage entries, I ran into an issue whereby the storage entry in Proxy.Proxies keyed by account ID (twox64_concated) 0x0E6DE68B13B82479FBE988AB9ECB16BAD446B67B993CDD9198CD41C7C6259C49 and with value 0x0898c8a3d01da9877b7b30877c717ae8f2a7e726cefa176c5dfcdcebc9b6a122230098c8a3d01da9877b7b30877c717ae8f2a7e726cefa176c5dfcdcebc9b6a1222304005109bd2e0000000000000000000000 fails to decode. it decodes OK in spec version 18, and starts failing to decode as of spec version 23 and until the present day by the looks of it.

See polkadot-js/api#5974 for more details.

The value type for this map in spec version 18 is (Vec<(T::AccountId, T::ProxyType)>, BalanceOf<T>).

The value type as of spec version 23 is (Vec<ProxyDefinition<T::AccountId, T::ProxyType, T::BlockNumber>>, BalanceOf<T>), where ProxyDefinition is:

#[derive(Encode, Decode, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, RuntimeDebug)]
pub struct ProxyDefinition<AccountId, ProxyType, BlockNumber> {
	/// The account which may act on behalf of another.
	delegate: AccountId,
	/// A value defining the subset of calls that it is allowed to make.
	proxy_type: ProxyType,
	/// The number of blocks that an announcement must be in place for before the corresponding call
	/// may be dispatched. If zero, then no announcement is needed.
	delay: BlockNumber,
}

In other words, the size of the value type grew by 4 bytes when this delay: BlockNumber field was added (and then the tuple turned to a struct).

In my tests, the value of this field never changed when the type did, and so the value is unable to be decoded to this day.

Should a storage migration be applied to remove this entry from the map?

@jsdw
Copy link
Author

jsdw commented Sep 16, 2024

Kian also pointed me to https://github.com/paritytech/polkadot-sdk/pull/1805/files and https://paritytech.github.io/polkadot-sdk/master/asset_hub_westend_runtime/struct.DeleteUndecodableStorage.html, which may help.

One thought in this case was that the storage entry in Proxy.proxies that fails does in fact decode without errors, but it leaves some undecoded bytes at the end that weren't decoded. If we have a CI check in place (@kianenigma was it this one?: https://github.com/polkadot-fellows/runtimes/blob/main/.github/workflows/check-migrations.yml#L88) to test that storage can be decoded, it'd be worth making sure that it is actually running and that it checks that all bytes are consumed during decoding.

@kianenigma
Copy link
Contributor

kianenigma commented Sep 16, 2024

The lack of env.CHECKS is the main issue:

checks: ${{ env.CHECKS }}

@ggwpez / @mordamax I am guessing this was a deliberate choice to save us CI time right? or else do you know why we don't pass this arg in?

@ggwpez
Copy link
Member

ggwpez commented Sep 16, 2024

The relay decode checks are broken, i think because of nompools or something?
I had to disable them here:

echo "Disabling try-state checks on the relay"
CHECKS="pre-and-post"

But this disables them for all pallets...

but it leaves some undecoded bytes at the end that weren't decoded

Yea i can fix this. PS: Huh, looks like we use decode_all here: https://github.com/paritytech/polkadot-sdk/blob/7ecf3f757a5d6f622309cea7f788e8a547a5dce8/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs#L118

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants