Add `storage_version()` and `runtime_wasm_code()` to storage #1111

tadeohepperle · 2023-08-07T18:01:07Z

This PR adds two methods storage_version(pallet_name) and runtime_wasm_code to the storage to fetch the respective data from the node. I also wanted to add other keys (see mod well_known_keys) but retrieving data with them did not work (at least with the polakdot node I tried it with). I left them in the PR as private to maybe activate them in the future if necessary.

jsdw · 2023-08-08T10:33:33Z

subxt/src/storage/storage_type.rs

+        ));
+
+        // fetch the raw bytes and decode them into the StorageVersion struct:
+        let storage_version_bytes = self.fetch_raw(&key_bytes).await?.ok_or(format!(


Nit: ok_or_else here toa void formatting the string unless there's an error (I'd expect clippy to spot this but havent checked)

jsdw · 2023-08-08T10:39:43Z

subxt/src/storage/utils.rs

@@ -37,3 +37,50 @@ pub(crate) fn storage_address_root_bytes<Address: StorageAddress>(addr: &Address
    write_storage_address_root_bytes(addr, &mut bytes);
    bytes
 }
+
+/// this should match the constants in the module `sp_core::storage::well_known_keys`
+pub mod well_known_keys {


I'd probably keep this in subxt::storage and not utils (it's storage specific I think?).

Is there a use in exposing any of this stuff at all? I'd be inclined to just inline what you need into the runtime_wasm_code function for now, with a pointer to where you got the key from there so we can add more if need be or whatever.

Alternately we could expose this WellKnownKey<R> type in subxt::storage and then a single api.storage().fetch_well_known_key(key: WellKnownKey<R>) -> Result<R, Error> type function to fetch them (it sortof depends whether any of the other keys are actually useful anywhere; that might be worth finding out!).

This is in subxt::storage::utils not in the general subxt::utils. I wanted to expose a "nicer" interface in the hopes that many other well known keys might work in a similar fashion, such that we can abstract over their types and keys with a function like fetch_well_known_key() on the storage.
But the other keys do not seem to work as intended and I think you are right, it is less bloat to just hardcode the ":code" in the right place and remove the other stuff here. Also first I thought the returned value was scale decoded but that is also not the case apparently.

Ok that makes sense to me; let's keep it simple then until we see a use for allowing other keys!

It would be interesting to investigate what is causing the failure in Polkadot for the other known keys :D

lexnv · 2023-08-08T11:41:07Z

subxt/src/storage/storage_type.rs

@@ -237,6 +241,43 @@ where
            })
        }
    }
+
+    /// the storage version of a pallet.


nit: I prefer upper case for documentation and for consistency, but totally up to you

lexnv · 2023-08-08T11:50:24Z

testing/integration-tests/src/full_client/storage/mod.rs

+        .storage()
+        .at_latest()
+        .await?
+        .storage_version("System")


Crazy thought here; could we use this to do a quick version validation? Similar to our hashing, I guess that would require substrate to constantly modify these numbers appropriately with changes.

When would that version validation happen? I guess we would need to make about 30 concurrent calls to validate all pallets. Or we might just validate the pallet whenever a storage query/call is made for a specific pallet?

We already validate the keys and value types on a per-entry basis so probably what we have is already better (I imagine the version might increment for every storage change, but not really sure what it represents exactly?)

jsdw · 2023-08-09T14:44:24Z

subxt/src/storage/storage_type.rs

+        let data = self.fetch_raw(CODE).await?.ok_or(format!(
+            "Unexpected: entry for well known key \"{}\" not found",
+            std::str::from_utf8(CODE).expect("qed;")
+        ))?;


Suggested change

let data = self.fetch_raw(CODE).await?.ok_or(format!(

"Unexpected: entry for well known key \"{}\" not found",

std::str::from_utf8(CODE).expect("qed;")

))?;

let data = self.fetch_raw(CODE).await?.ok_or_else(|| format!(

"Unexpected: entry for well known key \"{}\" not found",

std::str::from_utf8(CODE).expect("qed;")

))?;

jsdw · 2023-08-09T14:45:58Z

subxt/src/storage/storage_type.rs

