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

contracts: Improve contract address derivation #12883

Merged
merged 14 commits into from
Dec 22, 2022
Merged

contracts: Improve contract address derivation #12883

merged 14 commits into from
Dec 22, 2022

Conversation

athei
Copy link
Member

@athei athei commented Dec 9, 2022 •
edited
Loading

  • Add a prefix in order to prevent collisions with other modules
  • Include the constructor arguments in the derivation as they change the behavior of a contract
  • Remove now obsolete trait bounds (we create the id using Decode now)
  • Changed instantiate benchmark to check for the influence of input bytes len on runtime (they are hashed now)

The bulk of the changes are within tests where we move away from calling Pallet::contract_address where possible and use Pallet::bare_instantiate instead which returns the new contracts address.

@athei athei added A0-please_review Pull request needs code review. B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Dec 9, 2022
@athei
Copy link
Member Author

athei commented Dec 9, 2022

/cmd queue -c bench-bot $ pallet dev pallet_contracts

@command-bot
Copy link

command-bot bot commented Dec 9, 2022 •
edited
Loading

@athei https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2139549 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.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 /cmd cancel 43-e5eca75f-14cb-4ea4-9aa5-788c45b3aa41 to cancel this command or /cmd cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Dec 9, 2022

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

@athei
Copy link
Member Author

athei commented Dec 9, 2022

/cmd queue -c bench-bot $ pallet dev pallet_contracts

@command-bot
Copy link

command-bot bot commented Dec 9, 2022 •
edited
Loading

@athei https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2141057 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.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 /cmd cancel 44-094e970b-fb71-4a9b-b91b-7f5aa94a7d47 to cancel this command or /cmd cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Dec 9, 2022

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

@jakoblell jakoblell added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Dec 12, 2022
agryaznov
agryaznov approved these changes Dec 13, 2022
View reviewed changes
frame/contracts/src/tests.rs Outdated Show resolved Hide resolved
frame/contracts/src/benchmarking/mod.rs Outdated Show resolved Hide resolved
@athei
Copy link
Member Author

athei commented Dec 22, 2022

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 96e71b5 into master Dec 22, 2022
@paritytech-processbot paritytech-processbot bot deleted the at/contract-addr branch December 22, 2022 14:52
@jasl jasl mentioned this pull request Jan 19, 2023
Merged
@librelois librelois mentioned this pull request Jan 23, 2023
Closed
@AurevoirXavier AurevoirXavier mentioned this pull request Feb 8, 2023
Closed
@kacperzuk-neti kacperzuk-neti mentioned this pull request Feb 14, 2023
Merged
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
@athei @agryaznov @ltfschoen
contracts: Improve contract address derivation (paritytech#12883)
69e5b4b 
* Add prefix to address derivation

* Extend benchmark

* Fix node test

* ".git/.scripts/bench-bot.sh" pallet dev pallet_contracts

* Adapt to new benchmark

* Update dispatchable benchmarks

* ".git/.scripts/bench-bot.sh" pallet dev pallet_contracts

* Use benchmark results

* Apply suggestions from code review

Co-authored-by: Sasha Gryaznov <[email protected]>

* Don't use T::AdressGenerator directly

* Rename constructor_args to input_data

Co-authored-by: command-bot <>
Co-authored-by: Sasha Gryaznov <[email protected]>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@athei @agryaznov @ark0f
contracts: Improve contract address derivation (paritytech#12883)
38fa012 
* Add prefix to address derivation

* Extend benchmark

* Fix node test

* ".git/.scripts/bench-bot.sh" pallet dev pallet_contracts

* Adapt to new benchmark

* Update dispatchable benchmarks

* ".git/.scripts/bench-bot.sh" pallet dev pallet_contracts

* Use benchmark results

* Apply suggestions from code review

Co-authored-by: Sasha Gryaznov <[email protected]>

* Don't use T::AdressGenerator directly

* Rename constructor_args to input_data

Co-authored-by: command-bot <>
Co-authored-by: Sasha Gryaznov <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@xermicus xermicus xermicus approved these changes

@agryaznov agryaznov agryaznov approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited
Projects
Smart Contracts
Status: Done ✅
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@athei @agryaznov @xermicus @jakoblell