From 2dec01352c175f83068b492569fd1762f150428e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marin=20Ver=C5=A1i=C4=87?= Date: Wed, 24 Jul 2024 11:53:50 +0200 Subject: [PATCH] refactor: remove `TriggeringEventFilterBox` (#4866) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marin Veršić --- core/src/smartcontracts/isi/triggers/mod.rs | 13 ++-- core/src/smartcontracts/isi/triggers/set.rs | 8 ++- .../isi/triggers/specialized.rs | 45 ++++++------ data_model/src/events/mod.rs | 71 +----------------- data_model/src/isi.rs | 4 -- data_model/src/query/mod.rs | 4 +- data_model/src/trigger.rs | 72 ++++++++++++++++--- docs/source/references/schema.json | 32 +-------- schema/gen/src/lib.rs | 1 - 9 files changed, 105 insertions(+), 145 deletions(-) diff --git a/core/src/smartcontracts/isi/triggers/mod.rs b/core/src/smartcontracts/isi/triggers/mod.rs index 62e2d1d259c..1d893f9b7c8 100644 --- a/core/src/smartcontracts/isi/triggers/mod.rs +++ b/core/src/smartcontracts/isi/triggers/mod.rs @@ -49,19 +49,19 @@ pub mod isi { let triggers = &mut state_transaction.world.triggers; let trigger_id = new_trigger.id().clone(); let success = match &new_trigger.action.filter { - TriggeringEventFilterBox::Data(_) => triggers.add_data_trigger( + EventFilterBox::Data(_) => triggers.add_data_trigger( &engine, new_trigger .try_into() .map_err(|e: &str| Error::Conversion(e.to_owned()))?, ), - TriggeringEventFilterBox::Pipeline(_) => triggers.add_pipeline_trigger( + EventFilterBox::Pipeline(_) => triggers.add_pipeline_trigger( &engine, new_trigger .try_into() .map_err(|e: &str| Error::Conversion(e.to_owned()))?, ), - TriggeringEventFilterBox::Time(time_filter) => { + EventFilterBox::Time(time_filter) => { if let ExecutionTime::Schedule(schedule) = time_filter.0 { match last_block_estimation { // Genesis block @@ -91,12 +91,15 @@ pub mod isi { .map_err(|e: &str| Error::Conversion(e.to_owned()))?, ) } - TriggeringEventFilterBox::ExecuteTrigger(_) => triggers.add_by_call_trigger( + EventFilterBox::ExecuteTrigger(_) => triggers.add_by_call_trigger( &engine, new_trigger .try_into() .map_err(|e: &str| Error::Conversion(e.to_owned()))?, ), + EventFilterBox::TriggerCompleted(_) => { + unreachable!("Disallowed during deserialization"); + } } .map_err(|e| InvalidParameterError::Wasm(e.to_string()))?; @@ -289,7 +292,7 @@ pub mod isi { .world .triggers .inspect_by_id(id, |action| -> Result<(), Error> { - let allow_execute = if let TriggeringEventFilterBox::ExecuteTrigger(filter) = + let allow_execute = if let EventFilterBox::ExecuteTrigger(filter) = action.clone_and_box().filter { filter.matches(&event) || action.authority() == authority diff --git a/core/src/smartcontracts/isi/triggers/set.rs b/core/src/smartcontracts/isi/triggers/set.rs index d7781f7acb2..4e3f264829d 100644 --- a/core/src/smartcontracts/isi/triggers/set.rs +++ b/core/src/smartcontracts/isi/triggers/set.rs @@ -612,6 +612,12 @@ impl<'set> SetBlock<'set> { } } +trait TriggeringEventFilter: EventFilter {} +impl TriggeringEventFilter for DataEventFilter {} +impl TriggeringEventFilter for PipelineEventFilterBox {} +impl TriggeringEventFilter for TimeEventFilter {} +impl TriggeringEventFilter for ExecuteTriggerEventFilter {} + impl<'block, 'set> SetTransaction<'block, 'set> { /// Apply transaction's changes pub fn apply(self) { @@ -704,7 +710,7 @@ impl<'block, 'set> SetTransaction<'block, 'set> { /// # Errors /// /// Return [`Err`] if failed to preload wasm trigger - fn add_to( + fn add_to( &mut self, engine: &wasmtime::Engine, trigger: SpecializedTrigger, diff --git a/core/src/smartcontracts/isi/triggers/specialized.rs b/core/src/smartcontracts/isi/triggers/specialized.rs index a3deafa5d83..e3dc4b196c5 100644 --- a/core/src/smartcontracts/isi/triggers/specialized.rs +++ b/core/src/smartcontracts/isi/triggers/specialized.rs @@ -4,7 +4,7 @@ use derive_more::Constructor; use iroha_crypto::HashOf; use iroha_data_model::{ account::AccountId, - events::{EventFilter, TriggeringEventFilterBox}, + events::{EventFilter, EventFilterBox}, metadata::Metadata, prelude::*, }; @@ -52,7 +52,7 @@ impl SpecializedAction { impl From> for Action where - F: Into, + F: Into, { fn from(value: SpecializedAction) -> Self { Action { @@ -81,7 +81,7 @@ macro_rules! impl_try_from_box { type Error = &'static str; fn try_from(boxed: Trigger) -> Result { - if let TriggeringEventFilterBox::$variant(concrete_filter) = boxed.action.filter { + if let EventFilterBox::$variant(concrete_filter) = boxed.action.filter { let action = SpecializedAction::new( boxed.action.executable, boxed.action.repeats, @@ -93,7 +93,7 @@ macro_rules! impl_try_from_box { action, }) } else { - Err(concat!("Expected `TriggeringEventFilterBox::", stringify!($variant),"`, but another variant found")) + Err(concat!("Expected `EventFilterBox::", stringify!($variant),"`, but another variant found")) } } } @@ -159,15 +159,13 @@ pub trait LoadedActionTrait { fn mintable(&self) -> bool; /// Convert action to a boxed representation - fn into_boxed(self) -> LoadedAction; + fn into_boxed(self) -> LoadedAction; /// Same as [`into_boxed()`](LoadedActionTrait::into_boxed) but clones `self` - fn clone_and_box(&self) -> LoadedAction; + fn clone_and_box(&self) -> LoadedAction; } -impl + Clone> LoadedActionTrait - for LoadedAction -{ +impl + Clone> LoadedActionTrait for LoadedAction { fn executable(&self) -> &ExecutableRef { &self.executable } @@ -196,7 +194,7 @@ impl + Clone> LoadedActionTrait self.filter.mintable() } - fn into_boxed(self) -> LoadedAction { + fn into_boxed(self) -> LoadedAction { let Self { executable, repeats, @@ -214,7 +212,7 @@ impl + Clone> LoadedActionTrait } } - fn clone_and_box(&self) -> LoadedAction { + fn clone_and_box(&self) -> LoadedAction { self.clone().into_boxed() } } @@ -225,30 +223,29 @@ mod tests { #[test] fn trigger_with_filterbox_can_be_unboxed() { - /// Should fail to compile if a new variant will be added to `TriggeringEventFilterBox` + /// Should fail to compile if a new variant will be added to `EventFilterBox` #[allow(dead_code)] fn compile_time_check(boxed: Trigger) { match &boxed.action.filter { - TriggeringEventFilterBox::Data(_) => { - SpecializedTrigger::::try_from(boxed) - .map(|_| ()) - .unwrap() - } - TriggeringEventFilterBox::Pipeline(_) => { + EventFilterBox::Data(_) => SpecializedTrigger::::try_from(boxed) + .map(|_| ()) + .unwrap(), + EventFilterBox::Pipeline(_) => { SpecializedTrigger::::try_from(boxed) .map(|_| ()) .unwrap() } - TriggeringEventFilterBox::Time(_) => { - SpecializedTrigger::::try_from(boxed) - .map(|_| ()) - .unwrap() - } - TriggeringEventFilterBox::ExecuteTrigger(_) => { + EventFilterBox::Time(_) => SpecializedTrigger::::try_from(boxed) + .map(|_| ()) + .unwrap(), + EventFilterBox::ExecuteTrigger(_) => { SpecializedTrigger::::try_from(boxed) .map(|_| ()) .unwrap() } + EventFilterBox::TriggerCompleted(_) => { + unreachable!("Disallowed during deserialization") + } } } } diff --git a/data_model/src/events/mod.rs b/data_model/src/events/mod.rs index f0208da4bc0..20705c4cbd6 100644 --- a/data_model/src/events/mod.rs +++ b/data_model/src/events/mod.rs @@ -96,35 +96,6 @@ mod model { /// Listen to trigger completion event with filter. TriggerCompleted(trigger_completed::TriggerCompletedEventFilter), } - - /// Event filter which could be attached to trigger. - #[allow(variant_size_differences)] - #[derive( - Debug, - Clone, - PartialEq, - Eq, - PartialOrd, - Ord, - FromVariant, - Decode, - Encode, - Deserialize, - Serialize, - IntoSchema, - )] - // TODO: Temporarily made opaque - #[ffi_type(opaque)] - pub enum TriggeringEventFilterBox { - /// Listen to pipeline events with filter. - Pipeline(pipeline::PipelineEventFilterBox), - /// Listen to data events with filter. - Data(data::DataEventFilter), - /// Listen to time events with filter. - Time(time::TimeEventFilter), - /// Listen to trigger execution event with filter. - ExecuteTrigger(execute_trigger::ExecuteTriggerEventFilter), - } } impl From for EventBox { @@ -232,32 +203,6 @@ impl EventFilter for EventFilterBox { } } -#[cfg(feature = "transparent_api")] -impl EventFilter for TriggeringEventFilterBox { - type Event = EventBox; - - /// Apply filter to event. - fn matches(&self, event: &EventBox) -> bool { - match (event, self) { - (EventBox::Pipeline(event), Self::Pipeline(filter)) => filter.matches(event), - (EventBox::Data(event), Self::Data(filter)) => filter.matches(event), - (EventBox::Time(event), Self::Time(filter)) => filter.matches(event), - (EventBox::ExecuteTrigger(event), Self::ExecuteTrigger(filter)) => { - filter.matches(event) - } - // Fail to compile in case when new variant to event or filter is added - ( - EventBox::Pipeline(_) - | EventBox::Data(_) - | EventBox::Time(_) - | EventBox::ExecuteTrigger(_) - | EventBox::TriggerCompleted(_), - Self::Pipeline(_) | Self::Data(_) | Self::Time(_) | Self::ExecuteTrigger(_), - ) => false, - } - } -} - mod conversions { use super::{ pipeline::{BlockEventFilter, TransactionEventFilter}, @@ -300,19 +245,6 @@ mod conversions { ConfigurationEventFilter => DataEventFilter => EventFilterBox, ExecutorEventFilter => DataEventFilter => EventFilterBox, - PeerEventFilter => DataEventFilter => TriggeringEventFilterBox, - DomainEventFilter => DataEventFilter => TriggeringEventFilterBox, - AccountEventFilter => DataEventFilter => TriggeringEventFilterBox, - AssetEventFilter => DataEventFilter => TriggeringEventFilterBox, - AssetDefinitionEventFilter => DataEventFilter => TriggeringEventFilterBox, - TriggerEventFilter => DataEventFilter => TriggeringEventFilterBox, - RoleEventFilter => DataEventFilter => TriggeringEventFilterBox, - ConfigurationEventFilter => DataEventFilter => TriggeringEventFilterBox, - ExecutorEventFilter => DataEventFilter => TriggeringEventFilterBox, - - TransactionEventFilter => PipelineEventFilterBox => TriggeringEventFilterBox, - BlockEventFilter => PipelineEventFilterBox => TriggeringEventFilterBox, - TransactionEventFilter => PipelineEventFilterBox => EventFilterBox, BlockEventFilter => PipelineEventFilterBox => EventFilterBox, } @@ -361,7 +293,6 @@ pub mod prelude { pub use super::EventFilter; pub use super::{ data::prelude::*, execute_trigger::prelude::*, pipeline::prelude::*, time::prelude::*, - trigger_completed::prelude::*, EventBox, EventFilterBox, TriggeringEventFilterBox, - TriggeringEventType, + trigger_completed::prelude::*, EventBox, EventFilterBox, TriggeringEventType, }; } diff --git a/data_model/src/isi.rs b/data_model/src/isi.rs index 60d6f2772ca..74a2ac1fbec 100644 --- a/data_model/src/isi.rs +++ b/data_model/src/isi.rs @@ -1467,10 +1467,6 @@ pub mod error { pub enum InvalidParameterError { /// Invalid WASM binary: {0} Wasm(String), - /// Name length violation - /// - /// i.e. too long [`AccountId`] - NameLength, /// Attempt to register a time-trigger with `start` point in the past TimeTriggerInThePast, } diff --git a/data_model/src/query/mod.rs b/data_model/src/query/mod.rs index 4520069e0b3..12bf6e11e05 100644 --- a/data_model/src/query/mod.rs +++ b/data_model/src/query/mod.rs @@ -32,7 +32,7 @@ use self::{ use crate::{ account::{Account, AccountId}, block::{BlockHeader, SignedBlock}, - events::TriggeringEventFilterBox, + events::EventFilterBox, seal, transaction::{CommittedTransaction, SignedTransaction, TransactionPayload}, IdBox, Identifiable, IdentifiableBox, @@ -1031,7 +1031,7 @@ pub mod trigger { use crate::{ account::AccountId, domain::prelude::*, - events::TriggeringEventFilterBox, + events::EventFilterBox, prelude::InstructionBox, trigger::{Trigger, TriggerId}, Executable, Identifiable, Name, diff --git a/data_model/src/trigger.rs b/data_model/src/trigger.rs index 8893ff88084..316755f5d95 100644 --- a/data_model/src/trigger.rs +++ b/data_model/src/trigger.rs @@ -115,9 +115,7 @@ pub mod action { /// triggers without gaps, the `Executable` wrapped in the action must /// be run before any of the ISIs are pushed into the queue of the /// next block. - #[derive( - Debug, Clone, PartialEq, Eq, Decode, Encode, Deserialize, Serialize, IntoSchema, - )] + #[derive(Debug, Clone, PartialEq, Eq, Encode, Serialize, IntoSchema)] #[ffi_type] pub struct Action { /// The executable linked to this action @@ -129,7 +127,7 @@ pub mod action { /// Account executing this action pub authority: AccountId, /// Defines events which trigger the `Action` - pub filter: TriggeringEventFilterBox, + pub filter: EventFilterBox, /// Metadata used as persistent storage for trigger data. pub metadata: Metadata, } @@ -170,27 +168,34 @@ pub mod action { &self.authority } /// Defines events which trigger the `Action` - pub fn filter(&self) -> &TriggeringEventFilterBox { + pub fn filter(&self) -> &EventFilterBox { &self.filter } } impl Action { /// Construct an action given `executable`, `repeats`, `authority` and `filter`. + /// Filter of type [`EventFilterBox::TriggerCompleted`] is not allowed. + /// + /// # Panics + /// + /// - if filter matches [`EventFilterBox::TriggerCompleted`] pub fn new( executable: impl Into, repeats: impl Into, authority: AccountId, - filter: impl Into, + filter: impl Into, ) -> Self { - Self { + let action = candidate::ActionCandidate { executable: executable.into(), repeats: repeats.into(), // TODO: At this point the authority is meaningless. authority, filter: filter.into(), metadata: Metadata::default(), - } + }; + + action.validate().unwrap() } /// Add [`Metadata`] to the trigger replacing previously defined @@ -244,6 +249,57 @@ pub mod action { } } + mod candidate { + use parity_scale_codec::Input; + + use super::*; + + #[derive(Decode, Deserialize)] + pub(super) struct ActionCandidate { + pub executable: Executable, + pub repeats: Repeats, + pub authority: AccountId, + pub filter: EventFilterBox, + pub metadata: Metadata, + } + + impl ActionCandidate { + pub(super) fn validate(self) -> Result { + if matches!(self.filter, EventFilterBox::TriggerCompleted(_)) { + return Err("TriggerCompleted cannot be used as filter for triggering actions"); + } + + Ok(Action { + executable: self.executable, + repeats: self.repeats, + authority: self.authority, + filter: self.filter, + metadata: self.metadata, + }) + } + } + + impl Decode for Action { + fn decode(input: &mut I) -> Result { + ActionCandidate::decode(input)? + .validate() + .map_err(Into::into) + } + } + impl<'de> Deserialize<'de> for Action { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + use serde::de::Error as _; + + ActionCandidate::deserialize(deserializer)? + .validate() + .map_err(D::Error::custom) + } + } + } + pub mod prelude { //! Re-exports of commonly used types. pub use super::{Action, Repeats}; diff --git a/docs/source/references/schema.json b/docs/source/references/schema.json index e6a73f5bedc..2ea1e61e437 100644 --- a/docs/source/references/schema.json +++ b/docs/source/references/schema.json @@ -167,7 +167,7 @@ }, { "name": "filter", - "type": "TriggeringEventFilterBox" + "type": "EventFilterBox" }, { "name": "metadata", @@ -2227,13 +2227,9 @@ "discriminant": 0, "type": "String" }, - { - "tag": "NameLength", - "discriminant": 1 - }, { "tag": "TimeTriggerInThePast", - "discriminant": 2 + "discriminant": 1 } ] }, @@ -4340,30 +4336,6 @@ } ] }, - "TriggeringEventFilterBox": { - "Enum": [ - { - "tag": "Pipeline", - "discriminant": 0, - "type": "PipelineEventFilterBox" - }, - { - "tag": "Data", - "discriminant": 1, - "type": "DataEventFilter" - }, - { - "tag": "Time", - "discriminant": 2, - "type": "TimeEventFilter" - }, - { - "tag": "ExecuteTrigger", - "discriminant": 3, - "type": "ExecuteTriggerEventFilter" - } - ] - }, "TypeError": { "Enum": [ { diff --git a/schema/gen/src/lib.rs b/schema/gen/src/lib.rs index 83ca017e11e..38bf08080c3 100644 --- a/schema/gen/src/lib.rs +++ b/schema/gen/src/lib.rs @@ -400,7 +400,6 @@ types!( TriggerEventSet, TriggerId, TriggerNumberOfExecutionsChanged, - TriggeringEventFilterBox, TypeError, Unregister, Unregister,