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

Add Mapping::contains(key) and Mapping::insert_return_size(key, val) #1224

Merged
merged 14 commits into from
May 3, 2022

Conversation

agryaznov
Copy link
Contributor

@agryaznov agryaznov commented Apr 22, 2022

This adds:

fn Mapping::contains(key) -> Option<u32>;
fn Mapping::insert_return_size(key, val) -> Option<u32>; 

both functions return the size of the (previously) stored value for the key.

Needed for link#1

@athei
Copy link
Contributor

athei commented Apr 22, 2022

Can you satisfy the CI?

@agryaznov agryaznov changed the title Add storage_contains() Add Mapping::contains() Apr 22, 2022
@agryaznov agryaznov changed the title Add Mapping::contains() Add Mapping::contains(key) and Mapping::insert_check(key) Apr 22, 2022
@agryaznov agryaznov changed the title Add Mapping::contains(key) and Mapping::insert_check(key) Add Mapping::contains(key) and Mapping::insert_check(key, val) Apr 22, 2022
@athei
Copy link
Contributor

athei commented Apr 24, 2022

Let's call it insert_return_size. I know it is long but this checked suffix usually suggests that it is somehow safer because something is checked beforehand. We will remove this function for ink! 4.0 anyways (just roll it into insert then).

@agryaznov agryaznov changed the title Add Mapping::contains(key) and Mapping::insert_check(key, val) Add Mapping::contains(key) and Mapping::insert_return_size(key, val) Apr 24, 2022
@agryaznov
Copy link
Contributor Author

Not sure why ink-waterfall fails in CI, but it looks like something unrelated to this PR.

crates/engine/src/ext.rs Outdated Show resolved Hide resolved
crates/env/src/api.rs Outdated Show resolved Hide resolved
crates/env/src/api.rs Outdated Show resolved Hide resolved
crates/env/src/backend.rs Outdated Show resolved Hide resolved
@paritytech-cicd-pr
Copy link

paritytech-cicd-pr commented May 2, 2022

🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑

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

Δ Optimized Size Δ Used Gas Total Optimized Size Total Used Gas
accumulator +0.01 K 1.04 K
adder +0.01 K 2.14 K
contract-introspection 2.37 K
contract-terminate 0.94 K 213_894
contract-transfer 8.13 K 13_894
delegate-calls +0.01 K +1 2.96 K 14_416
delegator +0.01 K +2 6.38 K 44_667
dns +0.01 K 8.85 K 41_682
erc1155 +0.01 K 17.35 K 83_364
erc20 +0.01 K 8.50 K 41_682
erc721 +0.01 K 11.81 K 111_152
flipper +0.01 K 1.32 K 13_894
forward-calls +0.01 K +1 2.89 K 28_475
incrementer +0.01 K 1.22 K
multisig +0.01 K +2 25.34 K 90_233
rand-extension +0.01 K 3.93 K 13_894
seal-code-hash 1.59 K
set-code-hash +0.01 K 1.58 K 27_788
subber +0.01 K 2.16 K
trait-erc20 +0.01 K 8.77 K 41_682
trait-flipper +0.01 K 1.01 K 13_894
trait-incrementer +0.01 K 1.19 K 27_788
updated-incrementer +0.01 K 9.70 K
upgradeable-flipper +0.01 K 1.56 K

Link to the run | Last update: Mon May 2 19:12:38 CEST 2022

agryaznov and others added 2 commits May 2, 2022 19:35
@codecov-commenter
Copy link

Codecov Report

Merging #1224 (c9b8b12) into master (67290c2) will decrease coverage by 15.95%.
The diff coverage is 68.75%.

@@             Coverage Diff             @@
##           master    #1224       +/-   ##
===========================================
- Coverage   78.80%   62.84%   -15.96%     
===========================================
  Files         229      228        -1     
  Lines        8681     8654       -27     
