Commit f4972157 by Leo Arias Committed by Nicolás Venturo

Remove abbreviations from parameters (#1142)

* Add an initial document for our code style

* Remove abbreviations from parameters

* Rename the param in AddressUtils

* fix comment
parent 7fdca7b0
...@@ -14,6 +14,8 @@ Any exception or additions specific to our project are documented below. ...@@ -14,6 +14,8 @@ Any exception or additions specific to our project are documented below.
### Naming ### Naming
* Try to avoid acronyms and abbreviations.
* Parameters must be prefixed with an underscore. * Parameters must be prefixed with an underscore.
``` ```
......
...@@ -10,10 +10,10 @@ library AddressUtils { ...@@ -10,10 +10,10 @@ library AddressUtils {
* Returns whether the target address is a contract * Returns whether the target address is a contract
* @dev This function will return false if invoked during the constructor of 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. * 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 * @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; uint256 size;
// XXX Currently there is no better way to check if there is a contract in an address // 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. // than to check the size of the code at that address.
...@@ -22,7 +22,7 @@ library AddressUtils { ...@@ -22,7 +22,7 @@ library AddressUtils {
// TODO Check this again before the Serenity release, because all addresses will be // TODO Check this again before the Serenity release, because all addresses will be
// contracts then. // contracts then.
// solium-disable-next-line security/no-inline-assembly // solium-disable-next-line security/no-inline-assembly
assembly { size := extcodesize(_addr) } assembly { size := extcodesize(_account) }
return size > 0; return size > 0;
} }
......
...@@ -13,9 +13,9 @@ library ECRecovery { ...@@ -13,9 +13,9 @@ library ECRecovery {
/** /**
* @dev Recover signer address from a message by using their signature * @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 _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 internal
pure pure
returns (address) returns (address)
...@@ -25,7 +25,7 @@ library ECRecovery { ...@@ -25,7 +25,7 @@ library ECRecovery {
uint8 v; uint8 v;
// Check the signature length // Check the signature length
if (_sig.length != 65) { if (_signature.length != 65) {
return (address(0)); return (address(0));
} }
...@@ -34,9 +34,9 @@ library ECRecovery { ...@@ -34,9 +34,9 @@ library ECRecovery {
// currently is to use assembly. // currently is to use assembly.
// solium-disable-next-line security/no-inline-assembly // solium-disable-next-line security/no-inline-assembly
assembly { assembly {
r := mload(add(_sig, 32)) r := mload(add(_signature, 32))
s := mload(add(_sig, 64)) s := mload(add(_signature, 64))
v := byte(0, mload(add(_sig, 96))) v := byte(0, mload(add(_signature, 96)))
} }
// Version of signature should be 27 or 28, but 0 and 1 are also possible versions // Version of signature should be 27 or 28, but 0 and 1 are also possible versions
......
...@@ -22,10 +22,10 @@ import "../ECRecovery.sol"; ...@@ -22,10 +22,10 @@ import "../ECRecovery.sol";
* `onlyValidSignatureAndData` can be used to restrict access to only a given method * `onlyValidSignatureAndData` can be used to restrict access to only a given method
* or a given method with given parameters respectively. * or a given method with given parameters respectively.
* See the tests Bouncer.test.js for specific usage examples. * 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 * 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 * 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 * 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. * much more complex. See https://ethereum.stackexchange.com/a/50616 for more details.
*/ */
...@@ -40,27 +40,27 @@ contract SignatureBouncer is Ownable, RBAC { ...@@ -40,27 +40,27 @@ contract SignatureBouncer is Ownable, RBAC {
/** /**
* @dev requires that a valid signature of a bouncer was provided * @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 * @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 * @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 { ...@@ -90,14 +90,14 @@ contract SignatureBouncer is Ownable, RBAC {
* @dev is the signature of `this + sender` from a bouncer? * @dev is the signature of `this + sender` from a bouncer?
* @return bool * @return bool
*/ */
function isValidSignature(address _address, bytes _sig) function isValidSignature(address _address, bytes _signature)
internal internal
view view
returns (bool) returns (bool)
{ {
return isValidDataHash( return isValidDataHash(
keccak256(abi.encodePacked(address(this), _address)), keccak256(abi.encodePacked(address(this), _address)),
_sig _signature
); );
} }
...@@ -105,7 +105,7 @@ contract SignatureBouncer is Ownable, RBAC { ...@@ -105,7 +105,7 @@ contract SignatureBouncer is Ownable, RBAC {
* @dev is the signature of `this + sender + methodId` from a bouncer? * @dev is the signature of `this + sender + methodId` from a bouncer?
* @return bool * @return bool
*/ */
function isValidSignatureAndMethod(address _address, bytes _sig) function isValidSignatureAndMethod(address _address, bytes _signature)
internal internal
view view
returns (bool) returns (bool)
...@@ -116,16 +116,16 @@ contract SignatureBouncer is Ownable, RBAC { ...@@ -116,16 +116,16 @@ contract SignatureBouncer is Ownable, RBAC {
} }
return isValidDataHash( return isValidDataHash(
keccak256(abi.encodePacked(address(this), _address, data)), keccak256(abi.encodePacked(address(this), _address, data)),
_sig _signature
); );
} }
/** /**
* @dev is the signature of `this + sender + methodId + params(s)` from a bouncer? * @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 * @return bool
*/ */
function isValidSignatureAndData(address _address, bytes _sig) function isValidSignatureAndData(address _address, bytes _signature)
internal internal
view view
returns (bool) returns (bool)
...@@ -137,7 +137,7 @@ contract SignatureBouncer is Ownable, RBAC { ...@@ -137,7 +137,7 @@ contract SignatureBouncer is Ownable, RBAC {
} }
return isValidDataHash( return isValidDataHash(
keccak256(abi.encodePacked(address(this), _address, data)), keccak256(abi.encodePacked(address(this), _address, data)),
_sig _signature
); );
} }
...@@ -146,14 +146,14 @@ contract SignatureBouncer is Ownable, RBAC { ...@@ -146,14 +146,14 @@ contract SignatureBouncer is Ownable, RBAC {
* and then recover the signature and check it against the bouncer role * and then recover the signature and check it against the bouncer role
* @return bool * @return bool
*/ */
function isValidDataHash(bytes32 _hash, bytes _sig) function isValidDataHash(bytes32 _hash, bytes _signature)
internal internal
view view
returns (bool) returns (bool)
{ {
address signer = _hash address signer = _hash
.toEthSignedMessageHash() .toEthSignedMessageHash()
.recover(_sig); .recover(_signature);
return hasRole(signer, ROLE_BOUNCER); return hasRole(signer, ROLE_BOUNCER);
} }
} }
...@@ -13,43 +13,43 @@ library Roles { ...@@ -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 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 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 * // reverts
*/ */
function check(Role storage _role, address _addr) function check(Role storage _role, address _account)
internal internal
view 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 * @return bool
*/ */
function has(Role storage _role, address _addr) function has(Role storage _role, address _account)
internal internal
view view
returns (bool) returns (bool)
{ {
return _role.bearer[_addr]; return _role.bearer[_account];
} }
} }
...@@ -41,26 +41,26 @@ contract RBACWithAdmin is RBAC { ...@@ -41,26 +41,26 @@ contract RBACWithAdmin is RBAC {
} }
/** /**
* @dev add a role to an address * @dev add a role to an account
* @param _addr address * @param _account the account that will have the role
* @param _roleName the name of the role * @param _roleName the name of the role
*/ */
function adminAddRole(address _addr, string _roleName) function adminAddRole(address _account, string _roleName)
public public
onlyAdmin onlyAdmin
{ {
addRole(_addr, _roleName); addRole(_account, _roleName);
} }
/** /**
* @dev remove a role from an address * @dev remove a role from an account
* @param _addr address * @param _account the account that will no longer have the role
* @param _roleName the name of the role * @param _roleName the name of the role
*/ */
function adminRemoveRole(address _addr, string _roleName) function adminRemoveRole(address _account, string _roleName)
public public
onlyAdmin onlyAdmin
{ {
removeRole(_addr, _roleName); removeRole(_account, _roleName);
} }
} }
...@@ -4,33 +4,33 @@ import "../access/SignatureBouncer.sol"; ...@@ -4,33 +4,33 @@ import "../access/SignatureBouncer.sol";
contract SignatureBouncerMock is SignatureBouncer { contract SignatureBouncerMock is SignatureBouncer {
function checkValidSignature(address _address, bytes _sig) function checkValidSignature(address _address, bytes _signature)
public public
view view
returns (bool) returns (bool)
{ {
return isValidSignature(_address, _sig); return isValidSignature(_address, _signature);
} }
function onlyWithValidSignature(bytes _sig) function onlyWithValidSignature(bytes _signature)
public public
onlyValidSignature(_sig) onlyValidSignature(_signature)
view view
{ {
} }
function checkValidSignatureAndMethod(address _address, bytes _sig) function checkValidSignatureAndMethod(address _address, bytes _signature)
public public
view view
returns (bool) returns (bool)
{ {
return isValidSignatureAndMethod(_address, _sig); return isValidSignatureAndMethod(_address, _signature);
} }
function onlyWithValidSignatureAndMethod(bytes _sig) function onlyWithValidSignatureAndMethod(bytes _signature)
public public
onlyValidSignatureAndMethod(_sig) onlyValidSignatureAndMethod(_signature)
view view
{ {
...@@ -40,18 +40,18 @@ contract SignatureBouncerMock is SignatureBouncer { ...@@ -40,18 +40,18 @@ contract SignatureBouncerMock is SignatureBouncer {
address _address, address _address,
bytes, bytes,
uint, uint,
bytes _sig bytes _signature
) )
public public
view view
returns (bool) returns (bool)
{ {
return isValidSignatureAndData(_address, _sig); return isValidSignatureAndData(_address, _signature);
} }
function onlyWithValidSignatureAndData(uint, bytes _sig) function onlyWithValidSignatureAndData(uint, bytes _signature)
public public
onlyValidSignatureAndData(_sig) onlyValidSignatureAndData(_signature)
view view
{ {
......
...@@ -7,12 +7,12 @@ import "../ECRecovery.sol"; ...@@ -7,12 +7,12 @@ import "../ECRecovery.sol";
contract ECRecoveryMock { contract ECRecoveryMock {
using ECRecovery for bytes32; using ECRecovery for bytes32;
function recover(bytes32 _hash, bytes _sig) function recover(bytes32 _hash, bytes _signature)
public public
pure pure
returns (address) returns (address)
{ {
return _hash.recover(_sig); return _hash.recover(_signature);
} }
function toEthSignedMessageHash(bytes32 _hash) function toEthSignedMessageHash(bytes32 _hash)
......
...@@ -55,15 +55,15 @@ contract RBACMock is RBACWithAdmin { ...@@ -55,15 +55,15 @@ contract RBACMock is RBACWithAdmin {
} }
// admins can remove advisor's role // admins can remove advisor's role
function removeAdvisor(address _addr) function removeAdvisor(address _account)
public public
onlyAdmin onlyAdmin
{ {
// revert if the user isn't an advisor // revert if the user isn't an advisor
// (perhaps you want to soft-fail here instead?) // (perhaps you want to soft-fail here instead?)
checkRole(_addr, ROLE_ADVISOR); checkRole(_account, ROLE_ADVISOR);
// remove the advisor's role // remove the advisor's role
removeRole(_addr, ROLE_ADVISOR); removeRole(_account, ROLE_ADVISOR);
} }
} }
...@@ -32,14 +32,14 @@ contract Superuser is Ownable, RBAC { ...@@ -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 public
view view
returns (bool) returns (bool)
{ {
return hasRole(_addr, ROLE_SUPERUSER); return hasRole(_account, ROLE_SUPERUSER);
} }
/** /**
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment