-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Secretary Program #347
base: main
Are you sure you want to change the base?
Secretary Program #347
Changes from 36 commits
939392f
acd23f2
7d03cac
1c4659b
188759e
154efb5
bea78a3
c203940
334c35c
482d0be
d7d4b87
2e0968a
572d398
09d317c
4e5116b
365d3c5
600d2c2
e884c30
bcd0a17
47ed39e
7b0064a
bdbc39b
ef75bf7
86d489f
313bb8c
d34fcb2
5722c25
b3a82cf
daca216
f8ab6a1
95a2cfb
c32eb13
51b94ef
eaaab37
ec53bb8
906bc10
1b1e81f
d4dac6f
02f8a49
a80a668
1962aef
fe184d9
53e60a6
fd82ec2
b34d4b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,197 @@ | ||
// Copyright (C) Parity Technologies (UK) Ltd. | ||
// This file is part of Cumulus. | ||
|
||
// Cumulus is free software: you can redistribute it and/or modify | ||
// it under the terms of the GNU General Public License as published by | ||
// the Free Software Foundation, either version 3 of the License, or | ||
// (at your option) any later version. | ||
|
||
// Cumulus is distributed in the hope that it will be useful, | ||
// but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
// GNU General Public License for more details. | ||
|
||
// You should have received a copy of the GNU General Public License | ||
// along with Cumulus. If not, see <http://www.gnu.org/licenses/>. | ||
|
||
//! The Polkadot Secretary Collective. | ||
|
||
mod origins; | ||
use core::marker::PhantomData; | ||
|
||
use crate::{fellowship::FellowshipAdminBodyId, *}; | ||
use frame_support::{ | ||
parameter_types, | ||
traits::{tokens::GetSalary, EitherOf, MapSuccess, PalletInfoAccess, PollStatus, Polling}, | ||
}; | ||
use frame_system::{pallet_prelude::BlockNumberFor, EnsureRootWithSuccess}; | ||
pub use origins::{pallet_origins as pallet_secretary_origins, Secretary}; | ||
use pallet_ranked_collective::{MemberIndex, TallyOf, Votes}; | ||
use pallet_xcm::{EnsureXcm, IsVoiceOfBody}; | ||
use polkadot_runtime_constants::time::HOURS; | ||
use sp_core::{ConstU128, ConstU32}; | ||
use sp_runtime::{ | ||
traits::{ConstU16, ConvertToValue, Identity, Replace}, | ||
DispatchError, | ||
}; | ||
|
||
use xcm::prelude::*; | ||
use xcm_builder::{AliasesIntoAccountId32, PayOverXcm}; | ||
|
||
use self::xcm_config::AssetHubUsdt; | ||
|
||
/// The Secretary members' ranks. | ||
pub mod ranks { | ||
use pallet_ranked_collective::Rank; | ||
|
||
pub const SECRETARY_CANDIDATE: Rank = 0; | ||
pub const SECRETARY: Rank = 1; | ||
} | ||
|
||
/// Origins of: | ||
/// - Root; | ||
/// - FellowshipAdmin (i.e. token holder referendum); | ||
/// - Plurality vote from Fellows can promote, demote, remove and approve rank retention | ||
/// of members of the Secretary Collective (rank `2`). | ||
type ApproveOrigin = EitherOf< | ||
EnsureRootWithSuccess<AccountId, ConstU16<65535>>, | ||
EitherOf< | ||
MapSuccess< | ||
EnsureXcm<IsVoiceOfBody<GovernanceLocation, FellowshipAdminBodyId>>, | ||
Replace<ConstU16<65535>>, | ||
>, | ||
MapSuccess<Fellows, Replace<ConstU16<65535>>>, | ||
>, | ||
>; | ||
|
||
impl pallet_secretary_origins::Config for Runtime {} | ||
|
||
pub struct SecretaryPolling<T: pallet_ranked_collective::Config<I>, I: 'static>( | ||
PhantomData<(T, I)>, | ||
); | ||
|
||
impl<T: pallet_ranked_collective::Config<I>, I: 'static> Polling<TallyOf<T, I>> | ||
for SecretaryPolling<T, I> | ||
{ | ||
type Index = MemberIndex; | ||
type Votes = Votes; | ||
type Class = u16; | ||
type Moment = BlockNumberFor<T>; | ||
|
||
fn classes() -> Vec<Self::Class> { | ||
vec![] | ||
} | ||
|
||
fn as_ongoing(_index: Self::Index) -> Option<(TallyOf<T, I>, Self::Class)> { | ||
None | ||
} | ||
|
||
fn access_poll<R>( | ||
_index: Self::Index, | ||
f: impl FnOnce(PollStatus<&mut TallyOf<T, I>, Self::Moment, Self::Class>) -> R, | ||
) -> R { | ||
f(PollStatus::None) | ||
} | ||
|
||
fn try_access_poll<R>( | ||
_index: Self::Index, | ||
f: impl FnOnce( | ||
PollStatus<&mut TallyOf<T, I>, Self::Moment, Self::Class>, | ||
) -> Result<R, DispatchError>, | ||
) -> Result<R, DispatchError> { | ||
f(PollStatus::None) | ||
} | ||
|
||
#[cfg(feature = "runtime-benchmarks")] | ||
fn create_ongoing(_class: Self::Class) -> Result<Self::Index, ()> { | ||
Err(()) | ||
} | ||
|
||
#[cfg(feature = "runtime-benchmarks")] | ||
fn end_ongoing(_index: Self::Index, _approved: bool) -> Result<(), ()> { | ||
Err(()) | ||
} | ||
|
||
#[cfg(feature = "runtime-benchmarks")] | ||
fn max_ongoing() -> (Self::Class, u32) { | ||
(0, 0) | ||
} | ||
} | ||
|
||
pub type SecretaryCollectiveInstance = pallet_ranked_collective::Instance3; | ||
|
||
impl pallet_ranked_collective::Config<SecretaryCollectiveInstance> for Runtime { | ||
type WeightInfo = (); // TODO weights::pallet_ranked_collective_secretary_collective::WeightInfo<Runtime>; | ||
type RuntimeEvent = RuntimeEvent; | ||
type AddOrigin = ApproveOrigin; | ||
type RemoveOrigin = ApproveOrigin; | ||
type PromoteOrigin = ApproveOrigin; | ||
type DemoteOrigin = ApproveOrigin; | ||
type ExchangeOrigin = ApproveOrigin; | ||
type Polls = SecretaryPolling<Runtime, SecretaryCollectiveInstance>; | ||
type MinRankOfClass = Identity; | ||
type MemberSwappedHandler = crate::SecretarySalary; | ||
type VoteWeight = pallet_ranked_collective::Geometric; | ||
#[cfg(feature = "runtime-benchmarks")] | ||
type BenchmarkSetup = crate::SecretarySalary; | ||
} | ||
|
||
pub type SecretarySalaryInstance = pallet_salary::Instance3; | ||
|
||
parameter_types! { | ||
// The interior location on AssetHub for the paying account. This is the Secretary Salary | ||
// pallet instance. This sovereign account will need funding. | ||
pub SecretarySalaryInteriorLocation: InteriorLocation = PalletInstance(<crate::SecretarySalary as PalletInfoAccess>::index() as u8).into(); | ||
} | ||
|
||
const USDT_UNITS: u128 = 1_000_000; | ||
Doordashcon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/// [`PayOverXcm`] setup to pay the Secretary salary on the AssetHub in USDT. | ||
pub type SecretarySalaryPaymaster = PayOverXcm< | ||
SecretarySalaryInteriorLocation, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be the fellowship interior account, because the secretary should be paid by the same account. Or we move some funds. @joepetrowski @muharem WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe better separate account for a separate accounting. But can be funded from the fellowship sub treasury. No strong opinion. Let's hear Joe opinion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we control the account using a fellowship proposal? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are the secretaries only for the Fellowship? I thought the program was meant to include secretaries for other collectives as well. If it's just for the Fellowship, then yeah we should just pay them from the Fellowship salary account. But if it's meant to serve others, then I think they can have their own sub-treasury and make proposals for funding to the collectives that they provide secretary services for. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the current idea is to enable the secretary collective for the technical fellowship for now until more ideas and participants are clearer, rather than having a tresury implementation for just one member. The salary payout account can be changed to the technical fellowship interior account IMO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Doordashcon what do you mean? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @joepetrowski highlighted that using the Fellowship Salary account would be a wrong direction because of needing migration later on. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not the "wrong" direction, it's a choice. I see two solutions:
In the case of (1), it would naturally just use the specific collective's sub-treasury. In the case of (2), each collective that the Secretary collective serves would have to chip in funds to pay for the Secretary collective's services. The only thing that is wrong IMO is to make one Secretary collective for every other collective (effectively halving the total number of collectives that the chain can support). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the clarification, @muharem suggested we don't need the Tresury implementation, the Secretary Salary account can be funded using the Technical fellowship sub-treasury. When more members join who perform operations for other collectives, those collectives can also fund the Salary account using their respective sub-tresuries, a blocker to this might be the refusal of those collectives to comply. But a need for another Secretary member would have to arise before considering adding another member to the collective. We won't have a Secretary collective for every other collective, just the one and add more member through the demand for another that serves a seperate collective. I hope I have been able to understand the suggestions clearly so far 🙏. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we do not need a sub treasury to pay salaries or fund the Secretary salary account. The whole Secretary can be setup as ranked collective pallet and salary pallet and can serve later secretaries for all collectives. We can even have a different salary accounts for different secretary ranks if needed later. They can be funded by other collective's sub treasuries or the Polkadot Treasury. We do not need referenda and origins because in all cases right now we just need to check if the account belongs to a certain rank of the collective, no plurality voice needed. |
||
crate::xcm_config::XcmRouter, | ||
crate::PolkadotXcm, | ||
ConstU32<{ 6 * HOURS }>, | ||
AccountId, | ||
(), | ||
ConvertToValue<AssetHubUsdt>, | ||
AliasesIntoAccountId32<(), AccountId>, | ||
>; | ||
|
||
pub struct SalaryForRank; | ||
impl GetSalary<u16, AccountId, Balance> for SalaryForRank { | ||
fn get_salary(rank: u16, _who: &AccountId) -> Balance { | ||
if rank == 1 { | ||
6666 * USDT_UNITS | ||
} else { | ||
0 | ||
} | ||
} | ||
} | ||
|
||
impl pallet_salary::Config<SecretarySalaryInstance> for Runtime { | ||
type WeightInfo = (); // TODO weights::pallet_salary_secretary_salary::WeightInfo<Runtime>; | ||
type RuntimeEvent = RuntimeEvent; | ||
|
||
#[cfg(not(feature = "runtime-benchmarks"))] | ||
type Paymaster = SecretarySalaryPaymaster; | ||
#[cfg(feature = "runtime-benchmarks")] | ||
type Paymaster = crate::impls::benchmarks::PayWithEnsure< | ||
SecretarySalaryPaymaster, | ||
crate::impls::benchmarks::OpenHrmpChannel<ConstU32<1000>>, | ||
>; | ||
type Members = pallet_ranked_collective::Pallet<Runtime, SecretaryCollectiveInstance>; | ||
|
||
#[cfg(not(feature = "runtime-benchmarks"))] | ||
type Salary = SalaryForRank; | ||
#[cfg(feature = "runtime-benchmarks")] | ||
type Salary = frame_support::traits::tokens::ConvertRank< | ||
crate::impls::benchmarks::RankToSalary<Balances>, | ||
>; | ||
// 15 days to register for a salary payment. | ||
type RegistrationPeriod = ConstU32<{ 15 * DAYS }>; | ||
// 15 days to claim the salary payment. | ||
type PayoutPeriod = ConstU32<{ 15 * DAYS }>; | ||
// Total monthly salary budget. | ||
type Budget = ConstU128<{ 6666 * USDT_UNITS }>; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be nice to get this no-op impl for unit or some type in frame, where the trait is defined. we can leave a TODO here with the link to polkadot-sdk PR if you open it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe all the parts where we have the functions
unimplemented!()
(salary::tests::integration.rs
,core-fellowship::tests::integration.rs
)?