===========================================
- Hits         6841     5439     -1402     
- Misses       1840     3215     +1375     
Impacted Files Coverage Δ
crates/env/src/backend.rs 83.33% <ø> (ø)
crates/storage/src/lazy/mapping.rs 54.28% <ø> (ø)
crates/env/src/api.rs 30.88% <25.00%> (-1.43%) ⬇️
crates/env/src/engine/off_chain/impls.rs 41.97% <50.00%> (-0.53%) ⬇️
crates/engine/src/ext.rs 73.17% <100.00%> (-0.33%) ⬇️
crates/storage/src/traits/impls/mod.rs 77.77% <100.00%> (ø)
crates/storage/src/traits/mod.rs 100.00% <100.00%> (ø)
crates/storage/src/traits/optspec.rs 93.10% <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%) ⬇️
... and 46 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 67290c2...c9b8b12. Read the comment docs.

Copy link
Collaborator

@cmichi cmichi left a comment

Choose a reason for hiding this comment

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

🙌

@cmichi cmichi merged commit 336d3e3 into master May 3, 2022
@cmichi cmichi deleted the ag-contains branch May 3, 2022 06:04
@dayday2019
Copy link

This adds:

fn Mapping::contains(key) -> Option<u32>;
fn Mapping::insert_return_size(key, val) -> Option<u32>; 

both functions return the size of the (previously) stored value for the key.

Needed for link#1

i have update dependencies version to 3.0.1 in cargo.toml ,however cargo +nightly contract build

This adds:

fn Mapping::contains(key) -> Option<u32>;
fn Mapping::insert_return_size(key, val) -> Option<u32>; 

both functions return the size of the (previously) stored value for the key.

Needed for link#1

i have updated to 3.0.1 in cargo.toml, however it displayed this : error[E0599]: no method named contains found for struct Mapping in the current scope ,when i run this command : cargo +nightly contract build.

[dependencies]
ink_primitives = { version = "3.0.1", default-features = false }
ink_prelude = { version = "3.0.1", default-features = false }
ink_metadata = { version = "3.0.1", default-features = false, features = ["derive"], optional = true }
ink_env = { version = "3.0.1", default-features = false }
ink_storage = { version = "3.0.1", default-features = false }
ink_lang = { version = "3.0.1", default-features = false }

@agryaznov
Copy link
Contributor Author

i have updated to 3.0.1 in cargo.toml, however it displayed this : error[E0599]: no method named contains found for struct Mapping in the current scope ,when i run this command : cargo +nightly contract build.

3.0.1 was released on 6th of April, it doesn't include this PR. Please use master branch or wait for the next release.

agryaznov added a commit that referenced this pull request Jun 6, 2022
agryaznov added a commit that referenced this pull request Jun 22, 2022
* Fix links in release notes (#1277)

* Revert "Optimise deny_payment. Use eerywhere semantic of deny. (#1267)"

This reverts commit 1bfccc7.

* Revert backward-incompatible piece of #1224: dependency on `[seal1] seal_set_storage`

* Revert backward-incompatible piece of #1233: removal of eth_compatibility crate

* bump crate versions + update RELEASES.md

* Mapping::insert_return_size is back, having now both `seal1` and `seal0` seal_set_storage versions used

* set_storage_silent -> set_storage_compat renaming

* spell fix

* Apply suggestions from code review

Co-authored-by: Michael Müller <mich@elmueller.net>
Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* Update crates/env/src/backend.rs

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* doc comments enhanced

* `Mapping::insert()` to use backwards compatible seal fn

* unreleased changes planned for 4.x removed from 3.x

* Add more details to the release notes

* fix catched issue with changed api function signature

* fix storage trait dependent func

* Apply new versions naming policy: step1. Old versions to keep their names.

* Apply new versions naming policy: step2. Add `deprecated` attr and `# Compatibility` docs section

* Apply suggestions from code review

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* fixes after next review round

* more fixes after the review round

* fmt

* spellcheck config fix

* Small wording fixes

Co-authored-by: Michael Müller <mich@elmueller.net>
Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
Co-authored-by: Hernando Castano <hernando@hcastano.com>
@ascjones ascjones mentioned this pull request Feb 15, 2023
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.

6 participants