refactor: apply final comments

pallets/api/src/fungibles/benchmarking.rs

@@ -32,8 +32,8 @@ mod benchmarks {
 	use super::*;
 
 	// Parameter:
-	// - 'a': whether `approve_transfer` has been called.
-	// - 'c': whether `cancel_approval` has been called.
+	// - 'a': whether `approve_transfer` is required.
+	// - 'c': whether `cancel_approval` is required.
 	#[benchmark]
 	fn approve(a: Linear<0, 1>, c: Linear<0, 1>) -> Result<(), BenchmarkError> {
 		let asset_id = AssetIdOf::<T>::zero();

pallets/api/src/fungibles/mod.rs

@@ -1,6 +1,7 @@
 /// The fungibles pallet serves as a wrapper around the pallet_assets, offering a streamlined
 /// interface for interacting with fungible assets. The goal is to provide a simplified, consistent
 /// API that adheres to standards in the smart contract space.
+
 #[cfg(feature = "runtime-benchmarks")]
 mod benchmarking;
 #[cfg(test)]
@@ -49,10 +50,22 @@ pub mod pallet {
 		TotalSupply(AssetIdOf<T>),
 		/// Account balance for a given asset ID.
 		#[codec(index = 1)]
-		BalanceOf(AssetIdOf<T>, AccountIdOf<T>),
+		BalanceOf {
+			/// The asset ID.
+			id: AssetIdOf<T>,
+			/// The account ID of the owner.
+			owner: AccountIdOf<T>,
+		},
 		/// Allowance for a spender approved by an owner, for a given asset ID.
 		#[codec(index = 2)]
-		Allowance(AssetIdOf<T>, AccountIdOf<T>, AccountIdOf<T>),
+		Allowance {
+			/// The asset ID.
+			id: AssetIdOf<T>,
+			/// The account ID of the owner.
+			owner: AccountIdOf<T>,
+			/// The account ID of the spender.
+			spender: AccountIdOf<T>,
+		},
 		/// Token name for a given asset ID.
 		#[codec(index = 8)]
 		TokenName(AssetIdOf<T>),
@@ -69,7 +82,7 @@ pub mod pallet {
 	pub trait Config: frame_system::Config + pallet_assets::Config<Self::AssetsInstance> {
 		/// The instance of pallet assets it is tightly coupled to.
 		type AssetsInstance;
-		/// Weight information for extrinsics in this pallet.
+		/// Weight information for dispatchables in this pallet.
 		type WeightInfo: WeightInfo;
 	}
 
@@ -118,21 +131,22 @@ pub mod pallet {
 			let current_allowance = AssetsOf::<T>::allowance(id.clone(), &who, &spender);
 			let spender = T::Lookup::unlookup(spender);
 			let id: AssetIdParameterOf<T> = id.into();
+
 			// If the new value is equal to the current allowance, do nothing.
-			if value == current_allowance {
-				return Ok(Some(weight(0, 0)).into());
+			let return_weight = if value == current_allowance {
+				weight(0, 0)
 			}
 			// If the new value is greater than the current allowance, approve the difference
-			// because `approve_transfer` works additively (see pallet-assets).
-			if value > current_allowance {
+			// because `approve_transfer` works additively (see `pallet-assets`).
+			else if value > current_allowance {
 				AssetsOf::<T>::approve_transfer(
 					origin,
 					id,
 					spender,
 					value.saturating_sub(current_allowance),
 				)
 				.map_err(|e| e.with_weight(weight(1, 0)))?;
-				Ok(Some(weight(1, 0)).into())
+				weight(1, 0)
 			} else {
 				// If the new value is less than the current allowance, cancel the approval and set the new value
 				AssetsOf::<T>::cancel_approval(origin.clone(), id.clone(), spender.clone())
@@ -141,8 +155,9 @@ pub mod pallet {
 					return Ok(Some(weight(0, 1)).into());
 				}
 				AssetsOf::<T>::approve_transfer(origin, id, spender, value)?;
-				Ok(().into())
-			}
+				weight(1, 1)
+			};
+			Ok(Some(return_weight).into())
 		}
 
 		/// Increases the allowance of a spender.

pallets/api/src/fungibles/tests.rs

@@ -33,6 +33,9 @@ fn approve_works() {
 		// Approves an amount to spend that is higher than the current allowance.
 		assert_ok!(Fungibles::approve(signed(ALICE), ASSET, BOB, amount * 2));
 		assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), amount * 2);
+		// Approves an amount to spend that is equal to the current allowance.
+		assert_ok!(Fungibles::approve(signed(ALICE), ASSET, BOB, amount * 2));
+		assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), amount * 2);
 		// Sets allowance to zero.
 		assert_ok!(Fungibles::approve(signed(ALICE), ASSET, BOB, 0));
 		assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), 0);

pop-api/Cargo.toml

@@ -6,7 +6,6 @@ version = "0.0.0"
 edition = "2021"
 
 [dependencies]
