Party DAO - 0xhacksmithh's results

[Gas-01] struct Can be packed more precisely

ETHPartyOptions struct can be re-order to save 1 storage slot

Previously 6 Slots used excluding arrays Now 5 Slots

Gas saved 2.1k

    struct ETHPartyOptions {
        // Name of the party.
        string name; // @audit string
        // Symbol of the party.
        string symbol;
        // The ID of the customization preset to use for the party card.
        uint256 customizationPresetId;
        // Options to initialize party governance with.
        Crowdfund.FixedGovernanceOpts governanceOpts;
        // Options to initialize party proposal engine with.
        ProposalStorage.ProposalEngineOpts proposalEngineOpts;
 +      uint40 rageQuitTimestamp;
        // The tokens that are considered precious by the party.These are
        // protected assets and are subject to extra restrictions in proposals
        // vs other assets.
        IERC721[] preciousTokens;
        // The IDs associated with each token in `preciousTokens`.
        uint256[] preciousTokenIds;
        // The timestamp until which ragequit is enabled.
 -      uint40 rageQuitTimestamp; // @audit G:: Re-arange to save gas
        // Initial authorities to set on the party
        address[] authorities;
    }

https://github.com/code-423n4/2023-10-party/blob/main/contracts/crowdfund/InitialETHCrowdfund.sol#L60

[Gas-02] Instead of >, != could used

