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: Don't fail fast if the Weight limit of a cross contract call is too big #3243

Merged
merged 4 commits into from
Feb 8, 2024

Conversation

athei
Copy link
Member

@athei athei commented Feb 7, 2024

When doing a cross contract call you can supply an optional Weight limit for that call. If one doesn't specify the limit (setting it to 0) the sub call will have all the remaining gas available. If one does specify the limit we subtract that amount eagerly from the Weight meter and fail fast if not enough Weight is available.

This is quite annoying because setting a fixed limit will set the gas_required in the gas estimation according to the specified limit. Even if in that dry-run the actual call didn't consume that whole amount. It effectively discards the more precise measurement it should have from the dry-run.

This PR changes the behaviour so that the supplied limit is an actual limit: We do the cross contract call even if the limit is higher than the remaining Weight. We then fail and roll back in the cub call in case there is not enough weight.

This makes the weight estimation in the dry-run no longer dependent on the weight limit supplied when doing a cross contract call.

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5141121

substrate/frame/contracts/src/tests.rs Outdated Show resolved Hide resolved
substrate/frame/contracts/src/tests.rs Outdated Show resolved Hide resolved
athei and others added 3 commits February 8, 2024 13:56
Co-authored-by: PG Herveou <pgherveou@gmail.com>
Co-authored-by: PG Herveou <pgherveou@gmail.com>
@athei athei added the T7-smart_contracts This PR/Issue is related to smart contracts. label Feb 8, 2024
@athei athei enabled auto-merge February 8, 2024 13:33
@athei athei added this pull request to the merge queue Feb 8, 2024
Merged via the queue into master with commit 28463a1 Feb 8, 2024
124 of 128 checks passed
@athei athei deleted the at/subcall-no-precharge branch February 8, 2024 15:37
bkchr pushed a commit that referenced this pull request Feb 23, 2024
By passing `RUST_LOG=info` to the check command, we will be able to see
the exact problem with a given prdoc.

Before:
```
PR #3243 -> ERR
```

After:
```
[2024-02-23T12:53:55Z INFO  prdoclib::commands::check] Checking directory prdoc
[2024-02-23T12:53:55Z INFO  prdoclib::commands::check] Using schema: /Users/sebastian/work/repos/polkadot-sdk/prdoc/schema_user.json
[2024-02-23T12:53:55Z WARN  prdoclib::schema] validation_result: false
[2024-02-23T12:53:55Z WARN  prdoclib::schema] validation_result_strict: false
[2024-02-23T12:53:55Z WARN  prdoclib::schema] errors: [
        Required {
            path: "/title",
        },
    ]
[2024-02-23T12:53:55Z WARN  prdoclib::schema] missing: []
[2024-02-23T12:53:55Z ERROR prdoclib::commands::check] Loading the schema failed:
[2024-02-23T12:53:55Z ERROR prdoclib::commands::check] ValidationErrors(ValidationState { errors: [Required { path: "/title" }], missing: [], replacement: None, evaluated: {"/doc/0/description", "/crates/0/name", "/doc/0", "/crates", "/crates/0", "", "/doc", "/doc/0/audience"} })
PR #3243 -> ERR
```
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
…call is too big (paritytech#3243)

When doing a cross contract call you can supply an optional Weight limit
for that call. If one doesn't specify the limit (setting it to 0) the
sub call will have all the remaining gas available. If one does specify
the limit we subtract that amount eagerly from the Weight meter and fail
fast if not enough `Weight` is available.

This is quite annoying because setting a fixed limit will set the
`gas_required` in the gas estimation according to the specified limit.
Even if in that dry-run the actual call didn't consume that whole
amount. It effectively discards the more precise measurement it should
have from the dry-run.

This PR changes the behaviour so that the supplied limit is an actual
limit: We do the cross contract call even if the limit is higher than
the remaining `Weight`. We then fail and roll back in the cub call in
case there is not enough weight.

This makes the weight estimation in the dry-run no longer dependent on
the weight limit supplied when doing a cross contract call.

---------

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