[feature] #2511: Restrict FFI types on wasm #2590

Erigara · 2022-08-03T15:03:56Z

Signed-off-by: Shanin Roman shanin1000@yandex.ru

Description of the Change

Add wasm feature to iroha_ffi crate;
Add PrimitiveRepr trait to substitute unsupported types with supported ones;
Refactor Vec transfer through ff-boundry.

Issue

Closes #2511.

Benefits

Protect from various errors (when user can pass u32 where u8 supposed to be).

Possible Drawbacks

More complex implementation.

Usage Examples or Tests [optional]

Alternate Designs [optional]

codecov · 2022-08-03T15:28:15Z

Codecov Report

Merging #2590 (47bc640) into iroha2-dev (a16d9c3) will decrease coverage by 1.74%.
The diff coverage is 63.26%.

❗ Current head 47bc640 differs from pull request most recent head 762bbb9. Consider uploading reports for the commit 762bbb9 to get more accurate results

@@              Coverage Diff               @@
##           iroha2-dev    #2590      +/-   ##
==============================================
- Coverage       67.61%   65.86%   -1.75%     
==============================================
  Files             140      156      +16     
  Lines           26173    28198    +2025     
==============================================
+ Hits            17696    18574     +878     
- Misses           8477     9624    +1147

Impacted Files	Coverage Δ
cli/derive/src/lib.rs	`74.72% <ø> (ø)`
cli/src/torii/mod.rs	`28.88% <ø> (ø)`
cli/src/torii/routing.rs	`69.92% <0.00%> (-2.87%)`	⬇️
client/src/client.rs	`43.36% <0.00%> (-2.96%)`	⬇️
client_cli/src/main.rs	`0.26% <ø> (ø)`
config/base/src/runtime_upgrades.rs	`35.63% <ø> (ø)`
core/src/block.rs	`70.00% <ø> (ø)`
core/src/queue.rs	`95.67% <ø> (-0.08%)`	⬇️
core/src/smartcontracts/isi/asset.rs	`54.82% <ø> (ø)`
core/src/smartcontracts/wasm.rs	`95.39% <ø> (+0.12%)`	⬆️
... and 154 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

appetrosyan · 2022-08-04T08:46:01Z

ffi/tests/ffi_export.rs

