[feature] hyperledger#2040: Add integration test with transaction exe…

…cution limit (hyperledger#2051)

Signed-off-by: Daniil Polyakov <arjentix@gmail.com>

client/tests/integration/mod.rs

@@ -15,6 +15,7 @@ mod offline_peers;
 mod pagination;
 mod permissions;
 mod restart_peer;
+mod roles;
 mod transfer_asset;
 mod triggers;
 mod tx_history;

client/tests/integration/roles.rs

@@ -0,0 +1,96 @@
+#![allow(clippy::restriction)]
+
+use std::{str::FromStr as _, time::Duration};
+
+use eyre::{eyre, Result};
+use iroha_client::client::{self, Client};
+use iroha_core::{prelude::AllowAll, smartcontracts::permissions::ValidatorBuilder};
+use iroha_data_model::{permissions::Permissions, prelude::*};
+use iroha_permissions_validators::public_blockchain::transfer;
+use test_network::{Peer as TestPeer, *};
+use tokio::runtime::Runtime;
+
+#[test]
+fn add_role_to_limit_transfer_count() -> Result<()> {
+    const PERIOD_MS: u64 = 5000;
+    const COUNT: u32 = 2;
+
+    // Setting up client and peer.
+    // Peer has a special permission validator we need for this test
+    let rt = Runtime::test();
+    let (_peer, mut test_client) = rt.block_on(<TestPeer>::start_test_with_permissions(
+        ValidatorBuilder::new()
+            .with_recursive_validator(transfer::ExecutionCountFitsInLimit)
+            .all_should_succeed(),
+        AllowAll.into(),
+    ));
+    wait_for_genesis_committed(&vec![test_client.clone()], 0);
+
+    let alice_id = <Account as Identifiable>::Id::from_str("alice@wonderland")?;
+    let bob_id = <Account as Identifiable>::Id::from_str("bob@wonderland")?;
+    let rose_definition_id = <AssetDefinition as Identifiable>::Id::from_str("rose#wonderland")?;
+    let alice_rose_id =
+        <Asset as Identifiable>::Id::new(rose_definition_id.clone(), alice_id.clone());
+    let bob_rose_id = <Asset as Identifiable>::Id::new(rose_definition_id, bob_id.clone());
+    let role_id = <Role as Identifiable>::Id::from_str("non_privileged_user")?;
+    let rose_value = get_asset_value(&mut test_client, alice_rose_id.clone())?;
+
+    // Alice already has roses from genesis
+    assert!(rose_value > COUNT + 1);
+
+    // Registering Bob
+    let register_bob = RegisterBox::new(Account::new(bob_id, []));
+    test_client.submit_blocking(register_bob)?;
+
+    // Registering new role which sets `Transfer` execution count limit to
+    // `COUNT` for every `PERIOD_MS` milliseconds
+    let permission_token = PermissionToken::new(
+        transfer::CAN_TRANSFER_ONLY_FIXED_NUMBER_OF_TIMES_PER_PERIOD.clone(),
+        [
+            (
+                transfer::PERIOD_PARAM_NAME.clone(),
+                Value::U128(PERIOD_MS.into()),
+            ),
+            (transfer::COUNT_PARAM_NAME.clone(), Value::U32(COUNT)),
+        ],
+    );
+    let permissions = Permissions::from([permission_token]);
+    let register_role = RegisterBox::new(Role::new(role_id.clone(), permissions));
+    test_client.submit_blocking(register_role)?;
+
+    // Granting new role to Alice
+    let grant_role = GrantBox::new(role_id, alice_id);
+    test_client.submit_blocking(grant_role)?;
+
+    // Exhausting limit
+    let transfer_rose = TransferBox::new(alice_rose_id.clone(), Value::U32(1), bob_rose_id.clone());
+    for _ in 0..COUNT {
+        test_client.submit_blocking(transfer_rose.clone())?;
+    }
+    let new_alice_rose_value = get_asset_value(&mut test_client, alice_rose_id.clone())?;
+    let new_bob_rose_value = get_asset_value(&mut test_client, bob_rose_id.clone())?;
+    assert_eq!(new_alice_rose_value, rose_value - COUNT);
+    assert_eq!(new_bob_rose_value, COUNT);
+
+    // Checking that Alice can't do one more transfer
+    if test_client.submit_blocking(transfer_rose.clone()).is_ok() {
+        return Err(eyre!("Transfer passed when it shouldn't"));
+    }
+
+    // Waiting for a new period
+    std::thread::sleep(Duration::from_millis(PERIOD_MS));
+
+    // Transfering one more rose from Alice to Bob
+    test_client.submit_blocking(transfer_rose)?;
+    let new_alice_rose_value = get_asset_value(&mut test_client, alice_rose_id)?;
+    let new_bob_rose_value = get_asset_value(&mut test_client, bob_rose_id)?;
+    assert_eq!(new_alice_rose_value, rose_value - COUNT - 1);
+    assert_eq!(new_bob_rose_value, COUNT + 1);
+
+    Ok(())
+}
+
+fn get_asset_value(client: &mut Client, asset_id: AssetId) -> Result<u32> {
+    let asset = client.request(client::asset::by_id(asset_id))?;
+    Ok(*TryAsRef::<u32>::try_as_ref(asset.value())?)
+}

core/src/smartcontracts/isi/mod.rs

@@ -325,6 +325,8 @@ impl<W: WorldTrait> Execute<W> for RegisterBox {
             RegistrableBox::Trigger(trigger) => {
                 Register::<Trigger>::new(*trigger).execute(authority, wsv)
             }
+            #[cfg(feature = "roles")]
+            RegistrableBox::Role(role) => Register::<Role>::new(*role).execute(authority, wsv),
             _ => Err(Error::Unsupported(InstructionType::Register)),
         }
     }

