diff --git a/pallets/carbon-credits-pool/src/mock.rs b/pallets/carbon-credits-pool/src/mock.rs index 04cc53e7..558128f8 100644 --- a/pallets/carbon-credits-pool/src/mock.rs +++ b/pallets/carbon-credits-pool/src/mock.rs @@ -151,6 +151,7 @@ impl pallet_carbon_credits::Config for Test { type MaxIpfsReferenceLength = ConstU32<20>; type MaxLongStringLength = ConstU32<100>; type MaxRoyaltyRecipients = ConstU32<5>; + type MaxCoordinatesLength = ConstU32<100>; type MaxShortStringLength = ConstU32<20>; type MinProjectId = ConstU32<1000>; type NFTHandler = Uniques; diff --git a/pallets/vesting-contract/src/lib.rs b/pallets/vesting-contract/src/lib.rs index 4c4d2063..7e407acf 100644 --- a/pallets/vesting-contract/src/lib.rs +++ b/pallets/vesting-contract/src/lib.rs @@ -119,6 +119,12 @@ pub mod pallet { pub type BulkContractRemove = BoundedVec<::AccountId, ::MaxContractInputLength>; + /// AuthorizedAccounts type of pallet + pub type AuthorizedAccountsListOf = BoundedVec< + ::AccountId, + ::MaxAuthorizedAccountCount, + >; + /// Pallet version of balance pub type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; @@ -142,6 +148,9 @@ pub mod pallet { #[pallet::constant] type PalletId: Get; + /// Maximum amount of authorised accounts permitted + type MaxAuthorizedAccountCount: Get; + /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; } @@ -162,6 +171,12 @@ pub mod pallet { // Ideally this should be equal to the pallet account balance. pub type VestingBalance = StorageValue<_, BalanceOf, ValueQuery>; + #[pallet::storage] + #[pallet::getter(fn authorized_accounts)] + // List of AuthorizedAccounts for the pallet + pub type AuthorizedAccounts = + StorageValue<_, AuthorizedAccountsListOf, ValueQuery>; + // Pallets use events to inform users when important changes are made. // https://docs.substrate.io/v3/runtime/events-and-errors #[pallet::event] @@ -173,6 +188,10 @@ pub mod pallet { ContractRemoved { recipient: T::AccountId }, /// An existing contract has been completed/withdrawn ContractWithdrawn { recipient: T::AccountId, expiry: T::BlockNumber, amount: BalanceOf }, + /// A new authorized account has been added to storage + AuthorizedAccountAdded { account_id: T::AccountId }, + /// An authorized account has been removed from storage + AuthorizedAccountRemoved { account_id: T::AccountId }, } // Errors inform users that something went wrong. @@ -190,6 +209,12 @@ pub mod pallet { ContractNotExpired, /// Contract already exists, remove old contract before adding new ContractAlreadyExists, + /// Not authorized to perform this operation + NotAuthorised, + /// Authorized account already exists + AuthorizedAccountAlreadyExists, + /// Too many authorized accounts + TooManyAuthorizedAccounts, } #[pallet::call] @@ -208,7 +233,8 @@ pub mod pallet { amount: BalanceOf, ) -> DispatchResultWithPostInfo { // ensure caller is allowed to add new recipient - T::ForceOrigin::ensure_origin(origin)?; + let sender = ensure_signed(origin)?; + Self::check_authorized_account(&sender)?; Self::do_add_new_contract(recipient, expiry, amount)?; Ok(().into()) } @@ -221,7 +247,8 @@ pub mod pallet { recipient: T::AccountId, ) -> DispatchResultWithPostInfo { // ensure caller is allowed to remove recipient - T::ForceOrigin::ensure_origin(origin)?; + let sender = ensure_signed(origin)?; + Self::check_authorized_account(&sender)?; Self::do_remove_contract(recipient)?; Ok(().into()) } @@ -235,7 +262,8 @@ pub mod pallet { recipients: BulkContractInputs, ) -> DispatchResultWithPostInfo { // ensure caller is allowed to add new recipient - T::ForceOrigin::ensure_origin(origin)?; + let sender = ensure_signed(origin)?; + Self::check_authorized_account(&sender)?; for input in recipients { Self::do_add_new_contract(input.recipient, input.expiry, input.amount)?; } @@ -251,7 +279,8 @@ pub mod pallet { recipients: BulkContractRemove, ) -> DispatchResultWithPostInfo { // ensure caller is allowed to remove recipients - T::ForceOrigin::ensure_origin(origin)?; + let sender = ensure_signed(origin)?; + Self::check_authorized_account(&sender)?; for recipient in recipients { Self::do_remove_contract(recipient)?; } @@ -286,9 +315,67 @@ pub mod pallet { recipient: T::AccountId, ) -> DispatchResultWithPostInfo { // ensure caller is allowed to force withdraw - T::ForceOrigin::ensure_origin(origin)?; + let sender = ensure_signed(origin)?; + Self::check_authorized_account(&sender)?; Self::do_withdraw_vested(recipient)?; Ok(().into()) } + + /// Add a new account to the list of authorised Accounts + /// The caller must be from a permitted origin + #[transactional] + #[pallet::weight(T::WeightInfo::force_withdraw_vested())] + pub fn force_add_authorized_account( + origin: OriginFor, + account_id: T::AccountId, + ) -> DispatchResult { + T::ForceOrigin::ensure_origin(origin)?; + // add the account_id to the list of authorized accounts + AuthorizedAccounts::::try_mutate(|account_list| -> DispatchResult { + ensure!( + !account_list.contains(&account_id), + Error::::AuthorizedAccountAlreadyExists + ); + + account_list + .try_push(account_id.clone()) + .map_err(|_| Error::::TooManyAuthorizedAccounts)?; + Ok(()) + })?; + + Self::deposit_event(Event::AuthorizedAccountAdded { account_id }); + Ok(()) + } + + /// Remove an account from the list of authorised accounts + #[transactional] + #[pallet::weight(T::WeightInfo::force_withdraw_vested())] + pub fn force_remove_authorized_account( + origin: OriginFor, + account_id: T::AccountId, + ) -> DispatchResult { + T::ForceOrigin::ensure_origin(origin)?; + // remove the account_id from the list of authorized accounts if already exists + AuthorizedAccounts::::try_mutate(|account_list| -> DispatchResult { + if let Ok(index) = account_list.binary_search(&account_id) { + account_list.swap_remove(index); + Self::deposit_event(Event::AuthorizedAccountRemoved { account_id }); + } + + Ok(()) + }) + } + } +} + +impl Pallet { + /// Checks if the given account_id is part of authorized account list + pub fn check_authorized_account(account_id: &T::AccountId) -> DispatchResult { + let authorized_accounts = AuthorizedAccounts::::get(); + if !authorized_accounts.contains(account_id) { + Err(Error::::NotAuthorised.into()) + } else { + Ok(()) + } } } diff --git a/pallets/vesting-contract/src/mock.rs b/pallets/vesting-contract/src/mock.rs index 1f928d37..2d59d4d6 100644 --- a/pallets/vesting-contract/src/mock.rs +++ b/pallets/vesting-contract/src/mock.rs @@ -79,6 +79,7 @@ impl pallet_balances::Config for Test { parameter_types! { pub const MaxContractInputLength : u32 = 10; + pub const MaxAuthorizedAccountCount : u32 = 2; pub const VestingContractPalletId: PalletId = PalletId(*b"bitg/vcp"); } @@ -88,6 +89,7 @@ impl vesting_contract::Config for Test { type ForceOrigin = frame_system::EnsureRoot; type MaxContractInputLength = MaxContractInputLength; type PalletId = VestingContractPalletId; + type MaxAuthorizedAccountCount = MaxAuthorizedAccountCount; type WeightInfo = (); } diff --git a/pallets/vesting-contract/src/tests.rs b/pallets/vesting-contract/src/tests.rs index e9433884..fd5f3d3b 100644 --- a/pallets/vesting-contract/src/tests.rs +++ b/pallets/vesting-contract/src/tests.rs @@ -2,7 +2,7 @@ // Copyright (C) 2022 BitGreen. // This code is licensed under MIT license (see LICENSE.txt for details) // -use frame_support::{assert_noop, assert_ok, error::BadOrigin, traits::Currency, PalletId}; +use frame_support::{assert_noop, assert_ok, traits::Currency, PalletId}; use frame_system::RawOrigin; use sp_runtime::traits::AccountIdConversion; @@ -16,6 +16,79 @@ fn load_initial_pallet_balance(amount: u32) { Balances::make_free_balance_be(&vesting_contract_pallet_acccount, amount.into()); } +#[test] +fn add_new_authorized_accounts_should_work() { + new_test_ext().execute_with(|| { + let authorised_account_one = 1; + let authorised_account_two = 2; + let authorised_account_three = 3; + assert_ok!(VestingContract::force_add_authorized_account( + RawOrigin::Root.into(), + authorised_account_one, + )); + + assert_eq!( + last_event(), + VestingContractEvent::AuthorizedAccountAdded { account_id: authorised_account_one } + .into() + ); + + assert_eq!(VestingContract::authorized_accounts().first(), Some(&authorised_account_one)); + + assert_noop!( + VestingContract::force_add_authorized_account( + RawOrigin::Root.into(), + authorised_account_one, + ), + Error::::AuthorizedAccountAlreadyExists + ); + + assert_ok!(VestingContract::force_add_authorized_account( + RawOrigin::Root.into(), + authorised_account_two, + )); + + assert_noop!( + VestingContract::force_add_authorized_account( + RawOrigin::Root.into(), + authorised_account_three, + ), + Error::::TooManyAuthorizedAccounts + ); + + assert_eq!( + last_event(), + VestingContractEvent::AuthorizedAccountAdded { account_id: authorised_account_two } + .into() + ); + }); +} + +#[test] +fn force_remove_authorized_accounts_should_work() { + new_test_ext().execute_with(|| { + let authorised_account_one = 1; + assert_ok!(VestingContract::force_add_authorized_account( + RawOrigin::Root.into(), + authorised_account_one, + )); + assert_eq!(VestingContract::authorized_accounts().first(), Some(&authorised_account_one)); + + assert_ok!(VestingContract::force_remove_authorized_account( + RawOrigin::Root.into(), + authorised_account_one, + )); + + assert_eq!( + last_event(), + VestingContractEvent::AuthorizedAccountRemoved { account_id: authorised_account_one } + .into() + ); + + assert_eq!(VestingContract::authorized_accounts().len(), 0); + }); +} + #[test] fn add_contract_fails_if_expiry_in_past() { new_test_ext().execute_with(|| { @@ -23,9 +96,16 @@ fn add_contract_fails_if_expiry_in_past() { let expiry_block = 1; let vesting_amount = 1u32; let recipient = 1; + let authorised_account = 10; + + assert_ok!(VestingContract::force_add_authorized_account( + RawOrigin::Root.into(), + authorised_account + )); + assert_noop!( VestingContract::add_new_contract( - RawOrigin::Root.into(), + RawOrigin::Signed(authorised_account).into(), recipient, expiry_block, vesting_amount.into() @@ -36,7 +116,7 @@ fn add_contract_fails_if_expiry_in_past() { } #[test] -fn add_contract_fails_if_caller_not_force_origin() { +fn add_contract_fails_if_caller_not_authorized() { new_test_ext().execute_with(|| { // can only add new contract if ForceOrigin let expiry_block = 10; @@ -49,7 +129,7 @@ fn add_contract_fails_if_caller_not_force_origin() { expiry_block, vesting_amount.into() ), - BadOrigin + Error::::NotAuthorised ); }); } @@ -60,9 +140,16 @@ fn add_contract_fails_if_pallet_out_of_funds() { let expiry_block = 10; let vesting_amount = 1u32; let recipient = 1; + let authorised_account = 10; + + assert_ok!(VestingContract::force_add_authorized_account( + RawOrigin::Root.into(), + authorised_account + )); + assert_noop!( VestingContract::add_new_contract( - RawOrigin::Root.into(), + RawOrigin::Signed(authorised_account).into(), recipient, expiry_block, vesting_amount.into() @@ -79,12 +166,29 @@ fn add_contract_works() { let recipient = 1; let pallet_intial_balance = 200u32; let vesting_amount = pallet_intial_balance / 2u32; + let authorised_account = 10; load_initial_pallet_balance(pallet_intial_balance); + // Should fail if unauthorised account + assert_noop!( + VestingContract::add_new_contract( + RawOrigin::Signed(20).into(), + recipient, + expiry_block, + vesting_amount.into() + ), + Error::::NotAuthorised + ); + + assert_ok!(VestingContract::force_add_authorized_account( + RawOrigin::Root.into(), + authorised_account + )); + // Adding new contract works assert_ok!(VestingContract::add_new_contract( - RawOrigin::Root.into(), + RawOrigin::Signed(authorised_account).into(), recipient, expiry_block, vesting_amount.into() @@ -112,7 +216,7 @@ fn add_contract_works() { load_initial_pallet_balance(pallet_intial_balance); assert_noop!( VestingContract::add_new_contract( - RawOrigin::Root.into(), + RawOrigin::Signed(authorised_account).into(), recipient, expiry_block, (vesting_amount * 2_u32).into() @@ -131,15 +235,25 @@ fn remove_contract_works() { let vesting_amount = 1u32; load_initial_pallet_balance(pallet_intial_balance); - assert_ok!(VestingContract::add_new_contract( + let authorised_account = 10; + + assert_ok!(VestingContract::force_add_authorized_account( RawOrigin::Root.into(), + authorised_account + )); + + assert_ok!(VestingContract::add_new_contract( + RawOrigin::Signed(authorised_account).into(), recipient, expiry_block, vesting_amount.into() )); assert_eq!(VestingBalance::::get(), vesting_amount.into()); - assert_ok!(VestingContract::remove_contract(RawOrigin::Root.into(), recipient)); + assert_ok!(VestingContract::remove_contract( + RawOrigin::Signed(authorised_account).into(), + recipient + )); // contract removed from storage assert_eq!(VestingContracts::::get(recipient), None); @@ -162,8 +276,15 @@ fn withdraw_contract_works() { Error::::ContractNotFound ); - assert_ok!(VestingContract::add_new_contract( + let authorised_account = 10; + + assert_ok!(VestingContract::force_add_authorized_account( RawOrigin::Root.into(), + authorised_account + )); + + assert_ok!(VestingContract::add_new_contract( + RawOrigin::Signed(authorised_account).into(), recipient, expiry_block, vesting_amount.into() @@ -206,16 +327,25 @@ fn force_withdraw_contract_works() { let recipient = 1; let pallet_intial_balance = 100u32; let vesting_amount = 1u32; + let authorised_account = 10; load_initial_pallet_balance(pallet_intial_balance); + assert_ok!(VestingContract::force_add_authorized_account( + RawOrigin::Root.into(), + authorised_account + )); + // cannot withdraw on non existent contract assert_noop!( - VestingContract::force_withdraw_vested(RawOrigin::Root.into(), recipient), + VestingContract::force_withdraw_vested( + RawOrigin::Signed(authorised_account).into(), + recipient + ), Error::::ContractNotFound ); assert_ok!(VestingContract::add_new_contract( - RawOrigin::Root.into(), + RawOrigin::Signed(authorised_account).into(), recipient, expiry_block, vesting_amount.into() @@ -223,13 +353,19 @@ fn force_withdraw_contract_works() { // cannot withdraw before expiry assert_noop!( - VestingContract::force_withdraw_vested(RawOrigin::Root.into(), recipient), + VestingContract::force_withdraw_vested( + RawOrigin::Signed(authorised_account).into(), + recipient + ), Error::::ContractNotExpired ); // time travel to after expiry block to withdraw vested amount System::set_block_number(expiry_block + 1); - assert_ok!(VestingContract::force_withdraw_vested(RawOrigin::Root.into(), recipient)); + assert_ok!(VestingContract::force_withdraw_vested( + RawOrigin::Signed(authorised_account).into(), + recipient + )); // the user balance should be updated assert_eq!(Balances::free_balance(recipient), vesting_amount.into()); diff --git a/runtime/bitgreen/src/lib.rs b/runtime/bitgreen/src/lib.rs index c2fcc3a4..1270f908 100644 --- a/runtime/bitgreen/src/lib.rs +++ b/runtime/bitgreen/src/lib.rs @@ -657,6 +657,7 @@ impl pallet_vesting_contract::Config for Runtime { type ForceOrigin = EnsureRoot; type MaxContractInputLength = MaxContractInputLength; type PalletId = VestingContractPalletId; + type MaxAuthorizedAccountCount = MaxAuthorizedAccountCount; type WeightInfo = (); } diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 2841999c..db6f9df9 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -656,6 +656,7 @@ impl pallet_vesting_contract::Config for Runtime { type ForceOrigin = EnsureRoot; type MaxContractInputLength = MaxContractInputLength; type PalletId = VestingContractPalletId; + type MaxAuthorizedAccountCount = MaxAuthorizedAccountCount; type WeightInfo = (); }