mirror of
https://github.com/pezkuwichain/pezkuwi-subxt.git
synced 2026-04-30 09:37:55 +00:00
bb8ddc46c1
I started this investigation/issue based on @liamaharon question [here](https://github.com/paritytech/polkadot-sdk/pull/1801#discussion_r1410452499). ## Problem The `pallet_balances` integrity test should correctly detect that the runtime has correct distinct `HoldReasons` variant count. I assume the same situation exists for RuntimeFreezeReason. It is not a critical problem, if we set `MaxHolds` with a sufficiently large value, everything should be ok. However, in this case, the integrity_test check becomes less useful. **Situation for "any" runtime:** - `HoldReason` enums from different pallets: ```rust /// from pallet_nis #[pallet::composite_enum] pub enum HoldReason { NftReceipt, } /// from pallet_preimage #[pallet::composite_enum] pub enum HoldReason { Preimage, } // from pallet_state-trie-migration #[pallet::composite_enum] pub enum HoldReason { SlashForContinueMigrate, SlashForMigrateCustomTop, SlashForMigrateCustomChild, } ``` - generated `RuntimeHoldReason` enum looks like: ```rust pub enum RuntimeHoldReason { #[codec(index = 32u8)] Preimage(pallet_preimage::HoldReason), #[codec(index = 38u8)] Nis(pallet_nis::HoldReason), #[codec(index = 42u8)] StateTrieMigration(pallet_state_trie_migration::HoldReason), } ``` - composite enum `RuntimeHoldReason` variant count is detected as `3` - we set `type MaxHolds = ConstU32<3>` - `pallet_balances::integrity_test` is ok with `3`(at least 3) However, the real problem can occur in a live runtime where some functionality might stop working. This is due to a total of 5 distinct hold reasons (for pallets with multi-instance support, it is even more), and not all of them can be used because of an incorrect `MaxHolds`, which is deemed acceptable according to the `integrity_test`: ``` // pseudo-code - if we try to call all of these: T::Currency::hold(&pallet_nis::HoldReason::NftReceipt.into(), &nft_owner, deposit)?; T::Currency::hold(&pallet_preimage::HoldReason::Preimage.into(), &nft_owner, deposit)?; T::Currency::hold(&pallet_state_trie_migration::HoldReason::SlashForContinueMigrate.into(), &nft_owner, deposit)?; // With `type MaxHolds = ConstU32<3>` these two will fail T::Currency::hold(&pallet_state_trie_migration::HoldReason::SlashForMigrateCustomTop.into(), &nft_owner, deposit)?; T::Currency::hold(&pallet_state_trie_migration::HoldReason::SlashForMigrateCustomChild.into(), &nft_owner, deposit)?; ``` ## Solutions A macro `#[pallet::*]` expansion is extended of `VariantCount` implementation for the `#[pallet::composite_enum]` enum type. This expansion generates the `VariantCount` implementation for pallets' `HoldReason`, `FreezeReason`, `LockId`, and `SlashReason`. Enum variants must be plain enum values without fields to ensure a deterministic count. The composite runtime enum, `RuntimeHoldReason` and `RuntimeFreezeReason`, now sets `VariantCount::VARIANT_COUNT` as the sum of pallets' enum `VariantCount::VARIANT_COUNT`: ```rust #[frame_support::pallet(dev_mode)] mod module_single_instance { #[pallet::composite_enum] pub enum HoldReason { ModuleSingleInstanceReason1, ModuleSingleInstanceReason2, } ... } #[frame_support::pallet(dev_mode)] mod module_multi_instance { #[pallet::composite_enum] pub enum HoldReason<I: 'static = ()> { ModuleMultiInstanceReason1, ModuleMultiInstanceReason2, ModuleMultiInstanceReason3, } ... } impl self::sp_api_hidden_includes_construct_runtime::hidden_include::traits::VariantCount for RuntimeHoldReason { const VARIANT_COUNT: u32 = 0 + module_single_instance::HoldReason::VARIANT_COUNT + module_multi_instance::HoldReason::<module_multi_instance::Instance1>::VARIANT_COUNT + module_multi_instance::HoldReason::<module_multi_instance::Instance2>::VARIANT_COUNT + module_multi_instance::HoldReason::<module_multi_instance::Instance3>::VARIANT_COUNT; } ``` In addition, `MaxHolds` is removed (as suggested [here](https://github.com/paritytech/polkadot-sdk/pull/2657#discussion_r1443324573)) from `pallet_balances`, and its `Holds` are now bounded to `RuntimeHoldReason::VARIANT_COUNT`. Therefore, there is no need to let the runtime specify `MaxHolds`. ## For reviewers Relevant changes can be found here: - `substrate/frame/support/procedural/src/lib.rs` - `substrate/frame/support/procedural/src/pallet/parse/composite.rs` - `substrate/frame/support/procedural/src/pallet/expand/composite.rs` - `substrate/frame/support/procedural/src/construct_runtime/expand/composite_helper.rs` - `substrate/frame/support/procedural/src/construct_runtime/expand/hold_reason.rs` - `substrate/frame/support/procedural/src/construct_runtime/expand/freeze_reason.rs` - `substrate/frame/support/src/traits/misc.rs` And the rest of the files is just about removed `MaxHolds` from `pallet_balances` ## Next steps Do the same for `MaxFreezes` https://github.com/paritytech/polkadot-sdk/issues/2997. --------- Co-authored-by: command-bot <> Co-authored-by: Bastian Köcher <git@kchr.de> Co-authored-by: Dónal Murray <donal.murray@parity.io> Co-authored-by: gupnik <nikhilgupta.iitk@gmail.com>
199 lines
5.8 KiB
Rust
199 lines
5.8 KiB
Rust
// 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.
|
|
|
|
//! Tests for pallet-example-basic.
|
|
|
|
use crate::*;
|
|
use frame_support::{
|
|
assert_ok, derive_impl,
|
|
dispatch::{DispatchInfo, GetDispatchInfo},
|
|
traits::{ConstU64, OnInitialize},
|
|
};
|
|
use sp_core::H256;
|
|
// The testing primitives are very useful for avoiding having to work with signatures
|
|
// or public keys. `u64` is used as the `AccountId` and no `Signature`s are required.
|
|
use sp_runtime::{
|
|
traits::{BlakeTwo256, IdentityLookup},
|
|
BuildStorage,
|
|
};
|
|
// Reexport crate as its pallet name for construct_runtime.
|
|
use crate as pallet_example_basic;
|
|
|
|
type Block = frame_system::mocking::MockBlock<Test>;
|
|
|
|
// For testing the pallet, we construct a mock runtime.
|
|
frame_support::construct_runtime!(
|
|
pub enum Test
|
|
{
|
|
System: frame_system,
|
|
Balances: pallet_balances,
|
|
Example: pallet_example_basic,
|
|
}
|
|
);
|
|
|
|
#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)]
|
|
impl frame_system::Config for Test {
|
|
type BaseCallFilter = frame_support::traits::Everything;
|
|
type BlockWeights = ();
|
|
type BlockLength = ();
|
|
type DbWeight = ();
|
|
type RuntimeOrigin = RuntimeOrigin;
|
|
type Nonce = u64;
|
|
type Hash = H256;
|
|
type RuntimeCall = RuntimeCall;
|
|
type Hashing = BlakeTwo256;
|
|
type AccountId = u64;
|
|
type Lookup = IdentityLookup<Self::AccountId>;
|
|
type Block = Block;
|
|
type RuntimeEvent = RuntimeEvent;
|
|
type BlockHashCount = ConstU64<250>;
|
|
type Version = ();
|
|
type PalletInfo = PalletInfo;
|
|
type AccountData = pallet_balances::AccountData<u64>;
|
|
type OnNewAccount = ();
|
|
type OnKilledAccount = ();
|
|
type SystemWeightInfo = ();
|
|
type SS58Prefix = ();
|
|
type OnSetCode = ();
|
|
type MaxConsumers = frame_support::traits::ConstU32<16>;
|
|
}
|
|
|
|
impl pallet_balances::Config for Test {
|
|
type MaxLocks = ();
|
|
type MaxReserves = ();
|
|
type ReserveIdentifier = [u8; 8];
|
|
type Balance = u64;
|
|
type DustRemoval = ();
|
|
type RuntimeEvent = RuntimeEvent;
|
|
type ExistentialDeposit = ConstU64<1>;
|
|
type AccountStore = System;
|
|
type WeightInfo = ();
|
|
type FreezeIdentifier = ();
|
|
type MaxFreezes = ();
|
|
type RuntimeHoldReason = ();
|
|
type RuntimeFreezeReason = ();
|
|
}
|
|
|
|
impl Config for Test {
|
|
type MagicNumber = ConstU64<1_000_000_000>;
|
|
type RuntimeEvent = RuntimeEvent;
|
|
type WeightInfo = ();
|
|
}
|
|
|
|
// This function basically just builds a genesis storage key/value store according to
|
|
// our desired mockup.
|
|
pub fn new_test_ext() -> sp_io::TestExternalities {
|
|
let t = RuntimeGenesisConfig {
|
|
// We use default for brevity, but you can configure as desired if needed.
|
|
system: Default::default(),
|
|
balances: Default::default(),
|
|
example: pallet_example_basic::GenesisConfig {
|
|
dummy: 42,
|
|
// we configure the map with (key, value) pairs.
|
|
bar: vec![(1, 2), (2, 3)],
|
|
foo: 24,
|
|
},
|
|
}
|
|
.build_storage()
|
|
.unwrap();
|
|
t.into()
|
|
}
|
|
|
|
#[test]
|
|
fn it_works_for_optional_value() {
|
|
new_test_ext().execute_with(|| {
|
|
// Check that GenesisBuilder works properly.
|
|
let val1 = 42;
|
|
let val2 = 27;
|
|
assert_eq!(Example::dummy(), Some(val1));
|
|
|
|
// Check that accumulate works when we have Some value in Dummy already.
|
|
assert_ok!(Example::accumulate_dummy(RuntimeOrigin::signed(1), val2));
|
|
assert_eq!(Example::dummy(), Some(val1 + val2));
|
|
|
|
// Check that accumulate works when we Dummy has None in it.
|
|
<Example as OnInitialize<u64>>::on_initialize(2);
|
|
assert_ok!(Example::accumulate_dummy(RuntimeOrigin::signed(1), val1));
|
|
assert_eq!(Example::dummy(), Some(val1 + val2 + val1));
|
|
});
|
|
}
|
|
|
|
#[test]
|
|
fn it_works_for_default_value() {
|
|
new_test_ext().execute_with(|| {
|
|
assert_eq!(Example::foo(), 24);
|
|
assert_ok!(Example::accumulate_foo(RuntimeOrigin::signed(1), 1));
|
|
assert_eq!(Example::foo(), 25);
|
|
});
|
|
}
|
|
|
|
#[test]
|
|
fn set_dummy_works() {
|
|
new_test_ext().execute_with(|| {
|
|
let test_val = 133;
|
|
assert_ok!(Example::set_dummy(RuntimeOrigin::root(), test_val.into()));
|
|
assert_eq!(Example::dummy(), Some(test_val));
|
|
});
|
|
}
|
|
|
|
#[test]
|
|
fn signed_ext_watch_dummy_works() {
|
|
new_test_ext().execute_with(|| {
|
|
let call = pallet_example_basic::Call::set_dummy { new_value: 10 }.into();
|
|
let info = DispatchInfo::default();
|
|
|
|
assert_eq!(
|
|
WatchDummy::<Test>(PhantomData)
|
|
.validate(&1, &call, &info, 150)
|
|
.unwrap()
|
|
.priority,
|
|
u64::MAX,
|
|
);
|
|
assert_eq!(
|
|
WatchDummy::<Test>(PhantomData).validate(&1, &call, &info, 250),
|
|
InvalidTransaction::ExhaustsResources.into(),
|
|
);
|
|
})
|
|
}
|
|
|
|
#[test]
|
|
fn counted_map_works() {
|
|
new_test_ext().execute_with(|| {
|
|
assert_eq!(CountedMap::<Test>::count(), 0);
|
|
CountedMap::<Test>::insert(3, 3);
|
|
assert_eq!(CountedMap::<Test>::count(), 1);
|
|
})
|
|
}
|
|
|
|
#[test]
|
|
fn weights_work() {
|
|
// must have a defined weight.
|
|
let default_call = pallet_example_basic::Call::<Test>::accumulate_dummy { increase_by: 10 };
|
|
let info1 = default_call.get_dispatch_info();
|
|
// aka. `let info = <Call<Test> as GetDispatchInfo>::get_dispatch_info(&default_call);`
|
|
// TODO: account for proof size weight
|
|
assert!(info1.weight.ref_time() > 0);
|
|
assert_eq!(info1.weight, <Test as Config>::WeightInfo::accumulate_dummy());
|
|
|
|
// `set_dummy` is simpler than `accumulate_dummy`, and the weight
|
|
// should be less.
|
|
let custom_call = pallet_example_basic::Call::<Test>::set_dummy { new_value: 20 };
|
|
let info2 = custom_call.get_dispatch_info();
|
|
// TODO: account for proof size weight
|
|
assert!(info1.weight.ref_time() > info2.weight.ref_time());
|
|
}
|