From d72a0df5e5a6c38c34c48706c0c316fe805cd21f Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Mon, 27 May 2024 16:52:53 +0900 Subject: [PATCH] feat!: recognize and activate accounts BREAKING_CHANGE: - `Account` contains `is_active` field - `AccountEvent::Created` divides into `Recognized` and `Activated` - `ValidationFail` contains `UnrecognizedAuthority` and `InactiveAuthority` variants Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- .../integration/account_registration/mod.rs | 2 + .../account_registration/private.rs | 90 ++++++++++++++++ .../account_registration/public.rs | 102 ++++++++++++++++++ client/tests/integration/mod.rs | 1 + .../integration/smartcontracts/Cargo.toml | 1 + .../trigger_activate_account/Cargo.toml | 18 ++++ .../trigger_activate_account/src/lib.rs | 30 ++++++ .../integration/triggers/data_trigger.rs | 2 +- core/benches/validation.rs | 3 +- core/src/block.rs | 12 ++- core/src/executor.rs | 40 ++++++- core/src/queue.rs | 9 +- core/src/smartcontracts/isi/account.rs | 42 ++++---- core/src/smartcontracts/isi/asset.rs | 86 +++++++++------ core/src/smartcontracts/isi/domain.rs | 46 ++++++-- core/src/smartcontracts/isi/mod.rs | 24 ++++- core/src/smartcontracts/isi/query.rs | 18 ++-- core/src/smartcontracts/wasm.rs | 3 +- core/src/sumeragi/main_loop.rs | 3 +- data_model/src/account.rs | 9 ++ data_model/src/events/data/events.rs | 6 +- data_model/src/events/data/filters.rs | 8 +- data_model/src/executor.rs | 2 +- data_model/src/lib.rs | 6 ++ docs/source/references/schema.json | 59 +++++----- torii/src/lib.rs | 1 + 26 files changed, 495 insertions(+), 128 deletions(-) create mode 100644 client/tests/integration/account_registration/mod.rs create mode 100644 client/tests/integration/account_registration/private.rs create mode 100644 client/tests/integration/account_registration/public.rs create mode 100644 client/tests/integration/smartcontracts/trigger_activate_account/Cargo.toml create mode 100644 client/tests/integration/smartcontracts/trigger_activate_account/src/lib.rs diff --git a/client/tests/integration/account_registration/mod.rs b/client/tests/integration/account_registration/mod.rs new file mode 100644 index 00000000000..c97008bdafd --- /dev/null +++ b/client/tests/integration/account_registration/mod.rs @@ -0,0 +1,2 @@ +mod private; +mod public; diff --git a/client/tests/integration/account_registration/private.rs b/client/tests/integration/account_registration/private.rs new file mode 100644 index 00000000000..1879ce73ed3 --- /dev/null +++ b/client/tests/integration/account_registration/private.rs @@ -0,0 +1,90 @@ +//! In private use cases, a new account should be: +//! - recognized when targeted as (a part of) a destination (or an object) of a creative instruction +//! - becomes able to hold assets, permissions, roles, and metadata +//! - cannot yet execute any queries or transactions +//! - activated when manually registered by an administrative account +//! - becomes able to execute queries and transactions + +use iroha::data_model::prelude::*; +use test_network::*; +use test_samples::{gen_account_in, ALICE_ID}; + +/// A new account e.g. "carol" should be: +/// - recognized when targeted as a destination of a transfer of numeric asset e.g. "rose" +/// - activated when manually registered by an administrative account e.g. "alice" +/// +/// # Scenario +/// +/// 0. new carol targeted ... carol recognized +/// 0. carol tries query ... err +/// 0. carol tries transaction ... err +/// 0. register carol ... carol activated +/// 0. carol tries query ... ok +/// 0. carol tries transaction ... ok +#[test] +fn on_transfer_asset_numeric() { + let (_rt, _peer, client_alice) = ::new().with_port(10_820).start_with_runtime(); + wait_for_genesis_committed(&[client_alice.clone()], 0); + let observer = client_alice.clone(); + + // new carol targeted ... carol recognized + let (carol_id, carol_keypair) = gen_account_in("wonderland"); + let _ = observer + .request(FindAccountById::new(carol_id.clone())) + .expect_err("carol should not be recognized at this point"); + let rose: AssetDefinitionId = "rose#wonderland".parse().unwrap(); + let rose_alice = AssetId::new(rose.clone(), ALICE_ID.clone()); + let n_roses_alice = observer + .request(FindAssetQuantityById::new(rose_alice.clone())) + .expect("alice should have roses"); + assert!(numeric!(3) < n_roses_alice); + let transfer = Transfer::asset_numeric(rose_alice, 3_u32, carol_id.clone()); + client_alice + .submit_blocking(transfer) + .expect("alice should succeed to transfer"); + let _ = observer + .request(FindAccountById::new(carol_id.clone())) + .expect("carol should be recognized now"); + let rose_carol = AssetId::new(rose.clone(), carol_id.clone()); + let n_roses_carol = observer + .request(FindAssetQuantityById::new(rose_carol.clone())) + .expect("carol should have roses"); + assert_eq!(n_roses_carol, numeric!(3)); + + // carol tries query ... err + let client_carol = { + let mut client = client_alice.clone(); + client.key_pair = carol_keypair; + client.account_id = carol_id.clone(); + client + }; + let query = FindAssetQuantityById::new(rose_carol.clone()); + let _ = client_carol + .request(query.clone()) + .expect_err("queries from inactive carol should be rejected"); + + // carol tries transaction ... err + let instruction = Log::new( + iroha_data_model::Level::DEBUG, + "the one least likely to be rejected".to_string(), + ); + let _ = client_carol + .submit_blocking(instruction.clone()) + .expect_err("transactions from inactive carol should be rejected"); + + // register carol ... carol activated + let register = Register::account(Account::new(carol_id.clone())); + client_alice + .submit_blocking(register) + .expect("alice should succeed to register"); + + // carol tries query ... ok + let _ = client_carol + .request(query) + .expect("queries from active carol should be accepted"); + + // carol tries transaction ... ok + client_carol + .submit_blocking(instruction) + .expect("transactions from active carol should be accepted"); +} diff --git a/client/tests/integration/account_registration/public.rs b/client/tests/integration/account_registration/public.rs new file mode 100644 index 00000000000..bdeb65143d3 --- /dev/null +++ b/client/tests/integration/account_registration/public.rs @@ -0,0 +1,102 @@ +//! In public use cases, a new account should be: +//! - recognized when targeted as (a part of) a destination (or an object) of a creative instruction +//! - becomes able to hold assets, permissions, roles, and metadata +//! - cannot yet execute any queries or transactions +//! - automatically activated by an administrative trigger +//! - becomes able to execute queries and transactions + +use eyre::Result; +use iroha::data_model::prelude::*; +use test_network::*; +use test_samples::{gen_account_in, ALICE_ID}; + +/// A new account e.g. "carol" should be: +/// - recognized when targeted as a destination of a transfer of numeric asset e.g. "rose" +/// - automatically activated by an administrative trigger +/// +/// # Scenario +/// +/// 0. ... administrative trigger registered +/// 0. new carol targeted ... carol recognized +/// 0. clock forward by one block ... carol activated +/// 0. carol tries query ... ok +/// 0. carol tries transaction ... ok +#[test] +fn on_transfer_asset_numeric() -> Result<()> { + let (_rt, _peer, client_alice) = ::new().with_port(10_810).start_with_runtime(); + wait_for_genesis_committed(&[client_alice.clone()], 0); + let observer = client_alice.clone(); + + // ... administrative trigger registered + let wasm = iroha_wasm_builder::Builder::new( + "tests/integration/smartcontracts/trigger_activate_account", + ) + .show_output() + .build()? + .optimize()? + .into_bytes()?; + let register_activator = Register::trigger(Trigger::new( + "activator$wonderland".parse().unwrap(), + Action::new( + WasmSmartContract::from_compiled(wasm), + Repeats::Indefinitely, + ALICE_ID.clone(), + AccountEventFilter::new().for_events(AccountEventSet::Recognized), + ), + )); + client_alice + .submit_blocking(register_activator) + .expect("alice should succeed to register activator"); + + // new carol targeted ... carol recognized + let (carol_id, carol_keypair) = gen_account_in("wonderland"); + let _ = observer + .request(FindAccountById::new(carol_id.clone())) + .expect_err("carol should not be recognized at this point"); + let rose: AssetDefinitionId = "rose#wonderland".parse().unwrap(); + let rose_alice = AssetId::new(rose.clone(), ALICE_ID.clone()); + let n_roses_alice = observer + .request(FindAssetQuantityById::new(rose_alice.clone())) + .expect("alice should have roses"); + assert!(numeric!(3) < n_roses_alice); + let transfer = Transfer::asset_numeric(rose_alice, 3_u32, carol_id.clone()); + client_alice + .submit_blocking(transfer) + .expect("alice should succeed to transfer"); + let _ = observer + .request(FindAccountById::new(carol_id.clone())) + .expect("carol should be recognized now"); + let rose_carol = AssetId::new(rose.clone(), carol_id.clone()); + let n_roses_carol = observer + .request(FindAssetQuantityById::new(rose_carol.clone())) + .expect("carol should have roses"); + assert_eq!(n_roses_carol, numeric!(3)); + + // clock forward by one block ... carol activated + let instruction = Log::new( + iroha_data_model::Level::DEBUG, + "the one least likely to be rejected".to_string(), + ); + client_alice + .submit_blocking(instruction.clone()) + .expect("instruction should succeed"); + + // carol tries query ... ok + let client_carol = { + let mut client = client_alice.clone(); + client.key_pair = carol_keypair; + client.account_id = carol_id.clone(); + client + }; + let query = FindAssetQuantityById::new(rose_carol.clone()); + let _ = client_carol + .request(query) + .expect("queries from active carol should be accepted"); + + // carol tries transaction ... ok + client_carol + .submit_blocking(instruction) + .expect("transactions from active carol should be accepted"); + + Ok(()) +} diff --git a/client/tests/integration/mod.rs b/client/tests/integration/mod.rs index 37299969665..10c9ceec6da 100644 --- a/client/tests/integration/mod.rs +++ b/client/tests/integration/mod.rs @@ -1,3 +1,4 @@ +mod account_registration; mod add_domain; mod asset; mod asset_propagation; diff --git a/client/tests/integration/smartcontracts/Cargo.toml b/client/tests/integration/smartcontracts/Cargo.toml index a03698db1b2..0eedfc8b3a2 100644 --- a/client/tests/integration/smartcontracts/Cargo.toml +++ b/client/tests/integration/smartcontracts/Cargo.toml @@ -17,6 +17,7 @@ members = [ "executor_with_migration_fail", "query_assets_and_save_cursor", "smart_contract_can_filter_queries", + "trigger_activate_account", ] [profile.dev] diff --git a/client/tests/integration/smartcontracts/trigger_activate_account/Cargo.toml b/client/tests/integration/smartcontracts/trigger_activate_account/Cargo.toml new file mode 100644 index 00000000000..a2a82226b56 --- /dev/null +++ b/client/tests/integration/smartcontracts/trigger_activate_account/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "trigger_activate_account" + +edition.workspace = true +version.workspace = true +authors.workspace = true + +license.workspace = true + +[lib] +crate-type = ['cdylib'] + +[dependencies] +iroha_trigger.workspace = true + +panic-halt.workspace = true +lol_alloc.workspace = true +getrandom.workspace = true diff --git a/client/tests/integration/smartcontracts/trigger_activate_account/src/lib.rs b/client/tests/integration/smartcontracts/trigger_activate_account/src/lib.rs new file mode 100644 index 00000000000..c4b4b4deb84 --- /dev/null +++ b/client/tests/integration/smartcontracts/trigger_activate_account/src/lib.rs @@ -0,0 +1,30 @@ +//! Trigger responsible for activating newly recognized accounts in "wonderland" + +#![no_std] + +#[cfg(not(test))] +extern crate panic_halt; + +use iroha_trigger::prelude::*; +use lol_alloc::{FreeListAllocator, LockedAllocator}; + +#[global_allocator] +static ALLOC: LockedAllocator = LockedAllocator::new(FreeListAllocator::new()); + +getrandom::register_custom_getrandom!(iroha_trigger::stub_getrandom); + +#[iroha_trigger::main] +fn main(_id: TriggerId, _owner: AccountId, event: EventBox) { + let EventBox::Data(DataEvent::Domain(DomainEvent::Account(AccountEvent::Recognized( + account_id, + )))) = event + else { + return; + }; + + Register::account(Account::new(account_id)) + .execute() + .dbg_expect( + "authority should be alice, and alice should succeed to register accounts in wonderland", + ); +} diff --git a/client/tests/integration/triggers/data_trigger.rs b/client/tests/integration/triggers/data_trigger.rs index 06ce5a2400e..b8b1e794c4b 100644 --- a/client/tests/integration/triggers/data_trigger.rs +++ b/client/tests/integration/triggers/data_trigger.rs @@ -32,7 +32,7 @@ fn must_execute_both_triggers() -> Result<()> { [instruction.clone()], Repeats::Indefinitely, account_id.clone(), - AccountEventFilter::new().for_events(AccountEventSet::Created), + AccountEventFilter::new().for_events(AccountEventSet::Activated), ), )); test_client.submit_blocking(register_trigger)?; diff --git a/core/benches/validation.rs b/core/benches/validation.rs index 7d0fe5d973c..d0dde1fc526 100644 --- a/core/benches/validation.rs +++ b/core/benches/validation.rs @@ -49,7 +49,8 @@ fn build_test_and_transient_state() -> State { { let (account_id, _account_keypair) = gen_account_in(&*STARTER_DOMAIN); let mut domain = Domain::new(STARTER_DOMAIN.clone()).build(&account_id); - let account = Account::new(account_id.clone()).build(&account_id); + let mut account = Account::new(account_id.clone()).build(&account_id); + account.activate(); assert!(domain.add_account(account).is_none()); World::with([domain], UniqueVec::new()) }, diff --git a/core/src/block.rs b/core/src/block.rs index 711ef6431d8..fc5471a41bc 100644 --- a/core/src/block.rs +++ b/core/src/block.rs @@ -821,7 +821,8 @@ mod tests { // Predefined world state let (alice_id, alice_keypair) = gen_account_in("wonderland"); - let account = Account::new(alice_id.clone()).build(&alice_id); + let mut account = Account::new(alice_id.clone()).build(&alice_id); + account.activate(); let domain_id = DomainId::from_str("wonderland").expect("Valid"); let mut domain = Domain::new(domain_id).build(&alice_id); assert!(domain.add_account(account).is_none()); @@ -876,7 +877,8 @@ mod tests { // Predefined world state let (alice_id, alice_keypair) = gen_account_in("wonderland"); - let account = Account::new(alice_id.clone()).build(&alice_id); + let mut account = Account::new(alice_id.clone()).build(&alice_id); + account.activate(); let domain_id = DomainId::from_str("wonderland").expect("Valid"); let mut domain = Domain::new(domain_id).build(&alice_id); assert!(domain.add_account(account).is_none()); @@ -949,7 +951,8 @@ mod tests { // Predefined world state let (alice_id, alice_keypair) = gen_account_in("wonderland"); - let account = Account::new(alice_id.clone()).build(&alice_id); + let mut account = Account::new(alice_id.clone()).build(&alice_id); + account.activate(); let domain_id = DomainId::from_str("wonderland").expect("Valid"); let mut domain = Domain::new(domain_id).build(&alice_id); assert!( @@ -1034,8 +1037,9 @@ mod tests { ); let mut genesis_domain = Domain::new(GENESIS_DOMAIN_ID.clone()).build(&genesis_correct_account_id); - let genesis_wrong_account = + let mut genesis_wrong_account = Account::new(genesis_wrong_account_id.clone()).build(&genesis_wrong_account_id); + genesis_wrong_account.activate(); assert!(genesis_domain.add_account(genesis_wrong_account).is_none(),); let world = World::with([genesis_domain], UniqueVec::new()); let kura = Kura::blank_kura_for_testing(); diff --git a/core/src/executor.rs b/core/src/executor.rs index 50a632e54ec..4850a78c291 100644 --- a/core/src/executor.rs +++ b/core/src/executor.rs @@ -17,7 +17,7 @@ use serde::{ use crate::{ smartcontracts::{wasm, Execute as _}, - state::{deserialize::WasmSeed, StateReadOnly, StateTransaction}, + state::{deserialize::WasmSeed, StateReadOnly, StateTransaction, WorldReadOnly}, }; impl From for ValidationFail { @@ -127,10 +127,12 @@ impl<'de> DeserializeSeed<'de> for WasmSeed<'_, Executor> { } impl Executor { - /// Validate [`SignedTransaction`]. + /// Validate [`SignedTransaction`] coming from clients. /// /// # Errors /// + /// - The authority account is not found + /// - The authority account is not active /// - Failed to prepare runtime for WASM execution; /// - Failed to execute the entrypoint of the WASM blob; /// - Executor denied the operation. @@ -142,6 +144,15 @@ impl Executor { ) -> Result<(), ValidationFail> { trace!("Running transaction validation"); + if 0 != state_transaction.height() { + let Ok(authority_account) = state_transaction.world.account(authority) else { + return Err(ValidationFail::UnrecognizedAuthority); + }; + if !authority_account.is_active { + return Err(ValidationFail::InactiveAuthority); + } + } + match self { Self::Initial => { let (_authority, Executable::Instructions(instructions)) = transaction.into() @@ -170,10 +181,12 @@ impl Executor { } } - /// Validate [`InstructionExpr`]. + /// Validate [`InstructionBox`] coming from Wasm executables. /// /// # Errors /// + /// - The authority account is not found + /// - The authority account is not active /// - Failed to prepare runtime for WASM execution; /// - Failed to execute the entrypoint of the WASM blob; /// - Executor denied the operation. @@ -185,6 +198,15 @@ impl Executor { ) -> Result<(), ValidationFail> { trace!("Running instruction validation"); + if 0 != state_transaction.height() { + let Ok(authority_account) = state_transaction.world.account(authority) else { + return Err(ValidationFail::UnrecognizedAuthority); + }; + if !authority_account.is_active { + return Err(ValidationFail::InactiveAuthority); + } + } + match self { Self::Initial => instruction .execute(authority, state_transaction) @@ -206,10 +228,12 @@ impl Executor { } } - /// Validate [`QueryBox`]. + /// Validate [`QueryBox`] coming from any paths. /// /// # Errors /// + /// - The authority account is not found + /// - The authority account is not active /// - Failed to prepare runtime for WASM execution; /// - Failed to execute the entrypoint of the WASM blob; /// - Executor denied the operation. @@ -221,6 +245,14 @@ impl Executor { ) -> Result<(), ValidationFail> { trace!("Running query validation"); + // assuming the genesis transaction does not include queries + let Ok(authority_account) = state_ro.world().account(authority) else { + return Err(ValidationFail::UnrecognizedAuthority); + }; + if !authority_account.is_active { + return Err(ValidationFail::InactiveAuthority); + } + match self { Self::Initial => Ok(()), Self::UserProvided(UserProvidedExecutor(loaded_executor)) => { diff --git a/core/src/queue.rs b/core/src/queue.rs index 702e2f13233..a499f9b78d3 100644 --- a/core/src/queue.rs +++ b/core/src/queue.rs @@ -448,7 +448,8 @@ pub mod tests { let domain_id = DomainId::from_str("wonderland").expect("Valid"); let (account_id, _account_keypair) = gen_account_in("wonderland"); let mut domain = Domain::new(domain_id).build(&account_id); - let account = Account::new(account_id.clone()).build(&account_id); + let mut account = Account::new(account_id.clone()).build(&account_id); + account.activate(); assert!(domain.add_account(account).is_none()); World::with([domain], PeersIds::new()) } @@ -844,8 +845,10 @@ pub mod tests { let world = { let domain_id = DomainId::from_str("wonderland").expect("Valid"); let mut domain = Domain::new(domain_id).build(&alice_id); - let alice_account = Account::new(alice_id.clone()).build(&alice_id); - let bob_account = Account::new(bob_id.clone()).build(&bob_id); + let mut alice_account = Account::new(alice_id.clone()).build(&alice_id); + alice_account.activate(); + let mut bob_account = Account::new(bob_id.clone()).build(&bob_id); + bob_account.activate(); assert!(domain.add_account(alice_account).is_none()); assert!(domain.add_account(bob_account).is_none()); World::with([domain], PeersIds::new()) diff --git a/core/src/smartcontracts/isi/account.rs b/core/src/smartcontracts/isi/account.rs index 236a3dc017e..3defb7a6a42 100644 --- a/core/src/smartcontracts/isi/account.rs +++ b/core/src/smartcontracts/isi/account.rs @@ -42,10 +42,11 @@ pub mod isi { #[metrics(+"register_asset")] fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { let asset_id = self.object.id; + recognize_account(asset_id.account_id().clone(), authority, state_transaction)?; match state_transaction.world.asset(&asset_id) { Err(err) => match err { @@ -58,7 +59,7 @@ pub mod isi { let asset = state_transaction .world .asset_or_insert(asset_id.clone(), self.object.value) - .expect("Account exists"); + .expect("account should exist"); match asset.value { AssetValue::Numeric(increment) => { @@ -134,7 +135,7 @@ pub mod isi { impl Execute for Transfer { fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { let Transfer { @@ -144,7 +145,7 @@ pub mod isi { } = self; let _ = state_transaction.world.account(&source_id)?; - let _ = state_transaction.world.account(&destination_id)?; + recognize_account(destination_id.clone(), authority, state_transaction)?; let asset_definition = state_transaction.world.asset_definition_mut(&object)?; @@ -153,6 +154,7 @@ pub mod isi { } asset_definition.owned_by = destination_id.clone(); + state_transaction .world .emit_events(Some(AssetDefinitionEvent::OwnerChanged( @@ -170,27 +172,24 @@ pub mod isi { #[metrics(+"set_account_key_value")] fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { let account_id = self.object_id; - let account_metadata_limits = state_transaction.config.account_metadata_limits; + recognize_account(account_id.clone(), authority, state_transaction)?; state_transaction .world .account_mut(&account_id) - .map_err(Error::from) - .and_then(|account| { - account - .metadata - .insert_with_limits( - self.key.clone(), - self.value.clone(), - account_metadata_limits, - ) - .map_err(Error::from) - })?; + .expect("account should exist") + .metadata + .insert_with_limits( + self.key.clone(), + self.value.clone(), + account_metadata_limits, + ) + .map_err(Error::from)?; state_transaction .world @@ -239,15 +238,14 @@ pub mod isi { #[metrics(+"grant_account_permission")] fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { let account_id = self.destination_id; let permission = self.object; let permission_id = permission.id.clone(); - // Check if account exists - state_transaction.world.account_mut(&account_id)?; + recognize_account(account_id.clone(), authority, state_transaction)?; if !state_transaction .world @@ -323,7 +321,7 @@ pub mod isi { #[metrics(+"grant_account_role")] fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { let account_id = self.destination_id; @@ -339,7 +337,7 @@ pub mod isi { .into_iter() .map(|token| token.id); - state_transaction.world.account(&account_id)?; + recognize_account(account_id.clone(), authority, state_transaction)?; if state_transaction .world diff --git a/core/src/smartcontracts/isi/asset.rs b/core/src/smartcontracts/isi/asset.rs index 20752dd167f..634356fc448 100644 --- a/core/src/smartcontracts/isi/asset.rs +++ b/core/src/smartcontracts/isi/asset.rs @@ -41,7 +41,7 @@ pub mod isi { #[metrics(+"set_asset_key_value")] fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { let asset_id = self.object_id; @@ -62,10 +62,12 @@ pub mod isi { .increase_asset_total_amount(&asset_id.definition_id, Numeric::ONE)?; } + recognize_account(asset_id.account_id().clone(), authority, state_transaction)?; let asset_metadata_limits = state_transaction.config.asset_metadata_limits; let asset = state_transaction .world - .asset_or_insert(asset_id.clone(), Metadata::new())?; + .asset_or_insert(asset_id.clone(), Metadata::new()) + .expect("account should exist"); { let AssetValue::Store(store) = &mut asset.value else { @@ -134,38 +136,42 @@ pub mod isi { #[metrics(+"transfer_store")] fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { - let asset_id = self.source_id; + let source_asset_id = self.source_id; assert_asset_type( - &asset_id.definition_id, + &source_asset_id.definition_id, state_transaction, expected_asset_value_type_store, )?; - let asset = state_transaction + let source_asset = state_transaction .world - .account_mut(&asset_id.account_id) + .account_mut(&source_asset_id.account_id) .and_then(|account| { account - .remove_asset(&asset_id.definition_id) - .ok_or_else(|| FindError::Asset(asset_id.clone())) + .remove_asset(&source_asset_id.definition_id) + .ok_or_else(|| FindError::Asset(source_asset_id.clone())) })?; - let destination_store = { - let destination_id = - AssetId::new(asset_id.definition_id.clone(), self.destination_id.clone()); - let destination_store_asset = state_transaction + let destination_asset = { + recognize_account(self.destination_id.clone(), authority, state_transaction)?; + let destination_asset_id = AssetId::new( + source_asset_id.definition_id.clone(), + self.destination_id.clone(), + ); + let destination_asset = state_transaction .world - .asset_or_insert(destination_id.clone(), asset.value)?; + .asset_or_insert(destination_asset_id, source_asset.value) + .expect("account should exist"); - destination_store_asset.clone() + destination_asset.clone() }; state_transaction.world.emit_events([ - AssetEvent::Deleted(asset_id), - AssetEvent::Created(destination_store), + AssetEvent::Deleted(source_asset_id), + AssetEvent::Created(destination_asset), ]); Ok(()) @@ -175,7 +181,7 @@ pub mod isi { impl Execute for Mint { fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { let asset_id = self.destination_id; @@ -186,11 +192,13 @@ pub mod isi { expected_asset_value_type_numeric, )?; assert_numeric_spec(&self.object, &asset_definition)?; - assert_can_mint(&asset_definition, state_transaction)?; + + recognize_account(asset_id.account_id().clone(), authority, state_transaction)?; let asset = state_transaction .world - .asset_or_insert(asset_id.clone(), Numeric::ZERO)?; + .asset_or_insert(asset_id.clone(), Numeric::ZERO) + .expect("account should exist"); let AssetValue::Numeric(quantity) = &mut asset.value else { return Err(Error::Conversion("Expected numeric asset type".to_owned())); }; @@ -276,40 +284,48 @@ pub mod isi { impl Execute for Transfer { fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { - let source_id = self.source_id; - let destination_id = - AssetId::new(source_id.definition_id.clone(), self.destination_id.clone()); + let source_asset_id = self.source_id; + let destination_asset_id = AssetId::new( + source_asset_id.definition_id.clone(), + self.destination_id.clone(), + ); let asset_definition = assert_asset_type( - &source_id.definition_id, + &source_asset_id.definition_id, state_transaction, expected_asset_value_type_numeric, )?; assert_numeric_spec(&self.object, &asset_definition)?; { - let account = state_transaction.world.account_mut(&source_id.account_id)?; - let asset = account + let source_account = state_transaction + .world + .account_mut(&source_asset_id.account_id)?; + let source_asset = source_account .assets - .get_mut(&source_id.definition_id) - .ok_or_else(|| FindError::Asset(source_id.clone()))?; - let AssetValue::Numeric(quantity) = &mut asset.value else { + .get_mut(&source_asset_id.definition_id) + .ok_or_else(|| FindError::Asset(source_asset_id.clone()))?; + let AssetValue::Numeric(quantity) = &mut source_asset.value else { return Err(Error::Conversion("Expected numeric asset type".to_owned())); }; *quantity = quantity .checked_sub(self.object) .ok_or(MathError::NotEnoughQuantity)?; - if asset.value.is_zero_value() { - assert!(account.remove_asset(&source_id.definition_id).is_some()); + if source_asset.value.is_zero_value() { + assert!(source_account + .remove_asset(&source_asset_id.definition_id) + .is_some()); } } + recognize_account(self.destination_id.clone(), authority, state_transaction)?; let destination_asset = state_transaction .world - .asset_or_insert(destination_id.clone(), Numeric::ZERO)?; + .asset_or_insert(destination_asset_id.clone(), Numeric::ZERO) + .expect("account should exist"); { let AssetValue::Numeric(quantity) = &mut destination_asset.value else { return Err(Error::Conversion("Expected numeric asset type".to_owned())); @@ -329,11 +345,11 @@ pub mod isi { state_transaction.world.emit_events([ AssetEvent::Removed(AssetChanged { - asset_id: source_id, + asset_id: source_asset_id, amount: self.object.into(), }), AssetEvent::Added(AssetChanged { - asset_id: destination_id, + asset_id: destination_asset_id, amount: self.object.into(), }), ]); diff --git a/core/src/smartcontracts/isi/domain.rs b/core/src/smartcontracts/isi/domain.rs index 15e5a6df06a..4dce24fedea 100644 --- a/core/src/smartcontracts/isi/domain.rs +++ b/core/src/smartcontracts/isi/domain.rs @@ -47,8 +47,7 @@ pub mod isi { authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { - let account: Account = self.object.build(authority); - let account_id = account.id().clone(); + let account_id = self.object.id().clone(); if *account_id.domain_id() == *iroha_genesis::GENESIS_DOMAIN_ID { return Err(InstructionExecutionError::InvariantViolation( @@ -56,19 +55,40 @@ pub mod isi { )); } - let domain = state_transaction.world.domain_mut(&account_id.domain_id)?; - if domain.accounts.contains_key(&account_id) { - return Err(RepetitionError { - instruction_type: InstructionType::Register, - id: IdBox::AccountId(account_id), + match state_transaction.world.account_mut(&account_id) { + Ok(existing_account) => { + if existing_account.is_active { + return Err(RepetitionError { + instruction_type: InstructionType::Register, + id: IdBox::AccountId(account_id), + } + .into()); + } + + existing_account.activate(); } - .into()); - } - domain.add_account(account.clone()); + Err(_account_not_found) => { + let mut account: Account = self.object.build(authority); + account.activate(); + + state_transaction + .world + .domain_mut(&account_id.domain_id)? + .add_account(account); + + state_transaction + .world + .emit_events(Some(DomainEvent::Account(AccountEvent::Recognized( + account_id.clone(), + )))); + } + }; state_transaction .world - .emit_events(Some(DomainEvent::Account(AccountEvent::Created(account)))); + .emit_events(Some(DomainEvent::Account(AccountEvent::Activated( + account_id, + )))); Ok(()) } @@ -358,6 +378,9 @@ pub mod isi { } = self; let _ = state_transaction.world.account(&source_id)?; + // Exceptionally, the destination account should not be recognized here. + // Otherwise, the risk is that the previous owner can no longer activate the current owner who cannot yet take any action by oneself. + // Thus, the destination account should be explicitly registered before this transfer let _ = state_transaction.world.account(&destination_id)?; let domain = state_transaction.world.domain_mut(&object)?; @@ -367,6 +390,7 @@ pub mod isi { } domain.owned_by = destination_id.clone(); + state_transaction .world .emit_events(Some(DomainEvent::OwnerChanged(DomainOwnerChanged { diff --git a/core/src/smartcontracts/isi/mod.rs b/core/src/smartcontracts/isi/mod.rs index 659f778ecea..a7d322f3711 100644 --- a/core/src/smartcontracts/isi/mod.rs +++ b/core/src/smartcontracts/isi/mod.rs @@ -1,5 +1,5 @@ //! This module contains enumeration of all possible Iroha Special -//! Instructions [`InstructionExpr`], generic instruction types and related +//! Instructions [`InstructionBox`], generic instruction types and related //! implementations. pub mod account; pub mod asset; @@ -230,6 +230,28 @@ impl Execute for RevokeBox { } } +fn recognize_account( + id: AccountId, + authority: &AccountId, + state_transaction: &mut StateTransaction<'_, '_>, +) -> Result<(), Error> { + if state_transaction.world.account(&id).is_ok() { + return Ok(()); + } + + let account = Account::new(id.clone()).build(authority); + state_transaction + .world + .domain_mut(&id.domain_id)? + .add_account(account); + + state_transaction + .world + .emit_events(Some(DomainEvent::Account(AccountEvent::Recognized(id)))); + + Ok(()) +} + pub mod prelude { //! Re-export important traits and types for glob import `(::*)` pub use super::*; diff --git a/core/src/smartcontracts/isi/query.rs b/core/src/smartcontracts/isi/query.rs index 29fd34c6c73..42d3fdca7f7 100644 --- a/core/src/smartcontracts/isi/query.rs +++ b/core/src/smartcontracts/isi/query.rs @@ -172,12 +172,12 @@ impl_lazy! { pub struct ValidQueryRequest(SignedQuery); impl ValidQueryRequest { - /// Validate query. + /// Validate [`SignedQuery`] coming from clients. /// /// # Errors - /// - Account doesn't exist - /// - Account doesn't have the correct public key - /// - Account has incorrect permissions + /// + /// - Signatory does not fill the authority + /// - Internal [`validate_query`](crate::executor::Executor::validate_query) fails pub fn validate( query: SignedQuery, state_ro: &impl StateReadOnly, @@ -317,7 +317,8 @@ mod tests { fn world_with_test_domains() -> World { let domain_id = DomainId::from_str("wonderland").expect("Valid"); let mut domain = Domain::new(domain_id).build(&ALICE_ID); - let account = Account::new(ALICE_ID.clone()).build(&ALICE_ID); + let mut account = Account::new(ALICE_ID.clone()).build(&ALICE_ID); + account.activate(); assert!(domain.add_account(account).is_none()); let asset_definition_id = AssetDefinitionId::from_str("rose#wonderland").expect("Valid"); assert!(domain @@ -331,6 +332,7 @@ mod tests { let mut domain = Domain::new(DomainId::from_str("wonderland").expect("Valid")).build(&ALICE_ID); let mut account = Account::new(ALICE_ID.clone()).build(&ALICE_ID); + account.activate(); assert!(domain .add_asset_definition( AssetDefinition::numeric(asset_definition_id.clone()).build(&ALICE_ID) @@ -362,9 +364,10 @@ mod tests { )?; let mut domain = Domain::new(DomainId::from_str("wonderland")?).build(&ALICE_ID); - let account = Account::new(ALICE_ID.clone()) + let mut account = Account::new(ALICE_ID.clone()) .with_metadata(metadata) .build(&ALICE_ID); + account.activate(); assert!(domain.add_account(account).is_none()); let asset_definition_id = AssetDefinitionId::from_str("rose#wonderland").expect("Valid"); assert!(domain @@ -617,7 +620,8 @@ mod tests { let mut domain = Domain::new(DomainId::from_str("wonderland")?) .with_metadata(metadata) .build(&ALICE_ID); - let account = Account::new(ALICE_ID.clone()).build(&ALICE_ID); + let mut account = Account::new(ALICE_ID.clone()).build(&ALICE_ID); + account.activate(); assert!(domain.add_account(account).is_none()); let asset_definition_id = AssetDefinitionId::from_str("rose#wonderland")?; assert!(domain diff --git a/core/src/smartcontracts/wasm.rs b/core/src/smartcontracts/wasm.rs index 1f6d2474193..85c74c4e47a 100644 --- a/core/src/smartcontracts/wasm.rs +++ b/core/src/smartcontracts/wasm.rs @@ -1718,7 +1718,8 @@ mod tests { fn world_with_test_account(authority: &AccountId) -> World { let domain_id = authority.domain_id.clone(); - let account = Account::new(authority.clone()).build(authority); + let mut account = Account::new(authority.clone()).build(authority); + account.activate(); let mut domain = Domain::new(domain_id).build(authority); assert!(domain.add_account(account).is_none()); diff --git a/core/src/sumeragi/main_loop.rs b/core/src/sumeragi/main_loop.rs index a1128c7a8bb..f2239c99183 100644 --- a/core/src/sumeragi/main_loop.rs +++ b/core/src/sumeragi/main_loop.rs @@ -1229,7 +1229,8 @@ mod tests { // Predefined world state let (alice_id, alice_keypair) = gen_account_in("wonderland"); let genesis_public_key = alice_keypair.public_key().clone(); - let account = Account::new(alice_id.clone()).build(&alice_id); + let mut account = Account::new(alice_id.clone()).build(&alice_id); + account.activate(); let domain_id = "wonderland".parse().expect("Valid"); let mut domain = Domain::new(domain_id).build(&alice_id); assert!(domain.add_account(account).is_none()); diff --git a/data_model/src/account.rs b/data_model/src/account.rs index 41d74388e7b..216091420f1 100644 --- a/data_model/src/account.rs +++ b/data_model/src/account.rs @@ -92,6 +92,8 @@ mod model { pub assets: AssetsMap, /// Metadata of this account as a key-value store. pub metadata: Metadata, + /// Whether the account can exercise authority or not. + pub is_active: bool, } /// Builder which should be submitted in a transaction to create a new [`Account`] @@ -159,6 +161,12 @@ impl Account { pub fn remove_asset(&mut self, asset_id: &AssetDefinitionId) -> Option { self.assets.remove(asset_id) } + + /// Activate the account to enable its authority + #[inline] + pub fn activate(&mut self) { + self.is_active = true + } } impl NewAccount { @@ -186,6 +194,7 @@ impl NewAccount { id: self.id, assets: AssetsMap::default(), metadata: self.metadata, + is_active: false, } } } diff --git a/data_model/src/events/data/events.rs b/data_model/src/events/data/events.rs index 58fdbae2b32..ee5b7e5ffdc 100644 --- a/data_model/src/events/data/events.rs +++ b/data_model/src/events/data/events.rs @@ -300,11 +300,9 @@ mod account { pub enum AccountEvent { #[has_origin(asset_event => &asset_event.origin_id().account_id)] Asset(AssetEvent), - #[has_origin(account => account.id())] - Created(Account), + Recognized(AccountId), + Activated(AccountId), Deleted(AccountId), - AuthenticationAdded(AccountId), - AuthenticationRemoved(AccountId), #[has_origin(permission_changed => &permission_changed.account_id)] PermissionAdded(AccountPermissionChanged), #[has_origin(permission_changed => &permission_changed.account_id)] diff --git a/data_model/src/events/data/filters.rs b/data_model/src/events/data/filters.rs index 58840eb68bc..5ef4d7d8de1 100644 --- a/data_model/src/events/data/filters.rs +++ b/data_model/src/events/data/filters.rs @@ -770,7 +770,7 @@ mod tests { // the second one is an account event with a domain event inside // the third one is an asset event with an account event with a domain event inside let domain_created = DomainEvent::Created(domain).into(); - let account_created = DomainEvent::Account(AccountEvent::Created(account)).into(); + let account_recognized = DomainEvent::Account(AccountEvent::Recognized(account.id)).into(); let asset_created = DomainEvent::Account(AccountEvent::Asset(AssetEvent::Created(asset))).into(); @@ -782,17 +782,17 @@ mod tests { // domain filter matches all of those, because all of those events happened in the same domain assert!(domain_filter.matches(&domain_created)); - assert!(domain_filter.matches(&account_created)); + assert!(domain_filter.matches(&account_recognized)); assert!(domain_filter.matches(&asset_created)); // account event does not match the domain created event, as it is not an account event assert!(!account_filter.matches(&domain_created)); - assert!(account_filter.matches(&account_created)); + assert!(account_filter.matches(&account_recognized)); assert!(account_filter.matches(&asset_created)); // asset event matches only the domain->account->asset event assert!(!asset_filter.matches(&domain_created)); - assert!(!asset_filter.matches(&account_created)); + assert!(!asset_filter.matches(&account_recognized)); assert!(asset_filter.matches(&asset_created)); } } diff --git a/data_model/src/executor.rs b/data_model/src/executor.rs index d0c0b225efd..fd3d4164bfc 100644 --- a/data_model/src/executor.rs +++ b/data_model/src/executor.rs @@ -22,7 +22,7 @@ mod model { /// executor that checks if an operation satisfies some conditions. /// /// Can be used with things like [`Transaction`]s, - /// [`InstructionExpr`]s, etc. + /// [`InstructionBox`]s, etc. #[derive( Debug, Clone, diff --git a/data_model/src/lib.rs b/data_model/src/lib.rs index c856e2f13f8..e95622cb568 100644 --- a/data_model/src/lib.rs +++ b/data_model/src/lib.rs @@ -814,6 +814,12 @@ mod model { #[skip_try_from] String, ), + /// Your account is not recognized on chain. + /// Any account needs to be targeted by some creative instruction to appear on chain + UnrecognizedAuthority, + /// Your account is recognized on chain but not activated. + /// Please apply to some administrative account to activate your account by RegisterAccount instruction + InactiveAuthority, } /// Log level for reading from environment and (de)serializing diff --git a/docs/source/references/schema.json b/docs/source/references/schema.json index 649fee7573d..97f166b24a3 100644 --- a/docs/source/references/schema.json +++ b/docs/source/references/schema.json @@ -12,6 +12,10 @@ { "name": "metadata", "type": "Metadata" + }, + { + "name": "is_active", + "type": "bool" } ] }, @@ -23,53 +27,48 @@ "type": "AssetEvent" }, { - "tag": "Created", + "tag": "Recognized", "discriminant": 1, - "type": "Account" + "type": "AccountId" }, { - "tag": "Deleted", + "tag": "Activated", "discriminant": 2, "type": "AccountId" }, { - "tag": "AuthenticationAdded", + "tag": "Deleted", "discriminant": 3, "type": "AccountId" }, - { - "tag": "AuthenticationRemoved", - "discriminant": 4, - "type": "AccountId" - }, { "tag": "PermissionAdded", - "discriminant": 5, + "discriminant": 4, "type": "AccountPermissionChanged" }, { "tag": "PermissionRemoved", - "discriminant": 6, + "discriminant": 5, "type": "AccountPermissionChanged" }, { "tag": "RoleRevoked", - "discriminant": 7, + "discriminant": 6, "type": "AccountRoleChanged" }, { "tag": "RoleGranted", - "discriminant": 8, + "discriminant": 7, "type": "AccountRoleChanged" }, { "tag": "MetadataInserted", - "discriminant": 9, + "discriminant": 8, "type": "MetadataChanged" }, { "tag": "MetadataRemoved", - "discriminant": 10, + "discriminant": 9, "type": "MetadataChanged" } ] @@ -95,44 +94,40 @@ "mask": 1 }, { - "name": "Created", + "name": "Recognized", "mask": 2 }, { - "name": "Deleted", + "name": "Activated", "mask": 4 }, { - "name": "AuthenticationAdded", + "name": "Deleted", "mask": 8 }, - { - "name": "AuthenticationRemoved", - "mask": 16 - }, { "name": "PermissionAdded", - "mask": 32 + "mask": 16 }, { "name": "PermissionRemoved", - "mask": 64 + "mask": 32 }, { "name": "RoleRevoked", - "mask": 128 + "mask": 64 }, { "name": "RoleGranted", - "mask": 256 + "mask": 128 }, { "name": "MetadataInserted", - "mask": 512 + "mask": 256 }, { "name": "MetadataRemoved", - "mask": 1024 + "mask": 512 } ] } @@ -4351,6 +4346,14 @@ { "tag": "InternalError", "discriminant": 4 + }, + { + "tag": "UnrecognizedAuthority", + "discriminant": 5 + }, + { + "tag": "InactiveAuthority", + "discriminant": 6 } ] }, diff --git a/torii/src/lib.rs b/torii/src/lib.rs index a987608ee3f..71d517ece67 100644 --- a/torii/src/lib.rs +++ b/torii/src/lib.rs @@ -375,6 +375,7 @@ impl Error { ); StatusCode::INTERNAL_SERVER_ERROR } + UnrecognizedAuthority | InactiveAuthority => StatusCode::UNAUTHORIZED, } }