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

contracts: Fix double charge of gas for host functions #3361

Merged
merged 8 commits into from
Feb 21, 2024
Merged

Conversation

athei
Copy link
Member

@athei athei commented Feb 16, 2024

This PR is fixing a bug in the sync mechanism between wasmi and pallet-contracts. This bug leads to essentially double charging all the gas that was used during the execution of the host function. When the call host function is used for recursion this will lead to a quadratic amount of gas consumption with regard to the nesting depth.We also took the chance to refactor the code in question and improve the rust docs.

The bug was caused by not updating GasMeter::executor_consumed (previously engine_consumed) when leaving the host function. This lead to the value being stale (too low) when entering another host function.

substrate/frame/contracts/src/tests.rs Outdated Show resolved Hide resolved
substrate/frame/contracts/src/gas.rs Outdated Show resolved Hide resolved
@athei athei added the T7-smart_contracts This PR/Issue is related to smart contracts. label Feb 21, 2024
@athei athei enabled auto-merge February 21, 2024 09:11
@athei athei added this pull request to the merge queue Feb 21, 2024
Merged via the queue into master with commit f3a6b6d Feb 21, 2024
127 of 129 checks passed
@athei athei deleted the at/gas-sync branch February 21, 2024 10:40
kostekIV pushed a commit to Cardinal-Cryptography/polkadot-sdk that referenced this pull request Feb 28, 2024
This PR is fixing a bug in the sync mechanism between wasmi and
pallet-contracts. This bug leads to essentially double charging all the
gas that was used during the execution of the host function. When the
`call` host function is used for recursion this will lead to a quadratic
amount of gas consumption with regard to the nesting depth.We also took
the chance to refactor the code in question and improve the rust docs.

The bug was caused by not updating `GasMeter::executor_consumed`
(previously `engine_consumed`) when leaving the host function. This lead
to the value being stale (too low) when entering another host function.

---------

Co-authored-by: PG Herveou <pgherveou@gmail.com>
kostekIV pushed a commit to Cardinal-Cryptography/polkadot-sdk that referenced this pull request Feb 28, 2024
This PR is fixing a bug in the sync mechanism between wasmi and
pallet-contracts. This bug leads to essentially double charging all the
gas that was used during the execution of the host function. When the
`call` host function is used for recursion this will lead to a quadratic
amount of gas consumption with regard to the nesting depth.We also took
the chance to refactor the code in question and improve the rust docs.

The bug was caused by not updating `GasMeter::executor_consumed`
(previously `engine_consumed`) when leaving the host function. This lead
to the value being stale (too low) when entering another host function.

---------

Co-authored-by: PG Herveou <pgherveou@gmail.com>
kostekIV added a commit to Cardinal-Cryptography/polkadot-sdk that referenced this pull request Feb 28, 2024
…) (#4)

This PR is fixing a bug in the sync mechanism between wasmi and
pallet-contracts. This bug leads to essentially double charging all the
gas that was used during the execution of the host function. When the
`call` host function is used for recursion this will lead to a quadratic
amount of gas consumption with regard to the nesting depth.We also took
the chance to refactor the code in question and improve the rust docs.

The bug was caused by not updating `GasMeter::executor_consumed`
(previously `engine_consumed`) when leaving the host function. This lead
to the value being stale (too low) when entering another host function.

---------

Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Co-authored-by: PG Herveou <pgherveou@gmail.com>
lesniak43 pushed a commit to Cardinal-Cryptography/polkadot-sdk that referenced this pull request Mar 11, 2024
…) (#4)

This PR is fixing a bug in the sync mechanism between wasmi and
pallet-contracts. This bug leads to essentially double charging all the
gas that was used during the execution of the host function. When the
`call` host function is used for recursion this will lead to a quadratic
amount of gas consumption with regard to the nesting depth.We also took
the chance to refactor the code in question and improve the rust docs.

The bug was caused by not updating `GasMeter::executor_consumed`
(previously `engine_consumed`) when leaving the host function. This lead
to the value being stale (too low) when entering another host function.

---------

Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Co-authored-by: PG Herveou <pgherveou@gmail.com>
pmikolajczyk41 pushed a commit to Cardinal-Cryptography/polkadot-sdk that referenced this pull request Mar 18, 2024
This PR is fixing a bug in the sync mechanism between wasmi and
pallet-contracts. This bug leads to essentially double charging all the
gas that was used during the execution of the host function. When the
`call` host function is used for recursion this will lead to a quadratic
amount of gas consumption with regard to the nesting depth.We also took
the chance to refactor the code in question and improve the rust docs.

The bug was caused by not updating `GasMeter::executor_consumed`
(previously `engine_consumed`) when leaving the host function. This lead
to the value being stale (too low) when entering another host function.

---------

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

(cherry picked from commit f3a6b6d)
pmikolajczyk41 pushed a commit to Cardinal-Cryptography/polkadot-sdk that referenced this pull request Mar 18, 2024
This PR is fixing a bug in the sync mechanism between wasmi and
pallet-contracts. This bug leads to essentially double charging all the
gas that was used during the execution of the host function. When the
`call` host function is used for recursion this will lead to a quadratic
amount of gas consumption with regard to the nesting depth.We also took
the chance to refactor the code in question and improve the rust docs.

The bug was caused by not updating `GasMeter::executor_consumed`
(previously `engine_consumed`) when leaving the host function. This lead
to the value being stale (too low) when entering another host function.

---------

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

(cherry picked from commit f3a6b6d)
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
This PR is fixing a bug in the sync mechanism between wasmi and
pallet-contracts. This bug leads to essentially double charging all the
gas that was used during the execution of the host function. When the
`call` host function is used for recursion this will lead to a quadratic
amount of gas consumption with regard to the nesting depth.We also took
the chance to refactor the code in question and improve the rust docs.

The bug was caused by not updating `GasMeter::executor_consumed`
(previously `engine_consumed`) when leaving the host function. This lead
to the value being stale (too low) when entering another host function.

---------

Co-authored-by: PG Herveou <pgherveou@gmail.com>
lesniak43 pushed a commit to Cardinal-Cryptography/polkadot-sdk that referenced this pull request Apr 8, 2024
…) (#4)

This PR is fixing a bug in the sync mechanism between wasmi and
pallet-contracts. This bug leads to essentially double charging all the
gas that was used during the execution of the host function. When the
`call` host function is used for recursion this will lead to a quadratic
amount of gas consumption with regard to the nesting depth.We also took
the chance to refactor the code in question and improve the rust docs.

The bug was caused by not updating `GasMeter::executor_consumed`
(previously `engine_consumed`) when leaving the host function. This lead
to the value being stale (too low) when entering another host function.

---------

Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Co-authored-by: PG Herveou <pgherveou@gmail.com>
lesniak43 pushed a commit to Cardinal-Cryptography/polkadot-sdk that referenced this pull request Apr 22, 2024
…) (#4)

This PR is fixing a bug in the sync mechanism between wasmi and
pallet-contracts. This bug leads to essentially double charging all the
gas that was used during the execution of the host function. When the
`call` host function is used for recursion this will lead to a quadratic
amount of gas consumption with regard to the nesting depth.We also took
the chance to refactor the code in question and improve the rust docs.

The bug was caused by not updating `GasMeter::executor_consumed`
(previously `engine_consumed`) when leaving the host function. This lead
to the value being stale (too low) when entering another host function.

---------

Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Co-authored-by: PG Herveou <pgherveou@gmail.com>
lesniak43 pushed a commit to Cardinal-Cryptography/polkadot-sdk that referenced this pull request May 24, 2024
…) (#4)

This PR is fixing a bug in the sync mechanism between wasmi and
pallet-contracts. This bug leads to essentially double charging all the
gas that was used during the execution of the host function. When the
`call` host function is used for recursion this will lead to a quadratic
amount of gas consumption with regard to the nesting depth.We also took
the chance to refactor the code in question and improve the rust docs.

The bug was caused by not updating `GasMeter::executor_consumed`
(previously `engine_consumed`) when leaving the host function. This lead
to the value being stale (too low) when entering another host function.

---------

Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Co-authored-by: PG Herveou <pgherveou@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T7-smart_contracts This PR/Issue is related to smart contracts.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants