Fix broken pallet_shards benchmarks (#1017)

pallets/shards/Cargo.toml

@@ -23,11 +23,13 @@ scale-info.workspace = true
 
 frame-support = { git = "https://github.com/analog-labs/polkadot-sdk", tag = "v1.13.0-patched", default-features = false }
 frame-system = { git = "https://github.com/analog-labs/polkadot-sdk", tag = "v1.13.0-patched", default-features = false }
+sp-core = { git = "https://github.com/analog-labs/polkadot-sdk", tag = "v1.13.0-patched", default-features = false }
 sp-runtime = { git = "https://github.com/analog-labs/polkadot-sdk", tag = "v1.13.0-patched", default-features = false }
 sp-std = { git = "https://github.com/analog-labs/polkadot-sdk", tag = "v1.13.0-patched", default-features = false }
 
 frame-benchmarking = { git = "https://github.com/analog-labs/polkadot-sdk", tag = "v1.13.0-patched", default-features = false, optional = true }
-
+pallet-balances = { git = "https://github.com/analog-labs/polkadot-sdk", tag = "v1.13.0-patched", default-features = false }
+pallet-members = { path = "../members", default-features = false }
 schnorr-evm = { version = "0.1.2", default-features = false }
 
 time-primitives = { path = "../../primitives", default-features = false }
@@ -37,7 +39,6 @@ simple-mermaid.workspace = true
 [dev-dependencies]
 #polkadot-sdk = { workspace = true, features = [ "pallet-balances", "sp-core", "sp-io" ] }
 
-pallet-balances = { git = "https://github.com/analog-labs/polkadot-sdk", tag = "v1.13.0-patched", default-features = false }
 sp-core = { git = "https://github.com/analog-labs/polkadot-sdk", tag = "v1.13.0-patched", default-features = false }
 sp-io = { git = "https://github.com/analog-labs/polkadot-sdk", tag = "v1.13.0-patched", default-features = false }
 
@@ -50,12 +51,14 @@ std = [
 	#"polkadot-sdk/std",
 	"frame-support/std",
 	"frame-system/std",
+	"sp-core/std",
 	"sp-runtime/std",
 	"sp-std/std",
 	"frame-benchmarking?/std",
 	"schnorr-evm/std",
 	"time-primitives/std",
-	"pallet-balances/std"
+	"pallet-balances/std",
+	"pallet-members/std"
 ]
 runtime-benchmarks = [
 	#"polkadot-sdk/runtime-benchmarks",

pallets/shards/src/benchmarking.rs

@@ -1,6 +1,7 @@
 use super::*;
 use crate::Pallet;
 use frame_benchmarking::benchmarks;
+use frame_support::traits::{Currency, Get};
 use frame_system::RawOrigin;
 use sp_std::vec;
 use sp_std::vec::Vec;
@@ -9,7 +10,11 @@ pub const ALICE: [u8; 32] = [1u8; 32];
 pub const BOB: [u8; 32] = [2u8; 32];
 pub const CHARLIE: [u8; 32] = [3u8; 32];
 pub const ETHEREUM: NetworkId = 0;
-use time_primitives::{Commitment, ProofOfKnowledge};
+use time_primitives::{Commitment, ProofOfKnowledge, PublicKey};
+
+fn public_key(acc: [u8; 32]) -> PublicKey {
+	PublicKey::Sr25519(sp_core::sr25519::Public::from_raw(acc))
+}
 
 /// Since benchmarks are no-std and we need std computation on constructing proof so
 /// these values are taken by running the code in pallets/shards/src/tests.rs
@@ -69,18 +74,31 @@ pub fn get_proof_of_knowledge(member: [u8; 32]) -> ProofOfKnowledge {
 }
 
 benchmarks! {
+	where_clause {  where T: pallet_members::Config }
 	commit {
-		let shard: Vec<AccountId> = vec![ALICE.into(), BOB.into(), CHARLIE.into()];
-		Pallet::<T>::create_shard(ETHEREUM, shard.clone(), 1);
+		let shard: Vec<[u8; 32]> = vec![ALICE, BOB, CHARLIE];
+		Pallet::<T>::create_shard(ETHEREUM, shard.clone().into_iter().map(|x| x.into()).collect::<Vec<AccountId>>(), 1);
 		let alice: AccountId = ALICE.into();
 		// benchmark commitment that changes shard status
 		for member in shard {
-			if member != alice {
+			let member_account: AccountId = member.into();
+			pallet_balances::Pallet::<T>::resolve_creating(
+				&member_account,
+				pallet_balances::Pallet::<T>::issue(<T as pallet_members::Config>::MinStake::get() * 100),
+			);
+			pallet_members::Pallet::<T>::register_member(
+				RawOrigin::Signed(member_account.clone()).into(),
+				ETHEREUM,
+				public_key(member),
+				member,
+				<T as pallet_members::Config>::MinStake::get(),
+			)?;
+			if member != ALICE {
 				Pallet::<T>::commit(
-					RawOrigin::Signed(member.clone()).into(),
+					RawOrigin::Signed(member_account.clone()).into(),
 					0,
-					get_commitment(member.clone().into()),
-					get_proof_of_knowledge(member.into()),
+					get_commitment(member_account.clone().into()),
+					get_proof_of_knowledge(member_account.into()),
 				)?;
 			}
 		}
@@ -89,27 +107,38 @@ benchmarks! {
 	verify { }
 
 	ready {
-		let shard: Vec<AccountId> = vec![ALICE.into(), BOB.into(), CHARLIE.into()];
-		Pallet::<T>::create_shard(ETHEREUM, shard.clone(), 1);
-		let alice: AccountId = ALICE.into();
+		let shard: Vec<[u8; 32]> = vec![ALICE, BOB, CHARLIE];
+		Pallet::<T>::create_shard(ETHEREUM, shard.clone().into_iter().map(|x| x.into()).collect::<Vec<AccountId>>(), 1);
 		for member in shard.clone() {
+			let member_account: AccountId = member.into();
+			pallet_balances::Pallet::<T>::resolve_creating(
+				&member_account,
+				pallet_balances::Pallet::<T>::issue(<T as pallet_members::Config>::MinStake::get() * 100),
+			);
+			pallet_members::Pallet::<T>::register_member(
+				RawOrigin::Signed(member_account.clone()).into(),
+				ETHEREUM,
+				public_key(member),
+				member,
+				<T as pallet_members::Config>::MinStake::get(),
+			)?;
 			Pallet::<T>::commit(
-				RawOrigin::Signed(member.clone()).into(),
+				RawOrigin::Signed(member_account.clone()).into(),
 				0,
-				get_commitment(member.clone().into()),
-				get_proof_of_knowledge(member.into()),
+				get_commitment(member_account.clone().into()),
+				get_proof_of_knowledge(member_account.into()),
 			)?;
 		}
 		// benchmark ready that changes shard status
 		for member in shard {
-			if member != alice {
+			if member != ALICE {
 				Pallet::<T>::ready(
-					RawOrigin::Signed(member.clone()).into(),
+					RawOrigin::Signed(member.into()).into(),
 					0,
 				)?;
 			}
 		}
-	}: _(RawOrigin::Signed(alice), 0)
+	}: _(RawOrigin::Signed(ALICE.into()), 0)
 	verify { }
 
 	force_shard_offline {

pallets/shards/src/lib.rs

@@ -81,9 +81,9 @@ pub mod pallet {
 	use sp_std::vec;
 	use sp_std::vec::Vec;
 	use time_primitives::{
-		AccountId, Commitment, ElectionsInterface, MemberEvents, MemberStatus, MemberStorage,
-		NetworkId, ProofOfKnowledge, PublicKey, ShardId, ShardStatus, ShardsInterface,
-		TasksInterface, TssPublicKey,
+		AccountId, Balance, Commitment, ElectionsInterface, MemberEvents, MemberStatus,
+		MemberStorage, NetworkId, ProofOfKnowledge, PublicKey, ShardId, ShardStatus,
+		ShardsInterface, TasksInterface, TssPublicKey,
 	};
 
 	/// Trait to define the weights for various extrinsics in the pallet.
@@ -117,7 +117,9 @@ pub mod pallet {
 	pub struct Pallet<T>(_);
 
 	#[pallet::config]
-	pub trait Config: frame_system::Config<AccountId = sp_runtime::AccountId32> {
+	pub trait Config:
+		frame_system::Config<AccountId = AccountId> + pallet_balances::Config<Balance = Balance>
+	{
 		type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
 		type WeightInfo: WeightInfo;
 		type Elections: ElectionsInterface;
@@ -202,6 +204,8 @@ pub mod pallet {
 		UnknownShard,
 		/// Indicates that an unexpected commitment was provided for the shard.
 		UnexpectedCommit,
+		/// Indicates that a peer id cannot be found for the member.
+		MemberPeerIdNotFound,
 		/// Indicates that an invalid commitment was provided.
 		InvalidCommitment,
 		/// Indicates that an invalid proof of knowledge was provided.
@@ -226,7 +230,7 @@ pub mod pallet {
 		///   6. If all members have committed, update the state of the shards to `Committed` and store the group commitment.
 		///   7. Emit the [`Event::ShardCommitted`] event.
 		#[pallet::call_index(0)]
-		#[pallet::weight(T::WeightInfo::commit())]
+		#[pallet::weight(<T as Config>::WeightInfo::commit())]
 		pub fn commit(
 			origin: OriginFor<T>,
 			shard_id: ShardId,
@@ -244,7 +248,7 @@ pub mod pallet {
 				ensure!(VerifyingKey::from_bytes(*c).is_ok(), Error::<T>::InvalidCommitment);
 			}
 			let peer_id =
-				T::Members::member_peer_id(&member).ok_or(Error::<T>::UnexpectedCommit)?;
+				T::Members::member_peer_id(&member).ok_or(Error::<T>::MemberPeerIdNotFound)?;
 			schnorr_evm::proof_of_knowledge::verify_proof_of_knowledge(
 				&peer_id,
 				&commitment,
@@ -288,7 +292,7 @@ pub mod pallet {
 		///   4. If all members are ready, update the state of the shard to `Online` and emit the [`Event::ShardOnline`] event.
 		///   5. Notify the task scheduler that the shard is online.
 		#[pallet::call_index(1)]
-		#[pallet::weight(T::WeightInfo::ready())]
+		#[pallet::weight(<T as Config>::WeightInfo::ready())]
 		pub fn ready(origin: OriginFor<T>, shard_id: ShardId) -> DispatchResult {
 			let member = ensure_signed(origin)?;
 			ensure!(
@@ -315,7 +319,7 @@ pub mod pallet {
 		///   1. Ensure the origin is the root.
 		///   2. Call the internal `remove_shard_offline` function to handle the shard offline process.
 		#[pallet::call_index(2)]
-		#[pallet::weight(T::WeightInfo::force_shard_offline())]
+		#[pallet::weight(<T as Config>::WeightInfo::force_shard_offline())]
 		pub fn force_shard_offline(origin: OriginFor<T>, shard_id: ShardId) -> DispatchResult {
 			ensure_root(origin)?;
 			Self::remove_shard_offline(shard_id);
@@ -454,7 +458,7 @@ pub mod pallet {
 		///   4. Determines the new_status of the shard based on the conditions:
 		///     - If transitioning to `Offline` and not previously `Offline`, calls `Function::remove_shard_offline`.
 		///     - Updates [`ShardState`] with the new new_status.
-		///   5. Returns the weight of the operation as specified by `T::WeightInfo::member_offline()`.
+		///   5. Returns the weight of the operation as specified by `<T as Config>::WeightInfo::member_offline()`.
 		fn member_offline(id: &AccountId, _: NetworkId) -> Weight {
 			let Some(shard_id) = MemberShard::<T>::get(id) else {
 				return T::DbWeight::get().reads(1);
@@ -488,7 +492,7 @@ pub mod pallet {
 			} else if !matches!(new_status, ShardStatus::Offline) {
 				ShardState::<T>::insert(shard_id, new_status);
 			}
-			T::WeightInfo::member_offline()
+			<T as Config>::WeightInfo::member_offline()
 		}
 	}
 

pallets/shards/src/mock.rs

@@ -1,14 +1,12 @@
 use crate::{self as pallet_shards};
 use frame_support::derive_impl;
 use frame_support::traits::OnInitialize;
-use sp_core::{ByteArray, ConstU128, ConstU64};
+use sp_core::{ConstU128, ConstU64};
 use sp_runtime::{
 	traits::{IdentifyAccount, IdentityLookup, Verify},
 	BuildStorage, MultiSignature,
 };
-use time_primitives::{
-	ElectionsInterface, MemberStorage, NetworkId, PeerId, PublicKey, ShardId, TasksInterface,
-};
+use time_primitives::{ElectionsInterface, NetworkId, ShardId, TasksInterface};
 
 pub type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
 type Block = frame_system::mocking::MockBlock<Test>;
@@ -22,26 +20,6 @@ impl TasksInterface for MockTaskScheduler {
 	fn shard_offline(_: ShardId, _: NetworkId) {}
 }
 
-pub struct MockMembers;
-
-impl MemberStorage for MockMembers {
-	fn member_stake(_: &AccountId) -> u128 {
-		0u128
-	}
-	fn member_peer_id(id: &AccountId) -> Option<PeerId> {
-		Some(id.as_slice().try_into().unwrap())
-	}
-	fn member_public_key(_account: &AccountId) -> Option<PublicKey> {
-		None
-	}
-	fn is_member_online(_: &AccountId) -> bool {
-		true
-	}
-	fn total_stake() -> u128 {
-		0u128
-	}
-}
-
 pub struct MockElections;
 
 impl ElectionsInterface for MockElections {
@@ -56,6 +34,7 @@ frame_support::construct_runtime!(
 	{
 		System: frame_system::{Pallet, Call, Config<T>, Storage, Event<T>},
 		Balances: pallet_balances::{Pallet, Call, Storage, Event<T>},
+		Members: pallet_members,
 		Shards: pallet_shards::{Pallet, Call, Storage, Event<T>},
 	}
 );
@@ -87,11 +66,19 @@ impl pallet_shards::Config for Test {
 	type RuntimeEvent = RuntimeEvent;
 	type WeightInfo = ();
 	type TaskScheduler = MockTaskScheduler;
-	type Members = MockMembers;
+	type Members = Members;
 	type Elections = MockElections;
 	type DkgTimeout = ConstU64<10>;
 }
 
+impl pallet_members::Config for Test {
+	type WeightInfo = ();
+	type RuntimeEvent = RuntimeEvent;
+	type Elections = Shards;
+	type MinStake = ConstU128<5>;
+	type HeartbeatTimeout = ConstU64<10>;
+}
+
 impl<LocalCall> frame_system::offchain::CreateSignedTransaction<LocalCall> for Test
 where
 	RuntimeCall: From<LocalCall>,

pallets/shards/src/tests.rs

@@ -1,13 +1,14 @@
 use crate::mock::*;
 use crate::{Event, ShardMembers, ShardNetwork, ShardState};
 use frame_support::assert_ok;
+use frame_support::traits::{Currency, Get};
 use frame_system::RawOrigin;
 use schnorr_evm::k256::elliptic_curve::PrimeField;
 use schnorr_evm::k256::{ProjectivePoint, Scalar};
 use schnorr_evm::proof_of_knowledge::construct_proof_of_knowledge;
 use schnorr_evm::VerifyingKey;
 use time_primitives::{
-	AccountId, MemberEvents, NetworkId, PeerId, ShardId, ShardStatus, ShardsInterface,
+	AccountId, MemberEvents, NetworkId, PeerId, PublicKey, ShardId, ShardStatus, ShardsInterface,
 };
 
 const ETHEREUM: NetworkId = 0;
@@ -19,6 +20,10 @@ struct Member {
 	public_key: [u8; 33],
 }
 
+fn public_key(acc: [u8; 32]) -> PublicKey {
+	PublicKey::Sr25519(sp_core::sr25519::Public::from_raw(acc))
+}
+
 impl Member {
 	pub fn new(i: u8) -> Self {
 		let scalar = Scalar::from_repr([i; 32].into()).unwrap();
@@ -59,6 +64,19 @@ fn shard() -> [Member; 3] {
 fn create_shard(shard_id: ShardId, shard: &[Member], threshold: u16) {
 	Shards::create_shard(ETHEREUM, shard.iter().map(|m| m.account_id.clone()).collect(), threshold);
 	for member in shard {
+		pallet_balances::Pallet::<Test>::resolve_creating(
+			&member.account_id,
+			pallet_balances::Pallet::<Test>::issue(
+				<<Test as pallet_members::Config>::MinStake as Get<u128>>::get() * 100u128,
+			),
+		);
+		assert_ok!(Members::register_member(
+			RawOrigin::Signed(member.account_id.clone()).into(),
+			ETHEREUM,
+			public_key(member.peer_id),
+			member.peer_id,
+			<Test as pallet_members::Config>::MinStake::get(),
+		));
 		assert_ok!(Shards::commit(
 			RawOrigin::Signed(member.account_id.clone()).into(),
 			shard_id,
@@ -91,6 +109,19 @@ fn test_register_shard() {
 		for (shard_id, shard) in shards.iter().enumerate() {
 			let threshold = Shards::get_shard_threshold(shard_id as _);
 			for member in shard {
+				pallet_balances::Pallet::<Test>::resolve_creating(
+					&member.account_id,
+					pallet_balances::Pallet::<Test>::issue(
+						<<Test as pallet_members::Config>::MinStake as Get<u128>>::get() * 100u128,
+					),
+				);
+				assert_ok!(Members::register_member(
+					RawOrigin::Signed(member.account_id.clone()).into(),
+					ETHEREUM,
+					public_key(member.peer_id),
+					member.peer_id,
+					<Test as pallet_members::Config>::MinStake::get(),
+				));
 				assert_ok!(Shards::commit(
 					RawOrigin::Signed(member.account_id.clone()).into(),
 					shard_id as _,