Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion pallets/identity/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl<T: Config> Pallet<T> {
authorized_by: from,
expiry,
auth_id: new_auth_id,
count: 50,
count: T::MaxAuthRetries::get() as u32,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: MaxAuthRetries const is u8 while Authorization.count param is u32, which requires a type conversion, which is safe but may be unnecessary and probably worth unifying the type for consistency. Probably u8 is enough for the purpose of the param (0-255)

};

Authorizations::<T>::insert(target.clone(), new_auth_id, auth);
Expand Down Expand Up @@ -166,6 +166,7 @@ impl<T: Config> Pallet<T> {
auth.expiry
.filter(|&expiry| <pallet_timestamp::Pallet<T>>::get() > expiry)
.is_none()
&& auth.count > 0
})
}

Expand Down Expand Up @@ -224,4 +225,14 @@ impl<T: Config> Pallet<T> {
}
Ok(auth)
}

/// Decreases the authorization count for the given target and auth_id.
pub fn decrease_authorization_count(target: &Signatory<T::AccountId>, auth_id: &u64) {
if let Some(mut auth) = Authorizations::<T>::get(target, auth_id) {
if auth.count > 0 {
auth.count -= 1;
Authorizations::<T>::insert(target, auth_id, auth);
}
}
}
}
4 changes: 4 additions & 0 deletions pallets/identity/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,10 @@ pub mod pallet {
/// Maximum number of authorizations an identity can give.
#[pallet::constant]
type MaxGivenAuths: Get<u32>;

/// Maximum number of authorizations an identity can give.
#[pallet::constant]
type MaxAuthRetries: Get<u8>;
}

