diff --git a/frame/crowdloan-rewards/src/lib.rs b/frame/crowdloan-rewards/src/lib.rs index 253e441c8b4..e056a2d3806 100644 --- a/frame/crowdloan-rewards/src/lib.rs +++ b/frame/crowdloan-rewards/src/lib.rs @@ -47,15 +47,7 @@ Reference for proof mechanism: https://github.com/paritytech/polkadot/blob/maste unused_extern_crates )] -use codec::{Decode, Encode}; -use frame_support::{ - pallet_prelude::{InvalidTransaction, ValidTransaction}, - traits::IsSubType, - unsigned::{TransactionValidity, TransactionValidityError}, -}; pub use pallet::*; -use scale_info::TypeInfo; -use sp_runtime::traits::{DispatchInfoOf, SignedExtension, Zero}; pub mod models; @@ -455,97 +447,42 @@ pub mod pallet { ); Some(addr) } -} - -/// Validate `associate` calls prior to execution. Needed to avoid a DoS attack since they are -/// otherwise free to place on chain. -#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] -#[scale_info(skip_type_params(T))] -pub struct PrevalidateAssociation(sp_std::marker::PhantomData) -where - ::Call: IsSubType>; - -impl sp_std::fmt::Debug for PrevalidateAssociation -where - ::Call: IsSubType>, -{ - #[cfg(feature = "std")] - fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { - write!(f, "PrevalidateAssociation") - } - - #[cfg(not(feature = "std"))] - fn fmt(&self, _: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { - Ok(()) - } -} -#[allow(clippy::new_without_default)] -impl PrevalidateAssociation -where - ::Call: IsSubType>, -{ - /// Create new `SignedExtension` to validate crowdloan rewards association - pub fn new() -> Self { - Self(sp_std::marker::PhantomData) - } -} - -impl SignedExtension for PrevalidateAssociation -where - ::Call: IsSubType>, -{ - type AccountId = T::AccountId; - type Call = ::Call; - type AdditionalSigned = (); - type Pre = (); - const IDENTIFIER: &'static str = "PrevalidateAssociation"; - - fn additional_signed(&self) -> Result { - Ok(()) - } - - // - // The weight of this logic is included in the `associate` dispatchable. - // - fn validate( - &self, - _who: &Self::AccountId, - call: &Self::Call, - _info: &DispatchInfoOf, - _len: usize, - ) -> TransactionValidity { - use frame_support::traits::Get; - - if let Some(Call::associate { reward_account, proof }) = IsSubType::is_sub_type(call) { - if Associations::::get(reward_account).is_some() { - return InvalidTransaction::Custom(ValidityError::AlreadyAssociated as u8).into() + #[pallet::validate_unsigned] + impl ValidateUnsigned for Pallet { + type Call = Call; + + fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { + if let Call::associate { reward_account, proof } = call { + if Associations::::get(reward_account).is_some() { + return InvalidTransaction::Custom(ValidityError::AlreadyAssociated as u8).into() + } + let remote_account = + get_remote_account::(proof.clone(), reward_account, T::Prefix::get()) + .map_err(|_| { + Into::::into(InvalidTransaction::Custom( + ValidityError::InvalidProof as u8, + )) + })?; + match Rewards::::get(remote_account.clone()) { + None => InvalidTransaction::Custom(ValidityError::NoReward as u8).into(), + Some(reward) if reward.total.is_zero() => + InvalidTransaction::Custom(ValidityError::NoReward as u8).into(), + Some(_) => + ValidTransaction::with_tag_prefix("CrowdloanRewardsAssociationCheck") + .and_provides(remote_account) + .build(), + } + } else { + Err(InvalidTransaction::Call.into()) } - - let remote_account = - get_remote_account::(proof.clone(), reward_account, T::Prefix::get()).map_err( - |_| { - Into::::into(InvalidTransaction::Custom( - ValidityError::InvalidProof as u8, - )) - }, - )?; - - match Rewards::::get(remote_account) { - None => InvalidTransaction::Custom(ValidityError::NoReward as u8).into(), - Some(reward) if reward.total.is_zero() => - InvalidTransaction::Custom(ValidityError::NoReward as u8).into(), - Some(_) => Ok(ValidTransaction::default()), - } - } else { - Ok(ValidTransaction::default()) } } -} -#[repr(u8)] -pub enum ValidityError { - InvalidProof = 0, - NoReward = 1, - AlreadyAssociated = 2, + #[repr(u8)] + pub enum ValidityError { + InvalidProof = 0, + NoReward = 1, + AlreadyAssociated = 2, + } } diff --git a/frame/crowdloan-rewards/src/tests.rs b/frame/crowdloan-rewards/src/tests.rs index 63f4b859c8b..e5b69e952fe 100644 --- a/frame/crowdloan-rewards/src/tests.rs +++ b/frame/crowdloan-rewards/src/tests.rs @@ -287,38 +287,21 @@ mod test_prevalidate_association { with_rewards, with_rewards_default, ClaimKey, DEFAULT_NB_OF_CONTRIBUTORS, DEFAULT_VESTING_PERIOD, }; - use crate::{ - mocks::{Call, CrowdloanRewards, Origin, Test}, - PrevalidateAssociation, ValidityError, + mocks::{CrowdloanRewards, Origin}, + ValidityError, }; - use frame_support::{ assert_ok, - dispatch::{Dispatchable, GetDispatchInfo}, - pallet_prelude::InvalidTransaction, + pallet_prelude::{InvalidTransaction, ValidateUnsigned}, unsigned::TransactionValidity, - weights::Pays, }; - use sp_runtime::{traits::SignedExtension, AccountId32}; + use sp_runtime::{transaction_validity::TransactionSource, AccountId32}; - fn setup_call( - remote_account: ClaimKey, - reward_account: &AccountId32, - ) -> (TransactionValidity, Call) { + fn setup_call(remote_account: ClaimKey, reward_account: &AccountId32) -> TransactionValidity { let proof = remote_account.proof(reward_account.clone()); - let call = Call::CrowdloanRewards(crate::Call::associate { - reward_account: reward_account.clone(), - proof, - }); - let dispatch_info = call.get_dispatch_info(); - let validate_result = PrevalidateAssociation::::new().validate( - reward_account, - &call, - &dispatch_info, - 0, - ); - (validate_result, call) + let call = crate::Call::associate { reward_account: reward_account.clone(), proof }; + CrowdloanRewards::validate_unsigned(TransactionSource::External, &call) } #[test] @@ -335,24 +318,11 @@ mod test_prevalidate_association { } for (reward_account, remote_account) in accounts { - let (validate_result, call) = setup_call(remote_account, &reward_account); - + let validate_result = setup_call(remote_account, &reward_account); assert_eq!( validate_result, Err(InvalidTransaction::Custom(ValidityError::AlreadyAssociated as u8).into()) ); - - // make sure that invalid transactions are not free - assert!(matches!( - call.dispatch(Origin::root()), - Err(sp_runtime::DispatchErrorWithPostInfo { - post_info: frame_support::dispatch::PostDispatchInfo { - actual_weight: _, - pays_fee: Pays::Yes - }, - error: _ - }) - )); } }); } @@ -363,24 +333,11 @@ mod test_prevalidate_association { assert_ok!(CrowdloanRewards::initialize(Origin::root())); for (reward_account, remote_account) in accounts { - let (validate_result, call) = setup_call(remote_account, &reward_account); - + let validate_result = setup_call(remote_account, &reward_account); assert_eq!( validate_result, Err(InvalidTransaction::Custom(ValidityError::NoReward as u8).into()) ); - - // make sure that invalid transactions are not free - assert!(matches!( - call.dispatch(Origin::root()), - Err(sp_runtime::DispatchErrorWithPostInfo { - post_info: frame_support::dispatch::PostDispatchInfo { - actual_weight: _, - pays_fee: Pays::Yes - }, - error: _ - }) - )); } }); } diff --git a/integration-tests/simnode/src/chain/dali.rs b/integration-tests/simnode/src/chain/dali.rs index 5b9c5bbe6de..fb6ec2108e3 100644 --- a/integration-tests/simnode/src/chain/dali.rs +++ b/integration-tests/simnode/src/chain/dali.rs @@ -53,7 +53,6 @@ impl substrate_simnode::ChainInfo for ChainInfo { ), system::CheckWeight::::new(), transaction_payment::ChargeTransactionPayment::::from(0), - crowdloan_rewards::PrevalidateAssociation::::new(), ) } } diff --git a/integration-tests/simnode/src/chain/picasso.rs b/integration-tests/simnode/src/chain/picasso.rs index 7d626c3dfc8..19d051b6904 100644 --- a/integration-tests/simnode/src/chain/picasso.rs +++ b/integration-tests/simnode/src/chain/picasso.rs @@ -54,7 +54,6 @@ impl substrate_simnode::ChainInfo for ChainInfo { ), system::CheckWeight::::new(), transaction_payment::ChargeTransactionPayment::::from(0), - crowdloan_rewards::PrevalidateAssociation::::new(), ) } } diff --git a/runtime/dali/src/lib.rs b/runtime/dali/src/lib.rs index 86b4d7089c0..47ffd211774 100644 --- a/runtime/dali/src/lib.rs +++ b/runtime/dali/src/lib.rs @@ -385,7 +385,6 @@ where system::CheckNonce::::from(nonce), system::CheckWeight::::new(), transaction_payment::ChargeTransactionPayment::::from(tip), - crowdloan_rewards::PrevalidateAssociation::::new(), ); let raw_payload = SignedPayload::new(call, extra) .map_err(|_e| { @@ -912,7 +911,7 @@ construct_runtime!( AssetsRegistry: assets_registry::{Pallet, Call, Storage, Event} = 55, GovernanceRegistry: governance_registry::{Pallet, Call, Storage, Event} = 56, Assets: assets::{Pallet, Call, Storage} = 57, - CrowdloanRewards: crowdloan_rewards::{Pallet, Call, Storage, Event} = 58, + CrowdloanRewards: crowdloan_rewards::{Pallet, Call, Storage, Event, ValidateUnsigned} = 58, Vesting: vesting::{Call, Event, Pallet, Storage} = 59, BondedFinance: bonded_finance::{Call, Event, Pallet, Storage} = 60, DutchAuction: dutch_auction::{Pallet, Call, Storage, Event} = 61, @@ -934,7 +933,6 @@ pub type SignedExtra = ( system::CheckNonce, system::CheckWeight, transaction_payment::ChargeTransactionPayment, - crowdloan_rewards::PrevalidateAssociation, ); /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; diff --git a/runtime/picasso/src/lib.rs b/runtime/picasso/src/lib.rs index 63c6db1f682..b067092c386 100644 --- a/runtime/picasso/src/lib.rs +++ b/runtime/picasso/src/lib.rs @@ -103,7 +103,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_version: 2000, impl_version: 1, apis: RUNTIME_API_VERSIONS, - transaction_version: 2, + transaction_version: 1, }; /// The version information used to identify this runtime when compiled natively. @@ -385,7 +385,6 @@ where system::CheckNonce::::from(nonce), system::CheckWeight::::new(), transaction_payment::ChargeTransactionPayment::::from(tip), - crowdloan_rewards::PrevalidateAssociation::::new(), ); let raw_payload = SignedPayload::new(call, extra) .map_err(|_e| { @@ -816,7 +815,7 @@ construct_runtime!( Factory: currency_factory::{Pallet, Storage, Event} = 53, GovernanceRegistry: governance_registry::{Pallet, Call, Storage, Event} = 54, Assets: assets::{Pallet, Call, Storage} = 55, - CrowdloanRewards: crowdloan_rewards::{Pallet, Call, Storage, Event} = 56, + CrowdloanRewards: crowdloan_rewards::{Pallet, Call, Storage, Event, ValidateUnsigned} = 56, Vesting: vesting::{Call, Event, Pallet, Storage} = 57, BondedFinance: bonded_finance::{Call, Event, Pallet, Storage} = 58, } @@ -836,7 +835,6 @@ pub type SignedExtra = ( system::CheckNonce, system::CheckWeight, transaction_payment::ChargeTransactionPayment, - crowdloan_rewards::PrevalidateAssociation, ); /// Unchecked extrinsic type as expected by this runtime.