Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

[contracts] Port host functions to Weight V2 and storage deposit limit #13565

Merged
merged 35 commits into from
Apr 26, 2023

Conversation

agryaznov
Copy link
Contributor

@agryaznov agryaznov commented Mar 8, 2023

Port host functions to Weight V2 and storage deposit limit

  • All host functions dealing with gas now have new version which accounts for the proof_size weight as well.
  • Added DefaultDepositLimit pallet config parameter, to allow chains set a fallback default value when a caller doesn't set storage_deposit_limit to a call.
  • New versions of host functions call and instantiate accept storage deposit limit parameter which allow to set the limit for the sub-call. This might be helpful for cross-contract calls. This limit is enforced right after execution returns from the sub-call, and the sub-call is rolled back if the limit has been exhausted during the call.

Drive-by changes

  • Add max_runtime_mem to schedule limits in order to make its configuration for integrity_test smooth.

@agryaznov agryaznov added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Mar 8, 2023
@agryaznov agryaznov self-assigned this Mar 8, 2023
@agryaznov agryaznov marked this pull request as ready for review March 10, 2023 15:30
@agryaznov agryaznov requested a review from athei as a code owner March 10, 2023 15:30
@agryaznov agryaznov marked this pull request as draft March 10, 2023 15:35
add test for nested call deposit limit

save: separate deposit limit for nested calls
save: works with test

cleaned up debugging outputs
@agryaznov agryaznov changed the title [contracts] Port host functions to Weight V2 and set default deposit limit [contracts] Port host functions to Weight V2 and storage deposit limit Mar 15, 2023
@agryaznov agryaznov marked this pull request as ready for review March 15, 2023 08:39
@athei
Copy link
Member

athei commented Mar 15, 2023

Tests are failing.

@agryaznov
Copy link
Contributor Author

@pgherveou your comments have been addressed

Copy link
Contributor

@pgherveou pgherveou left a comment

Choose a reason for hiding this comment

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

left a few more nits

frame/contracts/src/storage/meter.rs Outdated Show resolved Hide resolved
frame/contracts/src/storage/meter.rs Show resolved Hide resolved
frame/contracts/src/lib.rs Show resolved Hide resolved
@@ -1076,8 +1090,9 @@ pub mod env {

/// Clear the value at the given key in the contract storage.
///
/// Equivalent to the newer version [`super::seal1::Api::clear_storage`] with the exception of
/// the return type. Still a valid thing to call when not interested in the return value.
/// Equivalent to the newer [`seal1`][`super::api_doc::Version1::clear_storage`] version with
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we can use Current instead of Version1 to make these links more future proof

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice idea!

frame/contracts/src/benchmarking/mod.rs Outdated Show resolved Hide resolved
Co-authored-by: PG Herveou <pgherveou@gmail.com>
@agryaznov
Copy link
Contributor Author

bot merge

@paritytech-processbot
Copy link

Error: Statuses failed for 0dac6f6

@agryaznov
Copy link
Contributor Author

bot merge force

@paritytech-processbot paritytech-processbot bot merged commit af504bc into master Apr 26, 2023
@paritytech-processbot paritytech-processbot bot deleted the ag-proof_limit branch April 26, 2023 11:27
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-benches
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2734781

@ggwpez
Copy link
Member

ggwpez commented Apr 26, 2023

There is a failing test from this MR IIUC: deposit_limit_in_nested_instantiate CI log.
in #13945 after i rebased. @agryaznov

gpestana pushed a commit that referenced this pull request May 4, 2023
#13565)

* added [unstable][seal2] call()

* updated test to cover new seal_call proof_limit

* docs updated

* add [seal2][unstable] instantiate() and test

* add [seal2][unstable] weight_to_fee() + docs and test

* add [seal2][unstable] gas_left() + docs and test

* update benchmarks

* add DefaultDepositLimit to pallet Config

* specify deposit limit for nested call

add test for nested call deposit limit

save: separate deposit limit for nested calls

* specify deposit limit for nested instantiate

save: works with test

cleaned up debugging outputs

* update benchmarks

* added missing fixtures

* fix benches

* pass explicit deposit limit to storage bench

* explicit deposit limit for another set_storage bench

* add more deposit limit for storage benches

* moving to simplified benchmarks

* moved to simplified benchmarks

* fix seal_weight_to_fee bench

* fix seal_instantiate benchmark

* doc typo fix

* default dl for benchmarking

more dl for tests

dl for tests to max

deposit_limit fix in instantiate bench

fix instantiate bench

fix instantiate benchmark

fix instantiate bench again

remove dbg

fix seal bench again

fixing it still

seal_instantiate zero deposit

less runs to check if deposit enough

try

try 2

try 3

try 4

* max_runtime_mem to Schedule limits

* add default deposit limit fallback check to test

* weight params renaming

* fmt

* Update frame/contracts/src/benchmarking/mod.rs

Co-authored-by: PG Herveou <pgherveou@gmail.com>

* prettify inputs in tests

* typestate param refactored

---------

Co-authored-by: PG Herveou <pgherveou@gmail.com>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
paritytech#13565)

* added [unstable][seal2] call()

* updated test to cover new seal_call proof_limit

* docs updated

* add [seal2][unstable] instantiate() and test

* add [seal2][unstable] weight_to_fee() + docs and test

* add [seal2][unstable] gas_left() + docs and test

* update benchmarks

* add DefaultDepositLimit to pallet Config

* specify deposit limit for nested call

add test for nested call deposit limit

save: separate deposit limit for nested calls

* specify deposit limit for nested instantiate

save: works with test

cleaned up debugging outputs

* update benchmarks

* added missing fixtures

* fix benches

* pass explicit deposit limit to storage bench

* explicit deposit limit for another set_storage bench

* add more deposit limit for storage benches

* moving to simplified benchmarks

* moved to simplified benchmarks

* fix seal_weight_to_fee bench

* fix seal_instantiate benchmark

* doc typo fix

* default dl for benchmarking

more dl for tests

dl for tests to max

deposit_limit fix in instantiate bench

fix instantiate bench

fix instantiate benchmark

fix instantiate bench again

remove dbg

fix seal bench again

fixing it still

seal_instantiate zero deposit

less runs to check if deposit enough

try

try 2

try 3

try 4

* max_runtime_mem to Schedule limits

* add default deposit limit fallback check to test

* weight params renaming

* fmt

* Update frame/contracts/src/benchmarking/mod.rs

Co-authored-by: PG Herveou <pgherveou@gmail.com>

* prettify inputs in tests

* typestate param refactored

---------

Co-authored-by: PG Herveou <pgherveou@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants