Skip to content

Commit

Permalink
Associated type Hasher for QueryPreimage, StorePreimage and `Boun…
Browse files Browse the repository at this point in the history
…ded` (paritytech#1720)

I hope it's enough to fix paritytech#1701 
the only solution I found to make it happen is to put an associated type
to the `Bounded` enum as well.
@liamaharon @kianenigma @bkchr 

Polkadot address: 12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp

---------

Signed-off-by: muraca <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
  • Loading branch information
3 people authored Sep 27, 2023
1 parent 7f9c125 commit f777d3f
Show file tree
Hide file tree
Showing 20 changed files with 189 additions and 159 deletions.
7 changes: 3 additions & 4 deletions substrate/frame/democracy/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use frame_support::{
traits::{Currency, EnsureOrigin, Get, OnInitialize, UnfilteredDispatchable},
};
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
use sp_core::H256;
use sp_runtime::{traits::Bounded, BoundedVec};

use crate::Pallet as Democracy;
Expand All @@ -46,7 +45,7 @@ fn make_proposal<T: Config>(n: u32) -> BoundedCallOf<T> {
<T as Config>::Preimages::bound(call).unwrap()
}

fn add_proposal<T: Config>(n: u32) -> Result<H256, &'static str> {
fn add_proposal<T: Config>(n: u32) -> Result<T::Hash, &'static str> {
let other = funded_account::<T>("proposer", n);
let value = T::MinimumDeposit::get();
let proposal = make_proposal::<T>(n);
Expand All @@ -55,7 +54,7 @@ fn add_proposal<T: Config>(n: u32) -> Result<H256, &'static str> {
}

