fix: persist Applications instead of deleting and Remove applications (…

…#33)

Applications that are resolved (accepted/rejected) or expired will be
persisted in the chain state instead of being deleted to persist the changes
on the ledger and improve the consumption workflow of this data.

Also, now proposals can be used to _remove_ a key from the whitelist.

# Pull Request Checklist

Before submitting this PR, please make sure:

- [x] You have run `cargo clippy` and addressed any warnings
- [ ] You have added appropriate tests (if applicable)
- [ ] You have updated the documentation (if applicable)
- [x] You have reviewed your own code
- [ ] You have updated changelog (if applicable)

## Description

Please provide a brief description of the changes in this PR.

## Related Issues

Please link any related issues here

pallets/governance/src/application.rs

@@ -1,5 +1,5 @@
 use crate::frame::traits::ExistenceRequirement;
-use crate::{whitelist, AccountIdOf, BalanceOf, Block};
+use crate::{whitelist, AccountIdOf, AgentApplications, BalanceOf, Block};
 use codec::{Decode, Encode, MaxEncodedLen};
 use polkadot_sdk::frame_election_provider_support::Get;
 use polkadot_sdk::frame_support::dispatch::DispatchResult;
@@ -19,15 +19,43 @@ pub struct AgentApplication<T: crate::Config> {
     pub data: BoundedVec<u8, T::MaxApplicationDataLength>,
     pub cost: BalanceOf<T>,
     pub expires_at: Block,
+    pub action: ApplicationAction,
+    pub status: ApplicationStatus,
+}
+
+#[derive(DebugNoBound, Decode, Encode, TypeInfo, MaxEncodedLen, PartialEq, Eq)]
+pub enum ApplicationAction {
+    Add,
+    Remove,
+}
+
+#[derive(DebugNoBound, Decode, Encode, TypeInfo, MaxEncodedLen, PartialEq, Eq)]
+pub enum ApplicationStatus {
+    Open,
+    Resolved { accepted: bool },
+    Expired,
 }
 
 pub fn submit_application<T: crate::Config>(
     payer: AccountIdOf<T>,
     agent_key: AccountIdOf<T>,
     data: Vec<u8>,
+    removing: bool,
 ) -> DispatchResult {
-    if whitelist::is_whitelisted::<T>(&agent_key) {
+    if !removing && whitelist::is_whitelisted::<T>(&agent_key) {
         return Err(crate::Error::<T>::AlreadyWhitelisted.into());
+    } else if removing && !whitelist::is_whitelisted::<T>(&agent_key) {
+        return Err(crate::Error::<T>::NotWhitelisted.into());
+    }
+
+    let action = if removing {
+        ApplicationAction::Remove
+    } else {
+        ApplicationAction::Add
+    };
+
+    if exists_for_agent_key::<T>(&agent_key, &action) {
+        return Err(crate::Error::<T>::ApplicationKeyAlreadyUsed.into());
     }
 
     let config = crate::GlobalGovernanceConfig::<T>::get();
@@ -58,23 +86,24 @@ pub fn submit_application<T: crate::Config>(
 
     let expires_at = current_block + config.agent_application_expiration;
 
-    let application_id: u32 = crate::AgentApplicationId::<T>::mutate(|id| {
-        let last_id = *id;
-        *id = id.saturating_add(1);
-        last_id
-    });
+    let next_id = AgentApplications::<T>::iter()
+        .count()
+        .try_into()
+        .map_err(|_| crate::Error::<T>::InternalError)?;
 
     let application = AgentApplication::<T> {
-        id: application_id,
+        id: next_id,
         payer_key: payer,
         agent_key,
         data: BoundedVec::truncate_from(data),
         cost,
         expires_at,
+        status: ApplicationStatus::Open,
+        action,
     };
 
-    crate::AgentApplications::<T>::insert(application_id, application);
-    crate::Pallet::<T>::deposit_event(crate::Event::<T>::ApplicationCreated(application_id));
+    crate::AgentApplications::<T>::insert(next_id, application);
+    crate::Pallet::<T>::deposit_event(crate::Event::<T>::ApplicationCreated(next_id));
 
     Ok(())
 }
@@ -83,15 +112,32 @@ pub fn accept_application<T: crate::Config>(application_id: u32) -> DispatchResu
     let application = crate::AgentApplications::<T>::get(application_id)
         .ok_or(crate::Error::<T>::ApplicationNotFound)?;
 
-    crate::AgentApplications::<T>::remove(application.id);
+    match application.action {
+        ApplicationAction::Add => {
+            crate::Whitelist::<T>::insert(application.agent_key.clone(), ());
+            crate::Pallet::<T>::deposit_event(crate::Event::<T>::WhitelistAdded(
+                application.agent_key,
+            ));
+        }
+        ApplicationAction::Remove => {
+            crate::Whitelist::<T>::remove(&application.agent_key);
+            crate::Pallet::<T>::deposit_event(crate::Event::<T>::WhitelistRemoved(
+                application.agent_key,
+            ));
+        }
+    }
 
-    whitelist::add_to_whitelist::<T>(application.agent_key.clone())?;
+    crate::AgentApplications::<T>::mutate(application_id, |application| {
+        if let Some(app) = application {
+            app.status = ApplicationStatus::Resolved { accepted: true };
+        }
+    });
 
+    // Pay the application cost back to the applicant
     let _ =
         <T as crate::Config>::Currency::deposit_creating(&application.payer_key, application.cost);
 
     crate::Pallet::<T>::deposit_event(crate::Event::<T>::ApplicationAccepted(application.id));
-    crate::Pallet::<T>::deposit_event(crate::Event::<T>::WhitelistAdded(application.agent_key));
 
     Ok(())
 }
@@ -100,19 +146,40 @@ pub fn deny_application<T: crate::Config>(application_id: u32) -> DispatchResult
     let application = crate::AgentApplications::<T>::get(application_id)
         .ok_or(crate::Error::<T>::ApplicationNotFound)?;
 
-    crate::AgentApplications::<T>::remove(application.id);
+    crate::AgentApplications::<T>::mutate(application_id, |application| {
+        if let Some(app) = application {
+            app.status = ApplicationStatus::Resolved { accepted: false };
+        }
+    });
+
     crate::Pallet::<T>::deposit_event(crate::Event::<T>::ApplicationDenied(application.id));
 
     Ok(())
 }
 
-pub(crate) fn remove_expired_applications<T: crate::Config>(current_block: Block) {
+pub(crate) fn resolve_expired_applications<T: crate::Config>(current_block: Block) {
     for application in crate::AgentApplications::<T>::iter_values() {
         if current_block < application.expires_at {
             continue;
         }
 
-        crate::AgentApplications::<T>::remove(application.id);
+        crate::AgentApplications::<T>::mutate(application.id, |application| {
+            if let Some(app) = application {
+                app.status = ApplicationStatus::Expired;
+            }
+        });
+
         crate::Pallet::<T>::deposit_event(crate::Event::<T>::ApplicationExpired(application.id));
     }
 }
+
+pub(crate) fn exists_for_agent_key<T: crate::Config>(
+    key: &AccountIdOf<T>,
+    action: &ApplicationAction,
+) -> bool {
+    crate::AgentApplications::<T>::iter().any(|(_, application)| {
+        application.agent_key == *key
+            && application.status == ApplicationStatus::Open
+            && application.action == *action
+    })
+}

pallets/governance/src/lib.rs

@@ -68,9 +68,6 @@ pub mod pallet {
     #[pallet::storage]
     pub type AgentApplications<T: Config> = StorageMap<_, Identity, u32, AgentApplication<T>>;
 
-    #[pallet::storage]
-    pub type AgentApplicationId<T: Config> = StorageValue<_, u32, ValueQuery>;
-
     #[pallet::storage]
     pub type Whitelist<T: Config> = StorageMap<_, Identity, AccountIdOf<T>, ()>;
 
@@ -150,7 +147,7 @@ pub mod pallet {
                 .ok()
                 .expect("blockchain won't pass 2 ^ 64 blocks");
 
-            application::remove_expired_applications::<T>(current_block);
+            application::resolve_expired_applications::<T>(current_block);
             proposal::tick_proposals::<T>(current_block);
             proposal::tick_proposal_rewards::<T>(current_block);
 
@@ -233,9 +230,10 @@ pub mod pallet {
             origin: OriginFor<T>,
             agent_key: AccountIdOf<T>,
             metadata: Vec<u8>,
+            removing: bool,
         ) -> DispatchResult {
             let payer = ensure_signed(origin)?;
-            application::submit_application::<T>(payer, agent_key, metadata)
+            application::submit_application::<T>(payer, agent_key, metadata, removing)
         }
 
         #[pallet::call_index(10)]

pallets/governance/src/whitelist.rs

@@ -1,12 +1,16 @@
 use polkadot_sdk::frame_support::dispatch::DispatchResult;
 
-use crate::AccountIdOf;
+use crate::{application, AccountIdOf};
 
 pub fn add_to_whitelist<T: crate::Config>(key: AccountIdOf<T>) -> DispatchResult {
     if is_whitelisted::<T>(&key) {
         return Err(crate::Error::<T>::AlreadyWhitelisted.into());
     }
 
+    if application::exists_for_agent_key::<T>(&key, &application::ApplicationAction::Add) {
+        return Err(crate::Error::<T>::ApplicationKeyAlreadyUsed.into());
+    }
+
     crate::Whitelist::<T>::insert(key.clone(), ());
     crate::Pallet::<T>::deposit_event(crate::Event::<T>::WhitelistAdded(key));
     Ok(())
@@ -17,6 +21,10 @@ pub fn remove_from_whitelist<T: crate::Config>(key: AccountIdOf<T>) -> DispatchR
         return Err(crate::Error::<T>::NotWhitelisted.into());
     }
 
+    if application::exists_for_agent_key::<T>(&key, &application::ApplicationAction::Remove) {
+        return Err(crate::Error::<T>::ApplicationKeyAlreadyUsed.into());
+    }
+
     crate::Whitelist::<T>::remove(&key);
     let _ = pallet_torus0::agent::unregister::<T>(key.clone());
     crate::Pallet::<T>::deposit_event(crate::Event::<T>::WhitelistRemoved(key));