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

⚡️ Tokens: Save gas by removing calldatasize & parameter size checks #520

Open
Philogy opened this issue Jul 29, 2023 · 1 comment
Open

Comments

@Philogy
Copy link
Contributor

Philogy commented Jul 29, 2023

When a method such as transfer(address, uint256) is defined the Solidity compiler will add the following runtime checks:

  1. calldatasize is at least 68 bytes long (4 selector, 64-bytes ABI-encoded params)
  2. the address parameter is not encoded with any dirty bytes i.e. calldataload(0x4) == and(calldataload(0x4), 0xffffffffffffffffffffffffffffffffffffffff)

For the Solady tokens the second 2 check is arguably unnecessary and could be removed as all methods already account for and make sure to clean dirty bits in address parameters. The calldatasize check (1.) is arguably also unnecessary as it historically was mainly added to save users who had defective ABI-encoding implementations and would incorrectly concatenate parameters together.

I wanted to open the discussion to removing both of this in Solady with the following approach:

  1. Find signatures for methods such as transfer, balanceOf etc. whereby the selector is the same but the signature is a parameter-less function (this ensures the Solidity compiler does not add the additional calldatasize check)
  2. For these external methods load the parameters directly in inline-assembly from calldata e.g. let to := calldataload(0x4)

As a demo ERC20's transfer method would become:

    function transfer_50eMP04() external virtual returns (bool) {
        address to;
        uint256 amount;
        /// @solidity memory-safe-assembly
        assembly {
            to := calldataload(0x4)
            amount := calldataload(0x24)
        }       
        _beforeTokenTransfer(msg.sender, to, amount);
        /// @solidity memory-safe-assembly
        assembly {
            // Compute the balance slot and load its value.
            mstore(0x0c, _BALANCE_SLOT_SEED)
            mstore(0x00, caller())
            let fromBalanceSlot := keccak256(0x0c, 0x20)
            let fromBalance := sload(fromBalanceSlot)
            // Revert if insufficient balance.
            if gt(amount, fromBalance) {
                mstore(0x00, 0xf4d678b8) // `InsufficientBalance()`.
                revert(0x1c, 0x04)
            }
            // Subtract and store the updated balance.
            sstore(fromBalanceSlot, sub(fromBalance, amount))
            // Compute the balance slot of `to`.
            mstore(0x00, to)
            let toBalanceSlot := keccak256(0x0c, 0x20)
            // Add and store the updated balance of `to`.
            // Will not overflow because the sum of all user balances
            // cannot exceed the maximum uint256 value.
            sstore(toBalanceSlot, add(sload(toBalanceSlot), amount))
            // Emit the {Transfer} event.
            mstore(0x20, amount)
            log3(0x20, 0x20, _TRANSFER_EVENT_SIGNATURE, caller(), shr(96, mload(0x0c)))
        }
        _afterTokenTransfer(msg.sender, to, amount);
        return true;
    }

To mitigate their incorrect use this methods would have to be marked as external

@Vectorized
Copy link
Owner

Vectorized commented Jul 30, 2023

Ah. sounds so tempting, but the ERC20 contract can't be like an interface anymore. Lemme think more about it.

Oh just realized one more obstacle: Etherscan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants