refactor: api event naming conventions

extension/src/functions.rs

@@ -80,7 +80,7 @@
 
 /// A function for reading runtime state.
 pub struct ReadState<M, C, R, D, F, RC = DefaultConverter<<R as Readable>::Result>, E = (), L = ()>(
 	PhantomData<(M, C, R, D, F, RC, E, L)>,
 );
 impl<
 		Matcher: Matches,
@@ -88,7 +88,7 @@
 		Read: Readable + Debug,
 		Decoder: Decode<Output: codec::Decode + Into<Read>>,
 		Filter: Contains<Read>,
-		ResultConverter: Converter<Source = Read::Result, Target: Into<Vec<u8>>>,
+		ResultConverter: Converter<Source = Read::Result, Target: Into<Vec<u8>>, Error = DispatchError>,
 		Error: ErrorConverter,
 		Logger: LogTarget,
 	> Function for ReadState<Matcher, Config, Read, Decoder, Filter, ResultConverter, Error, Logger>
@@ -116,7 +116,7 @@
 		log::debug!(target: Logger::LOG_TARGET, "read: result={result:?}");
 		// Perform any final conversion. Any implementation is expected to charge weight as
 		// appropriate.
-		let result = ResultConverter::convert(result, env).into();
+		let result = ResultConverter::try_convert(result, env)?.into();
 		log::debug!(target: Logger::LOG_TARGET, "converted: result={result:?}");
 		// Charge appropriate weight for writing to contract, based on result length.
 		let weight = ContractWeightsOf::<Config>::seal_input(result.len() as u32);
@@ -146,12 +146,16 @@
 	fn read(self) -> Self::Result;
 }
 
-/// Trait for converting a value based on additional information available from the environment.
+/// Trait for fallible conversion of a value based on additional information available from the
+/// environment.
 pub trait Converter {
+	/// The type returned in the event of a conversion error.
+	type Error;
 	/// The type of value to be converted.
 	type Source;
 	/// The target type.
 	type Target;
+
 	/// The log target.
 	const LOG_TARGET: &'static str;
 
@@ -160,19 +164,29 @@
 	/// # Parameters
 	/// - `value` - The value to be converted.
 	/// - `env` - The current execution environment.
-	fn convert(value: Self::Source, env: &impl Environment) -> Self::Target;
+	fn try_convert(
+		value: Self::Source,
+		env: &impl Environment,
+	) -> core::result::Result<Self::Target, Self::Error>;
 }
 
 /// A default converter, for converting (encoding) from some type into a byte array.
 pub struct DefaultConverter<T>(PhantomData<T>);
 impl<T: Into<Vec<u8>>> Converter for DefaultConverter<T> {
+	/// The type returned in the event of a conversion error.
+	type Error = DispatchError;
+	/// The type of value to be converted.
 	type Source = T;
+	/// The target type.
 	type Target = Vec<u8>;
 
 	const LOG_TARGET: &'static str = "";
 
-	fn convert(value: Self::Source, _env: &impl Environment) -> Self::Target {
-		value.into()
+	fn try_convert(
+		value: Self::Source,
+		_env: &impl Environment,
+	) -> core::result::Result<Self::Target, Self::Error> {
+		Ok(value.into())
 	}
 }
 
@@ -462,7 +476,7 @@
 				Ok(Converging(0))
 			));
 			// Check if the contract environment buffer is written correctly.
-			assert_eq!(env.buffer, UppercaseConverter::convert(expected, &env));
+			assert_eq!(env.buffer, UppercaseConverter::try_convert(expected, &env).unwrap());
 		}
 
 		#[test]
@@ -538,6 +552,6 @@
 	fn default_conversion_works() {
 		let env = MockEnvironment::default();
 		let source = "pop".to_string();
-		assert_eq!(DefaultConverter::convert(source.clone(), &env), source.as_bytes());
+		assert_eq!(DefaultConverter::try_convert(source.clone(), &env).unwrap(), source.as_bytes());
 	}
 }

extension/src/matching.rs

@@ -72,13 +72,19 @@ mod tests {
 	}
 
 	#[test]
