chore: restructure pallet part 1, unit tests and internal mockup

Cargo.toml

@@ -36,11 +36,8 @@ move-vm-backend-common = { default-features = false, git = "https://github.com/e
 [dev-dependencies]
 hex = "0.4"
 rand = "0.8"
-lazy_static = "1.4"
 move-stdlib = { git = "https://github.com/eigerco/substrate-move.git", features = ["stdlib-bytecode"] }
-sp-core = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = "release-polkadot-v1.4.0" }
 sp-io = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = "release-polkadot-v1.4.0" }
-sp-runtime = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = "release-polkadot-v1.4.0" }
 pallet-balances = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = "release-polkadot-v1.4.0" }
 
 [features]
@@ -49,7 +46,7 @@ std = [
     "anyhow/std",
     "blake2/std",
     "codec/std",
-    "frame-benchmarking?/std",
+    "frame-benchmarking/std",
     "frame-support/std",
     "frame-system/std",
     "scale-info/std",
@@ -58,8 +55,17 @@ std = [
     "move-core-types/std",
     "move-vm-backend/std",
 ]
-runtime-benchmarks = ["frame-benchmarking/runtime-benchmarks"]
-try-runtime = ["frame-support/try-runtime"]
+runtime-benchmarks = [
+    "frame-benchmarking/runtime-benchmarks",
+    "frame-support/runtime-benchmarks",
+    "frame-system/runtime-benchmarks",
+    "sp-runtime/runtime-benchmarks",
+]
+try-runtime = [
+    "frame-support/try-runtime",
+    "frame-system/try-runtime",
+    "sp-runtime/try-runtime",
+]
 
 # Builds move projects for test purposes.
 #

build.rs

@@ -16,7 +16,7 @@ fn build_move_projects() -> Result<(), Box<dyn Error>> {
     println!("cargo:warning=Building move projects in tests/assets folder");
 
     let smove_run = Command::new("bash")
-        .args(["tests/assets/move-projects/smove-build-all.sh"])
+        .args(["src/tests/assets/move-projects/smove-build-all.sh"])
         .output()
         .expect("failed to execute script which builds necessary move modules");
 

src/benchmarking.rs

@@ -1,73 +1,42 @@
-//! Benchmarking setup for pallet-move
+//! Benchmarking setup for pallet-move.
+#![cfg(feature = "runtime-benchmarks")]
 
-use frame_benchmarking::v2::*;
+use frame_benchmarking::*;
 use frame_system::{Config as SysConfig, RawOrigin};
-use sp_runtime::traits::Zero;
+use sp_runtime::{traits::Zero, AccountId32};
 use sp_std::vec;
 
-use crate::{balance::BalanceOf, Config, *};
+use crate::{balance::BalanceOf, mock::*, *};
 
-#[benchmarks(
-    where
+benchmarks! {
+    where_clause { where
         T: Config + SysConfig,
         T::AccountId: From<AccountId32>,
         BalanceOf<T>: From<u128> + Into<u128>,
-)]
-mod benchmarks {
-    use sp_core::{crypto::Ss58Codec, sr25519::Public};
-    use sp_runtime::AccountId32;
-
-    use super::*;
-
-    const BOB_ADDR: &str = "5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty";
-
-    fn addr32_from_ss58(ss58addr: &str) -> AccountId32 {
-        let (pk, _) = Public::from_ss58check_with_version(ss58addr).unwrap();
-        pk.into()
-    }
-
-    #[benchmark]
-    fn execute() {
-        let caller: T::AccountId = addr32_from_ss58(BOB_ADDR).into();
-        let module =
-            include_bytes!("../tests/assets/move-projects/move-basics/build/move-basics/bytecode_scripts/empty_scr.mv")
-                .to_vec();
-        let transaction = ScriptTransaction {
-            bytecode: module,
-            type_args: vec![],
-            args: vec![],
-        };
-        let transaction_bc = bcs::to_bytes(&transaction).unwrap();
-        #[extrinsic_call]
-        execute(
-            RawOrigin::Signed(caller),
-            transaction_bc,
-            100_000,
-            BalanceOf::<T>::zero(),
-        );
     }
 
-    #[benchmark]
-    fn publish_module() {
-        let caller: T::AccountId = addr32_from_ss58(BOB_ADDR).into();
-        let module = include_bytes!(
-            "../tests/assets/move-projects/using_stdlib_natives/build/using_stdlib_natives/bytecode_modules/Vector.mv"
-        )
-        .to_vec();
-        #[extrinsic_call]
-        publish_module(RawOrigin::Signed(caller), module, 500_000);
-    }
-
-    #[benchmark]
-    fn publish_module_bundle() {
-        let caller: T::AccountId = addr32_from_ss58(BOB_ADDR).into();
-        let module = include_bytes!(
-            "../tests/assets/move-projects/using_stdlib_natives/build/using_stdlib_natives/bundles/using_stdlib_natives.mvb"
-        )
-        .to_vec();
-        #[extrinsic_call]
-        publish_module_bundle(RawOrigin::Signed(caller), module, 1_500_000);
-    }
-
-    impl_benchmark_test_suite!(MovePallet, crate::mock::new_test_ext(), crate::mock::Test);
+    execute {
+        let caller: T::AccountId = addr32_from_ss58(BOB_ADDR).unwrap().into();
+        let script = assets::read_script_from_project("move-basics", "empty_scr");
+        let transaction_bc = script_transaction!(script, no_type_args!());
+    }: _(RawOrigin::Signed(caller), transaction_bc, 100_000, BalanceOf::<T>::zero())
+    verify {}
+
+    publish_module {
+        let caller: T::AccountId = addr32_from_ss58(BOB_ADDR).unwrap().into();
+        let module = assets::read_module_from_project("using_stdlib_natives", "Vector");
+    }: _(RawOrigin::Signed(caller), module, 500_000)
+    verify {}
+
+    publish_module_bundle {
+        let caller: T::AccountId = addr32_from_ss58(BOB_ADDR).unwrap().into();
+        let bundle = assets::read_bundle_from_project("using_stdlib_natives", "using_stdlib_natives");
+    }: _(RawOrigin::Signed(caller), bundle, 1_500_000)
+    verify {}
 }
+
+impl_benchmark_test_suite!(
+    Pallet,
+    crate::mock::ExtBuilder::default().build(),
+    crate::mock::Test
+);

src/lib.rs

@@ -1,18 +1,20 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
-pub use crate::pallet::*;
-pub use crate::weights::*;
-
-#[cfg(feature = "runtime-benchmarks")]
-mod benchmarking;
-
 pub mod balance;
+#[cfg(all(test, feature = "runtime-benchmarks"))]
+mod benchmarking;
+#[cfg(test)]
+mod mock;
+mod result;
 mod signer;
 mod storage;
-
-mod result;
+#[cfg(test)]
+mod tests;
 pub mod weights;
 
+pub use pallet::*;
+pub use weights::*;
+
 // The pallet is defined below.
 #[frame_support::pallet]
 pub mod pallet {

tests/mock.rs → src/mock.rs

@@ -2,15 +2,31 @@ use frame_support::{
     parameter_types,
     traits::{ConstU128, ConstU16, ConstU32, ConstU64},
 };
-use move_core_types::account_address::AccountAddress;
 use sp_core::{crypto::Ss58Codec, sr25519::Public, H256};
 use sp_runtime::{
     traits::{BlakeTwo256, IdentityLookup},
-    AccountId32, BuildStorage,
+    BuildStorage,
 };
 
+use crate as pallet_move;
+use crate::Error;
+
+pub use move_core_types::account_address::AccountAddress;
+pub use move_vm_backend_common::types::ScriptTransaction;
+pub use sp_runtime::AccountId32;
+
 type Block = frame_system::mocking::MockBlock<Test>;
 
+// Configure a mock runtime to test the pallet.
+frame_support::construct_runtime!(
+    pub enum Test
+    {
+        System: frame_system,
+        Balances: pallet_balances,
+        MoveModule: pallet_move,
+    }
+);
+
 impl frame_system::Config for Test {
     type BaseCallFilter = frame_support::traits::Everything;
     type BlockWeights = ();
@@ -74,7 +90,6 @@ impl pallet_move::Config for Test {
 }
 
 /// Test Externalities Builder for an easier test setup.
-#[allow(dead_code)]
 #[derive(Default)]
 pub(crate) struct ExtBuilder {
     /// Overwrite default accounts with balances.
@@ -85,7 +100,6 @@ pub(crate) struct ExtBuilder {
     substrate_stdlib: Option<Vec<u8>>,
 }
 
-#[allow(dead_code)]
 impl ExtBuilder {
     /// Overwrites default balances on dev-test setup.
     pub(crate) fn with_balances(mut self, balances: Vec<(AccountId32, Balance)>) -> Self {
@@ -128,87 +142,78 @@ impl ExtBuilder {
     }
 }
 
-// Build genesis storage according to the mock runtime.
-#[allow(dead_code)]
-pub fn new_test_ext() -> sp_io::TestExternalities {
-    let mut storage = frame_system::GenesisConfig::<Test>::default()
-        .build_storage()
-        .unwrap();
-
-    let pallet_move_config = pallet_move::GenesisConfig::<Test> {
-        _phantom: core::marker::PhantomData,
-        change_default_move_stdlib_bundle_to: None,
-        change_default_substrate_stdlib_bundle_to: None,
-    };
-
-    pallet_move_config.assimilate_storage(&mut storage).unwrap();
-
-    storage.into()
-}
-
 // Common constants accross the tests.
-#[allow(dead_code)]
 pub const EMPTY_CHEQUE: u128 = 0; // Not all scripts need the `cheque_amount` parameter.
-pub const CAFE_ADDR: &str = "0xCAFE";
+pub const CAFE_ADDR: &str = "5C4hrfjw9DjXZTzV3MwzrrAr9P1MJhSrvWGWqi1eSv4fmh4G"; // == 0xCAFE
 pub const BOB_ADDR: &str = "5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty";
 pub const ALICE_ADDR: &str = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY";
 pub const DAVE_ADDR: &str = "5DAAnrj7VHTznn2AWBemMuyBwZWs6FNFjdyVXUeYum3PTXFy";
-lazy_static::lazy_static! {
-    pub static ref CAFE_ADDR_MOVE: AccountAddress = {
-        AccountAddress::from_hex_literal(CAFE_ADDR).unwrap()
-    };
-    pub static ref CAFE_ADDR_NATIVE: AccountId32 = {
-        MoveModule::to_native_account(&CAFE_ADDR_MOVE).unwrap()
-    };
-    pub static ref BOB_ADDR_NATIVE: AccountId32 = {
-        let (pk, _) = Public::from_ss58check_with_version(BOB_ADDR).unwrap();
-        pk.into()
-    };
-    pub static ref BOB_ADDR_MOVE: AccountAddress = {
-        MoveModule::to_move_address(&BOB_ADDR_NATIVE).unwrap()
-    };
-    pub static ref ALICE_ADDR_NATIVE: AccountId32 = {
-        let (pk, _) = Public::from_ss58check_with_version(ALICE_ADDR).unwrap();
-        pk.into()
-    };
-    pub static ref ALICE_ADDR_MOVE: AccountAddress = {
-        MoveModule::to_move_address(&ALICE_ADDR_NATIVE).unwrap()
-    };
-    pub static ref DAVE_ADDR_NATIVE: AccountId32 = {
-        let (pk, _) = Public::from_ss58check_with_version(DAVE_ADDR).unwrap();
-        pk.into()
-    };
-    pub static ref DAVE_ADDR_MOVE: AccountAddress = {
-        MoveModule::to_move_address(&DAVE_ADDR_NATIVE).unwrap()
-    };
-}
 
-#[allow(dead_code)]
-pub fn addr32_from_ss58(ss58addr: &str) -> AccountId32 {
-    let (pk, _) = Public::from_ss58check_with_version(ss58addr).unwrap();
-    pk.into()
+pub fn addr32_from_ss58(ss58addr: &str) -> Result<AccountId32, Error<Test>> {
+    let (pk, _) = Public::from_ss58check_with_version(ss58addr)
+        .map_err(|_| Error::<Test>::InvalidAccountSize)?;
+    let account: AccountId32 = pk.into();
+    Ok(account)
 }
 
-#[allow(dead_code)]
-pub fn addr32_to_move(addr32: &AccountId32) -> Result<AccountAddress, pallet_move::Error<Test>> {
+pub fn addr32_to_move(addr32: &AccountId32) -> Result<AccountAddress, Error<Test>> {
     MoveModule::to_move_address(addr32)
 }
 
-#[allow(dead_code)]
-pub fn addrs_from_ss58(
-    ss58: &str,
-) -> Result<(AccountId32, AccountAddress), pallet_move::Error<Test>> {
-    let addr_32 = addr32_from_ss58(ss58);
+pub fn addrs_from_ss58(ss58: &str) -> Result<(AccountId32, AccountAddress), Error<Test>> {
+    let addr_32 = addr32_from_ss58(ss58)?;
     let addr_mv = addr32_to_move(&addr_32)?;
     Ok((addr_32, addr_mv))
 }
 
-// Configure a mock runtime to test the pallet.
-frame_support::construct_runtime!(
-    pub enum Test
-    {
-        System: frame_system,
-        Balances: pallet_balances,
-        MoveModule: pallet_move,
+pub mod assets {
+    const MOVE_PROJECTS: &str = "src/tests/assets/move-projects";
+
+    /// Reads bytes from a file for the given path.
+    /// Can panic if the file doesn't exist.
+    fn read_bytes(file_path: &str) -> Vec<u8> {
+        std::fs::read(file_path)
+            .unwrap_or_else(|e| panic!("Can't read {file_path}: {e} - make sure you run pallet-move/tests/assets/move-projects/smove-build-all.sh"))
     }
-);
+
+    /// Reads a precompiled Move module from our assets directory.
+    pub fn read_module_from_project(project: &str, module_name: &str) -> Vec<u8> {
+        let path =
+            format!("{MOVE_PROJECTS}/{project}/build/{project}/bytecode_modules/{module_name}.mv");
+        read_bytes(&path)
+    }
+
+    /// Reads a precompiled Move bundle from our assets directory.
+    pub fn read_bundle_from_project(project: &str, bundle_name: &str) -> Vec<u8> {
+        let path = format!("{MOVE_PROJECTS}/{project}/build/{project}/bundles/{bundle_name}.mvb");
+        read_bytes(&path)
+    }
+
+    /// Reads a precompiled Move scripts from our assets directory.
+    pub fn read_script_from_project(project: &str, script_name: &str) -> Vec<u8> {
+        let path =
+            format!("{MOVE_PROJECTS}/{project}/build/{project}/bytecode_scripts/{script_name}.mv");
+        read_bytes(&path)
+    }
+}
+
+#[macro_export]
+macro_rules! script_transaction {
+    ($bytecode:expr, $type_args:expr $(, $args:expr)*) => {
+        {
+            let transaction = ScriptTransaction {
+                bytecode: $bytecode,
+                type_args: $type_args,
+                args: vec![$(bcs::to_bytes($args).unwrap()),*],
+            };
+            bcs::to_bytes(&transaction).unwrap()
+        }
+    }
+}
+
+#[macro_export]
+macro_rules! no_type_args {
+    () => {
+        vec![]
+    };
+}

src/tests.rs

@@ -0,0 +1,10 @@
+//! Container module for all test modules.
+
+mod address;
+mod balance;
+mod example;
+mod execute;
+mod modules;
+mod publish;
+mod signer;
+mod update_stdlib;