Skip to content
This repository has been archived by the owner on Jan 27, 2022. It is now read-only.

adding workerregistry and workorderregistry for the proxy model #72

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
/* Copyright 2019 iExec Blockchain Tech
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

pragma solidity ^0.5.0;

import "./libs/SignatureVerifier.sol";

contract WorkOrderRegistry is SignatureVerifier
{
uint256 constant public TIMEOUT = 1 days;

enum WorkOrderStatus
Copy link
Contributor

Choose a reason for hiding this comment

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

Work order status has pending and processing as per the spec -https://entethalliance.github.io/trusted-computing/spec.html#work-order-status-payload
Where are those status are captured?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These status assume a communication/report from the worker. This is part of the direct model but not in the proxy model. Smart contracts only know about the current state of "consensus" and the next step it expects. Smart contracts cannot say anything about any ongoing work.

{
NULL,
ACTIVE,
COMPLETED,
FAILED
}

struct WorkOrder
{
uint256 status;
uint256 timestamp;
bytes32 workerID;
bytes32 requesterID;
bytes request;
address verifier;
uint256 returnCode;
bytes response;
}

// workOrderID → workOrder
mapping(bytes32 => WorkOrder) m_workorders;

event workOrderSubmitted(
bytes32 indexed workOrderID,
bytes32 indexed workerID,
bytes32 indexed requesterID);

event workOrderCompleted(
bytes32 indexed workOrderID,
uint256 indexed workOrderReturnCode);

event workOrderTimedout(
bytes32 indexed workOrderID);

Copy link
Contributor

Choose a reason for hiding this comment

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

EEA spec talk about 4 events for work order. 1. New Work Order Event 2. Work Order Done Event 3. Encryption Key Set Event 4. Encryption Key Start Event. Please use same name as in spec eg. newWorkOrderEvent, workOrderDoneEvnet, etc
Are you planning to add encryption ket set and encryption key start event?

constructor()
public
{}

function workOrderSubmit(
bytes32 _workerID,
bytes32 _requesterID,
bytes memory _workOrderRequest,
address _verifier,
bytes32 _salt)
Copy link
Contributor

Choose a reason for hiding this comment

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

_verifier and _salt is not mentioned in EEA spec in section 6.2.1 Submitting a New Work Order. Could you add function comment with the details of parameters

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have a PR on the specs that explains that

Copy link
Contributor

@manojgop manojgop Sep 19, 2019

Choose a reason for hiding this comment

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

Could you share the link of the PR for reference

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't access this link. Seems it's a private repo

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, adding "salt" here is for the uniqueness

Copy link
Contributor

@reckeyzhang reckeyzhang Sep 19, 2019

Choose a reason for hiding this comment

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

you need to request eea admin to get the access, I think Eugene can help you on that

Copy link
Contributor

@manojgop manojgop Sep 19, 2019

Choose a reason for hiding this comment

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

Sure. I'll check with @EugeneYYY . Existing spec has workOrderId as input parameter in workOrderSubmit function. So was workOrderId used for uniqueness instead of salt ?

function workOrderSubmit( bytes32 workOrderId, bytes32 workerId, bytes32 requesterId, string workOrderRequest) public returns uint32 errorCode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The PR removes workOrderId has an input parameter. It is not clearly defined how this should be computed beforehand and it causes issues: what if the value provided is already used ? How to prevent race conditions ?

The solution (which is the PR) is for the requester to provide the parameters (and a random salt). The contract will then generate the workOrderId itself, which is very unlikely to collide.

Without the salt a colision would happen if a requester asks the same computation twice (same parameters) ... if we add a random salt generated on the requester's side we remove this issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have a secret spec? Why cannot the spec be publicly accessible?

public returns (
uint256 errorCode
) {
bytes32 workOrderID = keccak256(abi.encode(
_workerID,
_requesterID,
_workOrderRequest,
_verifier,
_salt
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add comments about salt parameter description.

));

WorkOrder storage wo = m_workorders[workOrderID];
require(wo.status == uint256(WorkOrderStatus.NULL), "wo-already-submitted");
wo.status = uint256(WorkOrderStatus.ACTIVE);
wo.timestamp = now;
wo.workerID = _workerID;
wo.requesterID = _requesterID;
wo.request = _workOrderRequest;
wo.verifier = _verifier;

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we persist the new workOrder in storage using m_workorders[workOrderID] = wo;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is already what's happening. Look at line 80!
I'm using storage, not memory so all changes to wo are 'saved'

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

Copy link
Contributor

@manojgop manojgop Sep 19, 2019

Choose a reason for hiding this comment

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

workOrderId is calculated inside this function. Section 6.2.1 in EEA spec mentions workOrderId as an input parameter for workOrderSubmit function
function workOrderSubmit( bytes32 workOrderId, bytes32 workerId, bytes32 requesterId, string workOrderRequest) public returns uint32 errorCode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my reply above about: workerorder should not be an input but produced by the smartcontract (need to avoid colisions)

emit workOrderSubmitted(
workOrderID,
_workerID,
_requesterID);
Copy link
Contributor

@manojgop manojgop Sep 19, 2019

Choose a reason for hiding this comment

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

I assume there are changes made to spec. Publicly released spec mentions workOrderNew event is emitted by workOrderSubmit .
event workOrderNew (bytes32 indexed workOrderId, bytes32 indexed workerId, bytes32 indexed requesterId, string workOrderRequest, uint32 errorCode, address senderAddress, byte4 version)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, part of the spec change was about improving name consistency


return 0;
}

function workOrderComplete(
bytes32 _workOrderID,
uint256 _workOrderReturnCode,
bytes memory _workOrderResponse,
bytes memory _workOrderSignature
) public returns (
uint256 errorCode
) {
WorkOrder storage wo = m_workorders[_workOrderID];
require(wo.status == uint256(WorkOrderStatus.ACTIVE), "wo-already-submitted");

require(checkSignature(
wo.verifier,
toEthSignedMessageHash(
keccak256(abi.encodePacked(
_workOrderID,
_workOrderReturnCode,
_workOrderResponse
))
),
_workOrderSignature
));

wo.status = uint256(WorkOrderStatus.COMPLETED);
wo.returnCode = _workOrderReturnCode;
wo.response = _workOrderResponse;

emit workOrderCompleted(
_workOrderID,
_workOrderReturnCode);

return 0;
}

function workOrderTimeout(
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the description about this function. This function is not there in EEA spec.

bytes32 _workOrderID
) public returns (
uint256 errorCode
) {
WorkOrder storage wo = m_workorders[_workOrderID];
require(wo.status == uint256(WorkOrderStatus.ACTIVE));
require(wo.timestamp + TIMEOUT < now);

wo.status = uint256(WorkOrderStatus.FAILED);

emit workOrderTimedout(_workOrderID);

return 0;
}

function workOrderGet(
bytes32 _workOrderID
) public view returns (
uint256 status,
uint256 timestamp,
bytes32 workerID,
bytes32 requesterID,
bytes memory request,
address verifier,
uint256 returnCode,
bytes memory response
) {
WorkOrder storage wo = m_workorders[_workOrderID];
return (
wo.status,
wo.timestamp,
Copy link
Contributor

@Ram-srini Ram-srini Sep 23, 2019

Choose a reason for hiding this comment

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

Please add description about timestamp and verifier parameters in the function comments.

wo.workerID,
wo.requesterID,
wo.request,
wo.verifier,
wo.returnCode,
wo.response);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/* Copyright 2019 iExec Blockchain Tech
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

pragma solidity ^0.5.0;

contract WorkerRegistry
{
enum WorkerStatus
{
NULL,
ACTIVE,
OFFLINE,
DECOMMISSIONED,
COMPROMISED
}

struct Worker
{
uint256 status;
uint256 workerType;
bytes32 organizationId;
bytes32[] appTypeIds;
string details;
}

// WorkerID → Worker
mapping(bytes32 => Worker) private m_workers;

// workerType → organizationId → appTypeId → WorkerID[]
Copy link
Contributor

@danintel danintel Sep 23, 2019

Choose a reason for hiding this comment

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

s/appTypeId/appTypeIds/

(I see no appTypeId here, only appTypeIds)

mapping(uint256 => mapping(bytes32 => mapping(bytes32 => bytes32[]))) m_workersDB;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this complex map structure? Can't we use one in simple map as it line 39?
Does it for lookup optimization?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what key-value do you propose and how would you maintain it?


constructor()
public
{}

function workerRegister(
bytes32 workerID,
uint256 workerType,
bytes32 organizationId,
bytes32[] memory appTypeIds,
string memory details
) public {
Worker storage worker = m_workers[workerID];

require(worker.status == uint256(WorkerStatus.NULL));
worker.status = uint256(WorkerStatus.ACTIVE);
worker.workerType = workerType;
worker.organizationId = organizationId;
worker.appTypeIds = appTypeIds;
worker.details = details;

require(workerType != uint256(0));
require(organizationId != bytes32(0));
m_workersDB[uint256(0)][bytes32(0) ][bytes32(0)].push(workerID);
m_workersDB[workerType][bytes32(0) ][bytes32(0)].push(workerID);
m_workersDB[uint256(0)][organizationId][bytes32(0)].push(workerID);
m_workersDB[workerType][organizationId][bytes32(0)].push(workerID);

for (uint256 p = 0; p < appTypeIds.length; ++p)
{
require(appTypeIds[p] != bytes32(0));
m_workersDB[uint256(0)][bytes32(0) ][appTypeIds[p]].push(workerID);
m_workersDB[workerType][bytes32(0) ][appTypeIds[p]].push(workerID);
m_workersDB[uint256(0)][organizationId][appTypeIds[p]].push(workerID);
m_workersDB[workerType][organizationId][appTypeIds[p]].push(workerID);
}
}

function workerUpdate(
bytes32 workerID,
string memory details
) public {
require(m_workers[workerID].status != uint256(WorkerStatus.NULL));
m_workers[workerID].details = details;
}

function workerSetStatus(
bytes32 workerID,
uint256 status
) public {
require(m_workers[workerID].status != uint256(WorkerStatus.NULL));
require(status != uint256(WorkerStatus.NULL));
m_workers[workerID].status = status;
}

function workerLookUp(
uint256 workerType,
bytes32 organizationId,
bytes32 appTypeId
) public view returns(
uint256 totalCount,
uint256 lookupTag,
bytes32[] memory ids
) {
return workerLookUpNext(workerType, organizationId, appTypeId, 0);
}

function workerLookUpNext(
uint256 workerType,
bytes32 organizationId,
bytes32 appTypeId,
uint256 /*lookUpTag*/
) public view returns(
uint256 totalCount,
uint256 newLookupTag,
bytes32[] memory ids
) {
bytes32[] storage matchs = m_workersDB[workerType][organizationId][appTypeId];
Copy link
Contributor

Choose a reason for hiding this comment

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

The Worker must match all input parameters (AND mode) to be included in the list and All input parameters are optional and can be provided in any combination to select Workers. Does it covers all the conditions of lookup function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know what to tell you but to read the code. Also, optional input parameters are not a thing in solidity, so you need to be creative about it (cf line 64 to 78).

return (matchs.length, 0, matchs);
}