-	fn func_id_matches() {
+	fn with_func_id_matches() {
 		let env = MockEnvironment::default();
 		assert!(WithFuncId::<ConstU32<0>>::matches(&env));
+
+		let env = MockEnvironment::new(1, vec![]);
+		assert!(WithFuncId::<ConstU32<1>>::matches(&env));
+
+		let env = MockEnvironment::new(100, vec![]);
+		assert!(WithFuncId::<ConstU32<100>>::matches(&env));
 	}
 
 	#[test]
-	fn func_id_does_not_match() {
+	fn with_func_id_does_not_match() {
 		let env = MockEnvironment::new(1, vec![]);
 		assert!(!WithFuncId::<ConstU32<0>>::matches(&env));
 	}

extension/src/mock.rs

@@ -9,7 +9,7 @@ use frame_support::{
 };
 use frame_system::{pallet_prelude::BlockNumberFor, EnsureSigned};
 use pallet_contracts::{chain_extension::RetVal, DefaultAddressGenerator, Frame, Schedule};
-use sp_runtime::{BuildStorage, Perbill};
+use sp_runtime::{BuildStorage, DispatchError, Perbill};
 
 use crate::{
 	decoding::Identity, environment, matching::WithFuncId, AccountIdOf, ContractWeightsOf,
@@ -373,14 +373,18 @@ pub(crate) fn new_test_ext() -> sp_io::TestExternalities {
 /// A converter for converting string results to uppercase.
 pub(crate) struct UppercaseConverter;
 impl Converter for UppercaseConverter {
+	type Error = DispatchError;
 	type Source = RuntimeResult;
 	type Target = Vec<u8>;
 
 	const LOG_TARGET: &'static str = "";
 
-	fn convert(value: Self::Source, _env: &impl crate::Environment) -> Self::Target {
+	fn try_convert(
+		value: Self::Source,
+		_env: &impl crate::Environment,
+	) -> Result<Self::Target, Self::Error> {
 		match value {
-			RuntimeResult::Pong(value) => value.to_uppercase().encode(),
+			RuntimeResult::Pong(value) => Ok(value.to_uppercase().encode()),
 		}
 	}
 }

pallets/api/src/extension.rs

@@ -2,7 +2,7 @@ use core::{fmt::Debug, marker::PhantomData};
 
 use frame_support::traits::Get;
 pub use pop_chain_extension::{
-	Config, ContractWeightsOf, DecodingFailed, DispatchCall, ReadState, Readable,
+	Config, ContractWeightsOf, DecodingFailed, DispatchCall, ErrorConverter, ReadState, Readable,
 };
 use pop_chain_extension::{
 	Converter, Decodes, Environment, LogTarget, Matches, Processor, Result, RetVal,
@@ -70,7 +70,7 @@ impl LogTarget for ReadStateLogTarget {
 
 /// Conversion of a `DispatchError` to a versioned error.
 pub struct VersionedErrorConverter<E>(PhantomData<E>);
-impl<Error: From<(DispatchError, u8)> + Into<u32> + Debug> pop_chain_extension::ErrorConverter
+impl<Error: TryFrom<(DispatchError, u8), Error = DispatchError> + Into<u32> + Debug> ErrorConverter
 	for VersionedErrorConverter<Error>
 {
 	/// The log target.
@@ -85,17 +85,19 @@ impl<Error: From<(DispatchError, u8)> + Into<u32> + Debug> pop_chain_extension::
 		// Defer to supplied versioned error conversion type
 		let version = version(env);
 		log::debug!(target: Self::LOG_TARGET, "versioned error converter: error={error:?}, version={version}");
-		let error: Error = (error, version).into();
+		let error: Error = (error, version).try_into()?;
 		log::debug!(target: Self::LOG_TARGET, "versioned error converter: converted error={error:?}");
 		Ok(RetVal::Converging(error.into()))
 	}
 }
 
 /// Conversion of a read result to a versioned read result.
 pub struct VersionedResultConverter<S, T>(PhantomData<(S, T)>);
-impl<Source: Debug, Target: From<(Source, u8)> + Debug> Converter
+impl<Source: Debug, Target: TryFrom<(Source, u8), Error = DispatchError> + Debug> Converter
 	for VersionedResultConverter<Source, Target>
 {
+	/// The type returned in the event of a conversion error.
+	type Error = DispatchError;
 	/// The type of value to be converted.
 	type Source = Source;
 	/// The target type.
@@ -109,47 +111,199 @@ impl<Source: Debug, Target: From<(Source, u8)> + Debug> Converter
 	/// # Parameters
 	/// - `value` - The value to be converted.
 	/// - `env` - The current execution environment.
-	fn convert(value: Self::Source, env: &impl Environment) -> Self::Target {
+	fn try_convert(value: Self::Source, env: &impl Environment) -> Result<Self::Target> {
 		// Defer to supplied versioned result conversion type
 		let version = version(env);
 		log::debug!(target: Self::LOG_TARGET, "versioned result converter: result={value:?}, version={version}");
-		let converted: Target = (value, version).into();
+		let converted: Target = (value, version).try_into()?;
 		log::debug!(target: Self::LOG_TARGET, "versioned result converter: converted result={converted:?}");
-		converted
+		Ok(converted)
 	}
 }
 
 fn func_id(env: &impl Environment) -> u8 {
-	// TODO: update once the encoding scheme order has been finalised: expected to be
-	// env.ext_id().to_le_bytes()[0]
-	env.func_id().to_le_bytes()[1]
+	env.func_id().to_le_bytes()[0]
 }
 
 fn module_and_index(env: &impl Environment) -> (u8, u8) {
-	// TODO: update once the encoding scheme order has been finalised: expected to be
-	// env.func_id().to_le_bytes()[0..1]
 	let bytes = env.ext_id().to_le_bytes();
 	(bytes[0], bytes[1])
 }
 
 fn version(env: &impl Environment) -> u8 {
-	// TODO: update once the encoding scheme order has been finalised: expected to be
-	// env.ext_id().to_le_bytes()[1]
-	env.func_id().to_le_bytes()[0]
+	env.func_id().to_le_bytes()[1]
 }
 
 #[cfg(test)]
 mod tests {
 	use frame_support::pallet_prelude::Weight;
 	use pop_chain_extension::Ext;
+	use sp_core::ConstU8;
 
-	use super::*;
+	use super::{DispatchError::*, *};
 	use crate::extension::Prepender;
 
+	#[test]
+	fn func_id_works() {
+		let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 };
+		assert_eq!(func_id(&env), 1);
+	}
+
+	#[test]
+	fn module_and_index_works() {
+		let env = MockEnvironment { func_id: 0u16, ext_id: u16::from_le_bytes([2, 3]) };
+		assert_eq!(module_and_index(&env), (2, 3));
+	}
+
+	#[test]
+	fn version_works() {
+		let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 };
+		assert_eq!(version(&env), 2);
+	}
+
 	#[test]
 	fn prepender_works() {
-		let env = MockEnvironment { func_id: 1, ext_id: u16::from_le_bytes([2, 3]) };
-		assert_eq!(Prepender::process(vec![0u8], &env), vec![1, 2, 3, 0]);
+		let env = MockEnvironment {
+			func_id: u16::from_le_bytes([1, 2]),
+			ext_id: u16::from_le_bytes([3, 4]),
+		};
+		assert_eq!(Prepender::process(vec![0u8], &env), vec![2, 3, 4, 0]);
+		assert_eq!(Prepender::process(vec![0u8, 5, 10], &env), vec![2, 3, 4, 0, 5, 10]);
+	}
+
+	#[test]
+	fn identified_by_first_byte_of_function_id_matches() {
+		let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 };
+		assert!(IdentifiedByFirstByteOfFunctionId::<ConstU8<1>>::matches(&env));
+	}
+
+	#[test]
+	fn identified_by_first_byte_of_function_id_does_not_match() {
+		let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 };
+		assert!(!IdentifiedByFirstByteOfFunctionId::<ConstU8<2>>::matches(&env));
+	}
+
+	#[test]
+	fn dispatch_call_log_target_works() {
+		assert!(matches!(
+			<DispatchCallLogTarget as LogTarget>::LOG_TARGET,
+			"pop-api::extension::dispatch"
+		));
+	}
+
+	#[test]
+	fn read_state_log_target_works() {
+		assert!(matches!(
+			<ReadStateLogTarget as LogTarget>::LOG_TARGET,
+			"pop-api::extension::read-state"
+		));
+	}
+
+	#[test]
+	fn versioned_error_converter_works() {
+		use super::RetVal::Converging;
+
+		// Mock versioned error.
+		#[derive(Debug)]
+		pub enum VersionedError {
+			V0(DispatchError),
+			V1(DispatchError),
+		}
+
+		impl TryFrom<(DispatchError, u8)> for VersionedError {
+			type Error = DispatchError;
+
+			fn try_from(value: (DispatchError, u8)) -> Result<Self> {
+				let (error, version) = value;
+				match version {
+					0 => Ok(VersionedError::V0(error)),
+					1 => Ok(VersionedError::V1(error)),
+					_ => Err(Other("DecodingFailed")),
+				}
+			}
+		}
+
+		impl From<VersionedError> for u32 {
+			// Mock conversion based on error and version.
+			fn from(value: VersionedError) -> Self {
+				match value {
+					VersionedError::V0(error) => match error {
+						BadOrigin => 1,
+						_ => 100,
+					},
+					VersionedError::V1(error) => match error {
+						BadOrigin => 2,
+						_ => 200,
+					},
+				}
+			}
+		}
+
+		// Because `Retval` does not implement the `Debug` trait the success and error test cases
+		// are separated.
+		for (version, error, expected_result) in vec![
+			(0, BadOrigin, 1),
+			(0, CannotLookup, 100),
+			(1, BadOrigin, 2),
+			(1, CannotLookup, 200),
+		] {
+			let env = MockEnvironment { func_id: u16::from_le_bytes([0, version]), ext_id: 0u16 };
+			// Again, due to missing `Debug` trait the result has to be unwrapped.
+			let Ok(Converging(result)) =
+				VersionedErrorConverter::<VersionedError>::convert(error, &env)
+			else {
+				panic!("should not happen")
+			};
+			assert_eq!(result, expected_result);
+		}
+
+		// Error case.
+		let env = MockEnvironment { func_id: u16::from_le_bytes([0, 2]), ext_id: 0u16 };
+		let result = VersionedErrorConverter::<VersionedError>::convert(BadOrigin, &env)
+			.err()
+			.unwrap(); // Again, can't use `unwrap_err()` due to missing `Debug` trait.
+		assert_eq!(result, Other("DecodingFailed"));
+	}
+
+	#[test]
+	fn versioned_result_converter_works() {
+		use VersionedRuntimeResult::{V0, V1};
+
+		// Mock versioned runtime result.
+		#[derive(Debug, PartialEq)]
+		pub enum VersionedRuntimeResult {
+			V0(u8),
+			V1(u8),
+		}
+
+		impl TryFrom<(u8, u8)> for VersionedRuntimeResult {
+			type Error = DispatchError;
+
+			// Mock conversion based on result and version.
+			fn try_from(value: (u8, u8)) -> Result<Self> {
+				let (result, version) = value;
+				match version {
+					0 if result <= 50 => Ok(V0(result)),
+					0 if result > 50 => Ok(V0(50)),
+					1 if result <= 100 => Ok(V1(result)),
+					1 if result > 100 => Ok(V1(100)),
+					_ => Err(Other("DecodingFailed")),
+				}
+			}
+		}
+
+		for (version, value, expected_result) in vec![
+			(0, 10, Ok(V0(10))),
+			(0, 100, Ok(V0(50))),
+			(1, 10, Ok(V1(10))),
+			(1, 100, Ok(V1(100))),
+			(2, 10, Err(Other("DecodingFailed"))),
+		] {
+			let env = MockEnvironment { func_id: u16::from_le_bytes([0, version]), ext_id: 0u16 };
+			let result =
+				VersionedResultConverter::<u8, VersionedRuntimeResult>::try_convert(value, &env);
+			assert_eq!(result, expected_result);
+		}
 	}
 
 	struct MockEnvironment {