Contracts: Fix legacy uapi (#3994)

Fix some broken legacy definitions of pallet_contracts_uapi storage host
functions

substrate/frame/contracts/fixtures/contracts/common/src/lib.rs

@@ -149,3 +149,13 @@ macro_rules! output {
 		$host_fn($($arg,)* $output);
 	};
 }
+
+/// Similar to `output!` but unwraps the result.
+#[macro_export]
+macro_rules! unwrap_output {
+	($output: ident, $buffer: expr, $host_fn:path, $($arg:expr),*) => {
+		let mut $output = $buffer;
+		let $output = &mut &mut $output[..];
+		$host_fn($($arg,)* $output).unwrap();
+	};
+}

substrate/frame/contracts/fixtures/contracts/storage.rs

@@ -0,0 +1,63 @@
+// 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 contract tests the storage APIs. It sets and clears storage values using the different
+//! versions of the storage APIs.
+#![no_std]
+#![no_main]
+
+use common::unwrap_output;
+use uapi::{HostFn, HostFnImpl as api};
+
+#[no_mangle]
+#[polkavm_derive::polkavm_export]
+pub extern "C" fn deploy() {}
+
+#[no_mangle]
+#[polkavm_derive::polkavm_export]
+pub extern "C" fn call() {
+	const KEY: [u8; 32] = [1u8; 32];
+	const VALUE_1: [u8; 4] = [1u8; 4];
+	const VALUE_2: [u8; 4] = [2u8; 4];
+	const VALUE_3: [u8; 4] = [3u8; 4];
+
+	api::set_storage(&KEY, &VALUE_1);
+	assert_eq!(api::contains_storage(&KEY), Some(VALUE_1.len() as _));
+	unwrap_output!(val, [0u8; 4], api::get_storage, &KEY);
+	assert_eq!(**val, VALUE_1);
+
+	let existing = api::set_storage_v1(&KEY, &VALUE_2);
+	assert_eq!(existing, Some(VALUE_1.len() as _));
+	unwrap_output!(val, [0u8; 4], api::get_storage, &KEY);
+	assert_eq!(**val, VALUE_2);
+
+	api::clear_storage(&KEY);
+	assert_eq!(api::contains_storage(&KEY), None);
+
+	let existing = api::set_storage_v2(&KEY, &VALUE_3);
+	assert_eq!(existing, None);
+	assert_eq!(api::contains_storage_v1(&KEY), Some(VALUE_1.len() as _));
+	unwrap_output!(val, [0u8; 32], api::get_storage_v1, &KEY);
+	assert_eq!(**val, VALUE_3);
+
+	api::clear_storage_v1(&KEY);
+	assert_eq!(api::contains_storage_v1(&KEY), None);
+	let existing = api::set_storage_v2(&KEY, &VALUE_3);
+	assert_eq!(existing, None);
+	unwrap_output!(val, [0u8; 32], api::take_storage, &KEY);
+	assert_eq!(**val, VALUE_3);
+}

substrate/frame/contracts/src/tests.rs

@@ -913,6 +913,21 @@ fn instantiate_unique_trie_id() {
 	});
 }
 
