From e3ad32b34847a6b9c70c7de15a9174b77a91aa6f Mon Sep 17 00:00:00 2001 From: epiqueras Date: Wed, 28 Feb 2018 11:40:25 -0800 Subject: [PATCH] feat(proxy): use Arbitrator instead of KlerosPOC and factor out unneeded functionality --- .../proxy/ArbitratorVersioningProxy.sol | 118 +++++------------- .../standard/proxy/ExperimentalProxy.sol | 109 ++++++++++++++++ contracts/standard/proxy/Proxy.sol | 93 +------------- .../proxy/SimpleArbitratorVersioningProxy.sol | 72 ----------- contracts/standard/proxy/VersioningProxy.sol | 57 +++++---- 5 files changed, 169 insertions(+), 280 deletions(-) create mode 100644 contracts/standard/proxy/ExperimentalProxy.sol delete mode 100644 contracts/standard/proxy/SimpleArbitratorVersioningProxy.sol diff --git a/contracts/standard/proxy/ArbitratorVersioningProxy.sol b/contracts/standard/proxy/ArbitratorVersioningProxy.sol index a6c82af0..3d6d7588 100644 --- a/contracts/standard/proxy/ArbitratorVersioningProxy.sol +++ b/contracts/standard/proxy/ArbitratorVersioningProxy.sol @@ -1,125 +1,65 @@ pragma solidity ^0.4.15; -import "kleros/contracts/KlerosPOC.sol"; +import "../arbitration/Arbitrator.sol"; import "./VersioningProxy.sol"; /** * @title ArbitratorVersioningProxy * @author Enrique Piqueras - - * @notice A contract derived from VersioningProxy to manage the deployment of new versions of an Arbitrator contract. + * @notice A proxy that only exposes methods in the Arbitrator spec. */ contract ArbitratorVersioningProxy is VersioningProxy { - /* Enums */ - - - - /* Structs */ - - - - /* Events */ - - - /* Storage */ mapping (uint256 => address) public disputes; /* Modifiers */ - + modifier onlyIfDisputeExists(uint256 _disputeID) { + require(disputes[_disputeID] != address(0)); + _; + } /* Constructor */ /** * @notice Constructs the arbitrator versioning proxy with the first arbitrator contract version address and tags it v0.0.1. - * @param firstAddress The address of the first arbitrator contract version. + * @param _firstAddress The address of the first arbitrator contract version. */ - function ArbitratorVersioningProxy(address firstAddress) VersioningProxy(false, "0.0.1", firstAddress) public {} - - /* Fallback */ - - - - /* External */ - - - - /* External Views */ - - + function ArbitratorVersioningProxy(address _firstAddress) VersioningProxy("0.0.1", _firstAddress) public {} /* Public */ + function createDispute(uint256 _choices, bytes _extraData) public payable returns(uint256 _disputeID) { + _disputeID = Arbitrator(implementation).createDispute(_choices, _extraData); + disputes[_disputeID] = implementation; // Remember arbitrator + return _disputeID; + } + function appeal(uint256 _disputeID, bytes _extraData) public payable onlyIfDisputeExists(_disputeID) returns(uint256 _newDisputeID) { + if (disputes[_disputeID] != implementation) // Arbitrator has been upgraded, create a new dispute in the new arbitrator + return createDispute((Arbitrator(disputes[_disputeID]).disputes(_disputeID).choices), _extraData); + + Arbitrator(implementation).appeal(_disputeID, _extraData); + return _disputeID; + } /* Public Views */ - - - /* Internal */ - - - - /* Internal Views */ - - - - /* Private */ - - function bytesToBytes32(bytes b) private pure returns (bytes32) { - bytes32 out = 0; - - for (uint i = 31; i > 32; i--) { // Loop from lower order to higher order bytes - out |= bytes32(b[i]) << (i * 8); // Combine with out - } - - return out; + function arbitrationCost(bytes _extraData) public view returns(uint256 _fees) { + return Arbitrator(implementation).arbitrationCost(_extraData); } - /** - * @notice On-chain handler that gets called with call data and the 'implementation' contract's return data after a call is successfully proxied. - * @dev @overwrite Proxy. - * @param sig The function signature of the called function. - * @param data The data passed into the call. - * @param retData The return data of the 'implementation' contract for the proxied call. - */ - function handleProxySuccess(bytes4 sig, bytes data, bytes retData) private { - if (sig == bytes4(keccak256("createDispute(uint256,bytes)"))) { // `createDispute` succeeded - uint256 disputeID = uint256(bytesToBytes32(retData)); // We know this is a uint256 - - disputes[disputeID] = implementation; // Remember which arbitrator this dispute belongs to - } + function appealCost(uint256 _disputeID, bytes _extraData) public view returns(uint256 _fees) { + return Arbitrator(implementation).appealCost(_disputeID, _extraData); } - /* Private Views */ - - /** - * @notice Function for dynamically getting the 'implementation' contract address. - * @dev @overwrite Proxy. - * @param sig The function signature of the called function. - * @param data The data passed into the call. - * @return The resolved 'implementation' contract address. - */ - function getImplementation(bytes4 sig, bytes data) private view returns (address) { - if (sig == bytes4(keccak256("appeal(uint256,bytes)"))) { // `appeal` called - uint256 disputeID = uint256(bytesToBytes32(data)); // We know the first param is a uint256 - address arbitrator = disputes[disputeID]; // The arbitrator this dispute belongs to - - // We have changed arbitrators, create a new dispute - if (arbitrator != implementation) { - KlerosPOC oldArbitrator = KlerosPOC(arbitrator); - KlerosPOC newArbitrator = KlerosPOC(implementation); - - uint256 choices = oldArbitrator.disputes(disputeID).choices; - newArbitrator.createDispute(choices, bytes(0)); // TODO: Extra Data? - } - } - - // TODO: We might need to add disputeID as the first parameter of all calls to be able to resolve the right arbitrator - - return implementation; + function currentRuling(uint256 _disputeID) public view onlyIfDisputeExists(_disputeID) returns(uint256 _ruling) { + return Arbitrator(disputes[_disputeID]).currentRuling(_disputeID); } + function disputeStatus(uint256 _disputeID) public view onlyIfDisputeExists(_disputeID) returns(Arbitrator.DisputeStatus _status) { + return Arbitrator(disputes[_disputeID]).disputeStatus(_disputeID); + } } diff --git a/contracts/standard/proxy/ExperimentalProxy.sol b/contracts/standard/proxy/ExperimentalProxy.sol new file mode 100644 index 00000000..27ae0cdc --- /dev/null +++ b/contracts/standard/proxy/ExperimentalProxy.sol @@ -0,0 +1,109 @@ +pragma solidity ^0.4.15; + +/** + * @title ExperimentalProxy + * @author Enrique Piqueras - + * @notice An experimental base proxy contract that forwards all calls to the 'implementation' contract and optionally keeps all storage. + */ + contract ExperimentalProxy { + /* Storage */ + + bool public storageIsEternal; + address public implementation; + + /* Constructor */ + + /** + * @notice Constructs the proxy with the eternal storage flag and an initial 'implementation' contract address. + * @param _storageIsEternal Wether this contract should store all storage. I.e. Use 'delegatecall'. + * @param _implementation The initial 'implementation' contract address. + */ + function ExperimentalProxy(bool _storageIsEternal, address _implementation) public { + storageIsEternal = _storageIsEternal; + implementation = _implementation; + } + + /* Fallback */ + + /** + * @notice The fallback function that forwards calls to the 'implementation' contract. + * @return The result of calling the requested function on the 'implementation' contract. + */ + function () payable external { + require(implementation != address(0)); // Make sure address is valid + + // Store necessary data for assembly in local memory + bool _storageIsEternal = storageIsEternal; + bytes memory _data = msg.data; + address _implementation = getImplementation(msg.sig, _data); + + // Return data + bytes memory _retData; + + assembly { + // Start of payload raw data (skip over size slot) + let _dataPtr := add(_data, 0x20) + + // Payload's size + let _dataSize := mload(_data) + + // Figure out what OPCODE to use and forward call + let _result + switch _storageIsEternal + case 0 { // Not eternal, use implementation's storage + _result := call(gas, _implementation, callvalue, _dataPtr, _dataSize, 0, 0) + } + default { // Eternal, use current contract's storage + _result := delegatecall(gas, _implementation, _dataPtr, _dataSize, 0, 0) + } + + // Size of the returned data + let _retSize := returndatasize + + let _retPtr := mload(0x40) // Start of free memory + let _retDataPtr := add(_retPtr, 0x20) // Make space for 'bytes' size + + // Build `_retData` 'bytes' + mstore(_retPtr, _retSize) // Copy size + returndatacopy(_retDataPtr, 0, _retSize) // Copy returned data + + // Figure out wether to revert or continue with the returned data + switch _result + case 0 { // Error + revert(_retDataPtr, _retSize) + } + default { // Success + _retData := _retPtr + } + } + + // Call on-chain handler + handleProxySuccess(msg.sig, _data, _retData); + + assembly { + return(add(_retData, 0x20), mload(_retData)) // Return returned data + } + } + + /* Private */ + + /** + * @notice On-chain handler that gets called with call data and the 'implementation' contract's return data after a call is successfully proxied. + * @dev Overwrite this function to handle the results of proxied calls in this contract. + * @param _sig The function signature of the called function. + * @param _data The data passed into the call. + * @param _retData The return data of the 'implementation' contract for the proxied call. + */ + function handleProxySuccess(bytes4 _sig, bytes _data, bytes _retData) private {} + + /* Private Views */ + + /** + * @notice Function for dynamically getting the 'implementation' contract address. + * @dev Overwrite this function to implement custom resolving logic based on the function being called and the data passed in. + * @param _sig The function signature of the called function. + * @param _data The data passed into the call. + * @return The resolved 'implementation' contract address. + */ + function getImplementation(bytes4 _sig, bytes _data) private view returns(address _implementation) { return implementation; } +} diff --git a/contracts/standard/proxy/Proxy.sol b/contracts/standard/proxy/Proxy.sol index f3d9c13f..54b19508 100644 --- a/contracts/standard/proxy/Proxy.sol +++ b/contracts/standard/proxy/Proxy.sol @@ -3,107 +3,20 @@ pragma solidity ^0.4.15; /** * @title Proxy * @author Enrique Piqueras - - * @notice A base proxy contract that forwards all calls to the 'implementation' contract and optionally keeps all storage. + * @notice A base proxy contract. */ contract Proxy { /* Storage */ - bool public storageIsEternal; address public implementation; /* Constructor */ /** - * @notice Constructs the proxy with the eternal storage flag and an initial 'implementation' contract address. - * @param _storageIsEternal Wether this contract should store all storage. I.e. Use 'delegatecall'. + * @notice Constructs the proxy with the initial 'implementation' contract address. * @param _implementation The initial 'implementation' contract address. */ - function Proxy(bool _storageIsEternal, address _implementation) public { - storageIsEternal = _storageIsEternal; + function Proxy(address _implementation) public { implementation = _implementation; } - - /* Fallback */ - - /** - * @notice The fallback function that forwards calls to the 'implementation' contract. - * @return The result of calling the requested function on the 'implementation' contract. - */ - function () payable external { - require(implementation != address(0)); // Make sure address is valid - - // Store necessary data for assembly in local memory - bool _storageIsEternal = storageIsEternal; - bytes memory data = msg.data; - address _implementation = getImplementation(msg.sig, data); - - // Return data - bytes memory retData; - - assembly { - // Start of payload raw data (skip over size slot) - let dataPtr := add(data, 0x20) - - // Payload's size - let dataSize := mload(data) - - // Figure out what OPCODE to use and forward call - let result - switch _storageIsEternal - case 0 { // Not eternal, use implementation's storage - result := call(gas, _implementation, callvalue, dataPtr, dataSize, 0, 0) - } - default { // Eternal, use current contract's storage - result := delegatecall(gas, _implementation, dataPtr, dataSize, 0, 0) - } - - // Size of the returned data - let retSize := returndatasize - - let retPtr := mload(0x40) // Start of free memory - let retDataPtr := add(retPtr, 0x20) // Make space for 'bytes' size - - // Build `retData` 'bytes' - mstore(retPtr, retSize) // Copy size - returndatacopy(retDataPtr, 0, retSize) // Copy returned data - - // Figure out wether to revert or continue with the returned data - switch result - case 0 { // Error - revert(retDataPtr, retSize) - } - default { // Success - retData := retPtr - } - } - - // Call on-chain handler - handleProxySuccess(msg.sig, data, retData); - - assembly { - return(add(retData, 0x20), mload(retData)) // Return returned data - } - } - - /* Private */ - - /** - * @notice On-chain handler that gets called with call data and the 'implementation' contract's return data after a call is successfully proxied. - * @dev Overwrite this function to handle the results of proxied calls in this contract. - * @param sig The function signature of the called function. - * @param data The data passed into the call. - * @param retData The return data of the 'implementation' contract for the proxied call. - */ - function handleProxySuccess(bytes4 sig, bytes data, bytes retData) private {} - - /* Private Views */ - - /** - * @notice Function for dynamically getting the 'implementation' contract address. - * @dev Overwrite this function to implement custom resolving logic based on the function being called and the data passed in. - * @param sig The function signature of the called function. - * @param data The data passed into the call. - * @return The resolved 'implementation' contract address. - */ - function getImplementation(bytes4 sig, bytes data) private view returns (address) { return implementation; } } diff --git a/contracts/standard/proxy/SimpleArbitratorVersioningProxy.sol b/contracts/standard/proxy/SimpleArbitratorVersioningProxy.sol deleted file mode 100644 index a7e4c8e4..00000000 --- a/contracts/standard/proxy/SimpleArbitratorVersioningProxy.sol +++ /dev/null @@ -1,72 +0,0 @@ -pragma solidity ^0.4.15; - -import "kleros/contracts/KlerosPOC.sol"; - -import "./VersioningProxy.sol"; - -/** - * @title SimpleArbitratorVersioningProxy - * @author Enrique Piqueras - - * @notice A simpler ArbitratorVersioningProxy that only exposes methods in the Arbitrator spec. - */ - contract SimpleArbitratorVersioningProxy is VersioningProxy { - /* Storage */ - - mapping (uint256 => address) public disputes; - - /* Modifiers */ - - modifier onlyIfDisputeExists { - require(disputes[_disputeID] != address(0)); - _; - } - - /* Constructor */ - - /** - * @notice Constructs the arbitrator versioning proxy with the first arbitrator contract version address and tags it v0.0.1. - * @param firstAddress The address of the first arbitrator contract version. - */ - function SimpleArbitratorVersioningProxy(address firstAddress) VersioningProxy(false, "0.0.1", firstAddress) public {} - - /* Fallback */ - - /** - * @notice Overwrites default Proxy behavior. - * @dev @overwrite Proxy. - */ - function() private {} - - /* Public */ - - function createDispute(bytes _extraData) public payable returns(uint256 disputeID) { - uint256 disputeID = KlerosPOC(stable._address).createDispute(_extraData); - disputes[disputeID] = stable._address; // Remember arbitrator - return disputeID; - } - - function appeal(uint _disputeID, bytes _extraData) public payable onlyIfDisputeExists returns(uint256 disputeID) { - if (disputes[_disputeID] != stable._address) // Arbitrator has been upgraded, create a new dispute in the new arbitrator - return createDispute(_extraData); - - return KlerosPOC(stable._address).appeal(_disputeID, _extraData); - } - - /* Public Views */ - - function arbitrationCost(bytes _extraData) public view returns(uint256 fees) { - return KlerosPOC(stable._address).arbitrationCost(_extraData); - } - - function appealCost(uint256 _disputeID, bytes _extraData) public view returns(uint256 fees) { - return KlerosPOC(stable._address).appealCost(_disputeID, _extraData); - } - - function currentRuling(uint _disputeID) public view onlyIfDisputeExists returns (uint ruling) { - return KlerosPOC(disputes[_disputeID]).currentRuling(_disputeID); - } - - function disputeStatus(uint _disputeID) public view onlyIfDisputeExists returns (DisputeStatus status) { - return KlerosPOC(disputes[_disputeID]).disputeStatus(_disputeID); - } -} diff --git a/contracts/standard/proxy/VersioningProxy.sol b/contracts/standard/proxy/VersioningProxy.sol index 6218e27f..cf56795a 100644 --- a/contracts/standard/proxy/VersioningProxy.sol +++ b/contracts/standard/proxy/VersioningProxy.sol @@ -19,12 +19,12 @@ contract VersioningProxy is Proxy { /** * @notice Called whenever 'stable' changes for off-chain handling. - * @param prevTag The previous 'stable' managed contract version tag. - * @param prevAddress The previous 'stable' managed contract address. - * @param nextTag The next 'stable' managed contract version tag. - * @param nextAddress The next 'stable' managed contract address. + * @param _prevTag The previous 'stable' managed contract version tag. + * @param _prevAddress The previous 'stable' managed contract address. + * @param _nextTag The next 'stable' managed contract version tag. + * @param _nextAddress The next 'stable' managed contract address. */ - event OnStableChange(bytes32 prevTag, address prevAddress, bytes32 nextTag, address nextAddress); + event OnStableChange(bytes32 _prevTag, address _prevAddress, bytes32 _nextTag, address _nextAddress); /* Storage */ @@ -51,12 +51,11 @@ contract VersioningProxy is Proxy { /** * @notice Constructs the versioning proxy with the proxy eternal storage flag and the first version of the managed contract, `firstTag`, at `firstAddress`. - * @param storageIsEternal Wether this contract should store all storage. I.e. Use 'delegatecall'. - * @param firstTag The version tag of the first version of the managed contract. - * @param firstAddress The address of the first verion of the managed contract. + * @param _firstTag The version tag of the first version of the managed contract. + * @param _firstAddress The address of the first verion of the managed contract. */ - function VersioningProxy(bool storageIsEternal, bytes32 firstTag, address firstAddress) Proxy(storageIsEternal, firstAddress) public { - publish(firstTag, firstAddress); + function VersioningProxy(bytes32 _firstTag, address _firstAddress) Proxy(_firstAddress) public { + publish(_firstTag, _firstAddress); } /* External */ @@ -65,7 +64,7 @@ contract VersioningProxy is Proxy { * @notice Rolls back 'stable' to the previous deployment, and returns true, if one exists, returns false otherwise. * @return True if there was a previous version and the rollback succeeded, false otherwise. */ - function rollback() external onlyOwner returns(bool) { + function rollback() external onlyOwner returns(bool _success) { uint256 tagsLen = tags.length; if (tagsLen <= 2) // We don't have a previous deployment, return false return false; @@ -82,7 +81,7 @@ contract VersioningProxy is Proxy { * @notice Returns all deployed version tags. * @return All of the deployed version tags. */ - function allTags() external view returns(bytes32[]) { + function allTags() external view returns(bytes32[] _tags) { return tags; } @@ -90,25 +89,25 @@ contract VersioningProxy is Proxy { /** * @notice Publishes the next version of the managed contract, `nextTag`, at `nextAddress`. - * @param nextTag The next version tag. - * @param nextAddress The next address of the managed contract. + * @param _nextTag The next version tag. + * @param _nextAddress The next address of the managed contract. */ - function publish(bytes32 nextTag, address nextAddress) public onlyOwner { + function publish(bytes32 _nextTag, address _nextAddress) public onlyOwner { // Publish - tags.push(nextTag); // Push next tag - addresses[nextTag] = nextAddress; // Set next address + tags.push(_nextTag); // Push next tag + addresses[_nextTag] = _nextAddress; // Set next address // Set 'stable' - setStable(nextTag); + setStable(_nextTag); } /** * @notice Sets the value of 'stable' to the address of `nextTag`. - * @param nextTag The already published version tag. + * @param _nextTag The already published version tag. */ - function setStable(bytes32 nextTag) public onlyOwner { + function setStable(bytes32 _nextTag) public onlyOwner { // Make sure this version has already been published - address nextAddress = addresses[nextTag]; + address nextAddress = addresses[_nextTag]; require(nextAddress != address(0)); // Save current tag and address for handlers @@ -116,11 +115,11 @@ contract VersioningProxy is Proxy { address prevAddress = stable._address; // Set 'stable' - stable = Deployment({tag: nextTag, _address: nextAddress}); + stable = Deployment({tag: _nextTag, _address: nextAddress}); // Call handler and fire event - handleStableChange(prevTag, prevAddress, nextTag, nextAddress); // on-chain - OnStableChange(prevTag, prevAddress, nextTag, nextAddress); // off-chain + handleStableChange(prevTag, prevAddress, _nextTag, nextAddress); // on-chain + OnStableChange(prevTag, prevAddress, _nextTag, nextAddress); // off-chain // Change proxy target implementation = nextAddress; @@ -131,10 +130,10 @@ contract VersioningProxy is Proxy { /** * @notice Called whenever 'stable' changes for on-chain handling. * @dev Overwrite this function to handle 'stable' changes on-chain. - * @param prevTag The previous 'stable' managed contract version tag. - * @param prevAddress The previous 'stable' managed contract address. - * @param nextTag The next 'stable' managed contract version tag. - * @param nextAddress The next 'stable' managed contract address. + * @param _prevTag The previous 'stable' managed contract version tag. + * @param _prevAddress The previous 'stable' managed contract address. + * @param _nextTag The next 'stable' managed contract version tag. + * @param _nextAddress The next 'stable' managed contract address. */ - function handleStableChange(bytes32 prevTag, address prevAddress, bytes32 nextTag, address nextAddress) private {} + function handleStableChange(bytes32 _prevTag, address _prevAddress, bytes32 _nextTag, address _nextAddress) private {} }