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

[contracts] Forbid calling back to contracts after switching to runtime #13443

Merged
merged 19 commits into from
Mar 6, 2023

Conversation

agryaznov
Copy link
Contributor

Pallet Contracts provides ways to call runtime from a contract, i.e. call_runtime and chain extensions.

With this PR, we guard contracts from back-forth invocation after switching once from contracts engine to runtime, to avoid possible re-entrancy flaws.

@agryaznov agryaznov added A0-please_review Pull request needs code review. C3-medium PR touches the given topic and has a medium impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. B1-note_worthy Changes should be noted in the release notes labels Feb 22, 2023
@agryaznov agryaznov changed the title [contracts] forbid calling back to contracts after switching to runtime [contracts] Forbid calling back to contracts after switching to runtime Feb 22, 2023
@agryaznov agryaznov marked this pull request as ready for review February 22, 2023 19:51
@agryaznov agryaznov requested a review from athei as a code owner February 22, 2023 19:51
@agryaznov agryaznov requested a review from xermicus February 22, 2023 19:55
@athei athei added T4-smart_contracts This PR/Issue is related to smart contracts. T1-runtime This PR/Issue is related to the topic “runtime”. labels Feb 22, 2023
frame/contracts/Cargo.toml Show resolved Hide resolved
frame/contracts/CHANGELOG.md Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.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 Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/tests.rs Outdated Show resolved Hide resolved
@athei athei removed the T4-smart_contracts This PR/Issue is related to smart contracts. label Feb 23, 2023
@agryaznov
Copy link
Contributor Author

@athei your feedback is addressed, thanks!

@agryaznov agryaznov requested review from athei and xermicus February 24, 2023 16:15
Copy link
Member

@athei athei left a comment

Choose a reason for hiding this comment

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

Just more nits.

frame/contracts/src/tests.rs Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
@agryaznov agryaznov requested a review from athei March 2, 2023 13:22
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
frame/contracts/src/lib.rs Outdated Show resolved Hide resolved
agryaznov and others added 2 commits March 3, 2023 14:43
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
@athei athei requested a review from xermicus March 3, 2023 12:55
@agryaznov
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 1d9a139 into master Mar 6, 2023
@paritytech-processbot paritytech-processbot bot deleted the ag-reentrancy branch March 6, 2023 08:40
@jakoblell jakoblell added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Mar 6, 2023
ukint-vs pushed a commit to gear-tech/substrate that referenced this pull request Apr 10, 2023
…me (paritytech#13443)

* save: compiles and tests pass

* save: added global

* done + test

* cleanup

* changelog update

* cleanup

* address feedback, step 1

* address feedback, step 2

* address feedback, step 3

* returned updated gas_estimation_call_runtime test

* clippy fix

* address feedback, step 4

* address feedback, step 5

* move data from context to inputs

* docs fix

* Apply suggestions from code review

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

* address feedback, step 6

---------

Co-authored-by: Alexander Theißen <alex.theissen@me.com>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
…me (paritytech#13443)

* save: compiles and tests pass

* save: added global

* done + test

* cleanup

* changelog update

* cleanup

* address feedback, step 1

* address feedback, step 2

* address feedback, step 3

* returned updated gas_estimation_call_runtime test

* clippy fix

* address feedback, step 4

* address feedback, step 5

* move data from context to inputs

* docs fix

* Apply suggestions from code review

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

* address feedback, step 6

---------

Co-authored-by: Alexander Theißen <alex.theissen@me.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. B1-note_worthy Changes should be noted in the release notes C3-medium PR touches the given topic and has a medium impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants