Skip to content

Commit

Permalink
refactor: introduce custom errors for better gas costs
Browse files Browse the repository at this point in the history 
This commit introduces custom errors for `BaseToken` and
`CommunityERC20` to reduce the gas costs in revert cases. Using
`require()` with a string error message requires every character of the
message to be store in memory which costs more gas than a fixed sized
error selector. Hence, it's recommended to use custom errors instead.

A gas snapshot is attached in this commit to show the improved gas
costs.
  • Loading branch information
@0x-r4bbit
0x-r4bbit committed Sep 4, 2023
1 parent 0c4e1d1 commit c5aabdb
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 47 deletions.
30 changes: 15 additions & 15 deletions .gas-snapshot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,25 @@ CommunityERC20Test:test_Deployment() (gas: 29720)
MintToTest:test_Deployment() (gas: 29742)
MintToTest:test_Deployment() (gas: 38626)
MintToTest:test_Deployment() (gas: 85415)
MintToTest:test_MintTo() (gas: 509977)
MintToTest:test_RevertWhen_AddressesAndAmountsAreNotEqualLength() (gas: 24193)
MintToTest:test_RevertWhen_MaxSupplyIsReached() (gas: 23267)
MintToTest:test_RevertWhen_MaxSupplyIsReached() (gas: 505463)
MintToTest:test_RevertWhen_MaxSupplyReached() (gas: 123426)
MintToTest:test_RevertWhen_SenderIsNotOwner() (gas: 36358)
MintToTest:test_MintTo() (gas: 509989)
MintToTest:test_RevertWhen_AddressesAndAmountsAreNotEqualLength() (gas: 23778)
MintToTest:test_RevertWhen_MaxSupplyIsReached() (gas: 22859)
MintToTest:test_RevertWhen_MaxSupplyIsReached() (gas: 505067)
MintToTest:test_RevertWhen_MaxSupplyReached() (gas: 122988)
MintToTest:test_RevertWhen_SenderIsNotOwner() (gas: 35956)
OwnerTokenTest:test_Deployment() (gas: 85415)
RemoteBurnTest:test_Deployment() (gas: 38626)
RemoteBurnTest:test_Deployment() (gas: 85437)
RemoteBurnTest:test_RemoteBurn() (gas: 455285)
RemoteBurnTest:test_RevertWhen_RemoteBurn() (gas: 19499)
RemoteBurnTest:test_RevertWhen_SenderIsNotOwner() (gas: 25211)
RemoteBurnTest:test_RemoteBurn() (gas: 455309)
RemoteBurnTest:test_RevertWhen_RemoteBurn() (gas: 19091)
RemoteBurnTest:test_RevertWhen_SenderIsNotOwner() (gas: 24791)
SetMaxSupplyTest:test_Deployment() (gas: 29720)
SetMaxSupplyTest:test_Deployment() (gas: 85437)
SetMaxSupplyTest:test_RevertWhen_CalledBecauseMaxSupplyIsLocked() (gas: 16521)
SetMaxSupplyTest:test_RevertWhen_MaxSupplyLowerThanTotalSupply() (gas: 149095)
SetMaxSupplyTest:test_RevertWhen_SenderIsNotOwner() (gas: 12852)
SetMaxSupplyTest:test_RevertWhen_SenderIsNotOwner() (gas: 17335)
SetMaxSupplyTest:test_RevertWhen_CalledBecauseMaxSupplyIsLocked() (gas: 16533)
SetMaxSupplyTest:test_RevertWhen_MaxSupplyLowerThanTotalSupply() (gas: 148572)
SetMaxSupplyTest:test_RevertWhen_SenderIsNotOwner() (gas: 12846)
SetMaxSupplyTest:test_RevertWhen_SenderIsNotOwner() (gas: 16939)
SetMaxSupplyTest:test_SetMaxSupply() (gas: 15597)
SetSignerPublicKeyTest:test_Deployment() (gas: 85415)
SetSignerPublicKeyTest:test_RevertWhen_SenderIsNotOwner() (gas: 18036)
SetSignerPublicKeyTest:test_SetSignerPublicKey() (gas: 26357)
SetSignerPublicKeyTest:test_RevertWhen_SenderIsNotOwner() (gas: 17634)
SetSignerPublicKeyTest:test_SetSignerPublicKey() (gas: 26369)
35 changes: 24 additions & 11 deletions contracts/BaseToken.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ import "@openzeppelin/contracts/utils/Counters.sol";
abstract contract BaseToken is Context, ERC721Enumerable {
using Counters for Counters.Counter;

error BaseToken_InvalidTokenAddress();
error BaseToken_NotAuthorized();
error BaseToken_MaxSupplyLowerThanTotalSupply();
error BaseToken_MaxSupplyReached();
error BaseToken_NotRemoteBurnable();
error BaseToken_NotTransferable();

// State variables

Counters.Counter private _tokenIdTracker;
Expand Down Expand Up @@ -54,16 +61,18 @@ abstract contract BaseToken is Context, ERC721Enumerable {
ownerToken = _ownerToken;
masterToken = _masterToken;

require(ownerToken != address(0x0) || masterToken != address(0x0), "owner or master tokens required");
if (ownerToken == address(0) && masterToken == address(0)) {
revert BaseToken_InvalidTokenAddress();
}
}

modifier onlyOwner() {
require(
(ownerToken == address(0) || IERC721(ownerToken).balanceOf(msg.sender) > 0)
|| (masterToken == address(0) || IERC721(masterToken).balanceOf(msg.sender) > 0),
"Not authorized"
);

if (
(ownerToken != address(0) && IERC721(ownerToken).balanceOf(msg.sender) == 0)
&& (masterToken != address(0) && IERC721(masterToken).balanceOf(msg.sender) == 0)
) {
revert BaseToken_NotAuthorized();
}
_;
}

Expand All @@ -72,7 +81,9 @@ abstract contract BaseToken is Context, ERC721Enumerable {
// External functions

function setMaxSupply(uint256 newMaxSupply) external virtual onlyOwner {
require(newMaxSupply >= totalSupply(), "MAX_SUPPLY_LOWER_THAN_TOTAL_SUPPLY");
if (newMaxSupply < totalSupply()) {
revert BaseToken_MaxSupplyLowerThanTotalSupply();
}
maxSupply = newMaxSupply;
}

Expand All @@ -83,7 +94,9 @@ abstract contract BaseToken is Context, ERC721Enumerable {
*
*/
function mintTo(address[] memory addresses) public onlyOwner {
require(_tokenIdTracker.current() + addresses.length <= maxSupply, "MAX_SUPPLY_REACHED");
if (_tokenIdTracker.current() + addresses.length > maxSupply) {
revert BaseToken_MaxSupplyReached();
}
_mintTo(addresses);
}

Expand All @@ -98,7 +111,7 @@ abstract contract BaseToken is Context, ERC721Enumerable {
* @param tokenIds The list of token IDs to be burned
*/
function remoteBurn(uint256[] memory tokenIds) public onlyOwner {
require(remoteBurnable, "NOT_REMOTE_BURNABLE");
if (!remoteBurnable) revert BaseToken_NotRemoteBurnable();

for (uint256 i = 0; i < tokenIds.length; i++) {
_burn(tokenIds[i]);
Expand Down Expand Up @@ -146,7 +159,7 @@ abstract contract BaseToken is Context, ERC721Enumerable {
override(ERC721Enumerable)
{
if (from != address(0) && to != address(0) && !transferable) {
revert("not transferable");
revert BaseToken_NotTransferable();
}
super._beforeTokenTransfer(from, to, firstTokenId, batchSize);
}
Expand Down
16 changes: 13 additions & 3 deletions contracts/CommunityERC20.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/utils/Context.sol";

contract CommunityERC20 is Context, Ownable, ERC20 {
error CommunityERC20_MaxSupplyLowerThanTotalSupply();
error CommunityERC20_MaxSupplyReached();
error CommunityERC20_MismatchingAddressesAndAmountsLengths();

/**
* If we want unlimited total supply we should set maxSupply to 2^256-1.
*/
Expand All @@ -30,7 +34,9 @@ contract CommunityERC20 is Context, Ownable, ERC20 {
// External functions

function setMaxSupply(uint256 newMaxSupply) external onlyOwner {
require(newMaxSupply >= totalSupply(), "MAX_SUPPLY_LOWER_THAN_TOTAL_SUPPLY");
if (newMaxSupply < totalSupply()) {
revert CommunityERC20_MaxSupplyLowerThanTotalSupply();
}
maxSupply = newMaxSupply;
}

Expand All @@ -40,11 +46,15 @@ contract CommunityERC20 is Context, Ownable, ERC20 {
*
*/
function mintTo(address[] memory addresses, uint256[] memory amounts) external onlyOwner {
require(addresses.length == amounts.length, "WRONG_LENGTHS");
if (addresses.length != amounts.length) {
revert CommunityERC20_MismatchingAddressesAndAmountsLengths();
}

for (uint256 i = 0; i < addresses.length; i++) {
uint256 amount = amounts[i];
require(totalSupply() + amount <= maxSupply, "MAX_SUPPLY_REACHED");
if (totalSupply() + amount > maxSupply) {
revert CommunityERC20_MaxSupplyReached();
}
_mint(addresses[i], amount);
}
}
Expand Down
16 changes: 8 additions & 8 deletions script/DeployOwnerToken.s.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ contract DeployOwnerToken is BaseScript {
vm.recordLogs();
vm.startBroadcast(broadcaster);
OwnerToken ownerToken = new OwnerToken(
ownerTokenConfig.name,
ownerTokenConfig.symbol,
ownerTokenConfig.baseURI,
masterTokenConfig.name,
masterTokenConfig.symbol,
masterTokenConfig.baseURI,
ownerTokenConfig.signerPublicKey
);
ownerTokenConfig.name,
ownerTokenConfig.symbol,
ownerTokenConfig.baseURI,
masterTokenConfig.name,
masterTokenConfig.symbol,
masterTokenConfig.baseURI,
ownerTokenConfig.signerPublicKey
);

// Need to retrieve master token address from logs as
// we can't access it otherwise
Expand Down
7 changes: 4 additions & 3 deletions test/CollectibleV1.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity ^0.8.17;
import { Test } from "forge-std/Test.sol";
import { DeployOwnerToken } from "../script/DeployOwnerToken.s.sol";
import { DeploymentConfig } from "../script/DeploymentConfig.s.sol";
import { BaseToken } from "../contracts/BaseToken.sol";
import { OwnerToken } from "../contracts/OwnerToken.sol";
import { MasterToken } from "../contracts/MasterToken.sol";
import { CollectibleV1 } from "../contracts/CollectibleV1.sol";
Expand Down Expand Up @@ -59,7 +60,7 @@ contract MintToTest is CollectibleV1Test {
}

function test_RevertWhen_SenderIsNotOwner() public {
vm.expectRevert(bytes("Not authorized"));
vm.expectRevert(BaseToken.BaseToken_NotAuthorized.selector);
collectibleV1.mintTo(accounts);
}

Expand All @@ -69,7 +70,7 @@ contract MintToTest is CollectibleV1Test {

address[] memory otherAddresses = new address[](1);
otherAddresses[0] = makeAddr("anotherAccount");
vm.expectRevert(bytes("MAX_SUPPLY_REACHED"));
vm.expectRevert(BaseToken.BaseToken_MaxSupplyReached.selector);
collectibleV1.mintTo(otherAddresses);

assertEq(collectibleV1.maxSupply(), maxSupply);
Expand Down Expand Up @@ -97,7 +98,7 @@ contract RemoteBurnTest is CollectibleV1Test {
function test_RevertWhen_SenderIsNotOwner() public {
uint256[] memory ids = new uint256[](1);
ids[0] = 0;
vm.expectRevert(bytes("Not authorized"));
vm.expectRevert(BaseToken.BaseToken_NotAuthorized.selector);
collectibleV1.remoteBurn(ids);
}

Expand Down
6 changes: 3 additions & 3 deletions test/CommunityERC20.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ contract SetMaxSupplyTest is CommunityERC20Test {
amounts[2] = 5;
amounts[3] = 20;
communityToken.mintTo(accounts, amounts); // totalSupply is now 50
vm.expectRevert(bytes("MAX_SUPPLY_LOWER_THAN_TOTAL_SUPPLY"));
vm.expectRevert(CommunityERC20.CommunityERC20_MaxSupplyLowerThanTotalSupply.selector);
communityToken.setMaxSupply(40);
}

Expand All @@ -77,7 +77,7 @@ contract MintToTest is CommunityERC20Test {
amounts[1] = 15;
amounts[2] = 5;

vm.expectRevert(bytes("WRONG_LENGTHS"));
vm.expectRevert(CommunityERC20.CommunityERC20_MismatchingAddressesAndAmountsLengths.selector);
communityToken.mintTo(accounts, amounts);
}

Expand All @@ -88,7 +88,7 @@ contract MintToTest is CommunityERC20Test {
amounts[2] = 25;
amounts[3] = 1; // this should exceed max supply

vm.expectRevert(bytes("MAX_SUPPLY_REACHED"));
vm.expectRevert(CommunityERC20.CommunityERC20_MaxSupplyReached.selector);
communityToken.mintTo(accounts, amounts);
}
}
9 changes: 5 additions & 4 deletions test/OwnerToken.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity ^0.8.17;
import { Test } from "forge-std/Test.sol";
import { DeployOwnerToken } from "../script/DeployOwnerToken.s.sol";
import { DeploymentConfig } from "../script/DeploymentConfig.s.sol";
import { BaseToken } from "../contracts/BaseToken.sol";
import { OwnerToken } from "../contracts/OwnerToken.sol";
import { MasterToken } from "../contracts/MasterToken.sol";

Expand Down Expand Up @@ -42,7 +43,7 @@ contract SetMaxSupplyTest is OwnerTokenTest {
}

function test_RevertWhen_SenderIsNotOwner() public {
vm.expectRevert(bytes("Not authorized"));
vm.expectRevert(BaseToken.BaseToken_NotAuthorized.selector);
ownerToken.setMaxSupply(1000);
}

Expand All @@ -59,7 +60,7 @@ contract SetSignerPublicKeyTest is OwnerTokenTest {
}

function test_RevertWhen_SenderIsNotOwner() public {
vm.expectRevert(bytes("Not authorized"));
vm.expectRevert(BaseToken.BaseToken_NotAuthorized.selector);
ownerToken.setSignerPublicKey(bytes("some key"));
}

Expand All @@ -81,7 +82,7 @@ contract MintToTest is OwnerTokenTest {
accounts[0] = makeAddr("anotherAccount");

vm.startPrank(deployer);
vm.expectRevert(bytes("MAX_SUPPLY_REACHED"));
vm.expectRevert(BaseToken.BaseToken_MaxSupplyReached.selector);
ownerToken.mintTo(accounts);
}
}
Expand All @@ -93,7 +94,7 @@ contract RemoteBurnTest is OwnerTokenTest {

function test_RevertWhen_RemoteBurn() public {
vm.startPrank(deployer);
vm.expectRevert(bytes("NOT_REMOTE_BURNABLE"));
vm.expectRevert(BaseToken.BaseToken_NotRemoteBurnable.selector);
uint256[] memory ids = new uint256[](1);
ids[0] = 0;
ownerToken.remoteBurn(ids);
Expand Down

0 comments on commit c5aabdb

Please sign in to comment.