Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
122 commits
Select commit Hold shift + click to select a range
4307e5b
Replace error strings with custom errors
ernestognw May 18, 2023
ddf387c
Lint
ernestognw May 18, 2023
0800e32
Finish original list of errors
ernestognw May 19, 2023
199093b
Finish missing require statements
ernestognw May 19, 2023
d3703bd
Self review
ernestognw May 19, 2023
11931ca
Finish revert statements
ernestognw May 19, 2023
bb3a12f
Applied spreadsheet suggestion
ernestognw May 19, 2023
9f58996
Refactor Address.sol
ernestognw May 19, 2023
a76e4b6
Finish custom errors replacement
ernestognw May 19, 2023
c65b76b
Fix SignatureChecker
ernestognw May 20, 2023
f58c06b
Add account to `GovernorAlreadyCastVote` error
ernestognw May 21, 2023
810468d
Finish access, finance, and governance testing
ernestognw May 22, 2023
adbe841
Fix proxy tests
ernestognw May 22, 2023
c29e041
First round of reviews
ernestognw May 22, 2023
bc00094
Fix tests for ERC20 and ERC1155 tokens
ernestognw May 23, 2023
faf33a4
Lint
ernestognw May 23, 2023
cdbea1e
Bump Pragma to 0.8.19
ernestognw May 23, 2023
77eee99
Finish token tests
ernestognw May 23, 2023
ba42df6
Fix ERC20Capped
ernestognw May 23, 2023
c31e10d
Fix Address tests
ernestognw May 23, 2023
b3b7e08
Advancements on utils
ernestognw May 24, 2023
960066b
Fix utils test
ernestognw May 24, 2023
b815a4f
Remove unnecessary test file
ernestognw May 24, 2023
99347e4
Fix conflicts with master
ernestognw May 24, 2023
d7d3f90
Lint
ernestognw May 24, 2023
5bc0812
Add changeset
ernestognw May 24, 2023
3276d35
Fix generation script
ernestognw May 24, 2023
23a7be2
Fix flaky Create2 test
ernestognw May 24, 2023
3f4f84a
Update comment in Ownable
ernestognw May 24, 2023
3dc21e2
Add `Unset` state to TimelockController
ernestognw May 24, 2023
1c119f7
Replace `GovernorMissingETA` with `GovernorProposalNotQueued`
ernestognw May 24, 2023
6541481
Revert interface changes to MinimalForwarder
ernestognw May 24, 2023
9c71d9c
Remove ERC-6093 from token interfaces
ernestognw May 24, 2023
5f8417b
Replace `msg.sender` with `_msgSender()` in ERC721Wrapper
ernestognw May 24, 2023
55f0eef
Make `onRevert` view visilibity
ernestognw May 24, 2023
5fe9511
Change Ownable2Step's error for invalid ownership acceptance
ernestognw May 24, 2023
c927bc3
Replace SafeERC20's domain
ernestognw May 25, 2023
0a5bb3f
Merge branch 'next-v5.0' into lib-839-custom-errors
ernestognw May 25, 2023
eac424e
Lint fix
ernestognw May 25, 2023
e11f4fa
Merge branch 'next-v5.0' into lib-839-custom-errors
ernestognw May 29, 2023
5dc5ada
Fix checkpoint tests
ernestognw May 29, 2023
44535a2
Enabled all tests
ernestognw May 29, 2023
0d1d6c0
Remove GovernorDuplicatedProposal error
ernestognw May 29, 2023
07d4fc9
Merge branch 'next-v5.0' into lib-839-custom-errors
ernestognw May 29, 2023
058b672
Lint
ernestognw May 29, 2023
1d63212
Update contracts/governance/IGovernor.sol
ernestognw May 30, 2023
276331c
Update contracts/utils/Address.sol
ernestognw May 30, 2023
28e2e16
Round of review
ernestognw May 30, 2023
2b0ce34
Merge branch 'next-v5.0' into lib-839-custom-errors
ernestognw May 30, 2023
4946dd9
Rename AccessContol renounce error and parameter
ernestognw May 31, 2023
3173d53
Rename `Unsuccessful` -> `Failed`
ernestognw May 31, 2023
6c8d0e1
Merge Address failed call errors
ernestognw May 31, 2023
cb79ab3
Rename *FailedLowLevelCall to *FailedCall
ernestognw May 31, 2023
be68001
Round of review
ernestognw Jun 1, 2023
f02af6e
Change token paused errors
ernestognw Jun 1, 2023
2d735e6
Fix tests
ernestognw Jun 1, 2023
2ecfd14
Round of review
ernestognw Jun 1, 2023
b3af988
Round of review
ernestognw Jun 1, 2023
c862ca5
More review
ernestognw Jun 1, 2023
5069bdf
Fixed example in StorageSlot.sol
ernestognw Jun 1, 2023
48fe940
Changed Checkpoint error
ernestognw Jun 1, 2023
c15fbf8
Added value to StringsInsufficientHexLength
ernestognw Jun 1, 2023
a10ee8b
Added `useCheckedNonce` for Nonces
ernestognw Jun 1, 2023
8d62e91
Merge branch 'master' into lib-839-custom-errors
ernestognw Jun 1, 2023
861d341
Replace ERC20DecreasedAllowance for SafeERC20
ernestognw Jun 1, 2023
61a6026
Merge branch 'master' into lib-839-custom-errors
ernestognw Jun 2, 2023
c796d15
Improved MinimalForwarder errors
ernestognw Jun 2, 2023
6a9c40f
Merge branch 'master' into lib-839-custom-errors
ernestognw Jun 2, 2023
a4063c9
Review
ernestognw Jun 2, 2023
5876d1c
Remove mutable variables in ERC1155 tests
ernestognw Jun 2, 2023
6d8c498
Remove unnecessary error
ernestognw Jun 2, 2023
219923b
Rename `Inexistent` to `Nonexistent`
ernestognw Jun 2, 2023
1fa5acc
Rename AccessControlDefaultAdminRules errors
ernestognw Jun 2, 2023
602df5a
Overflown -> Overflowed
ernestognw Jun 2, 2023
4a14bec
Simplified SafeCast errors
ernestognw Jun 2, 2023
ff12505
Review round
ernestognw Jun 2, 2023
65ad2e5
Lint
ernestognw Jun 2, 2023
91bdb7c
Mooar review
ernestognw Jun 3, 2023
a33a4c8
Revert ERC721 changes
ernestognw Jun 3, 2023
2253c40
Rename Paused errors by using a library
ernestognw Jun 3, 2023
892dbcc
Lint
ernestognw Jun 3, 2023
028f383
Fix tests
ernestognw Jun 5, 2023
9c70af8
Review suggestions
ernestognw Jun 5, 2023
7647774
Move Pausable errors to its own file
ernestognw Jun 6, 2023
45ce67f
Merge branch 'master' into lib-839-custom-errors
ernestognw Jun 6, 2023
11092da
Moar suggestions
ernestognw Jun 6, 2023
6009844
Moar suggestions
ernestognw Jun 6, 2023
55433e7
Improved custom error matcher
ernestognw Jun 7, 2023
4dac3c7
Lint
ernestognw Jun 7, 2023
aa44323
Merge branch 'master' into lib-839-custom-errors
ernestognw Jun 7, 2023
cb4594f
Merge branch 'master' into lib-839-custom-errors
ernestognw Jun 7, 2023
6df52b0
Simplify custom error matcher regex
ernestognw Jun 7, 2023
ce83461
Attempt to remove the optimizations branch check for custom errors
ernestognw Jun 7, 2023
e5475a2
Simplify custom error matcher
ernestognw Jun 8, 2023
03f1152
Revert ERC1155Supply _update order
ernestognw Jun 8, 2023
7e320a7
Merge branch 'master' into lib-839-custom-errors
ernestognw Jun 8, 2023
0e158b5
Update GUIDELINES.md
ernestognw Jun 8, 2023
f1717a6
Use verifyCallResult in Timelock
ernestognw Jun 9, 2023
db4bcf9
Fix TimelockController tests
ernestognw Jun 9, 2023
e0949a3
Add domain to DoubleEndedQueue errors
ernestognw Jun 9, 2023
46904d7
Suggestions
ernestognw Jun 9, 2023
3a74c47
Change Pausable error names
ernestognw Jun 9, 2023
7bf1afc
Remove address context from UUPSUnauthorizedCallContext
ernestognw Jun 9, 2023
13dd54a
More suggestions
ernestognw Jun 9, 2023
c97d95c
Remove errorPrefix in AccessControl tests
Amxx Jun 9, 2023
fc1e4b0
Remove errorPrefix in ERC20 tests
Amxx Jun 9, 2023
27b15e3
move ERC20Permit.test.js out of draft
Amxx Jun 9, 2023
2ad6e65
Remove errorPrefix in ERC721 tests
Amxx Jun 9, 2023
9304b9b
Fix test
ernestognw Jun 9, 2023
d4f7071
Remove Pausable.errors.sol library
ernestognw Jun 9, 2023
88cf256
Applied suggestions
ernestognw Jun 9, 2023
4432ee6
More suggestions
ernestognw Jun 9, 2023
b2aaf9e
Remove _custom revert functions
ernestognw Jun 9, 2023
d5815a8
Fix tests
ernestognw Jun 9, 2023
d4e27a0
increase gas for test
frangio Jun 9, 2023
73ac6dd
document unspecified revert, and add custom error check
Amxx Jun 12, 2023
8994817
document bubbling of panic code
Amxx Jun 12, 2023
c1c6a75
Update contracts/utils/Address.sol
ernestognw Jun 12, 2023
3da76b0
Update test/helpers/customError.js
ernestognw Jun 12, 2023
4aa35c8
Remove `bitsize` from SafeCastOverflowed cast errors
ernestognw Jun 12, 2023
007d6b0
Lint
ernestognw Jun 12, 2023
1cd28fe
Merge branch 'master' into lib-839-custom-errors
ernestognw Jun 12, 2023
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
Prev Previous commit
Next Next commit
Refactor Address.sol
  • Loading branch information
ernestognw committed May 19, 2023
commit 9f58996785b885e92eb87e53d4d98fbe60b32970
23 changes: 16 additions & 7 deletions contracts/governance/Governor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,13 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
address proposer = _msgSender();
uint256 currentTimepoint = clock();

uint256 proposerVotes = getVotes(proposer, currentTimepoint - 1);
uint256 votesThreshold = proposalThreshold();
if (proposerVotes < votesThreshold) {
revert GovernorProposerInvalidTreshold(proposer, proposerVotes, votesThreshold);
// Avoid stack too deep
{
uint256 proposerVotes = getVotes(proposer, currentTimepoint - 1);
uint256 votesThreshold = proposalThreshold();
if (proposerVotes < votesThreshold) {
revert GovernorProposerInvalidTreshold(proposer, proposerVotes, votesThreshold);
}
}

uint256 proposalId = hashProposal(targets, values, calldatas, keccak256(bytes(description)));
Expand Down Expand Up @@ -382,10 +385,9 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
bytes[] memory calldatas,
bytes32 /*descriptionHash*/
) internal virtual {
string memory errorMessage = "Governor: call reverted without message";
for (uint256 i = 0; i < targets.length; ++i) {
(bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]);
Address.verifyCallResult(success, returndata, errorMessage);
Address.verifyCallResult(success, returndata, _onGovernorCallRevert);
}
}

