refactor!: move multisig logics to custom instructions

BREAKING CHANGES: - (api-changes) `MultisigRegister` `MultisigPropose` `MultisigApprove` custom instructions Signed-off-by: Shunkichi Sato <[email protected]>
hyperledger-iroha · Nov 2, 2024 · a60a933 · a60a933
1 parent 508da4b
commit a60a933
Show file tree

Hide file tree

Showing 28 changed files with 726 additions and 1,026 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/iroha/tests/multisig.rs b/crates/iroha/tests/multisig.rs
diff --git a/crates/iroha_cli/src/main.rs b/crates/iroha_cli/src/main.rs
@@ -1179,9 +1179,7 @@ mod json {
 mod multisig {
     use std::io::{BufReader, Read as _};
 
-    use iroha::multisig_data_model::{
-        MultisigAccountArgs, MultisigTransactionArgs, DEFAULT_MULTISIG_TTL_MS,
-    };
+    use iroha::multisig_data_model::*;
 
     use super::*;
 
@@ -1190,7 +1188,7 @@ mod multisig {
     pub enum Args {
         /// Register a multisig account
         Register(Register),
-        /// Propose a multisig transaction
+        /// Propose a multisig transaction, with `Vec<InstructionBox>` stdin
         Propose(Propose),
         /// Approve a multisig transaction
         Approve(Approve),
@@ -1230,30 +1228,18 @@ mod multisig {
 
     impl RunArgs for Register {
         fn run(self, context: &mut dyn RunContext) -> Result<()> {
-            let Self {
-                account,
-                signatories,
-                weights,
-                quorum,
-                transaction_ttl,
-            } = self;
-            if signatories.len() != weights.len() {
+            if self.signatories.len() != self.weights.len() {
                 return Err(eyre!("signatories and weights must be equal in length"));
             }
-            let registry_id: TriggerId = format!("multisig_accounts_{}", account.domain())
-                .parse()
-                .unwrap();
-            let args = MultisigAccountArgs {
-                account: account.signatory().clone(),
-                signatories: signatories.into_iter().zip(weights).collect(),
-                quorum,
-                transaction_ttl_ms: transaction_ttl
+            let register_multisig_account = MultisigRegister::new(
+                self.account,
+                self.signatories.into_iter().zip(self.weights).collect(),
+                self.quorum,
+                self.transaction_ttl
                     .as_millis()
                     .try_into()
                     .expect("ttl must be within 584942417 years"),
-            };
-            let register_multisig_account =
-                iroha::data_model::isi::ExecuteTrigger::new(registry_id).with_args(&args);
+            );
 
             submit([register_multisig_account], Metadata::default(), context)
                 .wrap_err("Failed to register multisig account")
@@ -1270,14 +1256,6 @@ mod multisig {
 
     impl RunArgs for Propose {
         fn run(self, context: &mut dyn RunContext) -> Result<()> {
-            let Self { account } = self;
-            let registry_id: TriggerId = format!(
-                "multisig_transactions_{}_{}",
-                account.signatory(),
-                account.domain()
-            )
-            .parse()
-            .unwrap();
             let instructions: Vec<InstructionBox> = {
                 let mut reader = BufReader::new(stdin());
                 let mut raw_content = Vec::new();
@@ -1287,9 +1265,7 @@ mod multisig {
             };
             let instructions_hash = HashOf::new(&instructions);
             println!("{instructions_hash}");
-            let args = MultisigTransactionArgs::Propose(instructions);
-            let propose_multisig_transaction =
-                iroha::data_model::isi::ExecuteTrigger::new(registry_id).with_args(&args);
+            let propose_multisig_transaction = MultisigPropose::new(self.account, instructions);
 
             submit([propose_multisig_transaction], Metadata::default(), context)
                 .wrap_err("Failed to propose transaction")
@@ -1309,20 +1285,8 @@ mod multisig {
 
     impl RunArgs for Approve {
         fn run(self, context: &mut dyn RunContext) -> Result<()> {
-            let Self {
-                account,
-                instructions_hash,
-            } = self;
-            let registry_id: TriggerId = format!(
-                "multisig_transactions_{}_{}",
-                account.signatory(),
-                account.domain()
-            )
-            .parse()
-            .unwrap();
-            let args = MultisigTransactionArgs::Approve(instructions_hash);
             let approve_multisig_transaction =
-                iroha::data_model::isi::ExecuteTrigger::new(registry_id).with_args(&args);
+                MultisigApprove::new(self.account, self.instructions_hash);
 
             submit([approve_multisig_transaction], Metadata::default(), context)
                 .wrap_err("Failed to approve transaction")
@@ -1353,42 +1317,27 @@ mod multisig {
     ) -> Result<()> {
         let Ok(multisig_roles) = client
             .query(FindRolesByAccountId::new(account))
-            .filter_with(|role_id| role_id.name.starts_with("multisig_signatory_"))
+            .filter_with(|role_id| role_id.name.starts_with(MULTISIG_SIGNATORY_))
             .execute_all()
         else {
             return Ok(());
         };
 
         for role_id in multisig_roles {
-            let super_account: AccountId = role_id
-                .name()
-                .as_ref()
-                .strip_prefix("multisig_signatory_")
-                .unwrap()
-                .replacen('_', "@", 1)
-                .parse()
-                .unwrap();
-
-            trace_back_from(super_account, client, context)?;
-
-            let transactions_registry_id: TriggerId = role_id
-                .name()
-                .as_ref()
-                .replace("signatory", "transactions")
-                .parse()
-                .unwrap();
-
-            context.print_data(&transactions_registry_id)?;
-
-            let transactions_registry = client
-                .query(FindTriggers::new())
-                .filter_with(|trigger| trigger.id.eq(transactions_registry_id))
+            let super_account_id: AccountId = multisig_account_from(&role_id).unwrap();
+
+            trace_back_from(super_account_id.clone(), client, context)?;
+
+            context.print_data(&super_account_id)?;
+
+            let super_account = client
+                .query(FindAccounts)
+                .filter_with(|account| account.id.eq(super_account_id))
                 .execute_single()?;
-            let proposal_kvs = transactions_registry
-                .action()
+            let proposal_kvs = super_account
                 .metadata()
                 .iter()
-                .filter(|kv| kv.0.as_ref().starts_with("proposals"));
+                .filter(|kv| kv.0.as_ref().starts_with(PROPOSALS));
 
             proposal_kvs.fold("", |acc, (k, v)| {
                 let mut path = k.as_ref().split('/');

diff --git a/crates/iroha_executor/Cargo.toml b/crates/iroha_executor/Cargo.toml
@@ -17,6 +17,7 @@ debug = ["iroha_smart_contract/debug"]
 iroha_executor_derive = { path = "../iroha_executor_derive" }
 
 iroha_executor_data_model.workspace = true
+iroha_multisig_data_model.workspace = true
 iroha_smart_contract_utils.workspace = true
 iroha_smart_contract.workspace = true
 iroha_schema.workspace = true

diff --git a/crates/iroha_executor/src/default.rs b/crates/iroha_executor/src/default.rs
@@ -1167,7 +1167,8 @@ pub mod parameter {
 }
 
 pub mod role {
-    use iroha_executor_data_model::permission::{role::CanManageRoles, trigger::CanExecuteTrigger};
+    use iroha_executor_data_model::permission::role::CanManageRoles;
+    use iroha_multisig_data_model::multisig_account_from;
     use iroha_smart_contract::{data_model::role::Role, Iroha};
 
     use super::*;
@@ -1239,36 +1240,20 @@ pub mod role {
 
         // Exception for multisig roles
         let mut is_multisig_role = false;
-        if let Some(tail) = role
-            .id()
-            .name()
-            .as_ref()
-            .strip_prefix("multisig_signatory_")
-        {
-            let Ok(account_id) = tail.replacen('_', "@", 1).parse::<AccountId>() else {
-                deny!(executor, "Violates multisig role format")
-            };
+        if let Some(multisig_account) = multisig_account_from(role.id()) {
             if crate::permission::domain::is_domain_owner(
-                account_id.domain(),
+                multisig_account.domain(),
                 &executor.context().authority,
                 executor.host(),
             )
             .unwrap_or_default()
             {
-                // Bind this role to this permission here, regardless of the given contains
-                let permission = CanExecuteTrigger {
-                    trigger: format!(
-                        "multisig_transactions_{}_{}",
-                        account_id.signatory(),
-                        account_id.domain()
-                    )
-                    .parse()
-                    .unwrap(),
-                };
-                new_role = new_role.add_permission(permission);
                 is_multisig_role = true;
             } else {
-                deny!(executor, "Can't register multisig role")
+                deny!(
+                    executor,
+                    "role name conflicts with the reserved multisig role names"
+                )
             }
         }
 
@@ -1374,37 +1359,6 @@ pub mod trigger {
         let trigger = isi.object();
         let is_genesis = executor.context().curr_block.is_genesis();
 
-        let trigger_name = trigger.id().name().as_ref();
-
-        #[expect(clippy::option_if_let_else)] // clippy suggestion spoils readability
-        let naming_is_ok = if let Some(tail) = trigger_name.strip_prefix("multisig_accounts_") {
-            let system_account: AccountId =
-                // predefined in `GenesisBuilder::default`
-                "ed0120D8B64D62FD8E09B9F29FE04D9C63E312EFB1CB29F1BF6AF00EBC263007AE75F7@system"
-                    .parse()
-                    .unwrap();
-            tail.parse::<DomainId>().is_ok()
-                && (is_genesis || executor.context().authority == system_account)
-        } else if let Some(tail) = trigger_name.strip_prefix("multisig_transactions_") {
-            tail.replacen('_', "@", 1)
-                .parse::<AccountId>()
-                .ok()
-                .and_then(|account_id| {
-                    is_domain_owner(
-                        account_id.domain(),
-                        &executor.context().authority,
-                        executor.host(),
-                    )
-                    .ok()
-                })
-                .unwrap_or_default()
-        } else {
-            true
-        };
-        if !naming_is_ok {
-            deny!(executor, "Violates trigger naming restrictions");
-        }
-
         if is_genesis
             || {
                 match is_domain_owner(
@@ -1557,20 +1511,6 @@ pub mod trigger {
         if can_execute_trigger_token.is_owned_by(authority, executor.host()) {
             execute!(executor, isi);
         }
-        // Any account in domain can call multisig accounts registry to register any multisig account in the domain
-        // TODO Restrict access to the multisig signatories?
-        // TODO Impose proposal and approval process?
-        if trigger_id
-            .name()
-            .as_ref()
-            .strip_prefix("multisig_accounts_")
-            .and_then(|s| s.parse::<DomainId>().ok())
-            .map_or(false, |registry_domain| {
-                *authority.domain() == registry_domain
-            })
-        {
-            execute!(executor, isi);
-        }
 
         deny!(executor, "Can't execute trigger owned by another account");
     }

diff --git a/crates/iroha_executor/src/lib.rs b/crates/iroha_executor/src/lib.rs
@@ -265,6 +265,10 @@ pub trait Execute {
     /// Represents the current state of the world
     fn context(&self) -> &prelude::Context;
 
+    /// Mutable context for e.g. switching to another authority after validation before execution.
+    /// Note that mutations are persistent to the instance unless reset
+    fn context_mut(&mut self) -> &mut prelude::Context;
+
     /// Executor verdict.
     fn verdict(&self) -> &Result;
 
@@ -282,8 +286,8 @@ pub mod prelude {
 
     pub use super::{
         data_model::{
-            executor::Result, smart_contract::payloads::ExecutorContext as Context, visit::Visit,
-            ValidationFail,
+            executor::Result, isi::Instruction,
+            smart_contract::payloads::ExecutorContext as Context, visit::Visit, ValidationFail,
         },
         deny, execute, DataModelBuilder, Execute,
     };

diff --git a/crates/iroha_executor_derive/src/default.rs b/crates/iroha_executor_derive/src/default.rs
@@ -235,6 +235,10 @@ pub fn impl_derive_execute(emitter: &mut Emitter, input: &syn::DeriveInput) -> T
                 &self.context
             }
 
+            fn context_mut(&mut self) -> &mut ::iroha_executor::prelude::Context {
+                &mut self.context
+            }
+
             fn verdict(&self) -> &::iroha_executor::prelude::Result {
                 &self.verdict
             }

diff --git a/crates/iroha_genesis/src/lib.rs b/crates/iroha_genesis/src/lib.rs
@@ -251,31 +251,22 @@ impl GenesisBuilder {
 
     /// Entry system entities to serve standard functionality.
     pub fn install_libs(self) -> Self {
-        // Register a trigger that reacts to domain creation (or owner changes) and registers (or replaces) a multisig accounts registry for the domain
-        let multisig_domains_initializer = GenesisWasmTrigger::new(
-            "multisig_domains".parse().unwrap(),
-            GenesisWasmAction::new(
-                "multisig_domains.wasm",
-                Repeats::Indefinitely,
-                SYSTEM_ACCOUNT_ID.clone(),
-                DomainEventFilter::new()
-                    .for_events(DomainEventSet::Created | DomainEventSet::OwnerChanged),
-            ),
-        );
         let instructions = vec![
             Register::domain(Domain::new(SYSTEM_DOMAIN_ID.clone())).into(),
             Register::account(Account::new(SYSTEM_ACCOUNT_ID.clone())).into(),
             Grant::account_permission(CanRegisterAnyTrigger, SYSTEM_ACCOUNT_ID.clone()).into(),
             Grant::account_permission(CanUnregisterAnyTrigger, SYSTEM_ACCOUNT_ID.clone()).into(),
         ];
 
+        let wasm_triggers = vec![];
+
         Self {
             chain: self.chain,
             executor: self.executor,
             parameters: self.parameters,
             instructions,
             wasm_dir: self.wasm_dir,
-            wasm_triggers: vec![multisig_domains_initializer],
+            wasm_triggers,
             topology: self.topology,
         }
     }