Skip to content

Commit

Permalink
Add support for Trezor-style message signing
Browse files Browse the repository at this point in the history
  • Loading branch information
@area
area committed May 15, 2018
1 parent 4bb1682 commit 4afdc90
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 34 deletions.
32 changes: 19 additions & 13 deletions contracts/ColonyTask.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,23 +139,30 @@ contract ColonyTask is ColonyStorage, DSMath {
return taskChangeNonce;
}

function getSignedMessageHash(uint256 _value, bytes _data) private returns (bytes32 txHash) {
return keccak256(
"\x19Ethereum Signed Message:\n32",
keccak256(
address(this),
address(this),
_value,
_data,
taskChangeNonce
)

function getSignedMessageHash(uint256 _value, bytes _data, uint8 _mode) private returns (bytes32 txHash) {
bytes32 hash = keccak256(
address(this),
address(this),
_value,
_data,
taskChangeNonce
);
if (_mode==0) {
// 'Normal' mode - geth, etc.
return keccak256("\x19Ethereum Signed Message:\n32", hash);
} else {
// Trezor mode
// Correct incantation helpfull cribbed from https:/trezor/trezor-mcu/issues/163#issuecomment-368435292
return keccak256("\x19Ethereum Signed Message:\n\x20", hash);
}
}

function executeTaskChange(
uint8[] _sigV,
bytes32[] _sigR,
bytes32[] _sigS,
uint8[] _mode,
uint256 _value,
bytes _data) public
{
Expand All @@ -177,12 +184,11 @@ contract ColonyTask is ColonyStorage, DSMath {
// Checks at least one of the two reviewers registered is different to the task manager
require(r1 != MANAGER || r2 != MANAGER);

bytes32 txHash = getSignedMessageHash(_value, _data);

address[] memory reviewerAddresses = new address[](2);
for (uint i = 0; i < 2; i++) {
reviewerAddresses[i] = ecrecover(txHash, _sigV[i], _sigR[i], _sigS[i]);
reviewerAddresses[i] = ecrecover(getSignedMessageHash(_value, _data, _mode[i]), _sigV[i], _sigR[i], _sigS[i]);
}

require(task.roles[r1].user == reviewerAddresses[0] || task.roles[r1].user == reviewerAddresses[1]);
require(task.roles[r2].user == reviewerAddresses[0] || task.roles[r2].user == reviewerAddresses[1]);

Expand Down
3 changes: 2 additions & 1 deletion contracts/IColony.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,11 @@ contract IColony {
/// @param _sigV recovery id
/// @param _sigR r output of the ECDSA signature of the transaction
/// @param _sigS s output of the ECDSA signature of the transaction
/// @param _mode How the signature was generated - 0 for Geth-style (usual), 1 for Trezor-style (only Trezor does this)
/// @param _value The transaction value, i.e. number of wei to be sent when the transaction is executed
/// Currently we only accept 0 value transactions but this is kept as a future option
/// @param _data The transaction data
function executeTaskChange(uint8[] _sigV, bytes32[] _sigR, bytes32[] _sigS, uint256 _value, bytes _data) public;
function executeTaskChange(uint8[] _sigV, bytes32[] _sigR, bytes32[] _sigS, uint8[] _mode, uint256 _value, bytes _data) public;

/// @notice Submit a hashed secret of the rating for work in task `_id` which was performed by user with task role id `_role`
/// Allowed within 5 days period starting which whichever is first from either the deliverable being submitted or the dueDate been reached
Expand Down
8 changes: 4 additions & 4 deletions gasCosts/gasCosts.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,14 @@ contract("All", accounts => {
// setTaskBrief
txData = await colony.contract.setTaskBrief.getData(1, SPECIFICATION_HASH);
sigs = await createSignatures(colony, [MANAGER, WORKER], 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData);

// setTaskDueDate
let dueDate = await currentBlockTime();
dueDate += SECONDS_PER_DAY * 5;
txData = await colony.contract.setTaskDueDate.getData(1, dueDate);
sigs = await createSignatures(colony, [MANAGER, WORKER], 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData);

// moveFundsBetweenPots
await colony.moveFundsBetweenPots(1, 2, 150, tokenAddress);
Expand All @@ -134,12 +134,12 @@ contract("All", accounts => {
// setTaskEvaluatorPayout
txData = await colony.contract.setTaskEvaluatorPayout.getData(1, tokenAddress, 40);
sigs = await createSignatures(colony, [MANAGER, EVALUATOR], 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData);

// setTaskWorkerPayout
txData = await colony.contract.setTaskWorkerPayout.getData(1, tokenAddress, 100);
sigs = await createSignatures(colony, [MANAGER, WORKER], 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData);

// submitTaskDeliverable
await colony.submitTaskDeliverable(1, DELIVERABLE_HASH, { from: WORKER, gasPrice });
Expand Down
6 changes: 3 additions & 3 deletions helpers/test-data-generator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export async function setupAssignedTask({ colonyNetwork, colony, dueDate, domain
const txData = await colony.contract.setTaskDueDate.getData(taskId, dueDateTimestamp);
const signers = [MANAGER, WORKER];
const sigs = await createSignatures(colony, signers, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData);

return taskId;
}
Expand Down Expand Up @@ -85,11 +85,11 @@ export async function setupFundedTask({

txData = await colony.contract.setTaskEvaluatorPayout.getData(taskId, tokenAddress, evaluatorPayout.toString());
sigs = await createSignatures(colony, [MANAGER, EVALUATOR], 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData);

txData = await colony.contract.setTaskWorkerPayout.getData(taskId, tokenAddress, workerPayout.toString());
sigs = await createSignatures(colony, [MANAGER, WORKER], 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData);
return taskId;
}

Expand Down
26 changes: 13 additions & 13 deletions test/colony.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ contract("Colony", addresses => {
let txData = await colony.contract.setTaskBrief.getData(1, SPECIFICATION_HASH_UPDATED);
const signers = [MANAGER, WORKER];
let sigs = await createSignatures(colony, signers, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData);

let taskChangeNonce = await colony.getTaskChangeNonce.call();
assert.equal(taskChangeNonce, 1);
Expand All @@ -308,7 +308,7 @@ contract("Colony", addresses => {
const dueDate = await currentBlockTime();
txData = await colony.contract.setTaskDueDate.getData(1, dueDate);
sigs = await createSignatures(colony, signers, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData);

taskChangeNonce = await colony.getTaskChangeNonce.call();
assert.equal(taskChangeNonce, 2);
Expand All @@ -320,7 +320,7 @@ contract("Colony", addresses => {
const txData = await colony.contract.setTaskBrief.getData(1, SPECIFICATION_HASH_UPDATED);
const signers = [MANAGER, WORKER];
const sigs = await createSignatures(colony, signers, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData);
const task = await colony.getTask.call(1);
assert.equal(hexToUtf8(task[0]), SPECIFICATION_HASH_UPDATED);
});
Expand All @@ -333,7 +333,7 @@ contract("Colony", addresses => {
const txData = await colony.contract.setTaskDueDate.getData(1, dueDate);
const signers = [MANAGER, WORKER];
const sigs = await createSignatures(colony, signers, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData);

const task = await colony.getTask.call(1);
assert.equal(task[4], dueDate);
Expand All @@ -352,7 +352,7 @@ contract("Colony", addresses => {
const signers = [MANAGER, OTHER];
const sigs = await createSignatures(colony, signers, 0, txData);

await checkErrorRevert(colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData));
await checkErrorRevert(colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData));
});

it("should fail update of task brief signed by manager and evaluator", async () => {
Expand All @@ -364,7 +364,7 @@ contract("Colony", addresses => {
const signers = [MANAGER, EVALUATOR];
const sigs = await createSignatures(colony, signers, 0, txData);

await checkErrorRevert(colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData));
await checkErrorRevert(colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData));
});

it("should fail to execute task change for a non-registered function signature", async () => {
Expand All @@ -373,7 +373,7 @@ contract("Colony", addresses => {
const signers = [MANAGER, EVALUATOR];
const sigs = await createSignatures(colony, signers, 0, txData);

await checkErrorRevert(colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData));
await checkErrorRevert(colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData));
});

it("should fail to execute change of task brief, using an invalid task id", async () => {
Expand All @@ -382,7 +382,7 @@ contract("Colony", addresses => {
const signers = [MANAGER, EVALUATOR];
const sigs = await createSignatures(colony, signers, 0, txData);

await checkErrorRevert(colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData));
await checkErrorRevert(colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData));
});

it("should fail to execute task change, if the task is already finalized", async () => {
Expand All @@ -394,7 +394,7 @@ contract("Colony", addresses => {
const signers = [MANAGER, EVALUATOR];
const sigs = await createSignatures(colony, signers, 0, txData);

await checkErrorRevert(colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData));
await checkErrorRevert(colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData));
});
});

Expand Down Expand Up @@ -581,21 +581,21 @@ contract("Colony", addresses => {
// Set the evaluator payout as 1000 ethers
const txData1 = await colony.contract.setTaskEvaluatorPayout.getData(1, 0x0, 1000);
const sigs = await createSignatures(colony, [MANAGER, EVALUATOR], 0, txData1);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, 0, txData1);
await colony.executeTaskChange(sigs.sigV, sigs.sigR, sigs.sigS, [0, 0], 0, txData1);

// Set the evaluator payout as 40 colony tokens
const txData2 = await colony.contract.setTaskEvaluatorPayout.getData(1, token.address, 40);
const sigs2 = await createSignatures(colony, [MANAGER, EVALUATOR], 0, txData2);
await colony.executeTaskChange(sigs2.sigV, sigs2.sigR, sigs2.sigS, 0, txData2);
await colony.executeTaskChange(sigs2.sigV, sigs2.sigR, sigs2.sigS, [0, 0], 0, txData2);

// Set the worker payout as 98000 wei and 200 colony tokens
const txData3 = await colony.contract.setTaskWorkerPayout.getData(1, 0x0, 98000);
const sigs3 = await createSignatures(colony, [MANAGER, WORKER], 0, txData3);
await colony.executeTaskChange(sigs3.sigV, sigs3.sigR, sigs3.sigS, 0, txData3);
await colony.executeTaskChange(sigs3.sigV, sigs3.sigR, sigs3.sigS, [0, 0], 0, txData3);

const txData4 = await colony.contract.setTaskWorkerPayout.getData(1, token.address, 200);
const sigs4 = await createSignatures(colony, [MANAGER, WORKER], 0, txData4);
await colony.executeTaskChange(sigs4.sigV, sigs4.sigR, sigs4.sigS, 0, txData4);
await colony.executeTaskChange(sigs4.sigV, sigs4.sigR, sigs4.sigS, [0, 0], 0, txData4);

const taskPayoutManager1 = await colony.getTaskPayout.call(1, MANAGER_ROLE, 0x0);
assert.equal(taskPayoutManager1.toNumber(), 5000);
Expand Down

0 comments on commit 4afdc90

Please sign in to comment.