function workerRetrieve(
bytes32 workerID
) public view returns (
uint256 status,
uint256 workerType,
bytes32 organizationId,
bytes32[] memory appTypeIds,
string memory details
) {
Worker storage worker = m_workers[workerID];
return (
worker.status,
worker.workerType,
worker.organizationId,
worker.appTypeIds,
worker.details
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/* Copyright 2019 iExec Blockchain Tech
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

pragma solidity ^0.5.0;

contract IERC1271
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the description of IERC1271 in the comments.

{
// bytes4(keccak256("isValidSignature(bytes,bytes)")
bytes4 constant internal MAGICVALUE = 0x20c13b0b;

/**
* @dev Should return whether the signature provided is valid for the provided data
* @param _data Arbitrary length data signed on the behalf of address(this)
* @param _signature Signature byte array associated with _data
*
* MUST return the bytes4 magic value 0x20c13b0b when function passes.
* MUST NOT modify state (using STATICCALL for solc < 0.5, view modifier for solc > 0.5)
* MUST allow external calls
*/
// function isValidSignature(
// bytes memory _data,
// bytes memory _signature)
// public
// view
// returns (bytes4 magicValue);

// Newer version ? From 0x V2
function isValidSignature(
bytes32 _data,
bytes memory _signature
)
public
view
returns (bool isValid);
}
Loading