From 2657cfba042e01e619cb8d4f0a71382c6e6a0a08 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Molina=20Colmenero?= <jose@blockdeep.io>
Date: Fri, 21 Jun 2024 15:43:00 +0200
Subject: [PATCH] Do not make pallet-nfts benchmarks signature-dependent
 (#4756)

This PR:

- Adds extra functionality to pallet-nfts's `BenchmarkHelper` to provide
signers and sign message.
- Abstracts away the explicit link with Sr25519 schema in the
benchmarks, allowing parachains with a different one to be able to run
them and calculate the weights.
- Adds a default implementation for the empty tuple that leaves the code
equivalent.
---
 prdoc/pr_4756.prdoc                      | 15 ++++++++++
 substrate/frame/nfts/src/benchmarking.rs | 22 ++++-----------
 substrate/frame/nfts/src/lib.rs          | 36 ++++++++++++++++++++++--
 3 files changed, 53 insertions(+), 20 deletions(-)
 create mode 100644 prdoc/pr_4756.prdoc

diff --git a/prdoc/pr_4756.prdoc b/prdoc/pr_4756.prdoc
new file mode 100644
index 00000000000..064a79fb066
--- /dev/null
+++ b/prdoc/pr_4756.prdoc
@@ -0,0 +1,15 @@
+# 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: Do not make pallet-nfts benchmarks signature-dependent
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      - Adds extra functionality to pallet-nfts's BenchmarkHelper to provide signers and sign message.
+      - Abstracts away the explicit link with Sr25519 schema in the benchmarks, allowing parachains with a different one to be able to run them and calculate the weights.
+      - Adds a default implementation for the empty tuple that leaves the code equivalent.
+
+crates:
+    - name: pallet-nfts
+      bump: minor
diff --git a/substrate/frame/nfts/src/benchmarking.rs b/substrate/frame/nfts/src/benchmarking.rs
index 8792af675fc..80860bc5a53 100644
--- a/substrate/frame/nfts/src/benchmarking.rs
+++ b/substrate/frame/nfts/src/benchmarking.rs
@@ -30,11 +30,7 @@ use frame_support::{
 	BoundedVec,
 };
 use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin as SystemOrigin};
-use sp_io::crypto::{sr25519_generate, sr25519_sign};
-use sp_runtime::{
-	traits::{Bounded, IdentifyAccount, One},
-	AccountId32, MultiSignature, MultiSigner,
-};
+use sp_runtime::traits::{Bounded, One};
 use sp_std::prelude::*;
 
 use crate::Pallet as Nfts;