-       if (initialContribution > 0) { 

+       if (initialContribution != 0) { 

-  if (amount > 0) { // @audit
+  if (amount != 0) { 
            // Get contributor to refund.
            address payable contributor = payable(party.ownerOf(tokenId));

-    fundingSplitBps_ > 0
+    fundingSplitBps_ != 0

https://github.com/code-423n4/2023-10-party/blob/main/contracts/crowdfund/InitialETHCrowdfund.sol#L141
https://github.com/code-423n4/2023-10-party/blob/main/contracts/crowdfund/InitialETHCrowdfund.sol#L331

https://github.com/code-423n4/2023-10-party/blob/main/contracts/crowdfund/ETHCrowdfundBase.sol#L324

[Gas-03] Unnecessary TypeCasting to IGateKeeper

Assembly could be used

-     if (_gateKeeper != IGateKeeper(address(0))) {
+     if (_gateKeeper != address(0)) {

https://github.com/code-423n4/2023-10-party/blob/main/contracts/crowdfund/InitialETHCrowdfund.sol#L289

[Gas-04] Instead of value + 1, unchecked{ ++value } could be used

    function _createParty(
        ETHPartyOptions memory opts,
        MetadataProvider customMetadataProvider,
        bytes memory customMetadata
    ) private returns (Party) {
-       uint256 authoritiesLength = opts.authorities.length + 1; 

+       uint256 authoritiesLength = unchecked{ ++opts.authorities.length}; 

https://github.com/code-423n4/2023-10-party/blob/main/contracts/crowdfund/InitialETHCrowdfund.sol#L377

            } else {
                // Entry is older. This is our best guess for now.
-               low = mid + 1;  // @audit G ::
+               low = unchecked{ ++mid };
            }

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L440

[Gas-05] Redundant state variable getters

Getters for public state variables are automatically generated by the solidity compiler so there is no need to code them manually as this increases deployment cost.

 mapping(uint256 => uint256) public votingPowerByTokenId; 

    function getDistributionShareOf(uint256 tokenId) external view returns (uint256) {  // @audit already defauylt getter there
        return votingPowerByTokenId[tokenId];
    }

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernanceNFT.sol#L146-L148

[Gas-06] Instead of type(uint96).max, VETO_VALUE should used

        uint96 private constant VETO_VALUE = type(uint96).max;

-       if (pv.votes == type(uint96).max) {
+       if (pv.votes == VETO_VALUE) {
            return ProposalStatus.Defeated;
        }

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L1083

[Gas-07] Use of Magic Number

        if (govOpts.feeBps > 1e4) { // @audit G :: Magic Numbuer
            revert InvalidBpsError(govOpts.feeBps);
        }
        if (govOpts.passThresholdBps > 1e4) { // @audit G ::
            revert InvalidBpsError(govOpts.passThresholdBps);
        }

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L277
https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L280

[Gas-08] i++ should be unchecked{++i} in loops. [This was missed in Bot report]

PartyGovernanceNFT.sol ( #L102, #L272, #L378, #L391, #L408 ):

102:             for (uint256 i; i < authorities.length; ++i) {

272:         for (uint256 i; i < tokenIds.length; ++i) {

378:             for (uint256 i; i < withdrawTokens.length; ++i) {

391:                 for (uint256 j; j < tokenIds.length; ++j) {

408:             for (uint256 i; i < withdrawTokens.length; ++i) {

PartyGovernance.sol ( #L305 )

305:         for (uint256 i = 0; i < govOpts.hosts.length; ++i) {

InitialETHCrowdfund.sol ( #L260 )

260:         for (uint256 i; i < args.recipients.length; ++i) {

[Gas-09] Assigning default value to variables, during creation of those.

-       uint256 low = 0;
+       uint256 low;
        while (low < high) {
            uint256 mid = (low + high) / 2; 

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L432

function _getProposalFlags(ProposalStateValues memory pv) private pure returns (uint256) {
-       uint256 flags = 0; 
+       uint256 flags; 

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L1054

[Gas-10] _getSharedProposalStorage() should be cached in memory rather than calling it again

          if (msg.sender != address(this)) {
            // Must not require a vote to create a distribution, otherwise
            // distributions can only be created through a distribution
            // proposal.
-           if (_getSharedProposalStorage().opts.distributionsRequireVote) { 
                revert DistributionsRequireVoteError();
            }
            // Must be an active member.
            VotingPowerSnapshot memory snap = _getLastVotingPowerSnapshotForVoter(msg.sender);
            if (snap.intrinsicVotingPower == 0 && snap.delegatedVotingPower == 0) {
                revert NotAuthorized();
            }
        }
        // Prevent creating a distribution if the party has not started.
-       if (_getSharedProposalStorage().governanceValues.totalVotingPower == 0) {
            revert PartyNotStartedError();
        }

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L500
https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L510

[Gas-11] type(uint256).max should stored in a constant state variable

emit BatchMetadataUpdate(0, type(uint256).max);

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L688

        emit BatchMetadataUpdate(0, type(uint256).max);

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L833

        return getVotingPowerAt(voter, timestamp, type(uint256).max);

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L359

return high == 0 ? type(uint256).max : high - 1;

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L445

        emit BatchMetadataUpdate(0, type(uint256).max);

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L520

 emit BatchMetadataUpdate(0, type(uint256).max);

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L581

            emit BatchMetadataUpdate(0, type(uint256).max);

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L655

emit BatchMetadataUpdate(0, type(uint256).max);

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L897

 if (hintIndex != type(uint256).max) {

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L926

[Gas-12] 2 times storage write to info.values could reduced to 1

        ProposalState storage info = _proposalStateByProposalId[proposalId];
        ProposalStateValues memory values = info.values;
........
........
        values.votes += votingPower; // @audit votes and voating power mixed here check pls
        if (isHost[msg.sender]) {
            ++values.numHostsAccepted;
        }
-       info.values = values; // @audit G :: 2 time storage write
        emit ProposalAccepted(proposalId, msg.sender, votingPower);

        // Update the proposal status if it has reached the pass threshold.
        if (
            values.passedTime == 0 &&
            _areVotesPassing(
                values.votes,
                values.totalVotingPower,
                _getSharedProposalStorage().governanceValues.passThresholdBps
            )
        ) {
-           info.values.passedTime = uint40(block.timestamp); // @audit here again
+           values.passedTime = uint40(block.timestamp);
+           info.values = values;
            emit ProposalPassed(proposalId);
            // Notify third-party platforms that the governance NFT metadata has
            // updated for all tokens.
            emit BatchMetadataUpdate(0, type(uint256).max);
        }
        return values.votes;

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L639
https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L651

[Gas-13] info.values could directly used instead of caching it to memory

    function veto(uint256 proposalId) external {
        _assertHost();
        // Setting `votes` to -1 indicates a veto.
        ProposalState storage info = _proposalStateByProposalId[proposalId];
-       ProposalStateValues memory values = info.values;

        {
-           ProposalStatus status = _getProposalStatus(values); // @audit G:: info.values
+           ProposalStatus status = _getProposalStatus(info.values);
            // Proposal must be in one of the following states.
            if (
                status != ProposalStatus.Voting &&
                status != ProposalStatus.Passed &&
                status != ProposalStatus.Ready
            ) {
                revert BadProposalStatusError(status);
            }
        }

        // -1 indicates veto.
        info.values.votes = VETO_VALUE;
        emit ProposalVetoed(proposalId, msg.sender);
        // Notify third-party platforms that the governance NFT metadata has
        // updated for all tokens.
        emit BatchMetadataUpdate(0, type(uint256).max); // @audit G::
    }

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L672

[Gas-14] Use named returns for local variables of pure functions where it is possible

Following test shows Gas difference

library NoNamedReturnArithmetic {

    function sum(uint256 num1, uint256 num2) internal pure returns(uint256){
        return num1 + num2;
    }
}

contract NoNamedReturn {
    using NoNamedReturnArithmetic for uint256;

    uint256 public stateVar;

    function add2State(uint256 num) public {
        stateVar = stateVar.sum(num);
    }
}

test for test/NoNamedReturn.t.sol:NamedReturnTest
[PASS] test_Increment() (gas: 27639)

library NamedReturnArithmetic {

    function sum(uint256 num1, uint256 num2) internal pure returns(uint256 theSum){
        theSum = num1 + num2;
    }
}

contract NamedReturn {
    using NamedReturnArithmetic for uint256;

    uint256 public stateVar;

    function add2State(uint256 num) public {
        stateVar = stateVar.sum(num);
    }
}

test for test/NamedReturn.t.sol:NamedReturnTest
[PASS] test_Increment() (gas: 27613)

    function _isUnanimousVotes(
        uint96 totalVotes,
        uint96 totalVotingPower
-   ) private pure returns (bool) { 
+   ) private pure returns (bool val) {  // @audit
        uint256 acceptanceRatio = (totalVotes * 1e4) / totalVotingPower;
-       return acceptanceRatio >= 0.9999e4;
+       val = acceptanceRatio >= 0.9999e4;
    }

    function _hostsAccepted(
        uint8 snapshotNumHosts,
        uint8 numHostsAccepted
-   ) private pure returns (bool) {  // @audit
-       return snapshotNumHosts > 0 && snapshotNumHosts == numHostsAccepted;
+   ) private pure returns (bool val) {  
+       val = snapshotNumHosts > 0 && snapshotNumHosts == numHostsAccepted;
    }

    function _areVotesPassing(
        uint96 voteCount,
        uint96 totalVotingPower,
        uint16 passThresholdBps
-   ) private pure returns (bool) {  // @audit
-       return (uint256(voteCount) * 1e4) / uint256(totalVotingPower) >= uint256(passThresholdBps);
+   ) private pure returns (bool) {  // @audit
+       return (uint256(voteCount) * 1e4) / uint256(totalVotingPower) >= uint256(passThresholdBps);
    }

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L1114
https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L1125
https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L1133