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] #1425: Wasm helper crate #1816

Merged
merged 2 commits into from
Jan 27, 2022

Conversation

mversic
Copy link
Contributor

@mversic mversic commented Jan 14, 2022

Description of the Change

Smartcontract example:

#![no_std]
#![no_main]

use iroha_wasm::{data_model::prelude::*, prelude::*};

#[iroha_wasm]
#[allow(clippy::needless_pass_by_value)]
fn _entry_point(_account_id: AccountId) {
    let new_account_id = AccountId::test("mad_hatter", "wonderland");
    let register_isi = RegisterBox::new(NewAccount::new(new_account_id));

    Instruction::Register(register_isi).execute();
}

The pattern I used for testing to inject mocked objects is something I saw in the mockall crate. I like it because it doesn't infect the external API unlike the pattern we use with traits. I previously used this crate successfully, although I think I encountered some bugs but didn't investigate into it.

Problems

  • including iroha_wasm in the workspace seems to affect cfg(test) resolution
  • iroha_wasm it has to be build with nightly compiler

Issue

Closes #1425

Benefits

Internal are hidden from the user. All user has to do is annotate one of their methods with iroha_wasm

Possible Drawbacks

Usage Examples or Tests [optional]

Alternate Designs [optional]

@mversic mversic added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Jan 14, 2022
@mversic mversic changed the title Wasm helper crate [feature] #1425: Wasm helper crate Jan 14, 2022
@mversic mversic force-pushed the wasm_client branch 2 times, most recently from b516ee2 to 26327cd Compare January 14, 2022 14:33
@codecov
Copy link

codecov bot commented Jan 14, 2022

Codecov Report

Merging #1816 (a143433) into iroha2-dev (d4dc200) will increase coverage by 0.53%.
The diff coverage is 100.00%.

Impacted file tree graph

@@              Coverage Diff               @@
##           iroha2-dev    #1816      +/-   ##
==============================================
+ Coverage       77.85%   78.38%   +0.53%     
==============================================
  Files             148      148              
  Lines           21501    21511      +10     
==============================================
+ Hits            16739    16862     +123     
+ Misses           4762     4649     -113     
Impacted Files Coverage Δ
crypto/src/hash.rs 97.72% <ø> (ø)
crypto/src/signature.rs 83.66% <ø> (-0.40%) ⬇️
data_model/src/lib.rs 68.78% <ø> (ø)
data_model/src/merkle.rs 93.75% <ø> (ø)
core/src/smartcontracts/wasm.rs 97.99% <100.00%> (+0.05%) ⬆️
data_model/src/metadata.rs 90.04% <100.00%> (ø)
data_model/src/query.rs 33.24% <100.00%> (ø)
core/src/block_sync.rs 89.30% <0.00%> (-1.26%) ⬇️
p2p/src/peer.rs 82.03% <0.00%> (ø)
... and 3 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 d4dc200...a143433. Read the comment docs.

quote! {
#[no_mangle]
unsafe extern "C" fn _iroha_wasm_main(ptr: u32, len: u32) {
#fn_name(iroha_wasm::_decode_from_raw::<AccountId>(ptr, len))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should the function #fn_name receive account_id as a parameter?

wasm/src/lib.rs Outdated Show resolved Hide resolved
@mversic mversic force-pushed the wasm_client branch 2 times, most recently from d2c862d to 0f9d1ff Compare January 14, 2022 17:06
@mversic mversic force-pushed the wasm_client branch 3 times, most recently from e7c26fd to 29a3f5c Compare January 14, 2022 17:22
wasm/src/lib.rs Outdated Show resolved Hide resolved
core/src/smartcontracts/wasm.rs Show resolved Hide resolved
wasm/derive/src/lib.rs Outdated Show resolved Hide resolved
wasm/src/lib.rs Show resolved Hide resolved
@mversic mversic force-pushed the wasm_client branch 4 times, most recently from 91c1360 to 947c58d Compare January 19, 2022 08:16
appetrosyan
appetrosyan previously approved these changes Jan 24, 2022
@mversic mversic force-pushed the wasm_client branch 8 times, most recently from 2fec581 to 238044a Compare January 26, 2022 07:01
.github/workflows/iroha2-dev-pr-wasm.yaml Outdated Show resolved Hide resolved
.github/workflows/iroha2-dev-pr-static.yml Show resolved Hide resolved
.github/workflows/iroha2-dev-pr-wasm.yaml Outdated Show resolved Hide resolved
.github/workflows/iroha2-dev-pr-wasm.yaml Outdated Show resolved Hide resolved
.github/workflows/iroha2-dev-pr-wasm.yaml Outdated Show resolved Hide resolved
.github/workflows/iroha2-dev-pr-wasm.yaml Outdated Show resolved Hide resolved
.github/workflows/iroha2.yml Outdated Show resolved Hide resolved
.github/workflows/iroha2-dev-pr-wasm.yaml Outdated Show resolved Hide resolved
wasm/derive/src/lib.rs Show resolved Hide resolved
@mversic mversic force-pushed the wasm_client branch 7 times, most recently from 59c2a50 to 565d0a3 Compare January 26, 2022 15:43
@mversic mversic force-pushed the wasm_client branch 3 times, most recently from 6e9dc8d to 1410de1 Compare January 26, 2022 17:10
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
@@ -2,6 +2,7 @@
**/blocks/
**/*.rs.bk
**/rusty-tags.vi
/**/Cargo.lock
Copy link
Contributor

Choose a reason for hiding this comment

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

That could break cargo chef in the iroha2-dev docker file. Are you sure you want to exclude it?

@mversic mversic merged commit 204163b into hyperledger-iroha:iroha2-dev Jan 27, 2022
@mversic mversic deleted the wasm_client branch April 16, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants