Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions CODE_STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,22 @@ Any exception or additions specific to our project are documented below.

```
function test() {
uint256 functionVar;
...
uint256 functionVar;
...
}
```

* Internal and private functions should have an underscore prefix.

```
function _testInternal() internal {
...
}
```

```
function _testPrivate() private {
...
}
```

Expand Down
6 changes: 3 additions & 3 deletions contracts/Bounty.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ contract Bounty is PullPayment, Destructible {
* @return A target contract
*/
function createTarget() public returns(Target) {
Target target = Target(deployContract());
Target target = Target(_deployContract());
researchers[target] = msg.sender;
emit TargetCreated(target);
return target;
Expand All @@ -43,15 +43,15 @@ contract Bounty is PullPayment, Destructible {
require(researcher != address(0));
// Check Target contract invariants
require(!_target.checkInvariant());
asyncTransfer(researcher, address(this).balance);
_asyncTransfer(researcher, address(this).balance);
claimed = true;
}

/**
* @dev Internal function to deploy the target contract.
* @return A target contract address
*/
function deployContract() internal returns(address);
function _deployContract() internal returns(address);

}

Expand Down
26 changes: 13 additions & 13 deletions contracts/access/SignatureBouncer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import "../ECRecovery.sol";
* 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).
* `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.
Expand All @@ -42,7 +42,7 @@ contract SignatureBouncer is Ownable, RBAC {
*/
modifier onlyValidSignature(bytes _signature)
{
require(isValidSignature(msg.sender, _signature));
require(_isValidSignature(msg.sender, _signature));
_;
}

Expand All @@ -51,7 +51,7 @@ contract SignatureBouncer is Ownable, RBAC {
*/
modifier onlyValidSignatureAndMethod(bytes _signature)
{
require(isValidSignatureAndMethod(msg.sender, _signature));
require(_isValidSignatureAndMethod(msg.sender, _signature));
_;
}

Expand All @@ -60,7 +60,7 @@ contract SignatureBouncer is Ownable, RBAC {
*/
modifier onlyValidSignatureAndData(bytes _signature)
{
require(isValidSignatureAndData(msg.sender, _signature));
require(_isValidSignatureAndData(msg.sender, _signature));
_;
}

Expand All @@ -72,7 +72,7 @@ contract SignatureBouncer is Ownable, RBAC {
onlyOwner
{
require(_bouncer != address(0));
addRole(_bouncer, ROLE_BOUNCER);
_addRole(_bouncer, ROLE_BOUNCER);
}

/**
Expand All @@ -82,19 +82,19 @@ contract SignatureBouncer is Ownable, RBAC {
public
onlyOwner
{
removeRole(_bouncer, ROLE_BOUNCER);
_removeRole(_bouncer, ROLE_BOUNCER);
}

/**
* @dev is the signature of `this + sender` from a bouncer?
* @return bool
*/
function isValidSignature(address _address, bytes _signature)
function _isValidSignature(address _address, bytes _signature)
internal
view
returns (bool)
{
return isValidDataHash(
return _isValidDataHash(
keccak256(abi.encodePacked(address(this), _address)),
_signature
);
Expand All @@ -104,7 +104,7 @@ contract SignatureBouncer is Ownable, RBAC {
* @dev is the signature of `this + sender + methodId` from a bouncer?
* @return bool
*/
function isValidSignatureAndMethod(address _address, bytes _signature)
function _isValidSignatureAndMethod(address _address, bytes _signature)
internal
view
returns (bool)
Expand All @@ -113,7 +113,7 @@ contract SignatureBouncer is Ownable, RBAC {
for (uint i = 0; i < data.length; i++) {
data[i] = msg.data[i];
}
return isValidDataHash(
return _isValidDataHash(
keccak256(abi.encodePacked(address(this), _address, data)),
_signature
);
Expand All @@ -124,7 +124,7 @@ contract SignatureBouncer is Ownable, RBAC {
* @notice the _signature parameter of the method being validated must be the "last" parameter
* @return bool
*/
function isValidSignatureAndData(address _address, bytes _signature)
function _isValidSignatureAndData(address _address, bytes _signature)
internal
view
returns (bool)
Expand All @@ -134,7 +134,7 @@ contract SignatureBouncer is Ownable, RBAC {
for (uint i = 0; i < data.length; i++) {
data[i] = msg.data[i];
}
return isValidDataHash(
return _isValidDataHash(
keccak256(abi.encodePacked(address(this), _address, data)),
_signature
);
Expand All @@ -145,7 +145,7 @@ contract SignatureBouncer is Ownable, RBAC {
* and then recover the signature and check it against the bouncer role
* @return bool
*/
function isValidDataHash(bytes32 _hash, bytes _signature)
function _isValidDataHash(bytes32 _hash, bytes _signature)
internal
view
returns (bool)
Expand Down
4 changes: 2 additions & 2 deletions contracts/access/Whitelist.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ contract Whitelist is Ownable, RBAC {
public
onlyOwner
{
addRole(_operator, ROLE_WHITELISTED);
_addRole(_operator, ROLE_WHITELISTED);
}

/**
Expand Down Expand Up @@ -70,7 +70,7 @@ contract Whitelist is Ownable, RBAC {
public
onlyOwner
{
removeRole(_operator, ROLE_WHITELISTED);
_removeRole(_operator, ROLE_WHITELISTED);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions contracts/access/rbac/RBAC.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ contract RBAC {
* @param _operator address
* @param _role the name of the role
*/
function addRole(address _operator, string _role)
function _addRole(address _operator, string _role)
internal
{
roles[_role].add(_operator);
Expand All @@ -64,7 +64,7 @@ contract RBAC {
* @param _operator address
* @param _role the name of the role
*/
function removeRole(address _operator, string _role)
function _removeRole(address _operator, string _role)
internal
{
roles[_role].remove(_operator);
Expand Down
6 changes: 3 additions & 3 deletions contracts/crowdsale/distribution/FinalizableCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,18 @@ contract FinalizableCrowdsale is Ownable, TimedCrowdsale {
require(!isFinalized);
require(hasClosed());

finalization();
_finalization();
emit CrowdsaleFinalized();

isFinalized = true;
}

/**
* @dev Can be overridden to add finalization logic. The overriding function
* should call super.finalization() to ensure the chain of finalization is
* should call super._finalization() to ensure the chain of finalization is
* executed entirely.
*/
function finalization() internal {
function _finalization() internal {
}

}
4 changes: 2 additions & 2 deletions contracts/crowdsale/distribution/RefundableCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ contract RefundableCrowdsale is FinalizableCrowdsale {
/**
* @dev escrow finalization task, called when owner calls finalize()
*/
function finalization() internal {
function _finalization() internal {
if (goalReached()) {
escrow_.close();
escrow_.beneficiaryWithdraw();
} else {
escrow_.enableRefunds();
}

super.finalization();
super._finalization();
}

/**
Expand Down
6 changes: 3 additions & 3 deletions contracts/examples/RBACWithAdmin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ contract RBACWithAdmin is RBAC {
constructor()
public
{
addRole(msg.sender, ROLE_ADMIN);
_addRole(msg.sender, ROLE_ADMIN);
}

/**
Expand All @@ -49,7 +49,7 @@ contract RBACWithAdmin is RBAC {
public
onlyAdmin
{
addRole(_account, _roleName);
_addRole(_account, _roleName);
}

/**
Expand All @@ -61,6 +61,6 @@ contract RBACWithAdmin is RBAC {
public
onlyAdmin
{
removeRole(_account, _roleName);
_removeRole(_account, _roleName);
}
}
6 changes: 3 additions & 3 deletions contracts/mocks/BouncerMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ contract SignatureBouncerMock is SignatureBouncer {
view
returns (bool)
{
return isValidSignature(_address, _signature);
return _isValidSignature(_address, _signature);
}

function onlyWithValidSignature(bytes _signature)
Expand All @@ -25,7 +25,7 @@ contract SignatureBouncerMock is SignatureBouncer {
view
returns (bool)
{
return isValidSignatureAndMethod(_address, _signature);
return _isValidSignatureAndMethod(_address, _signature);
}

function onlyWithValidSignatureAndMethod(bytes _signature)
Expand All @@ -46,7 +46,7 @@ contract SignatureBouncerMock is SignatureBouncer {
view
returns (bool)
{
return isValidSignatureAndData(_address, _signature);
return _isValidSignatureAndData(_address, _signature);
}

function onlyWithValidSignatureAndData(uint, bytes _signature)
Expand Down
4 changes: 2 additions & 2 deletions contracts/mocks/ERC721TokenMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ contract ERC721TokenMock is ERC721Token {
super._setTokenURI(_tokenId, _uri);
}

function _removeTokenFrom(address _from, uint256 _tokenId) public {
super.removeTokenFrom(_from, _tokenId);
function removeTokenFrom(address _from, uint256 _tokenId) public {
super._removeTokenFrom(_from, _tokenId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think super. is not needed here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

supers removed. This is ready for a new review.

}
}
2 changes: 1 addition & 1 deletion contracts/mocks/InsecureTargetBounty.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ contract InsecureTargetMock is Target {


contract InsecureTargetBounty is Bounty {
function deployContract() internal returns (address) {
function _deployContract() internal returns (address) {
return new InsecureTargetMock();
}
}
2 changes: 1 addition & 1 deletion contracts/mocks/PullPaymentMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ contract PullPaymentMock is PullPayment {

// test helper function to call asyncTransfer
function callTransfer(address _dest, uint256 _amount) public {
asyncTransfer(_dest, _amount);
_asyncTransfer(_dest, _amount);
}

}
6 changes: 3 additions & 3 deletions contracts/mocks/RBACMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ contract RBACMock is RBACWithAdmin {
constructor(address[] _advisors)
public
{
addRole(msg.sender, ROLE_ADVISOR);
_addRole(msg.sender, ROLE_ADVISOR);

for (uint256 i = 0; i < _advisors.length; i++) {
addRole(_advisors[i], ROLE_ADVISOR);
_addRole(_advisors[i], ROLE_ADVISOR);
}
}

Expand Down Expand Up @@ -64,6 +64,6 @@ contract RBACMock is RBACWithAdmin {
checkRole(_account, ROLE_ADVISOR);

// remove the advisor's role
removeRole(_account, ROLE_ADVISOR);
_removeRole(_account, ROLE_ADVISOR);
}
}
2 changes: 1 addition & 1 deletion contracts/mocks/SecureTargetBounty.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ contract SecureTargetMock is Target {


contract SecureTargetBounty is Bounty {
function deployContract() internal returns (address) {
function _deployContract() internal returns (address) {
return new SecureTargetMock();
}
}
6 changes: 3 additions & 3 deletions contracts/ownership/Heritable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ contract Heritable is Ownable {
* have to wait for `heartbeatTimeout` seconds.
*/
function proclaimDeath() public onlyHeir {
require(ownerLives());
require(_ownerLives());
emit OwnerProclaimedDead(owner, heir_, timeOfDeath_);
// solium-disable-next-line security/no-block-members
timeOfDeath_ = block.timestamp;
Expand All @@ -104,7 +104,7 @@ contract Heritable is Ownable {
* @dev Allows heir to transfer ownership only if heartbeat has timed out.
*/
function claimHeirOwnership() public onlyHeir {
require(!ownerLives());
require(!_ownerLives());
// solium-disable-next-line security/no-block-members
require(block.timestamp >= timeOfDeath_ + heartbeatTimeout_);
emit OwnershipTransferred(owner, heir_);
Expand All @@ -113,7 +113,7 @@ contract Heritable is Ownable {
timeOfDeath_ = 0;
}

function ownerLives() internal view returns (bool) {
function _ownerLives() internal view returns (bool) {
return timeOfDeath_ == 0;
}
}
6 changes: 3 additions & 3 deletions contracts/ownership/Superuser.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ contract Superuser is Ownable, RBAC {
string public constant ROLE_SUPERUSER = "superuser";

constructor () public {
addRole(msg.sender, ROLE_SUPERUSER);
_addRole(msg.sender, ROLE_SUPERUSER);
}

/**
Expand Down Expand Up @@ -48,8 +48,8 @@ contract Superuser is Ownable, RBAC {
*/
function transferSuperuser(address _newSuperuser) public onlySuperuser {
require(_newSuperuser != address(0));
removeRole(msg.sender, ROLE_SUPERUSER);
addRole(_newSuperuser, ROLE_SUPERUSER);
_removeRole(msg.sender, ROLE_SUPERUSER);
_addRole(_newSuperuser, ROLE_SUPERUSER);
}

/**
Expand Down
Loading