diff --git a/CODE_STYLE.md b/CODE_STYLE.md index e2927234c..11614a246 100644 --- a/CODE_STYLE.md +++ b/CODE_STYLE.md @@ -14,6 +14,8 @@ Any exception or additions specific to our project are documented below. ### Naming +* Try to avoid acronyms and abbreviations. + * Parameters must be prefixed with an underscore. ``` diff --git a/contracts/AddressUtils.sol b/contracts/AddressUtils.sol index 62d911428..b07447d38 100644 --- a/contracts/AddressUtils.sol +++ b/contracts/AddressUtils.sol @@ -10,10 +10,10 @@ library AddressUtils { * Returns whether the target address is a contract * @dev This function will return false if invoked during the constructor of a contract, * as the code is not actually created until after the constructor finishes. - * @param _addr address to check + * @param _account address of the account to check * @return whether the target address is a contract */ - function isContract(address _addr) internal view returns (bool) { + function isContract(address _account) internal view returns (bool) { uint256 size; // XXX Currently there is no better way to check if there is a contract in an address // than to check the size of the code at that address. @@ -22,7 +22,7 @@ library AddressUtils { // TODO Check this again before the Serenity release, because all addresses will be // contracts then. // solium-disable-next-line security/no-inline-assembly - assembly { size := extcodesize(_addr) } + assembly { size := extcodesize(_account) } return size > 0; } diff --git a/contracts/ECRecovery.sol b/contracts/ECRecovery.sol index 5b566c98e..66c494507 100644 --- a/contracts/ECRecovery.sol +++ b/contracts/ECRecovery.sol @@ -13,9 +13,9 @@ library ECRecovery { /** * @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 _signature bytes signature, the signature is generated using web3.eth.sign() */ - function recover(bytes32 _hash, bytes _sig) + function recover(bytes32 _hash, bytes _signature) internal pure returns (address) @@ -25,7 +25,7 @@ library ECRecovery { uint8 v; // Check the signature length - if (_sig.length != 65) { + if (_signature.length != 65) { return (address(0)); } @@ -34,9 +34,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(_signature, 32)) + s := mload(add(_signature, 64)) + v := byte(0, mload(add(_signature, 96))) } // Version of signature should be 27 or 28, but 0 and 1 are also possible versions diff --git a/contracts/access/SignatureBouncer.sol b/contracts/access/SignatureBouncer.sol index 9514369aa..7741bbdca 100644 --- a/contracts/access/SignatureBouncer.sol +++ b/contracts/access/SignatureBouncer.sol @@ -22,10 +22,10 @@ import "../ECRecovery.sol"; * `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 + * @notice A method that uses the `onlyValidSignatureAndData` modifier must make the _signature * 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. + * length of the _signature data and the _signaature 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. */ @@ -40,27 +40,27 @@ contract SignatureBouncer is Ownable, RBAC { /** * @dev requires that a valid signature of a bouncer was provided */ - modifier onlyValidSignature(bytes _sig) + modifier onlyValidSignature(bytes _signature) { - require(isValidSignature(msg.sender, _sig)); + require(isValidSignature(msg.sender, _signature)); _; } /** * @dev requires that a valid signature with a specifed method of a bouncer was provided */ - modifier onlyValidSignatureAndMethod(bytes _sig) + modifier onlyValidSignatureAndMethod(bytes _signature) { - require(isValidSignatureAndMethod(msg.sender, _sig)); + require(isValidSignatureAndMethod(msg.sender, _signature)); _; } /** * @dev requires that a valid signature with a specifed method and params of a bouncer was provided */ - modifier onlyValidSignatureAndData(bytes _sig) + modifier onlyValidSignatureAndData(bytes _signature) { - require(isValidSignatureAndData(msg.sender, _sig)); + require(isValidSignatureAndData(msg.sender, _signature)); _; } @@ -90,14 +90,14 @@ contract SignatureBouncer is Ownable, RBAC { * @dev is the signature of `this + sender` from a bouncer? * @return bool */ - function isValidSignature(address _address, bytes _sig) + function isValidSignature(address _address, bytes _signature) internal view returns (bool) { return isValidDataHash( keccak256(abi.encodePacked(address(this), _address)), - _sig + _signature ); } @@ -105,7 +105,7 @@ contract SignatureBouncer is Ownable, RBAC { * @dev is the signature of `this + sender + methodId` from a bouncer? * @return bool */ - function isValidSignatureAndMethod(address _address, bytes _sig) + function isValidSignatureAndMethod(address _address, bytes _signature) internal view returns (bool) @@ -116,16 +116,16 @@ contract SignatureBouncer is Ownable, RBAC { } return isValidDataHash( keccak256(abi.encodePacked(address(this), _address, data)), - _sig + _signature ); } /** * @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 + * @notice the _signature parameter of the method being validated must be the "last" parameter * @return bool */ - function isValidSignatureAndData(address _address, bytes _sig) + function isValidSignatureAndData(address _address, bytes _signature) internal view returns (bool) @@ -137,7 +137,7 @@ contract SignatureBouncer is Ownable, RBAC { } return isValidDataHash( keccak256(abi.encodePacked(address(this), _address, data)), - _sig + _signature ); } @@ -146,14 +146,14 @@ contract SignatureBouncer is Ownable, RBAC { * and then recover the signature and check it against the bouncer role * @return bool */ - function isValidDataHash(bytes32 _hash, bytes _sig) + function isValidDataHash(bytes32 _hash, bytes _signature) internal view returns (bool) { address signer = _hash .toEthSignedMessageHash() - .recover(_sig); + .recover(_signature); return hasRole(signer, ROLE_BOUNCER); } } diff --git a/contracts/access/rbac/Roles.sol b/contracts/access/rbac/Roles.sol index e9ad58450..de4689409 100644 --- a/contracts/access/rbac/Roles.sol +++ b/contracts/access/rbac/Roles.sol @@ -13,43 +13,43 @@ library Roles { } /** - * @dev give an address access to this role + * @dev give an account access to this role */ - function add(Role storage _role, address _addr) + function add(Role storage _role, address _account) internal { - _role.bearer[_addr] = true; + _role.bearer[_account] = true; } /** - * @dev remove an address' access to this role + * @dev remove an account's access to this role */ - function remove(Role storage _role, address _addr) + function remove(Role storage _role, address _account) internal { - _role.bearer[_addr] = false; + _role.bearer[_account] = false; } /** - * @dev check if an address has this role + * @dev check if an account has this role * // reverts */ - function check(Role storage _role, address _addr) + function check(Role storage _role, address _account) internal view { - require(has(_role, _addr)); + require(has(_role, _account)); } /** - * @dev check if an address has this role + * @dev check if an account has this role * @return bool */ - function has(Role storage _role, address _addr) + function has(Role storage _role, address _account) internal view returns (bool) { - return _role.bearer[_addr]; + return _role.bearer[_account]; } } diff --git a/contracts/examples/RBACWithAdmin.sol b/contracts/examples/RBACWithAdmin.sol index 8b8d18887..479bb95ac 100644 --- a/contracts/examples/RBACWithAdmin.sol +++ b/contracts/examples/RBACWithAdmin.sol @@ -41,26 +41,26 @@ contract RBACWithAdmin is RBAC { } /** - * @dev add a role to an address - * @param _addr address + * @dev add a role to an account + * @param _account the account that will have the role * @param _roleName the name of the role */ - function adminAddRole(address _addr, string _roleName) + function adminAddRole(address _account, string _roleName) public onlyAdmin { - addRole(_addr, _roleName); + addRole(_account, _roleName); } /** - * @dev remove a role from an address - * @param _addr address + * @dev remove a role from an account + * @param _account the account that will no longer have the role * @param _roleName the name of the role */ - function adminRemoveRole(address _addr, string _roleName) + function adminRemoveRole(address _account, string _roleName) public onlyAdmin { - removeRole(_addr, _roleName); + removeRole(_account, _roleName); } } diff --git a/contracts/mocks/BouncerMock.sol b/contracts/mocks/BouncerMock.sol index 0d43ff3fb..e5bd31a10 100644 --- a/contracts/mocks/BouncerMock.sol +++ b/contracts/mocks/BouncerMock.sol @@ -4,33 +4,33 @@ import "../access/SignatureBouncer.sol"; contract SignatureBouncerMock is SignatureBouncer { - function checkValidSignature(address _address, bytes _sig) + function checkValidSignature(address _address, bytes _signature) public view returns (bool) { - return isValidSignature(_address, _sig); + return isValidSignature(_address, _signature); } - function onlyWithValidSignature(bytes _sig) + function onlyWithValidSignature(bytes _signature) public - onlyValidSignature(_sig) + onlyValidSignature(_signature) view { } - function checkValidSignatureAndMethod(address _address, bytes _sig) + function checkValidSignatureAndMethod(address _address, bytes _signature) public view returns (bool) { - return isValidSignatureAndMethod(_address, _sig); + return isValidSignatureAndMethod(_address, _signature); } - function onlyWithValidSignatureAndMethod(bytes _sig) + function onlyWithValidSignatureAndMethod(bytes _signature) public - onlyValidSignatureAndMethod(_sig) + onlyValidSignatureAndMethod(_signature) view { @@ -40,18 +40,18 @@ contract SignatureBouncerMock is SignatureBouncer { address _address, bytes, uint, - bytes _sig + bytes _signature ) public view returns (bool) { - return isValidSignatureAndData(_address, _sig); + return isValidSignatureAndData(_address, _signature); } - function onlyWithValidSignatureAndData(uint, bytes _sig) + function onlyWithValidSignatureAndData(uint, bytes _signature) public - onlyValidSignatureAndData(_sig) + onlyValidSignatureAndData(_signature) view { diff --git a/contracts/mocks/ECRecoveryMock.sol b/contracts/mocks/ECRecoveryMock.sol index 83cfeabaf..a2c5ebcd8 100644 --- a/contracts/mocks/ECRecoveryMock.sol +++ b/contracts/mocks/ECRecoveryMock.sol @@ -7,12 +7,12 @@ import "../ECRecovery.sol"; contract ECRecoveryMock { using ECRecovery for bytes32; - function recover(bytes32 _hash, bytes _sig) + function recover(bytes32 _hash, bytes _signature) public pure returns (address) { - return _hash.recover(_sig); + return _hash.recover(_signature); } function toEthSignedMessageHash(bytes32 _hash) diff --git a/contracts/mocks/RBACMock.sol b/contracts/mocks/RBACMock.sol index ef4dcc4f5..7c2dbf79f 100644 --- a/contracts/mocks/RBACMock.sol +++ b/contracts/mocks/RBACMock.sol @@ -55,15 +55,15 @@ contract RBACMock is RBACWithAdmin { } // admins can remove advisor's role - function removeAdvisor(address _addr) + function removeAdvisor(address _account) public onlyAdmin { // revert if the user isn't an advisor // (perhaps you want to soft-fail here instead?) - checkRole(_addr, ROLE_ADVISOR); + checkRole(_account, ROLE_ADVISOR); // remove the advisor's role - removeRole(_addr, ROLE_ADVISOR); + removeRole(_account, ROLE_ADVISOR); } } diff --git a/contracts/ownership/Superuser.sol b/contracts/ownership/Superuser.sol index 690dbf0b9..ec8120d67 100644 --- a/contracts/ownership/Superuser.sol +++ b/contracts/ownership/Superuser.sol @@ -32,14 +32,14 @@ contract Superuser is Ownable, RBAC { } /** - * @dev getter to determine if address has superuser role + * @dev getter to determine if an account has superuser role */ - function isSuperuser(address _addr) + function isSuperuser(address _account) public view returns (bool) { - return hasRole(_addr, ROLE_SUPERUSER); + return hasRole(_account, ROLE_SUPERUSER); } /**