Skip to content
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

Open
wants to merge 45 commits into
base: main
Choose a base branch
from

Conversation

Doordashcon
Copy link
Contributor

@Doordashcon Doordashcon commented Jun 10, 2024

Closes polkadot-fellows/help-center#10

On Chain Structure

The on-chain program consists of one rank(SECRETARY) and the default Candidate(SECRETARY_CANDIDATE):

Membership Management

Members themselves are without the power to promote or demote, this right is reserved to token holders via Polkadot OpenGov and votes by Fellow of the Technical Fellowship.

@Doordashcon
Copy link
Contributor Author

Hopefully this is ready for review @ggwpez @muharem

@Doordashcon
Copy link
Contributor Author

Screenshot 2024-06-23 at 13 53 08

@Doordashcon
Copy link
Contributor Author

Doordashcon commented Jun 23, 2024

Tested with Chopsticks

@Doordashcon
Copy link
Contributor Author

Ready for review @muharem @ggwpez @franciscoaguirre

@Doordashcon
Copy link
Contributor Author

Please review @muharem @bkchr @ggwpez 🙏

Copy link
Contributor

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are almost there, just some last tiny questions.


/// [`PayOverXcm`] setup to pay the Secretary salary on the AssetHub in USDT.
pub type SecretarySalaryPaymaster = PayOverXcm<
SecretarySalaryInteriorLocation,
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we control the account using a fellowship proposal?

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Doordashcon what do you mean?

Copy link
Contributor Author

@Doordashcon Doordashcon Aug 9, 2024

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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:

  1. Special rank of Secretary within Ranked Collective (it doesn't need to be called that in the pallet, could even be a rank 10 for example and the origins are configured to treat that as special);
  2. Separate Secretary collective with its own sub-treasury.

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).

Copy link
Contributor Author

@Doordashcon Doordashcon Aug 9, 2024

Choose a reason for hiding this comment

The 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 🙏.

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

@@ -35,6 +35,8 @@ pub mod account {
pub const FELLOWSHIP_TREASURY_PALLET_ID: PalletId = PalletId(*b"py/feltr");
/// Ambassador treasury pallet ID
pub const AMBASSADOR_TREASURY_PALLET_ID: PalletId = PalletId(*b"py/ambtr");
/// Secretary treasury pallet ID
pub const SECRETARY_TREASURY_PALLET_ID: PalletId = PalletId(*b"py/secrt");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we do not need this anymore

pub use pallet_origins::*;

#[frame_support::pallet]
pub mod pallet_origins {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably no need for this pallet as well

PhantomData<(T, I)>,
);

impl<T: pallet_ranked_collective::Config<I>, I: 'static> Polling<TallyOf<T, I>>
Copy link
Contributor

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.

Copy link
Contributor Author

@Doordashcon Doordashcon Aug 10, 2024

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)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a secretary collective
4 participants