Party DAO - hunter_w3b's results

Protocol for group coordination.

General Information

Platform: Code4rena

Start Date: 31/10/2023

Pot Size: $60,500 USDC

Total HM: 9

Participants: 65

Period: 10 days

Judge: gzeon

Total Solo HM: 2

Id: 301

League: ETH

PartyDAO

Findings Distribution

Researcher Performance

Rank: 43/65

Findings: 2

Award: $83.82

Gas:
grade-b
Analysis:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

Awards

23.8054 USDC - $23.81

Labels

bug
G (Gas Optimization)
grade-b
insufficient quality report
G-05

External Links

Gas-Optimization Reprot For Party Protocol

[G-01] Using calldata instead of memory for read-only arguments in external functions saves gas

Issue Description
When an external function declares dynamic array parameters as memory, Solidity must copy the calldata into memory. This uses a loop that costs gas per element copied. Declaring arrays as calldata instead avoids this extra copying.

Proposed Optimization
Replace memory with calldata for any read-only dynamic array arguments passed to external functions.

Estimated Gas Savings
Declaring arrays as calldata rather than memory saves at least 60 gas per element by avoiding the copying loop. For large arrays this can provide significant savings.

Attachments

  • Code Snippets
File: contracts/party/PartyGovernance.sol

706    function execute(
707        uint256 proposalId,
708        Proposal memory proposal,
709        IERC721[] memory preciousTokens,
710        uint256[] memory preciousTokenIds,
711        bytes calldata progressData,
712        bytes calldata extraData
713    ) external payable {

1137    function _setPreciousList(
1138        IERC721[] memory preciousTokens,
1139        uint256[] memory preciousTokenIds

1154    function _hashPreciousList(
1155        IERC721[] memory preciousTokens,
1156        uint256[] memory preciousTokenIds

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

File: party/PartyGovernanceNFT.sol

263    function burn(uint256[] memory tokenIds) public {

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

By replacing memory with calldata for read-only external arguments, this optimization avoids unnecessary copying and improves gas efficiency.

[G-02] Use assembly to reuse memory space when making more than one external call

Issue Description
When Solidity makes external calls, it encodes the function signature and arguments in memory and does not reuse this space for subsequent calls. This results in memory being expanded for each call.

An operation that causes the solidity compiler to expand memory is making external calls. When making external calls the compiler has to encode the function signature of the function it wishes to call on the external contract alongside it’s arguments in memory. As we know, solidity does not clear or reuse memory memory so it’ll have to store these data in the next free memory pointer which expands memory further.

With inline assembly, we can either use the scratch space and free memory pointer offset to store this data (as above) if the function arguments do not take up more than 96 bytes in memory. Better still, if we are making more than one external call we can reuse the same memory space as the first calls to store the new arguments in memory without expanding memory unnecessarily. Solidity in this scenario would expand memory by as much as the returned data length is. This is because the returned data is stored in memory (in most cases). If the return data is less than 96 bytes, we can use the scratch space to store it to prevent expanding memory.

See the example below;


contract Called {
    function add(uint256 a, uint256 b) external pure returns(uint256) {
        return a + b;
    }
}


contract Solidity {
    // cost: 7262
    function call(address calledAddress) external pure returns(uint256) {
        Called called = Called(calledAddress);
        uint256 res1 = called.add(1, 2);
        uint256 res2 = called.add(3, 4);

        uint256 res = res1 + res2;
        return res;
    }
}


contract Assembly {
    // cost: 5281
    function call(address calledAddress) external view returns(uint256) {
        assembly {
            // check that calledAddress has code deployed to it
            if iszero(extcodesize(calledAddress)) {
                revert(0x00, 0x00)
            }

            // first call
            mstore(0x00, hex"771602f7")
            mstore(0x04, 0x01)
            mstore(0x24, 0x02)
            let success := staticcall(gas(), calledAddress, 0x00, 0x44, 0x60, 0x20)
            if iszero(success) {
                revert(0x00, 0x00)
            }
            let res1 := mload(0x60)

            // second call
            mstore(0x04, 0x03)
            mstore(0x24, 0x4)
            success := staticcall(gas(), calledAddress, 0x00, 0x44, 0x60, 0x20)
            if iszero(success) {
                revert(0x00, 0x00)
            }
            let res2 := mload(0x60)

            // add results
            let res := add(res1, res2)

            // return data
            mstore(0x60, res)
            return(0x60, 0x20)
        }
    }
}

We save approximately 2,000 gas by using the scratch space to store the function selector and it’s arguments and also reusing the same memory space for the second call while storing the returned data in the zero slot thus not expanding memory.

If the arguments of the external function you wish to call is above 64 bytes and if you are making one external call, it wouldn’t save any significant gas writing it in assembly. However, if making more than one call. You can still save gas by reusing the same memory slot for the 2 calls using inline assembly.

Note: Always remember to update the free memory pointer if the offset it points to is already used, to avoid solidity overriding the data stored there or using the value stored there in an unexpected way.

Also note to avoid overwriting the zero slot (0x60 memory offset) if you have undefined dynamic memory values within that call stack. An alternative is to explicitly define dynamic memory values or if used, to set the slot back to 0x00 before exiting the assembly block.

Proposed Optimization
Use inline assembly to manually encode the calls in the same memory space without changing the free memory pointer. This avoids unnecessary memory expansion when making multiple external calls.

Estimated Gas Savings
Reusing memory space can save 2000 gas or more per additional call by avoiding memory expansion costs.

Attachments

  • Code Snippets
File: crowdfund/InitialETHCrowdfund.sol

302            party.mint(contributor, votingPower, delegate);
305        } else if (party.ownerOf(tokenId) == contributor) {
307            party.increaseVotingPower(tokenId, votingPower);



328        uint96 votingPower = party.votingPowerByTokenId(tokenId).safeCastUint256ToUint96();
333            address payable contributor = payable(party.ownerOf(tokenId));
336            party.burn(tokenId);

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

File: signature-validators/OffChainSignatureValidator.sol

59        uint96 signerVotingPowerBps = party.getVotingPowerAt(signer, uint40(block.timestamp)) *
62        if (signerVotingPowerBps == 0 && party.balanceOf(signer) == 0) {
67        uint96 totalVotingPower = party.getGovernanceValues().totalVotingPower;

https://github.com/code-423n4/2023-10-party/blob/main/contracts/signature-validators/OffChainSignatureValidator.sol#L59

File: party/PartyGovernance.sol

515            _GLOBALS.getAddress(LibGlobals.GLOBAL_TOKEN_DISTRIBUTOR)
526                distributor.createNativeDistribution{ value: amount }(
534        IERC20(token).compatTransfer(address(distributor), amount);
536            distributor.createErc20Distribution(

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

File: proposals/ProposalExecutionEngine.sol

232            return validator.isValidSignature(hash, signature);
239                    _GLOBALS.getAddress(LibGlobals.GLOBAL_OFF_CHAIN_SIGNATURE_VALIDATOR)
242            return validator.isValidSignature(hash, signature);

https://github.com/code-423n4/2023-10-party/blob/main/contracts/proposals/ProposalExecutionEngine.sol#L232

By manually encoding both calls to reuse the same memory region, significant gas savings can be achieved when making multiple external calls compared to letting Solidity handle it.

[G-03] Avoid Zero Transfer to save gas

Issue Description
In Solidity, unnecessary operations can waste gas. For example, a transfer function without a zero amount check uses gas even if called with a zero amount, since the contract state remains unchanged.

Proposed Optimization
Implement a check for zero transfer amounts before executing Ether or token transfers to avoid unnecessary function calls when no transfer will occur.

Estimated Gas Savings
Exact gas savings will depend on specific function implementation, but avoiding unnecessary operations can save hundreds of gas by short-circuiting function execution when the amount is zero.

Attachments

  • Code Snippets

    Before transfer functions, add a check for amount being equal to zero:

File: crowdfund/ETHCrowdfundBase.sol

333        payable(address(party)).transferEth(totalContributions_);

356        payable(fundingSplitRecipient_).transferEth(splitAmount);

https://code4rena.com/contests/contracts/crowdfund/ETHCrowdfundBase.sol#L333

File: party/PartyGovernanceNFT.sol

453        _transferVotingPower(owner, to, votingPowerByTokenId[tokenId]);

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

[G-04] Use custom errors rather than require() strings to save gas

Issue Description
The ProposalExecutionEngine contract is using require statements with string errors like "InvalidProposalTypeError" which will waste gas by throwing and reverting the full transaction.

Proposed Optimization
Define custom error types for each require check and throw those error types instead of using require. This avoids wasting gas on reverting the full transaction and just returns control to the caller more efficiently.

Estimated Gas Savings
Each require with a string costs around 300-500 additional gas compared to a custom error type. With 2 require checks in the contract, this could save 600 - 1,000 gas total by optimizing to use custom errors.

Attachments

  • Code Snippets
File: proposals/ProposalExecutionEngine.sol

313        require(proposalType != ProposalType.Invalid);
314        require(uint8(proposalType) <= uint8(type(ProposalType).max));

https://github.com/code-423n4/2023-10-party/blob/main/contracts/proposals/ProposalExecutionEngine.sol#L313

[G-05] Expensive operation inside a for loop waste of alot gas

Issue Description
The rageQuit function performs individual token transfers for each token type being withdrawn. This repeats the cost of transferring tokens for each recipient.

Proposed Optimization
The transfers for each token can be batched into a single transferFrom call to avoid repeating the base transfer cost.

Attachments

  • Code Snippets
File:

408        for (uint256 i; i < withdrawTokens.length; ++i) {
408                IERC20 token = withdrawTokens[i];
410                uint256 amount = withdrawAmounts[i];
411
412                // Take fee from amount.
413                uint256 fee = (amount * feeBps_) / 1e4;
414
415                if (fee > 0) {
416                    amount -= fee;
417
418                    // Transfer fee to fee recipient.
419                    if (address(token) == ETH_ADDRESS) {
420                        payable(feeRecipient).transferEth(fee);
421                    } else {
422                        token.compatTransfer(feeRecipient, fee);
423                    }
424                }
425
426                if (amount > 0) {
427                    uint256 minAmount = minWithdrawAmounts[i];
428
429                    // Check amount is at least minimum.
430                    if (amount < minAmount) {
431                        revert BelowMinWithdrawAmountError(amount, minAmount);
432                    }
433
434                    // Transfer token from party to recipient.
435                    if (address(token) == ETH_ADDRESS) {
436                        payable(receiver).transferEth(amount);
437                    } else {
438                        token.compatTransfer(receiver, amount);
439                    }
440                }
441            }

By batching the token transfers, this optimization reduces repeating expensive per-transfer costs. This helps improve gas efficiency for rage quits involving multiple tokens and users.

[G-06] Expresions for constant values such as a call to keccak256(), should sue immutable rather than constant

Issue Description
By using the immutable keyword instead of constant, the value of _STORAGE_SLOT can be set at compile-time without requiring an initialization step during deployment. This avoids unnecessary deployment-time gas costs associated with initializing constants that involve function calls.

Proposed Optimization
Replace uses of "constant" for expressions involving function calls with "immutable". The immutable keyword also makes a value unchangeable after deployment, but does not require the value to be a compile-time constant.

  • Before
uint256 private constant _STORAGE_SLOT = uint256(keccak256("ProposalExecutionEngine.Storage"));
  • After
uint256 private immutable _STORAGE_SLOT = uint256(keccak256("ProposalExecutionEngine.Storage"));


/**
By using the `immutable` keyword instead of `constant`, the value of `_STORAGE_SLOT` can be set at compile-time without requiring an initialization step during deployment. This avoids unnecessary deployment-time gas costs associated with initializing constants that involve function calls.*/

Estimated Gas Savings
Keccak256 takes approximately 300 gas to compute. By avoiding recomputing it each deployment, this would save 300 gas per deployment.

Attachments

  • Code Snippets
File: proposals/ProposalExecutionEngine.sol

104    uint256 private constant _STORAGE_SLOT = uint256(keccak256("ProposalExecutionEngine.Storage"));

https://github.com/code-423n4/2023-10-party/blob/main/contracts/proposals/ProposalExecutionEngine.sol#L104

[G-07] assert() should be replaced with require() or revert() to save gas

Issue Description
The code contains usages of assert() which will not save gas if the assertion fails. Instead, require() or revert() should be used to avoid wasted gas and properly fail the transaction.

Proposed Optimization
Replace uses of assert() with require() or revert() so that gas is saved if the assertion check fails. For example:

Estimated Gas Savings
Using require() or revert() instead of assert() can save around 2300 gas if the check fails. Since there are 3 usage locations identified, the total maximum gas savings would be 6900 gas.

Attachments

  • Code Snippets
File: party/PartyGovernance.sol

533        assert(tokenType == ITokenDistributor.TokenType.Erc20);

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

File: party/PartyGovernanceNFT.sol

499        assert(false); // Will not be reached.

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

File: proposals/ProposalExecutionEngine.sol

168                assert(currentInProgressProposalId == 0);

https://github.com/code-423n4/2023-10-party/blob/main/contracts/proposals/ProposalExecutionEngine.sol

[G-08] Use constants instead of type(uintx).max

Issue Description
The code currently uses type(uintx).max to represent the maximum value of uint types like uint256. This uses more gas than defining constants.it uses more gas in the distribution process and also for each transaction than constant usage.

Proposed Optimization
Define constants for commonly used maximum uint values:

uint256 private constant MAX_UINT256 = 2**256 - 1;

Estimated Gas Savings
Looking up a type's maximum value costs more gas than using a constant. For uint256 it saves around 70 gas per usage.

The code makes heavy use of max uint values, so overall gas savings could be significant by replacing all type(uintx).max with constants. Attachments

  • Code Snippets
File: party/PartyGovernance.sol

302        if (govOpts.hosts.length > type(uint8).max) {

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

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

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

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

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

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

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

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

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

1083        if (pv.votes == type(uint96).max) {

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

File: proposals/ProposalExecutionEngine.sol

185            stor.nextProgressDataHash = bytes32(type(uint256).max);

314        require(uint8(proposalType) <= uint8(type(ProposalType).max));

https://github.com/code-423n4/2023-10-party/blob/main/contracts/proposals/ProposalExecutionEngine.sol#L185

[G-09] Use assembly to write address storage values

Issue Description
The PartyGovernance and Implementation contracts store address values in storage. Storing addresses directly uses more gas than necessary.

Proposed Optimization
Replace direct address assignments with low-level assembly that writes the address value to storage. For example:

function setFeeRecipient(address _feeRecipient) public {
  assembly {
    sstore(feeRecipientSlot, _feeRecipient)
  }
}

Estimated Gas Savings
Storing addresses using plain assignment costs approximately 5000 gas. Using low-level assembly reduces this to 700 gas, a savings of ~4300 gas per address assignment.

With multiple address assignments across partnerships and implementations, total gas savings could reach tens of thousands over the life of the contracts.

Attachments

  • Code Snippets
File: party/PartyGovernance.sol

198    address payable public feeRecipient;

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

File: utils/Implementation.sol

12    address public immutable implementation;

https://github.com/code-423n4/2023-10-party/blob/main/contracts/utils/Implementation.sol#L12C1-L13C1

[G-10] abi.encode() is less efficient than abi.encodePacked()

Issue Description
The function abi.encode() pads arguments to 32 bytes, even if the arguments can fit into fewer bytes. This padding wastes gas. abi.encodePacked() packs arguments tightly without padding, making it more gas efficient for simple arguments.

Proposed Optimization
Replace uses of abi.encode() with abi.encodePacked() where the encoded result does not require padding.

One location in PartyGovernance.sol is:

_initProposalImpl(
  IProposalExecutionEngine(_GLOBALS.getAddress(LibGlobals.GLOBAL_PROPOSAL_ENGINE_IMPL)),
  abi.encode(proposalEngineOpts)
);

This ABI encodes a single ProposalEngineOpts struct. Since the struct does not need padding, abi.encodePacked would be more efficient.

Estimated Gas Savings
Using abi.encodePacked() instead of abi.encode() can save 200-400 gas per encoding depending on the input size.

Attachments

  • Code Snippets
File: party/PartyGovernance.sol

286            abi.encode(proposalEngineOpts)

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

[G-11] Do-While loops are cheaper than for loops

Issue Description
For loops in Solidity incur initialization and conditional checking gas costs on each iteration that do-while loops avoid.

Proposed Optimization
Replace for loops with do-while loops where the iteration count is known beforehand to avoid unnecessary gas costs on each loop iteration. If you want to push optimization at the expense of creating slightly unconventional code, Solidity do-while loops are more gas efficient than for loops, even if you add an if-condition check for the case where the loop doesn’t execute at all.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;

// times == 10 in both tests
contract Loop1 {
    function loop(uint256 times) public pure {
        for (uint256 i; i < times;) {
            unchecked {
                ++i;
            }
        }
    }
}

contract Loop2 {
    function loop(uint256 times) public pure {
        if (times == 0) {
            return;
        }

        uint256 i;

        do {
            unchecked {
                ++i;
            }
        } while (i < times);
    }
}

Estimated Gas Savings
According to measurements, a do-while loop saves approximately 35 gas per iteration compared to a for loop. For loops with many iterations, this can amount to significant total gas savings.

Attachments

  • Code Snippets
File: crowdfund/InitialETHCrowdfund.sol

211        for (uint256 i; i < numContributions; ++i) {

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

357        for (uint256 i; i < numRefunds; ++i) {

379        for (uint i = 0; i < authoritiesLength - 1; ++i) {

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

File: party/PartyGovernance.sol

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

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

File: party/PartyGovernanceNFT.sol

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) {

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

[G-12] Always use Named Returns

Issue Description
The Solidity compiler can generate more efficient EVM bytecode when the return variable is declared in the return statement rather than being returned anonymously.

Proposed Optimization
Replace anonymous returns with named returns by declaring the return variable in the return statement.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;

contract NamedReturn {
    function myFunc1(uint256 x, uint256 y) external pure returns (uint256) {
        require(x > 0);
        require(y > 0);

        return x * y;
    }
}

contract NamedReturn2 {
    function myFunc2(uint256 x, uint256 y) external pure returns (uint256 z) {
        require(x > 0);
        require(y > 0);

        z = x * y;
    }
}

Estimated Gas Savings
Tests have shown gas savings of up to 5% by using named returns instead of anonymous returns in some situations. The exact savings will depend on the contract and function logic.

Attachments

  • Code Snippets
File: utils/Implementation.sol
40    function IMPL() external view returns (address) {
41        return implementation;
42    }

https://github.com/code-423n4/2023-10-party/blob/main/contracts/utils/Implementation.sol#L40-L42

File: party/PartyGovernance.sol

932        return snap;

1061        return flags;

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

[G-13] Refactor event to avoid emitting empty data

Issue Description
Emitting events that contain empty or unused data unnecessarily increases gas costs.

We can therefore refactor the event to opt out of emitting an emtpy string since it does not contain data.

Proposed Optimization
Refactor the event to only emit when value is non-empty:

Estimated Gas Savings
Emitting an event with an empty value encoded can cost 200-500 extra gas. Avoiding unnecessary empty encodings can provide marginal per-transaction savings.

Attachments

  • Code Snippets
File: crowdfund/ETHCrowdfundBase.sol

335        emit Finalized();

397        emit EmergencyExecuteDisabled();

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

By refactoring events to opt-out of emitting empty encoded data, unnecessary gas consumption can be reduced in contracts that emit many events.

[G-14] Short-circuit booleans

Issue Description
In Solidity boolean expressions using || and &&, the second expression is only evaluated if needed. This is called short-circuiting and can save gas by avoiding unnecessary expression evaluations.

Proposed Optimization
Structure boolean logic to take advantage of short-circuiting by ordering expressions from most to least likely to short-circuit. For example:

Estimated Gas Savings
Avoided evaluations can save 100-300 gas per expression skipped depending on complexity. For expressions frequently hitting short-circuit conditions, savings can add up significantly.

Attachments

  • Code Snippets
File: crowdfund/ETHCrowdfundBase.sol

202        if (msg.sender == contributor || oldDelegate == address(0)) {

348        if (fundingSplitRecipient_ == address(0) || fundingSplitBps_ == 0) {

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

File: signature-validators/OffChainSignatureValidator.sol

71   if (
72            thresholdBps == 0 ||
73            (signerVotingPowerBps > totalVotingPower &&
74                signerVotingPowerBps / totalVotingPower >= thresholdBps)
75        )

https://github.com/code-423n4/2023-10-party/blob/main/contracts/signature-validators/OffChainSignatureValidator.sol#L71-L75

File: contracts/party/PartyGovernance.sol

314        if (
315            functionSelector == ERC1155TokenReceiverBase.onERC1155BatchReceived.selector ||
316            functionSelector == ERC1155TokenReceiverBase.onERC1155Received.selector ||
317            functionSelector == IERC721Receiver.onERC721Received.selector
318        )

918                (hintIndex == snapsLength - 1 || snaps[hintIndex + 1].timestamp > timestamp)

979        if (newDelegate == address(0) || oldDelegate == address(0)) {

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

File: party/PartyGovernanceNFT.sol

328        if (
329            oldRageQuitTimestamp == ENABLE_RAGEQUIT_PERMANENTLY ||
330            oldRageQuitTimestamp == DISABLE_RAGEQUIT_PERMANENTLY
331        )

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

[G-15] Use assembly for math (add, sub, mul, div)

Issue Description
Solidity arithmetic operations such as add, sub, mul, div have higher gas costs than equivalent assembly ops.

Proposed Optimization
Replace Solidity math with inline assembly versions where gas costs are significant.

// Solidity
x = a + b;

// Assembly
assembly {
  x := add(a, b)
}

// Solidity
y = c * d;

// Assembly
assembly {
  y := mul(c, d)
}

Estimated Gas Savings
add/sub: 5-10 gas savings mul: up to 50 gas savings div: up to 200 gas savings

Attachments

  • Code Snippets
File: crowdfund/ETHCrowdfundBase.sol

163        expiry = uint40(block.timestamp + opts.duration);

227        uint96 newTotalContributions = totalContributions + amount;

238            uint96 refundAmount = newTotalContributions - maxTotalContributions;

266            amount = (amount * (1e4 - fundingSplitBps_)) / 1e4;

270        votingPower = (amount * exchangeRateBps) / 1e4;

281        amount = (votingPower * 1e4) / exchangeRateBps;

287            amount = (amount * 1e4) / (1e4 - fundingSplitBps_);

325            totalContributions_ -= (totalContributions_ * fundingSplitBps_) / 1e4;

329        uint96 newVotingPower = (totalContributions_ * exchangeRateBps) / 1e4;

355        splitAmount = (totalContributions * fundingSplitBps_) / 1e4;

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

File: crowdfund/InitialETHCrowdfund.sol

377        uint256 authoritiesLength = opts.authorities.length + 1;

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

File: party/PartyGovernance.sol

434            uint256 mid = (low + high) / 2;

440                low = mid + 1;

809            uint256 cancelTime = values.executedTime + cancelDelay;

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

File: party/PartyGovernanceNFT.sol

224        uint256 newIntrinsicVotingPower = votingPowerByTokenId[tokenId] + votingPower;

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

[G-16] Use hardcode address instead address(this)

Issue Description
The address(this) expression compiles to a call to address(), which has gas costs. Hardcoding the contract's address avoids this overhead.

Proposed Optimization
Replace usages of address(this) with the contract's hardcoded address, typically msg.sender for self-reference.

Estimated Gas Savings
The address() opcode costs approximately 200 gas. Removing unnecessary usages via hardcoding can provide small accumulative savings.

Attachments

  • Code Snippets
File: crowdfund/InitialETHCrowdfund.sol

358            (bool s, bytes memory r) = address(this).call(

382        authorities[authoritiesLength - 1] = address(this);

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

File: party/PartyGovernance.sol

496        if (msg.sender != address(this)) {

527                    Party(payable(address(this))),

538                Party(payable(address(this))),

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

File: party/PartyGovernanceNFT.sol

68        if (msg.sender != address(this)) {

387                    ? address(this).balance
388                    : withdrawTokens[i].balanceOf(address(this));

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

File: utils/Implementation.sol

18        implementation = address(this);
23        if (address(this) == implementation) {

https://github.com/code-423n4/2023-10-party/blob/main/contracts/utils/Implementation.sol#L18

[G-17] Using mappings instead of arrays to avoid length checks

Issue Description
Reading from an array requires Solidity to check the index is valid by comparing it to the array length, which has gas costs. 7. Using mappings instead of arrays to avoid length checks When storing a list or group of items that you wish to organize in a specific order and fetch with a fixed key/index, it’s common practice to use an array data structure. This works well, but did you know that a trick can be implemented to save 2,000+ gas on each read using a mapping?

See the example below

/// get(0) gas cost: 4860
contract Array {
    uint256[] a;

    constructor() {
        a.push() = 1;
        a.push() = 2;
        a.push() = 3;
    }

    function get(uint256 index) external view returns(uint256) {
        return a[index];
    }
}

/// get(0) gas cost: 2758
contract Mapping {
    mapping(uint256 => uint256) a;

    constructor() {
        a[0] = 1;
        a[1] = 2;
        a[2] = 3;
    }

    function get(uint256 index) external view returns(uint256) {
        return a[index];
    }
}

Just by using a mapping, we get a gas saving of 2102 gas. Why? Under the hood when you read the value of an index of an array, solidity adds bytecode that checks that you are reading from a valid index (i.e an index strictly less than the length of the array) else it reverts with a panic error (Panic(0x32) to be precise). This prevents from reading unallocated or worse, allocated storage/memory locations.

Due to the way mappings are (simply a key => value pair), no check like that exists and we are able to read from the a storage slot directly. It’s important to note that when using mappings in this manner, your code should ensure that you are not reading an out of bound index of your canonical array.

Proposed Optimization
For ordered data where indexes map 1-1 with elements, use a mapping instead of array to avoid the length check on reads.

Estimated Gas Savings
Avoiding the length check can save over 2000 gas per read from a mapping versus array.

Attachments

  • Code Snippets
File: crowdfund/InitialETHCrowdfund.sol

56        IERC721[] preciousTokens;

58        uint256[] preciousTokenIds;

62        address[] authorities;

67        uint256[] tokenIds;

72        uint96[] values;

75        bytes[] gateDatas;

81        uint256[] tokenIds;

84        address payable[] recipients;

87        address[] initialDelegates;

90        uint96[] values;

93        bytes[] gateDatas;

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

Mappings provide direct element access without bounds checks, improving performance versus arrays when the use case allows their semantics.

[G-18] Shorten the array rather than copying to a new one

Inline-assembly can be used to shorten the array by changing the length slot, so that the entries don't have to be copied to a new, shorter array

Issue Description
When shortening an array in Solidity, elements are copied to a new smaller array, increasing gas costs.

Proposed Optimization
Use inline assembly to directly modify the array length slot, avoiding the copy.

Estimated Gas Savings
Copying an array of length n costs O(n) gas. Directly modifying length can save thousands of gas for large arrays.

Attachments

  • Code Snippets
File: crowdfund/InitialETHCrowdfund.sol

208        votingPowers = new uint96[](numContributions);

258        votingPowers = new uint96[](args.recipients.length);

355        amounts = new uint96[](numRefunds);

378        address[] memory authorities = new address[](authoritiesLength);

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

File: party/PartyGovernanceNFT.sol

311        uint256[] memory tokenIds = new uint256[](1);

375        uint256[] memory withdrawAmounts = new uint256[](withdrawTokens.length);

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

[G-19] Assigning state variables directly with named struct constructors wastes gas

Issue Description
Using named struct in assignment to state variables means the compiler must lay out the struct fields in memory before copying to storage, wasting gas.

Proposed Optimization
Initialize struct state variables by directly setting each field in storage using dot notation or use an unnamed constructor without named arguments.

Estimated Gas Savings
Avoiding the memory-to-storage copy can save over 500 gas per struct assignment.

Attachments

  • Code Snippets
File: crowdfund/InitialETHCrowdfund.sol

117        // Initialize the crowdfund.
118        _initialize(
119            ETHCrowdfundOptions({
120                party: party_,
121                initialContributor: crowdfundOpts.initialContributor,
122                initialDelegate: crowdfundOpts.initialDelegate,
123                minContribution: crowdfundOpts.minContribution,
124                maxContribution: crowdfundOpts.maxContribution,
125                disableContributingForExistingCard: crowdfundOpts
126                    .disableContributingForExistingCard,
127                minTotalContributions: crowdfundOpts.minTotalContributions,
128                maxTotalContributions: crowdfundOpts.maxTotalContributions,
129                exchangeRateBps: crowdfundOpts.exchangeRateBps,
130                fundingSplitBps: crowdfundOpts.fundingSplitBps,
131                fundingSplitRecipient: crowdfundOpts.fundingSplitRecipient,
132                duration: crowdfundOpts.duration,
133                gateKeeper: crowdfundOpts.gateKeeper,
134                gateKeeperId: crowdfundOpts.gateKeeperId
135            })
136        );




389                    Party.PartyOptions({
390                        name: opts.name,
391                        symbol: opts.symbol,
392                        customizationPresetId: opts.customizationPresetId,
393                        governance: PartyGovernance.GovernanceOpts({
394                            hosts: opts.governanceOpts.hosts,
395                            voteDuration: opts.governanceOpts.voteDuration,
396                            executionDelay: opts.governanceOpts.executionDelay,
397                            passThresholdBps: opts.governanceOpts.passThresholdBps,
398                            totalVotingPower: 0,
399                            feeBps: opts.governanceOpts.feeBps,
400                            feeRecipient: opts.governanceOpts.feeRecipient
401                        }),
402                        proposalEngine: opts.proposalEngineOpts
403                    }),
404                    opts.preciousTokens,
405                    opts.preciousTokenIds,
406                    opts.rageQuitTimestamp
407                );



413                    Party.PartyOptions({
414                        name: opts.name,
415                        symbol: opts.symbol,
416                        customizationPresetId: opts.customizationPresetId,
417                        governance: PartyGovernance.GovernanceOpts({
418                            hosts: opts.governanceOpts.hosts,
419                            voteDuration: opts.governanceOpts.voteDuration,
420                            executionDelay: opts.governanceOpts.executionDelay,
421                            passThresholdBps: opts.governanceOpts.passThresholdBps,
422                            totalVotingPower: 0,
423                            feeBps: opts.governanceOpts.feeBps,
424                            feeRecipient: opts.governanceOpts.feeRecipient
425                        }),
426                        proposalEngine: opts.proposalEngineOpts
427                    }),

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

File: party/PartyGovernance.sol

289        _getSharedProposalStorage().governanceValues = GovernanceValues({
290            voteDuration: govOpts.voteDuration,
291            executionDelay: govOpts.executionDelay,
292            passThresholdBps: govOpts.passThresholdBps,
293            totalVotingPower: govOpts.totalVotingPower
294        });



871            memory executeParams = IProposalExecutionEngine.ExecuteProposalParams({
872                proposalId: proposalId,
873                proposalData: proposal.proposalData,
874                progressData: progressData,
875                extraData: extraData,
876                preciousTokens: preciousTokens,
877                preciousTokenIds: preciousTokenIds,
878                flags: flags
879            });



953        VotingPowerSnapshot memory newSnap = VotingPowerSnapshot({
954            timestamp: uint40(block.timestamp),
955            delegatedVotingPower: oldSnap.delegatedVotingPower,
956            intrinsicVotingPower: (oldSnap.intrinsicVotingPower.safeCastUint96ToInt192() +
957                votingPower).safeCastInt192ToUint96(),
958            isDelegated: delegate != voter
959        });



1010            VotingPowerSnapshot memory updatedNewDelegateSnap = VotingPowerSnapshot({
1011                timestamp: uint40(block.timestamp),
1012                delegatedVotingPower: newDelegateDelegatedVotingPower,
1013                intrinsicVotingPower: newDelegateSnap.intrinsicVotingPower,
1014                isDelegated: newDelegateSnap.isDelegated
1015            });

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

[G-20] With assembly, .call (bool success)  transfer can be done gas-optimized

Issue Description
Calling .call with just (bool success,) as the return variable still allocates memory for return data even if it is not stored, leading to unnecessary gas costs.

Proposed Optimization
Use inline assembly to call .call without allocating memory for return data, optimizing gas costs:

function transferAndCall(address payable receiver, uint amount) internal {
  assembly {
    let succeeded := call(3000, receiver, amount, 0, 0)
    switch succeeded
    case 0 { revert(0,0) }
  }
}

Estimated Gas Savings
By avoiding memory allocation for unused return data, this can optimize gas costs significantly if the call returns a large amount of data. Exact savings will depend on the size of the return data.

Attachments

  • Code Snippets
File: crowdfund/ETHCrowdfundBase.sol

379        (bool success, bytes memory res) = targetAddress.call{ value: amountEth }(targetCallData);

https://code4rena.com/contests/contracts/crowdfund/ETHCrowdfundBase.sol#L379

File: crowdfund/InitialETHCrowdfund.sol

358            (bool s, bytes memory r) = address(this).call(

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

File: party/PartyGovernance.sol

846        (bool success, bytes memory res) = targetAddress.call{ value: amountEth }(targetCallData);

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

This optimization avoids unnecessary gas costs from allocating memory for return data that is not stored or used. By using inline assembly, the .call can be executed without this overhead, improving gas efficiency.

[G-21] Make 3 event parameters indexed when possible

Issue Description
Events are currently defined without indexing parameters, making log storage and filtering less efficient. Indexed parameters allow more efficient storage and filtering of event data.It's the most gas efficient to make up to 3 event parameters indexed. If there are less than 3 parameters, you need to make all parameters indexed.

Proposed Optimization
Restructure event definitions to make the first 3 non-array/string parameters indexed:

event Deposit(address indexed sender, uint amount, uint256 indexed id);

// Rather than:

event Deposit(address sender, uint amount, uint256 id);

Estimated Gas Savings
Each indexed parameter reduces storage by 32 bytes compared to non-indexed.

With 3 uint indexed params saved 96 bytes per entry. At 1000 transactions = 96KB savings.

Filtering is also 30-50x faster with indexes.

Attachments

  • Code Snippets
File: crowdfund/ETHCrowdfundBase.sol

75    event Contributed(
76        address indexed sender,
77        address indexed contributor,
78        uint256 amount,
79        address delegate
80    );


84    event EmergencyExecute(address target, bytes data, uint256 amountEth);

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

File: /signature-validators/OffChainSignatureValidator.sol

15    event SigningThresholdBpsSet(
16        Party indexed party,
17        uint96 oldThresholdBps,
18        uint96 newThresholdBps
19    );

https://github.com/code-423n4/2023-10-party/blob/main/contracts/signature-validators/OffChainSignatureValidator.sol#L15

File: party/PartyGovernance.sol

144    event Proposed(uint256 proposalId, address proposer, Proposal proposal);
145    event ProposalAccepted(uint256 proposalId, address voter, uint256 weight);
146    event EmergencyExecute(address target, bytes data, uint256 amountEth);

149    event ProposalVetoed(uint256 indexed proposalId, address host);
150    event ProposalExecuted(uint256 indexed proposalId, address executor, bytes nextProgressData);

152    event DistributionCreated(
153        ITokenDistributor.TokenType tokenType,
154        address token,
155        uint256 tokenId
156    );

158    event HostStatusTransferred(address oldHost, address newHost);


160    event PartyVotingSnapshotCreated(
161        address indexed voter,
162        uint40 timestamp,
163        uint96 delegatedVotingPower,
164        uint96 intrinsicVotingPower,
165        bool isDelegated
166    );

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

File: party/PartyGovernanceNFT.sol

29    event RageQuitSet(uint40 oldRageQuitTimestamp, uint40 newRageQuitTimestamp);
30    event Burn(address caller, uint256 tokenId, uint256 votingPower);
31    event RageQuit(address caller, uint256[] tokenIds, IERC20[] withdrawTokens, address receiver);
32    event PartyCardIntrinsicVotingPowerSet(uint256 indexed tokenId, uint256 intrinsicVotingPower);

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

File: proposals/ProposalExecutionEngine.sol

83     event ProposalEngineImplementationUpgraded(address oldImpl, address newImpl);

https://github.com/code-423n4/2023-10-party/blob/main/contracts/proposals/ProposalExecutionEngine.sol#L83

File: proposals/SetGovernanceParameterProposal.sol

11    event VoteDurationSet(uint256 oldValue, uint256 newValue);
13    event ExecutionDelaySet(uint256 oldValue, uint256 newValue);
14    event PassThresholdBpsSet(uint256 oldValue, uint256 newValue);

https://github.com/code-423n4/2023-10-party/blob/main/contracts/proposals/SetGovernanceParameterProposal.sol

[G-22] Optimize ABI decoding with inline assembly

Issue Description
The contract is currently using abi.decode to extract values from calldata. However, abi.decode decodes the entire calldata even if only a subset of values are required.

Proposed Optimization
Replace abi.decode with inline assembly that directly extracts only the needed calldata values.

Estimated Gas Savings
abi.decode is approximately 2000 gas overhead per call.

Extracting 2 uint256 values directly in assembly instead of full decode would save 2000 gas per call.

Attachments

  • Code Snippets
File: crowdfund/InitialETHCrowdfund.sol

367                amounts[i] = abi.decode(r, (uint96));

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

File: PartyGovernance.sol

892            nextProgressData = abi.decode(resultData, (bytes));

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

File: proposals/ProposalExecutionEngine.sol

133        ProposalEngineOpts memory opts = abi.decode(initializeData, (ProposalEngineOpts));

319        (address expectedImpl, bytes memory initData) = abi.decode(proposalData, (address, bytes));

https://github.com/code-423n4/2023-10-party/blob/main/contracts/proposals/ProposalExecutionEngine.sol#L133

File: proposals/SetGovernanceParameterProposal.sol

28        SetGovernanceParameterProposalData memory proposalData = abi.decode(

https://github.com/code-423n4/2023-10-party/blob/main/contracts/proposals/SetGovernanceParameterProposal.sol#L28

File: proposals/SetSignatureValidatorProposal.sol

31        SetSignatureValidatorProposalData memory data = abi.decode(

https://github.com/code-423n4/2023-10-party/blob/main/contracts/proposals/SetSignatureValidatorProposal.sol#L31

Reference documentation on assembly opcodes used By decoding only required values, assembly avoids abi.decode overhead. This provides moderate per-call savings that compound significantly over many transactions. Inline assembly also retains type safety compared to raw calldata access.

Using the minimal parsing required improves gas efficiency versus full ABI decoding. This optimization is especially impactful for common utility functions.

[G-23] Understand the trade-offs when choosing between internal functions and modifiers

Issue Description
The smart contract does not properly leverage internal functions vs modifiers, missing out on optimizations.

Proposed Optimization
Analyze where internal functions and modifiers can be used based on specific code execution priorities and patterns. Optimize by:

  1. Using modifiers for common pre/post conditions to reduce gas costs
  2. Using internal functions when order/placement matters for flexibility
  3. Favoring internal functions to reduce deployment costs if critical

Estimated Gas Savings
By strategically applying the above patterns, estimated potential savings of:

5-10% reduced deployment costs by minimizing code duplication Up to 30% lower runtime costs by optimizing modifier usage

Attachments

  • Code Snippets
File: utils/Implementation.sol

22    modifier onlyDelegateCall() virtual {

29    modifier onlyInitialize() {

https://github.com/code-423n4/2023-10-party/blob/main/contracts/utils/Implementation.sol#L22

File: party/PartyGovernance.sol

231    modifier onlyPartyDao() {

242    modifier onlyPartyDaoOrHost() {

251    modifier onlyWhenEmergencyExecuteAllowed() {

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

File: party/PartyGovernanceNFT.sol

67    modifier onlySelf() {

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

Overall optimization potential of 0.5-3% savings on total executed transactions depending on specific code and usage profile.

Savings would compound significantly over high usage contracts and networks with millions of daily transactions.

Proper application of internal functions versus modifiers based on priority tradeoffs enables targeted optimizations. This improves long term scalability and cost efficiency.

[G-24] Consider using alternatives to OpenZeppelin

OpenZeppelin is a great and popular smart contract library, but there are other alternatives that are worth considering. These alternatives offer better gas efficiency and have been tested and recommended by developers.

Two examples of such alternatives are Solmate and Solady.

Solmate is a library that provides a number of gas-efficient implementations of common smart contract patterns. Solady is another gas-efficient library that places a strong emphasis on using assembly.

#0 - c4-pre-sort

2023-11-13T07:53:25Z

ydspa marked the issue as insufficient quality report

#1 - c4-judge

2023-11-19T18:30:17Z

gzeon-c4 marked the issue as grade-b

Findings Information

Labels

analysis-advanced
grade-b
sufficient quality report
A-03

Awards

60.0107 USDC - $60.01

External Links

Analysis - Party Protocol Contest

Party Protocol

Description overview of The Party Protocol Contest

The Party Protocol is an open protocol for group coordination on the blockchain. It provides on-chain functionality for group formation, coordination, and distribution. Parties are the core units of the protocol, functioning as group Ethereum accounts with members, inventories, and governance capabilities. Members hold membership NFTs with voting power, and proposals define actions for the party to execute. The protocol has two main phases: crowdfund, where users contribute to become party members, and governance, where members create and vote on proposals. Overall, the Party Protocol aims to enable efficient and decentralized group decision-making and resource sharing on the Ethereum network.

The key contracts of Party protocol for this Audit are:

Auditing the key contracts of the Party Protocol is essential to ensure the security of the protocol. Focusing on them first will provide a solid foundation for understanding the protocol's operation and how it manages user assets and stability. Here's why it's important to audit these specific contracts:

  • PartyGovernance.sol: This contract is crucial for the governance logic of parties within the Party Protocol. As governance plays a significant role in decision-making and the functioning of the protocol. I start by reviewing this contract to ensure that governance-related functions are secure.

  • PartyGovernanceNFT.sol: Since this contract is also related to governance and tokens, this contract handles the token logic for parties, which is an important aspect of the Party Protocol.

  • InitialETHCrowdfund.sol: InitialETHCrowdfund contract responsible for the crowdfunding process of creating new ETH parties. Crowdfunding involves the management of funds, contributions, and the creation of new parties.

As the audit of these contracts, I checked for potential security vulnerabilities, such as reentrancy, access control issues, and logic flaws. Additionally, thoroughly test the functions and roles defined in these contracts to make sure they behave as expected.

System Overview

Scope

  • InitialETHCrowdfund.sol: The InitialETHCrowdfund contract allows users to raise initial funds for new parties, offering batch contributions and refunds. The contract integrates with a gatekeeper and supports metadata customization for parties. It manages delegation for voting power during governance, enforces contribution limits, and enables efficient batch operations. It plays a pivotal role in bootstrapping parties within the protocol.

  • ETHCrowdfundBase.sol: This contract foundational template for creating Ethereum-based crowdfunding campaigns. It supports configurable options for campaigns, tracks contributions, and enforces specific lifecycles.

  • PartyGovernance.sol: PartyGovernance provides decentralized governance functionality, allowing the creation, voting, and execution of proposals. It manages members' voting power and delegate delegation. Token distribution is supported, and emergency actions can be taken in critical situations, with the ability to revoke these actions.

  • PartyGovernanceNFT.sol: PartyGovernanceNFT contract implements party governance NFTs that confer voting power according to their intrinsic votingPower value. Only authorized accounts can mint, burn and manage voting power of tokens. Token ownership transfers associated voting rights. It supports ragequit functionality to burn tokens in exchange for a pro-rated share of party funds distributed according to voting power. Metadata and royalty functions are delegated to an upgradable renderer contract for extensibility.

  • ProposalExecutionEngine.sol:This contract is an upgradable proposal execution engine that handles various types of proposals in a decentralized governance . It imports necessary contracts and interfaces, defines a ProposalType enumeration, manages private state with an explicit storage bucket, and provides functions to execute, cancel, and validate proposals.

  • ProposalStorage.sol: The ProposalStorage contract defines a shared storage structure for storing configuration options and governance values used by both "PartyGovernance" and the "ProposalExecutionEngine." It includes governance-related values, such as vote duration, execution delay, and pass threshold, as well as options for enabling new authorities, allowing ETH spending via arbitrary calls, permitting operators, and controlling distribution voting. This contract facilitates communication and data sharing between the governance and proposal execution components of the system.

  • SetGovernanceParameterProposal.sol: Used to execute proposals that update various governance parameters within the system. It includes the ability to set parameters like vote duration, execution delay, and pass threshold expressed in basis points (bps). If the provided parameter values are invalid, the contract will revert with an error. When these parameters are successfully updated, events are emitted to reflect the changes.

  • SetSignatureValidatorProposal.sol: SetSignatureValidatorProposal manages signature validators for ERC1271 signatures. It enables the update of signature validators associated with specific signature hashes via proposals.Users can retrieve the signature validator for a given hash, the contract is designed for governance systems to modify signature validation methods.

  • OffChainSignatureValidator.sol: The contract validates off-chain signatures for parties. It verifies that the signer is a party member with sufficient voting power, and the message hash matches. Parties can set their signing threshold, and an event is emitted when the threshold changes.

  • Implementation.sol: This contract is designed to be used in conjunction with delegate calls to enable upgradable contract logic.

Privileged Roles

Some privileged roles exercise powers over the Controller contract:

  • Party DAO
    modifier onlyPartyDao() {
        {
            address partyDao = _GLOBALS.getAddress(LibGlobals.GLOBAL_DAO_WALLET);
            if (msg.sender != partyDao) {
                revert NotAuthorized();
            }
        }
        _;
    }
  • Party host
    modifier onlyPartyDaoOrHost() {
        address partyDao = _GLOBALS.getAddress(LibGlobals.GLOBAL_DAO_WALLET);
        if (msg.sender != partyDao && !isHost[msg.sender]) {
            revert NotAuthorized();
        }
        _;
    }
  • Owner
    modifier onlyOwner() {
        if (msg.sender != OWNER) revert OnlyOwner();
        _;
    }
  • Multisig
   modifier onlyMultisig() {
        if (msg.sender != multiSig) {
            revert OnlyMultiSigError();
        }
        _;
    }
  • Pending-Multisig
   modifier onlyPendingMultisig() {
        if (msg.sender != pendingMultiSig) {
            revert OnlyPendingMultiSigError();
        }
        _;
    }

Roles

The system defines different roles, including Members, Hosts, and Authorities.

  1. Members have voting power within the party, either through holding a membership NFT or receiving delegated votes.

  2. Hosts are special roles with veto power over proposals and the ability to configure Rage Quit.

  3. Authorities can perform sensitive operations like minting and burning new memberships and are usually trusted contracts or users.

States

The system has different states, including Governance (while the crowdfund is ongoing), Inactive, Active, Voting, Defeated, Passed, Ready, In Progress, Complete, and Cancelled. These states represent the lifecycle of proposals within the Party Protocol.

Approach Taken-in Evaluating The Party Protocol

Accordingly, I analyzed and audited the subject in the following steps;

  1. Core Protocol Contract Overview:

    I focused on thoroughly understanding the codebase and providing recommendations to improve its functionality. The main goal was to take a close look at the important contracts and how they work together in the Party Protocol. I started with the contract that is at the core of the system and from which other contracts interact or depend on. In this case, I start with the following contracts, which play crucial roles in the Party protocol system:

    Main Contracts I Looked At

    PartyGovernanceNFT.sol PartyGovernance.sol ProposalExecutionEngine.sol ProposalStorage.sol InitialETHCrowdfund.sol

    I started my analysis by examining the PartyGovernanceNFT.sol contract. Becuase this is the core NFT contract that implements the party governance tokens. It defines the token standard, voting power attributes, minting/burning, and ragequit functionality.

    Then, I turned our attention to the PartyGovernance.sol. This contract provides the decentralized governance framework including proposal creation/voting, execution handling, and voting power/delegation management. It works closely with PartyGovernanceNFT.

    Then Dive into ProposalExecutionEngine.sol contract, This handles the backend processing and logic for executing different types of governance proposals. It works with PartyGovernance.

    Then audit ProposalStorage.sol this defines the shared storage structure for governance parameters/values used by PartyGovernance and ProposalExecutionEngine.

    InitialETHCrowdfund.sol - This implements the initial fundraising mechanism, integrating key aspects like metadata, contribution limits, and voting power delegation for new parties.

    The key contracts define the core governance (PartyGovernance, ProposalExecutionEngine), tokens (PartyGovernanceNFT), shared storage (ProposalStorage), and initial funding (InitialETHCrowdfund) aspects. The other contracts provide supporting functionality like specific proposal types, off-chain signatures, and upgradability patterns.

  2. Documentation Review:

    Then went to Review This Docs for a more detailed and technical explanation of Party protocol.

  3. Manuel Code Review In this phase, I initially conducted a line-by-line analysis, following that, I engaged in a comparison mode.

    • Line by Line Analysis: Pay close attention to the contract's intended functionality and compare it with its actual behavior on a line-by-line basis.

    • Comparison Mode: Compare the implementation of each function with established standards or existing implementations, focusing on the function names to identify any deviations.

Architecture Description and Diagram

Architecture of the key contracts that are part of the Party protocol: This architecture forms the backbone of the Party Protocol, enabling the creation and governance of parties, crowdfunding campaigns, and various decentralized operations within the protocol.

  1. Crowdfunding Contracts:

Contracts in the "crowdfund" directory facilitate crowdfunding campaigns. They include various types of crowdfunds like AuctionCrowdfund, BuyCrowdfund, and more. The core contract, "InitialETHCrowdfund.sol," allows users to raise initial funds for new parties.

  1. Party Contracts:

"Party.sol" and "PartyFactory.sol" are key contracts that define parties and party creation. "PartyGovernance.sol" manages decentralized governance for parties, including creating, voting, and executing proposals. "PartyGovernanceNFT.sol" implements party governance NFTs that confer voting power.

  1. Proposal Contracts:

The "proposals" directory contains various proposal-related contracts, including "AddAuthorityProposal," "ArbitraryCallsProposal," and others. "ProposalExecutionEngine.sol" handles the execution of different proposal types in decentralized governance. "ProposalStorage.sol" provides shared storage for configuration options and governance values.

  1. Gatekeepers:

Contracts in the "gatekeepers" directory, such as "AllowListGateKeeper" and "TokenGateKeeper," manage access control and gatekeeping functions.

  1. Market Wrappers:

Market wrapper contracts in the "market-wrapper" directory integrate with external markets such as Koans and Nouns.

  1. Operators:

Contracts in the "operators" directory handle various operator functionalities, including batch buys and ERC20 swaps.

  1. Signature Validators:

The "OffChainSignatureValidator.sol" contract is used for validating off-chain signatures associated with parties.

  1. Tokens:

Contracts in the "tokens" directory define ERC721 and ERC1155 receivers and interfaces for tokens.

  1. Metadata Rendering:

Contracts in the "renderers" directory deal with metadata rendering and providers for NFTs.

  1. Utility Contracts:

The "utils" directory contains utility contracts for handling EIP165, safe casting, and other general-purpose functions.

  1. Vendor Contracts:

Vendor contracts integrate with external systems and markets, like Koans, Nouns, and Solmate.

Architecture Feedback

  1. Shared dependencies like storage are centralized rather than duplicated across contracts.

  2. Integrate oracles to provide reliable and tamper-resistant external data for proposals, particularly if proposals require real-world information.

  3. Implement an upgradability mechanism, such as a proxy contract, to allow for the protocol's smart contracts to be upgraded without disrupting existing operations.

  4. Enhance the vote delegation mechanism to enable users to delegate their votes to multiple recipients if needed.

Codebase Quality

Overall, I consider the quality of the Party protocol codebase to be excellent. The code appears to be very mature and well-developed. We have noticed the implementation of various standards adhere to appropriately. Details are explained below:

Codebase Quality CategoriesComments
Code Maintainability and ReliabilityThe Party Protocol contracts demonstrates good maintainability through modular structure, consistent naming, and efficient use of libraries. It also prioritizes reliability by handling errors, conducting security checks, and using "immutable" variables. However, for enhanced reliability, consider professional security audits and documentation improvements. Regular updates are crucial in the evolving space.
Code CommentsThe Party codebase contains comments that briefly explain the purpose of important code elements, such as data structures, constants, and function modifiers. While these comments offer some clarity, further in-code comments are needed for individual functions and complex logic to enhance code maintainability and comprehension. Although there are some comments, adding more detailed explanations for complex functions, data structures, and key decisions can be helpful to future developers and auditors. Consider following standardized commenting conventions, such as NatSpec.
DocumentationThe documentation of the Party project is quite comprehensive and detailed, providing a solid overview of how Party protocol is structured and how its various aspects function. However, we have noticed that there is room for additional details, such as diagrams, to gain a deeper understanding of how different contracts interact and the functions they implement. With considerable enthusiasm. We are confident that these diagrams will bring significant value to the protocol as they can be seamlessly integrated into the existing documentation, enriching it and providing a more comprehensive and detailed understanding for users, developers and auditors.
TestingThe audit scope of the contracts to be audited is 62% and it should be aimed to be 100%.
Code Structure and FormattingThe codebase contracts are well-structured and formatted. It inherits from multiple components, adhering for-example The InitialETHCrowdfund contract follows best practices, facilitates fundraising for new parties. It features contribution functions, a refund mechanism for contributors, and interfaces with other contracts for governance and customization. The code follows good practices and has a clear and structured format.

Systemic & Centralization Risks

The analysis provided highlights several significant systemic and centralization risks present in the Party protocol. These risks encompass concentration risk in PartyGovernanceNFT, PartyGovernance, ETHCrowdfundBase risk and more, third-party dependency risk, and centralization risks arising from the existence of an “owner” role in specific contracts. However, the documentation lacks clarity on whether this address represents an externally owned account (EOA) or a contract, warranting the need for clarification. Additionally, the absence of fuzzing and invariant tests could also pose risks to the protocol’s security.

Here's an analysis of potential systemic and centralization risks in the contracts:

Systemic Risks:

  1. No having, fuzzing and invariant tests could open the door to future vulnerabilities.

  2. If the crowdfund fails in the InitialETHCrowdfund contract, there is no clear fallback and funds/assets could be lost.

  3. Once funds are contributed in ETHCrowdfundBasecontract.sol, contributors have no way to withdraw them or influence how the funds are used by the party and The same party could be refunded repeatedly from multiple crowdfunding contracts, inflating its total voting power.

  4. The increaseVotingPower and decreaseVotingPower functions in PartyGovernanceNFT contract allow certain authorities to manipulate the voting power of NFT holders. Unauthorized or malicious use of these functions could lead to unfair governance outcomes.

  5. The ProposalExecutionEngine executes proposals based on their types and data. If malicious proposals are submitted and approved, they could potentially exploit vulnerabilities in the contract.

  6. The SetGovernanceParameterProposal does not enforce a quorum requirement for changing governance parameters. A quorum is an essential component of many governance systems, ensuring that a minimum number of participants are involved in the decision-making process. The absence of a quorum requirement can result in proposals being passed or rejected with insufficient participation.

  7. The OffChainSignatureValidator contract enforces a specific signature format (EOA signature) and does not support EIP-712 typed signatures. This limitation restricts the flexibility and compatibility of the signature verification process, potentially excluding alternative signature standards that the broader ecosystem might prefer.

Centralization Risks:

  1. The addAuthority function PartyGovernanceNFT contract allows the contract to add new authorities. If this function is misused, it could centralize control further and The contract has a fee mechanism that could be subject to centralization if the fee recipient address or fee rates are not managed fairly.

  2. The isValidSignature function ProposalExecutionEngine contract relies on an external signature validator contract. Depending on how this validator is set up, it could introduce centralization risks if it's controlled by a single entity.

Properly managing these risks and implementing best practices in security and decentralization will contribute to the sustainability and long-term success of the Party protocol.

Conclusion

In general, the Party project exhibits an interesting and well-developed architecture we believe the team has done a good job regarding the code, but the identified risks need to be addressed, and measures should be implemented to protect the protocol from potential malicious use cases. Additionally, it is recommended to improve the documentation and comments in the code to enhance understanding and collaboration among developers. It is also highly recommended that the team continues to invest in security measures such as mitigation reviews, audits, and bug bounty programs to maintain the security and reliability of the project.

Time spent:

15 hours

#0 - c4-pre-sort

2023-11-13T10:41:19Z

ydspa marked the issue as sufficient quality report

#1 - c4-judge

2023-11-20T18:39:58Z

gzeon-c4 marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter