Skip to content

Commit

Permalink
refactor: apply suggestions from code review
Browse files Browse the repository at this point in the history
Signed-off-by: 0x009922 <43530070+0x009922@users.noreply.github.com>
  • Loading branch information
0x009922 committed May 24, 2024
1 parent 79e4916 commit c1fe4ab
Show file tree
Hide file tree
Showing 10 changed files with 45 additions and 35 deletions.
8 changes: 7 additions & 1 deletion client/tests/integration/permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use iroha_client::{
};
use iroha_data_model::{
permission::Permission, role::RoleId, transaction::error::TransactionRejectionReason,
JsonString,
};
use iroha_genesis::GenesisNetwork;
use serde_json::json;
Expand Down Expand Up @@ -317,7 +318,12 @@ fn stored_vs_granted_token_payload() -> Result<()> {
let allow_alice_to_set_key_value_in_mouse_asset = Grant::permission(
Permission::new(
"CanSetKeyValueInUserAsset".parse().unwrap(),
json!({ "asset_id": format!("xor#wonderland#{mouse_id}") }),
JsonString::from_json_string_unchecked(format!(
// Introducing some whitespaces
// This way, if the executor compares just JSON strings, this test would fail
r##"{{ "asset_id" : "xor#wonderland#{}" }}"##,
mouse_id
)),
),
alice_id,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ extern crate panic_halt;
use alloc::string::String;

use anyhow::anyhow;
use iroha_executor::{permission::Permission as _, prelude::*, DataModelBuilder};
use iroha_executor::{prelude::*, DataModelBuilder};
use iroha_schema::IntoSchema;
use lol_alloc::{FreeListAllocator, LockedAllocator};
use parity_scale_codec::{Decode, Encode};
Expand Down Expand Up @@ -118,7 +118,7 @@ impl Executor {
.iter()
.try_for_each(|(account, domain_id)| {
Revoke::permission(
PermissionObject::new(
Permission::new(
can_unregister_domain_definition_id.clone(),
&json!({ "domain_id": domain_id }),
),
Expand All @@ -137,10 +137,7 @@ impl Executor {
})?;

Grant::permission(
PermissionObject::new(
can_control_domain_lives_definition_id.clone(),
&json!(null),
),
Permission::new(can_control_domain_lives_definition_id.clone(), &json!(null)),
account.id().clone(),
)
.execute()
Expand Down
12 changes: 6 additions & 6 deletions client_cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ mod account {

use iroha_client::client::{self};

use super::{Permission as PermissionObject, *};
use super::*;

/// subcommands for account subcommand
#[derive(clap::Subcommand, Debug)]
Expand Down Expand Up @@ -603,22 +603,22 @@ mod account {
pub id: AccountId,
/// The JSON/JSON5 file with a permission token
#[arg(short, long)]
pub permission: Permission,
pub permission: PermissionWrap,
#[command(flatten)]
pub metadata: MetadataArgs,
}

/// [`PermissionObject`] wrapper implementing [`FromStr`]
/// [`Permission`] wrapper implementing [`FromStr`]
#[derive(Debug, Clone)]
pub struct Permission(PermissionObject);
pub struct PermissionWrap(Permission);

impl FromStr for Permission {
impl FromStr for PermissionWrap {
type Err = Error;

fn from_str(s: &str) -> Result<Self> {
let content = fs::read_to_string(s)
.wrap_err(format!("Failed to read the permission token file {}", &s))?;
let permission: PermissionObject = json5::from_str(&content).wrap_err(format!(
let permission: Permission = json5::from_str(&content).wrap_err(format!(
"Failed to deserialize the permission token from file {}",
&s
))?;
Expand Down
7 changes: 7 additions & 0 deletions data_model/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,13 @@ impl JsonString {
// so it should be a valid JSON string
Ok(Self(serialized))
}

/// Create without checking whether the input is a valid JSON string.
///
/// The caller must guarantee that the value is valid.
pub fn from_json_string_unchecked(value: String) -> Self {
Self(value)
}
}

impl From<&serde_json::Value> for JsonString {
Expand Down
8 changes: 4 additions & 4 deletions smart_contract/executor/derive/src/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,14 @@ pub fn impl_derive_visit(emitter: &mut Emitter, input: &syn::DeriveInput) -> Tok
"fn visit_transfer_asset_definition(operation: &Transfer<Account, AssetDefinitionId, Account>)",
"fn visit_set_asset_definition_key_value(operation: &SetKeyValue<AssetDefinition>)",
"fn visit_remove_asset_definition_key_value(operation: &RemoveKeyValue<AssetDefinition>)",
"fn visit_grant_account_permission(operation: &Grant<PermissionObject, Account>)",
"fn visit_revoke_account_permission(operation: &Revoke<PermissionObject, Account>)",
"fn visit_grant_account_permission(operation: &Grant<Permission, Account>)",
"fn visit_revoke_account_permission(operation: &Revoke<Permission, Account>)",
"fn visit_register_role(operation: &Register<Role>)",
"fn visit_unregister_role(operation: &Unregister<Role>)",
"fn visit_grant_account_role(operation: &Grant<RoleId, Account>)",
"fn visit_revoke_account_role(operation: &Revoke<RoleId, Account>)",
"fn visit_grant_role_permission(operation: &Grant<PermissionObject, Role>)",
"fn visit_revoke_role_permission(operation: &Revoke<PermissionObject, Role>)",
"fn visit_grant_role_permission(operation: &Grant<Permission, Role>)",
"fn visit_revoke_role_permission(operation: &Revoke<Permission, Role>)",
"fn visit_register_trigger(operation: &Register<Trigger>)",
"fn visit_unregister_trigger(operation: &Unregister<Trigger>)",
"fn visit_mint_trigger_repetitions(operation: &Mint<u32, Trigger>)",
Expand Down
3 changes: 1 addition & 2 deletions smart_contract/executor/derive/src/entrypoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,8 @@ fn impl_migrate_entrypoint(fn_item: syn::ItemFn) -> TokenStream {

let migrate_fn_name = syn::Ident::new(export::EXECUTOR_MIGRATE, proc_macro2::Span::call_site());

// FIXME: is doc accurate?
quote! {
/// Executor `permission_schema` entrypoint
/// Executor `data_model_schema` entrypoint
///
/// # Memory safety
///
Expand Down
5 changes: 4 additions & 1 deletion smart_contract/executor/src/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ pub use trigger::{
visit_unregister_trigger,
};

use crate::{permission::Permission as _, prelude::*};
use crate::{
permission::Permission as _,
prelude::{Permission as PermissionObject, *},
};

// NOTE: If any new `visit_..` functions are introduced in this module, one should
// not forget to update the default executor boilerplate too, specifically the
Expand Down
12 changes: 3 additions & 9 deletions smart_contract/executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,6 @@ pub fn get_migrate_payload() -> payloads::Migrate {
unsafe { decode_with_length_prefix_from_raw(host::get_migrate_payload()) }
}

// pub trait IntoDataModel {
// fn into_data_model(&self) -> &ExecutorDataModel;
// }

/// Set new [`ExecutorDataModel`].
///
/// # Errors
Expand Down Expand Up @@ -291,9 +287,7 @@ pub mod prelude {
entrypoint, Constructor, Permission, Validate, ValidateEntrypoints, ValidateGrantRevoke,
Visit,
};
pub use iroha_smart_contract::prelude::{
Parameter as ParameterObject, Permission as PermissionObject, *,
};
pub use iroha_smart_contract::prelude::*;

pub use super::{
data_model::{
Expand All @@ -302,8 +296,8 @@ pub mod prelude {
ValidationFail,
},
deny, execute,
parameter::Parameter,
permission::Permission,
parameter::Parameter as ParameterTrait,
permission::Permission as PermissionTrait,
DataModelBuilder, Validate,
};
}
11 changes: 6 additions & 5 deletions smart_contract/executor/src/parameter.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
//! Executor-defined configuration parameters

use iroha_executor::prelude::{ParameterId, ParameterObject};
use iroha_schema::IntoSchema;
use iroha_smart_contract_utils::debug::DebugExpectExt;
use serde::{de::DeserializeOwned, Serialize};

use crate::{data_model::JsonString, TryFromDataModelObjectError};
use crate::{
data_model::JsonString,
prelude::{Parameter as ParameterObject, *},
TryFromDataModelObjectError,
};

/// Marker trait for parameters.
///
Expand Down Expand Up @@ -36,9 +39,7 @@ pub trait Parameter: Serialize + DeserializeOwned + IntoSchema {
/// Try to convert from [`ParameterObject`]
/// # Errors
/// See [`TryFromDataModelObjectError`]
fn try_from_object(
object: &ParameterObject,
) -> crate::prelude::Result<Self, TryFromDataModelObjectError> {
fn try_from_object(object: &ParameterObject) -> Result<Self, TryFromDataModelObjectError> {
if *object.id() != <Self as Parameter>::id() {
return Err(TryFromDataModelObjectError::Id(object.id().name().clone()));
}
Expand Down
5 changes: 4 additions & 1 deletion smart_contract/executor/src/permission.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ use iroha_smart_contract::{data_model::JsonString, QueryOutputCursor};
use iroha_smart_contract_utils::debug::DebugExpectExt as _;
use serde::{de::DeserializeOwned, Serialize};

use crate::{prelude::*, TryFromDataModelObjectError};
use crate::{
prelude::{Permission as PermissionObject, *},
TryFromDataModelObjectError,
};

/// Is used to check if the permission token is owned by the account.
pub trait Permission:
Expand Down

0 comments on commit c1fe4ab

Please sign in to comment.