Skip to content

Commit

Permalink
Merge branch 'master' into donal-auto-renew-benchmarks
Browse files Browse the repository at this point in the history
  • Loading branch information
seadanda authored Dec 12, 2024
2 parents a7e0b6c + 389e221 commit fd071d4
Show file tree
Hide file tree
Showing 21 changed files with 882 additions and 587 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/check-semver.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ jobs:
- name: install parity-publish
# Set the target dir to cache the build.
run: CARGO_TARGET_DIR=./target/ cargo install [email protected].2 --locked -q
run: CARGO_TARGET_DIR=./target/ cargo install [email protected].3 --locked -q

- name: check semver
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish-check-compile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
cache-on-failure: true

- name: install parity-publish
run: cargo install [email protected].2 --locked -q
run: cargo install [email protected].3 --locked -q

- name: parity-publish update plan
run: parity-publish --color always plan --skip-check --prdoc prdoc/
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish-check-crates.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
cache-on-failure: true

- name: install parity-publish
run: cargo install [email protected].2 --locked -q
run: cargo install [email protected].3 --locked -q

- name: parity-publish check
run: parity-publish --color always check --allow-unpublished
2 changes: 1 addition & 1 deletion .github/workflows/publish-claim-crates.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
cache-on-failure: true

- name: install parity-publish
run: cargo install [email protected].2 --locked -q
run: cargo install [email protected].3 --locked -q

- name: parity-publish claim
env:
Expand Down
16 changes: 16 additions & 0 deletions prdoc/pr_6759.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
title: 'pallet-revive: Statically verify imports on code deployment'
doc:
- audience: Runtime Dev
description: |-
Previously, we failed at runtime if an unknown or unstable host function was called. This requires us to keep track of when a host function was added and when a code was deployed. We used the `api_version` to track at which API version each code was deployed. This made sure that when a new host function was added that old code won't have access to it. This is necessary as otherwise the behavior of a contract that made calls to this previously non existent host function would change from "trap" to "do something".

In this PR we remove the API version. Instead, we statically verify on upload that no non-existent host function is ever used in the code. This will allow us to add new host function later without needing to keep track when they were added.

This simplifies the code and also gives an immediate feedback if unknown host functions are used.
crates:
- name: pallet-revive-proc-macro
bump: major
- name: pallet-revive
bump: major
- name: pallet-revive-fixtures
bump: major
12 changes: 12 additions & 0 deletions prdoc/pr_6835.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
title: '[pallet-revive] implement the call data load API'
doc:
- audience: Runtime Dev
description: |-
This PR implements the call data load API akin to [how it works on ethereum](https://www.evm.codes/?fork=cancun#35).
crates:
- name: pallet-revive-fixtures
bump: minor
- name: pallet-revive
bump: minor
- name: pallet-revive-uapi
bump: minor
2 changes: 1 addition & 1 deletion substrate/frame/revive/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ Driven by the desire to have an iterative approach in developing new contract in
concept of an unstable interface. Akin to the rust nightly compiler it allows us to add new interfaces but mark them as
unstable so that contract languages can experiment with them and give feedback before we stabilize those.

In order to access interfaces which don't have a stable `#[api_version(x)]` in [`runtime.rs`](src/wasm/runtime.rs)
In order to access interfaces which don't have a stable `#[stable]` in [`runtime.rs`](src/wasm/runtime.rs)
one need to set `pallet_revive::Config::UnsafeUnstableInterface` to `ConstU32<true>`.
**It should be obvious that any production runtime should never be compiled with this feature: In addition to be
subject to change or removal those interfaces might not have proper weights associated with them and are therefore
Expand Down
44 changes: 44 additions & 0 deletions substrate/frame/revive/fixtures/contracts/call_data_load.rs
Original file line number Diff line number Diff line change
@@ -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 uses the call data load API to first the first input byte.
//! This single input byte is used as the offset for a second call
//! to the call data load API.
//! The output of the second API call is returned.
#![no_std]
#![no_main]

extern crate common;
use uapi::{HostFn, HostFnImpl as api, ReturnFlags};

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn deploy() {}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call() {
let mut buf = [0; 32];
api::call_data_load(&mut buf, 0);

let offset = buf[31] as u32;
let mut buf = [0; 32];
api::call_data_load(&mut buf, offset);

api::return_value(ReturnFlags::empty(), &buf);
}
44 changes: 44 additions & 0 deletions substrate/frame/revive/fixtures/contracts/unknown_syscall.rs
Original file line number Diff line number Diff line change
@@ -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.
#![no_std]
#![no_main]

extern crate common;

#[polkavm_derive::polkavm_import]
extern "C" {
pub fn __this_syscall_does_not_exist__();
}

// Export that is never called. We can put code here that should be in the binary
// but is never supposed to be run.
#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call_never() {
// make sure it is not optimized away
unsafe {
__this_syscall_does_not_exist__();
}
}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn deploy() {}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call() {}
44 changes: 44 additions & 0 deletions substrate/frame/revive/fixtures/contracts/unstable_interface.rs
Original file line number Diff line number Diff line change
@@ -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.
#![no_std]
#![no_main]

