jtriley-eth / ERC-6909

58 stars 6 forks source link

♻️ Unnecessary custom errors #1

Open 0xClandestine opened 1 year ago

0xClandestine commented 1 year ago

First of all sick proposal 👏👏👏, just thought I would suggest some gas optimizations.

1) These custom errors aren't really needed, and increase gas since an unnecessary conditional check is needed to throw the error. In both transfer and transferFrom the checked arithmetic would throw a revert regardless of the conditional. The same applies to the InsufficientPermission check within transferFrom.

/// @dev Thrown when owner balance for id is insufficient.
/// @param owner The address of the owner.
/// @param id The id of the token.
error InsufficientBalance(address owner, uint256 id);

/// @dev Thrown when spender allowance for id is insufficient.
/// @param spender The address of the spender.
/// @param id The id of the token.
error InsufficientPermission(address spender, uint256 id);

/// @notice Transfers an amount of an id from the caller to a receiver.
/// @param receiver The address of the receiver.
/// @param id The id of the token.
/// @param amount The amount of the token.
function transfer(address receiver, uint256 id, uint256 amount) public {
    if (balanceOf[msg.sender][id] < amount) revert InsufficientBalance(msg.sender, id);
    balanceOf[msg.sender][id] -= amount;
    balanceOf[receiver][id] += amount;
    emit Transfer(msg.sender, receiver, id, amount);
}

2) You can also use unchecked when modifying the receivers balance since totalSupply cannot exceed 2**256-1. Checkout transmissions/solmate/tokens/ERC20.sol for reference.

jtriley-eth commented 1 year ago

gm, thanks for the issue & pr!

so the reference implementation is meant more for readability and making the specification explicit. so tradeoffs were made in favor of readability over gas efficiency.

would you mind moving the optimized implementation to the alt/ directory? it can take the name of ERC6909Optimized.sol. from there we can make further optimizations and stray further from readability.