From e8a304574cd49fae998b2d2fc1843ffe102f4ada Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Tue, 10 Sep 2024 11:15:33 +0200 Subject: [PATCH 01/29] refactor: encoding scheme --- extension/src/matching.rs | 6 ++++++ pallets/api/src/extension.rs | 15 ++++++--------- pop-api/src/lib.rs | 17 +++++++++-------- pop-api/src/v0/mod.rs | 7 ++++--- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/extension/src/matching.rs b/extension/src/matching.rs index 0dcdd34d..09e28be9 100644 --- a/extension/src/matching.rs +++ b/extension/src/matching.rs @@ -75,6 +75,12 @@ mod tests { fn func_id_matches() { let env = MockEnvironment::default(); assert!(WithFuncId::>::matches(&env)); + + let env = MockEnvironment::new(1, vec![]); + assert!(WithFuncId::>::matches(&env)); + + let env = MockEnvironment::new(100, vec![]); + assert!(WithFuncId::>::matches(&env)); } #[test] diff --git a/pallets/api/src/extension.rs b/pallets/api/src/extension.rs index 138cdee6..fae89911 100644 --- a/pallets/api/src/extension.rs +++ b/pallets/api/src/extension.rs @@ -120,22 +120,16 @@ impl + Debug> Converter } 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)] @@ -148,7 +142,10 @@ mod tests { #[test] fn prepender_works() { - let env = MockEnvironment { func_id: 1, ext_id: u16::from_le_bytes([2, 3]) }; + let env = MockEnvironment { + func_id: u16::from_le_bytes([0, 1]), + ext_id: u16::from_le_bytes([2, 3]), + }; assert_eq!(Prepender::process(vec![0u8], &env), vec![1, 2, 3, 0]); } diff --git a/pop-api/src/lib.rs b/pop-api/src/lib.rs index 0a080c56..c3ac139e 100644 --- a/pop-api/src/lib.rs +++ b/pop-api/src/lib.rs @@ -1,9 +1,10 @@ -//! The `pop-api` crate provides an API for smart contracts to interact with the Pop Network runtime. +//! The `pop-api` crate provides an API for smart contracts to interact with the Pop Network +//! runtime. //! -//! This crate abstracts away complexities to deliver a streamlined developer experience while supporting -//! multiple API versions to ensure backward compatibility. It is designed with a focus on stability, -//! future-proofing, and storage efficiency, allowing developers to easily integrate powerful runtime -//! features into their contracts without unnecessary overhead. +//! This crate abstracts away complexities to deliver a streamlined developer experience while +//! supporting multiple API versions to ensure backward compatibility. It is designed with a focus +//! on stability, future-proofing, and storage efficiency, allowing developers to easily integrate +//! powerful runtime features into their contracts without unnecessary overhead. #![cfg_attr(not(feature = "std"), no_std, no_main)] @@ -38,17 +39,17 @@ mod constants { // Helper method to build a dispatch call or a call to read state. // // Parameters: -// - 'version': The version of the chain extension. // - 'function': The ID of the function. +// - 'version': The version of the chain extension. // - 'module': The index of the runtime module. // - 'dispatchable': The index of the module dispatchable functions. fn build_extension_method( - version: u8, function: u8, + version: u8, module: u8, dispatchable: u8, ) -> ChainExtensionMethod<(), (), (), false> { - ChainExtensionMethod::build(u32::from_le_bytes([version, function, module, dispatchable])) + ChainExtensionMethod::build(u32::from_le_bytes([function, version, module, dispatchable])) } /// Represents a status code returned by the runtime. diff --git a/pop-api/src/v0/mod.rs b/pop-api/src/v0/mod.rs index 8ed35058..304c3f73 100644 --- a/pop-api/src/v0/mod.rs +++ b/pop-api/src/v0/mod.rs @@ -1,10 +1,11 @@ +use ink::env::chain_extension::ChainExtensionMethod; + use crate::{ build_extension_method, constants::{DISPATCH, READ_STATE}, primitives::Error, StatusCode, }; -use ink::env::chain_extension::ChainExtensionMethod; /// APIs for fungible tokens. #[cfg(feature = "fungibles")] @@ -24,7 +25,7 @@ impl From for Error { // - 'module': The index of the runtime module. // - 'dispatchable': The index of the module dispatchable functions. fn build_dispatch(module: u8, dispatchable: u8) -> ChainExtensionMethod<(), (), (), false> { - build_extension_method(V0, DISPATCH, module, dispatchable) + build_extension_method(DISPATCH, V0, module, dispatchable) } // Helper method to build a call to read state. @@ -33,5 +34,5 @@ fn build_dispatch(module: u8, dispatchable: u8) -> ChainExtensionMethod<(), (), // - 'module': The index of the runtime module. // - 'state_query': The index of the runtime state query. fn build_read_state(module: u8, state_query: u8) -> ChainExtensionMethod<(), (), (), false> { - build_extension_method(V0, READ_STATE, module, state_query) + build_extension_method(READ_STATE, V0, module, state_query) } From b16430bf25f0d1a0b9dd31cde66b28def80bca5c Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Tue, 10 Sep 2024 12:50:29 +0200 Subject: [PATCH 02/29] tests: pallet api extension --- extension/src/matching.rs | 4 +- pallets/api/src/extension.rs | 166 +++++++++++++++++++++++++++++++++-- 2 files changed, 162 insertions(+), 8 deletions(-) diff --git a/extension/src/matching.rs b/extension/src/matching.rs index 09e28be9..94939597 100644 --- a/extension/src/matching.rs +++ b/extension/src/matching.rs @@ -72,7 +72,7 @@ mod tests { } #[test] - fn func_id_matches() { + fn with_func_id_matches() { let env = MockEnvironment::default(); assert!(WithFuncId::>::matches(&env)); @@ -84,7 +84,7 @@ mod tests { } #[test] - fn func_id_does_not_match() { + fn with_func_id_does_not_match() { let env = MockEnvironment::new(1, vec![]); assert!(!WithFuncId::>::matches(&env)); } diff --git a/pallets/api/src/extension.rs b/pallets/api/src/extension.rs index fae89911..ae85b453 100644 --- a/pallets/api/src/extension.rs +++ b/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(PhantomData); -impl + Into + Debug> pop_chain_extension::ErrorConverter +impl + Into + Debug> ErrorConverter for VersionedErrorConverter { /// The log target. @@ -136,17 +136,171 @@ fn version(env: &impl Environment) -> u8 { 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); + let env = MockEnvironment { func_id: u16::from_le_bytes([2, 1]), ext_id: 0u16 }; + assert_eq!(func_id(&env), 2); + } + + #[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)); + let env = MockEnvironment { func_id: 0u16, ext_id: u16::from_le_bytes([3, 2]) }; + assert_eq!(module_and_index(&env), (3, 2)); + } + + #[test] + fn version_works() { + let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 }; + assert_eq!(version(&env), 2); + let env = MockEnvironment { func_id: u16::from_le_bytes([2, 1]), ext_id: 0u16 }; + assert_eq!(version(&env), 1); + } + #[test] fn prepender_works() { let env = MockEnvironment { - func_id: u16::from_le_bytes([0, 1]), - ext_id: u16::from_le_bytes([2, 3]), + func_id: u16::from_le_bytes([1, 2]), + ext_id: u16::from_le_bytes([3, 4]), }; - assert_eq!(Prepender::process(vec![0u8], &env), vec![1, 2, 3, 0]); + 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]); + + let env = MockEnvironment { + func_id: u16::from_le_bytes([2, 1]), + ext_id: u16::from_le_bytes([4, 3]), + }; + assert_eq!(Prepender::process(vec![0u8], &env), vec![1, 4, 3, 0]); + assert_eq!(Prepender::process(vec![0u8, 5, 10], &env), vec![1, 4, 3, 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::>::matches(&env)); + let env = MockEnvironment { func_id: u16::from_le_bytes([2, 1]), ext_id: 0u16 }; + assert!(IdentifiedByFirstByteOfFunctionId::>::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::>::matches(&env)); + let env = MockEnvironment { func_id: u16::from_le_bytes([2, 1]), ext_id: 0u16 }; + assert!(!IdentifiedByFirstByteOfFunctionId::>::matches(&env)); + } + + #[test] + fn dispatch_call_log_target_works() { + assert!(matches!( + ::LOG_TARGET, + "pop-api::extension::dispatch" + )); + } + + #[test] + fn read_state_log_target_works() { + assert!(matches!( + ::LOG_TARGET, + "pop-api::extension::read-state" + )); + } + + #[test] + fn versioned_error_converter_works() { + // Mock versioned error. + #[derive(Debug)] + pub enum VersionedError { + V0(DispatchError), + V1(DispatchError), + } + + impl From<(DispatchError, u8)> for VersionedError { + fn from(value: (DispatchError, u8)) -> Self { + let (error, version) = value; + match version { + 0 => VersionedError::V0(error), + 1 => VersionedError::V1(error), + _ => unimplemented!(), + } + } + } + + impl From 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, + }, + } + } + } + + 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 }; + let RetVal::Converging(result) = + VersionedErrorConverter::::convert(error, &env) + .expect("should always result `Ok`") + else { + unimplemented!(); + }; + assert_eq!(result, expected_result); + } + } + + #[test] + fn versioned_result_converter_works() { + // Mock versioned runtime result. + #[derive(Debug, PartialEq)] + pub enum VersionedRuntimeResult { + V0(u8), + V1(u8), + } + + impl From<(u8, u8)> for VersionedRuntimeResult { + // Mock conversion based on result and version. + fn from(value: (u8, u8)) -> Self { + let (result, version) = value; + match version { + 0 if result <= 50 => VersionedRuntimeResult::V0(result), + 0 if result > 50 => VersionedRuntimeResult::V0(50), + 1 if result <= 100 => VersionedRuntimeResult::V1(result), + 1 if result > 100 => VersionedRuntimeResult::V1(100), + _ => unimplemented!(), + } + } + } + + for (version, value, expected_result) in vec![ + (0, 10, VersionedRuntimeResult::V0(10)), + (0, 100, VersionedRuntimeResult::V0(50)), + (1, 10, VersionedRuntimeResult::V1(10)), + (1, 100, VersionedRuntimeResult::V1(100)), + ] { + let env = MockEnvironment { func_id: u16::from_le_bytes([0, version]), ext_id: 0u16 }; + let result = + VersionedResultConverter::::convert(value, &env); + assert_eq!(result, expected_result); + } } struct MockEnvironment { From 2179645bbe3bd0f14f9acccf496d2f98e9adff81 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Tue, 10 Sep 2024 12:56:15 +0200 Subject: [PATCH 03/29] docs: tests --- pallets/api/src/extension.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pallets/api/src/extension.rs b/pallets/api/src/extension.rs index ae85b453..39619c2f 100644 --- a/pallets/api/src/extension.rs +++ b/pallets/api/src/extension.rs @@ -143,6 +143,7 @@ mod tests { #[test] fn func_id_works() { + // Test ensuring little endian. let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 }; assert_eq!(func_id(&env), 1); let env = MockEnvironment { func_id: u16::from_le_bytes([2, 1]), ext_id: 0u16 }; @@ -151,6 +152,7 @@ mod tests { #[test] fn module_and_index_works() { + // Test ensuring little endian. let env = MockEnvironment { func_id: 0u16, ext_id: u16::from_le_bytes([2, 3]) }; assert_eq!(module_and_index(&env), (2, 3)); let env = MockEnvironment { func_id: 0u16, ext_id: u16::from_le_bytes([3, 2]) }; @@ -159,6 +161,7 @@ mod tests { #[test] fn version_works() { + // Test ensuring little endian. let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 }; assert_eq!(version(&env), 2); let env = MockEnvironment { func_id: u16::from_le_bytes([2, 1]), ext_id: 0u16 }; @@ -167,6 +170,7 @@ mod tests { #[test] fn prepender_works() { + // Test ensuring little endian. let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: u16::from_le_bytes([3, 4]), @@ -184,6 +188,7 @@ mod tests { #[test] fn identified_by_first_byte_of_function_id_matches() { + // Test ensuring little endian. let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 }; assert!(IdentifiedByFirstByteOfFunctionId::>::matches(&env)); let env = MockEnvironment { func_id: u16::from_le_bytes([2, 1]), ext_id: 0u16 }; @@ -192,6 +197,7 @@ mod tests { #[test] fn identified_by_first_byte_of_function_id_does_not_match() { + // Test ensuring little endian. let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 }; assert!(!IdentifiedByFirstByteOfFunctionId::>::matches(&env)); let env = MockEnvironment { func_id: u16::from_le_bytes([2, 1]), ext_id: 0u16 }; @@ -257,6 +263,7 @@ mod tests { (1, CannotLookup, 200), ] { let env = MockEnvironment { func_id: u16::from_le_bytes([0, version]), ext_id: 0u16 }; + // Because `Retval` does not implement the `Debug` trait: let RetVal::Converging(result) = VersionedErrorConverter::::convert(error, &env) .expect("should always result `Ok`") From dad4beb369239f087d3555d86a1138b80613e891 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Tue, 10 Sep 2024 13:38:09 +0200 Subject: [PATCH 04/29] refactor: remove unnecessary --- pallets/api/src/extension.rs | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/pallets/api/src/extension.rs b/pallets/api/src/extension.rs index 39619c2f..b88bd305 100644 --- a/pallets/api/src/extension.rs +++ b/pallets/api/src/extension.rs @@ -143,65 +143,42 @@ mod tests { #[test] fn func_id_works() { - // Test ensuring little endian. let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 }; assert_eq!(func_id(&env), 1); - let env = MockEnvironment { func_id: u16::from_le_bytes([2, 1]), ext_id: 0u16 }; - assert_eq!(func_id(&env), 2); } #[test] fn module_and_index_works() { - // Test ensuring little endian. let env = MockEnvironment { func_id: 0u16, ext_id: u16::from_le_bytes([2, 3]) }; assert_eq!(module_and_index(&env), (2, 3)); - let env = MockEnvironment { func_id: 0u16, ext_id: u16::from_le_bytes([3, 2]) }; - assert_eq!(module_and_index(&env), (3, 2)); } #[test] fn version_works() { - // Test ensuring little endian. let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 }; assert_eq!(version(&env), 2); - let env = MockEnvironment { func_id: u16::from_le_bytes([2, 1]), ext_id: 0u16 }; - assert_eq!(version(&env), 1); } #[test] fn prepender_works() { - // Test ensuring little endian. 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]); - - let env = MockEnvironment { - func_id: u16::from_le_bytes([2, 1]), - ext_id: u16::from_le_bytes([4, 3]), - }; - assert_eq!(Prepender::process(vec![0u8], &env), vec![1, 4, 3, 0]); - assert_eq!(Prepender::process(vec![0u8, 5, 10], &env), vec![1, 4, 3, 0, 5, 10]); } #[test] fn identified_by_first_byte_of_function_id_matches() { - // Test ensuring little endian. let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 }; assert!(IdentifiedByFirstByteOfFunctionId::>::matches(&env)); - let env = MockEnvironment { func_id: u16::from_le_bytes([2, 1]), ext_id: 0u16 }; - assert!(IdentifiedByFirstByteOfFunctionId::>::matches(&env)); } #[test] fn identified_by_first_byte_of_function_id_does_not_match() { - // Test ensuring little endian. let env = MockEnvironment { func_id: u16::from_le_bytes([1, 2]), ext_id: 0u16 }; assert!(!IdentifiedByFirstByteOfFunctionId::>::matches(&env)); - let env = MockEnvironment { func_id: u16::from_le_bytes([2, 1]), ext_id: 0u16 }; - assert!(!IdentifiedByFirstByteOfFunctionId::>::matches(&env)); } #[test] From 1c6f03a1a9ecf35538ce11a9d401010b34d9e6ef Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Tue, 10 Sep 2024 14:52:33 +0200 Subject: [PATCH 05/29] test: primitives --- primitives/src/lib.rs | 57 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 5 deletions(-) diff --git a/primitives/src/lib.rs b/primitives/src/lib.rs index e6942432..34f589eb 100644 --- a/primitives/src/lib.rs +++ b/primitives/src/lib.rs @@ -1,3 +1,5 @@ +//! The `pop-primitives` crate provides types used by other crates. + #![cfg_attr(not(feature = "std"), no_std, no_main)] use codec::{Decode, Encode}; @@ -8,6 +10,7 @@ pub use v0::*; /// The identifier of a token. pub type TokenId = u32; +/// The first version of primitives' types. pub mod v0 { pub use error::*; @@ -16,8 +19,8 @@ pub mod v0 { mod error { use super::*; - /// Reason why a Pop API call failed. - #[derive(Encode, Decode, Debug, Eq, PartialEq)] + /// Reason why a call failed. + #[derive(Encode, Decode, Debug, Eq, PartialEq, Clone)] #[cfg_attr(feature = "std", derive(TypeInfo))] #[repr(u8)] #[allow(clippy::unnecessary_cast)] @@ -102,7 +105,7 @@ pub mod v0 { } /// Description of what went wrong when trying to complete an operation on a token. - #[derive(Encode, Decode, Debug, Eq, PartialEq)] + #[derive(Encode, Decode, Debug, Eq, PartialEq, Clone)] #[cfg_attr(feature = "std", derive(TypeInfo))] pub enum TokenError { /// Funds are unavailable. @@ -129,7 +132,7 @@ pub mod v0 { } /// Arithmetic errors. - #[derive(Encode, Decode, Debug, Eq, PartialEq)] + #[derive(Encode, Decode, Debug, Eq, PartialEq, Clone)] #[cfg_attr(feature = "std", derive(TypeInfo))] pub enum ArithmeticError { /// Underflow. @@ -141,7 +144,7 @@ pub mod v0 { } /// Errors related to transactional storage layers. - #[derive(Encode, Decode, Debug, Eq, PartialEq)] + #[derive(Encode, Decode, Debug, Eq, PartialEq, Clone)] #[cfg_attr(feature = "std", derive(TypeInfo))] pub enum TransactionalError { /// Too many transactional layers have been spawned. @@ -150,4 +153,48 @@ pub mod v0 { NoLayer, } } + + #[cfg(test)] + mod tests { + use super::*; + + #[test] + fn test_error_u32_conversion_with_all_variants() { + let error_variants = vec![ + Error::Other, + Error::CannotLookup, + Error::BadOrigin, + Error::ConsumerRemaining, + Error::NoProviders, + Error::TooManyConsumers, + Error::Token(TokenError::FundsUnavailable), + Error::Arithmetic(ArithmeticError::Underflow), + Error::Transactional(TransactionalError::LimitReached), + Error::Exhausted, + Error::Corruption, + Error::Unavailable, + Error::RootNotAllowed, + Error::DecodingFailed, + Error::Unknown { dispatch_error_index: 1, error_index: 2, error: 3 }, + ]; + + // Test conversion for all Error variants + for error in error_variants { + let original_u32: u32 = error.clone().into(); + let decoded_error: Error = original_u32.into(); + assert_eq!(error, decoded_error); + } + } + + #[test] + fn test_invalid_u32_values_result_in_decoding_failed() { + // These are u32 values that don't map to any valid Error. + let invalid_u32_values = vec![111u32, 999u32, 1234u32]; + + for invalid_value in invalid_u32_values { + let error: Error = invalid_value.into(); + assert_eq!(error, Error::DecodingFailed,); + } + } + } } From a51d619c7b2cae408b9b9d1379cf0f395d5bb18b Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Tue, 10 Sep 2024 14:57:30 +0200 Subject: [PATCH 06/29] style: naming variable --- primitives/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/src/lib.rs b/primitives/src/lib.rs index 34f589eb..9790b946 100644 --- a/primitives/src/lib.rs +++ b/primitives/src/lib.rs @@ -180,8 +180,8 @@ pub mod v0 { // Test conversion for all Error variants for error in error_variants { - let original_u32: u32 = error.clone().into(); - let decoded_error: Error = original_u32.into(); + let u32_value: u32 = error.clone().into(); + let decoded_error: Error = u32_value.into(); assert_eq!(error, decoded_error); } } From 9ff99158ea3458069ac419f738841e280b3f3329 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Tue, 10 Sep 2024 16:21:39 +0200 Subject: [PATCH 07/29] test: devnet runtime versioning --- pallets/api/src/fungibles/mod.rs | 4 +- runtime/devnet/src/config/api/mod.rs | 4 +- runtime/devnet/src/config/api/versioning.rs | 52 +++++++++++++++++++-- 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 311b717e..31ab8328 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -44,7 +44,7 @@ pub mod pallet { use super::*; /// State reads for the fungibles API with required input. - #[derive(Encode, Decode, Debug, MaxEncodedLen)] + #[derive(Encode, Decode, Debug, MaxEncodedLen, PartialEq, Clone)] #[repr(u8)] #[allow(clippy::unnecessary_cast)] pub enum Read { @@ -84,7 +84,7 @@ pub mod pallet { } /// Results of state reads for the fungibles API. - #[derive(Debug)] + #[derive(Debug, PartialEq, Clone)] pub enum ReadResult { /// Total token supply for a specified token. TotalSupply(BalanceOf), diff --git a/runtime/devnet/src/config/api/mod.rs b/runtime/devnet/src/config/api/mod.rs index 6d27fcde..340feefb 100644 --- a/runtime/devnet/src/config/api/mod.rs +++ b/runtime/devnet/src/config/api/mod.rs @@ -25,7 +25,7 @@ type DecodesAs = pallet_api::extension::DecodesAs< >; /// A query of runtime state. -#[derive(Decode, Debug)] +#[derive(Decode, Debug, PartialEq, Clone)] #[repr(u8)] pub enum RuntimeRead { /// Fungible token queries. @@ -54,7 +54,7 @@ impl Readable for RuntimeRead { } /// The result of a runtime state read. -#[derive(Debug)] +#[derive(Debug, PartialEq, Clone)] pub enum RuntimeResult { /// Fungible token read results. Fungibles(fungibles::ReadResult), diff --git a/runtime/devnet/src/config/api/versioning.rs b/runtime/devnet/src/config/api/versioning.rs index 317496df..5dd9c986 100644 --- a/runtime/devnet/src/config/api/versioning.rs +++ b/runtime/devnet/src/config/api/versioning.rs @@ -39,7 +39,7 @@ impl From for RuntimeRead { } /// Versioned runtime state read results. -#[derive(Debug)] +#[derive(Debug, PartialEq, Clone)] pub enum VersionedRuntimeResult { /// Version zero of runtime read results. V0(RuntimeResult), @@ -67,7 +67,7 @@ impl From for Vec { } /// Versioned errors. -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub enum VersionedError { /// Version zero of errors. V0(pop_primitives::v0::Error), @@ -93,6 +93,7 @@ impl From for u32 { } } +// Type for `pop_primitives::Error` to avoid taking a dependency of sp_runtime on pop-primitives. struct V0Error(pop_primitives::v0::Error); impl From for V0Error { fn from(error: DispatchError) -> Self { @@ -157,15 +158,60 @@ impl From for V0Error { #[cfg(test)] mod tests { + use frame_system::Call; use pop_primitives::{ArithmeticError::*, Error, TokenError::*, TransactionalError::*}; use sp_runtime::ModuleError; use DispatchError::*; use super::*; + #[test] + fn from_versioned_runtime_call_to_runtime_call_works() { + let call = + RuntimeCall::System(Call::remark_with_event { remark: "pop".as_bytes().to_vec() }); + assert_eq!(RuntimeCall::from(VersionedRuntimeCall::V0(call.clone())), call); + } + + #[test] + fn from_versioned_runtime_read_to_runtime_read_works() { + let read = RuntimeRead::Fungibles(fungibles::Read::::TotalSupply(42)); + assert_eq!(RuntimeRead::from(VersionedRuntimeRead::V0(read.clone())), read); + } + + #[test] + fn from_versioned_runtime_result_works() { + let result = RuntimeResult::Fungibles(fungibles::ReadResult::::TotalSupply(1_000)); + let version = 0; + assert_eq!( + VersionedRuntimeResult::from((result.clone(), version)), + VersionedRuntimeResult::V0(result) + ); + } + + #[test] + fn from_versioned_runtime_result_to_bytes_works() { + let result = RuntimeResult::Fungibles(fungibles::ReadResult::::TotalSupply(1_000)); + assert_eq!(>::from(VersionedRuntimeResult::V0(result.clone())), result.encode()); + } + + #[test] + fn from_versioned_error_works() { + let error = BadOrigin; + let version = 0; + assert_eq!( + VersionedError::from((error, version)), + VersionedError::V0(V0Error::from(error).0) + ); + } + + #[test] + fn from_versioned_error_to_u32_works() { + assert_eq!(u32::from(VersionedError::V0(Error::BadOrigin)), 2); + } + // Compare all the different `DispatchError` variants with the expected `Error`. #[test] - fn dispatch_error_to_error() { + fn from_dispatch_error_to_error_works() { let test_cases = vec![ (Other(""), (Error::Other)), (Other("UnknownCall"), Error::Other), From a2b9e762337460363f95c9be2802b4d9732c5c3c Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Tue, 10 Sep 2024 16:59:16 +0200 Subject: [PATCH 08/29] test: runtime devnet mod (small tests) --- runtime/devnet/src/config/api/mod.rs | 117 +++++++++++++++++---------- 1 file changed, 73 insertions(+), 44 deletions(-) diff --git a/runtime/devnet/src/config/api/mod.rs b/runtime/devnet/src/config/api/mod.rs index 340feefb..569446a3 100644 --- a/runtime/devnet/src/config/api/mod.rs +++ b/runtime/devnet/src/config/api/mod.rs @@ -163,53 +163,82 @@ impl Contains for Filter { } } -#[test] -fn filter_prevents_runtime_filtered_calls() { - use pallet_balances::{AdjustmentDirection, Call::*}; - use sp_runtime::MultiAddress; - use RuntimeCall::Balances; - - const CALLS: [RuntimeCall; 4] = [ - Balances(force_adjust_total_issuance { - direction: AdjustmentDirection::Increase, - delta: 0, - }), - Balances(force_set_balance { who: MultiAddress::Address32([0u8; 32]), new_free: 0 }), - Balances(force_transfer { - source: MultiAddress::Address32([0u8; 32]), - dest: MultiAddress::Address32([0u8; 32]), - value: 0, - }), - Balances(force_unreserve { who: MultiAddress::Address32([0u8; 32]), amount: 0 }), - ]; - - for call in CALLS { - assert!(!Filter::::contains(&call)) - } -} - -#[test] -fn filter_allows_fungibles_calls() { +#[cfg(test)] +mod tests { use pallet_api::fungibles::Call::*; use sp_core::crypto::AccountId32; - use RuntimeCall::Fungibles; + use RuntimeCall::{Balances, Fungibles}; + + use super::*; const ACCOUNT: AccountId32 = AccountId32::new([0u8; 32]); - const CALLS: [RuntimeCall; 11] = [ - Fungibles(transfer { token: 0, to: ACCOUNT, value: 0 }), - Fungibles(transfer_from { token: 0, from: ACCOUNT, to: ACCOUNT, value: 0 }), - Fungibles(approve { token: 0, spender: ACCOUNT, value: 0 }), - Fungibles(increase_allowance { token: 0, spender: ACCOUNT, value: 0 }), - Fungibles(decrease_allowance { token: 0, spender: ACCOUNT, value: 0 }), - Fungibles(create { id: 0, admin: ACCOUNT, min_balance: 0 }), - Fungibles(set_metadata { token: 0, name: vec![], symbol: vec![], decimals: 0 }), - Fungibles(start_destroy { token: 0 }), - Fungibles(clear_metadata { token: 0 }), - Fungibles(mint { token: 0, account: ACCOUNT, value: 0 }), - Fungibles(burn { token: 0, account: ACCOUNT, value: 0 }), - ]; - - for call in CALLS { - assert!(Filter::::contains(&call)) + + #[test] + fn runtime_result_encode_works() { + let result = fungibles::ReadResult::::TotalSupply(1_000); + assert_eq!(RuntimeResult::Fungibles(result.clone()).encode(), result.encode()); + } + + #[test] + fn filter_prevents_runtime_filtered_calls() { + use pallet_balances::{AdjustmentDirection, Call::*}; + use sp_runtime::MultiAddress; + + const CALLS: [RuntimeCall; 4] = [ + Balances(force_adjust_total_issuance { + direction: AdjustmentDirection::Increase, + delta: 0, + }), + Balances(force_set_balance { who: MultiAddress::Address32([0u8; 32]), new_free: 0 }), + Balances(force_transfer { + source: MultiAddress::Address32([0u8; 32]), + dest: MultiAddress::Address32([0u8; 32]), + value: 0, + }), + Balances(force_unreserve { who: MultiAddress::Address32([0u8; 32]), amount: 0 }), + ]; + + for call in CALLS { + assert!(!Filter::::contains(&call)) + } + } + + #[test] + fn filter_allows_fungibles_calls() { + const CALLS: [RuntimeCall; 11] = [ + Fungibles(transfer { token: 0, to: ACCOUNT, value: 0 }), + Fungibles(transfer_from { token: 0, from: ACCOUNT, to: ACCOUNT, value: 0 }), + Fungibles(approve { token: 0, spender: ACCOUNT, value: 0 }), + Fungibles(increase_allowance { token: 0, spender: ACCOUNT, value: 0 }), + Fungibles(decrease_allowance { token: 0, spender: ACCOUNT, value: 0 }), + Fungibles(create { id: 0, admin: ACCOUNT, min_balance: 0 }), + Fungibles(set_metadata { token: 0, name: vec![], symbol: vec![], decimals: 0 }), + Fungibles(start_destroy { token: 0 }), + Fungibles(clear_metadata { token: 0 }), + Fungibles(mint { token: 0, account: ACCOUNT, value: 0 }), + Fungibles(burn { token: 0, account: ACCOUNT, value: 0 }), + ]; + + for call in CALLS { + assert!(Filter::::contains(&call)) + } + } + + #[test] + fn filter_allows_fungible_reads() { + use super::{fungibles::Read::*, RuntimeRead::*}; + const READS: [RuntimeRead; 7] = [ + Fungibles(TotalSupply(1)), + Fungibles(BalanceOf { token: 1, owner: ACCOUNT }), + Fungibles(Allowance { token: 1, owner: ACCOUNT, spender: ACCOUNT }), + Fungibles(TokenName(1)), + Fungibles(TokenSymbol(1)), + Fungibles(TokenDecimals(10)), + Fungibles(TokenExists(1)), + ]; + + for read in READS { + assert!(Filter::::contains(&read)) + } } } From dc0f90a3e5ae577e756b64310e370e137868b26e Mon Sep 17 00:00:00 2001 From: Frank Bell Date: Tue, 10 Sep 2024 21:57:26 +0100 Subject: [PATCH 09/29] refactor: fallible conversion --- extension/src/functions.rs | 30 +++++++++++++++------ extension/src/mock.rs | 10 ++++--- pallets/api/src/extension.rs | 16 ++++++----- runtime/devnet/src/config/api/versioning.rs | 22 ++++++++------- 4 files changed, 50 insertions(+), 28 deletions(-) diff --git a/extension/src/functions.rs b/extension/src/functions.rs index b1a046ea..5297f256 100644 --- a/extension/src/functions.rs +++ b/extension/src/functions.rs @@ -88,7 +88,7 @@ impl< Read: Readable + Debug, Decoder: Decode>, Filter: Contains, - ResultConverter: Converter>>, + ResultConverter: Converter>, Error = DispatchError>, Error: ErrorConverter, Logger: LogTarget, > Function for ReadState @@ -116,7 +116,7 @@ impl< 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::::seal_input(result.len() as u32); @@ -146,12 +146,16 @@ pub trait Readable { 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 @@ pub trait 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, + ) -> core::result::Result; } /// A default converter, for converting (encoding) from some type into a byte array. pub struct DefaultConverter(PhantomData); impl>> Converter for DefaultConverter { + /// 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; 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 { + Ok(value.into()) } } @@ -462,7 +476,7 @@ mod tests { 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 @@ mod tests { 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()); } } diff --git a/extension/src/mock.rs b/extension/src/mock.rs index 13ffbf47..c085aebe 100644 --- a/extension/src/mock.rs +++ b/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; 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 { match value { - RuntimeResult::Pong(value) => value.to_uppercase().encode(), + RuntimeResult::Pong(value) => Ok(value.to_uppercase().encode()), } } } diff --git a/pallets/api/src/extension.rs b/pallets/api/src/extension.rs index 138cdee6..2dd93372 100644 --- a/pallets/api/src/extension.rs +++ b/pallets/api/src/extension.rs @@ -70,8 +70,8 @@ impl LogTarget for ReadStateLogTarget { /// Conversion of a `DispatchError` to a versioned error. pub struct VersionedErrorConverter(PhantomData); -impl + Into + Debug> pop_chain_extension::ErrorConverter - for VersionedErrorConverter +impl + Into + Debug> + pop_chain_extension::ErrorConverter for VersionedErrorConverter { /// The log target. const LOG_TARGET: &'static str = "pop-api::extension::converters::versioned-error"; @@ -85,7 +85,7 @@ impl + Into + 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())) } @@ -93,9 +93,11 @@ impl + Into + Debug> pop_chain_extension:: /// Conversion of a read result to a versioned read result. pub struct VersionedResultConverter(PhantomData<(S, T)>); -impl + Debug> Converter +impl + Debug> Converter for VersionedResultConverter { + /// 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,13 +111,13 @@ impl + 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 { // 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) } } diff --git a/runtime/devnet/src/config/api/versioning.rs b/runtime/devnet/src/config/api/versioning.rs index 317496df..2f3d240f 100644 --- a/runtime/devnet/src/config/api/versioning.rs +++ b/runtime/devnet/src/config/api/versioning.rs @@ -45,14 +45,15 @@ pub enum VersionedRuntimeResult { V0(RuntimeResult), } -impl From<(RuntimeResult, Version)> for VersionedRuntimeResult { - fn from(value: (RuntimeResult, Version)) -> Self { +impl TryFrom<(RuntimeResult, Version)> for VersionedRuntimeResult { + type Error = DispatchError; + + fn try_from(value: (RuntimeResult, Version)) -> Result { let (result, version) = value; match version { // Allows mapping from current `RuntimeResult` to a specific/prior version - 0 => VersionedRuntimeResult::V0(result), - // TODO: should never occur due to version processing/validation when request received - _ => unimplemented!(), + 0 => Ok(VersionedRuntimeResult::V0(result)), + _ => Err(pallet_contracts::Error::::DecodingFailed.into()), } } } @@ -73,14 +74,15 @@ pub enum VersionedError { V0(pop_primitives::v0::Error), } -impl From<(DispatchError, Version)> for VersionedError { - fn from(value: (DispatchError, Version)) -> Self { +impl TryFrom<(DispatchError, Version)> for VersionedError { + type Error = DispatchError; + + fn try_from(value: (DispatchError, Version)) -> Result { let (error, version) = value; match version { // Allows mapping from current `DispatchError` to a specific/prior version of `Error` - 0 => VersionedError::V0(V0Error::from(error).0), - // TODO: should never occur due to version processing/validation when request received - _ => unimplemented!(), + 0 => Ok(VersionedError::V0(V0Error::from(error).0)), + _ => Err(pallet_contracts::Error::::DecodingFailed.into()), } } } From 6f8205e29c24eae729ae12d29438dde2cc4203bb Mon Sep 17 00:00:00 2001 From: Frank Bell <60948618+evilrobot-01@users.noreply.github.com> Date: Wed, 11 Sep 2024 08:49:24 +0100 Subject: [PATCH 10/29] refactor: fallible conversion (#275) Co-authored-by: Daanvdplas --- extension/src/functions.rs | 30 +++++++++++++++------ extension/src/mock.rs | 10 ++++--- pallets/api/src/extension.rs | 14 +++++----- runtime/devnet/src/config/api/versioning.rs | 22 ++++++++------- 4 files changed, 49 insertions(+), 27 deletions(-) diff --git a/extension/src/functions.rs b/extension/src/functions.rs index b1a046ea..5297f256 100644 --- a/extension/src/functions.rs +++ b/extension/src/functions.rs @@ -88,7 +88,7 @@ impl< Read: Readable + Debug, Decoder: Decode>, Filter: Contains, - ResultConverter: Converter>>, + ResultConverter: Converter>, Error = DispatchError>, Error: ErrorConverter, Logger: LogTarget, > Function for ReadState @@ -116,7 +116,7 @@ impl< 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::::seal_input(result.len() as u32); @@ -146,12 +146,16 @@ pub trait Readable { 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 @@ pub trait 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, + ) -> core::result::Result; } /// A default converter, for converting (encoding) from some type into a byte array. pub struct DefaultConverter(PhantomData); impl>> Converter for DefaultConverter { + /// 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; 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 { + Ok(value.into()) } } @@ -462,7 +476,7 @@ mod tests { 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 @@ mod tests { 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()); } } diff --git a/extension/src/mock.rs b/extension/src/mock.rs index 13ffbf47..c085aebe 100644 --- a/extension/src/mock.rs +++ b/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; 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 { match value { - RuntimeResult::Pong(value) => value.to_uppercase().encode(), + RuntimeResult::Pong(value) => Ok(value.to_uppercase().encode()), } } } diff --git a/pallets/api/src/extension.rs b/pallets/api/src/extension.rs index b88bd305..77aac89f 100644 --- a/pallets/api/src/extension.rs +++ b/pallets/api/src/extension.rs @@ -70,7 +70,7 @@ impl LogTarget for ReadStateLogTarget { /// Conversion of a `DispatchError` to a versioned error. pub struct VersionedErrorConverter(PhantomData); -impl + Into + Debug> ErrorConverter +impl + Into + Debug> ErrorConverter for VersionedErrorConverter { /// The log target. @@ -85,7 +85,7 @@ impl + Into + Debug> ErrorConverter // 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())) } @@ -93,9 +93,11 @@ impl + Into + Debug> ErrorConverter /// Conversion of a read result to a versioned read result. pub struct VersionedResultConverter(PhantomData<(S, T)>); -impl + Debug> Converter +impl + Debug> Converter for VersionedResultConverter { + /// 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,13 +111,13 @@ impl + 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 { // 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) } } diff --git a/runtime/devnet/src/config/api/versioning.rs b/runtime/devnet/src/config/api/versioning.rs index 5dd9c986..b15bf2e6 100644 --- a/runtime/devnet/src/config/api/versioning.rs +++ b/runtime/devnet/src/config/api/versioning.rs @@ -45,14 +45,15 @@ pub enum VersionedRuntimeResult { V0(RuntimeResult), } -impl From<(RuntimeResult, Version)> for VersionedRuntimeResult { - fn from(value: (RuntimeResult, Version)) -> Self { +impl TryFrom<(RuntimeResult, Version)> for VersionedRuntimeResult { + type Error = DispatchError; + + fn try_from(value: (RuntimeResult, Version)) -> Result { let (result, version) = value; match version { // Allows mapping from current `RuntimeResult` to a specific/prior version - 0 => VersionedRuntimeResult::V0(result), - // TODO: should never occur due to version processing/validation when request received - _ => unimplemented!(), + 0 => Ok(VersionedRuntimeResult::V0(result)), + _ => Err(pallet_contracts::Error::::DecodingFailed.into()), } } } @@ -73,14 +74,15 @@ pub enum VersionedError { V0(pop_primitives::v0::Error), } -impl From<(DispatchError, Version)> for VersionedError { - fn from(value: (DispatchError, Version)) -> Self { +impl TryFrom<(DispatchError, Version)> for VersionedError { + type Error = DispatchError; + + fn try_from(value: (DispatchError, Version)) -> Result { let (error, version) = value; match version { // Allows mapping from current `DispatchError` to a specific/prior version of `Error` - 0 => VersionedError::V0(V0Error::from(error).0), - // TODO: should never occur due to version processing/validation when request received - _ => unimplemented!(), + 0 => Ok(VersionedError::V0(V0Error::from(error).0)), + _ => Err(pallet_contracts::Error::::DecodingFailed.into()), } } } From 1547dca84f8e3d7a4553d6b22de9842b9c84f2ce Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 11 Sep 2024 10:01:07 +0200 Subject: [PATCH 11/29] fix: tests after fallible conversion feat --- runtime/devnet/src/config/api/versioning.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/runtime/devnet/src/config/api/versioning.rs b/runtime/devnet/src/config/api/versioning.rs index b15bf2e6..0e4cafcb 100644 --- a/runtime/devnet/src/config/api/versioning.rs +++ b/runtime/devnet/src/config/api/versioning.rs @@ -183,10 +183,14 @@ mod tests { #[test] fn from_versioned_runtime_result_works() { let result = RuntimeResult::Fungibles(fungibles::ReadResult::::TotalSupply(1_000)); - let version = 0; assert_eq!( - VersionedRuntimeResult::from((result.clone(), version)), - VersionedRuntimeResult::V0(result) + VersionedRuntimeResult::try_from((result.clone(), 0)), + Ok(VersionedRuntimeResult::V0(result.clone())) + ); + // Unknown version. + assert_eq!( + VersionedRuntimeResult::try_from((result.clone(), 1)), + Err(pallet_contracts::Error::::DecodingFailed.into()) ); } @@ -199,10 +203,14 @@ mod tests { #[test] fn from_versioned_error_works() { let error = BadOrigin; - let version = 0; assert_eq!( - VersionedError::from((error, version)), - VersionedError::V0(V0Error::from(error).0) + VersionedError::try_from((error, 0)), + Ok(VersionedError::V0(V0Error::from(error).0)) + ); + // Unknown version. + assert_eq!( + VersionedError::try_from((error, 1)), + Err(pallet_contracts::Error::::DecodingFailed.into()) ); } From 36e8735b43304a50482c2e1227d280d5987e3ea5 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 11 Sep 2024 11:05:39 +0200 Subject: [PATCH 12/29] fix: test after fallible conversion feat --- pallets/api/src/extension.rs | 63 +++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 23 deletions(-) diff --git a/pallets/api/src/extension.rs b/pallets/api/src/extension.rs index d6f70ea6..ff0dde6b 100644 --- a/pallets/api/src/extension.rs +++ b/pallets/api/src/extension.rs @@ -70,8 +70,8 @@ impl LogTarget for ReadStateLogTarget { /// Conversion of a `DispatchError` to a versioned error. pub struct VersionedErrorConverter(PhantomData); -impl + Into + Debug> - ErrorConverter for VersionedErrorConverter +impl + Into + Debug> ErrorConverter + for VersionedErrorConverter { /// The log target. const LOG_TARGET: &'static str = "pop-api::extension::converters::versioned-error"; @@ -201,6 +201,8 @@ mod tests { #[test] fn versioned_error_converter_works() { + use super::RetVal::Converging; + // Mock versioned error. #[derive(Debug)] pub enum VersionedError { @@ -208,13 +210,15 @@ mod tests { V1(DispatchError), } - impl From<(DispatchError, u8)> for VersionedError { - fn from(value: (DispatchError, u8)) -> Self { + impl TryFrom<(DispatchError, u8)> for VersionedError { + type Error = DispatchError; + + fn try_from(value: (DispatchError, u8)) -> Result { let (error, version) = value; match version { - 0 => VersionedError::V0(error), - 1 => VersionedError::V1(error), - _ => unimplemented!(), + 0 => Ok(VersionedError::V0(error)), + 1 => Ok(VersionedError::V1(error)), + _ => Err(Other("DecodingFailed")), } } } @@ -235,6 +239,8 @@ mod tests { } } + // 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), @@ -242,19 +248,27 @@ mod tests { (1, CannotLookup, 200), ] { let env = MockEnvironment { func_id: u16::from_le_bytes([0, version]), ext_id: 0u16 }; - // Because `Retval` does not implement the `Debug` trait: - let RetVal::Converging(result) = + // Again, due to missing `Debug` trait the result has to be unwrapped. + let Ok(Converging(result)) = VersionedErrorConverter::::convert(error, &env) - .expect("should always result `Ok`") else { - unimplemented!(); + 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::::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 { @@ -262,29 +276,32 @@ mod tests { V1(u8), } - impl From<(u8, u8)> for VersionedRuntimeResult { + impl TryFrom<(u8, u8)> for VersionedRuntimeResult { + type Error = DispatchError; + // Mock conversion based on result and version. - fn from(value: (u8, u8)) -> Self { + fn try_from(value: (u8, u8)) -> Result { let (result, version) = value; match version { - 0 if result <= 50 => VersionedRuntimeResult::V0(result), - 0 if result > 50 => VersionedRuntimeResult::V0(50), - 1 if result <= 100 => VersionedRuntimeResult::V1(result), - 1 if result > 100 => VersionedRuntimeResult::V1(100), - _ => unimplemented!(), + 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, VersionedRuntimeResult::V0(10)), - (0, 100, VersionedRuntimeResult::V0(50)), - (1, 10, VersionedRuntimeResult::V1(10)), - (1, 100, VersionedRuntimeResult::V1(100)), + (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::::convert(value, &env); + VersionedResultConverter::::try_convert(value, &env); assert_eq!(result, expected_result); } } From a1ee377c5115a99dac8878d75c3f6888c21815fb Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 11 Sep 2024 11:32:50 +0200 Subject: [PATCH 13/29] test: config contracts --- runtime/devnet/src/config/contracts.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/runtime/devnet/src/config/contracts.rs b/runtime/devnet/src/config/contracts.rs index 6cf923b9..7d5113ed 100644 --- a/runtime/devnet/src/config/contracts.rs +++ b/runtime/devnet/src/config/contracts.rs @@ -1,6 +1,6 @@ use frame_support::{ parameter_types, - traits::{ConstBool, ConstU32, Randomness}, + traits::{ConstBool, ConstU32, Contains, Randomness}, }; use frame_system::{pallet_prelude::BlockNumberFor, EnsureSigned}; @@ -12,7 +12,7 @@ use crate::{ pub enum AllowBalancesCall {} -impl frame_support::traits::Contains for AllowBalancesCall { +impl Contains for AllowBalancesCall { fn contains(call: &RuntimeCall) -> bool { matches!(call, RuntimeCall::Balances(BalancesCall::transfer_allow_death { .. })) } @@ -91,3 +91,13 @@ impl pallet_contracts::Config for Runtime { type WeightPrice = pallet_transaction_payment::Pallet; type Xcm = pallet_xcm::Pallet; } + +#[test] +fn filter_allows_transfer_allow_death_call() { + assert!(AllowBalancesCall::contains(&RuntimeCall::Balances( + BalancesCall::transfer_allow_death { + dest: sp_runtime::MultiAddress::Address32([0u8; 32]), + value: 0 + } + ))); +} From 8949e134dd1570e48bf76dd2cbfebfb499013e50 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 11 Sep 2024 13:57:29 +0200 Subject: [PATCH 14/29] test: ensure signed test coverage --- pallets/api/src/fungibles/mod.rs | 12 ++++-------- pallets/api/src/fungibles/tests.rs | 30 ++++++++++++++++++++++++++++-- pallets/api/src/mock.rs | 15 ++++++++------- 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 31ab8328..bdfe1f50 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -14,16 +14,12 @@ mod tests; pub mod weights; type AccountIdOf = ::AccountId; -type TokenIdOf = > as Inspect< - ::AccountId, ->>::AssetId; +type TokenIdOf = as Inspect<::AccountId>>::AssetId; type TokenIdParameterOf = >>::AssetIdParameter; type AssetsOf = pallet_assets::Pallet>; type AssetsInstanceOf = ::AssetsInstance; type AssetsWeightInfoOf = >>::WeightInfo; -type BalanceOf = > as Inspect< - ::AccountId, ->>::Balance; +type BalanceOf = as Inspect<::AccountId>>::Balance; #[frame_support::pallet] pub mod pallet { @@ -78,7 +74,7 @@ pub mod pallet { /// Decimals for the specified token. #[codec(index = 10)] TokenDecimals(TokenIdOf), - /// Check if a specified token exists. + /// Whether a specified token exists. #[codec(index = 18)] TokenExists(TokenIdOf), } @@ -123,7 +119,7 @@ pub mod pallet { pub trait Config: frame_system::Config + pallet_assets::Config { /// Because this pallet emits events, it depends on the runtime's definition of an event. type RuntimeEvent: From> + IsType<::RuntimeEvent>; - /// The instance of pallet assets it is tightly coupled to. + /// The instance of pallet-assets. type AssetsInstance; /// Weight information for dispatchables in this pallet. type WeightInfo: WeightInfo; diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index c57c753c..cbf18ea2 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -1,7 +1,7 @@ use codec::Encode; use frame_support::{ - assert_ok, - sp_runtime::traits::Zero, + assert_noop, assert_ok, + sp_runtime::{traits::Zero, DispatchError::BadOrigin}, traits::fungibles::{ approvals::Inspect as ApprovalInspect, metadata::Inspect as MetadataInspect, Inspect, }, @@ -22,6 +22,9 @@ fn transfer_works() { let to = Some(BOB); create_token_and_mint_to(ALICE, token, ALICE, value * 2); + for origin in vec![root(), none()] { + assert_noop!(Fungibles::transfer(origin, token, BOB, value), BadOrigin); + } let balance_before_transfer = Assets::balance(token, &BOB); assert_ok!(Fungibles::transfer(signed(ALICE), token, BOB, value)); let balance_after_transfer = Assets::balance(token, &BOB); @@ -40,6 +43,10 @@ fn transfer_from_works() { // Approve CHARLIE to transfer up to `value` to BOB. create_token_mint_and_approve(ALICE, token, ALICE, value * 2, CHARLIE, value); + // TODO: weight + for origin in vec![root(), none()] { + assert_noop!(Fungibles::transfer_from(origin, token, ALICE, BOB, value), BadOrigin); + } // Successfully call transfer from. let alice_balance_before_transfer = Assets::balance(token, &ALICE); let bob_balance_before_transfer = Assets::balance(token, &BOB); @@ -101,6 +108,10 @@ fn increase_allowance_works() { let spender = BOB; create_token_and_mint_to(ALICE, token, ALICE, value); + // TODO: weight + for origin in vec![root(), none()] { + assert_noop!(Fungibles::increase_allowance(origin, token, BOB, value), BadOrigin); + } assert_eq!(0, Assets::allowance(token, &ALICE, &BOB)); assert_ok!(Fungibles::increase_allowance(signed(ALICE), token, BOB, value)); assert_eq!(Assets::allowance(token, &ALICE, &BOB), value); @@ -123,6 +134,10 @@ fn decrease_allowance_works() { let spender = BOB; create_token_mint_and_approve(ALICE, token, ALICE, value, BOB, value); + // TODO: weight + for origin in vec![root(), none()] { + assert_noop!(Fungibles::decrease_allowance(origin, token, BOB, 0), BadOrigin); + } assert_eq!(Assets::allowance(token, &ALICE, &BOB), value); // Owner balance is not changed if decreased by zero. assert_ok!(Fungibles::decrease_allowance(signed(ALICE), token, BOB, 0)); @@ -147,6 +162,9 @@ fn create_works() { let creator = ALICE; let admin = ALICE; + for origin in vec![root(), none()] { + assert_noop!(Fungibles::create(origin, id, admin, 100), BadOrigin); + } assert!(!Assets::asset_exists(id)); assert_ok!(Fungibles::create(signed(creator), id, admin, 100)); assert!(Assets::asset_exists(id)); @@ -297,6 +315,14 @@ fn signed(account: AccountId) -> RuntimeOrigin { RuntimeOrigin::signed(account) } +fn root() -> RuntimeOrigin { + RuntimeOrigin::root() +} + +fn none() -> RuntimeOrigin { + RuntimeOrigin::none() +} + fn create_token(owner: AccountId, token: TokenId) { assert_ok!(Assets::create(signed(owner), token, owner, 1)); } diff --git a/pallets/api/src/mock.rs b/pallets/api/src/mock.rs index b736656f..42c8bf0e 100644 --- a/pallets/api/src/mock.rs +++ b/pallets/api/src/mock.rs @@ -9,6 +9,12 @@ use sp_runtime::{ BuildStorage, }; +pub(crate) const ALICE: AccountId = 1; +pub(crate) const BOB: AccountId = 2; +pub(crate) const CHARLIE: AccountId = 3; +pub(crate) const INIT_AMOUNT: Balance = 100_000_000 * UNIT; +pub(crate) const UNIT: Balance = 10_000_000_000; + type Block = frame_system::mocking::MockBlock; pub(crate) type AccountId = u64; pub(crate) type Balance = u128; @@ -80,7 +86,7 @@ impl pallet_assets::Config for Test { type AssetAccountDeposit = ConstU128<10>; type AssetDeposit = ConstU128<1>; type AssetId = TokenId; - type AssetIdParameter = u32; + type AssetIdParameter = TokenId; type Balance = Balance; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); @@ -97,18 +103,13 @@ impl pallet_assets::Config for Test { type StringLimit = ConstU32<50>; type WeightInfo = (); } + impl crate::fungibles::Config for Test { type AssetsInstance = AssetsInstance; type RuntimeEvent = RuntimeEvent; type WeightInfo = (); } -pub(crate) const ALICE: AccountId = 1; -pub(crate) const BOB: AccountId = 2; -pub(crate) const CHARLIE: AccountId = 3; -pub(crate) const INIT_AMOUNT: Balance = 100_000_000 * UNIT; -pub(crate) const UNIT: Balance = 10_000_000_000; - pub(crate) fn new_test_ext() -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::::default() .build_storage() From d2eeb082beec8577b381ce4a2441a1793ca80f99 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 11 Sep 2024 15:44:08 +0200 Subject: [PATCH 15/29] test: coverage fungibles pallet --- pallets/api/src/fungibles/mod.rs | 40 +++++----- pallets/api/src/fungibles/tests.rs | 121 ++++++++++++++++++++++------- 2 files changed, 112 insertions(+), 49 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index bdfe1f50..6ad7f762 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -20,6 +20,7 @@ type AssetsOf = pallet_assets::Pallet>; type AssetsInstanceOf = ::AssetsInstance; type AssetsWeightInfoOf = >>::WeightInfo; type BalanceOf = as Inspect<::AccountId>>::Balance; +type WeightInfoOf = ::WeightInfo; #[frame_support::pallet] pub mod pallet { @@ -235,12 +236,12 @@ pub mod pallet { value: BalanceOf, ) -> DispatchResultWithPostInfo { let owner = ensure_signed(origin.clone()) - .map_err(|e| e.with_weight(Self::weight_approve(0, 0)))?; + .map_err(|e| e.with_weight(WeightInfoOf::::approve(0, 0)))?; let current_allowance = AssetsOf::::allowance(token.clone(), &owner, &spender); let weight = match value.cmp(¤t_allowance) { // If the new value is equal to the current allowance, do nothing. - Equal => Self::weight_approve(0, 0), + Equal => WeightInfoOf::::approve(0, 0), // If the new value is greater than the current allowance, approve the difference // because `approve_transfer` works additively (see `pallet-assets`). Greater => { @@ -250,8 +251,8 @@ pub mod pallet { T::Lookup::unlookup(spender.clone()), value.saturating_sub(current_allowance), ) - .map_err(|e| e.with_weight(Self::weight_approve(1, 0)))?; - Self::weight_approve(1, 0) + .map_err(|e| e.with_weight(WeightInfoOf::::approve(1, 0)))?; + WeightInfoOf::::approve(1, 0) }, // If the new value is less than the current allowance, cancel the approval and // set the new value. @@ -263,9 +264,9 @@ pub mod pallet { token_param.clone(), spender_source.clone(), ) - .map_err(|e| e.with_weight(Self::weight_approve(0, 1)))?; + .map_err(|e| e.with_weight(WeightInfoOf::::approve(0, 1)))?; if value.is_zero() { - Self::weight_approve(0, 1) + WeightInfoOf::::approve(0, 1) } else { AssetsOf::::approve_transfer( origin, @@ -273,7 +274,7 @@ pub mod pallet { spender_source, value, )?; - Self::weight_approve(1, 1) + WeightInfoOf::::approve(1, 1) } }, }; @@ -296,7 +297,7 @@ pub mod pallet { value: BalanceOf, ) -> DispatchResultWithPostInfo { let owner = ensure_signed(origin.clone()) - .map_err(|e| e.with_weight(Self::weight_approve(0, 0)))?; + .map_err(|e| e.with_weight(WeightInfoOf::::approve(0, 0)))?; AssetsOf::::approve_transfer( origin, token.clone().into(), @@ -324,24 +325,24 @@ pub mod pallet { value: BalanceOf, ) -> DispatchResultWithPostInfo { let owner = ensure_signed(origin.clone()) - .map_err(|e| e.with_weight(Self::weight_approve(0, 0)))?; + .map_err(|e| e.with_weight(WeightInfoOf::::approve(0, 0)))?; if value.is_zero() { - return Ok(Some(Self::weight_approve(0, 0)).into()); + return Ok(Some(WeightInfoOf::::approve(0, 0)).into()); } let current_allowance = AssetsOf::::allowance(token.clone(), &owner, &spender); let spender_source = T::Lookup::unlookup(spender.clone()); let token_param: TokenIdParameterOf = token.clone().into(); - // Cancel the approval and set the new value if `new_allowance` is more than zero. + // Cancel the approval and approve `new_allowance` if difference is more than zero. AssetsOf::::cancel_approval( origin.clone(), token_param.clone(), spender_source.clone(), ) - .map_err(|e| e.with_weight(Self::weight_approve(0, 1)))?; + .map_err(|e| e.with_weight(WeightInfoOf::::approve(0, 1)))?; let new_allowance = current_allowance.saturating_sub(value); let weight = if new_allowance.is_zero() { - Self::weight_approve(0, 1) + WeightInfoOf::::approve(0, 1) } else { AssetsOf::::approve_transfer( origin, @@ -349,7 +350,7 @@ pub mod pallet { spender_source, new_allowance, )?; - Self::weight_approve(1, 1) + WeightInfoOf::::approve(1, 1) }; Self::deposit_event(Event::Approval { token, owner, spender, value: new_allowance }); Ok(Some(weight).into()) @@ -382,6 +383,11 @@ pub mod pallet { /// Start the process of destroying a token. /// + /// See `pallet-assets` documentation for more information. Related dispatchables are: + /// - `destroy_accounts` + /// - `destroy_approvals` + /// - `finish_destroy` + /// /// # Parameters /// - `token` - The token to be destroyed. #[pallet::call_index(12)] @@ -469,12 +475,6 @@ pub mod pallet { } } - impl Pallet { - fn weight_approve(approve: u32, cancel: u32) -> Weight { - ::WeightInfo::approve(cancel, approve) - } - } - impl crate::Read for Pallet { /// The type of read requested. type Read = Read; diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index cbf18ea2..651a7c9a 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -1,17 +1,27 @@ use codec::Encode; use frame_support::{ assert_noop, assert_ok, + dispatch::WithPostDispatchInfo, sp_runtime::{traits::Zero, DispatchError::BadOrigin}, traits::fungibles::{ approvals::Inspect as ApprovalInspect, metadata::Inspect as MetadataInspect, Inspect, }, }; -use crate::{fungibles::Read::*, mock::*, Read}; +use crate::{ + fungibles::{ + weights::WeightInfo as WeightInfoTrait, AssetsWeightInfoOf, AssetsWeightInfoTrait, Config, + Read::*, + }, + mock::*, + Read, +}; const TOKEN: u32 = 42; type Event = crate::fungibles::Event; +type WeightInfo = ::WeightInfo; +type AssetsWeightInfo = AssetsWeightInfoOf; #[test] fn transfer_works() { @@ -41,9 +51,8 @@ fn transfer_from_works() { let from = Some(ALICE); let to = Some(BOB); - // Approve CHARLIE to transfer up to `value` to BOB. + // Approve CHARLIE to transfer up to `value`. create_token_mint_and_approve(ALICE, token, ALICE, value * 2, CHARLIE, value); - // TODO: weight for origin in vec![root(), none()] { assert_noop!(Fungibles::transfer_from(origin, token, ALICE, BOB, value), BadOrigin); } @@ -70,30 +79,51 @@ fn approve_works() { let spender = BOB; create_token_and_mint_to(ALICE, token, ALICE, value); + for origin in vec![root(), none()] { + assert_noop!( + Fungibles::approve(origin, token, BOB, value), + BadOrigin.with_weight(WeightInfo::approve(0, 0)) + ); + } assert_eq!(0, Assets::allowance(token, &ALICE, &BOB)); - assert_ok!(Fungibles::approve(signed(ALICE), token, BOB, value)); + assert_eq!( + Fungibles::approve(signed(ALICE), token, BOB, value), + Ok(Some(WeightInfo::approve(1, 0)).into()) + ); assert_eq!(Assets::allowance(token, &ALICE, &BOB), value); System::assert_last_event(Event::Approval { token, owner, spender, value }.into()); - // Approves an value to spend that is lower than the current allowance. - assert_ok!(Fungibles::approve(signed(ALICE), token, BOB, value / 2)); + // Approves a value to spend that is lower than the current allowance. + assert_eq!( + Fungibles::approve(signed(ALICE), token, BOB, value / 2), + Ok(Some(WeightInfo::approve(1, 1)).into()) + ); assert_eq!(Assets::allowance(token, &ALICE, &BOB), value / 2); System::assert_last_event( Event::Approval { token, owner, spender, value: value / 2 }.into(), ); - // Approves an value to spend that is higher than the current allowance. - assert_ok!(Fungibles::approve(signed(ALICE), token, BOB, value * 2)); + // Approves a value to spend that is higher than the current allowance. + assert_eq!( + Fungibles::approve(signed(ALICE), token, BOB, value * 2), + Ok(Some(WeightInfo::approve(1, 0)).into()) + ); assert_eq!(Assets::allowance(token, &ALICE, &BOB), value * 2); System::assert_last_event( Event::Approval { token, owner, spender, value: value * 2 }.into(), ); - // Approves an value to spend that is equal to the current allowance. - assert_ok!(Fungibles::approve(signed(ALICE), token, BOB, value * 2)); + // Approves a value to spend that is equal to the current allowance. + assert_eq!( + Fungibles::approve(signed(ALICE), token, BOB, value * 2), + Ok(Some(WeightInfo::approve(0, 0)).into()) + ); assert_eq!(Assets::allowance(token, &ALICE, &BOB), value * 2); System::assert_last_event( Event::Approval { token, owner, spender, value: value * 2 }.into(), ); // Sets allowance to zero. - assert_ok!(Fungibles::approve(signed(ALICE), token, BOB, 0)); + assert_eq!( + Fungibles::approve(signed(ALICE), token, BOB, 0), + Ok(Some(WeightInfo::approve(0, 1)).into()) + ); assert_eq!(Assets::allowance(token, &ALICE, &BOB), 0); System::assert_last_event(Event::Approval { token, owner, spender, value: 0 }.into()); }); @@ -108,9 +138,11 @@ fn increase_allowance_works() { let spender = BOB; create_token_and_mint_to(ALICE, token, ALICE, value); - // TODO: weight for origin in vec![root(), none()] { - assert_noop!(Fungibles::increase_allowance(origin, token, BOB, value), BadOrigin); + assert_noop!( + Fungibles::increase_allowance(origin, token, BOB, value), + BadOrigin.with_weight(WeightInfo::approve(0, 0)) + ); } assert_eq!(0, Assets::allowance(token, &ALICE, &BOB)); assert_ok!(Fungibles::increase_allowance(signed(ALICE), token, BOB, value)); @@ -134,22 +166,33 @@ fn decrease_allowance_works() { let spender = BOB; create_token_mint_and_approve(ALICE, token, ALICE, value, BOB, value); - // TODO: weight for origin in vec![root(), none()] { - assert_noop!(Fungibles::decrease_allowance(origin, token, BOB, 0), BadOrigin); + assert_noop!( + Fungibles::decrease_allowance(origin, token, BOB, 0), + BadOrigin.with_weight(WeightInfo::approve(0, 0)) + ); } assert_eq!(Assets::allowance(token, &ALICE, &BOB), value); // Owner balance is not changed if decreased by zero. - assert_ok!(Fungibles::decrease_allowance(signed(ALICE), token, BOB, 0)); + assert_eq!( + Fungibles::decrease_allowance(signed(ALICE), token, BOB, 0), + Ok(Some(WeightInfo::approve(0, 0)).into()) + ); assert_eq!(Assets::allowance(token, &ALICE, &BOB), value); // Decrease allowance successfully. - assert_ok!(Fungibles::decrease_allowance(signed(ALICE), token, BOB, value / 2)); + assert_eq!( + Fungibles::decrease_allowance(signed(ALICE), token, BOB, value / 2), + Ok(Some(WeightInfo::approve(1, 1)).into()) + ); assert_eq!(Assets::allowance(token, &ALICE, &BOB), value / 2); System::assert_last_event( Event::Approval { token, owner, spender, value: value / 2 }.into(), ); // Saturating if current allowance is decreased more than the owner balance. - assert_ok!(Fungibles::decrease_allowance(signed(ALICE), token, BOB, value)); + assert_eq!( + Fungibles::decrease_allowance(signed(ALICE), token, BOB, value), + Ok(Some(WeightInfo::approve(0, 1)).into()) + ); assert_eq!(Assets::allowance(token, &ALICE, &BOB), 0); System::assert_last_event(Event::Approval { token, owner, spender, value: 0 }.into()); }); @@ -179,6 +222,7 @@ fn start_destroy_works() { create_token(ALICE, token); assert_ok!(Fungibles::start_destroy(signed(ALICE), token)); + assert!(Fungibles::start_destroy(signed(BOB), token).is_err()); }); } @@ -241,10 +285,13 @@ fn burn_works() { let token = TOKEN; let from = Some(BOB); let to = None; + let total_supply = value * 2; - create_token_and_mint_to(ALICE, token, BOB, value); + create_token_and_mint_to(ALICE, token, BOB, total_supply); + assert_eq!(Assets::total_supply(TOKEN), total_supply); let balance_before_burn = Assets::balance(token, &BOB); assert_ok!(Fungibles::burn(signed(ALICE), token, BOB, value)); + assert_eq!(Assets::total_supply(TOKEN), total_supply - value); let balance_after_burn = Assets::balance(token, &BOB); assert_eq!(balance_after_burn, balance_before_burn - value); System::assert_last_event(Event::Transfer { token, from, to, value }.into()); @@ -254,21 +301,28 @@ fn burn_works() { #[test] fn total_supply_works() { new_test_ext().execute_with(|| { - create_token_and_mint_to(ALICE, TOKEN, ALICE, 100); + let total_supply = INIT_AMOUNT; + create_token_and_mint_to(ALICE, TOKEN, ALICE, total_supply); assert_eq!( + Fungibles::read(TotalSupply(TOKEN)).encode(), Assets::total_supply(TOKEN).encode(), - Fungibles::read(TotalSupply(TOKEN)).encode() ); + assert_eq!(Fungibles::read(TotalSupply(TOKEN)).encode(), total_supply.encode(),); }); } #[test] fn balance_of_works() { new_test_ext().execute_with(|| { - create_token_and_mint_to(ALICE, TOKEN, ALICE, 100); + let value = 1_000 * UNIT; + create_token_and_mint_to(ALICE, TOKEN, ALICE, value); assert_eq!( + Fungibles::read(BalanceOf { token: TOKEN, owner: ALICE }).encode(), Assets::balance(TOKEN, ALICE).encode(), - Fungibles::read(BalanceOf { token: TOKEN, owner: ALICE }).encode() + ); + assert_eq!( + Fungibles::read(BalanceOf { token: TOKEN, owner: ALICE }).encode(), + value.encode() ); }); } @@ -276,10 +330,15 @@ fn balance_of_works() { #[test] fn allowance_works() { new_test_ext().execute_with(|| { - create_token_mint_and_approve(ALICE, TOKEN, BOB, 100, ALICE, 50); + let value = 1_000 * UNIT; + create_token_mint_and_approve(ALICE, TOKEN, ALICE, value * 2, BOB, value); assert_eq!( + Fungibles::read(Allowance { token: TOKEN, owner: ALICE, spender: BOB }).encode(), Assets::allowance(TOKEN, &ALICE, &BOB).encode(), - Fungibles::read(Allowance { token: TOKEN, owner: ALICE, spender: BOB }).encode() + ); + assert_eq!( + Fungibles::read(Allowance { token: TOKEN, owner: ALICE, spender: BOB }).encode(), + value.encode() ); }); } @@ -291,12 +350,15 @@ fn token_metadata_works() { let symbol: Vec = vec![21, 22, 23]; let decimals: u8 = 69; create_token_and_set_metadata(ALICE, TOKEN, name.clone(), symbol.clone(), decimals); - assert_eq!(Assets::name(TOKEN).encode(), Fungibles::read(TokenName(TOKEN)).encode()); - assert_eq!(Assets::symbol(TOKEN).encode(), Fungibles::read(TokenSymbol(TOKEN)).encode()); + assert_eq!(Fungibles::read(TokenName(TOKEN)).encode(), Assets::name(TOKEN).encode()); + assert_eq!(Fungibles::read(TokenSymbol(TOKEN)).encode(), Assets::symbol(TOKEN).encode()); assert_eq!( + Fungibles::read(TokenDecimals(TOKEN)).encode(), Assets::decimals(TOKEN).encode(), - Fungibles::read(TokenDecimals(TOKEN)).encode() ); + assert_eq!(Fungibles::read(TokenName(TOKEN)).encode(), name.encode()); + assert_eq!(Fungibles::read(TokenSymbol(TOKEN)).encode(), symbol.encode()); + assert_eq!(Fungibles::read(TokenDecimals(TOKEN)).encode(), decimals.encode()); }); } @@ -305,9 +367,10 @@ fn token_exists_works() { new_test_ext().execute_with(|| { create_token(ALICE, TOKEN); assert_eq!( + Fungibles::read(TokenExists(TOKEN)).encode(), Assets::asset_exists(TOKEN).encode(), - Fungibles::read(TokenExists(TOKEN)).encode() ); + assert_eq!(Fungibles::read(TokenExists(TOKEN)).encode(), true.encode()); }); } From 538b19e331aa78daa7f70b1322582cccf2cac644 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Wed, 11 Sep 2024 19:41:08 +0200 Subject: [PATCH 16/29] fix: comment --- pallets/api/src/fungibles/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 6ad7f762..4f2d1857 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -155,7 +155,7 @@ pub mod pallet { /// The amount transferred (or minted/burned). value: BalanceOf, }, - /// Event emitted when an token is created. + /// Event emitted when a token is created. Create { /// The token identifier. id: TokenIdOf, From ca1ccca3c8302c56130c47b7cf9b200d871a5a27 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Thu, 12 Sep 2024 14:02:48 +0200 Subject: [PATCH 17/29] test: error case test coverage --- pallets/api/src/fungibles/tests.rs | 122 ++++++++++++++++++++++------- 1 file changed, 94 insertions(+), 28 deletions(-) diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index 651a7c9a..079757f8 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -10,8 +10,8 @@ use frame_support::{ use crate::{ fungibles::{ - weights::WeightInfo as WeightInfoTrait, AssetsWeightInfoOf, AssetsWeightInfoTrait, Config, - Read::*, + weights::WeightInfo as WeightInfoTrait, AssetsInstanceOf, AssetsWeightInfoOf, + AssetsWeightInfoTrait, Config, Read::*, }, mock::*, Read, @@ -19,9 +19,10 @@ use crate::{ const TOKEN: u32 = 42; +type AssetsError = pallet_assets::Error>; +type AssetsWeightInfo = AssetsWeightInfoOf; type Event = crate::fungibles::Event; type WeightInfo = ::WeightInfo; -type AssetsWeightInfo = AssetsWeightInfoOf; #[test] fn transfer_works() { @@ -31,7 +32,9 @@ fn transfer_works() { let from = Some(ALICE); let to = Some(BOB); - create_token_and_mint_to(ALICE, token, ALICE, value * 2); + // Check error works for `Assets::transfer_keep_alive()`. + assert_noop!(Fungibles::transfer(signed(ALICE), token, BOB, value), AssetsError::Unknown); + pallet_assets_create_and_mint_to(ALICE, token, ALICE, value * 2); for origin in vec![root(), none()] { assert_noop!(Fungibles::transfer(origin, token, BOB, value), BadOrigin); } @@ -51,8 +54,13 @@ fn transfer_from_works() { let from = Some(ALICE); let to = Some(BOB); + // Check error works for `Assets::transfer_approved()`. + assert_noop!( + Fungibles::transfer_from(signed(CHARLIE), token, ALICE, BOB, value), + AssetsError::Unknown + ); // Approve CHARLIE to transfer up to `value`. - create_token_mint_and_approve(ALICE, token, ALICE, value * 2, CHARLIE, value); + pallet_assets_create_mint_and_approve(ALICE, token, ALICE, value * 2, CHARLIE, value); for origin in vec![root(), none()] { assert_noop!(Fungibles::transfer_from(origin, token, ALICE, BOB, value), BadOrigin); } @@ -78,7 +86,14 @@ fn approve_works() { let owner = ALICE; let spender = BOB; - create_token_and_mint_to(ALICE, token, ALICE, value); + // Approves a value to spend. + // + // Check error works for `Assets::approve_transfer()` in `Greater` match arm. + assert_noop!( + Fungibles::approve(signed(ALICE), token, BOB, value), + AssetsError::Unknown.with_weight(WeightInfo::approve(1, 0)) + ); + pallet_assets_create_and_mint_to(ALICE, token, ALICE, value); for origin in vec![root(), none()] { assert_noop!( Fungibles::approve(origin, token, BOB, value), @@ -93,6 +108,15 @@ fn approve_works() { assert_eq!(Assets::allowance(token, &ALICE, &BOB), value); System::assert_last_event(Event::Approval { token, owner, spender, value }.into()); // Approves a value to spend that is lower than the current allowance. + // + // Check error works for `Assets::cancel_approval()` in `Less` match arm. No error test for + // `approve_transfer` in `Less` arm because it is not possible. + pallet_assets_freeze_asset(ALICE, token); + assert_noop!( + Fungibles::approve(signed(ALICE), token, BOB, value / 2), + AssetsError::AssetNotLive.with_weight(WeightInfo::approve(0, 1)) + ); + pallet_assets_thaw_asset(ALICE, token); assert_eq!( Fungibles::approve(signed(ALICE), token, BOB, value / 2), Ok(Some(WeightInfo::approve(1, 1)).into()) @@ -137,7 +161,12 @@ fn increase_allowance_works() { let owner = ALICE; let spender = BOB; - create_token_and_mint_to(ALICE, token, ALICE, value); + // Check error works for `Assets::approve_transfer()`. + assert_noop!( + Fungibles::increase_allowance(signed(ALICE), token, BOB, value), + AssetsError::Unknown.with_weight(AssetsWeightInfo::approve_transfer()) + ); + pallet_assets_create_and_mint_to(ALICE, token, ALICE, value); for origin in vec![root(), none()] { assert_noop!( Fungibles::increase_allowance(origin, token, BOB, value), @@ -165,7 +194,13 @@ fn decrease_allowance_works() { let owner = ALICE; let spender = BOB; - create_token_mint_and_approve(ALICE, token, ALICE, value, BOB, value); + // Check error works for `Assets::cancel_approval()`. No error test for `approve_transfer` + // because it is not possible. + assert_noop!( + Fungibles::decrease_allowance(signed(ALICE), token, BOB, value / 2), + AssetsError::Unknown.with_weight(WeightInfo::approve(0, 1)) + ); + pallet_assets_create_mint_and_approve(ALICE, token, ALICE, value, BOB, value); for origin in vec![root(), none()] { assert_noop!( Fungibles::decrease_allowance(origin, token, BOB, 0), @@ -212,6 +247,8 @@ fn create_works() { assert_ok!(Fungibles::create(signed(creator), id, admin, 100)); assert!(Assets::asset_exists(id)); System::assert_last_event(Event::Create { id, creator, admin }.into()); + // Check error works for `Assets::create()`. + assert_noop!(Fungibles::create(signed(creator), id, admin, 100), AssetsError::InUse); }); } @@ -220,7 +257,9 @@ fn start_destroy_works() { new_test_ext().execute_with(|| { let token = TOKEN; - create_token(ALICE, token); + // Check error works for `Assets::start_destroy()`. + assert_noop!(Fungibles::start_destroy(signed(ALICE), token), AssetsError::Unknown); + pallet_assets_create(ALICE, token); assert_ok!(Fungibles::start_destroy(signed(ALICE), token)); assert!(Fungibles::start_destroy(signed(BOB), token).is_err()); }); @@ -234,7 +273,12 @@ fn set_metadata_works() { let symbol = vec![42]; let decimals = 42; - create_token(ALICE, token); + // Check error works for `Assets::set_metadata()`. + assert_noop!( + Fungibles::set_metadata(signed(ALICE), token, name.clone(), symbol.clone(), decimals), + AssetsError::Unknown + ); + pallet_assets_create(ALICE, token); assert_ok!(Fungibles::set_metadata( signed(ALICE), token, @@ -253,7 +297,9 @@ fn clear_metadata_works() { new_test_ext().execute_with(|| { let token = TOKEN; - create_token_and_set_metadata(ALICE, token, vec![42], vec![42], 42); + // Check error works for `Assets::clear_metadata()`. + assert_noop!(Fungibles::clear_metadata(signed(ALICE), token), AssetsError::Unknown); + pallet_assets_create_and_set_metadata(ALICE, token, vec![42], vec![42], 42); assert_ok!(Fungibles::clear_metadata(signed(ALICE), token)); assert!(Assets::name(token).is_empty()); assert!(Assets::symbol(token).is_empty()); @@ -269,7 +315,12 @@ fn mint_works() { let from = None; let to = Some(BOB); - create_token(ALICE, token); + // Check error works for `Assets::mint()`. + assert_noop!( + Fungibles::mint(signed(ALICE), token, BOB, value), + sp_runtime::TokenError::UnknownAsset + ); + pallet_assets_create(ALICE, token); let balance_before_mint = Assets::balance(token, &BOB); assert_ok!(Fungibles::mint(signed(ALICE), token, BOB, value)); let balance_after_mint = Assets::balance(token, &BOB); @@ -287,7 +338,9 @@ fn burn_works() { let to = None; let total_supply = value * 2; - create_token_and_mint_to(ALICE, token, BOB, total_supply); + // Check error works for `Assets::burn()`. + assert_noop!(Fungibles::burn(signed(ALICE), token, BOB, value), AssetsError::Unknown); + pallet_assets_create_and_mint_to(ALICE, token, BOB, total_supply); assert_eq!(Assets::total_supply(TOKEN), total_supply); let balance_before_burn = Assets::balance(token, &BOB); assert_ok!(Fungibles::burn(signed(ALICE), token, BOB, value)); @@ -302,7 +355,7 @@ fn burn_works() { fn total_supply_works() { new_test_ext().execute_with(|| { let total_supply = INIT_AMOUNT; - create_token_and_mint_to(ALICE, TOKEN, ALICE, total_supply); + pallet_assets_create_and_mint_to(ALICE, TOKEN, ALICE, total_supply); assert_eq!( Fungibles::read(TotalSupply(TOKEN)).encode(), Assets::total_supply(TOKEN).encode(), @@ -315,7 +368,7 @@ fn total_supply_works() { fn balance_of_works() { new_test_ext().execute_with(|| { let value = 1_000 * UNIT; - create_token_and_mint_to(ALICE, TOKEN, ALICE, value); + pallet_assets_create_and_mint_to(ALICE, TOKEN, ALICE, value); assert_eq!( Fungibles::read(BalanceOf { token: TOKEN, owner: ALICE }).encode(), Assets::balance(TOKEN, ALICE).encode(), @@ -331,7 +384,7 @@ fn balance_of_works() { fn allowance_works() { new_test_ext().execute_with(|| { let value = 1_000 * UNIT; - create_token_mint_and_approve(ALICE, TOKEN, ALICE, value * 2, BOB, value); + pallet_assets_create_mint_and_approve(ALICE, TOKEN, ALICE, value * 2, BOB, value); assert_eq!( Fungibles::read(Allowance { token: TOKEN, owner: ALICE, spender: BOB }).encode(), Assets::allowance(TOKEN, &ALICE, &BOB).encode(), @@ -349,7 +402,7 @@ fn token_metadata_works() { let name: Vec = vec![11, 12, 13]; let symbol: Vec = vec![21, 22, 23]; let decimals: u8 = 69; - create_token_and_set_metadata(ALICE, TOKEN, name.clone(), symbol.clone(), decimals); + pallet_assets_create_and_set_metadata(ALICE, TOKEN, name.clone(), symbol.clone(), decimals); assert_eq!(Fungibles::read(TokenName(TOKEN)).encode(), Assets::name(TOKEN).encode()); assert_eq!(Fungibles::read(TokenSymbol(TOKEN)).encode(), Assets::symbol(TOKEN).encode()); assert_eq!( @@ -365,7 +418,7 @@ fn token_metadata_works() { #[test] fn token_exists_works() { new_test_ext().execute_with(|| { - create_token(ALICE, TOKEN); + pallet_assets_create(ALICE, TOKEN); assert_eq!( Fungibles::read(TokenExists(TOKEN)).encode(), Assets::asset_exists(TOKEN).encode(), @@ -386,20 +439,25 @@ fn none() -> RuntimeOrigin { RuntimeOrigin::none() } -fn create_token(owner: AccountId, token: TokenId) { +fn pallet_assets_create(owner: AccountId, token: TokenId) { assert_ok!(Assets::create(signed(owner), token, owner, 1)); } -fn mint_token(owner: AccountId, token: TokenId, to: AccountId, value: Balance) { +fn pallet_assets_mint(owner: AccountId, token: TokenId, to: AccountId, value: Balance) { assert_ok!(Assets::mint(signed(owner), token, to, value)); } -fn create_token_and_mint_to(owner: AccountId, token: TokenId, to: AccountId, value: Balance) { - create_token(owner, token); - mint_token(owner, token, to, value) +fn pallet_assets_create_and_mint_to( + owner: AccountId, + token: TokenId, + to: AccountId, + value: Balance, +) { + pallet_assets_create(owner, token); + pallet_assets_mint(owner, token, to, value) } -fn create_token_mint_and_approve( +fn pallet_assets_create_mint_and_approve( owner: AccountId, token: TokenId, to: AccountId, @@ -407,11 +465,11 @@ fn create_token_mint_and_approve( spender: AccountId, approve: Balance, ) { - create_token_and_mint_to(owner, token, to, mint); + pallet_assets_create_and_mint_to(owner, token, to, mint); assert_ok!(Assets::approve_transfer(signed(to), token, spender, approve,)); } -fn create_token_and_set_metadata( +fn pallet_assets_create_and_set_metadata( owner: AccountId, token: TokenId, name: Vec, @@ -419,10 +477,10 @@ fn create_token_and_set_metadata( decimals: u8, ) { assert_ok!(Assets::create(signed(owner), token, owner, 100)); - set_metadata_token(owner, token, name, symbol, decimals); + pallet_assets_set_metadata(owner, token, name, symbol, decimals); } -fn set_metadata_token( +fn pallet_assets_set_metadata( owner: AccountId, token: TokenId, name: Vec, @@ -431,3 +489,11 @@ fn set_metadata_token( ) { assert_ok!(Assets::set_metadata(signed(owner), token, name, symbol, decimals)); } + +fn pallet_assets_freeze_asset(owner: AccountId, token: TokenId) { + assert_ok!(Assets::freeze_asset(signed(owner), token)); +} + +fn pallet_assets_thaw_asset(owner: AccountId, token: TokenId) { + assert_ok!(Assets::thaw_asset(signed(owner), token)); +} From 1060a91d66a981ecb6b562a01325de54d645e4a0 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Thu, 12 Sep 2024 15:22:38 +0200 Subject: [PATCH 18/29] test: no state set test --- pallets/api/src/fungibles/tests.rs | 46 ++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index 079757f8..e84333c8 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -11,7 +11,7 @@ use frame_support::{ use crate::{ fungibles::{ weights::WeightInfo as WeightInfoTrait, AssetsInstanceOf, AssetsWeightInfoOf, - AssetsWeightInfoTrait, Config, Read::*, + AssetsWeightInfoTrait, Config, Read::*, ReadResult, }, mock::*, Read, @@ -355,12 +355,16 @@ fn burn_works() { fn total_supply_works() { new_test_ext().execute_with(|| { let total_supply = INIT_AMOUNT; + assert_eq!( + Fungibles::read(TotalSupply(TOKEN)), + ReadResult::TotalSupply(Default::default()) + ); pallet_assets_create_and_mint_to(ALICE, TOKEN, ALICE, total_supply); + assert_eq!(Fungibles::read(TotalSupply(TOKEN)), ReadResult::TotalSupply(total_supply)); assert_eq!( Fungibles::read(TotalSupply(TOKEN)).encode(), Assets::total_supply(TOKEN).encode(), ); - assert_eq!(Fungibles::read(TotalSupply(TOKEN)).encode(), total_supply.encode(),); }); } @@ -368,14 +372,18 @@ fn total_supply_works() { fn balance_of_works() { new_test_ext().execute_with(|| { let value = 1_000 * UNIT; + assert_eq!( + Fungibles::read(BalanceOf { token: TOKEN, owner: ALICE }), + ReadResult::BalanceOf(Default::default()) + ); pallet_assets_create_and_mint_to(ALICE, TOKEN, ALICE, value); assert_eq!( - Fungibles::read(BalanceOf { token: TOKEN, owner: ALICE }).encode(), - Assets::balance(TOKEN, ALICE).encode(), + Fungibles::read(BalanceOf { token: TOKEN, owner: ALICE }), + ReadResult::BalanceOf(value) ); assert_eq!( Fungibles::read(BalanceOf { token: TOKEN, owner: ALICE }).encode(), - value.encode() + Assets::balance(TOKEN, ALICE).encode(), ); }); } @@ -384,14 +392,18 @@ fn balance_of_works() { fn allowance_works() { new_test_ext().execute_with(|| { let value = 1_000 * UNIT; + assert_eq!( + Fungibles::read(Allowance { token: TOKEN, owner: ALICE, spender: BOB }), + ReadResult::Allowance(Default::default()) + ); pallet_assets_create_mint_and_approve(ALICE, TOKEN, ALICE, value * 2, BOB, value); assert_eq!( - Fungibles::read(Allowance { token: TOKEN, owner: ALICE, spender: BOB }).encode(), - Assets::allowance(TOKEN, &ALICE, &BOB).encode(), + Fungibles::read(Allowance { token: TOKEN, owner: ALICE, spender: BOB }), + ReadResult::Allowance(value) ); assert_eq!( Fungibles::read(Allowance { token: TOKEN, owner: ALICE, spender: BOB }).encode(), - value.encode() + Assets::allowance(TOKEN, &ALICE, &BOB).encode(), ); }); } @@ -402,28 +414,38 @@ fn token_metadata_works() { let name: Vec = vec![11, 12, 13]; let symbol: Vec = vec![21, 22, 23]; let decimals: u8 = 69; + assert_eq!(Fungibles::read(TokenName(TOKEN)), ReadResult::TokenName(Default::default())); + assert_eq!( + Fungibles::read(TokenSymbol(TOKEN)), + ReadResult::TokenSymbol(Default::default()) + ); + assert_eq!( + Fungibles::read(TokenDecimals(TOKEN)), + ReadResult::TokenDecimals(Default::default()) + ); pallet_assets_create_and_set_metadata(ALICE, TOKEN, name.clone(), symbol.clone(), decimals); + assert_eq!(Fungibles::read(TokenName(TOKEN)), ReadResult::TokenName(name)); + assert_eq!(Fungibles::read(TokenSymbol(TOKEN)), ReadResult::TokenSymbol(symbol)); + assert_eq!(Fungibles::read(TokenDecimals(TOKEN)), ReadResult::TokenDecimals(decimals)); assert_eq!(Fungibles::read(TokenName(TOKEN)).encode(), Assets::name(TOKEN).encode()); assert_eq!(Fungibles::read(TokenSymbol(TOKEN)).encode(), Assets::symbol(TOKEN).encode()); assert_eq!( Fungibles::read(TokenDecimals(TOKEN)).encode(), Assets::decimals(TOKEN).encode(), ); - assert_eq!(Fungibles::read(TokenName(TOKEN)).encode(), name.encode()); - assert_eq!(Fungibles::read(TokenSymbol(TOKEN)).encode(), symbol.encode()); - assert_eq!(Fungibles::read(TokenDecimals(TOKEN)).encode(), decimals.encode()); }); } #[test] fn token_exists_works() { new_test_ext().execute_with(|| { + assert_eq!(Fungibles::read(TokenExists(TOKEN)), ReadResult::TokenExists(false)); pallet_assets_create(ALICE, TOKEN); + assert_eq!(Fungibles::read(TokenExists(TOKEN)), ReadResult::TokenExists(true)); assert_eq!( Fungibles::read(TokenExists(TOKEN)).encode(), Assets::asset_exists(TOKEN).encode(), ); - assert_eq!(Fungibles::read(TokenExists(TOKEN)).encode(), true.encode()); }); } From 98e89dd4f2791d927b42cc122f5a1f834b410cf0 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Thu, 12 Sep 2024 15:44:28 +0200 Subject: [PATCH 19/29] refactor: test variables --- pallets/api/src/fungibles/tests.rs | 164 ++++++++++++++++------------- 1 file changed, 90 insertions(+), 74 deletions(-) diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index e84333c8..b0af511c 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -24,25 +24,30 @@ type AssetsWeightInfo = AssetsWeightInfoOf; type Event = crate::fungibles::Event; type WeightInfo = ::WeightInfo; +#[test] +fn encoding_read_result_works() {} + #[test] fn transfer_works() { new_test_ext().execute_with(|| { let value: Balance = 100 * UNIT; let token = TOKEN; - let from = Some(ALICE); - let to = Some(BOB); + let from = ALICE; + let to = BOB; // Check error works for `Assets::transfer_keep_alive()`. - assert_noop!(Fungibles::transfer(signed(ALICE), token, BOB, value), AssetsError::Unknown); - pallet_assets_create_and_mint_to(ALICE, token, ALICE, value * 2); + assert_noop!(Fungibles::transfer(signed(from), token, to, value), AssetsError::Unknown); + pallet_assets_create_and_mint_to(from, token, from, value * 2); for origin in vec![root(), none()] { - assert_noop!(Fungibles::transfer(origin, token, BOB, value), BadOrigin); + assert_noop!(Fungibles::transfer(origin, token, to, value), BadOrigin); } - let balance_before_transfer = Assets::balance(token, &BOB); - assert_ok!(Fungibles::transfer(signed(ALICE), token, BOB, value)); - let balance_after_transfer = Assets::balance(token, &BOB); + let balance_before_transfer = Assets::balance(token, &to); + assert_ok!(Fungibles::transfer(signed(from), token, to, value)); + let balance_after_transfer = Assets::balance(token, &to); assert_eq!(balance_after_transfer, balance_before_transfer + value); - System::assert_last_event(Event::Transfer { token, from, to, value }.into()); + System::assert_last_event( + Event::Transfer { token, from: Some(from), to: Some(to), value }.into(), + ); }); } @@ -51,29 +56,32 @@ fn transfer_from_works() { new_test_ext().execute_with(|| { let value: Balance = 100 * UNIT; let token = TOKEN; - let from = Some(ALICE); - let to = Some(BOB); + let from = ALICE; + let to = BOB; + let spender = CHARLIE; // Check error works for `Assets::transfer_approved()`. assert_noop!( - Fungibles::transfer_from(signed(CHARLIE), token, ALICE, BOB, value), + Fungibles::transfer_from(signed(spender), token, from, to, value), AssetsError::Unknown ); - // Approve CHARLIE to transfer up to `value`. - pallet_assets_create_mint_and_approve(ALICE, token, ALICE, value * 2, CHARLIE, value); + // Approve `spender` to transfer up to `value`. + pallet_assets_create_mint_and_approve(spender, token, from, value * 2, spender, value); for origin in vec![root(), none()] { - assert_noop!(Fungibles::transfer_from(origin, token, ALICE, BOB, value), BadOrigin); + assert_noop!(Fungibles::transfer_from(origin, token, from, to, value), BadOrigin); } // Successfully call transfer from. - let alice_balance_before_transfer = Assets::balance(token, &ALICE); - let bob_balance_before_transfer = Assets::balance(token, &BOB); - assert_ok!(Fungibles::transfer_from(signed(CHARLIE), token, ALICE, BOB, value)); - let alice_balance_after_transfer = Assets::balance(token, &ALICE); - let bob_balance_after_transfer = Assets::balance(token, &BOB); - // Check that BOB receives the `value` and ALICE `amount` is spent successfully by CHARLIE. + let alice_balance_before_transfer = Assets::balance(token, &from); + let bob_balance_before_transfer = Assets::balance(token, &to); + assert_ok!(Fungibles::transfer_from(signed(spender), token, from, to, value)); + let alice_balance_after_transfer = Assets::balance(token, &from); + let bob_balance_after_transfer = Assets::balance(token, &to); + // Check that `to` has received the `value` tokens from `from`. assert_eq!(bob_balance_after_transfer, bob_balance_before_transfer + value); assert_eq!(alice_balance_after_transfer, alice_balance_before_transfer - value); - System::assert_last_event(Event::Transfer { token, from, to, value }.into()); + System::assert_last_event( + Event::Transfer { token, from: Some(from), to: Some(to), value }.into(), + ); }); } @@ -90,65 +98,65 @@ fn approve_works() { // // Check error works for `Assets::approve_transfer()` in `Greater` match arm. assert_noop!( - Fungibles::approve(signed(ALICE), token, BOB, value), + Fungibles::approve(signed(owner), token, spender, value), AssetsError::Unknown.with_weight(WeightInfo::approve(1, 0)) ); - pallet_assets_create_and_mint_to(ALICE, token, ALICE, value); + pallet_assets_create_and_mint_to(owner, token, owner, value); for origin in vec![root(), none()] { assert_noop!( - Fungibles::approve(origin, token, BOB, value), + Fungibles::approve(origin, token, spender, value), BadOrigin.with_weight(WeightInfo::approve(0, 0)) ); } - assert_eq!(0, Assets::allowance(token, &ALICE, &BOB)); + assert_eq!(0, Assets::allowance(token, &owner, &spender)); assert_eq!( - Fungibles::approve(signed(ALICE), token, BOB, value), + Fungibles::approve(signed(owner), token, spender, value), Ok(Some(WeightInfo::approve(1, 0)).into()) ); - assert_eq!(Assets::allowance(token, &ALICE, &BOB), value); + assert_eq!(Assets::allowance(token, &owner, &spender), value); System::assert_last_event(Event::Approval { token, owner, spender, value }.into()); // Approves a value to spend that is lower than the current allowance. // // Check error works for `Assets::cancel_approval()` in `Less` match arm. No error test for // `approve_transfer` in `Less` arm because it is not possible. - pallet_assets_freeze_asset(ALICE, token); + pallet_assets_freeze_asset(owner, token); assert_noop!( - Fungibles::approve(signed(ALICE), token, BOB, value / 2), + Fungibles::approve(signed(owner), token, spender, value / 2), AssetsError::AssetNotLive.with_weight(WeightInfo::approve(0, 1)) ); - pallet_assets_thaw_asset(ALICE, token); + pallet_assets_thaw_asset(owner, token); assert_eq!( - Fungibles::approve(signed(ALICE), token, BOB, value / 2), + Fungibles::approve(signed(owner), token, spender, value / 2), Ok(Some(WeightInfo::approve(1, 1)).into()) ); - assert_eq!(Assets::allowance(token, &ALICE, &BOB), value / 2); + assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); System::assert_last_event( Event::Approval { token, owner, spender, value: value / 2 }.into(), ); // Approves a value to spend that is higher than the current allowance. assert_eq!( - Fungibles::approve(signed(ALICE), token, BOB, value * 2), + Fungibles::approve(signed(owner), token, spender, value * 2), Ok(Some(WeightInfo::approve(1, 0)).into()) ); - assert_eq!(Assets::allowance(token, &ALICE, &BOB), value * 2); + assert_eq!(Assets::allowance(token, &owner, &spender), value * 2); System::assert_last_event( Event::Approval { token, owner, spender, value: value * 2 }.into(), ); // Approves a value to spend that is equal to the current allowance. assert_eq!( - Fungibles::approve(signed(ALICE), token, BOB, value * 2), + Fungibles::approve(signed(owner), token, spender, value * 2), Ok(Some(WeightInfo::approve(0, 0)).into()) ); - assert_eq!(Assets::allowance(token, &ALICE, &BOB), value * 2); + assert_eq!(Assets::allowance(token, &owner, &spender), value * 2); System::assert_last_event( Event::Approval { token, owner, spender, value: value * 2 }.into(), ); // Sets allowance to zero. assert_eq!( - Fungibles::approve(signed(ALICE), token, BOB, 0), + Fungibles::approve(signed(owner), token, spender, 0), Ok(Some(WeightInfo::approve(0, 1)).into()) ); - assert_eq!(Assets::allowance(token, &ALICE, &BOB), 0); + assert_eq!(Assets::allowance(token, &owner, &spender), 0); System::assert_last_event(Event::Approval { token, owner, spender, value: 0 }.into()); }); } @@ -163,23 +171,23 @@ fn increase_allowance_works() { // Check error works for `Assets::approve_transfer()`. assert_noop!( - Fungibles::increase_allowance(signed(ALICE), token, BOB, value), + Fungibles::increase_allowance(signed(owner), token, spender, value), AssetsError::Unknown.with_weight(AssetsWeightInfo::approve_transfer()) ); - pallet_assets_create_and_mint_to(ALICE, token, ALICE, value); + pallet_assets_create_and_mint_to(owner, token, owner, value); for origin in vec![root(), none()] { assert_noop!( - Fungibles::increase_allowance(origin, token, BOB, value), + Fungibles::increase_allowance(origin, token, spender, value), BadOrigin.with_weight(WeightInfo::approve(0, 0)) ); } - assert_eq!(0, Assets::allowance(token, &ALICE, &BOB)); - assert_ok!(Fungibles::increase_allowance(signed(ALICE), token, BOB, value)); - assert_eq!(Assets::allowance(token, &ALICE, &BOB), value); + assert_eq!(0, Assets::allowance(token, &owner, &spender)); + assert_ok!(Fungibles::increase_allowance(signed(owner), token, spender, value)); + assert_eq!(Assets::allowance(token, &owner, &spender), value); System::assert_last_event(Event::Approval { token, owner, spender, value }.into()); // Additive. - assert_ok!(Fungibles::increase_allowance(signed(ALICE), token, BOB, value)); - assert_eq!(Assets::allowance(token, &ALICE, &BOB), value * 2); + assert_ok!(Fungibles::increase_allowance(signed(owner), token, spender, value)); + assert_eq!(Assets::allowance(token, &owner, &spender), value * 2); System::assert_last_event( Event::Approval { token, owner, spender, value: value * 2 }.into(), ); @@ -197,38 +205,38 @@ fn decrease_allowance_works() { // Check error works for `Assets::cancel_approval()`. No error test for `approve_transfer` // because it is not possible. assert_noop!( - Fungibles::decrease_allowance(signed(ALICE), token, BOB, value / 2), + Fungibles::decrease_allowance(signed(owner), token, spender, value / 2), AssetsError::Unknown.with_weight(WeightInfo::approve(0, 1)) ); - pallet_assets_create_mint_and_approve(ALICE, token, ALICE, value, BOB, value); + pallet_assets_create_mint_and_approve(owner, token, owner, value, spender, value); for origin in vec![root(), none()] { assert_noop!( - Fungibles::decrease_allowance(origin, token, BOB, 0), + Fungibles::decrease_allowance(origin, token, spender, 0), BadOrigin.with_weight(WeightInfo::approve(0, 0)) ); } - assert_eq!(Assets::allowance(token, &ALICE, &BOB), value); + assert_eq!(Assets::allowance(token, &owner, &spender), value); // Owner balance is not changed if decreased by zero. assert_eq!( - Fungibles::decrease_allowance(signed(ALICE), token, BOB, 0), + Fungibles::decrease_allowance(signed(owner), token, spender, 0), Ok(Some(WeightInfo::approve(0, 0)).into()) ); - assert_eq!(Assets::allowance(token, &ALICE, &BOB), value); + assert_eq!(Assets::allowance(token, &owner, &spender), value); // Decrease allowance successfully. assert_eq!( - Fungibles::decrease_allowance(signed(ALICE), token, BOB, value / 2), + Fungibles::decrease_allowance(signed(owner), token, spender, value / 2), Ok(Some(WeightInfo::approve(1, 1)).into()) ); - assert_eq!(Assets::allowance(token, &ALICE, &BOB), value / 2); + assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); System::assert_last_event( Event::Approval { token, owner, spender, value: value / 2 }.into(), ); // Saturating if current allowance is decreased more than the owner balance. assert_eq!( - Fungibles::decrease_allowance(signed(ALICE), token, BOB, value), + Fungibles::decrease_allowance(signed(owner), token, spender, value), Ok(Some(WeightInfo::approve(0, 1)).into()) ); - assert_eq!(Assets::allowance(token, &ALICE, &BOB), 0); + assert_eq!(Assets::allowance(token, &owner, &spender), 0); System::assert_last_event(Event::Approval { token, owner, spender, value: 0 }.into()); }); } @@ -261,7 +269,11 @@ fn start_destroy_works() { assert_noop!(Fungibles::start_destroy(signed(ALICE), token), AssetsError::Unknown); pallet_assets_create(ALICE, token); assert_ok!(Fungibles::start_destroy(signed(ALICE), token)); - assert!(Fungibles::start_destroy(signed(BOB), token).is_err()); + // Check that the token is not live after starting the destroy process. + assert_noop!( + Assets::mint(signed(ALICE), token, ALICE, 10 * UNIT), + AssetsError::AssetNotLive + ); }); } @@ -312,20 +324,22 @@ fn mint_works() { new_test_ext().execute_with(|| { let value: Balance = 100 * UNIT; let token = TOKEN; - let from = None; - let to = Some(BOB); + let from = ALICE; + let to = BOB; // Check error works for `Assets::mint()`. assert_noop!( - Fungibles::mint(signed(ALICE), token, BOB, value), + Fungibles::mint(signed(from), token, to, value), sp_runtime::TokenError::UnknownAsset ); - pallet_assets_create(ALICE, token); - let balance_before_mint = Assets::balance(token, &BOB); - assert_ok!(Fungibles::mint(signed(ALICE), token, BOB, value)); - let balance_after_mint = Assets::balance(token, &BOB); + pallet_assets_create(from, token); + let balance_before_mint = Assets::balance(token, &to); + assert_ok!(Fungibles::mint(signed(from), token, to, value)); + let balance_after_mint = Assets::balance(token, &to); assert_eq!(balance_after_mint, balance_before_mint + value); - System::assert_last_event(Event::Transfer { token, from, to, value }.into()); + System::assert_last_event( + Event::Transfer { token, from: None, to: Some(to), value }.into(), + ); }); } @@ -334,20 +348,22 @@ fn burn_works() { new_test_ext().execute_with(|| { let value: Balance = 100 * UNIT; let token = TOKEN; - let from = Some(BOB); - let to = None; + let owner = ALICE; + let from = BOB; let total_supply = value * 2; // Check error works for `Assets::burn()`. - assert_noop!(Fungibles::burn(signed(ALICE), token, BOB, value), AssetsError::Unknown); - pallet_assets_create_and_mint_to(ALICE, token, BOB, total_supply); + assert_noop!(Fungibles::burn(signed(owner), token, from, value), AssetsError::Unknown); + pallet_assets_create_and_mint_to(owner, token, from, total_supply); assert_eq!(Assets::total_supply(TOKEN), total_supply); - let balance_before_burn = Assets::balance(token, &BOB); - assert_ok!(Fungibles::burn(signed(ALICE), token, BOB, value)); + let balance_before_burn = Assets::balance(token, &from); + assert_ok!(Fungibles::burn(signed(owner), token, from, value)); assert_eq!(Assets::total_supply(TOKEN), total_supply - value); - let balance_after_burn = Assets::balance(token, &BOB); + let balance_after_burn = Assets::balance(token, &from); assert_eq!(balance_after_burn, balance_before_burn - value); - System::assert_last_event(Event::Transfer { token, from, to, value }.into()); + System::assert_last_event( + Event::Transfer { token, from: Some(from), to: None, value }.into(), + ); }); } From 0ad8b36458fc92cde8a8fdffb11bb7f0cd7725a8 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Thu, 12 Sep 2024 16:12:15 +0200 Subject: [PATCH 20/29] test: encoding read result --- pallets/api/src/fungibles/tests.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index b0af511c..a405158d 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -25,7 +25,21 @@ type Event = crate::fungibles::Event; type WeightInfo = ::WeightInfo; #[test] -fn encoding_read_result_works() {} +fn encoding_read_result_works() { + use crate::fungibles::ReadResult::*; + let value = 100 * UNIT; + assert_eq!(TotalSupply::(value).encode(), value.encode()); + assert_eq!(BalanceOf::(value).encode(), value.encode()); + assert_eq!(Allowance::(value).encode(), value.encode()); + let value = vec![42, 42, 42, 42, 42]; + let value_encoded = value.encode(); + assert_eq!(TokenName::(value.clone()).encode(), value_encoded); + assert_eq!(TokenSymbol::(value).encode(), value_encoded); + let value = 42; + assert_eq!(Allowance::(value).encode(), value.encode()); + let value = true; + assert_eq!(TokenExists::(value).encode(), value.encode()); +} #[test] fn transfer_works() { From 682e8f3c06c71988bd9fc7fb17f2e58345d703ac Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Fri, 13 Sep 2024 15:54:58 +0200 Subject: [PATCH 21/29] refactor: pallet fungibles mod.rs --- pallets/api/src/fungibles/mod.rs | 37 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 4f2d1857..e38a8980 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -20,7 +20,7 @@ type AssetsOf = pallet_assets::Pallet>; type AssetsInstanceOf = ::AssetsInstance; type AssetsWeightInfoOf = >>::WeightInfo; type BalanceOf = as Inspect<::AccountId>>::Balance; -type WeightInfoOf = ::WeightInfo; +type WeightOf = ::WeightInfo; #[frame_support::pallet] pub mod pallet { @@ -236,12 +236,12 @@ pub mod pallet { value: BalanceOf, ) -> DispatchResultWithPostInfo { let owner = ensure_signed(origin.clone()) - .map_err(|e| e.with_weight(WeightInfoOf::::approve(0, 0)))?; + .map_err(|e| e.with_weight(WeightOf::::approve(0, 0)))?; let current_allowance = AssetsOf::::allowance(token.clone(), &owner, &spender); let weight = match value.cmp(¤t_allowance) { // If the new value is equal to the current allowance, do nothing. - Equal => WeightInfoOf::::approve(0, 0), + Equal => WeightOf::::approve(0, 0), // If the new value is greater than the current allowance, approve the difference // because `approve_transfer` works additively (see `pallet-assets`). Greater => { @@ -251,8 +251,8 @@ pub mod pallet { T::Lookup::unlookup(spender.clone()), value.saturating_sub(current_allowance), ) - .map_err(|e| e.with_weight(WeightInfoOf::::approve(1, 0)))?; - WeightInfoOf::::approve(1, 0) + .map_err(|e| e.with_weight(WeightOf::::approve(1, 0)))?; + WeightOf::::approve(1, 0) }, // If the new value is less than the current allowance, cancel the approval and // set the new value. @@ -264,9 +264,9 @@ pub mod pallet { token_param.clone(), spender_source.clone(), ) - .map_err(|e| e.with_weight(WeightInfoOf::::approve(0, 1)))?; + .map_err(|e| e.with_weight(WeightOf::::approve(0, 1)))?; if value.is_zero() { - WeightInfoOf::::approve(0, 1) + WeightOf::::approve(0, 1) } else { AssetsOf::::approve_transfer( origin, @@ -274,7 +274,7 @@ pub mod pallet { spender_source, value, )?; - WeightInfoOf::::approve(1, 1) + WeightOf::::approve(1, 1) } }, }; @@ -297,7 +297,7 @@ pub mod pallet { value: BalanceOf, ) -> DispatchResultWithPostInfo { let owner = ensure_signed(origin.clone()) - .map_err(|e| e.with_weight(WeightInfoOf::::approve(0, 0)))?; + .map_err(|e| e.with_weight(WeightOf::::approve(0, 0)))?; AssetsOf::::approve_transfer( origin, token.clone().into(), @@ -325,9 +325,9 @@ pub mod pallet { value: BalanceOf, ) -> DispatchResultWithPostInfo { let owner = ensure_signed(origin.clone()) - .map_err(|e| e.with_weight(WeightInfoOf::::approve(0, 0)))?; + .map_err(|e| e.with_weight(WeightOf::::approve(0, 0)))?; if value.is_zero() { - return Ok(Some(WeightInfoOf::::approve(0, 0)).into()); + return Ok(Some(WeightOf::::approve(0, 0)).into()); } let current_allowance = AssetsOf::::allowance(token.clone(), &owner, &spender); let spender_source = T::Lookup::unlookup(spender.clone()); @@ -339,10 +339,10 @@ pub mod pallet { token_param.clone(), spender_source.clone(), ) - .map_err(|e| e.with_weight(WeightInfoOf::::approve(0, 1)))?; + .map_err(|e| e.with_weight(WeightOf::::approve(0, 1)))?; let new_allowance = current_allowance.saturating_sub(value); let weight = if new_allowance.is_zero() { - WeightInfoOf::::approve(0, 1) + WeightOf::::approve(0, 1) } else { AssetsOf::::approve_transfer( origin, @@ -350,7 +350,7 @@ pub mod pallet { spender_source, new_allowance, )?; - WeightInfoOf::::approve(1, 1) + WeightOf::::approve(1, 1) }; Self::deposit_event(Event::Approval { token, owner, spender, value: new_allowance }); Ok(Some(weight).into()) @@ -383,13 +383,12 @@ pub mod pallet { /// Start the process of destroying a token. /// - /// See `pallet-assets` documentation for more information. Related dispatchables are: - /// - `destroy_accounts` - /// - `destroy_approvals` - /// - `finish_destroy` - /// /// # Parameters /// - `token` - The token to be destroyed. + // See `pallet-assets` documentation for more information. Related dispatchables are: + // - `destroy_accounts` + // - `destroy_approvals` + // - `finish_destroy` #[pallet::call_index(12)] #[pallet::weight(AssetsWeightInfoOf::::start_destroy())] pub fn start_destroy(origin: OriginFor, token: TokenIdOf) -> DispatchResult { From db382a844218e3cebc8244340bea764a5fc22e77 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Fri, 13 Sep 2024 15:55:51 +0200 Subject: [PATCH 22/29] refactor: split tests and test badorigin first --- pallets/api/src/fungibles/tests.rs | 272 +++++++++++++++++------------ 1 file changed, 161 insertions(+), 111 deletions(-) diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index a405158d..54944d24 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -24,21 +24,50 @@ type AssetsWeightInfo = AssetsWeightInfoOf; type Event = crate::fungibles::Event; type WeightInfo = ::WeightInfo; -#[test] -fn encoding_read_result_works() { - use crate::fungibles::ReadResult::*; - let value = 100 * UNIT; - assert_eq!(TotalSupply::(value).encode(), value.encode()); - assert_eq!(BalanceOf::(value).encode(), value.encode()); - assert_eq!(Allowance::(value).encode(), value.encode()); - let value = vec![42, 42, 42, 42, 42]; - let value_encoded = value.encode(); - assert_eq!(TokenName::(value.clone()).encode(), value_encoded); - assert_eq!(TokenSymbol::(value).encode(), value_encoded); - let value = 42; - assert_eq!(Allowance::(value).encode(), value.encode()); - let value = true; - assert_eq!(TokenExists::(value).encode(), value.encode()); +mod encoding_read_result { + use super::*; + + #[test] + fn total_supply() { + let total_supply = 1_000_000 * UNIT; + assert_eq!(ReadResult::TotalSupply::(total_supply).encode(), total_supply.encode()); + } + + #[test] + fn balance_of() { + let balance = 100 * UNIT; + assert_eq!(ReadResult::BalanceOf::(balance).encode(), balance.encode()); + } + + #[test] + fn allowance() { + let allowance = 100 * UNIT; + assert_eq!(ReadResult::Allowance::(allowance).encode(), allowance.encode()); + } + + #[test] + fn token_name() { + let name = vec![42, 42, 42, 42, 42]; + assert_eq!(ReadResult::TokenName::(name.clone()).encode(), name.encode()); + } + + #[test] + fn token_symbol() { + let symbol = vec![42, 42, 42, 42, 42]; + assert_eq!(ReadResult::TokenSymbol::(symbol.clone()).encode(), symbol.encode()); + } + + #[test] + fn token_decimals() { + let decimals = 42; + assert_eq!(ReadResult::TokenDecimals::(decimals).encode(), decimals.encode()); + } + + #[test] + fn token_exists() { + let exists = true; + assert_eq!(ReadResult::TokenExists::(exists).encode(), exists.encode()); + } } #[test] @@ -49,12 +78,12 @@ fn transfer_works() { let from = ALICE; let to = BOB; - // Check error works for `Assets::transfer_keep_alive()`. - assert_noop!(Fungibles::transfer(signed(from), token, to, value), AssetsError::Unknown); - pallet_assets_create_and_mint_to(from, token, from, value * 2); for origin in vec![root(), none()] { assert_noop!(Fungibles::transfer(origin, token, to, value), BadOrigin); } + // Check error works for `Assets::transfer_keep_alive()`. + assert_noop!(Fungibles::transfer(signed(from), token, to, value), AssetsError::Unknown); + pallet_assets_create_and_mint_to(from, token, from, value * 2); let balance_before_transfer = Assets::balance(token, &to); assert_ok!(Fungibles::transfer(signed(from), token, to, value)); let balance_after_transfer = Assets::balance(token, &to); @@ -74,6 +103,9 @@ fn transfer_from_works() { let to = BOB; let spender = CHARLIE; + for origin in vec![root(), none()] { + assert_noop!(Fungibles::transfer_from(origin, token, from, to, value), BadOrigin); + } // Check error works for `Assets::transfer_approved()`. assert_noop!( Fungibles::transfer_from(signed(spender), token, from, to, value), @@ -81,98 +113,116 @@ fn transfer_from_works() { ); // Approve `spender` to transfer up to `value`. pallet_assets_create_mint_and_approve(spender, token, from, value * 2, spender, value); - for origin in vec![root(), none()] { - assert_noop!(Fungibles::transfer_from(origin, token, from, to, value), BadOrigin); - } // Successfully call transfer from. - let alice_balance_before_transfer = Assets::balance(token, &from); - let bob_balance_before_transfer = Assets::balance(token, &to); + let from_balance_before_transfer = Assets::balance(token, &from); + let to_balance_before_transfer = Assets::balance(token, &to); assert_ok!(Fungibles::transfer_from(signed(spender), token, from, to, value)); - let alice_balance_after_transfer = Assets::balance(token, &from); - let bob_balance_after_transfer = Assets::balance(token, &to); + let from_balance_after_transfer = Assets::balance(token, &from); + let to_balance_after_transfer = Assets::balance(token, &to); // Check that `to` has received the `value` tokens from `from`. - assert_eq!(bob_balance_after_transfer, bob_balance_before_transfer + value); - assert_eq!(alice_balance_after_transfer, alice_balance_before_transfer - value); + assert_eq!(to_balance_after_transfer, to_balance_before_transfer + value); + assert_eq!(from_balance_after_transfer, from_balance_before_transfer - value); System::assert_last_event( Event::Transfer { token, from: Some(from), to: Some(to), value }.into(), ); }); } -// Non-additive, sets new value. -#[test] -fn approve_works() { - new_test_ext().execute_with(|| { - let value: Balance = 100 * UNIT; - let token = TOKEN; - let owner = ALICE; - let spender = BOB; - - // Approves a value to spend. - // - // Check error works for `Assets::approve_transfer()` in `Greater` match arm. - assert_noop!( - Fungibles::approve(signed(owner), token, spender, value), - AssetsError::Unknown.with_weight(WeightInfo::approve(1, 0)) - ); - pallet_assets_create_and_mint_to(owner, token, owner, value); - for origin in vec![root(), none()] { +mod approve { + use super::*; + + #[test] + fn ensure_signed_works() { + new_test_ext().execute_with(|| { + let value: Balance = 100 * UNIT; + let token = TOKEN; + let spender = BOB; + + for origin in vec![root(), none()] { + assert_noop!( + Fungibles::approve(origin, token, spender, value), + BadOrigin.with_weight(WeightInfo::approve(0, 0)) + ); + } + }); + } + + #[test] + fn ensure_error_cases_from_pallet_assets_work() { + new_test_ext().execute_with(|| { + let value: Balance = 100 * UNIT; + let token = TOKEN; + let owner = ALICE; + let spender = BOB; + + for origin in vec![root(), none()] { + assert_noop!( + Fungibles::approve(origin, token, spender, value), + BadOrigin.with_weight(WeightInfo::approve(0, 0)) + ); + } + // Check error works for `Assets::approve_transfer()` in `Greater` match arm. assert_noop!( - Fungibles::approve(origin, token, spender, value), - BadOrigin.with_weight(WeightInfo::approve(0, 0)) + Fungibles::approve(signed(owner), token, spender, value), + AssetsError::Unknown.with_weight(WeightInfo::approve(1, 0)) ); - } - assert_eq!(0, Assets::allowance(token, &owner, &spender)); - assert_eq!( - Fungibles::approve(signed(owner), token, spender, value), - Ok(Some(WeightInfo::approve(1, 0)).into()) - ); - assert_eq!(Assets::allowance(token, &owner, &spender), value); - System::assert_last_event(Event::Approval { token, owner, spender, value }.into()); - // Approves a value to spend that is lower than the current allowance. - // - // Check error works for `Assets::cancel_approval()` in `Less` match arm. No error test for - // `approve_transfer` in `Less` arm because it is not possible. - pallet_assets_freeze_asset(owner, token); - assert_noop!( - Fungibles::approve(signed(owner), token, spender, value / 2), - AssetsError::AssetNotLive.with_weight(WeightInfo::approve(0, 1)) - ); - pallet_assets_thaw_asset(owner, token); - assert_eq!( - Fungibles::approve(signed(owner), token, spender, value / 2), - Ok(Some(WeightInfo::approve(1, 1)).into()) - ); - assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); - System::assert_last_event( - Event::Approval { token, owner, spender, value: value / 2 }.into(), - ); - // Approves a value to spend that is higher than the current allowance. - assert_eq!( - Fungibles::approve(signed(owner), token, spender, value * 2), - Ok(Some(WeightInfo::approve(1, 0)).into()) - ); - assert_eq!(Assets::allowance(token, &owner, &spender), value * 2); - System::assert_last_event( - Event::Approval { token, owner, spender, value: value * 2 }.into(), - ); - // Approves a value to spend that is equal to the current allowance. - assert_eq!( - Fungibles::approve(signed(owner), token, spender, value * 2), - Ok(Some(WeightInfo::approve(0, 0)).into()) - ); - assert_eq!(Assets::allowance(token, &owner, &spender), value * 2); - System::assert_last_event( - Event::Approval { token, owner, spender, value: value * 2 }.into(), - ); - // Sets allowance to zero. - assert_eq!( - Fungibles::approve(signed(owner), token, spender, 0), - Ok(Some(WeightInfo::approve(0, 1)).into()) - ); - assert_eq!(Assets::allowance(token, &owner, &spender), 0); - System::assert_last_event(Event::Approval { token, owner, spender, value: 0 }.into()); - }); + pallet_assets_create_mint_and_approve(owner, token, owner, value, spender, value); + // Check error works for `Assets::cancel_approval()` in `Less` match arm. + pallet_assets_freeze_asset(owner, token); + assert_noop!( + Fungibles::approve(signed(owner), token, spender, value / 2), + AssetsError::AssetNotLive.with_weight(WeightInfo::approve(0, 1)) + ); + pallet_assets_thaw_asset(owner, token); + // No error test for `approve_transfer` in `Less` arm because it is not possible. + }); + } + + // Non-additive, sets new value. + #[test] + fn approve_works() { + new_test_ext().execute_with(|| { + let value: Balance = 100 * UNIT; + let token = TOKEN; + let owner = ALICE; + let spender = BOB; + + // Approves a value to spend that is higher than the current allowance. + pallet_assets_create_and_mint_to(owner, token, owner, value); + assert_eq!(Assets::allowance(token, &owner, &spender), 0); + assert_eq!( + Fungibles::approve(signed(owner), token, spender, value), + Ok(Some(WeightInfo::approve(1, 0)).into()) + ); + assert_eq!(Assets::allowance(token, &owner, &spender), value); + System::assert_last_event(Event::Approval { token, owner, spender, value }.into()); + // Approves a value to spend that is lower than the current allowance. + assert_eq!( + Fungibles::approve(signed(owner), token, spender, value / 2), + Ok(Some(WeightInfo::approve(1, 1)).into()) + ); + assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); + System::assert_last_event( + Event::Approval { token, owner, spender, value: value / 2 }.into(), + ); + // Approves a value to spend that is equal to the current allowance. + assert_eq!( + Fungibles::approve(signed(owner), token, spender, value / 2), + Ok(Some(WeightInfo::approve(0, 0)).into()) + ); + assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); + System::assert_last_event( + Event::Approval { token, owner, spender, value: value / 2 }.into(), + ); + // Sets allowance to zero. + assert_eq!( + Fungibles::approve(signed(owner), token, spender, 0), + Ok(Some(WeightInfo::approve(0, 1)).into()) + ); + assert_eq!(Assets::allowance(token, &owner, &spender), 0); + System::assert_last_event(Event::Approval { token, owner, spender, value: 0 }.into()); + }); + } } #[test] @@ -183,18 +233,18 @@ fn increase_allowance_works() { let owner = ALICE; let spender = BOB; - // Check error works for `Assets::approve_transfer()`. - assert_noop!( - Fungibles::increase_allowance(signed(owner), token, spender, value), - AssetsError::Unknown.with_weight(AssetsWeightInfo::approve_transfer()) - ); - pallet_assets_create_and_mint_to(owner, token, owner, value); for origin in vec![root(), none()] { assert_noop!( Fungibles::increase_allowance(origin, token, spender, value), BadOrigin.with_weight(WeightInfo::approve(0, 0)) ); } + // Check error works for `Assets::approve_transfer()`. + assert_noop!( + Fungibles::increase_allowance(signed(owner), token, spender, value), + AssetsError::Unknown.with_weight(AssetsWeightInfo::approve_transfer()) + ); + pallet_assets_create_and_mint_to(owner, token, owner, value); assert_eq!(0, Assets::allowance(token, &owner, &spender)); assert_ok!(Fungibles::increase_allowance(signed(owner), token, spender, value)); assert_eq!(Assets::allowance(token, &owner, &spender), value); @@ -216,6 +266,12 @@ fn decrease_allowance_works() { let owner = ALICE; let spender = BOB; + for origin in vec![root(), none()] { + assert_noop!( + Fungibles::decrease_allowance(origin, token, spender, 0), + BadOrigin.with_weight(WeightInfo::approve(0, 0)) + ); + } // Check error works for `Assets::cancel_approval()`. No error test for `approve_transfer` // because it is not possible. assert_noop!( @@ -223,12 +279,6 @@ fn decrease_allowance_works() { AssetsError::Unknown.with_weight(WeightInfo::approve(0, 1)) ); pallet_assets_create_mint_and_approve(owner, token, owner, value, spender, value); - for origin in vec![root(), none()] { - assert_noop!( - Fungibles::decrease_allowance(origin, token, spender, 0), - BadOrigin.with_weight(WeightInfo::approve(0, 0)) - ); - } assert_eq!(Assets::allowance(token, &owner, &spender), value); // Owner balance is not changed if decreased by zero. assert_eq!( From 497b2f74afb485cddb136fc75a60a4dbb613d820 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Fri, 13 Sep 2024 16:09:48 +0200 Subject: [PATCH 23/29] refactor: assets helper functions --- pallets/api/src/fungibles/tests.rs | 123 ++++++++++++----------------- 1 file changed, 51 insertions(+), 72 deletions(-) diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index 54944d24..747785c1 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -83,7 +83,7 @@ fn transfer_works() { } // Check error works for `Assets::transfer_keep_alive()`. assert_noop!(Fungibles::transfer(signed(from), token, to, value), AssetsError::Unknown); - pallet_assets_create_and_mint_to(from, token, from, value * 2); + assets::create_and_mint_to(from, token, from, value * 2); let balance_before_transfer = Assets::balance(token, &to); assert_ok!(Fungibles::transfer(signed(from), token, to, value)); let balance_after_transfer = Assets::balance(token, &to); @@ -112,7 +112,7 @@ fn transfer_from_works() { AssetsError::Unknown ); // Approve `spender` to transfer up to `value`. - pallet_assets_create_mint_and_approve(spender, token, from, value * 2, spender, value); + assets::create_mint_and_approve(spender, token, from, value * 2, spender, value); // Successfully call transfer from. let from_balance_before_transfer = Assets::balance(token, &from); let to_balance_before_transfer = Assets::balance(token, &to); @@ -166,14 +166,14 @@ mod approve { Fungibles::approve(signed(owner), token, spender, value), AssetsError::Unknown.with_weight(WeightInfo::approve(1, 0)) ); - pallet_assets_create_mint_and_approve(owner, token, owner, value, spender, value); + assets::create_mint_and_approve(owner, token, owner, value, spender, value); // Check error works for `Assets::cancel_approval()` in `Less` match arm. - pallet_assets_freeze_asset(owner, token); + assert_ok!(Assets::freeze_asset(signed(owner), token)); assert_noop!( Fungibles::approve(signed(owner), token, spender, value / 2), AssetsError::AssetNotLive.with_weight(WeightInfo::approve(0, 1)) ); - pallet_assets_thaw_asset(owner, token); + assert_ok!(Assets::thaw_asset(signed(owner), token)); // No error test for `approve_transfer` in `Less` arm because it is not possible. }); } @@ -188,7 +188,7 @@ mod approve { let spender = BOB; // Approves a value to spend that is higher than the current allowance. - pallet_assets_create_and_mint_to(owner, token, owner, value); + assets::create_and_mint_to(owner, token, owner, value); assert_eq!(Assets::allowance(token, &owner, &spender), 0); assert_eq!( Fungibles::approve(signed(owner), token, spender, value), @@ -244,7 +244,7 @@ fn increase_allowance_works() { Fungibles::increase_allowance(signed(owner), token, spender, value), AssetsError::Unknown.with_weight(AssetsWeightInfo::approve_transfer()) ); - pallet_assets_create_and_mint_to(owner, token, owner, value); + assets::create_and_mint_to(owner, token, owner, value); assert_eq!(0, Assets::allowance(token, &owner, &spender)); assert_ok!(Fungibles::increase_allowance(signed(owner), token, spender, value)); assert_eq!(Assets::allowance(token, &owner, &spender), value); @@ -278,7 +278,7 @@ fn decrease_allowance_works() { Fungibles::decrease_allowance(signed(owner), token, spender, value / 2), AssetsError::Unknown.with_weight(WeightInfo::approve(0, 1)) ); - pallet_assets_create_mint_and_approve(owner, token, owner, value, spender, value); + assets::create_mint_and_approve(owner, token, owner, value, spender, value); assert_eq!(Assets::allowance(token, &owner, &spender), value); // Owner balance is not changed if decreased by zero. assert_eq!( @@ -331,7 +331,7 @@ fn start_destroy_works() { // Check error works for `Assets::start_destroy()`. assert_noop!(Fungibles::start_destroy(signed(ALICE), token), AssetsError::Unknown); - pallet_assets_create(ALICE, token); + assert_ok!(Assets::create(signed(ALICE), token, ALICE, 1)); assert_ok!(Fungibles::start_destroy(signed(ALICE), token)); // Check that the token is not live after starting the destroy process. assert_noop!( @@ -354,7 +354,7 @@ fn set_metadata_works() { Fungibles::set_metadata(signed(ALICE), token, name.clone(), symbol.clone(), decimals), AssetsError::Unknown ); - pallet_assets_create(ALICE, token); + assert_ok!(Assets::create(signed(ALICE), token, ALICE, 1)); assert_ok!(Fungibles::set_metadata( signed(ALICE), token, @@ -375,7 +375,7 @@ fn clear_metadata_works() { // Check error works for `Assets::clear_metadata()`. assert_noop!(Fungibles::clear_metadata(signed(ALICE), token), AssetsError::Unknown); - pallet_assets_create_and_set_metadata(ALICE, token, vec![42], vec![42], 42); + assets::create_and_set_metadata(ALICE, token, vec![42], vec![42], 42); assert_ok!(Fungibles::clear_metadata(signed(ALICE), token)); assert!(Assets::name(token).is_empty()); assert!(Assets::symbol(token).is_empty()); @@ -396,7 +396,7 @@ fn mint_works() { Fungibles::mint(signed(from), token, to, value), sp_runtime::TokenError::UnknownAsset ); - pallet_assets_create(from, token); + assert_ok!(Assets::create(signed(from), token, from, 1)); let balance_before_mint = Assets::balance(token, &to); assert_ok!(Fungibles::mint(signed(from), token, to, value)); let balance_after_mint = Assets::balance(token, &to); @@ -418,7 +418,7 @@ fn burn_works() { // Check error works for `Assets::burn()`. assert_noop!(Fungibles::burn(signed(owner), token, from, value), AssetsError::Unknown); - pallet_assets_create_and_mint_to(owner, token, from, total_supply); + assets::create_and_mint_to(owner, token, from, total_supply); assert_eq!(Assets::total_supply(TOKEN), total_supply); let balance_before_burn = Assets::balance(token, &from); assert_ok!(Fungibles::burn(signed(owner), token, from, value)); @@ -439,7 +439,7 @@ fn total_supply_works() { Fungibles::read(TotalSupply(TOKEN)), ReadResult::TotalSupply(Default::default()) ); - pallet_assets_create_and_mint_to(ALICE, TOKEN, ALICE, total_supply); + assets::create_and_mint_to(ALICE, TOKEN, ALICE, total_supply); assert_eq!(Fungibles::read(TotalSupply(TOKEN)), ReadResult::TotalSupply(total_supply)); assert_eq!( Fungibles::read(TotalSupply(TOKEN)).encode(), @@ -456,7 +456,7 @@ fn balance_of_works() { Fungibles::read(BalanceOf { token: TOKEN, owner: ALICE }), ReadResult::BalanceOf(Default::default()) ); - pallet_assets_create_and_mint_to(ALICE, TOKEN, ALICE, value); + assets::create_and_mint_to(ALICE, TOKEN, ALICE, value); assert_eq!( Fungibles::read(BalanceOf { token: TOKEN, owner: ALICE }), ReadResult::BalanceOf(value) @@ -476,7 +476,7 @@ fn allowance_works() { Fungibles::read(Allowance { token: TOKEN, owner: ALICE, spender: BOB }), ReadResult::Allowance(Default::default()) ); - pallet_assets_create_mint_and_approve(ALICE, TOKEN, ALICE, value * 2, BOB, value); + assets::create_mint_and_approve(ALICE, TOKEN, ALICE, value * 2, BOB, value); assert_eq!( Fungibles::read(Allowance { token: TOKEN, owner: ALICE, spender: BOB }), ReadResult::Allowance(value) @@ -503,7 +503,7 @@ fn token_metadata_works() { Fungibles::read(TokenDecimals(TOKEN)), ReadResult::TokenDecimals(Default::default()) ); - pallet_assets_create_and_set_metadata(ALICE, TOKEN, name.clone(), symbol.clone(), decimals); + assets::create_and_set_metadata(ALICE, TOKEN, name.clone(), symbol.clone(), decimals); assert_eq!(Fungibles::read(TokenName(TOKEN)), ReadResult::TokenName(name)); assert_eq!(Fungibles::read(TokenSymbol(TOKEN)), ReadResult::TokenSymbol(symbol)); assert_eq!(Fungibles::read(TokenDecimals(TOKEN)), ReadResult::TokenDecimals(decimals)); @@ -520,7 +520,7 @@ fn token_metadata_works() { fn token_exists_works() { new_test_ext().execute_with(|| { assert_eq!(Fungibles::read(TokenExists(TOKEN)), ReadResult::TokenExists(false)); - pallet_assets_create(ALICE, TOKEN); + assert_ok!(Assets::create(signed(ALICE), TOKEN, ALICE, 1)); assert_eq!(Fungibles::read(TokenExists(TOKEN)), ReadResult::TokenExists(true)); assert_eq!( Fungibles::read(TokenExists(TOKEN)).encode(), @@ -541,61 +541,40 @@ fn none() -> RuntimeOrigin { RuntimeOrigin::none() } -fn pallet_assets_create(owner: AccountId, token: TokenId) { - assert_ok!(Assets::create(signed(owner), token, owner, 1)); -} - -fn pallet_assets_mint(owner: AccountId, token: TokenId, to: AccountId, value: Balance) { - assert_ok!(Assets::mint(signed(owner), token, to, value)); -} - -fn pallet_assets_create_and_mint_to( - owner: AccountId, - token: TokenId, - to: AccountId, - value: Balance, -) { - pallet_assets_create(owner, token); - pallet_assets_mint(owner, token, to, value) -} - -fn pallet_assets_create_mint_and_approve( - owner: AccountId, - token: TokenId, - to: AccountId, - mint: Balance, - spender: AccountId, - approve: Balance, -) { - pallet_assets_create_and_mint_to(owner, token, to, mint); - assert_ok!(Assets::approve_transfer(signed(to), token, spender, approve,)); -} - -fn pallet_assets_create_and_set_metadata( - owner: AccountId, - token: TokenId, - name: Vec, - symbol: Vec, - decimals: u8, -) { - assert_ok!(Assets::create(signed(owner), token, owner, 100)); - pallet_assets_set_metadata(owner, token, name, symbol, decimals); -} +// Helper functions for interacting with pallet-assets. +mod assets { + use super::*; -fn pallet_assets_set_metadata( - owner: AccountId, - token: TokenId, - name: Vec, - symbol: Vec, - decimals: u8, -) { - assert_ok!(Assets::set_metadata(signed(owner), token, name, symbol, decimals)); -} + pub(super) fn create_and_mint_to( + owner: AccountId, + token: TokenId, + to: AccountId, + value: Balance, + ) { + assert_ok!(Assets::create(signed(owner), token, owner, 1)); + assert_ok!(Assets::mint(signed(owner), token, to, value)); + } -fn pallet_assets_freeze_asset(owner: AccountId, token: TokenId) { - assert_ok!(Assets::freeze_asset(signed(owner), token)); -} + pub(super) fn create_mint_and_approve( + owner: AccountId, + token: TokenId, + to: AccountId, + mint: Balance, + spender: AccountId, + approve: Balance, + ) { + create_and_mint_to(owner, token, to, mint); + assert_ok!(Assets::approve_transfer(signed(to), token, spender, approve,)); + } -fn pallet_assets_thaw_asset(owner: AccountId, token: TokenId) { - assert_ok!(Assets::thaw_asset(signed(owner), token)); + pub(super) fn create_and_set_metadata( + owner: AccountId, + token: TokenId, + name: Vec, + symbol: Vec, + decimals: u8, + ) { + assert_ok!(Assets::create(signed(owner), token, owner, 1)); + assert_ok!(Assets::set_metadata(signed(owner), token, name, symbol, decimals)); + } } From 65ad654ac727a123c9638c205ca020212fbb8fd8 Mon Sep 17 00:00:00 2001 From: chungquantin <56880684+chungquantin@users.noreply.github.com> Date: Fri, 13 Sep 2024 21:45:34 +0700 Subject: [PATCH 24/29] refactor: api event naming conventions --- pallets/api/src/fungibles/mod.rs | 42 +++++++++++++++++++----------- pallets/api/src/fungibles/tests.rs | 26 +++++++++--------- 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index e38a8980..58f78747 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -134,7 +134,7 @@ pub mod pallet { #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { /// Event emitted when allowance by `owner` to `spender` changes. - Approval { + Approved { /// The token. token: TokenIdOf, /// The owner providing the allowance. @@ -145,7 +145,7 @@ pub mod pallet { value: BalanceOf, }, /// Event emitted when a token transfer occurs. - Transfer { + Transferred { /// The token. token: TokenIdOf, /// The source of the transfer. `None` when minting. @@ -156,7 +156,7 @@ pub mod pallet { value: BalanceOf, }, /// Event emitted when a token is created. - Create { + Created { /// The token identifier. id: TokenIdOf, /// The creator of the token. @@ -189,7 +189,12 @@ pub mod pallet { T::Lookup::unlookup(to.clone()), value, )?; - Self::deposit_event(Event::Transfer { token, from: Some(from), to: Some(to), value }); + Self::deposit_event(Event::Transferred { + token, + from: Some(from), + to: Some(to), + value, + }); Ok(()) } @@ -217,7 +222,12 @@ pub mod pallet { T::Lookup::unlookup(to.clone()), value, )?; - Self::deposit_event(Event::Transfer { token, from: Some(from), to: Some(to), value }); + Self::deposit_event(Event::Transferred { + token, + from: Some(from), + to: Some(to), + value, + }); Ok(()) } @@ -278,7 +288,7 @@ pub mod pallet { } }, }; - Self::deposit_event(Event::Approval { token, owner, spender, value }); + Self::deposit_event(Event::Approved { token, owner, spender, value }); Ok(Some(weight).into()) } @@ -306,7 +316,7 @@ pub mod pallet { ) .map_err(|e| e.with_weight(AssetsWeightInfoOf::::approve_transfer()))?; let value = AssetsOf::::allowance(token.clone(), &owner, &spender); - Self::deposit_event(Event::Approval { token, owner, spender, value }); + Self::deposit_event(Event::Approved { token, owner, spender, value }); Ok(().into()) } @@ -352,7 +362,7 @@ pub mod pallet { )?; WeightOf::::approve(1, 1) }; - Self::deposit_event(Event::Approval { token, owner, spender, value: new_allowance }); + Self::deposit_event(Event::Approved { token, owner, spender, value: new_allowance }); Ok(Some(weight).into()) } @@ -377,7 +387,7 @@ pub mod pallet { T::Lookup::unlookup(admin.clone()), min_balance, )?; - Self::deposit_event(Event::Create { id, creator, admin }); + Self::deposit_event(Event::Created { id, creator, admin }); Ok(()) } @@ -445,7 +455,7 @@ pub mod pallet { T::Lookup::unlookup(account.clone()), value, )?; - Self::deposit_event(Event::Transfer { token, from: None, to: Some(account), value }); + Self::deposit_event(Event::Transferred { token, from: None, to: Some(account), value }); Ok(()) } @@ -469,7 +479,7 @@ pub mod pallet { T::Lookup::unlookup(account.clone()), value, )?; - Self::deposit_event(Event::Transfer { token, from: Some(account), to: None, value }); + Self::deposit_event(Event::Transferred { token, from: Some(account), to: None, value }); Ok(()) } } @@ -498,10 +508,12 @@ pub mod pallet { use Read::*; match request { TotalSupply(token) => ReadResult::TotalSupply(AssetsOf::::total_supply(token)), - BalanceOf { token, owner } => - ReadResult::BalanceOf(AssetsOf::::balance(token, owner)), - Allowance { token, owner, spender } => - ReadResult::Allowance(AssetsOf::::allowance(token, &owner, &spender)), + BalanceOf { token, owner } => { + ReadResult::BalanceOf(AssetsOf::::balance(token, owner)) + }, + Allowance { token, owner, spender } => { + ReadResult::Allowance(AssetsOf::::allowance(token, &owner, &spender)) + }, TokenName(token) => ReadResult::TokenName( as MetadataInspect< AccountIdOf, >>::name(token)), diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index 747785c1..1467446b 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -89,7 +89,7 @@ fn transfer_works() { let balance_after_transfer = Assets::balance(token, &to); assert_eq!(balance_after_transfer, balance_before_transfer + value); System::assert_last_event( - Event::Transfer { token, from: Some(from), to: Some(to), value }.into(), + Event::Transferred { token, from: Some(from), to: Some(to), value }.into(), ); }); } @@ -123,7 +123,7 @@ fn transfer_from_works() { assert_eq!(to_balance_after_transfer, to_balance_before_transfer + value); assert_eq!(from_balance_after_transfer, from_balance_before_transfer - value); System::assert_last_event( - Event::Transfer { token, from: Some(from), to: Some(to), value }.into(), + Event::Transferred { token, from: Some(from), to: Some(to), value }.into(), ); }); } @@ -195,7 +195,7 @@ mod approve { Ok(Some(WeightInfo::approve(1, 0)).into()) ); assert_eq!(Assets::allowance(token, &owner, &spender), value); - System::assert_last_event(Event::Approval { token, owner, spender, value }.into()); + System::assert_last_event(Event::Approved { token, owner, spender, value }.into()); // Approves a value to spend that is lower than the current allowance. assert_eq!( Fungibles::approve(signed(owner), token, spender, value / 2), @@ -203,7 +203,7 @@ mod approve { ); assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); System::assert_last_event( - Event::Approval { token, owner, spender, value: value / 2 }.into(), + Event::Approved { token, owner, spender, value: value / 2 }.into(), ); // Approves a value to spend that is equal to the current allowance. assert_eq!( @@ -212,7 +212,7 @@ mod approve { ); assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); System::assert_last_event( - Event::Approval { token, owner, spender, value: value / 2 }.into(), + Event::Approved { token, owner, spender, value: value / 2 }.into(), ); // Sets allowance to zero. assert_eq!( @@ -220,7 +220,7 @@ mod approve { Ok(Some(WeightInfo::approve(0, 1)).into()) ); assert_eq!(Assets::allowance(token, &owner, &spender), 0); - System::assert_last_event(Event::Approval { token, owner, spender, value: 0 }.into()); + System::assert_last_event(Event::Approved { token, owner, spender, value: 0 }.into()); }); } } @@ -248,12 +248,12 @@ fn increase_allowance_works() { assert_eq!(0, Assets::allowance(token, &owner, &spender)); assert_ok!(Fungibles::increase_allowance(signed(owner), token, spender, value)); assert_eq!(Assets::allowance(token, &owner, &spender), value); - System::assert_last_event(Event::Approval { token, owner, spender, value }.into()); + System::assert_last_event(Event::Approved { token, owner, spender, value }.into()); // Additive. assert_ok!(Fungibles::increase_allowance(signed(owner), token, spender, value)); assert_eq!(Assets::allowance(token, &owner, &spender), value * 2); System::assert_last_event( - Event::Approval { token, owner, spender, value: value * 2 }.into(), + Event::Approved { token, owner, spender, value: value * 2 }.into(), ); }); } @@ -293,7 +293,7 @@ fn decrease_allowance_works() { ); assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); System::assert_last_event( - Event::Approval { token, owner, spender, value: value / 2 }.into(), + Event::Approved { token, owner, spender, value: value / 2 }.into(), ); // Saturating if current allowance is decreased more than the owner balance. assert_eq!( @@ -301,7 +301,7 @@ fn decrease_allowance_works() { Ok(Some(WeightInfo::approve(0, 1)).into()) ); assert_eq!(Assets::allowance(token, &owner, &spender), 0); - System::assert_last_event(Event::Approval { token, owner, spender, value: 0 }.into()); + System::assert_last_event(Event::Approved { token, owner, spender, value: 0 }.into()); }); } @@ -318,7 +318,7 @@ fn create_works() { assert!(!Assets::asset_exists(id)); assert_ok!(Fungibles::create(signed(creator), id, admin, 100)); assert!(Assets::asset_exists(id)); - System::assert_last_event(Event::Create { id, creator, admin }.into()); + System::assert_last_event(Event::Created { id, creator, admin }.into()); // Check error works for `Assets::create()`. assert_noop!(Fungibles::create(signed(creator), id, admin, 100), AssetsError::InUse); }); @@ -402,7 +402,7 @@ fn mint_works() { let balance_after_mint = Assets::balance(token, &to); assert_eq!(balance_after_mint, balance_before_mint + value); System::assert_last_event( - Event::Transfer { token, from: None, to: Some(to), value }.into(), + Event::Transferred { token, from: None, to: Some(to), value }.into(), ); }); } @@ -426,7 +426,7 @@ fn burn_works() { let balance_after_burn = Assets::balance(token, &from); assert_eq!(balance_after_burn, balance_before_burn - value); System::assert_last_event( - Event::Transfer { token, from: Some(from), to: None, value }.into(), + Event::Transferred { token, from: Some(from), to: None, value }.into(), ); }); } From 61631320d21cd0769b4791323cfb8acbd6bfb358 Mon Sep 17 00:00:00 2001 From: chungquantin <56880684+chungquantin@users.noreply.github.com> Date: Fri, 13 Sep 2024 21:53:11 +0700 Subject: [PATCH 25/29] fix: formatting --- pallets/api/src/fungibles/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 58f78747..2d9c9281 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -508,12 +508,10 @@ pub mod pallet { use Read::*; match request { TotalSupply(token) => ReadResult::TotalSupply(AssetsOf::::total_supply(token)), - BalanceOf { token, owner } => { - ReadResult::BalanceOf(AssetsOf::::balance(token, owner)) - }, - Allowance { token, owner, spender } => { - ReadResult::Allowance(AssetsOf::::allowance(token, &owner, &spender)) - }, + BalanceOf { token, owner } => + ReadResult::BalanceOf(AssetsOf::::balance(token, owner)), + Allowance { token, owner, spender } => + ReadResult::Allowance(AssetsOf::::allowance(token, &owner, &spender)), TokenName(token) => ReadResult::TokenName( as MetadataInspect< AccountIdOf, >>::name(token)), From 9b7a62dc749aca7effdc138e9b7d479665b461b0 Mon Sep 17 00:00:00 2001 From: chungquantin <56880684+chungquantin@users.noreply.github.com> Date: Fri, 13 Sep 2024 22:37:56 +0700 Subject: [PATCH 26/29] fix: event naming --- pallets/api/src/fungibles/mod.rs | 38 ++++++++++++------------------ pallets/api/src/fungibles/tests.rs | 24 +++++++++---------- 2 files changed, 27 insertions(+), 35 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 2d9c9281..de9b6fac 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -134,7 +134,7 @@ pub mod pallet { #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { /// Event emitted when allowance by `owner` to `spender` changes. - Approved { + Approve { /// The token. token: TokenIdOf, /// The owner providing the allowance. @@ -145,7 +145,7 @@ pub mod pallet { value: BalanceOf, }, /// Event emitted when a token transfer occurs. - Transferred { + Transfer { /// The token. token: TokenIdOf, /// The source of the transfer. `None` when minting. @@ -189,12 +189,7 @@ pub mod pallet { T::Lookup::unlookup(to.clone()), value, )?; - Self::deposit_event(Event::Transferred { - token, - from: Some(from), - to: Some(to), - value, - }); + Self::deposit_event(Event::Transfer { token, from: Some(from), to: Some(to), value }); Ok(()) } @@ -222,12 +217,7 @@ pub mod pallet { T::Lookup::unlookup(to.clone()), value, )?; - Self::deposit_event(Event::Transferred { - token, - from: Some(from), - to: Some(to), - value, - }); + Self::deposit_event(Event::Transfer { token, from: Some(from), to: Some(to), value }); Ok(()) } @@ -288,7 +278,7 @@ pub mod pallet { } }, }; - Self::deposit_event(Event::Approved { token, owner, spender, value }); + Self::deposit_event(Event::Approve { token, owner, spender, value }); Ok(Some(weight).into()) } @@ -316,7 +306,7 @@ pub mod pallet { ) .map_err(|e| e.with_weight(AssetsWeightInfoOf::::approve_transfer()))?; let value = AssetsOf::::allowance(token.clone(), &owner, &spender); - Self::deposit_event(Event::Approved { token, owner, spender, value }); + Self::deposit_event(Event::Approve { token, owner, spender, value }); Ok(().into()) } @@ -362,7 +352,7 @@ pub mod pallet { )?; WeightOf::::approve(1, 1) }; - Self::deposit_event(Event::Approved { token, owner, spender, value: new_allowance }); + Self::deposit_event(Event::Approve { token, owner, spender, value: new_allowance }); Ok(Some(weight).into()) } @@ -455,7 +445,7 @@ pub mod pallet { T::Lookup::unlookup(account.clone()), value, )?; - Self::deposit_event(Event::Transferred { token, from: None, to: Some(account), value }); + Self::deposit_event(Event::Transfer { token, from: None, to: Some(account), value }); Ok(()) } @@ -479,7 +469,7 @@ pub mod pallet { T::Lookup::unlookup(account.clone()), value, )?; - Self::deposit_event(Event::Transferred { token, from: Some(account), to: None, value }); + Self::deposit_event(Event::Transfer { token, from: Some(account), to: None, value }); Ok(()) } } @@ -508,10 +498,12 @@ pub mod pallet { use Read::*; match request { TotalSupply(token) => ReadResult::TotalSupply(AssetsOf::::total_supply(token)), - BalanceOf { token, owner } => - ReadResult::BalanceOf(AssetsOf::::balance(token, owner)), - Allowance { token, owner, spender } => - ReadResult::Allowance(AssetsOf::::allowance(token, &owner, &spender)), + BalanceOf { token, owner } => { + ReadResult::BalanceOf(AssetsOf::::balance(token, owner)) + }, + Allowance { token, owner, spender } => { + ReadResult::Allowance(AssetsOf::::allowance(token, &owner, &spender)) + }, TokenName(token) => ReadResult::TokenName( as MetadataInspect< AccountIdOf, >>::name(token)), diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index 1467446b..92a3c8ed 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -89,7 +89,7 @@ fn transfer_works() { let balance_after_transfer = Assets::balance(token, &to); assert_eq!(balance_after_transfer, balance_before_transfer + value); System::assert_last_event( - Event::Transferred { token, from: Some(from), to: Some(to), value }.into(), + Event::Transfer { token, from: Some(from), to: Some(to), value }.into(), ); }); } @@ -123,7 +123,7 @@ fn transfer_from_works() { assert_eq!(to_balance_after_transfer, to_balance_before_transfer + value); assert_eq!(from_balance_after_transfer, from_balance_before_transfer - value); System::assert_last_event( - Event::Transferred { token, from: Some(from), to: Some(to), value }.into(), + Event::Transfer { token, from: Some(from), to: Some(to), value }.into(), ); }); } @@ -195,7 +195,7 @@ mod approve { Ok(Some(WeightInfo::approve(1, 0)).into()) ); assert_eq!(Assets::allowance(token, &owner, &spender), value); - System::assert_last_event(Event::Approved { token, owner, spender, value }.into()); + System::assert_last_event(Event::Approve { token, owner, spender, value }.into()); // Approves a value to spend that is lower than the current allowance. assert_eq!( Fungibles::approve(signed(owner), token, spender, value / 2), @@ -203,7 +203,7 @@ mod approve { ); assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); System::assert_last_event( - Event::Approved { token, owner, spender, value: value / 2 }.into(), + Event::Approve { token, owner, spender, value: value / 2 }.into(), ); // Approves a value to spend that is equal to the current allowance. assert_eq!( @@ -212,7 +212,7 @@ mod approve { ); assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); System::assert_last_event( - Event::Approved { token, owner, spender, value: value / 2 }.into(), + Event::Approve { token, owner, spender, value: value / 2 }.into(), ); // Sets allowance to zero. assert_eq!( @@ -220,7 +220,7 @@ mod approve { Ok(Some(WeightInfo::approve(0, 1)).into()) ); assert_eq!(Assets::allowance(token, &owner, &spender), 0); - System::assert_last_event(Event::Approved { token, owner, spender, value: 0 }.into()); + System::assert_last_event(Event::Approve { token, owner, spender, value: 0 }.into()); }); } } @@ -248,12 +248,12 @@ fn increase_allowance_works() { assert_eq!(0, Assets::allowance(token, &owner, &spender)); assert_ok!(Fungibles::increase_allowance(signed(owner), token, spender, value)); assert_eq!(Assets::allowance(token, &owner, &spender), value); - System::assert_last_event(Event::Approved { token, owner, spender, value }.into()); + System::assert_last_event(Event::Approve { token, owner, spender, value }.into()); // Additive. assert_ok!(Fungibles::increase_allowance(signed(owner), token, spender, value)); assert_eq!(Assets::allowance(token, &owner, &spender), value * 2); System::assert_last_event( - Event::Approved { token, owner, spender, value: value * 2 }.into(), + Event::Approve { token, owner, spender, value: value * 2 }.into(), ); }); } @@ -293,7 +293,7 @@ fn decrease_allowance_works() { ); assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); System::assert_last_event( - Event::Approved { token, owner, spender, value: value / 2 }.into(), + Event::Approve { token, owner, spender, value: value / 2 }.into(), ); // Saturating if current allowance is decreased more than the owner balance. assert_eq!( @@ -301,7 +301,7 @@ fn decrease_allowance_works() { Ok(Some(WeightInfo::approve(0, 1)).into()) ); assert_eq!(Assets::allowance(token, &owner, &spender), 0); - System::assert_last_event(Event::Approved { token, owner, spender, value: 0 }.into()); + System::assert_last_event(Event::Approve { token, owner, spender, value: 0 }.into()); }); } @@ -402,7 +402,7 @@ fn mint_works() { let balance_after_mint = Assets::balance(token, &to); assert_eq!(balance_after_mint, balance_before_mint + value); System::assert_last_event( - Event::Transferred { token, from: None, to: Some(to), value }.into(), + Event::Transfer { token, from: None, to: Some(to), value }.into(), ); }); } @@ -426,7 +426,7 @@ fn burn_works() { let balance_after_burn = Assets::balance(token, &from); assert_eq!(balance_after_burn, balance_before_burn - value); System::assert_last_event( - Event::Transferred { token, from: Some(from), to: None, value }.into(), + Event::Transfer { token, from: Some(from), to: None, value }.into(), ); }); } From b38aedea8f617db04537562dad4ff7b82faf15fb Mon Sep 17 00:00:00 2001 From: chungquantin <56880684+chungquantin@users.noreply.github.com> Date: Fri, 13 Sep 2024 22:49:07 +0700 Subject: [PATCH 27/29] fix: formatting --- pallets/api/src/fungibles/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index de9b6fac..c045fb84 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -498,12 +498,10 @@ pub mod pallet { use Read::*; match request { TotalSupply(token) => ReadResult::TotalSupply(AssetsOf::::total_supply(token)), - BalanceOf { token, owner } => { - ReadResult::BalanceOf(AssetsOf::::balance(token, owner)) - }, - Allowance { token, owner, spender } => { - ReadResult::Allowance(AssetsOf::::allowance(token, &owner, &spender)) - }, + BalanceOf { token, owner } => + ReadResult::BalanceOf(AssetsOf::::balance(token, owner)), + Allowance { token, owner, spender } => + ReadResult::Allowance(AssetsOf::::allowance(token, &owner, &spender)), TokenName(token) => ReadResult::TokenName( as MetadataInspect< AccountIdOf, >>::name(token)), From 35bedb6296ee5b025ad5ffa6e8fc9b326c00be5e Mon Sep 17 00:00:00 2001 From: Peter White Date: Fri, 13 Sep 2024 11:59:34 -0600 Subject: [PATCH 28/29] refactor: event Approve -> Approval --- pallets/api/src/fungibles/mod.rs | 8 ++++---- pallets/api/src/fungibles/tests.rs | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index c045fb84..bdfe30db 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -134,7 +134,7 @@ pub mod pallet { #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { /// Event emitted when allowance by `owner` to `spender` changes. - Approve { + Approval { /// The token. token: TokenIdOf, /// The owner providing the allowance. @@ -278,7 +278,7 @@ pub mod pallet { } }, }; - Self::deposit_event(Event::Approve { token, owner, spender, value }); + Self::deposit_event(Event::Approval { token, owner, spender, value }); Ok(Some(weight).into()) } @@ -306,7 +306,7 @@ pub mod pallet { ) .map_err(|e| e.with_weight(AssetsWeightInfoOf::::approve_transfer()))?; let value = AssetsOf::::allowance(token.clone(), &owner, &spender); - Self::deposit_event(Event::Approve { token, owner, spender, value }); + Self::deposit_event(Event::Approval { token, owner, spender, value }); Ok(().into()) } @@ -352,7 +352,7 @@ pub mod pallet { )?; WeightOf::::approve(1, 1) }; - Self::deposit_event(Event::Approve { token, owner, spender, value: new_allowance }); + Self::deposit_event(Event::Approval { token, owner, spender, value: new_allowance }); Ok(Some(weight).into()) } diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index 92a3c8ed..b1913cec 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -195,7 +195,7 @@ mod approve { Ok(Some(WeightInfo::approve(1, 0)).into()) ); assert_eq!(Assets::allowance(token, &owner, &spender), value); - System::assert_last_event(Event::Approve { token, owner, spender, value }.into()); + System::assert_last_event(Event::Approval { token, owner, spender, value }.into()); // Approves a value to spend that is lower than the current allowance. assert_eq!( Fungibles::approve(signed(owner), token, spender, value / 2), @@ -203,7 +203,7 @@ mod approve { ); assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); System::assert_last_event( - Event::Approve { token, owner, spender, value: value / 2 }.into(), + Event::Approval { token, owner, spender, value: value / 2 }.into(), ); // Approves a value to spend that is equal to the current allowance. assert_eq!( @@ -212,7 +212,7 @@ mod approve { ); assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); System::assert_last_event( - Event::Approve { token, owner, spender, value: value / 2 }.into(), + Event::Approval { token, owner, spender, value: value / 2 }.into(), ); // Sets allowance to zero. assert_eq!( @@ -220,7 +220,7 @@ mod approve { Ok(Some(WeightInfo::approve(0, 1)).into()) ); assert_eq!(Assets::allowance(token, &owner, &spender), 0); - System::assert_last_event(Event::Approve { token, owner, spender, value: 0 }.into()); + System::assert_last_event(Event::Approval { token, owner, spender, value: 0 }.into()); }); } } @@ -248,12 +248,12 @@ fn increase_allowance_works() { assert_eq!(0, Assets::allowance(token, &owner, &spender)); assert_ok!(Fungibles::increase_allowance(signed(owner), token, spender, value)); assert_eq!(Assets::allowance(token, &owner, &spender), value); - System::assert_last_event(Event::Approve { token, owner, spender, value }.into()); + System::assert_last_event(Event::Approval { token, owner, spender, value }.into()); // Additive. assert_ok!(Fungibles::increase_allowance(signed(owner), token, spender, value)); assert_eq!(Assets::allowance(token, &owner, &spender), value * 2); System::assert_last_event( - Event::Approve { token, owner, spender, value: value * 2 }.into(), + Event::Approval { token, owner, spender, value: value * 2 }.into(), ); }); } @@ -293,7 +293,7 @@ fn decrease_allowance_works() { ); assert_eq!(Assets::allowance(token, &owner, &spender), value / 2); System::assert_last_event( - Event::Approve { token, owner, spender, value: value / 2 }.into(), + Event::Approval { token, owner, spender, value: value / 2 }.into(), ); // Saturating if current allowance is decreased more than the owner balance. assert_eq!( @@ -301,7 +301,7 @@ fn decrease_allowance_works() { Ok(Some(WeightInfo::approve(0, 1)).into()) ); assert_eq!(Assets::allowance(token, &owner, &spender), 0); - System::assert_last_event(Event::Approve { token, owner, spender, value: 0 }.into()); + System::assert_last_event(Event::Approval { token, owner, spender, value: 0 }.into()); }); } From 3df17e1ff8fab67bcdf665c2f427276e35465e43 Mon Sep 17 00:00:00 2001 From: Peter White Date: Fri, 13 Sep 2024 12:05:42 -0600 Subject: [PATCH 29/29] docs: add comment for differing event names --- pallets/api/src/fungibles/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index bdfe30db..c28cf88a 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -134,6 +134,7 @@ pub mod pallet { #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { /// Event emitted when allowance by `owner` to `spender` changes. + // Differing style: event name abides by the PSP22 standard. Approval { /// The token. token: TokenIdOf, @@ -145,6 +146,7 @@ pub mod pallet { value: BalanceOf, }, /// Event emitted when a token transfer occurs. + // Differing style: event name abides by the PSP22 standard. Transfer { /// The token. token: TokenIdOf,