#[pallet::event]
Expand Down
322 changes: 211 additions & 111 deletions pallets/runtime/common/src/fee_details.rs

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions pallets/runtime/develop/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ parameter_types! {
// Identity:
pub const InitialPOLYX: Balance = 0;
pub const MaxGivenAuths: u32 = 1024;
pub const MaxAuthRetries: u8 = 10;

// Contracts:
pub Schedule: pallet_contracts::Schedule<Runtime> = Default::default();
Expand Down Expand Up @@ -213,6 +214,7 @@ impl pallet_identity::Config for Runtime {
type SchedulerOrigin = OriginCaller;
type InitialPOLYX = InitialPOLYX;
type MaxGivenAuths = MaxGivenAuths;
type MaxAuthRetries = MaxAuthRetries;
}

impl pallet_committee::Config<GovernanceCommittee> for Runtime {
Expand Down
2 changes: 2 additions & 0 deletions pallets/runtime/mainnet/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ parameter_types! {
BondingDuration::get() as u64 * SessionsPerEra::get() as u64 * EpochDuration::get();

pub const MaxGivenAuths: u32 = 1024;
pub const MaxAuthRetries: u8 = 10;

// State trie Migration
pub const MigrationSignedDepositPerItem: Balance = 0;
Expand All @@ -212,6 +213,7 @@ impl pallet_identity::Config for Runtime {
type SchedulerOrigin = OriginCaller;
type InitialPOLYX = InitialPOLYX;
type MaxGivenAuths = MaxGivenAuths;
type MaxAuthRetries = MaxAuthRetries;
}

impl pallet_committee::Config<GovernanceCommittee> for Runtime {
Expand Down
2 changes: 2 additions & 0 deletions pallets/runtime/testnet/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ parameter_types! {
BondingDuration::get() as u64 * SessionsPerEra::get() as u64 * EpochDuration::get();

pub MaxGivenAuths: u32 = 1024;
pub MaxAuthRetries: u8 = 10;
}

polymesh_runtime_common::misc_pallet_impls!();
Expand All @@ -215,6 +216,7 @@ impl pallet_identity::Config for Runtime {
type SchedulerOrigin = OriginCaller;
type InitialPOLYX = InitialPOLYX;
type MaxGivenAuths = MaxGivenAuths;
type MaxAuthRetries = MaxAuthRetries;
}

impl pallet_committee::Config<GovernanceCommittee> for Runtime {
Expand Down
22 changes: 11 additions & 11 deletions pallets/runtime/tests/src/fee_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ fn cdd_checks() {
&RuntimeCall::MultiSig(multisig::Call::change_sigs_required {
sigs_required: 1
}),
&alice_account
alice_account.clone()
),
Ok(Some(AccountKeyring::Alice.to_account_id()))
);
Expand All @@ -49,7 +49,7 @@ fn cdd_checks() {
assert_noop!(
CddHandler::get_valid_payer(
&RuntimeCall::MultiSig(multisig::Call::accept_multisig_signer { auth_id: 0 }),
&alice_account
alice_account.clone()
),
InvalidTransaction::Custom(TransactionError::InvalidAuthorization as u8)
);
Expand All @@ -66,7 +66,7 @@ fn cdd_checks() {
&RuntimeCall::MultiSig(multisig::Call::accept_multisig_signer {
auth_id: alice_auth_id
}),
&alice_account
alice_account.clone()
),
Ok(Some(AccountKeyring::Charlie.to_account_id()))
);
Expand All @@ -78,7 +78,7 @@ fn cdd_checks() {
auth_id: alice_auth_id,
auth_issuer_pays: true
}),
&alice_account
alice_account.clone()
),
Ok(Some(AccountKeyring::Charlie.to_account_id()))
);
Expand All @@ -90,7 +90,7 @@ fn cdd_checks() {
auth_id: alice_auth_id,
auth_issuer_pays: false
}),
&alice_account
alice_account.clone()
),
Ok(Some(AccountKeyring::Alice.to_account_id()))
);
Expand All @@ -110,7 +110,7 @@ fn cdd_checks() {
auth_id: alice_auth_id,
auth_issuer_pays: false
}),
&alice_account
alice_account.clone()
),
Ok(Some(AccountKeyring::Alice.to_account_id()))
);
Expand All @@ -122,7 +122,7 @@ fn cdd_checks() {
auth_id: alice_auth_id,
auth_issuer_pays: true
}),
&alice_account
alice_account.clone()
),
Ok(Some(AccountKeyring::Charlie.to_account_id()))
);
Expand All @@ -142,7 +142,7 @@ fn cdd_checks() {
auth_id: charlie_auth_id,
auth_issuer_pays: true
}),
&charlie_account
charlie_account.clone()
),
Ok(Some(AccountKeyring::Alice.to_account_id()))
);
Expand All @@ -155,7 +155,7 @@ fn cdd_checks() {
auth_id: charlie_auth_id,
auth_issuer_pays: false
}),
&charlie_account
charlie_account.clone()
),
Ok(Some(AccountKeyring::Charlie.to_account_id()))
);
Expand All @@ -174,7 +174,7 @@ fn cdd_checks() {
&RuntimeCall::MultiSig(multisig::Call::accept_multisig_signer {
auth_id: alice_auth_id
}),
&alice_account
alice_account.clone()
),
Ok(Some(AccountKeyring::Charlie.to_account_id()))
);
Expand All @@ -185,7 +185,7 @@ fn cdd_checks() {
&RuntimeCall::MultiSig(multisig::Call::change_sigs_required {
sigs_required: 1
}),
&charlie_account
charlie_account
),
Ok(Some(AccountKeyring::Charlie.to_account_id()))
);
Expand Down
2 changes: 1 addition & 1 deletion pallets/runtime/tests/src/identity_test.rs
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably worth adding some basic authorization tests to check if retry count is properly decreased and user is rejected when count reaches 0

Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ fn frozen_secondary_keys_cdd_verification_test_we() {
value: 1_000,
memo: None,
}),
&AccountKeyring::Bob.to_account_id(),
AccountKeyring::Bob.to_account_id(),
);
assert!(payer.is_ok());

