Changes to the working group: make leader hireable.

[shamil-gadelshin]

The working group leader should be hirable like a regular worker. All working group operations should be applicable to the leader. Only the council is allowed to invoke operations with the leader.

Changes

• hiring workflow
• roles operations
• stake management
• improve comments
• stake refunding
• move out content working group structs to the separate file in the runtime
• optional slashing on the role termination
• changed get_worker_ids() (excluded the leader and renamed)
• new extrinsic added (update_reward_amount)

[shamil-gadelshin]

I made a refactoring in the runtime/lib.rs. Moved out content working group structs to the separate file and plan to move all structs of the versioned store module.

runtime/lib.rs is 1K lines of code. It is a very complex and concept rich file.
Could we introduce a rule - "Don't place the integration code in the runtime/lib.rs"? @mnaamani @bedeho

[bedeho]

• A heroic effort on all these updates, not easy to review though. I think it would have been better to separate out the name changes in a separate PR from other substntial conceptual changes, any way you could separate the two now?

• I don't think the representation of the lead vs. worker is quite right, fragments such as this in update_role_account are a telltale sign

            // Update role account
            WorkerById::<T, I>::mutate(worker_id, |worker| {
                worker.role_account = new_role_account_id.clone()
            });

            // Update lead data if it is necessary.
            let lead = <CurrentLead::<T, I>>::get();
            if let Some(lead) = lead {
                if lead.worker_id == worker_id {
                    let new_lead = Lead{
                        role_account_id: new_role_account_id.clone(),
                        ..lead
                    };
                    <CurrentLead::<T, I>>::put(new_lead);
                }
            }

We are basically having to go and keep two separate but duplicated state fields in synch.
Or the fact that when you check that a worker is the lead, you do not even need to consult whether CurrentLead is actually set.

Initially, I was going to say that we should drop Lead type and make CurrentLead just hold a member id, but now that I think of it, why do we even need to hold on to any internal representation of who the current lead is in the module at all? We could just drop both? Let me know if I am lost at sea here, if not, lets do this change before I review any further.

[shamil-gadelshin]

I tried to have ID cache for the leader, but I agree with you that it creates more problems than solves. I change it to the worker_id.

[bedeho]

Looks good!

I would say that this still was too much for one PR, quite a few changes did seem orthogonal and could have been delivered separately.

[bedeho]

LGTM, nice job.