From b2f2f25ecbcff63f9a995197660870dcca3510a2 Mon Sep 17 00:00:00 2001 From: Deepesh Kumar Nath Date: Sun, 21 Oct 2018 19:42:40 +0530 Subject: [PATCH 1/5] Added VoteMessage and Vote struct, VoteMessage is hashed according to EIP-712 --- contracts/core/PollingPlace.sol | 191 ++++++++++++++++++++++---------- test/core/PollingPlace.js | 6 + 2 files changed, 139 insertions(+), 58 deletions(-) diff --git a/contracts/core/PollingPlace.sol b/contracts/core/PollingPlace.sol index eca2a3c2..bc3d7409 100644 --- a/contracts/core/PollingPlace.sol +++ b/contracts/core/PollingPlace.sol @@ -70,6 +70,54 @@ contract PollingPlace is PollingPlaceInterface { uint256 endHeight; } + /** Vote message */ + struct VoteMessage { + + /** A unique identifier that identifies what chain this vote is about. */ + bytes20 coreIdentifier; + + /** + * The hash of the transition object of the meta-block that would + * result from the source block being finalised and proposed to origin. + */ + bytes32 transitionHash; + + /** The hash of the source block. */ + bytes32 source; + + /** The hash of the target block. */ + bytes32 target; + + /** The height of the source block. */ + uint256 sourceHeight; + + /** The height of the target block. */ + uint256 targetHeight; + } + + /** Vote object */ + struct Vote { + /** Vote message. */ + VoteMessage voteMessage; + + /** Vote hash. */ + bytes32 voteHash; + + /** v component of signature */ + uint8 v; + + /** r component of signature */ + bytes32 r; + + /** s component of signature */ + bytes32 s; + } + + /** To hash vote message according to EIP-712, a type hash is required. */ + bytes32 constant VOTE_MESSAGE_TYPEHASH = keccak256( + "VoteMessage(bytes20 coreIdentifier,bytes32 transitionHash,bytes32 source,bytes32 target,uint256 sourceHeight,uint256 targetHeight)" + ); + /* Public Variables */ /** @@ -300,7 +348,7 @@ contract PollingPlace is PollingPlaceInterface { BlockStoreInterface blockStore = blockStores[_coreIdentifier]; require( address(blockStore) != address(0), - "The providede core identifier must be known to the PollingPlace." + "The provided core identifier must be known to the PollingPlace." ); require( @@ -308,20 +356,19 @@ contract PollingPlace is PollingPlaceInterface { "The provided vote is not valid according to the block store." ); - bytes32 voteHash = hashVote( + Vote memory voteObject = getVoteObject( _coreIdentifier, _transitionHash, _source, _target, _sourceHeight, - _targetHeight - ); - Validator storage validator = getValidatorFromVote( - voteHash, + _targetHeight, _v, _r, - _s - ); + _s); + + Validator storage validator = getValidatorFromVote(voteObject); + require(validator.auxiliaryAddress != address(0), "Vote by unknown validator."); require( @@ -330,10 +377,7 @@ contract PollingPlace is PollingPlaceInterface { ); storeVote( - voteHash, - _source, - _target, - _targetHeight, + voteObject, validator, blockStore ); @@ -428,26 +472,19 @@ contract PollingPlace is PollingPlaceInterface { * @dev All requirement checks must have been made before calling this * method. * - * @param _voteHash The hash of the vote object. The hash is used to - * identify the vote. - * @param _source The hash of the source checkpoint of the vote. - * @param _target The hash of the target checkpoint of the vote. - * @param _targetHeight The height of the target block of the vote. - * @param _validator The validator that signed the vote. - * @param _blockStore The block store that this vote is about. + * @param _voteObject Vote object. */ function storeVote( - bytes32 _voteHash, - bytes32 _source, - bytes32 _target, - uint256 _targetHeight, + Vote memory _voteObject, Validator storage _validator, BlockStoreInterface _blockStore ) private { - validatorTargetHeights[_validator.auxiliaryAddress] = _targetHeight; - votesWeights[_voteHash] += validatorWeight(_validator, currentMetaBlockHeight); + VoteMessage memory voteMessage = _voteObject.voteMessage; + + validatorTargetHeights[_validator.auxiliaryAddress] = voteMessage.targetHeight; + votesWeights[_voteObject.voteHash] += validatorWeight(_validator, currentMetaBlockHeight); /* * Because the target must be within the currently open meta-block, the @@ -455,8 +492,8 @@ contract PollingPlace is PollingPlaceInterface { */ uint256 required = requiredWeight(currentMetaBlockHeight); - if (votesWeights[_voteHash] >= required) { - _blockStore.justify(_source, _target); + if (votesWeights[_voteObject.voteHash] >= required) { + _blockStore.justify(voteMessage.source, voteMessage.target); } } @@ -482,25 +519,22 @@ contract PollingPlace is PollingPlaceInterface { * @notice Uses the signature of a vote to recover the public address of * the signer. * - * @param _voteHash The hashed vote. It is the message that was signed. - * @param _v V of the signature. - * @param _r R of teh signature. - * @param _s S of the signature. + * @param _voteObject Vote object. * * @return The `Validator` that signed the given message with the given * signature. */ - function getValidatorFromVote( - bytes32 _voteHash, - uint8 _v, - bytes32 _r, - bytes32 _s - ) + function getValidatorFromVote(Vote memory _voteObject) private view returns (Validator storage validator_) { - address signer = ecrecover(_voteHash, _v, _r, _s); + address signer = ecrecover( + _voteObject.voteHash, + _voteObject.v, + _voteObject.r, + _voteObject.s + ); validator_ = validators[signer]; } @@ -564,35 +598,24 @@ contract PollingPlace is PollingPlaceInterface { * @notice Creates the hash of o vote object. This is the same hash that * the validator has signed. * - * @param _coreIdentifier Core identifier of the vote object. - * @param _transitionHash Transition hash of the vote object. - * @param _source Source block hash of the vote object. - * @param _target Target block hash of the vote object. - * @param _sourceHeight Source height of the vote object. - * @param _targetHeight Target height of the vote object. + * @param _voteMessage Vote message object. * * @return The hash of the given vote. */ - function hashVote( - bytes20 _coreIdentifier, - bytes32 _transitionHash, - bytes32 _source, - bytes32 _target, - uint256 _sourceHeight, - uint256 _targetHeight - ) + function hashVote(VoteMessage memory _voteMessage) private pure returns (bytes32 hashed_) { hashed_ = keccak256( abi.encodePacked( - _coreIdentifier, - _transitionHash, - _source, - _target, - _sourceHeight, - _targetHeight + VOTE_MESSAGE_TYPEHASH, + _voteMessage.coreIdentifier, + _voteMessage.transitionHash, + _voteMessage.source, + _voteMessage.target, + _voteMessage.sourceHeight, + _voteMessage.targetHeight ) ); @@ -605,4 +628,56 @@ contract PollingPlace is PollingPlaceInterface { ) ); } + + /** + * @notice Creates a vote object + * + * @param _coreIdentifier A unique identifier that identifies what chain + * this vote is about. + * @param _transitionHash The hash of the transition object of the + * meta-block that would result from the source + * block being finalised and proposed to origin. + * @param _source The hash of the source block. + * @param _target The hash of the target block. + * @param _sourceHeight The height of the source block. + * @param _targetHeight The height of the target block. + * @param _v V of the signature. + * @param _r R of the signature. + * @param _s S of the signature. + * + * @return vote object + */ + function getVoteObject( + bytes20 _coreIdentifier, + bytes32 _transitionHash, + bytes32 _source, + bytes32 _target, + uint256 _sourceHeight, + uint256 _targetHeight, + uint8 _v, + bytes32 _r, + bytes32 _s + ) + private + returns (Vote memory voteObject_) + { + VoteMessage memory voteMessage = VoteMessage( + _coreIdentifier, + _transitionHash, + _source, + _target, + _sourceHeight, + _targetHeight + ); + + bytes32 voteHash = hashVote(voteMessage); + + voteObject_ = Vote( + voteMessage, + voteHash, + _v, + _r, + _s + ); + } } diff --git a/test/core/PollingPlace.js b/test/core/PollingPlace.js index b26f4471..0753939a 100644 --- a/test/core/PollingPlace.js +++ b/test/core/PollingPlace.js @@ -34,6 +34,10 @@ const ValidatorIndexEnded = 2; const ValidatorIndexStartHeight = 3; const ValidatorIndexEndHeight = 4; +const VOTE_MESSAGE_TYPEHASH = web3.utils.sha3( + "VoteMessage(bytes20 coreIdentifier,bytes32 transitionHash,bytes32 source,bytes32 target,uint256 sourceHeight,uint256 targetHeight)" +); + contract('PollingPlace', async (accounts) => { /* * Make the first address the default meta-block gate to be able to @@ -1071,7 +1075,9 @@ contract('PollingPlace', async (accounts) => { * @returns {object} The signature of the vote (r, s, and v). */ async function signVote(address, vote) { + let voteDigest = web3.utils.soliditySha3( + {type: 'bytes32', value: VOTE_MESSAGE_TYPEHASH}, {type: 'bytes20', value: web3.utils.toChecksumAddress(vote.coreIdentifier)}, {type: 'bytes32', value: vote.transitionHash}, {type: 'bytes32', value: vote.source}, From 4bab4816c464e8d77f945dd015541b2caa354f75 Mon Sep 17 00:00:00 2001 From: Deepesh Kumar Nath Date: Mon, 22 Oct 2018 20:53:59 +0530 Subject: [PATCH 2/5] Removed voteHash from Vote struct --- contracts/core/PollingPlace.sol | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/contracts/core/PollingPlace.sol b/contracts/core/PollingPlace.sol index bc3d7409..1bb1df1b 100644 --- a/contracts/core/PollingPlace.sol +++ b/contracts/core/PollingPlace.sol @@ -97,12 +97,10 @@ contract PollingPlace is PollingPlaceInterface { /** Vote object */ struct Vote { + /** Vote message. */ VoteMessage voteMessage; - /** Vote hash. */ - bytes32 voteHash; - /** v component of signature */ uint8 v; @@ -482,9 +480,10 @@ contract PollingPlace is PollingPlaceInterface { private { VoteMessage memory voteMessage = _voteObject.voteMessage; + bytes32 voteHash = hashVote(_voteObject.voteMessage); validatorTargetHeights[_validator.auxiliaryAddress] = voteMessage.targetHeight; - votesWeights[_voteObject.voteHash] += validatorWeight(_validator, currentMetaBlockHeight); + votesWeights[voteHash] += validatorWeight(_validator, currentMetaBlockHeight); /* * Because the target must be within the currently open meta-block, the @@ -492,7 +491,7 @@ contract PollingPlace is PollingPlaceInterface { */ uint256 required = requiredWeight(currentMetaBlockHeight); - if (votesWeights[_voteObject.voteHash] >= required) { + if (votesWeights[voteHash] >= required) { _blockStore.justify(voteMessage.source, voteMessage.target); } } @@ -529,8 +528,9 @@ contract PollingPlace is PollingPlaceInterface { view returns (Validator storage validator_) { + bytes32 voteHash = hashVote(_voteObject.voteMessage); address signer = ecrecover( - _voteObject.voteHash, + voteHash, _voteObject.v, _voteObject.r, _voteObject.s @@ -670,11 +670,8 @@ contract PollingPlace is PollingPlaceInterface { _targetHeight ); - bytes32 voteHash = hashVote(voteMessage); - voteObject_ = Vote( voteMessage, - voteHash, _v, _r, _s From 1b8e15d959ae08798fea22cc0b209a424e68b15c Mon Sep 17 00:00:00 2001 From: Deepesh Kumar Nath Date: Tue, 23 Oct 2018 11:05:08 +0530 Subject: [PATCH 3/5] Passing voteHash as param --- contracts/core/PollingPlace.sol | 35 +++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/contracts/core/PollingPlace.sol b/contracts/core/PollingPlace.sol index 1bb1df1b..a7d1d483 100644 --- a/contracts/core/PollingPlace.sol +++ b/contracts/core/PollingPlace.sol @@ -365,7 +365,12 @@ contract PollingPlace is PollingPlaceInterface { _r, _s); - Validator storage validator = getValidatorFromVote(voteObject); + bytes32 voteHash = hashVote(voteObject.voteMessage); + + Validator storage validator = getValidatorFromVote( + voteObject, + voteHash + ); require(validator.auxiliaryAddress != address(0), "Vote by unknown validator."); @@ -376,6 +381,7 @@ contract PollingPlace is PollingPlaceInterface { storeVote( voteObject, + voteHash, validator, blockStore ); @@ -468,22 +474,29 @@ contract PollingPlace is PollingPlaceInterface { * store. * * @dev All requirement checks must have been made before calling this - * method. + * method. As this function is private, we can trust that vote hash + * will be correct. In case if this function changes to external or + * public then we should calculate the vote hash from the vote object in this + * function. + * * * @param _voteObject Vote object. + * @param _voteHash Hash of the VoteMessage. + * @param _validator The validator that signed the vote. + * @param _blockStore The block store that this vote is about. */ function storeVote( Vote memory _voteObject, + bytes32 _voteHash, Validator storage _validator, BlockStoreInterface _blockStore ) private { VoteMessage memory voteMessage = _voteObject.voteMessage; - bytes32 voteHash = hashVote(_voteObject.voteMessage); validatorTargetHeights[_validator.auxiliaryAddress] = voteMessage.targetHeight; - votesWeights[voteHash] += validatorWeight(_validator, currentMetaBlockHeight); + votesWeights[_voteHash] += validatorWeight(_validator, currentMetaBlockHeight); /* * Because the target must be within the currently open meta-block, the @@ -491,7 +504,7 @@ contract PollingPlace is PollingPlaceInterface { */ uint256 required = requiredWeight(currentMetaBlockHeight); - if (votesWeights[voteHash] >= required) { + if (votesWeights[_voteHash] >= required) { _blockStore.justify(voteMessage.source, voteMessage.target); } } @@ -518,19 +531,24 @@ contract PollingPlace is PollingPlaceInterface { * @notice Uses the signature of a vote to recover the public address of * the signer. * + * @dev As this function is private, we can trust that vote hash will be + * correct. In case if this function changes to external or public + * then we should calculate the vote hash from the vote object in this + * function. + * * @param _voteObject Vote object. + * @param _voteHash Hash of vote message. * * @return The `Validator` that signed the given message with the given * signature. */ - function getValidatorFromVote(Vote memory _voteObject) + function getValidatorFromVote(Vote memory _voteObject, bytes32 _voteHash) private view returns (Validator storage validator_) { - bytes32 voteHash = hashVote(_voteObject.voteMessage); address signer = ecrecover( - voteHash, + _voteHash, _voteObject.v, _voteObject.r, _voteObject.s @@ -659,6 +677,7 @@ contract PollingPlace is PollingPlaceInterface { bytes32 _s ) private + pure returns (Vote memory voteObject_) { VoteMessage memory voteMessage = VoteMessage( From 825965876d5716ab60dc8e89f27d3f33046193ea Mon Sep 17 00:00:00 2001 From: Deepesh Kumar Nath Date: Tue, 23 Oct 2018 11:09:14 +0530 Subject: [PATCH 4/5] Review changes --- contracts/core/PollingPlace.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contracts/core/PollingPlace.sol b/contracts/core/PollingPlace.sol index a7d1d483..9deebf78 100644 --- a/contracts/core/PollingPlace.sol +++ b/contracts/core/PollingPlace.sol @@ -363,7 +363,8 @@ contract PollingPlace is PollingPlaceInterface { _targetHeight, _v, _r, - _s); + _s + ); bytes32 voteHash = hashVote(voteObject.voteMessage); From 7774af9ed72639b417d30f9e19e56142c373ff79 Mon Sep 17 00:00:00 2001 From: Deepesh Kumar Nath Date: Tue, 23 Oct 2018 17:54:56 +0530 Subject: [PATCH 5/5] Added comment for coreIdentifier --- contracts/core/PollingPlace.sol | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/contracts/core/PollingPlace.sol b/contracts/core/PollingPlace.sol index 9deebf78..ef1cbb31 100644 --- a/contracts/core/PollingPlace.sol +++ b/contracts/core/PollingPlace.sol @@ -73,7 +73,12 @@ contract PollingPlace is PollingPlaceInterface { /** Vote message */ struct VoteMessage { - /** A unique identifier that identifies what chain this vote is about. */ + /** + * A unique identifier that identifies what chain this vote is about. + * To generate the vote hash coreIdentifier is needed. As the votes are + * for both origin and auxiliary chain, the core identifier information + * is stored in this struct. + */ bytes20 coreIdentifier; /**