fix review comments

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
hyperledger-iroha · Aug 4, 2022 · dfb2d38 · dfb2d38
1 parent e03f957
commit dfb2d38
Show file tree

Hide file tree

Showing 22 changed files with 354 additions and 242 deletions.
diff --git a/crypto/Cargo.toml b/crypto/Cargo.toml
@@ -13,8 +13,8 @@ default = ["std"]
 std = ["ursa"]
 # Force static linking
 vendored = ["openssl-sys"]
-## Replace structures and methods with FFI equivalents. Facilitates dynamic linkage
-#ffi = ["iroha_ffi/client"]
+## Replace structures and methods with FFI equivalents. Facilitates dynamic linkage (mainly used in smartcontracts)
+#ffi = ["iroha_ffi/client"] # TODO: Doesn't produce valid code yet
 
 # Expose FFI API for dynamic linking (Internal use only)
 ffi_api = ["std"]

diff --git a/crypto/src/lib.rs b/crypto/src/lib.rs
@@ -374,7 +374,7 @@ ffi! {
     pub struct PublicKey {
         /// Digest function
         digest_function: ConstString,
-        /// payload of key
+        /// Key payload
         payload: Vec<u8>,
     }
 }
@@ -505,7 +505,7 @@ ffi! {
     pub struct PrivateKey {
         /// Digest function
         digest_function: ConstString,
-        /// key payload. WARNING! Do not use `"string".as_bytes()` to obtain the key.
+        /// Key payload. WARNING! Do not use `"string".as_bytes()` to obtain the key.
         #[serde(with = "hex::serde")]
         payload: Vec<u8>,
     }
@@ -577,15 +577,17 @@ impl<'de> Deserialize<'de> for PrivateKey {
     }
 }
 
-iroha_ffi::handles! {0, KeyPair, PublicKey, PrivateKey}
 #[cfg(any(feature = "ffi_api", feature = "ffi"))]
