Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[feature] #2536: Introduce ffi_import for FFI clients #2557

Merged
merged 3 commits into from
Aug 8, 2022
Merged

[feature] #2536: Introduce ffi_import for FFI clients #2557

merged 3 commits into from
Aug 8, 2022

Conversation

mversic
Copy link
Contributor

@mversic mversic commented Jul 28, 2022 •
edited
Loading

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

Description of the Change

  • introduce mutually exclusive ffi_import and ffi_export features for iroha_data_model and iroha_crypto
  • introduce additional ffi! macro that convert the wrapped struct into opaque pointer
  • rename TryFromFfi -> TryFromReprC
  • make ffi crate no_std
  • refactor of FFI crate - reduce duplicated code
  • closes FFI import flag for clients #2536

NOTE

To make the change manageable, this PR only introduces the ffi_import flag. The flag does not expand to completely valid code yet and is commented out. This is expected to be completed with #2231 . I'm trying to get this merged so that @Erigara and I don't step on each other's toes anymore

Issue

@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Jul 28, 2022
@mversic mversic changed the title [feature] #2536: refactor ffi, implement method wrapping for FFI client [feature] #2536: FFI client Jul 28, 2022
@mversic mversic force-pushed the ffi_client branch 3 times, most recently from f9cf8a0 to df1c26b Compare July 28, 2022 19:18
@mversic mversic marked this pull request as ready for review July 28, 2022 19:22
@mversic mversic mentioned this pull request Jul 28, 2022
Closed
mversic
mversic commented Jul 28, 2022
View reviewed changes
crypto/src/lib.rs Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 28, 2022 •
edited
Loading

Codecov Report

Merging #2557 (54a718d) into iroha2-dev (a16d9c3) will decrease coverage by 1.76%.
The diff coverage is 63.04%.

@@              Coverage Diff               @@
##           iroha2-dev    #2557      +/-   ##
==============================================
- Coverage       67.61%   65.84%   -1.77%     
==============================================
  Files             140      156      +16     
  Lines           26173    28135    +1962     
==============================================
+ Hits            17696    18525     +829     
- Misses           8477     9610    +1133
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.

mversic
mversic commented Jul 28, 2022
View reviewed changes
crypto/Cargo.toml Outdated Show resolved Hide resolved
mversic
mversic commented Jul 28, 2022
View reviewed changes
crypto/Cargo.toml Outdated Show resolved Hide resolved
@Erigara Erigara self-assigned this Jul 29, 2022
Arjentix
Arjentix reviewed Jul 31, 2022
View reviewed changes
crypto/src/lib.rs Outdated Show resolved Hide resolved
crypto/src/lib.rs Outdated Show resolved Hide resolved
data_model/src/account.rs Outdated Show resolved Hide resolved
data_model/src/account.rs Outdated Show resolved Hide resolved
data_model/src/name.rs Outdated Show resolved Hide resolved
ffi/derive/tests/ui_fail/derive_skip_field.stderr Show resolved Hide resolved
ffi/derive/tests/ui_fail/derive_skip_struct.rs Show resolved Hide resolved
ffi/derive/tests/ui_pass/valid.rs Outdated Show resolved Hide resolved
ffi/src/handle.rs Outdated Show resolved Hide resolved
data_model/src/name.rs Show resolved Hide resolved
@mversic mversic force-pushed the ffi_client branch 2 times, most recently from 9703fd1 to f571dd6 Compare July 31, 2022 21:19
@mversic mversic requested a review from Arjentix July 31, 2022 21:19
@mversic mversic force-pushed the ffi_client branch 3 times, most recently from f5475ab to 6ca495b Compare August 4, 2022 20:35
outoftardis
outoftardis reviewed Aug 5, 2022
View reviewed changes
data_model/Cargo.toml Outdated Show resolved Hide resolved
crypto/Cargo.toml Outdated Show resolved Hide resolved
data_model/src/account.rs Outdated Show resolved Hide resolved
data_model/src/account.rs Outdated Show resolved Hide resolved
ffi/src/lib.rs Outdated Show resolved Hide resolved
ffi/derive/src/util.rs Outdated Show resolved Hide resolved
ffi/derive/src/lib.rs Outdated Show resolved Hide resolved
appetrosyan
appetrosyan suggested changes Aug 5, 2022
View reviewed changes
crypto/build.rs Show resolved Hide resolved
Erigara
Erigara reviewed Aug 5, 2022
View reviewed changes
ffi/src/handle.rs Outdated Show resolved Hide resolved
ffi/src/handle.rs Outdated Show resolved Hide resolved
ffi/derive/src/convert.rs Show resolved Hide resolved
ffi/derive/src/impl_visitor.rs Show resolved Hide resolved
ffi/src/lib.rs Show resolved Hide resolved
@mversic mversic force-pushed the ffi_client branch 4 times, most recently from c5884cc to 607af90 Compare August 7, 2022 21:42
@mversic mversic requested review from outoftardis, Erigara and appetrosyan and removed request for Erigara August 7, 2022 22:26
@mversic
replace client flag with ffi_export/ffi_import
54a718d 
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Erigara
Erigara approved these changes Aug 8, 2022
View reviewed changes
Copy link
Contributor

