refactor: optimising pop api and additional logic + test

pop-api/Cargo.toml

@@ -5,15 +5,12 @@ license = "GPL-3.0-only"
 version = "0.0.0"
 edition = "2021"
 
-[workspace]
-exclude = [
-    "integration-tests",
-]
 [dependencies]
 enumflags2 = { version = "0.7.7" }
 ink = { version = "5.0.0", default-features = false }
 scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] }
 scale-info = { version = "2.10.0", default-features = false, features = ["derive"] }
+sp-io = { version = "31.0.0", default-features = false, features = ["disable_panic_handler", "disable_oom", "disable_allocator"] }
 sp-runtime = { version = "32.0.0", default-features = false }
 
 pop-primitives = { path = "../primitives", features = ["devnet"], default-features = false }
@@ -31,6 +28,7 @@ std = [
     "pop-primitives/std",
     "scale/std",
     "scale-info/std",
+"sp-io/std",
     "sp-runtime/std",
 ]
 assets = ["pop-primitives/assets"]

pop-api/examples/fungibles/lib.rs

@@ -7,12 +7,12 @@
 ///
 use ink::prelude::vec::Vec;
 use pop_api::{
-	assets::fungibles::{self as api, FungiblesError},
-	error::{Error, StatusCode},
+	assets::fungibles::{self as api},
+	error::StatusCode,
 	primitives::{AccountId as AccountId32, AssetId},
 };
 
-pub type Result<T> = core::result::Result<T, Error>;
+pub type Result<T> = core::result::Result<T, StatusCode>;
 
 #[ink::contract(env = pop_api::Environment)]
 mod fungibles {
@@ -41,12 +41,14 @@ mod fungibles {
 
 		#[ink(message)]
 		pub fn total_supply(&self, id: AssetId) -> Result<Balance> {
-			api::total_supply(id).map_err(|e| e.into())
+			// api::total_supply(id).map_err(|e| e.into())
+			api::total_supply(id)
 		}
 
 		#[ink(message)]
 		pub fn balance_of(&self, id: AssetId, owner: AccountId32) -> Result<Balance> {
-			api::balance_of(id, owner).map_err(|e| e.into())
+			// api::balance_of(id, owner).map_err(|e| e.into())
+			api::balance_of(id, owner)
 		}
 
 		#[ink(message)]
@@ -56,7 +58,8 @@ mod fungibles {
 			owner: AccountId32,
 			spender: AccountId32,
 		) -> Result<Balance> {
-			api::allowance(id, owner, spender).map_err(|e| e.into())
+			// api::allowance(id, owner, spender).map_err(|e| e.into())
+			api::allowance(id, owner, spender)
 		}
 
 		#[ink(message)]
@@ -70,7 +73,8 @@ mod fungibles {
 
 			let result = api::transfer(id, to, value);
 			ink::env::debug_println!("Result: {:?}", result);
-			result.map_err(|e| e.into())
+			// result.map_err(|e| e.into())
+			result
 		}
 
 		#[ink(message)]
@@ -93,7 +97,8 @@ mod fungibles {
 
 			let result = api::transfer_from(id, from, to, value, &data);
 			ink::env::debug_println!("Result: {:?}", result);
-			result.map_err(|e| e.into())
+			// result.map_err(|e| e.into())
+			result
 		}
 
 		/// 2. PSP-22 Metadata Interface:
@@ -120,7 +125,8 @@ mod fungibles {
 			);
 			let result = api::create(id, admin, min_balance);
 			ink::env::debug_println!("Result: {:?}", result);
-			result.map_err(|e| e.into())
+			// result.map_err(|e| e.into())
+			result
 		}
 
 		#[ink(message)]
@@ -140,12 +146,14 @@ mod fungibles {
 			);
 			let result = api::set_metadata(id, name, symbol, decimals);
 			ink::env::debug_println!("Result: {:?}", result);
-			result.map_err(|e| e.into())
+			// result.map_err(|e| e.into())
+			result
 		}
 
 		#[ink(message)]
 		pub fn asset_exists(&self, id: AssetId) -> Result<bool> {
-			api::asset_exists(id).map_err(|e| e.into())
+			// api::asset_exists(id).map_err(|e| e.into())
+			api::asset_exists(id)
 		}
 	}
 

pop-api/src/error.rs

@@ -18,7 +18,10 @@ impl FromStatusCode for StatusCode {
 			0 => Ok(()),
 			_ => {
 				let mut encoded = status_code.to_le_bytes();
-				convert_unknown_errors(&mut encoded);
+				if unknown_errors(&encoded) {
+					encoded[..].rotate_right(1);
+					encoded[0] = 0u8;
+				};
 				Err(StatusCode::from(u32::from_le_bytes(encoded)))
 			},
 		}
