Skip to content

Commit

Permalink
Do not make pallet-nfts benchmarks signature-dependent (paritytech#4756)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Moliholy authored and TarekkMA committed Aug 2, 2024
1 parent ac006ab commit 4836bf9
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 20 deletions.
15 changes: 15 additions & 0 deletions prdoc/pr_4756.prdoc
Original file line number Diff line number Diff line change
@@ -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
22 changes: 5 additions & 17 deletions substrate/frame/nfts/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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());
Expand All @@ -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());

Expand All @@ -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())
Expand Down
36 changes: 33 additions & 3 deletions substrate/frame/nfts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 4836bf9

Please sign in to comment.