diff --git a/README.md b/README.md index f25b334..d855678 100644 --- a/README.md +++ b/README.md @@ -1,13 +1,8 @@ # Codex Protocol | Codex Registry Contracts [![Build Status](https://travis-ci.org/codex-protocol/contract.codex-registry.svg?branch=master)](https://travis-ci.org/codex-protocol/contract.codex-registry) -## To do -- Token Ejection (similar to BiddableEscrow) -- 100% code coverage -- Gas optimization? - ## Notes -- If you accidentally send ERC-20 tokens to the address where this is deployed, please email contact@codexprotocol.com to discuss retrieval +- If you accidentally send ERC-20 tokens to the address where this is deployed, please email help@codexprotocol.com to discuss retrieval ## Thanks to - OpenZeppelin for providing tons of contract functionality in their repo at https://github.com/OpenZeppelin/zeppelin-solidity. Licensing for files taken from that repo can be found at OZ_LICENSE. diff --git a/contracts/CodexRecord.sol b/contracts/CodexRecord.sol index fee32dd..6d750fd 100644 --- a/contracts/CodexRecord.sol +++ b/contracts/CodexRecord.sol @@ -15,4 +15,13 @@ contract CodexRecord is CodexRecordAccess { * @dev Constructor function */ constructor() public ERC721Token("Codex Record", "CR") { } + + /** + * @dev Reclaim all ERC20Basic compatible tokens + * @param token ERC20Basic The address of the token contract + */ + function reclaimToken(ERC20Basic token) external onlyOwner { + uint256 balance = token.balanceOf(this); + token.transfer(owner, balance); + } } diff --git a/contracts/CodexRecordCore.sol b/contracts/CodexRecordCore.sol index a41f760..4658757 100644 --- a/contracts/CodexRecordCore.sol +++ b/contracts/CodexRecordCore.sol @@ -47,11 +47,9 @@ contract CodexRecordCore is CodexRecordFees { ) public { - // For now, all new tokens will be the last entry in the array + // All new tokens will be the last entry in the array uint256 newTokenId = allTokens.length; - - // Add a new token to the allTokens array - super._mint(_to, newTokenId); + internalMint(_to, newTokenId); // Add metadata to the newly created token // @@ -68,4 +66,18 @@ contract CodexRecordCore is CodexRecordFees { emit Minted(newTokenId, _providerId, _providerMetadataId); } } + + function internalMint(address _to, uint256 _tokenId) internal { + require(_to != address(0)); + + tokenOwner[_tokenId] = _to; + ownedTokensCount[_to] = ownedTokensCount[_to].add(1); + + ownedTokensIndex[_tokenId] = ownedTokens[_to].length; + ownedTokens[_to].push(_tokenId); + + allTokens.push(_tokenId); + + emit Transfer(address(0), _to, _tokenId); + } } diff --git a/contracts/CodexRecordFees.sol b/contracts/CodexRecordFees.sol index deff9fc..4c2ec55 100644 --- a/contracts/CodexRecordFees.sol +++ b/contracts/CodexRecordFees.sol @@ -4,15 +4,15 @@ import "./CodexRecordMetadata.sol"; import "./ERC20/ERC20.sol"; import "./ERC900/ERC900.sol"; -import "./library/Pausable.sol"; +import "./library/DelayedPausable.sol"; /** * @title CodexRecordFees * @dev Storage, mutators, and modifiers for fees when using the token. - * This also includes the Pausable contract for the onlyOwner modifier. + * This also includes the DelayedPausable contract for the onlyOwner modifier. */ -contract CodexRecordFees is CodexRecordMetadata, Pausable { +contract CodexRecordFees is CodexRecordMetadata, DelayedPausable { // Implementation of the ERC20 Codex Protocol Token, used for fees in the contract ERC20 public codexCoin; diff --git a/contracts/CodexRecordMetadata.sol b/contracts/CodexRecordMetadata.sol index b11d69c..8586bd2 100644 --- a/contracts/CodexRecordMetadata.sol +++ b/contracts/CodexRecordMetadata.sol @@ -32,17 +32,6 @@ contract CodexRecordMetadata is ERC721Token { // via the tokenURI method string public tokenURIPrefix; - /** - * @dev Modifier that checks to see if the token exists - * @param _tokenId uint256 The token ID - */ - modifier tokenExists(uint256 _tokenId) { - require( - exists(_tokenId), - "Token doesn't exist"); - _; - } - /** * @dev Updates token metadata hashes to whatever is passed in * @param _tokenId uint256 The token ID @@ -62,8 +51,8 @@ contract CodexRecordMetadata is ERC721Token { string _providerId, // TODO: convert to bytes32? string _providerMetadataId // TODO: convert to bytes32? ) - tokenExists(_tokenId) - public onlyOwnerOf(_tokenId) + public + onlyOwnerOf(_tokenId) { // nameHash is only overridden if it's not a blank string, since name is a // required value @@ -112,7 +101,6 @@ contract CodexRecordMetadata is ERC721Token { ) public view - tokenExists(_tokenId) returns (bytes32 nameHash, bytes32 descriptionHash, bytes32[] fileHashes) { return ( @@ -143,7 +131,6 @@ contract CodexRecordMetadata is ERC721Token { ) public view - tokenExists(_tokenId) returns (string) { bytes memory prefix = bytes(tokenURIPrefix); diff --git a/contracts/CodexStakeContainer.sol b/contracts/CodexStakeContainer.sol index a2d658b..5db737c 100644 --- a/contracts/CodexStakeContainer.sol +++ b/contracts/CodexStakeContainer.sol @@ -2,14 +2,13 @@ pragma solidity 0.4.24; import "./ERC900/ERC900BasicStakeContainer.sol"; -// @TODO: Add Pausable functionality to prevent staking/unstaking? -import "./library/Pausable.sol"; +import "./library/Ownable.sol"; /** * @title CodexStakeContainer */ -contract CodexStakeContainer is ERC900BasicStakeContainer, Pausable { +contract CodexStakeContainer is ERC900BasicStakeContainer, Ownable { /** * @dev Constructor function * @param _stakingToken ERC20 The address of the token used for staking diff --git a/contracts/ERC721/ERC721Basic.sol b/contracts/ERC721/ERC721Basic.sol index bf3a0e5..da45396 100644 --- a/contracts/ERC721/ERC721Basic.sol +++ b/contracts/ERC721/ERC721Basic.sol @@ -17,9 +17,9 @@ contract ERC721Basic { // bytes4(keccak256('safeTransferFrom(address,address,uint256,bytes)')); bytes4 constant INTERFACE_ERC721 = 0x80ac58cd; - event Transfer(address indexed _from, address indexed _to, uint256 _tokenId); - event Approval(address indexed _owner, address indexed _approved, uint256 _tokenId); - event ApprovalForAll(address indexed _owner, address indexed _operator, bool _approved); + event Transfer(address indexed _from, address indexed _to, uint256 indexed _tokenId); + event Approval(address indexed _owner, address indexed _approved, uint256 indexed _tokenId); + event ApprovalForAll(address indexed _owner, address indexed _operator, bool indexed _approved); function balanceOf(address _owner) public view returns (uint256 _balance); function ownerOf(uint256 _tokenId) public view returns (address _owner); diff --git a/contracts/ERC721/ERC721BasicToken.sol b/contracts/ERC721/ERC721BasicToken.sol index 28b5038..29b0d2b 100644 --- a/contracts/ERC721/ERC721BasicToken.sol +++ b/contracts/ERC721/ERC721BasicToken.sol @@ -16,9 +16,9 @@ contract ERC721BasicToken is ERC721Basic, ERC165 { using SafeMath for uint256; using AddressUtils for address; - // Equals to `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))` + // Equals to `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))` // which can be also obtained as `ERC721Receiver(0).onERC721Received.selector` - bytes4 constant ERC721_RECEIVED = 0xf0b9e5ba; + bytes4 constant ERC721_RECEIVED = 0x150b7a02; // Mapping from token ID to owner mapping (uint256 => address) internal tokenOwner; @@ -41,15 +41,6 @@ contract ERC721BasicToken is ERC721Basic, ERC165 { _; } - /** - * @dev Checks msg.sender can transfer a token, by being owner, approved, or operator - * @param _tokenId uint256 ID of the token to validate - */ - modifier canTransfer(uint256 _tokenId) { - require(isApprovedOrOwner(msg.sender, _tokenId)); - _; - } - /** * @dev Checks if the smart contract includes a specific interface. * @param _interfaceID The interface identifier, as specified in ERC-165. @@ -150,9 +141,9 @@ contract ERC721BasicToken is ERC721Basic, ERC165 { function transferFrom( address _from, address _to, - uint256 _tokenId) + uint256 _tokenId + ) public - canTransfer(_tokenId) { internalTransferFrom( _from, @@ -164,7 +155,7 @@ contract ERC721BasicToken is ERC721Basic, ERC165 { * @dev Safely transfers the ownership of a given token ID to another address * @dev If the target address is a contract, it must implement `onERC721Received`, * which is called upon a safe transfer, and return the magic value - * `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`; otherwise, + * `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`; otherwise, * the transfer is reverted. * @dev Requires the msg sender to be the owner, approved, or operator * @param _from current owner of the token @@ -174,9 +165,9 @@ contract ERC721BasicToken is ERC721Basic, ERC165 { function safeTransferFrom( address _from, address _to, - uint256 _tokenId) + uint256 _tokenId + ) public - canTransfer(_tokenId) { internalSafeTransferFrom( _from, @@ -189,7 +180,7 @@ contract ERC721BasicToken is ERC721Basic, ERC165 { * @dev Safely transfers the ownership of a given token ID to another address * @dev If the target address is a contract, it must implement `onERC721Received`, * which is called upon a safe transfer, and return the magic value - * `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`; otherwise, + * `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`; otherwise, * the transfer is reverted. * @dev Requires the msg sender to be the owner, approved, or operator * @param _from current owner of the token @@ -201,9 +192,9 @@ contract ERC721BasicToken is ERC721Basic, ERC165 { address _from, address _to, uint256 _tokenId, - bytes _data) + bytes _data + ) public - canTransfer(_tokenId) { internalSafeTransferFrom( _from, @@ -215,15 +206,29 @@ contract ERC721BasicToken is ERC721Basic, ERC165 { function internalTransferFrom( address _from, address _to, - uint256 _tokenId) + uint256 _tokenId + ) internal { - require(_from != address(0)); + address owner = ownerOf(_tokenId); + require(_from == owner); require(_to != address(0)); - clearApproval(_from, _tokenId); - removeTokenFrom(_from, _tokenId); - addTokenTo(_to, _tokenId); + address sender = msg.sender; + + require( + sender == owner || isApprovedForAll(owner, sender) || getApproved(_tokenId) == sender, + "Not authorized to transfer" + ); + + // Resetting the approved address if it's set + if (tokenApprovals[_tokenId] != address(0)) { + tokenApprovals[_tokenId] = address(0); + } + + tokenOwner[_tokenId] = _to; + ownedTokensCount[_from] = ownedTokensCount[_from].sub(1); + ownedTokensCount[_to] = ownedTokensCount[_to].add(1); emit Transfer(_from, _to, _tokenId); } @@ -232,7 +237,8 @@ contract ERC721BasicToken is ERC721Basic, ERC165 { address _from, address _to, uint256 _tokenId, - bytes _data) + bytes _data + ) internal { internalTransferFrom(_from, _to, _tokenId); @@ -246,66 +252,6 @@ contract ERC721BasicToken is ERC721Basic, ERC165 { ); } - /** - * @dev Returns whether the given spender can transfer a given token ID - * @param _spender address of the spender to query - * @param _tokenId uint256 ID of the token to be transferred - * @return bool whether the msg.sender is approved for the given token ID, - * is an operator of the owner, or is the owner of the token - */ - function isApprovedOrOwner(address _spender, uint256 _tokenId) internal view returns (bool) { - address owner = ownerOf(_tokenId); - return _spender == owner || getApproved(_tokenId) == _spender || isApprovedForAll(owner, _spender); - } - - /** - * @dev Internal function to mint a new token - * @dev Reverts if the given token ID already exists - * @param _to The address that will own the minted token - * @param _tokenId uint256 ID of the token to be minted by the msg.sender - */ - function _mint(address _to, uint256 _tokenId) internal { - require(_to != address(0)); - addTokenTo(_to, _tokenId); - emit Transfer(address(0), _to, _tokenId); - } - - /** - * @dev Internal function to clear current approval of a given token ID - * @dev Reverts if the given address is not indeed the owner of the token - * @param _owner owner of the token - * @param _tokenId uint256 ID of the token to be transferred - */ - function clearApproval(address _owner, uint256 _tokenId) internal { - require(ownerOf(_tokenId) == _owner); - if (tokenApprovals[_tokenId] != address(0)) { - tokenApprovals[_tokenId] = address(0); - emit Approval(_owner, address(0), _tokenId); - } - } - - /** - * @dev Internal function to add a token ID to the list of a given address - * @param _to address representing the new owner of the given token ID - * @param _tokenId uint256 ID of the token to be added to the tokens list of the given address - */ - function addTokenTo(address _to, uint256 _tokenId) internal { - require(tokenOwner[_tokenId] == address(0)); - tokenOwner[_tokenId] = _to; - ownedTokensCount[_to] = ownedTokensCount[_to].add(1); - } - - /** - * @dev Internal function to remove a token ID from the list of a given address - * @param _from address representing the previous owner of the given token ID - * @param _tokenId uint256 ID of the token to be removed from the tokens list of the given address - */ - function removeTokenFrom(address _from, uint256 _tokenId) internal { - require(ownerOf(_tokenId) == _from); - ownedTokensCount[_from] = ownedTokensCount[_from].sub(1); - tokenOwner[_tokenId] = address(0); - } - /** * @dev Internal function to invoke `onERC721Received` on a target address * @dev The call is not executed if the target address is not a contract @@ -316,13 +262,26 @@ contract ERC721BasicToken is ERC721Basic, ERC165 { * @return whether the call correctly returned the expected magic value */ function checkAndCallSafeTransfer( - address _from, address _to, uint256 _tokenId, bytes _data) internal returns (bool) + address _from, + address _to, + uint256 _tokenId, + bytes _data + ) + internal + returns (bool) { if (!_to.isContract()) { return true; } - bytes4 retval = ERC721Receiver(_to).onERC721Received(_from, _tokenId, _data); + bytes4 retval = ERC721Receiver(_to) + .onERC721Received( + msg.sender, + _from, + _tokenId, + _data + ); + return (retval == ERC721_RECEIVED); } } diff --git a/contracts/ERC721/ERC721Receiver.sol b/contracts/ERC721/ERC721Receiver.sol index 3d13d64..9326f10 100644 --- a/contracts/ERC721/ERC721Receiver.sol +++ b/contracts/ERC721/ERC721Receiver.sol @@ -9,22 +9,28 @@ pragma solidity 0.4.24; contract ERC721Receiver { /** * @dev Magic value to be returned upon successful reception of an NFT - * Equals to `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`, + * Equals to `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`, * which can be also obtained as `ERC721Receiver(0).onERC721Received.selector` */ - bytes4 constant ERC721_RECEIVED = 0xf0b9e5ba; + bytes4 constant ERC721_RECEIVED = 0x150b7a02; /** * @notice Handle the receipt of an NFT * @dev The ERC721 smart contract calls this function on the recipient * after a `safetransfer`. This function MAY throw to revert and reject the - * transfer. This function MUST use 50,000 gas or less. Return of other - * than the magic value MUST result in the transaction being reverted. + * transfer. Returns other than the magic value MUST result in the + * transaction being reverted. * Note: the contract address is always the message sender. * @param _from The sending address * @param _tokenId The NFT identifier which is being transfered * @param _data Additional data with no specified format - * @return `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))` + * @return `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))` */ - function onERC721Received(address _from, uint256 _tokenId, bytes _data) public returns(bytes4); + function onERC721Received( + address _operator, + address _from, + uint256 _tokenId, + bytes _data) + public + returns(bytes4); } diff --git a/contracts/ERC721/ERC721Token.sol b/contracts/ERC721/ERC721Token.sol index c1e9b45..ac9fb66 100644 --- a/contracts/ERC721/ERC721Token.sol +++ b/contracts/ERC721/ERC721Token.sol @@ -101,6 +101,27 @@ contract ERC721Token is ERC721, ERC721BasicToken { return allTokens[_index]; } + function internalTransferFrom( + address _from, + address _to, + uint256 _tokenId + ) + internal + { + super.internalTransferFrom(_from, _to, _tokenId); + + uint256 removeTokenIndex = ownedTokensIndex[_tokenId]; + uint256 lastTokenIndex = ownedTokens[_from].length.sub(1); + uint256 lastToken = ownedTokens[_from][lastTokenIndex]; + + ownedTokens[_from][removeTokenIndex] = lastToken; + ownedTokens[_from].length--; + ownedTokensIndex[lastToken] = removeTokenIndex; + + ownedTokens[_to].push(_tokenId); + ownedTokensIndex[_tokenId] = ownedTokens[_to].length - 1; + } + /** * @dev Internal function to set the token URI for a given token * @dev Reverts if the token ID does not exist @@ -111,51 +132,4 @@ contract ERC721Token is ERC721, ERC721BasicToken { require(exists(_tokenId)); tokenURIs[_tokenId] = _uri; } - - /** - * @dev Internal function to add a token ID to the list of a given address - * @param _to address representing the new owner of the given token ID - * @param _tokenId uint256 ID of the token to be added to the tokens list of the given address - */ - function addTokenTo(address _to, uint256 _tokenId) internal { - super.addTokenTo(_to, _tokenId); - uint256 length = ownedTokens[_to].length; - ownedTokens[_to].push(_tokenId); - ownedTokensIndex[_tokenId] = length; - } - - /** - * @dev Internal function to remove a token ID from the list of a given address - * @param _from address representing the previous owner of the given token ID - * @param _tokenId uint256 ID of the token to be removed from the tokens list of the given address - */ - function removeTokenFrom(address _from, uint256 _tokenId) internal { - super.removeTokenFrom(_from, _tokenId); - - uint256 tokenIndex = ownedTokensIndex[_tokenId]; - uint256 lastTokenIndex = ownedTokens[_from].length.sub(1); - uint256 lastToken = ownedTokens[_from][lastTokenIndex]; - - ownedTokens[_from][tokenIndex] = lastToken; - ownedTokens[_from][lastTokenIndex] = 0; - // Note that this will handle single-element arrays. In that case, both tokenIndex and lastTokenIndex are going to - // be zero. Then we can make sure that we will remove _tokenId from the ownedTokens list since we are first swapping - // the lastToken to the first position, and then dropping the element placed in the last position of the list - - ownedTokens[_from].length--; - ownedTokensIndex[_tokenId] = 0; - ownedTokensIndex[lastToken] = tokenIndex; - } - - /** - * @dev Internal function to mint a new token - * @dev Reverts if the given token ID already exists - * @param _to address the beneficiary that will own the minted token - * @param _tokenId uint256 ID of the token to be minted by the msg.sender - */ - function _mint(address _to, uint256 _tokenId) internal { - super._mint(_to, _tokenId); - - allTokens.push(_tokenId); - } } diff --git a/contracts/library/Debuggable.sol b/contracts/library/Debuggable.sol index 8412550..c33bd28 100644 --- a/contracts/library/Debuggable.sol +++ b/contracts/library/Debuggable.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.24; +pragma solidity 0.4.24; /** diff --git a/contracts/library/DelayedOwnable.sol b/contracts/library/DelayedOwnable.sol index 4bc290c..5c46273 100644 --- a/contracts/library/DelayedOwnable.sol +++ b/contracts/library/DelayedOwnable.sol @@ -23,8 +23,11 @@ contract DelayedOwnable { } function initializeOwnable(address _owner) external { - require(!isInitialized, "The owner has already been set"); + require( + !isInitialized, + "The owner has already been set"); + isInitialized = true; owner = _owner; } diff --git a/contracts/library/DelayedPausable.sol b/contracts/library/DelayedPausable.sol new file mode 100644 index 0000000..34523c7 --- /dev/null +++ b/contracts/library/DelayedPausable.sol @@ -0,0 +1,49 @@ +pragma solidity 0.4.24; + + +import "./DelayedOwnable.sol"; + + +/** + * @title Pausable + * @dev Base contract which allows children to implement an emergency stop mechanism. + */ +contract DelayedPausable is DelayedOwnable { + event Pause(); + event Unpause(); + + bool public paused = false; + + + /** + * @dev Modifier to make a function callable only when the contract is not paused. + */ + modifier whenNotPaused() { + require(!paused); + _; + } + + /** + * @dev Modifier to make a function callable only when the contract is paused. + */ + modifier whenPaused() { + require(paused); + _; + } + + /** + * @dev called by the owner to pause, triggers stopped state + */ + function pause() onlyOwner whenNotPaused public { + paused = true; + emit Pause(); + } + + /** + * @dev called by the owner to unpause, returns to normal state + */ + function unpause() onlyOwner whenPaused public { + paused = false; + emit Unpause(); + } +} diff --git a/contracts/library/Ownable.sol b/contracts/library/Ownable.sol new file mode 100644 index 0000000..7745de6 --- /dev/null +++ b/contracts/library/Ownable.sol @@ -0,0 +1,64 @@ +pragma solidity 0.4.24; + + +/** + * @title Ownable + * @dev The Ownable contract has an owner address, and provides basic authorization control + * functions, this simplifies the implementation of "user permissions". + */ +contract Ownable { + address public owner; + + + event OwnershipRenounced(address indexed previousOwner); + event OwnershipTransferred( + address indexed previousOwner, + address indexed newOwner + ); + + + /** + * @dev The Ownable constructor sets the original `owner` of the contract to the sender + * account. + */ + constructor() public { + owner = msg.sender; + } + + /** + * @dev Throws if called by any account other than the owner. + */ + modifier onlyOwner() { + require(msg.sender == owner); + _; + } + + /** + * @dev Allows the current owner to relinquish control of the contract. + * @notice Renouncing to ownership will leave the contract without an owner. + * It will not be possible to call the functions with the `onlyOwner` + * modifier anymore. + */ + function renounceOwnership() public onlyOwner { + emit OwnershipRenounced(owner); + owner = address(0); + } + + /** + * @dev Allows the current owner to transfer control of the contract to a newOwner. + * @param _newOwner The address to transfer ownership to. + */ + function transferOwnership(address _newOwner) public onlyOwner { + _transferOwnership(_newOwner); + } + + /** + * @dev Transfers control of the contract to a newOwner. + * @param _newOwner The address to transfer ownership to. + */ + function _transferOwnership(address _newOwner) internal { + require(_newOwner != address(0)); + emit OwnershipTransferred(owner, _newOwner); + owner = _newOwner; + } +} diff --git a/contracts/library/Pausable.sol b/contracts/library/Pausable.sol index 45c4cb7..8bca18d 100644 --- a/contracts/library/Pausable.sol +++ b/contracts/library/Pausable.sol @@ -1,14 +1,14 @@ pragma solidity 0.4.24; -import "./DelayedOwnable.sol"; +import "./Ownable.sol"; /** * @title Pausable * @dev Base contract which allows children to implement an emergency stop mechanism. */ -contract Pausable is DelayedOwnable { +contract Pausable is Ownable { event Pause(); event Unpause(); diff --git a/contracts/mocks/DelayedPausableMock.sol b/contracts/mocks/DelayedPausableMock.sol new file mode 100644 index 0000000..ff80eca --- /dev/null +++ b/contracts/mocks/DelayedPausableMock.sol @@ -0,0 +1,25 @@ +pragma solidity 0.4.24; + + +import "../library/DelayedPausable.sol"; + + +// mock class using DelayedPausable +contract DelayedPausableMock is DelayedPausable { + bool public drasticMeasureTaken; + uint256 public count; + + constructor() public { + drasticMeasureTaken = false; + count = 0; + } + + function normalProcess() external whenNotPaused { + count++; + } + + function drasticMeasure() external whenPaused { + drasticMeasureTaken = true; + } + +} diff --git a/contracts/mocks/ERC20/PausableToken.sol b/contracts/mocks/ERC20/PausableToken.sol index f3b4721..33c9aff 100644 --- a/contracts/mocks/ERC20/PausableToken.sol +++ b/contracts/mocks/ERC20/PausableToken.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.24; +pragma solidity 0.4.24; import "./StandardToken.sol"; import "../../library/Pausable.sol"; diff --git a/contracts/mocks/ERC721/ERC721BasicTokenMock.sol b/contracts/mocks/ERC721/ERC721BasicTokenMock.sol index 770e673..a5e8d81 100644 --- a/contracts/mocks/ERC721/ERC721BasicTokenMock.sol +++ b/contracts/mocks/ERC721/ERC721BasicTokenMock.sol @@ -9,6 +9,11 @@ import "../../ERC721/ERC721BasicToken.sol"; */ contract ERC721BasicTokenMock is ERC721BasicToken { function mint(address _to, uint256 _tokenId) public { - super._mint(_to, _tokenId); + require(_to != address(0)); + require(!exists(_tokenId)); + + tokenOwner[_tokenId] = _to; + ownedTokensCount[_to] = ownedTokensCount[_to].add(1); + emit Transfer(address(0), _to, _tokenId); } } diff --git a/contracts/mocks/ERC721/ERC721ReceiverMock.sol b/contracts/mocks/ERC721/ERC721ReceiverMock.sol index f009529..340a41b 100644 --- a/contracts/mocks/ERC721/ERC721ReceiverMock.sol +++ b/contracts/mocks/ERC721/ERC721ReceiverMock.sol @@ -7,18 +7,33 @@ contract ERC721ReceiverMock is ERC721Receiver { bytes4 retval; bool reverts; - event Received(address _address, uint256 _tokenId, bytes _data, uint256 _gas); + event Received( + address _operator, + address _from, + uint256 _tokenId, + bytes _data, + uint256 _gas + ); constructor(bytes4 _retval, bool _reverts) public { retval = _retval; reverts = _reverts; } - function onERC721Received(address _address, uint256 _tokenId, bytes _data) public returns(bytes4) { + function onERC721Received( + address _operator, + address _from, + uint256 _tokenId, + bytes _data + ) + public + returns(bytes4) + { require(!reverts); emit Received( - _address, + _operator, + _from, _tokenId, _data, gasleft()); diff --git a/contracts/mocks/ERC721/ERC721TokenMock.sol b/contracts/mocks/ERC721/ERC721TokenMock.sol index 9cb71bf..04029d5 100644 --- a/contracts/mocks/ERC721/ERC721TokenMock.sol +++ b/contracts/mocks/ERC721/ERC721TokenMock.sol @@ -13,7 +13,18 @@ contract ERC721TokenMock is ERC721Token { { } function mint(address _to, uint256 _tokenId) payable public { - super._mint(_to, _tokenId); + require(_to != address(0)); + require(!exists(_tokenId)); + + tokenOwner[_tokenId] = _to; + ownedTokensCount[_to] = ownedTokensCount[_to].add(1); + + ownedTokensIndex[_tokenId] = ownedTokens[_to].length; + ownedTokens[_to].push(_tokenId); + + allTokens.push(_tokenId); + + emit Transfer(address(0), _to, _tokenId); } function setTokenURI(uint256 _tokenId, string _uri) public { diff --git a/licenses/OZ_LICENSE b/licenses/OZ_LICENSE index 522f0aa..c5fb8fc 100644 --- a/licenses/OZ_LICENSE +++ b/licenses/OZ_LICENSE @@ -9,6 +9,14 @@ contracts/ERC721/ERC721BasicToken.sol contracts/ERC721/ERC721Receiver.sol contracts/ERC721/ERC721Token.sol +contracts/library/AddressUtils.sol +contracts/library/DelayedOwnable.sol +contracts/library/DelayedPausable.sol +contracts/library/Ownable.sol +contracts/library/Pausable.sol +contracts/library/ProxyOwnable.sol +contracts/library/SafeMath.sol + contracts/mocks/ERC20/BasicToken.sol contracts/mocks/ERC20/PausableToken.sol contracts/mocks/ERC20/StandardToken.sol @@ -18,13 +26,8 @@ contracts/mocks/ERC721/ERC721TokenMock.sol contracts/mocks/PausableMock.sol contracts/mocks/SafeMathMock.sol -contracts/library/AddressUtils.sol -contracts/library/DelayedOwnable.sol -contracts/library/Pausable.sol -contracts/library/ProxyOwnable.sol -contracts/library/SafeMath.sol - test/library/DelayedOwnable.test.js +test/library/DelayedPausable.test.js test/library/Pausable.test.js test/library/ProxyOwnable.test.js test/library/SafeMath.test.js diff --git a/migrations/7_transfer_ownership.js b/migrations/7_transfer_ownership.js index 53a1b43..7c9b12d 100644 --- a/migrations/7_transfer_ownership.js +++ b/migrations/7_transfer_ownership.js @@ -28,9 +28,11 @@ module.exports = async (deployer, network, accounts) => { throw new Error('No ownership transfer defined for this network') } + console.log('Transferring ownership') + // Transfer ownership of CodexStakeContainer const stakeContainer = await CodexStakeContainer.deployed() - await stakeContainer.initializeOwnable(newOwner) + stakeContainer.transferOwnership(newOwner) // Transfer ownership of CodexRecord from the perspective of CodexRecordProxy. // Initialization of this storage slot has already taken place earlier in migration diff --git a/test/behaviors/ERC721BasicToken.behavior.js b/test/behaviors/ERC721BasicToken.behavior.js index 68d4f63..7120bbd 100644 --- a/test/behaviors/ERC721BasicToken.behavior.js +++ b/test/behaviors/ERC721BasicToken.behavior.js @@ -17,7 +17,7 @@ export default function shouldBehaveLikeERC721BasicToken(accounts, customMintFun const unknownTokenId = 2 const creator = accounts[0] const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000' - const RECEIVER_MAGIC_VALUE = '0xf0b9e5ba' + const RECEIVER_MAGIC_VALUE = '0x150b7a02' let mintFunction if (customMintFunction) { @@ -111,7 +111,7 @@ export default function shouldBehaveLikeERC721BasicToken(accounts, customMintFun await this.token.setApprovalForAll(operator, true, { from: owner }) }) - const transferWasSuccessful = function (expctedOwner, expectedTokenId, expectedApproved) { + const transferWasSuccessful = function (expectedOwner, expectedTokenId) { it('transfers the ownership of the given token ID to the given address', async function () { const newOwner = await this.token.ownerOf(expectedTokenId) newOwner.should.be.equal(this.to) @@ -122,34 +122,19 @@ export default function shouldBehaveLikeERC721BasicToken(accounts, customMintFun approvedAccount.should.be.equal(ZERO_ADDRESS) }) - if (expectedApproved) { - it('emits an approval and transfer events', async function () { - logs.length.should.be.equal(2) - logs[0].event.should.be.eq('Approval') - logs[0].args._owner.should.be.equal(expctedOwner) - logs[0].args._approved.should.be.equal(ZERO_ADDRESS) - logs[0].args._tokenId.should.be.bignumber.equal(expectedTokenId) - - logs[1].event.should.be.eq('Transfer') - logs[1].args._from.should.be.equal(expctedOwner) - logs[1].args._to.should.be.equal(this.to) - logs[1].args._tokenId.should.be.bignumber.equal(expectedTokenId) - }) - } else { - it('emits only a transfer event', async function () { - logs.length.should.be.equal(1) - logs[0].event.should.be.eq('Transfer') - logs[0].args._from.should.be.equal(expctedOwner) - logs[0].args._to.should.be.equal(this.to) - logs[0].args._tokenId.should.be.bignumber.equal(expectedTokenId) - }) - } + it('emits only a transfer event', async function () { + logs.length.should.be.equal(1) + logs[0].event.should.be.eq('Transfer') + logs[0].args._from.should.be.equal(expectedOwner) + logs[0].args._to.should.be.equal(this.to) + logs[0].args._tokenId.should.be.bignumber.equal(expectedTokenId) + }) it('adjusts owners balances', async function () { const newOwnerBalance = await this.token.balanceOf(this.to) newOwnerBalance.should.be.bignumber.equal(1) - const previousOwnerBalance = await this.token.balanceOf(expctedOwner) + const previousOwnerBalance = await this.token.balanceOf(expectedOwner) previousOwnerBalance.should.be.bignumber.equal(1) }) @@ -159,7 +144,7 @@ export default function shouldBehaveLikeERC721BasicToken(accounts, customMintFun const newOwnerToken = await this.token.tokenOfOwnerByIndex(this.to, 0) newOwnerToken.toNumber().should.be.equal(expectedTokenId) - const previousOwnerToken = await this.token.tokenOfOwnerByIndex(expctedOwner, 0) + const previousOwnerToken = await this.token.tokenOfOwnerByIndex(expectedOwner, 0) previousOwnerToken.toNumber().should.not.be.equal(expectedTokenId) }) } @@ -169,21 +154,21 @@ export default function shouldBehaveLikeERC721BasicToken(accounts, customMintFun beforeEach(async function () { ({ logs } = await transferFunction.call(this, owner, this.to, tokenId, { from: owner })) }) - transferWasSuccessful(owner, tokenId, approved) + transferWasSuccessful(owner, tokenId) }) describe('when called by the approved individual', function () { beforeEach(async function () { ({ logs } = await transferFunction.call(this, owner, this.to, tokenId, { from: approved })) }) - transferWasSuccessful(owner, tokenId, approved) + transferWasSuccessful(owner, tokenId) }) describe('when called by the operator', function () { beforeEach(async function () { ({ logs } = await transferFunction.call(this, owner, this.to, tokenId, { from: operator })) }) - transferWasSuccessful(owner, tokenId, approved) + transferWasSuccessful(owner, tokenId) }) describe('when called by the owner without an approved user', function () { @@ -191,7 +176,7 @@ export default function shouldBehaveLikeERC721BasicToken(accounts, customMintFun await this.token.approve(ZERO_ADDRESS, tokenId, { from: owner }) ;({ logs } = await transferFunction.call(this, owner, this.to, tokenId, { from: operator })) }) - transferWasSuccessful(owner, tokenId, null) + transferWasSuccessful(owner, tokenId) }) describe('when sent to the owner', function () { @@ -209,17 +194,12 @@ export default function shouldBehaveLikeERC721BasicToken(accounts, customMintFun approvedAccount.should.be.equal(ZERO_ADDRESS) }) - it('emits an approval and transfer events', async function () { - logs.length.should.be.equal(2) - logs[0].event.should.be.eq('Approval') - logs[0].args._owner.should.be.equal(owner) - logs[0].args._approved.should.be.equal(ZERO_ADDRESS) + it('emits only a transfer events', async function () { + logs.length.should.be.equal(1) + logs[0].event.should.be.eq('Transfer') + logs[0].args._from.should.be.equal(owner) + logs[0].args._to.should.be.equal(owner) logs[0].args._tokenId.should.be.bignumber.equal(tokenId) - - logs[1].event.should.be.eq('Transfer') - logs[1].args._from.should.be.equal(owner) - logs[1].args._to.should.be.equal(owner) - logs[1].args._tokenId.should.be.bignumber.equal(tokenId) }) it('keeps the owner balance', async function () { @@ -295,10 +275,22 @@ export default function shouldBehaveLikeERC721BasicToken(accounts, customMintFun it('should call onERC721Received', async function () { const result = await transferFun.call(this, owner, this.to, tokenId, { from: owner }) - result.receipt.logs.length.should.be.equal(3) - const [log] = decodeLogs([result.receipt.logs[2]], ERC721Receiver, this.receiver.address) + result.receipt.logs.length.should.be.equal(2) + const [log] = decodeLogs([result.receipt.logs[1]], ERC721Receiver, this.receiver.address) + log.event.should.be.eq('Received') + log.args._operator.should.be.equal(owner) + log.args._from.should.be.equal(owner) + log.args._tokenId.toNumber().should.be.equal(tokenId) + log.args._data.should.be.equal(transferData) + }) + + it('should call onERC721Received from approved', async function () { + const result = await transferFun.call(this, owner, this.to, tokenId, { from: approved }) + result.receipt.logs.length.should.be.equal(2) + const [log] = decodeLogs([result.receipt.logs[1]], ERC721Receiver, this.receiver.address) log.event.should.be.eq('Received') - log.args._address.should.be.equal(owner) + log.args._operator.should.be.equal(approved) + log.args._from.should.be.equal(owner) log.args._tokenId.toNumber().should.be.equal(tokenId) log.args._data.should.be.equal(transferData) }) diff --git a/test/core/CodexStakeContainer.test.js b/test/core/CodexStakeContainer.test.js index 45b444e..f01eec7 100644 --- a/test/core/CodexStakeContainer.test.js +++ b/test/core/CodexStakeContainer.test.js @@ -74,10 +74,6 @@ contract('CodexStakeContainer', function (accounts) { describe('when changed', function () { const newLockInDuration = lockInDuration * 2 - beforeEach(async function () { - await this.stakeContainer.initializeOwnable(creator) - }) - it('should fail when not called by the owner', async function () { await assertRevert( this.stakeContainer.setLockInDuration(newLockInDuration, { from: otherUser }) @@ -314,7 +310,6 @@ contract('CodexStakeContainer', function (accounts) { describe('when the transfer from the contract fails', function () { it('should revert', async function () { // Pausing the token contract so that the transfer will fail - await this.codexCoin.initializeOwnable(creator) await this.codexCoin.pause() // Changing the timestamp of the next block so the stake is unlocked diff --git a/test/library/DelayedOwnable.test.js b/test/library/DelayedOwnable.test.js index c8976a2..4c6e69f 100644 --- a/test/library/DelayedOwnable.test.js +++ b/test/library/DelayedOwnable.test.js @@ -1,38 +1,62 @@ - import assertRevert from '../helpers/assertRevert' +const { BigNumber } = web3 const DelayedOwnable = artifacts.require('DelayedOwnable') +require('chai') + .use(require('chai-bignumber')(BigNumber)) + .should() + contract('DelayedOwnable', function (accounts) { + const creator = accounts[0] + const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000' let delayedOwnable beforeEach(async function () { delayedOwnable = await DelayedOwnable.new() - await delayedOwnable.initializeOwnable(accounts[0]) }) - it('should have an owner', async function () { - const owner = await delayedOwnable.owner() - assert.isTrue(owner !== 0) + describe('before being initialized', function () { + it('should have isInitialized set to false', async function () { + const isInitialized = await delayedOwnable.isInitialized() + isInitialized.should.be.equal(false) + }) + + it('should not have an owner', async function () { + const owner = await delayedOwnable.owner() + owner.should.be.equal(ZERO_ADDRESS) + }) }) - it('changes owner after transfer', async function () { - const other = accounts[1] - await delayedOwnable.transferOwnership(other) - const owner = await delayedOwnable.owner() + describe('after being initialized', function () { + beforeEach(async function () { + await delayedOwnable.initializeOwnable(creator) + }) - assert.isTrue(owner === other) - }) + it('should have an owner', async function () { + const owner = await delayedOwnable.owner() + owner.should.be.equal(creator) + }) - it('should prevent non-owners from transfering', async function () { - const other = accounts[2] - const owner = await delayedOwnable.owner.call() - assert.isTrue(owner !== other) - await assertRevert(delayedOwnable.transferOwnership(other, { from: other })) - }) + it('changes owner after transfer', async function () { + const other = accounts[1] + await delayedOwnable.transferOwnership(other) + + const owner = await delayedOwnable.owner() + owner.should.be.equal(other) + }) + + it('reverts when initializeOwnable is call more than once', async function () { + await assertRevert(delayedOwnable.initializeOwnable(creator)) + }) + + it('should prevent non-owners from transferring', async function () { + const other = accounts[1] + await assertRevert(delayedOwnable.transferOwnership(other, { from: other })) + }) - it('should guard ownership against stuck state', async function () { - const originalOwner = await delayedOwnable.owner() - await assertRevert(delayedOwnable.transferOwnership(null, { from: originalOwner })) + it('should guard ownership against stuck state', async function () { + await assertRevert(delayedOwnable.transferOwnership(ZERO_ADDRESS)) + }) }) }) diff --git a/test/library/DelayedPausable.test.js b/test/library/DelayedPausable.test.js new file mode 100644 index 0000000..5d0c2d7 --- /dev/null +++ b/test/library/DelayedPausable.test.js @@ -0,0 +1,77 @@ +import assertRevert from '../helpers/assertRevert' + +const DelayedPausableMock = artifacts.require('DelayedPausableMock') + +contract('DelayedPausable', function (accounts) { + const creator = accounts[0] + + it('can perform normal process in non-pause', async function () { + const DelayedPausable = await DelayedPausableMock.new() + await DelayedPausable.initializeOwnable(creator) + + const count0 = await DelayedPausable.count() + assert.equal(count0, 0) + + await DelayedPausable.normalProcess() + const count1 = await DelayedPausable.count() + assert.equal(count1, 1) + }) + + it('can not perform normal process in pause', async function () { + const DelayedPausable = await DelayedPausableMock.new() + await DelayedPausable.initializeOwnable(creator) + + await DelayedPausable.pause() + const count0 = await DelayedPausable.count() + assert.equal(count0, 0) + + await assertRevert(DelayedPausable.normalProcess()) + const count1 = await DelayedPausable.count() + assert.equal(count1, 0) + }) + + it('can not take drastic measure in non-pause', async function () { + const DelayedPausable = await DelayedPausableMock.new() + await DelayedPausable.initializeOwnable(creator) + + await assertRevert(DelayedPausable.drasticMeasure()) + const drasticMeasureTaken = await DelayedPausable.drasticMeasureTaken() + assert.isFalse(drasticMeasureTaken) + }) + + it('can take a drastic measure in a pause', async function () { + const DelayedPausable = await DelayedPausableMock.new() + await DelayedPausable.initializeOwnable(creator) + + await DelayedPausable.pause() + await DelayedPausable.drasticMeasure() + const drasticMeasureTaken = await DelayedPausable.drasticMeasureTaken() + + assert.isTrue(drasticMeasureTaken) + }) + + it('should resume allowing normal process after pause is over', async function () { + const DelayedPausable = await DelayedPausableMock.new() + await DelayedPausable.initializeOwnable(creator) + + await DelayedPausable.pause() + await DelayedPausable.unpause() + await DelayedPausable.normalProcess() + const count0 = await DelayedPausable.count() + + assert.equal(count0, 1) + }) + + it('should prevent drastic measure after pause is over', async function () { + const DelayedPausable = await DelayedPausableMock.new() + await DelayedPausable.initializeOwnable(creator) + + await DelayedPausable.pause() + await DelayedPausable.unpause() + + await assertRevert(DelayedPausable.drasticMeasure()) + + const drasticMeasureTaken = await DelayedPausable.drasticMeasureTaken() + assert.isFalse(drasticMeasureTaken) + }) +}) diff --git a/test/library/Pausable.test.js b/test/library/Pausable.test.js index aee5d24..1945a4c 100644 --- a/test/library/Pausable.test.js +++ b/test/library/Pausable.test.js @@ -1,14 +1,11 @@ - import assertRevert from '../helpers/assertRevert' const PausableMock = artifacts.require('PausableMock') -contract('Pausable', function (accounts) { - const creator = accounts[0] +contract('Pausable', function () { it('can perform normal process in non-pause', async function () { const Pausable = await PausableMock.new() - await Pausable.initializeOwnable(creator) const count0 = await Pausable.count() assert.equal(count0, 0) @@ -20,7 +17,6 @@ contract('Pausable', function (accounts) { it('can not perform normal process in pause', async function () { const Pausable = await PausableMock.new() - await Pausable.initializeOwnable(creator) await Pausable.pause() const count0 = await Pausable.count() @@ -33,7 +29,6 @@ contract('Pausable', function (accounts) { it('can not take drastic measure in non-pause', async function () { const Pausable = await PausableMock.new() - await Pausable.initializeOwnable(creator) await assertRevert(Pausable.drasticMeasure()) const drasticMeasureTaken = await Pausable.drasticMeasureTaken() @@ -42,7 +37,6 @@ contract('Pausable', function (accounts) { it('can take a drastic measure in a pause', async function () { const Pausable = await PausableMock.new() - await Pausable.initializeOwnable(creator) await Pausable.pause() await Pausable.drasticMeasure() @@ -53,7 +47,6 @@ contract('Pausable', function (accounts) { it('should resume allowing normal process after pause is over', async function () { const Pausable = await PausableMock.new() - await Pausable.initializeOwnable(creator) await Pausable.pause() await Pausable.unpause() @@ -65,7 +58,6 @@ contract('Pausable', function (accounts) { it('should prevent drastic measure after pause is over', async function () { const Pausable = await PausableMock.new() - await Pausable.initializeOwnable(creator) await Pausable.pause() await Pausable.unpause() diff --git a/test/library/ProxyOwnable.test.js b/test/library/ProxyOwnable.test.js index bcdb9eb..f56966c 100644 --- a/test/library/ProxyOwnable.test.js +++ b/test/library/ProxyOwnable.test.js @@ -1,4 +1,3 @@ - import assertRevert from '../helpers/assertRevert' const ProxyOwnable = artifacts.require('ProxyOwnable')