diff --git a/pallets/governance/src/application.rs b/pallets/governance/src/application.rs index 771176a..6974089 100644 --- a/pallets/governance/src/application.rs +++ b/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 { pub data: BoundedVec, pub cost: BalanceOf, 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( payer: AccountIdOf, agent_key: AccountIdOf, data: Vec, + removing: bool, ) -> DispatchResult { - if whitelist::is_whitelisted::(&agent_key) { + if !removing && whitelist::is_whitelisted::(&agent_key) { return Err(crate::Error::::AlreadyWhitelisted.into()); + } else if removing && !whitelist::is_whitelisted::(&agent_key) { + return Err(crate::Error::::NotWhitelisted.into()); + } + + let action = if removing { + ApplicationAction::Remove + } else { + ApplicationAction::Add + }; + + if exists_for_agent_key::(&agent_key, &action) { + return Err(crate::Error::::ApplicationKeyAlreadyUsed.into()); } let config = crate::GlobalGovernanceConfig::::get(); @@ -58,23 +86,24 @@ pub fn submit_application( let expires_at = current_block + config.agent_application_expiration; - let application_id: u32 = crate::AgentApplicationId::::mutate(|id| { - let last_id = *id; - *id = id.saturating_add(1); - last_id - }); + let next_id = AgentApplications::::iter() + .count() + .try_into() + .map_err(|_| crate::Error::::InternalError)?; let application = AgentApplication:: { - 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::::insert(application_id, application); - crate::Pallet::::deposit_event(crate::Event::::ApplicationCreated(application_id)); + crate::AgentApplications::::insert(next_id, application); + crate::Pallet::::deposit_event(crate::Event::::ApplicationCreated(next_id)); Ok(()) } @@ -83,15 +112,32 @@ pub fn accept_application(application_id: u32) -> DispatchResu let application = crate::AgentApplications::::get(application_id) .ok_or(crate::Error::::ApplicationNotFound)?; - crate::AgentApplications::::remove(application.id); + match application.action { + ApplicationAction::Add => { + crate::Whitelist::::insert(application.agent_key.clone(), ()); + crate::Pallet::::deposit_event(crate::Event::::WhitelistAdded( + application.agent_key, + )); + } + ApplicationAction::Remove => { + crate::Whitelist::::remove(&application.agent_key); + crate::Pallet::::deposit_event(crate::Event::::WhitelistRemoved( + application.agent_key, + )); + } + } - whitelist::add_to_whitelist::(application.agent_key.clone())?; + crate::AgentApplications::::mutate(application_id, |application| { + if let Some(app) = application { + app.status = ApplicationStatus::Resolved { accepted: true }; + } + }); + // Pay the application cost back to the applicant let _ = ::Currency::deposit_creating(&application.payer_key, application.cost); crate::Pallet::::deposit_event(crate::Event::::ApplicationAccepted(application.id)); - crate::Pallet::::deposit_event(crate::Event::::WhitelistAdded(application.agent_key)); Ok(()) } @@ -100,19 +146,40 @@ pub fn deny_application(application_id: u32) -> DispatchResult let application = crate::AgentApplications::::get(application_id) .ok_or(crate::Error::::ApplicationNotFound)?; - crate::AgentApplications::::remove(application.id); + crate::AgentApplications::::mutate(application_id, |application| { + if let Some(app) = application { + app.status = ApplicationStatus::Resolved { accepted: false }; + } + }); + crate::Pallet::::deposit_event(crate::Event::::ApplicationDenied(application.id)); Ok(()) } -pub(crate) fn remove_expired_applications(current_block: Block) { +pub(crate) fn resolve_expired_applications(current_block: Block) { for application in crate::AgentApplications::::iter_values() { if current_block < application.expires_at { continue; } - crate::AgentApplications::::remove(application.id); + crate::AgentApplications::::mutate(application.id, |application| { + if let Some(app) = application { + app.status = ApplicationStatus::Expired; + } + }); + crate::Pallet::::deposit_event(crate::Event::::ApplicationExpired(application.id)); } } + +pub(crate) fn exists_for_agent_key( + key: &AccountIdOf, + action: &ApplicationAction, +) -> bool { + crate::AgentApplications::::iter().any(|(_, application)| { + application.agent_key == *key + && application.status == ApplicationStatus::Open + && application.action == *action + }) +} diff --git a/pallets/governance/src/lib.rs b/pallets/governance/src/lib.rs index 1b191de..0e2588e 100644 --- a/pallets/governance/src/lib.rs +++ b/pallets/governance/src/lib.rs @@ -68,9 +68,6 @@ pub mod pallet { #[pallet::storage] pub type AgentApplications = StorageMap<_, Identity, u32, AgentApplication>; - #[pallet::storage] - pub type AgentApplicationId = StorageValue<_, u32, ValueQuery>; - #[pallet::storage] pub type Whitelist = StorageMap<_, Identity, AccountIdOf, ()>; @@ -150,7 +147,7 @@ pub mod pallet { .ok() .expect("blockchain won't pass 2 ^ 64 blocks"); - application::remove_expired_applications::(current_block); + application::resolve_expired_applications::(current_block); proposal::tick_proposals::(current_block); proposal::tick_proposal_rewards::(current_block); @@ -233,9 +230,10 @@ pub mod pallet { origin: OriginFor, agent_key: AccountIdOf, metadata: Vec, + removing: bool, ) -> DispatchResult { let payer = ensure_signed(origin)?; - application::submit_application::(payer, agent_key, metadata) + application::submit_application::(payer, agent_key, metadata, removing) } #[pallet::call_index(10)] diff --git a/pallets/governance/src/whitelist.rs b/pallets/governance/src/whitelist.rs index 7610d8b..fadcc1f 100644 --- a/pallets/governance/src/whitelist.rs +++ b/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(key: AccountIdOf) -> DispatchResult { if is_whitelisted::(&key) { return Err(crate::Error::::AlreadyWhitelisted.into()); } + if application::exists_for_agent_key::(&key, &application::ApplicationAction::Add) { + return Err(crate::Error::::ApplicationKeyAlreadyUsed.into()); + } + crate::Whitelist::::insert(key.clone(), ()); crate::Pallet::::deposit_event(crate::Event::::WhitelistAdded(key)); Ok(()) @@ -17,6 +21,10 @@ pub fn remove_from_whitelist(key: AccountIdOf) -> DispatchR return Err(crate::Error::::NotWhitelisted.into()); } + if application::exists_for_agent_key::(&key, &application::ApplicationAction::Remove) { + return Err(crate::Error::::ApplicationKeyAlreadyUsed.into()); + } + crate::Whitelist::::remove(&key); let _ = pallet_torus0::agent::unregister::(key.clone()); crate::Pallet::::deposit_event(crate::Event::::WhitelistRemoved(key)); diff --git a/pallets/governance/tests/application.rs b/pallets/governance/tests/application.rs index 6dd2605..4e06c9a 100644 --- a/pallets/governance/tests/application.rs +++ b/pallets/governance/tests/application.rs @@ -1,9 +1,119 @@ +use pallet_governance::application::ApplicationStatus; use pallet_governance::AgentApplications; use pallet_governance::GlobalGovernanceConfig; +use polkadot_sdk::frame_support::assert_err; use test_utils::*; #[test] -fn whitelist_executes_application_correctly() { +fn error_is_thrown_on_applying_add_already_whitelisted() { + new_test_ext().execute_with(|| { + zero_min_burn(); + + let key = 0; + let adding_key = 1; + pallet_governance::Curators::::insert(key, ()); + pallet_governance::Whitelist::::insert(adding_key, ()); + + let proposal_cost = GlobalGovernanceConfig::::get().agent_application_cost; + let data = "test".as_bytes().to_vec(); + + add_balance(key, proposal_cost + 1); + + assert_err!( + pallet_governance::Pallet::::submit_application( + get_origin(key), + adding_key, + data.clone(), + false + ), + pallet_governance::Error::::AlreadyWhitelisted + ); + }); +} + +#[test] +fn error_is_thrown_on_applying_remove_not_whitelisted() { + new_test_ext().execute_with(|| { + zero_min_burn(); + + let key = 0; + let adding_key = 1; + pallet_governance::Curators::::insert(key, ()); + + let proposal_cost = GlobalGovernanceConfig::::get().agent_application_cost; + let data = "test".as_bytes().to_vec(); + + add_balance(key, proposal_cost + 1); + + assert_err!( + pallet_governance::Pallet::::submit_application( + get_origin(key), + adding_key, + data.clone(), + true + ), + pallet_governance::Error::::NotWhitelisted + ); + }); +} + +#[test] +fn whitelist_executes_application_correctly_add() { + new_test_ext().execute_with(|| { + zero_min_burn(); + + let key = 0; + let adding_key = 1; + pallet_governance::Curators::::insert(key, ()); + + let proposal_cost = GlobalGovernanceConfig::::get().agent_application_cost; + let data = "test".as_bytes().to_vec(); + + add_balance(key, proposal_cost + 1); + // first submit an application + let balance_before = get_balance(key); + + assert_ok!(pallet_governance::Pallet::::submit_application( + get_origin(key), + adding_key, + data.clone(), + false, + )); + + let balance_after = get_balance(key); + assert_eq!(balance_after, balance_before - proposal_cost); + + let mut application_id: u32 = u32::MAX; + for (_, value) in AgentApplications::::iter() { + assert_eq!(value.agent_key, adding_key); + assert_eq!(value.data, data); + application_id = value.id; + } + + assert_ok!(pallet_governance::Pallet::::accept_application( + get_origin(key), + application_id + )); + + let balance_after_accept = get_balance(key); + + assert_eq!(balance_after_accept, balance_before); + + assert!(pallet_governance::whitelist::is_whitelisted::( + &adding_key + )); + + let application = + pallet_governance::AgentApplications::::get(application_id).unwrap(); + assert_eq!( + application.status, + ApplicationStatus::Resolved { accepted: true } + ); + }); +} + +#[test] +fn whitelist_executes_application_correctly_remove() { new_test_ext().execute_with(|| { zero_min_burn(); @@ -22,6 +132,7 @@ fn whitelist_executes_application_correctly() { get_origin(key), adding_key, data.clone(), + false )); let balance_after = get_balance(key); @@ -46,6 +157,46 @@ fn whitelist_executes_application_correctly() { assert!(pallet_governance::whitelist::is_whitelisted::( &adding_key )); + + let application = + pallet_governance::AgentApplications::::get(application_id).unwrap(); + assert_eq!( + application.status, + ApplicationStatus::Resolved { accepted: true } + ); + }); +} + +#[test] +fn error_is_thrown_on_multiple_applications_same_key() { + new_test_ext().execute_with(|| { + zero_min_burn(); + + let key = 0; + let adding_key = 1; + pallet_governance::Curators::::insert(key, ()); + + let proposal_cost = GlobalGovernanceConfig::::get().agent_application_cost; + let data = "test".as_bytes().to_vec(); + + add_balance(key, (proposal_cost * 2) + 1); + + assert_ok!(pallet_governance::Pallet::::submit_application( + get_origin(key), + adding_key, + data.clone(), + false + )); + + assert_err!( + pallet_governance::Pallet::::submit_application( + get_origin(key), + adding_key, + data.clone(), + false + ), + pallet_governance::Error::::ApplicationKeyAlreadyUsed + ); }); } @@ -66,6 +217,7 @@ fn application_denied_doesnt_add_to_whitelist() { get_origin(key), adding_key, data.clone(), + false, )); let balance_after = get_balance(key); @@ -90,5 +242,48 @@ fn application_denied_doesnt_add_to_whitelist() { assert!(!pallet_governance::whitelist::is_whitelisted::( &adding_key )); + + let application = + pallet_governance::AgentApplications::::get(application_id).unwrap(); + assert_eq!( + application.status, + ApplicationStatus::Resolved { accepted: false } + ); + }); +} + +#[test] +fn application_expires() { + new_test_ext().execute_with(|| { + let key = 0; + let adding_key = 1; + pallet_governance::Curators::::insert(key, ()); + + let proposal_cost = GlobalGovernanceConfig::::get().agent_application_cost; + let data = "test".as_bytes().to_vec(); + + add_balance(key, proposal_cost + 1); + + assert_ok!(pallet_governance::Pallet::::submit_application( + get_origin(key), + adding_key, + data.clone(), + false + )); + + let mut application_id: u32 = u32::MAX; + for (_, value) in AgentApplications::::iter() { + assert_eq!(value.agent_key, adding_key); + assert_eq!(value.data, data); + application_id = value.id; + } + + step_block( + pallet_governance::GlobalGovernanceConfig::::get().agent_application_expiration, + ); + + let application = + pallet_governance::AgentApplications::::get(application_id).unwrap(); + assert_eq!(application.status, ApplicationStatus::Expired); }); }