extern crate common;

#[polkavm_derive::polkavm_import]
extern "C" {
pub fn set_code_hash();
}

// Export that is never called. We can put code here that should be in the binary
// but is never supposed to be run.
#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call_never() {
// make sure it is not optimized away
unsafe {
set_code_hash();
}
}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn deploy() {}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call() {}
72 changes: 37 additions & 35 deletions substrate/frame/revive/proc-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ struct EnvDef {
/// Parsed host function definition.
struct HostFn {
item: syn::ItemFn,
api_version: Option<u16>,
is_stable: bool,
name: String,
returns: HostFnReturn,
cfg: Option<syn::Attribute>,
Expand Down Expand Up @@ -183,22 +183,21 @@ impl HostFn {
};

// process attributes
let msg = "Only #[api_version(<u16>)], #[cfg] and #[mutating] attributes are allowed.";
let msg = "Only #[stable], #[cfg] and #[mutating] attributes are allowed.";
let span = item.span();
let mut attrs = item.attrs.clone();
attrs.retain(|a| !a.path().is_ident("doc"));
let mut api_version = None;
let mut is_stable = false;
let mut mutating = false;
let mut cfg = None;
while let Some(attr) = attrs.pop() {
let ident = attr.path().get_ident().ok_or(err(span, msg))?.to_string();
match ident.as_str() {
"api_version" => {
if api_version.is_some() {
return Err(err(span, "#[api_version] can only be specified once"))
"stable" => {
if is_stable {
return Err(err(span, "#[stable] can only be specified once"))
}
api_version =
Some(attr.parse_args::<syn::LitInt>().and_then(|lit| lit.base10_parse())?);
is_stable = true;
},
"mutating" => {
if mutating {
Expand Down Expand Up @@ -313,7 +312,7 @@ impl HostFn {
_ => Err(err(arg1.span(), &msg)),
}?;

Ok(Self { item, api_version, name, returns, cfg })
Ok(Self { item, is_stable, name, returns, cfg })
},
_ => Err(err(span, &msg)),
}
Expand Down Expand Up @@ -411,19 +410,23 @@ fn expand_env(def: &EnvDef) -> TokenStream2 {
let impls = expand_functions(def);
let bench_impls = expand_bench_functions(def);
let docs = expand_func_doc(def);
let highest_api_version =
def.host_funcs.iter().filter_map(|f| f.api_version).max().unwrap_or_default();
let stable_syscalls = expand_func_list(def, false);
let all_syscalls = expand_func_list(def, true);

quote! {
#[cfg(test)]
pub const HIGHEST_API_VERSION: u16 = #highest_api_version;
pub fn list_syscalls(include_unstable: bool) -> &'static [&'static [u8]] {
if include_unstable {
#all_syscalls
} else {
#stable_syscalls
}
}

impl<'a, E: Ext, M: PolkaVmInstance<E::T>> Runtime<'a, E, M> {
fn handle_ecall(
&mut self,
memory: &mut M,
__syscall_symbol__: &[u8],
__available_api_version__: ApiVersion,
) -> Result<Option<u64>, TrapReason>
{
#impls
Expand Down Expand Up @@ -474,10 +477,6 @@ fn expand_functions(def: &EnvDef) -> TokenStream2 {
let body = &f.item.block;
let map_output = f.returns.map_output();
let output = &f.item.sig.output;
let api_version = match f.api_version {
Some(version) => quote! { Some(#version) },
None => quote! { None },
};

// wrapped host function body call with host function traces
// see https://github.com/paritytech/polkadot-sdk/tree/master/substrate/frame/contracts#host-function-tracing
Expand Down Expand Up @@ -513,7 +512,7 @@ fn expand_functions(def: &EnvDef) -> TokenStream2 {

quote! {
#cfg
#syscall_symbol if __is_available__(#api_version) => {
#syscall_symbol => {
// closure is needed so that "?" can infere the correct type
(|| #output {
#arg_decoder
Expand All @@ -534,18 +533,6 @@ fn expand_functions(def: &EnvDef) -> TokenStream2 {
// This is the overhead to call an empty syscall that always needs to be charged.
self.charge_gas(crate::wasm::RuntimeCosts::HostFn).map_err(TrapReason::from)?;

// Not all APIs are available depending on configuration or when the code was deployed.
// This closure will be used by syscall specific code to perform this check.
let __is_available__ = |syscall_version: Option<u16>| {
match __available_api_version__ {
ApiVersion::UnsafeNewest => true,
ApiVersion::Versioned(max_available_version) =>
syscall_version
.map(|required_version| max_available_version >= required_version)
.unwrap_or(false),
}
};

// They will be mapped to variable names by the syscall specific code.
let (__a0__, __a1__, __a2__, __a3__, __a4__, __a5__) = memory.read_input_regs();

Expand Down Expand Up @@ -607,10 +594,8 @@ fn expand_func_doc(def: &EnvDef) -> TokenStream2 {
});
quote! { #( #docs )* }
};
let availability = if let Some(version) = func.api_version {
let info = format!(
"\n# Required API version\nThis API was added in version **{version}**.",
);
let availability = if func.is_stable {
let info = "\n# Stable API\nThis API is stable and will never change.";
quote! { #[doc = #info] }
} else {
let info =
Expand All @@ -632,3 +617,20 @@ fn expand_func_doc(def: &EnvDef) -> TokenStream2 {
#( #docs )*
}
}

fn expand_func_list(def: &EnvDef, include_unstable: bool) -> TokenStream2 {
let docs = def.host_funcs.iter().filter(|f| include_unstable || f.is_stable).map(|f| {
let name = Literal::byte_string(f.name.as_bytes());
quote! {
#name.as_slice()
}
});
let len = docs.clone().count();

quote! {
{
static FUNCS: [&[u8]; #len] = [#(#docs),*];
FUNCS.as_slice()
}
}
}
10 changes: 2 additions & 8 deletions substrate/frame/revive/src/benchmarking/call_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{
exec::{ExportedFunction, Ext, Key, Stack},
storage::meter::Meter,
transient_storage::MeterEntry,
wasm::{ApiVersion, PreparedCall, Runtime},
wasm::{PreparedCall, Runtime},
BalanceOf, Config, DebugBuffer, Error, GasMeter, MomentOf, Origin, WasmBlob, Weight,
};
use alloc::{vec, vec::Vec};
Expand Down Expand Up @@ -164,13 +164,7 @@ where
module: WasmBlob<T>,
input: Vec<u8>,
) -> PreparedCall<'a, StackExt<'a, T>> {
module
.prepare_call(
Runtime::new(ext, input),
ExportedFunction::Call,
ApiVersion::UnsafeNewest,
)
.unwrap()
module.prepare_call(Runtime::new(ext, input), ExportedFunction::Call).unwrap()
}

/// Add transient_storage
Expand Down
Loading

0 comments on commit fd071d4

Please sign in to comment.