From 0f9c233657c5414922fff6d888c6b695512f4c42 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Fri, 16 Nov 2018 02:23:57 -0200 Subject: [PATCH 1/9] kill zombie code, fix conventions, fix approveAndCall, allow create fail --- .../contracts/identity/IdentityGasRelay.sol | 223 +++++++++--------- 1 file changed, 110 insertions(+), 113 deletions(-) diff --git a/test-dapp/contracts/identity/IdentityGasRelay.sol b/test-dapp/contracts/identity/IdentityGasRelay.sol index 99a80a6..5e7a7f2 100644 --- a/test-dapp/contracts/identity/IdentityGasRelay.sol +++ b/test-dapp/contracts/identity/IdentityGasRelay.sol @@ -11,63 +11,19 @@ import "../token/ERC20Token.sol"; */ contract IdentityGasRelay is Identity { - bytes4 public constant MSG_CALL_PREFIX = bytes4(keccak256("callGasRelay(address,uint256,bytes32,uint256,uint256,address)")); - bytes4 public constant MSG_DEPLOY_PREFIX = bytes4(keccak256("deployGasRelay(uint256,bytes32,uint256,uint256,address)")); - bytes4 public constant MSG_APPROVEANDCALL_PREFIX = bytes4(keccak256("approveAndCallGasRelay(address,address,uint256,bytes32,uint256,uint256)")); + bytes4 public constant MSG_CALL_PREFIX = bytes4( + keccak256("callGasRelay(address,uint256,bytes32,uint256,uint256,address)") + ); + bytes4 public constant MSG_DEPLOY_PREFIX = bytes4( + keccak256("deployGasRelay(uint256,bytes32,uint256,uint256,address)") + ); + bytes4 public constant MSG_APPROVEANDCALL_PREFIX = bytes4( + keccak256("approveAndCallGasRelay(address,address,uint256,bytes32,uint256,uint256)") + ); event ExecutedGasRelayed(bytes32 messageHash); event ContractDeployed(address deployedAddress); - /** - * @param _messageHash that is signed - * @param _requiredPurpose key purpose for this type of call - * @param _nonce current identity nonce - * @param _gasPrice price in SNT paid back to msg.sender for each gas unit used - * @param _gasLimit minimal gasLimit required to execute this call - * @param _gasToken token being used for paying `msg.sender` - * @param _messageSignatures rsv concatenated ethereum signed message signatures required - */ - modifier executeGasRelayed ( - bytes32 _messageHash, - uint256 _requiredPurpose, - uint _nonce, - uint _gasPrice, - uint _gasLimit, - address _gasToken, - bytes _messageSignatures - ) { - //query current gas available - uint startGas = gasleft(); - - //verify transaction parameters - require(startGas >= _gasLimit); - require(_nonce == nonce); - - //verify if signatures are valid and came from correct actors; - verifySignatures( - _requiredPurpose, - _messageHash, - _messageSignatures - ); - - //increase nonce - nonce++; - - //executes transaction - _; - - //refund gas used using contract held ERC20 tokens or ETH - if (_gasPrice > 0) { - uint256 _amount = 21000 + (startGas - gasleft()); - _amount = _amount * _gasPrice; - if (_gasToken == address(0)) { - address(msg.sender).transfer(_amount); - } else { - ERC20Token(_gasToken).transfer(msg.sender, _amount); - } - } - } - constructor( bytes32[] _keys, uint256[] _purposes, @@ -120,7 +76,7 @@ contract IdentityGasRelay is Identity { //verify transaction parameters //require(startGas >= _gasLimit); // TODO: Tune this - require(_nonce == nonce); + require(_nonce == nonce, "Wrong nonce"); //verify if signatures are valid and came from correct actors; verifySignatures( @@ -185,8 +141,8 @@ contract IdentityGasRelay is Identity { uint startGas = gasleft(); //verify transaction parameters - require(startGas >= _gasLimit); - require(_nonce == nonce); + require(startGas >= _gasLimit, "Bad gas limit"); + require(_nonce == nonce, "Bad nonce"); //verify if signatures are valid and came from correct actors; verifySignatures( @@ -226,14 +182,13 @@ contract IdentityGasRelay is Identity { * (`ERC20Token(baseToken).approve(_to, _value)` + `_to.call(_data)`). * in return of gas proportional amount multiplied by `_gasPrice` of `_gasToken` * fixes race condition in double transaction for ERC20. - * @param _baseToken token approved for `_to` + * @param _baseToken token approved for `_to` and token being used for paying `msg.sender` * @param _to destination of call * @param _value call value (in `_baseToken`) * @param _data call data * @param _nonce current identity nonce * @param _gasPrice price in SNT paid back to msg.sender for each gas unit used * @param _gasLimit minimal gasLimit required to execute this call - * @param _gasToken token being used for paying `msg.sender` * @param _messageSignatures rsv concatenated ethereum signed message signatures required */ function approveAndCallGasRelayed( @@ -244,7 +199,6 @@ contract IdentityGasRelay is Identity { uint _nonce, uint _gasPrice, uint _gasLimit, - address _gasToken, bytes _messageSignatures ) external @@ -255,19 +209,20 @@ contract IdentityGasRelay is Identity { uint startGas = gasleft(); //verify transaction parameters - // require(startGas >= _gasLimit); // TODO: tune this - require(_nonce == nonce); + // require(startGas >= _gasLimit, "Bad gas limit"); // TODO: tune this + require(_nonce == nonce, "Bad nonce"); //verify if signatures are valid and came from correct actors; verifySignatures( ACTION_KEY, - deployGasRelayHash( + approveAndCallHash( + _baseToken, + _to, _value, keccak256(_data), _nonce, _gasPrice, - _gasLimit, - _gasToken + _gasLimit ), _messageSignatures ); @@ -275,9 +230,9 @@ contract IdentityGasRelay is Identity { //increase nonce nonce++; - require(_baseToken != address(0)); //_baseToken should be something! - require(_to != address(this)); //no management with approveAndCall - require(_to != address(0)); //need valid destination + require(_baseToken != address(0), "Bad token address"); //_baseToken should be something! + require(_to != address(this), "Unauthorized call"); //no management with approveAndCall + require(_to != address(0), "Bad destination"); //need valid destination ERC20Token(_baseToken).approve(_to, _value); success = _commitCall(_nonce, _to, 0, _data); @@ -285,11 +240,7 @@ contract IdentityGasRelay is Identity { if (_gasPrice > 0) { uint256 _amount = 21000 + (startGas - gasleft()); _amount = _amount * _gasPrice; - if (_gasToken == address(0)) { - address(msg.sender).transfer(_amount); - } else { - ERC20Token(_gasToken).transfer(msg.sender, _amount); - } + ERC20Token(_baseToken).transfer(msg.sender, _amount); } } @@ -312,7 +263,7 @@ contract IdentityGasRelay is Identity { // calculates signHash bytes32 signHash = getSignHash(_messageHash); uint _amountSignatures = _messageSignatures.length / 65; - require(_amountSignatures == purposeThreshold[_requiredKey]); + require(_amountSignatures == purposeThreshold[_requiredKey], "Too few signatures"); bytes32 _lastKey = 0; for (uint256 i = 0; i < _amountSignatures; i++) { bytes32 _currentKey = recoverKey( @@ -320,8 +271,8 @@ contract IdentityGasRelay is Identity { _messageSignatures, i ); - require(_currentKey > _lastKey); //assert keys are different - require(keyHasPurpose(_currentKey, _requiredKey)); + require(_currentKey > _lastKey, "Bad signatures order"); //assert keys are different + require(keyHasPurpose(_currentKey, _requiredKey), "Bad key"); _lastKey = _currentKey; } return true; @@ -353,19 +304,21 @@ contract IdentityGasRelay is Identity { returns (bytes32 _callGasRelayHash) { _callGasRelayHash = keccak256( - address(this), - MSG_CALL_PREFIX, - _to, - _value, - _dataHash, - _nonce, - _gasPrice, - _gasLimit, - _gasToken + abi.encodePacked( + address(this), + MSG_CALL_PREFIX, + _to, + _value, + _dataHash, + _nonce, + _gasPrice, + _gasLimit, + _gasToken + ) ); } -/** + /** * @notice get callHash * @param _value call value (ether) * @param _dataHash call data hash @@ -388,14 +341,54 @@ contract IdentityGasRelay is Identity { returns (bytes32 _callGasRelayHash) { _callGasRelayHash = keccak256( - address(this), - MSG_DEPLOY_PREFIX, - _value, - _dataHash, - _nonce, - _gasPrice, - _gasLimit, - _gasToken + abi.encodePacked( + address(this), + MSG_DEPLOY_PREFIX, + _value, + _dataHash, + _nonce, + _gasPrice, + _gasLimit, + _gasToken + ) + ); + } + + /** + * @notice get callHash + * @param _value call value (ether) + * @param _dataHash call data hash + * @param _nonce current identity nonce + * @param _gasPrice price in SNT paid back to msg.sender for each gas unit used + * @param _gasLimit minimal gasLimit required to execute this call + * @param _gasToken token being used for paying `msg.sender` + * @return callGasRelayHash the hash to be signed by wallet + */ + function approveAndCallHash( + address _baseToken, + address _to, + uint256 _value, + bytes32 _dataHash, + uint _nonce, + uint256 _gasPrice, + uint256 _gasLimit + ) + public + view + returns (bytes32 _callGasRelayHash) + { + _callGasRelayHash = keccak256( + abi.encodePacked( + address(this), + MSG_APPROVEANDCALL_PREFIX, + _baseToken, + _to, + _value, + _dataHash, + _nonce, + _gasPrice, + _gasLimit + ) ); } @@ -426,16 +419,18 @@ contract IdentityGasRelay is Identity { returns (bytes32 _callGasRelayHash) { _callGasRelayHash = keccak256( - address(this), - MSG_APPROVEANDCALL_PREFIX, - _baseToken, - _to, - _value, - _dataHash, - _nonce, - _gasPrice, - _gasLimit, - _gasToken + abi.encodePacked( + address(this), + MSG_APPROVEANDCALL_PREFIX, + _baseToken, + _to, + _value, + _dataHash, + _nonce, + _gasPrice, + _gasLimit, + _gasToken + ) ); } @@ -445,13 +440,13 @@ contract IdentityGasRelay is Identity { * @param _messageSignature message `_signHash` signature * @param _pos which signature to read */ - function recoverKey ( + function recoverKey( bytes32 _signHash, bytes _messageSignature, uint256 _pos ) - pure public + pure returns(bytes32) { uint8 v; @@ -459,11 +454,13 @@ contract IdentityGasRelay is Identity { bytes32 s; (v,r,s) = signatureSplit(_messageSignature, _pos); return keccak256( - ecrecover( - _signHash, - v, - r, - s + abi.encodePacked( + ecrecover( + _signHash, + v, + r, + s + ) ) ); } @@ -474,8 +471,8 @@ contract IdentityGasRelay is Identity { * @param _signatures concatenated vrs signatures */ function signatureSplit(bytes _signatures, uint256 _pos) - pure internal + pure returns (uint8 v, bytes32 r, bytes32 s) { uint pos = _pos + 1; @@ -493,7 +490,7 @@ contract IdentityGasRelay is Identity { v := and(mload(add(_signatures, mul(65,pos))), 0xff) } - require(v == 27 || v == 28); + require(v == 27 || v == 28, "Bad signature"); } /** @@ -513,7 +510,7 @@ contract IdentityGasRelay is Identity { createdContract := create(_value, add(_code, 0x20), mload(_code)) failed := iszero(extcodesize(createdContract)) } - require(!failed); + //require(!failed); removed as startGas needs to be lower then inner gasLimit } } \ No newline at end of file From c1548bcbbd360556d6f5cab7599dedc4cf8bf355 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Fri, 16 Nov 2018 02:24:38 -0200 Subject: [PATCH 2/9] fix convention --- test-dapp/contracts/identity/IdentityFactory.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-dapp/contracts/identity/IdentityFactory.sol b/test-dapp/contracts/identity/IdentityFactory.sol index af46dce..84eda1a 100644 --- a/test-dapp/contracts/identity/IdentityFactory.sol +++ b/test-dapp/contracts/identity/IdentityFactory.sol @@ -23,7 +23,7 @@ contract IdentityFactory is Factory { bytes32[] memory initKeys = new bytes32[](2); uint256[] memory initPurposes = new uint256[](2); uint256[] memory initTypes = new uint256[](2); - initKeys[0] = keccak256(msg.sender); + initKeys[0] = keccak256(abi.encodePacked(msg.sender)); initKeys[1] = initKeys[0]; initPurposes[0] = 1; initPurposes[1] = 2; From 6378e81a00e63bb44a025051539977d0f1491f71 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Fri, 16 Nov 2018 02:44:44 -0200 Subject: [PATCH 3/9] enable startGas check, rename _gasLimit to _gasMinimal --- .../contracts/identity/IdentityGasRelay.sol | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/test-dapp/contracts/identity/IdentityGasRelay.sol b/test-dapp/contracts/identity/IdentityGasRelay.sol index 5e7a7f2..ac74e96 100644 --- a/test-dapp/contracts/identity/IdentityGasRelay.sol +++ b/test-dapp/contracts/identity/IdentityGasRelay.sol @@ -53,7 +53,7 @@ contract IdentityGasRelay is Identity { * @param _data call data * @param _nonce current identity nonce * @param _gasPrice price in SNT paid back to msg.sender for each gas unit used - * @param _gasLimit minimal gasLimit required to execute this call + * @param _gasMinimal minimal startGas required to execute this call * @param _gasToken token being used for paying `msg.sender` * @param _messageSignatures rsv concatenated ethereum signed message signatures required */ @@ -63,7 +63,7 @@ contract IdentityGasRelay is Identity { bytes _data, uint _nonce, uint _gasPrice, - uint _gasLimit, + uint _gasMinimal, address _gasToken, bytes _messageSignatures ) @@ -75,7 +75,7 @@ contract IdentityGasRelay is Identity { uint startGas = gasleft(); //verify transaction parameters - //require(startGas >= _gasLimit); // TODO: Tune this + require(startGas >= _gasMinimal, "Bad gas left"); require(_nonce == nonce, "Wrong nonce"); //verify if signatures are valid and came from correct actors; @@ -87,7 +87,7 @@ contract IdentityGasRelay is Identity { keccak256(_data), _nonce, _gasPrice, - _gasLimit, + _gasMinimal, _gasToken ), _messageSignatures @@ -120,7 +120,7 @@ contract IdentityGasRelay is Identity { * @param _data contract code data * @param _nonce current identity nonce * @param _gasPrice price in SNT paid back to msg.sender for each gas unit used - * @param _gasLimit minimal gasLimit required to execute this call + * @param _gasMinimal minimal startGas required to execute this call * @param _gasToken token being used for paying `msg.sender` * @param _messageSignatures rsv concatenated ethereum signed message signatures required */ @@ -129,7 +129,7 @@ contract IdentityGasRelay is Identity { bytes _data, uint _nonce, uint _gasPrice, - uint _gasLimit, + uint _gasMinimal, address _gasToken, bytes _messageSignatures ) @@ -141,7 +141,7 @@ contract IdentityGasRelay is Identity { uint startGas = gasleft(); //verify transaction parameters - require(startGas >= _gasLimit, "Bad gas limit"); + require(startGas >= _gasMinimal, "Bad gas left"); require(_nonce == nonce, "Bad nonce"); //verify if signatures are valid and came from correct actors; @@ -152,7 +152,7 @@ contract IdentityGasRelay is Identity { keccak256(_data), _nonce, _gasPrice, - _gasLimit, + _gasMinimal, _gasToken ), _messageSignatures @@ -188,7 +188,7 @@ contract IdentityGasRelay is Identity { * @param _data call data * @param _nonce current identity nonce * @param _gasPrice price in SNT paid back to msg.sender for each gas unit used - * @param _gasLimit minimal gasLimit required to execute this call + * @param _gasMinimal minimal startGas required to execute this call * @param _messageSignatures rsv concatenated ethereum signed message signatures required */ function approveAndCallGasRelayed( @@ -198,7 +198,7 @@ contract IdentityGasRelay is Identity { bytes _data, uint _nonce, uint _gasPrice, - uint _gasLimit, + uint _gasMinimal, bytes _messageSignatures ) external @@ -209,7 +209,7 @@ contract IdentityGasRelay is Identity { uint startGas = gasleft(); //verify transaction parameters - // require(startGas >= _gasLimit, "Bad gas limit"); // TODO: tune this + require(startGas >= _gasMinimal, "Bad gas left"); require(_nonce == nonce, "Bad nonce"); //verify if signatures are valid and came from correct actors; @@ -222,7 +222,7 @@ contract IdentityGasRelay is Identity { keccak256(_data), _nonce, _gasPrice, - _gasLimit + _gasMinimal ), _messageSignatures ); @@ -286,7 +286,7 @@ contract IdentityGasRelay is Identity { * @param _dataHash call data hash * @param _nonce current identity nonce * @param _gasPrice price in SNT paid back to msg.sender for each gas unit used - * @param _gasLimit minimal gasLimit required to execute this call + * @param _gasMinimal minimal startGas required to execute this call * @param _gasToken token being used for paying `msg.sender` * @return callGasRelayHash the hash to be signed by wallet */ @@ -296,7 +296,7 @@ contract IdentityGasRelay is Identity { bytes32 _dataHash, uint _nonce, uint256 _gasPrice, - uint256 _gasLimit, + uint256 _gasMinimal, address _gasToken ) public @@ -312,7 +312,7 @@ contract IdentityGasRelay is Identity { _dataHash, _nonce, _gasPrice, - _gasLimit, + _gasMinimal, _gasToken ) ); @@ -324,7 +324,7 @@ contract IdentityGasRelay is Identity { * @param _dataHash call data hash * @param _nonce current identity nonce * @param _gasPrice price in SNT paid back to msg.sender for each gas unit used - * @param _gasLimit minimal gasLimit required to execute this call + * @param _gasMinimal minimal startGas required to execute this call * @param _gasToken token being used for paying `msg.sender` * @return callGasRelayHash the hash to be signed by wallet */ @@ -333,7 +333,7 @@ contract IdentityGasRelay is Identity { bytes32 _dataHash, uint256 _nonce, uint256 _gasPrice, - uint256 _gasLimit, + uint256 _gasMinimal, address _gasToken ) public @@ -348,7 +348,7 @@ contract IdentityGasRelay is Identity { _dataHash, _nonce, _gasPrice, - _gasLimit, + _gasMinimal, _gasToken ) ); @@ -360,7 +360,7 @@ contract IdentityGasRelay is Identity { * @param _dataHash call data hash * @param _nonce current identity nonce * @param _gasPrice price in SNT paid back to msg.sender for each gas unit used - * @param _gasLimit minimal gasLimit required to execute this call + * @param _gasMinimal minimal startGas required to execute this call * @param _gasToken token being used for paying `msg.sender` * @return callGasRelayHash the hash to be signed by wallet */ @@ -371,7 +371,7 @@ contract IdentityGasRelay is Identity { bytes32 _dataHash, uint _nonce, uint256 _gasPrice, - uint256 _gasLimit + uint256 _gasMinimal ) public view @@ -387,7 +387,7 @@ contract IdentityGasRelay is Identity { _dataHash, _nonce, _gasPrice, - _gasLimit + _gasMinimal ) ); } @@ -400,7 +400,7 @@ contract IdentityGasRelay is Identity { * @param _dataHash call data hash * @param _nonce current identity nonce * @param _gasPrice price in SNT paid back to msg.sender for each gas unit used - * @param _gasLimit minimal gasLimit required to execute this call + * @param _gasMinimal minimal startGas required to execute this call * @param _gasToken token being used for paying `msg.sender` * @return callGasRelayHash the hash to be signed by wallet */ @@ -411,7 +411,7 @@ contract IdentityGasRelay is Identity { bytes32 _dataHash, uint _nonce, uint256 _gasPrice, - uint256 _gasLimit, + uint256 _gasMinimal, address _gasToken ) public @@ -428,7 +428,7 @@ contract IdentityGasRelay is Identity { _dataHash, _nonce, _gasPrice, - _gasLimit, + _gasMinimal, _gasToken ) ); @@ -510,7 +510,7 @@ contract IdentityGasRelay is Identity { createdContract := create(_value, add(_code, 0x20), mload(_code)) failed := iszero(extcodesize(createdContract)) } - //require(!failed); removed as startGas needs to be lower then inner gasLimit + //require(!failed); removed as startGas needs to be lower then inner _gasMinimal } } \ No newline at end of file From cb79d4f3e6598a354ad9d8866b0626c527ff2e2c Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Fri, 16 Nov 2018 02:54:21 -0200 Subject: [PATCH 4/9] fix convention --- test-dapp/contracts/status/SNTController.sol | 57 ++++++++++++-------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/test-dapp/contracts/status/SNTController.sol b/test-dapp/contracts/status/SNTController.sol index 9e2856f..070c5a5 100644 --- a/test-dapp/contracts/status/SNTController.sol +++ b/test-dapp/contracts/status/SNTController.sol @@ -14,8 +14,12 @@ import "../token/MiniMeToken.sol"; contract SNTController is TokenController, Owned, MessageSigned { - bytes4 public constant TRANSFER_PREFIX = bytes4(keccak256("transferSNT(address,uint256,uint256,uint256)")); - bytes4 public constant EXECUTE_PREFIX = bytes4(keccak256("executeGasRelayed(address,bytes,uint256,uint256,uint256)")); + bytes4 public constant TRANSFER_PREFIX = bytes4( + keccak256("transferSNT(address,uint256,uint256,uint256)") + ); + bytes4 public constant EXECUTE_PREFIX = bytes4( + keccak256("executeGasRelayed(address,bytes,uint256,uint256,uint256)") + ); MiniMeToken public snt; mapping (address => uint256) public signNonce; @@ -31,7 +35,7 @@ contract SNTController is TokenController, Owned, MessageSigned { * @param _owner Authority address * @param _snt SNT token */ - function SNTController(address _owner, address _snt) public { + constructor(address _owner, address _snt) public { owner = _owner; snt = MiniMeToken(_snt); } @@ -64,10 +68,17 @@ contract SNTController is TokenController, Owned, MessageSigned { ); address msgSigner = recoverAddress(msgSigned, _signature); - require(signNonce[msgSigner] == _nonce); + require(signNonce[msgSigner] == _nonce, "Bad nonce"); signNonce[msgSigner]++; if (snt.transferFrom(msgSigner, _to, _amount)) { - require(snt.transferFrom(msgSigner, msg.sender, (21000 + startGas-gasleft()) * _gasPrice)); + require( + snt.transferFrom( + msgSigner, + msg.sender, + (21000 + startGas - gasleft()) * _gasPrice + ), + "Gas transfer fail" + ); } } @@ -91,7 +102,7 @@ contract SNTController is TokenController, Owned, MessageSigned { external { uint256 startGas = gasleft(); - require(startGas >= _gasMinimal); + require(startGas >= _gasMinimal, "Bad gas left"); bytes32 msgSigned = getSignHash( getExecuteGasRelayedHash( _allowedContract, @@ -103,7 +114,7 @@ contract SNTController is TokenController, Owned, MessageSigned { ); address msgSigner = recoverAddress(msgSigned, _signature); - require(signNonce[msgSigner] == _nonce); + require(signNonce[msgSigner] == _nonce, "Bad nonce"); signNonce[msgSigner]++; bool success = _allowedContract.call(_data); emit GasRelayedExecution(msgSigner, msgSigned, success); @@ -146,7 +157,7 @@ contract SNTController is TokenController, Owned, MessageSigned { snt.claimTokens(_token); } if (_token == 0x0) { - address(owner).transfer(this.balance); + address(owner).transfer(address(this).balance); return; } @@ -194,13 +205,15 @@ contract SNTController is TokenController, Owned, MessageSigned { returns (bytes32 execHash) { execHash = keccak256( - address(this), - EXECUTE_PREFIX, - _allowedContract, - keccak256(_data), - _nonce, - _gasPrice, - _gasMinimal + abi.encodePacked( + address(this), + EXECUTE_PREFIX, + _allowedContract, + keccak256(_data), + _nonce, + _gasPrice, + _gasMinimal + ) ); } @@ -222,12 +235,14 @@ contract SNTController is TokenController, Owned, MessageSigned { returns (bytes32 txHash) { txHash = keccak256( - address(this), - TRANSFER_PREFIX, - _to, - _amount, - _nonce, - _gasPrice + abi.encodePacked( + address(this), + TRANSFER_PREFIX, + _to, + _amount, + _nonce, + _gasPrice + ) ); } From 5120e42cd1f11b4107186b4f20045a448e00d454 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Fri, 16 Nov 2018 02:58:18 -0200 Subject: [PATCH 5/9] fix convention --- test-dapp/contracts/status/SNTController.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test-dapp/contracts/status/SNTController.sol b/test-dapp/contracts/status/SNTController.sol index 070c5a5..7da290b 100644 --- a/test-dapp/contracts/status/SNTController.sol +++ b/test-dapp/contracts/status/SNTController.sol @@ -123,7 +123,8 @@ contract SNTController is TokenController, Owned, MessageSigned { msgSigner, msg.sender, (21000 + startGas-gasleft()) * _gasPrice - ) + ), + "Gas transfer fail" ); } From afbd33fb793fec3516037bf816e4ab148cf57f06 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Sun, 18 Nov 2018 04:00:32 -0200 Subject: [PATCH 6/9] remove zombie code --- .../contracts/identity/IdentityGasRelay.sol | 43 ------------------- 1 file changed, 43 deletions(-) diff --git a/test-dapp/contracts/identity/IdentityGasRelay.sol b/test-dapp/contracts/identity/IdentityGasRelay.sol index ac74e96..7975dea 100644 --- a/test-dapp/contracts/identity/IdentityGasRelay.sol +++ b/test-dapp/contracts/identity/IdentityGasRelay.sol @@ -393,47 +393,6 @@ contract IdentityGasRelay is Identity { } - /** - * @notice get callHash - * @param _to destination of call - * @param _value call value (ether) - * @param _dataHash call data hash - * @param _nonce current identity nonce - * @param _gasPrice price in SNT paid back to msg.sender for each gas unit used - * @param _gasMinimal minimal startGas required to execute this call - * @param _gasToken token being used for paying `msg.sender` - * @return callGasRelayHash the hash to be signed by wallet - */ - function approveAndCallGasRelayHash( - address _baseToken, - address _to, - uint256 _value, - bytes32 _dataHash, - uint _nonce, - uint256 _gasPrice, - uint256 _gasMinimal, - address _gasToken - ) - public - view - returns (bytes32 _callGasRelayHash) - { - _callGasRelayHash = keccak256( - abi.encodePacked( - address(this), - MSG_APPROVEANDCALL_PREFIX, - _baseToken, - _to, - _value, - _dataHash, - _nonce, - _gasPrice, - _gasMinimal, - _gasToken - ) - ); - } - /** * @notice recovers key who signed the message * @param _signHash operation ethereum signed message hash @@ -505,10 +464,8 @@ contract IdentityGasRelay is Identity { internal returns (address createdContract) { - bool failed; assembly { createdContract := create(_value, add(_code, 0x20), mload(_code)) - failed := iszero(extcodesize(createdContract)) } //require(!failed); removed as startGas needs to be lower then inner _gasMinimal } From 93a482d2f8425f6e88461af338d095ebb82e83e2 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Sun, 18 Nov 2018 04:07:32 -0200 Subject: [PATCH 7/9] fix convention of a public function name --- test-dapp/contracts/identity/IdentityGasRelay.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test-dapp/contracts/identity/IdentityGasRelay.sol b/test-dapp/contracts/identity/IdentityGasRelay.sol index 7975dea..b9a4413 100644 --- a/test-dapp/contracts/identity/IdentityGasRelay.sol +++ b/test-dapp/contracts/identity/IdentityGasRelay.sol @@ -215,7 +215,7 @@ contract IdentityGasRelay is Identity { //verify if signatures are valid and came from correct actors; verifySignatures( ACTION_KEY, - approveAndCallHash( + approveAndCallGasRelayHash( _baseToken, _to, _value, @@ -364,7 +364,7 @@ contract IdentityGasRelay is Identity { * @param _gasToken token being used for paying `msg.sender` * @return callGasRelayHash the hash to be signed by wallet */ - function approveAndCallHash( + function approveAndCallGasRelayHash( address _baseToken, address _to, uint256 _value, From 2b7837eef50f2686a8d40ede04c7abda7e799019 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Mon, 19 Nov 2018 16:44:27 -0200 Subject: [PATCH 8/9] add requirement check for allowed contract calls --- test-dapp/contracts/status/SNTController.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/test-dapp/contracts/status/SNTController.sol b/test-dapp/contracts/status/SNTController.sol index 7da290b..3ab06ba 100644 --- a/test-dapp/contracts/status/SNTController.sol +++ b/test-dapp/contracts/status/SNTController.sol @@ -103,6 +103,7 @@ contract SNTController is TokenController, Owned, MessageSigned { { uint256 startGas = gasleft(); require(startGas >= _gasMinimal, "Bad gas left"); + require(allowPublicExecution[_allowedContract], "Unallowed call"); bytes32 msgSigned = getSignHash( getExecuteGasRelayedHash( _allowedContract, From 6fda1fc72eb85e51eaf918b31828741688092147 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Mon, 19 Nov 2018 16:45:33 -0200 Subject: [PATCH 9/9] remove zombie event --- test-dapp/contracts/identity/IdentityGasRelay.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/test-dapp/contracts/identity/IdentityGasRelay.sol b/test-dapp/contracts/identity/IdentityGasRelay.sol index b9a4413..7e66837 100644 --- a/test-dapp/contracts/identity/IdentityGasRelay.sol +++ b/test-dapp/contracts/identity/IdentityGasRelay.sol @@ -21,7 +21,6 @@ contract IdentityGasRelay is Identity { keccak256("approveAndCallGasRelay(address,address,uint256,bytes32,uint256,uint256)") ); - event ExecutedGasRelayed(bytes32 messageHash); event ContractDeployed(address deployedAddress); constructor(