From 8d0aab812ebb70e39b0af893862ba204dc098860 Mon Sep 17 00:00:00 2001 From: PG Herveou Date: Thu, 12 Sep 2024 19:26:59 +0200 Subject: [PATCH] [pallet-revive] fix xcm tests (#5684) fix https://github.com/paritytech/polkadot-sdk/issues/5683 --- Cargo.lock | 1 + prdoc/pr_5684.prdoc | 19 +++++++ substrate/frame/revive/fixtures/Cargo.toml | 2 + substrate/frame/revive/fixtures/src/lib.rs | 1 + .../frame/revive/mock-network/Cargo.toml | 1 + .../frame/revive/mock-network/src/tests.rs | 55 +++++++++---------- substrate/frame/revive/src/wasm/runtime.rs | 7 ++- .../frame/revive/uapi/src/host/riscv32.rs | 9 ++- umbrella/Cargo.toml | 1 + 9 files changed, 64 insertions(+), 32 deletions(-) create mode 100644 prdoc/pr_5684.prdoc diff --git a/Cargo.lock b/Cargo.lock index e5b12a60da2a..5f0fa0f613f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11471,6 +11471,7 @@ version = "0.1.0" dependencies = [ "anyhow", "frame-system", + "log", "parity-wasm", "polkavm-linker 0.10.0", "sp-core", diff --git a/prdoc/pr_5684.prdoc b/prdoc/pr_5684.prdoc new file mode 100644 index 000000000000..a17bacd2fb94 --- /dev/null +++ b/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 diff --git a/substrate/frame/revive/fixtures/Cargo.toml b/substrate/frame/revive/fixtures/Cargo.toml index db284c7cc062..903298d2df21 100644 --- a/substrate/frame/revive/fixtures/Cargo.toml +++ b/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", diff --git a/substrate/frame/revive/fixtures/src/lib.rs b/substrate/frame/revive/fixtures/src/lib.rs index 54e32130635a..5548dca66d07 100644 --- a/substrate/frame/revive/fixtures/src/lib.rs +++ b/substrate/frame/revive/fixtures/src/lib.rs @@ -24,6 +24,7 @@ extern crate alloc; pub fn compile_module(fixture_name: &str) -> anyhow::Result<(Vec, 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))) diff --git a/substrate/frame/revive/mock-network/Cargo.toml b/substrate/frame/revive/mock-network/Cargo.toml index 0d597bbdc228..85656a57b49c 100644 --- a/substrate/frame/revive/mock-network/Cargo.toml +++ b/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", diff --git a/substrate/frame/revive/mock-network/src/tests.rs b/substrate/frame/revive/mock-network/src/tests.rs index 9259dd6f169e..bd05726a1a45 100644 --- a/substrate/frame/revive/mock-network/src/tests.rs +++ b/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 { +fn bare_call(dest: H160) -> BareCallBuilder { BareCallBuilder::::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::(name).unwrap(); +fn instantiate_test_contract(name: &str) -> Contract { + let (wasm, _) = compile_module(name).unwrap(); // Instantiate contract. - let contract_addr = ParaA::execute_with(|| { + let contract = ParaA::execute_with(|| { BareInstantiateBuilder::::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,7 +97,7 @@ 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); }); } @@ -106,7 +105,7 @@ fn test_xcm_execute() { 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,15 +123,13 @@ 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); }); } @@ -140,11 +137,11 @@ fn test_xcm_execute_incomplete() { 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::(), 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 = ¶chain_account_sovereign_account_id(1, contract_addr); + let derived_contract_addr = ¶chain_account_sovereign_account_id(1, account_id); assert_eq!( INITIAL_BALANCE - amount, relay_chain::Balances::free_balance(derived_contract_addr) diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index d9257d38b66e..02d60ee9e34d 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -1790,6 +1790,7 @@ pub mod env { &mut self, memory: &mut M, dest_ptr: u32, + dest_len: u32, msg_ptr: u32, msg_len: u32, output_ptr: u32, @@ -1797,10 +1798,12 @@ pub mod env { 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 = <::Xcm as SendController<_>>::WeightInfo::send(); self.charge_gas(RuntimeCosts::CallRuntime(weight))?; let origin = crate::RawOrigin::Signed(self.ext.account_id().clone()).into(); diff --git a/substrate/frame/revive/uapi/src/host/riscv32.rs b/substrate/frame/revive/uapi/src/host/riscv32.rs index b7b660c40837..89cf92786445 100644 --- a/substrate/frame/revive/uapi/src/host/riscv32.rs +++ b/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() } diff --git a/umbrella/Cargo.toml b/umbrella/Cargo.toml index 6d380a4bcbb7..b7c1c375094f 100644 --- a/umbrella/Cargo.toml +++ b/umbrella/Cargo.toml @@ -607,6 +607,7 @@ tuples-96 = [ ] riscv = [ "pallet-revive-fixtures?/riscv", + "pallet-revive-mock-network?/riscv", "pallet-revive?/riscv", ]