+        const CODE: &[u8] = b":code";
+        let data = self.fetch_raw(CODE).await?.ok_or(format!(
+            "Unexpected: entry for well known key \"{}\" not found",
+            std::str::from_utf8(CODE).expect("qed;")


If you wanted to simplify this you could just have:

const CODE: &str = ":code"; let data = self.fetch_raw(CODE.as_bytes()).await?.ok_or_else(|| format!( "Unexpected: entry for well known key \"{CODE}\" not found",

jsdw · 2023-08-09T14:48:24Z

subxt/src/utils/mod.rs

+    scale_encode::EncodeAsType,
+    scale_decode::DecodeAsType,
+)]
+pub struct StorageVersion(u16);


Since we return this to the user we should either:

Make the u16 pub (that's probably what they care about but currently it's inaccessible, or

Just return a u16 from storage_version (that's my preference; I don't think this struct adds anything)

jsdw

This looks good to me now; nice one :)

lexnv · 2023-08-10T12:24:28Z

subxt/src/storage/storage_type.rs

+            STORAGE_VERSION_STORAGE_KEY_POSTFIX,
+        ));
+
+        // fetch the raw bytes and decode them into the StorageVersion struct:


nit: Does this comment needs updating?

tadeohepperle added 3 commits August 7, 2023 19:58

add storage_version function

391e0ef

get runtime code

2791ea1

Merge branch 'master' into tadeo-hepperle-add-pallet-version-storage-api

41727e0

tadeohepperle changed the title ~~Add storage_version() to storage~~ Add storage_version() and runtime_wasm_code() storage Aug 8, 2023

tadeohepperle changed the title ~~Add storage_version() and runtime_wasm_code() storage~~ Add storage_version() and runtime_wasm_code() to storage Aug 8, 2023

tadeohepperle marked this pull request as ready for review August 8, 2023 08:47

tadeohepperle requested a review from a team as a code owner August 8, 2023 08:47

tadeohepperle added 2 commits August 8, 2023 10:55

add tests

6e3f8f5

clippy fix

49c2a6e

jsdw reviewed Aug 8, 2023

View reviewed changes

lexnv reviewed Aug 8, 2023

View reviewed changes

tadeohepperle added 2 commits August 8, 2023 15:21

just support CODE, remove other well known keys

3760b13

Merge branch 'master' into tadeo-hepperle-add-pallet-version-storage-api

5267dc4

jsdw reviewed Aug 9, 2023

View reviewed changes

tadeohepperle added 2 commits August 9, 2023 19:02

remove u16 wrapper

13cf184

Merge branch 'master' into tadeo-hepperle-add-pallet-version-storage-api

591de35

jsdw approved these changes Aug 10, 2023

View reviewed changes

lexnv reviewed Aug 10, 2023

View reviewed changes

lexnv approved these changes Aug 10, 2023

View reviewed changes

tadeohepperle merged commit 9723a50 into master Aug 10, 2023
8 checks passed

tadeohepperle deleted the tadeo-hepperle-add-pallet-version-storage-api branch August 10, 2023 13:40

jsdw mentioned this pull request Sep 27, 2023

Prep to release 0.32.0 #1178

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `storage_version()` and `runtime_wasm_code()` to storage #1111

Add `storage_version()` and `runtime_wasm_code()` to storage #1111

tadeohepperle commented Aug 7, 2023 •

edited

Loading

jsdw Aug 8, 2023

jsdw Aug 8, 2023

tadeohepperle Aug 8, 2023

jsdw Aug 8, 2023

lexnv Aug 8, 2023

jsdw Aug 8, 2023

lexnv Aug 8, 2023

lexnv Aug 8, 2023

tadeohepperle Aug 8, 2023

jsdw Aug 8, 2023

jsdw Aug 9, 2023

jsdw Aug 9, 2023 •

edited

Loading

jsdw Aug 9, 2023

jsdw left a comment

lexnv Aug 10, 2023

Add storage_version() and runtime_wasm_code() to storage #1111

Add storage_version() and runtime_wasm_code() to storage #1111

Conversation

tadeohepperle commented Aug 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsdw Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsdw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add `storage_version()` and `runtime_wasm_code()` to storage #1111

Add `storage_version()` and `runtime_wasm_code()` to storage #1111

tadeohepperle commented Aug 7, 2023 •

edited

Loading

jsdw Aug 9, 2023 •

edited

Loading