From 310ef5ce1086affdc522c4d1736211de2a7dd99e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= <alex.theissen@me.com>
Date: Wed, 18 Sep 2024 16:48:30 +0200
Subject: [PATCH] revive: Limit the amount of static memory a contract can use
(#5726)
This will make sure that when uploading new code that the declared
static memory fits within a defined limit. We apply different limits to
code and data. Reason is that code will consume much more memory per
byte once decoded during lazy execution.
This PR:
1) Remove the MaxCodeLen from the `Config` to we maintain tight control
over it.
2) Defines a single `STATIC_MEMORY_BYTES` knob that limits the maximum
decoded size.
3) Enforces them only on upload but not on execution so we can raise
them later.
4) Adapt the worst case calculation in `integrity_check`.
5) Bumps the max stack depth from 5 to 10 as this will still fit within
our memory envelope.
6) The memory limit per contract is now a cool 1MB that can be spent on
data or code.
7) Bump PolkaVM for good measure
8) The blob is limited to 256kb which is just a sanity check to not even
try parsing very big inputs.
---------
Co-authored-by: Cyrill Leutwiler <cyrill@parity.io>
---
Cargo.lock | 62 +++++------
prdoc/pr_5726.prdoc | 14 +++
substrate/bin/node/runtime/src/lib.rs | 1 -
substrate/frame/revive/Cargo.toml | 2 +-
substrate/frame/revive/fixtures/Cargo.toml | 2 +-
.../frame/revive/fixtures/build/Cargo.toml | 2 +-
.../frame/revive/fixtures/contracts/oom_ro.rs | 44 ++++++++
.../fixtures/contracts/oom_rw_included.rs | 44 ++++++++
.../fixtures/contracts/oom_rw_trailing.rs | 44 ++++++++
.../frame/revive/src/benchmarking/mod.rs | 30 +++---
substrate/frame/revive/src/lib.rs | 100 ++++++-----------
substrate/frame/revive/src/limits.rs | 101 ++++++++++++++++--
substrate/frame/revive/src/tests.rs | 38 +++++++
substrate/frame/revive/src/wasm/mod.rs | 20 ++--
substrate/frame/revive/uapi/Cargo.toml | 2 +-
15 files changed, 371 insertions(+), 135 deletions(-)
create mode 100644 prdoc/pr_5726.prdoc
create mode 100644 substrate/frame/revive/fixtures/contracts/oom_ro.rs
create mode 100644 substrate/frame/revive/fixtures/contracts/oom_rw_included.rs
create mode 100644 substrate/frame/revive/fixtures/contracts/oom_rw_trailing.rs
diff --git a/Cargo.lock b/Cargo.lock
index 50e42667b94..dd75a9ec43f 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 00000000000..ce666647bad
--- /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 c8409078af5..bfcaf5e9af8 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<Self>;
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<false>;
diff --git a/substrate/frame/revive/Cargo.toml b/substrate/frame/revive/Cargo.toml
index 667328ac2d0..177b3c3e9ee 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 903298d2df2..53db08a3911 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 7dead51b230..5b3a21b5122 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 00000000000..41c080d5847
--- /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 00000000000..2cdcf7bafed
--- /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 00000000000..993be8e9cda
--- /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 332c425d714..cbc4cc62d48 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::<T>::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::<T>::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::<T>());
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::<T>::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::<T>::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::<T>::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::<T>::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::<Vec<_>>();
let message_len = message.len() as u32;
diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs
index ccbc65873d0..bf980369056 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<u8, ConstU32<128>>;
type BalanceOf<T> =
<<T as Config>::Currency as Inspect<<T as frame_system::Config>::AccountId>>::Balance;
-type CodeVec<T> = BoundedVec<u8, <T as Config>::MaxCodeLen>;
+type CodeVec = BoundedVec<u8, ConstU32<{ limits::code::BLOB_BYTES }>>;
type EventRecordOf<T> =
EventRecord<<T as frame_system::Config>::RuntimeEvent, <T as frame_system::Config>::Hash>;
type DebugBuffer = BoundedVec<u8, ConstU32<{ limits::DEBUG_BUFFER_BYTES }>>;
@@ -232,14 +232,6 @@ pub mod pallet {
#[pallet::no_default_bounds]
type AddressMapper: AddressMapper<AccountIdOf<Self>>;
- /// 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<u32>;
-
/// 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<true>;
type UploadOrigin = EnsureSigned<AccountId>;
@@ -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<T: Config> = StorageMap<_, Identity, H256, CodeVec<T>>;
+ pub(crate) type PristineCode<T: Config> = 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<T>,
) -> CodeUploadResult<BalanceOf<T>> {
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<u8>,
storage_deposit_limit: BalanceOf<T>,
- mut debug_message: Option<&mut DebugBuffer>,
) -> Result<(WasmBlob<T>, BalanceOf<T>), 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, <Error<T>>::StorageDepositLimitExhausted);
Ok((module, deposit))
diff --git a/substrate/frame/revive/src/limits.rs b/substrate/frame/revive/src/limits.rs
index 1a714a89d48..f712493d3bc 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<usize>() + 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<T: Config>(blob: Vec<u8>) -> Result<CodeVec, DispatchError> {
+ 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(|_| <Error<T>>::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::<T>::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, <Error<T>>::StaticMemoryTooLarge);
+
+ Ok(blob)
+ }
+}
diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs
index 8a18c09e86f..167e8da201b 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::<Test>(),
+ ),
+ <Error<Test>>::StaticMemoryTooLarge
+ );
+
+ assert_err!(
+ Contracts::upload_code(
+ RuntimeOrigin::signed(ALICE),
+ oom_rw_included,
+ deposit_limit::<Test>(),
+ ),
+ <Error<Test>>::BlobTooLarge
+ );
+
+ assert_err!(
+ Contracts::upload_code(
+ RuntimeOrigin::signed(ALICE),
+ oom_ro,
+ deposit_limit::<Test>(),
+ ),
+ <Error<Test>>::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 b8f6eef126b..cd274873975 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<T: Config> {
- code: CodeVec<T>,
+ code: CodeVec,
// This isn't needed for contract execution and is not stored alongside it.
#[codec(skip)]
code_info: CodeInfo<T>,
@@ -128,12 +129,11 @@ where
BalanceOf<T>: Into<U256> + TryFrom<U256>,
{
/// We only check for size and nothing else when the code is uploaded.
- pub fn from_code(
- code: Vec<u8>,
- owner: AccountIdOf<T>,
- ) -> Result<Self, (DispatchError, &'static str)> {
- let code: CodeVec<T> =
- code.try_into().map_err(|_| (<Error<T>>::CodeTooLarge.into(), ""))?;
+ pub fn from_code(code: Vec<u8>, owner: AccountIdOf<T>) -> Result<Self, DispatchError> {
+ // 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::<T>(code)?;
+
let code_len = code.len() as u32;
let bytes_added = code_len.saturating_add(<CodeInfo<T>>::max_encoded_len() as u32);
let deposit = Diff { bytes_added, items_added: 2, ..Default::default() }
@@ -283,16 +283,16 @@ impl<T: Config> WasmBlob<T> {
entry_point: ExportedFunction,
api_version: ApiVersion,
) -> Result<PreparedCall<E>, 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::<T>::CodeRejected
})?;
diff --git a/substrate/frame/revive/uapi/Cargo.toml b/substrate/frame/revive/uapi/Cargo.toml
index 862bf36f07c..52de77a1094 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"]
--
GitLab