Fix FFI type diversion (#8261)

A #7375 follow-up fix

Author: s0me0ne-unkn0wn

prdoc/pr_7375.prdoc

@@ -50,6 +50,11 @@ doc:
       The `#[runtime_interface]` machinery automatically "strips" away the marshalling strategy wrappers,
       so neither the body of the `say_hello_world` function here nor its callers need to be changed.
 
+      Please pay attention that `enum`s with explicit discriminant numbers (if different from implicit, that is,
+      not starting from zero or not sequential) using the old `PassByEnum` strategy, should be carefully
+      migrated to preserve compatibility, as `PassByEnum` was always using implicitly generated discriminants
+      starting from zero, and the `PassAs` strategy introduced by this PR passes actual values.
+
       Furthermore, to explicitly distinguish between the runtime compilation mode and native mode,
       `#[cfg(substrate_runtime)]` is now used instead of `#[cfg(not(feature = "std"))]`. That allows for fine-tuning
       the compilation behavior without relying solely on the `std` feature.

substrate/frame/benchmarking/src/utils.rs

@@ -25,7 +25,8 @@ use serde::{Deserialize, Serialize};
 use sp_io::hashing::blake2_256;
 use sp_runtime::{traits::TrailingZeroInput, DispatchError};
 use sp_runtime_interface::pass_by::{
-	AllocateAndReturnByCodec, PassFatPointerAndDecode, PassFatPointerAndRead, PassPointerAndWrite,
+	AllocateAndReturnByCodec, AllocateAndReturnPointer, PassFatPointerAndDecode,
+	PassFatPointerAndRead,
 };
 use sp_storage::TrackedStorageKey;
 
@@ -253,9 +254,7 @@ sp_api::decl_runtime_apis! {
 /// WARNING! This is a non-deterministic call. Do not use this within
 /// consensus critical logic.
 pub fn current_time() -> u128 {
-	let mut out = [0; 16];
-	self::benchmarking::current_time(&mut out);
-	u128::from_le_bytes(out)
+	u128::from_le_bytes(self::benchmarking::current_time())
 }
 
 /// Interface that provides functions for benchmarking the runtime.
@@ -267,12 +266,12 @@ pub trait Benchmarking {
 	///
 	/// WARNING! This is a non-deterministic call. Do not use this within
 	/// consensus critical logic.
-	fn current_time(out: PassPointerAndWrite<&mut [u8; 16], 16>) {
-		*out = std::time::SystemTime::now()
+	fn current_time() -> AllocateAndReturnPointer<[u8; 16], 16> {
+		std::time::SystemTime::now()
 			.duration_since(std::time::SystemTime::UNIX_EPOCH)
 			.expect("Unix time doesn't go backwards; qed")
 			.as_nanos()
-			.to_le_bytes();
+			.to_le_bytes()
 	}
 
 	/// Reset the trie database to the genesis state.

substrate/primitives/core/src/lib.rs

@@ -200,53 +200,53 @@ pub trait TypeId {
 /// A log level matching the one from `log` crate.
 ///
 /// Used internally by `sp_io::logging::log` method.
-#[derive(Encode, Decode, Copy, Clone)]
-pub enum LogLevel {
+#[derive(Copy, Clone)]
+pub enum RuntimeInterfaceLogLevel {
 	/// `Error` log level.
-	Error = 1_isize,
+	Error = 0_isize,
 	/// `Warn` log level.
-	Warn = 2_isize,
+	Warn = 1_isize,
 	/// `Info` log level.
-	Info = 3_isize,
+	Info = 2_isize,
 	/// `Debug` log level.
-	Debug = 4_isize,
+	Debug = 3_isize,
 	/// `Trace` log level.
-	Trace = 5_isize,
+	Trace = 4_isize,
 }
 
-impl TryFrom<u8> for LogLevel {
+impl TryFrom<u8> for RuntimeInterfaceLogLevel {
 	type Error = ();
 	fn try_from(value: u8) -> Result<Self, ()> {
 		match value {
-			1 => Ok(Self::Error),
-			2 => Ok(Self::Warn),
-			3 => Ok(Self::Info),
-			4 => Ok(Self::Debug),
-			5 => Ok(Self::Trace),
+			0 => Ok(Self::Error),
+			1 => Ok(Self::Warn),
+			2 => Ok(Self::Info),
+			3 => Ok(Self::Debug),
+			4 => Ok(Self::Trace),
 			_ => Err(()),
 		}
 	}
 }
 
-impl From<LogLevel> for u8 {
-	fn from(value: LogLevel) -> Self {
+impl From<RuntimeInterfaceLogLevel> for u8 {
+	fn from(value: RuntimeInterfaceLogLevel) -> Self {
 		value as Self
 	}
 }
 
-impl From<u32> for LogLevel {
+impl From<u32> for RuntimeInterfaceLogLevel {
 	fn from(val: u32) -> Self {
 		match val {
-			x if x == LogLevel::Warn as u32 => LogLevel::Warn,
-			x if x == LogLevel::Info as u32 => LogLevel::Info,
-			x if x == LogLevel::Debug as u32 => LogLevel::Debug,
-			x if x == LogLevel::Trace as u32 => LogLevel::Trace,
-			_ => LogLevel::Error,
+			x if x == RuntimeInterfaceLogLevel::Warn as u32 => RuntimeInterfaceLogLevel::Warn,
+			x if x == RuntimeInterfaceLogLevel::Info as u32 => RuntimeInterfaceLogLevel::Info,
+			x if x == RuntimeInterfaceLogLevel::Debug as u32 => RuntimeInterfaceLogLevel::Debug,
+			x if x == RuntimeInterfaceLogLevel::Trace as u32 => RuntimeInterfaceLogLevel::Trace,
+			_ => RuntimeInterfaceLogLevel::Error,
 		}
 	}
 }
 
-impl From<log::Level> for LogLevel {
+impl From<log::Level> for RuntimeInterfaceLogLevel {
 	fn from(l: log::Level) -> Self {
 		use log::Level::*;
 		match l {
@@ -259,9 +259,9 @@ impl From<log::Level> for LogLevel {
 	}
 }
 
-impl From<LogLevel> for log::Level {
-	fn from(l: LogLevel) -> Self {
-		use self::LogLevel::*;
+impl From<RuntimeInterfaceLogLevel> for log::Level {
+	fn from(l: RuntimeInterfaceLogLevel) -> Self {
+		use self::RuntimeInterfaceLogLevel::*;
 		match l {
 			Error => Self::Error,
 			Warn => Self::Warn,

substrate/primitives/core/src/offchain/mod.rs

@@ -56,7 +56,7 @@ pub trait OffchainStorage: Clone + Send + Sync {
 }
 
 /// A type of supported crypto.
-#[derive(Clone, Copy, PartialEq, Eq, Encode, Decode, RuntimeDebug)]
+#[derive(Clone, Copy, PartialEq, Eq, RuntimeDebug)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[repr(C)]
 pub enum StorageKind {
@@ -66,12 +66,12 @@ pub enum StorageKind {
 	/// that is re-run at block `N(hash2)`.
 	/// This storage can be used by offchain workers to handle forks
 	/// and coordinate offchain workers running on different forks.
-	PERSISTENT = 1_isize,
+	PERSISTENT = 0_isize,
 	/// Local storage is revertible and fork-aware. It means that any value
 	/// set by the offchain worker triggered at block `N(hash1)` is reverted
 	/// if that block is reverted as non-canonical and is NOT available for the worker
 	/// that is re-run at block `N(hash2)`.
-	LOCAL = 2_isize,
+	LOCAL = 1_isize,
 }
 
 impl TryFrom<u32> for StorageKind {
@@ -120,11 +120,14 @@ impl From<HttpRequestId> for u32 {
 #[repr(C)]
 pub enum HttpError {
 	/// The requested action couldn't been completed within a deadline.
-	DeadlineReached = 1_isize,
+	#[codec(index = 1)]
+	DeadlineReached = 0_isize,
 	/// There was an IO Error while processing the request.
-	IoError = 2_isize,
+	#[codec(index = 2)]
+	IoError = 1_isize,
 	/// The ID of the request is invalid in this context.
-	Invalid = 3_isize,
+	#[codec(index = 3)]
+	Invalid = 2_isize,
 }
 
 impl TryFrom<u32> for HttpError {

substrate/primitives/io/src/lib.rs

@@ -104,7 +104,7 @@ use sp_core::{
 	},
 	sr25519,
 	storage::StateVersion,
-	LogLevel, LogLevelFilter, OpaquePeerId, H256,
+	LogLevelFilter, OpaquePeerId, RuntimeInterfaceLogLevel, H256,
 };
 
 #[cfg(feature = "bls-experimental")]
@@ -1767,7 +1767,7 @@ pub trait Logging {
 	///
 	/// Instead of using directly, prefer setting up `RuntimeLogger` and using `log` macros.
 	fn log(
-		level: PassAs<LogLevel, u8>,
+		level: PassAs<RuntimeInterfaceLogLevel, u8>,
 		target: PassFatPointerAndRead<&str>,
 		message: PassFatPointerAndRead<&[u8]>,
 	) {
@@ -1933,7 +1933,7 @@ pub fn panic(info: &core::panic::PanicInfo) -> ! {
 	}
 	#[cfg(not(feature = "improved_panic_error_reporting"))]
 	{
-		logging::log(LogLevel::Error, "runtime", message.as_bytes());
+		logging::log(RuntimeInterfaceLogLevel::Error, "runtime", message.as_bytes());
 		unreachable();
 	}
 }
@@ -1948,7 +1948,11 @@ pub fn oom(_: core::alloc::Layout) -> ! {
 	}
 	#[cfg(not(feature = "improved_panic_error_reporting"))]
 	{
-		logging::log(LogLevel::Error, "runtime", b"Runtime memory exhausted. Aborting");
+		logging::log(
+			RuntimeInterfaceLogLevel::Error,
+			"runtime",
+			b"Runtime memory exhausted. Aborting",
+		);
 		unreachable();
 	}
 }

substrate/primitives/runtime-interface/src/pass_by.rs

@@ -465,7 +465,7 @@ where
 		<U as FromFFIValue>::from_ffi_value(context, arg).and_then(|value| value.try_into()
 			.map_err(|_| format!(
 				"failed to convert '{}' (passed as '{}') into '{}' when marshalling hostcall's arguments through the FFI boundary",
-				type_name::<T>(),
+				type_name::<U>(),
 				type_name::<Self::FFIType>(),
 				type_name::<Self::Owned>()
 			)))