From 5d61f1eba7cf908bb4f91228066280ff88dd5caa Mon Sep 17 00:00:00 2001 From: iorveth Date: Fri, 27 Nov 2020 23:00:39 +0200 Subject: [PATCH] Introduce TransactionFailed event and adjust related core logic & test coverage --- runtime-modules/content-directory/src/lib.rs | 147 ++++++++++++------ runtime-modules/content-directory/src/mock.rs | 3 +- .../content-directory/src/tests.rs | 2 +- .../src/tests/add_class_schema.rs | 2 +- .../src/tests/add_curator_group.rs | 2 +- .../src/tests/add_curator_to_group.rs | 2 +- .../src/tests/add_maintainer_to_class.rs | 2 +- .../src/tests/clear_entity_property_vector.rs | 2 +- .../src/tests/create_class.rs | 2 +- .../src/tests/create_entity.rs | 2 +- .../tests/insert_at_entity_property_vector.rs | 2 +- .../tests/remove_at_entity_property_vector.rs | 2 +- .../src/tests/remove_curator_from_group.rs | 2 +- .../src/tests/remove_curator_group.rs | 2 +- .../src/tests/remove_entity.rs | 2 +- .../src/tests/remove_maintainer_from_class.rs | 2 +- .../src/tests/set_curator_group_status.rs | 2 +- .../src/tests/transaction.rs | 57 ++++++- .../src/tests/transfer_entity_ownership.rs | 2 +- .../src/tests/update_class_permissions.rs | 2 +- .../src/tests/update_class_schema_status.rs | 2 +- .../tests/update_entity_creation_voucher.rs | 4 +- .../src/tests/update_entity_permissions.rs | 2 +- .../tests/update_entity_property_values.rs | 2 +- 24 files changed, 179 insertions(+), 72 deletions(-) diff --git a/runtime-modules/content-directory/src/lib.rs b/runtime-modules/content-directory/src/lib.rs index bd389065a3..e66e2e4a6c 100755 --- a/runtime-modules/content-directory/src/lib.rs +++ b/runtime-modules/content-directory/src/lib.rs @@ -142,7 +142,11 @@ use codec::{Codec, Decode, Encode}; use frame_support::storage::IterableStorageMap; use frame_support::{ - decl_event, decl_module, decl_storage, dispatch::DispatchResult, ensure, traits::Get, Parameter, + decl_event, decl_module, decl_storage, + dispatch::{DispatchError, DispatchResult}, + ensure, + traits::Get, + Parameter, }; #[cfg(feature = "std")] pub use serde::{Deserialize, Serialize}; @@ -1625,64 +1629,115 @@ decl_module! { Ok(()) } - /// Batch transaction - #[weight = 10_000_000] // TODO: adjust weight - pub fn transaction(origin, actor: Actor, operations: Vec>) -> DispatchResult { + /// Batch transaction + #[weight = 10_000_000] // TODO: adjust weight + pub fn transaction(origin, actor: Actor, operations: Vec>) -> DispatchResult { - // Ensure maximum number of operations during atomic batching limit not reached - Self::ensure_number_of_operations_during_atomic_batching_limit_not_reached(&operations)?; + // Ensure maximum number of operations during atomic batching limit not reached + Self::ensure_number_of_operations_during_atomic_batching_limit_not_reached(&operations)?; - // - // == MUTATION SAFE == - // + // + // == MUTATION SAFE == + // - // This BTreeMap holds the T::EntityId of the entity created as a result of executing a `CreateEntity` `Operation` - let mut entity_created_in_operation = BTreeMap::new(); + // This BTreeMap holds the T::EntityId of the entity created as a result of executing a `CreateEntity` `Operation` + let mut entity_created_in_operation = BTreeMap::new(); - // Create raw origin - let raw_origin = origin.into().map_err(|_| Error::::OriginCanNotBeMadeIntoRawOrigin)?; + // Create raw origin + let raw_origin = origin.into().map_err(|_| Error::::OriginCanNotBeMadeIntoRawOrigin)?; - for (index, operation_type) in operations.into_iter().enumerate() { - let origin = T::Origin::from(raw_origin.clone()); - match operation_type { - OperationType::CreateEntity(create_entity_operation) => { - Self::create_entity(origin, create_entity_operation.class_id, actor)?; + for (index, operation_type) in operations.into_iter().enumerate() { + let origin = T::Origin::from(raw_origin.clone()); + match operation_type { + OperationType::CreateEntity(create_entity_operation) => { + Self::ensure_transaction_failed_event( + Self::create_entity(origin, create_entity_operation.class_id, actor), + actor, + index + )?; // entity id of newly created entity let entity_id = Self::next_entity_id() - T::EntityId::one(); entity_created_in_operation.insert(index, entity_id); - }, - OperationType::AddSchemaSupportToEntity(add_schema_support_to_entity_operation) => { - let entity_id = operations::parametrized_entity_to_entity_id( - &entity_created_in_operation, add_schema_support_to_entity_operation.entity_id + }, + OperationType::AddSchemaSupportToEntity(add_schema_support_to_entity_operation) => { + let entity_id = + Self::ensure_transaction_failed_event( + operations::parametrized_entity_to_entity_id( + &entity_created_in_operation, add_schema_support_to_entity_operation.entity_id + ), + actor, + index + )?; + + let schema_id = add_schema_support_to_entity_operation.schema_id; + + let property_values = + Self::ensure_transaction_failed_event( + operations::parametrized_property_values_to_property_values( + &entity_created_in_operation, add_schema_support_to_entity_operation.parametrized_property_values + ), + actor, + index + )?; + Self::ensure_transaction_failed_event( + Self::add_schema_support_to_entity(origin, actor, entity_id, schema_id, property_values), + actor, + index )?; - let schema_id = add_schema_support_to_entity_operation.schema_id; - let property_values = operations::parametrized_property_values_to_property_values( - &entity_created_in_operation, add_schema_support_to_entity_operation.parametrized_property_values - )?; - Self::add_schema_support_to_entity(origin, actor, entity_id, schema_id, property_values)?; - }, - OperationType::UpdatePropertyValues(update_property_values_operation) => { - let entity_id = operations::parametrized_entity_to_entity_id( - &entity_created_in_operation, update_property_values_operation.entity_id - )?; - let property_values = operations::parametrized_property_values_to_property_values( - &entity_created_in_operation, update_property_values_operation.new_parametrized_property_values - )?; - Self::update_entity_property_values(origin, actor, entity_id, property_values)?; - }, - } - } - - // Trigger event - Self::deposit_event(RawEvent::TransactionCompleted(actor)); - - Ok(()) - } + }, + OperationType::UpdatePropertyValues(update_property_values_operation) => { + let entity_id = + Self::ensure_transaction_failed_event( + operations::parametrized_entity_to_entity_id( + &entity_created_in_operation, update_property_values_operation.entity_id + ), + actor, + index + )?; + + let property_values = + Self::ensure_transaction_failed_event( + operations::parametrized_property_values_to_property_values( + &entity_created_in_operation, update_property_values_operation.new_parametrized_property_values + ), + actor, + index + )?; + + Self::ensure_transaction_failed_event( + Self::update_entity_property_values(origin, actor, entity_id, property_values), + actor, + index + )?; + }, + } + } + + // Trigger event + Self::deposit_event(RawEvent::TransactionCompleted(actor)); + + Ok(()) + } } } impl Module { + /// Deposits an `TransactionFailed` event if an error during `transaction` extrinsic execution occured + fn ensure_transaction_failed_event>( + result: Result, + actor: Actor, + index: usize, + ) -> Result { + match result { + Err(e) => { + Self::deposit_event(RawEvent::TransactionFailed(actor, index as u32)); + Err(e.into()) + } + Ok(result) => Ok(result), + } + } + /// Updates corresponding `Entity` `reference_counter` by `reference_counter_delta`. fn update_entity_rc( entity_id: T::EntityId, @@ -2827,6 +2882,7 @@ decl_event!( Nonce = ::Nonce, SideEffects = Option>, SideEffect = Option<(::EntityId, EntityReferenceCounterSideEffect)>, + FailedAt = u32, { CuratorGroupAdded(CuratorGroupId), CuratorGroupRemoved(CuratorGroupId), @@ -2851,5 +2907,6 @@ decl_event!( InsertedAtVectorIndex(Actor, EntityId, PropertyId, VecMaxLength, Nonce, SideEffect), EntityOwnershipTransfered(EntityId, EntityController, SideEffects), TransactionCompleted(Actor), + TransactionFailed(Actor, FailedAt), } ); diff --git a/runtime-modules/content-directory/src/mock.rs b/runtime-modules/content-directory/src/mock.rs index 51a99eaab5..07f6c56f0c 100644 --- a/runtime-modules/content-directory/src/mock.rs +++ b/runtime-modules/content-directory/src/mock.rs @@ -433,13 +433,14 @@ type RawTestEvent = RawEvent< Nonce, Option>, Option<(EntityId, EntityReferenceCounterSideEffect)>, + u32, >; pub fn get_test_event(raw_event: RawTestEvent) -> TestEvent { TestEvent::test_events(raw_event) } -pub fn assert_event_success(tested_event: TestEvent, number_of_events_after_call: usize) { +pub fn assert_event(tested_event: TestEvent, number_of_events_after_call: usize) { // Ensure runtime events length is equal to expected number of events after call assert_eq!(System::events().len(), number_of_events_after_call); diff --git a/runtime-modules/content-directory/src/tests.rs b/runtime-modules/content-directory/src/tests.rs index 24f8aab0cf..930c0bc7ba 100644 --- a/runtime-modules/content-directory/src/tests.rs +++ b/runtime-modules/content-directory/src/tests.rs @@ -145,7 +145,7 @@ pub fn add_entity_schemas_support() -> ( )); // Last event checked - assert_event_success( + assert_event( entity_schema_support_added_event, number_of_events_before_calls + 2, ); diff --git a/runtime-modules/content-directory/src/tests/add_class_schema.rs b/runtime-modules/content-directory/src/tests/add_class_schema.rs index 70b8532d63..6f1f55f349 100644 --- a/runtime-modules/content-directory/src/tests/add_class_schema.rs +++ b/runtime-modules/content-directory/src/tests/add_class_schema.rs @@ -52,7 +52,7 @@ fn add_class_schema_success() { get_test_event(RawEvent::ClassSchemaAdded(FIRST_CLASS_ID, SECOND_SCHEMA_ID)); // Last event checked - assert_event_success(class_schema_added_event, number_of_events_before_call + 2); + assert_event(class_schema_added_event, number_of_events_before_call + 2); }) } diff --git a/runtime-modules/content-directory/src/tests/add_curator_group.rs b/runtime-modules/content-directory/src/tests/add_curator_group.rs index dbedef6395..eab59e0071 100644 --- a/runtime-modules/content-directory/src/tests/add_curator_group.rs +++ b/runtime-modules/content-directory/src/tests/add_curator_group.rs @@ -31,7 +31,7 @@ fn add_curator_group_success() { get_test_event(RawEvent::CuratorGroupAdded(FIRST_CURATOR_GROUP_ID)); // Event checked - assert_event_success( + assert_event( curator_group_created_event, number_of_events_before_call + 1, ); diff --git a/runtime-modules/content-directory/src/tests/add_curator_to_group.rs b/runtime-modules/content-directory/src/tests/add_curator_to_group.rs index eac7817a9f..e62f522510 100644 --- a/runtime-modules/content-directory/src/tests/add_curator_to_group.rs +++ b/runtime-modules/content-directory/src/tests/add_curator_to_group.rs @@ -31,7 +31,7 @@ fn add_curator_to_group_success() { )); // Event checked - assert_event_success( + assert_event( curator_group_curator_added_event, number_of_events_before_call + 1, ); diff --git a/runtime-modules/content-directory/src/tests/add_maintainer_to_class.rs b/runtime-modules/content-directory/src/tests/add_maintainer_to_class.rs index 11c89b1f17..60c62da1fc 100644 --- a/runtime-modules/content-directory/src/tests/add_maintainer_to_class.rs +++ b/runtime-modules/content-directory/src/tests/add_maintainer_to_class.rs @@ -35,7 +35,7 @@ fn add_maintainer_to_class_success() { )); // Event checked - assert_event_success(maintainer_added_event, number_of_events_before_call + 1); + assert_event(maintainer_added_event, number_of_events_before_call + 1); }) } diff --git a/runtime-modules/content-directory/src/tests/clear_entity_property_vector.rs b/runtime-modules/content-directory/src/tests/clear_entity_property_vector.rs index 3d12fa7ebd..101d0980d2 100644 --- a/runtime-modules/content-directory/src/tests/clear_entity_property_vector.rs +++ b/runtime-modules/content-directory/src/tests/clear_entity_property_vector.rs @@ -53,7 +53,7 @@ fn clear_entity_property_vector_success() { )); // Last event checked - assert_event_success( + assert_event( entity_property_vector_cleared_event, number_of_events_before_calls + 1, ); diff --git a/runtime-modules/content-directory/src/tests/create_class.rs b/runtime-modules/content-directory/src/tests/create_class.rs index f05f3cf605..c84d270871 100644 --- a/runtime-modules/content-directory/src/tests/create_class.rs +++ b/runtime-modules/content-directory/src/tests/create_class.rs @@ -23,7 +23,7 @@ fn create_class_success() { let class_created_event = get_test_event(RawEvent::ClassCreated(FIRST_CLASS_ID)); // Event checked - assert_event_success(class_created_event, number_of_events_before_call + 1); + assert_event(class_created_event, number_of_events_before_call + 1); }) } diff --git a/runtime-modules/content-directory/src/tests/create_entity.rs b/runtime-modules/content-directory/src/tests/create_entity.rs index 5a6f13e72d..b0e5386f1b 100644 --- a/runtime-modules/content-directory/src/tests/create_entity.rs +++ b/runtime-modules/content-directory/src/tests/create_entity.rs @@ -71,7 +71,7 @@ fn create_entity_success() { get_test_event(RawEvent::EntityCreated(actor, next_entity_id() - 1)); // Last event checked - assert_event_success(entity_created_event, number_of_events_before_call + 1); + assert_event(entity_created_event, number_of_events_before_call + 1); }) } diff --git a/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs b/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs index af52a88118..ed251b03cc 100644 --- a/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs +++ b/runtime-modules/content-directory/src/tests/insert_at_entity_property_vector.rs @@ -64,7 +64,7 @@ fn insert_at_entity_property_vector_success() { )); // Last event checked - assert_event_success( + assert_event( inserted_at_vector_index_event, number_of_events_before_calls + 1, ); diff --git a/runtime-modules/content-directory/src/tests/remove_at_entity_property_vector.rs b/runtime-modules/content-directory/src/tests/remove_at_entity_property_vector.rs index 677bcc802c..bf5b66109b 100644 --- a/runtime-modules/content-directory/src/tests/remove_at_entity_property_vector.rs +++ b/runtime-modules/content-directory/src/tests/remove_at_entity_property_vector.rs @@ -59,7 +59,7 @@ fn remove_at_entity_property_vector_success() { )); // Last event checked - assert_event_success( + assert_event( removed_at_vector_index_event, number_of_events_before_calls + 1, ); diff --git a/runtime-modules/content-directory/src/tests/remove_curator_from_group.rs b/runtime-modules/content-directory/src/tests/remove_curator_from_group.rs index e0e99098a3..ef779bb9eb 100644 --- a/runtime-modules/content-directory/src/tests/remove_curator_from_group.rs +++ b/runtime-modules/content-directory/src/tests/remove_curator_from_group.rs @@ -44,7 +44,7 @@ fn remove_curator_from_group_success() { )); // Event checked - assert_event_success( + assert_event( curator_group_curator_removed_event, number_of_events_before_call + 1, ); diff --git a/runtime-modules/content-directory/src/tests/remove_curator_group.rs b/runtime-modules/content-directory/src/tests/remove_curator_group.rs index a61cbe54c5..6be51feb13 100644 --- a/runtime-modules/content-directory/src/tests/remove_curator_group.rs +++ b/runtime-modules/content-directory/src/tests/remove_curator_group.rs @@ -24,7 +24,7 @@ fn remove_curator_group_success() { get_test_event(RawEvent::CuratorGroupRemoved(FIRST_CURATOR_GROUP_ID)); // Event checked - assert_event_success( + assert_event( curator_group_removed_event, number_of_events_before_call + 1, ); diff --git a/runtime-modules/content-directory/src/tests/remove_entity.rs b/runtime-modules/content-directory/src/tests/remove_entity.rs index fe8c2dc37d..1300b609e7 100644 --- a/runtime-modules/content-directory/src/tests/remove_entity.rs +++ b/runtime-modules/content-directory/src/tests/remove_entity.rs @@ -38,7 +38,7 @@ fn remove_entity_success() { get_test_event(RawEvent::EntityRemoved(actor, next_entity_id() - 1)); // Last event checked - assert_event_success(entity_removed_event, number_of_events_before_call + 1); + assert_event(entity_removed_event, number_of_events_before_call + 1); }) } diff --git a/runtime-modules/content-directory/src/tests/remove_maintainer_from_class.rs b/runtime-modules/content-directory/src/tests/remove_maintainer_from_class.rs index 6b99070802..fba5ae3bdb 100644 --- a/runtime-modules/content-directory/src/tests/remove_maintainer_from_class.rs +++ b/runtime-modules/content-directory/src/tests/remove_maintainer_from_class.rs @@ -54,7 +54,7 @@ fn remove_maintainer_from_class_success() { )); // Event checked - assert_event_success(maintainer_removed_event, number_of_events_before_call + 1); + assert_event(maintainer_removed_event, number_of_events_before_call + 1); }) } diff --git a/runtime-modules/content-directory/src/tests/set_curator_group_status.rs b/runtime-modules/content-directory/src/tests/set_curator_group_status.rs index 92dc040e4f..c223dbdf35 100644 --- a/runtime-modules/content-directory/src/tests/set_curator_group_status.rs +++ b/runtime-modules/content-directory/src/tests/set_curator_group_status.rs @@ -31,7 +31,7 @@ fn set_curator_group_status_success() { )); // Event checked - assert_event_success( + assert_event( curator_group_status_set_event, number_of_events_before_call + 1, ); diff --git a/runtime-modules/content-directory/src/tests/transaction.rs b/runtime-modules/content-directory/src/tests/transaction.rs index 778c609f61..56dbf8e98c 100644 --- a/runtime-modules/content-directory/src/tests/transaction.rs +++ b/runtime-modules/content-directory/src/tests/transaction.rs @@ -62,12 +62,11 @@ fn transaction_success() { // Runtime tested state after call - let entity_ownership_transfered_event = - get_test_event(RawEvent::TransactionCompleted(actor)); + let transaction_completed_event = get_test_event(RawEvent::TransactionCompleted(actor)); // Last event checked - assert_event_success( - entity_ownership_transfered_event, + assert_event( + transaction_completed_event, number_of_events_before_calls + operations_count + 1, ); }) @@ -104,3 +103,53 @@ fn transaction_limit_reached() { ); }) } + +#[test] +fn transaction_failed() { + with_test_externalities(|| { + // Create class with default permissions + assert_ok!(create_simple_class(LEAD_ORIGIN, ClassType::Valid)); + + let operation = OperationType::CreateEntity(CreateEntityOperation { + class_id: FIRST_CLASS_ID, + }); + + let failed_operation = OperationType::CreateEntity(CreateEntityOperation { + class_id: UNKNOWN_CLASS_ID, + }); + + let operations = vec![ + operation.clone(), + operation.clone(), + failed_operation, + operation, + ]; + + // Runtime state before tested call + + // Events number before tested call + let number_of_events_before_call = System::events().len(); + + let actor = Actor::Lead; + + // Make an attempt to complete transaction with CreateEntity operation, when provided class_id does not exist on runtime level + let transaction_result = transaction(LEAD_ORIGIN, actor, operations.clone()); + + let failed_operation_index = 2; + + // Failure checked + + // Ensure call result is equal to expected error + assert_err!(transaction_result, Error::::ClassNotFound); + + let transaction_failed_event = + get_test_event(RawEvent::TransactionFailed(actor, failed_operation_index)); + + // Last event checked + assert_event( + transaction_failed_event, + // two operations succeded and one TransactionFailed event + number_of_events_before_call + operations[..failed_operation_index as usize].len() + 1, + ); + }) +} diff --git a/runtime-modules/content-directory/src/tests/transfer_entity_ownership.rs b/runtime-modules/content-directory/src/tests/transfer_entity_ownership.rs index e41ac018b8..0d18dbb035 100644 --- a/runtime-modules/content-directory/src/tests/transfer_entity_ownership.rs +++ b/runtime-modules/content-directory/src/tests/transfer_entity_ownership.rs @@ -51,7 +51,7 @@ fn transfer_entity_ownership_success() { ); // Last event checked - assert_event_success( + assert_event( entity_ownership_transfered_event, number_of_events_before_calls + 1, ); diff --git a/runtime-modules/content-directory/src/tests/update_class_permissions.rs b/runtime-modules/content-directory/src/tests/update_class_permissions.rs index 7560a8b6d7..1d22b30561 100644 --- a/runtime-modules/content-directory/src/tests/update_class_permissions.rs +++ b/runtime-modules/content-directory/src/tests/update_class_permissions.rs @@ -103,7 +103,7 @@ fn update_class_permissions_success() { ); // Event checked - assert_event_success( + assert_event( class_permissions_updated_event, number_of_events_before_call + 2, ); diff --git a/runtime-modules/content-directory/src/tests/update_class_schema_status.rs b/runtime-modules/content-directory/src/tests/update_class_schema_status.rs index dfca46d9c1..5edc50e211 100644 --- a/runtime-modules/content-directory/src/tests/update_class_schema_status.rs +++ b/runtime-modules/content-directory/src/tests/update_class_schema_status.rs @@ -48,7 +48,7 @@ fn update_class_schema_status_success() { )); // Last event checked - assert_event_success( + assert_event( class_schema_status_updated_event, number_of_events_before_call + 1, ); diff --git a/runtime-modules/content-directory/src/tests/update_entity_creation_voucher.rs b/runtime-modules/content-directory/src/tests/update_entity_creation_voucher.rs index 25c70588f4..7f360950af 100644 --- a/runtime-modules/content-directory/src/tests/update_entity_creation_voucher.rs +++ b/runtime-modules/content-directory/src/tests/update_entity_creation_voucher.rs @@ -40,7 +40,7 @@ fn create_entity_creation_voucher_success() { ); // Last event checked - assert_event_success( + assert_event( entity_creation_voucher_created_event, number_of_events_before_call + 1, ); @@ -101,7 +101,7 @@ fn update_entity_creation_voucher_success() { ); // Last event checked - assert_event_success( + assert_event( entity_creation_voucher_created_event, number_of_events_before_call + 1, ); diff --git a/runtime-modules/content-directory/src/tests/update_entity_permissions.rs b/runtime-modules/content-directory/src/tests/update_entity_permissions.rs index e49d30a15a..74aa7ea1b1 100644 --- a/runtime-modules/content-directory/src/tests/update_entity_permissions.rs +++ b/runtime-modules/content-directory/src/tests/update_entity_permissions.rs @@ -43,7 +43,7 @@ fn update_entity_permissions_success() { get_test_event(RawEvent::EntityPermissionsUpdated(FIRST_ENTITY_ID)); // Last event checked - assert_event_success( + assert_event( entity_permissions_updated_event, number_of_events_before_call + 1, ); diff --git a/runtime-modules/content-directory/src/tests/update_entity_property_values.rs b/runtime-modules/content-directory/src/tests/update_entity_property_values.rs index bab34cd726..b66678df46 100644 --- a/runtime-modules/content-directory/src/tests/update_entity_property_values.rs +++ b/runtime-modules/content-directory/src/tests/update_entity_property_values.rs @@ -55,7 +55,7 @@ fn update_entity_property_values_success() { ); // Last event checked - assert_event_success( + assert_event( entity_property_values_updated_event, number_of_events_before_calls + 1, );