From e37bb4d5a9587d3245276e26905bec7c9a3d8377 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Fri, 1 Nov 2024 17:19:42 +0900 Subject: [PATCH 01/28] refactor!: move multisig logics to custom instructions BREAKING CHANGES: - (api-changes) `MultisigRegister` `MultisigPropose` `MultisigApprove` custom instructions Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- Cargo.lock | 2 + crates/iroha/tests/multisig.rs | 296 +++++------------- crates/iroha_cli/src/main.rs | 97 ++---- crates/iroha_executor/Cargo.toml | 1 + crates/iroha_executor/src/default.rs | 76 +---- crates/iroha_executor/src/lib.rs | 8 +- crates/iroha_executor_derive/src/default.rs | 4 + crates/iroha_genesis/src/lib.rs | 15 +- crates/iroha_kagami/src/genesis/generate.rs | 22 +- crates/iroha_multisig_data_model/Cargo.toml | 1 + crates/iroha_multisig_data_model/src/lib.rs | 150 +++++++-- crates/iroha_schema_gen/src/lib.rs | 18 +- crates/iroha_test_network/src/lib.rs | 2 +- defaults/genesis.json | 36 +-- docs/source/references/schema.json | 59 +++- scripts/build_wasm.sh | 3 - scripts/tests/instructions.json | 2 +- scripts/tests/multisig.recursion.sh | 13 +- wasm/libs/default_executor/Cargo.toml | 1 + wasm/libs/default_executor/src/lib.rs | 48 ++- .../default_executor/src/multisig/account.rs | 103 ++++++ .../libs/default_executor/src/multisig/mod.rs | 34 ++ .../src/multisig/transaction.rs | 267 ++++++++++++++++ wasm/libs/multisig_accounts/Cargo.toml | 23 -- wasm/libs/multisig_accounts/src/lib.rs | 155 --------- wasm/libs/multisig_domains/Cargo.toml | 22 -- wasm/libs/multisig_domains/src/lib.rs | 82 ----- wasm/libs/multisig_transactions/Cargo.toml | 22 -- wasm/libs/multisig_transactions/src/lib.rs | 233 -------------- 29 files changed, 751 insertions(+), 1044 deletions(-) create mode 100644 wasm/libs/default_executor/src/multisig/account.rs create mode 100644 wasm/libs/default_executor/src/multisig/mod.rs create mode 100644 wasm/libs/default_executor/src/multisig/transaction.rs delete mode 100644 wasm/libs/multisig_accounts/Cargo.toml delete mode 100644 wasm/libs/multisig_accounts/src/lib.rs delete mode 100644 wasm/libs/multisig_domains/Cargo.toml delete mode 100644 wasm/libs/multisig_domains/src/lib.rs delete mode 100644 wasm/libs/multisig_transactions/Cargo.toml delete mode 100644 wasm/libs/multisig_transactions/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index bd767492c67..d4aff00ed1e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3197,6 +3197,7 @@ version = "2.0.0-rc.1.0" dependencies = [ "iroha_executor_data_model", "iroha_executor_derive", + "iroha_multisig_data_model", "iroha_schema", "iroha_smart_contract", "iroha_smart_contract_utils", @@ -3370,6 +3371,7 @@ dependencies = [ name = "iroha_multisig_data_model" version = "2.0.0-rc.1.0" dependencies = [ + "derive_more", "iroha_data_model", "iroha_schema", "parity-scale-codec", diff --git a/crates/iroha/tests/multisig.rs b/crates/iroha/tests/multisig.rs index 84c6c453f9a..84dbc4a61dd 100644 --- a/crates/iroha/tests/multisig.rs +++ b/crates/iroha/tests/multisig.rs @@ -7,10 +7,10 @@ use eyre::Result; use iroha::{ client::Client, crypto::KeyPair, - data_model::{prelude::*, query::trigger::FindTriggers, Level}, + data_model::{prelude::*, Level}, + multisig_data_model::*, }; -use iroha_data_model::events::execute_trigger::ExecuteTriggerEventFilter; -use iroha_multisig_data_model::{MultisigAccountArgs, MultisigTransactionArgs}; +use iroha_multisig_data_model::approvals_key; use iroha_test_network::*; use iroha_test_samples::{ gen_account_in, ALICE_ID, BOB_ID, BOB_KEYPAIR, CARPENTER_ID, CARPENTER_KEYPAIR, @@ -28,22 +28,12 @@ fn multisig_expires() -> Result<()> { /// # Scenario /// -/// Proceeds from top left to bottom right. Starred operations are the responsibility of the user -/// -/// ``` -/// | world level | domain level | account level | transaction level | -/// |---------------------------|-----------------------------|---------------------------------|----------------------| -/// | given domains initializer | | | | -/// | | * creates domain | | | -/// | domains initializer | generates accounts registry | | | -/// | | | * creates signatories | | -/// | | * calls accounts registry | generates multisig account | | -/// | | accounts registry | generates transactions registry | | -/// | | | * calls transactions registry | proposes transaction | -/// | | | * calls transactions registry | approves transaction | -/// | | | transactions registry | executes transaction | -/// ``` -#[allow(clippy::cast_possible_truncation)] +/// 1. Signatories are populated and ready to join a multisig account +/// 2. Someone in the domain registers a multisig account +/// 3. One of the signatories of the multisig account proposes a multisig transaction +/// 4. Other signatories approve the multisig transaction +/// 5. When the quorum is met, if it is not expired, the multisig transaction executes +#[expect(clippy::cast_possible_truncation)] fn multisig_base(transaction_ttl_ms: Option) -> Result<()> { const N_SIGNATORIES: usize = 5; @@ -59,17 +49,6 @@ fn multisig_base(transaction_ttl_ms: Option) -> Result<()> { ]; test_client.submit_all_blocking(register_and_transfer_kingdom)?; - // One more block to generate a multisig accounts registry for the domain - test_client.submit_blocking(Log::new(Level::DEBUG, "Just ticking time".to_string()))?; - - // Check that the multisig accounts registry has been generated - let multisig_accounts_registry_id = multisig_accounts_registry_of(&kingdom); - let _trigger = test_client - .query(FindTriggers::new()) - .filter_with(|trigger| trigger.id.eq(multisig_accounts_registry_id.clone())) - .execute_single() - .expect("multisig accounts registry should be generated after domain creation"); - // Populate residents in the domain let mut residents = core::iter::repeat_with(|| gen_account_in(&kingdom)) .take(1 + N_SIGNATORIES) @@ -83,24 +62,26 @@ fn multisig_base(transaction_ttl_ms: Option) -> Result<()> { )?; // Create a multisig account ID and discard the corresponding private key + // FIXME #5022 refuse user input to prevent multisig monopoly and pre-registration hijacking let multisig_account_id = gen_account_in(&kingdom).0; + // DEBUG: You could target unauthorized one (e.g. Alice) to fail + let transaction_target = multisig_account_id.clone(); + let not_signatory = residents.pop_first().unwrap(); let mut signatories = residents; - let args = &MultisigAccountArgs { - account: multisig_account_id.signatory().clone(), - signatories: signatories + let register_multisig_account = MultisigRegister::new( + multisig_account_id.clone(), + signatories .keys() .enumerate() .map(|(weight, id)| (id.clone(), 1 + weight as u8)) .collect(), - // Can be met without the first signatory - quorum: (1..=N_SIGNATORIES).skip(1).sum::() as u16, - transaction_ttl_ms: transaction_ttl_ms.unwrap_or(u64::MAX), - }; - let register_multisig_account = - ExecuteTrigger::new(multisig_accounts_registry_id).with_args(args); + // Quorum can be reached without the first signatory + (1..=N_SIGNATORIES).skip(1).sum::() as u16, + transaction_ttl_ms.unwrap_or(u64::MAX), + ); // Any account in another domain cannot register a multisig account without special permission let _err = alt_client( @@ -120,30 +101,22 @@ fn multisig_base(transaction_ttl_ms: Option) -> Result<()> { .query(FindAccounts::new()) .filter_with(|account| account.id.eq(multisig_account_id.clone())) .execute_single() - .expect("multisig account should be created by calling the multisig accounts registry"); + .expect("multisig account should be created"); - // Check that the multisig transactions registry has been generated - let multisig_transactions_registry_id = multisig_transactions_registry_of(&multisig_account_id); - let _trigger = test_client - .query(FindTriggers::new()) - .filter_with(|trigger| trigger.id.eq(multisig_transactions_registry_id.clone())) - .execute_single() - .expect("multisig transactions registry should be generated along with the corresponding multisig account"); - - let key: Name = "key".parse().unwrap(); + let key: Name = "success_marker".parse().unwrap(); let instructions = vec![SetKeyValue::account( - multisig_account_id.clone(), + transaction_target.clone(), key.clone(), - "value".parse::().unwrap(), + "congratulations".parse::().unwrap(), ) .into()]; let instructions_hash = HashOf::new(&instructions); let proposer = signatories.pop_last().unwrap(); - let approvers = signatories; + // All but the first signatory approve the proposal + let mut approvers = signatories.into_iter().skip(1); - let args = &MultisigTransactionArgs::Propose(instructions); - let propose = ExecuteTrigger::new(multisig_transactions_registry_id.clone()).with_args(args); + let propose = MultisigPropose::new(multisig_account_id.clone(), instructions); alt_client(proposer, &test_client).submit_blocking(propose)?; @@ -153,7 +126,7 @@ fn multisig_base(transaction_ttl_ms: Option) -> Result<()> { multisig_account_id.clone(), key.clone(), )) - .expect_err("key-value shouldn't be set without enough approvals"); + .expect_err("instructions shouldn't execute without enough approvals"); // Allow time to elapse to test the expiration if let Some(ms) = transaction_ttl_ms { @@ -161,24 +134,24 @@ fn multisig_base(transaction_ttl_ms: Option) -> Result<()> { }; test_client.submit_blocking(Log::new(Level::DEBUG, "Just ticking time".to_string()))?; - // All but the first signatory approve the multisig transaction - for approver in approvers.into_iter().skip(1) { - let args = &MultisigTransactionArgs::Approve(instructions_hash); - let approve = - ExecuteTrigger::new(multisig_transactions_registry_id.clone()).with_args(args); + let approve = MultisigApprove::new(multisig_account_id.clone(), instructions_hash); + + // Approve once to see if the proposal expires + let approver = approvers.next().unwrap(); + alt_client(approver, &test_client).submit_blocking(approve.clone())?; - alt_client(approver, &test_client).submit_blocking(approve)?; + // Subsequent approvals should succeed unless the proposal is expired + for approver in approvers { + match alt_client(approver, &test_client).submit_blocking(approve.clone()) { + Ok(_hash) => assert!(transaction_ttl_ms.is_none()), + Err(_err) => assert!(transaction_ttl_ms.is_some()), + } } - // Check that the multisig transaction has executed - let res = test_client.query_single(FindAccountMetadata::new( - multisig_account_id.clone(), - key.clone(), - )); - if transaction_ttl_ms.is_some() { - let _err = res.expect_err("key-value shouldn't be set despite enough approvals"); - } else { - res.expect("key-value should be set with enough approvals"); + // Check if the multisig transaction has executed + match test_client.query_single(FindAccountMetadata::new(transaction_target, key.clone())) { + Ok(_value) => assert!(transaction_ttl_ms.is_none()), + Err(_err) => assert!(transaction_ttl_ms.is_some()), } Ok(()) @@ -196,13 +169,12 @@ fn multisig_base(transaction_ttl_ms: Option) -> Result<()> { /// 0 1 2 3 4 5 <--- personal signatories /// ``` #[test] -#[allow(clippy::similar_names, clippy::too_many_lines)] +#[expect(clippy::similar_names)] fn multisig_recursion() -> Result<()> { let (network, _rt) = NetworkBuilder::new().start_blocking()?; let test_client = network.client(); let wonderland = "wonderland"; - let ms_accounts_registry_id = multisig_accounts_registry_of(&wonderland.parse().unwrap()); // Populate signatories in the domain let signatories = core::iter::repeat_with(|| gen_account_in(wonderland)) @@ -227,14 +199,12 @@ fn multisig_recursion() -> Result<()> { .into_iter() .map(|sigs| { let ms_account_id = gen_account_in(wonderland).0; - let args = MultisigAccountArgs { - account: ms_account_id.signatory().clone(), - signatories: sigs.iter().copied().map(|id| (id.clone(), 1)).collect(), - quorum: sigs.len().try_into().unwrap(), - transaction_ttl_ms: u64::MAX, - }; - let register_ms_account = - ExecuteTrigger::new(ms_accounts_registry_id.clone()).with_args(&args); + let register_ms_account = MultisigRegister::new( + ms_account_id.clone(), + sigs.iter().copied().map(|id| (id.clone(), 1)).collect(), + sigs.len().try_into().unwrap(), + u64::MAX, + ); test_client .submit_blocking(register_ms_account) @@ -264,55 +234,38 @@ fn multisig_recursion() -> Result<()> { let msa_012345 = msas[0].clone(); // One of personal signatories proposes a multisig transaction - let key: Name = "key".parse().unwrap(); + let key: Name = "success_marker".parse().unwrap(); let instructions = vec![SetKeyValue::account( msa_012345.clone(), key.clone(), - "value".parse::().unwrap(), + "congratulations".parse::().unwrap(), ) .into()]; let instructions_hash = HashOf::new(&instructions); let proposer = sigs_0.pop_last().unwrap(); - let ms_transactions_registry_id = multisig_transactions_registry_of(&msa_012345); - let args = MultisigTransactionArgs::Propose(instructions); - let propose = ExecuteTrigger::new(ms_transactions_registry_id.clone()).with_args(&args); + let propose = MultisigPropose::new(msa_012345.clone(), instructions); alt_client(proposer, &test_client).submit_blocking(propose)?; - // Ticks as many times as the multisig recursion - (0..2).for_each(|_| { - test_client - .submit_blocking(Log::new(Level::DEBUG, "Just ticking time".to_string())) - .unwrap(); - }); - - // Check that the entire authentication policy has been deployed down to one of the leaf registries + // Check that the entire authentication policy has been deployed down to one of the leaf signatories let approval_hash_to_12345 = { let approval_hash_to_012345 = { - let registry_id = multisig_transactions_registry_of(&msa_012345); - let args = MultisigTransactionArgs::Approve(instructions_hash); - let approve: InstructionBox = ExecuteTrigger::new(registry_id.clone()) - .with_args(&args) - .into(); + let approve: InstructionBox = + MultisigApprove::new(msa_012345.clone(), instructions_hash).into(); HashOf::new(&vec![approve]) }; - let registry_id = multisig_transactions_registry_of(&msa_12345); - let args = MultisigTransactionArgs::Approve(approval_hash_to_012345); - let approve: InstructionBox = ExecuteTrigger::new(registry_id.clone()) - .with_args(&args) - .into(); + let approve: InstructionBox = + MultisigApprove::new(msa_12345.clone(), approval_hash_to_012345).into(); HashOf::new(&vec![approve]) }; let approvals_at_12: BTreeSet = test_client - .query_single(FindTriggerMetadata::new( - multisig_transactions_registry_of(&msa_12), - format!("proposals/{approval_hash_to_12345}/approvals") - .parse() - .unwrap(), + .query_single(FindAccountMetadata::new( + msa_12.clone(), + approvals_key(&approval_hash_to_12345), )) .expect("leaf approvals should be initialized by the root proposal") .try_into_any() @@ -323,16 +276,14 @@ fn multisig_recursion() -> Result<()> { // Check that the multisig transaction has not yet executed let _err = test_client .query_single(FindAccountMetadata::new(msa_012345.clone(), key.clone())) - .expect_err("key-value shouldn't be set without enough approvals"); + .expect_err("instructions shouldn't execute without enough approvals"); // All the rest signatories approve the multisig transaction let approve_for_each = |approvers: BTreeMap, instructions_hash: HashOf>, ms_account: &AccountId| { for approver in approvers { - let registry_id = multisig_transactions_registry_of(ms_account); - let args = MultisigTransactionArgs::Approve(instructions_hash); - let approve = ExecuteTrigger::new(registry_id.clone()).with_args(&args); + let approve = MultisigApprove::new(ms_account.clone(), instructions_hash); alt_client(approver, &test_client) .submit_blocking(approve) @@ -343,47 +294,10 @@ fn multisig_recursion() -> Result<()> { approve_for_each(sigs_12, approval_hash_to_12345, &msa_12); approve_for_each(sigs_345, approval_hash_to_12345, &msa_345); - // Let the intermediate registry (12345) collect approvals and approve the original proposal - test_client.submit_blocking(Log::new(Level::DEBUG, "Just ticking time".to_string()))?; - - // Let the root registry (012345) collect approvals and execute the original proposal - test_client.submit_blocking(Log::new(Level::DEBUG, "Just ticking time".to_string()))?; - // Check that the multisig transaction has executed test_client .query_single(FindAccountMetadata::new(msa_012345.clone(), key.clone())) - .expect("key-value should be set with enough approvals"); - - Ok(()) -} - -#[test] -fn persistent_domain_level_authority() -> Result<()> { - let (network, _rt) = NetworkBuilder::new().start_blocking()?; - let test_client = network.client(); - - let wonderland: DomainId = "wonderland".parse().unwrap(); - - let ms_accounts_registry_id = multisig_accounts_registry_of(&wonderland); - - // Domain owner changes from Alice to Bob - test_client.submit_blocking(Transfer::domain( - ALICE_ID.clone(), - wonderland, - BOB_ID.clone(), - ))?; - - // One block gap to follow the domain owner change - test_client.submit_blocking(Log::new(Level::DEBUG, "Just ticking time".to_string()))?; - - // Bob is the authority of the wonderland multisig accounts registry - let ms_accounts_registry = test_client - .query(FindTriggers::new()) - .filter_with(|trigger| trigger.id.eq(ms_accounts_registry_id.clone())) - .execute_single() - .expect("multisig accounts registry should survive before and after a domain owner change"); - - assert!(*ms_accounts_registry.action().authority() == BOB_ID.clone()); + .expect("instructions should execute with enough approvals"); Ok(()) } @@ -396,61 +310,12 @@ fn reserved_names() { let account_in_another_domain = gen_account_in("garden_of_live_flowers").0; { - let reserved_prefix = "multisig_accounts_"; let register = { - let id: TriggerId = format!("{reserved_prefix}{}", account_in_another_domain.domain()) - .parse() - .unwrap(); - let action = Action::new( - Vec::::new(), - Repeats::Indefinitely, - ALICE_ID.clone(), - ExecuteTriggerEventFilter::new(), - ); - Register::trigger(Trigger::new(id, action)) + let role = multisig_role_for(&account_in_another_domain); + Register::role(Role::new(role, ALICE_ID.clone())) }; let _err = test_client.submit_blocking(register).expect_err( - "trigger with this name shouldn't be registered by anyone other than multisig system", - ); - } - - { - let reserved_prefix = "multisig_transactions_"; - let register = { - let id: TriggerId = format!( - "{reserved_prefix}{}_{}", - account_in_another_domain.signatory(), - account_in_another_domain.domain() - ) - .parse() - .unwrap(); - let action = Action::new( - Vec::::new(), - Repeats::Indefinitely, - ALICE_ID.clone(), - ExecuteTriggerEventFilter::new(), - ); - Register::trigger(Trigger::new(id, action)) - }; - let _err = test_client.submit_blocking(register).expect_err( - "trigger with this name shouldn't be registered by anyone other than domain owner", - ); - } - - { - let reserved_prefix = "multisig_signatory_"; - let register = { - let id: RoleId = format!( - "{reserved_prefix}{}_{}", - account_in_another_domain.signatory(), - account_in_another_domain.domain() - ) - .parse() - .unwrap(); - Register::role(Role::new(id, ALICE_ID.clone())) - }; - let _err = test_client.submit_blocking(register).expect_err( - "role with this name shouldn't be registered by anyone other than domain owner", + "role with this name shouldn't be registered by anyone other than the domain owner", ); } } @@ -463,28 +328,13 @@ fn alt_client(signatory: (AccountId, KeyPair), base_client: &Client) -> Client { } } -fn multisig_accounts_registry_of(domain: &DomainId) -> TriggerId { - format!("multisig_accounts_{domain}",).parse().unwrap() -} - -fn multisig_transactions_registry_of(multisig_account: &AccountId) -> TriggerId { - format!( - "multisig_transactions_{}_{}", - multisig_account.signatory(), - multisig_account.domain() - ) - .parse() - .unwrap() -} - -#[allow(dead_code)] -fn debug_mst_registry(msa: &AccountId, client: &Client) { - let mst_registry = client - .query(FindTriggers::new()) - .filter_with(|trigger| trigger.id.eq(multisig_transactions_registry_of(msa))) +#[expect(dead_code)] +fn debug_account(account_id: &AccountId, client: &Client) { + let account = client + .query(FindAccounts) + .filter_with(|account| account.id.eq(account_id.clone())) .execute_single() .unwrap(); - let mst_metadata = mst_registry.action().metadata(); - iroha_logger::error!(%msa, ?mst_metadata); + iroha_logger::error!(?account); } diff --git a/crates/iroha_cli/src/main.rs b/crates/iroha_cli/src/main.rs index 41c8d252843..2d4f4e0ee28 100644 --- 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` 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 = { 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 index 40f012a1dad..780a7f66bab 100644 --- 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 index d3a3fa20029..c819fce0277 100644 --- 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::() 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::().is_ok() - && (is_genesis || executor.context().authority == system_account) - } else if let Some(tail) = trigger_name.strip_prefix("multisig_transactions_") { - tail.replacen('_', "@", 1) - .parse::() - .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::().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 index a35f01f0ec2..3df84bdb351 100644 --- 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 index c5def5c00b6..d6474d4763c 100644 --- 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 index 7d11ccd9f63..10bfd5301cb 100644 --- a/crates/iroha_genesis/src/lib.rs +++ b/crates/iroha_genesis/src/lib.rs @@ -251,17 +251,6 @@ 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(), @@ -269,13 +258,15 @@ impl GenesisBuilder { 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, } } diff --git a/crates/iroha_kagami/src/genesis/generate.rs b/crates/iroha_kagami/src/genesis/generate.rs index 441e294c8f2..3f39fb6e87d 100644 --- a/crates/iroha_kagami/src/genesis/generate.rs +++ b/crates/iroha_kagami/src/genesis/generate.rs @@ -9,9 +9,7 @@ use iroha_data_model::{isi::InstructionBox, parameter::Parameters, prelude::*}; use iroha_executor_data_model::permission::{ domain::CanRegisterDomain, parameter::CanSetParameters, }; -use iroha_genesis::{ - GenesisBuilder, GenesisWasmAction, GenesisWasmTrigger, RawGenesisTransaction, GENESIS_DOMAIN_ID, -}; +use iroha_genesis::{GenesisBuilder, RawGenesisTransaction, GENESIS_DOMAIN_ID}; use iroha_test_samples::{gen_account_in, ALICE_ID, BOB_ID, CARPENTER_ID}; use crate::{Outcome, RunArgs}; @@ -151,24 +149,6 @@ pub fn generate_default( builder = builder.append_instruction(isi); } - // Manually register a multisig accounts registry for wonderland whose creation in genesis does not trigger the initializer - let multisig_accounts_registry_for_wonderland = { - let domain_owner = ALICE_ID.clone(); - let registry_id = "multisig_accounts_wonderland".parse::().unwrap(); - - GenesisWasmTrigger::new( - registry_id.clone(), - GenesisWasmAction::new( - "multisig_accounts.wasm", - Repeats::Indefinitely, - domain_owner, - ExecuteTriggerEventFilter::new().for_trigger(registry_id), - ), - ) - }; - - builder = builder.append_wasm_trigger(multisig_accounts_registry_for_wonderland); - Ok(builder.build_raw()) } diff --git a/crates/iroha_multisig_data_model/Cargo.toml b/crates/iroha_multisig_data_model/Cargo.toml index a104d502956..2c9700de387 100644 --- a/crates/iroha_multisig_data_model/Cargo.toml +++ b/crates/iroha_multisig_data_model/Cargo.toml @@ -14,6 +14,7 @@ workspace = true iroha_data_model.workspace = true iroha_schema.workspace = true +derive_more = { workspace = true, features = ["constructor", "from"] } parity-scale-codec = { workspace = true, features = ["derive"] } serde.workspace = true serde_json.workspace = true diff --git a/crates/iroha_multisig_data_model/src/lib.rs b/crates/iroha_multisig_data_model/src/lib.rs index 6e83490cd62..c8b7fa66487 100644 --- a/crates/iroha_multisig_data_model/src/lib.rs +++ b/crates/iroha_multisig_data_model/src/lib.rs @@ -1,4 +1,4 @@ -//! Arguments attached on executing triggers for multisig accounts or transactions +//! Types for multisig operations #![no_std] @@ -6,14 +6,29 @@ extern crate alloc; use alloc::{collections::btree_map::BTreeMap, format, string::String, vec::Vec}; -use iroha_data_model::prelude::*; +use derive_more::{Constructor, From}; +use iroha_data_model::{ + isi::{CustomInstruction, Instruction, InstructionBox}, + prelude::{Json, *}, +}; use iroha_schema::IntoSchema; use parity_scale_codec::{Decode, Encode}; use serde::{Deserialize, Serialize}; -/// Arguments to register multisig account -#[derive(Debug, Clone, Decode, Encode, Serialize, Deserialize, IntoSchema)] -pub struct MultisigAccountArgs { +/// Multisig-related instructions +#[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, From)] +pub enum MultisigInstructionBox { + /// Register a multisig account, which is a prerequisite of multisig transactions + Register(MultisigRegister), + /// Propose a multisig transaction and initialize approvals with the proposer's one + Propose(MultisigPropose), + /// Approve a certain multisig transaction + Approve(MultisigApprove), +} + +/// Register a multisig account, which is a prerequisite of multisig transactions +#[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, Constructor)] +pub struct MultisigRegister { /// Multisig account to be registered ///
/// @@ -22,10 +37,10 @@ pub struct MultisigAccountArgs { ///
// FIXME #5022 prevent multisig monopoly // FIXME #5022 stop accepting user input: otherwise, after #4426 pre-registration account will be hijacked as a multisig account - pub account: PublicKey, - /// List of accounts and their relative weights of responsibility for the multisig + pub account: AccountId, + /// List of signatories and their relative weights of responsibility for the multisig account pub signatories: BTreeMap, - /// Threshold of total weight at which the multisig is considered authenticated + /// Threshold of total weight at which the multisig account is considered authenticated pub quorum: u16, /// Multisig transaction time-to-live in milliseconds based on block timestamps. Defaults to [`DEFAULT_MULTISIG_TTL_MS`] pub transaction_ttl_ms: u64, @@ -36,39 +51,110 @@ type Weight = u8; /// Default multisig transaction time-to-live in milliseconds based on block timestamps pub const DEFAULT_MULTISIG_TTL_MS: u64 = 60 * 60 * 1_000; // 1 hour -/// Arguments to propose or approve multisig transaction -#[derive(Debug, Clone, Decode, Encode, Serialize, Deserialize, IntoSchema)] -pub enum MultisigTransactionArgs { - /// Propose instructions and initialize approvals with the proposer's one - Propose(Vec), - /// Approve certain instructions - Approve(HashOf>), +/// Propose a multisig transaction and initialize approvals with the proposer's one +#[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, Constructor)] +pub struct MultisigPropose { + /// Multisig account to propose + pub account: AccountId, + /// Proposal contents + pub instructions: Vec, } -impl From for Json { - fn from(details: MultisigAccountArgs) -> Self { - Json::new(details) - } +/// Approve a certain multisig transaction +#[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, Constructor)] +pub struct MultisigApprove { + /// Multisig account to approve + pub account: AccountId, + /// Proposal to approve + pub instructions_hash: HashOf>, } -impl TryFrom<&Json> for MultisigAccountArgs { - type Error = serde_json::Error; +macro_rules! impl_custom_instruction { + ($box:ty, $($instruction:ty)|+) => { + impl Instruction for $box {} - fn try_from(payload: &Json) -> serde_json::Result { - serde_json::from_str::(payload.as_ref()) - } + impl From<$box> for InstructionBox { + fn from(value: $box) -> Self { + Self::Custom(value.into()) + } + } + + impl From<$box> for CustomInstruction { + fn from(value: $box) -> Self { + let payload = serde_json::to_value(&value) + .expect(concat!("INTERNAL BUG: Couldn't serialize ", stringify!($box))); + + Self::new(payload) + } + } + + impl TryFrom<&Json> for $box { + type Error = serde_json::Error; + + fn try_from(payload: &Json) -> serde_json::Result { + serde_json::from_str::(payload.as_ref()) + } + } + + $( + impl Instruction for $instruction {} + + impl From<$instruction> for InstructionBox { + fn from(value: $instruction) -> Self { + Self::Custom(<$box>::from(value).into()) + } + } + )+ + }; } -impl From for Json { - fn from(details: MultisigTransactionArgs) -> Self { - Json::new(details) +impl_custom_instruction!( + MultisigInstructionBox, + MultisigRegister | MultisigPropose | MultisigApprove +); + +#[allow(missing_docs)] +mod names { + use super::*; + + pub const SIGNATORIES: &str = "signatories"; + pub const QUORUM: &str = "quorum"; + pub const TRANSACTION_TTL_MS: &str = "transaction_ttl_ms"; + pub const PROPOSALS: &str = "proposals"; + pub const MULTISIG_SIGNATORY_: &str = "MULTISIG_SIGNATORY_"; + + pub fn instructions_key(hash: &HashOf>) -> Name { + format!("{PROPOSALS}/{hash}/instructions").parse().unwrap() + } + + pub fn proposed_at_ms_key(hash: &HashOf>) -> Name { + format!("{PROPOSALS}/{hash}/proposed_at_ms") + .parse() + .unwrap() } -} -impl TryFrom<&Json> for MultisigTransactionArgs { - type Error = serde_json::Error; + pub fn approvals_key(hash: &HashOf>) -> Name { + format!("{PROPOSALS}/{hash}/approvals").parse().unwrap() + } - fn try_from(payload: &Json) -> serde_json::Result { - serde_json::from_str::(payload.as_ref()) + pub fn multisig_role_for(account: &AccountId) -> RoleId { + format!( + "{MULTISIG_SIGNATORY_}{}_{}", + account.signatory(), + account.domain() + ) + .parse() + .unwrap() + } + + pub fn multisig_account_from(role: &RoleId) -> Option { + role.name() + .as_ref() + .strip_prefix(MULTISIG_SIGNATORY_)? + .replacen('_', "@", 1) + .parse() + .ok() } } + +pub use names::*; diff --git a/crates/iroha_schema_gen/src/lib.rs b/crates/iroha_schema_gen/src/lib.rs index 7fd0f8887a6..91a69e73a42 100644 --- a/crates/iroha_schema_gen/src/lib.rs +++ b/crates/iroha_schema_gen/src/lib.rs @@ -94,9 +94,11 @@ pub fn build_schemas() -> MetaMap { permission::trigger::CanModifyTriggerMetadata, permission::executor::CanUpgradeExecutor, - // Arguments attached to multi-signature operations - multisig::MultisigAccountArgs, - multisig::MultisigTransactionArgs, + // Multi-signature operations + multisig::MultisigInstructionBox, + multisig::MultisigRegister, + multisig::MultisigPropose, + multisig::MultisigApprove, // Genesis file - used by SDKs to generate the genesis block // TODO: IMO it could/should be removed from the schema @@ -287,8 +289,10 @@ types!( MintabilityError, Mintable, Mismatch, - MultisigAccountArgs, - MultisigTransactionArgs, + MultisigInstructionBox, + MultisigRegister, + MultisigPropose, + MultisigApprove, Name, NewAccount, NewAssetDefinition, @@ -551,7 +555,9 @@ pub mod complete_data_model { Level, }; pub use iroha_genesis::{GenesisWasmAction, GenesisWasmTrigger, WasmPath}; - pub use iroha_multisig_data_model::{MultisigAccountArgs, MultisigTransactionArgs}; + pub use iroha_multisig_data_model::{ + MultisigApprove, MultisigInstructionBox, MultisigPropose, MultisigRegister, + }; pub use iroha_primitives::{const_vec::ConstVec, conststr::ConstString, json::Json}; pub use iroha_schema::Compact; } diff --git a/crates/iroha_test_network/src/lib.rs b/crates/iroha_test_network/src/lib.rs index 187ca60eb0c..4c63b2c7b2c 100644 --- a/crates/iroha_test_network/src/lib.rs +++ b/crates/iroha_test_network/src/lib.rs @@ -783,7 +783,7 @@ impl NetworkPeer { /// Generated [`PeerId`] pub fn peer_id(&self) -> PeerId { - self.id.id.clone() + self.id.id().clone() } /// Check whether the peer is running diff --git a/defaults/genesis.json b/defaults/genesis.json index 58e7993996d..fb90e53d4ae 100644 --- a/defaults/genesis.json +++ b/defaults/genesis.json @@ -191,40 +191,6 @@ } ], "wasm_dir": "libs", - "wasm_triggers": [ - { - "id": "multisig_domains", - "action": { - "executable": "multisig_domains.wasm", - "repeats": "Indefinitely", - "authority": "ed0120D8B64D62FD8E09B9F29FE04D9C63E312EFB1CB29F1BF6AF00EBC263007AE75F7@system", - "filter": { - "Data": { - "Domain": { - "id_matcher": null, - "event_set": [ - "Created", - "OwnerChanged" - ] - } - } - } - } - }, - { - "id": "multisig_accounts_wonderland", - "action": { - "executable": "multisig_accounts.wasm", - "repeats": "Indefinitely", - "authority": "ed0120CE7FA46C9DCE7EA4B125E2E36BDB63EA33073E7590AC92816AE1E861B7048B03@wonderland", - "filter": { - "ExecuteTrigger": { - "trigger_id": "multisig_accounts_wonderland", - "authority": null - } - } - } - } - ], + "wasm_triggers": [], "topology": [] } diff --git a/docs/source/references/schema.json b/docs/source/references/schema.json index 375ab8fcc43..2b9aaf7edf1 100644 --- a/docs/source/references/schema.json +++ b/docs/source/references/schema.json @@ -2601,37 +2601,66 @@ } ] }, - "MultisigAccountArgs": { + "MultisigApprove": { "Struct": [ { "name": "account", - "type": "PublicKey" + "type": "AccountId" }, { - "name": "signatories", - "type": "SortedMap" + "name": "instructions_hash", + "type": "HashOf>" + } + ] + }, + "MultisigInstructionBox": { + "Enum": [ + { + "tag": "Register", + "discriminant": 0, + "type": "MultisigRegister" }, { - "name": "quorum", - "type": "u16" + "tag": "Propose", + "discriminant": 1, + "type": "MultisigPropose" }, { - "name": "transaction_ttl_ms", - "type": "u64" + "tag": "Approve", + "discriminant": 2, + "type": "MultisigApprove" } ] }, - "MultisigTransactionArgs": { - "Enum": [ + "MultisigPropose": { + "Struct": [ { - "tag": "Propose", - "discriminant": 0, + "name": "account", + "type": "AccountId" + }, + { + "name": "instructions", "type": "Vec" + } + ] + }, + "MultisigRegister": { + "Struct": [ + { + "name": "account", + "type": "AccountId" }, { - "tag": "Approve", - "discriminant": 1, - "type": "HashOf>" + "name": "signatories", + "type": "SortedMap" + }, + { + "name": "quorum", + "type": "u16" + }, + { + "name": "transaction_ttl_ms", + "type": "u64" } ] }, diff --git a/scripts/build_wasm.sh b/scripts/build_wasm.sh index 8e28b719594..da0ee864647 100755 --- a/scripts/build_wasm.sh +++ b/scripts/build_wasm.sh @@ -10,9 +10,6 @@ build() { "libs") NAMES=( # order by dependency - "multisig_transactions" - "multisig_accounts" - "multisig_domains" "default_executor" ) ;; diff --git a/scripts/tests/instructions.json b/scripts/tests/instructions.json index 5385f812f03..a7dc30cfffb 100644 --- a/scripts/tests/instructions.json +++ b/scripts/tests/instructions.json @@ -3,7 +3,7 @@ "SetKeyValue": { "Account": { "object": "ed01201F89368A4F322263C6F1AEF156759A83FB1AD7D93BAA66BFDFA973ACBADA462F@wonderland", - "key": "key", + "key": "success_marker", "value": "congratulations" } } diff --git a/scripts/tests/multisig.recursion.sh b/scripts/tests/multisig.recursion.sh index cc69c33723e..1561ca2cf3e 100644 --- a/scripts/tests/multisig.recursion.sh +++ b/scripts/tests/multisig.recursion.sh @@ -68,25 +68,14 @@ INSTRUCTIONS="../scripts/tests/instructions.json" propose_stdout=($(cat $INSTRUCTIONS | ./iroha --config "client.0.toml" multisig propose --account $MSA_012345)) INSTRUCTIONS_HASH=${propose_stdout[0]} -# ticks as many times as the multisig recursion -TICK="../scripts/tests/tick.json" -for i in $(seq 0 1); do - cat $TICK | ./iroha json transaction -done - # check that one of the leaf signatories is involved LIST=$(./iroha --config "client.5.toml" multisig list all) echo "$LIST" | grep $INSTRUCTIONS_HASH # approve the multisig transaction -HASH_TO_12345=$(echo "$LIST" | grep -A1 "multisig_transactions" | sed 's/_/@/g' | grep -A1 $MSA_345 | tail -n 1 | tr -d '"') +HASH_TO_12345=$(echo "$LIST" | grep -A1 $MSA_345 | tail -n 1 | tr -d '"') ./iroha --config "client.5.toml" multisig approve --account $MSA_345 --instructions-hash $HASH_TO_12345 -# ticks as many times as the multisig recursion -for i in $(seq 0 1); do - cat $TICK | ./iroha json transaction -done - # check that the multisig transaction is executed ./iroha account list all | grep "congratulations" ! ./iroha --config "client.5.toml" multisig list all | grep $INSTRUCTIONS_HASH diff --git a/wasm/libs/default_executor/Cargo.toml b/wasm/libs/default_executor/Cargo.toml index 3efbe8de352..e38d4c9f4d4 100644 --- a/wasm/libs/default_executor/Cargo.toml +++ b/wasm/libs/default_executor/Cargo.toml @@ -11,6 +11,7 @@ crate-type = ['cdylib'] [dependencies] iroha_executor.workspace = true +iroha_multisig_data_model.workspace = true panic-halt.workspace = true dlmalloc.workspace = true diff --git a/wasm/libs/default_executor/src/lib.rs b/wasm/libs/default_executor/src/lib.rs index 7b1fb13d3e8..24471ee5060 100644 --- a/wasm/libs/default_executor/src/lib.rs +++ b/wasm/libs/default_executor/src/lib.rs @@ -5,9 +5,14 @@ #[cfg(not(test))] extern crate panic_halt; +extern crate alloc; + use dlmalloc::GlobalDlmalloc; use iroha_executor::{ - data_model::block::BlockHeader, debug::dbg_panic, prelude::*, DataModelBuilder, + data_model::block::BlockHeader, + debug::{dbg_panic, DebugExpectExt}, + prelude::*, + DataModelBuilder, }; #[global_allocator] @@ -15,12 +20,15 @@ static ALLOC: GlobalDlmalloc = GlobalDlmalloc; getrandom::register_custom_getrandom!(iroha_executor::stub_getrandom); +mod multisig; + /// Executor that replaces some of [`Execute`]'s methods with sensible defaults /// /// # Warning /// /// The defaults are not guaranteed to be stable. #[derive(Debug, Clone, Visit, Execute, Entrypoints)] +#[visit(custom(visit_custom))] struct Executor { host: Iroha, context: Context, @@ -38,6 +46,37 @@ impl Executor { } } +fn visit_custom(executor: &mut Executor, isi: &CustomInstruction) { + if let Ok(isi) = multisig::MultisigInstructionBox::try_from(isi.payload()) { + return isi.visit_execute(executor); + }; + + deny!(executor, "unexpected custom instruction"); +} + +trait VisitExecute: Instruction { + fn visit_execute(self, executor: &mut Executor) { + let init_authority = executor.context().authority.clone(); + self.visit(executor); + if executor.verdict().is_ok() { + if let Err(err) = self.execute(executor, &init_authority) { + executor.deny(err); + } + } + // reset authority per instruction + // TODO seek a more proper way + executor.context_mut().authority = init_authority; + } + + fn visit(&self, executor: &mut Executor); + + fn execute( + self, + executor: &mut Executor, + init_authority: &AccountId, + ) -> Result<(), ValidationFail>; +} + /// Migrate previous executor to the current version. /// Called by Iroha once just before upgrading executor. /// @@ -50,5 +89,10 @@ impl Executor { #[entrypoint] fn migrate(host: Iroha, context: Context) { Executor::ensure_genesis(context.curr_block); - DataModelBuilder::with_default_permissions().build_and_set(&host); + DataModelBuilder::with_default_permissions() + .add_instruction::() + .add_instruction::() + .add_instruction::() + .add_instruction::() + .build_and_set(&host); } diff --git a/wasm/libs/default_executor/src/multisig/account.rs b/wasm/libs/default_executor/src/multisig/account.rs new file mode 100644 index 00000000000..7e15bc19a94 --- /dev/null +++ b/wasm/libs/default_executor/src/multisig/account.rs @@ -0,0 +1,103 @@ +//! Validation and execution logic of instructions for multisig accounts + +use super::*; + +impl VisitExecute for MultisigRegister { + fn visit(&self, executor: &mut Executor) { + let host = executor.host(); + let target_domain = self.account.domain(); + + // Any account in a domain can register any multisig account in the domain + // TODO Restrict access to the multisig signatories? + // TODO Impose proposal and approval process? + if target_domain != executor.context().authority.domain() { + deny!( + executor, + "multisig account and its registrant must be in the same domain" + ) + } + + let Ok(domain_found) = host + .query(FindDomains) + .filter_with(|domain| domain.id.eq(target_domain.clone())) + .execute_single() + else { + deny!( + executor, + "domain must exist before registering multisig account" + ); + }; + + for signatory in self.signatories.keys().cloned() { + let Ok(_signatory_found) = host + .query(FindAccounts) + .filter_with(|account| account.id.eq(signatory)) + .execute_single() + else { + deny!( + executor, + "signatories must exist before registering multisig account" + ); + }; + } + + // Pass validation and elevate to the domain owner authority + executor.context_mut().authority = domain_found.owned_by().clone(); + } + + fn execute( + self, + executor: &mut Executor, + _init_authority: &AccountId, + ) -> Result<(), ValidationFail> { + let host = executor.host(); + let domain_owner = executor.context().authority.clone(); + let multisig_account = self.account; + let multisig_role = multisig_role_for(&multisig_account); + + host.submit(&Register::account(Account::new(multisig_account.clone()))) + .dbg_expect("domain owner should successfully register a multisig account"); + + host.submit(&SetKeyValue::account( + multisig_account.clone(), + SIGNATORIES.parse().unwrap(), + Json::new(&self.signatories), + )) + .dbg_unwrap(); + + host.submit(&SetKeyValue::account( + multisig_account.clone(), + QUORUM.parse().unwrap(), + Json::new(&self.quorum), + )) + .dbg_unwrap(); + + host.submit(&SetKeyValue::account( + multisig_account.clone(), + TRANSACTION_TTL_MS.parse().unwrap(), + Json::new(&self.transaction_ttl_ms), + )) + .dbg_unwrap(); + + host.submit(&Register::role( + // Temporarily grant a multisig role to the domain owner to delegate the role to the signatories + Role::new(multisig_role.clone(), domain_owner.clone()), + )) + .dbg_expect("domain owner should successfully register a multisig role"); + + for signatory in self.signatories.keys().cloned() { + host.submit(&Grant::account_role(multisig_role.clone(), signatory)) + .dbg_expect( + "domain owner should successfully grant the multisig role to signatories", + ); + } + + // FIXME No roles to revoke found, which should have been granted to the domain owner + // host.submit(&Revoke::account_role(multisig_role, domain_owner)) + // .dbg_expect( + // "domain owner should successfully revoke the multisig role from the domain owner", + // ); + + Ok(()) + } +} diff --git a/wasm/libs/default_executor/src/multisig/mod.rs b/wasm/libs/default_executor/src/multisig/mod.rs new file mode 100644 index 00000000000..7e3e9cac37c --- /dev/null +++ b/wasm/libs/default_executor/src/multisig/mod.rs @@ -0,0 +1,34 @@ +use super::*; + +mod account; +mod transaction; + +impl VisitExecute for MultisigInstructionBox { + fn visit(&self, executor: &mut Executor) { + match self { + MultisigInstructionBox::Register(instruction) => instruction.visit(executor), + MultisigInstructionBox::Propose(instruction) => instruction.visit(executor), + MultisigInstructionBox::Approve(instruction) => instruction.visit(executor), + } + } + + fn execute( + self, + executor: &mut Executor, + init_authority: &AccountId, + ) -> Result<(), ValidationFail> { + match self { + MultisigInstructionBox::Register(instruction) => { + instruction.execute(executor, init_authority) + } + MultisigInstructionBox::Propose(instruction) => { + instruction.execute(executor, init_authority) + } + MultisigInstructionBox::Approve(instruction) => { + instruction.execute(executor, init_authority) + } + } + } +} + +pub(super) use iroha_multisig_data_model::*; diff --git a/wasm/libs/default_executor/src/multisig/transaction.rs b/wasm/libs/default_executor/src/multisig/transaction.rs new file mode 100644 index 00000000000..785e0908cf9 --- /dev/null +++ b/wasm/libs/default_executor/src/multisig/transaction.rs @@ -0,0 +1,267 @@ +//! Validation and execution logic of instructions for multisig transactions + +use alloc::{ + collections::{btree_map::BTreeMap, btree_set::BTreeSet}, + vec::Vec, +}; + +use super::*; + +impl VisitExecute for MultisigPropose { + fn visit(&self, executor: &mut Executor) { + let host = executor.host(); + let proposer = executor.context().authority.clone(); + let target_account = self.account.clone(); + let multisig_role = multisig_role_for(&target_account); + let instructions_hash = HashOf::new(&self.instructions); + let is_top_down_proposal = host + .query_single(FindAccountMetadata::new( + proposer.clone(), + SIGNATORIES.parse().unwrap(), + )) + .map_or(false, |proposer_signatories| { + proposer_signatories + .try_into_any::>() + .dbg_unwrap() + .contains_key(&target_account) + }); + + if !is_top_down_proposal { + let Ok(_role_found) = host + .query(FindRolesByAccountId::new(proposer)) + .filter_with(|role_id| role_id.eq(multisig_role)) + .execute_single() + else { + deny!(executor, "not qualified to propose multisig"); + }; + } + + let Err(_proposal_not_found) = host.query_single(FindAccountMetadata::new( + target_account.clone(), + approvals_key(&instructions_hash), + )) else { + deny!(executor, "multisig proposal duplicates") + }; + + // Pass validation and elevate to the multisig account authority + executor.context_mut().authority = target_account; + } + + fn execute( + self, + executor: &mut Executor, + init_authority: &AccountId, + ) -> Result<(), ValidationFail> { + let host = executor.host().clone(); + let target_account = self.account; + let instructions_hash = HashOf::new(&self.instructions); + let signatories: BTreeMap = host + .query_single(FindAccountMetadata::new( + target_account.clone(), + SIGNATORIES.parse().unwrap(), + )) + .dbg_unwrap() + .try_into_any() + .dbg_unwrap(); + + // Recursively deploy multisig authentication down to the personal leaf signatories + for signatory in signatories.keys().cloned() { + let is_multisig_again = host + .query(FindRoleIds) + .filter_with(|role_id| role_id.eq(multisig_role_for(&signatory))) + .execute_single_opt() + .dbg_unwrap() + .is_some(); + + if is_multisig_again { + let propose_to_approve_me = { + let approve_me = + MultisigApprove::new(target_account.clone(), instructions_hash.clone()); + + MultisigPropose::new(signatory, [approve_me.into()].to_vec()) + }; + propose_to_approve_me.visit_execute(executor); + } + } + + let now_ms: u64 = executor + .context() + .curr_block + .creation_time() + .as_millis() + .try_into() + .dbg_expect("shouldn't overflow within 584942417 years"); + + let approvals = BTreeSet::from([init_authority]); + + host.submit(&SetKeyValue::account( + target_account.clone(), + instructions_key(&instructions_hash).clone(), + Json::new(&self.instructions), + )) + .dbg_unwrap(); + + host.submit(&SetKeyValue::account( + target_account.clone(), + proposed_at_ms_key(&instructions_hash).clone(), + Json::new(&now_ms), + )) + .dbg_unwrap(); + + host.submit(&SetKeyValue::account( + target_account.clone(), + approvals_key(&instructions_hash).clone(), + Json::new(&approvals), + )) + .dbg_unwrap(); + + Ok(()) + } +} + +impl VisitExecute for MultisigApprove { + fn visit(&self, executor: &mut Executor) { + let host = executor.host(); + let approver = executor.context().authority.clone(); + let target_account = self.account.clone(); + let multisig_role = multisig_role_for(&target_account); + let instructions_hash = self.instructions_hash; + + let Ok(_role_found) = host + .query(FindRolesByAccountId::new(approver)) + .filter_with(|role_id| role_id.eq(multisig_role)) + .execute_single() + else { + deny!(executor, "not qualified to approve multisig"); + }; + + let Ok(_proposal_found) = host.query_single(FindAccountMetadata::new( + target_account.clone(), + approvals_key(&instructions_hash), + )) else { + deny!(executor, "no proposals to approve") + }; + + // Pass validation and elevate to the multisig account authority + executor.context_mut().authority = target_account; + } + + fn execute( + self, + executor: &mut Executor, + init_authority: &AccountId, + ) -> Result<(), ValidationFail> { + let host = executor.host().clone(); + let target_account = self.account; + let instructions_hash = self.instructions_hash; + let signatories: BTreeMap = host + .query_single(FindAccountMetadata::new( + target_account.clone(), + SIGNATORIES.parse().unwrap(), + )) + .dbg_unwrap() + .try_into_any() + .dbg_unwrap(); + let quorum: u16 = host + .query_single(FindAccountMetadata::new( + target_account.clone(), + QUORUM.parse().unwrap(), + )) + .dbg_unwrap() + .try_into_any() + .dbg_unwrap(); + let transaction_ttl_ms: u64 = host + .query_single(FindAccountMetadata::new( + target_account.clone(), + TRANSACTION_TTL_MS.parse().unwrap(), + )) + .dbg_unwrap() + .try_into_any() + .dbg_unwrap(); + let instructions: Vec = host + .query_single(FindAccountMetadata::new( + target_account.clone(), + instructions_key(&instructions_hash), + )) + .dbg_unwrap() + .try_into_any() + .dbg_unwrap(); + let proposed_at_ms: u64 = host + .query_single(FindAccountMetadata::new( + target_account.clone(), + proposed_at_ms_key(&instructions_hash), + )) + .dbg_unwrap() + .try_into_any() + .dbg_unwrap(); + let mut approvals: BTreeSet = host + .query_single(FindAccountMetadata::new( + target_account.clone(), + approvals_key(&instructions_hash), + )) + .dbg_unwrap() + .try_into_any() + .dbg_unwrap(); + + approvals.insert(init_authority.clone()); + + host.submit(&SetKeyValue::account( + target_account.clone(), + approvals_key(&instructions_hash), + Json::new(&approvals), + )) + .dbg_unwrap(); + + let now_ms: u64 = executor + .context() + .curr_block + .creation_time() + .as_millis() + .try_into() + .dbg_expect("shouldn't overflow within 584942417 years"); + + let is_authenticated = quorum + <= signatories + .into_iter() + .filter(|(id, _)| approvals.contains(&id)) + .map(|(_, weight)| weight as u16) + .sum(); + + let is_expired = proposed_at_ms.saturating_add(transaction_ttl_ms) < now_ms; + + if is_authenticated || is_expired { + // Cleanup the transaction entry + host.submit(&RemoveKeyValue::account( + target_account.clone(), + approvals_key(&instructions_hash), + )) + .dbg_unwrap(); + + host.submit(&RemoveKeyValue::account( + target_account.clone(), + proposed_at_ms_key(&instructions_hash), + )) + .dbg_unwrap(); + + host.submit(&RemoveKeyValue::account( + target_account.clone(), + instructions_key(&instructions_hash), + )) + .dbg_unwrap(); + + if !is_expired { + // Execute instructions proposal which collected enough approvals + for isi in instructions { + match isi { + InstructionBox::Custom(instruction) => visit_custom(executor, &instruction), + builtin => host.submit(&builtin).dbg_unwrap(), + } + } + } else { + // TODO Notify that the proposal has expired, while returning Ok for the entry deletion to take effect + } + } + + Ok(()) + } +} diff --git a/wasm/libs/multisig_accounts/Cargo.toml b/wasm/libs/multisig_accounts/Cargo.toml deleted file mode 100644 index 476b201367d..00000000000 --- a/wasm/libs/multisig_accounts/Cargo.toml +++ /dev/null @@ -1,23 +0,0 @@ -[package] -name = "multisig_accounts" - -edition.workspace = true -version.workspace = true -authors.workspace = true - -license.workspace = true - -[lib] -crate-type = ['cdylib'] - -[dependencies] -iroha_trigger.workspace = true -iroha_executor_data_model.workspace = true -iroha_multisig_data_model.workspace = true - -panic-halt.workspace = true -dlmalloc.workspace = true -getrandom.workspace = true - -serde = { workspace = true, features = ["derive"] } -serde_json = { workspace = true, default-features = false } diff --git a/wasm/libs/multisig_accounts/src/lib.rs b/wasm/libs/multisig_accounts/src/lib.rs deleted file mode 100644 index 7ad18e3a2f7..00000000000 --- a/wasm/libs/multisig_accounts/src/lib.rs +++ /dev/null @@ -1,155 +0,0 @@ -//! Trigger given per domain to control multi-signature accounts and corresponding triggers - -#![no_std] - -extern crate alloc; -#[cfg(not(test))] -extern crate panic_halt; - -use alloc::format; - -use dlmalloc::GlobalDlmalloc; -use iroha_executor_data_model::permission::trigger::CanExecuteTrigger; -use iroha_multisig_data_model::MultisigAccountArgs; -use iroha_trigger::{ - debug::{dbg_panic, DebugExpectExt as _}, - prelude::*, -}; - -#[global_allocator] -static ALLOC: GlobalDlmalloc = GlobalDlmalloc; - -getrandom::register_custom_getrandom!(iroha_trigger::stub_getrandom); - -// Binary containing common logic to each multisig account for handling multisig transactions -const MULTISIG_TRANSACTIONS_WASM: &[u8] = core::include_bytes!(concat!( - core::env!("CARGO_MANIFEST_DIR"), - "/../../target/prebuilt/libs/multisig_transactions.wasm" -)); - -#[iroha_trigger::main] -fn main(host: Iroha, context: Context) { - let EventBox::ExecuteTrigger(event) = context.event else { - dbg_panic("trigger misused: must be triggered only by a call"); - }; - let args: MultisigAccountArgs = event - .args() - .try_into_any() - .dbg_expect("args should be for a multisig account"); - let domain_id = context - .id - .name() - .as_ref() - .strip_prefix("multisig_accounts_") - .and_then(|s| s.parse::().ok()) - .dbg_unwrap(); - let account_id = AccountId::new(domain_id, args.account); - - host.submit(&Register::account(Account::new(account_id.clone()))) - .dbg_expect("accounts registry should successfully register a multisig account"); - - let multisig_transactions_registry_id: TriggerId = format!( - "multisig_transactions_{}_{}", - account_id.signatory(), - account_id.domain() - ) - .parse() - .dbg_unwrap(); - - let multisig_transactions_registry = Trigger::new( - multisig_transactions_registry_id.clone(), - Action::new( - WasmSmartContract::from_compiled(MULTISIG_TRANSACTIONS_WASM.to_vec()), - Repeats::Indefinitely, - account_id.clone(), - ExecuteTriggerEventFilter::new().for_trigger(multisig_transactions_registry_id.clone()), - ), - ); - - host.submit(&Register::trigger(multisig_transactions_registry)) - .dbg_expect("accounts registry should successfully register a transactions registry"); - - host.submit(&SetKeyValue::trigger( - multisig_transactions_registry_id.clone(), - "signatories".parse().unwrap(), - Json::new(&args.signatories), - )) - .dbg_unwrap(); - - host.submit(&SetKeyValue::trigger( - multisig_transactions_registry_id.clone(), - "quorum".parse().unwrap(), - Json::new(&args.quorum), - )) - .dbg_unwrap(); - - host.submit(&SetKeyValue::trigger( - multisig_transactions_registry_id.clone(), - "transaction_ttl_ms".parse().unwrap(), - Json::new(&args.transaction_ttl_ms), - )) - .dbg_unwrap(); - - let role_id: RoleId = format!( - "multisig_signatory_{}_{}", - account_id.signatory(), - account_id.domain() - ) - .parse() - .dbg_unwrap(); - - host.submit(&Register::role( - // Temporarily grant a multisig role to the trigger authority to delegate the role to the signatories - Role::new(role_id.clone(), context.authority.clone()), - )) - .dbg_expect("accounts registry should successfully register a multisig role"); - - for signatory in args.signatories.keys().cloned() { - let is_multisig_again = { - let sub_role_id: RoleId = format!( - "multisig_signatory_{}_{}", - signatory.signatory(), - signatory.domain() - ) - .parse() - .dbg_unwrap(); - - host.query(FindRoleIds) - .filter_with(|role_id| role_id.eq(sub_role_id)) - .execute_single_opt() - .dbg_unwrap() - .is_some() - }; - - if is_multisig_again { - // Allow the transactions registry to write to the sub registry - let sub_registry_id: TriggerId = format!( - "multisig_transactions_{}_{}", - signatory.signatory(), - signatory.domain() - ) - .parse() - .dbg_unwrap(); - - host.submit(&Grant::account_permission( - CanExecuteTrigger { - trigger: sub_registry_id, - }, - account_id.clone(), - )) - .dbg_expect( - "accounts registry should successfully grant permission to the multisig account", - ); - } - - host.submit(&Grant::account_role(role_id.clone(), signatory)) - .dbg_expect( - "accounts registry should successfully grant the multisig role to signatories", - ); - } - - host.submit(&Revoke::account_role(role_id.clone(), context.authority)) - .dbg_expect( - "accounts registry should successfully revoke the multisig role from the trigger authority", - ); -} diff --git a/wasm/libs/multisig_domains/Cargo.toml b/wasm/libs/multisig_domains/Cargo.toml deleted file mode 100644 index ae37e394289..00000000000 --- a/wasm/libs/multisig_domains/Cargo.toml +++ /dev/null @@ -1,22 +0,0 @@ -[package] -name = "multisig_domains" - -edition.workspace = true -version.workspace = true -authors.workspace = true - -license.workspace = true - -[lib] -crate-type = ['cdylib'] - -[dependencies] -iroha_trigger.workspace = true -iroha_executor_data_model.workspace = true - -panic-halt.workspace = true -dlmalloc.workspace = true -getrandom.workspace = true - -serde = { workspace = true, features = ["derive"] } -serde_json = { workspace = true, default-features = false } diff --git a/wasm/libs/multisig_domains/src/lib.rs b/wasm/libs/multisig_domains/src/lib.rs deleted file mode 100644 index c60e3a9e802..00000000000 --- a/wasm/libs/multisig_domains/src/lib.rs +++ /dev/null @@ -1,82 +0,0 @@ -//! Trigger of world-level authority to enable multisig functionality for domains - -#![no_std] - -extern crate alloc; -#[cfg(not(test))] -extern crate panic_halt; - -use alloc::format; - -use dlmalloc::GlobalDlmalloc; -use iroha_trigger::{ - debug::{dbg_panic, DebugExpectExt as _}, - prelude::*, -}; - -#[global_allocator] -static ALLOC: GlobalDlmalloc = GlobalDlmalloc; - -getrandom::register_custom_getrandom!(iroha_trigger::stub_getrandom); - -// Binary containing common logic to each domain for handling multisig accounts -const MULTISIG_ACCOUNTS_WASM: &[u8] = core::include_bytes!(concat!( - core::env!("CARGO_MANIFEST_DIR"), - "/../../target/prebuilt/libs/multisig_accounts.wasm" -)); - -#[iroha_trigger::main] -fn main(host: Iroha, context: Context) { - let EventBox::Data(DataEvent::Domain(event)) = context.event else { - dbg_panic("trigger misused: must be triggered only by a domain event"); - }; - let (domain_id, domain_owner, owner_changed) = match event { - DomainEvent::Created(domain) => (domain.id().clone(), domain.owned_by().clone(), false), - DomainEvent::OwnerChanged(owner_changed) => ( - owner_changed.domain().clone(), - owner_changed.new_owner().clone(), - true, - ), - _ => dbg_panic( - "trigger misused: must be triggered only when domain created or owner changed", - ), - }; - - let accounts_registry_id: TriggerId = format!("multisig_accounts_{}", domain_id) - .parse() - .dbg_unwrap(); - - let accounts_registry = if owner_changed { - let existing = host - .query(FindTriggers::new()) - .filter_with(|trigger| trigger.id.eq(accounts_registry_id.clone())) - .execute_single() - .dbg_expect("accounts registry should be existing"); - - host.submit(&Unregister::trigger(existing.id().clone())) - .dbg_expect("accounts registry should be successfully unregistered"); - - Trigger::new( - existing.id().clone(), - Action::new( - existing.action().executable().clone(), - existing.action().repeats().clone(), - domain_owner, - existing.action().filter().clone(), - ), - ) - } else { - Trigger::new( - accounts_registry_id.clone(), - Action::new( - WasmSmartContract::from_compiled(MULTISIG_ACCOUNTS_WASM.to_vec()), - Repeats::Indefinitely, - domain_owner, - ExecuteTriggerEventFilter::new().for_trigger(accounts_registry_id.clone()), - ), - ) - }; - - host.submit(&Register::trigger(accounts_registry)) - .dbg_expect("accounts registry should be successfully registered"); -} diff --git a/wasm/libs/multisig_transactions/Cargo.toml b/wasm/libs/multisig_transactions/Cargo.toml deleted file mode 100644 index ab208ce239b..00000000000 --- a/wasm/libs/multisig_transactions/Cargo.toml +++ /dev/null @@ -1,22 +0,0 @@ -[package] -name = "multisig_transactions" - -edition.workspace = true -version.workspace = true -authors.workspace = true - -license.workspace = true - -[lib] -crate-type = ['cdylib'] - -[dependencies] -iroha_trigger.workspace = true -iroha_multisig_data_model.workspace = true - -panic-halt.workspace = true -dlmalloc.workspace = true -getrandom.workspace = true - -serde = { workspace = true, features = ["derive"] } -serde_json = { workspace = true, default-features = false } diff --git a/wasm/libs/multisig_transactions/src/lib.rs b/wasm/libs/multisig_transactions/src/lib.rs deleted file mode 100644 index f4ac819cce4..00000000000 --- a/wasm/libs/multisig_transactions/src/lib.rs +++ /dev/null @@ -1,233 +0,0 @@ -//! Trigger given per multi-signature account to control multi-signature transactions - -#![no_std] - -extern crate alloc; -#[cfg(not(test))] -extern crate panic_halt; - -use alloc::{ - collections::{btree_map::BTreeMap, btree_set::BTreeSet}, - format, - vec::Vec, -}; - -use dlmalloc::GlobalDlmalloc; -use iroha_multisig_data_model::MultisigTransactionArgs; -use iroha_trigger::{ - debug::{dbg_panic, DebugExpectExt as _}, - prelude::*, -}; - -#[global_allocator] -static ALLOC: GlobalDlmalloc = GlobalDlmalloc; - -getrandom::register_custom_getrandom!(iroha_trigger::stub_getrandom); - -#[iroha_trigger::main] -fn main(host: Iroha, context: Context) { - let EventBox::ExecuteTrigger(event) = context.event else { - dbg_panic("trigger misused: must be triggered only by a call"); - }; - let trigger_id = context.id; - let args: MultisigTransactionArgs = event - .args() - .try_into_any() - .dbg_expect("args should be for a multisig transaction"); - let signatory = event.authority().clone(); - - let instructions_hash = match &args { - MultisigTransactionArgs::Propose(instructions) => HashOf::new(instructions), - MultisigTransactionArgs::Approve(instructions_hash) => *instructions_hash, - }; - let instructions_metadata_key: Name = format!("proposals/{instructions_hash}/instructions") - .parse() - .unwrap(); - let proposed_at_ms_metadata_key: Name = format!("proposals/{instructions_hash}/proposed_at_ms") - .parse() - .unwrap(); - let approvals_metadata_key: Name = format!("proposals/{instructions_hash}/approvals") - .parse() - .unwrap(); - - let signatories: BTreeMap = host - .query_single(FindTriggerMetadata::new( - trigger_id.clone(), - "signatories".parse().unwrap(), - )) - .dbg_unwrap() - .try_into_any() - .dbg_unwrap(); - - // Recursively deploy multisig authentication down to the personal leaf signatories - for account_id in signatories.keys() { - let sub_transactions_registry_id: TriggerId = format!( - "multisig_transactions_{}_{}", - account_id.signatory(), - account_id.domain() - ) - .parse() - .unwrap(); - - if let Ok(_sub_registry) = host - .query(FindTriggers::new()) - .filter_with(|trigger| trigger.id.eq(sub_transactions_registry_id.clone())) - .execute_single() - { - let propose_to_approve_me: InstructionBox = { - let approve_me: InstructionBox = { - let args = MultisigTransactionArgs::Approve(instructions_hash); - ExecuteTrigger::new(trigger_id.clone()) - .with_args(&args) - .into() - }; - let args = MultisigTransactionArgs::Propose([approve_me].to_vec()); - - ExecuteTrigger::new(sub_transactions_registry_id.clone()) - .with_args(&args) - .into() - }; - host.submit(&propose_to_approve_me) - .dbg_expect("should successfully write to sub registry"); - } - } - - let mut block_headers = host.query(FindBlockHeaders).execute().dbg_unwrap(); - let now_ms: u64 = block_headers - .next() - .dbg_unwrap() - .dbg_unwrap() - .creation_time() - .as_millis() - .try_into() - .dbg_unwrap(); - - let (approvals, instructions) = match args { - MultisigTransactionArgs::Propose(instructions) => { - host.query_single(FindTriggerMetadata::new( - trigger_id.clone(), - approvals_metadata_key.clone(), - )) - .expect_err("instructions shouldn't already be proposed"); - - let approvals = BTreeSet::from([signatory.clone()]); - - host.submit(&SetKeyValue::trigger( - trigger_id.clone(), - instructions_metadata_key.clone(), - Json::new(&instructions), - )) - .dbg_unwrap(); - - host.submit(&SetKeyValue::trigger( - trigger_id.clone(), - proposed_at_ms_metadata_key.clone(), - Json::new(&now_ms), - )) - .dbg_unwrap(); - - host.submit(&SetKeyValue::trigger( - trigger_id.clone(), - approvals_metadata_key.clone(), - Json::new(&approvals), - )) - .dbg_unwrap(); - - (approvals, instructions) - } - MultisigTransactionArgs::Approve(_instructions_hash) => { - let mut approvals: BTreeSet = host - .query_single(FindTriggerMetadata::new( - trigger_id.clone(), - approvals_metadata_key.clone(), - )) - .dbg_expect("instructions should be proposed first") - .try_into_any() - .dbg_unwrap(); - - approvals.insert(signatory.clone()); - - host.submit(&SetKeyValue::trigger( - trigger_id.clone(), - approvals_metadata_key.clone(), - Json::new(&approvals), - )) - .dbg_unwrap(); - - let instructions: Vec = host - .query_single(FindTriggerMetadata::new( - trigger_id.clone(), - instructions_metadata_key.clone(), - )) - .dbg_unwrap() - .try_into_any() - .dbg_unwrap(); - - (approvals, instructions) - } - }; - - let quorum: u16 = host - .query_single(FindTriggerMetadata::new( - trigger_id.clone(), - "quorum".parse().unwrap(), - )) - .dbg_unwrap() - .try_into_any() - .dbg_unwrap(); - - let is_authenticated = quorum - <= signatories - .into_iter() - .filter(|(id, _)| approvals.contains(&id)) - .map(|(_, weight)| weight as u16) - .sum(); - - let is_expired = { - let proposed_at_ms: u64 = host - .query_single(FindTriggerMetadata::new( - trigger_id.clone(), - proposed_at_ms_metadata_key.clone(), - )) - .dbg_unwrap() - .try_into_any() - .dbg_unwrap(); - - let transaction_ttl_ms: u64 = host - .query_single(FindTriggerMetadata::new( - trigger_id.clone(), - "transaction_ttl_ms".parse().unwrap(), - )) - .dbg_unwrap() - .try_into_any() - .dbg_unwrap(); - - proposed_at_ms.saturating_add(transaction_ttl_ms) < now_ms - }; - - if is_authenticated || is_expired { - // Cleanup approvals and instructions - host.submit(&RemoveKeyValue::trigger( - trigger_id.clone(), - approvals_metadata_key, - )) - .dbg_unwrap(); - host.submit(&RemoveKeyValue::trigger( - trigger_id.clone(), - proposed_at_ms_metadata_key, - )) - .dbg_unwrap(); - host.submit(&RemoveKeyValue::trigger( - trigger_id.clone(), - instructions_metadata_key, - )) - .dbg_unwrap(); - - if !is_expired { - // Execute instructions proposal which collected enough approvals - for isi in instructions { - host.submit(&isi).dbg_unwrap(); - } - } - } -} From 00ec66724899e1bcf6074d2e397155c753cda71f Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Sat, 2 Nov 2024 19:45:13 +0900 Subject: [PATCH 02/28] fix: don't continue to grant a role on failing to register the role Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha_executor/src/default.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/iroha_executor/src/default.rs b/crates/iroha_executor/src/default.rs index c819fce0277..778ae28b12e 100644 --- a/crates/iroha_executor/src/default.rs +++ b/crates/iroha_executor/src/default.rs @@ -1288,7 +1288,7 @@ pub mod role { let grant_role = &Grant::account_role(role.id().clone(), role.grant_to().clone()); let isi = &Register::role(new_role); if let Err(err) = executor.host().submit(isi) { - executor.deny(err); + deny!(executor, err); } execute!(executor, grant_role); From a4ac5b012a3063482fa39cf10c5293cf162ec20a Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Sun, 3 Nov 2024 05:12:58 +0900 Subject: [PATCH 03/28] docs: fix doc test Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- Cargo.lock | 1 + crates/iroha_telemetry_derive/Cargo.toml | 2 ++ 2 files changed, 3 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index d4aff00ed1e..57ecd75936e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3579,6 +3579,7 @@ dependencies = [ name = "iroha_telemetry_derive" version = "2.0.0-rc.1.0" dependencies = [ + "iroha_core", "iroha_macro_utils", "manyhow", "proc-macro2", diff --git a/crates/iroha_telemetry_derive/Cargo.toml b/crates/iroha_telemetry_derive/Cargo.toml index fc6ac2b76dd..22dab89d120 100644 --- a/crates/iroha_telemetry_derive/Cargo.toml +++ b/crates/iroha_telemetry_derive/Cargo.toml @@ -30,4 +30,6 @@ manyhow = { workspace = true } iroha_macro_utils = { workspace = true } [dev-dependencies] +iroha_core = { workspace = true } + trybuild = { workspace = true } From 657d3ad6b22ec7a6236ab9dc8dfd5c9eb6d1106b Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Sun, 3 Nov 2024 06:07:29 +0900 Subject: [PATCH 04/28] chore: remove `CanRegisterAnyTrigger` `CanUnregisterAnyTrigger` permissions Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha_executor/src/default.rs | 20 +++-------- crates/iroha_executor/src/permission.rs | 34 ++----------------- .../src/permission.rs | 10 ------ crates/iroha_genesis/src/lib.rs | 5 --- crates/iroha_schema_gen/src/lib.rs | 8 ----- defaults/genesis.json | 22 ------------ docs/source/references/schema.json | 2 -- 7 files changed, 7 insertions(+), 94 deletions(-) diff --git a/crates/iroha_executor/src/default.rs b/crates/iroha_executor/src/default.rs index 778ae28b12e..91aacb31d9e 100644 --- a/crates/iroha_executor/src/default.rs +++ b/crates/iroha_executor/src/default.rs @@ -368,9 +368,7 @@ pub mod domain { AnyPermission::CanRegisterTrigger(permission) => { permission.authority.domain() == domain_id } - AnyPermission::CanRegisterAnyTrigger(_) - | AnyPermission::CanUnregisterAnyTrigger(_) - | AnyPermission::CanUnregisterTrigger(_) + AnyPermission::CanUnregisterTrigger(_) | AnyPermission::CanExecuteTrigger(_) | AnyPermission::CanModifyTrigger(_) | AnyPermission::CanModifyTriggerMetadata(_) @@ -548,9 +546,7 @@ pub mod account { AnyPermission::CanBurnAsset(permission) => permission.asset.account() == account_id, AnyPermission::CanTransferAsset(permission) => permission.asset.account() == account_id, AnyPermission::CanRegisterTrigger(permission) => permission.authority == *account_id, - AnyPermission::CanRegisterAnyTrigger(_) - | AnyPermission::CanUnregisterAnyTrigger(_) - | AnyPermission::CanUnregisterTrigger(_) + AnyPermission::CanUnregisterTrigger(_) | AnyPermission::CanExecuteTrigger(_) | AnyPermission::CanModifyTrigger(_) | AnyPermission::CanModifyTriggerMetadata(_) @@ -816,8 +812,6 @@ pub mod asset_definition { AnyPermission::CanUnregisterAccount(_) | AnyPermission::CanRegisterAsset(_) | AnyPermission::CanModifyAccountMetadata(_) - | AnyPermission::CanRegisterAnyTrigger(_) - | AnyPermission::CanUnregisterAnyTrigger(_) | AnyPermission::CanRegisterTrigger(_) | AnyPermission::CanUnregisterTrigger(_) | AnyPermission::CanExecuteTrigger(_) @@ -1342,8 +1336,8 @@ pub mod role { pub mod trigger { use iroha_executor_data_model::permission::trigger::{ - CanExecuteTrigger, CanModifyTrigger, CanModifyTriggerMetadata, CanRegisterAnyTrigger, - CanRegisterTrigger, CanUnregisterAnyTrigger, CanUnregisterTrigger, + CanExecuteTrigger, CanModifyTrigger, CanModifyTriggerMetadata, CanRegisterTrigger, + CanUnregisterTrigger, }; use iroha_smart_contract::data_model::trigger::Trigger; @@ -1377,7 +1371,6 @@ pub mod trigger { can_register_user_trigger_token .is_owned_by(&executor.context().authority, executor.host()) } - || CanRegisterAnyTrigger.is_owned_by(&executor.context().authority, executor.host()) { execute!(executor, isi) } @@ -1402,7 +1395,6 @@ pub mod trigger { can_unregister_user_trigger_token .is_owned_by(&executor.context().authority, executor.host()) } - || CanUnregisterAnyTrigger.is_owned_by(&executor.context().authority, executor.host()) { let mut err = None; for (owner_id, permission) in accounts_permissions(executor.host()) { @@ -1584,9 +1576,7 @@ pub mod trigger { AnyPermission::CanModifyTriggerMetadata(permission) => { &permission.trigger == trigger_id } - AnyPermission::CanRegisterAnyTrigger(_) - | AnyPermission::CanUnregisterAnyTrigger(_) - | AnyPermission::CanRegisterTrigger(_) + AnyPermission::CanRegisterTrigger(_) | AnyPermission::CanManagePeers(_) | AnyPermission::CanRegisterDomain(_) | AnyPermission::CanUnregisterDomain(_) diff --git a/crates/iroha_executor/src/permission.rs b/crates/iroha_executor/src/permission.rs index ed65e8185a0..ebe7a873d0f 100644 --- a/crates/iroha_executor/src/permission.rs +++ b/crates/iroha_executor/src/permission.rs @@ -117,8 +117,6 @@ declare_permissions! { iroha_executor_data_model::permission::parameter::{CanSetParameters}, iroha_executor_data_model::permission::role::{CanManageRoles}, - iroha_executor_data_model::permission::trigger::{CanRegisterAnyTrigger}, - iroha_executor_data_model::permission::trigger::{CanUnregisterAnyTrigger}, iroha_executor_data_model::permission::trigger::{CanRegisterTrigger}, iroha_executor_data_model::permission::trigger::{CanUnregisterTrigger}, iroha_executor_data_model::permission::trigger::{CanModifyTrigger}, @@ -756,8 +754,8 @@ pub mod account { pub mod trigger { //! Module with pass conditions for trigger related tokens use iroha_executor_data_model::permission::trigger::{ - CanExecuteTrigger, CanModifyTrigger, CanModifyTriggerMetadata, CanRegisterAnyTrigger, - CanRegisterTrigger, CanUnregisterAnyTrigger, CanUnregisterTrigger, + CanExecuteTrigger, CanModifyTrigger, CanModifyTriggerMetadata, CanRegisterTrigger, + CanUnregisterTrigger, }; use super::*; @@ -821,34 +819,6 @@ pub mod trigger { } } - impl ValidateGrantRevoke for CanRegisterAnyTrigger { - fn validate_grant(&self, authority: &AccountId, context: &Context, host: &Iroha) -> Result { - OnlyGenesis::from(self).validate(authority, host, context) - } - fn validate_revoke( - &self, - authority: &AccountId, - context: &Context, - host: &Iroha, - ) -> Result { - OnlyGenesis::from(self).validate(authority, host, context) - } - } - - impl ValidateGrantRevoke for CanUnregisterAnyTrigger { - fn validate_grant(&self, authority: &AccountId, context: &Context, host: &Iroha) -> Result { - OnlyGenesis::from(self).validate(authority, host, context) - } - fn validate_revoke( - &self, - authority: &AccountId, - context: &Context, - host: &Iroha, - ) -> Result { - OnlyGenesis::from(self).validate(authority, host, context) - } - } - impl ValidateGrantRevoke for CanRegisterTrigger { fn validate_grant(&self, authority: &AccountId, context: &Context, host: &Iroha) -> Result { super::account::Owner::from(self).validate(authority, host, context) diff --git a/crates/iroha_executor_data_model/src/permission.rs b/crates/iroha_executor_data_model/src/permission.rs index dc950a197bb..27778496268 100644 --- a/crates/iroha_executor_data_model/src/permission.rs +++ b/crates/iroha_executor_data_model/src/permission.rs @@ -178,16 +178,6 @@ pub mod asset { pub mod trigger { use super::*; - permission! { - #[derive(Copy)] - pub struct CanRegisterAnyTrigger; - } - - permission! { - #[derive(Copy)] - pub struct CanUnregisterAnyTrigger; - } - permission! { pub struct CanRegisterTrigger { pub authority: AccountId, diff --git a/crates/iroha_genesis/src/lib.rs b/crates/iroha_genesis/src/lib.rs index 10bfd5301cb..a931693c0a6 100644 --- a/crates/iroha_genesis/src/lib.rs +++ b/crates/iroha_genesis/src/lib.rs @@ -12,9 +12,6 @@ use derive_more::Constructor; use eyre::{eyre, Result, WrapErr}; use iroha_crypto::KeyPair; use iroha_data_model::{block::SignedBlock, parameter::Parameter, prelude::*}; -use iroha_executor_data_model::permission::trigger::{ - CanRegisterAnyTrigger, CanUnregisterAnyTrigger, -}; use iroha_schema::IntoSchema; use parity_scale_codec::{Decode, Encode}; use serde::{Deserialize, Serialize}; @@ -254,8 +251,6 @@ impl GenesisBuilder { 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![]; diff --git a/crates/iroha_schema_gen/src/lib.rs b/crates/iroha_schema_gen/src/lib.rs index 91a69e73a42..c99a1a1ccd0 100644 --- a/crates/iroha_schema_gen/src/lib.rs +++ b/crates/iroha_schema_gen/src/lib.rs @@ -85,8 +85,6 @@ pub fn build_schemas() -> MetaMap { permission::asset::CanModifyAssetMetadata, permission::parameter::CanSetParameters, permission::role::CanManageRoles, - permission::trigger::CanRegisterAnyTrigger, - permission::trigger::CanUnregisterAnyTrigger, permission::trigger::CanRegisterTrigger, permission::trigger::CanExecuteTrigger, permission::trigger::CanUnregisterTrigger, @@ -627,12 +625,6 @@ mod tests { insert_into_test_map!(iroha_executor_data_model::permission::asset::CanModifyAssetMetadata); insert_into_test_map!(iroha_executor_data_model::permission::parameter::CanSetParameters); insert_into_test_map!(iroha_executor_data_model::permission::role::CanManageRoles); - insert_into_test_map!( - iroha_executor_data_model::permission::trigger::CanRegisterAnyTrigger - ); - insert_into_test_map!( - iroha_executor_data_model::permission::trigger::CanUnregisterAnyTrigger - ); insert_into_test_map!(iroha_executor_data_model::permission::trigger::CanRegisterTrigger); insert_into_test_map!(iroha_executor_data_model::permission::trigger::CanExecuteTrigger); insert_into_test_map!(iroha_executor_data_model::permission::trigger::CanUnregisterTrigger); diff --git a/defaults/genesis.json b/defaults/genesis.json index fb90e53d4ae..4194ea0f4f8 100644 --- a/defaults/genesis.json +++ b/defaults/genesis.json @@ -41,28 +41,6 @@ } } }, - { - "Grant": { - "Permission": { - "object": { - "name": "CanRegisterAnyTrigger", - "payload": null - }, - "destination": "ed0120D8B64D62FD8E09B9F29FE04D9C63E312EFB1CB29F1BF6AF00EBC263007AE75F7@system" - } - } - }, - { - "Grant": { - "Permission": { - "object": { - "name": "CanUnregisterAnyTrigger", - "payload": null - }, - "destination": "ed0120D8B64D62FD8E09B9F29FE04D9C63E312EFB1CB29F1BF6AF00EBC263007AE75F7@system" - } - } - }, { "Register": { "Domain": { diff --git a/docs/source/references/schema.json b/docs/source/references/schema.json index 2b9aaf7edf1..4420ce213d1 100644 --- a/docs/source/references/schema.json +++ b/docs/source/references/schema.json @@ -872,7 +872,6 @@ } ] }, - "CanRegisterAnyTrigger": null, "CanRegisterAsset": { "Struct": [ { @@ -930,7 +929,6 @@ } ] }, - "CanUnregisterAnyTrigger": null, "CanUnregisterAsset": { "Struct": [ { From b158c59ca611ee75d451a9caa133c3cb4792ab81 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Sun, 3 Nov 2024 16:12:04 +0900 Subject: [PATCH 05/28] refactor: don't branch into `visit` and `execute` until leaf instructions Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- wasm/libs/default_executor/src/lib.rs | 12 ++++++--- .../libs/default_executor/src/multisig/mod.rs | 26 +++---------------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/wasm/libs/default_executor/src/lib.rs b/wasm/libs/default_executor/src/lib.rs index 24471ee5060..fdc3ce9c611 100644 --- a/wasm/libs/default_executor/src/lib.rs +++ b/wasm/libs/default_executor/src/lib.rs @@ -68,13 +68,17 @@ trait VisitExecute: Instruction { executor.context_mut().authority = init_authority; } - fn visit(&self, executor: &mut Executor); + fn visit(&self, _executor: &mut Executor) { + unimplemented!("should be overridden unless `Self::visit_execute` is overridden") + } fn execute( self, - executor: &mut Executor, - init_authority: &AccountId, - ) -> Result<(), ValidationFail>; + _executor: &mut Executor, + _init_authority: &AccountId, + ) -> Result<(), ValidationFail> { + unimplemented!("should be overridden unless `Self::visit_execute` is overridden") + } } /// Migrate previous executor to the current version. diff --git a/wasm/libs/default_executor/src/multisig/mod.rs b/wasm/libs/default_executor/src/multisig/mod.rs index 7e3e9cac37c..a982ebe6a52 100644 --- a/wasm/libs/default_executor/src/multisig/mod.rs +++ b/wasm/libs/default_executor/src/multisig/mod.rs @@ -4,29 +4,11 @@ mod account; mod transaction; impl VisitExecute for MultisigInstructionBox { - fn visit(&self, executor: &mut Executor) { + fn visit_execute(self, executor: &mut Executor) { match self { - MultisigInstructionBox::Register(instruction) => instruction.visit(executor), - MultisigInstructionBox::Propose(instruction) => instruction.visit(executor), - MultisigInstructionBox::Approve(instruction) => instruction.visit(executor), - } - } - - fn execute( - self, - executor: &mut Executor, - init_authority: &AccountId, - ) -> Result<(), ValidationFail> { - match self { - MultisigInstructionBox::Register(instruction) => { - instruction.execute(executor, init_authority) - } - MultisigInstructionBox::Propose(instruction) => { - instruction.execute(executor, init_authority) - } - MultisigInstructionBox::Approve(instruction) => { - instruction.execute(executor, init_authority) - } + MultisigInstructionBox::Register(instruction) => instruction.visit_execute(executor), + MultisigInstructionBox::Propose(instruction) => instruction.visit_execute(executor), + MultisigInstructionBox::Approve(instruction) => instruction.visit_execute(executor), } } } From 4eedd61d4d206599e21d8bbc1f19c3bf78f5d6a4 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Sun, 3 Nov 2024 17:30:23 +0900 Subject: [PATCH 06/28] refactor: clone and drop executor instance on recursive execution Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha_executor/src/lib.rs | 2 +- wasm/libs/default_executor/src/lib.rs | 12 ++----- .../default_executor/src/multisig/account.rs | 29 ++++++--------- .../src/multisig/transaction.rs | 36 ++++++++----------- 4 files changed, 29 insertions(+), 50 deletions(-) diff --git a/crates/iroha_executor/src/lib.rs b/crates/iroha_executor/src/lib.rs index 3df84bdb351..5108038d8b5 100644 --- a/crates/iroha_executor/src/lib.rs +++ b/crates/iroha_executor/src/lib.rs @@ -265,7 +265,7 @@ 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. + /// Mutable context for e.g. switching to another authority on recursive execution. /// Note that mutations are persistent to the instance unless reset fn context_mut(&mut self) -> &mut prelude::Context; diff --git a/wasm/libs/default_executor/src/lib.rs b/wasm/libs/default_executor/src/lib.rs index fdc3ce9c611..d7b31f115ac 100644 --- a/wasm/libs/default_executor/src/lib.rs +++ b/wasm/libs/default_executor/src/lib.rs @@ -56,27 +56,19 @@ fn visit_custom(executor: &mut Executor, isi: &CustomInstruction) { trait VisitExecute: Instruction { fn visit_execute(self, executor: &mut Executor) { - let init_authority = executor.context().authority.clone(); self.visit(executor); if executor.verdict().is_ok() { - if let Err(err) = self.execute(executor, &init_authority) { + if let Err(err) = self.execute(executor) { executor.deny(err); } } - // reset authority per instruction - // TODO seek a more proper way - executor.context_mut().authority = init_authority; } fn visit(&self, _executor: &mut Executor) { unimplemented!("should be overridden unless `Self::visit_execute` is overridden") } - fn execute( - self, - _executor: &mut Executor, - _init_authority: &AccountId, - ) -> Result<(), ValidationFail> { + fn execute(self, _executor: &Executor) -> Result<(), ValidationFail> { unimplemented!("should be overridden unless `Self::visit_execute` is overridden") } } diff --git a/wasm/libs/default_executor/src/multisig/account.rs b/wasm/libs/default_executor/src/multisig/account.rs index 7e15bc19a94..e6a2e1ee05b 100644 --- a/wasm/libs/default_executor/src/multisig/account.rs +++ b/wasm/libs/default_executor/src/multisig/account.rs @@ -17,7 +17,7 @@ impl VisitExecute for MultisigRegister { ) } - let Ok(domain_found) = host + let Ok(_domain_found) = host .query(FindDomains) .filter_with(|domain| domain.id.eq(target_domain.clone())) .execute_single() @@ -40,23 +40,16 @@ impl VisitExecute for MultisigRegister { ); }; } - - // Pass validation and elevate to the domain owner authority - executor.context_mut().authority = domain_found.owned_by().clone(); } - fn execute( - self, - executor: &mut Executor, - _init_authority: &AccountId, - ) -> Result<(), ValidationFail> { + fn execute(self, executor: &Executor) -> Result<(), ValidationFail> { let host = executor.host(); - let domain_owner = executor.context().authority.clone(); + let registrant = executor.context().authority.clone(); let multisig_account = self.account; let multisig_role = multisig_role_for(&multisig_account); host.submit(&Register::account(Account::new(multisig_account.clone()))) - .dbg_expect("domain owner should successfully register a multisig account"); + .dbg_expect("registrant should successfully register a multisig account"); host.submit(&SetKeyValue::account( multisig_account.clone(), @@ -80,22 +73,22 @@ impl VisitExecute for MultisigRegister { .dbg_unwrap(); host.submit(&Register::role( - // Temporarily grant a multisig role to the domain owner to delegate the role to the signatories - Role::new(multisig_role.clone(), domain_owner.clone()), + // Temporarily grant a multisig role to the registrant to delegate the role to the signatories + Role::new(multisig_role.clone(), registrant.clone()), )) - .dbg_expect("domain owner should successfully register a multisig role"); + .dbg_expect("registrant should successfully register a multisig role"); for signatory in self.signatories.keys().cloned() { host.submit(&Grant::account_role(multisig_role.clone(), signatory)) .dbg_expect( - "domain owner should successfully grant the multisig role to signatories", + "registrant should successfully grant the multisig role to signatories", ); } - // FIXME No roles to revoke found, which should have been granted to the domain owner - // host.submit(&Revoke::account_role(multisig_role, domain_owner)) + // FIXME No roles to revoke found, which should have been granted to the registrant + // host.submit(&Revoke::account_role(multisig_role, registrant)) // .dbg_expect( - // "domain owner should successfully revoke the multisig role from the domain owner", + // "registrant should successfully revoke the multisig role from the registrant", // ); Ok(()) diff --git a/wasm/libs/default_executor/src/multisig/transaction.rs b/wasm/libs/default_executor/src/multisig/transaction.rs index 785e0908cf9..d4e3d5f3b99 100644 --- a/wasm/libs/default_executor/src/multisig/transaction.rs +++ b/wasm/libs/default_executor/src/multisig/transaction.rs @@ -42,17 +42,11 @@ impl VisitExecute for MultisigPropose { )) else { deny!(executor, "multisig proposal duplicates") }; - - // Pass validation and elevate to the multisig account authority - executor.context_mut().authority = target_account; } - fn execute( - self, - executor: &mut Executor, - init_authority: &AccountId, - ) -> Result<(), ValidationFail> { + fn execute(self, executor: &Executor) -> Result<(), ValidationFail> { let host = executor.host().clone(); + let proposer = executor.context().authority.clone(); let target_account = self.account; let instructions_hash = HashOf::new(&self.instructions); let signatories: BTreeMap = host @@ -80,7 +74,9 @@ impl VisitExecute for MultisigPropose { MultisigPropose::new(signatory, [approve_me.into()].to_vec()) }; - propose_to_approve_me.visit_execute(executor); + let mut executor = executor.clone(); + executor.context_mut().authority = target_account.clone(); + propose_to_approve_me.execute(&executor)?; } } @@ -92,7 +88,7 @@ impl VisitExecute for MultisigPropose { .try_into() .dbg_expect("shouldn't overflow within 584942417 years"); - let approvals = BTreeSet::from([init_authority]); + let approvals = BTreeSet::from([proposer]); host.submit(&SetKeyValue::account( target_account.clone(), @@ -109,7 +105,7 @@ impl VisitExecute for MultisigPropose { .dbg_unwrap(); host.submit(&SetKeyValue::account( - target_account.clone(), + target_account, approvals_key(&instructions_hash).clone(), Json::new(&approvals), )) @@ -141,17 +137,11 @@ impl VisitExecute for MultisigApprove { )) else { deny!(executor, "no proposals to approve") }; - - // Pass validation and elevate to the multisig account authority - executor.context_mut().authority = target_account; } - fn execute( - self, - executor: &mut Executor, - init_authority: &AccountId, - ) -> Result<(), ValidationFail> { + fn execute(self, executor: &Executor) -> Result<(), ValidationFail> { let host = executor.host().clone(); + let approver = executor.context().authority.clone(); let target_account = self.account; let instructions_hash = self.instructions_hash; let signatories: BTreeMap = host @@ -203,7 +193,7 @@ impl VisitExecute for MultisigApprove { .try_into_any() .dbg_unwrap(); - approvals.insert(init_authority.clone()); + approvals.insert(approver); host.submit(&SetKeyValue::account( target_account.clone(), @@ -253,7 +243,11 @@ impl VisitExecute for MultisigApprove { // Execute instructions proposal which collected enough approvals for isi in instructions { match isi { - InstructionBox::Custom(instruction) => visit_custom(executor, &instruction), + InstructionBox::Custom(instruction) => { + let mut executor = executor.clone(); + executor.context_mut().authority = target_account.clone(); + visit_custom(&mut executor, &instruction) + } builtin => host.submit(&builtin).dbg_unwrap(), } } From da8e7fb51a68f3efbc8e550ba9d4318ff5e3dc6d Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Mon, 4 Nov 2024 15:47:55 +0900 Subject: [PATCH 07/28] fix: don't just execute arbitrary approved instructions Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- .../default_executor/src/multisig/transaction.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/wasm/libs/default_executor/src/multisig/transaction.rs b/wasm/libs/default_executor/src/multisig/transaction.rs index d4e3d5f3b99..51bbe47a51b 100644 --- a/wasm/libs/default_executor/src/multisig/transaction.rs +++ b/wasm/libs/default_executor/src/multisig/transaction.rs @@ -240,16 +240,12 @@ impl VisitExecute for MultisigApprove { .dbg_unwrap(); if !is_expired { - // Execute instructions proposal which collected enough approvals - for isi in instructions { - match isi { - InstructionBox::Custom(instruction) => { - let mut executor = executor.clone(); - executor.context_mut().authority = target_account.clone(); - visit_custom(&mut executor, &instruction) - } - builtin => host.submit(&builtin).dbg_unwrap(), - } + // Validate and execute the authenticated multisig transaction + for instruction in instructions { + // Create an instance per instruction to reset the context mutation + let mut executor = executor.clone(); + executor.context_mut().authority = target_account.clone(); + executor.visit_instruction(&instruction) } } else { // TODO Notify that the proposal has expired, while returning Ok for the entry deletion to take effect From f54bd6e095f9fcd53fd96a38502d9d9e217d41f1 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Mon, 4 Nov 2024 19:25:30 +0900 Subject: [PATCH 08/28] fix: propagate executor verdict Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha/tests/multisig.rs | 140 +++++++++++++----- wasm/libs/default_executor/src/lib.rs | 2 +- .../default_executor/src/multisig/account.rs | 2 +- .../src/multisig/transaction.rs | 46 +++--- 4 files changed, 126 insertions(+), 64 deletions(-) diff --git a/crates/iroha/tests/multisig.rs b/crates/iroha/tests/multisig.rs index 84dbc4a61dd..550410a63f2 100644 --- a/crates/iroha/tests/multisig.rs +++ b/crates/iroha/tests/multisig.rs @@ -3,6 +3,7 @@ use std::{ time::Duration, }; +use derive_more::Constructor; use eyre::Result; use iroha::{ client::Client, @@ -16,14 +17,55 @@ use iroha_test_samples::{ gen_account_in, ALICE_ID, BOB_ID, BOB_KEYPAIR, CARPENTER_ID, CARPENTER_KEYPAIR, }; +#[derive(Constructor)] +struct TestSuite { + domain: DomainId, + multisig_account_id: AccountId, + unauthorized_target_opt: Option, + transaction_ttl_ms_opt: Option, +} + #[test] -fn multisig() -> Result<()> { - multisig_base(None) +fn multisig_normal() -> Result<()> { + // New domain for this test + let domain = "kingdom".parse().unwrap(); + // Create a multisig account ID and discard the corresponding private key + // FIXME #5022 refuse user input to prevent multisig monopoly and pre-registration hijacking + let multisig_account_id = gen_account_in(&domain).0; + // Make some changes to the multisig account itself + let unauthorized_target_opt = None; + // Semi-permanently valid + let transaction_ttl_ms_opt = None; + + let suite = TestSuite::new( + domain, + multisig_account_id, + unauthorized_target_opt, + transaction_ttl_ms_opt, + ); + multisig_base(suite) +} + +#[test] +fn multisig_unauthorized() -> Result<()> { + let domain = "kingdom".parse().unwrap(); + let multisig_account_id = gen_account_in(&domain).0; + // Someone that the multisig account has no permission to access + let unauthorized_target_opt = Some(ALICE_ID.clone()); + + let suite = TestSuite::new(domain, multisig_account_id, unauthorized_target_opt, None); + multisig_base(suite) } #[test] fn multisig_expires() -> Result<()> { - multisig_base(Some(2)) + let domain = "kingdom".parse().unwrap(); + let multisig_account_id = gen_account_in(&domain).0; + // Expires after 1 sec + let transaction_ttl_ms_opt = Some(1_000); + + let suite = TestSuite::new(domain, multisig_account_id, None, transaction_ttl_ms_opt); + multisig_base(suite) } /// # Scenario @@ -32,25 +74,34 @@ fn multisig_expires() -> Result<()> { /// 2. Someone in the domain registers a multisig account /// 3. One of the signatories of the multisig account proposes a multisig transaction /// 4. Other signatories approve the multisig transaction -/// 5. When the quorum is met, if it is not expired, the multisig transaction executes +/// 5. The multisig transaction executes when all of the following are met: +/// - Quorum reached: authenticated +/// - Transaction has not expired +/// - Every instruction validated against the multisig account: authorized +/// 6. Either execution or expiration on approval deletes the transaction entry #[expect(clippy::cast_possible_truncation)] -fn multisig_base(transaction_ttl_ms: Option) -> Result<()> { +fn multisig_base(suite: TestSuite) -> Result<()> { const N_SIGNATORIES: usize = 5; + let TestSuite { + domain, + multisig_account_id, + unauthorized_target_opt, + transaction_ttl_ms_opt, + } = suite; + let (network, _rt) = NetworkBuilder::new().start_blocking()?; let test_client = network.client(); - let kingdom: DomainId = "kingdom".parse().unwrap(); - // Assume some domain registered after genesis let register_and_transfer_kingdom: [InstructionBox; 2] = [ - Register::domain(Domain::new(kingdom.clone())).into(), - Transfer::domain(ALICE_ID.clone(), kingdom.clone(), BOB_ID.clone()).into(), + Register::domain(Domain::new(domain.clone())).into(), + Transfer::domain(ALICE_ID.clone(), domain.clone(), BOB_ID.clone()).into(), ]; test_client.submit_all_blocking(register_and_transfer_kingdom)?; // Populate residents in the domain - let mut residents = core::iter::repeat_with(|| gen_account_in(&kingdom)) + let mut residents = core::iter::repeat_with(|| gen_account_in(&domain)) .take(1 + N_SIGNATORIES) .collect::>(); alt_client((BOB_ID.clone(), BOB_KEYPAIR.clone()), &test_client).submit_all_blocking( @@ -61,13 +112,6 @@ fn multisig_base(transaction_ttl_ms: Option) -> Result<()> { .map(Register::account), )?; - // Create a multisig account ID and discard the corresponding private key - // FIXME #5022 refuse user input to prevent multisig monopoly and pre-registration hijacking - let multisig_account_id = gen_account_in(&kingdom).0; - - // DEBUG: You could target unauthorized one (e.g. Alice) to fail - let transaction_target = multisig_account_id.clone(); - let not_signatory = residents.pop_first().unwrap(); let mut signatories = residents; @@ -80,7 +124,7 @@ fn multisig_base(transaction_ttl_ms: Option) -> Result<()> { .collect(), // Quorum can be reached without the first signatory (1..=N_SIGNATORIES).skip(1).sum::() as u16, - transaction_ttl_ms.unwrap_or(u64::MAX), + transaction_ttl_ms_opt.unwrap_or(u64::MAX), ); // Any account in another domain cannot register a multisig account without special permission @@ -104,6 +148,10 @@ fn multisig_base(transaction_ttl_ms: Option) -> Result<()> { .expect("multisig account should be created"); let key: Name = "success_marker".parse().unwrap(); + let transaction_target = unauthorized_target_opt + .as_ref() + .unwrap_or(&multisig_account_id) + .clone(); let instructions = vec![SetKeyValue::account( transaction_target.clone(), key.clone(), @@ -120,38 +168,60 @@ fn multisig_base(transaction_ttl_ms: Option) -> Result<()> { alt_client(proposer, &test_client).submit_blocking(propose)?; - // Check that the multisig transaction has not yet executed - let _err = test_client - .query_single(FindAccountMetadata::new( - multisig_account_id.clone(), - key.clone(), - )) - .expect_err("instructions shouldn't execute without enough approvals"); - // Allow time to elapse to test the expiration - if let Some(ms) = transaction_ttl_ms { + if let Some(ms) = transaction_ttl_ms_opt { std::thread::sleep(Duration::from_millis(ms)) }; test_client.submit_blocking(Log::new(Level::DEBUG, "Just ticking time".to_string()))?; - let approve = MultisigApprove::new(multisig_account_id.clone(), instructions_hash); + let approve = MultisigApprove::new(multisig_account_id.clone(), instructions_hash.clone()); // Approve once to see if the proposal expires let approver = approvers.next().unwrap(); alt_client(approver, &test_client).submit_blocking(approve.clone())?; // Subsequent approvals should succeed unless the proposal is expired - for approver in approvers { - match alt_client(approver, &test_client).submit_blocking(approve.clone()) { - Ok(_hash) => assert!(transaction_ttl_ms.is_none()), - Err(_err) => assert!(transaction_ttl_ms.is_some()), + for _ in 0..(N_SIGNATORIES - 4) { + let approver = approvers.next().unwrap(); + let res = alt_client(approver, &test_client).submit_blocking(approve.clone()); + match &transaction_ttl_ms_opt { + None => assert!(res.is_ok()), + _ => assert!(res.is_err()), } } + // Check that the multisig transaction has not yet executed + let _err = test_client + .query_single(FindAccountMetadata::new( + transaction_target.clone(), + key.clone(), + )) + .expect_err("instructions shouldn't execute without enough approvals"); + + // The last approve to proceed to validate and execute the instructions + let approver = approvers.next().unwrap(); + let res = alt_client(approver, &test_client).submit_blocking(approve.clone()); + match (&transaction_ttl_ms_opt, &unauthorized_target_opt) { + (None, None) => assert!(res.is_ok()), + _ => assert!(res.is_err()), + } + // Check if the multisig transaction has executed - match test_client.query_single(FindAccountMetadata::new(transaction_target, key.clone())) { - Ok(_value) => assert!(transaction_ttl_ms.is_none()), - Err(_err) => assert!(transaction_ttl_ms.is_some()), + let res = test_client.query_single(FindAccountMetadata::new(transaction_target, key.clone())); + match (&transaction_ttl_ms_opt, &unauthorized_target_opt) { + (None, None) => assert!(res.is_ok()), + _ => assert!(res.is_err()), + } + + // Check if the transaction entry is deleted + let res = test_client.query_single(FindAccountMetadata::new( + multisig_account_id, + instructions_key(&instructions_hash), + )); + match (&transaction_ttl_ms_opt, &unauthorized_target_opt) { + // In case failing validation, the entry can exit only by expiring + (None, Some(_)) => assert!(res.is_ok()), + _ => assert!(res.is_err()), } Ok(()) diff --git a/wasm/libs/default_executor/src/lib.rs b/wasm/libs/default_executor/src/lib.rs index d7b31f115ac..03b8b7cf440 100644 --- a/wasm/libs/default_executor/src/lib.rs +++ b/wasm/libs/default_executor/src/lib.rs @@ -68,7 +68,7 @@ trait VisitExecute: Instruction { unimplemented!("should be overridden unless `Self::visit_execute` is overridden") } - fn execute(self, _executor: &Executor) -> Result<(), ValidationFail> { + fn execute(self, _executor: &mut Executor) -> Result<(), ValidationFail> { unimplemented!("should be overridden unless `Self::visit_execute` is overridden") } } diff --git a/wasm/libs/default_executor/src/multisig/account.rs b/wasm/libs/default_executor/src/multisig/account.rs index e6a2e1ee05b..36ae67220ef 100644 --- a/wasm/libs/default_executor/src/multisig/account.rs +++ b/wasm/libs/default_executor/src/multisig/account.rs @@ -42,7 +42,7 @@ impl VisitExecute for MultisigRegister { } } - fn execute(self, executor: &Executor) -> Result<(), ValidationFail> { + fn execute(self, executor: &mut Executor) -> Result<(), ValidationFail> { let host = executor.host(); let registrant = executor.context().authority.clone(); let multisig_account = self.account; diff --git a/wasm/libs/default_executor/src/multisig/transaction.rs b/wasm/libs/default_executor/src/multisig/transaction.rs index 51bbe47a51b..6870fa52e21 100644 --- a/wasm/libs/default_executor/src/multisig/transaction.rs +++ b/wasm/libs/default_executor/src/multisig/transaction.rs @@ -14,27 +14,14 @@ impl VisitExecute for MultisigPropose { let target_account = self.account.clone(); let multisig_role = multisig_role_for(&target_account); let instructions_hash = HashOf::new(&self.instructions); - let is_top_down_proposal = host - .query_single(FindAccountMetadata::new( - proposer.clone(), - SIGNATORIES.parse().unwrap(), - )) - .map_or(false, |proposer_signatories| { - proposer_signatories - .try_into_any::>() - .dbg_unwrap() - .contains_key(&target_account) - }); - if !is_top_down_proposal { - let Ok(_role_found) = host - .query(FindRolesByAccountId::new(proposer)) - .filter_with(|role_id| role_id.eq(multisig_role)) - .execute_single() - else { - deny!(executor, "not qualified to propose multisig"); - }; - } + let Ok(_role_found) = host + .query(FindRolesByAccountId::new(proposer)) + .filter_with(|role_id| role_id.eq(multisig_role)) + .execute_single() + else { + deny!(executor, "not qualified to propose multisig"); + }; let Err(_proposal_not_found) = host.query_single(FindAccountMetadata::new( target_account.clone(), @@ -44,7 +31,7 @@ impl VisitExecute for MultisigPropose { }; } - fn execute(self, executor: &Executor) -> Result<(), ValidationFail> { + fn execute(self, executor: &mut Executor) -> Result<(), ValidationFail> { let host = executor.host().clone(); let proposer = executor.context().authority.clone(); let target_account = self.account; @@ -74,9 +61,13 @@ impl VisitExecute for MultisigPropose { MultisigPropose::new(signatory, [approve_me.into()].to_vec()) }; - let mut executor = executor.clone(); + let init_authority = executor.context().authority.clone(); + // Authorize as the multisig account executor.context_mut().authority = target_account.clone(); - propose_to_approve_me.execute(&executor)?; + propose_to_approve_me + .execute(executor) + .dbg_expect("top-down proposals shouldn't fail"); + executor.context_mut().authority = init_authority; } } @@ -139,7 +130,7 @@ impl VisitExecute for MultisigApprove { }; } - fn execute(self, executor: &Executor) -> Result<(), ValidationFail> { + fn execute(self, executor: &mut Executor) -> Result<(), ValidationFail> { let host = executor.host().clone(); let approver = executor.context().authority.clone(); let target_account = self.account; @@ -242,10 +233,11 @@ impl VisitExecute for MultisigApprove { if !is_expired { // Validate and execute the authenticated multisig transaction for instruction in instructions { - // Create an instance per instruction to reset the context mutation - let mut executor = executor.clone(); + let init_authority = executor.context().authority.clone(); + // Authorize as the multisig account executor.context_mut().authority = target_account.clone(); - executor.visit_instruction(&instruction) + executor.visit_instruction(&instruction); + executor.context_mut().authority = init_authority; } } else { // TODO Notify that the proposal has expired, while returning Ok for the entry deletion to take effect From d75e93260557a70ec49cf3bdb8d41361046493e9 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Tue, 5 Nov 2024 15:15:59 +0900 Subject: [PATCH 09/28] review: address up to `#issuecomment-2453963837` Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- Cargo.lock | 1 - crates/iroha/tests/multisig.rs | 21 +++++-- crates/iroha_cli/src/main.rs | 18 +++++- crates/iroha_executor/src/default.rs | 24 +++----- crates/iroha_genesis/src/lib.rs | 35 ----------- crates/iroha_kagami/src/genesis/generate.rs | 2 +- crates/iroha_multisig_data_model/src/lib.rs | 60 ++----------------- crates/iroha_schema_gen/src/lib.rs | 3 - crates/iroha_telemetry_derive/Cargo.toml | 2 - defaults/genesis.json | 17 ------ .../libs/default_executor/src/multisig/mod.rs | 37 ++++++++++++ 11 files changed, 83 insertions(+), 137 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 57ecd75936e..d4aff00ed1e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3579,7 +3579,6 @@ dependencies = [ name = "iroha_telemetry_derive" version = "2.0.0-rc.1.0" dependencies = [ - "iroha_core", "iroha_macro_utils", "manyhow", "proc-macro2", diff --git a/crates/iroha/tests/multisig.rs b/crates/iroha/tests/multisig.rs index 550410a63f2..025e3179d3e 100644 --- a/crates/iroha/tests/multisig.rs +++ b/crates/iroha/tests/multisig.rs @@ -11,7 +11,6 @@ use iroha::{ data_model::{prelude::*, Level}, multisig_data_model::*, }; -use iroha_multisig_data_model::approvals_key; use iroha_test_network::*; use iroha_test_samples::{ gen_account_in, ALICE_ID, BOB_ID, BOB_KEYPAIR, CARPENTER_ID, CARPENTER_KEYPAIR, @@ -79,7 +78,7 @@ fn multisig_expires() -> Result<()> { /// - Transaction has not expired /// - Every instruction validated against the multisig account: authorized /// 6. Either execution or expiration on approval deletes the transaction entry -#[expect(clippy::cast_possible_truncation)] +#[expect(clippy::cast_possible_truncation, clippy::too_many_lines)] fn multisig_base(suite: TestSuite) -> Result<()> { const N_SIGNATORIES: usize = 5; @@ -174,7 +173,7 @@ fn multisig_base(suite: TestSuite) -> Result<()> { }; test_client.submit_blocking(Log::new(Level::DEBUG, "Just ticking time".to_string()))?; - let approve = MultisigApprove::new(multisig_account_id.clone(), instructions_hash.clone()); + let approve = MultisigApprove::new(multisig_account_id.clone(), instructions_hash); // Approve once to see if the proposal expires let approver = approvers.next().unwrap(); @@ -216,7 +215,9 @@ fn multisig_base(suite: TestSuite) -> Result<()> { // Check if the transaction entry is deleted let res = test_client.query_single(FindAccountMetadata::new( multisig_account_id, - instructions_key(&instructions_hash), + format!("proposals/{instructions_hash}/instructions") + .parse() + .unwrap(), )); match (&transaction_ttl_ms_opt, &unauthorized_target_opt) { // In case failing validation, the entry can exit only by expiring @@ -335,7 +336,9 @@ fn multisig_recursion() -> Result<()> { let approvals_at_12: BTreeSet = test_client .query_single(FindAccountMetadata::new( msa_12.clone(), - approvals_key(&approval_hash_to_12345), + format!("proposals/{approval_hash_to_12345}/approvals") + .parse() + .unwrap(), )) .expect("leaf approvals should be initialized by the root proposal") .try_into_any() @@ -381,7 +384,13 @@ fn reserved_names() { { let register = { - let role = multisig_role_for(&account_in_another_domain); + let role = format!( + "MULTISIG_SIGNATORY/{}/{}", + account_in_another_domain.domain(), + account_in_another_domain.signatory() + ) + .parse() + .unwrap(); Register::role(Role::new(role, ALICE_ID.clone())) }; let _err = test_client.submit_blocking(register).expect_err( diff --git a/crates/iroha_cli/src/main.rs b/crates/iroha_cli/src/main.rs index 2d4f4e0ee28..a09ea2109a8 100644 --- a/crates/iroha_cli/src/main.rs +++ b/crates/iroha_cli/src/main.rs @@ -1309,6 +1309,22 @@ mod multisig { } } + const DELIMITER: char = '/'; + const PROPOSALS: &str = "proposals"; + const MULTISIG_SIGNATORY: &str = "MULTISIG_SIGNATORY"; + + fn multisig_account_from(role: &RoleId) -> Option { + role.name() + .as_ref() + .strip_prefix(MULTISIG_SIGNATORY)? + .rsplit_once(DELIMITER) + .and_then(|(init, last)| { + format!("{last}@{}", init.trim_matches(DELIMITER)) + .parse() + .ok() + }) + } + /// Recursively trace back to the root multisig account fn trace_back_from( account: AccountId, @@ -1317,7 +1333,7 @@ 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(()); diff --git a/crates/iroha_executor/src/default.rs b/crates/iroha_executor/src/default.rs index 91aacb31d9e..cb3f288f367 100644 --- a/crates/iroha_executor/src/default.rs +++ b/crates/iroha_executor/src/default.rs @@ -1162,7 +1162,6 @@ pub mod parameter { pub mod role { 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::*; @@ -1229,26 +1228,18 @@ pub mod role { executor: &mut V, isi: &Register, ) { + const MULTISIG_SIGNATORY: &str = "MULTISIG_SIGNATORY"; + let role = isi.object(); let mut new_role = Role::new(role.id().clone(), role.grant_to().clone()); // Exception for multisig roles - let mut is_multisig_role = false; - if let Some(multisig_account) = multisig_account_from(role.id()) { - if crate::permission::domain::is_domain_owner( - multisig_account.domain(), - &executor.context().authority, - executor.host(), + if role.id().name().as_ref().starts_with(MULTISIG_SIGNATORY) { + // Multisig roles should only automatically be registered bypassing validation + deny!( + executor, + "role name conflicts with the reserved multisig role names" ) - .unwrap_or_default() - { - is_multisig_role = true; - } else { - deny!( - executor, - "role name conflicts with the reserved multisig role names" - ) - } } for permission in role.inner().permissions() { @@ -1277,7 +1268,6 @@ pub mod role { if executor.context().curr_block.is_genesis() || CanManageRoles.is_owned_by(&executor.context().authority, executor.host()) - || is_multisig_role { let grant_role = &Grant::account_role(role.id().clone(), role.grant_to().clone()); let isi = &Register::role(new_role); diff --git a/crates/iroha_genesis/src/lib.rs b/crates/iroha_genesis/src/lib.rs index a931693c0a6..da671bafec9 100644 --- a/crates/iroha_genesis/src/lib.rs +++ b/crates/iroha_genesis/src/lib.rs @@ -19,21 +19,6 @@ use serde::{Deserialize, Serialize}; /// Domain of the genesis account, technically required for the pre-genesis state pub static GENESIS_DOMAIN_ID: LazyLock = LazyLock::new(|| "genesis".parse().unwrap()); -/// Domain of the system account, implicitly registered in the genesis -pub static SYSTEM_DOMAIN_ID: LazyLock = LazyLock::new(|| "system".parse().unwrap()); - -/// The root authority for internal operations, implicitly registered in the genesis -// FIXME #5022 deny external access -// kagami crypto --seed "system" -pub static SYSTEM_ACCOUNT_ID: LazyLock = LazyLock::new(|| { - AccountId::new( - SYSTEM_DOMAIN_ID.clone(), - "ed0120D8B64D62FD8E09B9F29FE04D9C63E312EFB1CB29F1BF6AF00EBC263007AE75F7" - .parse() - .unwrap(), - ) -}); - /// Genesis block. /// /// First transaction must contain single [`Upgrade`] instruction to set executor. @@ -246,26 +231,6 @@ impl GenesisBuilder { } } - /// Entry system entities to serve standard functionality. - pub fn install_libs(self) -> Self { - let instructions = vec![ - Register::domain(Domain::new(SYSTEM_DOMAIN_ID.clone())).into(), - Register::account(Account::new(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, - topology: self.topology, - } - } - /// Entry a domain registration and transition to [`GenesisDomainBuilder`]. pub fn domain(self, domain_name: Name) -> GenesisDomainBuilder { self.domain_with_metadata(domain_name, Metadata::default()) diff --git a/crates/iroha_kagami/src/genesis/generate.rs b/crates/iroha_kagami/src/genesis/generate.rs index 3f39fb6e87d..227b7395ec6 100644 --- a/crates/iroha_kagami/src/genesis/generate.rs +++ b/crates/iroha_kagami/src/genesis/generate.rs @@ -64,7 +64,7 @@ impl RunArgs for Args { } = self; let chain = ChainId::from("00000000-0000-0000-0000-000000000000"); - let builder = GenesisBuilder::new(chain, executor, wasm_dir).install_libs(); + let builder = GenesisBuilder::new(chain, executor, wasm_dir); let genesis = match mode.unwrap_or_default() { Mode::Default => generate_default(builder, genesis_public_key), Mode::Synthetic { diff --git a/crates/iroha_multisig_data_model/src/lib.rs b/crates/iroha_multisig_data_model/src/lib.rs index c8b7fa66487..a0b18db74b1 100644 --- a/crates/iroha_multisig_data_model/src/lib.rs +++ b/crates/iroha_multisig_data_model/src/lib.rs @@ -94,17 +94,15 @@ macro_rules! impl_custom_instruction { fn try_from(payload: &Json) -> serde_json::Result { serde_json::from_str::(payload.as_ref()) } - } + } $( - $( - impl Instruction for $instruction {} + impl Instruction for $instruction {} - impl From<$instruction> for InstructionBox { - fn from(value: $instruction) -> Self { - Self::Custom(<$box>::from(value).into()) - } + impl From<$instruction> for InstructionBox { + fn from(value: $instruction) -> Self { + Self::Custom(<$box>::from(value).into()) } - )+ + })+ }; } @@ -112,49 +110,3 @@ impl_custom_instruction!( MultisigInstructionBox, MultisigRegister | MultisigPropose | MultisigApprove ); - -#[allow(missing_docs)] -mod names { - use super::*; - - pub const SIGNATORIES: &str = "signatories"; - pub const QUORUM: &str = "quorum"; - pub const TRANSACTION_TTL_MS: &str = "transaction_ttl_ms"; - pub const PROPOSALS: &str = "proposals"; - pub const MULTISIG_SIGNATORY_: &str = "MULTISIG_SIGNATORY_"; - - pub fn instructions_key(hash: &HashOf>) -> Name { - format!("{PROPOSALS}/{hash}/instructions").parse().unwrap() - } - - pub fn proposed_at_ms_key(hash: &HashOf>) -> Name { - format!("{PROPOSALS}/{hash}/proposed_at_ms") - .parse() - .unwrap() - } - - pub fn approvals_key(hash: &HashOf>) -> Name { - format!("{PROPOSALS}/{hash}/approvals").parse().unwrap() - } - - pub fn multisig_role_for(account: &AccountId) -> RoleId { - format!( - "{MULTISIG_SIGNATORY_}{}_{}", - account.signatory(), - account.domain() - ) - .parse() - .unwrap() - } - - pub fn multisig_account_from(role: &RoleId) -> Option { - role.name() - .as_ref() - .strip_prefix(MULTISIG_SIGNATORY_)? - .replacen('_', "@", 1) - .parse() - .ok() - } -} - -pub use names::*; diff --git a/crates/iroha_schema_gen/src/lib.rs b/crates/iroha_schema_gen/src/lib.rs index c99a1a1ccd0..e81abfad024 100644 --- a/crates/iroha_schema_gen/src/lib.rs +++ b/crates/iroha_schema_gen/src/lib.rs @@ -94,9 +94,6 @@ pub fn build_schemas() -> MetaMap { // Multi-signature operations multisig::MultisigInstructionBox, - multisig::MultisigRegister, - multisig::MultisigPropose, - multisig::MultisigApprove, // Genesis file - used by SDKs to generate the genesis block // TODO: IMO it could/should be removed from the schema diff --git a/crates/iroha_telemetry_derive/Cargo.toml b/crates/iroha_telemetry_derive/Cargo.toml index 22dab89d120..fc6ac2b76dd 100644 --- a/crates/iroha_telemetry_derive/Cargo.toml +++ b/crates/iroha_telemetry_derive/Cargo.toml @@ -30,6 +30,4 @@ manyhow = { workspace = true } iroha_macro_utils = { workspace = true } [dev-dependencies] -iroha_core = { workspace = true } - trybuild = { workspace = true } diff --git a/defaults/genesis.json b/defaults/genesis.json index 4194ea0f4f8..a859de841cf 100644 --- a/defaults/genesis.json +++ b/defaults/genesis.json @@ -24,23 +24,6 @@ } }, "instructions": [ - { - "Register": { - "Domain": { - "id": "system", - "logo": null, - "metadata": {} - } - } - }, - { - "Register": { - "Account": { - "id": "ed0120D8B64D62FD8E09B9F29FE04D9C63E312EFB1CB29F1BF6AF00EBC263007AE75F7@system", - "metadata": {} - } - } - }, { "Register": { "Domain": { diff --git a/wasm/libs/default_executor/src/multisig/mod.rs b/wasm/libs/default_executor/src/multisig/mod.rs index a982ebe6a52..d3e79b5e2f0 100644 --- a/wasm/libs/default_executor/src/multisig/mod.rs +++ b/wasm/libs/default_executor/src/multisig/mod.rs @@ -1,3 +1,5 @@ +use alloc::format; + use super::*; mod account; @@ -13,4 +15,39 @@ impl VisitExecute for MultisigInstructionBox { } } +const DELIMITER: char = '/'; +const SIGNATORIES: &str = "signatories"; +const QUORUM: &str = "quorum"; +const TRANSACTION_TTL_MS: &str = "transaction_ttl_ms"; +const PROPOSALS: &str = "proposals"; +const MULTISIG_SIGNATORY: &str = "MULTISIG_SIGNATORY"; + +fn instructions_key(hash: &HashOf>) -> Name { + format!("{PROPOSALS}{DELIMITER}{hash}{DELIMITER}instructions") + .parse() + .unwrap() +} + +fn proposed_at_ms_key(hash: &HashOf>) -> Name { + format!("{PROPOSALS}{DELIMITER}{hash}{DELIMITER}proposed_at_ms") + .parse() + .unwrap() +} + +fn approvals_key(hash: &HashOf>) -> Name { + format!("{PROPOSALS}{DELIMITER}{hash}{DELIMITER}approvals") + .parse() + .unwrap() +} + +fn multisig_role_for(account: &AccountId) -> RoleId { + format!( + "{MULTISIG_SIGNATORY}{DELIMITER}{}{DELIMITER}{}", + account.domain(), + account.signatory(), + ) + .parse() + .unwrap() +} + pub(super) use iroha_multisig_data_model::*; From 0e8f7e63edb4ad748a902aadb4be161839bffd26 Mon Sep 17 00:00:00 2001 From: Ekaterina Mekhnetsova Date: Tue, 5 Nov 2024 14:36:44 +0300 Subject: [PATCH 10/28] docs: Update readmes (#5219) Signed-off-by: Ekaterina Mekhnetsova --- crates/iroha_cli/README.md | 71 +++++++------------------------- docs/README.md | 18 +++----- docs/source/guides/hot-reload.md | 71 -------------------------------- 3 files changed, 20 insertions(+), 140 deletions(-) delete mode 100644 docs/source/guides/hot-reload.md diff --git a/crates/iroha_cli/README.md b/crates/iroha_cli/README.md index 30295aac38b..57a50db8136 100644 --- a/crates/iroha_cli/README.md +++ b/crates/iroha_cli/README.md @@ -1,12 +1,6 @@ # Iroha CLI Client -With the Iroha CLI Client, you can interact with Iroha Peers Web API. -It is a "thin" wrapper around functionality exposed in the `iroha` crate. Specifically, it should be used as a reference for using `iroha`'s features, and not as a production-ready client. As such, the CLI client is not guaranteed to support all features supported by the client library. - -## Features - -* Submit Transactions with your Iroha Special Instructions to Iroha Peers -* Send Requests with your Queries to Iroha Peers +Iroha Client CLI is a "thin" wrapper around functionality exposed in the `iroha` crate. Specifically, it should be used as a reference for using `iroha`'s features, and not as a production-ready client. As such, the CLI client is not guaranteed to support all features supported by the client library. Check [Iroha 2 documentation](https://docs.iroha.tech/get-started/operate-iroha-2-via-cli.html) for a detailed tutorial on working with Iroha Client CLI. ## Installation @@ -52,8 +46,6 @@ iroha [OPTIONS] Refer to [Iroha Special Instructions](https://docs.iroha.tech/blockchain/instructions.html) for more information about Iroha instructions such as register, mint, grant, and so on. -Check the [getting started guide](https://docs.iroha.tech/get-started/operate-iroha-2-via-cli.html) for detailed instructions on working with Iroha Client CLI. - ## Examples :grey_exclamation: All examples below are Unix-oriented. If you're working on Windows, we would highly encourage you to consider using WSL, as most documentation assumes a POSIX-like shell running on your system. Please be advised that the differences in the syntax may go beyond executing `iroha.exe` instead of `iroha`. @@ -68,35 +60,24 @@ Check the [getting started guide](https://docs.iroha.tech/get-started/operate-ir In this section we will show you how to use Iroha CLI Client to do the following: -- [Iroha CLI Client](#iroha-cli-client) - - [Features](#features) - - [Installation](#installation) - - [Usage](#usage) - - [Options](#options) - - [Subcommands](#subcommands) - - [Examples](#examples) - - [Create new Domain](#create-new-domain) - - [Create new Account](#create-new-account) - - [Mint Asset to Account](#mint-asset-to-account) - - [Query Account Assets Quantity](#query-account-assets-quantity) - - [Execute WASM transaction](#execute-wasm-transaction) - - [Execute Multi-instruction Transactions](#execute-multi-instruction-transactions) + - [Create new Domain](#create-new-domain) + - [Create new Account](#create-new-account) + - [Mint Asset to Account](#mint-asset-to-account) + - [Query Account Assets Quantity](#query-account-assets-quantity) + - [Execute WASM transaction](#execute-wasm-transaction) + - [Execute Multi-instruction Transactions](#execute-multi-instruction-transactions) ### Create new Domain -Let's start with domain creation. To create a domain, you need to specify the entity type first (`domain` in our case) and then the command (`register`) with a list of required parameters. - -For the `domain` entity, you only need to provide the `id` argument as a string that doesn't contain the `@` and `#` symbols. +To create a domain, you need to specify the entity type first (`domain` in our case) and then the command (`register`) with a list of required parameters. For the `domain` entity, you only need to provide the `id` argument as a string that doesn't contain the `@` and `#` symbols. ```bash ./iroha domain register --id="Soramitsu" ``` -Now you have a domain without any accounts. - ### Create new Account -Let's create a new account. Like in the previous example, specify the entity type (`account`) and the command (`register`). Then define the value of the `id` argument in "signatory@domain" format, where signatory is the account's public key in multihash representation. +To create an account, specify the entity type (`account`) and the command (`register`). Then define the value of the `id` argument in "signatory@domain" format, where signatory is the account's public key in multihash representation: ```bash ./iroha account register --id="ed01204A3C5A6B77BBE439969F95F0AA4E01AE31EC45A0D68C131B2C622751FCC5E3B6@Soramitsu" @@ -104,33 +85,18 @@ Let's create a new account. Like in the previous example, specify the entity typ ### Mint Asset to Account -It's time to give something to the Account you created. Let's add some Assets of the type `Quantity` to the account. - -To do so, you must first register an Asset Definition and only then add some Assets to the account. Specify the `asset` entity and then use the `register` and `mint` commands respectively. - -Every asset has its own value spec. In this example, it is defined as `Numeric`, a 96-bit unsigned decimal. We also support `Store` for key-value structured data. +To add assets to the account, you must first register an Asset Definition. Specify the `asset` entity and then use the `register` and `mint` commands respectively. Here is an example of adding Assets of the type `Quantity` to the account: ```bash ./iroha asset register --id="XOR#Soramitsu" --type=Numeric ./iroha asset mint --account="ed01204A3C5A6B77BBE439969F95F0AA4E01AE31EC45A0D68C131B2C622751FCC5E3B6@Soramitsu" --asset="XOR#Soramitsu" --quantity=1010 ``` -You created `XOR#Soramitsu`, an asset of type `Numeric`, and then gave `1010` units of this asset to the account `ed01204A3C5A6B77BBE439969F95F0AA4E01AE31EC45A0D68C131B2C622751FCC5E3B6@Soramitsu`. +With this, you created `XOR#Soramitsu`, an asset of type `Numeric`, and then gave `1010` units of this asset to the account `ed01204A3C5A6B77BBE439969F95F0AA4E01AE31EC45A0D68C131B2C622751FCC5E3B6@Soramitsu`. ### Query Account Assets Quantity -Because distributed systems heavily rely on the concept of _eventual_ consistency and Iroha works by awaiting consensus between peers, your request is not guaranteed to be processed (or be accepted) even if it is correctly formed. -While the Iroha Client will successfully send your transactions and the Iroha Peer will confirm receiving them, it is possible that your request will not appear in the next block. - -Different causes such as a transaction timeout, a faulty peer in the network, catastrophic failure of the peer that you've sent your data towards, and many other conditions naturally occurring inside of any blockchain may lead to a rejection of your transaction at many different stages of processing. - -It should be noted that Iroha is designed to reduce the incidence of such rejections, and only rejects properly formed transactions in situations when not rejecting it would lead to data corruption and a hard-fork of the network. - -As such it's important to check that your instructions were applied and the _world_ is now in the desired state. -For this you need to use Query API. - -Let's use Get Account Assets Query as an example. -To know how many units of a particular asset an account has, use `asset get` with the specified account and asset: +You can use Query API to check that your instructions were applied and the _world_ is in the desired state. For example, to know how many units of a particular asset an account has, use `asset get` with the specified account and asset: ```bash ./iroha asset get --account="ed01204A3C5A6B77BBE439969F95F0AA4E01AE31EC45A0D68C131B2C622751FCC5E3B6@Soramitsu" --asset="XOR#Soramitsu" @@ -138,24 +104,15 @@ To know how many units of a particular asset an account has, use `asset get` wit This query returns the quantity of `XOR#Soramitsu` asset for the `ed01204A3C5A6B77BBE439969F95F0AA4E01AE31EC45A0D68C131B2C622751FCC5E3B6@Soramitsu` account. -It's possible to filter based on either account, asset or domain id by using the filtering API provided by the Iroha client CLI. - -Generally it looks like this: - -```bash -./iroha ENTITY list filter PREDICATE -``` - -Where ENTITY is asset, account or domain and PREDICATE is condition used for filtering serialized using JSON5 (check `iroha::data_model::predicate::value::ValuePredicate` type). +You can also filter based on either account, asset or domain id by using the filtering API provided by the Iroha client CLI. Generally, filtering follows the `./iroha ENTITY list filter PREDICATE` pattern, where ENTITY is asset, account or domain and PREDICATE is condition used for filtering serialized using JSON5 (check `iroha::data_model::predicate::value::ValuePredicate` type). -Examples: +Here are some examples of filtering: ```bash # Filter domains by id ./iroha domain list filter '{"Identifiable": {"Is": "wonderland"}}' # Filter accounts by domain ./iroha account list filter '{"Identifiable": {"EndsWith": "@wonderland"}}' -# It is possible to combine filters using "Or" or "And" # Filter asset by domain ./iroha asset list filter '{"Or": [{"Identifiable": {"Contains": "#wonderland#"}}, {"And": [{"Identifiable": {"Contains": "##"}}, {"Identifiable": {"EndsWith": "@wonderland"}}]}]}' ``` diff --git a/docs/README.md b/docs/README.md index b907992e83b..457f3a3cf17 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1,23 +1,17 @@ # Iroha 2 Documentation -This is the main Iroha 2 documentation that you will find useful: +In the [main Iroha 2 documentation](https://docs.iroha.tech/) you will find: -- [Tutorial](https://docs.iroha.tech/guide/tutorials/) +- [Get Started Guide](https://docs.iroha.tech/get-started/) +- [SDK Tutorials](https://docs.iroha.tech/guide/tutorials/) for Rust, Python, Javascript, and Java/Kotlin - [API Reference](https://docs.iroha.tech/reference/torii-endpoints.html) - -- [Iroha 2 Whitepaper](./source/iroha_2_whitepaper.md) + +You can also check out [Iroha 2 Whitepaper](./source/iroha_2_whitepaper.md) for Iroha 2 specification. ## Tools -Documentation for Iroha 2 tools: +In this repository you can find documentation for Iroha 2 tools: - [Kagami](../crates/iroha_kagami/README.md) - [Kura Inspector](../crates/kura_inspector/README.md) - [Parity Scale Decoder Tool](../crates/iroha_codec/README.md) - -## Development - -The following is useful for development: - -- [Hot reload Iroha in a Docker container](./source/guides/hot-reload.md) -- [Benchmark your code](../crates/iroha/benches/tps/README.md) diff --git a/docs/source/guides/hot-reload.md b/docs/source/guides/hot-reload.md deleted file mode 100644 index 421e5ccd64f..00000000000 --- a/docs/source/guides/hot-reload.md +++ /dev/null @@ -1,71 +0,0 @@ -# How to hot reload Iroha in a Docker container - -Here is the overall procedure for hot reloading Iroha in a Docker container: - -1. Build Iroha on your host OS. - - To avoid issues with dynamic linking, run: - - ```bash - cargo build --release --target x86_64-unknown-linux-musl - ``` - -
An explanation for using `cargo build` with these parameters. - - You may experience an issue with dynamic linking if your host OS has a newer version of `glibc` compared to the one in the Docker container. The options used in the command above resolve the issue: - - - `--target x86_64-unknown-linux-musl` forces static linking against `musl` libc implementation - -
- -2. Enter Docker container. For example: - - ```bash - docker exec -it iroha-iroha0-1 bash - ``` - -3. Copy Iroha to the current directory: - - ```bash - docker cp root/soramitsu/iroha/target/x86_64-unknown-linux-musl/release/irohad . - ``` - -4. (Optional) Make any modifications you need: - - - [Recommit genesis block](#wiping-previous-blockchain-state-recommit-genesis) - - [Use custom configuration files](#use-custom-configuration-files) - - [Use custom environment variables](#use-custom-environment-variables) - -5. Exit docker container and restart it using `docker restart`. - - **Note:** If you use the combination of `container down` and `container up`, any modifications you made on the previous step will be lost. Use `docker restart` to preserve changes. - -If you skip the optional step (step 4), the state of the blockchain after hot reload will be the same as it was before the Docker container was restarted. - -Note that if you get the `Kura initialisation failed` error message, it might mean one of two things: corruption or binary incompatibility of the stored block. To fix this, remove the `blocks/` directory. - -## Wiping previous blockchain state (recommit genesis) - -To recommit a custom genesis block, remove the previously stored blocks before restarting the container: - -```bash -rm blocks/* -``` - -The new genesis block will be automatically recommitted upon container restart. - -## Use custom configuration files - -To use custom configuration files, such as `config.json` or `genesis.json`, copy (or bind mount) them to the `config/` subvolume before restarting the Docker container. - -The changes will take effect upon container restart. - -## Use custom environment variables - -To use custom environment variables (e.g. `IROHA_PUBLIC_KEY`), simply modify them before restarting the Docker container. For example: - -```bash - IROHA_PUBLIC_KEY= docker restart -``` - -The changes will take effect upon container restart. From 3c1888d85a754aef832bc6dadb2ab5283a1f641f Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Wed, 6 Nov 2024 17:58:54 +0900 Subject: [PATCH 11/28] review: rename to `visit_custom_instructions` Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha_data_model/src/visit.rs | 6 +++--- crates/iroha_executor/src/default.rs | 6 +++--- crates/iroha_executor_derive/src/default.rs | 2 +- wasm/libs/default_executor/src/lib.rs | 4 ++-- .../samples/executor_custom_instructions_complex/src/lib.rs | 4 ++-- wasm/samples/executor_custom_instructions_simple/src/lib.rs | 4 ++-- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/iroha_data_model/src/visit.rs b/crates/iroha_data_model/src/visit.rs index 92b8709c618..164d1df80ef 100644 --- a/crates/iroha_data_model/src/visit.rs +++ b/crates/iroha_data_model/src/visit.rs @@ -49,7 +49,7 @@ pub trait Visit { visit_execute_trigger(&ExecuteTrigger), visit_set_parameter(&SetParameter), visit_log(&Log), - visit_custom(&CustomInstruction), + visit_custom_instructions(&CustomInstruction), // Visit SingularQueryBox visit_find_asset_quantity_by_id(&FindAssetQuantityById), @@ -230,7 +230,7 @@ pub fn visit_instruction(visitor: &mut V, isi: &InstructionBo InstructionBox::Transfer(variant_value) => visitor.visit_transfer(variant_value), InstructionBox::Unregister(variant_value) => visitor.visit_unregister(variant_value), InstructionBox::Upgrade(variant_value) => visitor.visit_upgrade(variant_value), - InstructionBox::Custom(custom) => visitor.visit_custom(custom), + InstructionBox::Custom(custom) => visitor.visit_custom_instructions(custom), } } @@ -373,7 +373,7 @@ leaf_visitors! { visit_set_parameter(&SetParameter), visit_execute_trigger(&ExecuteTrigger), visit_log(&Log), - visit_custom(&CustomInstruction), + visit_custom_instructions(&CustomInstruction), // Singular Quert visitors visit_find_asset_quantity_by_id(&FindAssetQuantityById), diff --git a/crates/iroha_executor/src/default.rs b/crates/iroha_executor/src/default.rs index cb3f288f367..4c14248d7cc 100644 --- a/crates/iroha_executor/src/default.rs +++ b/crates/iroha_executor/src/default.rs @@ -17,7 +17,7 @@ pub use asset_definition::{ visit_set_asset_definition_key_value, visit_transfer_asset_definition, visit_unregister_asset_definition, }; -pub use custom::visit_custom; +pub use custom::visit_custom_instructions; pub use domain::{ visit_register_domain, visit_remove_domain_key_value, visit_set_domain_key_value, visit_transfer_domain, visit_unregister_domain, @@ -117,7 +117,7 @@ pub fn visit_instruction(executor: &mut V, isi: &In executor.visit_upgrade(isi); } InstructionBox::Custom(isi) => { - executor.visit_custom(isi); + executor.visit_custom_instructions(isi); } } } @@ -1669,7 +1669,7 @@ pub mod log { pub mod custom { use super::*; - pub fn visit_custom(executor: &mut V, _isi: &CustomInstruction) { + pub fn visit_custom_instructions(executor: &mut V, _isi: &CustomInstruction) { deny!( executor, "Custom instructions should be handled in custom executor" diff --git a/crates/iroha_executor_derive/src/default.rs b/crates/iroha_executor_derive/src/default.rs index d6474d4763c..37ada85459d 100644 --- a/crates/iroha_executor_derive/src/default.rs +++ b/crates/iroha_executor_derive/src/default.rs @@ -155,7 +155,7 @@ pub fn impl_derive_visit(emitter: &mut Emitter, input: &syn::DeriveInput) -> Tok "fn visit_set_parameter(operation: &SetParameter)", "fn visit_upgrade(operation: &Upgrade)", "fn visit_log(operation: &Log)", - "fn visit_custom(operation: &CustomInstruction)", + "fn visit_custom_instructions(operation: &CustomInstruction)", ] .into_iter() .map(|item| { diff --git a/wasm/libs/default_executor/src/lib.rs b/wasm/libs/default_executor/src/lib.rs index 03b8b7cf440..b357bff935b 100644 --- a/wasm/libs/default_executor/src/lib.rs +++ b/wasm/libs/default_executor/src/lib.rs @@ -28,7 +28,7 @@ mod multisig; /// /// The defaults are not guaranteed to be stable. #[derive(Debug, Clone, Visit, Execute, Entrypoints)] -#[visit(custom(visit_custom))] +#[visit(custom(visit_custom_instructions))] struct Executor { host: Iroha, context: Context, @@ -46,7 +46,7 @@ impl Executor { } } -fn visit_custom(executor: &mut Executor, isi: &CustomInstruction) { +fn visit_custom_instructions(executor: &mut Executor, isi: &CustomInstruction) { if let Ok(isi) = multisig::MultisigInstructionBox::try_from(isi.payload()) { return isi.visit_execute(executor); }; diff --git a/wasm/samples/executor_custom_instructions_complex/src/lib.rs b/wasm/samples/executor_custom_instructions_complex/src/lib.rs index 549c7e3c162..73f6f409cbc 100644 --- a/wasm/samples/executor_custom_instructions_complex/src/lib.rs +++ b/wasm/samples/executor_custom_instructions_complex/src/lib.rs @@ -25,14 +25,14 @@ static ALLOC: GlobalDlmalloc = GlobalDlmalloc; getrandom::register_custom_getrandom!(iroha_executor::stub_getrandom); #[derive(Visit, Execute, Entrypoints)] -#[visit(custom(visit_custom))] +#[visit(custom(visit_custom_instructions))] struct Executor { host: Iroha, context: iroha_executor::prelude::Context, verdict: Result, } -fn visit_custom(executor: &mut Executor, isi: &CustomInstruction) { +fn visit_custom_instructions(executor: &mut Executor, isi: &CustomInstruction) { let Ok(isi) = CustomInstructionExpr::try_from(isi.payload()) else { deny!(executor, "Failed to parse custom instruction"); }; diff --git a/wasm/samples/executor_custom_instructions_simple/src/lib.rs b/wasm/samples/executor_custom_instructions_simple/src/lib.rs index 88afdac4eed..8c745232444 100644 --- a/wasm/samples/executor_custom_instructions_simple/src/lib.rs +++ b/wasm/samples/executor_custom_instructions_simple/src/lib.rs @@ -18,14 +18,14 @@ static ALLOC: GlobalDlmalloc = GlobalDlmalloc; getrandom::register_custom_getrandom!(iroha_executor::stub_getrandom); #[derive(Visit, Execute, Entrypoints)] -#[visit(custom(visit_custom))] +#[visit(custom(visit_custom_instructions))] struct Executor { host: Iroha, context: Context, verdict: Result, } -fn visit_custom(executor: &mut Executor, isi: &CustomInstruction) { +fn visit_custom_instructions(executor: &mut Executor, isi: &CustomInstruction) { let Ok(isi) = CustomInstructionBox::try_from(isi.payload()) else { deny!(executor, "Failed to parse custom instruction"); }; From bc1d497fb97e354784c4f30178f8ce43cb8d3ad6 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Wed, 6 Nov 2024 20:54:19 +0900 Subject: [PATCH 12/28] review: move to `{iroha_executor::default, iroha_executor_data_model}::custom::multisig` Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- Cargo.lock | 17 +-- Cargo.toml | 2 - crates/iroha/Cargo.toml | 3 +- crates/iroha/src/lib.rs | 2 +- crates/iroha/tests/multisig.rs | 2 +- crates/iroha_cli/src/main.rs | 2 +- crates/iroha_data_model/src/isi.rs | 1 + crates/iroha_executor/Cargo.toml | 1 - .../iroha_executor/src/default/custom/mod.rs | 37 ++++++ .../src/default/custom}/multisig/account.rs | 4 +- .../src/default/custom}/multisig/mod.rs | 9 +- .../default/custom}/multisig/transaction.rs | 13 +- .../src/{default.rs => default/mod.rs} | 13 +- crates/iroha_executor/src/lib.rs | 4 +- crates/iroha_executor_data_model/Cargo.toml | 2 + .../iroha_executor_data_model/src/custom.rs | 117 ++++++++++++++++++ crates/iroha_executor_data_model/src/lib.rs | 1 + crates/iroha_multisig_data_model/Cargo.toml | 20 --- crates/iroha_multisig_data_model/src/lib.rs | 112 ----------------- crates/iroha_schema_gen/Cargo.toml | 1 - crates/iroha_schema_gen/src/lib.rs | 7 +- wasm/Cargo.toml | 1 - wasm/libs/default_executor/Cargo.toml | 1 - wasm/libs/default_executor/src/lib.rs | 44 +------ 24 files changed, 185 insertions(+), 231 deletions(-) create mode 100644 crates/iroha_executor/src/default/custom/mod.rs rename {wasm/libs/default_executor/src => crates/iroha_executor/src/default/custom}/multisig/account.rs (95%) rename {wasm/libs/default_executor/src => crates/iroha_executor/src/default/custom}/multisig/mod.rs (86%) rename {wasm/libs/default_executor/src => crates/iroha_executor/src/default/custom}/multisig/transaction.rs (95%) rename crates/iroha_executor/src/{default.rs => default/mod.rs} (99%) create mode 100644 crates/iroha_executor_data_model/src/custom.rs delete mode 100644 crates/iroha_multisig_data_model/Cargo.toml delete mode 100644 crates/iroha_multisig_data_model/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index d4aff00ed1e..700e4450810 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2924,7 +2924,6 @@ dependencies = [ "iroha_executor_data_model", "iroha_genesis", "iroha_logger", - "iroha_multisig_data_model", "iroha_primitives", "iroha_telemetry", "iroha_test_network", @@ -3197,7 +3196,6 @@ version = "2.0.0-rc.1.0" dependencies = [ "iroha_executor_data_model", "iroha_executor_derive", - "iroha_multisig_data_model", "iroha_schema", "iroha_smart_contract", "iroha_smart_contract_utils", @@ -3209,9 +3207,11 @@ dependencies = [ name = "iroha_executor_data_model" version = "2.0.0-rc.1.0" dependencies = [ + "derive_more", "iroha_data_model", "iroha_executor_data_model_derive", "iroha_schema", + "parity-scale-codec", "serde", "serde_json", ] @@ -3367,18 +3367,6 @@ dependencies = [ "syn 2.0.79", ] -[[package]] -name = "iroha_multisig_data_model" -version = "2.0.0-rc.1.0" -dependencies = [ - "derive_more", - "iroha_data_model", - "iroha_schema", - "parity-scale-codec", - "serde", - "serde_json", -] - [[package]] name = "iroha_numeric" version = "2.0.0-rc.1.0" @@ -3484,7 +3472,6 @@ dependencies = [ "iroha_data_model", "iroha_executor_data_model", "iroha_genesis", - "iroha_multisig_data_model", "iroha_primitives", "iroha_schema", ] diff --git a/Cargo.toml b/Cargo.toml index 9dc7f75c3e1..e99ba6408ea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,8 +46,6 @@ iroha_smart_contract_utils = { version = "=2.0.0-rc.1.0", path = "crates/iroha_s iroha_executor = { version = "=2.0.0-rc.1.0", path = "crates/iroha_executor" } iroha_executor_data_model = { version = "=2.0.0-rc.1.0", path = "crates/iroha_executor_data_model" } -iroha_multisig_data_model = { version = "=2.0.0-rc.1.0", path = "crates/iroha_multisig_data_model" } - iroha_test_network = { version = "=2.0.0-rc.1.0", path = "crates/iroha_test_network" } iroha_test_samples = { version = "=2.0.0-rc.1.0", path = "crates/iroha_test_samples" } diff --git a/crates/iroha/Cargo.toml b/crates/iroha/Cargo.toml index 8b029bb9ff2..dbce2fd5b09 100644 --- a/crates/iroha/Cargo.toml +++ b/crates/iroha/Cargo.toml @@ -52,12 +52,12 @@ iroha_config = { workspace = true } iroha_config_base = { workspace = true } iroha_crypto = { workspace = true } iroha_data_model = { workspace = true, features = ["http"] } +iroha_executor_data_model = { workspace = true } iroha_primitives = { workspace = true } iroha_logger = { workspace = true } iroha_telemetry = { workspace = true } iroha_torii_const = { workspace = true } iroha_version = { workspace = true } -iroha_multisig_data_model = { workspace = true } attohttpc = { version = "0.28.0", default-features = false } eyre = { workspace = true } @@ -84,7 +84,6 @@ nonzero_ext = { workspace = true } iroha_genesis = { workspace = true } iroha_test_samples = { workspace = true } iroha_test_network = { workspace = true } -iroha_executor_data_model = { workspace = true } executor_custom_data_model = { version = "=2.0.0-rc.1.0", path = "../../wasm/samples/executor_custom_data_model" } tokio = { workspace = true, features = ["rt-multi-thread"] } diff --git a/crates/iroha/src/lib.rs b/crates/iroha/src/lib.rs index 0daf1930e55..02c3553fb1d 100644 --- a/crates/iroha/src/lib.rs +++ b/crates/iroha/src/lib.rs @@ -9,4 +9,4 @@ mod secrecy; pub use iroha_crypto as crypto; pub use iroha_data_model as data_model; -pub use iroha_multisig_data_model as multisig_data_model; +pub use iroha_executor_data_model as executor_data_model; diff --git a/crates/iroha/tests/multisig.rs b/crates/iroha/tests/multisig.rs index 025e3179d3e..575e2af6666 100644 --- a/crates/iroha/tests/multisig.rs +++ b/crates/iroha/tests/multisig.rs @@ -9,7 +9,7 @@ use iroha::{ client::Client, crypto::KeyPair, data_model::{prelude::*, Level}, - multisig_data_model::*, + executor_data_model::custom::multisig::*, }; use iroha_test_network::*; use iroha_test_samples::{ diff --git a/crates/iroha_cli/src/main.rs b/crates/iroha_cli/src/main.rs index a09ea2109a8..3301d7831c1 100644 --- a/crates/iroha_cli/src/main.rs +++ b/crates/iroha_cli/src/main.rs @@ -1179,7 +1179,7 @@ mod json { mod multisig { use std::io::{BufReader, Read as _}; - use iroha::multisig_data_model::*; + use iroha::executor_data_model::custom::multisig::*; use super::*; diff --git a/crates/iroha_data_model/src/isi.rs b/crates/iroha_data_model/src/isi.rs index 75b3b341c32..4c93855a3ac 100644 --- a/crates/iroha_data_model/src/isi.rs +++ b/crates/iroha_data_model/src/isi.rs @@ -979,6 +979,7 @@ mod transparent { /// integration tests #[derive(Display)] #[display(fmt = "CUSTOM({payload})")] + // TODO #5221 pub struct RawCustomInstruction { pub struct CustomInstruction { /// Custom payload pub payload: Json, diff --git a/crates/iroha_executor/Cargo.toml b/crates/iroha_executor/Cargo.toml index 780a7f66bab..40f012a1dad 100644 --- a/crates/iroha_executor/Cargo.toml +++ b/crates/iroha_executor/Cargo.toml @@ -17,7 +17,6 @@ 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/custom/mod.rs b/crates/iroha_executor/src/default/custom/mod.rs new file mode 100644 index 00000000000..e92b3cecda6 --- /dev/null +++ b/crates/iroha_executor/src/default/custom/mod.rs @@ -0,0 +1,37 @@ +use iroha_executor_data_model::custom::multisig::MultisigInstructionBox; + +use super::*; +use crate::prelude::{Execute, Vec, Visit}; + +mod multisig; + +pub fn visit_custom_instructions( + executor: &mut V, + isi: &CustomInstruction, +) { + if let Ok(isi) = MultisigInstructionBox::try_from(isi.payload()) { + return isi.visit_execute(executor); + }; + + deny!(executor, "unexpected custom instruction"); +} + +// TODO #5221 trait VisitExecute: CustomInstruction { +trait VisitExecute: crate::data_model::isi::Instruction { + fn visit_execute(self, executor: &mut V) { + self.visit(executor); + if executor.verdict().is_ok() { + if let Err(err) = self.execute(executor) { + executor.deny(err); + } + } + } + + fn visit(&self, _executor: &mut V) { + unimplemented!("should be overridden unless `Self::visit_execute` is overridden") + } + + fn execute(self, _executor: &mut V) -> Result<(), ValidationFail> { + unimplemented!("should be overridden unless `Self::visit_execute` is overridden") + } +} diff --git a/wasm/libs/default_executor/src/multisig/account.rs b/crates/iroha_executor/src/default/custom/multisig/account.rs similarity index 95% rename from wasm/libs/default_executor/src/multisig/account.rs rename to crates/iroha_executor/src/default/custom/multisig/account.rs index 36ae67220ef..d9fd9518248 100644 --- a/wasm/libs/default_executor/src/multisig/account.rs +++ b/crates/iroha_executor/src/default/custom/multisig/account.rs @@ -3,7 +3,7 @@ use super::*; impl VisitExecute for MultisigRegister { - fn visit(&self, executor: &mut Executor) { + fn visit(&self, executor: &mut V) { let host = executor.host(); let target_domain = self.account.domain(); @@ -42,7 +42,7 @@ impl VisitExecute for MultisigRegister { } } - fn execute(self, executor: &mut Executor) -> Result<(), ValidationFail> { + fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { let host = executor.host(); let registrant = executor.context().authority.clone(); let multisig_account = self.account; diff --git a/wasm/libs/default_executor/src/multisig/mod.rs b/crates/iroha_executor/src/default/custom/multisig/mod.rs similarity index 86% rename from wasm/libs/default_executor/src/multisig/mod.rs rename to crates/iroha_executor/src/default/custom/multisig/mod.rs index d3e79b5e2f0..4dd23d77ce4 100644 --- a/wasm/libs/default_executor/src/multisig/mod.rs +++ b/crates/iroha_executor/src/default/custom/multisig/mod.rs @@ -1,12 +1,15 @@ -use alloc::format; +use iroha_executor_data_model::custom::multisig::*; use super::*; +// SATO remove after merge +use crate::smart_contract::debug::{DebugExpectExt as _, DebugUnwrapExt}; + mod account; mod transaction; impl VisitExecute for MultisigInstructionBox { - fn visit_execute(self, executor: &mut Executor) { + fn visit_execute(self, executor: &mut V) { match self { MultisigInstructionBox::Register(instruction) => instruction.visit_execute(executor), MultisigInstructionBox::Propose(instruction) => instruction.visit_execute(executor), @@ -49,5 +52,3 @@ fn multisig_role_for(account: &AccountId) -> RoleId { .parse() .unwrap() } - -pub(super) use iroha_multisig_data_model::*; diff --git a/wasm/libs/default_executor/src/multisig/transaction.rs b/crates/iroha_executor/src/default/custom/multisig/transaction.rs similarity index 95% rename from wasm/libs/default_executor/src/multisig/transaction.rs rename to crates/iroha_executor/src/default/custom/multisig/transaction.rs index 6870fa52e21..e3c30310dd3 100644 --- a/wasm/libs/default_executor/src/multisig/transaction.rs +++ b/crates/iroha_executor/src/default/custom/multisig/transaction.rs @@ -1,14 +1,11 @@ //! Validation and execution logic of instructions for multisig transactions -use alloc::{ - collections::{btree_map::BTreeMap, btree_set::BTreeSet}, - vec::Vec, -}; +use alloc::collections::{btree_map::BTreeMap, btree_set::BTreeSet}; use super::*; impl VisitExecute for MultisigPropose { - fn visit(&self, executor: &mut Executor) { + fn visit(&self, executor: &mut V) { let host = executor.host(); let proposer = executor.context().authority.clone(); let target_account = self.account.clone(); @@ -31,7 +28,7 @@ impl VisitExecute for MultisigPropose { }; } - fn execute(self, executor: &mut Executor) -> Result<(), ValidationFail> { + fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { let host = executor.host().clone(); let proposer = executor.context().authority.clone(); let target_account = self.account; @@ -107,7 +104,7 @@ impl VisitExecute for MultisigPropose { } impl VisitExecute for MultisigApprove { - fn visit(&self, executor: &mut Executor) { + fn visit(&self, executor: &mut V) { let host = executor.host(); let approver = executor.context().authority.clone(); let target_account = self.account.clone(); @@ -130,7 +127,7 @@ impl VisitExecute for MultisigApprove { }; } - fn execute(self, executor: &mut Executor) -> Result<(), ValidationFail> { + fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { let host = executor.host().clone(); let approver = executor.context().authority.clone(); let target_account = self.account; diff --git a/crates/iroha_executor/src/default.rs b/crates/iroha_executor/src/default/mod.rs similarity index 99% rename from crates/iroha_executor/src/default.rs rename to crates/iroha_executor/src/default/mod.rs index 4c14248d7cc..0274159b733 100644 --- a/crates/iroha_executor/src/default.rs +++ b/crates/iroha_executor/src/default/mod.rs @@ -44,6 +44,8 @@ use crate::{ Execute, }; +pub mod custom; + // NOTE: If any new `visit_..` functions are introduced in this module, one should // not forget to update the default executor boilerplate too, specifically the // `iroha_executor::derive::default::impl_derive_visit` function @@ -1665,14 +1667,3 @@ pub mod log { execute!(executor, isi) } } - -pub mod custom { - use super::*; - - pub fn visit_custom_instructions(executor: &mut V, _isi: &CustomInstruction) { - deny!( - executor, - "Custom instructions should be handled in custom executor" - ) - } -} diff --git a/crates/iroha_executor/src/lib.rs b/crates/iroha_executor/src/lib.rs index 5108038d8b5..4ba894084a4 100644 --- a/crates/iroha_executor/src/lib.rs +++ b/crates/iroha_executor/src/lib.rs @@ -286,8 +286,8 @@ pub mod prelude { pub use super::{ data_model::{ - executor::Result, isi::Instruction, - smart_contract::payloads::ExecutorContext as Context, visit::Visit, ValidationFail, + executor::Result, smart_contract::payloads::ExecutorContext as Context, visit::Visit, + ValidationFail, }, deny, execute, DataModelBuilder, Execute, }; diff --git a/crates/iroha_executor_data_model/Cargo.toml b/crates/iroha_executor_data_model/Cargo.toml index 4627792101d..6f71451ac1b 100644 --- a/crates/iroha_executor_data_model/Cargo.toml +++ b/crates/iroha_executor_data_model/Cargo.toml @@ -16,5 +16,7 @@ iroha_executor_data_model_derive = { path = "../iroha_executor_data_model_derive iroha_data_model.workspace = true iroha_schema.workspace = true +derive_more = { workspace = true, features = ["constructor", "from"] } +parity-scale-codec.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/crates/iroha_executor_data_model/src/custom.rs b/crates/iroha_executor_data_model/src/custom.rs new file mode 100644 index 00000000000..2524ce08e50 --- /dev/null +++ b/crates/iroha_executor_data_model/src/custom.rs @@ -0,0 +1,117 @@ +//! Types for custom instructions + +use alloc::{collections::btree_map::BTreeMap, format, string::String, vec::Vec}; + +use derive_more::{Constructor, From}; +use iroha_data_model::{ + isi::{CustomInstruction, Instruction, InstructionBox}, + prelude::{Json, *}, +}; +use iroha_schema::IntoSchema; +use parity_scale_codec::{Decode, Encode}; +use serde::{Deserialize, Serialize}; + +use super::*; + +// TODO #5221 #[proc_macro_derive(CustomInstruction)] +macro_rules! impl_custom_instruction { + ($box:ty, $($instruction:ty)|+) => { + impl Instruction for $box {} + + impl From<$box> for InstructionBox { + fn from(value: $box) -> Self { + Self::Custom(value.into()) + } + } + + impl From<$box> for CustomInstruction { + fn from(value: $box) -> Self { + let payload = serde_json::to_value(&value) + .expect(concat!("INTERNAL BUG: Couldn't serialize ", stringify!($box))); + + Self::new(payload) + } + } + + impl TryFrom<&Json> for $box { + type Error = serde_json::Error; + + fn try_from(payload: &Json) -> serde_json::Result { + serde_json::from_str::(payload.as_ref()) + } + } $( + + impl Instruction for $instruction {} + + impl From<$instruction> for InstructionBox { + fn from(value: $instruction) -> Self { + Self::Custom(<$box>::from(value).into()) + } + })+ + }; +} + +/// Types for multisig instructions +pub mod multisig { + use super::*; + + /// Multisig-related instructions + #[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, From)] + pub enum MultisigInstructionBox { + /// Register a multisig account, which is a prerequisite of multisig transactions + Register(MultisigRegister), + /// Propose a multisig transaction and initialize approvals with the proposer's one + Propose(MultisigPropose), + /// Approve a certain multisig transaction + Approve(MultisigApprove), + } + + /// Register a multisig account, which is a prerequisite of multisig transactions + #[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, Constructor)] + pub struct MultisigRegister { + /// Multisig account to be registered + ///
+ /// + /// Any corresponding private key allows the owner to manipulate this account as a ordinary personal account + /// + ///
+ // FIXME #5022 prevent multisig monopoly + // FIXME #5022 stop accepting user input: otherwise, after #4426 pre-registration account will be hijacked as a multisig account + pub account: AccountId, + /// List of signatories and their relative weights of responsibility for the multisig account + pub signatories: BTreeMap, + /// Threshold of total weight at which the multisig account is considered authenticated + pub quorum: u16, + /// Multisig transaction time-to-live in milliseconds based on block timestamps. Defaults to [`DEFAULT_MULTISIG_TTL_MS`] + pub transaction_ttl_ms: u64, + } + + type Weight = u8; + + /// Default multisig transaction time-to-live in milliseconds based on block timestamps + pub const DEFAULT_MULTISIG_TTL_MS: u64 = 60 * 60 * 1_000; // 1 hour + + /// Propose a multisig transaction and initialize approvals with the proposer's one + #[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, Constructor)] + pub struct MultisigPropose { + /// Multisig account to propose + pub account: AccountId, + /// Proposal contents + pub instructions: Vec, + } + + /// Approve a certain multisig transaction + #[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, Constructor)] + pub struct MultisigApprove { + /// Multisig account to approve + pub account: AccountId, + /// Proposal to approve + pub instructions_hash: HashOf>, + } + + // TODO #5221 #[derive(CustomInstruction)] + impl_custom_instruction!( + MultisigInstructionBox, + MultisigRegister | MultisigPropose | MultisigApprove + ); +} diff --git a/crates/iroha_executor_data_model/src/lib.rs b/crates/iroha_executor_data_model/src/lib.rs index 7695f7e384b..b30cdfa67c3 100644 --- a/crates/iroha_executor_data_model/src/lib.rs +++ b/crates/iroha_executor_data_model/src/lib.rs @@ -4,6 +4,7 @@ extern crate alloc; extern crate self as iroha_executor_data_model; +pub mod custom; pub mod parameter; pub mod permission; diff --git a/crates/iroha_multisig_data_model/Cargo.toml b/crates/iroha_multisig_data_model/Cargo.toml deleted file mode 100644 index 2c9700de387..00000000000 --- a/crates/iroha_multisig_data_model/Cargo.toml +++ /dev/null @@ -1,20 +0,0 @@ -[package] -name = "iroha_multisig_data_model" - -edition.workspace = true -version.workspace = true -authors.workspace = true - -license.workspace = true - -[lints] -workspace = true - -[dependencies] -iroha_data_model.workspace = true -iroha_schema.workspace = true - -derive_more = { workspace = true, features = ["constructor", "from"] } -parity-scale-codec = { workspace = true, features = ["derive"] } -serde.workspace = true -serde_json.workspace = true diff --git a/crates/iroha_multisig_data_model/src/lib.rs b/crates/iroha_multisig_data_model/src/lib.rs deleted file mode 100644 index a0b18db74b1..00000000000 --- a/crates/iroha_multisig_data_model/src/lib.rs +++ /dev/null @@ -1,112 +0,0 @@ -//! Types for multisig operations - -#![no_std] - -extern crate alloc; - -use alloc::{collections::btree_map::BTreeMap, format, string::String, vec::Vec}; - -use derive_more::{Constructor, From}; -use iroha_data_model::{ - isi::{CustomInstruction, Instruction, InstructionBox}, - prelude::{Json, *}, -}; -use iroha_schema::IntoSchema; -use parity_scale_codec::{Decode, Encode}; -use serde::{Deserialize, Serialize}; - -/// Multisig-related instructions -#[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, From)] -pub enum MultisigInstructionBox { - /// Register a multisig account, which is a prerequisite of multisig transactions - Register(MultisigRegister), - /// Propose a multisig transaction and initialize approvals with the proposer's one - Propose(MultisigPropose), - /// Approve a certain multisig transaction - Approve(MultisigApprove), -} - -/// Register a multisig account, which is a prerequisite of multisig transactions -#[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, Constructor)] -pub struct MultisigRegister { - /// Multisig account to be registered - ///
- /// - /// Any corresponding private key allows the owner to manipulate this account as a ordinary personal account - /// - ///
- // FIXME #5022 prevent multisig monopoly - // FIXME #5022 stop accepting user input: otherwise, after #4426 pre-registration account will be hijacked as a multisig account - pub account: AccountId, - /// List of signatories and their relative weights of responsibility for the multisig account - pub signatories: BTreeMap, - /// Threshold of total weight at which the multisig account is considered authenticated - pub quorum: u16, - /// Multisig transaction time-to-live in milliseconds based on block timestamps. Defaults to [`DEFAULT_MULTISIG_TTL_MS`] - pub transaction_ttl_ms: u64, -} - -type Weight = u8; - -/// Default multisig transaction time-to-live in milliseconds based on block timestamps -pub const DEFAULT_MULTISIG_TTL_MS: u64 = 60 * 60 * 1_000; // 1 hour - -/// Propose a multisig transaction and initialize approvals with the proposer's one -#[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, Constructor)] -pub struct MultisigPropose { - /// Multisig account to propose - pub account: AccountId, - /// Proposal contents - pub instructions: Vec, -} - -/// Approve a certain multisig transaction -#[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, Constructor)] -pub struct MultisigApprove { - /// Multisig account to approve - pub account: AccountId, - /// Proposal to approve - pub instructions_hash: HashOf>, -} - -macro_rules! impl_custom_instruction { - ($box:ty, $($instruction:ty)|+) => { - impl Instruction for $box {} - - impl From<$box> for InstructionBox { - fn from(value: $box) -> Self { - Self::Custom(value.into()) - } - } - - impl From<$box> for CustomInstruction { - fn from(value: $box) -> Self { - let payload = serde_json::to_value(&value) - .expect(concat!("INTERNAL BUG: Couldn't serialize ", stringify!($box))); - - Self::new(payload) - } - } - - impl TryFrom<&Json> for $box { - type Error = serde_json::Error; - - fn try_from(payload: &Json) -> serde_json::Result { - serde_json::from_str::(payload.as_ref()) - } - } $( - - impl Instruction for $instruction {} - - impl From<$instruction> for InstructionBox { - fn from(value: $instruction) -> Self { - Self::Custom(<$box>::from(value).into()) - } - })+ - }; -} - -impl_custom_instruction!( - MultisigInstructionBox, - MultisigRegister | MultisigPropose | MultisigApprove -); diff --git a/crates/iroha_schema_gen/Cargo.toml b/crates/iroha_schema_gen/Cargo.toml index 47822214c03..9fa7345a374 100644 --- a/crates/iroha_schema_gen/Cargo.toml +++ b/crates/iroha_schema_gen/Cargo.toml @@ -14,7 +14,6 @@ workspace = true # TODO: `transparent_api` feature shouldn't be activated/required here iroha_data_model = { workspace = true, features = ["http", "transparent_api"] } iroha_executor_data_model = { workspace = true } -iroha_multisig_data_model = { workspace = true } iroha_primitives = { workspace = true } iroha_genesis = { workspace = true } diff --git a/crates/iroha_schema_gen/src/lib.rs b/crates/iroha_schema_gen/src/lib.rs index e81abfad024..966e5637e21 100644 --- a/crates/iroha_schema_gen/src/lib.rs +++ b/crates/iroha_schema_gen/src/lib.rs @@ -34,8 +34,7 @@ macro_rules! types { /// shall be included recursively. pub fn build_schemas() -> MetaMap { use iroha_data_model::prelude::*; - use iroha_executor_data_model::permission; - use iroha_multisig_data_model as multisig; + use iroha_executor_data_model::{custom::multisig, permission}; macro_rules! schemas { ($($t:ty),* $(,)?) => {{ @@ -549,10 +548,10 @@ pub mod complete_data_model { }, Level, }; - pub use iroha_genesis::{GenesisWasmAction, GenesisWasmTrigger, WasmPath}; - pub use iroha_multisig_data_model::{ + pub use iroha_executor_data_model::custom::multisig::{ MultisigApprove, MultisigInstructionBox, MultisigPropose, MultisigRegister, }; + pub use iroha_genesis::{GenesisWasmAction, GenesisWasmTrigger, WasmPath}; pub use iroha_primitives::{const_vec::ConstVec, conststr::ConstString, json::Json}; pub use iroha_schema::Compact; } diff --git a/wasm/Cargo.toml b/wasm/Cargo.toml index 8a4be9978dc..672cf0351c4 100644 --- a/wasm/Cargo.toml +++ b/wasm/Cargo.toml @@ -32,7 +32,6 @@ iroha_executor = { version = "=2.0.0-rc.1.0", path = "../crates/iroha_executor", iroha_schema = { version = "=2.0.0-rc.1.0", path = "../crates/iroha_schema" } iroha_data_model = { version = "=2.0.0-rc.1.0", path = "../crates/iroha_data_model", default-features = false } iroha_executor_data_model = { version = "=2.0.0-rc.1.0", path = "../crates/iroha_executor_data_model" } -iroha_multisig_data_model = { version = "=2.0.0-rc.1.0", path = "../crates/iroha_multisig_data_model" } parity-scale-codec = { version = "3.2.1", default-features = false } anyhow = { version = "1.0.71", default-features = false } diff --git a/wasm/libs/default_executor/Cargo.toml b/wasm/libs/default_executor/Cargo.toml index e38d4c9f4d4..3efbe8de352 100644 --- a/wasm/libs/default_executor/Cargo.toml +++ b/wasm/libs/default_executor/Cargo.toml @@ -11,7 +11,6 @@ crate-type = ['cdylib'] [dependencies] iroha_executor.workspace = true -iroha_multisig_data_model.workspace = true panic-halt.workspace = true dlmalloc.workspace = true diff --git a/wasm/libs/default_executor/src/lib.rs b/wasm/libs/default_executor/src/lib.rs index b357bff935b..7b1fb13d3e8 100644 --- a/wasm/libs/default_executor/src/lib.rs +++ b/wasm/libs/default_executor/src/lib.rs @@ -5,14 +5,9 @@ #[cfg(not(test))] extern crate panic_halt; -extern crate alloc; - use dlmalloc::GlobalDlmalloc; use iroha_executor::{ - data_model::block::BlockHeader, - debug::{dbg_panic, DebugExpectExt}, - prelude::*, - DataModelBuilder, + data_model::block::BlockHeader, debug::dbg_panic, prelude::*, DataModelBuilder, }; #[global_allocator] @@ -20,15 +15,12 @@ static ALLOC: GlobalDlmalloc = GlobalDlmalloc; getrandom::register_custom_getrandom!(iroha_executor::stub_getrandom); -mod multisig; - /// Executor that replaces some of [`Execute`]'s methods with sensible defaults /// /// # Warning /// /// The defaults are not guaranteed to be stable. #[derive(Debug, Clone, Visit, Execute, Entrypoints)] -#[visit(custom(visit_custom_instructions))] struct Executor { host: Iroha, context: Context, @@ -46,33 +38,6 @@ impl Executor { } } -fn visit_custom_instructions(executor: &mut Executor, isi: &CustomInstruction) { - if let Ok(isi) = multisig::MultisigInstructionBox::try_from(isi.payload()) { - return isi.visit_execute(executor); - }; - - deny!(executor, "unexpected custom instruction"); -} - -trait VisitExecute: Instruction { - fn visit_execute(self, executor: &mut Executor) { - self.visit(executor); - if executor.verdict().is_ok() { - if let Err(err) = self.execute(executor) { - executor.deny(err); - } - } - } - - fn visit(&self, _executor: &mut Executor) { - unimplemented!("should be overridden unless `Self::visit_execute` is overridden") - } - - fn execute(self, _executor: &mut Executor) -> Result<(), ValidationFail> { - unimplemented!("should be overridden unless `Self::visit_execute` is overridden") - } -} - /// Migrate previous executor to the current version. /// Called by Iroha once just before upgrading executor. /// @@ -85,10 +50,5 @@ trait VisitExecute: Instruction { #[entrypoint] fn migrate(host: Iroha, context: Context) { Executor::ensure_genesis(context.curr_block); - DataModelBuilder::with_default_permissions() - .add_instruction::() - .add_instruction::() - .add_instruction::() - .add_instruction::() - .build_and_set(&host); + DataModelBuilder::with_default_permissions().build_and_set(&host); } From bdd2a093260dd4bb830001ce79b0686718843ba6 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Wed, 6 Nov 2024 23:43:58 +0900 Subject: [PATCH 13/28] review: minor updates since `#pullrequestreview-2414675315` Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- .../src/default/custom/multisig/account.rs | 18 +++-------- .../src/default/custom/multisig/mod.rs | 1 - .../default/custom/multisig/transaction.rs | 30 +++++++++++-------- 3 files changed, 21 insertions(+), 28 deletions(-) diff --git a/crates/iroha_executor/src/default/custom/multisig/account.rs b/crates/iroha_executor/src/default/custom/multisig/account.rs index d9fd9518248..6a61fa89f58 100644 --- a/crates/iroha_executor/src/default/custom/multisig/account.rs +++ b/crates/iroha_executor/src/default/custom/multisig/account.rs @@ -17,28 +17,18 @@ impl VisitExecute for MultisigRegister { ) } - let Ok(_domain_found) = host - .query(FindDomains) - .filter_with(|domain| domain.id.eq(target_domain.clone())) - .execute_single() - else { - deny!( - executor, - "domain must exist before registering multisig account" - ); - }; - for signatory in self.signatories.keys().cloned() { - let Ok(_signatory_found) = host + if host .query(FindAccounts) .filter_with(|account| account.id.eq(signatory)) .execute_single() - else { + .is_err() + { deny!( executor, "signatories must exist before registering multisig account" ); - }; + } } } diff --git a/crates/iroha_executor/src/default/custom/multisig/mod.rs b/crates/iroha_executor/src/default/custom/multisig/mod.rs index 4dd23d77ce4..2bf1012b6d4 100644 --- a/crates/iroha_executor/src/default/custom/multisig/mod.rs +++ b/crates/iroha_executor/src/default/custom/multisig/mod.rs @@ -1,7 +1,6 @@ use iroha_executor_data_model::custom::multisig::*; use super::*; - // SATO remove after merge use crate::smart_contract::debug::{DebugExpectExt as _, DebugUnwrapExt}; diff --git a/crates/iroha_executor/src/default/custom/multisig/transaction.rs b/crates/iroha_executor/src/default/custom/multisig/transaction.rs index e3c30310dd3..b69461c8e2a 100644 --- a/crates/iroha_executor/src/default/custom/multisig/transaction.rs +++ b/crates/iroha_executor/src/default/custom/multisig/transaction.rs @@ -12,18 +12,22 @@ impl VisitExecute for MultisigPropose { let multisig_role = multisig_role_for(&target_account); let instructions_hash = HashOf::new(&self.instructions); - let Ok(_role_found) = host + if host .query(FindRolesByAccountId::new(proposer)) .filter_with(|role_id| role_id.eq(multisig_role)) .execute_single() - else { + .is_err() + { deny!(executor, "not qualified to propose multisig"); }; - let Err(_proposal_not_found) = host.query_single(FindAccountMetadata::new( - target_account.clone(), - approvals_key(&instructions_hash), - )) else { + if host + .query_single(FindAccountMetadata::new( + target_account.clone(), + approvals_key(&instructions_hash), + )) + .is_ok() + { deny!(executor, "multisig proposal duplicates") }; } @@ -54,7 +58,7 @@ impl VisitExecute for MultisigPropose { if is_multisig_again { let propose_to_approve_me = { let approve_me = - MultisigApprove::new(target_account.clone(), instructions_hash.clone()); + MultisigApprove::new(target_account.clone(), instructions_hash); MultisigPropose::new(signatory, [approve_me.into()].to_vec()) }; @@ -88,7 +92,7 @@ impl VisitExecute for MultisigPropose { host.submit(&SetKeyValue::account( target_account.clone(), proposed_at_ms_key(&instructions_hash).clone(), - Json::new(&now_ms), + Json::new(now_ms), )) .dbg_unwrap(); @@ -201,8 +205,8 @@ impl VisitExecute for MultisigApprove { let is_authenticated = quorum <= signatories .into_iter() - .filter(|(id, _)| approvals.contains(&id)) - .map(|(_, weight)| weight as u16) + .filter(|(id, _)| approvals.contains(id)) + .map(|(_, weight)| u16::from(weight)) .sum(); let is_expired = proposed_at_ms.saturating_add(transaction_ttl_ms) < now_ms; @@ -227,7 +231,9 @@ impl VisitExecute for MultisigApprove { )) .dbg_unwrap(); - if !is_expired { + if is_expired { + // TODO Notify that the proposal has expired, while returning Ok for the entry deletion to take effect + } else { // Validate and execute the authenticated multisig transaction for instruction in instructions { let init_authority = executor.context().authority.clone(); @@ -236,8 +242,6 @@ impl VisitExecute for MultisigApprove { executor.visit_instruction(&instruction); executor.context_mut().authority = init_authority; } - } else { - // TODO Notify that the proposal has expired, while returning Ok for the entry deletion to take effect } } From 255782ee5968cb395abedab8f7c08b6cec9b384c Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Thu, 7 Nov 2024 00:19:54 +0900 Subject: [PATCH 14/28] review: submit a new account with metadata filled Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- .../src/default/custom/multisig/account.rs | 42 ++++++++----------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/crates/iroha_executor/src/default/custom/multisig/account.rs b/crates/iroha_executor/src/default/custom/multisig/account.rs index 6a61fa89f58..9b985fd52d2 100644 --- a/crates/iroha_executor/src/default/custom/multisig/account.rs +++ b/crates/iroha_executor/src/default/custom/multisig/account.rs @@ -35,33 +35,27 @@ impl VisitExecute for MultisigRegister { fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { let host = executor.host(); let registrant = executor.context().authority.clone(); - let multisig_account = self.account; - let multisig_role = multisig_role_for(&multisig_account); + let multisig_role = multisig_role_for(&self.account); + let multisig_account = { + let metadata = [ + (SIGNATORIES.parse().unwrap(), Json::new(&self.signatories)), + (QUORUM.parse().unwrap(), Json::new(self.quorum)), + ( + TRANSACTION_TTL_MS.parse().unwrap(), + Json::new(self.transaction_ttl_ms), + ), + ] + .into_iter() + .fold(Metadata::default(), |mut acc, (k, v)| { + acc.insert(k, v).unwrap(); + acc + }); + Account::new(self.account.clone()).with_metadata(metadata) + }; - host.submit(&Register::account(Account::new(multisig_account.clone()))) + host.submit(&Register::account(multisig_account)) .dbg_expect("registrant should successfully register a multisig account"); - host.submit(&SetKeyValue::account( - multisig_account.clone(), - SIGNATORIES.parse().unwrap(), - Json::new(&self.signatories), - )) - .dbg_unwrap(); - - host.submit(&SetKeyValue::account( - multisig_account.clone(), - QUORUM.parse().unwrap(), - Json::new(&self.quorum), - )) - .dbg_unwrap(); - - host.submit(&SetKeyValue::account( - multisig_account.clone(), - TRANSACTION_TTL_MS.parse().unwrap(), - Json::new(&self.transaction_ttl_ms), - )) - .dbg_unwrap(); - host.submit(&Register::role( // Temporarily grant a multisig role to the registrant to delegate the role to the signatories Role::new(multisig_role.clone(), registrant.clone()), From b0208cc0a7a37aaacdffbaeb4eca0954743c0f7f Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Thu, 7 Nov 2024 00:21:34 +0900 Subject: [PATCH 15/28] Revert "review: submit a new account with metadata filled" This reverts commit 255782ee5968cb395abedab8f7c08b6cec9b384c. Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- .../src/default/custom/multisig/account.rs | 42 +++++++++++-------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/crates/iroha_executor/src/default/custom/multisig/account.rs b/crates/iroha_executor/src/default/custom/multisig/account.rs index 9b985fd52d2..6a61fa89f58 100644 --- a/crates/iroha_executor/src/default/custom/multisig/account.rs +++ b/crates/iroha_executor/src/default/custom/multisig/account.rs @@ -35,27 +35,33 @@ impl VisitExecute for MultisigRegister { fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { let host = executor.host(); let registrant = executor.context().authority.clone(); - let multisig_role = multisig_role_for(&self.account); - let multisig_account = { - let metadata = [ - (SIGNATORIES.parse().unwrap(), Json::new(&self.signatories)), - (QUORUM.parse().unwrap(), Json::new(self.quorum)), - ( - TRANSACTION_TTL_MS.parse().unwrap(), - Json::new(self.transaction_ttl_ms), - ), - ] - .into_iter() - .fold(Metadata::default(), |mut acc, (k, v)| { - acc.insert(k, v).unwrap(); - acc - }); - Account::new(self.account.clone()).with_metadata(metadata) - }; + let multisig_account = self.account; + let multisig_role = multisig_role_for(&multisig_account); - host.submit(&Register::account(multisig_account)) + host.submit(&Register::account(Account::new(multisig_account.clone()))) .dbg_expect("registrant should successfully register a multisig account"); + host.submit(&SetKeyValue::account( + multisig_account.clone(), + SIGNATORIES.parse().unwrap(), + Json::new(&self.signatories), + )) + .dbg_unwrap(); + + host.submit(&SetKeyValue::account( + multisig_account.clone(), + QUORUM.parse().unwrap(), + Json::new(&self.quorum), + )) + .dbg_unwrap(); + + host.submit(&SetKeyValue::account( + multisig_account.clone(), + TRANSACTION_TTL_MS.parse().unwrap(), + Json::new(&self.transaction_ttl_ms), + )) + .dbg_unwrap(); + host.submit(&Register::role( // Temporarily grant a multisig role to the registrant to delegate the role to the signatories Role::new(multisig_role.clone(), registrant.clone()), From 45b24d93f5f46154a70902ef62c4ea40cffdacf7 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Thu, 7 Nov 2024 04:07:25 +0900 Subject: [PATCH 16/28] fix: prevent a double grant to the registrant who is also a signatory Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha_executor/src/default/custom/mod.rs | 6 +++--- .../src/default/custom/multisig/account.rs | 11 +++++------ .../src/default/custom/multisig/transaction.rs | 16 ++++++++++------ 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/crates/iroha_executor/src/default/custom/mod.rs b/crates/iroha_executor/src/default/custom/mod.rs index e92b3cecda6..fc08b2d7694 100644 --- a/crates/iroha_executor/src/default/custom/mod.rs +++ b/crates/iroha_executor/src/default/custom/mod.rs @@ -7,10 +7,10 @@ mod multisig; pub fn visit_custom_instructions( executor: &mut V, - isi: &CustomInstruction, + instruction: &CustomInstruction, ) { - if let Ok(isi) = MultisigInstructionBox::try_from(isi.payload()) { - return isi.visit_execute(executor); + if let Ok(instruction) = MultisigInstructionBox::try_from(instruction.payload()) { + return instruction.visit_execute(executor); }; deny!(executor, "unexpected custom instruction"); diff --git a/crates/iroha_executor/src/default/custom/multisig/account.rs b/crates/iroha_executor/src/default/custom/multisig/account.rs index ebaeae953cd..ef1c9db9116 100644 --- a/crates/iroha_executor/src/default/custom/multisig/account.rs +++ b/crates/iroha_executor/src/default/custom/multisig/account.rs @@ -34,7 +34,6 @@ impl VisitExecute for MultisigRegister { fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { let host = executor.host(); - let registrant = executor.context().authority.clone(); let multisig_account = self.account; let multisig_role = multisig_role_for(&multisig_account); @@ -63,8 +62,8 @@ impl VisitExecute for MultisigRegister { .dbg_unwrap(); host.submit(&Register::role( - // Temporarily grant a multisig role to the registrant to delegate the role to the signatories - Role::new(multisig_role.clone(), registrant.clone()), + // No use, but temporarily grant a multisig role to the multisig account due to specifications + Role::new(multisig_role.clone(), multisig_account.clone()), )) .dbg_expect("registrant should successfully register a multisig role"); @@ -75,10 +74,10 @@ impl VisitExecute for MultisigRegister { ); } - // FIXME No roles to revoke found, which should have been granted to the registrant - // host.submit(&Revoke::account_role(multisig_role, registrant)) + // FIXME No roles to revoke found, which should have been granted to the multisig account + // host.submit(&Revoke::account_role(multisig_role, multisig_account)) // .dbg_expect( - // "registrant should successfully revoke the multisig role from the registrant", + // "registrant should successfully revoke the multisig role from the multisig account", // ); Ok(()) diff --git a/crates/iroha_executor/src/default/custom/multisig/transaction.rs b/crates/iroha_executor/src/default/custom/multisig/transaction.rs index b69461c8e2a..caf2f63986a 100644 --- a/crates/iroha_executor/src/default/custom/multisig/transaction.rs +++ b/crates/iroha_executor/src/default/custom/multisig/transaction.rs @@ -115,18 +115,22 @@ impl VisitExecute for MultisigApprove { let multisig_role = multisig_role_for(&target_account); let instructions_hash = self.instructions_hash; - let Ok(_role_found) = host + if host .query(FindRolesByAccountId::new(approver)) .filter_with(|role_id| role_id.eq(multisig_role)) .execute_single() - else { + .is_err() + { deny!(executor, "not qualified to approve multisig"); }; - let Ok(_proposal_found) = host.query_single(FindAccountMetadata::new( - target_account.clone(), - approvals_key(&instructions_hash), - )) else { + if host + .query_single(FindAccountMetadata::new( + target_account.clone(), + approvals_key(&instructions_hash), + )) + .is_err() + { deny!(executor, "no proposals to approve") }; } From e0a96f48f5d45f36b994f54c22ae6a50bce1dbfa Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Thu, 7 Nov 2024 19:54:27 +0900 Subject: [PATCH 17/28] review: minor updates since `#pullrequestreview-2418103555` Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha/tests/multisig.rs | 2 +- crates/iroha_data_model/src/isi.rs | 1 - crates/iroha_data_model/src/visit.rs | 6 +++--- crates/iroha_executor/src/default/{custom => isi}/mod.rs | 5 ++--- .../src/default/{custom => isi}/multisig/account.rs | 0 .../src/default/{custom => isi}/multisig/mod.rs | 2 +- .../src/default/{custom => isi}/multisig/transaction.rs | 0 crates/iroha_executor/src/default/mod.rs | 6 +++--- crates/iroha_executor_data_model/src/{custom.rs => isi.rs} | 4 ++-- crates/iroha_executor_data_model/src/lib.rs | 2 +- crates/iroha_executor_derive/src/default.rs | 2 +- crates/iroha_schema_gen/src/lib.rs | 4 ++-- .../samples/executor_custom_instructions_complex/src/lib.rs | 4 ++-- wasm/samples/executor_custom_instructions_simple/src/lib.rs | 4 ++-- 14 files changed, 20 insertions(+), 22 deletions(-) rename crates/iroha_executor/src/default/{custom => isi}/mod.rs (84%) rename crates/iroha_executor/src/default/{custom => isi}/multisig/account.rs (100%) rename crates/iroha_executor/src/default/{custom => isi}/multisig/mod.rs (96%) rename crates/iroha_executor/src/default/{custom => isi}/multisig/transaction.rs (100%) rename crates/iroha_executor_data_model/src/{custom.rs => isi.rs} (97%) diff --git a/crates/iroha/tests/multisig.rs b/crates/iroha/tests/multisig.rs index 575e2af6666..c0d32462cfd 100644 --- a/crates/iroha/tests/multisig.rs +++ b/crates/iroha/tests/multisig.rs @@ -9,7 +9,7 @@ use iroha::{ client::Client, crypto::KeyPair, data_model::{prelude::*, Level}, - executor_data_model::custom::multisig::*, + executor_data_model::isi::multisig::*, }; use iroha_test_network::*; use iroha_test_samples::{ diff --git a/crates/iroha_data_model/src/isi.rs b/crates/iroha_data_model/src/isi.rs index 4c93855a3ac..75b3b341c32 100644 --- a/crates/iroha_data_model/src/isi.rs +++ b/crates/iroha_data_model/src/isi.rs @@ -979,7 +979,6 @@ mod transparent { /// integration tests #[derive(Display)] #[display(fmt = "CUSTOM({payload})")] - // TODO #5221 pub struct RawCustomInstruction { pub struct CustomInstruction { /// Custom payload pub payload: Json, diff --git a/crates/iroha_data_model/src/visit.rs b/crates/iroha_data_model/src/visit.rs index 164d1df80ef..dae06cd82cd 100644 --- a/crates/iroha_data_model/src/visit.rs +++ b/crates/iroha_data_model/src/visit.rs @@ -49,7 +49,7 @@ pub trait Visit { visit_execute_trigger(&ExecuteTrigger), visit_set_parameter(&SetParameter), visit_log(&Log), - visit_custom_instructions(&CustomInstruction), + visit_custom_instruction(&CustomInstruction), // Visit SingularQueryBox visit_find_asset_quantity_by_id(&FindAssetQuantityById), @@ -230,7 +230,7 @@ pub fn visit_instruction(visitor: &mut V, isi: &InstructionBo InstructionBox::Transfer(variant_value) => visitor.visit_transfer(variant_value), InstructionBox::Unregister(variant_value) => visitor.visit_unregister(variant_value), InstructionBox::Upgrade(variant_value) => visitor.visit_upgrade(variant_value), - InstructionBox::Custom(custom) => visitor.visit_custom_instructions(custom), + InstructionBox::Custom(custom) => visitor.visit_custom_instruction(custom), } } @@ -373,7 +373,7 @@ leaf_visitors! { visit_set_parameter(&SetParameter), visit_execute_trigger(&ExecuteTrigger), visit_log(&Log), - visit_custom_instructions(&CustomInstruction), + visit_custom_instruction(&CustomInstruction), // Singular Quert visitors visit_find_asset_quantity_by_id(&FindAssetQuantityById), diff --git a/crates/iroha_executor/src/default/custom/mod.rs b/crates/iroha_executor/src/default/isi/mod.rs similarity index 84% rename from crates/iroha_executor/src/default/custom/mod.rs rename to crates/iroha_executor/src/default/isi/mod.rs index fc08b2d7694..2f7576dca1f 100644 --- a/crates/iroha_executor/src/default/custom/mod.rs +++ b/crates/iroha_executor/src/default/isi/mod.rs @@ -1,11 +1,11 @@ -use iroha_executor_data_model::custom::multisig::MultisigInstructionBox; +use iroha_executor_data_model::isi::multisig::MultisigInstructionBox; use super::*; use crate::prelude::{Execute, Vec, Visit}; mod multisig; -pub fn visit_custom_instructions( +pub fn visit_custom_instruction( executor: &mut V, instruction: &CustomInstruction, ) { @@ -16,7 +16,6 @@ pub fn visit_custom_instructions( deny!(executor, "unexpected custom instruction"); } -// TODO #5221 trait VisitExecute: CustomInstruction { trait VisitExecute: crate::data_model::isi::Instruction { fn visit_execute(self, executor: &mut V) { self.visit(executor); diff --git a/crates/iroha_executor/src/default/custom/multisig/account.rs b/crates/iroha_executor/src/default/isi/multisig/account.rs similarity index 100% rename from crates/iroha_executor/src/default/custom/multisig/account.rs rename to crates/iroha_executor/src/default/isi/multisig/account.rs diff --git a/crates/iroha_executor/src/default/custom/multisig/mod.rs b/crates/iroha_executor/src/default/isi/multisig/mod.rs similarity index 96% rename from crates/iroha_executor/src/default/custom/multisig/mod.rs rename to crates/iroha_executor/src/default/isi/multisig/mod.rs index 1d0e7563ac9..6ed09717677 100644 --- a/crates/iroha_executor/src/default/custom/multisig/mod.rs +++ b/crates/iroha_executor/src/default/isi/multisig/mod.rs @@ -1,4 +1,4 @@ -use iroha_executor_data_model::custom::multisig::*; +use iroha_executor_data_model::isi::multisig::*; use super::*; use crate::smart_contract::{DebugExpectExt as _, DebugUnwrapExt}; diff --git a/crates/iroha_executor/src/default/custom/multisig/transaction.rs b/crates/iroha_executor/src/default/isi/multisig/transaction.rs similarity index 100% rename from crates/iroha_executor/src/default/custom/multisig/transaction.rs rename to crates/iroha_executor/src/default/isi/multisig/transaction.rs diff --git a/crates/iroha_executor/src/default/mod.rs b/crates/iroha_executor/src/default/mod.rs index bcd5fb8aae2..277942eac3e 100644 --- a/crates/iroha_executor/src/default/mod.rs +++ b/crates/iroha_executor/src/default/mod.rs @@ -17,13 +17,13 @@ pub use asset_definition::{ visit_set_asset_definition_key_value, visit_transfer_asset_definition, visit_unregister_asset_definition, }; -pub use custom::visit_custom_instructions; pub use domain::{ visit_register_domain, visit_remove_domain_key_value, visit_set_domain_key_value, visit_transfer_domain, visit_unregister_domain, }; pub use executor::visit_upgrade; use iroha_smart_contract::data_model::{prelude::*, visit::Visit}; +pub use isi::visit_custom_instruction; pub use log::visit_log; pub use parameter::visit_set_parameter; pub use peer::{visit_register_peer, visit_unregister_peer}; @@ -44,7 +44,7 @@ use crate::{ Execute, }; -pub mod custom; +pub mod isi; // NOTE: If any new `visit_..` functions are introduced in this module, one should // not forget to update the default executor boilerplate too, specifically the @@ -119,7 +119,7 @@ pub fn visit_instruction(executor: &mut V, isi: &In executor.visit_upgrade(isi); } InstructionBox::Custom(isi) => { - executor.visit_custom_instructions(isi); + executor.visit_custom_instruction(isi); } } } diff --git a/crates/iroha_executor_data_model/src/custom.rs b/crates/iroha_executor_data_model/src/isi.rs similarity index 97% rename from crates/iroha_executor_data_model/src/custom.rs rename to crates/iroha_executor_data_model/src/isi.rs index 2524ce08e50..d21992fa119 100644 --- a/crates/iroha_executor_data_model/src/custom.rs +++ b/crates/iroha_executor_data_model/src/isi.rs @@ -13,7 +13,6 @@ use serde::{Deserialize, Serialize}; use super::*; -// TODO #5221 #[proc_macro_derive(CustomInstruction)] macro_rules! impl_custom_instruction { ($box:ty, $($instruction:ty)|+) => { impl Instruction for $box {} @@ -86,6 +85,8 @@ pub mod multisig { pub transaction_ttl_ms: u64, } + /// Relative weight of responsibility for the multisig account. + /// 0 is allowed for observers who don't join governance type Weight = u8; /// Default multisig transaction time-to-live in milliseconds based on block timestamps @@ -109,7 +110,6 @@ pub mod multisig { pub instructions_hash: HashOf>, } - // TODO #5221 #[derive(CustomInstruction)] impl_custom_instruction!( MultisigInstructionBox, MultisigRegister | MultisigPropose | MultisigApprove diff --git a/crates/iroha_executor_data_model/src/lib.rs b/crates/iroha_executor_data_model/src/lib.rs index b30cdfa67c3..125a05dcf4e 100644 --- a/crates/iroha_executor_data_model/src/lib.rs +++ b/crates/iroha_executor_data_model/src/lib.rs @@ -4,7 +4,7 @@ extern crate alloc; extern crate self as iroha_executor_data_model; -pub mod custom; +pub mod isi; pub mod parameter; pub mod permission; diff --git a/crates/iroha_executor_derive/src/default.rs b/crates/iroha_executor_derive/src/default.rs index aa2832aaac9..03c52f33998 100644 --- a/crates/iroha_executor_derive/src/default.rs +++ b/crates/iroha_executor_derive/src/default.rs @@ -155,7 +155,7 @@ pub fn impl_derive_visit(emitter: &mut Emitter, input: &syn::DeriveInput) -> Tok "fn visit_set_parameter(operation: &SetParameter)", "fn visit_upgrade(operation: &Upgrade)", "fn visit_log(operation: &Log)", - "fn visit_custom_instructions(operation: &CustomInstruction)", + "fn visit_custom_instruction(operation: &CustomInstruction)", ] .into_iter() .map(|item| { diff --git a/crates/iroha_schema_gen/src/lib.rs b/crates/iroha_schema_gen/src/lib.rs index 966e5637e21..beceac65797 100644 --- a/crates/iroha_schema_gen/src/lib.rs +++ b/crates/iroha_schema_gen/src/lib.rs @@ -34,7 +34,7 @@ macro_rules! types { /// shall be included recursively. pub fn build_schemas() -> MetaMap { use iroha_data_model::prelude::*; - use iroha_executor_data_model::{custom::multisig, permission}; + use iroha_executor_data_model::{isi::multisig, permission}; macro_rules! schemas { ($($t:ty),* $(,)?) => {{ @@ -548,7 +548,7 @@ pub mod complete_data_model { }, Level, }; - pub use iroha_executor_data_model::custom::multisig::{ + pub use iroha_executor_data_model::isi::multisig::{ MultisigApprove, MultisigInstructionBox, MultisigPropose, MultisigRegister, }; pub use iroha_genesis::{GenesisWasmAction, GenesisWasmTrigger, WasmPath}; diff --git a/wasm/samples/executor_custom_instructions_complex/src/lib.rs b/wasm/samples/executor_custom_instructions_complex/src/lib.rs index 16f9299dba5..3d6e3f425e1 100644 --- a/wasm/samples/executor_custom_instructions_complex/src/lib.rs +++ b/wasm/samples/executor_custom_instructions_complex/src/lib.rs @@ -23,14 +23,14 @@ use iroha_executor::{ static ALLOC: GlobalDlmalloc = GlobalDlmalloc; #[derive(Visit, Execute, Entrypoints)] -#[visit(custom(visit_custom_instructions))] +#[visit(custom(visit_custom_instruction))] struct Executor { host: Iroha, context: iroha_executor::prelude::Context, verdict: Result, } -fn visit_custom_instructions(executor: &mut Executor, isi: &CustomInstruction) { +fn visit_custom_instruction(executor: &mut Executor, isi: &CustomInstruction) { let Ok(isi) = CustomInstructionExpr::try_from(isi.payload()) else { deny!(executor, "Failed to parse custom instruction"); }; diff --git a/wasm/samples/executor_custom_instructions_simple/src/lib.rs b/wasm/samples/executor_custom_instructions_simple/src/lib.rs index 4db2ba7b036..1f9203eb862 100644 --- a/wasm/samples/executor_custom_instructions_simple/src/lib.rs +++ b/wasm/samples/executor_custom_instructions_simple/src/lib.rs @@ -16,14 +16,14 @@ use iroha_executor::{data_model::isi::CustomInstruction, prelude::*}; static ALLOC: GlobalDlmalloc = GlobalDlmalloc; #[derive(Visit, Execute, Entrypoints)] -#[visit(custom(visit_custom_instructions))] +#[visit(custom(visit_custom_instruction))] struct Executor { host: Iroha, context: Context, verdict: Result, } -fn visit_custom_instructions(executor: &mut Executor, isi: &CustomInstruction) { +fn visit_custom_instruction(executor: &mut Executor, isi: &CustomInstruction) { let Ok(isi) = CustomInstructionBox::try_from(isi.payload()) else { deny!(executor, "Failed to parse custom instruction"); }; From 183ff26f113cdfc7ee61a798848466b5097e0d25 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Thu, 7 Nov 2024 19:59:37 +0900 Subject: [PATCH 18/28] review: nonzero `quorum` and `transaction_ttl_ms` Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha/tests/multisig.rs | 21 +++++++++++++++++---- crates/iroha_cli/src/main.rs | 13 +++++++++---- crates/iroha_executor_data_model/src/isi.rs | 6 ++++-- crates/iroha_schema/src/lib.rs | 4 ++-- crates/iroha_schema_gen/src/lib.rs | 3 ++- docs/source/references/schema.json | 5 +++-- 6 files changed, 37 insertions(+), 15 deletions(-) diff --git a/crates/iroha/tests/multisig.rs b/crates/iroha/tests/multisig.rs index c0d32462cfd..a668efeddb7 100644 --- a/crates/iroha/tests/multisig.rs +++ b/crates/iroha/tests/multisig.rs @@ -1,5 +1,6 @@ use std::{ collections::{BTreeMap, BTreeSet}, + num::{NonZeroU16, NonZeroU64}, time::Duration, }; @@ -122,8 +123,16 @@ fn multisig_base(suite: TestSuite) -> Result<()> { .map(|(weight, id)| (id.clone(), 1 + weight as u8)) .collect(), // Quorum can be reached without the first signatory - (1..=N_SIGNATORIES).skip(1).sum::() as u16, - transaction_ttl_ms_opt.unwrap_or(u64::MAX), + (1..=N_SIGNATORIES) + .skip(1) + .sum::() + .try_into() + .ok() + .and_then(NonZeroU16::new) + .unwrap(), + transaction_ttl_ms_opt + .and_then(NonZeroU64::new) + .unwrap_or(NonZeroU64::MAX), ); // Any account in another domain cannot register a multisig account without special permission @@ -273,8 +282,12 @@ fn multisig_recursion() -> Result<()> { let register_ms_account = MultisigRegister::new( ms_account_id.clone(), sigs.iter().copied().map(|id| (id.clone(), 1)).collect(), - sigs.len().try_into().unwrap(), - u64::MAX, + sigs.len() + .try_into() + .ok() + .and_then(NonZeroU16::new) + .unwrap(), + NonZeroU64::new(u64::MAX).unwrap(), ); test_client diff --git a/crates/iroha_cli/src/main.rs b/crates/iroha_cli/src/main.rs index 3301d7831c1..c0fb1e9761e 100644 --- a/crates/iroha_cli/src/main.rs +++ b/crates/iroha_cli/src/main.rs @@ -1177,9 +1177,12 @@ mod json { } mod multisig { - use std::io::{BufReader, Read as _}; + use std::{ + io::{BufReader, Read as _}, + num::{NonZeroU16, NonZeroU64}, + }; - use iroha::executor_data_model::custom::multisig::*; + use iroha::executor_data_model::isi::multisig::*; use super::*; @@ -1234,11 +1237,13 @@ mod multisig { let register_multisig_account = MultisigRegister::new( self.account, self.signatories.into_iter().zip(self.weights).collect(), - self.quorum, + NonZeroU16::new(self.quorum).expect("quorum should not be 0"), self.transaction_ttl .as_millis() .try_into() - .expect("ttl must be within 584942417 years"), + .ok() + .and_then(NonZeroU64::new) + .expect("ttl should be between 1 ms and 584942417 years"), ); submit([register_multisig_account], Metadata::default(), context) diff --git a/crates/iroha_executor_data_model/src/isi.rs b/crates/iroha_executor_data_model/src/isi.rs index d21992fa119..8bbf9c196c3 100644 --- a/crates/iroha_executor_data_model/src/isi.rs +++ b/crates/iroha_executor_data_model/src/isi.rs @@ -52,6 +52,8 @@ macro_rules! impl_custom_instruction { /// Types for multisig instructions pub mod multisig { + use core::num::{NonZeroU16, NonZeroU64}; + use super::*; /// Multisig-related instructions @@ -80,9 +82,9 @@ pub mod multisig { /// List of signatories and their relative weights of responsibility for the multisig account pub signatories: BTreeMap, /// Threshold of total weight at which the multisig account is considered authenticated - pub quorum: u16, + pub quorum: NonZeroU16, /// Multisig transaction time-to-live in milliseconds based on block timestamps. Defaults to [`DEFAULT_MULTISIG_TTL_MS`] - pub transaction_ttl_ms: u64, + pub transaction_ttl_ms: NonZeroU64, } /// Relative weight of responsibility for the multisig account. diff --git a/crates/iroha_schema/src/lib.rs b/crates/iroha_schema/src/lib.rs index 7318842a8d3..45c9e78c5e6 100644 --- a/crates/iroha_schema/src/lib.rs +++ b/crates/iroha_schema/src/lib.rs @@ -16,7 +16,7 @@ use alloc::{ vec::Vec, }; use core::{ - num::{NonZeroU32, NonZeroU64}, + num::{NonZeroU16, NonZeroU32, NonZeroU64}, ops::RangeInclusive, }; @@ -342,7 +342,7 @@ macro_rules! impl_schema_non_zero_int { )*}; } -impl_schema_non_zero_int!(NonZeroU64 => u64, NonZeroU32 => u32); +impl_schema_non_zero_int!(NonZeroU64 => u64, NonZeroU32 => u32, NonZeroU16 => u16); impl TypeId for String { fn id() -> String { diff --git a/crates/iroha_schema_gen/src/lib.rs b/crates/iroha_schema_gen/src/lib.rs index beceac65797..7e7f5dc6770 100644 --- a/crates/iroha_schema_gen/src/lib.rs +++ b/crates/iroha_schema_gen/src/lib.rs @@ -292,6 +292,7 @@ types!( NewAssetDefinition, NewDomain, NewRole, + NonZeroU16, NonZeroU32, NonZeroU64, Numeric, @@ -505,7 +506,7 @@ types!( pub mod complete_data_model { //! Complete set of types participating in the schema - pub use core::num::{NonZeroU32, NonZeroU64}; + pub use core::num::{NonZeroU16, NonZeroU32, NonZeroU64}; pub use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; pub use iroha_crypto::*; diff --git a/docs/source/references/schema.json b/docs/source/references/schema.json index 4420ce213d1..dc3c8ea26b3 100644 --- a/docs/source/references/schema.json +++ b/docs/source/references/schema.json @@ -2654,11 +2654,11 @@ }, { "name": "quorum", - "type": "u16" + "type": "NonZero" }, { "name": "transaction_ttl_ms", - "type": "u64" + "type": "NonZero" } ] }, @@ -2727,6 +2727,7 @@ } ] }, + "NonZero": "u16", "NonZero": "u32", "NonZero": "u64", "Numeric": { From 70d51d0cbe091df7ccb7a74224e1fb815b8caefc Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Fri, 8 Nov 2024 05:36:51 +0900 Subject: [PATCH 19/28] review: replace `host.submit` with `executor.visit_*` Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha/tests/multisig.rs | 33 ++++- crates/iroha_executor/src/default/isi/mod.rs | 15 +- .../src/default/isi/multisig/account.rs | 87 ++++++----- .../src/default/isi/multisig/transaction.rs | 140 +++++++++--------- crates/iroha_executor/src/default/mod.rs | 50 +++++-- crates/iroha_executor/src/lib.rs | 2 +- scripts/tests/multisig.sh | 4 +- 7 files changed, 209 insertions(+), 122 deletions(-) diff --git a/crates/iroha/tests/multisig.rs b/crates/iroha/tests/multisig.rs index a668efeddb7..1ac73daca89 100644 --- a/crates/iroha/tests/multisig.rs +++ b/crates/iroha/tests/multisig.rs @@ -12,6 +12,7 @@ use iroha::{ data_model::{prelude::*, Level}, executor_data_model::isi::multisig::*, }; +use iroha_executor_data_model::permission::account::CanRegisterAccount; use iroha_test_network::*; use iroha_test_samples::{ gen_account_in, ALICE_ID, BOB_ID, BOB_KEYPAIR, CARPENTER_ID, CARPENTER_KEYPAIR, @@ -112,7 +113,7 @@ fn multisig_base(suite: TestSuite) -> Result<()> { .map(Register::account), )?; - let not_signatory = residents.pop_first().unwrap(); + let non_signatory = residents.pop_first().unwrap(); let mut signatories = residents; let register_multisig_account = MultisigRegister::new( @@ -143,10 +144,28 @@ fn multisig_base(suite: TestSuite) -> Result<()> { .submit_blocking(register_multisig_account.clone()) .expect_err("multisig account should not be registered by account of another domain"); - // Any account in the same domain can register a multisig account without special permission - alt_client(not_signatory, &test_client) + // Non-signatory account in the same domain cannot register a multisig account without special permission + let _err = alt_client(non_signatory.clone(), &test_client) + .submit_blocking(register_multisig_account.clone()) + .expect_err( + "multisig account should not be registered by non-signatory account of the same domain", + ); + + // All but the first signatory approve the proposal + let signatory = signatories.pop_first().unwrap(); + + // Signatory account cannot register a multisig account without special permission + let _err = alt_client(signatory, &test_client) + .submit_blocking(register_multisig_account.clone()) + .expect_err("multisig account should not be registered by signatory account"); + + // Account with permission can register a multisig account + alt_client((BOB_ID.clone(), BOB_KEYPAIR.clone()), &test_client).submit_blocking( + Grant::account_permission(CanRegisterAccount { domain }, non_signatory.0.clone()), + )?; + alt_client(non_signatory, &test_client) .submit_blocking(register_multisig_account) - .expect("multisig account should be registered by account of the same domain"); + .expect("multisig account should be registered by account with permission"); // Check that the multisig account has been registered test_client @@ -169,11 +188,9 @@ fn multisig_base(suite: TestSuite) -> Result<()> { let instructions_hash = HashOf::new(&instructions); let proposer = signatories.pop_last().unwrap(); - // All but the first signatory approve the proposal - let mut approvers = signatories.into_iter().skip(1); + let mut approvers = signatories.into_iter(); let propose = MultisigPropose::new(multisig_account_id.clone(), instructions); - alt_client(proposer, &test_client).submit_blocking(propose)?; // Allow time to elapse to test the expiration @@ -249,7 +266,7 @@ fn multisig_base(suite: TestSuite) -> Result<()> { /// 0 1 2 3 4 5 <--- personal signatories /// ``` #[test] -#[expect(clippy::similar_names)] +#[expect(clippy::similar_names, clippy::too_many_lines)] fn multisig_recursion() -> Result<()> { let (network, _rt) = NetworkBuilder::new().start_blocking()?; let test_client = network.client(); diff --git a/crates/iroha_executor/src/default/isi/mod.rs b/crates/iroha_executor/src/default/isi/mod.rs index 2f7576dca1f..1795a86975a 100644 --- a/crates/iroha_executor/src/default/isi/mod.rs +++ b/crates/iroha_executor/src/default/isi/mod.rs @@ -3,8 +3,6 @@ use iroha_executor_data_model::isi::multisig::MultisigInstructionBox; use super::*; use crate::prelude::{Execute, Vec, Visit}; -mod multisig; - pub fn visit_custom_instruction( executor: &mut V, instruction: &CustomInstruction, @@ -18,12 +16,14 @@ pub fn visit_custom_instruction( trait VisitExecute: crate::data_model::isi::Instruction { fn visit_execute(self, executor: &mut V) { + let init_authority = executor.context().authority.clone(); self.visit(executor); if executor.verdict().is_ok() { if let Err(err) = self.execute(executor) { executor.deny(err); } } + executor.context_mut().authority = init_authority; } fn visit(&self, _executor: &mut V) { @@ -34,3 +34,14 @@ trait VisitExecute: crate::data_model::isi::Instruction { unimplemented!("should be overridden unless `Self::visit_execute` is overridden") } } + +macro_rules! visit_seq { + ($executor:ident.$visit:ident($instruction:expr)) => { + $executor.$visit($instruction); + if $executor.verdict().is_err() { + return $executor.verdict().clone(); + } + }; +} + +mod multisig; diff --git a/crates/iroha_executor/src/default/isi/multisig/account.rs b/crates/iroha_executor/src/default/isi/multisig/account.rs index ef1c9db9116..d48cb1b7bf6 100644 --- a/crates/iroha_executor/src/default/isi/multisig/account.rs +++ b/crates/iroha_executor/src/default/isi/multisig/account.rs @@ -1,20 +1,37 @@ //! Validation and execution logic of instructions for multisig accounts +use iroha_executor_data_model::permission::account::CanRegisterAccount; + use super::*; +use crate::permission::domain::is_domain_owner; impl VisitExecute for MultisigRegister { fn visit(&self, executor: &mut V) { - let host = executor.host(); + let registrant = executor.context().authority.clone(); let target_domain = self.account.domain(); + let host = executor.host(); + + let Ok(is_domain_owner) = is_domain_owner(target_domain, ®istrant, host) else { + deny!( + executor, + "domain must exist before registering multisig account" + ); + }; - // Any account in a domain can register any multisig account in the domain - // TODO Restrict access to the multisig signatories? - // TODO Impose proposal and approval process? - if target_domain != executor.context().authority.domain() { + let has_permission = { + CanRegisterAccount { + domain: target_domain.clone(), + } + .is_owned_by(®istrant, host) + }; + + // Impose the same restriction as for personal account registrations + // TODO Allow the signatories to register the multisig account? With propose and approve procedures? + if !(is_domain_owner || has_permission) { deny!( executor, - "multisig account and its registrant must be in the same domain" - ) + "registrant must have sufficient permission to register an account" + ); } for signatory in self.signatories.keys().cloned() { @@ -33,52 +50,56 @@ impl VisitExecute for MultisigRegister { } fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { - let host = executor.host(); + let host = executor.host().clone(); + let domain_owner = host + .query(FindDomains) + .filter_with(|domain| domain.id.eq(self.account.domain().clone())) + .execute_single() + .dbg_unwrap() + .owned_by() + .clone(); + + // Authorize as the domain owner: + // Just having permission to register accounts is insufficient to register multisig roles + executor.context_mut().authority = domain_owner.clone(); + let multisig_account = self.account; let multisig_role = multisig_role_for(&multisig_account); - host.submit(&Register::account(Account::new(multisig_account.clone()))) - .dbg_expect("registrant should successfully register a multisig account"); + visit_seq!(executor + .visit_register_account(&Register::account(Account::new(multisig_account.clone())))); - host.submit(&SetKeyValue::account( + visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( multisig_account.clone(), SIGNATORIES.parse().unwrap(), Json::new(&self.signatories), - )) - .dbg_unwrap(); + ))); - host.submit(&SetKeyValue::account( + visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( multisig_account.clone(), QUORUM.parse().unwrap(), Json::new(self.quorum), - )) - .dbg_unwrap(); + ))); - host.submit(&SetKeyValue::account( + visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( multisig_account.clone(), TRANSACTION_TTL_MS.parse().unwrap(), Json::new(self.transaction_ttl_ms), - )) - .dbg_unwrap(); + ))); - host.submit(&Register::role( - // No use, but temporarily grant a multisig role to the multisig account due to specifications - Role::new(multisig_role.clone(), multisig_account.clone()), - )) - .dbg_expect("registrant should successfully register a multisig role"); + visit_seq!(executor.visit_register_role(&Register::role( + // Temporarily grant a multisig role to the domain owner to delegate the role to the signatories + Role::new(multisig_role.clone(), domain_owner.clone()), + ))); for signatory in self.signatories.keys().cloned() { - host.submit(&Grant::account_role(multisig_role.clone(), signatory)) - .dbg_expect( - "registrant should successfully grant the multisig role to signatories", - ); + visit_seq!(executor + .visit_grant_account_role(&Grant::account_role(multisig_role.clone(), signatory))); } - // FIXME No roles to revoke found, which should have been granted to the multisig account - // host.submit(&Revoke::account_role(multisig_role, multisig_account)) - // .dbg_expect( - // "registrant should successfully revoke the multisig role from the multisig account", - // ); + visit_seq!( + executor.visit_revoke_account_role(&Revoke::account_role(multisig_role, domain_owner)) + ); Ok(()) } diff --git a/crates/iroha_executor/src/default/isi/multisig/transaction.rs b/crates/iroha_executor/src/default/isi/multisig/transaction.rs index caf2f63986a..6bd61942e68 100644 --- a/crates/iroha_executor/src/default/isi/multisig/transaction.rs +++ b/crates/iroha_executor/src/default/isi/multisig/transaction.rs @@ -6,18 +6,29 @@ use super::*; impl VisitExecute for MultisigPropose { fn visit(&self, executor: &mut V) { - let host = executor.host(); let proposer = executor.context().authority.clone(); let target_account = self.account.clone(); - let multisig_role = multisig_role_for(&target_account); + let host = executor.host(); let instructions_hash = HashOf::new(&self.instructions); - - if host + let multisig_role = multisig_role_for(&target_account); + let is_downward_proposal = host + .query_single(FindAccountMetadata::new( + proposer.clone(), + SIGNATORIES.parse().unwrap(), + )) + .map_or(false, |proposer_signatories| { + proposer_signatories + .try_into_any::>() + .dbg_unwrap() + .contains_key(&target_account) + }); + let has_multisig_role = host .query(FindRolesByAccountId::new(proposer)) .filter_with(|role_id| role_id.eq(multisig_role)) .execute_single() - .is_err() - { + .is_ok(); + + if !(is_downward_proposal || has_multisig_role) { deny!(executor, "not qualified to propose multisig"); }; @@ -33,9 +44,13 @@ impl VisitExecute for MultisigPropose { } fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { - let host = executor.host().clone(); let proposer = executor.context().authority.clone(); let target_account = self.account; + + // Authorize as the multisig account + executor.context_mut().authority = target_account.clone(); + + let host = executor.host().clone(); let instructions_hash = HashOf::new(&self.instructions); let signatories: BTreeMap = host .query_single(FindAccountMetadata::new( @@ -45,6 +60,14 @@ impl VisitExecute for MultisigPropose { .dbg_unwrap() .try_into_any() .dbg_unwrap(); + let now_ms: u64 = executor + .context() + .curr_block + .creation_time() + .as_millis() + .try_into() + .dbg_expect("shouldn't overflow within 584942417 years"); + let approvals = BTreeSet::from([proposer]); // Recursively deploy multisig authentication down to the personal leaf signatories for signatory in signatories.keys().cloned() { @@ -62,46 +85,28 @@ impl VisitExecute for MultisigPropose { MultisigPropose::new(signatory, [approve_me.into()].to_vec()) }; - let init_authority = executor.context().authority.clone(); - // Authorize as the multisig account - executor.context_mut().authority = target_account.clone(); - propose_to_approve_me - .execute(executor) - .dbg_expect("top-down proposals shouldn't fail"); - executor.context_mut().authority = init_authority; + + propose_to_approve_me.visit_execute(executor); } } - let now_ms: u64 = executor - .context() - .curr_block - .creation_time() - .as_millis() - .try_into() - .dbg_expect("shouldn't overflow within 584942417 years"); - - let approvals = BTreeSet::from([proposer]); - - host.submit(&SetKeyValue::account( + visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( target_account.clone(), instructions_key(&instructions_hash).clone(), Json::new(&self.instructions), - )) - .dbg_unwrap(); + ))); - host.submit(&SetKeyValue::account( + visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( target_account.clone(), proposed_at_ms_key(&instructions_hash).clone(), Json::new(now_ms), - )) - .dbg_unwrap(); + ))); - host.submit(&SetKeyValue::account( + visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( target_account, approvals_key(&instructions_hash).clone(), Json::new(&approvals), - )) - .dbg_unwrap(); + ))); Ok(()) } @@ -109,11 +114,11 @@ impl VisitExecute for MultisigPropose { impl VisitExecute for MultisigApprove { fn visit(&self, executor: &mut V) { - let host = executor.host(); let approver = executor.context().authority.clone(); let target_account = self.account.clone(); - let multisig_role = multisig_role_for(&target_account); + let host = executor.host(); let instructions_hash = self.instructions_hash; + let multisig_role = multisig_role_for(&target_account); if host .query(FindRolesByAccountId::new(approver)) @@ -136,9 +141,13 @@ impl VisitExecute for MultisigApprove { } fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { - let host = executor.host().clone(); let approver = executor.context().authority.clone(); let target_account = self.account; + + // Authorize as the multisig account + executor.context_mut().authority = target_account.clone(); + + let host = executor.host(); let instructions_hash = self.instructions_hash; let signatories: BTreeMap = host .query_single(FindAccountMetadata::new( @@ -180,6 +189,13 @@ impl VisitExecute for MultisigApprove { .dbg_unwrap() .try_into_any() .dbg_unwrap(); + let now_ms: u64 = executor + .context() + .curr_block + .creation_time() + .as_millis() + .try_into() + .dbg_expect("shouldn't overflow within 584942417 years"); let mut approvals: BTreeSet = host .query_single(FindAccountMetadata::new( target_account.clone(), @@ -191,20 +207,11 @@ impl VisitExecute for MultisigApprove { approvals.insert(approver); - host.submit(&SetKeyValue::account( + visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( target_account.clone(), approvals_key(&instructions_hash), Json::new(&approvals), - )) - .dbg_unwrap(); - - let now_ms: u64 = executor - .context() - .curr_block - .creation_time() - .as_millis() - .try_into() - .dbg_expect("shouldn't overflow within 584942417 years"); + ))); let is_authenticated = quorum <= signatories @@ -217,34 +224,33 @@ impl VisitExecute for MultisigApprove { if is_authenticated || is_expired { // Cleanup the transaction entry - host.submit(&RemoveKeyValue::account( - target_account.clone(), - approvals_key(&instructions_hash), - )) - .dbg_unwrap(); + visit_seq!( + executor.visit_remove_account_key_value(&RemoveKeyValue::account( + target_account.clone(), + approvals_key(&instructions_hash), + )) + ); - host.submit(&RemoveKeyValue::account( - target_account.clone(), - proposed_at_ms_key(&instructions_hash), - )) - .dbg_unwrap(); + visit_seq!( + executor.visit_remove_account_key_value(&RemoveKeyValue::account( + target_account.clone(), + proposed_at_ms_key(&instructions_hash), + )) + ); - host.submit(&RemoveKeyValue::account( - target_account.clone(), - instructions_key(&instructions_hash), - )) - .dbg_unwrap(); + visit_seq!( + executor.visit_remove_account_key_value(&RemoveKeyValue::account( + target_account.clone(), + instructions_key(&instructions_hash), + )) + ); if is_expired { // TODO Notify that the proposal has expired, while returning Ok for the entry deletion to take effect } else { // Validate and execute the authenticated multisig transaction for instruction in instructions { - let init_authority = executor.context().authority.clone(); - // Authorize as the multisig account - executor.context_mut().authority = target_account.clone(); - executor.visit_instruction(&instruction); - executor.context_mut().authority = init_authority; + visit_seq!(executor.visit_instruction(&instruction)); } } } diff --git a/crates/iroha_executor/src/default/mod.rs b/crates/iroha_executor/src/default/mod.rs index 277942eac3e..7a8479a8ee3 100644 --- a/crates/iroha_executor/src/default/mod.rs +++ b/crates/iroha_executor/src/default/mod.rs @@ -1230,18 +1230,51 @@ pub mod role { executor: &mut V, isi: &Register, ) { - const MULTISIG_SIGNATORY: &str = "MULTISIG_SIGNATORY"; - let role = isi.object(); + let grant_role = &Grant::account_role(role.id().clone(), role.grant_to().clone()); let mut new_role = Role::new(role.id().clone(), role.grant_to().clone()); // Exception for multisig roles - if role.id().name().as_ref().starts_with(MULTISIG_SIGNATORY) { - // Multisig roles should only automatically be registered bypassing validation - deny!( - executor, - "role name conflicts with the reserved multisig role names" - ) + { + use crate::permission::domain::is_domain_owner; + + const DELIMITER: char = '/'; + const MULTISIG_SIGNATORY: &str = "MULTISIG_SIGNATORY"; + + fn multisig_account_from(role: &RoleId) -> Option { + role.name() + .as_ref() + .strip_prefix(MULTISIG_SIGNATORY)? + .rsplit_once(DELIMITER) + .and_then(|(init, last)| { + format!("{last}@{}", init.trim_matches(DELIMITER)) + .parse() + .ok() + }) + } + + if role.id().name().as_ref().starts_with(MULTISIG_SIGNATORY) { + if let Some(multisig_account) = multisig_account_from(role.id()) { + if is_domain_owner( + multisig_account.domain(), + &executor.context().authority, + executor.host(), + ) + .unwrap_or_default() + { + let isi = &Register::role(new_role); + if let Err(err) = executor.host().submit(isi) { + deny!(executor, err); + } + execute!(executor, grant_role); + } + deny!( + executor, + "only the domain owner can register multisig roles" + ) + } + deny!(executor, "violates multisig role name format") + } } for permission in role.inner().permissions() { @@ -1271,7 +1304,6 @@ pub mod role { if executor.context().curr_block.is_genesis() || CanManageRoles.is_owned_by(&executor.context().authority, executor.host()) { - let grant_role = &Grant::account_role(role.id().clone(), role.grant_to().clone()); let isi = &Register::role(new_role); if let Err(err) = executor.host().submit(isi) { deny!(executor, err); diff --git a/crates/iroha_executor/src/lib.rs b/crates/iroha_executor/src/lib.rs index 643c7d67338..bce5b1cbe12 100644 --- a/crates/iroha_executor/src/lib.rs +++ b/crates/iroha_executor/src/lib.rs @@ -294,7 +294,7 @@ pub trait Execute { /// Represents the current state of the world fn context(&self) -> &prelude::Context; - /// Mutable context for e.g. switching to another authority on recursive execution. + /// 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; diff --git a/scripts/tests/multisig.sh b/scripts/tests/multisig.sh index 272e52b0cfb..c3bfa298d86 100644 --- a/scripts/tests/multisig.sh +++ b/scripts/tests/multisig.sh @@ -38,12 +38,12 @@ for signatory in ${SIGNATORIES[@]}; do ./iroha account register --id $signatory done -# register a multisig account +# register a multisig account by the domain owner MULTISIG_ACCOUNT=$(gen_account_id "msa") WEIGHTS=($(yes 1 | head -n $N_SIGNATORIES)) # equal votes QUORUM=$N_SIGNATORIES # unanimous TRANSACTION_TTL="1y 6M 2w 3d 12h 30m 30s 500ms" -./iroha --config "client.1.toml" multisig register --account $MULTISIG_ACCOUNT --signatories ${SIGNATORIES[*]} --weights ${WEIGHTS[*]} --quorum $QUORUM --transaction-ttl "$TRANSACTION_TTL" +./iroha --config "client.toml" multisig register --account $MULTISIG_ACCOUNT --signatories ${SIGNATORIES[*]} --weights ${WEIGHTS[*]} --quorum $QUORUM --transaction-ttl "$TRANSACTION_TTL" # propose a multisig transaction INSTRUCTIONS="../scripts/tests/instructions.json" From 814b23b064e3282f341340f7e2092bbfe4e39986 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Fri, 8 Nov 2024 18:17:46 +0900 Subject: [PATCH 20/28] review: minor updates since `#issuecomment-2463859606` Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha_executor/src/default/isi/mod.rs | 2 ++ crates/iroha_executor/src/default/isi/multisig/account.rs | 4 ++-- .../iroha_executor/src/default/isi/multisig/transaction.rs | 7 ++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/iroha_executor/src/default/isi/mod.rs b/crates/iroha_executor/src/default/isi/mod.rs index 1795a86975a..d3fddf374c0 100644 --- a/crates/iroha_executor/src/default/isi/mod.rs +++ b/crates/iroha_executor/src/default/isi/mod.rs @@ -35,6 +35,8 @@ trait VisitExecute: crate::data_model::isi::Instruction { } } +/// Validate and execute instructions in sequence without returning back to the visit root, +/// checking the sanity of the executor verdict macro_rules! visit_seq { ($executor:ident.$visit:ident($instruction:expr)) => { $executor.$visit($instruction); diff --git a/crates/iroha_executor/src/default/isi/multisig/account.rs b/crates/iroha_executor/src/default/isi/multisig/account.rs index d48cb1b7bf6..5608bc9944b 100644 --- a/crates/iroha_executor/src/default/isi/multisig/account.rs +++ b/crates/iroha_executor/src/default/isi/multisig/account.rs @@ -50,8 +50,8 @@ impl VisitExecute for MultisigRegister { } fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { - let host = executor.host().clone(); - let domain_owner = host + let domain_owner = executor + .host() .query(FindDomains) .filter_with(|domain| domain.id.eq(self.account.domain().clone())) .execute_single() diff --git a/crates/iroha_executor/src/default/isi/multisig/transaction.rs b/crates/iroha_executor/src/default/isi/multisig/transaction.rs index 6bd61942e68..b3c7295d8d3 100644 --- a/crates/iroha_executor/src/default/isi/multisig/transaction.rs +++ b/crates/iroha_executor/src/default/isi/multisig/transaction.rs @@ -50,9 +50,9 @@ impl VisitExecute for MultisigPropose { // Authorize as the multisig account executor.context_mut().authority = target_account.clone(); - let host = executor.host().clone(); let instructions_hash = HashOf::new(&self.instructions); - let signatories: BTreeMap = host + let signatories: BTreeMap = executor + .host() .query_single(FindAccountMetadata::new( target_account.clone(), SIGNATORIES.parse().unwrap(), @@ -71,7 +71,8 @@ impl VisitExecute for MultisigPropose { // Recursively deploy multisig authentication down to the personal leaf signatories for signatory in signatories.keys().cloned() { - let is_multisig_again = host + let is_multisig_again = executor + .host() .query(FindRoleIds) .filter_with(|role_id| role_id.eq(multisig_role_for(&signatory))) .execute_single_opt() From f54d3d9aa8242aa8827e6d4fd9f759269e73f945 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Fri, 8 Nov 2024 21:59:14 +0900 Subject: [PATCH 21/28] review: exclude custom instructions from schema `types!` Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- Cargo.lock | 1 - crates/iroha_executor_data_model/Cargo.toml | 1 - crates/iroha_executor_data_model/src/isi.rs | 9 ++++----- crates/iroha_schema_gen/src/lib.rs | 12 +++++------- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a561b5f9286..b4768ad5921 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3212,7 +3212,6 @@ dependencies = [ "iroha_data_model", "iroha_executor_data_model_derive", "iroha_schema", - "parity-scale-codec", "serde", "serde_json", ] diff --git a/crates/iroha_executor_data_model/Cargo.toml b/crates/iroha_executor_data_model/Cargo.toml index 6f71451ac1b..df1d60ab4da 100644 --- a/crates/iroha_executor_data_model/Cargo.toml +++ b/crates/iroha_executor_data_model/Cargo.toml @@ -17,6 +17,5 @@ iroha_data_model.workspace = true iroha_schema.workspace = true derive_more = { workspace = true, features = ["constructor", "from"] } -parity-scale-codec.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/crates/iroha_executor_data_model/src/isi.rs b/crates/iroha_executor_data_model/src/isi.rs index 8bbf9c196c3..255df1df20e 100644 --- a/crates/iroha_executor_data_model/src/isi.rs +++ b/crates/iroha_executor_data_model/src/isi.rs @@ -8,7 +8,6 @@ use iroha_data_model::{ prelude::{Json, *}, }; use iroha_schema::IntoSchema; -use parity_scale_codec::{Decode, Encode}; use serde::{Deserialize, Serialize}; use super::*; @@ -57,7 +56,7 @@ pub mod multisig { use super::*; /// Multisig-related instructions - #[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, From)] + #[derive(Debug, Clone, Serialize, Deserialize, IntoSchema, From)] pub enum MultisigInstructionBox { /// Register a multisig account, which is a prerequisite of multisig transactions Register(MultisigRegister), @@ -68,7 +67,7 @@ pub mod multisig { } /// Register a multisig account, which is a prerequisite of multisig transactions - #[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, Constructor)] + #[derive(Debug, Clone, Serialize, Deserialize, IntoSchema, Constructor)] pub struct MultisigRegister { /// Multisig account to be registered ///
@@ -95,7 +94,7 @@ pub mod multisig { pub const DEFAULT_MULTISIG_TTL_MS: u64 = 60 * 60 * 1_000; // 1 hour /// Propose a multisig transaction and initialize approvals with the proposer's one - #[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, Constructor)] + #[derive(Debug, Clone, Serialize, Deserialize, IntoSchema, Constructor)] pub struct MultisigPropose { /// Multisig account to propose pub account: AccountId, @@ -104,7 +103,7 @@ pub mod multisig { } /// Approve a certain multisig transaction - #[derive(Debug, Clone, Encode, Decode, Serialize, Deserialize, IntoSchema, Constructor)] + #[derive(Debug, Clone, Serialize, Deserialize, IntoSchema, Constructor)] pub struct MultisigApprove { /// Multisig account to approve pub account: AccountId, diff --git a/crates/iroha_schema_gen/src/lib.rs b/crates/iroha_schema_gen/src/lib.rs index 7e7f5dc6770..0c13c559cc4 100644 --- a/crates/iroha_schema_gen/src/lib.rs +++ b/crates/iroha_schema_gen/src/lib.rs @@ -283,10 +283,6 @@ types!( MintabilityError, Mintable, Mismatch, - MultisigInstructionBox, - MultisigRegister, - MultisigPropose, - MultisigApprove, Name, NewAccount, NewAssetDefinition, @@ -549,9 +545,6 @@ pub mod complete_data_model { }, Level, }; - pub use iroha_executor_data_model::isi::multisig::{ - MultisigApprove, MultisigInstructionBox, MultisigPropose, MultisigRegister, - }; pub use iroha_genesis::{GenesisWasmAction, GenesisWasmTrigger, WasmPath}; pub use iroha_primitives::{const_vec::ConstVec, conststr::ConstString, json::Json}; pub use iroha_schema::Compact; @@ -631,6 +624,11 @@ mod tests { ); insert_into_test_map!(iroha_executor_data_model::permission::executor::CanUpgradeExecutor); + insert_into_test_map!(iroha_executor_data_model::isi::multisig::MultisigInstructionBox); + insert_into_test_map!(iroha_executor_data_model::isi::multisig::MultisigRegister); + insert_into_test_map!(iroha_executor_data_model::isi::multisig::MultisigPropose); + insert_into_test_map!(iroha_executor_data_model::isi::multisig::MultisigApprove); + map } From bd20861846c6dee5be22738778d6143d4fc21301 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Fri, 8 Nov 2024 22:32:24 +0900 Subject: [PATCH 22/28] review: dedup and delegate `MultisigRegister` validation to execution Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- .../src/default/isi/multisig/account.rs | 62 +++---------------- 1 file changed, 10 insertions(+), 52 deletions(-) diff --git a/crates/iroha_executor/src/default/isi/multisig/account.rs b/crates/iroha_executor/src/default/isi/multisig/account.rs index 5608bc9944b..1b27223b93f 100644 --- a/crates/iroha_executor/src/default/isi/multisig/account.rs +++ b/crates/iroha_executor/src/default/isi/multisig/account.rs @@ -1,61 +1,25 @@ //! Validation and execution logic of instructions for multisig accounts -use iroha_executor_data_model::permission::account::CanRegisterAccount; - use super::*; -use crate::permission::domain::is_domain_owner; impl VisitExecute for MultisigRegister { - fn visit(&self, executor: &mut V) { - let registrant = executor.context().authority.clone(); - let target_domain = self.account.domain(); - let host = executor.host(); - - let Ok(is_domain_owner) = is_domain_owner(target_domain, ®istrant, host) else { - deny!( - executor, - "domain must exist before registering multisig account" - ); - }; - - let has_permission = { - CanRegisterAccount { - domain: target_domain.clone(), - } - .is_owned_by(®istrant, host) - }; + fn visit(&self, _executor: &mut V) {} - // Impose the same restriction as for personal account registrations - // TODO Allow the signatories to register the multisig account? With propose and approve procedures? - if !(is_domain_owner || has_permission) { - deny!( - executor, - "registrant must have sufficient permission to register an account" - ); - } + fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { + let multisig_account = self.account; + let multisig_role = multisig_role_for(&multisig_account); - for signatory in self.signatories.keys().cloned() { - if host - .query(FindAccounts) - .filter_with(|account| account.id.eq(signatory)) - .execute_single() - .is_err() - { - deny!( - executor, - "signatories must exist before registering multisig account" - ); - } - } - } + // The multisig registrant needs to have sufficient permission to register personal accounts + // TODO Loosen to just being one of the signatories? But impose the procedure of propose and approve? + visit_seq!(executor + .visit_register_account(&Register::account(Account::new(multisig_account.clone())))); - fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { let domain_owner = executor .host() .query(FindDomains) - .filter_with(|domain| domain.id.eq(self.account.domain().clone())) + .filter_with(|domain| domain.id.eq(multisig_account.domain().clone())) .execute_single() - .dbg_unwrap() + .dbg_expect("domain should be found as the preceding account registration succeeded") .owned_by() .clone(); @@ -63,12 +27,6 @@ impl VisitExecute for MultisigRegister { // Just having permission to register accounts is insufficient to register multisig roles executor.context_mut().authority = domain_owner.clone(); - let multisig_account = self.account; - let multisig_role = multisig_role_for(&multisig_account); - - visit_seq!(executor - .visit_register_account(&Register::account(Account::new(multisig_account.clone())))); - visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( multisig_account.clone(), SIGNATORIES.parse().unwrap(), From 17287d3b272dc7c83d68ad158d7332437fe172e4 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Mon, 11 Nov 2024 22:14:50 +0900 Subject: [PATCH 23/28] review: dedup and delegate `MultisigPropose` validation to execution Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- .../src/default/isi/multisig/transaction.rs | 47 +++++++++---------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/crates/iroha_executor/src/default/isi/multisig/transaction.rs b/crates/iroha_executor/src/default/isi/multisig/transaction.rs index b3c7295d8d3..d2dc0a73a47 100644 --- a/crates/iroha_executor/src/default/isi/multisig/transaction.rs +++ b/crates/iroha_executor/src/default/isi/multisig/transaction.rs @@ -6,35 +6,13 @@ use super::*; impl VisitExecute for MultisigPropose { fn visit(&self, executor: &mut V) { - let proposer = executor.context().authority.clone(); let target_account = self.account.clone(); - let host = executor.host(); let instructions_hash = HashOf::new(&self.instructions); - let multisig_role = multisig_role_for(&target_account); - let is_downward_proposal = host - .query_single(FindAccountMetadata::new( - proposer.clone(), - SIGNATORIES.parse().unwrap(), - )) - .map_or(false, |proposer_signatories| { - proposer_signatories - .try_into_any::>() - .dbg_unwrap() - .contains_key(&target_account) - }); - let has_multisig_role = host - .query(FindRolesByAccountId::new(proposer)) - .filter_with(|role_id| role_id.eq(multisig_role)) - .execute_single() - .is_ok(); - - if !(is_downward_proposal || has_multisig_role) { - deny!(executor, "not qualified to propose multisig"); - }; - if host + if executor + .host() .query_single(FindAccountMetadata::new( - target_account.clone(), + target_account, approvals_key(&instructions_hash), )) .is_ok() @@ -51,15 +29,32 @@ impl VisitExecute for MultisigPropose { executor.context_mut().authority = target_account.clone(); let instructions_hash = HashOf::new(&self.instructions); + let is_downward_proposal = executor + .host() + .query_single(FindAccountMetadata::new( + proposer.clone(), + SIGNATORIES.parse().unwrap(), + )) + .map_or(false, |proposer_signatories| { + proposer_signatories + .try_into_any::>() + .dbg_unwrap() + .contains_key(&target_account) + }); let signatories: BTreeMap = executor .host() .query_single(FindAccountMetadata::new( target_account.clone(), SIGNATORIES.parse().unwrap(), )) - .dbg_unwrap() + .dbg_expect("preceding duplication check should mean the target account is multisig") .try_into_any() .dbg_unwrap(); + if !(is_downward_proposal || signatories.contains_key(&proposer)) { + return Err(ValidationFail::NotPermitted( + "not qualified to propose multisig".into(), + )); + } let now_ms: u64 = executor .context() .curr_block From d1a167c1f97f88a0e74ec992e13bf8e91986bec6 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Tue, 12 Nov 2024 00:16:40 +0900 Subject: [PATCH 24/28] Revert "review: dedup and delegate `MultisigPropose` validation to execution" This reverts commit 17287d3b272dc7c83d68ad158d7332437fe172e4. Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- .../src/default/isi/multisig/transaction.rs | 47 ++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/crates/iroha_executor/src/default/isi/multisig/transaction.rs b/crates/iroha_executor/src/default/isi/multisig/transaction.rs index d2dc0a73a47..b3c7295d8d3 100644 --- a/crates/iroha_executor/src/default/isi/multisig/transaction.rs +++ b/crates/iroha_executor/src/default/isi/multisig/transaction.rs @@ -6,13 +6,35 @@ use super::*; impl VisitExecute for MultisigPropose { fn visit(&self, executor: &mut V) { + let proposer = executor.context().authority.clone(); let target_account = self.account.clone(); + let host = executor.host(); let instructions_hash = HashOf::new(&self.instructions); + let multisig_role = multisig_role_for(&target_account); + let is_downward_proposal = host + .query_single(FindAccountMetadata::new( + proposer.clone(), + SIGNATORIES.parse().unwrap(), + )) + .map_or(false, |proposer_signatories| { + proposer_signatories + .try_into_any::>() + .dbg_unwrap() + .contains_key(&target_account) + }); + let has_multisig_role = host + .query(FindRolesByAccountId::new(proposer)) + .filter_with(|role_id| role_id.eq(multisig_role)) + .execute_single() + .is_ok(); - if executor - .host() + if !(is_downward_proposal || has_multisig_role) { + deny!(executor, "not qualified to propose multisig"); + }; + + if host .query_single(FindAccountMetadata::new( - target_account, + target_account.clone(), approvals_key(&instructions_hash), )) .is_ok() @@ -29,32 +51,15 @@ impl VisitExecute for MultisigPropose { executor.context_mut().authority = target_account.clone(); let instructions_hash = HashOf::new(&self.instructions); - let is_downward_proposal = executor - .host() - .query_single(FindAccountMetadata::new( - proposer.clone(), - SIGNATORIES.parse().unwrap(), - )) - .map_or(false, |proposer_signatories| { - proposer_signatories - .try_into_any::>() - .dbg_unwrap() - .contains_key(&target_account) - }); let signatories: BTreeMap = executor .host() .query_single(FindAccountMetadata::new( target_account.clone(), SIGNATORIES.parse().unwrap(), )) - .dbg_expect("preceding duplication check should mean the target account is multisig") + .dbg_unwrap() .try_into_any() .dbg_unwrap(); - if !(is_downward_proposal || signatories.contains_key(&proposer)) { - return Err(ValidationFail::NotPermitted( - "not qualified to propose multisig".into(), - )); - } let now_ms: u64 = executor .context() .curr_block From e41ba8841e6d666361113452e3bfb14e44657c4b Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Tue, 12 Nov 2024 00:23:49 +0900 Subject: [PATCH 25/28] review: dedup and delegate `MultisigApprove` validation to execution Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- .../src/default/isi/multisig/transaction.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/crates/iroha_executor/src/default/isi/multisig/transaction.rs b/crates/iroha_executor/src/default/isi/multisig/transaction.rs index b3c7295d8d3..7cdd959b4e1 100644 --- a/crates/iroha_executor/src/default/isi/multisig/transaction.rs +++ b/crates/iroha_executor/src/default/isi/multisig/transaction.rs @@ -118,7 +118,6 @@ impl VisitExecute for MultisigApprove { let approver = executor.context().authority.clone(); let target_account = self.account.clone(); let host = executor.host(); - let instructions_hash = self.instructions_hash; let multisig_role = multisig_role_for(&target_account); if host @@ -129,16 +128,6 @@ impl VisitExecute for MultisigApprove { { deny!(executor, "not qualified to approve multisig"); }; - - if host - .query_single(FindAccountMetadata::new( - target_account.clone(), - approvals_key(&instructions_hash), - )) - .is_err() - { - deny!(executor, "no proposals to approve") - }; } fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { @@ -178,8 +167,7 @@ impl VisitExecute for MultisigApprove { .query_single(FindAccountMetadata::new( target_account.clone(), instructions_key(&instructions_hash), - )) - .dbg_unwrap() + ))? .try_into_any() .dbg_unwrap(); let proposed_at_ms: u64 = host From 0bb62506aef32e7a28d32fdc7939603bb4897996 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Fri, 15 Nov 2024 12:56:27 +0900 Subject: [PATCH 26/28] review: minor updates since `issuecomment-2468473163` Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- .../src/default/isi/multisig/transaction.rs | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/crates/iroha_executor/src/default/isi/multisig/transaction.rs b/crates/iroha_executor/src/default/isi/multisig/transaction.rs index 7cdd959b4e1..542fcc1636b 100644 --- a/crates/iroha_executor/src/default/isi/multisig/transaction.rs +++ b/crates/iroha_executor/src/default/isi/multisig/transaction.rs @@ -7,10 +7,10 @@ use super::*; impl VisitExecute for MultisigPropose { fn visit(&self, executor: &mut V) { let proposer = executor.context().authority.clone(); - let target_account = self.account.clone(); + let multisig_account = self.account.clone(); let host = executor.host(); let instructions_hash = HashOf::new(&self.instructions); - let multisig_role = multisig_role_for(&target_account); + let multisig_role = multisig_role_for(&multisig_account); let is_downward_proposal = host .query_single(FindAccountMetadata::new( proposer.clone(), @@ -20,7 +20,7 @@ impl VisitExecute for MultisigPropose { proposer_signatories .try_into_any::>() .dbg_unwrap() - .contains_key(&target_account) + .contains_key(&multisig_account) }); let has_multisig_role = host .query(FindRolesByAccountId::new(proposer)) @@ -34,7 +34,7 @@ impl VisitExecute for MultisigPropose { if host .query_single(FindAccountMetadata::new( - target_account.clone(), + multisig_account.clone(), approvals_key(&instructions_hash), )) .is_ok() @@ -45,16 +45,16 @@ impl VisitExecute for MultisigPropose { fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { let proposer = executor.context().authority.clone(); - let target_account = self.account; + let multisig_account = self.account; // Authorize as the multisig account - executor.context_mut().authority = target_account.clone(); + executor.context_mut().authority = multisig_account.clone(); let instructions_hash = HashOf::new(&self.instructions); let signatories: BTreeMap = executor .host() .query_single(FindAccountMetadata::new( - target_account.clone(), + multisig_account.clone(), SIGNATORIES.parse().unwrap(), )) .dbg_unwrap() @@ -82,7 +82,7 @@ impl VisitExecute for MultisigPropose { if is_multisig_again { let propose_to_approve_me = { let approve_me = - MultisigApprove::new(target_account.clone(), instructions_hash); + MultisigApprove::new(multisig_account.clone(), instructions_hash); MultisigPropose::new(signatory, [approve_me.into()].to_vec()) }; @@ -92,19 +92,19 @@ impl VisitExecute for MultisigPropose { } visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( - target_account.clone(), + multisig_account.clone(), instructions_key(&instructions_hash).clone(), Json::new(&self.instructions), ))); visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( - target_account.clone(), + multisig_account.clone(), proposed_at_ms_key(&instructions_hash).clone(), Json::new(now_ms), ))); visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( - target_account, + multisig_account, approvals_key(&instructions_hash).clone(), Json::new(&approvals), ))); @@ -116,9 +116,9 @@ impl VisitExecute for MultisigPropose { impl VisitExecute for MultisigApprove { fn visit(&self, executor: &mut V) { let approver = executor.context().authority.clone(); - let target_account = self.account.clone(); + let multisig_account = self.account.clone(); let host = executor.host(); - let multisig_role = multisig_role_for(&target_account); + let multisig_role = multisig_role_for(&multisig_account); if host .query(FindRolesByAccountId::new(approver)) @@ -132,16 +132,16 @@ impl VisitExecute for MultisigApprove { fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { let approver = executor.context().authority.clone(); - let target_account = self.account; + let multisig_account = self.account; // Authorize as the multisig account - executor.context_mut().authority = target_account.clone(); + executor.context_mut().authority = multisig_account.clone(); let host = executor.host(); let instructions_hash = self.instructions_hash; let signatories: BTreeMap = host .query_single(FindAccountMetadata::new( - target_account.clone(), + multisig_account.clone(), SIGNATORIES.parse().unwrap(), )) .dbg_unwrap() @@ -149,7 +149,7 @@ impl VisitExecute for MultisigApprove { .dbg_unwrap(); let quorum: u16 = host .query_single(FindAccountMetadata::new( - target_account.clone(), + multisig_account.clone(), QUORUM.parse().unwrap(), )) .dbg_unwrap() @@ -157,7 +157,7 @@ impl VisitExecute for MultisigApprove { .dbg_unwrap(); let transaction_ttl_ms: u64 = host .query_single(FindAccountMetadata::new( - target_account.clone(), + multisig_account.clone(), TRANSACTION_TTL_MS.parse().unwrap(), )) .dbg_unwrap() @@ -165,14 +165,14 @@ impl VisitExecute for MultisigApprove { .dbg_unwrap(); let instructions: Vec = host .query_single(FindAccountMetadata::new( - target_account.clone(), + multisig_account.clone(), instructions_key(&instructions_hash), ))? .try_into_any() .dbg_unwrap(); let proposed_at_ms: u64 = host .query_single(FindAccountMetadata::new( - target_account.clone(), + multisig_account.clone(), proposed_at_ms_key(&instructions_hash), )) .dbg_unwrap() @@ -187,7 +187,7 @@ impl VisitExecute for MultisigApprove { .dbg_expect("shouldn't overflow within 584942417 years"); let mut approvals: BTreeSet = host .query_single(FindAccountMetadata::new( - target_account.clone(), + multisig_account.clone(), approvals_key(&instructions_hash), )) .dbg_unwrap() @@ -197,7 +197,7 @@ impl VisitExecute for MultisigApprove { approvals.insert(approver); visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( - target_account.clone(), + multisig_account.clone(), approvals_key(&instructions_hash), Json::new(&approvals), ))); @@ -215,21 +215,21 @@ impl VisitExecute for MultisigApprove { // Cleanup the transaction entry visit_seq!( executor.visit_remove_account_key_value(&RemoveKeyValue::account( - target_account.clone(), + multisig_account.clone(), approvals_key(&instructions_hash), )) ); visit_seq!( executor.visit_remove_account_key_value(&RemoveKeyValue::account( - target_account.clone(), + multisig_account.clone(), proposed_at_ms_key(&instructions_hash), )) ); visit_seq!( executor.visit_remove_account_key_value(&RemoveKeyValue::account( - target_account.clone(), + multisig_account.clone(), instructions_key(&instructions_hash), )) ); From 2522e18eaadb95069f9319e9e9b6ae18413d4c99 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Fri, 15 Nov 2024 13:09:55 +0900 Subject: [PATCH 27/28] review: `visit_grant_account_role` in `visit_register_role` Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha_executor/src/default/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/iroha_executor/src/default/mod.rs b/crates/iroha_executor/src/default/mod.rs index 7a8479a8ee3..4b85c85509e 100644 --- a/crates/iroha_executor/src/default/mod.rs +++ b/crates/iroha_executor/src/default/mod.rs @@ -1266,7 +1266,7 @@ pub mod role { if let Err(err) = executor.host().submit(isi) { deny!(executor, err); } - execute!(executor, grant_role); + return executor.visit_grant_account_role(grant_role); } deny!( executor, @@ -1309,7 +1309,7 @@ pub mod role { deny!(executor, err); } - execute!(executor, grant_role); + return executor.visit_grant_account_role(grant_role); } deny!(executor, "Can't register role"); From 11800b62c1772facaf4338b670160446b55683e8 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Fri, 15 Nov 2024 13:10:12 +0900 Subject: [PATCH 28/28] Revert "review: `visit_grant_account_role` in `visit_register_role`" This reverts commit 9108bb71f4465a3300e414a0601b21142fdecd17. Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- crates/iroha_executor/src/default/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/iroha_executor/src/default/mod.rs b/crates/iroha_executor/src/default/mod.rs index 4b85c85509e..7a8479a8ee3 100644 --- a/crates/iroha_executor/src/default/mod.rs +++ b/crates/iroha_executor/src/default/mod.rs @@ -1266,7 +1266,7 @@ pub mod role { if let Err(err) = executor.host().submit(isi) { deny!(executor, err); } - return executor.visit_grant_account_role(grant_role); + execute!(executor, grant_role); } deny!( executor, @@ -1309,7 +1309,7 @@ pub mod role { deny!(executor, err); } - return executor.visit_grant_account_role(grant_role); + execute!(executor, grant_role); } deny!(executor, "Can't register role");