diff --git a/pallets/asset_management/src/functions.rs b/pallets/asset_management/src/functions.rs index 2bdb4995..b7ac42d1 100644 --- a/pallets/asset_management/src/functions.rs +++ b/pallets/asset_management/src/functions.rs @@ -6,7 +6,7 @@ pub use scale_info::prelude::boxed::Box; pub use sp_core::H256; use sp_runtime::traits::{StaticLookup, Zero}; impl Pallet { - pub fn approve_representative(origin: OriginFor, who: T::AccountId) -> DispatchResult { + pub fn approve_representative_role(origin: OriginFor, who: T::AccountId) -> DispatchResult { let caller = ensure_signed(origin.clone())?; let mut representative = Roles::Pallet::::get_pending_representatives(&who).unwrap(); @@ -18,7 +18,7 @@ impl Pallet { //Check if we're dealing with an already registered representative let registered = Roles::RepresentativeLog::::contains_key(&who); - if registered == false { + if !registered { representative.activated = true; representative.assets_accounts.clear(); representative.assets_accounts.push(caller); @@ -49,7 +49,7 @@ impl Pallet { } } - if check0 == false { + if !check0 { //Set the representative as a registrar Ident::Pallet::::add_registrar(origin, who2).ok(); @@ -63,7 +63,7 @@ impl Pallet { let fees = bals0.ident_bal; Ident::Pallet::::set_fee(origin2, index, fees).ok(); - if registered == false { + if !registered { //Update Rep number if not yet a registered Representative index += 1; Roles::RepNumber::::put(index); @@ -73,7 +73,7 @@ impl Pallet { Ok(()) } - pub fn revoke_representative(who: T::AccountId) -> DispatchResult { + pub fn revoke_representative_role(who: T::AccountId) -> DispatchResult { Roles::RepresentativeLog::::mutate(&who, |val| { let mut val0 = val.clone().unwrap(); val0.activated = false; @@ -116,7 +116,7 @@ impl Pallet { //Store payment details let details = Payment::Pallet::::get_payment_details(&from, &creator).unwrap(); - GuarantyPayment::::insert(from.clone(), creator.clone(), details); + GuarantyPayment::::insert(from, creator, details); Ok(()) } @@ -171,7 +171,7 @@ impl Pallet { let rent = bals.roles_bal; bals = BalanceType::::convert_to_balance(rent1); let year_rent = bals.roles_bal; - val0.rent = rent.into(); + val0.rent = rent; val0.asset_account = Some(asset_account); val0.remaining_rent = year_rent; val0.remaining_payments = time as u8; @@ -227,7 +227,7 @@ impl Pallet { match Dem::Pallet::::note_preimage(origin, proposal_encoded) { Ok(_) => (), Err(x) if x == Error::::DuplicatePreimage.into() => (), - Err(x) => panic!("{:?}", x), + Err(x) => panic!("{x:?}"), } proposal_hash } @@ -306,7 +306,7 @@ impl Pallet { let tenants = Roles::Pallet::::tenant_list(); for i in tenants { let tenant = Roles::Pallet::::tenants(i).unwrap(); - if !tenant.asset_account.is_none() { + if tenant.asset_account.is_some() { let time = ::Lease::get(); let remaining_p = tenant.remaining_payments; let contract_begin = tenant.contract_start; @@ -317,7 +317,7 @@ impl Pallet { //Calculate rent per block let total_blocks = ::ContractLength::get(); - let mut rpb = Self::blocknumber_to_u128(total_blocks.clone()).unwrap(); + let mut rpb = Self::blocknumber_to_u128(total_blocks).unwrap(); let mut rpb_float = rpb as f64; rpb_float = (rent_float / rpb_float).round(); rpb = rpb_float as u128; @@ -327,7 +327,7 @@ impl Pallet { let amount_due = blocks.saturating_mul(rpb); //check how many rents were payed - let payed = (time as u128 - remaining_p as u128) * rent.clone(); + let payed = (time as u128 - remaining_p as u128) * rent; let asset_account = tenant.asset_account.clone().unwrap(); let asset_account_free_balance = ::Currency::free_balance(&asset_account); @@ -345,33 +345,32 @@ impl Pallet { //Get Asset_tokens infos let token_id = infos.token_id; - let total_issuance = - Assetss::Pallet::::total_supply(token_id.clone().into()); + let total_issuance = Assetss::Pallet::::total_supply(token_id.into()); let total_issuance_float = Self::assets_bal_to_u128(total_issuance).unwrap() as f64; //Remove maintenance fees from rent and convert it to f64 - let maintenance = T::Maintenance::get() * rent1.clone(); - let distribute = rent1.saturating_sub(maintenance.clone()); + let maintenance = T::Maintenance::get() * rent1; + let distribute = rent1.saturating_sub(maintenance); //Get the total amount to distribute - let distribute_float = (Self::manage_bal_to_u128(distribute.clone()) - .unwrap() * infos.rent_nbr as u128) as f64; + let distribute_float = (Self::manage_bal_to_u128(distribute).unwrap() * + infos.rent_nbr as u128) as f64; - debug_assert!(distribute.clone() > Zero::zero()); - debug_assert!(distribute.clone() < rent1.clone()); - debug_assert!(maintenance.clone() < asset_account_free_balance); + debug_assert!(distribute > Zero::zero()); + debug_assert!(distribute < rent1); + debug_assert!(maintenance < asset_account_free_balance); //Reserve maintenance fees let reservation = - ::Currency::reserve(&asset_account, maintenance.into()); + ::Currency::reserve(&asset_account, maintenance); //Emmit maintenance fee payment event Self::deposit_event(Event::MaintenanceFeesPayment { tenant: tenant.account_id.clone(), when: now, asset_account: tenant.asset_account.unwrap(), - amount: maintenance.clone(), + amount: maintenance, }); debug_assert!(reservation.is_ok()); @@ -381,10 +380,10 @@ impl Pallet { //Get owner's share: we divide //the owner's tokens by the total token issuance, and multiply the // result by the total amount to be distributed. - let share = Assetss::Pallet::::balance(token_id.clone().into(), &i); + let share = Assetss::Pallet::::balance(token_id.into(), &i); let share_float = Self::assets_bal_to_u128(share).unwrap() as f64 / total_issuance_float; - let amount_float = share_float * distribute_float.clone(); + let amount_float = share_float * distribute_float; let bals0 = BalanceType::::convert_to_balance(amount_float as u128); let amount = bals0.manage_bal; ::Currency::transfer( @@ -406,9 +405,9 @@ impl Pallet { //Now return the awaiting payment number to 0 let ownership_infos = Share::Virtual::::iter_keys(); for (i, j) in ownership_infos { - let infos = Share::Pallet::::virtual_acc(&i, &j).unwrap(); + let infos = Share::Pallet::::virtual_acc(i, j).unwrap(); if infos.virtual_account == asset_account { - Share::Virtual::::mutate(i.clone(), j.clone(), |val| { + Share::Virtual::::mutate(i, j, |val| { let mut val0 = val.clone().unwrap(); val0.rent_nbr = 0; *val = Some(val0); diff --git a/pallets/asset_management/src/lib.rs b/pallets/asset_management/src/lib.rs index 70be926a..729e920e 100644 --- a/pallets/asset_management/src/lib.rs +++ b/pallets/asset_management/src/lib.rs @@ -29,7 +29,7 @@ //! proposals: //! - Admit a Tenant for a given asset. //! - Evict a Tenant from a given asset. -//! The Representative has to submit a judgement about the tenant profile. This judgement +//! The Representative has to submit a judgement about the tenant profile. This judgement //! will be considered by the owners before voting. //! Representatives receive a judgement fee from the aspiring tenant. //! A positive result of the referendum will send a guaranty_deposit payment request to the @@ -251,12 +251,20 @@ pub mod pallet { NotARepresentative, /// Not an active Representative NotAnActiveRepresentative, + /// The asset is already linked with a representative + AssetAlreadyLinkedWithRepresentative, + /// The asset is not linked with a representative + AssetNotLinkedWithRepresentative, + /// The given representative is not linked with the asset + InvalidRepresentative, /// The asset is not linked to the representative AssetOutOfControl, /// The candidate is not a tenant NotATenant, - /// An asset is already linked to the provided account - AlreadyLinkedWithAsset, + /// An asset is already linked with the representative + RepresentativeAlreadyLinkedWithAsset, + /// An asset is already linked with the tenant + TenantAlreadyLinkedWithAsset, /// The tenant is not linked to the asset TenantAssetNotLinked, /// Errors should have helpful documentation associated with them. @@ -331,52 +339,75 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { let caller = ensure_signed(origin.clone())?; - //Get the asset virtual account if it exists + // Get asset virtual account if it exists let collection_id: T::NftCollectionId = asset_type.value().into(); + let ownership = Share::Pallet::::virtual_acc(collection_id, asset_id); ensure!(ownership.is_some(), Error::::NotAnAsset); + let ownership = ownership.unwrap(); - //Ensure that the caller is an owner related to the virtual account - ensure!( - Self::caller_can_vote(&caller, ownership.clone().unwrap()), - Error::::NotAnOwner - ); + let asset = Onboarding::Pallet::::houses(collection_id, asset_id); + ensure!(asset.is_some(), Error::::NotAnAsset); + let asset = asset.unwrap(); - let virtual_account = ownership.clone().unwrap().virtual_account; - let deposit = T::MinimumDeposit::get(); + // Ensure that the caller is one of the asset owners + ensure!(ownership.owners.contains(&caller), Error::::NotAnOwner); - //Ensure that the virtual account has enough funds - for f in ownership.unwrap().owners { - ::Currency::transfer( - &f, - &virtual_account, - deposit, - ExistenceRequirement::AllowDeath, - ) - .ok(); - } + let virtual_account = ownership.virtual_account; - //Create the call + // Create the call let proposal_call = match proposal { VoteProposals::Election => { - //Check that the account is in the representative waiting list + // Check if the account is in the representative waiting list + let rep = Roles::Pallet::::get_pending_representatives(&representative); + ensure!(rep.is_some(), Error::::NotAPendingRepresentative); + + // Ensure that the asset doesn't have a representative yet + ensure!( + asset.representative.is_none(), + Error::::AssetAlreadyLinkedWithRepresentative + ); + + //Ensure that the Representative is not already connected to this asset ensure!( - Roles::Pallet::::get_pending_representatives(&representative).is_some(), - Error::::NotAPendingRepresentative + !rep.unwrap().assets_accounts.contains(&virtual_account), + Error::::RepresentativeAlreadyLinkedWithAsset ); + Call::::representative_approval { rep_account: representative.clone(), collection: collection_id, item: asset_id, } }, - VoteProposals::Demotion => Call::::demote_representative { - rep_account: representative.clone(), - collection: collection_id, - item: asset_id, + VoteProposals::Demotion => { + // Ensure that the asset is linked with the representative + let asset_rep = asset.representative; + ensure!(asset_rep.is_some(), Error::::AssetNotLinkedWithRepresentative); + ensure!( + asset_rep == Some(representative.clone()), + Error::::InvalidRepresentative + ); + Call::::demote_representative { + rep_account: representative.clone(), + collection: collection_id, + item: asset_id, + } }, }; + let deposit = T::MinimumDeposit::get(); + //Ensure that the virtual account has enough funds + for f in ownership.owners { + ::Currency::transfer( + &f, + &virtual_account, + deposit, + ExistenceRequirement::AllowDeath, + ) + .ok(); + } + //Format the call and create the proposal Hash let proposal_hash = Self::create_proposal_hash_and_note(virtual_account.clone(), proposal_call); @@ -476,18 +507,16 @@ pub mod pallet { Share::Pallet::::virtual_acc(collection, item).unwrap().virtual_account; //Check that the caller is a stored virtual account - ensure!(caller == asset_account.clone(), Error::::NotAnAssetAccount); - - //Ensure that the Representative is not already connected to this asset - let representative = - Roles::Pallet::::get_pending_representatives(&rep_account).unwrap(); - let rep_assets = representative.assets_accounts; - for i in rep_assets { - ensure!(i != asset_account, Error::::AlreadyLinkedWithAsset); - } + ensure!(caller == asset_account, Error::::NotAnAssetAccount); + + Onboarding::Houses::::mutate(collection, item, |asset| { + let mut asset0 = asset.clone().unwrap(); + asset0.representative = Some(rep_account.clone()); + *asset = Some(asset0); + }); //Approve role request - Self::approve_representative(origin, rep_account.clone()).ok(); + Self::approve_representative_role(origin, rep_account.clone()).ok(); Self::deposit_event(Event::RepresentativeCandidateApproved { candidate: rep_account, @@ -520,7 +549,12 @@ pub mod pallet { ); //revoke Representative Role - Self::revoke_representative(rep_account.clone()).ok(); + Self::revoke_representative_role(rep_account.clone()).ok(); + Onboarding::Houses::::mutate(collection, item, |asset| { + let mut asset0 = asset.clone().unwrap(); + asset0.representative = None; + *asset = Some(asset0); + }); Self::deposit_event(Event::RepresentativeDemoted { candidate: rep_account, @@ -561,7 +595,7 @@ pub mod pallet { //Compare guaranty payment amount+fees with tenant free_balance let guaranty = Self::calculate_guaranty(collection_id, asset_id); let fee0 = Self::manage_bal_to_u128(T::RepFees::get()).unwrap(); - let bals0 = BalanceType::::convert_to_balance(guaranty.clone()); + let bals0 = BalanceType::::convert_to_balance(guaranty); let fee1 = T::IncentivePercentage::get() * bals0.manage_bal; let total_amount = guaranty + fee0 + Self::manage_bal_to_u128(fee1).unwrap(); let tenant_bal0: BalanceOf = ::Currency::free_balance(&tenant); @@ -575,13 +609,16 @@ pub mod pallet { ensure!(tenant0.is_some(), Error::::NotATenant); // Ensure that the tenant is registered let tenant_infos = Roles::Pallet::::tenants(tenant.clone()).unwrap(); - ensure!(tenant_infos.registered == true, Error::::NotARegisteredTenant); + ensure!(tenant_infos.registered, Error::::NotARegisteredTenant); let tenant0 = tenant0.unwrap(); match proposal { VoteProposals::Election => { // Ensure that the tenant is not linked to an asset - ensure!(tenant0.asset_account.is_none(), Error::::AlreadyLinkedWithAsset); + ensure!( + tenant0.asset_account.is_none(), + Error::::TenantAlreadyLinkedWithAsset + ); //Ensure there is no existing payment request for this asset ensure!( Self::guaranty(&tenant0.account_id, &asset_account).is_none(), @@ -592,13 +629,8 @@ pub mod pallet { //provide judgement let index = rep.index; let target = T::Lookup::unlookup(tenant.clone()); - Ident::Pallet::::provide_judgement( - origin.clone(), - index.into(), - target, - judgement.clone(), - ) - .ok(); + Ident::Pallet::::provide_judgement(origin.clone(), index, target, judgement) + .ok(); }, VoteProposals::Demotion => { // Ensure that the tenant is linked to the asset diff --git a/pallets/asset_management/src/mock.rs b/pallets/asset_management/src/mock.rs index c5462a2f..08d6fe24 100644 --- a/pallets/asset_management/src/mock.rs +++ b/pallets/asset_management/src/mock.rs @@ -340,7 +340,7 @@ impl pallet_sudo::Config for Test { } parameter_types! { - pub const MaxMembers:u32 =7; + pub const MaxMembers:u32 =8; } impl pallet_roles::Config for Test { type Event = Event; diff --git a/pallets/asset_management/src/tests.rs b/pallets/asset_management/src/tests.rs index de0284a3..0b1f0d1a 100644 --- a/pallets/asset_management/src/tests.rs +++ b/pallets/asset_management/src/tests.rs @@ -75,9 +75,13 @@ pub fn prep_test( )); } -// FIXME: write more tests for the representative flow #[test] fn test_representative() { + // TODO: write more tests for the representative flow + // edge cases to cover + // - asset is already linked with the given representative(election) + // - asset is already linked with a representative(election) + // ... ExtBuilder::default().build().execute_with(|| { //submit a request for representative role RoleModule::set_role(Origin::signed(CHARLIE), CHARLIE, Acc::REPRESENTATIVE).ok(); @@ -284,7 +288,7 @@ fn test_integration_test() { assert!(Roles::RepresentativeLog::::contains_key(FERDIE)); assert!(Roles::AccountsRolesLog::::contains_key(FERDIE)); - // TODO: after fixing the issue of the representative flow, please uncomment + // TODO: after fixing the issue of the representative flow, please comment assert_err!( AssetManagement::launch_representative_session( origin_eve.clone(), @@ -296,6 +300,10 @@ fn test_integration_test() { Error::::NotAPendingRepresentative ); + let asset = Onboarding::Pallet::::houses(NftColl::OFFICESTEST.value(), item_id0); + assert!(asset.is_some()); + assert_eq!(asset.unwrap().representative, Some(FERDIE)); + // FIXME: the following test should not fail // assert_ok!(AssetManagement::launch_representative_session( // origin_eve.clone(), @@ -479,10 +487,11 @@ fn test_integration_test() { VoteProposals::Election, Ident::Judgement::Reasonable, ), - Error::::AlreadyLinkedWithAsset + Error::::TenantAlreadyLinkedWithAsset ); println!("\n\nlaunch_tenant_session - : THE TENANT IS ALREADY LINKED WITH AN ASSET"); + assert!(Roles::TenantLog::::contains_key(PEGGY)); //change PEGGY to a RegisteredTenant Roles::TenantLog::::mutate(PEGGY, |val| { let mut val0 = val.clone().unwrap(); @@ -501,7 +510,7 @@ fn test_integration_test() { ), Error::::NotEnoughTenantFunds ); - println!("\n\nlaunch_tenant_session - : THE TENANT IS ALREADY LINKED WITH AN ASSET"); + println!("\n\nlaunch_tenant_session - : THE TENANT DOESN'T HAVE ENOUGH FUNDS"); //change HUNTER to a RegisteredTenant Roles::TenantLog::::mutate(HUNTER, |val| { @@ -673,5 +682,8 @@ fn test_integration_test() { //The line below evaluate the results of TEST_0, TEST_1, & TEST_2 by looking for the result // of a correctly executed call. assert!(!Roles::AccountsRolesLog::::contains_key(FERDIE)); + + let asset = Onboarding::Pallet::::houses(NftColl::OFFICESTEST.value(), item_id0); + assert!(asset.unwrap().representative.is_none()); }); } diff --git a/pallets/onboarding/src/tests.rs b/pallets/onboarding/src/tests.rs index 17a2116c..e1a5d236 100644 --- a/pallets/onboarding/src/tests.rs +++ b/pallets/onboarding/src/tests.rs @@ -47,6 +47,8 @@ fn create_proposal() { let item_id = pallet_nft::ItemsCount::::get()[coll_id as usize] - 1; let status: AssetStatus = Houses::::get(coll_id, item_id).unwrap().status; + assert!(Houses::::get(coll_id, item_id).unwrap().representative.is_none()); + expect_events(vec![ crate::Event::ProposalCreated { who: BOB, diff --git a/pallets/onboarding/src/types.rs b/pallets/onboarding/src/types.rs index fd53a983..730fe403 100644 --- a/pallets/onboarding/src/types.rs +++ b/pallets/onboarding/src/types.rs @@ -36,6 +36,8 @@ pub struct Asset { pub(super) infos: ItemInfoOf, /// NFT Price pub price: Option>, + /// Representative + pub representative: Option, /// Tenants pub tenants: Vec, /// Proposal hash @@ -56,6 +58,7 @@ impl Asset { created, infos, price, + representative: None, tenants: Default::default(), proposal_hash: Default::default(), }; diff --git a/pallets/roles/src/functions.rs b/pallets/roles/src/functions.rs index 9fd3c532..2f7ecd34 100644 --- a/pallets/roles/src/functions.rs +++ b/pallets/roles/src/functions.rs @@ -85,9 +85,10 @@ impl Pallet { _ => false, }; ensure!(success, Error::::NotInWaitingList); - Ok(()) + Self::increase_total_members() } + // TODO: This function can be updated pub fn check_account_role(caller: T::AccountId) -> DispatchResult { ensure!(!HouseSellerLog::::contains_key(&caller), Error::::OneRoleAllowed); ensure!(!InvestorLog::::contains_key(&caller), Error::::OneRoleAllowed); @@ -175,9 +176,9 @@ impl Pallet { let now = >::block_number(); let rep_count = representatives.len() as u32; for (index, account) in representatives.iter().enumerate() { - AccountsRolesLog::::insert(&account, Accounts::REPRESENTATIVE); + AccountsRolesLog::::insert(account, Accounts::REPRESENTATIVE); RepresentativeLog::::insert( - &account, + account, Representative:: { account_id: account.clone(), age: now, @@ -193,4 +194,12 @@ impl Pallet { let reps = Self::rep_num(); RepNumber::::put(reps.saturating_add(rep_count)); } + + pub fn increase_total_members() -> DispatchResult { + let members: u32 = Self::total_members(); + ensure!(members < T::MaxMembers::get(), Error::::TotalMembersExceeded); + TotalMembers::::put(members.saturating_add(1)); + + Ok(()) + } } diff --git a/pallets/roles/src/lib.rs b/pallets/roles/src/lib.rs index 78899d92..3d520574 100644 --- a/pallets/roles/src/lib.rs +++ b/pallets/roles/src/lib.rs @@ -300,7 +300,7 @@ pub mod pallet { )); Investor::::new(investor).map_err(|_| >::InitializationError)?; AccountsRolesLog::::insert(&account, Accounts::INVESTOR); - TotalMembers::::put(members + 1); + Self::increase_total_members().ok(); Self::deposit_event(Event::InvestorCreated(now, account.clone())); }, Accounts::SELLER => { @@ -317,7 +317,7 @@ pub mod pallet { )); Tenant::::new(tenant).map_err(|_| >::InitializationError)?; AccountsRolesLog::::insert(&account, Accounts::TENANT); - TotalMembers::::put(members + 1); + Self::increase_total_members().ok(); Self::deposit_event(Event::TenantCreated(now, account.clone())); }, Accounts::SERVICER => { @@ -353,6 +353,8 @@ pub mod pallet { ); if need_approval { RequestedRoles::::insert(&account, account_type); + } else { + TotalMembers::::put(members + 1); } Ok(()) @@ -372,9 +374,7 @@ pub mod pallet { ensure!(role != Some(Accounts::REPRESENTATIVE), Error::::UnAuthorized); - let members = Self::total_members(); Self::approve_account(sender, account.clone())?; - TotalMembers::::put(members + 1); let now = >::block_number(); Self::deposit_event(Event::AccountCreationApproved(now, account)); Ok(()) diff --git a/pallets/roles/src/tests.rs b/pallets/roles/src/tests.rs index 92c5456b..54ebd3aa 100644 --- a/pallets/roles/src/tests.rs +++ b/pallets/roles/src/tests.rs @@ -193,6 +193,7 @@ fn test_account_creation() { assert_eq!(RoleModule::total_members(), 4); //Servicer user5 successfully assign Investor role to 7 assert_ok!(RoleModule::set_role(user5, 7, Acc::INVESTOR)); + assert_eq!(RoleModule::total_members(), 5); //No additional member can be added assert_noop!( RoleModule::set_role(user4, 5, Acc::TENANT),