fix review comments

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>

crypto/Cargo.toml

@@ -11,9 +11,10 @@ default = ["std"]
 # Enable static linkage of the rust standard library.
 # Please refer to https://docs.rust-embedded.org/book/intro/no-std.html
 std = ["ursa"]
-ffi = ["iroha_ffi"]
 # Force static linking
 vendored = ["openssl-sys"]
+# Generate extern functions callable via FFI
+ffi = ["iroha_ffi"]
 
 [dependencies]
 iroha_primitives = { path = "../primitives", version = "=2.0.0-pre-rc.5", default-features = false }

data_model/Cargo.toml

@@ -22,6 +22,7 @@ default = ["std"]
 # Disabled for WASM interoperability, to reduce the binary size.
 # Please refer to https://docs.rust-embedded.org/book/intro/no-std.html
 std = ["iroha_macro/std", "iroha_version/std", "iroha_version/warp", "iroha_crypto/std", "iroha_primitives/std", "thiserror", "strum/std", "dashmap", "tokio"]
+# Generate extern functions callable via FFI
 ffi = ["iroha_crypto/ffi", "iroha_ffi"]
 
 # Expose API for mutating structures (Internal use only)

data_model/src/account.rs

@@ -192,7 +192,7 @@ impl NewAccount {
     }
 
     /// Identification
-    pub fn id(&self) -> &<Account as Identifiable>::Id {
+    pub(crate) fn id(&self) -> &<Account as Identifiable>::Id {
         &self.id
     }
 }

data_model/src/asset.rs