@@ -229,12 +225,6 @@ fn make_filled_vec(value: u16, length: usize) -> Vec<u8> {
 }
 
 benchmarks_instance_pallet! {
-	where_clause {
-		where
-			T::OffchainSignature: From<MultiSignature>,
-			T::AccountId: From<AccountId32>,
-	}
-
 	create {
 		let collection = T::Helper::collection(0);
 		let origin = T::CreateOrigin::try_successful_origin(&collection)
@@ -800,8 +790,7 @@ benchmarks_instance_pallet! {
 
 	mint_pre_signed {
 		let n in 0 .. T::MaxAttributesPerCall::get() as u32;
-		let caller_public = sr25519_generate(0.into(), None);
-		let caller = MultiSigner::Sr25519(caller_public).into_account().into();
+		let (caller_public, caller) = T::Helper::signer();
 		T::Currency::make_free_balance_be(&caller, DepositBalanceOf::<T, I>::max_value());
 		let caller_lookup = T::Lookup::unlookup(caller.clone());
 
@@ -830,7 +819,7 @@ benchmarks_instance_pallet! {
 			mint_price: Some(DepositBalanceOf::<T, I>::min_value()),
 		};
 		let message = Encode::encode(&mint_data);
-		let signature = MultiSignature::Sr25519(sr25519_sign(0.into(), &caller_public, &message).unwrap());
+		let signature = T::Helper::sign(&caller_public, &message);
 
 		let target: T::AccountId = account("target", 0, SEED);
 		T::Currency::make_free_balance_be(&target, DepositBalanceOf::<T, I>::max_value());
@@ -848,8 +837,7 @@ benchmarks_instance_pallet! {
 		let item_owner: T::AccountId = account("item_owner", 0, SEED);
 		let item_owner_lookup = T::Lookup::unlookup(item_owner.clone());
 
-		let signer_public = sr25519_generate(0.into(), None);
-		let signer: T::AccountId = MultiSigner::Sr25519(signer_public).into_account().into();
+		let (signer_public, signer) = T::Helper::signer();
 
 		T::Currency::make_free_balance_be(&item_owner, DepositBalanceOf::<T, I>::max_value());
 
@@ -876,7 +864,7 @@ benchmarks_instance_pallet! {
 			deadline: One::one(),
 		};
 		let message = Encode::encode(&pre_signed_data);
-		let signature = MultiSignature::Sr25519(sr25519_sign(0.into(), &signer_public, &message).unwrap());
+		let signature = T::Helper::sign(&signer_public, &message);
 
 		frame_system::Pallet::<T>::set_block_number(One::one());
 	}: _(SystemOrigin::Signed(item_owner.clone()), pre_signed_data, signature.into(), signer.clone())
diff --git a/substrate/frame/nfts/src/lib.rs b/substrate/frame/nfts/src/lib.rs
index 615720268fe..0406cac6e2c 100644
--- a/substrate/frame/nfts/src/lib.rs
+++ b/substrate/frame/nfts/src/lib.rs
@@ -84,18 +84,42 @@ pub mod pallet {
 	pub struct Pallet<T, I = ()>(PhantomData<(T, I)>);
 
 	#[cfg(feature = "runtime-benchmarks")]
-	pub trait BenchmarkHelper<CollectionId, ItemId> {
+	pub trait BenchmarkHelper<CollectionId, ItemId, Public, AccountId, Signature> {
 		fn collection(i: u16) -> CollectionId;
 		fn item(i: u16) -> ItemId;
+		fn signer() -> (Public, AccountId);
+		fn sign(signer: &Public, message: &[u8]) -> Signature;
 	}
 	#[cfg(feature = "runtime-benchmarks")]
-	impl<CollectionId: From<u16>, ItemId: From<u16>> BenchmarkHelper<CollectionId, ItemId> for () {
+	impl<CollectionId, ItemId>
+		BenchmarkHelper<
+			CollectionId,
+			ItemId,
+			sp_runtime::MultiSigner,
+			sp_runtime::AccountId32,
+			sp_runtime::MultiSignature,
+		> for ()
+	where
+		CollectionId: From<u16>,
+		ItemId: From<u16>,
+	{
 		fn collection(i: u16) -> CollectionId {
 			i.into()
 		}
 		fn item(i: u16) -> ItemId {
 			i.into()
 		}
+		fn signer() -> (sp_runtime::MultiSigner, sp_runtime::AccountId32) {
+			let public = sp_io::crypto::sr25519_generate(0.into(), None);
+			let account = sp_runtime::MultiSigner::Sr25519(public).into_account();
+			(public.into(), account)
+		}
+		fn sign(signer: &sp_runtime::MultiSigner, message: &[u8]) -> sp_runtime::MultiSignature {
+			sp_runtime::MultiSignature::Sr25519(
+				sp_io::crypto::sr25519_sign(0.into(), &signer.clone().try_into().unwrap(), message)
+					.unwrap(),
+			)
+		}
 	}
 
 	#[pallet::config]
@@ -206,7 +230,13 @@ pub mod pallet {
 
 		#[cfg(feature = "runtime-benchmarks")]
 		/// A set of helper functions for benchmarking.
-		type Helper: BenchmarkHelper<Self::CollectionId, Self::ItemId>;
+		type Helper: BenchmarkHelper<
+			Self::CollectionId,
+			Self::ItemId,
+			Self::OffchainPublic,
+			Self::AccountId,
+			Self::OffchainSignature,
+		>;
 
 		/// Weight information for extrinsics in this pallet.
 		type WeightInfo: WeightInfo;
-- 
GitLab