@@ -27,18 +30,17 @@ impl FromStatusCode for StatusCode {
 
 impl From<scale::Error> for StatusCode {
 	fn from(_: scale::Error) -> Self {
-		DecodingFailed.into()
+		u32::from_le_bytes([255, 0, 0, 0]).into()
 	}
 }
 
-// If an unknown variant of the `DispatchError` is detected the error needs to be converted
-// into the encoded value of `Error::Other`. This conversion is performed by shifting the bytes one
-// position forward (discarding the last byte as it is not used) and setting the first byte to the
-// encoded value of `Other` (0u8). This ensures the error is correctly categorized as an `Other`
-// variant which provides all the necessary information to debug which error occurred in the runtime.
+// Converts unknown variants of the `DispatchError` into the encoded value of `PopApiError::Other`.
+// This conversion is done by shifting the bytes one position forward (discarding the last byte
+// as it is not used) and setting the first byte to the encoded value of `Other` (0u8). This ensures
+// the error is correctly categorized as an `Other` variant, providing necessary information for debugging.
 //
 // Byte layout explanation:
-// - Byte 0: index of the variant within `Error`
+// - Byte 0: index of the variant within `PopApiError`
 // - Byte 1:
 //   - Must be zero for `UNIT_ERRORS`.
 //   - Represents the nested error in `SINGLE_NESTED_ERRORS`.
@@ -48,45 +50,34 @@ impl From<scale::Error> for StatusCode {
 // - Byte 3:
 //   - Unused or represents further nested information.
 //
-// This mechanism ensures backward compatibility by correctly categorizing any unknown errors
-// into the `Other` variant, thus preventing issues caused by breaking changes.
-fn convert_unknown_errors(encoded_error: &mut [u8; 4]) {
-	let all_errors = [
-		UNIT_ERRORS.as_slice(),
-		SINGLE_NESTED_ERRORS.as_slice(),
-		DOUBLE_NESTED_ERRORS.as_slice(),
-		// `DecodingFailed`.
-		&[255u8],
-	]
-	.concat();
-	// Unknown errors, i.e. an encoded value where the first byte is non-zero (indicating a variant
-	// in `Error`) but unknown.
-	if !all_errors.contains(&encoded_error[0]) {
-		encoded_error[..].rotate_right(1);
-		encoded_error[0] = 0u8;
+// This mechanism ensures backward compatibility by categorizing unknown errors into the `Other` variant,
+// thus preventing issues caused by breaking changes.
+fn unknown_errors(encoded_error: &[u8; 4]) -> bool {
+	match encoded_error[0] {
+		code if UNIT_ERRORS.contains(&code) => nested_errors(&encoded_error[1..], None),
+		// Single nested errors with a limit in their nesting.
+		//
+		// `TokenError`: has ten variants - translated to a limit of nine.
+		7 => nested_errors(&encoded_error[1..], Some(9)),
+		// `ArithmeticError`: has 3 variants - translated to a limit of two.
+		8 => nested_errors(&encoded_error[1..], Some(2)),
+		// `TransactionalError`: has 2 variants - translated to a limit of one.
+		9 => nested_errors(&encoded_error[1..], Some(1)),
+		code if DOUBLE_NESTED_ERRORS.contains(&code) => nested_errors(&encoded_error[3..], None),
+		_ => true,
 	}
-	convert_unknown_nested_errors(encoded_error);
 }
 
-// If an unknown nested variant of the `DispatchError` is detected (i.e. when any of the subsequent
-// bytes are non-zero).
-fn convert_unknown_nested_errors(encoded_error: &mut [u8; 4]) {
-	// Converts single nested errors that are known to the Pop API as unit errors into `Other`.
-	if UNIT_ERRORS.contains(&encoded_error[0]) && encoded_error[1..].iter().any(|x| *x != 0u8) {
-		encoded_error[..].rotate_right(1);
-		encoded_error[0] = 0u8;
-	// Converts double nested errors that are known to the Pop API as single nested errors into
-	// `Other`.
-	} else if SINGLE_NESTED_ERRORS.contains(&encoded_error[0])
-		&& encoded_error[2..].iter().any(|x| *x != 0u8)
-	{
-		encoded_error[..].rotate_right(1);
-		encoded_error[0] = 0u8;
-	} else if DOUBLE_NESTED_ERRORS.contains(&encoded_error[0])
-		&& encoded_error[3..].iter().any(|x| *x != 0u8)
-	{
-		encoded_error[..].rotate_right(1);
-		encoded_error[0] = 0u8;
+// Checks for unknown nested errors within the `DispatchError`.
+// - For single nested errors with a limit, it verifies if the nested value exceeds the limit.
+// - For other nested errors, it checks if any subsequent bytes are non-zero.
+//
+// `nested_error` - The slice of bytes representing the nested error.
+// `limit` - An optional limit for single nested errors.
+fn nested_errors(nested_error: &[u8], limit: Option<u8>) -> bool {
+	match limit {
+		Some(l) => nested_error[0] > l || nested_error[1..].iter().any(|&x| x != 0u8),
+		None => nested_error.iter().any(|&x| x != 0u8),
 	}
 }
 
@@ -150,18 +141,14 @@ pub enum Error {
 // - DecodingFailed: 255,
 const UNIT_ERRORS: [u8; 10] = [1, 2, 4, 5, 6, 10, 11, 12, 13, 255];
 
-// Single nested `Error` variants.
-// (variant: index):
-// - Token: 7,
-// - Arithmetic: 8,
-// - Transaction: 9,
 const SINGLE_NESTED_ERRORS: [u8; 3] = [7, 8, 9];
 
 // Double nested `Error` variants
 // (variant: index):
 // - Module: 3,
 const DOUBLE_NESTED_ERRORS: [u8; 1] = [3];
 
+#[cfg(test)]
 impl From<Error> for StatusCode {
 	fn from(value: Error) -> Self {
 		let mut encoded_error = value.encode();
@@ -391,6 +378,37 @@ mod tests {
 		);
 	}
 
+	#[test]
+	fn single_nested_unknown_variants() {
+		// Unknown `TokenError` variant.
+		assert_eq!(
+			into_error([7, 10, 0, 0]),
+			Other { dispatch_error_index: 7, error_index: 10, error: 0 }
+		);
+		assert_eq!(
+			into_status_code([7, 10, 0, 0]),
+			Other { dispatch_error_index: 7, error_index: 10, error: 0 }.into()
+		);
+		// Unknown `Arithmetic` variant.
+		assert_eq!(
+			into_error([8, 3, 0, 0]),
+			Other { dispatch_error_index: 8, error_index: 3, error: 0 }
+		);
+		assert_eq!(
+			into_status_code([8, 3, 0, 0]),
+			Other { dispatch_error_index: 8, error_index: 3, error: 0 }.into()
+		);
+		// Unknown `Transactional` variant.
+		assert_eq!(
+			into_error([9, 2, 0, 0]),
+			Other { dispatch_error_index: 9, error_index: 2, error: 0 }
+		);
+		assert_eq!(
+			into_status_code([9, 2, 0, 0]),
+			Other { dispatch_error_index: 9, error_index: 2, error: 0 }.into()
+		);
+	}
+
 	#[test]
 	fn test_random_encoded_values() {
 		assert_eq!(

pop-api/src/v0/assets/fungibles.rs

@@ -367,12 +367,25 @@ mod tests {
 		TokenError::*,
 		TransactionalError::*,
 	};
+	use scale::Decode;
 
 	fn into_fungibles_error(error: Error) -> FungiblesError {
 		let status_code: StatusCode = error.into();
 		status_code.into()
 	}
 
+	#[test]
+	fn status_code_vs_encoded() {
+		assert_eq!(u32::decode(&mut &[3u8, 10, 2, 0][..]).unwrap(), 133635u32);
+		assert_eq!(u32::decode(&mut &[3u8, 52, 0, 0][..]).unwrap(), 13315u32);
+		assert_eq!(u32::decode(&mut &[3u8, 52, 1, 0][..]).unwrap(), 78851u32);
+		assert_eq!(u32::decode(&mut &[3u8, 52, 2, 0][..]).unwrap(), 144387u32);
+		assert_eq!(u32::decode(&mut &[3u8, 52, 3, 0][..]).unwrap(), 209923u32);
+		assert_eq!(u32::decode(&mut &[3u8, 52, 5, 0][..]).unwrap(), 340995u32);
+		assert_eq!(u32::decode(&mut &[3u8, 52, 7, 0][..]).unwrap(), 472067u32);
+		assert_eq!(u32::decode(&mut &[3u8, 52, 10, 0][..]).unwrap(), 668675u32);
+	}
+
 	#[test]
 	fn conversion_status_code_into_fungibles_error_works() {
 		let errors = vec![

pop-api/src/v0/state.rs

@@ -2,5 +2,7 @@ use crate::{primitives::storage_keys::RuntimeStateKeys, read_state, Error};
 use scale::Decode;
 
 pub fn read<T: Decode>(key: RuntimeStateKeys) -> crate::Result<T> {
-	read_state(key).and_then(|v| T::decode(&mut &v[..]).map_err(|_e| Error::DecodingFailed.into()))
+	read_state(key).and_then(|v| {
+		T::decode(&mut &v[..]).map_err(|_e| u32::from_le_bytes([255, 0, 0, 0]).into())
+	})
 }