Nouns DAO contest - RaymondFam's results

Voter Refund

In rules 3 and 4, partial or no refund will be provided when contract runs very low in balance or no balance. To avoid voter grievances, I suggest implementing chainlink keeper to auto-fund contract with ETH when the contract balance is low.

Typo Mistake

Line 85 to 86 The comment is wrong. Instead of 4,000 basis points, it should be 6,000.

_withdraw() function

For best practices, the function should revert when address(this).balance == 0, and / or revert when bool sent is false. Note: Not checking bool refundSent on function _refundGas is understandable so that function castRefundableVoteInternal would still go through regardless of the internal refund status.

Use Custom Errors instead of Revert Strings

Considering implementing more (if not 100% of) custom errors instead of require() code lines to further reduce contract size.

Shorten require messages to less than 32 characters

Strings that are more than 32 characters will require more than 1 storage slot, costing more gas. Consider reducing the message length to less than 32 characters on all require() code lines not intended to be replaced by custom errors.

Avoid using uint8

When dealing with function arguments or memory values, there is no inherent benefit because the compiler does not pack these values. Your contract’s gas usage may be higher. because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size. The EVM needs to properly enforce the limits of this smaller type. It is only more efficient when you can pack variables of uint8 into the same storage slot with other neighboring variables smaller than 32 bytes. Note: I wouldn't comment much on uint16 and uint32 considering safe16 and safe 32 pure functions have been introduced dealing respectively with them both.

++i costs less gas compared to i++

++i costs less gas compared to i++ or i += 1 for unsigned integers considering the pre-increment operation is cheaper (about 5 GAS per iteration). i++ increments i and makes the compiler create a temporary variable for returning the initial value of i. In contrast, ++i returns the actual incremented value without making the compiler do extra job. As an example, the for loop in lines 292 - 300 could be refactored as:

for (uint256 i = 0; i < proposal.targets.length;) {
            queueOrRevertInternal(
                proposal.targets[i],
                proposal.values[i],
                proposal.signatures[i],
                proposal.calldatas[i],
                eta
            );

           unchecked {
                ++i;
            }
        }

Note: Unchecking safemath further saves gas in the above for loop.

Split require() statements that use &&

Split each single require statement with multiple conditions into multiple require statements with only 1 condition per require statement. This will save 3 GAS per &&.

> 0 is less efficient than != 0 for unsigned integers

!= 0 costs 6 less GAS compared to > 0 for unsigned integers in require statements or if statements (involving custom errors) with the optimizer enabled.

for !=0 before optimization
PUSH1 0x00
DUP2
EQ
ISZERO
PUSH1 [cont offset]
JUMPI 

after optimization
DUP1
PUSH1 [revert offset]
JUMPI

Public vs external visibility

Public functions (initialize, propose, _burnVetoPower, proposalThreshold, maxQuorumVotes) not called internally may be changed to the external visibility to save gas.

Variable initialization

Every variable assignment in Solidity costs gas. When initializing variables, we often waste gas by assigning default values. For instance, uint256 value; is cheaper than uint256 value = 0; Consider removing the assignments of 0 and false to the newProposal struct variables from lines 231 to 243.