Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove domain from trigger identity #4640

Merged
merged 4 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions client/examples/register_1000_triggers.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! Example of registering multiple triggers
//! Used to show Iroha's trigger deduplication capabilities
use std::str::FromStr;

use iroha::{client::Client, data_model::prelude::*};
use iroha_crypto::KeyPair;
Expand Down Expand Up @@ -47,7 +46,7 @@ fn generate_genesis(

let builder = (0..num_triggers)
.map(|i| {
let trigger_id = TriggerId::new(None, Name::from_str(&i.to_string()).unwrap());
let trigger_id = i.to_string().parse::<TriggerId>().unwrap();
let trigger = build_trigger(trigger_id);
Register::trigger(trigger)
})
Expand Down
18 changes: 15 additions & 3 deletions client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1527,9 +1527,21 @@ pub mod trigger {
//! Module with queries for triggers
use super::*;

/// Construct a query to get triggers by domain id
pub fn by_domain_id(domain_id: DomainId) -> FindTriggersByDomainId {
FindTriggersByDomainId::new(domain_id)
/// Construct a query to get a trigger by its id
pub fn by_id(trigger_id: TriggerId) -> FindTriggerById {
FindTriggerById::new(trigger_id)
}

/// Construct a query to find all triggers executable
/// on behalf of the given account.
pub fn by_authority(account_id: AccountId) -> FindTriggersByAuthorityId {
FindTriggersByAuthorityId::new(account_id)
}

/// Construct a query to find all triggers executable
/// on behalf of any account belonging to the given domain.
pub fn by_domain_of_authority(domain_id: DomainId) -> FindTriggersByAuthorityDomainId {
FindTriggersByAuthorityDomainId::new(domain_id)
}
}

Expand Down
2 changes: 1 addition & 1 deletion client/tests/integration/domain_owner_permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ fn domain_owner_trigger_permissions() -> Result<()> {

let asset_definition_id = "rose#wonderland".parse()?;
let asset_id = AssetId::new(asset_definition_id, alice_id.clone());
let trigger_id: TriggerId = "trigger$kingdom".parse()?;
let trigger_id: TriggerId = "my_trigger".parse()?;

let trigger_instructions = vec![Mint::asset_numeric(1u32, asset_id)];
let register_trigger = Register::trigger(Trigger::new(
Expand Down
80 changes: 16 additions & 64 deletions client/tests/integration/triggers/data_trigger.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use eyre::Result;
use iroha::{client, data_model::prelude::*};
use iroha_data_model::asset::AssetValue;
use test_network::*;
use test_samples::{gen_account_in, ALICE_ID};

Expand All @@ -13,6 +12,17 @@ fn must_execute_both_triggers() -> Result<()> {
let asset_definition_id = "rose#wonderland".parse()?;
let asset_id = AssetId::new(asset_definition_id, account_id.clone());

let get_asset_value = |iroha: &client::Client, asset_id: AssetId| -> Numeric {
match *iroha
.request(client::asset::by_id(asset_id))
.unwrap()
.value()
{
AssetValue::Numeric(val) => val,
_ => panic!("Expected u32 asset value"),
}
};

let prev_value = get_asset_value(&test_client, asset_id.clone());

let instruction = Mint::asset_numeric(1u32, asset_id.clone());
Expand Down Expand Up @@ -41,72 +51,14 @@ fn must_execute_both_triggers() -> Result<()> {
test_client.submit_blocking(Register::account(Account::new(
gen_account_in("wonderland").0,
)))?;
test_client.submit_blocking(Register::domain(Domain::new("neverland".parse()?)))?;

let new_value = get_asset_value(&test_client, asset_id);
assert_eq!(new_value, prev_value.checked_add(numeric!(2)).unwrap());

Ok(())
}

#[test]
fn domain_scoped_trigger_must_be_executed_only_on_events_in_its_domain() -> Result<()> {
let (_rt, _peer, test_client) = <PeerBuilder>::new().with_port(10_655).start_with_runtime();
wait_for_genesis_committed(&[test_client.clone()], 0);

let create_neverland_domain: InstructionBox =
Register::domain(Domain::new("neverland".parse()?)).into();

let (account_id, _account_keypair) = gen_account_in("neverland");
let create_sapporo_account = Register::account(Account::new(account_id.clone())).into();

let asset_definition_id: AssetDefinitionId = "sakura#neverland".parse()?;
let create_sakura_asset_definition =
Register::asset_definition(AssetDefinition::numeric(asset_definition_id.clone())).into();

let asset_id = AssetId::new(asset_definition_id, account_id.clone());
let create_sakura_asset = Register::asset(Asset::new(asset_id.clone(), 0_u32)).into();

test_client.submit_all_blocking([
create_neverland_domain,
create_sapporo_account,
create_sakura_asset_definition,
create_sakura_asset,
])?;

let prev_value = get_asset_value(&test_client, asset_id.clone());

let register_trigger = Register::trigger(Trigger::new(
"mint_sakura$neverland".parse()?,
Action::new(
[Mint::asset_numeric(1u32, asset_id.clone())],
Repeats::Indefinitely,
account_id,
AccountEventFilter::new().for_events(AccountEventSet::Created),
),
));
test_client.submit_blocking(register_trigger)?;

test_client.submit_blocking(Register::account(Account::new(
gen_account_in("wonderland").0,
)))?;
let new_value = get_asset_value(&test_client, asset_id.clone());
assert_eq!(new_value, prev_value.checked_add(numeric!(1)).unwrap());

test_client.submit_blocking(Register::account(Account::new(
gen_account_in("neverland").0,
)))?;
test_client.submit_blocking(Register::domain(Domain::new("neverland".parse()?)))?;

let new_value = get_asset_value(&test_client, asset_id);
assert_eq!(new_value, prev_value.checked_add(Numeric::ONE).unwrap());
let newer_value = get_asset_value(&test_client, asset_id);
assert_eq!(newer_value, new_value.checked_add(numeric!(1)).unwrap());
dima74 marked this conversation as resolved.
Show resolved Hide resolved

Ok(())
}

fn get_asset_value(client: &client::Client, asset_id: AssetId) -> Numeric {
let asset = client.request(client::asset::by_id(asset_id)).unwrap();

let AssetValue::Numeric(val) = *asset.value() else {
panic!("Expected u32 asset value")
};

val
}
1 change: 1 addition & 0 deletions client/tests/integration/triggers/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod by_call_trigger;
mod data_trigger;
mod event_trigger;
mod orphans;
mod time_trigger;
mod trigger_rollback;
78 changes: 78 additions & 0 deletions client/tests/integration/triggers/orphans.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
use iroha::{
client::{trigger, Client},
data_model::prelude::*,
};
use test_network::{wait_for_genesis_committed, Peer, PeerBuilder};
use test_samples::gen_account_in;
use tokio::runtime::Runtime;

fn find_trigger(iroha: &Client, trigger_id: TriggerId) -> Option<TriggerId> {
iroha
.build_query(trigger::by_id(trigger_id))
.execute()
.ok()
.map(|trigger| trigger.id)
}

fn set_up_trigger(
port: u16,
) -> eyre::Result<(Runtime, Peer, Client, DomainId, AccountId, TriggerId)> {
let (rt, peer, iroha) = <PeerBuilder>::new().with_port(port).start_with_runtime();
wait_for_genesis_committed(&[iroha.clone()], 0);

let failand: DomainId = "failand".parse()?;
let create_failand: InstructionBox = Register::domain(Domain::new(failand.clone())).into();

let (the_one_who_fails, _account_keypair) = gen_account_in(failand.name());
let create_the_one_who_fails: InstructionBox =
Register::account(Account::new(the_one_who_fails.clone())).into();

let fail_on_account_events = "fail".parse::<TriggerId>()?;
let register_fail_on_account_events: InstructionBox = Register::trigger(Trigger::new(
fail_on_account_events.clone(),
Action::new(
[Fail::new(":(".to_owned())],
Repeats::Indefinitely,
the_one_who_fails.clone(),
AccountEventFilter::new(),
),
))
.into();
iroha.submit_all_blocking([
create_failand,
create_the_one_who_fails,
register_fail_on_account_events,
])?;
Ok((
rt,
peer,
iroha,
failand,
the_one_who_fails,
fail_on_account_events,
))
}

#[test]
fn trigger_must_be_removed_on_action_authority_account_removal() -> eyre::Result<()> {
let (_rt, _peer, iroha, _, the_one_who_fails, fail_on_account_events) = set_up_trigger(10_565)?;
assert_eq!(
find_trigger(&iroha, fail_on_account_events.clone()),
Some(fail_on_account_events.clone())
);
iroha.submit_blocking(Unregister::account(the_one_who_fails.clone()))?;
assert_eq!(find_trigger(&iroha, fail_on_account_events.clone()), None);
Ok(())
}

#[test]
fn trigger_must_be_removed_on_action_authority_domain_removal() -> eyre::Result<()> {
let (_rt, _peer, iroha, failand, _, fail_on_account_events) = set_up_trigger(10_505)?;
assert_eq!(
find_trigger(&iroha, fail_on_account_events.clone()),
Some(fail_on_account_events.clone())
);
iroha.submit_blocking(Unregister::domain(failand.clone()))?;
assert_eq!(find_trigger(&iroha, fail_on_account_events.clone()), None);
Ok(())
}
4 changes: 2 additions & 2 deletions client/tests/integration/triggers/time_trigger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,8 @@ fn mint_nft_for_every_user_every_1_sec() -> Result<()> {

// Registering trigger
let start_time = curr_time();
let schedule =
TimeSchedule::starting_at(start_time).with_period(Duration::from_millis(TRIGGER_PERIOD_MS));
let schedule = TimeSchedule::starting_at(start_time + Duration::from_secs(5))
.with_period(Duration::from_millis(TRIGGER_PERIOD_MS));
let register_trigger = Register::trigger(Trigger::new(
"mint_nft_for_all".parse()?,
Action::new(
Expand Down
6 changes: 3 additions & 3 deletions client_cli/pytests/common/consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ class Stderr(Enum):
INVALID_CHARACTER = "Failed to parse"
INVALID_VALUE_TYPE = "should be either `Store` or `Numeric`"
RESERVED_CHARACTER = (
"The `@` character is reserved for `account@domain` constructs,"
" `#` — for `asset#domain` and `$` — for `trigger$domain`."
"The `@` character is reserved for `account@domain` constructs, "
"and `#` — for `asset#domain`."
)
WHITESPACES = "White space not allowed"
INSUFFICIENT_FUNDS = "Not enough quantity to transfer/burn"
Expand All @@ -34,7 +34,7 @@ class ReservedChars(Enum):
Enum for reserved characters in names.
"""

SPECIAL = "@#$"
SPECIAL = "@#"
WHITESPACES = string.whitespace
ALL = SPECIAL + WHITESPACES

Expand Down
Binary file modified configs/swarm/executor.wasm
Binary file not shown.
2 changes: 1 addition & 1 deletion core/src/query/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl LiveQueryStore {
}

fn remove(&mut self, query_id: &str) -> Option<LiveQuery> {
self.queries.remove(query_id).map(|(output, _)| output)
self.queries.swap_remove(query_id).map(|(output, _)| output)
}
}

Expand Down
30 changes: 25 additions & 5 deletions core/src/smartcontracts/isi/domain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,36 @@ pub mod isi {
) -> Result<(), Error> {
let account_id = self.object_id;

let domain = state_transaction.world.domain_mut(&account_id.domain_id)?;
if domain.remove_account(&account_id).is_none() {
state_transaction
.world()
.triggers()
.inspect_by_action(
|action| action.authority() == &account_id,
|trigger_id, _| trigger_id.clone(),
)
.collect::<Vec<_>>()
.into_iter()
.for_each(|trigger_id| {
state_transaction
.world
.triggers
.remove(trigger_id)
.then_some(())
.expect("should succeed")
});

if state_transaction
.world
.domain_mut(&account_id.domain_id)?
.remove_account(&account_id)
.is_none()
{
return Err(FindError::Account(account_id).into());
}

state_transaction
.world
.emit_events(Some(DomainEvent::Account(AccountEvent::Deleted(
account_id,
))));
.emit_events(Some(AccountEvent::Deleted(account_id)));

Ok(())
}
Expand Down
3 changes: 2 additions & 1 deletion core/src/smartcontracts/isi/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ impl ValidQuery for QueryBox {
FindTransactionsByAccountId,
FindPermissionsByAccountId,
FindAllActiveTriggerIds,
FindTriggersByDomainId,
FindTriggersByAuthorityId,
FindTriggersByAuthorityDomainId,
FindAllRoles,
FindAllRoleIds,
FindRolesByAccountId,
Expand Down
Loading
Loading