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>
## Xcm changes:
- Fix `pallet_xcm::execute`, move the logic into The `ExecuteController`
so it can be shared with anything that implement that trait.
- Make `ExecuteController::execute` retursn `DispatchErrorWithPostInfo`
instead of `DispatchError`, so that we don't charge the full
`max_weight` provided if the execution is incomplete (useful for
force_batch or contracts calls)
- Fix docstring for `pallet_xcm::execute`, to reflect the changes from
#2405
- Update the signature for `ExecuteController::execute`, we don't need
to return the `Outcome` anymore since we only care about
`Outcome::Complete`
## Contracts changes:
- Update host fn `xcm_exexute`, we don't need to write the `Outcome` to
the sandbox memory anymore. This was also not charged as well before so
it if fixes this too.
- One of the issue was that the dry_run of a contract that call
`xcm_execute` would exhaust the `gas_limit`.
This is because `XcmExecuteController::execute` takes a `max_weight`
argument, and since we don't want the user to specify it manually we
were passing everything left by pre-charghing
`ctx.ext.gas_meter().gas_left()`
- To fix it I added a `fn influence_lowest_limit` on the `Token` trait
and make it return false for `RuntimeCost::XcmExecute`.
- Got rid of the `RuntimeToken` indirection, we can just use
`RuntimeCost` directly.
---------
Co-authored-by: command-bot <>
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>
First in a series of PRs that reduces our use of sp-std with a view to
deprecating it.
This is just looking at /substrate and moving some of the references
from `sp-std` to `core`.
These particular changes should be uncontroversial.
Where macros are used `::core` should be used to remove any ambiguity.
part of https://github.com/paritytech/polkadot-sdk/issues/2101
* Change copyright year to 2023 from 2022
* Fix incorrect update of copyright year
* Remove years from copy right header
* Fix remaining files
* Fix typo in a header and remove update-copyright.sh
* Run cargo fmt on the whole code base
* Second run
* Add CI check
* Fix compilation
* More unnecessary braces
* Handle weights
* Use --all
* Use correct attributes...
* Fix UI tests
* AHHHHHHHHH
* 🤦
* Docs
* Fix compilation
* 🤷
* Please stop
* 🤦 x 2
* More
* make rustfmt.toml consistent with polkadot
Co-authored-by: André Silva <andrerfosilva@gmail.com>
* Remove pre-charging for code size
* Remove pre charging when reading values of fixed size
* Add new versions of API functions that leave out parameters
* Update CHANGELOG.md
* Apply suggestions from code review
Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
* Add v1 for seal_set_rent_allowance
* Remove unneeded trait bound
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
* Move public functions up in rent.rs
* Added RentStatus
* Fix test name for consistency
Co-authored-by: Michael Müller <michi@parity.io>
* Mark rent functions as unstable
* Add unstable interfaces to README
* Fix doc typos
Co-authored-by: Andrew Jones <ascjones@gmail.com>
* Use DefaultNoBound
* Simplify calc_share(1)
* Don't output empty debug messages
* Make `seal_debug_message` unstable
Co-authored-by: Michael Müller <michi@parity.io>
Co-authored-by: Andrew Jones <ascjones@gmail.com>
* contracts: Add default implementation for Executable::occupied_storage()
* contracts: Refactor the exec module
* Let runtime specify the backing type of the call stack
This removes the need for a runtime check of the specified
`MaxDepth`. We can now garantuee that we don't need to
allocate when a new call frame is pushed.
* Fix doc typo
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
* cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_contracts --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/contracts/src/weights.rs --template=./.maintain/frame-weight-template.hbs
* Review nits
* Fix defect in contract info caching behaviour
* Add more docs
* Fix wording and typos
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Parity Benchmarking Bot <admin@parity.io>
* Simplify some Option / Result / ? operator patterns
When those match a combinator exactly.
Tool-aided by [comby-rust](https://github.com/huitseeker/comby-rust).
* adjust after review
* adjust post-review
* contracts: Add RPC that allows instantiating of a contract
* Encode `debug_message` as bytes because usage of `String` is forbidden
* Remove erroneous derive attribute
* Fix rpc tests for new `debug_message` encoding
* Fix typo
Co-authored-by: Andrew Jones <ascjones@gmail.com>
Co-authored-by: Andrew Jones <ascjones@gmail.com>
* contracts: Convert to framev2
* Reduce the API surface of the crate
* Remove unused import
* Merge import block
* Use pallet::metadata to reduce metadata diff
* Remove the explicit "Null" from AccountCounter
* contracts: Consider contract size in weights
* Bump spec version
* Whitespace fix
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
* Correct pre-charged code weight even in the error case
* Use the instrumented code size in weight calculation
* Charge the cost of re-instrumentation from the gas meter
* Fix benchmark
* cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_contracts --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/contracts/src/weights.rs --template=./.maintain/frame-weight-template.hbs
* Better documentation of return types
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Parity Benchmarking Bot <admin@parity.io>
* contracts: Implement refcounting for wasm code
* contracts: Charge rent for code storage
* contracts: Fix dispatchables erroneously refunding base costs
* Fixed typos in comments.
Co-authored-by: Andrew Jones <ascjones@gmail.com>
* Remove awkward empty line
* Fix more typos in docs
* Fix typos in docs
Co-authored-by: Andrew Jones <ascjones@gmail.com>
* Split up complicated expression
Co-authored-by: Andrew Jones <ascjones@gmail.com>
* review: Remove unused return value
* Fix typos
Co-authored-by: Andrew Jones <ascjones@gmail.com>
* review: Fix refcount being reset to one on re-instrumentation
* Document evictable_code parameter
* Make Executable::execute consume and store itself
* Added comments about stale values
* Disregard struct size in occupied_storage()
Co-authored-by: Andrew Jones <ascjones@gmail.com>
* Make host functions return TrapReason
This avoids the need to manually store any trap reasons
to the `Runtime` from the host function. This adds the following
benefits:
* It properly composes with the upcoming chain extensions
* Missing to set a trap value is now a compile error
* Add chain extension
The chain extension is a way for the contract author to add new
host functions for contracts to call.
* Add tests for chain extensions
* Fix regression in set_rent.wat fixture
Not all offsets where properly updated when changing the fixtures
for the new salt on instantiate.
* Pre-charge a weight amount based off the specified length
* Improve fn write docs
* Renamed state to phantom
* Fix typo
* contracts: Make use of existing type aliases for runtime API types
* contracts: Refactor the contracts call runtime API
* review: Fix comment typo
Co-authored-by: Andrew Jones <ascjones@gmail.com>
* Update frame/contracts/common/src/lib.rs
Co-authored-by: Nikolay Volf <nikvolf@gmail.com>
* Update frame/contracts/common/src/lib.rs
Co-authored-by: Nikolay Volf <nikvolf@gmail.com>
* Update frame/contracts/common/src/lib.rs
Co-authored-by: Nikolay Volf <nikvolf@gmail.com>
* Update frame/contracts/common/src/lib.rs
Co-authored-by: Nikolay Volf <nikvolf@gmail.com>
* Update frame/contracts/common/src/lib.rs
Co-authored-by: Nikolay Volf <nikvolf@gmail.com>
* Update lib.rs
* review: Group crate imports
Co-authored-by: Andrew Jones <ascjones@gmail.com>
Co-authored-by: Addie Wagenknecht <addie@nortd.com>
Co-authored-by: Nikolay Volf <nikvolf@gmail.com>
* seal: Rework ext_transfer, ext_instantiate, ext_call error handling
* Deny calling plain accounts (must use transfer now)
* Return proper module error rather than ad-hoc strings
* Return the correct error codes from call,instantiate (documentation was wrong)
* Make ext_transfer fallible again to make it consistent with ext_call
* seal: Improve error messages on memory access failures
* seal: Convert contract trapped to module error
* seal: Add additional tests for transfer, call, instantiate
These tests verify that those functions return the error types
which are declared in its docs.
* Make it more pronounced that to_execution_result handles trap_reason
* Improve ReturnCode docs
* Fix whitespace issues in wat files
* Improve ReturnCode doc
* Improve ErrorOrigin doc and variant naming
* Improve docs on ExecResult and ExecError
* Encode u32 sentinel value as hex
* with_nested_context no longer accepts an Option for trie
* Fix successful typo
* Rename InvalidContractCalled to NotCallable
* Transition getter functions to not use scratch buffer
* Remove scratch buffer from ext_get_storage
* Remove scratch buffer from ext_call
* Remove scratch buffer from ext_instantiate
* Add ext_input and remove scratch buffer
* Rework error handling (changes RPC exposed data)
* ext_return passes a flags field instead of a return code
* Flags is only for seal and not for the caller
* flags: u32 replaced status_code: u8 in RPC exposed type
* API functions use a unified error type (ReturnCode)
* ext_transfer now traps on error to be consistent with call and instantiate
* Remove the no longer used `Dispatched` event
* Updated inline documentation
* Prevent skipping of copying the output for getter API
* Return gas_consumed from the RPC contracts call interface
* Updated COMPLEXTITY.md
* Rename ext_gas_price to ext_weight_to_fee
* Align comments with spaces
* Removed no longer used `ExecError`
* Remove possible panic in `from_typed_value`
* Use a struct as associated data for SpecialTrap::Return
* Fix nits in COMPLEXITY.md
* Renamed SpecialTrap to TrapReason
* Fix test
* Finish renaming special_trap -> trap_reason
* Remove no longer used get_runtime_storage
* fixup! Remove no longer used get_runtime_storage
* Removed tabs for comment aligment
* Give perthigns the trait it always deserved.
* Make staking and phragmen work with the new generic per_thing
* Make everything work together 🔨
* a bit of cleanup
* Clean usage
* Bump.
* Fix name
* fix grumbles
* hopefully fix the ui test
* Some grumbles
* revamp traits again
* Better naming again.
* Make `decl_error!` errors usable
This pr implements support for returning errors of different pallets in
a pallet. These errors need to be declared with `decl_error!`.
The pr changes the following:
- Each dispatchable function now returns a `DispatchResult` which is an
alias for `Result<(), DispatchError>`.
- `DispatchError` is an enum that has 4 variants:
- `Other`: For storing string error messages
- `CannotLookup`: Variant that is returned when something returns a
`sp_runtime::LookupError`
- `BadOrigin`: Variant that is returned for any kind of bad origin
- `Module`: The error of a specific module. Contains the `index`,
`error` and the `message`. The index is the index of the module in
`construct_runtime!`. `error` is the index of the error in the error
enum declared by `decl_error!`. `message` is the message to the error
variant (this will not be encoded).
- `construct_runtime!` now creates a new struct `ModuleToIndex`. This
struct implements the trait `ModuleToIndex`.
- `frame_system::Trait` has a new associated type: `ModuleToIndex` that
expects the `ModuleToIndex` generated by `construct_runtime!`.
- All error strings returned in any module are being converted now to `DispatchError`.
- `BadOrigin` is the default error returned by any type that implements `EnsureOrigin`.
* Fix frame system benchmarks
* Adding script for rename, could be applicable for nodes on top of it, too
* add stderr and gitlab ci features
* apply script
* fix now minor details in expected stderr
* Update the Cargo.lock
* fix name: sc-transaction -> sc-tracing
* fix rename in script, too
Alexander Theißen