Skip to content

Commit

Permalink
Fix new @appetrosyan remarks
Browse files Browse the repository at this point in the history
Signed-off-by: Arjentix <arjentix@gmail.com>
  • Loading branch information
Arjentix committed Feb 22, 2022
1 parent 9e1de23 commit dd98e1a
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 110 deletions.
4 changes: 1 addition & 3 deletions client/tests/integration/events/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,7 @@ fn transaction_execution_should_produce_events(executable: Executable) -> Result
// assertion
for i in 0..4_usize {
let domain_id = DomainId::test(&i.to_string());
let expected_event =
DomainEvent::StatusUpdated(DomainStatusUpdated::new(domain_id, DataStatus::Created))
.into();
let expected_event = DomainEvent::Created(domain_id).into();
let event: DataEvent = event_receiver.recv()??.try_into()?;
assert_eq!(event, expected_event);
}
Expand Down
21 changes: 9 additions & 12 deletions core/src/smartcontracts/isi/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub mod isi {
wsv.modify_account(&account_id, |account| {
account.signatories.push(public_key);

Ok(AccountStatusUpdated::new(account_id.clone(), Updated::Authentication).into())
Ok(AccountEvent::Authentication(account_id.clone()))
})
}
}
Expand All @@ -50,7 +50,7 @@ pub mod isi {
wsv.modify_account(&account_id, |account| {
account.signature_check_condition = signature_check_condition;

Ok(AccountStatusUpdated::new(account_id.clone(), Updated::Authentication).into())
Ok(AccountEvent::Authentication(account_id.clone()))
})
}
}
Expand Down Expand Up @@ -81,10 +81,7 @@ pub mod isi {
account.signatories.remove(index);
}

Ok(AccountStatusUpdated::new(
account_id.clone(),
Updated::Authentication,
).into())
Ok(AccountEvent::Authentication(account_id.clone()))
})
}
}
Expand All @@ -109,7 +106,7 @@ pub mod isi {
account_metadata_limits,
)?;

Ok(AccountStatusUpdated::new(account_id.clone(), MetadataUpdated::Inserted).into())
Ok(AccountEvent::MetadataInserted(account_id.clone()))
})
}
}
Expand All @@ -131,7 +128,7 @@ pub mod isi {
.remove(&self.key)
.ok_or(FindError::MetadataKey(self.key))?;

Ok(AccountStatusUpdated::new(account_id.clone(), MetadataUpdated::Removed).into())
Ok(AccountEvent::MetadataRemoved(account_id.clone()))
})
}
}
Expand All @@ -151,7 +148,7 @@ pub mod isi {
wsv.modify_account(&account_id, |account| {
let _ = account.permission_tokens.insert(permission);

Ok(AccountStatusUpdated::new(account_id.clone(), Updated::Permission).into())
Ok(AccountEvent::Permission(account_id.clone()))
})
}
}
Expand All @@ -171,7 +168,7 @@ pub mod isi {
wsv.modify_account(&account_id, |account| {
let _ = account.permission_tokens.remove(permission);

Ok(AccountStatusUpdated::new(account_id.clone(), Updated::Permission).into())
Ok(AccountEvent::Permission(account_id.clone()))
})
}
}
Expand All @@ -196,7 +193,7 @@ pub mod isi {
wsv.modify_account(&self.destination_id, |account| {
let _ = account.roles.insert(role);

Ok(AccountStatusUpdated::new(self.destination_id, Updated::Permission).into())
Ok(AccountEvent::Permission(self.destination_id.clone()))
})
}
}
Expand All @@ -221,7 +218,7 @@ pub mod isi {
wsv.modify_account(&self.destination_id, |account| {
let _ = account.roles.remove(&role);

Ok(AccountStatusUpdated::new(self.destination_id, Updated::Permission).into())
Ok(AccountEvent::Permission(self.destination_id.clone()))
})
}
}
Expand Down
42 changes: 22 additions & 20 deletions core/src/smartcontracts/isi/domain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,9 @@ pub mod isi {
)),
Entry::Vacant(entry) => {
let _ = entry.insert(account.into());
Ok(AccountEvent::StatusUpdated(AccountStatusUpdated::new(
Ok(DomainEvent::Account(AccountEvent::Created(
account_id.clone(),
DataStatus::Created,
))
.into())
)))
}
}
})
Expand All @@ -66,11 +64,9 @@ pub mod isi {
let account_id = self.object_id;
wsv.modify_domain(&account_id.domain_id, |domain| {
domain.accounts.remove(&account_id);
Ok(AccountEvent::StatusUpdated(AccountStatusUpdated::new(
Ok(DomainEvent::Account(AccountEvent::Deleted(
account_id.clone(),
DataStatus::Deleted,
))
.into())
)))
})
}
}
Expand All @@ -85,21 +81,24 @@ pub mod isi {
wsv: &WorldStateView<W>,
) -> Result<(), Self::Error> {
let asset_definition = self.object;
let asset_id = asset_definition.id.clone();
asset_id
let asset_definition_id = asset_definition.id.clone();
asset_definition_id
.name
.validate_len(wsv.config.ident_length_limits)
.map_err(Error::Validate)?;
let domain_id = asset_id.domain_id.clone();
let domain_id = asset_definition_id.domain_id.clone();

wsv.modify_domain(&domain_id, |domain| {
match domain.asset_definitions.entry(asset_id.clone()) {
match domain.asset_definitions.entry(asset_definition_id.clone()) {
Entry::Vacant(entry) => {
let _ = entry.insert(AssetDefinitionEntry {
definition: asset_definition,
registered_by: authority,
});
Ok(AssetDefinitionEvent::new(asset_id, DataStatus::Created).into())
Ok(DomainEvent::AssetDefinition(AssetDefinitionEvent::new(
asset_definition_id,
DataStatus::Created,
)))
}
Entry::Occupied(entry) => Err(Error::Repetition(
InstructionType::Register,
Expand All @@ -122,10 +121,10 @@ pub mod isi {
let asset_definition_id = self.object_id;
wsv.modify_domain(&asset_definition_id.domain_id, |domain| {
domain.asset_definitions.remove(&asset_definition_id);
Ok(
AssetDefinitionEvent::new(asset_definition_id.clone(), DataStatus::Deleted)
.into(),
)
Ok(DomainEvent::AssetDefinition(AssetDefinitionEvent::new(
asset_definition_id.clone(),
DataStatus::Deleted,
)))
})?;

for domain in wsv.domains() {
Expand All @@ -139,7 +138,10 @@ pub mod isi {
for id in &keys {
wsv.modify_account(account_id, |account_mut| {
account_mut.assets.remove(id);
Ok(AssetEvent::new(id.clone(), DataStatus::Deleted).into())
Ok(AccountEvent::Asset(AssetEvent::new(
id.clone(),
DataStatus::Deleted,
)))
})?;
}
}
Expand Down Expand Up @@ -223,7 +225,7 @@ pub mod isi {
.metadata
.insert_with_limits(self.key, self.value, limits)?;

Ok(DomainStatusUpdated::new(domain_id.clone(), MetadataUpdated::Inserted).into())
Ok(DomainEvent::MetadataInserted(domain_id.clone()))
})
}
}
Expand All @@ -245,7 +247,7 @@ pub mod isi {
.remove(&self.key)
.ok_or(FindError::MetadataKey(self.key))?;

Ok(DomainStatusUpdated::new(domain_id.clone(), MetadataUpdated::Removed).into())
Ok(DomainEvent::MetadataRemoved(domain_id.clone()))
})
}
}
Expand Down
12 changes: 2 additions & 10 deletions core/src/smartcontracts/isi/world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,7 @@ pub mod isi {

wsv.modify_world(|world| {
world.domains.insert(domain_id.clone(), domain);
Ok(DomainEvent::StatusUpdated(DomainStatusUpdated::new(
domain_id,
DataStatus::Created,
))
.into())
Ok(DomainEvent::Created(domain_id).into())
})?;

wsv.metrics.domains.inc();
Expand All @@ -97,11 +93,7 @@ pub mod isi {
wsv.modify_world(|world| {
// TODO: Should we fail if no domain found?
world.domains.remove(&domain_id);
Ok(DomainEvent::StatusUpdated(DomainStatusUpdated::new(
domain_id,
DataStatus::Deleted,
))
.into())
Ok(DomainEvent::Deleted(domain_id).into())
})?;

wsv.metrics.domains.dec();
Expand Down
15 changes: 9 additions & 6 deletions core/src/wsv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,14 +359,14 @@ impl<W: WorldTrait> WorldStateView<W> {
AccountEvent::Asset(asset_event) => {
events.push(DataEvent::Asset(asset_event.clone()))
}
AccountEvent::StatusUpdated(_) => (),
_ => (),
}
events.push(DataEvent::Account(account_event.clone()));
}
DomainEvent::AssetDefinition(asset_definition_event) => {
events.push(DataEvent::AssetDefinition(asset_definition_event.clone()))
}
DomainEvent::StatusUpdated(_) => (),
_ => (),
}
events.push(DataEvent::Domain(domain_event.clone()));
}
Expand Down Expand Up @@ -467,7 +467,7 @@ impl<W: WorldTrait> WorldStateView<W> {
.accounts
.get_mut(id)
.ok_or_else(|| FindError::Account(id.clone()))?;
f(account).map(Into::into)
f(account).map(DomainEvent::Account)
})
}

Expand Down Expand Up @@ -523,7 +523,7 @@ impl<W: WorldTrait> WorldStateView<W> {
if asset.value.is_zero_value() {
account.assets.remove(id);
}
event_result.map(Into::into)
event_result.map(AccountEvent::Asset)
})
}

