feat: implement currency locking and extend testing

.github/workflows/common.yml

@@ -3,7 +3,7 @@ name: Common jobs (fmt, clippy, test)
 on:
   pull_request:
     paths:
-      - src/**
+      - pallet/**
       - Cargo.toml
       - rpc/**
       - .github/workflows/common.yml

pallet/src/lib.rs

@@ -34,14 +34,16 @@ pub mod pallet {
     extern crate alloc;
     #[cfg(not(feature = "std"))]
     use alloc::format;
-    use blake2::{Blake2s256, Digest};
+    use blake2::{digest::consts::U8, Blake2b, Blake2s256, Digest};
     use core::convert::AsRef;
 
     use codec::{FullCodec, FullEncode};
     use frame_support::{
         dispatch::DispatchResultWithPostInfo,
         pallet_prelude::*,
-        traits::{Currency, Get, LockableCurrency, ReservableCurrency},
+        traits::{
+            tokens::currency::LockIdentifier, Currency, Get, LockableCurrency, ReservableCurrency,
+        },
     };
     use frame_system::pallet_prelude::*;
     pub use move_core_types::language_storage::TypeTag;
@@ -247,7 +249,6 @@ pub mod pallet {
             gas_limit: u64,
             cheque_limit: BalanceOf<T>,
         ) -> DispatchResultWithPostInfo {
-            // TODO(neutrinoks): Add currency locking in multi-signature executions.
             // A signer for the extrinsic and a signer for the Move script.
             let who = ensure_signed(origin)?;
 
@@ -286,7 +287,8 @@ pub mod pallet {
                 )
             };
             if signer_count > 0 {
-                signature_handler.sign_script(&who, cheque_limit.into())?;
+                let lock_id = Self::multi_signer_lock_id(&who, &transaction_bc[..], &cheque_limit);
+                signature_handler.sign_script(&who, cheque_limit.into(), lock_id)?;
             }
 
             // If the script is signed correctly, we can execute it in MoveVM and update the
@@ -561,6 +563,18 @@ pub mod pallet {
             hasher.update(transaction_bc);
             hasher.finalize().into()
         }
+
+        pub fn multi_signer_lock_id(
+            who: &T::AccountId,
+            transaction_bc: &[u8],
+            cheque_limit: &BalanceOf<T>,
+        ) -> LockIdentifier {
+            let mut hasher = Blake2b::<U8>::new();
+            hasher.update(transaction_bc);
+            hasher.update(&who.encode()[..]);
+            hasher.update(&cheque_limit.encode()[..]);
+            hasher.finalize().into()
+        }
     }
 
     #[pallet::error]

pallet/src/mock.rs

@@ -1,4 +1,6 @@
 use frame_support::{
+    dispatch::DispatchErrorWithPostInfo,
+    pallet_prelude::{DispatchError, DispatchResultWithPostInfo},
     parameter_types,
     traits::{ConstU128, ConstU16, ConstU32, ConstU64, OnFinalize, OnIdle, OnInitialize},
 };
@@ -181,6 +183,22 @@ pub(crate) fn last_event() -> RuntimeEvent {
     System::events().pop().expect("Event expected").event
 }
 
+pub(crate) fn verify_module_error_with_msg(
+    res: DispatchResultWithPostInfo,
+    e_msg: &str,
+) -> Result<bool, String> {
+    if let Err(DispatchErrorWithPostInfo {
+        error: DispatchError::Module(moderr),
+        ..
+    }) = res
+    {
+        if let Some(msg) = moderr.message {
+            return Ok(msg == e_msg);
+        }
+    }
+    Err(format!("{res:?} does not match '{e_msg}'"))
+}
+
 pub mod assets {
     const MOVE_PROJECTS: &str = "src/tests/assets/move-projects";
 

pallet/src/signer.rs

@@ -1,7 +1,17 @@
 use core::marker::PhantomData;
 
 use codec::{Decode, Encode, MaxEncodedLen};
-use frame_support::{pallet_prelude::RuntimeDebug, traits::Get, BoundedBTreeMap, Parameter};
+use frame_support::{
+    pallet_prelude::RuntimeDebug,
+    traits::{
+        tokens::{
+            currency::{LockIdentifier, LockableCurrency},
+            WithdrawReasons,
+        },
+        Get,
+    },
+    BoundedBTreeMap, Parameter,
+};
 use frame_system::{pallet_prelude::BlockNumberFor, Config as SysConfig};
 use scale_info::TypeInfo;
 use sp_runtime::traits::MaybeSerializeDeserialize;
@@ -54,6 +64,8 @@ pub struct SignerData {
     pub signature: Signature,
     /// Individual cheque-limit.
     pub cheque_limit: u128,
+    /// Lock ID for locked currency.
+    pub lock_id: LockIdentifier,
 }
 
 /// Storage struct definition for a multi-signer request.
@@ -183,13 +195,17 @@ where
         &mut self,
         account: &T::AccountId,
         cheque_limit: u128,
+        lock_id: LockIdentifier,
     ) -> Result<(), Error<T>> {
         if let Some(ms_data) = self.multisig_info.get_mut(account) {
             if matches!(ms_data.signature, Signature::Approved) {
                 Err(Error::<T>::UserHasAlreadySigned)
             } else {
                 ms_data.signature = Signature::Approved;
                 ms_data.cheque_limit = cheque_limit;
+                ms_data.lock_id = lock_id;
+                let amount = BalanceOf::<T>::from(cheque_limit);
+                T::Currency::set_lock(lock_id, account, amount, WithdrawReasons::all());
                 Ok(())
             }
         } else {
@@ -209,6 +225,7 @@ where
     pub(crate) fn write_cheques(&self) -> Result<BalanceAdapter<T>, Error<T>> {
         let mut balances = BalanceAdapter::<T>::new();
         for (account, ms_data) in self.multisig_info.iter() {
+            T::Currency::remove_lock(ms_data.lock_id, account);
             balances
                 .write_cheque(account, &BalanceOf::<T>::from(ms_data.cheque_limit))
                 .map_err(|_| Error::<T>::InsufficientBalance)?;

pallet/src/tests/signer.rs

@@ -1,6 +1,10 @@
 use crate::{mock::*, no_type_args, script_transaction, Error, Event, MultisigStorage};
 
-use frame_support::{assert_err, assert_ok, pallet_prelude::DispatchResultWithPostInfo};
+use frame_support::{
+    assert_err, assert_ok,
+    pallet_prelude::DispatchResultWithPostInfo,
+    traits::{tokens::WithdrawReasons, Currency},
+};
 use move_core_types::{language_storage::TypeTag, u256::U256};
 use move_vm_backend::types::MAX_GAS_AMOUNT;
 use rand::{distributions::Standard, prelude::Distribution, rngs::ThreadRng, Rng};
@@ -296,6 +300,7 @@ fn script_with_vector_containing_signer_fails() {
 #[test]
 fn multiple_signers_in_multisig_script_works() {
     const BALANCE: Balance = 80_000_000_000_000;
+    const CHANGE: Balance = 20_000_000_000_000;
     let (bob_addr_32, bob_addr_mv) = addrs_from_ss58(BOB_ADDR).unwrap();
     let (alice_addr_32, alice_addr_mv) = addrs_from_ss58(ALICE_ADDR).unwrap();
     let (dave_addr_32, dave_addr_mv) = addrs_from_ss58(DAVE_ADDR).unwrap();
@@ -353,7 +358,29 @@ fn multiple_signers_in_multisig_script_works() {
                 &2u8
             );
             let call_hash = MoveModule::transaction_bc_call_hash(&transaction_bc[..]);
+
+            // Verify that no lock has been set so far and that the Multisig request entry cannot
+            // be found in storage.
             assert!(MultisigStorage::<Test>::try_get(call_hash).is_err());
+            assert_ok!(Balances::ensure_can_withdraw(
+                &alice_addr_32,
+                BALANCE,
+                WithdrawReasons::TRANSFER,
+                0
+            ));
+            assert_ok!(Balances::ensure_can_withdraw(
+                &dave_addr_32,
+                BALANCE,
+                WithdrawReasons::TRANSFER,
+                0
+            ));
+            assert_ok!(Balances::ensure_can_withdraw(
+                &eve_addr_32,
+                BALANCE,
+                WithdrawReasons::TRANSFER,
+                0
+            ));
+
             assert_ok!(MoveModule::execute(
                 RuntimeOrigin::signed(alice_addr_32.clone()),
                 transaction_bc.clone(),
@@ -367,6 +394,25 @@ fn multiple_signers_in_multisig_script_works() {
                 })
             );
             assert!(MultisigStorage::<Test>::try_get(call_hash).is_ok());
+            assert!(Balances::ensure_can_withdraw(
+                &alice_addr_32,
+                BALANCE,
+                WithdrawReasons::TRANSFER,
+                0
+            )
+            .is_err());
+            assert_ok!(Balances::ensure_can_withdraw(
+                &dave_addr_32,
+                BALANCE,
+                WithdrawReasons::TRANSFER,
+                0
+            ));
+            assert_ok!(Balances::ensure_can_withdraw(
+                &eve_addr_32,
+                BALANCE,
+                WithdrawReasons::TRANSFER,
+                0
+            ));
 
             assert_ok!(MoveModule::execute(
                 RuntimeOrigin::signed(dave_addr_32.clone()),
@@ -380,6 +426,27 @@ fn multiple_signers_in_multisig_script_works() {
                     who: dave_addr_32.clone()
                 })
             );
+            // Now this candidate should also not be able to transfer the locked tokens.
+            assert!(Balances::ensure_can_withdraw(
+                &alice_addr_32,
+                BALANCE,
+                WithdrawReasons::TRANSFER,
+                0
+            )
+            .is_err());
+            assert!(Balances::ensure_can_withdraw(
+                &dave_addr_32,
+                BALANCE,
+                WithdrawReasons::TRANSFER,
+                0
+            )
+            .is_err());
+            assert_ok!(Balances::ensure_can_withdraw(
+                &eve_addr_32,
+                BALANCE,
+                WithdrawReasons::TRANSFER,
+                0
+            ));
 
             assert_ok!(MoveModule::execute(
                 RuntimeOrigin::signed(eve_addr_32.clone()),
@@ -397,6 +464,24 @@ fn multiple_signers_in_multisig_script_works() {
                     ]
                 })
             );
+            assert_ok!(Balances::ensure_can_withdraw(
+                &alice_addr_32,
+                CHANGE,
+                WithdrawReasons::TRANSFER,
+                0
+            ));
+            assert_ok!(Balances::ensure_can_withdraw(
+                &dave_addr_32,
+                CHANGE,
+                WithdrawReasons::TRANSFER,
+                0
+            ));
+            assert_ok!(Balances::ensure_can_withdraw(
+                &eve_addr_32,
+                CHANGE,
+                WithdrawReasons::TRANSFER,
+                0
+            ));
             // Now after all signers have signed the multsig script, we can expect that the script
             // will be executed and the multisg storage will remove the pending mutlisig script
             // data, since the script has been executed.
@@ -448,7 +533,8 @@ fn verify_old_multi_signer_requests_getting_removed() {
                 no_type_args!(),
                 &alice_addr_mv,
                 &dave_addr_mv,
-                &eve_addr_mv
+                &eve_addr_mv,
+                &2u8
             );
             assert_ok!(MoveModule::execute(
                 RuntimeOrigin::signed(alice_addr_32.clone()),
@@ -497,3 +583,71 @@ fn verify_old_multi_signer_requests_getting_removed() {
             );
         })
 }
+
+#[test]
+fn cheque_limit_in_multi_signer_execution_works() {
+    const BALANCE: Balance = 80_000_000_000_000;
+    let (bob_addr_32, bob_addr_mv) = addrs_from_ss58(BOB_ADDR).unwrap();
+    let (alice_addr_32, alice_addr_mv) = addrs_from_ss58(ALICE_ADDR).unwrap();
+    let (dave_addr_32, dave_addr_mv) = addrs_from_ss58(DAVE_ADDR).unwrap();
+    let (eve_addr_32, eve_addr_mv) = addrs_from_ss58(EVE_ADDR).unwrap();
+
+    ExtBuilder::default()
+        .with_balances(vec![
+            (bob_addr_32.clone(), BALANCE),
+            (alice_addr_32.clone(), BALANCE),
+            (dave_addr_32.clone(), BALANCE),
+            (eve_addr_32.clone(), BALANCE),
+        ])
+        .build()
+        .execute_with(|| {
+            // Roll to first block in case of block based event checkings and processes.
+            roll_to(1);
+
+            // Initialisation & Setup by developer Bob.
+            let module = assets::read_module_from_project("multiple-signers", "Dorm");
+            assert_ok!(MoveModule::publish_module(
+                RuntimeOrigin::signed(bob_addr_32.clone()),
+                module,
+                MAX_GAS_AMOUNT
+            ));
+            let script = assets::read_script_from_project("multiple-signers", "init_module");
+            let transaction_bc = script_transaction!(script, no_type_args!(), &bob_addr_mv);
+            assert_ok!(MoveModule::execute(
+                RuntimeOrigin::signed(bob_addr_32.clone()),
+                transaction_bc.clone(),
+                MAX_GAS_AMOUNT,
+                BALANCE,
+            ));
+
+            // Now only 2 of 3 planned signers will sign the script execution.
+            let script = assets::read_script_from_project("multiple-signers", "rent_apartment");
+            let transaction_bc = script_transaction!(
+                script,
+                no_type_args!(),
+                &alice_addr_mv,
+                &dave_addr_mv,
+                &eve_addr_mv,
+                &2u8
+            );
+            assert_ok!(MoveModule::execute(
+                RuntimeOrigin::signed(dave_addr_32.clone()),
+                transaction_bc.clone(),
+                MAX_GAS_AMOUNT,
+                BALANCE,
+            ));
+            assert_ok!(MoveModule::execute(
+                RuntimeOrigin::signed(eve_addr_32.clone()),
+                transaction_bc.clone(),
+                MAX_GAS_AMOUNT,
+                BALANCE,
+            ));
+            let res = MoveModule::execute(
+                RuntimeOrigin::signed(alice_addr_32.clone()),
+                transaction_bc.clone(),
+                MAX_GAS_AMOUNT,
+                BALANCE / 2,
+            );
+            assert!(verify_module_error_with_msg(res, "Aborted").unwrap());
+        })
+}

pallet/src/tests/update_stdlib.rs

@@ -3,9 +3,7 @@
 use crate::mock::*;
 use crate::{no_type_args, script_transaction};
 
-use frame_support::{
-    assert_err, assert_ok, dispatch::DispatchErrorWithPostInfo, pallet_prelude::*,
-};
+use frame_support::{assert_err, assert_ok, pallet_prelude::*};
 use move_stdlib::move_stdlib_bundle;
 use move_vm_backend::types::MAX_GAS_AMOUNT;
 
@@ -17,22 +15,6 @@ fn mock_substrate_stdlib() -> Vec<u8> {
     assets::read_bundle_from_project("testing-substrate-stdlib", "testing-substrate-stdlib")
 }
 
-fn verify_module_error_with_msg(
-    res: DispatchResultWithPostInfo,
-    e_msg: &str,
-) -> Result<bool, String> {
-    if let Err(DispatchErrorWithPostInfo {
-        error: DispatchError::Module(moderr),
-        ..
-    }) = res
-    {
-        if let Some(msg) = moderr.message {
-            return Ok(msg == e_msg);
-        }
-    }
-    Err(format!("{res:?} does not match '{e_msg}'"))
-}
-
 #[test]
 fn regular_user_update_fail() {
     let bob_addr_native = addr32_from_ss58(BOB_ADDR).unwrap();