Expand Down
2 changes: 1 addition & 1 deletion pallets/runtime/tests/src/relayer_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ fn do_relayer_accept_cdd_and_fees_test() {
assert_eq!(
CddHandler::get_valid_payer(
&DevRuntimeCall::Relayer(pallet_relayer::Call::accept_paying_key { auth_id }),
&bob.acc()
bob.acc()
),
Ok(Some(alice.acc()))
);
Expand Down
8 changes: 7 additions & 1 deletion pallets/runtime/tests/src/staking/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,13 +307,15 @@ impl pallet_identity::Config for Test {
type SchedulerOrigin = OriginCaller;
type InitialPOLYX = InitialPOLYX;
type MaxGivenAuths = MaxGivenAuths;
type MaxAuthRetries = MaxAuthRetries;
}

parameter_types! {
pub const InitialPOLYX: Balance = 0;
pub MaximumSchedulerWeight: Weight = Perbill::from_percent(80) * MaximumBlockWeight::get();
pub const MaxScheduledPerBlock: u32 = 50;
pub const MaxGivenAuths: u32 = 1024;
pub const MaxAuthRetries: u8 = 10;
}

impl pallet_scheduler::Config for Test {
Expand Down Expand Up @@ -347,7 +349,7 @@ impl pallet_preimage::Config for Test {
impl polymesh_primitives::traits::CddAndFeeDetails<AccountId, Call> for Test {
fn get_valid_payer(
_: &Call,
_: &AccountId,
_: AccountId,
) -> Result<Option<AccountId>, sp_runtime::transaction_validity::InvalidTransaction> {
Ok(None)
}
Expand All @@ -356,6 +358,10 @@ impl polymesh_primitives::traits::CddAndFeeDetails<AccountId, Call> for Test {
fn get_payer_from_context() -> Option<AccountId> {
None
}
fn decrease_authorization_count(_caller: &AccountId, _auth_id: Option<u64>) {}
fn get_authorization_id(_call: &RuntimeCall) -> Option<u64> {
None
}
}

impl SubsidiserTrait<AccountId, RuntimeCall> for Test {
Expand Down
9 changes: 7 additions & 2 deletions pallets/runtime/tests/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ parameter_types! {
pub const MaxInstructionMediators: u32 = 4;
pub const MaxAssetMediators: u32 = 4;
pub const MaxGivenAuths: u32 = 1024;
pub const MaxAuthRetries: u8 = 10;
pub const MigrationSignedDepositPerItem: Balance = 0;
pub const MigrationSignedDepositBase: Balance = 0;
pub const MaxKeyLen: u32 = 2048;
Expand Down Expand Up @@ -502,9 +503,8 @@ type CddHandler = TestStorage;
impl CddAndFeeDetails<AccountId, RuntimeCall> for TestStorage {
fn get_valid_payer(
_: &RuntimeCall,
caller: &AccountId,
caller: AccountId,
) -> Result<Option<AccountId>, InvalidTransaction> {
let caller: AccountId = caller.clone();
Ok(Some(caller))
}
fn clear_context() {
Expand All @@ -516,6 +516,10 @@ impl CddAndFeeDetails<AccountId, RuntimeCall> for TestStorage {
fn get_payer_from_context() -> Option<AccountId> {
Context::current_payer::<Identity>()
}
fn decrease_authorization_count(_caller: &AccountId, _auth_id: Option<u64>) {}
fn get_authorization_id(_call: &RuntimeCall) -> Option<u64> {
None
}
}

pub struct WeightToFee;
Expand Down Expand Up @@ -625,6 +629,7 @@ impl pallet_identity::Config for TestStorage {
type SchedulerOrigin = OriginCaller;
type InitialPOLYX = InitialPOLYX;
type MaxGivenAuths = MaxGivenAuths;
type MaxAuthRetries = MaxAuthRetries;
}

impl example::Config for TestStorage {}
Expand Down
21 changes: 15 additions & 6 deletions pallets/transaction-payment/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ where

fn withdraw_fee(
&self,
who: &T::AccountId,
who: T::AccountId,
call: &T::RuntimeCall,
info: &DispatchInfoOf<T::RuntimeCall>,
len: usize,
Expand Down Expand Up @@ -858,6 +858,8 @@ where
<<T as Config>::OnChargeTransaction as OnChargeTransaction<T>>::LiquidityInfo,
// Polymesh: Subsidiser
Option<Self::AccountId>,
// The AuthId of the call
Option<u64>,
);
fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> {
Ok(())
Expand All @@ -872,7 +874,7 @@ where
) -> TransactionValidity {
let tip = self.ensure_valid_tip(who, info)?;

let (_fee, _, _) = self.withdraw_fee(who, call, info, len)?;
let (_fee, _, _) = self.withdraw_fee(who.clone(), call, info, len)?;
// Polymesh: `tip` can only be used by GC/CDD members.
Ok(ValidTransaction {
priority: tip.saturated_into::<TransactionPriority>(),
Expand All @@ -888,21 +890,28 @@ where
len: usize,
) -> Result<Self::Pre, TransactionValidityError> {
let tip = self.ensure_valid_tip(who, info)?;
let (_fee, imbalance, subsidiser) = self.withdraw_fee(who, call, info, len)?;
Ok((tip, who.clone(), imbalance, subsidiser))
let (_fee, imbalance, subsidiser) = self.withdraw_fee(who.clone(), call, info, len)?;
let auth_id = T::CddHandler::get_authorization_id(call);
Ok((tip, who.clone(), imbalance, subsidiser, auth_id))
}

fn post_dispatch(
pre: Option<Self::Pre>,
info: &DispatchInfoOf<Self::Call>,
post_info: &PostDispatchInfoOf<Self::Call>,
len: usize,
_result: &DispatchResult,
result: &DispatchResult,
) -> Result<(), TransactionValidityError> {
let (tip, who, imbalance, subsidiser) = match pre {
let (tip, who, imbalance, subsidiser, auth_id) = match pre {
Some(pre) => pre,
None => return Ok(()),
};

// We want to decrease the counter of Authorization.count when the caller is not paying for the fee and the call failed
if result.is_err() {
T::CddHandler::decrease_authorization_count(&who, auth_id);
}

let actual_fee = Pallet::<T>::compute_actual_fee(len as u32, info, post_info, tip);

// Fee returned to original payer.
Expand Down
16 changes: 15 additions & 1 deletion primitives/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,27 @@ pub use asset::*;

// Polymesh note: This was specifically added for Polymesh
pub trait CddAndFeeDetails<AccountId, Call> {
/// Returns the account that will pay for the call.
fn get_valid_payer(
call: &Call,
caller: &AccountId,
caller: AccountId,
) -> Result<Option<AccountId>, InvalidTransaction>;
/// Clears context. Should be called in post_dispatch
fn clear_context();
/// Sets payer in context. Should be called by the signed extension that first charges fee.
fn set_payer_context(payer: Option<AccountId>);
/// Fetches fee payer for further payments (forwarded calls)
fn get_payer_from_context() -> Option<AccountId>;
/// Decreases the authorization count if any of the following extrinsics failed:
/// - pallet-identity (accept_primary_key, join_identity_as_key, rotate_primary_key_to_secondary)
/// - pallet-relayer (accept_paying_key)
/// - pallet-multisig (accept_multisig_signer)
fn decrease_authorization_count(caller: &AccountId, auth_id: Option<u64>);
/// Returns Some(auth_id) of the call if any of the following extrinsics are called:
/// - pallet-identity (accept_primary_key, join_identity_as_key, rotate_primary_key_to_secondary)
/// - pallet-relayer (accept_paying_key)
/// - pallet-multisig (accept_multisig_signer)
fn get_authorization_id(call: &Call) -> Option<u64>;
}

pub trait CheckCdd<AccountId> {
Expand Down