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

Gas Optimizations #112

Open
code423n4 opened this issue Aug 14, 2022 · 0 comments
Open

Gas Optimizations #112

code423n4 opened this issue Aug 14, 2022 · 0 comments
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

Using bools for storage incurs overhead

// Booleans are more expensive than uint256 or any type that takes up a full
// word because each write operation emits an extra SLOAD to first read the
// slot's contents, replace the bits taken up by the boolean, and then write
// back. This is the compiler's defense against contract upgrades and
// pointer aliasing, and it cannot be disabled.

features/Blocklist.sol:10:    mapping(address => bool) private _blocklist;

make i++ into ++i to save 3 gas on a copy

VotingEscrow.sol:309:        for (uint256 i = 0; i < 255; i++) {
VotingEscrow.sol:717:        for (uint256 i = 0; i < 128; i++) {
VotingEscrow.sol:739:        for (uint256 i = 0; i < 128; i++) {
VotingEscrow.sol:834:        for (uint256 i = 0; i < 255; i++) {

save gas by making require (uint amount >0); into

require (!= 0);
because a uint is ether 0 or greater so it saves gas .
https://github.com/code-423n4/2022-08-fiatdao/blob/5a254ab15a387bd65a7dc50ac8371cb77de1e5d5/contracts/VotingEscrow.sol#L448

use custom errors instead require to save gas

VotingEscrow.sol:116:        require(decimals <= 18, "Exceeds max decimals");
VotingEscrow.sol:125:        require(
VotingEscrow.sol:140:        require(msg.sender == owner, "Only owner");
VotingEscrow.sol:147:        require(msg.sender == owner, "Only owner");
VotingEscrow.sol:154:        require(msg.sender == owner, "Only owner");
VotingEscrow.sol:162:        require(msg.sender == owner, "Only owner");
VotingEscrow.sol:171:        require(msg.sender == blocklist, "Only Blocklist");
VotingEscrow.sol:412:        require(_value > 0, "Only non zero amount");
VotingEscrow.sol:413:        require(locked_.amount == 0, "Lock exists");
VotingEscrow.sol:414:        require(unlock_time >= locked_.end, "Only increase lock end"); // from using quitLock, user should increaseAmount instead
VotingEscrow.sol:415:        require(unlock_time > block.timestamp, "Only future lock end");
VotingEscrow.sol:416:        require(unlock_time <= block.timestamp + MAXTIME, "Exceeds maxtime");
VotingEscrow.sol:425:        require(
VotingEscrow.sol:448:        require(_value > 0, "Only non zero amount");
VotingEscrow.sol:449:        require(locked_.amount > 0, "No lock");
VotingEscrow.sol:450:        require(locked_.end > block.timestamp, "Lock expired");
VotingEscrow.sol:469:            require(locked_.amount > 0, "Delegatee has no lock");
VotingEscrow.sol:470:            require(locked_.end > block.timestamp, "Delegatee lock expired");
VotingEscrow.sol:485:        require(
VotingEscrow.sol:502:        require(locked_.amount > 0, "No lock");
VotingEscrow.sol:503:        require(unlock_time > locked_.end, "Only increase lock end");
VotingEscrow.sol:504:        require(unlock_time <= block.timestamp + MAXTIME, "Exceeds maxtime");
VotingEscrow.sol:511:            require(oldUnlockTime > block.timestamp, "Lock expired");
VotingEscrow.sol:529:        require(locked_.amount > 0, "No lock");
VotingEscrow.sol:530:        require(locked_.end <= block.timestamp, "Lock not expired");
VotingEscrow.sol:531:        require(locked_.delegatee == msg.sender, "Lock delegated");
VotingEscrow.sol:546:        require(token.transfer(msg.sender, value), "Transfer failed");
VotingEscrow.sol:563:        require(!IBlocklist(blocklist).isBlocked(_addr), "Blocked contract");
VotingEscrow.sol:564:        require(locked_.amount > 0, "No lock");
VotingEscrow.sol:565:        require(locked_.delegatee != _addr, "Already delegated");
VotingEscrow.sol:587:        require(toLocked.amount > 0, "Delegatee has no lock");
VotingEscrow.sol:588:        require(toLocked.end > block.timestamp, "Delegatee lock expired");
VotingEscrow.sol:589:        require(toLocked.end >= fromLocked.end, "Only delegate to longer lock");
VotingEscrow.sol:635:        require(locked_.amount > 0, "No lock");
VotingEscrow.sol:636:        require(locked_.end > block.timestamp, "Lock expired");
VotingEscrow.sol:637:        require(locked_.delegatee == msg.sender, "Lock delegated");
VotingEscrow.sol:657:        require(token.transfer(msg.sender, remainingAmount), "Transfer failed");
VotingEscrow.sol:676:        require(token.transfer(penaltyRecipient, amount), "Transfer failed");
VotingEscrow.sol:776:        require(_blockNumber <= block.number, "Only past block number");
VotingEscrow.sol:877:        require(_blockNumber <= block.number, "Only past block number");

make address(0) into long form to save gas

ex: 0x00000000000000

VotingEscrow.sol:235:        if (_addr != address(0)) {
VotingEscrow.sol:354:        if (_addr != address(0)) {
VotingEscrow.sol:376:        if (_addr != address(0)) {
VotingEscrow.sol:401:        _checkpoint(address(0), empty, empty);
VotingEscrow.sol:425:        _checkpoint(msg.sender, LockedBalance(0, 0, 0, address(0)), locked_);
VotingEscrow.sol:540:        newLocked.delegatee = address(0);
VotingEscrow.sol:645:        newLocked.delegatee = address(0);



USE ASSEMBLY TO CHECK FOR ADDRESS(0)

ex: iszero

VotingEscrow.sol:235:        if (_addr != address(0)) {
VotingEscrow.sol:354:        if (_addr != address(0)) {
VotingEscrow.sol:376:        if (_addr != address(0)) {
VotingEscrow.sol:401:        _checkpoint(address(0), empty, empty);
VotingEscrow.sol:425:        _checkpoint(msg.sender, LockedBalance(0, 0, 0, address(0)), locked_);
VotingEscrow.sol:540:        newLocked.delegatee = address(0);
VotingEscrow.sol:645:        newLocked.delegatee = address(0);

FUNCTIONS GUARANTEED TO REVERT WHEN CALLED BY NORMAL USERS CAN BE MARKED PAYABLE

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.
https://github.com/code-423n4/2022-08-fiatdao/blob/5a254ab15a387bd65a7dc50ac8371cb77de1e5d5/contracts/VotingEscrow.sol#L142
https://github.com/code-423n4/2022-08-fiatdao/blob/5a254ab15a387bd65a7dc50ac8371cb77de1e5d5/contracts/VotingEscrow.sol#L146
https://github.com/code-423n4/2022-08-fiatdao/blob/5a254ab15a387bd65a7dc50ac8371cb77de1e5d5/contracts/VotingEscrow.sol#L156

make i unchecked when it wont overflow

you are saving gas on solidity not checking on an overflow

VotingEscrow.sol:309:        for (uint256 i = 0; i < 255; i++) {
VotingEscrow.sol:717:        for (uint256 i = 0; i < 128; i++) {
VotingEscrow.sol:739:        for (uint256 i = 0; i < 128; i++) {
VotingEscrow.sol:834:        for (uint256 i = 0; i < 255; i++) {




@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Aug 14, 2022
code423n4 added a commit that referenced this issue Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

1 participant