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

Improve on Mapping::contains + Migrate examples to new API #1242

Merged
merged 8 commits into from
May 18, 2022

Conversation

cmichi
Copy link
Collaborator

@cmichi cmichi commented May 3, 2022

Follow-up to #1224.

@paritytech-cicd-pr
Copy link

paritytech-cicd-pr commented May 3, 2022

🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑

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

Δ Optimized Size Δ Used Gas Total Optimized Size Total Used Gas
accumulator 1.04 K
adder 2.13 K
contract-introspection 2.37 K
contract-terminate 0.94 K 275_000
contract-transfer 8.13 K 75_000
data-structures 1.73 K
delegate-calls 2.96 K 76_276
delegator 6.38 K 232_281
dns +0.05 K 8.89 K 225_000
erc1155 -0.05 K 17.30 K 450_000
erc20 8.49 K 225_000
erc721 +0.01 K 11.82 K 600_000
flipper 1.31 K 75_000
forward-calls 2.88 K 151_427
incrementer 1.21 K
multisig -0.03 K -22 25.12 K 470_156
rand-extension 3.92 K 75_000
seal-code-hash 1.58 K
set-code-hash 1.57 K 150_000
subber 2.15 K
trait-erc20 8.77 K 225_000
trait-flipper 1.00 K 75_000
trait-incrementer 1.19 K 150_000
updated-incrementer 9.62 K
upgradeable-flipper 1.55 K

Link to the run | Last update: Fri May 13 12:03:38 CEST 2022

crates/engine/src/ext.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@agryaznov agryaznov left a comment

Choose a reason for hiding this comment

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

LGTM

crates/storage/src/lazy/mapping.rs Show resolved Hide resolved
crates/storage/src/lazy/mapping.rs Outdated Show resolved Hide resolved
crates/engine/src/ext.rs Outdated Show resolved Hide resolved
Co-authored-by: Alexander Gryaznov <hi@agryaznov.com>
@codecov-commenter
Copy link

codecov-commenter commented May 11, 2022

Codecov Report

Merging #1242 (a27a53b) into master (961c7ef) will decrease coverage by 0.07%.
The diff coverage is 0.00%.

❗ Current head a27a53b differs from pull request most recent head 1506bb3. Consider uploading reports for the commit 1506bb3 to get more accurate results

@@            Coverage Diff             @@
##           master    #1242      +/-   ##
==========================================
- Coverage   79.12%   79.05%   -0.08%     
==========================================
  Files         227      227              
  Lines        8638     8645       +7     
==========================================
- Hits         6835     6834       -1     
- Misses       1803     1811       +8     
Impacted Files Coverage Δ
crates/engine/src/ext.rs 70.17% <0.00%> (-3.00%) ⬇️
crates/env/src/engine/off_chain/impls.rs 44.70% <0.00%> (ø)
crates/storage/src/lazy/mapping.rs 54.28% <ø> (ø)
crates/lang/ir/src/ir/attrs.rs 81.99% <0.00%> (-0.28%) ⬇️

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 961c7ef...1506bb3. Read the comment docs.

@HCastano
Copy link
Contributor

From Michi, as to why this is marked as dont-merge

I'm just not merging it now because it breaks the examples/ if used with latest ink! 3.0.1 release.

@cmichi cmichi merged commit 3f6befa into master May 18, 2022
@cmichi cmichi deleted the cmichi-use-new-mapping-api-functions-in-examples branch May 18, 2022 05:54
@cmichi cmichi removed the E-dont-merge A pull request that should not be merged. label May 18, 2022
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.

7 participants