diff --git a/Cargo.lock b/Cargo.lock index 50e42667b942..dd75a9ec43fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12322,7 +12322,7 @@ dependencies = [ "pallet-utility", "parity-scale-codec", "paste", - "polkavm 0.10.0", + "polkavm 0.11.0", "pretty_assertions", "rlp", "scale-info", @@ -12347,7 +12347,7 @@ dependencies = [ "frame-system", "log", "parity-wasm", - "polkavm-linker 0.10.0", + "polkavm-linker 0.11.0", "sp-core 28.0.0", "sp-io 30.0.0", "sp-runtime 31.0.1", @@ -12407,7 +12407,7 @@ dependencies = [ "bitflags 1.3.2", "parity-scale-codec", "paste", - "polkavm-derive 0.10.0", + "polkavm-derive 0.11.0", "scale-info", ] @@ -16149,15 +16149,15 @@ dependencies = [ [[package]] name = "polkavm" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7ec0c5935f2eff23cfc4653002f4f8d12b37f87a720e0631282d188c32089d6" +checksum = "1195fbc12f11645143a4f3974bf909d25c7f7efddcc6f4e57688d9a518c90bae" dependencies = [ "libc", "log", - "polkavm-assembler 0.10.0", - "polkavm-common 0.10.0", - "polkavm-linux-raw 0.10.0", + "polkavm-assembler 0.11.0", + "polkavm-common 0.11.0", + "polkavm-linux-raw 0.11.0", ] [[package]] @@ -16171,9 +16171,9 @@ dependencies = [ [[package]] name = "polkavm-assembler" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d8e4fd5a43100bf1afe9727b8130d01f966f5cfc9144d5604b21e795c2bcd80e" +checksum = "f0b0399659fe7a5370c3e3464188888d29069cfa46d99631d19834a379c15826" dependencies = [ "log", ] @@ -16195,12 +16195,12 @@ dependencies = [ [[package]] name = "polkavm-common" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0097b48bc0bedf9f3f537ce8f37e8f1202d8d83f9b621bdb21ff2c59b9097c50" +checksum = "254b19b64ff9b57c06b32c0affed961cb9a32429b8d3e5cf2633cad7fbb3e270" dependencies = [ "log", - "polkavm-assembler 0.10.0", + "polkavm-assembler 0.11.0", ] [[package]] @@ -16223,11 +16223,11 @@ dependencies = [ [[package]] name = "polkavm-derive" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0dcc701385c08c31bdb0569f0c51a290c580d892fa77f1dd88a7352a62679ecf" +checksum = "f565f0106fbb3598d64b8528d5cb966b6a87a8dd93fbcfe09fb6388ff2865711" dependencies = [ - "polkavm-derive-impl-macro 0.10.0", + "polkavm-derive-impl-macro 0.11.0", ] [[package]] @@ -16256,11 +16256,11 @@ dependencies = [ [[package]] name = "polkavm-derive-impl" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7855353a5a783dd5d09e3b915474bddf66575f5a3cf45dec8d1c5e051ba320dc" +checksum = "314445fb5688b4769354087d92be2ac94c487e63ffe74a6fb7bb312e57f20827" dependencies = [ - "polkavm-common 0.10.0", + "polkavm-common 0.11.0", "proc-macro2 1.0.86", "quote 1.0.37", "syn 2.0.65", @@ -16288,11 +16288,11 @@ dependencies = [ [[package]] name = "polkavm-derive-impl-macro" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9324fe036de37c17829af233b46ef6b5562d4a0c09bb7fdb9f8378856dee30cf" +checksum = "1bf952e05bc5ce7d81293bae18cb44c271c78615b201d75e983cdcc40d5c6ef1" dependencies = [ - "polkavm-derive-impl 0.10.0", + "polkavm-derive-impl 0.11.0", "syn 2.0.65", ] @@ -16313,15 +16313,15 @@ dependencies = [ [[package]] name = "polkavm-linker" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d704edfe7bdcc876784f19436d53d515b65eb07bc9a0fae77085d552c2dbbb5" +checksum = "535a2095a186ccde2cd2fa721d6370c495586d47714817565b2f6621d31164b3" dependencies = [ "gimli 0.28.0", "hashbrown 0.14.5", "log", "object 0.36.1", - "polkavm-common 0.10.0", + "polkavm-common 0.11.0", "regalloc2 0.9.3", "rustc-demangle", ] @@ -16334,9 +16334,9 @@ checksum = "26e85d3456948e650dff0cfc85603915847faf893ed1e66b020bb82ef4557120" [[package]] name = "polkavm-linux-raw" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "26e45fa59c7e1bb12ef5289080601e9ec9b31435f6e32800a5c90c132453d126" +checksum = "3011697430dfcfe800d1d7c540ef69e3bdd66e9037cc38f01fee1c2e0908011e" [[package]] name = "polling" @@ -16766,7 +16766,7 @@ checksum = "80b776a1b2dc779f5ee0641f8ade0125bc1298dd41a9a0c16d8bd57b42d222b1" dependencies = [ "bytes", "heck 0.5.0", - "itertools 0.11.0", + "itertools 0.12.1", "log", "multimap", "once_cell", @@ -16787,7 +16787,7 @@ checksum = "f8650aabb6c35b860610e9cff5dc1af886c9e25073b7b1712a68972af4281302" dependencies = [ "bytes", "heck 0.5.0", - "itertools 0.11.0", + "itertools 0.12.1", "log", "multimap", "once_cell", @@ -16820,7 +16820,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "81bddcdb20abf9501610992b6759a4c888aef7d1a7247ef75e2404275ac24af1" dependencies = [ "anyhow", - "itertools 0.11.0", + "itertools 0.12.1", "proc-macro2 1.0.86", "quote 1.0.37", "syn 2.0.65", @@ -16833,7 +16833,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "acf0c195eebb4af52c752bec4f52f645da98b6e92077a04110c7f349477ae5ac" dependencies = [ "anyhow", - "itertools 0.11.0", + "itertools 0.12.1", "proc-macro2 1.0.86", "quote 1.0.37", "syn 2.0.65", diff --git a/prdoc/pr_5726.prdoc b/prdoc/pr_5726.prdoc new file mode 100644 index 000000000000..ce666647bad3 --- /dev/null +++ b/prdoc/pr_5726.prdoc @@ -0,0 +1,14 @@ +title: "revive: Limit the amount of static memory" + +doc: + - audience: Runtime Dev + description: | + Limit the amount of static memory a contract can declare. + +crates: + - name: pallet-revive + bump: major + - name: pallet-revive-fixtures + bump: minor + - name: pallet-revive-uapi + bump: patch diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index c8409078af57..bfcaf5e9af8f 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1410,7 +1410,6 @@ impl pallet_revive::Config for Runtime { type WeightInfo = pallet_revive::weights::SubstrateWeight; type ChainExtension = (); type AddressMapper = pallet_revive::DefaultAddressMapper; - type MaxCodeLen = ConstU32<{ 123 * 1024 }>; type RuntimeMemory = ConstU32<{ 128 * 1024 * 1024 }>; type PVFMemory = ConstU32<{ 512 * 1024 * 1024 }>; type UnsafeUnstableInterface = ConstBool; diff --git a/substrate/frame/revive/Cargo.toml b/substrate/frame/revive/Cargo.toml index 667328ac2d0d..177b3c3e9eec 100644 --- a/substrate/frame/revive/Cargo.toml +++ b/substrate/frame/revive/Cargo.toml @@ -19,7 +19,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] environmental = { workspace = true } paste = { workspace = true } -polkavm = { version = "0.10.0", default-features = false } +polkavm = { version = "0.11.0", default-features = false } bitflags = { workspace = true } codec = { features = [ "derive", diff --git a/substrate/frame/revive/fixtures/Cargo.toml b/substrate/frame/revive/fixtures/Cargo.toml index 903298d2df21..53db08a3911a 100644 --- a/substrate/frame/revive/fixtures/Cargo.toml +++ b/substrate/frame/revive/fixtures/Cargo.toml @@ -22,7 +22,7 @@ log = { workspace = true } parity-wasm = { workspace = true } tempfile = { workspace = true } toml = { workspace = true } -polkavm-linker = { version = "0.10.0" } +polkavm-linker = { version = "0.11.0" } anyhow = { workspace = true, default-features = true } [features] diff --git a/substrate/frame/revive/fixtures/build/Cargo.toml b/substrate/frame/revive/fixtures/build/Cargo.toml index 7dead51b2306..5b3a21b5122e 100644 --- a/substrate/frame/revive/fixtures/build/Cargo.toml +++ b/substrate/frame/revive/fixtures/build/Cargo.toml @@ -11,7 +11,7 @@ edition = "2021" [dependencies] uapi = { package = 'pallet-revive-uapi', path = "", default-features = false } common = { package = 'pallet-revive-fixtures-common', path = "" } -polkavm-derive = { version = "0.10.0" } +polkavm-derive = { version = "0.11.0" } [profile.release] opt-level = 3 diff --git a/substrate/frame/revive/fixtures/contracts/oom_ro.rs b/substrate/frame/revive/fixtures/contracts/oom_ro.rs new file mode 100644 index 000000000000..41c080d5847e --- /dev/null +++ b/substrate/frame/revive/fixtures/contracts/oom_ro.rs @@ -0,0 +1,44 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! This creates a large ro section. Even though it is zero +//! initialized we expect them to be included into the blob. +//! This means it will fail at the blob size check. + +#![no_std] +#![no_main] + +extern crate common; + +use uapi::{HostFn, HostFnImpl as api, ReturnFlags}; + +static BUFFER: [u8; 1025 * 1024] = [0; 1025 * 1024]; + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub extern "C" fn call_never() { + // make sure the buffer is not optimized away + api::return_value(ReturnFlags::empty(), &BUFFER); +} + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub extern "C" fn deploy() {} + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub extern "C" fn call() {} diff --git a/substrate/frame/revive/fixtures/contracts/oom_rw_included.rs b/substrate/frame/revive/fixtures/contracts/oom_rw_included.rs new file mode 100644 index 000000000000..2cdcf7bafed1 --- /dev/null +++ b/substrate/frame/revive/fixtures/contracts/oom_rw_included.rs @@ -0,0 +1,44 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! This creates a large rw section but with its contents +//! included into the blob. It should be rejected for its +//! blob size. + +#![no_std] +#![no_main] + +extern crate common; + +use uapi::{HostFn, HostFnImpl as api, ReturnFlags}; + +static mut BUFFER: [u8; 513 * 1024] = [42; 513 * 1024]; + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub unsafe extern "C" fn call_never() { + // make sure the buffer is not optimized away + api::return_value(ReturnFlags::empty(), &BUFFER); +} + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub extern "C" fn deploy() {} + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub extern "C" fn call() {} diff --git a/substrate/frame/revive/fixtures/contracts/oom_rw_trailing.rs b/substrate/frame/revive/fixtures/contracts/oom_rw_trailing.rs new file mode 100644 index 000000000000..993be8e9cda4 --- /dev/null +++ b/substrate/frame/revive/fixtures/contracts/oom_rw_trailing.rs @@ -0,0 +1,44 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! This creates a large rw section but the trailing zeroes +//! are removed by the linker. It should be rejected even +//! though the blob is small enough. + +#![no_std] +#![no_main] + +extern crate common; + +use uapi::{HostFn, HostFnImpl as api, ReturnFlags}; + +static mut BUFFER: [u8; 1025 * 1024] = [0; 1025 * 1024]; + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub unsafe extern "C" fn call_never() { + // make sure the buffer is not optimized away + api::return_value(ReturnFlags::empty(), &BUFFER); +} + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub extern "C" fn deploy() {} + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub extern "C" fn call() {} diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index 332c425d714e..cbc4cc62d481 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -258,7 +258,7 @@ mod benchmarks { // call_with_code_per_byte(0)`. #[benchmark(pov_mode = Measured)] fn call_with_code_per_byte( - c: Linear<0, { T::MaxCodeLen::get() }>, + c: Linear<0, { limits::code::BLOB_BYTES }>, ) -> Result<(), BenchmarkError> { let instance = Contract::::with_caller(whitelisted_caller(), WasmModule::sized(c), vec![])?; @@ -283,8 +283,8 @@ mod benchmarks { // `i`: Size of the input in bytes. #[benchmark(pov_mode = Measured)] fn instantiate_with_code( - c: Linear<0, { T::MaxCodeLen::get() }>, - i: Linear<0, { limits::MEMORY_BYTES }>, + c: Linear<0, { limits::code::BLOB_BYTES }>, + i: Linear<0, { limits::code::BLOB_BYTES }>, ) { let input = vec![42u8; i as usize]; let salt = [42u8; 32]; @@ -316,7 +316,7 @@ mod benchmarks { // `i`: Size of the input in bytes. // `s`: Size of e salt in bytes. #[benchmark(pov_mode = Measured)] - fn instantiate(i: Linear<0, { limits::MEMORY_BYTES }>) -> Result<(), BenchmarkError> { + fn instantiate(i: Linear<0, { limits::code::BLOB_BYTES }>) -> Result<(), BenchmarkError> { let input = vec![42u8; i as usize]; let salt = [42u8; 32]; let value = Pallet::::min_balance(); @@ -401,7 +401,7 @@ mod benchmarks { // It creates a maximum number of metering blocks per byte. // `c`: Size of the code in bytes. #[benchmark(pov_mode = Measured)] - fn upload_code(c: Linear<0, { T::MaxCodeLen::get() }>) { + fn upload_code(c: Linear<0, { limits::code::BLOB_BYTES }>) { let caller = whitelisted_caller(); T::Currency::set_balance(&caller, caller_funding::()); let WasmModule { code, hash, .. } = WasmModule::sized(c); @@ -695,7 +695,7 @@ mod benchmarks { } #[benchmark(pov_mode = Measured)] - fn seal_input(n: Linear<0, { limits::MEMORY_BYTES - 4 }>) { + fn seal_input(n: Linear<0, { limits::code::BLOB_BYTES - 4 }>) { let mut setup = CallSetup::::default(); let (mut ext, _) = setup.ext(); let mut runtime = crate::wasm::Runtime::new(&mut ext, vec![42u8; n as usize]); @@ -710,7 +710,7 @@ mod benchmarks { } #[benchmark(pov_mode = Measured)] - fn seal_return(n: Linear<0, { limits::MEMORY_BYTES - 4 }>) { + fn seal_return(n: Linear<0, { limits::code::BLOB_BYTES - 4 }>) { build_runtime!(runtime, memory: [n.to_le_bytes(), vec![42u8; n as usize], ]); let result; @@ -800,7 +800,7 @@ mod benchmarks { // buffer size, whichever is less. #[benchmark] fn seal_debug_message( - i: Linear<0, { (limits::MEMORY_BYTES).min(limits::DEBUG_BUFFER_BYTES) }>, + i: Linear<0, { (limits::code::BLOB_BYTES).min(limits::DEBUG_BUFFER_BYTES) }>, ) { let mut setup = CallSetup::::default(); setup.enable_debug_message(); @@ -1394,7 +1394,7 @@ mod benchmarks { // t: with or without some value to transfer // i: size of the input data #[benchmark(pov_mode = Measured)] - fn seal_call(t: Linear<0, 1>, i: Linear<0, { limits::MEMORY_BYTES }>) { + fn seal_call(t: Linear<0, 1>, i: Linear<0, { limits::code::BLOB_BYTES }>) { let Contract { account_id: callee, .. } = Contract::::with_index(1, WasmModule::dummy(), vec![]).unwrap(); let callee_bytes = callee.encode(); @@ -1469,7 +1469,7 @@ mod benchmarks { // t: value to transfer // i: size of input in bytes #[benchmark(pov_mode = Measured)] - fn seal_instantiate(i: Linear<0, { limits::MEMORY_BYTES }>) -> Result<(), BenchmarkError> { + fn seal_instantiate(i: Linear<0, { limits::code::BLOB_BYTES }>) -> Result<(), BenchmarkError> { let code = WasmModule::dummy(); let hash = Contract::::with_index(1, WasmModule::dummy(), vec![])?.info()?.code_hash; let hash_bytes = hash.encode(); @@ -1535,7 +1535,7 @@ mod benchmarks { // `n`: Input to hash in bytes #[benchmark(pov_mode = Measured)] - fn seal_hash_sha2_256(n: Linear<0, { limits::MEMORY_BYTES }>) { + fn seal_hash_sha2_256(n: Linear<0, { limits::code::BLOB_BYTES }>) { build_runtime!(runtime, memory: [[0u8; 32], vec![0u8; n as usize], ]); let result; @@ -1549,7 +1549,7 @@ mod benchmarks { // `n`: Input to hash in bytes #[benchmark(pov_mode = Measured)] - fn seal_hash_keccak_256(n: Linear<0, { limits::MEMORY_BYTES }>) { + fn seal_hash_keccak_256(n: Linear<0, { limits::code::BLOB_BYTES }>) { build_runtime!(runtime, memory: [[0u8; 32], vec![0u8; n as usize], ]); let result; @@ -1563,7 +1563,7 @@ mod benchmarks { // `n`: Input to hash in bytes #[benchmark(pov_mode = Measured)] - fn seal_hash_blake2_256(n: Linear<0, { limits::MEMORY_BYTES }>) { + fn seal_hash_blake2_256(n: Linear<0, { limits::code::BLOB_BYTES }>) { build_runtime!(runtime, memory: [[0u8; 32], vec![0u8; n as usize], ]); let result; @@ -1577,7 +1577,7 @@ mod benchmarks { // `n`: Input to hash in bytes #[benchmark(pov_mode = Measured)] - fn seal_hash_blake2_128(n: Linear<0, { limits::MEMORY_BYTES }>) { + fn seal_hash_blake2_128(n: Linear<0, { limits::code::BLOB_BYTES }>) { build_runtime!(runtime, memory: [[0u8; 16], vec![0u8; n as usize], ]); let result; @@ -1592,7 +1592,7 @@ mod benchmarks { // `n`: Message input length to verify in bytes. // need some buffer so the code size does not exceed the max code size. #[benchmark(pov_mode = Measured)] - fn seal_sr25519_verify(n: Linear<0, { T::MaxCodeLen::get() - 255 }>) { + fn seal_sr25519_verify(n: Linear<0, { limits::code::BLOB_BYTES - 255 }>) { let message = (0..n).zip((32u8..127u8).cycle()).map(|(_, c)| c).collect::>(); let message_len = message.len() as u32; diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs index ccbc65873d02..bf9803690560 100644 --- a/substrate/frame/revive/src/lib.rs +++ b/substrate/frame/revive/src/lib.rs @@ -90,7 +90,7 @@ pub use crate::wasm::SyscallDoc; type TrieId = BoundedVec>; type BalanceOf = <::Currency as Inspect<::AccountId>>::Balance; -type CodeVec = BoundedVec::MaxCodeLen>; +type CodeVec = BoundedVec>; type EventRecordOf = EventRecord<::RuntimeEvent, ::Hash>; type DebugBuffer = BoundedVec>; @@ -232,14 +232,6 @@ pub mod pallet { #[pallet::no_default_bounds] type AddressMapper: AddressMapper>; - /// The maximum length of a contract code in bytes. - /// - /// This value hugely affects the memory requirements of this pallet since all the code of - /// all contracts on the call stack will need to be held in memory. Setting of a correct - /// value will be enforced in [`Pallet::integrity_test`]. - #[pallet::constant] - type MaxCodeLen: Get; - /// Make contract callable functions marked as `#[unstable]` available. /// /// Contracts that use `#[unstable]` functions won't be able to be uploaded unless @@ -363,7 +355,6 @@ pub mod pallet { type CodeHashLockupDepositPercent = CodeHashLockupDepositPercent; type DepositPerByte = DepositPerByte; type DepositPerItem = DepositPerItem; - type MaxCodeLen = ConstU32<{ 123 * 1024 }>; type Time = Self; type UnsafeUnstableInterface = ConstBool; type UploadOrigin = EnsureSigned; @@ -475,9 +466,6 @@ pub mod pallet { MaxCallDepthReached, /// No contract was found at the specified address. ContractNotFound, - /// The code supplied to `instantiate_with_code` exceeds the limit specified in the - /// current schedule. - CodeTooLarge, /// No code could be found at the supplied code hash. CodeNotFound, /// No code info could be found at the supplied code hash. @@ -531,6 +519,11 @@ pub mod pallet { /// A more detailed error can be found on the node console if debug messages are enabled /// by supplying `-lruntime::revive=debug`. CodeRejected, + /// The code blob supplied is larger than [`limits::code::BLOB_BYTES`]. + BlobTooLarge, + /// The static memory consumption of the blob will be larger than + /// [`limits::code::STATIC_MEMORY_BYTES`]. + StaticMemoryTooLarge, /// The contract has reached its maximum number of delegate dependencies. MaxDelegateDependenciesReached, /// The dependency was not found in the contract's delegate dependencies. @@ -562,7 +555,7 @@ pub mod pallet { /// A mapping from a contract's code hash to its code. #[pallet::storage] - pub(crate) type PristineCode = StorageMap<_, Identity, H256, CodeVec>; + pub(crate) type PristineCode = StorageMap<_, Identity, H256, CodeVec>; /// A mapping from a contract's code hash to its code info. #[pallet::storage] @@ -602,13 +595,10 @@ pub mod pallet { } fn integrity_test() { - // Total runtime memory limit + use limits::code::STATIC_MEMORY_BYTES; + + // The memory available in the block building runtime let max_runtime_mem: u32 = T::RuntimeMemory::get(); - // Memory limits for a single contract: - // Value stack size: 1Mb per contract, default defined in wasmi - const MAX_STACK_SIZE: u32 = 1024 * 1024; - // Heap limit is normally 16 mempages of 64kb each = 1Mb per contract - let max_heap_size = limits::MEMORY_BYTES; // The root frame is not accounted in CALL_STACK_DEPTH let max_call_depth = limits::CALL_STACK_DEPTH.checked_add(1).expect("CallStack size is too big"); @@ -618,50 +608,36 @@ pub mod pallet { .checked_mul(2) .expect("MaxTransientStorageSize is too large"); - // Check that given configured `MaxCodeLen`, runtime heap memory limit can't be broken. - // - // In worst case, the decoded Wasm contract code would be `x16` times larger than the - // encoded one. This is because even a single-byte wasm instruction has 16-byte size in - // wasmi. This gives us `MaxCodeLen*16` safety margin. - // - // Next, the pallet keeps the Wasm blob for each - // contract, hence we add up `MaxCodeLen` to the safety margin. - // + // We only allow 50% of the runtime memory to be utilized by the contracts call + // stack, keeping the rest for other facilities, such as PoV, etc. + const TOTAL_MEMORY_DEVIDER: u32 = 2; + // The inefficiencies of the freeing-bump allocator // being used in the client for the runtime memory allocations, could lead to possible - // memory allocations for contract code grow up to `x4` times in some extreme cases, - // which gives us total multiplier of `17*4` for `MaxCodeLen`. - // - // That being said, for every contract executed in runtime, at least `MaxCodeLen*17*4` - // memory should be available. Note that maximum allowed heap memory and stack size per - // each contract (stack frame) should also be counted. - // - // The pallet holds transient storage with a size up to `max_transient_storage_size`. - // - // Finally, we allow 50% of the runtime memory to be utilized by the contracts call - // stack, keeping the rest for other facilities, such as PoV, etc. - // - // This gives us the following formula: + // memory allocations grow up to `x4` times in some extreme cases. + const MEMORY_ALLOCATOR_INEFFICENCY_DEVIDER: u32 = 4; + + // Check that the configured `STATIC_MEMORY_BYTES` fits into runtime memory. // - // `(MaxCodeLen * 17 * 4 + MAX_STACK_SIZE + max_heap_size) * max_call_depth + - // max_transient_storage_size < max_runtime_mem/2` + // `STATIC_MEMORY_BYTES` is the amount of memory that a contract can consume + // in memory and is enforced at upload time. // - // Hence the upper limit for the `MaxCodeLen` can be defined as follows: - let code_len_limit = max_runtime_mem - .saturating_div(2) + // Dynamic allocations are not available, yet. Hence are not taken into consideration + // here. + let static_memory_limit = max_runtime_mem + .saturating_div(TOTAL_MEMORY_DEVIDER) .saturating_sub(max_transient_storage_size) .saturating_div(max_call_depth) - .saturating_sub(max_heap_size) - .saturating_sub(MAX_STACK_SIZE) - .saturating_div(17 * 4); + .saturating_sub(STATIC_MEMORY_BYTES) + .saturating_div(MEMORY_ALLOCATOR_INEFFICENCY_DEVIDER); assert!( - T::MaxCodeLen::get() < code_len_limit, - "Given `CallStack` height {:?}, `MaxCodeLen` should be set less than {:?} \ + STATIC_MEMORY_BYTES < static_memory_limit, + "Given `CallStack` height {:?}, `STATIC_MEMORY_LIMIT` should be set less than {:?} \ (current value is {:?}), to avoid possible runtime oom issues.", max_call_depth, - code_len_limit, - T::MaxCodeLen::get(), + static_memory_limit, + STATIC_MEMORY_BYTES, ); // Validators are configured to be able to use more memory than block builders. This is @@ -1060,12 +1036,8 @@ where let (executable, upload_deposit) = match code { Code::Upload(code) => { let upload_account = T::UploadOrigin::ensure_origin(origin)?; - let (executable, upload_deposit) = Self::try_upload_code( - upload_account, - code, - storage_deposit_limit, - debug_message.as_mut(), - )?; + let (executable, upload_deposit) = + Self::try_upload_code(upload_account, code, storage_deposit_limit)?; storage_deposit_limit.saturating_reduce(upload_deposit); (executable, upload_deposit) }, @@ -1117,7 +1089,7 @@ where storage_deposit_limit: BalanceOf, ) -> CodeUploadResult> { let origin = T::UploadOrigin::ensure_origin(origin)?; - let (module, deposit) = Self::try_upload_code(origin, code, storage_deposit_limit, None)?; + let (module, deposit) = Self::try_upload_code(origin, code, storage_deposit_limit)?; Ok(CodeUploadReturnValue { code_hash: *module.code_hash(), deposit }) } @@ -1135,12 +1107,8 @@ where origin: T::AccountId, code: Vec, storage_deposit_limit: BalanceOf, - mut debug_message: Option<&mut DebugBuffer>, ) -> Result<(WasmBlob, BalanceOf), DispatchError> { - let mut module = WasmBlob::from_code(code, origin).map_err(|(err, msg)| { - debug_message.as_mut().map(|d| d.try_extend(msg.bytes())); - err - })?; + let mut module = WasmBlob::from_code(code, origin)?; let deposit = module.store_code()?; ensure!(storage_deposit_limit >= deposit, >::StorageDepositLimitExhausted); Ok((module, deposit)) diff --git a/substrate/frame/revive/src/limits.rs b/substrate/frame/revive/src/limits.rs index 1a714a89d486..f712493d3bc8 100644 --- a/substrate/frame/revive/src/limits.rs +++ b/substrate/frame/revive/src/limits.rs @@ -22,15 +22,21 @@ //! is meant for. This is true for either increasing or decreasing the limit. //! //! Limits in this file are different from the limits configured on the [`Config`] trait which are -//! generally only affect actions that cannot be performed by a contract: For example, uploading new -//! code only be done via a transaction but not by a contract. Hence the maximum contract size can -//! be raised (but not lowered) by the runtime configuration. +//! generally only affect actions that cannot be performed by a contract: For example things related +//! to deposits and weights are allowed to be changed as they are paid by root callers which +//! are not contracts. +//! +//! Exceptions to this rule apply: Limits in the [`code`] module can be increased +//! without emulating the old values for existing contracts. Reason is that those limits are only +//! applied **once** at code upload time. Since this action cannot be performed by contracts we +//! can change those limits without breaking existing contracts. Please keep in mind that we should +//! only ever **increase** those values but never decrease. /// The maximum depth of the call stack. /// /// A 0 means that no callings of other contracts are possible. In other words only the origin /// called "root contract" is allowed to execute then. -pub const CALL_STACK_DEPTH: u32 = 5; +pub const CALL_STACK_DEPTH: u32 = 10; /// The maximum number of topics a call to [`crate::SyscallDoc::deposit_event`] can emit. /// @@ -40,10 +46,7 @@ pub const NUM_EVENT_TOPICS: u32 = 4; /// The maximum number of code hashes a contract can lock. pub const DELEGATE_DEPENDENCIES: u32 = 32; -/// How much memory do we allow the contract to allocate. -pub const MEMORY_BYTES: u32 = 16 * 64 * 1024; - -/// Maximum size of events (excluding topics) and storage values. +/// Maximum size of events (including topics) and storage values. pub const PAYLOAD_BYTES: u32 = 512; /// The maximum size of the transient storage in bytes. @@ -58,3 +61,85 @@ pub const STORAGE_KEY_BYTES: u32 = 128; /// /// The buffer will always be disabled for on-chain execution. pub const DEBUG_BUFFER_BYTES: u32 = 2 * 1024 * 1024; + +/// The page size in which PolkaVM should allocate memory chunks. +pub const PAGE_SIZE: u32 = 4 * 1024; + +/// Limits that are only enforced on code upload. +/// +/// # Note +/// +/// This limit can be increased later without breaking existing contracts +/// as it is only enforced at code upload time. Code already uploaded +/// will not be affected by those limits. +pub mod code { + use super::PAGE_SIZE; + use crate::{CodeVec, Config, Error, LOG_TARGET}; + use alloc::vec::Vec; + use frame_support::ensure; + use sp_runtime::DispatchError; + + /// The maximum length of a code blob in bytes. + /// + /// This mostly exist to prevent parsing too big blobs and to + /// have a maximum encoded length. The actual memory calculation + /// is purely based off [`STATIC_MEMORY_BYTES`]. + pub const BLOB_BYTES: u32 = 256 * 1024; + + /// Maximum size the program is allowed to take in memory. + /// + /// This includes data and code. Increasing this limit will allow + /// for more code or more data. However, since code will decompress + /// into a bigger representation on compilation it will only increase + /// the allowed code size by [`BYTE_PER_INSTRUCTION`]. + pub const STATIC_MEMORY_BYTES: u32 = 1024 * 1024; + + /// How much memory each instruction will take in-memory after compilation. + /// + /// This is `size_of() + 16`. But we don't use `usize` here so it isn't + /// different on the native runtime (used for testing). + const BYTES_PER_INSTRUCTION: u32 = 20; + + /// The code is stored multiple times as part of the compiled program. + const EXTRA_OVERHEAD_PER_CODE_BYTE: u32 = 4; + + /// Make sure that the various program parts are within the defined limits. + pub fn enforce(blob: Vec) -> Result { + fn round_page(n: u32) -> u64 { + // performing the rounding in u64 in order to prevent overflow + u64::from(n).next_multiple_of(PAGE_SIZE.into()) + } + + let blob: CodeVec = blob.try_into().map_err(|_| >::BlobTooLarge)?; + + let program = polkavm::ProgramBlob::parse(blob.as_slice().into()).map_err(|err| { + log::debug!(target: LOG_TARGET, "failed to parse polkavm blob: {err:?}"); + Error::::CodeRejected + })?; + + // this is O(n) but it allows us to be more precise + let num_instructions = program.instructions().count() as u64; + + // The memory consumptions is the byte size of the whole blob, + // minus the RO data payload in the blob, + // minus the RW data payload in the blob, + // plus the RO data in memory (which is always equal or bigger than the RO payload), + // plus RW data in memory, plus stack size in memory. + // plus the overhead of instructions in memory which is derived from the code + // size itself and the number of instruction + let memory_size = (blob.len() as u64) + .saturating_add(round_page(program.ro_data_size())) + .saturating_sub(program.ro_data().len() as u64) + .saturating_add(round_page(program.rw_data_size())) + .saturating_sub(program.rw_data().len() as u64) + .saturating_add(round_page(program.stack_size())) + .saturating_add((num_instructions).saturating_mul(BYTES_PER_INSTRUCTION.into())) + .saturating_add( + (program.code().len() as u64).saturating_mul(EXTRA_OVERHEAD_PER_CODE_BYTE.into()), + ); + + ensure!(memory_size <= STATIC_MEMORY_BYTES as u64, >::StaticMemoryTooLarge); + + Ok(blob) + } +} diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index 8a18c09e86f0..167e8da201b8 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -4255,6 +4255,44 @@ mod run_tests { }); } + #[test] + fn static_data_limit_is_enforced() { + let (oom_rw_trailing, _) = compile_module("oom_rw_trailing").unwrap(); + let (oom_rw_included, _) = compile_module("oom_rw_included").unwrap(); + let (oom_ro, _) = compile_module("oom_ro").unwrap(); + + ExtBuilder::default().build().execute_with(|| { + let _ = Balances::set_balance(&ALICE, 1_000_000); + + assert_err!( + Contracts::upload_code( + RuntimeOrigin::signed(ALICE), + oom_rw_trailing, + deposit_limit::(), + ), + >::StaticMemoryTooLarge + ); + + assert_err!( + Contracts::upload_code( + RuntimeOrigin::signed(ALICE), + oom_rw_included, + deposit_limit::(), + ), + >::BlobTooLarge + ); + + assert_err!( + Contracts::upload_code( + RuntimeOrigin::signed(ALICE), + oom_ro, + deposit_limit::(), + ), + >::BlobTooLarge + ); + }); + } + #[test] fn call_diverging_out_len_works() { let (code, _) = compile_module("call_diverging_out_len").unwrap(); diff --git a/substrate/frame/revive/src/wasm/mod.rs b/substrate/frame/revive/src/wasm/mod.rs index b8f6eef126b2..cd274873975d 100644 --- a/substrate/frame/revive/src/wasm/mod.rs +++ b/substrate/frame/revive/src/wasm/mod.rs @@ -35,6 +35,7 @@ use crate::{ address::AddressMapper, exec::{ExecResult, Executable, ExportedFunction, Ext}, gas::{GasMeter, Token}, + limits, storage::meter::Diff, weights::WeightInfo, AccountIdOf, BadOrigin, BalanceOf, CodeInfoOf, CodeVec, Config, Error, Event, ExecError, @@ -56,7 +57,7 @@ use sp_runtime::DispatchError; #[codec(mel_bound())] #[scale_info(skip_type_params(T))] pub struct WasmBlob { - code: CodeVec, + code: CodeVec, // This isn't needed for contract execution and is not stored alongside it. #[codec(skip)] code_info: CodeInfo, @@ -128,12 +129,11 @@ where BalanceOf: Into + TryFrom, { /// We only check for size and nothing else when the code is uploaded. - pub fn from_code( - code: Vec, - owner: AccountIdOf, - ) -> Result { - let code: CodeVec = - code.try_into().map_err(|_| (>::CodeTooLarge.into(), ""))?; + pub fn from_code(code: Vec, owner: AccountIdOf) -> Result { + // We do size checks when new code is deployed. This allows us to increase + // the limits later without affecting already deployed code. + let code = limits::code::enforce::(code)?; + let code_len = code.len() as u32; let bytes_added = code_len.saturating_add(>::max_encoded_len() as u32); let deposit = Diff { bytes_added, items_added: 2, ..Default::default() } @@ -283,16 +283,16 @@ impl WasmBlob { entry_point: ExportedFunction, api_version: ApiVersion, ) -> Result, ExecError> { - let code = self.code.as_slice(); - let mut config = polkavm::Config::default(); config.set_backend(Some(polkavm::BackendKind::Interpreter)); let engine = polkavm::Engine::new(&config).expect("interpreter is available on all plattforms; qed"); let mut module_config = polkavm::ModuleConfig::new(); + module_config.set_page_size(limits::PAGE_SIZE); module_config.set_gas_metering(Some(polkavm::GasMeteringKind::Sync)); - let module = polkavm::Module::new(&engine, &module_config, code.into()).map_err(|err| { + let module = polkavm::Module::new(&engine, &module_config, self.code.into_inner().into()) + .map_err(|err| { log::debug!(target: LOG_TARGET, "failed to create polkavm module: {err:?}"); Error::::CodeRejected })?; diff --git a/substrate/frame/revive/uapi/Cargo.toml b/substrate/frame/revive/uapi/Cargo.toml index 862bf36f07cd..52de77a10944 100644 --- a/substrate/frame/revive/uapi/Cargo.toml +++ b/substrate/frame/revive/uapi/Cargo.toml @@ -21,7 +21,7 @@ codec = { features = [ ], optional = true, workspace = true } [target.'cfg(target_arch = "riscv32")'.dependencies] -polkavm-derive = { version = "0.10.0" } +polkavm-derive = { version = "0.11.0" } [package.metadata.docs.rs] default-target = ["wasm32-unknown-unknown"]