From e156941f306b851baed7c522e5d7f4d896ad7a93 Mon Sep 17 00:00:00 2001 From: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:59:17 +0900 Subject: [PATCH] [feature] #2085: Auto-register destination account on transfer Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com> --- .../integration/auto_register_account/mod.rs | 1 + .../auto_register_account/on_transfer.rs | 237 ++++++++++++++++++ client/tests/integration/mod.rs | 1 + configs/sample_params/src/.toml | 3 + configs/swarm/genesis.json | 9 + core/src/smartcontracts/isi/account.rs | 13 +- core/src/smartcontracts/isi/asset.rs | 24 +- core/src/smartcontracts/isi/domain.rs | 13 +- core/src/smartcontracts/isi/mod.rs | 48 ++++ tools/kagami/src/genesis.rs | 10 +- 10 files changed, 351 insertions(+), 8 deletions(-) create mode 100644 client/tests/integration/auto_register_account/mod.rs create mode 100644 client/tests/integration/auto_register_account/on_transfer.rs diff --git a/client/tests/integration/auto_register_account/mod.rs b/client/tests/integration/auto_register_account/mod.rs new file mode 100644 index 00000000000..cb43618cf55 --- /dev/null +++ b/client/tests/integration/auto_register_account/mod.rs @@ -0,0 +1 @@ +mod on_transfer; diff --git a/client/tests/integration/auto_register_account/on_transfer.rs b/client/tests/integration/auto_register_account/on_transfer.rs new file mode 100644 index 00000000000..6af31abdbd5 --- /dev/null +++ b/client/tests/integration/auto_register_account/on_transfer.rs @@ -0,0 +1,237 @@ +//! A new account should be automatically registered when being a destination of a transfer + +use iroha_client::data_model::prelude::*; +use iroha_sample_params::{alias::Alias, SampleParams}; +use serde_json::json; +use test_network::*; + +/// A new account should be automatically registered when being a destination of a transfer of numeric asset e.g. "rose" +/// +/// # Scenario +/// +/// 0. alice@wonderland: domain owner +/// 0. alice -> new carol ... ok (domain owner) +/// 0. carol -> new dave ... err (permission denied) +/// 0. grant `CanRegisterAccountInDomain` permission to carol +/// 0. carol -> new dave ... ok (authorized) +#[test] +fn asset_numeric() { + 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(); + + // alice@wonderland: domain owner + let alice: AccountId = "alice@wonderland".parse_alias(); + let wonderland = observer + .request(FindDomainById::new("wonderland".parse().unwrap())) + .expect("should be found"); + assert_eq!(*wonderland.owned_by(), alice); + + // alice -> new carol ... ok (domain owner) + let carol: AccountId = "carol@wonderland".parse_alias(); + let _ = observer + .request(FindAccountById::new(carol.clone())) + .expect_err("carol should not be on chain at this point"); + let rose_alice: AssetId = "rose##alice@wonderland".parse_alias(); + 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.clone()); + client_alice + .submit_blocking(transfer) + .expect("alice the domain owner should succeed to register carol in the domain"); + let _ = observer + .request(FindAccountById::new(carol.clone())) + .expect("carol should be on chain now"); + let rose_carol: AssetId = "rose##carol@wonderland".parse_alias(); + let n_roses_carol = observer + .request(FindAssetQuantityById::new(rose_carol.clone())) + .expect("carol should have roses"); + assert_eq!(n_roses_carol, numeric!(3)); + + // carol -> new dave ... err (permission denied) + let client_carol = { + let mut client = client_alice.clone(); + let sp = SampleParams::default(); + client.key_pair = sp.signatory["carol"].make_key_pair(); + client.account_id = carol.clone(); + client + }; + let dave: AccountId = "dave@wonderland".parse_alias(); + let _ = observer + .request(FindAccountById::new(dave.clone())) + .expect_err("dave should not be on chain at this point"); + let transfer = Transfer::asset_numeric(rose_carol, 1_u32, dave.clone()); + let _ = client_carol + .submit_blocking(transfer.clone()) + .expect_err("carol should fail to register dave in the domain"); + let _ = observer + .request(FindAccountById::new(dave.clone())) + .expect_err("dave should not be on chain yet"); + let rose_dave: AssetId = "rose##dave@wonderland".parse_alias(); + let _ = observer + .request(FindAssetQuantityById::new(rose_dave.clone())) + .expect_err("dave should not have roses yet"); + + // grant `CanRegisterAccountInDomain` permission to carol + let grant = { + let token = PermissionToken::new( + "CanRegisterAccountInDomain".parse().unwrap(), + &json!({ "domain_id": wonderland.id }), + ); + Grant::permission(token, carol.clone()) + }; + client_alice + .submit_blocking(grant) + .expect("alice should succeed to grant"); + + // carol -> new dave ... ok (authorized) + client_carol + .submit_blocking(transfer) + .expect("carol now authorized should succeed to register dave in the domain"); + let _ = observer + .request(FindAccountById::new(dave.clone())) + .expect("dave should be on chain now"); + let n_roses_dave = observer + .request(FindAssetQuantityById::new(rose_dave)) + .expect("dave should have roses"); + assert_eq!(n_roses_dave, numeric!(1)); +} + +/// A new account should be automatically registered when being a destination of a transfer of asset store e.g. "dict" +/// +/// # Scenario +/// +/// 0. alice@wonderland: domain owner +/// 0. alice -> new carol ... ok (domain owner) +/// +/// And how to authorize an account other than the domain owner is the same way as [`asset_numeric()`] +#[test] +fn asset_store() { + let (_rt, _peer, client_alice) = ::new().with_port(10_815).start_with_runtime(); + wait_for_genesis_committed(&[client_alice.clone()], 0); + let observer = client_alice.clone(); + + let dict_alice: AssetId = "dict##alice@wonderland".parse_alias(); + let register: InstructionBox = + Register::asset_definition(AssetDefinition::store(dict_alice.definition_id.clone())).into(); + let dict_key: Name = "raven".parse().unwrap(); + let dict_value: String = "nevar".into(); + let set_key_value = + SetKeyValue::asset(dict_alice.clone(), dict_key.clone(), dict_value.clone()).into(); + client_alice + .submit_all_blocking([register, set_key_value]) + .expect("should be committed"); + + // alice@wonderland: domain owner + let alice: AccountId = "alice@wonderland".parse_alias(); + let wonderland = observer + .request(FindDomainById::new("wonderland".parse().unwrap())) + .expect("should be found"); + assert_eq!(*wonderland.owned_by(), alice); + + // alice -> new carol ... ok (domain owner) + let carol: AccountId = "carol@wonderland".parse_alias(); + let _ = observer + .request(FindAccountById::new(carol.clone())) + .expect_err("carol should not be on chain at this point"); + let transfer = Transfer::asset_store(dict_alice, carol.clone()); + client_alice + .submit_blocking(transfer) + .expect("alice the domain owner should succeed to register carol in the domain"); + let _ = observer + .request(FindAccountById::new(carol.clone())) + .expect("carol should be on chain now"); + let dict_carol: AssetId = "dict##carol@wonderland".parse_alias(); + let dict_value_carol = observer + .request(FindAssetKeyValueByIdAndKey::new( + dict_carol.clone(), + dict_key, + )) + .expect("carol should have dicts"); + assert_eq!(dict_value_carol, dict_value.into()); +} + +/// A new account should be automatically registered when being a destination of a transfer of asset definition e.g. "rose" +/// +/// # Scenario +/// +/// 0. alice@wonderland: domain owner +/// 0. alice -> new carol ... ok (domain owner) +/// +/// And how to authorize an account other than the domain owner is the same way as [`asset_numeric()`] +#[test] +fn asset_definition() { + 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(); + + // alice@wonderland: domain owner + let alice: AccountId = "alice@wonderland".parse_alias(); + let wonderland = observer + .request(FindDomainById::new("wonderland".parse().unwrap())) + .expect("should be found"); + assert_eq!(*wonderland.owned_by(), alice); + + // alice -> new carol ... ok (domain owner) + let carol: AccountId = "carol@wonderland".parse_alias(); + let _ = observer + .request(FindAccountById::new(carol.clone())) + .expect_err("carol should not be on chain at this point"); + let rose: AssetDefinitionId = "rose#wonderland".parse().unwrap(); + let rose_def = observer + .request(FindAssetDefinitionById::new(rose.clone())) + .expect("wonderland should have rose definition"); + assert_eq!(*rose_def.owned_by(), alice); + let transfer = Transfer::asset_definition(alice, rose.clone(), carol.clone()); + client_alice + .submit_blocking(transfer) + .expect("alice the domain owner should succeed to register carol in the domain"); + let _ = observer + .request(FindAccountById::new(carol.clone())) + .expect("carol should be on chain now"); + let rose_def = observer + .request(FindAssetDefinitionById::new(rose.clone())) + .expect("wonderland should have rose definition"); + assert_eq!(*rose_def.owned_by(), carol); +} + +/// A new account should be automatically registered when being a destination of a transfer of domain e.g. "wonderland" +/// +/// # Scenario +/// +/// 0. alice@wonderland: domain owner +/// 0. alice -> new carol ... ok (domain owner) +/// +/// And how to authorize an account other than the domain owner is the same way as [`asset_numeric()`] +#[test] +fn domain() { + let (_rt, _peer, client_alice) = ::new().with_port(10_830).start_with_runtime(); + wait_for_genesis_committed(&[client_alice.clone()], 0); + let observer = client_alice.clone(); + + // alice@wonderland: domain owner + let alice: AccountId = "alice@wonderland".parse_alias(); + let wonderland = observer + .request(FindDomainById::new("wonderland".parse().unwrap())) + .expect("should be found"); + assert_eq!(*wonderland.owned_by(), alice); + + // alice -> new carol ... ok (domain owner) + let carol: AccountId = "carol@wonderland".parse_alias(); + let _ = observer + .request(FindAccountById::new(carol.clone())) + .expect_err("carol should not be on chain at this point"); + let transfer = Transfer::domain(alice, wonderland.id, carol.clone()); + client_alice + .submit_blocking(transfer) + .expect("alice the domain owner should succeed to register carol in the domain"); + let _ = observer + .request(FindAccountById::new(carol.clone())) + .expect("carol should be on chain now"); + let wonderland = observer + .request(FindDomainById::new("wonderland".parse().unwrap())) + .expect("should be found"); + assert_eq!(*wonderland.owned_by(), carol); +} diff --git a/client/tests/integration/mod.rs b/client/tests/integration/mod.rs index 37299969665..59180ef7ffa 100644 --- a/client/tests/integration/mod.rs +++ b/client/tests/integration/mod.rs @@ -1,6 +1,7 @@ mod add_domain; mod asset; mod asset_propagation; +mod auto_register_account; mod domain_owner_permissions; mod events; mod extra_functional; diff --git a/configs/sample_params/src/.toml b/configs/sample_params/src/.toml index b46d4de28fa..2bfed41e438 100644 --- a/configs/sample_params/src/.toml +++ b/configs/sample_params/src/.toml @@ -17,6 +17,9 @@ private_key = { algorithm = "ed25519", payload = "AF3F96DEEF44348FEB516C05755897 [signatory.carol] public_key = "ed01202067562532778E2966A902A593E1FCFB74F1E058FE1769E272B951A58769131B" private_key = { algorithm = "ed25519", payload = "F45657998C3D6DCCCF701318E4FF6D81ACAB48AEB84AAC40F5999AAE80A4AC0A2067562532778E2966A902A593E1FCFB74F1E058FE1769E272B951A58769131B" } +[signatory.dave] +public_key = "ed01205F71C5E560BCD49B4086281689BA2E0A60DD1859AF14ED3024FAED3E070C1600" +private_key = { algorithm = "ed25519", payload = "638EC7172F4B52A29569B7674338561C11A29F3DF5AEDFA1B8DD9D589799842E5F71C5E560BCD49B4086281689BA2E0A60DD1859AF14ED3024FAED3E070C1600" } [signatory.mouse] public_key = "ed012014A2900349A8CB6250D8CE5399D1964766B559A37F48E8C98C1EA83F365D8A72" private_key = { algorithm = "ed25519", payload = "67AAB998A3951A071324B815BAF425309D6AF8D2D2636A3FC2BEA01C322BC23B14A2900349A8CB6250D8CE5399D1964766B559A37F48E8C98C1EA83F365D8A72" } diff --git a/configs/swarm/genesis.json b/configs/swarm/genesis.json index 65c6dc1b3d3..1d8800c557e 100644 --- a/configs/swarm/genesis.json +++ b/configs/swarm/genesis.json @@ -93,6 +93,15 @@ } } }, + { + "Transfer": { + "AssetDefinition": { + "source_id": "ed0120E2ECD69DA5833EC10FB3DFAED83A07E5B9CBE9BC39484F0F7DDEC8B46253428B@genesis", + "object": "rose#wonderland", + "destination_id": "ed0120CE7FA46C9DCE7EA4B125E2E36BDB63EA33073E7590AC92816AE1E861B7048B03@wonderland" + } + } + }, { "Transfer": { "Domain": { diff --git a/core/src/smartcontracts/isi/account.rs b/core/src/smartcontracts/isi/account.rs index eff922b1dc2..146d6058cc8 100644 --- a/core/src/smartcontracts/isi/account.rs +++ b/core/src/smartcontracts/isi/account.rs @@ -135,9 +135,19 @@ pub mod isi { impl Execute for Transfer { fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { + if state_transaction + .world + .account(&self.destination_id) + .is_err() + { + let register_destination = + Register::account(Account::new(self.destination_id.clone())); + self.back_validate(register_destination, authority, state_transaction)? + } + let Transfer { source_id, object, @@ -145,7 +155,6 @@ pub mod isi { } = self; let _ = state_transaction.world.account(&source_id)?; - let _ = state_transaction.world.account(&destination_id)?; let asset_definition = state_transaction.world.asset_definition_mut(&object)?; diff --git a/core/src/smartcontracts/isi/asset.rs b/core/src/smartcontracts/isi/asset.rs index 78d7ff998b5..d9cba3255eb 100644 --- a/core/src/smartcontracts/isi/asset.rs +++ b/core/src/smartcontracts/isi/asset.rs @@ -134,9 +134,19 @@ pub mod isi { #[metrics(+"transfer_store")] fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { + if state_transaction + .world + .account(&self.destination_id) + .is_err() + { + let register_destination = + Register::account(Account::new(self.destination_id.clone())); + self.back_validate(register_destination, authority, state_transaction)? + } + let asset_id = self.source_id; assert_asset_type( &asset_id.definition_id, @@ -277,9 +287,19 @@ pub mod isi { impl Execute for Transfer { fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { + if state_transaction + .world + .account(&self.destination_id) + .is_err() + { + let register_destination = + Register::account(Account::new(self.destination_id.clone())); + self.back_validate(register_destination, authority, state_transaction)? + } + let source_id = self.source_id; let destination_id = AssetId::new(source_id.definition_id.clone(), self.destination_id.clone()); diff --git a/core/src/smartcontracts/isi/domain.rs b/core/src/smartcontracts/isi/domain.rs index b7f9e0849ab..e05fbcefcfc 100644 --- a/core/src/smartcontracts/isi/domain.rs +++ b/core/src/smartcontracts/isi/domain.rs @@ -323,9 +323,19 @@ pub mod isi { impl Execute for Transfer { fn execute( self, - _authority: &AccountId, + authority: &AccountId, state_transaction: &mut StateTransaction<'_, '_>, ) -> Result<(), Error> { + if state_transaction + .world + .account(&self.destination_id) + .is_err() + { + let register_destination = + Register::account(Account::new(self.destination_id.clone())); + self.back_validate(register_destination, authority, state_transaction)? + } + let Transfer { source_id, object, @@ -333,7 +343,6 @@ pub mod isi { } = self; let _ = state_transaction.world.account(&source_id)?; - let _ = state_transaction.world.account(&destination_id)?; let domain = state_transaction.world.domain_mut(&object)?; diff --git a/core/src/smartcontracts/isi/mod.rs b/core/src/smartcontracts/isi/mod.rs index 590786d6b85..2d8f9d22c1a 100644 --- a/core/src/smartcontracts/isi/mod.rs +++ b/core/src/smartcontracts/isi/mod.rs @@ -10,10 +10,13 @@ pub mod triggers; pub mod tx; pub mod world; +use std::fmt::Display; + use eyre::Result; use iroha_data_model::{ isi::{error::InstructionExecutionError as Error, *}, prelude::*, + ValidationFail, }; use iroha_logger::prelude::*; use storage::storage::StorageReadOnly; @@ -33,6 +36,51 @@ pub trait Registrable { fn build(self, authority: &AccountId) -> Self::Target; } +/// Validate and execute an additional instruction `T` during `Self` execution. +/// FIXME This means going backwards from execution phase to validation phase, which is contrary to current design principles +trait BackValidate +where + Self: Execute + Display, + T: Into, +{ + /// Validate and execute an additional `instruction` during `self` execution. + /// + /// # Errors + /// + /// - Fails if `instruction` fails for validation. This case returns an opaque [`Error::Fail`] that contains detailed string + /// - Fails if `instruction` fails for execution + fn back_validate( + &self, + instruction: T, + authority: &AccountId, + state_transaction: &mut StateTransaction<'_, '_>, + ) -> Result<(), Error> { + let Err(validation_err) = state_transaction + .world + .executor + .clone() + .validate_instruction(state_transaction, authority, instruction.into()) + else { + return Ok(()); + }; + let ValidationFail::InstructionFailed(execution_err) = validation_err else { + return Err(Error::Fail(format!( + "During execution of {self},\nadditional validation failed: {validation_err}" + ))); + }; + Err(execution_err) + } +} + +impl BackValidate> for Transfer +where + Self: Execute, + S: Identifiable, + ::Id: Display, + O: Display, +{ +} + impl Execute for InstructionBox { fn execute( self, diff --git a/tools/kagami/src/genesis.rs b/tools/kagami/src/genesis.rs index c2fe3242866..c8d589ac976 100644 --- a/tools/kagami/src/genesis.rs +++ b/tools/kagami/src/genesis.rs @@ -113,7 +113,12 @@ pub fn generate_default( PermissionToken::new("CanSetParameters".parse()?, &json!(null)), alice_id.clone(), ); - let transfer_domain_ownerhip = Transfer::domain( + let transfer_rose_ownership = Transfer::asset_definition( + "genesis@genesis".parse_alias(), + "rose#wonderland".parse()?, + alice_id.clone(), + ); + let transfer_wonderland_ownership = Transfer::domain( "genesis@genesis".parse_alias(), "wonderland".parse()?, alice_id.clone(), @@ -175,7 +180,8 @@ pub fn generate_default( for isi in [ mint.into(), mint_cabbage.into(), - transfer_domain_ownerhip.into(), + transfer_rose_ownership.into(), + transfer_wonderland_ownership.into(), grant_permission_to_set_parameters.into(), ] .into_iter()