@@ -285,12 +285,12 @@ fn return_result() {
    unsafe {
        assert_eq!(
            FfiResult::ExecutionFail,
-            FfiStruct__fallible_int_output(u8::from(false), output.as_mut_ptr())
+            FfiStruct__fallible_int_output(From::from(false), output.as_mut_ptr())


The point of this change, being that it makes the type checker do a better job?

When wasm feature is active ffi representation of bool would be u32 not u8, so I did this so that the test worked in both cases.

u8 for bool is problematic. Every operation involving booleans would be extremely slow (which is why it's almost always size_t)

ffi/src/handle.rs

appetrosyan · 2022-08-04T08:53:59Z

ffi/derive/src/lib.rs

+                source: Self::Source,
+                _: &'itm mut <Self as iroha_ffi::owned::TryFromReprCVec<'itm>>::Store,
+            ) -> Result<Vec<Self>, iroha_ffi::FfiResult> {
+                let slice = source.into_rust().ok_or(iroha_ffi::FfiResult::ArgIsNull)?;


Given that FfiResult has a lot more variants, wouldn't it make sense to add FfiResult::ConversionFailed? There's non-zero overlap between the two, but even if 100% of the failures were because ArgIsNull, we'd want to be able to handle it differently.

I have added FfiResult::ConversionFailed, however for this particular case FfiResult::ArgIsNull is fine, because only case in which slice.into_rust() return None is when slice is null.

ffi/tests/gen_shared_fns.rs

ffi/src/primitives.rs

appetrosyan · 2022-08-14T05:55:54Z

ffi/src/primitives.rs

 }

-primitive_impls! {u8, u16, u32, u64, u128, i8, i16, i32, i64}


So what do we do when we actually have a u128?

Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru> Signed-off-by: BAStos525 <jungle.vas@yandex.ru>

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

Erigara added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Aug 3, 2022

appetrosyan reviewed Aug 4, 2022

View reviewed changes

mversic requested changes Aug 4, 2022

View reviewed changes

appetrosyan assigned appetrosyan, Erigara and mversic Aug 5, 2022

Erigara force-pushed the restrict-wasm-ffi-types branch 2 times, most recently from 332e2a1 to 47bc640 Compare August 9, 2022 08:16

mversic self-requested a review August 9, 2022 09:02

Erigara changed the title ~~[feature] #2511: Restrict FFI types on wasm (WIP)~~ [feature] #2511: Restrict FFI types on wasm Aug 10, 2022

Erigara marked this pull request as ready for review August 10, 2022 13:35

Erigara requested review from s8sato, Arjentix, SamHSmith, QuentinI, outoftardis, ilchu and pesterev as code owners August 10, 2022 13:35

mversic approved these changes Aug 12, 2022

View reviewed changes

appetrosyan reviewed Aug 14, 2022

View reviewed changes

appetrosyan approved these changes Aug 14, 2022

View reviewed changes

Erigara added 3 commits August 14, 2022 09:59

Restrict FFI types on wasm

ef0530a

Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

Relace trait PrimitiveRepr with WasmRepr

51f8b9b

Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

Add ConversionFailed to FfiReturn

762bbb9

Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

appetrosyan force-pushed the restrict-wasm-ffi-types branch from 47bc640 to 762bbb9 Compare August 14, 2022 06:00

appetrosyan merged commit 8a0ff69 into hyperledger-iroha:iroha2-dev Aug 14, 2022

s8sato pushed a commit to s8sato/iroha that referenced this pull request Aug 15, 2022

[feature] hyperledger-iroha#2511: Restrict FFI types on wasm (hyperle…

e80675e

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

Erigara deleted the restrict-wasm-ffi-types branch August 18, 2022 03:26

mversic pushed a commit to mversic/iroha that referenced this pull request Sep 6, 2022

[feature] hyperledger-iroha#2511: Restrict FFI types on wasm (hyperle…

099f05a

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

mversic pushed a commit to mversic/iroha that referenced this pull request Sep 6, 2022

[feature] hyperledger-iroha#2511: Restrict FFI types on wasm (hyperle…

1c1d6a3

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

mversic pushed a commit to mversic/iroha that referenced this pull request Sep 6, 2022

[feature] hyperledger-iroha#2511: Restrict FFI types on wasm (hyperle…

be9176e

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

mversic pushed a commit to mversic/iroha that referenced this pull request Sep 7, 2022

[feature] hyperledger-iroha#2511: Restrict FFI types on wasm (hyperle…

52a3512

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

BAStos525 pushed a commit to mversic/iroha that referenced this pull request Sep 7, 2022

[feature] hyperledger-iroha#2511: Restrict FFI types on wasm (hyperle…

9ceecd7

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru> Signed-off-by: BAStos525 <jungle.vas@yandex.ru>

mversic pushed a commit to mversic/iroha that referenced this pull request Sep 8, 2022

[feature] hyperledger-iroha#2511: Restrict FFI types on wasm (hyperle…

9676c61

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

mversic pushed a commit to mversic/iroha that referenced this pull request Sep 9, 2022

[feature] hyperledger-iroha#2511: Restrict FFI types on wasm (hyperle…

7abc34f

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

mversic pushed a commit to mversic/iroha that referenced this pull request Sep 9, 2022

[feature] hyperledger-iroha#2511: Restrict FFI types on wasm (hyperle…

6fd113a

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

BAStos525 pushed a commit to mversic/iroha that referenced this pull request Sep 9, 2022

[feature] hyperledger-iroha#2511: Restrict FFI types on wasm (hyperle…

4e75673

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

mversic pushed a commit to mversic/iroha that referenced this pull request Sep 9, 2022

[feature] hyperledger-iroha#2511: Restrict FFI types on wasm (hyperle…

cf50548

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

BAStos525 pushed a commit to mversic/iroha that referenced this pull request Sep 9, 2022

[feature] hyperledger-iroha#2511: Restrict FFI types on wasm (hyperle…

336e1da

…dger-iroha#2590) Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature] #2511: Restrict FFI types on wasm #2590

[feature] #2511: Restrict FFI types on wasm #2590

Erigara commented Aug 3, 2022 •

edited

Loading

codecov bot commented Aug 3, 2022 •

edited

Loading

appetrosyan Aug 4, 2022

Erigara Aug 4, 2022

appetrosyan Aug 14, 2022

appetrosyan Aug 4, 2022

Erigara Aug 5, 2022

appetrosyan Aug 14, 2022

		}

		primitive_impls! {u8, u16, u32, u64, u128, i8, i16, i32, i64}

[feature] #2511: Restrict FFI types on wasm #2590

[feature] #2511: Restrict FFI types on wasm #2590

Conversation

Erigara commented Aug 3, 2022 • edited Loading

Description of the Change

Issue

Benefits

Possible Drawbacks

Usage Examples or Tests [optional]

Alternate Designs [optional]

codecov bot commented Aug 3, 2022 • edited Loading

Codecov Report

appetrosyan Aug 4, 2022

Choose a reason for hiding this comment

Erigara Aug 4, 2022

Choose a reason for hiding this comment

appetrosyan Aug 14, 2022

Choose a reason for hiding this comment

appetrosyan Aug 4, 2022

Choose a reason for hiding this comment

Erigara Aug 5, 2022

Choose a reason for hiding this comment

appetrosyan Aug 14, 2022

Choose a reason for hiding this comment

Erigara commented Aug 3, 2022 •

edited

Loading

codecov bot commented Aug 3, 2022 •

edited

Loading