diff --git a/contracts/ECRecovery.sol b/contracts/ECRecovery.sol index aacc7df0cfb..6946f3d16b3 100644 --- a/contracts/ECRecovery.sol +++ b/contracts/ECRecovery.sol @@ -1,5 +1,8 @@ pragma solidity ^0.4.24; +import "./signatures/ISignatureDelegate.sol"; +import "./introspection/ERC165Checker.sol"; + /** * @title Elliptic curve signature operations @@ -7,15 +10,18 @@ pragma solidity ^0.4.24; * TODO Remove this library once solidity supports passing a signature to ecrecover. * See https://github.com/ethereum/solidity/issues/864 */ - library ECRecovery { + using ERC165Checker for address; + + // @TODO - de-dup this constant once we can share constants in solidity without a hack + bytes4 internal constant InterfaceId_SignatureDelegate = 0x1626ba7e; /** * @dev Recover signer address from a message by using their signature - * @param hash bytes32 message, the hash is the signed message. What is recovered is the signer address. - * @param sig bytes signature, the signature is generated using web3.eth.sign() + * @param _hash bytes32 message, the hash is the signed message. What is recovered is the signer address. + * @param _sig bytes signature, the signature is generated using web3.eth.sign() */ - function recover(bytes32 hash, bytes sig) + function recover(bytes32 _hash, bytes _sig) internal pure returns (address) @@ -25,7 +31,7 @@ library ECRecovery { uint8 v; // Check the signature length - if (sig.length != 65) { + if (_sig.length != 65) { return (address(0)); } @@ -34,9 +40,9 @@ library ECRecovery { // currently is to use assembly. // solium-disable-next-line security/no-inline-assembly assembly { - r := mload(add(sig, 32)) - s := mload(add(sig, 64)) - v := byte(0, mload(add(sig, 96))) + r := mload(add(_sig, 32)) + s := mload(add(_sig, 64)) + v := byte(0, mload(add(_sig, 96))) } // Version of signature should be 27 or 28, but 0 and 1 are also possible versions @@ -49,16 +55,16 @@ library ECRecovery { return (address(0)); } else { // solium-disable-next-line arg-overflow - return ecrecover(hash, v, r, s); + return ecrecover(_hash, v, r, s); } } /** * toEthSignedMessageHash - * @dev prefix a bytes32 value with "\x19Ethereum Signed Message:" - * and hash the result + * @dev prefix a bytes32 value with "\x19Ethereum Signed Message:\n32" and hash the result + * @param _hash the original hash */ - function toEthSignedMessageHash(bytes32 hash) + function toEthSignedMessageHash(bytes32 _hash) internal pure returns (bytes32) @@ -66,7 +72,21 @@ library ECRecovery { // 32 is the length in bytes of hash, // enforced by the type signature above return keccak256( - abi.encodePacked("\x19Ethereum Signed Message:\n32", hash) + abi.encodePacked("\x19Ethereum Signed Message:\n32", _hash) ); } + + function isSignedBy(bytes32 _hash, address _delegate, bytes _sig) + internal + view + returns (bool) + { + // if the delegate address supports SignatureDelegation, delegate + if (_delegate.supportsInterface(InterfaceId_SignatureDelegate)) { + return ISignatureDelegate(_delegate).isValidSignature(_hash, _sig); + } + + // otherwise make sure the hash was personally signed by the EOA account + return _delegate == recover(toEthSignedMessageHash(_hash), _sig); + } } diff --git a/contracts/access/Bouncer.sol b/contracts/access/Bouncer.sol new file mode 100644 index 00000000000..2297e26e904 --- /dev/null +++ b/contracts/access/Bouncer.sol @@ -0,0 +1,176 @@ +pragma solidity ^0.4.24; + +import "../ownership/rbac/RBACOwnable.sol"; +import "../ECRecovery.sol"; +import "./BouncerUtils.sol"; +import "../signatures/SignatureDelegate.sol"; + + +/** + * @title Bouncer + * @author PhABC, Shrugs, and aflesher + * @dev Bouncer allows users to submit a `ticket` from a `delegate` as a permission to do an action. + * By default a `ticket` is a cryptographic signature + * generated via Ethereum ECDSA signing (web3.eth.personal_sign). + * See //test/helpers/sign.js for example ticket construction. + * + * If the ticket is from one of the authorized delegates, the ticket is valid. + The owner of the contract adds/removes delegates. + * + * Delegates can be individual servers signing grants or different + * users within a decentralized club that have permission to invite other members. + * + * Delegates can also be other contracts that implement `isValidSignature`, allowing you to write + * whatever access-control logic you want, like using alternative signature schemes or + * contract-as-identity patterns. + * + * This Bouncer technique is useful for whitelists and airdrops; instead of putting all + * valid addresses on-chain, simply sign a grant of the form + * keccak256(abi.encodePacked(`:delegate` + `:sender`)) using a valid signer. + * + * Then restrict access to your crowdsale/whitelist/airdrop using one of the only* modifiers, + * which allows users claiming your tokens to pay their own gas. + * + * @notice A method that uses the `onlyValidTicketForData` modifier must make the _sig + * parameter the "last" parameter. You cannot sign a message that has its own + * signature in it so the last 96 bytes of msg.data (which represents the _sig data itself) + * is ignored when constructing the data hash that is validated. + */ +contract Bouncer is RBACOwnable, SignatureDelegate { + using ECRecovery for bytes32; + using BouncerUtils for bytes32; + + string public constant ROLE_DELEGATE = "delegate"; + + /** + * @dev requires that a valid signature of a bouncer was provided + * @notice does not validate method arguments + */ + modifier onlyValidTicket(address _delegate, bytes _sig) + { + require(isValidTicket(_delegate, _sig)); + _; + } + + /** + * @dev requires that a valid signature with a specifed method of a bouncer was provided + * @notice validates methodId, but not method arguments + */ + modifier onlyValidTicketForMethod(address _delegate, bytes _sig) + { + require(isValidTicketAndMethod(_delegate, _sig)); + _; + } + + /** + * @dev requires that a valid signature with a specifed method and params of a bouncer was provided + * @notice verifies entire calldata, sans final signature + */ + modifier onlyValidTicketForData(address _delegate, bytes _sig) + { + require(isValidTicketAndData(_delegate, _sig)); + _; + } + + + constructor() + public + { + // this contract implements ISignatureDelegate for all of its EOA delegate accounts. + addRole(address(this), ROLE_DELEGATE); + } + + /** + * @dev allows the owner to add additional signer addresses + */ + function addDelegate(address _delegate) + onlyOwners + public + { + require(_delegate != address(0)); + addRole(_delegate, ROLE_DELEGATE); + } + + /** + * @dev allows the owner to remove signer addresses + */ + function removeDelegate(address _delegate) + onlyOwners + public + { + require(_delegate != address(0)); + removeRole(_delegate, ROLE_DELEGATE); + } + + /** + * @dev is the signature of `delegate + sender` from a bouncer? + * @return bool + */ + function isValidTicket(address _delegate, bytes _sig) + internal + view + returns (bool) + { + return isSignatureValidForHash( + _delegate, + keccak256(abi.encodePacked(_delegate)), + _sig + ); + } + + /** + * @dev is the signature of `delegate + sender + methodId` from a bouncer? + * @return bool + */ + function isValidTicketAndMethod(address _delegate, bytes _sig) + internal + view + returns (bool) + { + return isSignatureValidForHash( + _delegate, + keccak256(abi.encodePacked(_delegate, BouncerUtils.getMethodId())), + _sig + ); + } + + /** + * @dev is the signature of `delegate + sender + msg.data` from a bouncer? + * @notice the _sig parameter of the method being validated must be the "last" parameter + * @return bool + */ + function isValidTicketAndData(address _delegate, bytes _sig) + internal + view + returns (bool) + { + return isSignatureValidForHash( + _delegate, + keccak256(abi.encodePacked(_delegate, BouncerUtils.getMessageData())), + _sig + ); + } + + function isSignatureValidForHash(address _delegate, bytes32 _hash, bytes _sig) + internal + view + returns (bool) + { + bool isDelegate = hasRole(_delegate, ROLE_DELEGATE); + return isDelegate && _hash.isSignedBy(_delegate, _sig); + } + + function isValidSignature( + bytes32 _hash, + bytes _sig + ) + public + view + returns (bool) + { + return hasRole( + _hash.toEthSignedMessageHash().recover(_sig), + ROLE_DELEGATE + ); + } +} diff --git a/contracts/access/BouncerUtils.sol b/contracts/access/BouncerUtils.sol new file mode 100644 index 00000000000..fc30bc5de75 --- /dev/null +++ b/contracts/access/BouncerUtils.sol @@ -0,0 +1,142 @@ +pragma solidity 0.4.24; + +import "../ECRecovery.sol"; + + +/** + * @title BouncerUtils + * @author Matt Condon (@Shrugs) + * @dev Provides helpful logic for verifying method parameters. + * Use this contract if you want to implement your own Bouncer access-control logic that doesn't + * require delegated signature validity (i.e., if you need a multisig wallet or identity contract to + * "sign" for things, you need delegated signature validity.) + * + * ```solidity + * using BouncerUtils for bytes32; + * + * + * // require that the `owner` EOA account signs the submitted msg.data + * // include your custom arguments here if necessary + * modifier onlyValidMint(address _to, uint256 _amount, bytes _sig) { + * require( + * keccak256(abi.encodePacked( + * address(this), + * _to, + * _amount + * )).toEthSignedMessageHash().recover(_sig) == owner, + * "invalid signature" + * ); + * _; + * } + * + * function mint(address _to, uint256 _amount, bytes _sig) + * onlyValidMint(_to, _amount, _sig) + * public + * { + * // trust arguments + * MyToken.mint(_to, _amount); + * } + * + * // or, simpler, if you want to verify all arguments + * + * modifier onlyValidTransaction() { + * require( + * BouncerUtils.signerOfMessageData(address(this)) == owner, + * "invalid signature" + * ); + * } + * + * function mint(address _to, uint256 _amount, bytes _sig) + * onlyValidTransaction() + * public + * { + * // trust arguments + * MyToken.mint(_to, _amount); + * } + * ``` + */ +library BouncerUtils { + using ECRecovery for bytes32; + + // method ids are 4 bytes long + uint constant METHOD_ID_SIZE = 4; + // signature size is 65 bytes (tightly packed v + r + s), but gets padded to 96 bytes + uint constant SIGNATURE_SIZE = 96; + + /** + * @dev recover the signer of the msg.data, assuming all arguments are checked and the last + * argument is a compact vrs signature, padded to 96 bytes. + * @return address + */ + function signerOfMessageData(address _delegate) + internal + pure + returns (address) + { + bytes32 hashOfMessageData = keccak256( + abi.encodePacked( + _delegate, + getMessageData() + ) + ); + + return hashOfMessageData + .toEthSignedMessageHash() + .recover( + getSignatureArgument() + ); + } + + + /** + * @dev Returns the first METHOD_ID_SIZE bytes of msg.data, which is the method signature + */ + function getMethodId() + internal + pure + returns (bytes) + { + bytes memory data = new bytes(METHOD_ID_SIZE); + for (uint i = 0; i < data.length; i++) { + data[i] = msg.data[i]; + } + + return data; + } + + /** + * @dev returns msg.data, sans the last SIGNATURE_SIZE bytes + */ + function getMessageData() + internal + pure + returns (bytes) + { + require(msg.data.length > SIGNATURE_SIZE); + + bytes memory data = new bytes(msg.data.length - SIGNATURE_SIZE); + for (uint i = 0; i < data.length; i++) { + data[i] = msg.data[i]; + } + + return data; + } + + /** + * @dev returns the last 96 bytes of msg.data, the compacted vrs signature + */ + function getSignatureArgument() + internal + pure + returns (bytes) + { + require(msg.data.length > SIGNATURE_SIZE); + + bytes memory data = new bytes(SIGNATURE_SIZE); + for (uint i = msg.data.length - 1; i > data.length; i++) { + data[i] = msg.data[i]; + } + + return data; + } +} diff --git a/contracts/access/SignatureBouncer.sol b/contracts/access/SignatureBouncer.sol deleted file mode 100644 index d0f8f2a9247..00000000000 --- a/contracts/access/SignatureBouncer.sol +++ /dev/null @@ -1,159 +0,0 @@ -pragma solidity ^0.4.24; - -import "../ownership/Ownable.sol"; -import "../ownership/rbac/RBAC.sol"; -import "../ECRecovery.sol"; - - -/** - * @title SignatureBouncer - * @author PhABC, Shrugs and aflesher - * @dev Bouncer allows users to submit a signature as a permission to do an action. - * If the signature is from one of the authorized bouncer addresses, the signature - * is valid. The owner of the contract adds/removes bouncers. - * Bouncer addresses can be individual servers signing grants or different - * users within a decentralized club that have permission to invite other members. - * This technique is useful for whitelists and airdrops; instead of putting all - * valid addresses on-chain, simply sign a grant of the form - * keccak256(abi.encodePacked(`:contractAddress` + `:granteeAddress`)) using a valid bouncer address. - * Then restrict access to your crowdsale/whitelist/airdrop using the - * `onlyValidSignature` modifier (or implement your own using isValidSignature). - * In addition to `onlyValidSignature`, `onlyValidSignatureAndMethod` and - * `onlyValidSignatureAndData` can be used to restrict access to only a given method - * or a given method with given parameters respectively. - * See the tests Bouncer.test.js for specific usage examples. - * @notice A method that uses the `onlyValidSignatureAndData` modifier must make the _sig - * parameter the "last" parameter. You cannot sign a message that has its own - * signature in it so the last 128 bytes of msg.data (which represents the - * length of the _sig data and the _sig data itself) is ignored when validating. - * Also non fixed sized parameters make constructing the data in the signature - * much more complex. See https://ethereum.stackexchange.com/a/50616 for more details. - */ -contract SignatureBouncer is Ownable, RBAC { - using ECRecovery for bytes32; - - string public constant ROLE_BOUNCER = "bouncer"; - uint constant METHOD_ID_SIZE = 4; - // signature size is 65 bytes (tightly packed v + r + s), but gets padded to 96 bytes - uint constant SIGNATURE_SIZE = 96; - - /** - * @dev requires that a valid signature of a bouncer was provided - */ - modifier onlyValidSignature(bytes _sig) - { - require(isValidSignature(msg.sender, _sig)); - _; - } - - /** - * @dev requires that a valid signature with a specifed method of a bouncer was provided - */ - modifier onlyValidSignatureAndMethod(bytes _sig) - { - require(isValidSignatureAndMethod(msg.sender, _sig)); - _; - } - - /** - * @dev requires that a valid signature with a specifed method and params of a bouncer was provided - */ - modifier onlyValidSignatureAndData(bytes _sig) - { - require(isValidSignatureAndData(msg.sender, _sig)); - _; - } - - /** - * @dev allows the owner to add additional bouncer addresses - */ - function addBouncer(address _bouncer) - onlyOwner - public - { - require(_bouncer != address(0)); - addRole(_bouncer, ROLE_BOUNCER); - } - - /** - * @dev allows the owner to remove bouncer addresses - */ - function removeBouncer(address _bouncer) - onlyOwner - public - { - require(_bouncer != address(0)); - removeRole(_bouncer, ROLE_BOUNCER); - } - - /** - * @dev is the signature of `this + sender` from a bouncer? - * @return bool - */ - function isValidSignature(address _address, bytes _sig) - internal - view - returns (bool) - { - return isValidDataHash( - keccak256(abi.encodePacked(address(this), _address)), - _sig - ); - } - - /** - * @dev is the signature of `this + sender + methodId` from a bouncer? - * @return bool - */ - function isValidSignatureAndMethod(address _address, bytes _sig) - internal - view - returns (bool) - { - bytes memory data = new bytes(METHOD_ID_SIZE); - for (uint i = 0; i < data.length; i++) { - data[i] = msg.data[i]; - } - return isValidDataHash( - keccak256(abi.encodePacked(address(this), _address, data)), - _sig - ); - } - - /** - * @dev is the signature of `this + sender + methodId + params(s)` from a bouncer? - * @notice the _sig parameter of the method being validated must be the "last" parameter - * @return bool - */ - function isValidSignatureAndData(address _address, bytes _sig) - internal - view - returns (bool) - { - require(msg.data.length > SIGNATURE_SIZE); - bytes memory data = new bytes(msg.data.length - SIGNATURE_SIZE); - for (uint i = 0; i < data.length; i++) { - data[i] = msg.data[i]; - } - return isValidDataHash( - keccak256(abi.encodePacked(address(this), _address, data)), - _sig - ); - } - - /** - * @dev internal function to convert a hash to an eth signed message - * and then recover the signature and check it against the bouncer role - * @return bool - */ - function isValidDataHash(bytes32 hash, bytes _sig) - internal - view - returns (bool) - { - address signer = hash - .toEthSignedMessageHash() - .recover(_sig); - return hasRole(signer, ROLE_BOUNCER); - } -} diff --git a/contracts/introspection/ERC165Checker.sol b/contracts/introspection/ERC165Checker.sol new file mode 100644 index 00000000000..30bdbb1d7f8 --- /dev/null +++ b/contracts/introspection/ERC165Checker.sol @@ -0,0 +1,77 @@ +pragma solidity ^0.4.24; + + +/** + * @title ERC165Checker + * @dev Use `using ERC165Checker for address;` to include this library + */ +library ERC165Checker { + + bytes4 private constant InterfaceId_Invalid = 0xffffffff; + bytes4 private constant InterfaceId_ERC165 = 0x01ffc9a7; + + /** + * @notice Query if a contract implements an interface + * @param _interfaceId The interface identifier, as specified in ERC-165 + * @dev Interface identification is specified in ERC-165. + */ + function supportsInterface(address _address, bytes4 _interfaceId) + internal + view + returns (bool) + { + uint256 success; + uint256 result; + + // supports ERC165 (probably)? + (success, result) = noThrowCall(_address, InterfaceId_ERC165); + if ((success == 0) || (result == 0)) { + return false; + } + + // definitely supports ERC165? + (success, result) = noThrowCall(_address, InterfaceId_Invalid); + if ((success == 0) || (result != 0)) { + return false; + } + + // supports the _interfaceId we care about? + (success, result) = noThrowCall(_address, _interfaceId); + if ((success == 1) && (result == 1)) { + return true; + } + + return false; + } + + /** + * @dev noThrowCall via https://github.com/ethereum/EIPs/blob/master/EIPS/eip-165.md + */ + function noThrowCall( + address _address, + bytes4 _interfaceId + ) + internal + view + returns (uint256 success, uint256 result) + { + bytes4 erc165ID = InterfaceId_ERC165; + + // solium-disable-next-line security/no-inline-assembly + assembly { + let x := mload(0x40) // Find empty storage location using "free memory pointer" + mstore(x, erc165ID) // Place signature at begining of empty storage to call supportsInterface() + mstore(add(x, 0x04), _interfaceId) // Place first argument directly next to signature + + success := staticcall( + 30000, // 30k gas + _address, // To addr + x, // Inputs are stored at location x + 0x20, // Inputs are 32 bytes long + x, // Store output over input (saves space) + 0x20 // Outputs are 32 bytes long + ) + result := mload(x) // Load the result + } + } +} diff --git a/contracts/mocks/BouncerMock.sol b/contracts/mocks/BouncerMock.sol index 65009b0b040..6e5515e579e 100644 --- a/contracts/mocks/BouncerMock.sol +++ b/contracts/mocks/BouncerMock.sol @@ -1,43 +1,43 @@ pragma solidity ^0.4.24; -import "../access/SignatureBouncer.sol"; +import "../access/Bouncer.sol"; -contract SignatureBouncerMock is SignatureBouncer { - function checkValidSignature(address _address, bytes _sig) +contract BouncerMock is Bouncer { + function checkValidTicket(address _delegate, bytes _sig) public view returns (bool) { - return isValidSignature(_address, _sig); + return isValidTicket(_delegate, _sig); } - function onlyWithValidSignature(bytes _sig) - onlyValidSignature(_sig) + function onlyWithValidTicket(bytes _sig) + onlyValidTicket(address(this), _sig) public view { } - function checkValidSignatureAndMethod(address _address, bytes _sig) + function checkValidTicketAndMethod(address _delegate, bytes _sig) public view returns (bool) { - return isValidSignatureAndMethod(_address, _sig); + return isValidTicketAndMethod(_delegate, _sig); } - function onlyWithValidSignatureAndMethod(bytes _sig) - onlyValidSignatureAndMethod(_sig) + function onlyWithValidTicketAndMethod(bytes _sig) + onlyValidTicketForMethod(address(this), _sig) public view { } - function checkValidSignatureAndData( - address _address, + function checkValidTicketAndData( + address _delegate, bytes, uint, bytes _sig @@ -46,11 +46,11 @@ contract SignatureBouncerMock is SignatureBouncer { view returns (bool) { - return isValidSignatureAndData(_address, _sig); + return isValidTicketAndData(_delegate, _sig); } - function onlyWithValidSignatureAndData(uint, bytes _sig) - onlyValidSignatureAndData(_sig) + function onlyWithValidTicketAndData(uint, bytes _sig) + onlyValidTicketForData(address(this), _sig) public view { diff --git a/contracts/mocks/ERC165/ERC165GenerallySupported.sol b/contracts/mocks/ERC165/ERC165GenerallySupported.sol new file mode 100644 index 00000000000..64c63dacc28 --- /dev/null +++ b/contracts/mocks/ERC165/ERC165GenerallySupported.sol @@ -0,0 +1,8 @@ +pragma solidity ^0.4.24; + +import "../../introspection/SupportsInterfaceWithLookup.sol"; + + +contract ERC165GenerallySupported is SupportsInterfaceWithLookup { + +} diff --git a/contracts/mocks/ERC165/ERC165InterfaceSupported.sol b/contracts/mocks/ERC165/ERC165InterfaceSupported.sol new file mode 100644 index 00000000000..10d32965238 --- /dev/null +++ b/contracts/mocks/ERC165/ERC165InterfaceSupported.sol @@ -0,0 +1,12 @@ +pragma solidity ^0.4.24; + +import "../../introspection/SupportsInterfaceWithLookup.sol"; + + +contract ERC165InterfaceSupported is SupportsInterfaceWithLookup { + constructor (bytes4 _interfaceId) + public + { + _registerInterface(_interfaceId); + } +} diff --git a/contracts/mocks/ERC165/ERC165NotSupported.sol b/contracts/mocks/ERC165/ERC165NotSupported.sol new file mode 100644 index 00000000000..763f8fabadf --- /dev/null +++ b/contracts/mocks/ERC165/ERC165NotSupported.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.4.24; + + +contract ERC165NotSupported { + +} diff --git a/contracts/mocks/ERC165CheckerMock.sol b/contracts/mocks/ERC165CheckerMock.sol new file mode 100644 index 00000000000..249cbc785d2 --- /dev/null +++ b/contracts/mocks/ERC165CheckerMock.sol @@ -0,0 +1,16 @@ +pragma solidity ^0.4.24; + +import "../introspection/ERC165Checker.sol"; + + +contract ERC165CheckerMock { + using ERC165Checker for address; + + function supportsInterface(address _address, bytes4 _interfaceId) + public + view + returns (bool) + { + return _address.supportsInterface(_interfaceId); + } +} diff --git a/contracts/mocks/SignatureDelegateImpl.sol b/contracts/mocks/SignatureDelegateImpl.sol new file mode 100644 index 00000000000..3772e56d956 --- /dev/null +++ b/contracts/mocks/SignatureDelegateImpl.sol @@ -0,0 +1,37 @@ +pragma solidity 0.4.24; + +import "../signatures/SignatureDelegate.sol"; +import "./BouncerMock.sol"; + + +contract SignatureDelegateImpl is SignatureDelegate { + + bool public shouldPass = false; + BouncerMock public bouncer; + + constructor (bool _shouldPass, BouncerMock _bouncer) + public + { + shouldPass = _shouldPass; + bouncer = _bouncer; + } + + function isValidSignature( + bytes32, + bytes + ) + public + view + returns (bool) + { + return shouldPass; + } + + function forward() + view + public + { + require(bouncer.checkValidTicket(address(this), "")); + } + +} diff --git a/contracts/ownership/rbac/RBACOwnable.sol b/contracts/ownership/rbac/RBACOwnable.sol new file mode 100644 index 00000000000..e6b1087c2f9 --- /dev/null +++ b/contracts/ownership/rbac/RBACOwnable.sol @@ -0,0 +1,41 @@ +pragma solidity ^0.4.24; + +import "./RBAC.sol"; + + +/** + * @title RBACOwnable + * @author Matt Condon (@shrugs) + * @dev Ownable logic using RBAC. + * @dev Use RBACOwnable if you could have many different owners and you're ok with + * @dev the security profile of any owner being able to add another owner. + * @dev Only difference from Ownable.sol is that the owners are not stored as public variables. + */ +contract RBACOwnable is RBAC { + string public constant ROLE_OWNER = "owner"; + + constructor() + public + { + addRole(msg.sender, ROLE_OWNER); + } + + modifier onlyOwners() { + checkRole(msg.sender, ROLE_OWNER); + _; + } + + function addOwner(address _owner) + onlyOwners + public + { + addRole(_owner, ROLE_OWNER); + } + + function removeOwner(address _owner) + onlyOwners + public + { + removeRole(_owner, ROLE_OWNER); + } +} diff --git a/contracts/signatures/ISignatureDelegate.sol b/contracts/signatures/ISignatureDelegate.sol new file mode 100644 index 00000000000..dfada459409 --- /dev/null +++ b/contracts/signatures/ISignatureDelegate.sol @@ -0,0 +1,31 @@ +pragma solidity 0.4.24; + +import "../introspection/ERC165.sol"; + +/** + * @title ISignatureDelegate + * @dev Implement this interface so that your contract can validate signatures. + * inspired by https://github.com/0xProject/0x-monorepo/blob/v2-prototype/packages/contracts/src/2.0.0/protocol/Exchange/interfaces/IWallet.sol + */ +contract ISignatureDelegate is ERC165 { + + bytes4 internal constant InterfaceId_SignatureDelegate = 0x1626ba7e; + /** + * 0x1626ba7e === + * bytes4(keccak256('isValidSignature(bytes32,bytes)')) + */ + + /** + * @dev verifies that a signature of a hash is valid + * @param _hash message hash that is signed + * @param _sig the provided signature + * @return bool validity of signature for the hash + */ + function isValidSignature( + bytes32 _hash, + bytes _sig + ) + public + view + returns (bool); +} diff --git a/contracts/signatures/SignatureDelegate.sol b/contracts/signatures/SignatureDelegate.sol new file mode 100644 index 00000000000..6db684f37ce --- /dev/null +++ b/contracts/signatures/SignatureDelegate.sol @@ -0,0 +1,19 @@ +pragma solidity 0.4.24; + +import "./ISignatureDelegate.sol"; +import "../introspection/SupportsInterfaceWithLookup.sol"; + + +/** + * @title SignatureDelegate + * @dev Partial implementation of ISignatureDelegate that adds ERC165 support + * But you as the developer still have to add your signature validation logic. + * As a delegate, you must proxy calls from accounts you consider valid. + */ +contract SignatureDelegate is ISignatureDelegate, SupportsInterfaceWithLookup { + constructor () + public + { + _registerInterface(InterfaceId_SignatureDelegate); + } +} diff --git a/test/access/Bouncer.test.js b/test/access/Bouncer.test.js new file mode 100644 index 00000000000..563f4fbc9fc --- /dev/null +++ b/test/access/Bouncer.test.js @@ -0,0 +1,234 @@ + +const { assertRevert } = require('../helpers/assertRevert'); +const { getBouncerTicketGenerator } = require('../helpers/sign'); +const { makeInterfaceId } = require('../helpers/makeInterfaceId'); + +const Bouncer = artifacts.require('BouncerMock'); +const SignatureDelegateImpl = artifacts.require('SignatureDelegateImpl'); + +require('chai') + .use(require('chai-as-promised')) + .should(); + +const UINT_VALUE = 23; +const BYTES_VALUE = web3.toHex('test'); +const INVALID_SIGNATURE = '0xabcd'; + +contract('Bouncer', ([_, owner, anyone, delegate, newDelegate]) => { + beforeEach(async function () { + this.bouncer = await Bouncer.new({ from: owner }); + this.roleDelegate = await this.bouncer.ROLE_DELEGATE(); + this.roleOwner = await this.bouncer.ROLE_OWNER(); + this.generateTicket = getBouncerTicketGenerator(this.bouncer, delegate); + }); + + it('should have a default owner', async function () { + const hasRole = await this.bouncer.hasRole(owner, this.roleOwner); + hasRole.should.eq(true); + }); + + it('should allow owner to add a delegate', async function () { + await this.bouncer.addDelegate(delegate, { from: owner }); + const hasRole = await this.bouncer.hasRole(delegate, this.roleDelegate); + hasRole.should.eq(true); + }); + + it('should not allow anyone to add a delegate', async function () { + await assertRevert( + this.bouncer.addDelegate(delegate, { from: anyone }) + ); + }); + + context('EOA delegate', function () { + beforeEach(async function () { + await this.bouncer.addDelegate(delegate, { from: owner }); + }); + + context('modifiers', () => { + it('should allow valid signature', async function () { + await this.bouncer.onlyWithValidTicket( + this.generateTicket(), + { from: anyone } + ); + }); + it('should not allow invalid signature', async function () { + await assertRevert( + this.bouncer.onlyWithValidTicket( + INVALID_SIGNATURE, + { from: anyone } + ) + ); + }); + it('should allow valid signature with a valid method', async function () { + await this.bouncer.onlyWithValidTicketAndMethod( + this.generateTicket('onlyWithValidTicketAndMethod'), + { from: anyone } + ); + }); + it('should not allow invalid signature with method', async function () { + await assertRevert( + this.bouncer.onlyWithValidTicketAndMethod( + INVALID_SIGNATURE, + { from: anyone } + ) + ); + }); + it('should allow valid signature with a valid data', async function () { + await this.bouncer.onlyWithValidTicketAndData( + UINT_VALUE, + this.generateTicket('onlyWithValidTicketAndData', [UINT_VALUE]), + { from: anyone } + ); + }); + it('should not allow invalid signature with data', async function () { + await assertRevert( + this.bouncer.onlyWithValidTicketAndData( + UINT_VALUE, + INVALID_SIGNATURE, + { from: anyone } + ) + ); + }); + }); + + context('signatures', () => { + it('should accept valid ticket for valid delegate', async function () { + const isValid = await this.bouncer.checkValidTicket( + this.bouncer.address, + this.generateTicket() + ); + isValid.should.eq(true); + }); + it('should not accept invalid ticket for valid delegate', async function () { + const isValid = await this.bouncer.checkValidTicket( + this.bouncer.address, + 'abcd' + ); + isValid.should.eq(false); + }); + it('should accept valid ticket with valid method', async function () { + const isValid = await this.bouncer.checkValidTicketAndMethod( + this.bouncer.address, + this.generateTicket('checkValidTicketAndMethod') + ); + isValid.should.eq(true); + }); + it('should not accept valid message with an invalid method', async function () { + const isValid = await this.bouncer.checkValidTicketAndMethod( + this.bouncer.address, + this.generateTicket('theWrongMethod') + ); + isValid.should.eq(false); + }); + it('should not accept valid message with a valid method for an invalid user', async function () { + const isValid = await this.bouncer.checkValidTicketAndMethod( + anyone, + this.generateTicket('checkValidTicketAndMethod') + ); + isValid.should.eq(false); + }); + it('should accept valid method with valid params', async function () { + const isValid = await this.bouncer.checkValidTicketAndData( + this.bouncer.address, + BYTES_VALUE, + UINT_VALUE, + this.generateTicket('checkValidTicketAndData', [this.bouncer.address, BYTES_VALUE, UINT_VALUE]) + ); + isValid.should.eq(true); + }); + it('should not accept valid method with invalid params', async function () { + const isValid = await this.bouncer.checkValidTicketAndData( + this.bouncer.address, + BYTES_VALUE, + 500, + this.generateTicket('checkValidTicketAndData', [this.bouncer.address, BYTES_VALUE, UINT_VALUE]) + ); + isValid.should.eq(false); + }); + it('should not accept valid method with valid params for invalid delegate', async function () { + const isValid = await this.bouncer.checkValidTicketAndData( + anyone, + BYTES_VALUE, + UINT_VALUE, + this.generateTicket('checkValidTicketAndData', [anyone, BYTES_VALUE, UINT_VALUE]) + ); + isValid.should.eq(false); + }); + }); + + context('management', () => { + it('should not allow anyone to add delegates', async function () { + await assertRevert( + this.bouncer.addDelegate(newDelegate, { from: anyone }) + ); + }); + + it('should be able to add delegate', async function () { + await this.bouncer.addDelegate(newDelegate, { from: owner }) + .should.be.fulfilled; + }); + + it('should not allow adding invalid address', async function () { + await assertRevert( + this.bouncer.addDelegate('0x0', { from: owner }) + ); + }); + + it('should not allow anyone to remove delegate', async function () { + await assertRevert( + this.bouncer.removeDelegate(newDelegate, { from: anyone }) + ); + }); + + it('should be able to remove delegates', async function () { + await this.bouncer.removeDelegate(newDelegate, { from: owner }) + .should.be.fulfilled; + }); + }); + }); + + context('contract delegate', () => { + context('not a delegate', () => { + beforeEach(async function () { + this.delegateContract = await SignatureDelegateImpl.new(true, this.bouncer.address, { from: owner }); + }); + + it('should fail', async function () { + await assertRevert( + this.delegateContract.forward({ from: anyone }) + ); + }); + }); + + context('invalid delegate', () => { + beforeEach(async function () { + this.delegateContract = await SignatureDelegateImpl.new(false, this.bouncer.address, { from: owner }); + await this.bouncer.addDelegate(this.delegateContract.address, { from: owner }); + }); + + it('should be invalid', async function () { + await assertRevert( + this.delegateContract.forward({ from: anyone }) + ); + }); + }); + + context('valid delegate', () => { + beforeEach(async function () { + this.delegateContract = await SignatureDelegateImpl.new(true, this.bouncer.address, { from: owner }); + await this.bouncer.addDelegate(this.delegateContract.address, { from: owner }); + }); + + it('should support isValidSignature', async function () { + const supported = await this.delegateContract.supportsInterface(makeInterfaceId([ + 'isValidSignature(bytes32,bytes)', + ])); + supported.should.eq(true); + }); + + it('should be valid', async function () { + await this.delegateContract.forward({ from: anyone }); + }); + }); + }); +}); diff --git a/test/access/SignatureBouncer.test.js b/test/access/SignatureBouncer.test.js deleted file mode 100644 index 8e019701454..00000000000 --- a/test/access/SignatureBouncer.test.js +++ /dev/null @@ -1,245 +0,0 @@ -const { assertRevert } = require('../helpers/assertRevert'); -const { getBouncerSigner } = require('../helpers/sign'); - -const Bouncer = artifacts.require('SignatureBouncerMock'); - -require('chai') - .use(require('chai-as-promised')) - .should(); - -const UINT_VALUE = 23; -const BYTES_VALUE = web3.toHex('test'); -const INVALID_SIGNATURE = '0xabcd'; - -contract('Bouncer', ([_, owner, anyone, bouncerAddress, authorizedUser]) => { - beforeEach(async function () { - this.bouncer = await Bouncer.new({ from: owner }); - this.roleBouncer = await this.bouncer.ROLE_BOUNCER(); - }); - - context('management', () => { - it('has a default owner of self', async function () { - (await this.bouncer.owner()).should.eq(owner); - }); - - it('allows the owner to add a bouncer', async function () { - await this.bouncer.addBouncer(bouncerAddress, { from: owner }); - (await this.bouncer.hasRole(bouncerAddress, this.roleBouncer)).should.eq(true); - }); - - it('does not allow adding an invalid address', async function () { - await assertRevert( - this.bouncer.addBouncer('0x0', { from: owner }) - ); - }); - - it('allows the owner to remove a bouncer', async function () { - await this.bouncer.addBouncer(bouncerAddress, { from: owner }); - - await this.bouncer.removeBouncer(bouncerAddress, { from: owner }); - (await this.bouncer.hasRole(bouncerAddress, this.roleBouncer)).should.eq(false); - }); - - it('does not allow anyone to add a bouncer', async function () { - await assertRevert( - this.bouncer.addBouncer(bouncerAddress, { from: anyone }) - ); - }); - - it('does not allow anyone to remove a bouncer', async function () { - await this.bouncer.addBouncer(bouncerAddress, { from: owner }); - - await assertRevert( - this.bouncer.removeBouncer(bouncerAddress, { from: anyone }) - ); - }); - }); - - context('with bouncer address', () => { - beforeEach(async function () { - await this.bouncer.addBouncer(bouncerAddress, { from: owner }); - this.signFor = getBouncerSigner(this.bouncer, bouncerAddress); - }); - - describe('modifiers', () => { - context('plain signature', () => { - it('allows valid signature for sender', async function () { - await this.bouncer.onlyWithValidSignature(this.signFor(authorizedUser), { from: authorizedUser }); - }); - - it('does not allow invalid signature for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignature(INVALID_SIGNATURE, { from: authorizedUser }) - ); - }); - - it('does not allow valid signature for other sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignature(this.signFor(authorizedUser), { from: anyone }) - ); - }); - - it('does not allow valid signature for method for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignature(this.signFor(authorizedUser, 'onlyWithValidSignature'), - { from: authorizedUser }) - ); - }); - }); - - context('method signature', () => { - it('allows valid signature with correct method for sender', async function () { - await this.bouncer.onlyWithValidSignatureAndMethod( - this.signFor(authorizedUser, 'onlyWithValidSignatureAndMethod'), { from: authorizedUser } - ); - }); - - it('does not allow invalid signature with correct method for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndMethod(INVALID_SIGNATURE, { from: authorizedUser }) - ); - }); - - it('does not allow valid signature with correct method for other sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndMethod( - this.signFor(authorizedUser, 'onlyWithValidSignatureAndMethod'), { from: anyone } - ) - ); - }); - - it('does not allow valid method signature with incorrect method for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndMethod(this.signFor(authorizedUser, 'theWrongMethod'), - { from: authorizedUser }) - ); - }); - - it('does not allow valid non-method signature method for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndMethod(this.signFor(authorizedUser), { from: authorizedUser }) - ); - }); - }); - - context('method and data signature', () => { - it('allows valid signature with correct method and data for sender', async function () { - await this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE, - this.signFor(authorizedUser, 'onlyWithValidSignatureAndData', [UINT_VALUE]), { from: authorizedUser } - ); - }); - - it('does not allow invalid signature with correct method and data for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE, INVALID_SIGNATURE, { from: authorizedUser }) - ); - }); - - it('does not allow valid signature with correct method and incorrect data for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE + 10, - this.signFor(authorizedUser, 'onlyWithValidSignatureAndData', [UINT_VALUE]), - { from: authorizedUser } - ) - ); - }); - - it('does not allow valid signature with correct method and data for other sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE, - this.signFor(authorizedUser, 'onlyWithValidSignatureAndData', [UINT_VALUE]), - { from: anyone } - ) - ); - }); - - it('does not allow valid non-method signature for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE, - this.signFor(authorizedUser), { from: authorizedUser } - ) - ); - }); - }); - }); - - context('signature validation', () => { - context('plain signature', () => { - it('validates valid signature for valid user', async function () { - (await this.bouncer.checkValidSignature(authorizedUser, this.signFor(authorizedUser))).should.eq(true); - }); - - it('does not validate invalid signature for valid user', async function () { - (await this.bouncer.checkValidSignature(authorizedUser, INVALID_SIGNATURE)).should.eq(false); - }); - - it('does not validate valid signature for anyone', async function () { - (await this.bouncer.checkValidSignature(anyone, this.signFor(authorizedUser))).should.eq(false); - }); - - it('does not validate valid signature for method for valid user', async function () { - (await this.bouncer.checkValidSignature(authorizedUser, this.signFor(authorizedUser, 'checkValidSignature')) - ).should.eq(false); - }); - }); - - context('method signature', () => { - it('validates valid signature with correct method for valid user', async function () { - (await this.bouncer.checkValidSignatureAndMethod(authorizedUser, - this.signFor(authorizedUser, 'checkValidSignatureAndMethod')) - ).should.eq(true); - }); - - it('does not validate invalid signature with correct method for valid user', async function () { - (await this.bouncer.checkValidSignatureAndMethod(authorizedUser, INVALID_SIGNATURE)).should.eq(false); - }); - - it('does not validate valid signature with correct method for anyone', async function () { - (await this.bouncer.checkValidSignatureAndMethod(anyone, - this.signFor(authorizedUser, 'checkValidSignatureAndMethod')) - ).should.eq(false); - }); - - it('does not validate valid non-method signature with correct method for valid user', async function () { - (await this.bouncer.checkValidSignatureAndMethod(authorizedUser, this.signFor(authorizedUser)) - ).should.eq(false); - }); - }); - - context('method and data signature', () => { - it('validates valid signature with correct method and data for valid user', async function () { - (await this.bouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE, - this.signFor(authorizedUser, 'checkValidSignatureAndData', [authorizedUser, BYTES_VALUE, UINT_VALUE])) - ).should.eq(true); - }); - - it('does not validate invalid signature with correct method and data for valid user', async function () { - (await this.bouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE, INVALID_SIGNATURE) - ).should.eq(false); - }); - - it('does not validate valid signature with correct method and incorrect data for valid user', - async function () { - (await this.bouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE + 10, - this.signFor(authorizedUser, 'checkValidSignatureAndData', [authorizedUser, BYTES_VALUE, UINT_VALUE])) - ).should.eq(false); - } - ); - - it('does not validate valid signature with correct method and data for anyone', async function () { - (await this.bouncer.checkValidSignatureAndData(anyone, BYTES_VALUE, UINT_VALUE, - this.signFor(authorizedUser, 'checkValidSignatureAndData', [authorizedUser, BYTES_VALUE, UINT_VALUE])) - ).should.eq(false); - }); - - it('does not validate valid non-method-data signature with correct method and data for valid user', - async function () { - (await this.bouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE, - this.signFor(authorizedUser, 'checkValidSignatureAndData')) - ).should.eq(false); - } - ); - }); - }); - }); -}); diff --git a/test/helpers/assertRevert.js b/test/helpers/assertRevert.js index b82e0f195b5..0c4ed094cdf 100644 --- a/test/helpers/assertRevert.js +++ b/test/helpers/assertRevert.js @@ -1,7 +1,7 @@ async function assertRevert (promise) { try { await promise; - assert.fail('Expected revert not received'); + throw new Error('Transaction succeeded when it shouldn\'t have'); } catch (error) { const revertFound = error.message.search('revert') >= 0; assert(revertFound, `Expected "revert", got ${error} instead`); diff --git a/test/helpers/sign.js b/test/helpers/sign.js index a991d10a378..d2bea54dafb 100644 --- a/test/helpers/sign.js +++ b/test/helpers/sign.js @@ -3,11 +3,10 @@ const { soliditySha3 } = require('web3-utils'); const REAL_SIGNATURE_SIZE = 2 * 65; // 65 bytes in hexadecimal string legnth const PADDED_SIGNATURE_SIZE = 2 * 96; // 96 bytes in hexadecimal string length - -const DUMMY_SIGNATURE = `0x${web3.padLeft('', REAL_SIGNATURE_SIZE)}`; +const DUMMY_SIGNATURE = `0x${web3.padLeft('', REAL_SIGNATURE_SIZE)}`; // '0x' plus 130 '0's because hex /** - * Hash and add same prefix to the hash that ganache use. + * Hash and add same prefix to the hash that ganache uses. * @param {string} message the plaintext/ascii/original message * @return {string} the hash of the message, prefixed, and then hashed again */ @@ -40,40 +39,43 @@ const transformToFullName = function (json) { * fetcher because the method on the contract isn't actually the SolidityFunction object ಠ_ಠ * @param contract TruffleContract * @param signer address - * @param redeemer address * @param methodName string * @param methodArgs any[] */ -const getBouncerSigner = (contract, signer) => (redeemer, methodName, methodArgs = []) => { +const getBouncerTicketGenerator = (contract, signer) => (methodName, methodArgs = []) => { const parts = [ contract.address, - redeemer, ]; // if we have a method, add it to the parts that we're signing if (methodName) { if (methodArgs.length > 0) { + // construct the transaction data, but sub out the signature part + // (necessary to correctly calculate the offsets for any variable-length arguments parts.push( - contract.contract[methodName].getData(...methodArgs.concat([DUMMY_SIGNATURE])).slice( - 0, - -1 * PADDED_SIGNATURE_SIZE - ) + contract.contract[methodName] + .getData(...methodArgs.concat([DUMMY_SIGNATURE])) + .slice( + 0, + -1 * PADDED_SIGNATURE_SIZE + ) ); } else { const abi = contract.abi.find(abi => abi.name === methodName); const name = transformToFullName(abi); - const signature = web3.sha3(name).slice(0, 10); + const signature = web3.sha3(name).slice(0, 10); // 0xabcd parts.push(signature); } } - // ^ substr to remove `0x` because in solidity the address is a set of byes, not a string `0xabcd` + // hash the method data const hashOfMessage = soliditySha3(...parts); + // then sign that hash return signMessage(signer, hashOfMessage); }; module.exports = { hashMessage, signMessage, - getBouncerSigner, + getBouncerTicketGenerator, }; diff --git a/test/introspection/ERC165Checker.test.js b/test/introspection/ERC165Checker.test.js new file mode 100644 index 00000000000..698a7095059 --- /dev/null +++ b/test/introspection/ERC165Checker.test.js @@ -0,0 +1,49 @@ +const ERC165Checker = artifacts.require('ERC165CheckerMock'); +const ERC165NotSupported = artifacts.require('ERC165NotSupported'); +const ERC165GenerallySupported = artifacts.require('ERC165GenerallySupported'); +const ERC165InterfaceSupported = artifacts.require('ERC165InterfaceSupported'); + +const DUMMY_ID = '0xdeadbeef'; + +require('chai') + .use(require('chai-as-promised')) + .should(); + +contract('ERC165Checker', function (accounts) { + before(async function () { + this.mock = await ERC165Checker.new(); + }); + + context('not supported', () => { + beforeEach(async function () { + this.target = await ERC165NotSupported.new(); + }); + + it('fails', async function () { + const supported = await this.mock.supportsInterface(this.target.address, DUMMY_ID); + supported.should.eq(false); + }); + }); + + context('generally supported', () => { + beforeEach(async function () { + this.target = await ERC165GenerallySupported.new(); + }); + + it('fails', async function () { + const supported = await this.mock.supportsInterface(this.target.address, DUMMY_ID); + supported.should.eq(false); + }); + }); + + context('interface supported', () => { + beforeEach(async function () { + this.target = await ERC165InterfaceSupported.new(DUMMY_ID); + }); + + it('succeeds', async function () { + const supported = await this.mock.supportsInterface(this.target.address, DUMMY_ID); + supported.should.eq(true); + }); + }); +}); diff --git a/test/ownership/rbac/RBACOwnable.test.js b/test/ownership/rbac/RBACOwnable.test.js new file mode 100644 index 00000000000..442f6b100a0 --- /dev/null +++ b/test/ownership/rbac/RBACOwnable.test.js @@ -0,0 +1,34 @@ +const { assertRevert } = require('../../helpers/assertRevert'); + +const RBACOwnable = artifacts.require('RBACOwnable'); + +require('chai') + .use(require('chai-as-promised')) + .should(); + +contract('RBAC', function ([_, owner, newOwner, notOwner]) { + before(async function () { + this.ownable = await RBACOwnable.new({ from: owner }); + this.roleOwner = await this.ownable.ROLE_OWNER(); + }); + + it('should not allow notOwner to add or remove owners', async function () { + await assertRevert( + this.ownable.addOwner(newOwner, { from: notOwner }) + ); + + await assertRevert( + this.ownable.removeOwner(owner, { from: notOwner }) + ); + }); + + it('should allow existing owner to add and remove a newOwner', async function () { + await this.ownable.addOwner(newOwner, { from: owner }); + let hasRole = await this.ownable.hasRole(newOwner, this.roleOwner); + hasRole.should.eq(true); + + await this.ownable.removeOwner(newOwner, { from: owner }); + hasRole = await this.ownable.hasRole(newOwner, this.roleOwner); + hasRole.should.eq(false); + }); +}); diff --git a/test/token/ERC721/ERC721BasicToken.behaviour.js b/test/token/ERC721/ERC721BasicToken.behaviour.js index bf14a573a78..9b688df8d90 100644 --- a/test/token/ERC721/ERC721BasicToken.behaviour.js +++ b/test/token/ERC721/ERC721BasicToken.behaviour.js @@ -4,7 +4,7 @@ const { decodeLogs } = require('../../helpers/decodeLogs'); const { sendTransaction } = require('../../helpers/sendTransaction'); const _ = require('lodash'); -const ERC721Receiver = artifacts.require('ERC721ReceiverMock.sol'); +const ERC721Receiver = artifacts.require('ERC721ReceiverMock'); const BigNumber = web3.BigNumber; require('chai') diff --git a/test/token/ERC721/ERC721BasicToken.test.js b/test/token/ERC721/ERC721BasicToken.test.js index 65aeb12d5a0..a84af6f13ae 100644 --- a/test/token/ERC721/ERC721BasicToken.test.js +++ b/test/token/ERC721/ERC721BasicToken.test.js @@ -2,7 +2,7 @@ const { shouldBehaveLikeERC721BasicToken } = require('./ERC721BasicToken.behavio const { shouldBehaveLikeMintAndBurnERC721Token } = require('./ERC721MintBurn.behaviour'); const BigNumber = web3.BigNumber; -const ERC721BasicToken = artifacts.require('ERC721BasicTokenMock.sol'); +const ERC721BasicToken = artifacts.require('ERC721BasicTokenMock'); require('chai') .use(require('chai-as-promised')) diff --git a/test/token/ERC721/ERC721Token.test.js b/test/token/ERC721/ERC721Token.test.js index 1cd9240a72b..65167453902 100644 --- a/test/token/ERC721/ERC721Token.test.js +++ b/test/token/ERC721/ERC721Token.test.js @@ -5,7 +5,7 @@ const { shouldSupportInterfaces } = require('../../introspection/SupportsInterfa const _ = require('lodash'); const BigNumber = web3.BigNumber; -const ERC721Token = artifacts.require('ERC721TokenMock.sol'); +const ERC721Token = artifacts.require('ERC721TokenMock'); require('chai') .use(require('chai-as-promised'))