Skip to content

Commit

Permalink
Optimize bytecode for Tasks.sol
Browse files Browse the repository at this point in the history
  • Loading branch information
@kronosapiens
kronosapiens committed Oct 29, 2019
1 parent 3c47435 commit 36809e0
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 121 deletions.
100 changes: 54 additions & 46 deletions contracts/extensions/Tasks.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,33 +116,33 @@ contract Tasks is DSMath {
}

modifier self() {
require(address(this) == msg.sender, "colony-task-not-self");
require(address(this) == msg.sender, "task-not-self");
_;
}

ColonyDataTypes.ColonyRole constant ADMIN = ColonyDataTypes.ColonyRole.Administration;
modifier isAdmin(address _user, uint256 _permissionDomainId, uint256 _childSkillIndex, uint256 _domainId) {
ColonyDataTypes.ColonyRole admin = ColonyDataTypes.ColonyRole.Administration;
require(colony.hasInheritedUserRole(_user, _permissionDomainId, admin, _childSkillIndex, _domainId), "colony-task-not-admin");
require(colony.hasInheritedUserRole(_user, _permissionDomainId, ADMIN, _childSkillIndex, _domainId), "task-not-admin");
_;
}

modifier taskExists(uint256 _id) {
require(_id > 0 && _id <= taskCount, "colony-task-does-not-exist");
require(doesTaskExist(_id), "task-does-not-exist");
_;
}

modifier taskComplete(uint256 _id) {
require(tasks[_id].completionTimestamp > 0, "colony-task-not-complete");
require(isTaskComplete(_id), "task-not-complete");
_;
}

modifier taskNotComplete(uint256 _id) {
require(tasks[_id].completionTimestamp == 0, "colony-task-complete");
require(!isTaskComplete(_id), "task-complete");
_;
}

modifier confirmTaskRoleIdentity(uint256 _id, address _user, TaskRole _role) {
require(msg.sender == getTaskRoleUser(_id, _role), "colony-task-role-identity-mismatch");
require(getTaskRoleUser(_id, _role) == msg.sender, "task-role-identity-mismatch");
_;
}

Expand All @@ -156,17 +156,17 @@ contract Tasks is DSMath {
)
public
{
require(_value == 0, "colony-task-change-non-zero-value");
require(_sigR.length == _sigS.length && _sigR.length == _sigV.length, "colony-task-change-signatures-count-do-not-match");
require(_value == 0, "task-change-non-zero-value");
require(_sigR.length == _sigS.length && _sigR.length == _sigV.length, "task-change-sig-count-no-match");

bytes4 sig;
uint256 taskId;
(sig, taskId) = deconstructCall(_data);
require(taskId > 0 && taskId <= taskCount, "colony-task-does-not-exist");
require(!roleAssignmentSigs[sig], "colony-task-change-is-role-assignment");
require(taskId > 0 && taskId <= taskCount, "task-does-not-exist");
require(!roleAssignmentSigs[sig], "task-change-is-role-assign");

ColonyDataTypes.ExpenditureStatus status = colony.getExpenditure(tasks[taskId].expenditureId).status;
require(status != ColonyDataTypes.ExpenditureStatus.Finalized, "colony-task-finalized");
require(status != ColonyDataTypes.ExpenditureStatus.Finalized, "task-finalized");

uint8 nSignaturesRequired;
address taskRole1User = getTaskRoleUser(taskId, TaskRole(reviewers[sig][0]));
Expand All @@ -180,26 +180,26 @@ contract Tasks is DSMath {
} else {
nSignaturesRequired = 2;
}
require(_sigR.length == nSignaturesRequired, "colony-task-change-does-not-meet-signatures-required");
require(_sigR.length == nSignaturesRequired, "task-change-wrong-num-sigs");

bytes32 msgHash = keccak256(abi.encodePacked(address(this), address(this), _value, _data, tasks[taskId].changeNonce));
address[] memory reviewerAddresses = getReviewerAddresses(_sigV, _sigR, _sigS, _mode, msgHash);

require(
reviewerAddresses[0] == taskRole1User || reviewerAddresses[0] == taskRole2User,
"colony-task-signatures-do-not-match-reviewer-1"
"task-sigs-no-match-reviewer-1"
);

if (nSignaturesRequired == 2) {
require(reviewerAddresses[0] != reviewerAddresses[1], "colony-task-duplicate-reviewers");
require(reviewerAddresses[0] != reviewerAddresses[1], "task-duplicate-reviewers");
require(
reviewerAddresses[1] == taskRole1User || reviewerAddresses[1] == taskRole2User,
"colony-task-signatures-do-not-match-reviewer-2"
"task-sigs-no-match-reviewer-2"
);
}

tasks[taskId].changeNonce += 1;
require(executeCall(address(this), _value, _data), "colony-task-change-execution-failed");
require(executeCall(address(this), _value, _data), "task-change-execution-failed");
}

function executeTaskRoleAssignment(
Expand All @@ -212,18 +212,18 @@ contract Tasks is DSMath {
)
public
{
require(_value == 0, "colony-task-role-assignment-non-zero-value");
require(_sigR.length == _sigS.length && _sigR.length == _sigV.length, "colony-task-role-assignment-signatures-count-do-not-match");
require(_value == 0, "task-role-assign-non-zero-value");
require(_sigR.length == _sigS.length && _sigR.length == _sigV.length, "task-role-assign-sig-count-no-match");

bytes4 sig;
uint256 taskId;
address userAddress;
(sig, taskId, userAddress) = deconstructRoleChangeCall(_data);
require(taskId > 0 && taskId <= taskCount, "colony-task-does-not-exist");
require(roleAssignmentSigs[sig], "colony-task-change-is-not-role-assignment");
require(taskId > 0 && taskId <= taskCount, "task-does-not-exist");
require(roleAssignmentSigs[sig], "task-change-is-not-role-assign");

ColonyDataTypes.ExpenditureStatus status = colony.getExpenditure(tasks[taskId].expenditureId).status;
require(status != ColonyDataTypes.ExpenditureStatus.Finalized, "colony-task-finalized");
require(status != ColonyDataTypes.ExpenditureStatus.Finalized, "task-finalized");

uint8 nSignaturesRequired;
address manager = getTaskRoleUser(taskId, TaskRole.Manager);
Expand All @@ -233,36 +233,36 @@ contract Tasks is DSMath {
} else {
nSignaturesRequired = 2;
}
require(_sigR.length == nSignaturesRequired, "colony-task-role-assignment-does-not-meet-required-signatures");
require(_sigR.length == nSignaturesRequired, "task-role-assign-wrong-num-sigs");

bytes32 msgHash = keccak256(abi.encodePacked(address(this), address(this), _value, _data, tasks[taskId].changeNonce));
address[] memory reviewerAddresses = getReviewerAddresses(_sigV, _sigR, _sigS, _mode, msgHash);

if (nSignaturesRequired == 1) {
// Since we want to set a manager as an evaluator, require just manager's signature
require(reviewerAddresses[0] == manager, "colony-task-role-assignment-not-signed-by-manager");
require(reviewerAddresses[0] == manager, "task-role-assign-no-manager-sig");
} else {
// One of signers must be a manager
require(
reviewerAddresses[0] == manager ||
reviewerAddresses[1] == manager,
"colony-task-role-assignment-not-signed-by-manager"
"task-role-assign-no-manager-sig"
);

// One of the signers must be an address we want to set here
require(
userAddress == reviewerAddresses[0] || userAddress == reviewerAddresses[1],
"colony-task-role-assignment-not-signed-by-new-user-for-role"
"task-role-assign-no-new-user-sig"
);
// Require that signatures are not from the same address
// This will never throw, because we require that manager is one of the signers,
// and if manager is both signers, then `userAddress` must also be a manager, and if
// `userAddress` is a manager, then we require 1 signature (will be kept for possible future changes)
require(reviewerAddresses[0] != reviewerAddresses[1], "colony-task-role-assignment-duplicate-signatures");
require(reviewerAddresses[0] != reviewerAddresses[1], "task-role-assign-duplicate-sigs");
}

tasks[taskId].changeNonce += 1;
require(executeCall(address(this), _value, _data), "colony-task-role-assignment-execution-failed");
require(executeCall(address(this), _value, _data), "task-role-assign-exec-failed");
}

// Permissions pertain to the Administration role here
Expand Down Expand Up @@ -302,15 +302,15 @@ contract Tasks is DSMath {
taskComplete(_id)
{
if (_role == TaskRole.Manager) { // Manager rated by worker
require(msg.sender == getTaskRoleUser(_id, TaskRole.Worker), "colony-user-cannot-rate-task-manager");
require(msg.sender == getTaskRoleUser(_id, TaskRole.Worker), "task-user-cannot-rate-manager");
} else if (_role == TaskRole.Worker) { // Worker rated by evaluator
require(msg.sender == getTaskRoleUser(_id, TaskRole.Evaluator), "colony-user-cannot-rate-task-worker");
require(msg.sender == getTaskRoleUser(_id, TaskRole.Evaluator), "task-user-cannot-rate-worker");
} else {
revert("colony-unsupported-role-to-rate");
revert("task-unsupported-role-to-rate");
}

require(sub(now, tasks[_id].completionTimestamp) <= RATING_COMMIT_TIMEOUT, "colony-task-rating-secret-submit-period-closed");
require(ratingSecrets[_id].secret[uint8(_role)] == "", "colony-task-rating-secret-already-exists");
require(sub(now, tasks[_id].completionTimestamp) <= RATING_COMMIT_TIMEOUT, "task-secret-submissions-closed");
require(ratingSecrets[_id].secret[uint8(_role)] == "", "task-secret-already-exists");

ratingSecrets[_id].count += 1;
ratingSecrets[_id].timestamp = now;
Expand All @@ -326,18 +326,18 @@ contract Tasks is DSMath {
// Otherwise start the reveal period after the commit period has expired
// In both cases, keep reveal period open for 5 days
if (ratingSecrets[_id].count == 2) {
require(sub(now, ratingSecrets[_id].timestamp) <= RATING_REVEAL_TIMEOUT, "colony-task-rating-secret-reveal-period-closed");
require(sub(now, ratingSecrets[_id].timestamp) <= RATING_REVEAL_TIMEOUT, "task-secret-reveal-closed");
} else {
uint taskCompletionTime = tasks[_id].completionTimestamp;
require(sub(now, taskCompletionTime) > RATING_COMMIT_TIMEOUT, "colony-task-rating-secret-reveal-period-not-open");
require(sub(now, taskCompletionTime) <= add(RATING_COMMIT_TIMEOUT, RATING_REVEAL_TIMEOUT), "colony-task-rating-secret-reveal-period-closed");
require(sub(now, taskCompletionTime) > RATING_COMMIT_TIMEOUT, "task-secret-reveal-not-open");
require(sub(now, taskCompletionTime) <= add(RATING_COMMIT_TIMEOUT, RATING_REVEAL_TIMEOUT), "task-secret-reveal-closed");
}

bytes32 secret = generateSecret(_salt, _rating);
require(secret == ratingSecrets[_id].secret[uint8(_role)], "colony-task-rating-secret-mismatch");
require(secret == ratingSecrets[_id].secret[uint8(_role)], "task-secret-mismatch");

TaskRatings rating = TaskRatings(_rating);
require(rating != TaskRatings.None, "colony-task-rating-missing");
require(rating != TaskRatings.None, "task-rating-missing");
taskRoles[_id][uint8(_role)].rating = rating;

emit TaskWorkRatingRevealed(_id, _role, _rating);
Expand Down Expand Up @@ -366,15 +366,15 @@ contract Tasks is DSMath {

function setTaskEvaluatorRole(uint256 _id, address payable _user) public self {
// Can only assign role if no one is currently assigned to it
require(getTaskRoleUser(_id, TaskRole.Evaluator) == address(0x0), "colony-task-evaluator-role-already-assigned");
require(getTaskRoleUser(_id, TaskRole.Evaluator) == address(0x0), "task-evaluator-role-assigned");
setTaskRoleUser(_id, TaskRole.Evaluator, _user);
}

function setTaskWorkerRole(uint256 _id, address payable _user) public self {
// Can only assign role if no one is currently assigned to it
require(getTaskRoleUser(_id, TaskRole.Worker) == address(0x0), "colony-task-worker-role-already-assigned");
require(getTaskRoleUser(_id, TaskRole.Worker) == address(0x0), "task-worker-role-assigned");
uint256[] memory skills = colony.getExpenditureSlot(tasks[_id].expenditureId, uint256(TaskRole.Worker)).skills;
require(skills[0] > 0, "colony-task-skill-not-set"); // ignore-swc-110
require(skills.length > 0 && skills[0] > 0, "task-skill-not-set"); // ignore-swc-110
setTaskRoleUser(_id, TaskRole.Worker, _user);
}

Expand Down Expand Up @@ -413,8 +413,8 @@ contract Tasks is DSMath {
address evaluator = getTaskRoleUser(_id, TaskRole.Evaluator);
address worker = getTaskRoleUser(_id, TaskRole.Worker);

require(evaluator == manager || evaluator == address(0x0), "colony-funding-evaluator-already-set");
require(worker == manager || worker == address(0x0), "colony-funding-worker-already-set");
require(evaluator == manager || evaluator == address(0x0), "task-evaluator-already-set");
require(worker == manager || worker == address(0x0), "task-worker-already-set");

this.setTaskManagerPayout(_id, _token, _managerAmount);
this.setTaskEvaluatorPayout(_id, _token, _evaluatorAmount);
Expand Down Expand Up @@ -471,7 +471,7 @@ contract Tasks is DSMath {
taskNotComplete(_id)
confirmTaskRoleIdentity(_id, msg.sender, TaskRole.Manager)
{
require(now >= tasks[_id].dueDate, "colony-task-due-date-in-future");
require(now >= tasks[_id].dueDate, "task-due-date-in-future");
tasks[_id].completionTimestamp = now;

emit TaskCompleted(_id);
Expand Down Expand Up @@ -643,7 +643,7 @@ contract Tasks is DSMath {
}

function assignWorkRatings(uint256 _id) internal {
require(taskWorkRatingsAssigned(_id) || taskWorkRatingsClosed(_id), "colony-task-ratings-not-closed");
require(taskWorkRatingsAssigned(_id) || taskWorkRatingsClosed(_id), "task-ratings-not-closed");

// In the event of a user not committing/revealing within the rating window,
// their rating of their counterpart is assumed to be the maximum
Expand All @@ -668,7 +668,7 @@ contract Tasks is DSMath {
}

function setTaskRoleUser(uint256 _id, TaskRole _role, address payable _user)
private
internal
taskExists(_id)
taskNotComplete(_id)
{
Expand All @@ -679,4 +679,12 @@ contract Tasks is DSMath {

colony.setExpenditureRecipient(tasks[_id].expenditureId, uint256(_role), _user);
}

function doesTaskExist(uint256 _id) internal view returns (bool) {
return _id > 0 && _id <= taskCount;
}

function isTaskComplete(uint256 _id) internal view returns (bool) {
return tasks[_id].completionTimestamp > 0;
}
}
Loading

0 comments on commit 36809e0

Please sign in to comment.