@Erigara Erigara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, it's a bit inconvenient that we have to import alloc everywhere, but I think we could live with that.

@appetrosyan appetrosyan merged commit 3fae39a into hyperledger-iroha:iroha2-dev Aug 8, 2022
mversic added a commit to mversic/iroha that referenced this pull request Sep 6, 2022
@mversic
[feature] hyperledger-iroha#2536: Introduce ffi_import for FFI clie…
c692b2d 
…nts (hyperledger-iroha#2557)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
mversic added a commit to mversic/iroha that referenced this pull request Sep 6, 2022
@mversic
[feature] hyperledger-iroha#2536: Introduce ffi_import for FFI clie…
1dac2b8 
…nts (hyperledger-iroha#2557)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
mversic added a commit to mversic/iroha that referenced this pull request Sep 6, 2022
@mversic
[feature] hyperledger-iroha#2536: Introduce ffi_import for FFI clie…
0106711 
…nts (hyperledger-iroha#2557)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
mversic added a commit to mversic/iroha that referenced this pull request Sep 7, 2022
@mversic
[feature] hyperledger-iroha#2536: Introduce ffi_import for FFI clie…
77e5503 
…nts (hyperledger-iroha#2557)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
mversic added a commit to mversic/iroha that referenced this pull request Sep 8, 2022
@mversic
[feature] hyperledger-iroha#2536: Introduce ffi_import for FFI clie…
1a39347 
…nts (hyperledger-iroha#2557)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
mversic added a commit to mversic/iroha that referenced this pull request Sep 9, 2022
@mversic
[feature] hyperledger-iroha#2536: Introduce ffi_import for FFI clie…
0ed0345 
…nts (hyperledger-iroha#2557)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
mversic added a commit to mversic/iroha that referenced this pull request Sep 9, 2022
@mversic
[feature] hyperledger-iroha#2536: Introduce ffi_import for FFI clie…
046f4b5 
…nts (hyperledger-iroha#2557)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
mversic added a commit to mversic/iroha that referenced this pull request Sep 9, 2022
@mversic
[feature] hyperledger-iroha#2536: Introduce ffi_import for FFI clie…
93ee08e 
…nts (hyperledger-iroha#2557)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
@mversic mversic deleted the ffi_client branch December 27, 2022 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Erigara Erigara Erigara approved these changes

@appetrosyan appetrosyan appetrosyan approved these changes

@s8sato s8sato Awaiting requested review from s8sato

@SamHSmith SamHSmith Awaiting requested review from SamHSmith

@QuentinI QuentinI Awaiting requested review from QuentinI

@ilchu ilchu Awaiting requested review from ilchu

@pesterev pesterev Awaiting requested review from pesterev

@Arjentix Arjentix Awaiting requested review from Arjentix

@outoftardis outoftardis Awaiting requested review from outoftardis

Assignees

@appetrosyan appetrosyan

@Arjentix Arjentix

@Erigara Erigara

@mversic mversic

Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mversic @appetrosyan @outoftardis @Arjentix @Erigara