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

contracts: Refactor instantiate with code #14503

Merged
merged 31 commits into from
Jul 7, 2023
Merged

contracts: Refactor instantiate with code #14503

merged 31 commits into from
Jul 7, 2023

Conversation

pgherveou
Copy link
Contributor

@pgherveou pgherveou commented Jul 3, 2023 •
edited
Loading

Refactor instantiate_with_code, so that it always store the code first and then instantiate the contract.
This simplify the logic a bit and ensure that CodeInfo is always in storage before the code is instantiated.

Fixes #14494

@pgherveou pgherveou changed the title pg/simplify instantiate with code contracts: instantiate with code Jul 3, 2023
@pgherveou pgherveou changed the title contracts: instantiate with code contracts: Refactor instantiate with code Jul 3, 2023
@pgherveou pgherveou force-pushed the pg/simplify_instantiate_with_code branch from d8f2b02 to 48e5a34 Compare July 3, 2023 19:22
@pgherveou pgherveou marked this pull request as ready for review July 3, 2023 21:38
@pgherveou pgherveou requested a review from athei as a code owner July 3, 2023 21:38
@pgherveou pgherveou requested a review from a team July 3, 2023 21:38
@pgherveou pgherveou added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Jul 3, 2023
@pgherveou
Copy link
Contributor Author

bot bench $ pallet dev pallet_contracts

@command-bot
Copy link

command-bot bot commented Jul 3, 2023 •
edited
Loading

@pgherveou https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3115214 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_contracts. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 17-bb80dee3-9271-47e4-a0fe-ef82c4127ae8 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jul 3, 2023

@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_contracts has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3115214 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3115214/artifacts/download.

@pgherveou pgherveou force-pushed the pg/simplify_instantiate_with_code branch from 703115a to 6a2aa17 Compare July 4, 2023 05:22
@pgherveou
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Rebased

@athei
Copy link
Member

athei commented Jul 4, 2023

Does it fix #14494?

@pgherveou
Copy link
Contributor Author

Does it fix #14494?

ha nice missed that issue, it should yes, I am refactoring this other PR (currently broken) #14079 that depends on it, let me go through it to verify that it indeed improve things

@pgherveou
Copy link
Contributor Author

bot bench $ pallet dev pallet_contracts

@command-bot
Copy link

command-bot bot commented Jul 5, 2023 •
edited
Loading

@pgherveou https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3130950 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_contracts. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 3-5c51622d-76d2-4f1a-b25a-334bc22758e3 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jul 5, 2023

@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_contracts has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3130950 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3130950/artifacts/download.

@pgherveou
Copy link
Contributor Author

https://weights.tasty.limo/compare?unit=weight&ignore_errors=true&threshold=10&method=asymptotic&repo=substrate&old=pg/simplify_instantiate_with_code&new=master&path_pattern=frame/**/src/weights.rs

agryaznov
agryaznov reviewed Jul 5, 2023
View reviewed changes
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/wasm/mod.rs Outdated Show resolved Hide resolved
frame/contracts/src/wasm/mod.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Show resolved Hide resolved
frame/contracts/src/lib.rs Show resolved Hide resolved
frame/contracts/src/wasm/mod.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Show resolved Hide resolved
frame/contracts/src/lib.rs Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
pgherveou and others added 6 commits July 6, 2023 09:00
@pgherveou @agryaznov
Update frame/contracts/src/lib.rs
e8ceb0d 
Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>
@pgherveou @agryaznov
Update frame/contracts/src/wasm/mod.rs
9eb7833 
Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>
@pgherveou @agryaznov
Update frame/contracts/src/wasm/mod.rs
96e3dcf 
Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>
@pgherveou
PR review, rm duplicate increment_refcount
a9df0cc
@pgherveou
PR review
b291902
@pgherveou @agryaznov
Update frame/contracts/src/wasm/prepare.rs
bc2057d 
Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>
@pgherveou
Copy link
Contributor Author

image

@pgherveou
Copy link
Contributor Author

bot bench $ pallet dev pallet_contracts

@command-bot
Copy link

command-bot bot commented Jul 6, 2023 •
edited
Loading

@pgherveou https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3139251 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_contracts. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 1-a07cfa94-6594-4fbb-be71-0d2f39934036 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jul 6, 2023

@pgherveou Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_contracts has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3139251 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3139251/artifacts/download.

@pgherveou
Copy link
Contributor Author

The +155% performance decrease for seal_ecdsa is related to this PR that revert a recent attempt to start using a C lib instead of k256 see #14499

@pgherveou pgherveou mentioned this pull request Jul 7, 2023
Merged
@athei athei requested a review from juangirini July 7, 2023 13:15
@pgherveou
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 7aacdf1 into master Jul 7, 2023
@paritytech-processbot paritytech-processbot bot deleted the pg/simplify_instantiate_with_code branch July 7, 2023 13:46
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@pgherveou @agryaznov @nathanwhit
contracts: Refactor instantiate with code (paritytech#14503)
2c2f54f 
* wip

* fixes

* rm comment

* join fns

* clippy

* Fix limits

* reduce diff

* fix

* fix

* fix typo

* refactor store to  use self

* refactor run to take self by value

* pass tests

* rm comment

* fixes

* fix typo

* rm

* fix fmt

* clippy

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Update frame/contracts/src/lib.rs

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

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

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

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

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

* PR review, rm duplicate increment_refcount

* PR review

* Update frame/contracts/src/wasm/prepare.rs

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

* Add test for failing storage_deposit

* fix lint

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

---------

Co-authored-by: command-bot <>
Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@agryaznov agryaznov agryaznov approved these changes

@athei athei athei approved these changes

@juangirini juangirini Awaiting requested review from juangirini

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

contracts: Simplify instantiate_with_code logic
3 participants
@pgherveou @athei @agryaznov