Evaluator reputation update

[lazovicff]

Implements evaluator reputation update part for #38

• Setting rating of the evaluator when reputation is calculated.
• Updating reputation for every role using loop.
• Introduced payout for evaluator and updated tests accordingly.
• Split setTaskPayout to 3 functions, each for every role. Reviews are needed for setTaskEvaluatorPayout and setTaskWorkerPayout.
• setTaskManagerPayout can be only called by a manager of the task.
• Updated gas costs.

[elenadimitrova]

I know this looks 'clean' when used in the context with other role ratings but it will cost ~20,000 gas. Scaling the implementation is one of our biggest concerns so we get twitchy when it comes to gas costs.
It will still be good design if you extracted role.rating into a local memory variable (which costs little) when you're doing calculations and selectively assign it to 50 there.

[lazovicff]

Yes, initially it was done just before calculating the reputation.

I don't know exactly how objections will work, but the reason i decided to do it this way is because maybe we would want to allow objections to the evaluator rating to be made before actual calculation of the reputation.

[elenadimitrova]

You're right, objections will be done before reputation calculation but the evaluator rating won't be needed there still.

[elenadimitrova]

I don't see the benefit of calling another function 3 times from here. This incurs some unnecessary extra reads from storage. We can refactor out the internal function in cases where the logic lies in multiple functions but here it's all within finalizeTask. Plus with the suggestion from above of balancing gas costs and design you best keep it all in this function.

[elenadimitrova]

A small note here to switch the order in which appendReputationUpdateLog is called to match the order of role ids, i.e. manager (roleId = 0), evaluator (roleId = 1), worker (roleId = 2). Just so when getting the respective reputation log entries, it's more intuitive.

[lazovicff]

I was getting the 'stack too deep' error as i mentioned in description, because of too much local variables (16 is max i think).

I will try to bring back everything into finalizeTask, but i will probably remove some local variables like this one or this one and access them directly here.
What do you think?

[elenadimitrova]

I had problems when I tried to do that if I remember correctly. But otherwise yes, try cutting down some of the local variables.

[elenadimitrova]

skillId does not need to be explicit as well.

[elenadimitrova]

Note that setTaskPayout transaction reviewers are currently the task Manager and Worker. However when setting the Evaluator the reviewers logically have to be the Manager and Evaluator. With the current ColonyTransactionReviewer design this is not possible. So we have to either:
a) Split the setTaskPayout in two functions, e.g. setTaskWorkerPayout and setTaskEvaluatorPayout. The disadvantage here is that for additional roles we may want to support in future, i.e. in addition to manager, evaluator and worker, we'll have to keep introducing new functions so we lose 1 level of abstraction.

b) We refactor the ColonyTransactionReviewer to support this model.

I haven't explored option b) so feel free to experiment. Overall I'd prefer option a) for simplicity but I want to hear what @area thinks about it.

[lazovicff]

For now i created separate functions for every role. I will try to find better solution after this PR is merged, because these changes are required for #154.

[elenadimitrova]

@lazoviccorp This works for the taskFinalize logic I think

    Task storage task = tasks[_id];
    IColonyNetwork colonyNetworkContract = IColonyNetwork(colonyNetworkAddress);

    for (uint8 roleId = 0; roleId <= 2; roleId ++) {
      uint payout = task.payouts[roleId][token];
      Role storage role = task.roles[roleId];
      uint8 rating = (roleId == EVALUATOR) ? 50 : role.rating;
      int divider = (roleId == WORKER) ? 30 : 50;
      int reputation = SafeMath.mulInt(int(payout), (int(rating)*2 - 50)) / divider;
      //TODO: Change the skillId to represent the task domain
      colonyNetworkContract.appendReputationUpdateLog(role.user, reputation, task.skills[0]);
    }

    task.finalized = true;

Note that this doesn't implement the todo fix

[elenadimitrova]

It doesn't make sense to get the worker to agree to what the manager payout should be. Just don't permission the setTaskManagerPayout function for now.