fix: Don't prematurely execute time-triggers before their start tim…

…estamp

Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

client/tests/integration/triggers/time_trigger.rs

@@ -3,10 +3,15 @@ use std::{str::FromStr as _, time::Duration};
 use eyre::Result;
 use iroha_client::{
     client::{self, Client, QueryResult},
-    data_model::{prelude::*, transaction::WasmSmartContract},
+    data_model::{
+        asset::AssetId,
+        events::pipeline::{BlockEventFilter, BlockStatus},
+        prelude::*,
+        transaction::WasmSmartContract,
+        Level,
+    },
 };
 use iroha_config::parameters::defaults::chain_wide::DEFAULT_CONSENSUS_ESTIMATION;
-use iroha_data_model::events::pipeline::{BlockEventFilter, BlockStatus};
 use iroha_logger::info;
 use test_network::*;
 
@@ -96,46 +101,53 @@ fn time_trigger_execution_count_error_should_be_less_than_15_percent() -> Result
 }
 
 #[test]
-fn change_asset_metadata_after_1_sec() -> Result<()> {
-    const PERIOD: Duration = Duration::from_secs(1);
-
-    let (_rt, _peer, mut test_client) = <PeerBuilder>::new().with_port(10_660).start_with_runtime();
+fn mint_asset_after_1_sec() -> Result<()> {
+    let (_rt, _peer, test_client) = <PeerBuilder>::new().with_port(10_660).start_with_runtime();
     wait_for_genesis_committed(&vec![test_client.clone()], 0);
-    let start_time = curr_time();
-
-    // Start listening BEFORE submitting any transaction not to miss any block committed event
-    let event_listener = get_block_committed_event_listener(&test_client)?;
+    // Sleep to certainly bypass time interval analyzed by genesis
+    std::thread::sleep(DEFAULT_CONSENSUS_ESTIMATION);
 
     let asset_definition_id = AssetDefinitionId::from_str("rose#wonderland").expect("Valid");
     let account_id = AccountId::from_str("alice@wonderland").expect("Valid");
-    let key = Name::from_str("petal")?;
+    let asset_id = AssetId::new(asset_definition_id.clone(), account_id.clone());
 
-    let schedule = TimeSchedule::starting_at(start_time + PERIOD);
-    let instruction =
-        SetKeyValue::asset_definition(asset_definition_id.clone(), key.clone(), 3_u32);
+    let init_quantity = test_client.request(FindAssetQuantityById {
+        id: asset_id.clone(),
+    })?;
+
+    let start_time = curr_time();
+    // Create trigger with schedule which is in the future to the new block but within block estimation time
+    let schedule = TimeSchedule::starting_at(start_time + Duration::from_secs(1));
+    let instruction = Mint::asset_numeric(1_u32, asset_id.clone());
     let register_trigger = Register::trigger(Trigger::new(
-        "change_rose_metadata".parse().expect("Valid"),
+        "mint_rose".parse().expect("Valid"),
         Action::new(
             vec![instruction],
             Repeats::from(1_u32),
             account_id.clone(),
             TimeEventFilter::new(ExecutionTime::Schedule(schedule)),
         ),
     ));
-    test_client.submit(register_trigger)?;
-    submit_sample_isi_on_every_block_commit(
-        event_listener,
-        &mut test_client,
-        &account_id,
-        Duration::from_secs(1),
-        usize::try_from(PERIOD.as_millis() / DEFAULT_CONSENSUS_ESTIMATION.as_millis() + 1)?,
-    )?;
+    test_client.submit_blocking(register_trigger)?;
 
-    let value = test_client.request(FindAssetDefinitionKeyValueByIdAndKey {
-        id: asset_definition_id,
-        key,
+    let after_registration_quantity = test_client.request(FindAssetQuantityById {
+        id: asset_id.clone(),
     })?;
-    assert_eq!(value, numeric!(3).into());
+    // Schedule start was in the future so trigger isn't executed
+    assert_eq!(init_quantity, after_registration_quantity);
+
+    // Sleep long enough that trigger start is in the past
+    std::thread::sleep(DEFAULT_CONSENSUS_ESTIMATION);
+    test_client.submit_blocking(Log::new(Level::DEBUG, "Just to create block".to_string()))?;
+
+    let after_wait_quantity = test_client.request(FindAssetQuantityById {
+        id: asset_id.clone(),
+    })?;
+    // Schedule start was in the future so trigger shouldn't be executed
+    assert_eq!(
+        init_quantity.checked_add(1u32.into()).unwrap(),
+        after_wait_quantity
+    );
 
     Ok(())
 }

core/src/smartcontracts/isi/triggers/mod.rs

@@ -13,6 +13,8 @@ pub mod specialized;
 /// - TODO: authorities.
 /// - TODO: authority permissions.
 pub mod isi {
+    use std::time::Duration;
+
     use iroha_data_model::{
         events::EventFilter,
         isi::error::{InvalidParameterError, RepetitionError},
@@ -39,6 +41,11 @@ pub mod isi {
                 }
             }
 
+            let last_block_estimation = state_transaction.latest_block_ref().map(|block| {
+                block.header().timestamp()
+                    + Duration::from_millis(block.header().consensus_estimation_ms)
+            });
+
             let engine = state_transaction.engine.clone(); // Cloning engine is cheap
             let triggers = &mut state_transaction.world.triggers;
             let trigger_id = new_trigger.id().clone();
@@ -55,12 +62,32 @@ pub mod isi {
                         .try_into()
                         .map_err(|e: &str| Error::Conversion(e.to_owned()))?,
                 ),
-                TriggeringEventFilterBox::Time(_) => triggers.add_time_trigger(
-                    &engine,
-                    new_trigger
-                        .try_into()
-                        .map_err(|e: &str| Error::Conversion(e.to_owned()))?,
-                ),
+                TriggeringEventFilterBox::Time(time_filter) => {
+                    if let ExecutionTime::Schedule(schedule) = time_filter.0 {
+                        match last_block_estimation {
+                            // We're in genesis
+                            None => {
+                                return Err(Error::InvalidParameter(
+                                    InvalidParameterError::TimeTriggerInThePast,
+                                ));
+                            }
+                            Some(latest_block_estimation)
+                                if schedule.start < latest_block_estimation =>
+                            {
+                                return Err(Error::InvalidParameter(
+                                    InvalidParameterError::TimeTriggerInThePast,
+                                ));
+                            }
+                            Some(_) => (),
+                        }
+                    }
+                    triggers.add_time_trigger(
+                        &engine,
+                        new_trigger
+                            .try_into()
+                            .map_err(|e: &str| Error::Conversion(e.to_owned()))?,
+                    )
+                }
                 TriggeringEventFilterBox::ExecuteTrigger(_) => triggers.add_by_call_trigger(
                     &engine,
                     new_trigger

data_model/src/account.rs

@@ -398,8 +398,8 @@ impl SignatureCheckCondition {
         Self::AllAccountSignaturesAnd(ConstVec::new_empty())
     }
 
-    #[must_use]
     #[cfg(feature = "transparent_api")]
+    #[must_use]
     fn check(
         &self,
         account_signatories: &btree_set::BTreeSet<PublicKey>,
@@ -444,6 +444,7 @@ mod tests {
         KeyPair::random().public_key().clone()
     }
 
+    #[cfg(feature = "transparent_api")]
     fn check_signature_check_condition(
         condition: &SignatureCheckCondition,
         account_signatories: &[&PublicKey],
@@ -459,6 +460,7 @@ mod tests {
         );
     }
 
+    #[cfg(feature = "transparent_api")]
     #[test]
     fn signature_check_condition_default() {
         let key1 = make_key();
@@ -476,6 +478,7 @@ mod tests {
         check_signature_check_condition(&condition, &[&key1, &key2, &key3], &[&key3], true);
     }
 
+    #[cfg(feature = "transparent_api")]
     #[test]
     fn signature_check_condition_all() {
         let key1 = make_key();
@@ -499,6 +502,7 @@ mod tests {
         check_signature_check_condition(&condition, &[&key1, &key2], &[&key2, &key3], false);
     }
 
+    #[cfg(feature = "transparent_api")]
     #[test]
     fn signature_check_condition_any_or() {
         let key1 = make_key();
@@ -515,6 +519,7 @@ mod tests {
         check_signature_check_condition(&condition, &[&key1, &key2], &[&key2], true);
     }
 
+    #[cfg(feature = "transparent_api")]
     #[test]
     fn signature_check_condition_all_and() {
         let key1 = make_key();