diff --git a/Cargo.lock b/Cargo.lock index c589a2c65d1..65c495e40e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2960,6 +2960,7 @@ version = "2.0.0-rc.1.0" dependencies = [ "clap", "color-eyre", + "derive_more", "erased-serde", "error-stack", "eyre", @@ -2970,6 +2971,7 @@ dependencies = [ "json5", "serde", "serde_json", + "serde_with", "supports-color 2.1.0", "thiserror", "tokio", diff --git a/crates/iroha/tests/multisig.rs b/crates/iroha/tests/multisig.rs index 1ac73daca89..0e8f36e17d2 100644 --- a/crates/iroha/tests/multisig.rs +++ b/crates/iroha/tests/multisig.rs @@ -1,5 +1,5 @@ use std::{ - collections::{BTreeMap, BTreeSet}, + collections::BTreeMap, num::{NonZeroU16, NonZeroU64}, time::Duration, }; @@ -18,6 +18,36 @@ use iroha_test_samples::{ gen_account_in, ALICE_ID, BOB_ID, BOB_KEYPAIR, CARPENTER_ID, CARPENTER_KEYPAIR, }; +#[test] +fn multisig_normal() -> Result<()> { + multisig_base(TestSuite::normal()) +} + +#[test] +fn multisig_unauthorized() -> Result<()> { + multisig_base(TestSuite::unauthorized()) +} + +#[test] +fn multisig_expires() -> Result<()> { + multisig_base(TestSuite::expires()) +} + +#[test] +fn multisig_recursion_normal() -> Result<()> { + multisig_recursion_base(TestSuite::normal()) +} + +#[test] +fn multisig_recursion_unauthorized() -> Result<()> { + multisig_recursion_base(TestSuite::unauthorized()) +} + +#[test] +fn multisig_recursion_expires() -> Result<()> { + multisig_recursion_base(TestSuite::expires()) +} + #[derive(Constructor)] struct TestSuite { domain: DomainId, @@ -26,47 +56,43 @@ struct TestSuite { transaction_ttl_ms_opt: Option, } -#[test] -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) -} +impl TestSuite { + fn normal() -> Self { + // 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; + + Self::new( + domain, + multisig_account_id, + unauthorized_target_opt, + transaction_ttl_ms_opt, + ) + } -#[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()); + fn unauthorized() -> Self { + 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) -} + Self::new(domain, multisig_account_id, unauthorized_target_opt, None) + } -#[test] -fn multisig_expires() -> Result<()> { - 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); + fn expires() -> Self { + 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) + Self::new(domain, multisig_account_id, None, transaction_ttl_ms_opt) + } } /// # Scenario @@ -118,22 +144,24 @@ fn multisig_base(suite: TestSuite) -> Result<()> { let register_multisig_account = MultisigRegister::new( multisig_account_id.clone(), - signatories - .keys() - .enumerate() - .map(|(weight, id)| (id.clone(), 1 + weight as u8)) - .collect(), - // Quorum can be reached without the first signatory - (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), + MultisigSpec::new( + signatories + .keys() + .enumerate() + .map(|(weight, id)| (id.clone(), 1 + weight as u8)) + .collect(), + // Quorum can be reached without the first signatory + (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 @@ -190,7 +218,7 @@ fn multisig_base(suite: TestSuite) -> Result<()> { let proposer = signatories.pop_last().unwrap(); let mut approvers = signatories.into_iter(); - let propose = MultisigPropose::new(multisig_account_id.clone(), instructions); + let propose = MultisigPropose::new(multisig_account_id.clone(), instructions, None); alt_client(proposer, &test_client).submit_blocking(propose)?; // Allow time to elapse to test the expiration @@ -210,8 +238,12 @@ fn multisig_base(suite: TestSuite) -> Result<()> { 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()), + None => { + res.unwrap(); + } + _ => { + let _err = res.unwrap_err(); + } } } @@ -227,28 +259,40 @@ fn multisig_base(suite: TestSuite) -> Result<()> { 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()), + (None, None) => { + res.unwrap(); + } + _ => { + let _err = res.unwrap_err(); + } } // Check if the multisig transaction has executed 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()), + (None, None) => { + res.unwrap(); + } + _ => { + let _err = res.unwrap_err(); + } } // Check if the transaction entry is deleted let res = test_client.query_single(FindAccountMetadata::new( multisig_account_id, - format!("proposals/{instructions_hash}/instructions") + format!("multisig/proposals/{instructions_hash}") .parse() .unwrap(), )); 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()), + (None, Some(_)) => { + // In case failing validation, the entry can exit only by expiring + res.unwrap(); + } + _ => { + let _err = res.unwrap_err(); + } } Ok(()) @@ -265,9 +309,15 @@ fn multisig_base(suite: TestSuite) -> Result<()> { /// / / \ / | \ /// 0 1 2 3 4 5 <--- personal signatories /// ``` -#[test] #[expect(clippy::similar_names, clippy::too_many_lines)] -fn multisig_recursion() -> Result<()> { +fn multisig_recursion_base(suite: TestSuite) -> Result<()> { + 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(); @@ -289,144 +339,198 @@ fn multisig_recursion() -> Result<()> { let mut sigs = signatories.clone(); let sigs_345 = sigs.split_off(signatories.keys().nth(3).unwrap()); let sigs_12 = sigs.split_off(signatories.keys().nth(1).unwrap()); - let mut sigs_0 = sigs; - - let register_ms_accounts = |sigs_list: Vec>| { - sigs_list - .into_iter() - .map(|sigs| { - let ms_account_id = gen_account_in(wonderland).0; - let register_ms_account = MultisigRegister::new( - ms_account_id.clone(), - sigs.iter().copied().map(|id| (id.clone(), 1)).collect(), - sigs.len() - .try_into() - .ok() - .and_then(NonZeroU16::new) - .unwrap(), - NonZeroU64::new(u64::MAX).unwrap(), - ); - - test_client - .submit_blocking(register_ms_account) - .expect("multisig account should be registered by account of the same domain"); - - ms_account_id - }) - .collect::>() + let sig_0 = sigs.pop_last().unwrap(); + + let register_ms_account = |sigs: Vec<&AccountId>| { + let ms_account_id = gen_account_in(wonderland).0; + let spec = MultisigSpec::new( + // Equal votes + sigs.iter().copied().map(|id| (id.clone(), 1)).collect(), + // Unanimous + sigs.len() + .try_into() + .ok() + .and_then(NonZeroU16::new) + .unwrap(), + transaction_ttl_ms_opt + .and_then(NonZeroU64::new) + .unwrap_or(NonZeroU64::MAX), + ); + let register = MultisigRegister::new(ms_account_id.clone(), spec.clone()); + + test_client + .submit_blocking(register) + .expect("the domain owner should succeed to register a multisig account"); + + (ms_account_id, spec) }; - let sigs_list: Vec> = [&sigs_12, &sigs_345] - .into_iter() - .map(|sigs| sigs.keys().collect()) - .collect(); - let msas = register_ms_accounts(sigs_list); - let msa_12 = msas[0].clone(); - let msa_345 = msas[1].clone(); - - let sigs_list = vec![vec![&msa_12, &msa_345]]; - let msas = register_ms_accounts(sigs_list); - let msa_12345 = msas[0].clone(); - - let sig_0 = sigs_0.keys().next().unwrap().clone(); - let sigs_list = vec![vec![&sig_0, &msa_12345]]; - let msas = register_ms_accounts(sigs_list); + let (msa_12, _spec_12) = register_ms_account(sigs_12.keys().collect()); + let (msa_345, _spec_345) = register_ms_account(sigs_345.keys().collect()); + let (msa_12345, _spec_12345) = register_ms_account(vec![&msa_12, &msa_345]); // The root multisig account with 6 personal signatories under its umbrella - let msa_012345 = msas[0].clone(); + let (msa_012345, _spec_012345) = register_ms_account(vec![&sig_0.0, &msa_12345]); // One of personal signatories proposes a multisig transaction let key: Name = "success_marker".parse().unwrap(); + let transaction_target = unauthorized_target_opt + .as_ref() + .unwrap_or(&msa_012345) + .clone(); let instructions = vec![SetKeyValue::account( - msa_012345.clone(), + transaction_target.clone(), key.clone(), "congratulations".parse::().unwrap(), ) .into()]; let instructions_hash = HashOf::new(&instructions); - let proposer = sigs_0.pop_last().unwrap(); - let propose = MultisigPropose::new(msa_012345.clone(), instructions); + let proposer = sig_0; + let propose = MultisigPropose::new(msa_012345.clone(), instructions, None); alt_client(proposer, &test_client).submit_blocking(propose)?; + // Allow time to elapse to test the expiration + 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()))?; + // 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 approve: InstructionBox = - MultisigApprove::new(msa_012345.clone(), instructions_hash).into(); + let approve_to_012345: InstructionBox = + MultisigApprove::new(msa_012345.clone(), instructions_hash).into(); + let approval_hash_to_012345 = HashOf::new(&vec![approve_to_012345]); + + let approve_to_12345: InstructionBox = + MultisigApprove::new(msa_12345.clone(), approval_hash_to_012345).into(); + let approval_hash_to_12345 = HashOf::new(&vec![approve_to_12345.clone()]); + + let proposal_value_at = |msa: AccountId, mst_hash: HashOf>| { + test_client + .query_single(FindAccountMetadata::new( + msa.clone(), + format!("multisig/proposals/{mst_hash}").parse().unwrap(), + )) + .expect("should be initialized by the root proposal") + .try_into_any::() + .unwrap() + }; + let proposal_value_at_012345 = proposal_value_at(msa_012345.clone(), instructions_hash); + let proposal_value_at_12 = proposal_value_at(msa_12.clone(), approval_hash_to_12345); - HashOf::new(&vec![approve]) - }; - let approve: InstructionBox = - MultisigApprove::new(msa_12345.clone(), approval_hash_to_012345).into(); + assert_eq!(proposal_value_at_12.instructions, vec![approve_to_12345]); + assert_eq!( + proposal_value_at_12.proposed_at_ms, + proposal_value_at_012345.proposed_at_ms + ); + assert_eq!( + proposal_value_at_12.expires_at_ms, + proposal_value_at_012345.expires_at_ms + ); + assert!(proposal_value_at_12.approvals.is_empty()); + assert_eq!(proposal_value_at_12.is_relayed, Some(false)); - HashOf::new(&vec![approve]) - }; + // All the rest signatories try to approve the multisig transaction + let mut approvals_iter = sigs_12 + .into_iter() + .map(|sig| (sig, msa_12.clone())) + .chain(sigs_345.into_iter().map(|sig| (sig, msa_345.clone()))) + .map(|(sig, msa)| (sig, MultisigApprove::new(msa, approval_hash_to_12345))); - let approvals_at_12: BTreeSet = test_client - .query_single(FindAccountMetadata::new( - msa_12.clone(), - format!("proposals/{approval_hash_to_12345}/approvals") - .parse() - .unwrap(), - )) - .expect("leaf approvals should be initialized by the root proposal") - .try_into_any() - .unwrap(); + // Approve once to see if the proposal expires + let (approver, approve) = approvals_iter.next().unwrap(); + alt_client(approver, &test_client).submit_blocking(approve)?; - assert!(1 == approvals_at_12.len() && approvals_at_12.contains(&msa_12345)); + // Subsequent approvals should succeed unless the proposal is expired + for _ in 2..=4 { + let (approver, approve) = approvals_iter.next().unwrap(); + let res = alt_client(approver, &test_client).submit_blocking(approve.clone()); + match &transaction_ttl_ms_opt { + None => { + res.unwrap(); + } + _ => { + let _err = res.unwrap_err(); + } + } + } // Check that the multisig transaction has not yet executed let _err = test_client - .query_single(FindAccountMetadata::new(msa_012345.clone(), key.clone())) + .query_single(FindAccountMetadata::new( + transaction_target.clone(), + key.clone(), + )) .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 approve = MultisigApprove::new(ms_account.clone(), instructions_hash); - - alt_client(approver, &test_client) - .submit_blocking(approve) - .expect("should successfully approve the proposal"); + // The last approve to proceed to validate and execute the instructions + let (approver, approve) = approvals_iter.next().unwrap(); + let res = alt_client(approver, &test_client).submit_blocking(approve.clone()); + match (&transaction_ttl_ms_opt, &unauthorized_target_opt) { + (None, None) => { + res.unwrap(); } - }; + _ => { + let _err = res.unwrap_err(); + } + } - approve_for_each(sigs_12, approval_hash_to_12345, &msa_12); - approve_for_each(sigs_345, approval_hash_to_12345, &msa_345); + // Check if the multisig transaction has executed + let res = test_client.query_single(FindAccountMetadata::new(transaction_target, key.clone())); + match (&transaction_ttl_ms_opt, &unauthorized_target_opt) { + (None, None) => { + res.unwrap(); + } + _ => { + let _err = res.unwrap_err(); + } + } - // Check that the multisig transaction has executed - test_client - .query_single(FindAccountMetadata::new(msa_012345.clone(), key.clone())) - .expect("instructions should execute with enough approvals"); + // Check if the transaction entries are deleted + for (msa, mst_hash) in [ + (msa_12, approval_hash_to_12345), + (msa_345, approval_hash_to_12345), + (msa_12345, approval_hash_to_012345), + (msa_012345, instructions_hash), + ] { + let res = test_client.query_single(FindAccountMetadata::new( + msa, + format!("multisig/proposals/{mst_hash}").parse().unwrap(), + )); + match (&transaction_ttl_ms_opt, &unauthorized_target_opt) { + (None, Some(_)) => { + // In case the root proposal is failing validation, the relevant entries can exit only by expiring + res.unwrap(); + } + _ => { + let _err = res.unwrap_err(); + } + } + } Ok(()) } #[test] -fn reserved_names() { +fn reserved_roles() { let (network, _rt) = NetworkBuilder::new().start_blocking().unwrap(); let test_client = network.client(); let account_in_another_domain = gen_account_in("garden_of_live_flowers").0; + let register = { + 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 register = { - 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( - "role with this name shouldn't be registered by anyone other than the domain owner", - ); - } + let _err = test_client.submit_blocking(register).expect_err( + "role with this name shouldn't be registered by anyone other than the domain owner", + ); } fn alt_client(signatory: (AccountId, KeyPair), base_client: &Client) -> Client { @@ -445,5 +549,5 @@ fn debug_account(account_id: &AccountId, client: &Client) { .execute_single() .unwrap(); - iroha_logger::error!(?account); + eprintln!("{account:#?}"); } diff --git a/crates/iroha_cli/Cargo.toml b/crates/iroha_cli/Cargo.toml index f92581e2073..f6ba6beeae0 100644 --- a/crates/iroha_cli/Cargo.toml +++ b/crates/iroha_cli/Cargo.toml @@ -38,8 +38,10 @@ humantime = { workspace = true } json5 = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +serde_with = { workspace = true } erased-serde = "0.4.5" supports-color = { workspace = true } +derive_more = { workspace = true } tokio = { workspace = true, features = ["rt"] } futures = { workspace = true } diff --git a/crates/iroha_cli/src/main.rs b/crates/iroha_cli/src/main.rs index 2a523bad29d..2270d10e52b 100644 --- a/crates/iroha_cli/src/main.rs +++ b/crates/iroha_cli/src/main.rs @@ -1260,11 +1260,16 @@ mod json { mod multisig { use std::{ + collections::BTreeMap, io::{BufReader, Read as _}, num::{NonZeroU16, NonZeroU64}, + time::{Duration, SystemTime}, }; + use derive_more::{Constructor, Display}; use iroha::executor_data_model::isi::multisig::*; + use serde::Serialize; + use serde_with::{serde_as, DisplayFromStr, SerializeDisplay}; use super::*; @@ -1318,14 +1323,16 @@ mod multisig { } let register_multisig_account = MultisigRegister::new( self.account, - self.signatories.into_iter().zip(self.weights).collect(), - NonZeroU16::new(self.quorum).expect("quorum should not be 0"), - self.transaction_ttl - .as_millis() - .try_into() - .ok() - .and_then(NonZeroU64::new) - .expect("ttl should be between 1 ms and 584942417 years"), + MultisigSpec::new( + self.signatories.into_iter().zip(self.weights).collect(), + NonZeroU16::new(self.quorum).expect("quorum should not be 0"), + self.transaction_ttl + .as_millis() + .try_into() + .ok() + .and_then(NonZeroU64::new) + .expect("ttl should be between 1 ms and 584942417 years"), + ), ); submit([register_multisig_account], Metadata::default(), context) @@ -1339,6 +1346,9 @@ mod multisig { /// Multisig authority of the multisig transaction #[arg(short, long)] pub account: AccountId, + /// Time-to-live of multisig transactions that overrides to shorten the account default + #[arg(short, long)] + pub transaction_ttl: Option, } impl RunArgs for Propose { @@ -1350,9 +1360,20 @@ mod multisig { let string_content = String::from_utf8(raw_content)?; json5::from_str(&string_content)? }; + let transaction_ttl_ms = self.transaction_ttl.map(|duration| { + duration + .as_millis() + .try_into() + .ok() + .and_then(NonZeroU64::new) + .expect("ttl should be between 1 ms and 584942417 years") + }); + let instructions_hash = HashOf::new(&instructions); println!("{instructions_hash}"); - let propose_multisig_transaction = MultisigPropose::new(self.account, instructions); + + let propose_multisig_transaction = + MultisigPropose::new(self.account, instructions, transaction_ttl_ms); submit([propose_multisig_transaction], Metadata::default(), context) .wrap_err("Failed to propose transaction") @@ -1367,7 +1388,7 @@ mod multisig { pub account: AccountId, /// Instructions to approve #[arg(short, long)] - pub instructions_hash: HashOf>, + pub instructions_hash: ProposalKey, } impl RunArgs for Approve { @@ -1391,15 +1412,39 @@ mod multisig { fn run(self, context: &mut dyn RunContext) -> Result<()> { let client = context.client_from_config(); let me = client.account.clone(); + let Ok(my_multisig_roles) = client + .query(FindRolesByAccountId::new(me.clone())) + .filter_with(|role_id| role_id.name.starts_with(MULTISIG_SIGNATORY)) + .execute_all() + else { + return Ok(()); + }; + let mut stack = my_multisig_roles + .iter() + .filter_map(multisig_account_from) + .map(|account_id| Context::new(me.clone(), account_id, None)) + .collect(); + let mut proposals = BTreeMap::new(); + + fold_proposals(&mut proposals, &mut stack, &client)?; + context.print_data(&proposals)?; - trace_back_from(me, &client, context) + Ok(()) } } const DELIMITER: char = '/'; - const PROPOSALS: &str = "proposals"; + const MULTISIG: &str = "multisig"; const MULTISIG_SIGNATORY: &str = "MULTISIG_SIGNATORY"; + fn spec_key() -> Name { + format!("{MULTISIG}{DELIMITER}spec").parse().unwrap() + } + + fn proposal_key_prefix() -> String { + format!("{MULTISIG}{DELIMITER}proposals{DELIMITER}") + } + fn multisig_account_from(role: &RoleId) -> Option { role.name() .as_ref() @@ -1412,51 +1457,144 @@ mod multisig { }) } - /// Recursively trace back to the root multisig account - fn trace_back_from( - account: AccountId, - client: &Client, - context: &mut dyn RunContext, - ) -> Result<()> { - let Ok(multisig_roles) = client - .query(FindRolesByAccountId::new(account)) - .filter_with(|role_id| role_id.name.starts_with(MULTISIG_SIGNATORY)) - .execute_all() - else { - return Ok(()); - }; + type PendingProposals = BTreeMap; - for role_id in multisig_roles { - let super_account_id: AccountId = multisig_account_from(&role_id).unwrap(); + type ProposalKey = HashOf>; - trace_back_from(super_account_id.clone(), client, context)?; + #[serde_as] + #[derive(Debug, Serialize, Constructor)] + struct ProposalStatus { + instructions: Vec, + #[serde_as(as = "DisplayFromStr")] + proposed_at: humantime::Timestamp, + #[serde_as(as = "DisplayFromStr")] + expires_in: humantime::Duration, + approval_path: Vec, + } - context.print_data(&super_account_id)?; + impl Default for ProposalStatus { + fn default() -> Self { + Self::new( + Vec::new(), + SystemTime::UNIX_EPOCH.into(), + Duration::ZERO.into(), + Vec::new(), + ) + } + } - let super_account = client - .query(FindAccounts) - .filter_with(|account| account.id.eq(super_account_id)) - .execute_single()?; - let proposal_kvs = super_account - .metadata() - .iter() - .filter(|kv| kv.0.as_ref().starts_with(PROPOSALS)); + #[derive(Debug, SerializeDisplay, Display, Constructor)] + #[display(fmt = "{weight} {} [{got}/{quorum}] {target}", "self.relation()")] + struct ApprovalEdge { + weight: u8, + has_approved: bool, + got: u16, + quorum: u16, + target: AccountId, + } - proposal_kvs.fold("", |acc, (k, v)| { - let mut path = k.as_ref().split('/'); - let hash = path.nth(1).unwrap(); + impl ApprovalEdge { + fn relation(&self) -> &str { + if self.has_approved { + "joined" + } else { + "->" + } + } + } - if acc != hash { - context.print_data(&hash).unwrap(); - } - path.for_each(|seg| context.print_data(&seg).unwrap()); - context.print_data(&v).unwrap(); + #[derive(Debug, Constructor)] + struct Context { + child: AccountId, + this: AccountId, + key_span: Option<(ProposalKey, ProposalKey)>, + } - hash - }); + fn fold_proposals( + proposals: &mut PendingProposals, + stack: &mut Vec, + client: &Client, + ) -> Result<()> { + let Some(context) = stack.pop() else { + return Ok(()); + }; + let account = client + .query(FindAccounts) + .filter_with(|account| account.id.eq(context.this.clone())) + .execute_single()?; + let spec: MultisigSpec = account + .metadata() + .get(&spec_key()) + .unwrap() + .try_into_any()?; + for (proposal_key, proposal_value) in account + .metadata() + .iter() + .filter_map(|(k, v)| { + k.as_ref().strip_prefix(&proposal_key_prefix()).map(|k| { + ( + k.parse::().unwrap(), + v.try_into_any::().unwrap(), + ) + }) + }) + .filter(|(k, _v)| context.key_span.map_or(true, |(_, top)| *k == top)) + { + let mut is_root_proposal = true; + for instruction in &proposal_value.instructions { + let InstructionBox::Custom(instruction) = instruction else { + continue; + }; + let Ok(MultisigInstructionBox::Approve(approve)) = instruction.payload().try_into() + else { + continue; + }; + is_root_proposal = false; + let leaf = context.key_span.map_or(proposal_key, |(leaf, _)| leaf); + let top = approve.instructions_hash; + stack.push(Context::new( + context.this.clone(), + approve.account, + Some((leaf, top)), + )); + } + let proposal_status = match context.key_span { + None => proposals.entry(proposal_key).or_default(), + Some((leaf, _)) => proposals.get_mut(&leaf).unwrap(), + }; + let edge = ApprovalEdge::new( + *spec.signatories.get(&context.child).unwrap(), + proposal_value.approvals.contains(&context.child), + spec.signatories + .iter() + .filter(|(id, _)| proposal_value.approvals.contains(id)) + .map(|(_, weight)| u16::from(*weight)) + .sum(), + spec.quorum.into(), + context.this.clone(), + ); + proposal_status.approval_path.push(edge); + if is_root_proposal { + proposal_status.instructions = proposal_value.instructions; + proposal_status.proposed_at = { + let proposed_at = Duration::from_secs( + Duration::from_millis(proposal_value.proposed_at_ms.into()).as_secs(), + ); + SystemTime::UNIX_EPOCH + .checked_add(proposed_at) + .unwrap() + .into() + }; + proposal_status.expires_in = { + let now = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap(); + let expires_at = Duration::from_millis(proposal_value.expires_at_ms.into()); + Duration::from_secs(expires_at.saturating_sub(now).as_secs()).into() + }; + } } - - Ok(()) + fold_proposals(proposals, stack, client) } } diff --git a/crates/iroha_executor/src/default/isi/multisig/account.rs b/crates/iroha_executor/src/default/isi/multisig/account.rs index 1b27223b93f..13f9b2b6a68 100644 --- a/crates/iroha_executor/src/default/isi/multisig/account.rs +++ b/crates/iroha_executor/src/default/isi/multisig/account.rs @@ -6,13 +6,19 @@ impl VisitExecute for MultisigRegister { fn visit(&self, _executor: &mut V) {} fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { - let multisig_account = self.account; + let (multisig_account, spec) = self.into(); let multisig_role = multisig_role_for(&multisig_account); + let metadata = { + let mut res = Metadata::default(); + res.insert(spec_key(), Json::new(&spec)); + res + }; // 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())))); + // TODO Relax the requirement to just being one of the signatories? But impose a proposal and approval procedure? + visit_seq!(executor.visit_register_account(&Register::account( + Account::new(multisig_account.clone()).with_metadata(metadata) + ))); let domain_owner = executor .host() @@ -27,30 +33,12 @@ impl VisitExecute for MultisigRegister { // Just having permission to register accounts is insufficient to register multisig roles executor.context_mut().authority = domain_owner.clone(); - visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( - multisig_account.clone(), - SIGNATORIES.parse().unwrap(), - Json::new(&self.signatories), - ))); - - visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( - multisig_account.clone(), - QUORUM.parse().unwrap(), - Json::new(self.quorum), - ))); - - visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( - multisig_account.clone(), - TRANSACTION_TTL_MS.parse().unwrap(), - Json::new(self.transaction_ttl_ms), - ))); - 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() { + for signatory in spec.signatories.keys().cloned() { visit_seq!(executor .visit_grant_account_role(&Grant::account_role(multisig_role.clone(), signatory))); } diff --git a/crates/iroha_executor/src/default/isi/multisig/mod.rs b/crates/iroha_executor/src/default/isi/multisig/mod.rs index 6ed09717677..58a7c84b37a 100644 --- a/crates/iroha_executor/src/default/isi/multisig/mod.rs +++ b/crates/iroha_executor/src/default/isi/multisig/mod.rs @@ -17,26 +17,15 @@ 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: &str = "multisig"; 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 spec_key() -> Name { + format!("{MULTISIG}{DELIMITER}spec").parse().unwrap() } -fn approvals_key(hash: &HashOf>) -> Name { - format!("{PROPOSALS}{DELIMITER}{hash}{DELIMITER}approvals") +fn proposal_key(hash: &HashOf>) -> Name { + format!("{MULTISIG}{DELIMITER}proposals{DELIMITER}{hash}") .parse() .unwrap() } diff --git a/crates/iroha_executor/src/default/isi/multisig/transaction.rs b/crates/iroha_executor/src/default/isi/multisig/transaction.rs index 542fcc1636b..bd93abc356e 100644 --- a/crates/iroha_executor/src/default/isi/multisig/transaction.rs +++ b/crates/iroha_executor/src/default/isi/multisig/transaction.rs @@ -1,32 +1,40 @@ //! Validation and execution logic of instructions for multisig transactions -use alloc::collections::{btree_map::BTreeMap, btree_set::BTreeSet}; +use alloc::{collections::btree_set::BTreeSet, vec}; +use core::num::NonZeroU64; + +use iroha_smart_contract::data_model::query::error::QueryExecutionFail; use super::*; +use crate::data_model::Level; impl VisitExecute for MultisigPropose { fn visit(&self, executor: &mut V) { + let host = executor.host(); let proposer = executor.context().authority.clone(); let multisig_account = self.account.clone(); - let host = executor.host(); let instructions_hash = HashOf::new(&self.instructions); - let multisig_role = multisig_role_for(&multisig_account); + let multisig_spec = match multisig_spec(multisig_account.clone(), executor) { + Ok(spec) => spec, + Err(err) => deny!(executor, err), + }; 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(&multisig_account) - }); + .query(FindRolesByAccountId::new(multisig_account.clone())) + .filter_with(|role_id| role_id.eq(multisig_role_for(&proposer))) + .execute_single() + .is_ok(); let has_multisig_role = host .query(FindRolesByAccountId::new(proposer)) - .filter_with(|role_id| role_id.eq(multisig_role)) + .filter_with(|role_id| role_id.eq(multisig_role_for(&multisig_account))) .execute_single() .is_ok(); + let has_not_longer_ttl = self.transaction_ttl_ms.map_or(true, |override_ttl_ms| { + override_ttl_ms <= multisig_spec.transaction_ttl_ms + }); + + if !(is_downward_proposal || has_not_longer_ttl) { + deny!(executor, "ttl violates the restriction"); + }; if !(is_downward_proposal || has_multisig_role) { deny!(executor, "not qualified to propose multisig"); @@ -34,8 +42,8 @@ impl VisitExecute for MultisigPropose { if host .query_single(FindAccountMetadata::new( - multisig_account.clone(), - approvals_key(&instructions_hash), + multisig_account, + proposal_key(&instructions_hash), )) .is_ok() { @@ -46,204 +54,280 @@ impl VisitExecute for MultisigPropose { fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { let proposer = executor.context().authority.clone(); let multisig_account = self.account; - - // Authorize as the multisig account - executor.context_mut().authority = multisig_account.clone(); - let instructions_hash = HashOf::new(&self.instructions); - let signatories: BTreeMap = executor - .host() - .query_single(FindAccountMetadata::new( - multisig_account.clone(), - SIGNATORIES.parse().unwrap(), - )) - .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]); + let spec = multisig_spec(multisig_account.clone(), executor)?; + + let now_ms = now_ms(executor); + let expires_at_ms = { + let ttl_ms = self.transaction_ttl_ms.unwrap_or(spec.transaction_ttl_ms); + now_ms.saturating_add(ttl_ms.into()) + }; + let proposal_value = MultisigProposalValue::new( + self.instructions, + now_ms, + expires_at_ms, + BTreeSet::from([proposer]), + None, + ); + let relay_value = |relay: MultisigApprove| { + MultisigProposalValue::new( + vec![relay.into()], + now_ms, + expires_at_ms, + BTreeSet::new(), + Some(false), + ) + }; + let approve_me = MultisigApprove::new(multisig_account.clone(), instructions_hash); // Recursively deploy multisig authentication down to the personal leaf signatories - for signatory in signatories.keys().cloned() { - let is_multisig_again = executor - .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(multisig_account.clone(), instructions_hash); - - MultisigPropose::new(signatory, [approve_me.into()].to_vec()) - }; - - propose_to_approve_me.visit_execute(executor); + for signatory in spec.signatories.keys().cloned() { + if is_multisig(&signatory, executor) { + deploy_relayer(signatory, approve_me.clone(), relay_value, executor)?; } } - visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( - multisig_account.clone(), - instructions_key(&instructions_hash).clone(), - Json::new(&self.instructions), - ))); - - visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( - multisig_account.clone(), - proposed_at_ms_key(&instructions_hash).clone(), - Json::new(now_ms), - ))); + // Authorize as the multisig account + executor.context_mut().authority = multisig_account.clone(); visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( multisig_account, - approvals_key(&instructions_hash).clone(), - Json::new(&approvals), + proposal_key(&instructions_hash), + Json::new(&proposal_value), ))); Ok(()) } } +fn deploy_relayer( + relayer: AccountId, + relay: MultisigApprove, + relay_value: impl Fn(MultisigApprove) -> MultisigProposalValue + Clone, + executor: &mut V, +) -> Result<(), ValidationFail> { + let spec = multisig_spec(relayer.clone(), executor)?; + + let relay_hash = HashOf::new(&vec![relay.clone().into()]); + let sub_relay = MultisigApprove::new(relayer.clone(), relay_hash); + + for signatory in spec.signatories.keys().cloned() { + if is_multisig(&signatory, executor) { + deploy_relayer(signatory, sub_relay.clone(), relay_value.clone(), executor)?; + } + } + + // Authorize as the relayer account + executor.context_mut().authority = relayer.clone(); + + visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( + relayer, + proposal_key(&relay_hash), + Json::new(relay_value(relay)), + ))); + + Ok(()) +} + +fn is_multisig(account: &AccountId, executor: &V) -> bool { + executor + .host() + .query(FindRoleIds) + .filter_with(|role_id| role_id.eq(multisig_role_for(account))) + .execute_single_opt() + .dbg_unwrap() + .is_some() +} + +fn multisig_spec( + multisig_account: AccountId, + executor: &V, +) -> Result { + executor + .host() + .query_single(FindAccountMetadata::new(multisig_account, spec_key()))? + .try_into_any() + .map_err(metadata_conversion_error) +} + +fn proposal_value( + multisig_account: AccountId, + instructions_hash: HashOf>, + executor: &V, +) -> Result { + executor + .host() + .query_single(FindAccountMetadata::new( + multisig_account, + proposal_key(&instructions_hash), + ))? + .try_into_any() + .map_err(metadata_conversion_error) +} + +fn now_ms(executor: &V) -> NonZeroU64 { + executor + .context() + .curr_block + .creation_time() + .as_millis() + .try_into() + .ok() + .and_then(NonZeroU64::new) + .dbg_expect("shouldn't overflow within 584942417 years") +} + impl VisitExecute for MultisigApprove { fn visit(&self, executor: &mut V) { let approver = executor.context().authority.clone(); let multisig_account = self.account.clone(); let host = executor.host(); - let multisig_role = multisig_role_for(&multisig_account); + let instructions_hash = self.instructions_hash; if host .query(FindRolesByAccountId::new(approver)) - .filter_with(|role_id| role_id.eq(multisig_role)) + .filter_with(|role_id| role_id.eq(multisig_role_for(&multisig_account))) .execute_single() .is_err() { deny!(executor, "not qualified to approve multisig"); }; + + if let Err(err) = proposal_value(multisig_account, instructions_hash, executor) { + deny!(executor, err) + }; } fn execute(self, executor: &mut V) -> Result<(), ValidationFail> { let approver = executor.context().authority.clone(); let multisig_account = self.account; + let instructions_hash = self.instructions_hash; + // Check if the proposal is expired // Authorize as the multisig account - 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( - multisig_account.clone(), - SIGNATORIES.parse().unwrap(), - )) - .dbg_unwrap() - .try_into_any() - .dbg_unwrap(); - let quorum: u16 = host - .query_single(FindAccountMetadata::new( - multisig_account.clone(), - QUORUM.parse().unwrap(), - )) - .dbg_unwrap() - .try_into_any() - .dbg_unwrap(); - let transaction_ttl_ms: u64 = host - .query_single(FindAccountMetadata::new( - multisig_account.clone(), - TRANSACTION_TTL_MS.parse().unwrap(), - )) - .dbg_unwrap() - .try_into_any() - .dbg_unwrap(); - let instructions: Vec = host - .query_single(FindAccountMetadata::new( - multisig_account.clone(), - instructions_key(&instructions_hash), - ))? - .try_into_any() - .dbg_unwrap(); - let proposed_at_ms: u64 = host - .query_single(FindAccountMetadata::new( - multisig_account.clone(), - proposed_at_ms_key(&instructions_hash), - )) - .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( - multisig_account.clone(), - approvals_key(&instructions_hash), - )) - .dbg_unwrap() - .try_into_any() - .dbg_unwrap(); + prune_expired(multisig_account.clone(), instructions_hash, executor)?; - approvals.insert(approver); + let Ok(mut proposal_value) = + proposal_value(multisig_account.clone(), instructions_hash, executor) + else { + // The proposal is pruned + // Notify that the proposal has expired, while returning Ok for the entry deletion to take effect + let log = Log::new(Level::INFO, format!("multisig proposal expired:\naccount: {multisig_account}\ninstructions hash: {instructions_hash}")); + visit_seq!(executor.visit_log(&log)); + return Ok(()); + }; + if let Some(true) = proposal_value.is_relayed { + // The relaying approval already has executed + return Ok(()); + } + proposal_value.approvals.insert(approver); visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( multisig_account.clone(), - approvals_key(&instructions_hash), - Json::new(&approvals), + proposal_key(&instructions_hash), + Json::new(&proposal_value), ))); - let is_authenticated = quorum - <= signatories + let spec = multisig_spec(multisig_account.clone(), executor)?; + let is_authenticated = u16::from(spec.quorum) + <= spec + .signatories .into_iter() - .filter(|(id, _)| approvals.contains(id)) + .filter(|(id, _)| proposal_value.approvals.contains(id)) .map(|(_, weight)| u16::from(weight)) .sum(); - let is_expired = proposed_at_ms.saturating_add(transaction_ttl_ms) < now_ms; - - if is_authenticated || is_expired { - // Cleanup the transaction entry - visit_seq!( - executor.visit_remove_account_key_value(&RemoveKeyValue::account( - multisig_account.clone(), - approvals_key(&instructions_hash), - )) - ); - - visit_seq!( - executor.visit_remove_account_key_value(&RemoveKeyValue::account( - multisig_account.clone(), - proposed_at_ms_key(&instructions_hash), - )) - ); - - visit_seq!( - executor.visit_remove_account_key_value(&RemoveKeyValue::account( - multisig_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 { - visit_seq!(executor.visit_instruction(&instruction)); + if is_authenticated { + match proposal_value.is_relayed { + None => { + // Cleanup the transaction entry + prune_down(multisig_account, instructions_hash, executor)?; + } + Some(false) => { + // Mark the relaying approval as executed + proposal_value.is_relayed = Some(true); + visit_seq!(executor.visit_set_account_key_value(&SetKeyValue::account( + multisig_account, + proposal_key(&instructions_hash), + proposal_value.clone(), + ))); } + _ => unreachable!(), + } + + for instruction in proposal_value.instructions { + visit_seq!(executor.visit_instruction(&instruction)); } } Ok(()) } } + +/// Remove an expired proposal and relevant entries, switching the executor authority to this multisig account +fn prune_expired( + multisig_account: AccountId, + instructions_hash: HashOf>, + executor: &mut V, +) -> Result<(), ValidationFail> { + let proposal_value = proposal_value(multisig_account.clone(), instructions_hash, executor)?; + + if now_ms(executor) < proposal_value.expires_at_ms { + // Authorize as the multisig account + executor.context_mut().authority = multisig_account.clone(); + return Ok(()); + } + + // Go upstream to the root through approvals + for instruction in proposal_value.instructions { + if let InstructionBox::Custom(instruction) = instruction { + if let Ok(MultisigInstructionBox::Approve(approve)) = instruction.payload().try_into() { + return prune_expired(approve.account, approve.instructions_hash, executor); + } + } + } + + // Go downstream, cleaning up relayers + prune_down(multisig_account, instructions_hash, executor) +} + +/// Remove an proposal and relevant entries, switching the executor authority to this multisig account +fn prune_down( + multisig_account: AccountId, + instructions_hash: HashOf>, + executor: &mut V, +) -> Result<(), ValidationFail> { + let spec = multisig_spec(multisig_account.clone(), executor)?; + + // Authorize as the multisig account + executor.context_mut().authority = multisig_account.clone(); + + visit_seq!( + executor.visit_remove_account_key_value(&RemoveKeyValue::account( + multisig_account.clone(), + proposal_key(&instructions_hash), + )) + ); + + for signatory in spec.signatories.keys().cloned() { + let relay_hash = { + let relay = MultisigApprove::new(multisig_account.clone(), instructions_hash); + HashOf::new(&vec![relay.into()]) + }; + if is_multisig(&signatory, executor) { + prune_down(signatory, relay_hash, executor)? + } + } + + // Restore the authority + executor.context_mut().authority = multisig_account; + + Ok(()) +} + +#[expect(clippy::needless_pass_by_value)] +fn metadata_conversion_error(err: serde_json::Error) -> ValidationFail { + ValidationFail::QueryFailed(QueryExecutionFail::Conversion(format!( + "multisig account metadata malformed:\n{err}" + ))) +} diff --git a/crates/iroha_executor/src/default/mod.rs b/crates/iroha_executor/src/default/mod.rs index 26d354a3f5d..dcdf43afa70 100644 --- a/crates/iroha_executor/src/default/mod.rs +++ b/crates/iroha_executor/src/default/mod.rs @@ -1202,51 +1202,49 @@ pub mod role { } 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); + let Some(multisig_account) = multisig_account_from(role.id()) else { + deny!(executor, "violates multisig role name format") + }; + 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); } - deny!( - executor, - "only the domain owner can register multisig roles" - ) + execute!(executor, grant_role); } - deny!(executor, "violates multisig role name format") + deny!( + executor, + "only the domain owner can register multisig roles" + ) } } for permission in role.inner().permissions() { iroha_smart_contract::log::debug!(&format!("Checking `{permission:?}`")); - if let Ok(any_permission) = AnyPermission::try_from(permission) { - if !executor.context().curr_block.is_genesis() { - if let Err(error) = crate::permission::ValidateGrantRevoke::validate_grant( - &any_permission, - role.grant_to(), - executor.context(), - executor.host(), - ) { - deny!(executor, error); - } - } - - new_role = new_role.add_permission(any_permission); - } else { + let Ok(any_permission) = AnyPermission::try_from(permission) else { deny!( executor, ValidationFail::NotPermitted(format!("{permission:?}: Unknown permission")) ); + }; + if !executor.context().curr_block.is_genesis() { + if let Err(error) = crate::permission::ValidateGrantRevoke::validate_grant( + &any_permission, + role.grant_to(), + executor.context(), + executor.host(), + ) { + deny!(executor, error); + } } + new_role = new_role.add_permission(any_permission); } if executor.context().curr_block.is_genesis() diff --git a/crates/iroha_executor_data_model/Cargo.toml b/crates/iroha_executor_data_model/Cargo.toml index df1d60ab4da..c1656fa8ffe 100644 --- a/crates/iroha_executor_data_model/Cargo.toml +++ b/crates/iroha_executor_data_model/Cargo.toml @@ -16,6 +16,6 @@ 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"] } +derive_more = { 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 255df1df20e..1b48b2b500f 100644 --- a/crates/iroha_executor_data_model/src/isi.rs +++ b/crates/iroha_executor_data_model/src/isi.rs @@ -51,6 +51,7 @@ macro_rules! impl_custom_instruction { /// Types for multisig instructions pub mod multisig { + use alloc::collections::btree_set::BTreeSet; use core::num::{NonZeroU16, NonZeroU64}; use super::*; @@ -78,12 +79,8 @@ pub mod multisig { // 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: NonZeroU16, - /// Multisig transaction time-to-live in milliseconds based on block timestamps. Defaults to [`DEFAULT_MULTISIG_TTL_MS`] - pub transaction_ttl_ms: NonZeroU64, + /// Specification of the multisig account + pub spec: MultisigSpec, } /// Relative weight of responsibility for the multisig account. @@ -100,6 +97,8 @@ pub mod multisig { pub account: AccountId, /// Proposal contents pub instructions: Vec, + /// Optional TTL to override the account default. Cannot be longer than the account default + pub transaction_ttl_ms: Option, } /// Approve a certain multisig transaction @@ -115,4 +114,64 @@ pub mod multisig { MultisigInstructionBox, MultisigRegister | MultisigPropose | MultisigApprove ); + + /// Metadata value for a multisig account specification + #[derive(Debug, Clone, Serialize, Deserialize, IntoSchema, Constructor)] + pub struct MultisigSpec { + /// 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: NonZeroU16, + /// Multisig transaction time-to-live in milliseconds based on block timestamps. Defaults to [`DEFAULT_MULTISIG_TTL_MS`] + pub transaction_ttl_ms: NonZeroU64, + } + + /// Metadata value for a multisig transaction proposal + #[derive(Debug, Clone, Serialize, Deserialize, IntoSchema, Constructor)] + pub struct MultisigProposalValue { + /// Proposal contents + pub instructions: Vec, + /// Time in milliseconds at which the proposal was made + pub proposed_at_ms: NonZeroU64, + /// Time in milliseconds at which the proposal will expire + pub expires_at_ms: NonZeroU64, + /// List of approvers of the proposal so far + pub approvals: BTreeSet, + /// In case this proposal is some relaying approval, indicates if it has executed or not + pub is_relayed: Option, + } + + impl From for Json { + fn from(details: MultisigSpec) -> Self { + Json::new(details) + } + } + + impl TryFrom<&Json> for MultisigSpec { + type Error = serde_json::Error; + + fn try_from(payload: &Json) -> serde_json::Result { + serde_json::from_str::(payload.as_ref()) + } + } + + impl From for Json { + fn from(details: MultisigProposalValue) -> Self { + Json::new(details) + } + } + + impl TryFrom<&Json> for MultisigProposalValue { + type Error = serde_json::Error; + + fn try_from(payload: &Json) -> serde_json::Result { + serde_json::from_str::(payload.as_ref()) + } + } + + impl From for (AccountId, MultisigSpec) { + fn from(value: MultisigRegister) -> Self { + (value.account, value.spec) + } + } } diff --git a/crates/iroha_schema_gen/src/lib.rs b/crates/iroha_schema_gen/src/lib.rs index ea09d3ce920..41a341c4e3c 100644 --- a/crates/iroha_schema_gen/src/lib.rs +++ b/crates/iroha_schema_gen/src/lib.rs @@ -102,6 +102,9 @@ pub fn build_schemas() -> MetaMap { // Multi-signature operations multisig::MultisigInstructionBox, + // Multi-signature account metadata + multisig::MultisigSpec, + multisig::MultisigProposalValue, // Genesis file - used by SDKs to generate the genesis block // TODO: IMO it could/should be removed from the schema @@ -160,6 +163,7 @@ types!( BTreeMap, BTreeMap, BTreeMap, + BTreeSet, BTreeSet, BTreeSet, BlockEvent, @@ -352,6 +356,7 @@ types!( Option, Option, Option, + Option, Option, Option, Pagination, @@ -600,6 +605,7 @@ types!( [u16; 8], [u8; 4], [u8; 32], + bool, u16, u32, u64, @@ -751,6 +757,8 @@ mod tests { 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); + insert_into_test_map!(iroha_executor_data_model::isi::multisig::MultisigSpec); + insert_into_test_map!(iroha_executor_data_model::isi::multisig::MultisigProposalValue); map } diff --git a/docs/source/references/schema.json b/docs/source/references/schema.json index 78dadd174c0..be7a0cc19d8 100644 --- a/docs/source/references/schema.json +++ b/docs/source/references/schema.json @@ -2980,6 +2980,30 @@ } ] }, + "MultisigProposalValue": { + "Struct": [ + { + "name": "instructions", + "type": "Vec" + }, + { + "name": "proposed_at_ms", + "type": "NonZero" + }, + { + "name": "expires_at_ms", + "type": "NonZero" + }, + { + "name": "approvals", + "type": "SortedVec" + }, + { + "name": "is_relayed", + "type": "Option" + } + ] + }, "MultisigPropose": { "Struct": [ { @@ -2989,6 +3013,10 @@ { "name": "instructions", "type": "Vec" + }, + { + "name": "transaction_ttl_ms", + "type": "Option>" } ] }, @@ -2998,6 +3026,14 @@ "name": "account", "type": "AccountId" }, + { + "name": "spec", + "type": "MultisigSpec" + } + ] + }, + "MultisigSpec": { + "Struct": [ { "name": "signatories", "type": "SortedMap" @@ -3178,6 +3214,9 @@ "Option": { "Option": "TriggerId" }, + "Option": { + "Option": "bool" + }, "Option": { "Option": "u32" }, @@ -4984,6 +5023,9 @@ "value": "TransactionRejectionReason" } }, + "SortedVec": { + "Vec": "AccountId" + }, "SortedVec": { "Vec": "Permission" }, @@ -5920,6 +5962,7 @@ ] }, "WasmSmartContract": "Vec", + "bool": "bool", "u16": { "Int": "FixedWidth" }, diff --git a/scripts/tests/multisig.recursion.sh b/scripts/tests/multisig.recursion.sh index 1561ca2cf3e..953c855c1de 100644 --- a/scripts/tests/multisig.recursion.sh +++ b/scripts/tests/multisig.recursion.sh @@ -46,7 +46,7 @@ WEIGHTS=($(yes 1 | head -n $N_SIGNATORIES)) # register a multisig account, namely msa12 MSA_12=$(gen_account_id "msa12") SIGS_12=(${SIGNATORIES[@]:1:2}) -./iroha multisig register --account $MSA_12 --signatories ${SIGS_12[*]} --weights 1 1 --quorum 2 +./iroha multisig register --account $MSA_12 --signatories ${SIGS_12[*]} --weights 1 1 --quorum 1 # register a multisig account, namely msa345 MSA_345=$(gen_account_id "msa345") @@ -56,7 +56,7 @@ SIGS_345=(${SIGNATORIES[@]:3:3}) # register a multisig account, namely msa12345 MSA_12345=$(gen_account_id "msa12345") SIGS_12345=($MSA_12 $MSA_345) -./iroha multisig register --account $MSA_12345 --signatories ${SIGS_12345[*]} --weights 1 1 --quorum 1 +./iroha multisig register --account $MSA_12345 --signatories ${SIGS_12345[*]} --weights 1 1 --quorum 2 # register a multisig account, namely msa012345 MSA_012345=$(gen_account_id "msa") @@ -65,20 +65,54 @@ SIGS_012345=(${SIGNATORIES[0]} $MSA_12345) # propose a multisig transaction INSTRUCTIONS="../scripts/tests/instructions.json" -propose_stdout=($(cat $INSTRUCTIONS | ./iroha --config "client.0.toml" multisig propose --account $MSA_012345)) -INSTRUCTIONS_HASH=${propose_stdout[0]} +cat $INSTRUCTIONS | ./iroha --config "client.0.toml" multisig propose --account $MSA_012345 + +get_list_as_signatory() { + ./iroha --config "client.$1.toml" multisig list all +} + +get_target_account() { + ./iroha account list filter '{"Atom": {"Id": {"Atom": {"Equals": "'$MSA_012345'"}}}}' +} + +# check that the root proposal is entered +LIST_0_INIT=$(get_list_as_signatory 0) +echo "$LIST_0_INIT" | jq '.[].instructions' | diff - <(cat $INSTRUCTIONS) # check that one of the leaf signatories is involved -LIST=$(./iroha --config "client.5.toml" multisig list all) -echo "$LIST" | grep $INSTRUCTIONS_HASH +LIST_2_INIT=$(get_list_as_signatory 2) +echo "$LIST_2_INIT" | jq '.[].instructions' | diff - <(cat $INSTRUCTIONS) + +LIST_5_INIT=$(get_list_as_signatory 5) +echo "$LIST_5_INIT" | jq '.[].instructions' | diff - <(cat $INSTRUCTIONS) -# approve the multisig transaction -HASH_TO_12345=$(echo "$LIST" | grep -A1 $MSA_345 | tail -n 1 | tr -d '"') +# check that the multisig transaction has not yet executed +TARGET_ACCOUNT_INIT=$(get_target_account) +# NOTE: without ` || false` this line passes even if `success_marker` exists +! echo "$TARGET_ACCOUNT_INIT" | jq -e '.[0].metadata.success_marker' || false + +# approve a relaying approval +HASH_TO_12345=$(echo "$LIST_2_INIT" | jq -r 'keys[0]') +./iroha --config "client.2.toml" multisig approve --account $MSA_12 --instructions-hash $HASH_TO_12345 + +# check that the relaying approval has passed but the whole entry stays in the list +LIST_2_RELAYED=$(get_list_as_signatory 2) +echo "$LIST_2_RELAYED" | jq '.[].instructions' | diff - <(cat $INSTRUCTIONS) + +# give the last approval to execute ./iroha --config "client.5.toml" multisig approve --account $MSA_345 --instructions-hash $HASH_TO_12345 -# check that the multisig transaction is executed -./iroha account list all | grep "congratulations" -! ./iroha --config "client.5.toml" multisig list all | grep $INSTRUCTIONS_HASH +# check that the transaction entry is deleted when seen from the last approver +LIST_5_EXECUTED=$(get_list_as_signatory 5) +! echo "$LIST_5_EXECUTED" | jq -e '.[].instructions' || false + +# check that the transaction entry is deleted when seen from another signatory +LIST_2_EXECUTED=$(get_list_as_signatory 2) +! echo "$LIST_2_EXECUTED" | jq -e '.[].instructions' || false + +# check that the multisig transaction has executed +TARGET_ACCOUNT_EXECUTED=$(get_target_account) +echo "$TARGET_ACCOUNT_EXECUTED" | jq -e '.[0].metadata.success_marker' cd - scripts/test_env.py cleanup diff --git a/scripts/tests/multisig.sh b/scripts/tests/multisig.sh index c3bfa298d86..63ca0ddb811 100644 --- a/scripts/tests/multisig.sh +++ b/scripts/tests/multisig.sh @@ -47,20 +47,38 @@ TRANSACTION_TTL="1y 6M 2w 3d 12h 30m 30s 500ms" # propose a multisig transaction INSTRUCTIONS="../scripts/tests/instructions.json" -propose_stdout=($(cat $INSTRUCTIONS | ./iroha --config "client.1.toml" multisig propose --account $MULTISIG_ACCOUNT)) -INSTRUCTIONS_HASH=${propose_stdout[0]} +cat $INSTRUCTIONS | ./iroha --config "client.1.toml" multisig propose --account $MULTISIG_ACCOUNT -# check that 2nd signatory is involved -./iroha --config "client.2.toml" multisig list all | grep $INSTRUCTIONS_HASH +get_list_as_signatory() { + ./iroha --config "client.$1.toml" multisig list all +} + +get_target_account() { + ./iroha account list filter '{"Atom": {"Id": {"Atom": {"Equals": "'$MULTISIG_ACCOUNT'"}}}}' +} + +# check that the 2nd signatory is involved +LIST_2_INIT=$(get_list_as_signatory 2) +echo "$LIST_2_INIT" | jq '.[].instructions' | diff - <(cat $INSTRUCTIONS) + +# check that the multisig transaction has not yet executed +TARGET_ACCOUNT_INIT=$(get_target_account) +# NOTE: without ` || false` this line passes even if `success_marker` exists +! echo "$TARGET_ACCOUNT_INIT" | jq -e '.[0].metadata.success_marker' || false # approve the multisig transaction +INSTRUCTIONS_HASH=$(echo "$LIST_2_INIT" | jq -r 'keys[0]') for i in $(seq 2 $N_SIGNATORIES); do ./iroha --config "client.$i.toml" multisig approve --account $MULTISIG_ACCOUNT --instructions-hash $INSTRUCTIONS_HASH done -# check that the multisig transaction is executed -./iroha account list all | grep "congratulations" -! ./iroha --config "client.2.toml" multisig list all | grep $INSTRUCTIONS_HASH +# check that the transaction entry is deleted +LIST_2_EXECUTED=$(get_list_as_signatory 2) +! echo "$LIST_2_EXECUTED" | jq -e '.[].instructions' || false + +# check that the multisig transaction has executed +TARGET_ACCOUNT_EXECUTED=$(get_target_account) +echo "$TARGET_ACCOUNT_EXECUTED" | jq -e '.[0].metadata.success_marker' cd - scripts/test_env.py cleanup