Skip to content

Commit

Permalink
change(fc-pallet-pass): ditch the skip check extension, use account p…
Browse files Browse the repository at this point in the history 
…roviders to handle the existence/dust of a session key account.
  • Loading branch information
@pandres95
pandres95 committed Nov 20, 2024
1 parent c4d994a commit 46159c9
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 113 deletions.
107 changes: 0 additions & 107 deletions pallets/pass/src/extension.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,110 +104,3 @@ where
S::post_dispatch(pre, info, post_info, len, result)
}
}

/// Skips some checks (like [`frame_system::CheckNonce`]) if the caller is a session key.
///
/// This extension should be deprecated in the future in favour of a couple of `TransactionExtension`s
/// that:
///
/// 1. Issues a new custom origin from `pallet-pass` to wrap the account and skip checks. Then,
/// 2. After those checks, replaces this custom origin with [`frame_system::RawOrigin::Signed`],
/// where the signed `account_id` is the derived `AccountId` associated to the account handled by
/// `pallet-pass`.
#[derive(Encode, Decode)]
pub struct SkipCheckIfPassAccount<S, T, I = ()>(S, PhantomData<(T, I)>);

impl<S: SignedExtension, T, I> SkipCheckIfPassAccount<S, T, I> {
pub fn new(s: S) -> Self {
Self(s, PhantomData)
}
}

impl<S: Clone, T, I> Clone for SkipCheckIfPassAccount<S, T, I> {
fn clone(&self) -> Self {
Self(self.0.clone(), PhantomData)
}
}

impl<S: Eq, T, I> Eq for SkipCheckIfPassAccount<S, T, I> {}
impl<S: PartialEq, T, I> PartialEq for SkipCheckIfPassAccount<S, T, I> {
fn eq(&self, other: &Self) -> bool {
self.0 == other.0
}
}

impl<S: SignedExtension + StaticTypeInfo, T, I> TypeInfo for SkipCheckIfPassAccount<S, T, I> {
type Identity = S;
fn type_info() -> scale_info::Type {
S::type_info()
}
}

impl<S: SignedExtension + Encode, T, I> core::fmt::Debug for SkipCheckIfPassAccount<S, T, I> {
#[cfg(feature = "std")]
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
write!(f, "SkipCheckIfPassAccount<{:?}>", self.0.encode())
}
#[cfg(not(feature = "std"))]
fn fmt(&self, _: &mut core::fmt::Formatter) -> core::fmt::Result {
Ok(())
}
}

impl<S, T, I> SignedExtension for SkipCheckIfPassAccount<S, T, I>
where
T: Config<I> + Send + Sync,
I: 'static + Send + Sync,
S: SignedExtension<AccountId = T::AccountId, Call = <T as frame_system::Config>::RuntimeCall>,
{
const IDENTIFIER: &'static str = S::IDENTIFIER;
type AccountId = S::AccountId;
type Call = S::Call;
type AdditionalSigned = S::AdditionalSigned;
type Pre = Option<S::Pre>;

fn additional_signed(&self) -> Result<Self::AdditionalSigned, TransactionValidityError> {
self.0.additional_signed()
}

fn validate(
&self,
who: &Self::AccountId,
call: &Self::Call,
info: &DispatchInfoOf<Self::Call>,
len: usize,
) -> frame_support::pallet_prelude::TransactionValidity {
if Pallet::<T, I>::signer_from_session_key(who).is_some() {
return Ok(Default::default());
}
self.0.validate(who, call, info, len)
}

fn pre_dispatch(
self,
who: &Self::AccountId,
call: &Self::Call,
info: &DispatchInfoOf<Self::Call>,
len: usize,
) -> Result<Self::Pre, TransactionValidityError> {
if Pallet::<T, I>::signer_from_session_key(who).is_some() {
return Ok(None);
}

self.0.pre_dispatch(who, call, info, len).map(Some)
}

fn post_dispatch(
pre: Option<Self::Pre>,
info: &DispatchInfoOf<Self::Call>,
post_info: &sp_runtime::traits::PostDispatchInfoOf<Self::Call>,
len: usize,
result: &sp_runtime::DispatchResult,
) -> Result<(), TransactionValidityError> {
if let Some(Some(pre)) = pre {
S::post_dispatch(Some(pre), info, post_info, len, result)
} else {
Ok(())
}
}
}
41 changes: 35 additions & 6 deletions pallets/pass/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ pub mod pallet {
) -> DispatchResult {
let who = ensure_signed(origin)?;
let account_id = Self::try_authenticate(&device_id, &credential)?;
Self::do_add_session(&who, &account_id, duration);
Self::try_add_session(&who, &account_id, duration)?;
Ok(())
}

Expand Down Expand Up @@ -197,11 +197,11 @@ pub mod pallet {
};

if let Some(next_session_key) = maybe_next_session_key {
Self::do_add_session(
Self::try_add_session(
&next_session_key,
&account_id,
Some(T::MaxSessionDuration::get()),
);
)?;
}

// Re-dispatch the call on behalf of the caller.
Expand Down Expand Up @@ -287,7 +287,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
let (account_id, until) =
Sessions::<T, I>::get(&who).ok_or(Error::<T, I>::SessionNotFound)?;
if frame_system::Pallet::<T>::block_number() > until {
Sessions::<T, I>::remove(who);
Self::try_remove_session(&who)?;
return Err(Error::<T, I>::SessionExpired.into());
}

Expand Down Expand Up @@ -320,12 +320,39 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Ok(account_id)
}

pub(crate) fn do_add_session(
fn try_remove_session(who: &T::AccountId) -> DispatchResult {
// Decrements the provider reference of this `Session` key account once it's expired.
//
// NOTE: This might not get called at all. We should explore an alternative (maybe a
// task) to remove the provider references on all expired sessions.
frame_system::Pallet::<T>::dec_providers(who)?;
Sessions::<T, I>::remove(who);
Ok(())
}

pub(crate) fn try_add_session(
session_key: &T::AccountId,
account_id: &T::AccountId,
duration: Option<BlockNumberFor<T>>,
) {
) -> DispatchResult {
// Let's try to remove an existing session that uses the same session key (if any). This is
// so we ensure we decrease the provider counter correctly.
if Sessions::<T, I>::contains_key(session_key) {
Self::try_remove_session(session_key)?;
}

let block_number = frame_system::Pallet::<T>::block_number();

// Add a consumer reference to this account, since we'll be using
// it meanwhile it stays active as a Session.
//
// NOTE: It is possible that this session might not be used at all, and therefore, this
// provider reference never removed.
//
// We should explore an alternative (maybe a task) to remove the provider references on all
// expired sessions.
frame_system::Pallet::<T>::inc_providers(session_key);

let session_duration = duration
.unwrap_or(T::MaxSessionDuration::get())
.min(T::MaxSessionDuration::get());
Expand All @@ -337,5 +364,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
session_key: session_key.clone(),
until,
});

Ok(())
}
}

0 comments on commit 46159c9

Please sign in to comment.