[pallet-revive] refactor uapi with better types (#5555)

start using better type for address, code_hash, and salt in runtime and the uapi crate fix #5575
paritytech · Sep 5, 2024 · cf330cc · cf330cc
1 parent d5346e7
commit cf330cc
Show file tree

Hide file tree

Showing 35 changed files with 211 additions and 201 deletions.
diff --git a/prdoc/pr_5555.prdoc b/prdoc/pr_5555.prdoc
@@ -0,0 +1,15 @@
+title: Make salt optional
+
+doc:
+  - audience: Runtime Dev
+    description: |
+     Remove address_len and salt_len from uapi as both are now fixed size
+
+crates:
+  - name: pallet-revive
+    bump: patch
+  - name: pallet-revive-uapi
+    bump: patch
+  - name: pallet-revive-fixtures
+    bump: patch
+
diff --git a/substrate/frame/revive/fixtures/build.rs b/substrate/frame/revive/fixtures/build.rs
@@ -159,6 +159,7 @@ mod build {
 	fn post_process(input_path: &Path, output_path: &Path) -> Result<()> {
 		let mut config = polkavm_linker::Config::default();
 		config.set_strip(true);
+		config.set_optimize(false);
 		let orig =
 			fs::read(input_path).with_context(|| format!("Failed to read {:?}", input_path))?;
 		let linked = polkavm_linker::program_from_elf(config, orig.as_ref())

diff --git a/substrate/frame/revive/fixtures/contracts/call.rs b/substrate/frame/revive/fixtures/contracts/call.rs
@@ -31,7 +31,7 @@ pub extern "C" fn deploy() {}
 pub extern "C" fn call() {
 	input!(
 		callee_input: [u8; 4],
-		callee_addr: [u8; 20],
+		callee_addr: &[u8; 20],
 	);
 
 	// Call the callee

diff --git a/substrate/frame/revive/fixtures/contracts/call_return_code.rs b/substrate/frame/revive/fixtures/contracts/call_return_code.rs
@@ -33,7 +33,7 @@ pub extern "C" fn deploy() {}
 pub extern "C" fn call() {
 	input!(
 		100,
-		callee_addr: [u8; 20],
+		callee_addr: &[u8; 20],
 		input: [u8],
 	);
 

diff --git a/substrate/frame/revive/fixtures/contracts/call_runtime_and_call.rs b/substrate/frame/revive/fixtures/contracts/call_runtime_and_call.rs
@@ -31,7 +31,7 @@ pub extern "C" fn call() {
 	input!(
 		512,
 		callee_input: [u8; 4],
-		callee_addr: [u8; 20],
+		callee_addr: &[u8; 20],
 		call: [u8],
 	);
 

diff --git a/substrate/frame/revive/fixtures/contracts/call_with_flags_and_value.rs b/substrate/frame/revive/fixtures/contracts/call_with_flags_and_value.rs
@@ -31,7 +31,7 @@ pub extern "C" fn deploy() {}
 pub extern "C" fn call() {
 	input!(
 		256,
-		callee_addr: [u8; 20],
+		callee_addr: &[u8; 20],
 		flags: u32,
 		value: u64,
 		forwarded_input: [u8],

diff --git a/substrate/frame/revive/fixtures/contracts/call_with_limit.rs b/substrate/frame/revive/fixtures/contracts/call_with_limit.rs
@@ -32,7 +32,7 @@ pub extern "C" fn deploy() {}
 pub extern "C" fn call() {
 	input!(
 		256,
-		callee_addr: [u8; 20],
+		callee_addr: &[u8; 20],
 		ref_time: u64,
 		proof_size: u64,
 		forwarded_input: [u8],

diff --git a/substrate/frame/revive/fixtures/contracts/caller_contract.rs b/substrate/frame/revive/fixtures/contracts/caller_contract.rs
@@ -28,7 +28,7 @@ pub extern "C" fn deploy() {}
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn call() {
-	input!(code_hash: [u8; 32],);
+	input!(code_hash: &[u8; 32],);
 
 	// The value to transfer on instantiation and calls. Chosen to be greater than existential
 	// deposit.
@@ -73,7 +73,6 @@ pub extern "C" fn call() {
 
 	// Deploy the contract successfully.
 	let mut callee = [0u8; 20];
-	let callee = &mut &mut callee[..];
 
 	api::instantiate(
 		code_hash,
@@ -82,17 +81,16 @@ pub extern "C" fn call() {
 		None, // No deposit limit.
 		&value,
 		&input,
-		Some(callee),
+		Some(&mut callee),
 		None,
 		&salt,
 	)
 	.unwrap();
-	assert_eq!(callee.len(), 20);
 
 	// Call the new contract and expect it to return failing exit code.
 	let res = api::call(
 		uapi::CallFlags::empty(),
-		callee,
+		&callee,
 		0u64, // How much ref_time weight to devote for the execution. 0 = all.
 		0u64, // How much proof_size weight to devote for the execution. 0 = all.
 		None, // No deposit limit.
@@ -105,7 +103,7 @@ pub extern "C" fn call() {
 	// Fail to call the contract due to insufficient ref_time weight.
 	let res = api::call(
 		uapi::CallFlags::empty(),
-		callee,
+		&callee,
 		1u64, // Too little ref_time weight.
 		0u64, // How much proof_size weight to devote for the execution. 0 = all.
 		None, // No deposit limit.
@@ -118,7 +116,7 @@ pub extern "C" fn call() {
 	// Fail to call the contract due to insufficient proof_size weight.
 	let res = api::call(
 		uapi::CallFlags::empty(),
-		callee,
+		&callee,
 		0u64, // How much ref_time weight to devote for the execution. 0 = all.
 		1u64, // too little proof_size weight
 		None, // No deposit limit.
@@ -132,7 +130,7 @@ pub extern "C" fn call() {
 	let mut output = [0u8; 4];
 	api::call(
 		uapi::CallFlags::empty(),
-		callee,
+		&callee,
 		0u64, // How much ref_time weight to devote for the execution. 0 = all.
 		0u64, // How much proof_size weight to devote for the execution. 0 = all.
 		None, // No deposit limit.

diff --git a/substrate/frame/revive/fixtures/contracts/chain_extension_temp_storage.rs b/substrate/frame/revive/fixtures/contracts/chain_extension_temp_storage.rs
@@ -20,7 +20,7 @@
 #![no_std]
 #![no_main]
 
-use common::{input, output};
+use common::input;
 use uapi::{HostFn, HostFnImpl as api};
 
 #[no_mangle]
@@ -47,12 +47,13 @@ pub extern "C" fn call() {
 		input[8] = 1u8;
 
 		// Read the contract address.
-		output!(addr, [0u8; 32], api::address,);
+		let mut addr = [0u8; 20];
+		api::address(&mut addr);
 
 		// call self
 		api::call(
 			uapi::CallFlags::ALLOW_REENTRY,
-			addr,
+			&addr,
 			0u64,                // How much ref_time to devote for the execution. 0 = all.
 			0u64,                // How much proof_size to devote for the execution. 0 = all.
 			None,                // No deposit limit.

diff --git a/substrate/frame/revive/fixtures/contracts/common/src/lib.rs b/substrate/frame/revive/fixtures/contracts/common/src/lib.rs
@@ -100,11 +100,23 @@ macro_rules! input {
 		input!(@inner $input, $cursor + $n, $($rest)*);
 	};
 
+	// Match an array reference of the given size.
+	// e.g input!(var1: &[u8; 32], );
+	(@inner $input:expr, $cursor:expr, $var:ident: &[u8; $n:expr], $($rest:tt)*) => {
+		let $var: &[u8; $n] = &$input[$cursor..$cursor+$n].try_into().unwrap();
+		input!(@inner $input, $cursor + $n, $($rest)*);
+	};
+
 	// Size of a u8 slice.
 	(@size $size:expr, $var:ident: [u8; $n:expr], $($rest:tt)*) => {
 		input!(@size $size + $n, $($rest)*)
 	};
 
+	// Size of an array reference.
+	(@size $size:expr, $var:ident: &[u8; $n:expr], $($rest:tt)*) => {
+		input!(@size $size + $n, $($rest)*)
+	};
+
 	// Entry point, with the buffer and it's size specified first.
 	// e.g input!(buffer, 512, var1: u32, var2: [u8], );
 	($buffer:ident, $size:expr, $($rest:tt)*) => {

diff --git a/substrate/frame/revive/fixtures/contracts/create_storage_and_call.rs b/substrate/frame/revive/fixtures/contracts/create_storage_and_call.rs
@@ -32,7 +32,7 @@ pub extern "C" fn call() {
 	input!(
 		buffer,
 		input: [u8; 4],
-		callee: [u8; 20],
+		callee: &[u8; 20],
 		deposit_limit: [u8; 8],
 	);
 

diff --git a/substrate/frame/revive/fixtures/contracts/create_storage_and_instantiate.rs b/substrate/frame/revive/fixtures/contracts/create_storage_and_instantiate.rs
@@ -31,14 +31,13 @@ pub extern "C" fn deploy() {}
 pub extern "C" fn call() {
 	input!(
 		input: [u8; 4],
-		code_hash: [u8; 32],
+		code_hash: &[u8; 32],
 		deposit_limit: [u8; 8],
 	);
 
 	let value = 10_000u64.to_le_bytes();
 	let salt = [0u8; 32];
 	let mut address = [0u8; 20];
-	let address = &mut &mut address[..];
 
 	api::instantiate(
 		code_hash,
@@ -47,12 +46,12 @@ pub extern "C" fn call() {
 		Some(deposit_limit),
 		&value,
 		input,
-		Some(address),
+		Some(&mut address),
 		None,
 		&salt,
 	)
 	.unwrap();
 
 	// Return the deployed contract address.
-	api::return_value(uapi::ReturnFlags::empty(), address);
+	api::return_value(uapi::ReturnFlags::empty(), &address);
 }
diff --git a/substrate/frame/revive/fixtures/contracts/create_transient_storage_and_call.rs b/substrate/frame/revive/fixtures/contracts/create_transient_storage_and_call.rs
@@ -35,7 +35,7 @@ pub extern "C" fn call() {
 		buffer,
 		len: u32,
 		input: [u8; 4],
-		callee: [u8; 20],
+		callee: &[u8; 20],
 	);
 
 	let rounds = len as usize / BUFFER.len();

diff --git a/substrate/frame/revive/fixtures/contracts/delegate_call.rs b/substrate/frame/revive/fixtures/contracts/delegate_call.rs
@@ -28,7 +28,7 @@ pub extern "C" fn deploy() {}
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn call() {
-	input!(code_hash: [u8; 32],);
+	input!(code_hash: &[u8; 32],);
 
 	let mut key = [0u8; 32];
 	key[0] = 1u8;

diff --git a/substrate/frame/revive/fixtures/contracts/delegate_call_lib.rs b/substrate/frame/revive/fixtures/contracts/delegate_call_lib.rs
@@ -44,6 +44,7 @@ pub extern "C" fn call() {
 	assert_eq!(value_transferred, 1337);
 
 	// Assert that ALICE is the caller of the contract.
-	output!(caller, [0u8; 20], api::caller,);
-	assert_eq!(&caller[..], &[1u8; 20]);
+	let mut caller = [0u8; 20];
+	api::caller(&mut caller);
+	assert_eq!(caller, [1u8; 20]);
 }
diff --git a/substrate/frame/revive/fixtures/contracts/delegate_call_simple.rs b/substrate/frame/revive/fixtures/contracts/delegate_call_simple.rs
@@ -28,7 +28,7 @@ pub extern "C" fn deploy() {}
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn call() {
-	input!(code_hash: [u8; 32],);
+	input!(code_hash: &[u8; 32],);
 
 	// Delegate call into passed code hash.
 	let input = [0u8; 0];

diff --git a/substrate/frame/revive/fixtures/contracts/destroy_and_transfer.rs b/substrate/frame/revive/fixtures/contracts/destroy_and_transfer.rs
@@ -27,11 +27,10 @@ const VALUE: [u8; 8] = [0, 0, 1u8, 0, 0, 0, 0, 0];
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn deploy() {
-	input!(code_hash: [u8; 32],);
+	input!(code_hash: &[u8; 32],);
 
 	let input = [0u8; 0];
 	let mut address = [0u8; 20];
-	let address = &mut &mut address[..];
 	let salt = [47u8; 32];
 
 	api::instantiate(
@@ -41,27 +40,28 @@ pub extern "C" fn deploy() {
 		None, // No deposit limit.
 		&VALUE,
 		&input,
-		Some(address),
+		Some(&mut address),
 		None,
 		&salt,
 	)
 	.unwrap();
 
 	// Return the deployed contract address.
-	api::set_storage(StorageFlags::empty(), &ADDRESS_KEY, address);
+	api::set_storage(StorageFlags::empty(), &ADDRESS_KEY, &address);
 }
 
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn call() {
 	let mut callee_addr = [0u8; 20];
-	let callee_addr = &mut &mut callee_addr[..];
-	api::get_storage(StorageFlags::empty(), &ADDRESS_KEY, callee_addr).unwrap();
+	let callee = &mut &mut callee_addr[..];
+	api::get_storage(StorageFlags::empty(), &ADDRESS_KEY, callee).unwrap();
+	assert!(callee.len() == 20);
 
 	// Calling the destination contract with non-empty input data should fail.
 	let res = api::call(
 		uapi::CallFlags::empty(),
-		callee_addr,
+		&callee_addr,
 		0u64, // How much ref_time weight to devote for the execution. 0 = all.
 		0u64, // How much proof_size weight to devote for the execution. 0 = all.
 		None, // No deposit limit.
@@ -74,7 +74,7 @@ pub extern "C" fn call() {
 	// Call the destination contract regularly, forcing it to self-destruct.
 	api::call(
 		uapi::CallFlags::empty(),
-		callee_addr,
+		&callee_addr,
 		0u64, // How much ref_time weight to devote for the execution. 0 = all.
 		0u64, // How much proof_size weight to devote for the execution. 0 = all.
 		None, // No deposit limit.

diff --git a/substrate/frame/revive/fixtures/contracts/drain.rs b/substrate/frame/revive/fixtures/contracts/drain.rs
@@ -39,6 +39,6 @@ pub extern "C" fn call() {
 
 	// Try to self-destruct by sending more balance to the 0 address.
 	// The call will fail because a contract transfer has a keep alive requirement.
-	let res = api::transfer(&[0u8; 32], &balance.to_le_bytes());
+	let res = api::transfer(&[0u8; 20], &balance.to_le_bytes());
 	assert!(matches!(res, Err(uapi::ReturnErrorCode::TransferFailed)));
 }
diff --git a/substrate/frame/revive/fixtures/contracts/instantiate_return_code.rs b/substrate/frame/revive/fixtures/contracts/instantiate_return_code.rs
@@ -28,7 +28,7 @@ pub extern "C" fn deploy() {}
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn call() {
-	input!(buffer, 36, code_hash: [u8; 32],);
+	input!(buffer, 36, code_hash: &[u8; 32],);
 	let input = &buffer[32..];
 
 	let err_code = match api::instantiate(
@@ -41,7 +41,7 @@ pub extern "C" fn call() {
 		input,
 		None,
 		None,
-		&[0u8; 0], // Empty salt.
+		&[0u8; 32], // Salt.
 	) {
 		Ok(_) => 0u32,
 		Err(code) => code as u32,

diff --git a/substrate/frame/revive/fixtures/contracts/locking_delegate_dependency.rs b/substrate/frame/revive/fixtures/contracts/locking_delegate_dependency.rs
@@ -30,7 +30,7 @@ const ETH_ALICE: [u8; 20] = [1u8; 20];
 fn load_input(delegate_call: bool) {
 	input!(
 		action: u32,
-		code_hash: [u8; 32],
+		code_hash: &[u8; 32],
 	);
 
 	match action {

diff --git a/substrate/frame/revive/fixtures/contracts/read_only_call.rs b/substrate/frame/revive/fixtures/contracts/read_only_call.rs
@@ -31,7 +31,7 @@ pub extern "C" fn deploy() {}
 pub extern "C" fn call() {
 	input!(
 		256,
-		callee_addr: [u8; 20],
+		callee_addr: &[u8; 20],
 		callee_input: [u8],
 	);
 

diff --git a/substrate/frame/revive/fixtures/contracts/recurse.rs b/substrate/frame/revive/fixtures/contracts/recurse.rs
@@ -20,7 +20,7 @@
 #![no_std]
 #![no_main]
 
-use common::{input, output};
+use common::input;
 use uapi::{HostFn, HostFnImpl as api};
 
 #[no_mangle]
@@ -33,15 +33,16 @@ pub extern "C" fn call() {
 	input!(calls_left: u32, );
 
 	// own address
-	output!(addr, [0u8; 32], api::address,);
+	let mut addr = [0u8; 20];
+	api::address(&mut addr);
 
 	if calls_left == 0 {
 		return
 	}
 
 	api::call(
 		uapi::CallFlags::ALLOW_REENTRY,
-		addr,
+		&addr,
 		0u64,                // How much ref_time to devote for the execution. 0 = all.
 		0u64,                // How much deposit_limit to devote for the execution. 0 = all.
 		None,                // No deposit limit.