Expand Down Expand Up @@ -616,7 +618,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
*/
function relay(address target, uint256 value, bytes calldata data) external payable virtual onlyGovernance {
(bool success, bytes memory returndata) = target.call{value: value}(data);
Address.verifyCallResult(success, returndata, "Governor: relay reverted without message");
Address.verifyCallResult(success, returndata, _onGovernorCallRevert);
}

/**
Expand Down Expand Up @@ -675,4 +677,11 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
function _encodeState(ProposalState proposalState) internal pure returns (bytes32) {
return bytes32(1 << uint32(proposalState));
}

/**
* @dev Default revert function for failed executed functions without any other bubbled up reason.
*/
function _onGovernorCallRevert() internal pure {
revert GovernorFailedLowLevelCall();
}
}
7 changes: 6 additions & 1 deletion contracts/governance/IGovernor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ abstract contract IGovernor is IERC165, IERC6372 {
* @dev The `proposalId` is duplicated.
*/
error GovernorDuplicatedProposal(uint256 proposalId);

/**
* @dev The `proposalId` doesn't exist.
*/
Expand All @@ -77,6 +77,11 @@ abstract contract IGovernor is IERC165, IERC6372 {
*/
error GovernorProposerInvalidTreshold(address proposer, uint256 votes, uint256 threshold);

/**
* @dev A low level call failed without any further reason.
*/
error GovernorFailedLowLevelCall();

/**
* @dev Emitted when a proposal is created.
*/
Expand Down
6 changes: 3 additions & 3 deletions contracts/governance/extensions/GovernorCountingSimple.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ abstract contract GovernorCountingSimple is Governor {
mapping(uint256 => ProposalVote) private _proposalVotes;

/**
* @dev Vote type is not in the {VoteType} enum. Matches Governor Bravo definition.
* @dev Vote type is not in the {VoteType} enum.
*/
error InvalidVoteType();
error GovernorInvalidVoteType();

/**
* @dev See {IGovernor-COUNTING_MODE}.
Expand Down Expand Up @@ -101,7 +101,7 @@ abstract contract GovernorCountingSimple is Governor {
} else if (support == uint8(VoteType.Abstain)) {
proposalVote.abstainVotes += weight;
} else {
revert InvalidVoteType();
revert GovernorInvalidVoteType();
}
}
}
1 change: 0 additions & 1 deletion contracts/mocks/token/ERC721ConsecutiveMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
pragma solidity ^0.8.0;

import "../../token/ERC721/extensions/ERC721Consecutive.sol";
import "../../token/ERC721/extensions/ERC721Enumerable.sol";
import "../../token/ERC721/extensions/ERC721Pausable.sol";
import "../../token/ERC721/extensions/ERC721Votes.sol";

Expand Down
11 changes: 10 additions & 1 deletion contracts/token/ERC20/utils/SafeERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ library SafeERC20 {
*/
error ERC20UnsuccessfulOperation(address token);

/**
* @dev A low level call failed without any further reason.
*/
error ERC20FailedLowLevelCall();

/**
* @dev Transfer `value` amount of `token` from the calling contract to `to`. If `token` returns no value,
* non-reverting calls are assumed to be successful.
Expand Down Expand Up @@ -110,7 +115,7 @@ library SafeERC20 {
// we're implementing it ourselves. We use {Address-functionCall} to perform this call, which verifies that
// the target address contains contract code and also asserts for success in the low-level call.

bytes memory returndata = address(token).functionCall(data, "SafeERC20: low-level call failed");
bytes memory returndata = address(token).functionCall(data, onERC20CallRevert);
if (returndata.length != 0 && !abi.decode(returndata, (bool))) {
revert ERC20UnsuccessfulOperation(address(token));
}
Expand All @@ -132,4 +137,8 @@ library SafeERC20 {
(bool success, bytes memory returndata) = address(token).call(data);
return success && (returndata.length == 0 || abi.decode(returndata, (bool))) && address(token).code.length > 0;
}

function onERC20CallRevert() internal pure {
revert ERC20FailedLowLevelCall();
}
}
4 changes: 2 additions & 2 deletions contracts/token/ERC721/extensions/ERC721Enumerable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
/**
* @dev Batch mint is not allowed.
*/
error ERC721ForbiddenBatchMint();
error ERC721EnumerableForbiddenBatchMint();

/**
* @dev See {IERC165-supportsInterface}.
Expand Down Expand Up @@ -83,7 +83,7 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {

if (batchSize > 1) {
// Will only trigger during construction. Batch transferring (minting) is not available afterwards.
revert ERC721ForbiddenBatchMint();
revert ERC721EnumerableForbiddenBatchMint();
}

uint256 tokenId = firstTokenId;
Expand Down
87 changes: 57 additions & 30 deletions contracts/utils/Address.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,28 @@ library Address {
/**
* @dev The ETH balance of the account is not enough to perform the operation.
*/
error ETHInsufficientBalance(address account);
error AddressInsufficientBalance(address account);

/**
* @dev A call to `target` failed. The `target` may have reverted.
*/
error FailedCall(address target);
error AddressFailedCall(address target);

/**
* @dev A low level call failed without any further reason.
*/
error AddressFailedLowLevelCall();

/**
* @dev There's no code at `target` (is not a contract).
*/
error EmptyCode(address target);
error AddressEmptyCode(address target);

/**
* @dev A revert was expected but didn't happen. This is caused if the
* custom revert function provided didn't revert.
*/
error AddressExpectedRevert();

/**
* @dev Replacement for Solidity's `transfer`: sends `amount` wei to
Expand All @@ -40,12 +51,12 @@ library Address {
*/
function sendValue(address payable recipient, uint256 amount) internal {
if (address(this).balance < amount) {
revert ETHInsufficientBalance(address(this));
revert AddressInsufficientBalance(address(this));
}

(bool success, ) = recipient.call{value: amount}("");
if (!success) {
revert FailedCall(recipient);
revert AddressFailedCall(recipient);
}
}

Expand All @@ -68,21 +79,25 @@ library Address {
* _Available since v3.1._
*/
function functionCall(address target, bytes memory data) internal returns (bytes memory) {
return functionCallWithValue(target, data, 0, "Address: low-level call failed");
return functionCallWithValue(target, data, 0, defaultOnRevert);
}

/**
* @dev Same as {xref-Address-functionCall-address-bytes-}[`functionCall`], but with
* `errorMessage` as a fallback revert reason when `target` reverts.
* @dev Same as {xref-Address-functionCall-address-bytes-}[`functionCall`], but with an
* `onRevert` function as a fallback when `target` reverts.
*
* Requirements:
*
* - `onRevert` must be a reverting function.
*
* _Available since v3.1._
*/
function functionCall(
address target,
bytes memory data,
string memory errorMessage
function() internal pure onRevert
) internal returns (bytes memory) {
return functionCallWithValue(target, data, 0, errorMessage);
return functionCallWithValue(target, data, 0, onRevert);
}

/**
Expand All @@ -97,26 +112,30 @@ library Address {
* _Available since v3.1._
*/
function functionCallWithValue(address target, bytes memory data, uint256 value) internal returns (bytes memory) {
return functionCallWithValue(target, data, value, "Address: low-level call with value failed");
return functionCallWithValue(target, data, value, defaultOnRevert);
}

/**
* @dev Same as {xref-Address-functionCallWithValue-address-bytes-uint256-}[`functionCallWithValue`], but
* with `errorMessage` as a fallback revert reason when `target` reverts.
* with an `onRevert` function as a fallback revert reason when `target` reverts.
*
* Requirements:
*
* - `onRevert` must be a reverting function.
*
* _Available since v3.1._
*/
function functionCallWithValue(
address target,
bytes memory data,
uint256 value,
string memory errorMessage
function() internal pure onRevert
) internal returns (bytes memory) {
if (address(this).balance < value) {
revert ETHInsufficientBalance(address(this));
revert AddressInsufficientBalance(address(this));
}
(bool success, bytes memory returndata) = target.call{value: value}(data);
return verifyCallResultFromTarget(target, success, returndata, errorMessage);
return verifyCallResultFromTarget(target, success, returndata, onRevert);
}

/**
Expand All @@ -126,7 +145,7 @@ library Address {
* _Available since v3.3._
*/
function functionStaticCall(address target, bytes memory data) internal view returns (bytes memory) {
return functionStaticCall(target, data, "Address: low-level static call failed");
return functionStaticCall(target, data, defaultOnRevert);
}

/**
Expand All @@ -138,10 +157,10 @@ library Address {
function functionStaticCall(
address target,
bytes memory data,
string memory errorMessage
function() internal pure onRevert
) internal view returns (bytes memory) {
(bool success, bytes memory returndata) = target.staticcall(data);
return verifyCallResultFromTarget(target, success, returndata, errorMessage);
return verifyCallResultFromTarget(target, success, returndata, onRevert);
}

/**
Expand All @@ -151,7 +170,7 @@ library Address {
* _Available since v3.4._
*/
function functionDelegateCall(address target, bytes memory data) internal returns (bytes memory) {
return functionDelegateCall(target, data, "Address: low-level delegate call failed");
return functionDelegateCall(target, data, defaultOnRevert);
}

/**
Expand All @@ -163,57 +182,64 @@ library Address {
function functionDelegateCall(
address target,
bytes memory data,
string memory errorMessage
function() internal pure onRevert
) internal returns (bytes memory) {
(bool success, bytes memory returndata) = target.delegatecall(data);
return verifyCallResultFromTarget(target, success, returndata, errorMessage);
return verifyCallResultFromTarget(target, success, returndata, onRevert);
}

/**
* @dev Tool to verify that a low level call to smart-contract was successful, and revert (either by bubbling
* the revert reason or using the provided one) in case of unsuccessful call or if target was not a contract.
* the revert reason or using the provided `onRevert`) in case of unsuccessful call or if target was not a contract.
*
* _Available since v4.8._
*/
function verifyCallResultFromTarget(
address target,
bool success,
bytes memory returndata,
string memory errorMessage
function() internal pure onRevert
) internal view returns (bytes memory) {
if (success) {
if (returndata.length == 0) {
// only check if target is a contract if the call was successful and the return data is empty
// otherwise we already know that it was a contract
if (target.code.length == 0) {
revert EmptyCode(target);
revert AddressEmptyCode(target);
}
}
return returndata;
} else {
_revert(returndata, errorMessage);
_revert(returndata, onRevert);
}
}

/**
* @dev Tool to verify that a low level call was successful, and revert if it wasn't, either by bubbling the
* revert reason or using the provided one.
* revert reason or using the provided `onRevert`.
*
* _Available since v4.3._
*/
function verifyCallResult(
bool success,
bytes memory returndata,
string memory errorMessage
function() internal pure onRevert
) internal pure returns (bytes memory) {
if (success) {
return returndata;
} else {
_revert(returndata, errorMessage);
_revert(returndata, onRevert);
}
}

function _revert(bytes memory returndata, string memory errorMessage) private pure {
/**
* @dev Default reverting function when no `onRevert` is provided in a function call.
*/
function defaultOnRevert() internal pure {
revert AddressFailedLowLevelCall();
}

function _revert(bytes memory returndata, function() internal pure onRevert) private pure {
// Look for revert reason and bubble it up if present
if (returndata.length > 0) {
// The easiest way to bubble the revert reason is using memory via assembly
Expand All @@ -223,7 +249,8 @@ library Address {
revert(add(32, returndata), returndata_size)
}
} else {
revert(errorMessage);
onRevert();
revert AddressExpectedRevert();
}
}
}