-enumflags2 = { version = "0.7.7" }
 ink = { version = "5.0.0", default-features = false }
 sp-io = { version = "31.0.0", default-features = false, features = ["disable_panic_handler", "disable_oom", "disable_allocator"] }
 
@@ -20,7 +19,6 @@ crate-type = ["rlib"]
 [features]
 default = ["std"]
 std = [
-    "enumflags2/std",
     "ink/std",
     "pop-primitives/std",
     "sp-io/std",

runtime/devnet/Cargo.toml

@@ -90,7 +90,6 @@ parachain-info.workspace = true
 
 [dev-dependencies]
 env_logger = "0.11.2"
-enumflags2 = "0.7.9"
 hex = "0.4.3"
 rand = "0.8.5"
 

runtime/devnet/src/config/api.rs

@@ -2,9 +2,11 @@ use crate::{config::assets::TrustBackedAssetsInstance, fungibles, Runtime, Runti
 use codec::{Decode, Encode, MaxEncodedLen};
 use frame_support::traits::Contains;
 
+/// A query of runtime state.
 #[derive(Encode, Decode, Debug, MaxEncodedLen)]
 #[repr(u8)]
-pub enum RuntimeStateKeys<T: fungibles::Config> {
+pub enum RuntimeRead<T: fungibles::Config> {
+	/// Fungible token queries.
 	#[codec(index = 150)]
 	Fungibles(fungibles::Read<T>),
 }

runtime/devnet/src/extensions/mod.rs

@@ -2,7 +2,7 @@ mod v0;
 
 use crate::{
 	config::{
-		api::{AllowedApiCalls, RuntimeStateKeys},
+		api::{AllowedApiCalls, RuntimeRead},
 		assets::TrustBackedAssetsInstance,
 	},
 	fungibles::{
@@ -178,7 +178,7 @@ where
 
 	let result = match key {
 		VersionedStateRead::V0(key) => match key {
-			RuntimeStateKeys::Fungibles(key) => read_fungibles_state::<T, E>(key, env),
+			RuntimeRead::Fungibles(key) => read_fungibles_state::<T, E>(key, env),
 		},
 	}?
 	.encode();
@@ -189,18 +189,18 @@ where
 	env.write(&result, false, None)
 }
 
-/// Wrapper to enable versioning of `RuntimeStateKeys`.
+/// Wrapper to enable versioning of runtime state reads.
 #[derive(Decode, Debug)]
 enum VersionedStateRead<T: fungibles::Config> {
-	/// Version zero of reading state from api.
+	/// Version zero of state reads.
 	#[codec(index = 0)]
-	V0(RuntimeStateKeys<T>),
+	V0(RuntimeRead<T>),
 }
 
-/// Wrapper to enable versioning of `RuntimeCall`.
+/// Wrapper to enable versioning of runtime calls.
 #[derive(Decode, Debug)]
 enum VersionedDispatch {
-	/// Version zero of dispatch calls from api.
+	/// Version zero of dispatch calls.
 	#[codec(index = 0)]
 	V0(RuntimeCall),
 }
@@ -299,17 +299,13 @@ where
 	env.charge_weight(T::DbWeight::get().reads(1_u64))?;
 	match key {
 		TotalSupply(id) => Ok(fungibles::Pallet::<T>::total_supply(id).encode()),
-		BalanceOf(id, owner) => Ok(fungibles::Pallet::<T>::balance_of(id, &owner).encode()),
-		Allowance(id, owner, spender) => {
+		BalanceOf { id, owner } => Ok(fungibles::Pallet::<T>::balance_of(id, &owner).encode()),
+		Allowance { id, owner, spender } => {
 			Ok(fungibles::Pallet::<T>::allowance(id, &owner, &spender).encode())
 		},
 		TokenName(id) => Ok(fungibles::Pallet::<T>::token_name(id).encode()),
 		TokenSymbol(id) => Ok(fungibles::Pallet::<T>::token_symbol(id).encode()),
 		TokenDecimals(id) => Ok(fungibles::Pallet::<T>::token_decimals(id).encode()),
-		// AssetsKeys::AssetExists(id) => {
-		// 	env.charge_weight(T::DbWeight::get().reads(1_u64))?;
-		// 	Ok(pallet_assets::Pallet::<T, TrustBackedAssetsInstance>::asset_exists(id).encode())
-		// },
 	}
 }
 

runtime/devnet/src/lib.rs

@@ -966,6 +966,8 @@ mod tests {
 	use crate::Runtime;
 	use std::any::TypeId;
 
+	// Ensures that the account id lookup does not perform any state reads. When this changes,
+	// `pallet_api::fungibles` dispatchables need to be re-evaluated.
 	#[test]
 	fn test_lookup_config() {
 		type ExpectedLookup = sp_runtime::traits::AccountIdLookup<sp_runtime::AccountId32, ()>;

runtime/testnet/Cargo.toml

@@ -90,7 +90,6 @@ parachain-info.workspace = true
 [dev-dependencies]
 env_logger = "0.11.2"
 hex = "0.4.3"
-enumflags2 = "0.7.9"
 
 [features]
 default = ["std"]