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.

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

[feature] #2161: generate FFI functions for data_model #2211

Merged

Conversation

mversic
Copy link
Contributor

@mversic mversic commented May 13, 2022

Description of the Change

You can find a description here: #1973

This PR introduces an ffi crate which holds a macro ffi_bindgen. This macro when attached to the impl block generates the equivalent ffi out of the methods listed in the impl block. The macro can also be attached to the structure definition where it integrates with getset to make ffi fn equivalents of the getters/setters produced by the getset.

Note:

  1. All FFI functions return FfiResult return code. The idea is that the returned code can be used to query the error
  2. Considering the previous points, all FFI functions take, as a last argument, a mutable pointer where the return value of the corresponding method is stored (the so called out-pointer pattern)

Limitations:

  1. mutable references are not supported for input arguments, except for the self argument, i.e. handle

Issue

Closes #2161

Benefits

Possible Drawbacks

Usage Examples or Tests [optional]

#[derive(Getters)]
#[getset(get = "pub")]
struct Struct {
    elem: u32,
    #[getset(skip)]
    elems: Vec<u32>,
}

#[ffi_bindgen]
impl Struct {
    pub fn new(elem: u32) -> Self {
        Self {elem, elems: vec![]}
    }
    pub fn get_elems(&self) -> impl ExactSizeIterator<&u32> {
        self.elems.iter()
    }
}

fn main() {
    let s = struct_new(42);
    let elems = MaybeUninit::<*const *const u32>::uninit();
    let elems_len = MaybeUninit::<usize>::uninit();
    if FfiResult::Ok != unsafe {struct_elems(&s, elems.as_mut_ptr(), elems_len.as_mut_ptr())} {
        // TODO: Handle error
    }
    let elems_len = unsafe {elems_len.assume_init()};
    let elems = unsafe {elems.assume_init()};
}

Alternate Designs [optional]

List out ffi functions manually. This is tedious and highly error prone because we're operating in the unsafe space

@mversic mversic force-pushed the data_model_generate_ffi_fns branch 2 times, most recently from d765a26 to 500e051 Compare May 13, 2022 17:43
@mversic mversic changed the base branch from main to iroha2-dev May 13, 2022 17:44
@mversic mversic force-pushed the data_model_generate_ffi_fns branch 2 times, most recently from 8c1d792 to 6afd741 Compare May 13, 2022 18:00
@codecov
Copy link

codecov bot commented May 13, 2022

Codecov Report

Merging #2211 (92c9f2f) into iroha2-dev (fe33f0f) will increase coverage by 0.80%.
The diff coverage is 83.28%.

@@              Coverage Diff               @@
##           iroha2-dev    #2211      +/-   ##
==============================================
+ Coverage       63.89%   64.69%   +0.80%     
==============================================
  Files             127      131       +4     
  Lines           23659    24589     +930     
==============================================
+ Hits            15117    15909     +792     
- Misses           8542     8680     +138     
Impacted Files Coverage Δ
client/src/config.rs 69.35% <0.00%> (ø)
client_cli/src/main.rs 0.29% <0.00%> (-0.01%) ⬇️
core/src/lib.rs 100.00% <ø> (ø)
core/src/smartcontracts/isi/domain.rs 39.13% <ø> (ø)
core/src/smartcontracts/isi/mod.rs 52.86% <ø> (ø)
data_model/src/permissions.rs 58.62% <0.00%> (-0.64%) ⬇️
data_model/src/role.rs 2.81% <0.00%> (-0.09%) ⬇️
data_model/src/trigger/mod.rs 22.82% <ø> (ø)
tools/kagami/src/main.rs 0.52% <0.00%> (+<0.01%) ⬆️
data_model/src/asset.rs 51.46% <46.00%> (-0.92%) ⬇️
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe33f0f...92c9f2f. Read the comment docs.

@mversic mversic force-pushed the data_model_generate_ffi_fns branch 2 times, most recently from 3ed1d34 to 2a71e93 Compare May 13, 2022 19:38
@appetrosyan appetrosyan added iroha2-dev The re-implementation of a BFT hyperledger in RUST Enhancement New feature or request labels May 17, 2022
@appetrosyan appetrosyan self-assigned this May 17, 2022
@mversic mversic marked this pull request as ready for review May 17, 2022 07:59
@mversic mversic force-pushed the data_model_generate_ffi_fns branch 2 times, most recently from b03f55e to e6198a7 Compare May 17, 2022 20:59
@s8sato s8sato self-assigned this May 19, 2022
@mversic mversic force-pushed the data_model_generate_ffi_fns branch 3 times, most recently from 11ef2f1 to 85586d1 Compare May 19, 2022 22:55
@appetrosyan
Copy link
Contributor

Please squash this PR with care

client/tests/integration/tx_rollback.rs Show resolved Hide resolved
configs/peer/genesis.json Show resolved Hide resolved
core/Cargo.toml Show resolved Hide resolved
core/src/genesis.rs Outdated Show resolved Hide resolved
core/src/smartcontracts/isi/domain.rs Show resolved Hide resolved
data_model/src/account.rs Outdated Show resolved Hide resolved
@mversic mversic force-pushed the data_model_generate_ffi_fns branch 3 times, most recently from d9d9c8d to b76050e Compare May 22, 2022 13:11
@mversic mversic force-pushed the data_model_generate_ffi_fns branch from 8665315 to e52a4a5 Compare May 23, 2022 08:41
mversic added 13 commits May 23, 2022 10:43
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
@mversic mversic force-pushed the data_model_generate_ffi_fns branch from e52a4a5 to e12c399 Compare May 23, 2022 08:43
@mversic mversic force-pushed the data_model_generate_ffi_fns branch 4 times, most recently from ca09f8f to 9190960 Compare May 23, 2022 09:08
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
@mversic mversic force-pushed the data_model_generate_ffi_fns branch from 9190960 to 92c9f2f Compare May 23, 2022 09:38
@appetrosyan appetrosyan merged commit 05a288c into hyperledger-iroha:iroha2-dev May 24, 2022
pesterev pushed a commit to pesterev/iroha that referenced this pull request May 25, 2022
…del` (hyperledger-iroha#2211)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
BAStos525 pushed a commit to BAStos525/soramitsu-iroha that referenced this pull request Jul 8, 2022
…del` (hyperledger-iroha#2211)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: BAStos525 <jungle.vas@yandex.ru>
@mversic mversic deleted the data_model_generate_ffi_fns branch December 27, 2022 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

data_model FFI bindings
3 participants