+#[test]
+fn storage_work() {
+	let (code, _code_hash) = compile_module::<Test>("storage").unwrap();
+
+	ExtBuilder::default().build().execute_with(|| {
+		let _ = <Test as Config>::Currency::set_balance(&ALICE, 1_000_000);
+		let min_balance = Contracts::min_balance();
+		let addr = builder::bare_instantiate(Code::Upload(code))
+			.value(min_balance * 100)
+			.build_and_unwrap_account_id();
+
+		builder::bare_call(addr).build_and_unwrap_result();
+	});
+}
+
 #[test]
 fn storage_max_value_limit() {
 	let (wasm, _code_hash) = compile_module::<Test>("storage_size").unwrap();
@@ -1385,19 +1400,7 @@ fn crypto_hashes() {
 			// We offset data in the contract tables by 1.
 			let mut params = vec![(n + 1) as u8];
 			params.extend_from_slice(input);
-			let result = <Pallet<Test>>::bare_call(
-				ALICE,
-				addr.clone(),
-				0,
-				GAS_LIMIT,
-				None,
-				params,
-				DebugInfo::Skip,
-				CollectEvents::Skip,
-				Determinism::Enforced,
-			)
-			.result
-			.unwrap();
+			let result = builder::bare_call(addr.clone()).data(params).build_and_unwrap_result();
 			assert!(!result.did_revert());
 			let expected = hash_fn(input.as_ref());
 			assert_eq!(&result.data[..*expected_size], &*expected);
@@ -2277,19 +2280,7 @@ fn ecdsa_recover() {
 		params.extend_from_slice(&signature);
 		params.extend_from_slice(&message_hash);
 		assert!(params.len() == 65 + 32);
-		let result = <Pallet<Test>>::bare_call(
-			ALICE,
-			addr.clone(),
-			0,
-			GAS_LIMIT,
-			None,
-			params,
-			DebugInfo::Skip,
-			CollectEvents::Skip,
-			Determinism::Enforced,
-		)
-		.result
-		.unwrap();
+		let result = builder::bare_call(addr.clone()).data(params).build_and_unwrap_result();
 		assert!(!result.did_revert());
 		assert_eq!(result.data, EXPECTED_COMPRESSED_PUBLIC_KEY);
 	})
@@ -2404,19 +2395,7 @@ fn sr25519_verify() {
 			params.extend_from_slice(&public_key);
 			params.extend_from_slice(message);
 
-			<Pallet<Test>>::bare_call(
-				ALICE,
-				addr.clone(),
-				0,
-				GAS_LIMIT,
-				None,
-				params,
-				DebugInfo::Skip,
-				CollectEvents::Skip,
-				Determinism::Enforced,
-			)
-			.result
-			.unwrap()
+			builder::bare_call(addr.clone()).data(params).build_and_unwrap_result()
 		};
 
 		// verification should succeed for "hello world"
@@ -3692,35 +3671,13 @@ fn cannot_instantiate_indeterministic_code() {
 
 		// Try to instantiate `code_hash` from another contract in deterministic mode
 		assert_err!(
-			<Pallet<Test>>::bare_call(
-				ALICE,
-				addr.clone(),
-				0,
-				GAS_LIMIT,
-				None,
-				code_hash.encode(),
-				DebugInfo::Skip,
-				CollectEvents::Skip,
-				Determinism::Enforced,
-			)
-			.result,
+			builder::bare_call(addr.clone()).data(code_hash.encode()).build().result,
 			<Error<Test>>::Indeterministic,
 		);
 
 		// Instantiations are not allowed even in non-determinism mode
 		assert_err!(
-			<Pallet<Test>>::bare_call(
-				ALICE,
-				addr.clone(),
-				0,
-				GAS_LIMIT,
-				None,
-				code_hash.encode(),
-				DebugInfo::Skip,
-				CollectEvents::Skip,
-				Determinism::Relaxed,
-			)
-			.result,
+			builder::bare_call(addr.clone()).data(code_hash.encode()).build().result,
 			<Error<Test>>::Indeterministic,
 		);
 	});
@@ -3747,18 +3704,7 @@ fn cannot_set_code_indeterministic_code() {
 
 		// We do not allow to set the code hash to a non-deterministic wasm
 		assert_err!(
-			<Pallet<Test>>::bare_call(
-				ALICE,
-				caller_addr.clone(),
-				0,
-				GAS_LIMIT,
-				None,
-				code_hash.encode(),
-				DebugInfo::Skip,
-				CollectEvents::Skip,
-				Determinism::Relaxed,
-			)
-			.result,
+			builder::bare_call(caller_addr.clone()).data(code_hash.encode()).build().result,
 			<Error<Test>>::Indeterministic,
 		);
 	});
@@ -3785,35 +3731,17 @@ fn delegate_call_indeterministic_code() {
 
 		// The delegate call will fail in deterministic mode
 		assert_err!(
-			<Pallet<Test>>::bare_call(
-				ALICE,
-				caller_addr.clone(),
-				0,
-				GAS_LIMIT,
-				None,
-				code_hash.encode(),
-				DebugInfo::Skip,
-				CollectEvents::Skip,
-				Determinism::Enforced,
-			)
-			.result,
+			builder::bare_call(caller_addr.clone()).data(code_hash.encode()).build().result,
 			<Error<Test>>::Indeterministic,
 		);
 
 		// The delegate call will work on non-deterministic mode
 		assert_ok!(
-			<Pallet<Test>>::bare_call(
-				ALICE,
-				caller_addr.clone(),
-				0,
-				GAS_LIMIT,
-				None,
-				code_hash.encode(),
-				DebugInfo::Skip,
-				CollectEvents::Skip,
-				Determinism::Relaxed,
-			)
-			.result
+			builder::bare_call(caller_addr.clone())
+				.data(code_hash.encode())
+				.determinism(Determinism::Relaxed)
+				.build()
+				.result
 		);
 	});
 }
