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

Silent dynamic changes in metadata constants #934

Open
Slesarew opened this issue Oct 5, 2021 · 20 comments
Open

Silent dynamic changes in metadata constants #934

Slesarew opened this issue Oct 5, 2021 · 20 comments

Comments

@Slesarew
Copy link

Slesarew commented Oct 5, 2021

Metadata in kusama changes once every block; similar "silent" change was observed in polkadot once today (2021-10-05). There is no associated spec_version bump, thus we end up with different metadata with colliding IDs.

How to reproduce:

  1. fetch kusama metadata (simplest - run default code in https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fkusama-rpc.polkadot.io#/js)
  2. wait 10 seconds
  3. fetch another kusama metadata
  4. diff two versions.

This is caused by this line:

https://github.com/paritytech/polkadot/blob/d26815243a2af9547568f13eadd226de3df07479/runtime/kusama/src/lib.rs#L1386

Metadata generation should not be accessing storage!

Another variable that silently changed today was batched_calls_limit: u32 in pallet utility in both kusama 9090 and polkadot 9090, there is obviously no way to reproduce this now.

This blocks Signer's development as it diverges from assumptions used for designing security logic.

@bkchr
Copy link
Member

bkchr commented Oct 5, 2021

Another variable that silently changed today was batched_calls_limit: u32 in pallet utility in both kusama 9090 and polkadot 9090, there is obviously no way to reproduce this now.

https://github.com/paritytech/substrate/blob/5413a1f0d996874b97032fafd3284854df14b4d1/frame/utility/src/lib.rs#L118-L125

This code should be static per compilation. How did the values change?

@bkchr
Copy link
Member

bkchr commented Oct 5, 2021

This will solve the gilt problem.

@bkchr
Copy link
Member

bkchr commented Oct 5, 2021

I would actually like to have test for this, that ensures that the metadata isn't changing. However, this would be required to be some longer running test. So, we should probably start to have some sort of "pre-release" tests.

@s3krit @chevdor any any how we can create these kind of pre-release tests? I just need a way it will be executed, writing a test for this should not be that hard. Maybe some custom gh action that we manually run before creating a release?

@Slesarew
Copy link
Author

Slesarew commented Oct 5, 2021

Another variable that silently changed today was batched_calls_limit: u32 in pallet utility in both kusama 9090 and polkadot 9090, there is obviously no way to reproduce this now.

https://github.com/paritytech/substrate/blob/5413a1f0d996874b97032fafd3284854df14b4d1/frame/utility/src/lib.rs#L118-L125

This code should be static per compilation. How did the values change?

Old hex-encoded metadata for Polkadot I had on September 15:

https://pastebin.com/tFPdSb0e

Today:

https://pastebin.com/7ZZzTiaQ

@bkchr
Copy link
Member

bkchr commented Oct 5, 2021

The links to pastebin are not working.

And do you have a non-hex encoded one?

@Slesarew
Copy link
Author

Slesarew commented Oct 5, 2021

Sorry for links. Try this, decoded. They diverge at symbol 178352

https://gist.github.com/Slesarew/0689113c83f9da248e454d316f0e0d43

@bkchr
Copy link
Member

bkchr commented Oct 5, 2021

@Slesarew do you have the blocks you fetched this metadata from?

@Slesarew
Copy link
Author

Slesarew commented Oct 5, 2021

No, but one is from today and another is from September 15 (and it did not change like that until today, I'm checking almost daily)

@bkchr
Copy link
Member

bkchr commented Oct 5, 2021

Are you checking against a public rpc node?

@Slesarew
Copy link
Author

Slesarew commented Oct 5, 2021

Yes, the one at wss://rpc.polkadot.io

@bkchr
Copy link
Member

bkchr commented Oct 5, 2021

Between 0x0352bc04c13f87bd795df58aa21e3f45dceb891f58d918534acc3dd03b898d54 and latest block the value is the same for me.

@chevdor
Copy link
Contributor

chevdor commented Oct 5, 2021

I did a test and I do not reproduce the issue when fetching the metadata directly from the chain:

code:

URL=wss://rpc.polkadot.io  
for i in {1..10}; do subwasm meta -j $URL | shasum; sleep 10; done;

result:

4371f74e9c05a67e89259484caa9157dc7e54ee9  -
4371f74e9c05a67e89259484caa9157dc7e54ee9  -
4371f74e9c05a67e89259484caa9157dc7e54ee9  -
4371f74e9c05a67e89259484caa9157dc7e54ee9  -
4371f74e9c05a67e89259484caa9157dc7e54ee9  -
4371f74e9c05a67e89259484caa9157dc7e54ee9  -
4371f74e9c05a67e89259484caa9157dc7e54ee9  -
...

As a reference, if we test another chain with subwasm meta -j wss://kusama-rpc.polkadot.io | shasum, we get:

269b25aae26a35c4c6fb9778c757e77cc3dccf54  -

@chevdor
Copy link
Contributor

chevdor commented Oct 5, 2021

Also checked using different endpoints:

export RUST_LOG=info; for i in {1..10}; do subwasm meta -j --chain polkadot | shasum; sleep 10; done;

The results remain consistent:

[2021-10-05T21:40:33Z INFO  subwasm] Running subwasm v0.14.1
[2021-10-05T21:40:33Z INFO  subwasm] ⏱️  Loading WASM from Chain(OnchainBlock { endpoint: WebSocket("wss://polkadot.elara.patract.io"), block_ref: None })
4371f74e9c05a67e89259484caa9157dc7e54ee9  -
[2021-10-05T21:40:50Z INFO  subwasm] ⏱️  Loading WASM from Chain(OnchainBlock { endpoint: WebSocket("wss://rpc.polkadot.io"), block_ref: None })
4371f74e9c05a67e89259484caa9157dc7e54ee9  -
[2021-10-05T21:41:01Z INFO  subwasm] ⏱️  Loading WASM from Chain(OnchainBlock { endpoint: WebSocket("wss://rpc.polkadot.io"), block_ref: None })
4371f74e9c05a67e89259484caa9157dc7e54ee9  -
[2021-10-05T21:41:13Z INFO  subwasm] ⏱️  Loading WASM from Chain(OnchainBlock { endpoint: WebSocket("wss://polkadot.api.onfinality.io/public-ws"), block_ref: None })
4371f74e9c05a67e89259484caa9157dc7e54ee9  -
[2021-10-05T21:41:28Z INFO  subwasm] ⏱️  Loading WASM from Chain(OnchainBlock { endpoint: WebSocket("wss://rpc.polkadot.io"), block_ref: None })
4371f74e9c05a67e89259484caa9157dc7e54ee9  -
[2021-10-05T21:41:40Z INFO  subwasm] ⏱️  Loading WASM from Chain(OnchainBlock { endpoint: WebSocket("wss://polkadot.api.onfinality.io/public-ws"), block_ref: None })

@chevdor
Copy link
Contributor

chevdor commented Oct 5, 2021

I also get the same for the block hash mentione by @bkchr:

 subwasm meta -j --chain polkadot -b 0x0352bc04c13f87bd795df58aa21e3f45dceb891f58d918534acc3dd03b898d54 | shasum
[2021-10-05T21:44:21Z INFO  subwasm] Running subwasm v0.14.1
[2021-10-05T21:44:21Z INFO  subwasm] ⏱️  Loading WASM from Chain(OnchainBlock { endpoint: WebSocket("wss://polkadot.api.onfinality.io/public-ws"), block_ref: Some("0x0352bc04c13f87bd795df58aa21e3f45dceb891f58d918534acc3dd03b898d54") })
4371f74e9c05a67e89259484caa9157dc7e54ee9  -

@Slesarew
Copy link
Author

Slesarew commented Oct 6, 2021

Between 0x0352bc04c13f87bd795df58aa21e3f45dceb891f58d918534acc3dd03b898d54 and latest block the value is the same for me.

This is indeed interesting!

I keep Signer metadata database that is updated as I work on it using, among others, these operations:

https://github.com/paritytech/parity-signer/blob/master/rust/meta_reading/src/fetch_metadata.rs

https://github.com/paritytech/parity-signer/blob/master/rust/generate_message/src/metadata_db_utils.rs#L147

So I get old versions saved and, mostly, kept in github (at least until things get stable enough to transfer db to some safe place and gitignore it here). It was never my intention to monitor changes in metadata, I've noticed this just because of cheap "good practices" checks shown above, so don't expect those archives to be exahustive or comfortable to use.

Anyway, all this hints to me that it's some subtle change in metadata generation, not the chain itself.

I did a test and I do not reproduce the issue when fetching the metadata directly from the chain:

code:

URL=wss://rpc.polkadot.io  
for i in {1..10}; do subwasm meta -j $URL | shasum; sleep 10; done;

@chevdor this test is supposed to reproduce more prominen kusama error that was already addressed. The subtle polkadot change could not be reproduced on purpose, we have only logs do tackle it.

@Slesarew
Copy link
Author

Metadata silently changed in polkadot between the beginning of this week and now (most probably around today).

Affected variable is:

batched_calls_limit: u32 in pallet utility

Gist (filenames self-descriptive): https://gist.github.com/Slesarew/24cab4e28a4589a573a5732bb7deeebc

As a side-effect, now the demo "polkadot9110 upgrade" Signer QR message that Martin might nave generated a week ago will result in non-matching metadata hash and show as error. Thus this is a blocker for public Signer release.

Meanwhile, Kusama still changes every block, the fix didn't get in release yet, right?

@bkchr
Copy link
Member

bkchr commented Oct 29, 2021

Meanwhile, Kusama still changes every block, the fix didn't get in release yet, right?

This should come this evening.

@bkchr
Copy link
Member

bkchr commented Oct 29, 2021

@Slesarew I please need the exact block hashes to check this.

@Slesarew
Copy link
Author

I'm sorry, I'm not storing these (yet?), I was just updating a database that was designed with assumption that metadata hash corresponds to spec_name+spec_version so I'm not storing blocks. All I know is that metadata output changed at these times and did not change (at least often) before, as I occasionally do this fetch for testing my code. Unfortunately, I haven't even generated any transactions with the "old" version of metadata, so I have no idea which blocks correspond to that "old" state.

On previous encounter of this silent change, you've already determined that fetching metadata does not reproduce the error - old blocks return "new" metadata. It should be in metadata generation - either the algorithm changed, or it queries some non-constant values somewhere. Really, I can't realistically think of any way to reproduce this error, unless there are snapshots of network state and runtime artefact from a week ago.

@bkchr
Copy link
Member

bkchr commented Oct 29, 2021

Ahh wait! I got some idea!

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

5 participants