// add a referendum with a metadata.
fn add_referendum<T: Config>(n: u32) -> (ReferendumIndex, H256, PreimageHash) {
fn add_referendum<T: Config>(n: u32) -> (ReferendumIndex, T::Hash, T::Hash) {
let vote_threshold = VoteThreshold::SimpleMajority;
let proposal = make_proposal::<T>(n);
let hash = proposal.hash();
Expand Down Expand Up @@ -85,7 +84,7 @@ fn assert_has_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
}

// note a new preimage.
fn note_preimage<T: Config>() -> PreimageHash {
fn note_preimage<T: Config>() -> T::Hash {
use core::sync::atomic::{AtomicU8, Ordering};
use sp_std::borrow::Cow;
// note a new preimage on every function invoke.
Expand Down
54 changes: 29 additions & 25 deletions substrate/frame/democracy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,8 @@ use frame_support::{
traits::{
defensive_prelude::*,
schedule::{v3::Named as ScheduleNamed, DispatchTime},
Bounded, Currency, EnsureOrigin, Get, Hash as PreimageHash, LockIdentifier,
LockableCurrency, OnUnbalanced, QueryPreimage, ReservableCurrency, StorePreimage,
WithdrawReasons,
Bounded, Currency, EnsureOrigin, Get, LockIdentifier, LockableCurrency, OnUnbalanced,
QueryPreimage, ReservableCurrency, StorePreimage, WithdrawReasons,
},
weights::Weight,
};
Expand Down Expand Up @@ -203,15 +202,14 @@ type NegativeImbalanceOf<T> = <<T as Config>::Currency as Currency<
<T as frame_system::Config>::AccountId,
>>::NegativeImbalance;
pub type CallOf<T> = <T as frame_system::Config>::RuntimeCall;
pub type BoundedCallOf<T> = Bounded<CallOf<T>>;
pub type BoundedCallOf<T> = Bounded<CallOf<T>, <T as frame_system::Config>::Hashing>;
type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;

#[frame_support::pallet]
pub mod pallet {
use super::{DispatchResult, *};
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;
use sp_core::H256;

/// The current storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
Expand All @@ -226,10 +224,15 @@ pub mod pallet {
type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;

/// The Scheduler.
type Scheduler: ScheduleNamed<BlockNumberFor<Self>, CallOf<Self>, Self::PalletsOrigin>;
type Scheduler: ScheduleNamed<
BlockNumberFor<Self>,
CallOf<Self>,
Self::PalletsOrigin,
Hasher = Self::Hashing,
>;

/// The Preimage provider.
type Preimages: QueryPreimage + StorePreimage;
type Preimages: QueryPreimage<H = Self::Hashing> + StorePreimage;

/// Currency type for this pallet.
type Currency: ReservableCurrency<Self::AccountId>
Expand Down Expand Up @@ -421,22 +424,22 @@ pub mod pallet {
pub type Blacklist<T: Config> = StorageMap<
_,
Identity,
H256,
T::Hash,
(BlockNumberFor<T>, BoundedVec<T::AccountId, T::MaxBlacklisted>),
>;

/// Record of all proposals that have been subject to emergency cancellation.
#[pallet::storage]
pub type Cancellations<T: Config> = StorageMap<_, Identity, H256, bool, ValueQuery>;
pub type Cancellations<T: Config> = StorageMap<_, Identity, T::Hash, bool, ValueQuery>;

/// General information concerning any proposal or referendum.
/// The `PreimageHash` refers to the preimage of the `Preimages` provider which can be a JSON
/// The `Hash` refers to the preimage of the `Preimages` provider which can be a JSON
/// dump or IPFS hash of a JSON file.
///
/// Consider a garbage collection for a metadata of finished referendums to `unrequest` (remove)
/// large preimages.
#[pallet::storage]
pub type MetadataOf<T: Config> = StorageMap<_, Blake2_128Concat, MetadataOwner, PreimageHash>;
pub type MetadataOf<T: Config> = StorageMap<_, Blake2_128Concat, MetadataOwner, T::Hash>;

#[pallet::genesis_config]
#[derive(frame_support::DefaultNoBound)]
Expand Down Expand Up @@ -476,9 +479,9 @@ pub mod pallet {
/// An account has cancelled a previous delegation operation.
Undelegated { account: T::AccountId },
/// An external proposal has been vetoed.
Vetoed { who: T::AccountId, proposal_hash: H256, until: BlockNumberFor<T> },
Vetoed { who: T::AccountId, proposal_hash: T::Hash, until: BlockNumberFor<T> },
/// A proposal_hash has been blacklisted permanently.
Blacklisted { proposal_hash: H256 },
Blacklisted { proposal_hash: T::Hash },
/// An account has voted in a referendum
Voted { voter: T::AccountId, ref_index: ReferendumIndex, vote: AccountVote<BalanceOf<T>> },
/// An account has secconded a proposal
Expand All @@ -490,14 +493,14 @@ pub mod pallet {
/// Metadata owner.
owner: MetadataOwner,
/// Preimage hash.
hash: PreimageHash,
hash: T::Hash,
},
/// Metadata for a proposal or a referendum has been cleared.
MetadataCleared {
/// Metadata owner.
owner: MetadataOwner,
/// Preimage hash.
hash: PreimageHash,
hash: T::Hash,
},
/// Metadata has been transferred to new owner.
MetadataTransferred {
Expand All @@ -506,7 +509,7 @@ pub mod pallet {
/// New metadata owner.
owner: MetadataOwner,
/// Preimage hash.
hash: PreimageHash,
hash: T::Hash,
},
}

Expand Down Expand Up @@ -775,7 +778,7 @@ pub mod pallet {
#[pallet::weight(T::WeightInfo::fast_track())]
pub fn fast_track(
origin: OriginFor<T>,
proposal_hash: H256,
proposal_hash: T::Hash,
voting_period: BlockNumberFor<T>,
delay: BlockNumberFor<T>,
) -> DispatchResult {
Expand Down Expand Up @@ -827,7 +830,7 @@ pub mod pallet {
/// Weight: `O(V + log(V))` where V is number of `existing vetoers`
#[pallet::call_index(8)]
#[pallet::weight(T::WeightInfo::veto_external())]
pub fn veto_external(origin: OriginFor<T>, proposal_hash: H256) -> DispatchResult {
pub fn veto_external(origin: OriginFor<T>, proposal_hash: T::Hash) -> DispatchResult {
let who = T::VetoOrigin::ensure_origin(origin)?;

if let Some((ext_proposal, _)) = NextExternal::<T>::get() {
Expand Down Expand Up @@ -1042,7 +1045,7 @@ pub mod pallet {
#[pallet::weight((T::WeightInfo::blacklist(), DispatchClass::Operational))]
pub fn blacklist(
origin: OriginFor<T>,
proposal_hash: H256,
proposal_hash: T::Hash,
maybe_ref_index: Option<ReferendumIndex>,
) -> DispatchResult {
T::BlacklistOrigin::ensure_origin(origin)?;
Expand Down Expand Up @@ -1139,7 +1142,7 @@ pub mod pallet {
pub fn set_metadata(
origin: OriginFor<T>,
owner: MetadataOwner,
maybe_hash: Option<PreimageHash>,
maybe_hash: Option<T::Hash>,
) -> DispatchResult {
match owner {
MetadataOwner::External => {
Expand Down Expand Up @@ -1173,15 +1176,16 @@ pub mod pallet {
}

pub trait EncodeInto: Encode {
fn encode_into<T: AsMut<[u8]> + Default>(&self) -> T {
fn encode_into<T: AsMut<[u8]> + Default, H: sp_core::Hasher>(&self) -> T {
let mut t = T::default();
self.using_encoded(|data| {
if data.len() <= t.as_mut().len() {
t.as_mut()[0..data.len()].copy_from_slice(data);
} else {
// encoded self is too big to fit into a T. hash it and use the first bytes of that
// instead.
let hash = sp_io::hashing::blake2_256(data);
// encoded self is too big to fit into a T.
// hash it and use the first bytes of that instead.
let hash = H::hash(&data);
let hash = hash.as_ref();
let l = t.as_mut().len().min(hash.len());
t.as_mut()[0..l].copy_from_slice(&hash[0..l]);
}
Expand Down Expand Up @@ -1610,7 +1614,7 @@ impl<T: Config> Pallet<T> {
// Earliest it can be scheduled for is next block.
let when = now.saturating_add(status.delay.max(One::one()));
if T::Scheduler::schedule_named(
(DEMOCRACY_ID, index).encode_into(),
(DEMOCRACY_ID, index).encode_into::<_, T::Hashing>(),
DispatchTime::At(when),
None,
63,
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/democracy/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ fn tally(r: ReferendumIndex) -> Tally<u64> {
}

/// note a new preimage without registering.
fn note_preimage(who: u64) -> PreimageHash {
fn note_preimage(who: u64) -> <Test as frame_system::Config>::Hash {
use std::sync::atomic::{AtomicU8, Ordering};
// note a new preimage on every function invoke.
static COUNTER: AtomicU8 = AtomicU8::new(0);
Expand Down
6 changes: 2 additions & 4 deletions substrate/frame/democracy/src/tests/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ use super::*;
#[test]
fn set_external_metadata_works() {
new_test_ext().execute_with(|| {
use frame_support::traits::Hash as PreimageHash;
// invalid preimage hash.
let invalid_hash: PreimageHash = [1u8; 32].into();
let invalid_hash: <Test as frame_system::Config>::Hash = [1u8; 32].into();
// metadata owner is an external proposal.
let owner = MetadataOwner::External;
// fails to set metadata if an external proposal does not exist.
Expand Down Expand Up @@ -83,9 +82,8 @@ fn clear_metadata_works() {
#[test]
fn set_proposal_metadata_works() {
new_test_ext().execute_with(|| {
use frame_support::traits::Hash as PreimageHash;
// invalid preimage hash.
let invalid_hash: PreimageHash = [1u8; 32].into();
let invalid_hash: <Test as frame_system::Config>::Hash = [1u8; 32].into();
// create an external proposal.
assert_ok!(propose_set_balance(1, 2, 5));
// metadata owner is a public proposal.
Expand Down
10 changes: 6 additions & 4 deletions substrate/frame/preimage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ use frame_support::{
ensure,
pallet_prelude::Get,
traits::{
Consideration, Currency, Defensive, FetchResult, Footprint, Hash as PreimageHash,
PreimageProvider, PreimageRecipient, QueryPreimage, ReservableCurrency, StorePreimage,
Consideration, Currency, Defensive, FetchResult, Footprint, PreimageProvider,
PreimageRecipient, QueryPreimage, ReservableCurrency, StorePreimage,
},
BoundedSlice, BoundedVec,
};
Expand Down Expand Up @@ -531,7 +531,9 @@ impl<T: Config> PreimageRecipient<T::Hash> for Pallet<T> {
}
}

impl<T: Config<Hash = PreimageHash>> QueryPreimage for Pallet<T> {
impl<T: Config> QueryPreimage for Pallet<T> {
type H = T::Hashing;

fn len(hash: &T::Hash) -> Option<u32> {
Pallet::<T>::len(hash)
}
Expand All @@ -555,7 +557,7 @@ impl<T: Config<Hash = PreimageHash>> QueryPreimage for Pallet<T> {
}
}

impl<T: Config<Hash = PreimageHash>> StorePreimage for Pallet<T> {
impl<T: Config> StorePreimage for Pallet<T> {
const MAX_LENGTH: usize = MAX_SIZE as usize;

fn note(bytes: Cow<[u8]>) -> Result<T::Hash, DispatchError> {
Expand Down
29 changes: 18 additions & 11 deletions substrate/frame/preimage/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,32 @@ use crate::mock::*;

use frame_support::{
assert_err, assert_noop, assert_ok, assert_storage_noop,
traits::{fungible::InspectHold, Bounded, BoundedInline, Hash as PreimageHash},
traits::{fungible::InspectHold, Bounded, BoundedInline},
StorageNoopGuard,
};
use sp_core::{blake2_256, H256};
use sp_runtime::{bounded_vec, TokenError};

/// Returns one `Inline`, `Lookup` and `Legacy` item each with different data and hash.
pub fn make_bounded_values() -> (Bounded<Vec<u8>>, Bounded<Vec<u8>>, Bounded<Vec<u8>>) {
pub fn make_bounded_values() -> (
Bounded<Vec<u8>, <Test as frame_system::Config>::Hashing>,
Bounded<Vec<u8>, <Test as frame_system::Config>::Hashing>,
Bounded<Vec<u8>, <Test as frame_system::Config>::Hashing>,
) {
let data: BoundedInline = bounded_vec![1];
let inline = Bounded::<Vec<u8>>::Inline(data);
let inline = Bounded::<Vec<u8>, <Test as frame_system::Config>::Hashing>::Inline(data);

let data = vec![1, 2];
let hash: H256 = blake2_256(&data[..]).into();
let hash = <Test as frame_system::Config>::Hashing::hash(&data[..]).into();
let len = data.len() as u32;
let lookup = Bounded::<Vec<u8>>::unrequested(hash, len);
let lookup =
Bounded::<Vec<u8>, <Test as frame_system::Config>::Hashing>::unrequested(hash, len);

let data = vec![1, 2, 3];
let hash: H256 = blake2_256(&data[..]).into();
let legacy = Bounded::<Vec<u8>>::Legacy { hash, dummy: Default::default() };
let hash = <Test as frame_system::Config>::Hashing::hash(&data[..]).into();
let legacy = Bounded::<Vec<u8>, <Test as frame_system::Config>::Hashing>::Legacy {
hash,
dummy: Default::default(),
};

(inline, lookup, legacy)
}
Expand Down Expand Up @@ -303,7 +310,7 @@ fn query_and_store_preimage_workflow() {
let bound = Preimage::bound(data.clone()).unwrap();
let (len, hash) = (bound.len().unwrap(), bound.hash());

assert_eq!(hash, blake2_256(&encoded).into());
assert_eq!(hash, <Test as frame_system::Config>::Hashing::hash(&encoded).into());
assert_eq!(bound.len(), Some(len));
assert!(bound.lookup_needed(), "Should not be Inlined");
assert_eq!(bound.lookup_len(), Some(len));
Expand Down Expand Up @@ -364,7 +371,7 @@ fn query_preimage_request_works() {
new_test_ext().execute_with(|| {
let _guard = StorageNoopGuard::default();
let data: Vec<u8> = vec![1; 10];
let hash: PreimageHash = blake2_256(&data[..]).into();
let hash = <Test as frame_system::Config>::Hashing::hash(&data[..]).into();

// Request the preimage.
<Preimage as QueryPreimage>::request(&hash);
Expand Down Expand Up @@ -454,7 +461,7 @@ fn store_preimage_basic_works() {

// Cleanup.
<Preimage as StorePreimage>::unnote(&bound.hash());
let data_hash = blake2_256(&data);
let data_hash = <Test as frame_system::Config>::Hashing::hash(&data);
<Preimage as StorePreimage>::unnote(&data_hash.into());

// No storage changes remain. Checked by `StorageNoopGuard`.
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/referenda/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use frame_benchmarking::v1::{
};
use frame_support::{
assert_ok,
traits::{Bounded, Currency, EnsureOrigin, EnsureOriginWithArg, UnfilteredDispatchable},
traits::{Currency, EnsureOrigin, EnsureOriginWithArg, UnfilteredDispatchable},
};
use frame_system::RawOrigin;
use sp_runtime::traits::Bounded as ArithBounded;
Expand All @@ -42,7 +42,7 @@ fn funded_account<T: Config<I>, I: 'static>(name: &'static str, index: u32) -> T
caller
}

fn dummy_call<T: Config<I>, I: 'static>() -> Bounded<<T as Config<I>>::RuntimeCall> {
fn dummy_call<T: Config<I>, I: 'static>() -> BoundedCallOf<T, I> {
let inner = frame_system::Call::remark { remark: vec![] };
let call = <T as Config<I>>::RuntimeCall::from(inner);
T::Preimages::bound(call).unwrap()
Expand Down
Loading

0 comments on commit f777d3f

Please sign in to comment.