[pallet-revive] fix xcm tests

prdoc/pr_5684.prdoc

@@ -0,0 +1,19 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: "[pallet-revive]"
+
+doc:
+  - audience: Runtime Devs
+    description: |
+      Update xcm runtime api, and fix pallet-revive xcm tests
+
+crates:
+  - name: pallet-revive
+    bump: patch
+  - name: pallet-revive-fixtures
+    bump: patch
+  - name: pallet-revive-mock-network
+    bump: patch
+  - name: polkadot-sdk
+    bump: patch

substrate/frame/revive/fixtures/Cargo.toml

@@ -16,6 +16,7 @@ sp-core = { workspace = true, default-features = true, optional = true }
 sp-io = { workspace = true, default-features = true, optional = true }
 sp-runtime = { workspace = true, default-features = true, optional = true }
 anyhow = { workspace = true, default-features = true, optional = true }
+log = { workspace = true }
 
 [build-dependencies]
 parity-wasm = { workspace = true }
@@ -34,6 +35,7 @@ riscv = []
 std = [
 	"anyhow",
 	"frame-system",
+	"log/std",
 	"sp-core",
 	"sp-io",
 	"sp-runtime",

substrate/frame/revive/fixtures/src/lib.rs

@@ -24,6 +24,7 @@ extern crate alloc;
 pub fn compile_module(fixture_name: &str) -> anyhow::Result<(Vec<u8>, sp_core::H256)> {
 	let out_dir: std::path::PathBuf = env!("OUT_DIR").into();
 	let fixture_path = out_dir.join(format!("{fixture_name}.polkavm"));
+	log::debug!("Loading fixture from {fixture_path:?}");
 	let binary = std::fs::read(fixture_path)?;
 	let code_hash = sp_io::hashing::keccak_256(&binary);
 	Ok((binary, sp_core::H256(code_hash)))

substrate/frame/revive/mock-network/Cargo.toml

@@ -48,6 +48,7 @@ pallet-revive-fixtures = { workspace = true }
 
 [features]
 default = ["std"]
+riscv = ["pallet-revive-fixtures/riscv"]
 std = [
 	"codec/std",
 	"frame-support/std",

substrate/frame/revive/mock-network/src/tests.rs

@@ -16,10 +16,8 @@
 // limitations under the License.
 
 use crate::{
-	parachain::{self, Runtime},
-	parachain_account_sovereign_account_id,
-	primitives::{AccountId, CENTS},
-	relay_chain, MockNet, ParaA, ParachainBalances, Relay, ALICE, BOB, INITIAL_BALANCE,
+	parachain, parachain_account_sovereign_account_id, primitives::CENTS, relay_chain, MockNet,
+	ParaA, ParachainBalances, Relay, ALICE, BOB, INITIAL_BALANCE,
 };
 use codec::{Decode, Encode};
 use frame_support::traits::{fungibles::Mutate, Currency};
@@ -30,6 +28,7 @@ use pallet_revive::{
 };
 use pallet_revive_fixtures::compile_module;
 use pallet_revive_uapi::ReturnErrorCode;
+use sp_core::H160;
 use xcm::{v4::prelude::*, VersionedLocation, VersionedXcm};
 use xcm_simulator::TestExt;
 
@@ -39,41 +38,43 @@ macro_rules! assert_return_code {
 	}};
 }
 
-fn bare_call(dest: sp_runtime::AccountId32) -> BareCallBuilder<parachain::Runtime> {
+fn bare_call(dest: H160) -> BareCallBuilder<parachain::Runtime> {
 	BareCallBuilder::<parachain::Runtime>::bare_call(RawOrigin::Signed(ALICE).into(), dest)
 }
 
 /// Instantiate the tests contract, and fund it with some balance and assets.
-fn instantiate_test_contract(name: &str) -> AccountId {
-	let (wasm, _) = compile_module::<Runtime>(name).unwrap();
+fn instantiate_test_contract(name: &str) -> Contract<parachain::Runtime> {
+	let (wasm, _) = compile_module(name).unwrap();
 
 	// Instantiate contract.
-	let contract_addr = ParaA::execute_with(|| {
+	let contract = ParaA::execute_with(|| {
 		BareInstantiateBuilder::<parachain::Runtime>::bare_instantiate(
 			RawOrigin::Signed(ALICE).into(),
 			Code::Upload(wasm),
 		)
-		.build_and_unwrap_account_id()
+		.storage_deposit_limit(1_000_000_000_000)
+		.build_and_unwrap_contract()
 	});
 
 	// Funds contract account with some balance and assets.
 	ParaA::execute_with(|| {
-		parachain::Balances::make_free_balance_be(&contract_addr, INITIAL_BALANCE);
-		parachain::Assets::mint_into(0u32.into(), &contract_addr, INITIAL_BALANCE).unwrap();
+		parachain::Balances::make_free_balance_be(&contract.account_id, INITIAL_BALANCE);
+		parachain::Assets::mint_into(0u32.into(), &contract.account_id, INITIAL_BALANCE).unwrap();
 	});
 	Relay::execute_with(|| {
-		let sovereign_account = parachain_account_sovereign_account_id(1u32, contract_addr.clone());
+		let sovereign_account =
+			parachain_account_sovereign_account_id(1u32, contract.account_id.clone());
 		relay_chain::Balances::make_free_balance_be(&sovereign_account, INITIAL_BALANCE);
 	});
 
-	contract_addr
+	contract
 }
 
 #[test]
 fn test_xcm_execute() {
 	MockNet::reset();
 
-	let contract_addr = instantiate_test_contract("xcm_execute");
+	let Contract { addr, account_id } = instantiate_test_contract("xcm_execute");
 
 	// Execute XCM instructions through the contract.
 	ParaA::execute_with(|| {
@@ -87,9 +88,7 @@ fn test_xcm_execute() {
 			.deposit_asset(assets, beneficiary)
 			.build();
 
-		let result = bare_call(contract_addr.clone())
-			.data(VersionedXcm::V4(message).encode())
-			.build();
+		let result = bare_call(addr).data(VersionedXcm::V4(message).encode()).build();
 
 		assert_eq!(result.gas_consumed, result.gas_required);
 		assert_return_code!(&result.result.unwrap(), ReturnErrorCode::Success);
@@ -98,15 +97,15 @@ fn test_xcm_execute() {
 		// Bob.
 		let initial = INITIAL_BALANCE;
 		assert_eq!(ParachainBalances::free_balance(BOB), initial + amount);
-		assert_eq!(ParachainBalances::free_balance(&contract_addr), initial - amount);
+		assert_eq!(ParachainBalances::free_balance(&account_id), initial - amount);
 	});
 }
 
 #[test]
 fn test_xcm_execute_incomplete() {
 	MockNet::reset();
 
-	let contract_addr = instantiate_test_contract("xcm_execute");
+	let Contract { addr, account_id } = instantiate_test_contract("xcm_execute");
 	let amount = 10 * CENTS;
 
 	// Execute XCM instructions through the contract.
@@ -124,27 +123,25 @@ fn test_xcm_execute_incomplete() {
 			.deposit_asset(assets, beneficiary)
 			.build();
 
-		let result = bare_call(contract_addr.clone())
-			.data(VersionedXcm::V4(message).encode())
-			.build();
+		let result = bare_call(addr).data(VersionedXcm::V4(message).encode()).build();
 
 		assert_eq!(result.gas_consumed, result.gas_required);
 		assert_return_code!(&result.result.unwrap(), ReturnErrorCode::XcmExecutionFailed);
 
 		assert_eq!(ParachainBalances::free_balance(BOB), INITIAL_BALANCE);
-		assert_eq!(ParachainBalances::free_balance(&contract_addr), INITIAL_BALANCE - amount);
+		assert_eq!(ParachainBalances::free_balance(&account_id), INITIAL_BALANCE - amount);
 	});
 }
 
 #[test]
 fn test_xcm_execute_reentrant_call() {
 	MockNet::reset();
 
-	let contract_addr = instantiate_test_contract("xcm_execute");
+	let Contract { addr, .. } = instantiate_test_contract("xcm_execute");
 
 	ParaA::execute_with(|| {
 		let transact_call = parachain::RuntimeCall::Contracts(pallet_revive::Call::call {
-			dest: contract_addr.clone(),
+			dest: addr,
 			gas_limit: 1_000_000.into(),
 			storage_deposit_limit: test_utils::deposit_limit::<parachain::Runtime>(),
 			data: vec![],
@@ -157,7 +154,7 @@ fn test_xcm_execute_reentrant_call() {
 			.expect_transact_status(MaybeErrorCode::Success)
 			.build();
 
-		let result = bare_call(contract_addr.clone())
+		let result = bare_call(addr)
 			.data(VersionedXcm::V4(message).encode())
 			.build_and_unwrap_result();
 
@@ -171,7 +168,7 @@ fn test_xcm_execute_reentrant_call() {
 #[test]
 fn test_xcm_send() {
 	MockNet::reset();
-	let contract_addr = instantiate_test_contract("xcm_send");
+	let Contract { addr, account_id } = instantiate_test_contract("xcm_send");
 	let amount = 1_000 * CENTS;
 	let fee = parachain::estimate_message_fee(4); // Accounts for the `DescendOrigin` instruction added by `send_xcm`
 
@@ -188,7 +185,7 @@ fn test_xcm_send() {
 			.deposit_asset(assets, beneficiary)
 			.build();
 
-		let result = bare_call(contract_addr.clone())
+		let result = bare_call(addr)
 			.data((dest, VersionedXcm::V4(message)).encode())
 			.build_and_unwrap_result();
 
@@ -197,7 +194,7 @@ fn test_xcm_send() {
 	});
 
 	Relay::execute_with(|| {
-		let derived_contract_addr = &parachain_account_sovereign_account_id(1, contract_addr);
+		let derived_contract_addr = &parachain_account_sovereign_account_id(1, account_id);
 		assert_eq!(
 			INITIAL_BALANCE - amount,
 			relay_chain::Balances::free_balance(derived_contract_addr)

substrate/frame/revive/src/wasm/runtime.rs

@@ -1790,17 +1790,20 @@ pub mod env {
 		&mut self,
 		memory: &mut M,
 		dest_ptr: u32,
+		dest_len: u32,
 		msg_ptr: u32,
 		msg_len: u32,
 		output_ptr: u32,
 	) -> Result<ReturnErrorCode, TrapReason> {
 		use xcm::{VersionedLocation, VersionedXcm};
 		use xcm_builder::{SendController, SendControllerWeightInfo};
 
-		self.charge_gas(RuntimeCosts::CopyFromContract(msg_len))?;
-		let dest: VersionedLocation = memory.read_as(dest_ptr)?;
+		self.charge_gas(RuntimeCosts::CopyFromContract(dest_len))?;
+		let dest: VersionedLocation = memory.read_as_unbounded(dest_ptr, dest_len)?;
 
+		self.charge_gas(RuntimeCosts::CopyFromContract(msg_len))?;
 		let message: VersionedXcm<()> = memory.read_as_unbounded(msg_ptr, msg_len)?;
+
 		let weight = <<E::T as Config>::Xcm as SendController<_>>::WeightInfo::send();
 		self.charge_gas(RuntimeCosts::CallRuntime(weight))?;
 		let origin = crate::RawOrigin::Signed(self.ext.account_id().clone()).into();

substrate/frame/revive/uapi/src/host/riscv32.rs

@@ -124,6 +124,7 @@ mod sys {
 		pub fn xcm_execute(msg_ptr: *const u8, msg_len: u32) -> ReturnCode;
 		pub fn xcm_send(
 			dest_ptr: *const u8,
+			dest_len: *const u8,
 			msg_ptr: *const u8,
 			msg_len: u32,
 			out_ptr: *mut u8,
@@ -530,7 +531,13 @@ impl HostFn for HostFnImpl {
 
 	fn xcm_send(dest: &[u8], msg: &[u8], output: &mut [u8; 32]) -> Result {
 		let ret_code = unsafe {
-			sys::xcm_send(dest.as_ptr(), msg.as_ptr(), msg.len() as _, output.as_mut_ptr())
+			sys::xcm_send(
+				dest.as_ptr(),
+				dest.len() as _,
+				msg.as_ptr(),
+				msg.len() as _,
+				output.as_mut_ptr(),
+			)
 		};
 		ret_code.into()
 	}

umbrella/Cargo.toml

@@ -607,6 +607,7 @@ tuples-96 = [
 ]
 riscv = [
 	"pallet-revive-fixtures?/riscv",
+	"pallet-revive-mock-network?/riscv",
 	"pallet-revive?/riscv",
 ]