refactor: apply suggestions from code review

- move `schema` in `struct ExecutorDataModel` lower
- rename `PermissionToken` to `Permission` everywhere, lots of changes
  - also rename `iroha_executor::permission::Token` to
    `iroha_executor::permission::Permission`, in compliance with
    `Parameter` trait. Also renamed the same-name derive macro
- use just `id`, not `definition_id`
- move `trait ExecutorDataModelObject` into `iroha_smart_contract`

Co-authored-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: 0x009922 <43530070+0x009922@users.noreply.github.com>

client/benches/tps/utils.rs

@@ -44,7 +44,7 @@ impl Config {
 
     pub fn measure(self) -> Result<Tps> {
         // READY
-        let (_rt, network, client) = Network::start_test_with_runtime(
+        let (_rt, network, _client) = Network::start_test_with_runtime(
             NetworkOptions::with_n_peers(self.peers)
                 .with_max_txs_in_block(self.max_txs_per_block.try_into().unwrap()),
         );

client/src/client.rs

@@ -1517,10 +1517,10 @@ pub mod permission {
     //! Module with queries for permission tokens
     use super::*;
 
-    /// Construct a query to get all [`PermissionToken`] granted
+    /// Construct a query to get all [`Permission`] granted
     /// to account with given [`Id`][AccountId]
-    pub fn by_account_id(account_id: AccountId) -> FindPermissionTokensByAccountId {
-        FindPermissionTokensByAccountId::new(account_id)
+    pub fn by_account_id(account_id: AccountId) -> FindPermissionsByAccountId {
+        FindPermissionsByAccountId::new(account_id)
     }
 }
 

client/tests/integration/asset.rs

@@ -296,7 +296,7 @@ fn find_rate_and_make_exchange_isi_should_succeed() {
     let alice_id = ALICE_ID.clone();
     let alice_can_transfer_asset = |asset_id: AssetId, owner_key_pair: KeyPair| {
         let instruction = Grant::permission(
-            PermissionToken::new(
+            Permission::new(
                 "CanTransferUserAsset".parse().unwrap(),
                 &json!({ "asset_id": asset_id }),
             ),

client/tests/integration/domain_owner_permissions.rs

@@ -46,7 +46,7 @@ fn domain_owner_domain_permissions() -> Result<()> {
     test_client.submit_blocking(Unregister::asset_definition(coin_id))?;
 
     // Granting a respective token also allows "bob@kingdom" to do so
-    let token = PermissionToken::new(
+    let token = Permission::new(
         "CanRegisterAssetDefinitionInDomain".parse().unwrap(),
         &json!({ "domain_id": kingdom_id }),
     );
@@ -64,7 +64,7 @@ fn domain_owner_domain_permissions() -> Result<()> {
     test_client.submit_blocking(RemoveKeyValue::domain(kingdom_id.clone(), key))?;
 
     // check that "alice@wonderland" as owner of domain can grant and revoke domain related permission tokens
-    let token = PermissionToken::new(
+    let token = Permission::new(
         "CanUnregisterDomain".parse().unwrap(),
         &json!({ "domain_id": kingdom_id }),
     );
@@ -104,7 +104,7 @@ fn domain_owner_account_permissions() -> Result<()> {
 
     // check that "alice@wonderland" as owner of domain can grant and revoke account related permission tokens in her domain
     let bob_id = BOB_ID.clone();
-    let token = PermissionToken::new(
+    let token = Permission::new(
         "CanUnregisterAccount".parse().unwrap(),
         &json!({ "account_id": mad_hatter_id }),
     );
@@ -139,7 +139,7 @@ fn domain_owner_asset_definition_permissions() -> Result<()> {
     test_client.submit_blocking(Register::account(rabbit))?;
 
     // Grant permission to register asset definitions to "bob@kingdom"
-    let token = PermissionToken::new(
+    let token = Permission::new(
         "CanRegisterAssetDefinitionInDomain".parse().unwrap(),
         &json!({ "domain_id": kingdom_id }),
     );
@@ -170,7 +170,7 @@ fn domain_owner_asset_definition_permissions() -> Result<()> {
     test_client.submit_blocking(RemoveKeyValue::asset_definition(coin_id.clone(), key))?;
 
     // check that "alice@wonderland" as owner of domain can grant and revoke asset definition related permission tokens in her domain
-    let token = PermissionToken::new(
+    let token = Permission::new(
         "CanUnregisterAssetDefinition".parse().unwrap(),
         &json!({ "asset_definition_id": coin_id }),
     );
@@ -204,7 +204,7 @@ fn domain_owner_asset_permissions() -> Result<()> {
     test_client.submit_blocking(Register::account(bob))?;
 
     // Grant permission to register asset definitions to "bob@kingdom"
-    let token = PermissionToken::new(
+    let token = Permission::new(
         "CanRegisterAssetDefinitionInDomain".parse().unwrap(),
         &json!({ "domain_id": kingdom_id }),
     );
@@ -240,7 +240,7 @@ fn domain_owner_asset_permissions() -> Result<()> {
     test_client.submit_blocking(RemoveKeyValue::asset(bob_store_id.clone(), key))?;
 
     // check that "alice@wonderland" as owner of domain can grant and revoke asset related permission tokens in her domain
-    let token = PermissionToken::new(
+    let token = Permission::new(
         "CanUnregisterUserAsset".parse().unwrap(),
         &json!({ "asset_id": bob_store_id }),
     );
@@ -291,7 +291,7 @@ fn domain_owner_trigger_permissions() -> Result<()> {
     let _result = test_client.submit_blocking(execute_trigger)?;
 
     // check that "alice@wonderland" as owner of domain can grant and revoke trigger related permission tokens in her domain
-    let token = PermissionToken::new(
+    let token = Permission::new(
         "CanUnregisterUserTrigger".parse().unwrap(),
         &json!({ "trigger_id": trigger_id }),
     );

client/tests/integration/events/data.rs

@@ -199,11 +199,11 @@ fn produce_multiple_events() -> Result<()> {
     // Registering role
     let alice_id = ALICE_ID.clone();
     let role_id = RoleId::from_str("TEST_ROLE")?;
-    let token_1 = PermissionToken::new(
+    let token_1 = Permission::new(
         "CanRemoveKeyValueInAccount".parse()?,
         &json!({ "account_id": alice_id }),
     );
-    let token_2 = PermissionToken::new(
+    let token_2 = Permission::new(
         "CanSetKeyValueInAccount".parse()?,
         &json!({ "account_id": alice_id }),
     );
@@ -240,13 +240,13 @@ fn produce_multiple_events() -> Result<()> {
         DataEvent::Domain(DomainEvent::Account(AccountEvent::PermissionAdded(
             AccountPermissionChanged {
                 account_id: bob_id.clone(),
-                permission_id: token_1.definition_id.clone(),
+                permission_id: token_1.id.clone(),
             },
         ))),
         DataEvent::Domain(DomainEvent::Account(AccountEvent::PermissionAdded(
             AccountPermissionChanged {
                 account_id: bob_id.clone(),
-                permission_id: token_2.definition_id.clone(),
+                permission_id: token_2.id.clone(),
             },
         ))),
         DataEvent::Domain(DomainEvent::Account(AccountEvent::RoleGranted(
@@ -258,13 +258,13 @@ fn produce_multiple_events() -> Result<()> {
         DataEvent::Domain(DomainEvent::Account(AccountEvent::PermissionRemoved(
             AccountPermissionChanged {
                 account_id: bob_id.clone(),
-                permission_id: token_1.definition_id,
+                permission_id: token_1.id,
             },
         ))),
         DataEvent::Domain(DomainEvent::Account(AccountEvent::PermissionRemoved(
             AccountPermissionChanged {
                 account_id: bob_id.clone(),
-                permission_id: token_2.definition_id,
+                permission_id: token_2.id,
             },
         ))),
         DataEvent::Domain(DomainEvent::Account(AccountEvent::RoleRevoked(

client/tests/integration/parameters.rs

@@ -1,4 +1,4 @@
-use std::{collections::BTreeSet, str::FromStr};
+use std::collections::BTreeSet;
 
 use eyre::Result;
 use iroha_client::{client, data_model::prelude::*};
@@ -13,42 +13,40 @@ fn playing_with_custom_parameter() -> Result<()> {
     assert!(
         client
             .request(client::executor::data_model())?
-            .parameter_ids()
+            .parameters()
             .is_empty(),
         "existing parameters should be empty"
     );
 
+    // Registering some domain while there is no prefix requirement
+    client
+        .submit_blocking(Register::domain(Domain::new("axolotl".parse().unwrap())))
+        .expect("should be fine");
+
     let _err = client
         .submit_blocking(SetParameter::new(Parameter::new(
             "Whatever".parse().unwrap(),
             json!({ "foo": "bar" }),
         )))
         .expect_err("should not allow setting unknown parameters");
 
-    // Registering some domain while there is no prefix requirement
-    client
-        .submit_blocking(Register::domain(Domain::new("axolotl".parse().unwrap())))
-        .expect("should be fine");
-
     super::upgrade::upgrade_executor(
         &client,
         "tests/integration/smartcontracts/executor_with_custom_parameter",
     )?;
 
     assert_eq!(
-        *client
-            .request(client::executor::data_model())?
-            .parameter_ids(),
+        *client.request(client::executor::data_model())?.parameters(),
         ["EnforceDomainPrefix"]
             .into_iter()
             .map(|x| ParameterId::new(x.parse().unwrap()))
             .collect::<BTreeSet<_>>(),
-        "data model should have these parameters after upgrade"
+        "data model should have this set of parameters after upgrade"
     );
 
     let parameter = Parameter::new(
         "EnforceDomainPrefix".parse().unwrap(),
-        json!({ "prefix": "disnay_" }),
+        json!({ "prefix": "disney_" }),
     );
 
     client
@@ -73,11 +71,11 @@ fn playing_with_custom_parameter() -> Result<()> {
 
     let _err = client
         .submit_blocking(Register::domain(Domain::new("land".parse().unwrap())))
-        .expect_err("should fail since `land` is not prefixed with `disnay_`");
+        .expect_err("should fail since `land` is not prefixed with `disney_`");
 
     client
         .submit_blocking(Register::domain(Domain::new(
-            "disnay_land".parse().unwrap(),
+            "disney_land".parse().unwrap(),
         )))
         .expect("should be fine, since we used prefix according to the parameter");
 

client/tests/integration/permissions.rs

@@ -7,7 +7,7 @@ use iroha_client::{
     data_model::prelude::*,
 };
 use iroha_data_model::{
-    permission::PermissionToken, role::RoleId, transaction::error::TransactionRejectionReason,
+    permission::Permission, role::RoleId, transaction::error::TransactionRejectionReason,
 };
 use iroha_genesis::GenesisNetwork;
 use serde_json::json;
@@ -22,7 +22,7 @@ fn genesis_transactions_are_validated() {
     // Setting up genesis
 
     let genesis = GenesisNetwork::test_with_instructions([Grant::permission(
-        PermissionToken::new("InvalidToken".parse().unwrap(), &json!(null)),
+        Permission::new("InvalidToken".parse().unwrap(), &json!(null)),
         ALICE_ID.clone(),
     )
     .into()]);
@@ -230,7 +230,7 @@ fn permissions_differ_not_only_by_names() {
     // Granting permission to Alice to modify metadata in Mouse's hats
     let mouse_hat_id = AssetId::new(hat_definition_id, mouse_id.clone());
     let allow_alice_to_set_key_value_in_hats = Grant::permission(
-        PermissionToken::new(
+        Permission::new(
             "CanSetKeyValueInUserAsset".parse().unwrap(),
             &json!({ "asset_id": mouse_hat_id }),
         ),
@@ -266,7 +266,7 @@ fn permissions_differ_not_only_by_names() {
 
     // Granting permission to Alice to modify metadata in Mouse's shoes
     let allow_alice_to_set_key_value_in_shoes = Grant::permission(
-        PermissionToken::new(
+        Permission::new(
             "CanSetKeyValueInUserAsset".parse().unwrap(),
             &json!({ "asset_id": mouse_shoes_id }),
         ),
@@ -315,7 +315,7 @@ fn stored_vs_granted_token_payload() -> Result<()> {
     // Allow alice to mint mouse asset and mint initial value
     let mouse_asset = AssetId::new(asset_definition_id, mouse_id.clone());
     let allow_alice_to_set_key_value_in_mouse_asset = Grant::permission(
-        PermissionToken::new(
+        Permission::new(
             "CanSetKeyValueInUserAsset".parse().unwrap(),
             json!({ "asset_id": format!("xor#wonderland#{mouse_id}") }),
         ),
@@ -340,23 +340,23 @@ fn stored_vs_granted_token_payload() -> Result<()> {
 
 #[test]
 #[allow(deprecated)]
-fn permission_tokens_are_unified() {
+fn permissions_are_unified() {
     let (_rt, _peer, iroha_client) = <PeerBuilder>::new().with_port(11_230).start_with_runtime();
     wait_for_genesis_committed(&[iroha_client.clone()], 0);
 
     // Given
     let alice_id = ALICE_ID.clone();
 
     let allow_alice_to_transfer_rose_1 = Grant::permission(
-        PermissionToken::new(
+        Permission::new(
             "CanTransferUserAsset".parse().unwrap(),
             json!({ "asset_id": format!("rose#wonderland#{alice_id}") }),
         ),
         alice_id.clone(),
     );
 
     let allow_alice_to_transfer_rose_2 = Grant::permission(
-        PermissionToken::new(
+        Permission::new(
             "CanTransferUserAsset".parse().unwrap(),
             // different content, but same meaning
             json!({ "asset_id": format!("rose##{alice_id}") }),
@@ -374,7 +374,7 @@ fn permission_tokens_are_unified() {
 }
 
 #[test]
-fn associated_permission_tokens_removed_on_unregister() {
+fn associated_permissions_removed_on_unregister() {
     let (_rt, _peer, iroha_client) = <PeerBuilder>::new().with_port(11_240).start_with_runtime();
     wait_for_genesis_committed(&[iroha_client.clone()], 0);
 
@@ -384,7 +384,7 @@ fn associated_permission_tokens_removed_on_unregister() {
 
     // register kingdom and give bob permissions in this domain
     let register_domain = Register::domain(kingdom);
-    let bob_to_set_kv_in_domain_token = PermissionToken::new(
+    let bob_to_set_kv_in_domain_token = Permission::new(
         "CanSetKeyValueInDomain".parse().unwrap(),
         &json!({ "domain_id": kingdom_id }),
     );
@@ -401,7 +401,7 @@ fn associated_permission_tokens_removed_on_unregister() {
     // check that bob indeed have granted permission
     assert!(iroha_client
         .request(client::permission::by_account_id(bob_id.clone()))
-        .and_then(std::iter::Iterator::collect::<QueryResult<Vec<PermissionToken>>>)
+        .and_then(std::iter::Iterator::collect::<QueryResult<Vec<Permission>>>)
         .expect("failed to get permissions for bob")
         .into_iter()
         .any(|token| { token == bob_to_set_kv_in_domain_token }));
@@ -414,14 +414,14 @@ fn associated_permission_tokens_removed_on_unregister() {
     // check that permission is removed from bob
     assert!(iroha_client
         .request(client::permission::by_account_id(bob_id))
-        .and_then(std::iter::Iterator::collect::<QueryResult<Vec<PermissionToken>>>)
+        .and_then(std::iter::Iterator::collect::<QueryResult<Vec<Permission>>>)
         .expect("failed to get permissions for bob")
         .into_iter()
         .all(|token| { token != bob_to_set_kv_in_domain_token }));
 }
 
 #[test]
-fn associated_permission_tokens_removed_from_role_on_unregister() {
+fn associated_permissions_removed_from_role_on_unregister() {
     let (_rt, _peer, iroha_client) = <PeerBuilder>::new().with_port(11_255).start_with_runtime();
     wait_for_genesis_committed(&[iroha_client.clone()], 0);
 
@@ -431,7 +431,7 @@ fn associated_permission_tokens_removed_from_role_on_unregister() {
 
     // register kingdom and give bob permissions in this domain
     let register_domain = Register::domain(kingdom);
-    let set_kv_in_domain_token = PermissionToken::new(
+    let set_kv_in_domain_token = Permission::new(
         "CanSetKeyValueInDomain".parse().unwrap(),
         &json!({ "domain_id": kingdom_id }),
     );

client/tests/integration/queries/role.rs

@@ -131,7 +131,7 @@ fn find_roles_by_account_id() -> Result<()> {
         .iter()
         .cloned()
         .map(|role_id| {
-            Register::role(Role::new(role_id).add_permission(PermissionToken::new(
+            Register::role(Role::new(role_id).add_permission(Permission::new(
                 "CanSetKeyValueInAccount".parse().unwrap(),
                 &json!({ "account_id": alice_id }),
             )))