Skip to content

Conversation

xlc
Copy link
Contributor

@xlc xlc commented Aug 31, 2023

This means it will be possible to schedule recurring payments using Treasurer track.

Original PR paritytech/polkadot#7637

@bkchr bkchr added the T8-polkadot This PR/Issue is related to/affects the Polkadot network. label Aug 31, 2023
@joepetrowski
Copy link
Contributor

This doesn't seem inherently wrong, but we already have a solution for this, namely the valid_from field in the PR to update Treasury to use multiple types of assets. Just needs to be ported over to the new repo. (cc @muharem )

// OpenGov to schedule periodic auctions.
type ScheduleOrigin = EitherOf<EnsureRoot<AccountId>, AuctionAdmin>;
// Also allow Treasurer to schedule recurring payments.
type ScheduleOrigin = EitherOf<EitherOf<EnsureRoot<AccountId>, AuctionAdmin>, Treasurer>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems more readable since root is "special" although perhaps the tracks should go into a tuple to avoid further EitherOf nesting?

Suggested change
type ScheduleOrigin = EitherOf<EitherOf<EnsureRoot<AccountId>, AuctionAdmin>, Treasurer>;
type ScheduleOrigin = EitherOf<EnsureRoot<AccountId>, EitherOf<Treasurer, AuctionAdmin>>;

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed on tuple, this is much more readable:

Suggested change
type ScheduleOrigin = EitherOf<EitherOf<EnsureRoot<AccountId>, AuctionAdmin>, Treasurer>;
type ScheduleOrigin = (EnsureRoot<AccountId>, AuctionAdmin, Treasurer);

However, I just took a look and realized that EnsureOrigin doesn't have implementations on tuples, so we can't write this.

Copy link
Member

@gavofyork gavofyork left a comment

Choose a reason for hiding this comment

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

Should be fine as Treasurer is controllable by governance only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T8-polkadot This PR/Issue is related to/affects the Polkadot network.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants