Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added message string for require() #1704

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
9f3096c
Error handling in ERC20 and ERC721
princesinha19 Apr 1, 2019
cd6274a
Added message string for require.
balajipachai Apr 1, 2019
111b790
Fixed solhint errors.
balajipachai Apr 1, 2019
92d580a
Updated PR as per issue #1709
balajipachai Apr 6, 2019
933ea08
Merge branch 'master' into enhancement/message-string-for-require
balajipachai Apr 6, 2019
589b407
changes as per #1709 and openzeppelin forum.
princesinha19 Apr 9, 2019
9f92066
Changes in require statement
princesinha19 Apr 10, 2019
1d6c2cb
Changes in require statement
princesinha19 Apr 10, 2019
f6c6360
build pipeline fix
princesinha19 Apr 10, 2019
9d0d54a
Merge branch 'master' into enhancement/message-string-for-require
nventuro Apr 12, 2019
4aafb81
Changes as per @nventuro's comment.
balajipachai Apr 13, 2019
04ce66d
Update revert reason strings.
nventuro Apr 15, 2019
13df19d
Merge remote-tracking branch 'princesinha19/master' into enhancement/…
nventuro Apr 16, 2019
9486ebd
Fianal update of revert reason strings.
nventuro Apr 16, 2019
488bb62
WIP: Updating reason strings in test cases
Apr 19, 2019
797bbeb
WIP: Added changes to ERC20 and ERC721
Apr 19, 2019
c804239
Fixes linting errors in *.tes.js files
balajipachai Apr 20, 2019
92133e0
Achieved 100% code coverage
balajipachai Apr 20, 2019
9ea1f50
Updated the test cases with shouldFail.reverting.withMessage()
balajipachai Apr 20, 2019
bb0e356
Merge branch 'master' into enhancement/message-string-for-require
balajipachai Apr 20, 2019
a946483
Fix package-lock.
nventuro Apr 22, 2019
3cd9d5b
address review comments
frangio Apr 23, 2019
a0eda42
fix linter issues
frangio Apr 23, 2019
1b9c2f1
Merge branch 'master' into enhancement/message-string-for-require
nventuro Apr 23, 2019
fb2aa30
fix remaining revert reasons
frangio Apr 23, 2019
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
8 changes: 3 additions & 5 deletions contracts/access/Roles.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,15 @@ library Roles {
* @dev Give an account access to this role.
*/
function add(Role storage role, address account) internal {
require(!has(role, account));

require(!has(role, account), "Roles: account already has role");
role.bearer[account] = true;
}

/**
* @dev Remove an account's access to this role.
*/
function remove(Role storage role, address account) internal {
require(has(role, account));

require(has(role, account), "Roles: account does not have role");
role.bearer[account] = false;
}

Expand All @@ -32,7 +30,7 @@ library Roles {
* @return bool
*/
function has(Role storage role, address account) internal view returns (bool) {
require(account != address(0));
require(account != address(0), "Roles: account is the zero address");
return role.bearer[account];
}
}
2 changes: 1 addition & 1 deletion contracts/access/roles/CapperRole.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ contract CapperRole {
}

modifier onlyCapper() {
require(isCapper(msg.sender));
require(isCapper(msg.sender), "CapperRole: caller does not have the Capper role");
_;
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/access/roles/MinterRole.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ contract MinterRole {
}

modifier onlyMinter() {
require(isMinter(msg.sender));
require(isMinter(msg.sender), "MinterRole: caller does not have the Minter role");
_;
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/access/roles/PauserRole.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ contract PauserRole {
}

modifier onlyPauser() {
require(isPauser(msg.sender));
require(isPauser(msg.sender), "PauserRole: caller does not have the Pauser role");
_;
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/access/roles/SignerRole.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ contract SignerRole {
}

modifier onlySigner() {
require(isSigner(msg.sender));
require(isSigner(msg.sender), "SignerRole: caller does not have the Signer role");
_;
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/access/roles/WhitelistAdminRole.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ contract WhitelistAdminRole {
}

modifier onlyWhitelistAdmin() {
require(isWhitelistAdmin(msg.sender));
require(isWhitelistAdmin(msg.sender), "WhitelistAdminRole: caller does not have the WhitelistAdmin role");
_;
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/access/roles/WhitelistedRole.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ contract WhitelistedRole is WhitelistAdminRole {
Roles.Role private _whitelisteds;

modifier onlyWhitelisted() {
require(isWhitelisted(msg.sender));
require(isWhitelisted(msg.sender), "WhitelistedRole: caller does not have the Whitelisted role");
_;
}

Expand Down
10 changes: 5 additions & 5 deletions contracts/crowdsale/Crowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ contract Crowdsale is ReentrancyGuard {
* @param token Address of the token being sold
*/
constructor (uint256 rate, address payable wallet, IERC20 token) public {
require(rate > 0);
require(wallet != address(0));
require(address(token) != address(0));
require(rate > 0, "Crowdsale: rate is 0");
require(wallet != address(0), "Crowdsale: wallet is the zero address");
require(address(token) != address(0), "Crowdsale: token is the zero address");

_rate = rate;
_wallet = wallet;
Expand Down Expand Up @@ -136,8 +136,8 @@ contract Crowdsale is ReentrancyGuard {
* @param weiAmount Value in wei involved in the purchase
*/
function _preValidatePurchase(address beneficiary, uint256 weiAmount) internal view {
require(beneficiary != address(0));
require(weiAmount != 0);
require(beneficiary != address(0), "Crowdsale: beneficiary is the zero address");
require(weiAmount != 0, "Crowdsale: weiAmount is 0");
}

/**
Expand Down
4 changes: 2 additions & 2 deletions contracts/crowdsale/distribution/FinalizableCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ contract FinalizableCrowdsale is TimedCrowdsale {
* work. Calls the contract's finalization function.
*/
function finalize() public {
require(!_finalized);
require(hasClosed());
require(!_finalized, "FinalizableCrowdsale: already finalized");
require(hasClosed(), "FinalizableCrowdsale: not closed");
frangio marked this conversation as resolved.
Show resolved Hide resolved

_finalized = true;

Expand Down
4 changes: 2 additions & 2 deletions contracts/crowdsale/distribution/PostDeliveryCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ contract PostDeliveryCrowdsale is TimedCrowdsale {
* @param beneficiary Whose tokens will be withdrawn.
*/
function withdrawTokens(address beneficiary) public {
require(hasClosed());
require(hasClosed(), "PostDeliveryCrowdsale: not closed");
uint256 amount = _balances[beneficiary];
require(amount > 0);
require(amount > 0, "PostDeliveryCrowdsale: beneficiary is not due any tokens");

_balances[beneficiary] = 0;
_vault.transfer(token(), beneficiary, amount);
Expand Down
6 changes: 3 additions & 3 deletions contracts/crowdsale/distribution/RefundableCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale {
* @param goal Funding goal
*/
constructor (uint256 goal) public {
require(goal > 0);
require(goal > 0, "RefundableCrowdsale: goal is 0");
_escrow = new RefundEscrow(wallet());
_goal = goal;
}
Expand All @@ -45,8 +45,8 @@ contract RefundableCrowdsale is FinalizableCrowdsale {
* @param refundee Whose refund will be claimed.
*/
function claimRefund(address payable refundee) public {
require(finalized());
require(!goalReached());
require(finalized(), "RefundableCrowdsale: not finalized");
require(!goalReached(), "RefundableCrowdsale: goal reached");

_escrow.withdraw(refundee);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import "./PostDeliveryCrowdsale.sol";
*/
contract RefundablePostDeliveryCrowdsale is RefundableCrowdsale, PostDeliveryCrowdsale {
function withdrawTokens(address beneficiary) public {
require(finalized());
require(goalReached());
require(finalized(), "RefundablePostDeliveryCrowdsale: not finalized");
require(goalReached(), "RefundablePostDeliveryCrowdsale: goal not reached");

super.withdrawTokens(beneficiary);
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/crowdsale/emission/AllowanceCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ contract AllowanceCrowdsale is Crowdsale {
* @param tokenWallet Address holding the tokens, which has approved allowance to the crowdsale.
*/
constructor (address tokenWallet) public {
require(tokenWallet != address(0));
require(tokenWallet != address(0), "AllowanceCrowdsale: token wallet is the zero address");
_tokenWallet = tokenWallet;
}

Expand Down
5 changes: 4 additions & 1 deletion contracts/crowdsale/emission/MintedCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ contract MintedCrowdsale is Crowdsale {
*/
function _deliverTokens(address beneficiary, uint256 tokenAmount) internal {
// Potentially dangerous assumption about the type of the token.
require(ERC20Mintable(address(token())).mint(beneficiary, tokenAmount));
require(
ERC20Mintable(address(token())).mint(beneficiary, tokenAmount),
"MintedCrowdsale: minting failed"
);
}
}
7 changes: 4 additions & 3 deletions contracts/crowdsale/price/IncreasingPriceCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ contract IncreasingPriceCrowdsale is TimedCrowdsale {
* @param finalRate Number of tokens a buyer gets per wei at the end of the crowdsale
*/
constructor (uint256 initialRate, uint256 finalRate) public {
require(finalRate > 0);
require(initialRate > finalRate);
require(finalRate > 0, "IncreasingPriceCrowdsale: final rate is 0");
// solhint-disable-next-line max-line-length
require(initialRate > finalRate, "IncreasingPriceCrowdsale: initial rate is not greater than final rate");
_initialRate = initialRate;
_finalRate = finalRate;
}
Expand All @@ -32,7 +33,7 @@ contract IncreasingPriceCrowdsale is TimedCrowdsale {
* all calls to it are a mistake.
*/
function rate() public view returns (uint256) {
revert();
revert("IncreasingPriceCrowdsale: rate() called");
}

/**
Expand Down
4 changes: 2 additions & 2 deletions contracts/crowdsale/validation/CappedCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ contract CappedCrowdsale is Crowdsale {
* @param cap Max amount of wei to be contributed
*/
constructor (uint256 cap) public {
require(cap > 0);
require(cap > 0, "CappedCrowdsale: cap is 0");
_cap = cap;
}

Expand All @@ -43,6 +43,6 @@ contract CappedCrowdsale is Crowdsale {
*/
function _preValidatePurchase(address beneficiary, uint256 weiAmount) internal view {
super._preValidatePurchase(beneficiary, weiAmount);
require(weiRaised().add(weiAmount) <= _cap);
require(weiRaised().add(weiAmount) <= _cap, "CappedCrowdsale: cap exceeded");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ contract IndividuallyCappedCrowdsale is Crowdsale, CapperRole {
*/
function _preValidatePurchase(address beneficiary, uint256 weiAmount) internal view {
super._preValidatePurchase(beneficiary, weiAmount);
require(_contributions[beneficiary].add(weiAmount) <= _caps[beneficiary]);
// solhint-disable-next-line max-line-length
require(_contributions[beneficiary].add(weiAmount) <= _caps[beneficiary], "IndividuallyCappedCrowdsale: beneficiary's cap exceeded");
}

/**
Expand Down
12 changes: 7 additions & 5 deletions contracts/crowdsale/validation/TimedCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ contract TimedCrowdsale is Crowdsale {
* @dev Reverts if not in crowdsale time range.
*/
modifier onlyWhileOpen {
require(isOpen());
require(isOpen(), "TimedCrowdsale: not open");
_;
}

Expand All @@ -35,8 +35,9 @@ contract TimedCrowdsale is Crowdsale {
*/
constructor (uint256 openingTime, uint256 closingTime) public {
// solhint-disable-next-line not-rely-on-time
require(openingTime >= block.timestamp);
require(closingTime > openingTime);
require(openingTime >= block.timestamp, "TimedCrowdsale: opening time is before current time");
// solhint-disable-next-line max-line-length
require(closingTime > openingTime, "TimedCrowdsale: opening time is not before closing time");

_openingTime = openingTime;
_closingTime = closingTime;
Expand Down Expand Up @@ -87,8 +88,9 @@ contract TimedCrowdsale is Crowdsale {
* @param newClosingTime Crowdsale closing time
*/
function _extendTime(uint256 newClosingTime) internal {
require(!hasClosed());
require(newClosingTime > _closingTime);
require(!hasClosed(), "TimedCrowdsale: already closed");
// solhint-disable-next-line max-line-length
require(newClosingTime > _closingTime, "TimedCrowdsale: new closing time is before current closing time");

emit TimedCrowdsaleExtended(_closingTime, newClosingTime);
_closingTime = newClosingTime;
Expand Down
2 changes: 1 addition & 1 deletion contracts/crowdsale/validation/WhitelistCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ contract WhitelistCrowdsale is WhitelistedRole, Crowdsale {
* @param _weiAmount Amount of wei contributed
*/
function _preValidatePurchase(address _beneficiary, uint256 _weiAmount) internal view {
require(isWhitelisted(_beneficiary));
require(isWhitelisted(_beneficiary), "WhitelistCrowdsale: beneficiary doesn't have the Whitelisted role");
super._preValidatePurchase(_beneficiary, _weiAmount);
}
}
11 changes: 6 additions & 5 deletions contracts/drafts/ERC20Migrator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ contract ERC20Migrator {
* @param legacyToken address of the old token contract
*/
constructor (IERC20 legacyToken) public {
require(address(legacyToken) != address(0));
require(address(legacyToken) != address(0), "ERC20Migrator: legacy token is the zero address");
_legacyToken = legacyToken;
}

Expand All @@ -68,9 +68,10 @@ contract ERC20Migrator {
* @param newToken_ the token that will be minted
*/
function beginMigration(ERC20Mintable newToken_) public {
require(address(_newToken) == address(0));
require(address(newToken_) != address(0));
require(newToken_.isMinter(address(this)));
require(address(_newToken) == address(0), "ERC20Migrator: migration already started");
require(address(newToken_) != address(0), "ERC20Migrator: new token is the zero address");
//solhint-disable-next-line max-line-length
require(newToken_.isMinter(address(this)), "ERC20Migrator: not a minter for new token");

_newToken = newToken_;
}
Expand All @@ -82,7 +83,7 @@ contract ERC20Migrator {
* @param amount amount of tokens to be migrated
*/
function migrate(address account, uint256 amount) public {
require(address(_newToken) != address(0));
require(address(_newToken) != address(0), "ERC20Migrator: migration not started");
_legacyToken.safeTransferFrom(account, address(this), amount);
_newToken.mint(account, amount);
}
Expand Down
5 changes: 3 additions & 2 deletions contracts/drafts/ERC20Snapshot.sol
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,9 @@ contract ERC20Snapshot is ERC20 {
function _valueAt(uint256 snapshotId, Snapshots storage snapshots)
private view returns (bool, uint256)
{
require(snapshotId > 0);
require(snapshotId <= _currentSnapshotId.current());
require(snapshotId > 0, "ERC20Snapshot: id is 0");
// solhint-disable-next-line max-line-length
require(snapshotId <= _currentSnapshotId.current(), "ERC20Snapshot: nonexistent id");

uint256 index = snapshots.ids.findUpperBound(snapshotId);

Expand Down
10 changes: 6 additions & 4 deletions contracts/drafts/SignatureBouncer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,25 @@ contract SignatureBouncer is SignerRole {
* @dev Requires that a valid signature of a signer was provided.
*/
modifier onlyValidSignature(bytes memory signature) {
require(_isValidSignature(msg.sender, signature));
require(_isValidSignature(msg.sender, signature), "SignatureBouncer: invalid signature for caller");
_;
}

/**
* @dev Requires that a valid signature with a specified method of a signer was provided.
*/
modifier onlyValidSignatureAndMethod(bytes memory signature) {
require(_isValidSignatureAndMethod(msg.sender, signature));
// solhint-disable-next-line max-line-length
require(_isValidSignatureAndMethod(msg.sender, signature), "SignatureBouncer: invalid signature for caller and method");
_;
}

/**
* @dev Requires that a valid signature with a specified method and params of a signer was provided.
*/
modifier onlyValidSignatureAndData(bytes memory signature) {
require(_isValidSignatureAndData(msg.sender, signature));
// solhint-disable-next-line max-line-length
require(_isValidSignatureAndData(msg.sender, signature), "SignatureBouncer: invalid signature for caller and data");
_;
}

Expand Down Expand Up @@ -97,7 +99,7 @@ contract SignatureBouncer is SignerRole {
* @return bool
*/
function _isValidSignatureAndData(address account, bytes memory signature) internal view returns (bool) {
require(msg.data.length > _SIGNATURE_SIZE);
require(msg.data.length > _SIGNATURE_SIZE, "SignatureBouncer: data is too short");

bytes memory data = new bytes(msg.data.length - _SIGNATURE_SIZE);
for (uint i = 0; i < data.length; i++) {
Expand Down
12 changes: 6 additions & 6 deletions contracts/drafts/SignedSafeMath.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ library SignedSafeMath {
return 0;
}

require(!(a == -1 && b == INT256_MIN)); // This is the only case of overflow not detected by the check below
require(!(a == -1 && b == INT256_MIN), "SignedSafeMath: multiplication overflow");

int256 c = a * b;
require(c / a == b);
require(c / a == b, "SignedSafeMath: multiplication overflow");

return c;
}
Expand All @@ -30,8 +30,8 @@ library SignedSafeMath {
* @dev Integer division of two signed integers truncating the quotient, reverts on division by zero.
*/
function div(int256 a, int256 b) internal pure returns (int256) {
require(b != 0); // Solidity only automatically asserts when dividing by 0
require(!(b == -1 && a == INT256_MIN)); // This is the only case of overflow
require(b != 0, "SignedSafeMath: division by zero");
require(!(b == -1 && a == INT256_MIN), "SignedSafeMath: division overflow");

int256 c = a / b;

Expand All @@ -43,7 +43,7 @@ library SignedSafeMath {
*/
function sub(int256 a, int256 b) internal pure returns (int256) {
int256 c = a - b;
require((b >= 0 && c <= a) || (b < 0 && c > a));
require((b >= 0 && c <= a) || (b < 0 && c > a), "SignedSafeMath: subtraction overflow");

return c;
}
Expand All @@ -53,7 +53,7 @@ library SignedSafeMath {
*/
function add(int256 a, int256 b) internal pure returns (int256) {
int256 c = a + b;
require((b >= 0 && c >= a) || (b < 0 && c < a));
require((b >= 0 && c >= a) || (b < 0 && c < a), "SignedSafeMath: addition overflow");

return c;
}
Expand Down
Loading