-iroha_ffi::gen_ffi_impl! { Clone: KeyPair, PublicKey, PrivateKey}
-#[cfg(any(feature = "ffi_api", feature = "ffi"))]
-iroha_ffi::gen_ffi_impl! { Eq: KeyPair, PublicKey, PrivateKey}
-#[cfg(any(feature = "ffi_api", feature = "ffi"))]
-iroha_ffi::gen_ffi_impl! { Ord: PublicKey }
-#[cfg(any(feature = "ffi_api", feature = "ffi"))]
-iroha_ffi::gen_ffi_impl! { Drop: KeyPair, PublicKey, PrivateKey}
+mod ffi {
+    use super::*;
+
+    iroha_ffi::handles! {KeyPair, PublicKey, PrivateKey}
+
+    iroha_ffi::ffi_fn! { Clone: KeyPair, PublicKey, PrivateKey}
+    iroha_ffi::ffi_fn! { Eq: KeyPair, PublicKey, PrivateKey}
+    iroha_ffi::ffi_fn! { Ord: PublicKey }
+    iroha_ffi::ffi_fn! { Drop: KeyPair, PublicKey, PrivateKey}
+}
 
 /// The prelude re-exports most commonly used traits, structs and macros from this crate.
 pub mod prelude {

diff --git a/data_model/Cargo.toml b/data_model/Cargo.toml
@@ -22,8 +22,8 @@ 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"]
-## Replace structures and methods with FFI equivalents. Facilitates dynamic linkage
-#ffi = ["iroha_ffi/client"]
+## Replace structures and methods with FFI equivalents. Facilitates dynamic linkage (mainly used in smartcontracts)
+#ffi = ["iroha_ffi/client"] # TODO: Doesn't produce valid code yet.
 
 # Expose FFI API for dynamic linking (Internal use only)
 ffi_api = ["std"]

diff --git a/data_model/src/lib.rs b/data_model/src/lib.rs
@@ -909,54 +909,55 @@ pub fn current_time() -> core::time::Duration {
         .expect("Failed to get the current system time")
 }
 
-iroha_ffi::handles! {0,
-    account::Account,
-    asset::Asset,
-    domain::Domain,
-    metadata::Metadata,
-    permissions::PermissionToken,
-    role::Role,
-    Name,
-}
-
-#[cfg(any(feature = "ffi_api", feature = "ffi"))]
-iroha_ffi::gen_ffi_impl! { pub Clone:
-    account::Account,
-    asset::Asset,
-    domain::Domain,
-    metadata::Metadata,
-    permissions::PermissionToken,
-    role::Role,
-    Name,
-}
-#[cfg(any(feature = "ffi_api", feature = "ffi"))]
-iroha_ffi::gen_ffi_impl! { pub Eq:
-    account::Account,
-    asset::Asset,
-    domain::Domain,
-    metadata::Metadata,
-    permissions::PermissionToken,
-    role::Role,
-    Name,
-}
-#[cfg(any(feature = "ffi_api", feature = "ffi"))]
-iroha_ffi::gen_ffi_impl! { pub Ord:
-    account::Account,
-    asset::Asset,
-    domain::Domain,
-    permissions::PermissionToken,
-    role::Role,
-    Name,
-}
 #[cfg(any(feature = "ffi_api", feature = "ffi"))]
-iroha_ffi::gen_ffi_impl! { pub Drop:
-    account::Account,
-    asset::Asset,
-    domain::Domain,
-    metadata::Metadata,
-    permissions::PermissionToken,
-    role::Role,
-    Name,
+mod ffi {
+    use super::*;
+
+    iroha_ffi::handles! {
+        account::Account,
+        asset::Asset,
+        domain::Domain,
+        metadata::Metadata,
+        permissions::PermissionToken,
+        role::Role,
+        Name,
+    }
+
+    iroha_ffi::ffi_fn! { pub Clone:
+        account::Account,
+        asset::Asset,
+        domain::Domain,
+        metadata::Metadata,
+        permissions::PermissionToken,
+        role::Role,
+        Name,
+    }
+    iroha_ffi::ffi_fn! { pub Eq:
+        account::Account,
+        asset::Asset,
+        domain::Domain,
+        metadata::Metadata,
+        permissions::PermissionToken,
+        role::Role,
+        Name,
+    }
+    iroha_ffi::ffi_fn! { pub Ord:
+        account::Account,
+        asset::Asset,
+        domain::Domain,
+        permissions::PermissionToken,
+        role::Role,
+        Name,
+    }
+    iroha_ffi::ffi_fn! { pub Drop:
+        account::Account,
+        asset::Asset,
+        domain::Domain,
+        metadata::Metadata,
+        permissions::PermissionToken,
+        role::Role,
+        Name,
+    }
 }
 
 pub mod prelude {

diff --git a/data_model/src/name.rs b/data_model/src/name.rs
@@ -105,15 +105,16 @@ impl FromStr for Name {
 unsafe extern "C" fn Name__from_str<'itm>(
     candidate: <&'itm str as iroha_ffi::TryFromReprC<'itm>>::Source,
     out_ptr: <<Name as iroha_ffi::IntoFfi>::Target as iroha_ffi::Output>::OutPtr,
-) -> iroha_ffi::FfiResult {
+) -> iroha_ffi::FfiReturn {
     let res = std::panic::catch_unwind(|| {
         // False positive - doesn't compile otherwise
         #[allow(clippy::let_unit_value)]
         let fn_body = || {
             let mut store = Default::default();
             let candidate: &str = iroha_ffi::TryFromReprC::try_from_repr_c(candidate, &mut store)?;
             let method_res = iroha_ffi::IntoFfi::into_ffi(
-                Name::from_str(candidate).map_err(|_e| iroha_ffi::FfiResult::ExecutionFail)?,
+                // TODO: Implement error handling (https://github.com/hyperledger/iroha/issues/2252)
+                Name::from_str(candidate).map_err(|_e| iroha_ffi::FfiReturn::ExecutionFail)?,
             );
             iroha_ffi::OutPtrOf::write(out_ptr, method_res)?;
             Ok(())
@@ -123,14 +124,14 @@ unsafe extern "C" fn Name__from_str<'itm>(
             return err;
         }
 
-        iroha_ffi::FfiResult::Ok
+        iroha_ffi::FfiReturn::Ok
     });
 
     match res {
         Ok(res) => res,
         Err(_) => {
             // TODO: Implement error handling (https://github.com/hyperledger/iroha/issues/2252)
-            iroha_ffi::FfiResult::UnrecoverableError
+            iroha_ffi::FfiReturn::UnrecoverableError
         }
     }
 }
@@ -203,7 +204,7 @@ mod tests {
             let mut name = core::mem::MaybeUninit::new(core::ptr::null_mut());
 
             assert_eq!(
-                iroha_ffi::FfiResult::Ok,
+                iroha_ffi::FfiReturn::Ok,
                 Name__from_str(candidate.into_ffi(), name.as_mut_ptr())
             );
 
@@ -212,8 +213,8 @@ mod tests {
             assert_eq!(Name::from_str(candidate)?, *name);
 
             assert_eq!(
-                iroha_ffi::FfiResult::Ok,
-                crate::__drop(Name::ID, name.cast())
+                iroha_ffi::FfiReturn::Ok,
+                crate::ffi::__drop(Name::ID, name.cast())
             );
         }
 

diff --git a/data_model/src/trigger/mod.rs b/data_model/src/trigger/mod.rs
@@ -32,10 +32,10 @@ impl Registered for Trigger<FilterBox> {
     type With = Self;
 }
 
-impl<F: Filter + Eq> PartialOrd for Trigger<F> {
+impl<F: Filter + PartialEq> PartialOrd for Trigger<F> {
     #[inline]
     fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> {
-        Some(self.cmp(other))
+        Some(self.id.cmp(&other.id))
     }
 }
 

diff --git a/ffi/derive/src/convert.rs b/ffi/derive/src/convert.rs
@@ -70,9 +70,12 @@ fn derive_try_from_ffi_for_opaque_item(name: &Ident) -> TokenStream2 {
             type Source = *mut Self;
             type Store = ();
 
-            unsafe fn try_from_repr_c(source: Self::Source, _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store) -> Result<Self, iroha_ffi::FfiResult> {
+            unsafe fn try_from_repr_c(
+                source: <Self as iroha_ffi::TryFromReprC<'itm>>::Source,
+                _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store
+            ) -> iroha_ffi::Result<Self> {
                 if source.is_null() {
-                    return Err(iroha_ffi::FfiResult::ArgIsNull);
+                    return Err(iroha_ffi::FfiReturn::ArgIsNull);
                 }
 
                 Ok(*Box::from_raw(source))
@@ -85,9 +88,12 @@ fn derive_try_from_ffi_for_opaque_item(name: &Ident) -> TokenStream2 {
             type Source = *mut Self;
             type Store = ();
 
-            unsafe fn try_from_repr_c(source: Self::Source, _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store) -> Result<Self, iroha_ffi::FfiResult> {
+            unsafe fn try_from_repr_c(
+                source: <Self as iroha_ffi::TryFromReprC<'itm>>::Source,
+                _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store
+            ) -> iroha_ffi::Result<Self> {
                 if source.is_null() {
-                    return Err(iroha_ffi::FfiResult::ArgIsNull);
+                    return Err(iroha_ffi::FfiReturn::ArgIsNull);
                 }
 
                 // TODO: Casting from non opaque to opaque.
@@ -103,25 +109,34 @@ fn derive_try_from_ffi_for_opaque_item(name: &Ident) -> TokenStream2 {
             type Source = *const #name;
             type Store = ();
 
-            unsafe fn try_from_repr_c(source: Self::Source, _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store) -> Result<Self, iroha_ffi::FfiResult> {
-                source.as_ref().ok_or(iroha_ffi::FfiResult::ArgIsNull)
+            unsafe fn try_from_repr_c(
+                source: <Self as iroha_ffi::TryFromReprC<'itm>>::Source,
+                _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store
+            ) -> iroha_ffi::Result<Self> {
+                source.as_ref().ok_or(iroha_ffi::FfiReturn::ArgIsNull)
             }
         }
         impl<'itm> iroha_ffi::TryFromReprC<'itm> for &'itm mut #name {
             type Source = *mut #name;
             type Store = ();
 
-            unsafe fn try_from_repr_c(source: Self::Source, _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store) -> Result<Self, iroha_ffi::FfiResult> {
-                source.as_mut().ok_or(iroha_ffi::FfiResult::ArgIsNull)
+            unsafe fn try_from_repr_c(
+                source: <Self as iroha_ffi::TryFromReprC<'itm>>::Source,
+                _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store
+            ) -> iroha_ffi::Result<Self> {
+                source.as_mut().ok_or(iroha_ffi::FfiReturn::ArgIsNull)
             }
         }
 
-        impl<'itm> iroha_ffi::slice::TryFromReprCSliceRef<'itm> for #name {
-            type Source = iroha_ffi::slice::SliceRef<'itm, <&'itm Self as iroha_ffi::TryFromReprC<'itm>>::Source>;
+        impl<'slice> iroha_ffi::slice::TryFromReprCSliceRef<'slice> for #name {
+            type Source = iroha_ffi::slice::SliceRef<'slice, <&'slice Self as iroha_ffi::TryFromReprC<'slice>>::Source>;
             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_rust().ok_or(iroha_ffi::FfiResult::ArgIsNull)?;
+            unsafe fn try_from_repr_c(
+                source: <Self as iroha_ffi::slice::TryFromReprCSliceRef<'slice>>::Source,
+                store: &'slice mut <Self as iroha_ffi::slice::TryFromReprCSliceRef<'slice>>::Store
+            ) -> iroha_ffi::Result<&'slice [Self]> {
+                let source = source.into_rust().ok_or(iroha_ffi::FfiReturn::ArgIsNull)?;
 
                 for elem in source {
                     store.push(Clone::clone(iroha_ffi::TryFromReprC::try_from_repr_c(*elem, &mut ())?));
@@ -171,56 +186,71 @@ fn derive_try_from_ffi_for_fieldless_enum(
             type Source = #ffi_type;
             type Store = ();
 
-            unsafe fn try_from_repr_c(source: Self::Source, _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store) -> Result<Self, iroha_ffi::FfiResult> {
+            unsafe fn try_from_repr_c(
+                source: <Self as iroha_ffi::TryFromReprC<'itm>>::Source,
+                _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store
+            ) -> iroha_ffi::Result<Self> {
                 #( #discriminants )*
 
                 match source {
                     #( #discriminant_names => Ok(#enum_name::#variant_names), )*
-                    _ => Err(iroha_ffi::FfiResult::TrapRepresentation),
+                    _ => Err(iroha_ffi::FfiReturn::TrapRepresentation),
                 }
             }
         }
         impl<'itm> iroha_ffi::TryFromReprC<'itm> for &'itm #enum_name {
             type Source = *const #ffi_type;
             type Store = ();
 
-            unsafe fn try_from_repr_c(source: Self::Source, _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store) -> Result<Self, iroha_ffi::FfiResult> {
+            unsafe fn try_from_repr_c(
+                source: <Self as iroha_ffi::TryFromReprC<'itm>>::Source,
+                _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store
+            ) -> iroha_ffi::Result<Self> {
                 #( #discriminants )*
 
                 unsafe { match *source {
                     #( | #discriminant_names )* => Ok(&*(source as *const _ as *const _)),
-                    _ => Err(iroha_ffi::FfiResult::TrapRepresentation),
+                    _ => Err(iroha_ffi::FfiReturn::TrapRepresentation),
                 }}
             }
         }
         impl<'itm> iroha_ffi::TryFromReprC<'itm> for &'itm mut #enum_name {
             type Source = *mut #ffi_type;
             type Store = ();
 
-            unsafe fn try_from_repr_c(source: Self::Source, _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store) -> Result<Self, iroha_ffi::FfiResult> {
+            unsafe fn try_from_repr_c(
+                source: <Self as iroha_ffi::TryFromReprC<'itm>>::Source,
+                _: &mut <Self as iroha_ffi::TryFromReprC<'itm>>::Store
+            ) -> iroha_ffi::Result<Self> {
                 #( #discriminants )*
 
                 unsafe { match *source {
                     #( | #discriminant_names )* => Ok(&mut *(source as *mut _ as *mut _)),
-                    _ => Err(iroha_ffi::FfiResult::TrapRepresentation),
+                    _ => Err(iroha_ffi::FfiReturn::TrapRepresentation),
                 }}
             }
         }
 
-        impl<'itm> iroha_ffi::slice::TryFromReprCSliceRef<'itm> for #enum_name {
-            type Source = iroha_ffi::slice::SliceRef<'itm, Self>;
+        impl<'slice> iroha_ffi::slice::TryFromReprCSliceRef<'slice> for #enum_name {
+            type Source = iroha_ffi::slice::SliceRef<'slice, Self>;
             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_rust().ok_or(iroha_ffi::FfiResult::ArgIsNull)
+            unsafe fn try_from_repr_c(
+                source: <Self as iroha_ffi::slice::TryFromReprCSliceRef<'slice>>::Source,
+                _: &mut <Self as iroha_ffi::slice::TryFromReprCSliceRef<'slice>>::Store
+            ) -> iroha_ffi::Result<&'slice [Self]> {
+                source.into_rust().ok_or(iroha_ffi::FfiReturn::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_rust().ok_or(iroha_ffi::FfiResult::ArgIsNull)
+            unsafe fn try_from_repr_c(
+                source: <Self as iroha_ffi::slice::TryFromReprCSliceMut<'slice>>::Source,
+                _: &mut <Self as iroha_ffi::slice::TryFromReprCSliceMut>::Store
+            ) -> iroha_ffi::Result<&'slice mut [Self]> {
+                source.into_rust().ok_or(iroha_ffi::FfiReturn::ArgIsNull)
             }
         }
     }