@@ -478,7 +478,7 @@ impl NewAssetDefinition {
 
     /// Identification
     #[inline]
-    pub fn id(&self) -> &<AssetDefinition as Identifiable>::Id {
+    pub(crate) fn id(&self) -> &<AssetDefinition as Identifiable>::Id {
         &self.id
     }
 }

data_model/src/domain.rs

@@ -136,7 +136,7 @@ impl NewDomain {
     }
 
     /// Identification
-    pub fn id(&self) -> &<Domain as Identifiable>::Id {
+    pub(crate) fn id(&self) -> &<Domain as Identifiable>::Id {
         &self.id
     }
 }

data_model/src/lib.rs

@@ -23,6 +23,7 @@ use derive_more::Into;
 use derive_more::{AsRef, Deref, Display, From};
 use events::FilterBox;
 use iroha_crypto::{Hash, PublicKey};
+#[cfg(feature = "ffi")]
 use iroha_ffi::{IntoFfi, TryFromFfi};
 use iroha_macro::{error::ErrorTryFromEnum, FromVariant};
 use iroha_primitives::{fixed, small, small::SmallVec};

data_model/src/name.rs

@@ -5,6 +5,7 @@ use alloc::{boxed::Box, format, string::String, vec::Vec};
 use core::{ops::RangeInclusive, str::FromStr};
 
 use derive_more::{DebugCustom, Display};
+#[cfg(feature = "ffi")]
 use iroha_ffi::{IntoFfi, TryFromFfi};
 use iroha_primitives::conststr::ConstString;
 use iroha_schema::IntoSchema;
@@ -94,7 +95,7 @@ impl FromStr for Name {
 #[allow(non_snake_case, unsafe_code)]
 pub unsafe extern "C" fn Name__from_str<'itm>(
     candidate: <&'itm str as iroha_ffi::TryFromReprC<'itm>>::Source,
-    output: <<Name as IntoFfi>::Target as iroha_ffi::FfiOutput>::OutPtr,
+    out_ptr: <<Name as iroha_ffi::IntoFfi>::Target as iroha_ffi::Output>::OutPtr,
 ) -> iroha_ffi::FfiResult {
     let res = std::panic::catch_unwind(|| {
         // False positive - doesn't compile otherwise
@@ -105,7 +106,7 @@ pub unsafe extern "C" fn Name__from_str<'itm>(
             let method_res = Name::from_str(candidate)
                 .map_err(|_e| iroha_ffi::FfiResult::ExecutionFail)?
                 .into_ffi();
-            iroha_ffi::FfiOutput::write(method_res, output)?;
+            iroha_ffi::OutPtrOf::write(out_ptr, method_res)?;
             Ok(())
         };
 
@@ -184,6 +185,7 @@ mod tests {
 
     #[test]
     #[allow(unsafe_code)]
+    #[cfg(feature = "ffi")]
     fn ffi_name_from_str() -> Result<(), ParseError> {
         use crate::ffi::{Handle, __drop};
 

data_model/src/role.rs

@@ -181,6 +181,12 @@ impl NewRole {
             },
         }
     }
+
+    /// Identification
+    #[inline]
+    pub(crate) fn id(&self) -> &<Role as Identifiable>::Id {
+        &self.inner.id
+    }
 }
 
 /// The prelude re-exports most commonly used traits, structs and macros from this module.

ffi/derive/Cargo.toml

@@ -12,6 +12,7 @@ syn = { version = "1.0", features = ["full", "visit", "visit-mut", "extra-traits
 quote = "1.0"
 proc-macro2 = "1.0"
 proc-macro-error = "1.0"
+derive_more = "0.99.17"
 
 [dev-dependencies]
 iroha_ffi = { version = "=2.0.0-pre-rc.5", path = "../" }

ffi/derive/src/derive.rs

@@ -115,7 +115,7 @@ fn gen_ffi_fn_args(handle: &Receiver, field: &impl Arg, derive: Derive) -> Token
             #handle_name: #handle_type, #field_name: #field_type,
         },
         Derive::Getter | Derive::MutGetter => quote! {
-            #handle_name: #handle_type, #field_name: <#field_type as iroha_ffi::FfiOutput>::OutPtr,
+            #handle_name: #handle_type, #field_name: <#field_type as iroha_ffi::Output>::OutPtr
         },
     }
 }
@@ -149,7 +149,7 @@ fn gen_ffi_fn_body(
                 let __out_ptr = #field_name;
                 let #field_name = #handle_name.#method_name();
                 #from_field
-                iroha_ffi::FfiOutput::write(#field_name, __out_ptr)?;
+                iroha_ffi::OutPtrOf::write(__out_ptr, #field_name)?;
                 Ok(())
             }}
         }

ffi/derive/src/export.rs

@@ -3,7 +3,7 @@ use proc_macro_error::OptionExt;
 use quote::quote;
 use syn::{parse_quote, Ident, Type};
 
-use crate::impl_visitor::{Arg, FnDescriptor};
+use crate::impl_visitor::{unwrap_result_type, Arg, FnDescriptor};
 
 pub fn gen_ffi_fn(fn_descriptor: &FnDescriptor) -> TokenStream {
     let ffi_fn_name = gen_ffi_fn_name(fn_descriptor);
@@ -172,7 +172,7 @@ pub fn gen_arg_ffi_to_src(arg: &impl crate::impl_visitor::Arg, is_output: bool)
     }
 
     quote! {
-        let mut store = Default::default();
+        let mut store = core::default::Default::default();
         let #arg_name: #src_type = iroha_ffi::TryFromReprC::try_from_repr_c(#arg_name, &mut store)?;
     }
 }
@@ -205,6 +205,17 @@ pub fn gen_arg_src_to_ffi(arg: &impl crate::impl_visitor::Arg, is_output: bool)
     };
 
     if is_output {
+        if unwrap_result_type(src_type).is_some() {
+            return quote! {
+                let #arg_name = if let Ok(ok) = #arg_name {
+                    iroha_ffi::IntoFfi::into_ffi(ok)
+                } else {
+                    // TODO: Implement error handling (https://github.com/hyperledger/iroha/issues/2252)
+                    return Err(FfiResult::ExecutionFail);
+                };
+            };
+        }
+
         return ffi_conversion;
     }
 
@@ -243,7 +254,7 @@ fn gen_output_assignment_stmts(fn_descriptor: &FnDescriptor) -> TokenStream {
 
         return quote! {
             #output_arg_conversion
-            <#arg_type as iroha_ffi::FfiOutput>::write(#arg_name, __out_ptr)?;
+            iroha_ffi::OutPtrOf::<#arg_type>::write(__out_ptr, #arg_name)?;
         };
     }
 
@@ -261,5 +272,5 @@ pub fn gen_ffi_fn_out_ptr_arg(arg: &impl crate::impl_visitor::Arg) -> TokenStrea
     let arg_name = arg.name();
     let arg_type = arg.ffi_type_resolved();
 
-    quote! { #arg_name: <#arg_type as iroha_ffi::FfiOutput>::OutPtr }
+    quote! { #arg_name: <#arg_type as iroha_ffi::Output>::OutPtr }
 }

ffi/derive/src/impl_visitor.rs

@@ -1,3 +1,4 @@
+use derive_more::Constructor;
 use proc_macro2::Span;
 use proc_macro_error::{abort, OptionExt};
 use syn::{parse_quote, visit::Visit, visit_mut::VisitMut, Ident, Type};
@@ -9,6 +10,7 @@ pub trait Arg {
     fn ffi_type_resolved(&self) -> Type;
 }
 
+#[derive(Constructor)]
 pub struct Receiver<'ast> {
     self_ty: &'ast syn::Path,
     name: Ident,
@@ -34,16 +36,6 @@ pub struct ImplDescriptor<'ast> {
     pub fns: Vec<FnDescriptor<'ast>>,
 }
 
-impl<'ast> Receiver<'ast> {
-    pub fn new(self_ty: &'ast syn::Path, name: Ident, type_: Type) -> Self {
-        Self {
-            self_ty,
-            name,
-            type_,
-        }
-    }
-}
-
 impl<'ast> InputArg<'ast> {
     pub fn new(self_ty: &'ast syn::Path, name: &'ast Ident, type_: &'ast Type) -> Self {
         Self {
@@ -121,6 +113,10 @@ fn resolve_ffi_type(self_ty: &syn::Path, mut arg_type: Type, is_output: bool) ->
     ImplTraitResolver.visit_type_mut(&mut arg_type);
 
     if is_output {
+        if let Some(result_type) = unwrap_result_type(&arg_type) {
+            return parse_quote! {<#result_type as iroha_ffi::IntoFfi>::Target};
+        }
+
         return parse_quote! {<#arg_type as iroha_ffi::IntoFfi>::Target};
     }
 
@@ -487,7 +483,6 @@ impl VisitMut for SelfResolver<'_> {
 }
 
 struct ImplTraitResolver;
-
 impl VisitMut for ImplTraitResolver {
     fn visit_type_mut(&mut self, node: &mut Type) {
         let mut new_node = None;
@@ -534,3 +529,19 @@ impl VisitMut for ImplTraitResolver {
 fn get_ident(path: &syn::Path) -> &Ident {
     &path.segments.last().expect_or_abort("Defined").ident
 }
+
+pub fn unwrap_result_type(node: &Type) -> Option<&Type> {
+    if let Type::Path(type_) = node {
+        let last_seg = type_.path.segments.last().expect_or_abort("Defined");
+
+        if last_seg.ident == "Result" {
+            if let syn::PathArguments::AngleBracketed(args) = &last_seg.arguments {
+                if let syn::GenericArgument::Type(result_type) = &args.args[0] {
+                    return Some(result_type);
+                }
+            }
+        }
+    }
+
+    None
+}

ffi/derive/src/lib.rs

@@ -13,6 +13,7 @@ mod derive;
 mod export;
 mod impl_visitor;
 
+/// Generate FFI functions
 #[proc_macro_attribute]
 #[proc_macro_error::proc_macro_error]
 pub fn ffi_export(_attr: TokenStream, item: TokenStream) -> TokenStream {
@@ -32,7 +33,7 @@ pub fn ffi_export(_attr: TokenStream, item: TokenStream) -> TokenStream {
                 abort!(item.vis, "Only public structs allowed in FFI");
             }
             if !item.generics.params.is_empty() {
-                abort!(item.generics, "Generic are not supported");
+                abort!(item.generics, "Generics are not supported");
             }
 
             let ffi_fns = gen_fns_from_derives(&item);
@@ -48,6 +49,7 @@ pub fn ffi_export(_attr: TokenStream, item: TokenStream) -> TokenStream {
     .into()
 }
 
+/// Derive implementations of traits required to convert into an FFI compatible type
 #[proc_macro_derive(IntoFfi)]
 #[proc_macro_error::proc_macro_error]
 pub fn into_ffi_derive(input: TokenStream) -> TokenStream {
@@ -69,6 +71,7 @@ pub fn into_ffi_derive(input: TokenStream) -> TokenStream {
     .into()
 }
 
+/// Derive implementations of traits required to convert from an FFI compatible type
 #[proc_macro_derive(TryFromFfi)]
 #[proc_macro_error::proc_macro_error]
 pub fn try_from_ffi_derive(input: TokenStream) -> TokenStream {
@@ -262,7 +265,7 @@ fn derive_try_from_ffi_for_opaque_item(name: &Ident) -> TokenStream2 {
             type Store = Vec<Self>;
 
             unsafe fn try_from_repr_c(source: Self::Source, store: &'itm mut <Self as iroha_ffi::slice::TryFromReprCSliceRef<'itm>>::Store) -> Result<&'itm [Self], iroha_ffi::FfiResult> {
-                let source = source.into_slice().ok_or(iroha_ffi::FfiResult::ArgIsNull)?;
+                let source = source.into_rust().ok_or(iroha_ffi::FfiResult::ArgIsNull)?;
 
                 for elem in source {
                     store.push(Clone::clone(iroha_ffi::TryFromReprC::try_from_repr_c(*elem, &mut ())?));
@@ -274,16 +277,14 @@ fn derive_try_from_ffi_for_opaque_item(name: &Ident) -> TokenStream2 {
     }
 }
 
+#[allow(clippy::restriction)]
 fn derive_into_ffi_for_item(_: &Ident) -> TokenStream2 {
-    quote! {
-        // TODO:
-    }
+    unimplemented!("https://github.com/hyperledger/iroha/issues/2510")
 }
 
+#[allow(clippy::restriction)]
 fn derive_try_from_ffi_for_item(_: &Ident) -> TokenStream2 {
-    quote! {
-        // TODO:
-    }
+    unimplemented!("https://github.com/hyperledger/iroha/issues/2510")
 }
 
 fn gen_fieldless_enum_into_ffi(enum_name: &Ident, repr: &[syn::NestedMeta]) -> TokenStream2 {
@@ -313,22 +314,6 @@ fn gen_fieldless_enum_into_ffi(enum_name: &Ident, repr: &[syn::NestedMeta]) -> T
                 self as *mut #enum_name as *mut #ffi_type
             }
         }
-
-        //impl iroha_ffi::OptionWrapped for #enum_name {
-        //    type FfiType = *mut #ffi_type;
-        //}
-
-        //impl<'store> iroha_ffi::FromOption<'store> for #enum_name {
-        //    type Store = #ffi_type;
-
-        //    // TODO: Rely on trap representation to represent None values
-        //    fn into_ffi(source: Option<Self>, store: &'store mut <Self as iroha_ffi::FromOption<'store>>::Store) -> <Self as iroha_ffi::OptionWrapped>::FfiType {
-        //        source.map_or_else(core::ptr::null_mut, |item| {
-        //            *store = item as #ffi_type;
-        //            iroha_ffi::IntoFfi::into_ffi(store, &mut ())
-        //        })
-        //    }
-        //}
     }
 }
 
@@ -405,15 +390,15 @@ fn gen_fieldless_enum_try_from_ffi(
             type Store = ();
 
             unsafe fn try_from_repr_c(source: Self::Source, _: &mut <Self as iroha_ffi::slice::TryFromReprCSliceRef<'itm>>::Store) -> Result<&'itm [Self], iroha_ffi::FfiResult> {
-                source.into_slice().ok_or(iroha_ffi::FfiResult::ArgIsNull)
+                source.into_rust().ok_or(iroha_ffi::FfiResult::ArgIsNull)
             }
         }
         impl<'slice> iroha_ffi::slice::TryFromReprCSliceMut<'slice> for #enum_name {
             type Source = iroha_ffi::slice::SliceMut<'slice, #enum_name>;
             type Store = ();
 
             unsafe fn try_from_repr_c(source: Self::Source, _: &mut <Self as iroha_ffi::slice::TryFromReprCSliceMut>::Store) -> Result<&'slice mut [Self], iroha_ffi::FfiResult> {
-                source.into_slice().ok_or(iroha_ffi::FfiResult::ArgIsNull)
+                source.into_rust().ok_or(iroha_ffi::FfiResult::ArgIsNull)
             }
         }
     }