feat(fc-pallet-pass): handle provisioning of system accounts for session keys

pallets/pass/src/extension.rs

@@ -1,12 +1,17 @@
 use core::marker::PhantomData;
 
+use crate::{Config, Pallet};
 use codec::{Decode, Encode};
 use frame_support::pallet_prelude::TransactionValidityError;
 use scale_info::{StaticTypeInfo, TypeInfo};
 use sp_runtime::traits::{DispatchInfoOf, SignedExtension};
 
-use crate::{Config, Pallet};
-
+/// Changes the origin account to inner extensions if the signer is a session key, so the validations
+/// and handling of these extensions (like charging to an account) happens on behalf of the `AccountId`
+/// of the account this session key is being associated to.
+///
+/// In the future, this extension would be deprecated in favour of a couple of an extension that issues
+/// authorized origins from `pallet-pass`.
 #[derive(Encode, Decode)]
 pub struct ChargeTransactionToPassAccount<S, T, I = ()>(S, PhantomData<(T, I)>);
 
@@ -67,26 +72,26 @@ where
         self.0.additional_signed()
     }
 
-    fn pre_dispatch(
-        self,
+    fn validate(
+        &self,
         who: &Self::AccountId,
         call: &Self::Call,
         info: &DispatchInfoOf<Self::Call>,
         len: usize,
-    ) -> Result<Self::Pre, TransactionValidityError> {
+    ) -> frame_support::pallet_prelude::TransactionValidity {
         let who = Pallet::<T, I>::signer_from_session_key(who).unwrap_or(who.clone());
-        self.0.pre_dispatch(&who, call, info, len)
+        self.0.validate(&who, call, info, len)
     }
 
-    fn validate(
-        &self,
+    fn pre_dispatch(
+        self,
         who: &Self::AccountId,
         call: &Self::Call,
         info: &DispatchInfoOf<Self::Call>,
         len: usize,
-    ) -> frame_support::pallet_prelude::TransactionValidity {
+    ) -> Result<Self::Pre, TransactionValidityError> {
         let who = Pallet::<T, I>::signer_from_session_key(who).unwrap_or(who.clone());
-        self.0.validate(&who, call, info, len)
+        self.0.pre_dispatch(&who, call, info, len)
     }
 
     fn post_dispatch(
@@ -95,7 +100,7 @@ where
         post_info: &sp_runtime::traits::PostDispatchInfoOf<Self::Call>,
         len: usize,
         result: &sp_runtime::DispatchResult,
-    ) -> Result<(), frame_support::pallet_prelude::TransactionValidityError> {
+    ) -> Result<(), TransactionValidityError> {
         S::post_dispatch(pre, info, post_info, len, result)
     }
 }

pallets/pass/src/lib.rs

@@ -80,7 +80,7 @@
         type BenchmarkHelper: BenchmarkHelper<Self, I>;
     }
 
     #[pallet::pallet]
     pub struct Pallet<T, I = ()>(_);
 
     // Storage
@@ -166,7 +166,7 @@
         ) -> 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(())
         }
 
@@ -197,11 +197,11 @@
             };
 
             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.
@@ -287,7 +287,7 @@
         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());
         }
 
@@ -320,12 +320,39 @@
         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());
@@ -337,5 +364,7 @@
             session_key: session_key.clone(),
             until,
         });
+
+        Ok(())
     }
 }

pallets/referenda-tracks/src/mock.rs

@@ -191,15 +191,13 @@ impl<Class> VoteTally<u32, Class> for Tally {
     fn setup(_: Class, _: Perbill) {}
 }
 
-pub fn new_test_ext(
-    maybe_tracks: Option<
-        Vec<(
-            TrackIdOf<Test, ()>,
-            TrackInfoOf<Test, ()>,
-            PalletsOriginOf<Test>,
-        )>,
-    >,
-) -> sp_io::TestExternalities {
+type TracksVec = Vec<(
+    TrackIdOf<Test, ()>,
+    TrackInfoOf<Test, ()>,
+    PalletsOriginOf<Test>,
+)>;
+
+pub fn new_test_ext(maybe_tracks: Option<TracksVec>) -> sp_io::TestExternalities {
     let balances = vec![(1, 100), (2, 100), (3, 100), (4, 100), (5, 100), (6, 100)];
 
     let t = RuntimeGenesisConfig {
@@ -220,7 +218,6 @@ pub fn new_test_ext(
             }
 
             System::reset_events();
-        } else {
         }
     });
 

traits/authn/src/util.rs

@@ -166,7 +166,7 @@ pub mod dummy {
 
     impl<A> Clone for DummyAttestation<A> {
         fn clone(&self) -> Self {
-            Self(self.0.clone(), PhantomData)
+            Self(self.0, PhantomData)
         }
     }
 
@@ -178,7 +178,7 @@ pub mod dummy {
 
     impl<A> Clone for DummyCredential<A> {
         fn clone(&self) -> Self {
-            Self(self.0.clone(), PhantomData)
+            Self(self.0, PhantomData)
         }
     }
 

traits/gas-tank/src/tests.rs

@@ -190,12 +190,11 @@ mod gas_burner {
     fn it_works_returning_which_item_was_used_to_burn_gas() {
         new_test_ext().execute_with(|| {
             // Assert "small" tank membership
-            let Some(remaining) = MembershipsGas::check_available_gas(
+            let remaining = MembershipsGas::check_available_gas(
                 &SmallMember::get(),
                 &<() as frame_system::WeightInfo>::remark(100),
-            ) else {
-                return assert!(false);
-            };
+            )
+            .expect("gas to burn equals tank capacity; qed");
 
             assert_eq!(
                 MembershipsGas::burn_gas(
@@ -207,35 +206,33 @@ mod gas_burner {
             );
 
             // Assert "medium" tank membership
-            let Some(remaining) = MembershipsGas::check_available_gas(
+            let remaining = MembershipsGas::check_available_gas(
                 &MediumMember::get(),
-                &<() as frame_system::WeightInfo>::remark(100),
-            ) else {
-                return assert!(false);
-            };
+                &<() as frame_system::WeightInfo>::remark(1000),
+            )
+            .expect("gas to burn equals tank capacity; qed");
 
             assert_eq!(
                 MembershipsGas::burn_gas(
-                    &SmallMember::get(),
+                    &MediumMember::get(),
                     &remaining,
-                    &<() as frame_system::WeightInfo>::remark(100)
+                    &<() as frame_system::WeightInfo>::remark(1000)
                 ),
                 Weight::zero()
             );
 
             // Assert "large" tank membership
-            let Some(remaining) = MembershipsGas::check_available_gas(
+            let remaining = MembershipsGas::check_available_gas(
                 &LargeMember::get(),
-                &<() as frame_system::WeightInfo>::remark(1000),
-            ) else {
-                return assert!(false);
-            };
+                &<() as frame_system::WeightInfo>::remark(10000),
+            )
+            .expect("gas to burn equals tank capacity; qed");
 
             assert_eq!(
                 MembershipsGas::burn_gas(
-                    &SmallMember::get(),
+                    &LargeMember::get(),
                     &remaining,
-                    &<() as frame_system::WeightInfo>::remark(1000)
+                    &<() as frame_system::WeightInfo>::remark(10000)
                 ),
                 Weight::zero()
             );

traits/memberships/src/tests.rs

@@ -1,5 +1,3 @@
-use super::*;
-
 use frame_support::{
     assert_ok, construct_runtime, derive_impl, parameter_types,
     traits::{AsEnsureOriginWithArg, ConstU128, ConstU32},