Skip to content

Implement utility methods for experimental off-chain testing env#1143

Merged
HCastano merged 10 commits intomasterfrom
cmichi-implement-utility-methods-for-new-off-chain-env
Feb 26, 2022
Merged

Implement utility methods for experimental off-chain testing env#1143
HCastano merged 10 commits intomasterfrom
cmichi-implement-utility-methods-for-new-off-chain-env

Conversation

@cmichi
Copy link
Copy Markdown
Collaborator

@cmichi cmichi commented Feb 22, 2022

Closes #787.

Well, except an implementation for gas_left, because I'm uncertain what a sensible logic for that would be.

Yes, the experimental off-chain env is currently limited in that it only supports ink_env::DefaultEnvironment, but IMHO the situation of us currently having two off-chain testing engines in every examples is worse.

So I would like to get the experimental engine into a stage where we can throw the old one out and just use this new one by default everywhere. From my pov this PR is the last prerequisite before we can do that.

@cmichi
Copy link
Copy Markdown
Collaborator Author

cmichi commented Feb 22, 2022

The waterfall CI failure is expected because of the bug in nightly Rust which causes cargo-contract to fail.

@paritytech-cicd-pr
Copy link
Copy Markdown

🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑

⚠️ The ink! master is ahead of your branch, this might skew the comparison data below.

These are the results when building the examples/* contracts from this branch with cargo-contract 0.17.0-ab41bd9 and comparing them to ink! master:

Δ Optimized Size Δ Used Gas Total Optimized Size Total Used Gas
accumulator 1.28 K
adder 2.23 K
contract-terminate 1.23 K 215_704
contract-transfer 8.37 K 15_704
delegator 6.26 K 33_142
dns 9.64 K 47_112
erc1155 27.71 K 94_224
erc20 9.12 K 47_112
erc721 14.40 K 125_632
flipper 1.57 K 15_704
incrementer 1.49 K 15_704
multisig 26.38 K 103_006
proxy 2.98 K 32_082
rand-extension 4.40 K 15_704
subber 2.24 K
trait-erc20 9.39 K 47_112
trait-flipper 1.35 K 15_704
trait-incrementer 1.45 K 31_408

Link to the run | Last update: Wed Feb 23 06:44:09 CET 2022

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 23, 2022

Codecov Report

Merging #1143 (8032fd1) into master (9fd0317) will decrease coverage by 15.62%.
The diff coverage is 60.97%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1143       +/-   ##
===========================================
- Coverage   78.67%   63.04%   -15.63%     
===========================================
  Files         252      252               
  Lines        9386     9411       +25     
===========================================
- Hits         7384     5933     -1451     
- Misses       2002     3478     +1476     
Impacted Files Coverage Δ
crates/engine/src/types.rs 66.66% <ø> (ø)
.../env/src/engine/experimental_off_chain/test_api.rs 83.67% <ø> (ø)
crates/engine/src/ext.rs 74.05% <45.83%> (+1.82%) ⬆️
crates/engine/src/test_api.rs 91.20% <57.14%> (-2.84%) ⬇️
crates/engine/src/exec_context.rs 100.00% <100.00%> (ø)
crates/lang/codegen/src/traits.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/ir/src/ir/contract.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/codegen/src/generator/env.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/codegen/src/generator/mod.rs 0.00% <0.00%> (-100.00%) ⬇️
crates/lang/codegen/src/enforced_error.rs 0.00% <0.00%> (-100.00%) ⬇️
... and 57 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 9fd0317...8032fd1. Read the comment docs.

Copy link
Copy Markdown
Contributor

@HCastano HCastano left a comment

Choose a reason for hiding this comment

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

Small things to clean up

Comment on lines +17 to +24
//! In the long-term all types in this module should be `Vec<u8>`, as to not
//! be dependent on any specific off-chain environment type. Then the `engine`
//! crate could be used with an arbitrary `Environment` configuration.
//!
//! Right now the issue is that if e.g. some test calls an `engine` API function
//! to transfer some balance, then there has to be some `+` operation to the
//! existing balance. And we can't just add a `Vec<u8>` onto another one, we
//! need to know the type.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems like an internal comment (and something that should be part of the Github issue), not something that should be in the module level comments

Copy link
Copy Markdown
Collaborator Author

@cmichi cmichi Feb 25, 2022

Choose a reason for hiding this comment

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

Removed the comment, will create a follow-up once merged.

Co-authored-by: Alexander Theißen <[email protected]>
@HCastano HCastano merged commit 34bd952 into master Feb 26, 2022
@HCastano HCastano deleted the cmichi-implement-utility-methods-for-new-off-chain-env branch February 26, 2022 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New off-chain engine: Implement utility methods

5 participants