diff --git a/smart-contracts/contracts/mixins/MixinConvenienceOwnable.sol b/smart-contracts/contracts/mixins/MixinConvenienceOwnable.sol index bc749c5a2b4..4c6e42c16ee 100644 --- a/smart-contracts/contracts/mixins/MixinConvenienceOwnable.sol +++ b/smart-contracts/contracts/mixins/MixinConvenienceOwnable.sol @@ -2,13 +2,14 @@ pragma solidity ^0.8.0; import './MixinLockCore.sol'; +import './MixinErrors.sol'; /** * @title Mixin to add support for `ownable()` * @dev `Mixins` are a design pattern seen in the 0x contracts. It simply * separates logically groupings of code to ease readability. */ -contract MixinConvenienceOwnable is MixinLockCore { +contract MixinConvenienceOwnable is MixinErrors, MixinLockCore { // used for `owner()`convenience helper address private _convenienceOwner; @@ -38,7 +39,9 @@ contract MixinConvenienceOwnable is MixinLockCore { */ function setOwner(address account) public { _onlyLockManager(); - require(account != address(0), 'OWNER_CANT_BE_ADDRESS_ZERO'); + if(account == address(0)) { + revert OWNER_CANT_BE_ADDRESS_ZERO(); + } address _previousOwner = _convenienceOwner; _convenienceOwner = account; emit OwnershipTransferred(_previousOwner, account); diff --git a/smart-contracts/contracts/mixins/MixinERC721Enumerable.sol b/smart-contracts/contracts/mixins/MixinERC721Enumerable.sol index 808ac4b778b..1f04cee3074 100644 --- a/smart-contracts/contracts/mixins/MixinERC721Enumerable.sol +++ b/smart-contracts/contracts/mixins/MixinERC721Enumerable.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.0; import './MixinKeys.sol'; import './MixinLockCore.sol'; -// import '@openzeppelin/contracts-upgradeable/token/ERC721/extensions/IERC721EnumerableUpgradeable.sol'; +import './MixinErrors.sol'; import '@openzeppelin/contracts-upgradeable/utils/introspection/ERC165StorageUpgradeable.sol'; @@ -12,6 +12,7 @@ import '@openzeppelin/contracts-upgradeable/utils/introspection/ERC165StorageUpg */ contract MixinERC721Enumerable is ERC165StorageUpgradeable, + MixinErrors, MixinLockCore, // Implements totalSupply MixinKeys { @@ -37,7 +38,9 @@ contract MixinERC721Enumerable is ) public view returns (uint256) { - require(_index < _totalSupply, 'OUT_OF_RANGE'); + if(_index >= _totalSupply) { + revert OUT_OF_RANGE(); + } return _index; } diff --git a/smart-contracts/contracts/mixins/MixinErrors.sol b/smart-contracts/contracts/mixins/MixinErrors.sol new file mode 100644 index 00000000000..aea2842b464 --- /dev/null +++ b/smart-contracts/contracts/mixins/MixinErrors.sol @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +/** + * @title List of all error messages + * (replace string errors message to save on contract size) + */ +contract MixinErrors { + + // generic + error OUT_OF_RANGE(); + error NULL_VALUE(); + error INVALID_ADDRESS(); + error INVALID_TOKEN(); + error INVALID_LENGTH(); + error UNAUTHORIZED(); + + // erc 721 + error NON_COMPLIANT_ERC721_RECEIVER(); + + // roles + error ONLY_LOCK_MANAGER_OR_KEY_GRANTER(); + error ONLY_KEY_MANAGER_OR_APPROVED(); + error UNAUTHORIZED_KEY_MANAGER_UPDATE(); + error ONLY_LOCK_MANAGER_OR_BENEFICIARY(); + error ONLY_LOCK_MANAGER(); + + // single key status + error KEY_NOT_VALID(); + error NO_SUCH_KEY(); + + // single key operations + error CANT_EXTEND_NON_EXPIRING_KEY(); + error NOT_ENOUGH_TIME(); + error NOT_ENOUGH_FUNDS(); + + // migration & data schema + error SCHEMA_VERSION_NOT_CORRECT(); + error MIGRATION_REQUIRED(); + + // lock status/settings + error OWNER_CANT_BE_ADDRESS_ZERO(); + error MAX_KEYS_REACHED(); + error KEY_TRANSFERS_DISABLED(); + error CANT_BE_SMALLER_THAN_SUPPLY(); + + // transfers and approvals + error TRANSFER_TO_SELF(); + error CANNOT_APPROVE_SELF(); + + // keys management + error LOCK_SOLD_OUT(); + + // purchase + error INSUFFICIENT_ERC20_VALUE(); + error INSUFFICIENT_VALUE(); + + // renewals + error NON_RENEWABLE_LOCK(); + error LOCK_HAS_CHANGED(); + error NOT_READY_FOR_RENEWAL(); + + // gas refund + error GAS_REFUND_FAILED(); + + // hooks + // NB: `hookIndex` designed the index of hook address in the params of `setEventHooks` + error INVALID_HOOK(uint8 hookIndex); + +} diff --git a/smart-contracts/contracts/mixins/MixinFunds.sol b/smart-contracts/contracts/mixins/MixinFunds.sol index 57c87970d11..7b8fc92ad8f 100644 --- a/smart-contracts/contracts/mixins/MixinFunds.sol +++ b/smart-contracts/contracts/mixins/MixinFunds.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; +import './MixinErrors.sol'; import '@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol'; import '@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol'; import '@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol'; @@ -9,7 +10,7 @@ import '@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeab * @title An implementation of the money related functions. * @author HardlyDifficult (unlock-protocol.com) */ -contract MixinFunds +contract MixinFunds is MixinErrors { using AddressUpgradeable for address payable; using SafeERC20Upgradeable for IERC20Upgradeable; @@ -34,10 +35,13 @@ contract MixinFunds internal view { - require( - _tokenAddress == address(0) || IERC20Upgradeable(_tokenAddress).totalSupply() > 0, - 'INVALID_TOKEN' - ); + if( + _tokenAddress != address(0) + && + IERC20Upgradeable(_tokenAddress).totalSupply() < 0 + ) { + revert INVALID_TOKEN(); + } } /** diff --git a/smart-contracts/contracts/mixins/MixinGrantKeys.sol b/smart-contracts/contracts/mixins/MixinGrantKeys.sol index c471270f5ce..a8105fc9c02 100644 --- a/smart-contracts/contracts/mixins/MixinGrantKeys.sol +++ b/smart-contracts/contracts/mixins/MixinGrantKeys.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.0; import './MixinKeys.sol'; import './MixinRoles.sol'; +import './MixinErrors.sol'; /** @@ -12,6 +13,7 @@ import './MixinRoles.sol'; * separates logically groupings of code to ease readability. */ contract MixinGrantKeys is + MixinErrors, MixinRoles, MixinKeys { @@ -25,10 +27,14 @@ contract MixinGrantKeys is address[] calldata _keyManagers ) external { _lockIsUpToDate(); - require(isKeyGranter(msg.sender) || isLockManager(msg.sender), 'ONLY_LOCK_MANAGER_OR_KEY_GRANTER'); + if(!isKeyGranter(msg.sender) && !isLockManager(msg.sender)) { + revert ONLY_LOCK_MANAGER_OR_KEY_GRANTER(); + } for(uint i = 0; i < _recipients.length; i++) { - require(_recipients[i] != address(0), 'INVALID_ADDRESS'); + if(_recipients[i] == address(0)) { + revert INVALID_ADDRESS(); + } // an event is triggered _createNewKey( diff --git a/smart-contracts/contracts/mixins/MixinKeys.sol b/smart-contracts/contracts/mixins/MixinKeys.sol index cbd5e72e681..7bc278b49be 100644 --- a/smart-contracts/contracts/mixins/MixinKeys.sol +++ b/smart-contracts/contracts/mixins/MixinKeys.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.0; import './MixinLockCore.sol'; +import './MixinErrors.sol'; /** * @title Mixin for managing `Key` data, as well as the * Approval related functions needed to meet the ERC721 @@ -11,6 +12,7 @@ import './MixinLockCore.sol'; * separates logically groupings of code to ease readability. */ contract MixinKeys is + MixinErrors, MixinLockCore { // The struct for a key @@ -96,12 +98,13 @@ contract MixinKeys is internal view { - require( - _isKeyManager(_tokenId, msg.sender) || - approved[_tokenId] == msg.sender || - isApprovedForAll(_ownerOf[_tokenId], msg.sender), - 'ONLY_KEY_MANAGER_OR_APPROVED' - ); + if( + !_isKeyManager(_tokenId, msg.sender) + && approved[_tokenId] != msg.sender + && !isApprovedForAll(_ownerOf[_tokenId], msg.sender) + ) { + revert ONLY_KEY_MANAGER_OR_APPROVED(); + } } /** @@ -114,10 +117,10 @@ contract MixinKeys is internal view { - require( - isValidKey(_tokenId), - 'KEY_NOT_VALID' - ); + if(!isValidKey(_tokenId)) { + revert KEY_NOT_VALID(); + } + } /** @@ -130,9 +133,9 @@ contract MixinKeys is internal view { - require( - _keys[_tokenId].expirationTimestamp != 0, 'NO_SUCH_KEY' - ); + if(_keys[_tokenId].expirationTimestamp == 0) { + revert NO_SUCH_KEY(); + } } /** @@ -187,7 +190,9 @@ contract MixinKeys is view returns (uint256) { - require(_index < balanceOf(_keyOwner), "OWNER_INDEX_OUT_OF_BOUNDS"); + if(_index >= balanceOf(_keyOwner)) { + revert OUT_OF_RANGE(); + } return _ownedKeyIds[_keyOwner][_index]; } @@ -239,7 +244,9 @@ contract MixinKeys is uint expirationTimestamp = _keys[_tokenId].expirationTimestamp; // prevent extending a valid non-expiring key - require(expirationTimestamp != type(uint).max, 'CANT_EXTEND_NON_EXPIRING_KEY'); + if(expirationTimestamp == type(uint).max){ + revert CANT_EXTEND_NON_EXPIRING_KEY(); + } // if non-expiring but not valid then extend if(expirationDuration == type(uint).max) { @@ -271,7 +278,9 @@ contract MixinKeys is uint length = balanceOf(_recipient); // make sure address does not have more keys than allowed - require(length < _maxKeysPerAddress, 'MAX_KEYS'); + if(length >= _maxKeysPerAddress) { + revert MAX_KEYS_REACHED(); + } // record new owner _ownedKeysIndex[_tokenId] = length; @@ -301,7 +310,11 @@ contract MixinKeys is _isKey(_tokenIdTo); // make sure there is enough time remaining - require(keyExpirationTimestampFor(_tokenIdFrom) - block.timestamp >= _amount, 'NOT_ENOUGH_TIME'); + if( + _amount > keyExpirationTimestampFor(_tokenIdFrom) - block.timestamp + ) { + revert NOT_ENOUGH_TIME(); + } // deduct time from parent key _timeMachine(_tokenIdFrom, _amount, false); @@ -373,7 +386,9 @@ contract MixinKeys is view returns (uint) { - require(_keyOwner != address(0), 'INVALID_ADDRESS'); + if(_keyOwner == address(0)) { + revert INVALID_ADDRESS(); + } return _balances[_keyOwner]; } @@ -463,11 +478,12 @@ contract MixinKeys is ) public { _isKey(_tokenId); - require( - _isKeyManager(_tokenId, msg.sender) || - isLockManager(msg.sender), - 'UNAUTHORIZED_KEY_MANAGER_UPDATE' - ); + if( + !_isKeyManager(_tokenId, msg.sender) + && !isLockManager(msg.sender) + ) { + revert UNAUTHORIZED_KEY_MANAGER_UPDATE(); + } _setKeyManagerOf(_tokenId, _keyManager); } @@ -495,7 +511,9 @@ contract MixinKeys is public { _onlyKeyManagerOrApproved(_tokenId); - require(msg.sender != _approved, 'APPROVE_SELF'); + if(msg.sender == _approved) { + revert CANNOT_APPROVE_SELF(); + } approved[_tokenId] = _approved; emit Approval(_ownerOf[_tokenId], _approved, _tokenId); @@ -605,7 +623,9 @@ contract MixinKeys is */ function setMaxNumberOfKeys (uint _maxNumberOfKeys) external { _onlyLockManager(); - require (_maxNumberOfKeys >= _totalSupply, "SMALLER_THAN_SUPPLY"); + if (_maxNumberOfKeys < _totalSupply) { + revert CANT_BE_SMALLER_THAN_SUPPLY(); + } maxNumberOfKeys = _maxNumberOfKeys; } @@ -627,7 +647,9 @@ contract MixinKeys is */ function setMaxKeysPerAddress(uint _maxKeys) external { _onlyLockManager(); - require(_maxKeys != 0, 'NULL_VALUE'); + if(_maxKeys == 0) { + revert NULL_VALUE(); + } _maxKeysPerAddress = _maxKeys; } diff --git a/smart-contracts/contracts/mixins/MixinLockCore.sol b/smart-contracts/contracts/mixins/MixinLockCore.sol index 0eabde2c9c1..16b7364eefe 100644 --- a/smart-contracts/contracts/mixins/MixinLockCore.sol +++ b/smart-contracts/contracts/mixins/MixinLockCore.sol @@ -5,6 +5,7 @@ pragma solidity ^0.8.0; import '@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol'; import './MixinDisable.sol'; import './MixinRoles.sol'; +import './MixinErrors.sol'; import '../interfaces/IUnlock.sol'; import './MixinFunds.sol'; import '../interfaces/hooks/ILockKeyCancelHook.sol'; @@ -91,10 +92,9 @@ contract MixinLockCore is // modifier to check if data has been upgraded function _lockIsUpToDate() internal view { - require( - schemaVersion == publicLockVersion(), - 'MIGRATION_REQUIRED' - ); + if(schemaVersion != publicLockVersion()) { + revert MIGRATION_REQUIRED(); + } } // modifier @@ -102,10 +102,9 @@ contract MixinLockCore is internal view { - require( - isLockManager(msg.sender) || msg.sender == beneficiary, - 'ONLY_LOCK_MANAGER_OR_BENEFICIARY' - ); + if(!isLockManager(msg.sender) && msg.sender != beneficiary) { + revert ONLY_LOCK_MANAGER_OR_BENEFICIARY(); + } } function _initializeMixinLockCore( @@ -161,7 +160,9 @@ contract MixinLockCore is uint amount; if(_amount == 0 || _amount > balance) { - require(balance > 0, 'NOT_ENOUGH_FUNDS'); + if(balance <= 0) { + revert NOT_ENOUGH_FUNDS(); + } amount = balance; } else @@ -202,7 +203,9 @@ contract MixinLockCore is address payable _beneficiary ) external { _onlyLockManagerOrBeneficiary(); - require(_beneficiary != address(0), 'INVALID_ADDRESS'); + if(_beneficiary == address(0)) { + revert INVALID_ADDRESS(); + } beneficiary = _beneficiary; } @@ -217,10 +220,12 @@ contract MixinLockCore is ) external { _onlyLockManager(); - require(_onKeyPurchaseHook == address(0) || _onKeyPurchaseHook.isContract(), 'INVALID_ON_KEY_SOLD_HOOK'); - require(_onKeyCancelHook == address(0) || _onKeyCancelHook.isContract(), 'INVALID_ON_KEY_CANCEL_HOOK'); - require(_onValidKeyHook == address(0) || _onValidKeyHook.isContract(), 'INVALID_ON_VALID_KEY_HOOK'); - require(_onTokenURIHook == address(0) || _onTokenURIHook.isContract(), 'INVALID_ON_TOKEN_URI_HOOK'); + + if(_onKeyPurchaseHook != address(0) && !_onKeyPurchaseHook.isContract()) { revert INVALID_HOOK(0); } + if(_onKeyCancelHook != address(0) && !_onKeyCancelHook.isContract()) { revert INVALID_HOOK(1); } + if(_onValidKeyHook != address(0) && !_onValidKeyHook.isContract()) { revert INVALID_HOOK(2); } + if(_onTokenURIHook != address(0) && !_onTokenURIHook.isContract()) { revert INVALID_HOOK(3); } + onKeyPurchaseHook = ILockKeyPurchaseHook(_onKeyPurchaseHook); onKeyCancelHook = ILockKeyCancelHook(_onKeyCancelHook); onTokenURIHook = ILockTokenURIHook(_onTokenURIHook); diff --git a/smart-contracts/contracts/mixins/MixinPurchase.sol b/smart-contracts/contracts/mixins/MixinPurchase.sol index c611ab50d9b..c774b0c6790 100644 --- a/smart-contracts/contracts/mixins/MixinPurchase.sol +++ b/smart-contracts/contracts/mixins/MixinPurchase.sol @@ -5,6 +5,7 @@ import './MixinDisable.sol'; import './MixinKeys.sol'; import './MixinLockCore.sol'; import './MixinFunds.sol'; +import './MixinErrors.sol'; /** * @title Mixin for the purchase-related functions. @@ -13,6 +14,7 @@ import './MixinFunds.sol'; * separates logically groupings of code to ease readability. */ contract MixinPurchase is + MixinErrors, MixinFunds, MixinDisable, MixinLockCore, @@ -92,9 +94,16 @@ contract MixinPurchase is ) external payable { _lockIsUpToDate(); - require(maxNumberOfKeys > _totalSupply, 'LOCK_SOLD_OUT'); - require(_recipients.length == _referrers.length, 'INVALID_REFERRERS_LENGTH'); - require(_recipients.length == _keyManagers.length, 'INVALID_KEY_MANAGERS_LENGTH'); + if(_totalSupply >= maxNumberOfKeys) { + revert LOCK_SOLD_OUT(); + } + if( + (_recipients.length != _referrers.length) + || + (_recipients.length != _keyManagers.length) + ) { + revert INVALID_LENGTH(); + } uint totalPriceToPay; uint tokenId; @@ -102,7 +111,9 @@ contract MixinPurchase is for (uint256 i = 0; i < _recipients.length; i++) { // check recipient address address _recipient = _recipients[i]; - require(_recipient != address(0), 'INVALID_ADDRESS'); + if(_recipient == address(0)) { + revert INVALID_ADDRESS(); + } // check for a non-expiring key if (expirationDuration == type(uint).max) { @@ -120,8 +131,7 @@ contract MixinPurchase is ); } - // price - + // price uint inMemoryKeyPrice = purchasePriceFor(_recipient, _referrers[i], _data[i]); totalPriceToPay = totalPriceToPay + inMemoryKeyPrice; @@ -130,8 +140,8 @@ contract MixinPurchase is _originalDurations[tokenId] = expirationDuration; _originalTokens[tokenId] = tokenAddress; - if(tokenAddress != address(0)) { - require(inMemoryKeyPrice <= _values[i], 'INSUFFICIENT_ERC20_VALUE'); + if(tokenAddress != address(0) && _values[i] < inMemoryKeyPrice) { + revert INSUFFICIENT_ERC20_VALUE(); } // store in unlock @@ -150,14 +160,13 @@ contract MixinPurchase is ); } } - // transfer the ERC20 tokens if(tokenAddress != address(0)) { IERC20Upgradeable token = IERC20Upgradeable(tokenAddress); token.transferFrom(msg.sender, address(this), totalPriceToPay); - } else { + } else if(msg.value < totalPriceToPay) { // We explicitly allow for greater amounts of ETH or tokens to allow 'donations' - require(totalPriceToPay <= msg.value, 'INSUFFICIENT_VALUE'); + revert INSUFFICIENT_VALUE(); } // refund gas @@ -192,12 +201,14 @@ contract MixinPurchase is uint inMemoryKeyPrice = purchasePriceFor(ownerOf(_tokenId), _referrer, _data); if(tokenAddress != address(0)) { - require(inMemoryKeyPrice <= _value, 'INSUFFICIENT_ERC20_VALUE'); + if(_value < inMemoryKeyPrice) { + revert INSUFFICIENT_ERC20_VALUE(); + } IERC20Upgradeable token = IERC20Upgradeable(tokenAddress); token.transferFrom(msg.sender, address(this), inMemoryKeyPrice); - } else { + } else if(msg.value < inMemoryKeyPrice) { // We explicitly allow for greater amounts of ETH or tokens to allow 'donations' - require(inMemoryKeyPrice <= msg.value, 'INSUFFICIENT_VALUE'); + revert INSUFFICIENT_VALUE(); } // refund gas (if applicable) @@ -218,17 +229,26 @@ contract MixinPurchase is _isKey(_tokenId); // check the lock - require(_originalDurations[_tokenId] != type(uint).max, 'NON_EXPIRING_LOCK'); - require(tokenAddress != address(0), 'NON_ERC20_LOCK'); + if(_originalDurations[_tokenId] == type(uint).max || tokenAddress == address(0)) { + revert NON_RENEWABLE_LOCK(); + } // make sure duration and pricing havent changed uint keyPrice = purchasePriceFor(ownerOf(_tokenId), _referrer, ''); - require(_originalPrices[_tokenId] == keyPrice, 'PRICE_CHANGED'); - require(_originalDurations[_tokenId] == expirationDuration, 'DURATION_CHANGED'); - require(_originalTokens[_tokenId] == tokenAddress, 'TOKEN_CHANGED'); + if( + _originalPrices[_tokenId] != keyPrice + || + _originalDurations[_tokenId] != expirationDuration + || + _originalTokens[_tokenId] != tokenAddress + ) { + revert LOCK_HAS_CHANGED(); + } // make sure key is ready for renewal - require(isValidKey(_tokenId) == false, 'NOT_READY'); + if(isValidKey(_tokenId)) { + revert NOT_READY_FOR_RENEWAL(); + } // extend key duration _extendKey(_tokenId); @@ -276,7 +296,9 @@ contract MixinPurchase is token.transferFrom(address(this), msg.sender, _gasRefundValue); } else { (bool success, ) = msg.sender.call{value: _gasRefundValue}(""); - require(success, "REFUND_FAILED"); + if(!success) { + revert GAS_REFUND_FAILED(); + } } emit GasRefunded(msg.sender, _gasRefundValue, tokenAddress); } diff --git a/smart-contracts/contracts/mixins/MixinRoles.sol b/smart-contracts/contracts/mixins/MixinRoles.sol index 6648899a998..127e33a58aa 100644 --- a/smart-contracts/contracts/mixins/MixinRoles.sol +++ b/smart-contracts/contracts/mixins/MixinRoles.sol @@ -5,8 +5,9 @@ pragma solidity ^0.8.0; // openzeppelin/contracts-ethereum-package/contracts/access/roles import '@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol'; +import './MixinErrors.sol'; -contract MixinRoles is AccessControlUpgradeable { +contract MixinRoles is AccessControlUpgradeable, MixinErrors { // roles bytes32 public constant LOCK_MANAGER_ROLE = keccak256("LOCK_MANAGER"); @@ -41,7 +42,9 @@ contract MixinRoles is AccessControlUpgradeable { internal view { - require( hasRole(LOCK_MANAGER_ROLE, msg.sender), 'ONLY_LOCK_MANAGER'); + if(!hasRole(LOCK_MANAGER_ROLE, msg.sender)) { + revert ONLY_LOCK_MANAGER(); + } } // lock manager functions diff --git a/smart-contracts/contracts/mixins/MixinTransfer.sol b/smart-contracts/contracts/mixins/MixinTransfer.sol index 63b570875f1..69f44752d2d 100644 --- a/smart-contracts/contracts/mixins/MixinTransfer.sol +++ b/smart-contracts/contracts/mixins/MixinTransfer.sol @@ -7,6 +7,7 @@ import './MixinKeys.sol'; import './MixinFunds.sol'; import './MixinLockCore.sol'; import './MixinPurchase.sol'; +import './MixinErrors.sol'; import '@openzeppelin/contracts-upgradeable/token/ERC721/IERC721ReceiverUpgradeable.sol'; import '@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol'; @@ -19,6 +20,7 @@ import '@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol'; */ contract MixinTransfer is + MixinErrors, MixinRoles, MixinFunds, MixinLockCore, @@ -53,13 +55,21 @@ contract MixinTransfer is ) public { _lockIsUpToDate(); - require(maxNumberOfKeys > _totalSupply, 'LOCK_SOLD_OUT'); + if(maxNumberOfKeys <= _totalSupply) { + revert LOCK_SOLD_OUT(); + } _onlyKeyManagerOrApproved(_tokenIdFrom); _isValidKey(_tokenIdFrom); - require(transferFeeBasisPoints < BASIS_POINTS_DEN, 'KEY_TRANSFERS_DISABLED'); - require(_to != address(0), 'INVALID_ADDRESS'); + if(transferFeeBasisPoints >= BASIS_POINTS_DEN) { + revert KEY_TRANSFERS_DISABLED(); + } + if(_to == address(0)) { + revert INVALID_ADDRESS(); + } address keyOwner = _ownerOf[_tokenIdFrom]; - require(keyOwner != _to, 'TRANSFER_TO_SELF'); + if(keyOwner == _to) { + revert TRANSFER_TO_SELF(); + } // store time to be added uint time; @@ -99,7 +109,9 @@ contract MixinTransfer is tokenIdTo ); - require(_checkOnERC721Received(keyOwner, _to, tokenIdTo, ''), 'NON_COMPLIANT_ERC721_RECEIVER'); + if(!_checkOnERC721Received(keyOwner, _to, tokenIdTo, '')) { + revert NON_COMPLIANT_ERC721_RECEIVER(); + } } @@ -112,10 +124,18 @@ contract MixinTransfer is { _isValidKey(_tokenId); _onlyKeyManagerOrApproved(_tokenId); - require(ownerOf(_tokenId) == _from, 'TRANSFER_FROM: NOT_KEY_OWNER'); - require(transferFeeBasisPoints < BASIS_POINTS_DEN, 'KEY_TRANSFERS_DISABLED'); - require(_recipient != address(0), 'INVALID_ADDRESS'); - require(_from != _recipient, 'TRANSFER_TO_SELF'); + if(ownerOf(_tokenId) != _from) { + revert UNAUTHORIZED(); + } + if(transferFeeBasisPoints >= BASIS_POINTS_DEN) { + revert KEY_TRANSFERS_DISABLED(); + } + if(_recipient == address(0)) { + revert INVALID_ADDRESS(); + } + if(_from == _recipient) { + revert TRANSFER_TO_SELF(); + } // subtract the fee from the senders key before the transfer _timeMachine(_tokenId, getTransferFee(_tokenId, 0), false); @@ -203,8 +223,12 @@ contract MixinTransfer is bool _approved ) public { - require(_to != msg.sender, 'APPROVE_SELF'); - require(transferFeeBasisPoints < BASIS_POINTS_DEN, 'KEY_TRANSFERS_DISABLED'); + if(_to == msg.sender) { + revert CANNOT_APPROVE_SELF(); + } + if(transferFeeBasisPoints >= BASIS_POINTS_DEN) { + revert KEY_TRANSFERS_DISABLED(); + } managerToOperatorApproved[msg.sender][_to] = _approved; emit ApprovalForAll(msg.sender, _to, _approved); } @@ -229,8 +253,9 @@ contract MixinTransfer is public { transferFrom(_from, _to, _tokenId); - require(_checkOnERC721Received(_from, _to, _tokenId, _data), 'NON_COMPLIANT_ERC721_RECEIVER'); - + if(!_checkOnERC721Received(_from, _to, _tokenId, _data)) { + revert NON_COMPLIANT_ERC721_RECEIVER(); + } } /** diff --git a/smart-contracts/test/Lock/erc721/transferFrom.js b/smart-contracts/test/Lock/erc721/transferFrom.js index ba253e8069e..3bf9d62ab47 100644 --- a/smart-contracts/test/Lock/erc721/transferFrom.js +++ b/smart-contracts/test/Lock/erc721/transferFrom.js @@ -74,7 +74,7 @@ contract('Lock / erc721 / transferFrom', (accounts) => { locks.FIRST.transferFrom(keyOwners[2], accounts[9], tokenIds[0], { from: keyOwners[0], }), - 'TRANSFER_FROM: NOT_KEY_OWNER' + 'UNAUTHORIZED' ) }) diff --git a/smart-contracts/test/Lock/onKeyCancelHook.js b/smart-contracts/test/Lock/onKeyCancelHook.js index a8cd3fc4c19..a16340c418f 100644 --- a/smart-contracts/test/Lock/onKeyCancelHook.js +++ b/smart-contracts/test/Lock/onKeyCancelHook.js @@ -56,7 +56,7 @@ contract('Lock / onKeyCancelHook', (accounts) => { it('cannot set the hook to a non-contract address', async () => { await reverts( lock.setEventHooks(ADDRESS_ZERO, accounts[1], ADDRESS_ZERO, ADDRESS_ZERO), - 'INVALID_ON_KEY_CANCEL_HOOK' + 'INVALID_HOOK(1)' ) }) }) diff --git a/smart-contracts/test/Lock/onKeyPurchaseHook.js b/smart-contracts/test/Lock/onKeyPurchaseHook.js index e7547972f52..9915da7f9c1 100644 --- a/smart-contracts/test/Lock/onKeyPurchaseHook.js +++ b/smart-contracts/test/Lock/onKeyPurchaseHook.js @@ -88,7 +88,7 @@ contract('Lock / onKeyPurchaseHook', (accounts) => { ADDRESS_ZERO, ADDRESS_ZERO ), - 'INVALID_ON_KEY_SOLD_HOOK' + 'INVALID_HOOK(0)' ) }) }) diff --git a/smart-contracts/test/Lock/onTokenURIHook.js b/smart-contracts/test/Lock/onTokenURIHook.js index 934f80a1d5a..10dd8cc8777 100644 --- a/smart-contracts/test/Lock/onTokenURIHook.js +++ b/smart-contracts/test/Lock/onTokenURIHook.js @@ -61,7 +61,7 @@ contract('Lock / onTokenURIHook', (accounts) => { it('cannot set the hook to a non-contract address', async () => { await reverts( lock.setEventHooks(ADDRESS_ZERO, ADDRESS_ZERO, ADDRESS_ZERO, accounts[3]), - 'INVALID_ON_TOKEN_URI_HOOK' + 'INVALID_HOOK(3)' ) }) }) diff --git a/smart-contracts/test/Lock/onValidKeyHook.js b/smart-contracts/test/Lock/onValidKeyHook.js index 3581f4efb91..53a190290d9 100644 --- a/smart-contracts/test/Lock/onValidKeyHook.js +++ b/smart-contracts/test/Lock/onValidKeyHook.js @@ -62,7 +62,7 @@ contract('Lock / onValidKeyHook', (accounts) => { it('cannot set the hook to a non-contract address', async () => { await reverts( lock.setEventHooks(ADDRESS_ZERO, ADDRESS_ZERO, accounts[3], ADDRESS_ZERO), - 'INVALID_ON_VALID_KEY_HOOK' + 'INVALID_HOOK(2)' ) }) }) diff --git a/smart-contracts/test/Lock/renewMembershipFor.js b/smart-contracts/test/Lock/renewMembershipFor.js index d5fe5246936..4bb7e3e4f78 100644 --- a/smart-contracts/test/Lock/renewMembershipFor.js +++ b/smart-contracts/test/Lock/renewMembershipFor.js @@ -81,7 +81,7 @@ contract('Lock / Recurring memberships', (accounts) => { await reverts( locks.NON_EXPIRING.renewMembershipFor(newTokenId, ADDRESS_ZERO), - 'NON_EXPIRING_LOCK' + 'NON_RENEWABLE_LOCK' ) }) @@ -103,7 +103,7 @@ contract('Lock / Recurring memberships', (accounts) => { await reverts( locks.FIRST.renewMembershipFor(newTokenId, ADDRESS_ZERO), - 'NON_ERC20_LOCK' + 'NON_RENEWABLE_LOCK' ) }) }) @@ -169,7 +169,7 @@ contract('Lock / Recurring memberships', (accounts) => { ) await reverts( lock.renewMembershipFor(tokenId, ADDRESS_ZERO), - 'PRICE_CHANGED' + 'LOCK_HAS_CHANGED' ) }) @@ -183,7 +183,7 @@ contract('Lock / Recurring memberships', (accounts) => { await lock.updateKeyPricing(keyPrice, dai2.address, { from: lockOwner }) await reverts( lock.renewMembershipFor(tokenId, ADDRESS_ZERO), - 'TOKEN_CHANGED' + 'LOCK_HAS_CHANGED' ) }) @@ -191,7 +191,7 @@ contract('Lock / Recurring memberships', (accounts) => { await lock.setExpirationDuration(1000, { from: lockOwner }) await reverts( lock.renewMembershipFor(tokenId, ADDRESS_ZERO), - 'DURATION_CHANGED' + 'LOCK_HAS_CHANGED' ) }) }) @@ -285,7 +285,7 @@ contract('Lock / Recurring memberships', (accounts) => { // should fail await reverts( lock.renewMembershipFor(tokenId, ADDRESS_ZERO), - 'DURATION_CHANGED' + 'LOCK_HAS_CHANGED' ) }) @@ -322,7 +322,7 @@ contract('Lock / Recurring memberships', (accounts) => { await reverts( lock.renewMembershipFor(tokenId, ADDRESS_ZERO), - 'DURATION_CHANGED' + 'LOCK_HAS_CHANGED' ) }) }) diff --git a/smart-contracts/test/Lock/timeMachine.js b/smart-contracts/test/Lock/timeMachine.js index df45da60061..fc5fb246162 100644 --- a/smart-contracts/test/Lock/timeMachine.js +++ b/smart-contracts/test/Lock/timeMachine.js @@ -3,11 +3,9 @@ const BigNumber = require('bignumber.js') const TimeMachineMock = artifacts.require('TimeMachineMock') -const { errorMessages, ADDRESS_ZERO } = require('../helpers/constants') +const { ADDRESS_ZERO } = require('../helpers/constants') const { reverts } = require('../helpers/errors') -const { VM_ERROR_REVERT_WITH_REASON } = errorMessages - contract('Lock / timeMachine', (accounts) => { let timeMachine const keyOwner = accounts[2] @@ -92,7 +90,7 @@ contract('Lock / timeMachine', (accounts) => { it('should not work for a non-existant key', async () => { await reverts( timeMachine.timeMachine(17, 42, true, { from: accounts[3] }), - `${VM_ERROR_REVERT_WITH_REASON} 'NO_SUCH_KEY'` + 'NO_SUCH_KEY' ) }) }) diff --git a/smart-contracts/test/Lock/transfer.js b/smart-contracts/test/Lock/transfer.js index 552676a78ef..9044cc08065 100644 --- a/smart-contracts/test/Lock/transfer.js +++ b/smart-contracts/test/Lock/transfer.js @@ -4,9 +4,7 @@ const deployLocks = require('../helpers/deployLocks') const unlockContract = artifacts.require('Unlock.sol') const getProxy = require('../helpers/proxy') -const { errorMessages, ADDRESS_ZERO } = require('../helpers/constants') - -const { VM_ERROR_REVERT_WITH_REASON } = errorMessages +const { ADDRESS_ZERO } = require('../helpers/constants') let unlock let lock @@ -115,7 +113,7 @@ contract('Lock / transfer', (accounts) => { lock.transfer(tokenIds[0], singleKeyOwner, 1000, { from: singleKeyOwner, }), - `${VM_ERROR_REVERT_WITH_REASON} 'TRANSFER_TO_SELF'` + 'TRANSFER_TO_SELF' ) })