Skip to content

Conversation

@mnaamani
Copy link
Member

@mnaamani mnaamani commented Mar 23, 2019

Builds on membership module, #3

Some refactoring of council elections, and runtime upgrade proposals module to only allow members to participate.

Adding a simple staking mechanism to enter the new roles introduced in athens. It follows a similar mechanism to validator staking, but allows for using a separate account than will be associated with a members account. A member can enter a role (become an actor for a role) with these steps:

  1. create a new account/keypair (the actor account)
  2. tx: with actor account, register a request to enter a specific role and use a particular member identity
  3. tx: (any account) transfer some tokens into actor account
  4. tx: using member account, approve request generated in step 2. If there is 'slot' to fill for the role
    and there is enough balance in the actor account it will be bonded and role is entered into.

Still todo:

  • paying out rewards
  • governance functions to
    • add/udpdate roles and role parameters
    • eject under performing actors
  • tests

@bwhm bwhm added this to the Finalize runtime for internal Athens Runtime Testnet milestone Mar 23, 2019
@mnaamani mnaamani mentioned this pull request Mar 25, 2019
2 tasks
@mnaamani mnaamani requested a review from jfinkhaeuser March 25, 2019 08:19
pub min_stake: BalanceOf<T>,

// the maximum number of spots available to fill for a role
pub max_actors: u32,
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to swap a position: first min_actors, then max_actors.
It's easier for perception like in counting: from smaller to bigger.

AvailableRoles get(available_roles) : Vec<Role>; // = vec![Role::Storage];

/// Actors list
Actors get(actors) : Vec<T::AccountId>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, this is a list of actors' account ids, not Actors

Suggested change
Actors get(actors) : Vec<T::AccountId>;
ActorIds get(actor_ids) : Vec<T::AccountId>;

Actors get(actors) : Vec<T::AccountId>;

/// actor accounts mapped to their actor
ActorsByAccountId get(actors_by_account_id) : map T::AccountId => Option<Actor<T>>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Actor, not Actors, because you return option of an actor, not a list of actors.

Suggested change
ActorsByAccountId get(actors_by_account_id) : map T::AccountId => Option<Actor<T>>;
ActorByAccountId get(actor_by_account_id) : map T::AccountId => Option<Actor<T>>;

ActorsByAccountId get(actors_by_account_id) : map T::AccountId => Option<Actor<T>>;

/// actor accounts associated with a role
AccountsByRole get(accounts_by_role) : map Role => Vec<T::AccountId>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
AccountsByRole get(accounts_by_role) : map Role => Vec<T::AccountId>;
AccountIdsByRole get(account_ids_by_role) : map Role => Vec<T::AccountId>;

AccountsByRole get(accounts_by_role) : map Role => Vec<T::AccountId>;

/// actor accounts associated with a member id
AccountsByMember get(accounts_by_member) : map MemberId<T> => Vec<T::AccountId>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
AccountsByMember get(accounts_by_member) : map MemberId<T> => Vec<T::AccountId>;
AccountIdsByMemberId get(account_ids_by_member_id) : map MemberId<T> => Vec<T::AccountId>;

}

impl<T: Trait> Module<T> {
fn role_is_available(role: Role) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fn role_is_available(role: Role) -> bool {
fn is_role_available(role: Role) -> bool {


fn on_initialise(now: T::BlockNumber) {
// clear expired requests
if now % T::BlockNumber::sa(DEFAULT_REQUEST_CLEARING_INTERVAL) == T::BlockNumber::zero() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider creating an extra variable just to make code easier to read, then you can get rid of a comment:

Suggested change
if now % T::BlockNumber::sa(DEFAULT_REQUEST_CLEARING_INTERVAL) == T::BlockNumber::zero() {
let is_request_expired = now % T::BlockNumber::sa(DEFAULT_REQUEST_CLEARING_INTERVAL) == T::BlockNumber::zero()
if is_request_expired {

Copy link
Member Author

Choose a reason for hiding this comment

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

I see you point in general, but in this exact case the that check is just a check to see if we reached the interval where we actually check each request.

@mnaamani mnaamani marked this pull request as ready for review April 1, 2019 22:11
@siman siman merged commit c9162fe into Joystream:development Apr 2, 2019
semeano pushed a commit to semeano/joystream that referenced this pull request May 14, 2020
semeano added a commit to semeano/joystream that referenced this pull request May 14, 2020
Lezek123 referenced this pull request in Lezek123/substrate-runtime-joystream May 21, 2020
* Add homepage URL

* Allow deployment to gh-pages branch

* Bump @polkadot/dev (with deploy:ghpages)
@mnaamani mnaamani deleted the staked-roles branch May 22, 2020 20:33
bedeho pushed a commit that referenced this pull request May 29, 2020
mnaamani referenced this pull request in mnaamani/joystream Jun 19, 2020
mnaamani pushed a commit that referenced this pull request Oct 19, 2020
bwhm pushed a commit that referenced this pull request Apr 6, 2021
bedeho pushed a commit that referenced this pull request Dec 14, 2021
Lezek123 pushed a commit that referenced this pull request Jul 7, 2022
Fix channel agent permissions test cases for initialize/cancel channel transfer
mnaamani pushed a commit that referenced this pull request Jan 4, 2023
…on-test

fix: terminateLeads integraion-tests job in full scenario
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.

3 participants