paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.8k stars 652 forks source link

Remove `sp_runtime::RuntimeString` and replace with `Cow<'static, str>` or `String` depending on use case #5693

Open nazar-pc opened 2 weeks ago

nazar-pc commented 2 weeks ago

Description

As described in https://github.com/paritytech/polkadot-sdk/issues/4001 RuntimeVersion was not encoded consistently using serde. Turned out it was a remnant of old times and no longer actually needed. As such I removed it completely in this PR and replaced with Cow<'static, str> for spec/impl names and String for error cases.

Fixes https://github.com/paritytech/polkadot-sdk/issues/4001.

Integration

For downstream projects the upgrade will primarily consist of following two changes:

#[sp_version::runtime_version]
pub const VERSION: RuntimeVersion = RuntimeVersion {
-   spec_name: create_runtime_str!("statemine"),
-   impl_name: create_runtime_str!("statemine"),
+   spec_name: alloc::borrow::Cow::Borrowed("statemine"),
+   impl_name: alloc::borrow::Cow::Borrowed("statemine"),
        fn dispatch_benchmark(
            config: frame_benchmarking::BenchmarkConfig
-       ) -> Result<Vec<frame_benchmarking::BenchmarkBatch>, sp_runtime::RuntimeString> {
+       ) -> Result<Vec<frame_benchmarking::BenchmarkBatch>, alloc::string::String> {

SCALE encoding/decoding remains the same as before, but serde encoding in runtime has changed from bytes to string (it was like this in std environment already), which most projects shouldn't have issues with. I consider the impact of serde encoding here low due to the type only being used in runtime version struct and mostly limited to runtime internals, where serde encoding/decoding of this data structure is quite unlikely (though we did hit exactly this edge-case ourselves :sweat_smile:).

Review Notes

Most of the changes are trivial and mechanical, the only non-trivial change is in substrate/primitives/version/proc-macro/src/decl_runtime_version.rs where macro call expectation in sp_version::runtime_version implementation was replaced with function call expectation.

Checklist

nazar-pc commented 2 weeks ago

I do not understand why formatter in CI wants those trailing whitespaces, I'm unable to commit them for some reason in IDE.

UPD: nano did it.

nazar-pc commented 1 day ago

Fixed merge conflicts and restored create_runtime_str