diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 4637f53b..fc0c35d6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -67,6 +67,18 @@ jobs: run: make build-release # Builds node binary AND runtime WASM (no need to rebuild separately) + - name: Check the PolkaVM guest ABI + run: | + # `frame/evm-polkavm/uapi` compiles its host-function ABI under + # `cfg(target_arch = "riscv64")`. Nothing else in the workspace builds for that + # target, so this half went unchecked from the commit that added it until + # someone tried by hand — and it did not compile. The target is installed here + # rather than in rust-toolchain.toml so a normal `cargo build` does not pay for + # a ~100 MB std nobody else needs. + rustup target add riscv64imac-unknown-none-elf + cargo check -p pallet-evm-polkavm-uapi \ + --target riscv64imac-unknown-none-elf + - name: Upload node binary uses: actions/upload-artifact@v4 with: diff --git a/Cargo.lock b/Cargo.lock index 8625ec2c..b9d1a775 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11155,7 +11155,7 @@ dependencies = [ "pallet-evm-polkavm-proc-macro", "pallet-evm-polkavm-uapi", "parity-scale-codec", - "polkavm 0.29.1", + "polkavm", "scale-info", "sp-core", "sp-runtime", @@ -11177,7 +11177,7 @@ dependencies = [ "bitflags 1.3.2", "pallet-evm-polkavm-proc-macro", "parity-scale-codec", - "polkavm-derive 0.30.0", + "polkavm-derive 0.33.0", "scale-info", ] @@ -12202,7 +12202,7 @@ dependencies = [ "pallet-transaction-payment", "parity-scale-codec", "paste", - "polkavm 0.33.1", + "polkavm", "polkavm-common 0.33.0", "rand 0.8.8", "rand_pcg 0.3.1", @@ -14996,19 +14996,6 @@ dependencies = [ "tracing-gum", ] -[[package]] -name = "polkavm" -version = "0.29.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "63c8211d36125b6cc451b3cbc46b8ee27fefb54521b67f43c8630bd1afbd44d4" -dependencies = [ - "libc", - "log", - "polkavm-assembler 0.29.0", - "polkavm-common 0.29.0", - "polkavm-linux-raw 0.29.0", -] - [[package]] name = "polkavm" version = "0.33.1" @@ -15018,18 +15005,9 @@ dependencies = [ "libc", "log", "picosimd", - "polkavm-assembler 0.33.0", + "polkavm-assembler", "polkavm-common 0.33.0", - "polkavm-linux-raw 0.33.0", -] - -[[package]] -name = "polkavm-assembler" -version = "0.29.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "914aacebfbc22da7772f5ecb6f79b39901dc4061121598bd4383a590a7506ebb" -dependencies = [ - "log", + "polkavm-linux-raw", ] [[package]] @@ -15041,16 +15019,6 @@ dependencies = [ "log", ] -[[package]] -name = "polkavm-common" -version = "0.29.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f634b46a6a47a5de381f56d1d8cced9f8640d063b2b1a44b0da6dbef91bbd400" -dependencies = [ - "log", - "polkavm-assembler 0.29.0", -] - [[package]] name = "polkavm-common" version = "0.30.0" @@ -15069,7 +15037,7 @@ dependencies = [ "blake3", "log", "picosimd", - "polkavm-assembler 0.33.0", + "polkavm-assembler", ] [[package]] @@ -15166,12 +15134,6 @@ dependencies = [ "rustc-demangle", ] -[[package]] -name = "polkavm-linux-raw" -version = "0.29.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "751fbbcf86635834dd9a700039c74ce8c7871b317acc84582d9667dad2ed9848" - [[package]] name = "polkavm-linux-raw" version = "0.33.0" @@ -17502,7 +17464,7 @@ version = "0.47.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6d66dcbbcf2966c3cc97c4a63ed461ede261600f36b55e4b62553dfd318941ca" dependencies = [ - "polkavm 0.33.1", + "polkavm", "sc-allocator", "sp-maybe-compressed-blob", "sp-wasm-interface", @@ -17517,7 +17479,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "98913e1b5f009040cdceb7607ebd3859f74a578152861c750550ff5780c94314" dependencies = [ "log", - "polkavm 0.33.1", + "polkavm", "sc-executor-common", "sp-runtime-interface", "sp-wasm-interface", @@ -18310,7 +18272,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3cd060d9893547a61c6088e2196c9997341ff64d2d7d96494828fee878e03a92" dependencies = [ "log", - "polkavm 0.33.1", + "polkavm", "sp-virtualization", ] diff --git a/frame/evm-polkavm/Cargo.toml b/frame/evm-polkavm/Cargo.toml index 92f4f6f8..642ff0c0 100644 --- a/frame/evm-polkavm/Cargo.toml +++ b/frame/evm-polkavm/Cargo.toml @@ -26,7 +26,7 @@ pallet-evm = { workspace = true } # PolkaVM executor pallet-evm-polkavm-proc-macro = { workspace = true } pallet-evm-polkavm-uapi = { workspace = true, features = ["scale"] } -polkavm = { version = "0.29.1", default-features = false } +polkavm = { version = "0.33.1", default-features = false } [features] default = ["std"] diff --git a/frame/evm-polkavm/src/lib.rs b/frame/evm-polkavm/src/lib.rs index c141b193..058fc70b 100644 --- a/frame/evm-polkavm/src/lib.rs +++ b/frame/evm-polkavm/src/lib.rs @@ -59,7 +59,7 @@ impl PrecompileSet for PolkaVmSet { ) -> Option> { let code_address = handle.code_address(); let code = pallet_evm::AccountCodes::::get(code_address); - if code[0..8] == vm::PREFIX { + if code.get(0..8) == Some(&vm::PREFIX[..]) { let mut run = || { let prepared_call: vm::PreparedCall<'_, T, _> = vm::PreparedCall::load(handle)?; prepared_call.call() @@ -90,7 +90,7 @@ impl PrecompileSet for PolkaVmSet { fn is_precompile(&self, address: H160, remaining_gas: u64) -> IsPrecompileResult { let code = pallet_evm::AccountCodes::::get(address); - if code[0..8] == vm::PREFIX { + if code.get(0..8) == Some(&vm::PREFIX[..]) { IsPrecompileResult::Answer { is_precompile: true, extra_cost: 0, @@ -134,6 +134,14 @@ pub mod pallet { NotPolkaVmContract, /// Contract already exist in state. AlreadyExist, + /// The code is not a parseable PolkaVM program blob. + InvalidProgramBlob, + /// The blob declares an instruction set this chain does not accept. + /// + /// Only `ReviveV1` and `JamV1` are allowed. The `Latest32`/`Latest64` sets include + /// the `sbrk` opcode, which lets a contract grow its heap at run time — memory + /// growth changes gas consumption, and this is consensus code. + UnsupportedInstructionSet, } #[pallet::call] @@ -150,10 +158,16 @@ pub mod pallet { return Err(Error::::MaxCodeSizeExceeded.into()); } - if code[0..8] != crate::vm::PREFIX { + if code.get(0..8) != Some(&crate::vm::PREFIX[..]) { return Err(Error::::NotPolkaVmContract.into()); } + let blob = polkavm::ProgramBlob::parse(code[8..].to_vec().into()) + .map_err(|_| Error::::InvalidProgramBlob)?; + if !crate::vm::is_accepted_isa(blob.isa()) { + return Err(Error::::UnsupportedInstructionSet.into()); + } + let caller = ensure_signed(origin)?; let address = ::CreateAddressScheme::create_address_scheme(caller, &code[..], salt); diff --git a/frame/evm-polkavm/src/vm/mod.rs b/frame/evm-polkavm/src/vm/mod.rs index e96ac139..614f8c3c 100644 --- a/frame/evm-polkavm/src/vm/mod.rs +++ b/frame/evm-polkavm/src/vm/mod.rs @@ -26,6 +26,25 @@ pub use self::runtime::{ExecResult, Runtime, RuntimeCosts, SupervisorError}; pub const PREFIX: [u8; 8] = [0xef, 0x70, 0x6F, 0x6C, 0x6B, 0x61, 0x76, 0x6D]; pub const CALL_IDENTIFIER: &str = "call"; pub const PAGE_SIZE: u32 = 4 * 1024; + +/// Whether this chain accepts contracts compiled for `isa`. +/// +/// Until polkavm 0.30 the host refused `sbrk` with `ModuleConfig::set_allow_sbrk(false)`. +/// That knob is gone: the blob's own instruction set now decides whether the opcode +/// decodes, and the linker takes the ISA as an explicit argument, so the choice belongs +/// to whoever built the contract. `Latest32`/`Latest64` include `sbrk` (opcode 101); +/// `ReviveV1` and `JamV1` do not. +/// +/// A growable heap changes gas consumption, and this runs in consensus, so the answer is +/// a whitelist rather than a blocklist: a future ISA is rejected until someone checks it. +pub fn is_accepted_isa(isa: polkavm::program::InstructionSetKind) -> bool { + use polkavm::program::InstructionSetKind as Isa; + match isa { + Isa::ReviveV1 | Isa::JamV1 => true, + Isa::Latest32 | Isa::Latest64 => false, + } +} + pub const SENTINEL: u32 = u32::MAX; pub const LOG_TARGET: &str = "runtime::evm::polkavm"; @@ -68,7 +87,6 @@ impl<'a, T: Config, H: PrecompileHandle> PreparedCall<'a, T, H> { let mut module_config = polkavm::ModuleConfig::new(); module_config.set_page_size(PAGE_SIZE); module_config.set_gas_metering(Some(polkavm::GasMeteringKind::Sync)); - module_config.set_allow_sbrk(false); let module = polkavm::Module::new(&engine, &module_config, polkavm_code.into()).map_err(|err| { log::debug!(target: LOG_TARGET, "failed to create polkavm module: {err:?}"); @@ -116,3 +134,94 @@ impl<'a, T: Config, H: PrecompileHandle> PreparedCall<'a, T, H> { exec_result } } + +#[cfg(test)] +mod tests { + use super::*; + use polkavm::program::InstructionSetKind as Isa; + + /// Smallest blob `ProgramBlob::parse` accepts, with the ISA we want to declare. + /// + /// Hand-built rather than linked from a guest program: the ISA lives in one header + /// byte, so a real riscv64 contract would add a toolchain dependency and prove + /// nothing extra about the check under test. + fn blob_with_isa(isa: Isa) -> Vec { + let version: u8 = match isa { + Isa::ReviveV1 => 0, + Isa::Latest32 => 1, + Isa::Latest64 => 2, + Isa::JamV1 => 3, + }; + + // jump_table_entry_count, jump_table_entry_size, code_length, code, bitmask. + // The bitmask is ceil(code_len / 8) bytes and marks instruction boundaries. + let code_section: Vec = vec![0, 0, 1, 0, 0b0000_0001]; + let mut body = vec![6u8]; // SECTION_CODE_AND_JUMP_TABLE + body.push(code_section.len() as u8); + body.extend_from_slice(&code_section); + body.push(0u8); // SECTION_END_OF_FILE + + // magic + version + u64 length-of-whole-blob + body + let total = (4 + 1 + 8 + body.len()) as u64; + let mut blob = vec![b'P', b'V', b'M', 0u8, version]; + blob.extend_from_slice(&total.to_le_bytes()); + blob.extend_from_slice(&body); + blob + } + + /// The blobs the test feeds the check must actually declare the ISA asked for, + /// otherwise the cases below would pass by accident. + #[test] + fn fixture_declares_the_requested_isa() { + for isa in [Isa::ReviveV1, Isa::JamV1, Isa::Latest32, Isa::Latest64] { + let parsed = polkavm::ProgramBlob::parse(blob_with_isa(isa).into()) + .expect("hand-built blob should parse"); + assert_eq!(parsed.isa(), isa); + } + } + + /// This is the guarantee `ModuleConfig::set_allow_sbrk(false)` used to provide: no + /// contract may grow its heap. `Latest32`/`Latest64` carry the `sbrk` opcode, so + /// accepting them would silently restore what the old knob forbade. + #[test] + fn only_sbrk_free_instruction_sets_are_accepted() { + assert!(is_accepted_isa(Isa::ReviveV1)); + assert!(is_accepted_isa(Isa::JamV1)); + assert!(!is_accepted_isa(Isa::Latest32)); + assert!(!is_accepted_isa(Isa::Latest64)); + } + + /// The prefix test must tolerate short and empty code. + /// + /// `AccountCodes::get` returns an empty `Vec` for every address without contract + /// code, and the precompile set consults it on each call, so `code[0..8]` panics on + /// the most ordinary path there is: a transfer to a plain account. + #[test] + fn prefix_check_tolerates_code_shorter_than_the_prefix() { + let matches = |code: &[u8]| code.get(0..8) == Some(&PREFIX[..]); + + assert!(!matches(&[])); + assert!(!matches(&PREFIX[..7])); + assert!(matches(&PREFIX)); + + let mut with_body = PREFIX.to_vec(); + with_body.extend_from_slice(b"blob"); + assert!(matches(&with_body)); + + let mut wrong = PREFIX; + wrong[0] = 0x00; + assert!(!matches(&wrong)); + } + + /// A blob that does not parse must be rejected before the ISA is consulted — the + /// deploy path calls `parse` first and maps the failure to its own error. + #[test] + fn malformed_blobs_do_not_parse() { + assert!(polkavm::ProgramBlob::parse(vec![].into()).is_err()); + assert!(polkavm::ProgramBlob::parse(b"PVM\0".to_vec().into()).is_err()); + // Right shape, unknown ISA version byte. + let mut bad = blob_with_isa(Isa::ReviveV1); + bad[4] = 99; + assert!(polkavm::ProgramBlob::parse(bad.into()).is_err()); + } +} diff --git a/frame/evm-polkavm/src/vm/runtime.rs b/frame/evm-polkavm/src/vm/runtime.rs index e76f2fba..0c92935d 100644 --- a/frame/evm-polkavm/src/vm/runtime.rs +++ b/frame/evm-polkavm/src/vm/runtime.rs @@ -61,7 +61,7 @@ pub trait Memory { /// Returns `Err` if one of the following conditions occurs: /// /// - requested buffer is not within the bounds of the sandbox memory. - fn read_into_buf(&self, ptr: u32, buf: &mut [u8]) -> Result<(), SupervisorError>; + fn read_into_buf(&mut self, ptr: u32, buf: &mut [u8]) -> Result<(), SupervisorError>; /// Write the given buffer to the designated location in the sandbox memory. /// @@ -82,40 +82,40 @@ pub trait Memory { /// Returns `Err` if one of the following conditions occurs: /// /// - requested buffer is not within the bounds of the sandbox memory. - fn read(&self, ptr: u32, len: u32) -> Result, SupervisorError> { + fn read(&mut self, ptr: u32, len: u32) -> Result, SupervisorError> { let mut buf = vec![0u8; len as usize]; self.read_into_buf(ptr, buf.as_mut_slice())?; Ok(buf) } /// Same as `read` but reads into a fixed size buffer. - fn read_array(&self, ptr: u32) -> Result<[u8; N], SupervisorError> { + fn read_array(&mut self, ptr: u32) -> Result<[u8; N], SupervisorError> { let mut buf = [0u8; N]; self.read_into_buf(ptr, &mut buf)?; Ok(buf) } /// Read a `u32` from the sandbox memory. - fn read_u32(&self, ptr: u32) -> Result { + fn read_u32(&mut self, ptr: u32) -> Result { let buf: [u8; 4] = self.read_array(ptr)?; Ok(u32::from_le_bytes(buf)) } /// Read a `U256` from the sandbox memory. - fn read_u256(&self, ptr: u32) -> Result { + fn read_u256(&mut self, ptr: u32) -> Result { let buf: [u8; 32] = self.read_array(ptr)?; Ok(U256::from_little_endian(&buf)) } /// Read a `H160` from the sandbox memory. - fn read_h160(&self, ptr: u32) -> Result { + fn read_h160(&mut self, ptr: u32) -> Result { let mut buf = H160::default(); self.read_into_buf(ptr, buf.as_bytes_mut())?; Ok(buf) } /// Read a `H256` from the sandbox memory. - fn read_h256(&self, ptr: u32) -> Result { + fn read_h256(&mut self, ptr: u32) -> Result { let mut code_hash = H256::default(); self.read_into_buf(ptr, code_hash.as_bytes_mut())?; Ok(code_hash) @@ -142,7 +142,7 @@ pub trait PolkaVmInstance: Memory { // in the streaming implementation while it could fail with a segfault in the copy implementation. #[cfg(feature = "runtime-benchmarks")] impl Memory for [u8] { - fn read_into_buf(&self, ptr: u32, buf: &mut [u8]) -> Result<(), SupervisorError> { + fn read_into_buf(&mut self, ptr: u32, buf: &mut [u8]) -> Result<(), SupervisorError> { let ptr = ptr as usize; let bound_checked = self .get(ptr..ptr + buf.len()) @@ -166,7 +166,7 @@ impl Memory for [u8] { } impl Memory for polkavm::RawInstance { - fn read_into_buf(&self, ptr: u32, buf: &mut [u8]) -> Result<(), SupervisorError> { + fn read_into_buf(&mut self, ptr: u32, buf: &mut [u8]) -> Result<(), SupervisorError> { self.read_memory_into(ptr, buf) .map(|_| ()) .map_err(|_| SupervisorError::OutOfBounds) diff --git a/frame/evm-polkavm/uapi/Cargo.toml b/frame/evm-polkavm/uapi/Cargo.toml index 2989cba4..2a58b676 100644 --- a/frame/evm-polkavm/uapi/Cargo.toml +++ b/frame/evm-polkavm/uapi/Cargo.toml @@ -18,7 +18,7 @@ scale-codec = { features = ["derive", "max-encoded-len"], optional = true, works scale-info = { features = ["derive"], optional = true, workspace = true } [target.'cfg(target_arch = "riscv64")'.dependencies] -polkavm-derive = { version = "0.30.0" } +polkavm-derive = { version = "0.33.0" } [features] default = ["scale"] diff --git a/frame/evm-polkavm/uapi/src/host/riscv64.rs b/frame/evm-polkavm/uapi/src/host/riscv64.rs index 3237b7ac..c724869f 100644 --- a/frame/evm-polkavm/uapi/src/host/riscv64.rs +++ b/frame/evm-polkavm/uapi/src/host/riscv64.rs @@ -18,10 +18,9 @@ #![allow(unused_variables)] use crate::{ - host::{CallFlags, HostFn, HostFnImpl, Result, StorageFlags}, - pack_hi_lo, ReturnFlags, + host::{HostFn, HostFnImpl}, + ReturnFlags, }; -use pallet_revive_proc_macro::unstable_hostfn; mod sys { use crate::ReturnCode; @@ -55,29 +54,6 @@ mod sys { } } -#[inline(always)] -fn extract_from_slice(output: &mut &mut [u8], new_len: usize) { - debug_assert!(new_len <= output.len()); - let tmp = core::mem::take(output); - *output = &mut tmp[..new_len]; -} - -#[inline(always)] -fn ptr_len_or_sentinel(data: &mut Option<&mut &mut [u8]>) -> (*mut u8, u32) { - match data { - Some(ref mut data) => (data.as_mut_ptr(), data.len() as _), - None => (crate::SENTINEL as _, 0), - } -} - -#[inline(always)] -fn ptr_or_sentinel(data: &Option<&[u8; 32]>) -> *const u8 { - match data { - Some(ref data) => data.as_ptr(), - None => crate::SENTINEL as _, - } -} - impl HostFn for HostFnImpl { fn deposit_event(topics: &[[u8; 32]], data: &[u8]) { unsafe {