core/src/wsv.rs

@@ -418,9 +418,7 @@ impl<W: WorldTrait> WorldStateView<W> {
     /// **Locking behaviour**: Holding references to blocks stored in the blockchain can induce
     /// deadlock. This limitation is imposed by the fact that blockchain is backed by [`dashmap::DashMap`]
     #[inline]
-    pub fn blocks(
-        &self,
-    ) -> impl Iterator<Item = impl Deref<Target = VersionedCommittedBlock> + '_> {
+    pub fn blocks(&self) -> crate::block::ChainIterator {
         self.blocks.iter()
     }
 

data_model/src/role.rs

@@ -5,6 +5,7 @@ use alloc::{boxed::Box, collections::btree_set, string::String};
 use core::fmt;
 #[cfg(feature = "std")]
 use std::collections::btree_set;
+use std::str::FromStr;
 
 use getset::Getters;
 use iroha_schema::IntoSchema;
@@ -13,7 +14,7 @@ use serde::{Deserialize, Serialize};
 
 use crate::{
     permissions::{PermissionToken, Permissions},
-    Identifiable, Name,
+    Identifiable, Name, ParseError,
 };
 
 /// Collection of [`RoleId`]s
@@ -42,8 +43,8 @@ pub struct Id {
 impl Id {
     /// Construct role id
     #[inline]
-    pub fn new(name: impl Into<Name>) -> Self {
-        Self { name: name.into() }
+    pub fn new(name: Name) -> Self {
+        Self { name }
     }
 }
 
@@ -53,6 +54,16 @@ impl fmt::Display for Id {
     }
 }
 
+impl FromStr for Id {
+    type Err = ParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(Self {
+            name: Name::from_str(s)?,
+        })
+    }
+}
+
 /// Role is a tag for a set of permission tokens.
 #[derive(
     Debug,
@@ -81,11 +92,11 @@ impl Role {
     /// Constructor.
     #[inline]
     pub fn new(
-        id: impl Into<Id>,
+        id: <Self as Identifiable>::Id,
         permissions: impl Into<Permissions>,
     ) -> <Self as Identifiable>::RegisteredWith {
         Self {
-            id: id.into(),
+            id,
             permissions: permissions.into(),
         }
     }

permissions_validators/src/public_blockchain/transfer.rs

@@ -1,13 +1,25 @@
 //! Module with permission for transfering
 
-use std::str::FromStr as _;
+use std::{str::FromStr as _, time::Duration};
 
 use super::*;
 
 #[allow(clippy::expect_used)]
 /// Can transfer user's assets permission token name.
 pub static CAN_TRANSFER_USER_ASSETS_TOKEN: Lazy<Name> =
-    Lazy::new(|| Name::from_str("can_transfer_user_assets").expect("Tested. Works.")); // See #1978
+    Lazy::new(|| Name::from_str("can_transfer_user_assets").expect("Valid")); // See #1978
+
+#[allow(clippy::expect_used)]
+/// Can transfer only fixed number of times per some time period
+pub static CAN_TRANSFER_ONLY_FIXED_NUMBER_OF_TIMES_PER_PERIOD: Lazy<Name> = Lazy::new(|| {
+    Name::from_str("can_transfer_only_fixed_number_of_times_per_period").expect("Valid")
+});
+#[allow(clippy::expect_used)]
+/// Name of `period` param for `CAN_TRANSFER_ONLY_FIXED_NUMBER_OF_TIMES_PER_PERIOD`
+pub static PERIOD_PARAM_NAME: Lazy<Name> = Lazy::new(|| Name::from_str("period").expect("Valid"));
+#[allow(clippy::expect_used)]
+/// Name of `count` param for `CAN_TRANSFER_ONLY_FIXED_NUMBER_OF_TIMES_PER_PERIOD`
+pub static COUNT_PARAM_NAME: Lazy<Name> = Lazy::new(|| Name::from_str("count").expect("Valid"));
 
 /// Checks that account transfers only the assets that he owns.
 #[derive(Debug, Copy, Clone)]
@@ -103,3 +115,135 @@ impl<W: WorldTrait> IsGrantAllowed<W> for GrantMyAssetAccess {
         check_asset_owner_for_token(&permission_token, authority)
     }
 }
+
+/// Validator that checks that `Transfer` instruction execution count
+/// fits well in some time period
+#[derive(Debug, Clone, Copy)]
+pub struct ExecutionCountFitsInLimit;
+
+impl_from_item_for_instruction_validator_box!(ExecutionCountFitsInLimit);
+
+impl<W: WorldTrait> IsAllowed<W, Instruction> for ExecutionCountFitsInLimit {
+    #[allow(clippy::expect_used, clippy::unwrap_in_result)]
+    fn check(
+        &self,
+        authority: &AccountId,
+        instruction: &Instruction,
+        wsv: &WorldStateView<W>,
+    ) -> Result<(), DenialReason> {
+        if !matches!(instruction, Instruction::Transfer(_)) {
+            return Ok(());
+        };
+
+        let params = retrieve_permission_params(wsv, authority)?;
+        if params.is_empty() {
+            return Ok(());
+        }
+
+        let period = retrieve_period(&params)?;
+        let count = retrieve_count(&params)?;
+        let executions_count: u32 = count_executions(wsv, authority, period)
+            .try_into()
+            .expect("`usize` should always fit in `u32`");
+        if executions_count >= count {
+            return Err(DenialReason::from(
+                "Transfer transaction limit for current period is exceed",
+            ));
+        }
+        Ok(())
+    }
+}
+
+/// Retrieve permission parameters for `ExecutionCountFitsInLimit` validator.
+/// Returns empty collection if nothing found
+///
+/// # Errors
+/// - Account doesn't exist
+fn retrieve_permission_params<W: WorldTrait>(
+    wsv: &WorldStateView<W>,
+    authority: &AccountId,
+) -> Result<BTreeMap<Name, Value>, DenialReason> {
+    wsv.map_account(authority, |account| {
+        wsv.account_permission_tokens(account)
+            .iter()
+            .filter(|token| token.name == *CAN_TRANSFER_ONLY_FIXED_NUMBER_OF_TIMES_PER_PERIOD)
+            .map(|token| token.params.clone())
+            .next()
+            .unwrap_or_default()
+    })
+    .map_err(|e| e.to_string())
+}
+
+/// Retrieve period from `params`
+///
+/// # Errors
+/// - There is no period parameter
+/// - Period has wrong value type
+/// - Failed conversion from `u128` to `u64`
+fn retrieve_period(params: &BTreeMap<Name, Value>) -> Result<Duration, DenialReason> {
+    match params
+        .get(&*PERIOD_PARAM_NAME)
+        .ok_or_else(|| format!("Expected `{}` parameter", *PERIOD_PARAM_NAME))?
+    {
+        Value::U128(period) => Ok(Duration::from_millis(
+            u64::try_from(*period).map_err(|e| e.to_string())?,
+        )),
+        _ => Err(format!(
+            "`{}` parameter has wrong value type. Expected `u128`",
+            *PERIOD_PARAM_NAME
+        )),
+    }
+}
+
+/// Retrieve count from `params`
+///
+/// # Errors
+/// - There is no count parameter
+/// - Count has wrong value type
+fn retrieve_count(params: &BTreeMap<Name, Value>) -> Result<u32, DenialReason> {
+    match params
+        .get(&*COUNT_PARAM_NAME)
+        .ok_or_else(|| format!("Expected `{}` parameter", *COUNT_PARAM_NAME))?
+    {
+        Value::U32(count) => Ok(*count),
+        _ => Err(format!(
+            "`{}` parameter has wrong value type. Expected `u32`",
+            *COUNT_PARAM_NAME
+        )),
+    }
+}
+
+/// Counts the number of `Transfer`s  which happened in the last `period`
+fn count_executions<W: WorldTrait>(
+    wsv: &WorldStateView<W>,
+    authority: &AccountId,
+    period: Duration,
+) -> usize {
+    let period_start_ms = current_time().saturating_sub(period).as_millis();
+
+    wsv.blocks()
+        .rev()
+        .take_while(|block| block.header().timestamp > period_start_ms)
+        .map(|block| -> usize {
+            block
+                .as_v1()
+                .transactions
+                .iter()
+                .filter_map(|tx| {
+                    let payload = tx.payload();
+                    if payload.account_id == *authority {
+                        if let Executable::Instructions(instructions) = &payload.instructions {
+                            return Some(
+                                instructions
+                                    .iter()
+                                    .filter(|isi| matches!(isi, Instruction::Transfer(_)))
+                                    .count(),
+                            );
+                        }
+                    }
+                    None
+                })
+                .sum()
+        })
+        .sum()
+}