Expand All @@ -543,7 +543,10 @@ impl<W: WorldTrait> WorldStateView<W> {
id.clone(),
Asset::new(id.clone(), default_asset_value.into()),
);
Ok(AssetEvent::new(id.clone(), DataStatus::Created).into())
Ok(AccountEvent::Asset(AssetEvent::new(
id.clone(),
DataStatus::Created,
)))
})
.map_err(|err| {
iroha_logger::warn!(?err);
Expand Down Expand Up @@ -582,7 +585,7 @@ impl<W: WorldTrait> WorldStateView<W> {
.asset_definitions
.get_mut(id)
.ok_or_else(|| FindError::AssetDefinition(id.clone()))?;
f(asset_definition_entry).map(Into::into)
f(asset_definition_entry).map(DomainEvent::AssetDefinition)
})
}

Expand Down
50 changes: 35 additions & 15 deletions data_model/src/events/data/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ use super::*;
pub type AssetEvent = SimpleEvent<AssetId>;
pub type AssetDefinitionEvent = SimpleEvent<AssetDefinitionId>;
pub type PeerEvent = SimpleEvent<PeerId>;
pub type AccountStatusUpdated = SimpleEvent<AccountId>;
pub type DomainStatusUpdated = SimpleEvent<DomainId>;
#[cfg(feature = "roles")]
pub type Role = SimpleEvent<RoleId>;

Expand All @@ -20,7 +18,7 @@ pub struct SimpleEvent<Id> {
status: Status,
}

impl<Id> SimpleEvent<Id> {
impl<Id: Into<IdBox> + Debug + Clone + Eq + Ord> SimpleEvent<Id> {
pub fn new(id: Id, status: impl Into<Status>) -> Self {
Self {
id,
Expand All @@ -44,14 +42,23 @@ impl<Id: Into<IdBox> + Debug + Clone + Eq + Ord> IdTrait for SimpleEvent<Id> {
}

/// Account event
#[derive(
Clone, PartialEq, Eq, Debug, Decode, Encode, Deserialize, Serialize, FromVariant, IntoSchema,
)]
#[derive(Clone, PartialEq, Eq, Debug, Decode, Encode, Deserialize, Serialize, IntoSchema)]
#[non_exhaustive]
pub enum AccountEvent {
/// Account change without asset changing
StatusUpdated(AccountStatusUpdated),
/// Asset change
Asset(AssetEvent),
/// Account registration
Created(AccountId),
/// Account deleting
Deleted(AccountId),
/// Authentication event
Authentication(AccountId),
/// Permission update
Permission(AccountId),
/// Metadata was inserted
MetadataInserted(AccountId),
/// Metadata was removed
MetadataRemoved(AccountId),
}

impl Identifiable for AccountEvent {
Expand All @@ -61,23 +68,33 @@ impl Identifiable for AccountEvent {
impl IdTrait for AccountEvent {
fn id(&self) -> &AccountId {
match self {
Self::StatusUpdated(change) => change.id(),
Self::Asset(asset) => &asset.id().account_id,
Self::Created(id) => &id,
Self::Deleted(id) => &id,
Self::Authentication(id) => &id,
Self::Permission(id) => &id,
Self::MetadataInserted(id) => &id,
Self::MetadataRemoved(id) => &id,
}
}
}

/// Domain Event
#[derive(
Clone, PartialEq, Eq, Debug, Decode, Encode, Deserialize, Serialize, FromVariant, IntoSchema,
)]
#[derive(Clone, PartialEq, Eq, Debug, Decode, Encode, Deserialize, Serialize, IntoSchema)]
#[non_exhaustive]
pub enum DomainEvent {
/// Domain change without account or asset definition change
StatusUpdated(DomainStatusUpdated),
/// Account change
Account(AccountEvent),
/// Asset definition change
AssetDefinition(AssetDefinitionEvent),
/// Domain registration
Created(DomainId),
/// Domain deleting
Deleted(DomainId),
/// Metadata was inserted
MetadataInserted(DomainId),
/// Metadata was removed
MetadataRemoved(DomainId),
}

impl Identifiable for DomainEvent {
Expand All @@ -87,9 +104,12 @@ impl Identifiable for DomainEvent {
impl IdTrait for DomainEvent {
fn id(&self) -> &DomainId {
match self {
Self::StatusUpdated(change) => change.id(),
Self::Account(account) => &account.id().domain_id,
Self::AssetDefinition(asset_definition) => &asset_definition.id().domain_id,
Self::Created(id) => &id,
Self::Deleted(id) => &id,
Self::MetadataInserted(id) => &id,
Self::MetadataRemoved(id) => &id,
}
}
}
Expand Down
Loading

0 comments on commit dd98e1a

Please sign in to comment.