Improve remote-externalities #8397
Conversation
add rpc method to get hash from block number allow passing of block numbers and hashes
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
break up lines that were too long use starts_with instead of match s.get use unwrap_or_default instead of unwrap_or(Vec::new())
|
I removed the ability to use a number for the block number as it was rather difficult to get the corresponding hash, so now only hashes are allowed. I also replaced async-std with tokio, to make the tests runnable. The live tests now all succeed except the one with the cache file. Here I still need to figure out what the best way to produce a small cache file is and add it to the repo. |
Add cache file (contains only "Proxy" module) for local test
|
I added a small cache file and moved the test that uses it out of the feature gated test module. Now all tests are succeeding. |
| modules | ||
| } => Builder::<B>::new().mode(Mode::Online(OnlineConfig { | ||
| uri: url.into(), | ||
| cache: cache_file.as_ref().map(|c| CacheConfig { |
There was a problem hiding this comment.
Having a CacheConfig::new(file, dir) might be worth considering.
| Builder::<Block>::new() | ||
| .mode(Mode::Online(OnlineConfig { | ||
| modules: vec!["Proxy".into()], | ||
| ..Default::default() | ||
| })) | ||
| .build() | ||
| .await | ||
| .unwrap() | ||
| .execute_with(|| {}); |
There was a problem hiding this comment.
My original tests were not really sophisticated, and I think you can do better here:
We need a way to assert that some data has actually been injected into to the proxy pallet. For example, if you scrape the system pallet, now you want to try and read something like System::block_number or some other storage item that is unlikely not to change.
Or, you can assume that you know all storage items in proxy are prefixed with twox128("Proxy"), and then assert that sp_io::storage::next_key(twox128("Proxy")) is some.
There are other ways to also do this.
Treat it as a bonus, if you want to explore the storage a bit.
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: David <dvdplm@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
|
@kianenigma I'm hazy on the labelling rules here, |
kianenigma
added
A0-please_review
|
done @dvdplm |
|
Let's avoid merging this until @hardliner66 has had time to make a few changes more. @hardliner66 ping us for a last once-over whenever you're ready, ok? :) |
|
@dvdplm After talking with @kianenigma I decided to create a separate pull request with the remaining changes. So the PR can be merged. |
* make builder generic to allow using different hash types * expose "cache", "block_number" and "modules" as cli options for live state * Change Builder to be generic over Block instead of Hash add rpc method to get hash from block number allow passing of block numbers and hashes * fix live tests * fix formatting in utils/frame/remote-externalities/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * change cli to only accept block hashes break up lines that were too long use starts_with instead of match s.get use unwrap_or_default instead of unwrap_or(Vec::new()) * improve error message * fix indentation * replace Block with sp_runtime::testing::Block * Move cache test out of remote-test feature tests Add cache file (contains only "Proxy" module) for local test * simplify match expression to and_then Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Combine the two cfg attributes into one Co-authored-by: David <dvdplm@gmail.com> * Restrict visibility of test_prelude use statements to crate level * Fix usage of and_then * Rename cache to snapshot * Remove fully qualified path for Debug * Refine naming. snapshot to state_snapshot * Remove unnecessary comment Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: David <dvdplm@gmail.com>
Fixes parts of #8379.
This PR changes the CLI parameters for try-runtime. It adds the capability to write to a cache file when using try-runtime with the
livestate. Additionally it adds the capabillity to specify the modules to scrape and the block number from which to start.It also changes
remote_externalities::Builderto be generic over theBlocktype.I still need to address the broken tests that are mentioned in #8379, but everything else should already work.