@@ -3845,19 +3773,8 @@ fn locking_delegate_dependency_works() {
 
 	// Call contract with the given input.
 	let call = |addr_caller: &AccountId32, input: &(u32, H256)| {
-		<Pallet<Test>>::bare_call(
-			ALICE,
-			addr_caller.clone(),
-			0,
-			GAS_LIMIT,
-			None,
-			input.encode(),
-			DebugInfo::UnsafeDebug,
-			CollectEvents::Skip,
-			Determinism::Enforced,
-		)
+		builder::bare_call(addr_caller.clone()).data(input.encode()).build()
 	};
-
 	const ED: u64 = 2000;
 	ExtBuilder::default().existential_deposit(ED).build().execute_with(|| {
 		let _ = Balances::set_balance(&ALICE, 1_000_000);
@@ -4023,19 +3940,9 @@ fn native_dependency_deposit_works() {
 			assert_eq!(res.storage_deposit.charge_or_zero(), deposit);
 
 			// call set_code_hash
-			<Pallet<Test>>::bare_call(
-				ALICE,
-				addr.clone(),
-				0,
-				GAS_LIMIT,
-				None,
-				dummy_code_hash.encode(),
-				DebugInfo::Skip,
-				CollectEvents::Skip,
-				Determinism::Enforced,
-			)
-			.result
-			.unwrap();
+			builder::bare_call(addr.clone())
+				.data(dummy_code_hash.encode())
+				.build_and_unwrap_result();
 
 			// Check updated storage_deposit
 			let code_deposit = test_utils::get_code_deposit(&dummy_code_hash);

substrate/frame/contracts/src/wasm/mod.rs

@@ -243,9 +243,10 @@ impl<T: Config> WasmBlob<T> {
 			.define("env", "memory", memory)
 			.expect("We just created the Linker. It has no definitions with this name; qed");
 
-		let instance = linker
-			.instantiate(&mut store, &contract.module)
-			.map_err(|_| "can't instantiate module with provided definitions")?;
+		let instance = linker.instantiate(&mut store, &contract.module).map_err(|err| {
+			log::debug!(target: LOG_TARGET, "failed to instantiate module: {:?}", err);
+			"can't instantiate module with provided definitions"
+		})?;
 
 		Ok((store, memory, instance))
 	}

substrate/frame/contracts/uapi/src/host/wasm32.rs

@@ -59,7 +59,7 @@ mod sys {
 
 		pub fn caller_is_root() -> ReturnCode;
 
-		pub fn clear_storage(key_ptr: *const u8, key_len: u32) -> ReturnCode;
+		pub fn clear_storage(key_ptr: *const u8);
 
 		pub fn code_hash(
 			account_id_ptr: *const u8,
@@ -67,7 +67,7 @@ mod sys {
 			output_len_ptr: *mut u32,
 		) -> ReturnCode;
 
-		pub fn contains_storage(key_ptr: *const u8, key_len: u32) -> ReturnCode;
+		pub fn contains_storage(key_ptr: *const u8) -> ReturnCode;
 
 		pub fn debug_message(str_ptr: *const u8, str_len: u32) -> ReturnCode;
 
@@ -599,7 +599,7 @@ impl HostFn for HostFnImpl {
 	}
 
 	fn clear_storage(key: &[u8]) {
-		unsafe { sys::clear_storage(key.as_ptr(), key.len() as u32) };
+		unsafe { sys::clear_storage(key.as_ptr()) };
 	}
 
 	fn clear_storage_v1(key: &[u8]) -> Option<u32> {
@@ -656,7 +656,7 @@ impl HostFn for HostFnImpl {
 	}
 
 	fn contains_storage(key: &[u8]) -> Option<u32> {
-		let ret_code = unsafe { sys::contains_storage(key.as_ptr(), key.len() as u32) };
+		let ret_code = unsafe { sys::contains_storage(key.as_ptr()) };
 		ret_code.into()
 	}