From 380b817f53c83652d1d35cc69fd5ff7f21f8c448 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Fri, 13 Oct 2023 11:56:16 +0200 Subject: [PATCH] fix nonce and divide api between submit now and later --- .../src/generic/cases/example.rs | 87 ++++++------ .../src/generic/environment.rs | 29 ++-- .../src/generic/envs/fudge_env.rs | 70 +++++++++- .../src/generic/envs/runtime_env.rs | 124 +++++++++++++++--- 4 files changed, 227 insertions(+), 83 deletions(-) diff --git a/runtime/integration-tests/src/generic/cases/example.rs b/runtime/integration-tests/src/generic/cases/example.rs index dd21f58e2b..e4e8fb7dac 100644 --- a/runtime/integration-tests/src/generic/cases/example.rs +++ b/runtime/integration-tests/src/generic/cases/example.rs @@ -33,14 +33,15 @@ fn transfer_balance() { ); // Call an extrinsic that would be processed immediately - env.submit( - Keyring::Alice, - pallet_balances::Call::transfer { - dest: Keyring::Bob.into(), - value: TRANSFER, - }, - ) - .unwrap(); + let fee = env + .submit_now( + Keyring::Alice, + pallet_balances::Call::transfer { + dest: Keyring::Bob.into(), + value: TRANSFER, + }, + ) + .unwrap(); // Check for an even occurred in this block env.check_event(pallet_balances::Event::Transfer { @@ -50,16 +51,20 @@ fn transfer_balance() { }) .unwrap(); - // Pass blocks to evolve the system - env.pass(Blocks::ByNumber(1)); - // Check the state env.state(|| { + assert_eq!( + pallet_balances::Pallet::::free_balance(Keyring::Alice.to_account_id()), + T::ExistentialDeposit::get() + FOR_FEES - fee, + ); assert_eq!( pallet_balances::Pallet::::free_balance(Keyring::Bob.to_account_id()), TRANSFER ); }); + + // Pass blocks to evolve the system + env.pass(Blocks::ByNumber(1)); } // Identical to `transfer_balance()` test but using fudge. @@ -78,7 +83,7 @@ fn fudge_transfer_balance() { .storage(), ); - env.submit( + env.submit_later( Keyring::Alice, pallet_balances::Call::transfer { dest: Keyring::Bob.into(), @@ -87,16 +92,9 @@ fn fudge_transfer_balance() { ) .unwrap(); + // submit-later will only take effect if a block has passed env.pass(Blocks::ByNumber(1)); - // Check the state - env.state(|| { - assert_eq!( - pallet_balances::Pallet::::free_balance(Keyring::Bob.to_account_id()), - TRANSFER - ); - }); - // Check for an even occurred in this block env.check_event(pallet_balances::Event::Transfer { from: Keyring::Alice.to_account_id(), @@ -104,6 +102,28 @@ fn fudge_transfer_balance() { amount: TRANSFER, }) .unwrap(); + + // Look for the fee for the last transaction + let fee = env + .find_event(|e| match e { + pallet_transaction_payment::Event::TransactionFeePaid { actual_fee, .. } => { + Some(actual_fee) + } + _ => None, + }) + .unwrap(); + + // Check the state + env.state(|| { + assert_eq!( + pallet_balances::Pallet::::free_balance(Keyring::Alice.to_account_id()), + T::ExistentialDeposit::get() + FOR_FEES - fee, + ); + assert_eq!( + pallet_balances::Pallet::::free_balance(Keyring::Bob.to_account_id()), + TRANSFER + ); + }); } fn call_api() { @@ -133,34 +153,7 @@ fn fudge_call_api() { }) } -fn check_fee() { - let mut env = RuntimeEnv::::from_storage( - Genesis::default() - .add(pallet_balances::GenesisConfig:: { - balances: vec![(Keyring::Alice.to_account_id(), 1 * CFG)], - }) - .storage(), - ); - - env.submit( - Keyring::Alice, - frame_system::Call::remark { remark: vec![] }, - ) - .unwrap(); - - // Get the fee of the last submitted extrinsic - let fee = env.last_fee(); - - env.state(|| { - assert_eq!( - pallet_balances::Pallet::::free_balance(Keyring::Alice.to_account_id()), - 1 * CFG - fee - ); - }); -} - crate::test_for_runtimes!([development, altair, centrifuge], transfer_balance); crate::test_for_runtimes!(all, call_api); -crate::test_for_runtimes!(all, check_fee); crate::test_for_runtimes!(all, fudge_transfer_balance); crate::test_for_runtimes!(all, fudge_call_api); diff --git a/runtime/integration-tests/src/generic/environment.rs b/runtime/integration-tests/src/generic/environment.rs index 8862e911a1..ba8ce42b72 100644 --- a/runtime/integration-tests/src/generic/environment.rs +++ b/runtime/integration-tests/src/generic/environment.rs @@ -1,9 +1,9 @@ -use cfg_primitives::{Address, Balance, BlockNumber, Moment}; +use cfg_primitives::{Address, Balance, BlockNumber, Index, Moment}; use codec::Encode; use sp_runtime::{ generic::{Era, SignedPayload}, traits::{Block, Extrinsic}, - DispatchResult, MultiSignature, Storage, + DispatchError, DispatchResult, MultiSignature, Storage, }; use crate::{generic::runtime::Runtime, utils::accounts::Keyring}; @@ -37,10 +37,10 @@ pub trait Env { &self, who: Keyring, call: impl Into, + nonce: Index, ) -> ::Extrinsic { self.state(|| { let runtime_call = call.into(); - let nonce = frame_system::Pallet::::account(who.to_account_id()).nonce; let signed_extra = ( frame_system::CheckNonZeroSender::::new(), frame_system::CheckSpecVersion::::new(), @@ -63,8 +63,16 @@ pub trait Env { }) } - /// Submit an extrinsic mutating the state - fn submit(&mut self, who: Keyring, call: impl Into) -> DispatchResult; + /// Submit an extrinsic mutating the state instantly and returning the + /// consumed fee + fn submit_now( + &mut self, + who: Keyring, + call: impl Into, + ) -> Result; + + /// Submit an extrinsic mutating the state when the block is finalized + fn submit_later(&mut self, who: Keyring, call: impl Into) -> DispatchResult; /// Pass any number of blocks fn pass(&mut self, blocks: Blocks) { @@ -137,16 +145,5 @@ pub trait Env { }) } - /// Retrieve the fees used in the last submit call - fn last_fee(&self) -> Balance { - self.find_event(|e| match e { - pallet_transaction_payment::Event::TransactionFeePaid { actual_fee, .. } => { - Some(actual_fee) - } - _ => None, - }) - .expect("Expected transaction") - } - fn __priv_build_block(&mut self, i: BlockNumber); } diff --git a/runtime/integration-tests/src/generic/envs/fudge_env.rs b/runtime/integration-tests/src/generic/envs/fudge_env.rs index c590be111b..3bf0166acf 100644 --- a/runtime/integration-tests/src/generic/envs/fudge_env.rs +++ b/runtime/integration-tests/src/generic/envs/fudge_env.rs @@ -1,6 +1,8 @@ pub mod handle; -use cfg_primitives::BlockNumber; +use std::collections::HashMap; + +use cfg_primitives::{Balance, BlockNumber, Index}; use fudge::primitives::Chain; use handle::{FudgeHandle, ParachainClient}; use sc_client_api::HeaderBackend; @@ -21,6 +23,7 @@ pub trait FudgeSupport: Runtime { /// Evironment that uses fudge to interact with the runtime pub struct FudgeEnv { handle: T::FudgeHandle, + nonce_storage: HashMap, } impl Env for FudgeEnv { @@ -29,11 +32,25 @@ impl Env for FudgeEnv { handle.evolve(); - Self { handle } + Self { + handle, + nonce_storage: HashMap::default(), + } + } + + fn submit_now( + &mut self, + _who: Keyring, + _call: impl Into, + ) -> Result { + unimplemented!("FudgeEnv does not support submit_now() try submit_later()") } - fn submit(&mut self, who: Keyring, call: impl Into) -> DispatchResult { - let extrinsic = self.create_extrinsic(who, call); + /// Submit an extrinsic mutating the state when the block is finalized + fn submit_later(&mut self, who: Keyring, call: impl Into) -> DispatchResult { + let nonce = *self.nonce_storage.entry(who).or_default(); + + let extrinsic = self.create_extrinsic(who, call, nonce); self.handle .parachain_mut() @@ -42,7 +59,11 @@ impl Env for FudgeEnv { .map_err(|_| { DispatchError::Other("Specific kind of DispatchError not supported by fudge now") // More information, issue: https://github.com/centrifuge/fudge/issues/67 - }) + })?; + + self.nonce_storage.insert(who, nonce + 1); + + Ok(()) } fn state_mut(&mut self, f: impl FnOnce() -> R) -> R { @@ -88,3 +109,42 @@ impl FudgeEnv { exec(api, best_hash); } } + +mod tests { + use cfg_primitives::CFG; + + use super::*; + use crate::generic::{environment::Blocks, utils::genesis::Genesis}; + + fn correct_nonce_for_submit_later() { + let mut env = FudgeEnv::::from_storage( + Genesis::default() + .add(pallet_balances::GenesisConfig:: { + balances: vec![(Keyring::Alice.to_account_id(), 1 * CFG)], + }) + .storage(), + ); + + env.submit_later( + Keyring::Alice, + frame_system::Call::remark { remark: vec![] }, + ) + .unwrap(); + + env.submit_later( + Keyring::Alice, + frame_system::Call::remark { remark: vec![] }, + ) + .unwrap(); + + env.pass(Blocks::ByNumber(1)); + + env.submit_later( + Keyring::Alice, + frame_system::Call::remark { remark: vec![] }, + ) + .unwrap(); + } + + crate::test_for_runtimes!(all, correct_nonce_for_submit_later); +} diff --git a/runtime/integration-tests/src/generic/envs/runtime_env.rs b/runtime/integration-tests/src/generic/envs/runtime_env.rs index 9e1fc13b26..050ad94a11 100644 --- a/runtime/integration-tests/src/generic/envs/runtime_env.rs +++ b/runtime/integration-tests/src/generic/envs/runtime_env.rs @@ -1,21 +1,17 @@ -use std::{cell::RefCell, marker::PhantomData, rc::Rc}; +use std::{cell::RefCell, marker::PhantomData, mem, rc::Rc}; -use cfg_primitives::{AuraId, BlockNumber, Header}; +use cfg_primitives::{AuraId, Balance, BlockNumber, Header}; use codec::Encode; use cumulus_primitives_core::PersistedValidationData; use cumulus_primitives_parachain_inherent::ParachainInherentData; use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder; use frame_support::{ inherent::{InherentData, ProvideInherent}, - storage::transactional, traits::GenesisBuild, }; use sp_consensus_aura::{Slot, AURA_ENGINE_ID}; use sp_core::{sr25519::Public, H256}; -use sp_runtime::{ - traits::Extrinsic, Digest, DigestItem, DispatchError, DispatchResult, Storage, - TransactionOutcome, -}; +use sp_runtime::{traits::Extrinsic, Digest, DigestItem, DispatchError, DispatchResult, Storage}; use sp_timestamp::Timestamp; use crate::{ @@ -27,6 +23,7 @@ use crate::{ /// without the usage of a client. pub struct RuntimeEnv { ext: Rc>, + pending_extrinsics: Vec<(Keyring, T::RuntimeCall)>, _config: PhantomData, } @@ -45,13 +42,35 @@ impl Env for RuntimeEnv { Self { ext: Rc::new(RefCell::new(ext)), + pending_extrinsics: Vec::default(), _config: PhantomData, } } - fn submit(&mut self, who: Keyring, call: impl Into) -> DispatchResult { - let extrinsic = self.create_extrinsic(who, call); - self.state_mut(|| T::apply_extrinsic(extrinsic).unwrap()) + fn submit_now( + &mut self, + who: Keyring, + call: impl Into, + ) -> Result { + let nonce = self.state(|| frame_system::Pallet::::account(who.to_account_id()).nonce); + let extrinsic = self.create_extrinsic(who, call, nonce); + self.state_mut(|| T::apply_extrinsic(extrinsic).unwrap())?; + + let fee = self + .find_event(|e| match e { + pallet_transaction_payment::Event::TransactionFeePaid { actual_fee, .. } => { + Some(actual_fee) + } + _ => None, + }) + .unwrap(); + + Ok(fee) + } + + fn submit_later(&mut self, who: Keyring, call: impl Into) -> DispatchResult { + self.pending_extrinsics.push((who, call.into())); + Ok(()) } fn state_mut(&mut self, f: impl FnOnce() -> R) -> R { @@ -60,15 +79,18 @@ impl Env for RuntimeEnv { fn state(&self, f: impl FnOnce() -> R) -> R { self.ext.borrow_mut().execute_with(|| { - transactional::with_transaction(|| { - // We revert all changes done by the closure to offer an inmutable state method - TransactionOutcome::Rollback::>(Ok(f())) - }) - .unwrap() + let version = frame_support::StateVersion::V1; + let hash = frame_support::storage_root(version); + + let result = f(); + + assert_eq!(hash, frame_support::storage_root(version)); + result }) } fn __priv_build_block(&mut self, i: BlockNumber) { + self.process_pending_extrinsics(); self.state_mut(|| { T::finalize_block(); Self::prepare_block(i); @@ -77,6 +99,16 @@ impl Env for RuntimeEnv { } impl RuntimeEnv { + fn process_pending_extrinsics(&mut self) { + let pending_extrinsics = mem::replace(&mut self.pending_extrinsics, Vec::default()); + for (who, call) in pending_extrinsics { + let nonce = + self.state(|| frame_system::Pallet::::account(who.to_account_id()).nonce); + let extrinsic = self.create_extrinsic(who, call, nonce); + self.state_mut(|| T::apply_extrinsic(extrinsic).unwrap().unwrap()); + } + } + fn prepare_block(i: BlockNumber) { let slot = Slot::from(i as u64); let digest = Digest { @@ -149,3 +181,65 @@ impl RuntimeEnv { .into() } } + +mod tests { + use cfg_primitives::CFG; + + use super::*; + use crate::generic::{environment::Blocks, utils::genesis::Genesis}; + + fn correct_nonce_for_submit_now() { + let mut env = RuntimeEnv::::from_storage( + Genesis::default() + .add(pallet_balances::GenesisConfig:: { + balances: vec![(Keyring::Alice.to_account_id(), 1 * CFG)], + }) + .storage(), + ); + + env.submit_now( + Keyring::Alice, + frame_system::Call::remark { remark: vec![] }, + ) + .unwrap(); + + env.submit_now( + Keyring::Alice, + frame_system::Call::remark { remark: vec![] }, + ) + .unwrap(); + } + + fn correct_nonce_for_submit_later() { + let mut env = RuntimeEnv::::from_storage( + Genesis::default() + .add(pallet_balances::GenesisConfig:: { + balances: vec![(Keyring::Alice.to_account_id(), 1 * CFG)], + }) + .storage(), + ); + + env.submit_later( + Keyring::Alice, + frame_system::Call::remark { remark: vec![] }, + ) + .unwrap(); + + env.submit_later( + Keyring::Alice, + frame_system::Call::remark { remark: vec![] }, + ) + .unwrap(); + + env.pass(Blocks::ByNumber(1)); + + env.submit_later( + Keyring::Alice, + frame_system::Call::remark { remark: vec![] }, + ) + .unwrap(); + } + + crate::test_for_runtimes!(all, correct_nonce_for_submit_now); + crate::test_for_runtimes!(all, correct_nonce_for_submit_later); +}