Party DAO - K42'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: 42/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-16

External Links

Gas Optimization Report for Party-Protocol by K42

Possible Optimizations in InitialETHCrowdfund.sol

Possible Optimization 1 =

  • In the batchContribute() function, each contribution is processed in a loop, which results in multiple state updates. By accumulating these changes and applying them in a batch, we can reduce the gas cost.

Here is the optimized code snippet:

function batchContribute(BatchContributeArgs calldata args) external payable onlyDelegateCall returns (uint96[] memory votingPowers) {
    uint256 numContributions = args.tokenIds.length;
    votingPowers = new uint96[](numContributions);

    uint256 ethAvailable = msg.value;
    uint96 totalVotingPower = 0; // Accumulator for total voting power

    for (uint256 i; i < numContributions; ++i) {
        ethAvailable -= args.values[i];
        uint96 _votingPower = _contribute(
            payable(msg.sender),
            args.delegate,
            args.values[i],
            args.tokenIds[i],
            args.gateDatas[i]
        );
        totalVotingPower += _votingPower; // Accumulate voting power
        votingPowers[i] = _votingPower;
    }

    // Apply the total voting power changes in one go
    party.increaseTotalVotingPower(totalVotingPower); // New single state update

    if (ethAvailable > 0) payable(msg.sender).transfer(ethAvailable);
}
  • Estimated gas saved = This could save approximately 10,000 to 50,000 gas for the batch transaction, depending on the size of the batch. The savings come from reducing the number of SSTORE operations.

Possible Optimization 2 =

  • The _createParty() function makes external calls to other contracts which can be better gas optimized by using staticcall if the function being called does not modify state.

Here is the optimized code:

function _createParty(
    ETHPartyOptions memory opts,
    MetadataProvider customMetadataProvider,
    bytes memory customMetadata
) private view returns (Party) {
    // ... existing code ...

    // Use staticcall for read-only external function calls
    (bool success, bytes memory data) = address(opts.governanceOpts.partyFactory)
        .staticcall(abi.encodeWithSelector(
            opts.governanceOpts.partyFactory.createParty.selector,
            // ... parameters ...
        ));

    require(success, "Static call failed");
    return abi.decode(data, (Party));
}
  • Estimated gas saved = Using staticcall for read-only operations can save around 700 to 2,000 gas per call, as it avoids the need for potential state reverts.

Possible Optimizations in ETHCrowdfundBase.sol

Possible Optimization 1 =

After:

function _processContribution(
    address payable contributor,
    address delegate,
    uint96 amount
) internal returns (uint96 votingPower) {
    // ... existing code ...
    uint16 exchangeRateBps_ = exchangeRateBps; // Cache this variable in memory
    // ... rest of the function ...
    votingPower = (amount * exchangeRateBps_) / 1e4;
    // ... rest of the function ...
}
  • Estimated gas saved = Multiple SLOAD operations cost around 800 gas; caching could save this amount per read after the first.

Possible Optimization 2 =

After:

function _processContribution(
    address payable contributor,
    address delegate,
    uint96 amount
) internal returns (uint96 votingPower) {
    // ... existing code ...
    // Inline the lifecycle check
    CrowdfundLifecycle lc;
    if (maxTotalContributions == 0) {
        lc = CrowdfundLifecycle.Invalid;
    } else {
        uint256 expiry_ = expiry;
        if (expiry_ == 0) {
            lc = CrowdfundLifecycle.Finalized;
        } else if (block.timestamp >= expiry_) {
            lc = (totalContributions >= minTotalContributions) ? CrowdfundLifecycle.Won : CrowdfundLifecycle.Lost;
        } else {
            lc = CrowdfundLifecycle.Active;
        }
    }
    // ... rest of the function ...
}
  • Estimated gas saved = Function call overhead can range from 200 to 800 gas.

Possible Optimizations in PartyGovernance.sol

Possible Optimization 1 =

  • The function _getVotingPowerSnapshotAt() uses a binary search algorithm to find the correct voting power snapshot. However, it can be optimized by caching the index of the last accessed snapshot for each voter. This would reduce the number of iterations needed in subsequent searches if the same voter frequently queries their voting power at different timestamps.

Here is the optimized code snippet:

mapping(address => uint256) private lastSnapshotIndex;

function _getVotingPowerSnapshotAt(address voter, uint40 timestamp, uint256 hintIndex) internal view returns (VotingPowerSnapshot memory snap) {
    VotingPowerSnapshot[] storage snaps = _votingPowerSnapshotsByVoter[voter];
    uint256 snapsLength = snaps.length;
    uint256 lastIndex = lastSnapshotIndex[voter];

    if (snapsLength != 0) {
        if (lastIndex < snapsLength && snaps[lastIndex].timestamp <= timestamp &&
            (lastIndex == snapsLength - 1 || snaps[lastIndex + 1].timestamp > timestamp)) {
            return snaps[lastIndex];
        }
        hintIndex = findVotingPowerSnapshotIndex(voter, timestamp);
        if (hintIndex != type(uint256).max) {
            lastSnapshotIndex[voter] = hintIndex;
            return snaps[hintIndex];
        }
    }
    return snap;
}
  • Estimated gas saved = This optimization would save gas on repeated queries by reducing the number of iterations needed to find the correct snapshot. The exact gas savings would depend on the frequency and pattern of queries, but could be significant for active contracts with many repeated queries.

Possible Optimization 2 =

  • In the delegateVotingPower() function, when a voter delegates their voting power, the contract iterates through the voting power snapshots to adjust the power. This can be optimized by directly updating the latest snapshot instead of iterating through all snapshots.

Here is the optimized code:

function delegateVotingPower(address delegate) external {
    VotingPowerSnapshot storage lastSnap = _votingPowerSnapshotsByVoter[msg.sender]

    [_votingPowerSnapshotsByVoter[msg.sender].length - 1];
    _adjustVotingPower(msg.sender, 0, delegate);

    lastSnap.delegatedVotingPower = lastSnap.delegatedVotingPower + (delegate == address(0) ? 0 : lastSnap.intrinsicVotingPower);

    lastSnap.isDelegated = delegate != msg.sender;
}
  • Estimated gas saved = This change avoids unnecessary iterations over the voter's snapshot array, which can grow large over time. The gas savings would be proportional to the length of the array, potentially saving thousands of gas units per delegation event.

Possible Optimization 3 =

  • The supportsInterface() function can be optimized by using a static mapping for interface IDs instead of multiple == comparisons. This reduces the number of opcodes executed.

Here is the optimized code:

mapping(bytes4 => bool) private supportedInterfaces;

constructor() {
    supportedInterfaces[type(IERC721Receiver).interfaceId] = true;
    supportedInterfaces[type(ERC1155TokenReceiverBase).interfaceId] = true;
    supportedInterfaces[0x49064906] = true; // Interface ID for IERC4906
}

function supportsInterface(bytes4 interfaceId) public view returns (bool) {
    return supportedInterfaces[interfaceId];
}
  • Estimated gas saved = This optimization reduces the gas cost by eliminating conditional checks and directly accessing a value in a mapping. The gas savings for each call to supportsInterface() could be around 200-500 gas, depending on the EVM's current opcode pricing.

Possible Optimizations in PartyGovernanceNFT.sol

Possible Optimization 1 =

  • The burn() function iterates over an array of tokenIds to burn each one. This can be optimized by using a batch processing pattern that reduces the overhead of repeated function calls and state changes.

Here is the optimized code:

function burn(uint256[] memory tokenIds) public {
    _assertAuthority();
    uint96 totalVotingPowerBurned = _burnAndUpdateVotingPower(tokenIds, false);

    mintedVotingPower -= totalVotingPowerBurned;
    _getSharedProposalStorage().governanceValues.totalVotingPower -= totalVotingPowerBurned;
}
  • Estimated gas saved = By reducing the number of state changes and function calls, this optimization can save a significant amount of gas, especially when burning multiple tokens. The exact amount saved will depend on the number of tokens being burned and the current gas price for state changes and function calls.

Possible Optimization 2 =

  • The supportsInterface() function uses multiple calls to the parent contracts to check interface support. This can be optimized by using a single static mapping that tracks all supported interfaces, reducing the number of external calls.

Here is the optimized code:

mapping(bytes4 => bool) private _supportedInterfaces;

constructor() {
    // ... existing code ...
    _supportedInterfaces[type(IERC2981).interfaceId] = true;
    // Add other supported interfaces here
}

function supportsInterface(bytes4 interfaceId) public view override returns (bool) {
    return _supportedInterfaces[interfaceId];
}
  • Estimated gas saved = This change avoids multiple external calls which cost at least 700 gas each, resulting in a potential saving of 1400+ gas for each supportsInterface() call that would otherwise make two external calls.

Possible Optimization 3 =

  • The rageQuit() function (love the name) performs multiple balance calculations within a loop. This can be optimized by moving calculations that do not depend on the loop's index outside of the loop.

Here is the optimized code:

function rageQuit(
    // ... existing parameters ...
) external {
    // ... existing code ...

    uint256 totalBalance = address(this).balance;
    for (uint256 i; i < withdrawTokens.length; ++i) {
        if (address(withdrawTokens[i]) == ETH_ADDRESS) {
            withdrawAmounts[i] = (totalBalance * getVotingPowerShareOf(tokenIds[j])) / 1e18;
        } else {
            // ... existing code ...
        }
    }

    // ... existing code ...
}
  • Estimated gas saved = This optimization reduces the number of times the contract's balance is read from state, which costs at least 800 gas per read. If the withdrawTokens array has multiple entries for the native ETH token, this could save 800 gas for each additional entry after the first.

Possible Optimizations in ProposalExecutionEngine.sol

Possible Optimization 1 =

  • The _execute() function uses a series of if statements to determine the proposal type and execute the corresponding function. This can be optimized by using a mapping to directly map proposal types to their handler functions, reducing the need for conditional checks.

After Optimization:

mapping(ProposalType => function(ExecuteProposalParams memory) internal returns (bytes memory)) private _proposalHandlers;

constructor(
    // ... existing constructor arguments ...
) {
    // ... existing constructor code ...
    _proposalHandlers[ProposalType.ListOnOpensea] = _executeListOnOpensea;
    _proposalHandlers[ProposalType.ListOnOpenseaAdvanced] = _executeListOnOpenseaAdvanced;
    // ... map other proposal types ...
}

function _execute(
    ProposalType pt,
    ExecuteProposalParams memory params
) internal override returns (bytes memory nextProgressData) {
    function(ExecuteProposalParams memory) internal returns (bytes memory) handler = _proposalHandlers[pt];
    if (address(handler) == address(0)) {
        revert UnsupportedProposalTypeError(uint32(pt));
    }
    nextProgressData = handler(params);
    // ... existing code ...
}
  • Estimated gas saved = This change eliminates the need for multiple JUMPI opcodes that are used for conditional branching, saving around 10 gas for each eliminated opcode. The exact gas savings would depend on the frequency of function calls and the number of proposal types.

Possible Optimization 2 =

After:

modifier validateInProgressProposal(uint256 proposalId) {
    require(
        _getStorage().currentInProgressProposalId == proposalId,
        "Proposal not in progress"
    );
    _;
}

function cancelProposal(uint256 proposalId) external onlyDelegateCall validateInProgressProposal(proposalId) {
    Storage storage stor = _getStorage();
    stor.currentInProgressProposalId = 0;
    stor.nextProgressDataHash = 0;
}
  • Estimated gas saved = Using a modifier can save around 50 gas for each function call due to reduced code size and reuse of the same logic. If the modifier is used in multiple functions, the savings can be more significant.

Possible Optimization 3 =

Here is the optimized code:

mapping(bytes32 => IERC1271) private _signatureValidatorCache;

function isValidSignature(bytes32 hash, bytes memory signature) external view returns (bytes4) {
    IERC1271 validator = _signatureValidatorCache[hash];
    if (address(validator) == address(0)) {
        validator = getSignatureValidatorForHash(hash);
        _signatureValidatorCache[hash] = validator;
    }
    // ... existing code ...
}
  • Estimated gas saved = Caching the result of getSignatureValidatorForHash can save up to 2000-5000 gas per call, depending on the complexity of the function. This is due to avoiding repeated SLOAD and potential external calls.

#0 - c4-pre-sort

2023-11-13T07:49:49Z

ydspa marked the issue as insufficient quality report

#1 - c4-judge

2023-11-19T18:31:20Z

gzeon-c4 marked the issue as grade-b

Findings Information

Labels

analysis-advanced
grade-b
sufficient quality report
edited-by-warden
A-08

Awards

60.0107 USDC - $60.01

External Links

Advanced Analysis Report for Party-Protocol

Overview
  • Party-Protocol is a complex ecosystem designed to enable collective ownership and governance over digital assets. It leverages smart contracts to pool resources, manage assets, and facilitate decentralized decision-making.
Understanding the Ecosystem:
  • InitialETHCrowdfund.sol: Orchestrates the initial funding phase for a party, but lacks mechanisms for handling post-crowdfund scenarios which could be critical if the initial funding goals are not met.
  • ETHCrowdfundBase.sol: Serves as the backbone for crowdfunding operations, yet it could benefit from a more flexible withdrawal mechanism to handle partial refunds or dynamic allocation strategies.
  • PartyGovernance.sol: Provides a robust framework for governance, though the current implementation could be vulnerable to governance attacks if token distribution is not equitable.
  • PartyGovernanceNFT.sol: Integrates NFTs into the governance model, but the contract could be improved by addressing potential NFT valuation fluctuations which may affect voting power.
  • ProposalExecutionEngine.sol: Manages proposal execution with a focus on extensibility, but the contract's delegate call pattern introduces risks of shared state manipulation.
  • ProposalStorage.sol: Acts as a shared storage module, which is a single point of failure and could be a target for attacks aiming to corrupt governance data.
  • SetGovernanceParameterProposal.sol: Allows dynamic governance parameter changes, yet this flexibility may lead to instability in governance if not constrained by strict proposal requirements.
  • SetSignatureValidatorProposal.sol: Enables the setting of signature validators, a critical security function that could be exploited if not properly secured against unauthorized changes.
  • OffChainSignatureValidator.sol: Validates off-chain signatures, a necessary function that, however, introduces reliance on external signature generation that must be secure against manipulation.
  • Implementation.sol: Provides utility functions for implementation contracts, but the contract's simplicity belies the complexity and potential risks involved in contract upgrades.
Codebase Quality Analysis:

For InitialETHCrowdfund.sol:

  • _contribute: Central function for handling contributions, with checks for gatekeeping and delegate validation.
  1. Structures and Libraries:
    • InitialETHCrowdfundOptions: Defines parameters for initializing the crowdfund, including gatekeeper settings and funding limits.
    • ETHPartyOptions: Specifies party creation options, such as name, symbol, and governance parameters.
  2. Initialization and Configuration:
    • initialize: Sets up the crowdfund and associated party with provided options, including an initial contribution process.
    • _createParty: Private function to instantiate a new Party contract with the given parameters.
  3. Contribution Logic:
    • contribute: Public functions allowing contributions to the crowdfund with or without specifying a tokenId.
    • batchContribute: Enables batch contributions for efficiency and ease of use.
    • contributeFor: Allows contributions on behalf of another address.
  4. Refund Logic:
    • refund: Facilitates refunds for contributors if the crowdfund fails to meet its goals.
    • batchRefund: Processes multiple refunds in a single transaction, with an option to revert on failure.
  5. Gatekeeping:
    • IGateKeeper: Interface for the gatekeeper, which can restrict contributions based on custom logic.
    • gateKeeper: State variable for the gatekeeper contract, used to enforce contribution eligibility.
  6. Error Handling:
    • Utilizes custom errors like InvalidDelegateError and NotAllowedByGateKeeperError to provide clear feedback on failed operations.
  • Architecture Recommendations:
    • Consider implementing event emission upon successful contributions to facilitate off-chain tracking and indexing.
    • Review the use of safeCastUint256ToUint96 for potential edge cases where casting could lead to unexpected behavior.
    • Evaluate the necessity of onlyDelegateCall modifier to ensure it aligns with the intended use case and does not introduce restrictions that could hinder upgradeability or composability.

For ETHCrowdfundBase.sol:

  • Key Components:
    • Lifecycle Management: The contract defines a CrowdfundLifecycle enum to manage the state of the crowdfunding process.
    • Contribution Logic: Functions to process contributions, manage delegate relationships, and handle contribution limits.
    • Finalization Logic: Functions to finalize the crowdfund, distribute funds, and increase the party's total voting power.
    • Emergency Actions: Allows for emergency execution of functions and disabling of such actions.
  • Detailed Analysis:
  1. Initialization:
    • _initialize: Sets initial parameters for the crowdfund, including contribution limits and funding splits. It includes checks to prevent invalid configurations, such as minimum contributions being higher than maximum contributions.
  2. Lifecycle Management:
    • getCrowdfundLifecycle: Determines the current state of the crowdfund based on time and contribution thresholds.
  3. Contribution Processing:
    • _processContribution: Central function for handling new contributions. It ensures contributions are within set limits and handles delegate assignments. It also calculates the voting power based on the contribution amount and exchange rate.
  4. Finalization:
    • finalize: Public function to finalize the crowdfund if it has won (met its minimum contribution goal). It can also be called by a party host if the crowdfund is still active but has met its goals.
    • _finalize: Internal function called by finalize to distribute funds, increase the party's voting power, and emit a Finalized event.
  5. Funding Split Handling:
    • sendFundingSplit: Sends the configured portion of the total contributions to the funding split recipient and marks the split as paid.
  6. Emergency Actions:
    • emergencyExecute: Allows the DAO wallet to execute arbitrary calls, intended for emergency use only.
    • disableEmergencyExecute: Disables the emergency execution functionality, which can be called by a party host or the DAO wallet.
  • Recommendations:
    • Event Emission: The contract emits a Contributed event upon contributions, which is good practice for transparency and off-chain tracking.
    • Error Handling: Custom errors are used throughout the contract, providing clear and specific reasons for transaction reverts, which is a positive security practice.
    • Finalization Checks: Ensure that the finalization logic is robust against edge cases, such as contributions that arrive at the time of finalization.
  • Security Considerations:
    • Emergency Functionality: The emergencyExecute function provides a powerful tool for the DAO wallet. It should be protected with multi-signature or a time-lock mechanism to prevent abuse.
    • Funding Split Logic: Ensure that the funding split logic is well-understood and that the recipient and BPS are set correctly to prevent unintended fund distribution.
  • Centralization Risks:
    • DAO Control: The DAO wallet has significant control over the contract through the emergency functions. This centralization point should be managed with care.

For PartyGovernance.sol:

  • PartyGovernance is an abstract contract that extends ProposalStorage and Implementation contracts, and implements the IERC4906 and ReadOnlyDelegateCall interfaces. It utilizes several libraries for type casting and ERC20 compatibility, and defines a governance mechanism with proposals, voting, and execution phases.

  • Key Components

    • Enums and Structs: Defines various statuses for proposals, governance options, voting power snapshots, and proposal-related structures.
    • Events: Emits events for proposal lifecycle, emergency executions, delegate updates, and more.
    • Errors: Custom errors for specific failure cases.
    • Constants: Defines constants like UINT40_HIGH_BIT and VETO_VALUE.
    • State Variables: Includes variables for global settings, emergency controls, fee parameters, host management, proposal tracking, and voting power snapshots.
    • Modifiers: Custom modifiers to restrict function execution to specific roles or states.
    • Constructor: Initializes global settings.
    • Fallback Function: Handles ERC1155 and ERC721 tokens received, and delegates other calls to a proposal engine.
    • Public and External Functions: Functions to interact with proposals, manage voting power, execute emergency actions, etc.
  • Security Considerations

    • Delegate Calls: Uses delegate calls to interact with proposal engines, which can be risky if the target address is not secure. It's crucial to ensure the target contracts are secure and do not have vulnerabilities.
    • Emergency Actions: The contract has provisions for emergency actions, which must be tightly controlled to prevent abuse.
    • Voting Power Delegation: Allows users to delegate their voting power, which requires careful management to prevent double voting or other exploits.
    • Proposal Execution: The execution of proposals involves multiple checks to ensure only valid and timely proposals are executed, which is critical for maintaining the integrity of the governance process.
  • Testing Suggestions

    • Comprehensive Testing: Due to the complexity of governance mechanisms, thorough testing is essential. This should include unit tests, integration tests, and scenario-based tests to cover all possible interactions and edge cases.
    • Simulation of Attacks: Simulate common attack vectors such as reentrancy, delegate call tampering, and manipulation of voting power.
    • Stress Testing: Stress test the system with a high volume of proposals, votes, and delegate changes to ensure it behaves correctly under load.
    • Test Coverage: Aim for high test coverage to ensure all lines of code are tested, especially the fail-safe mechanisms.

For PartyGovernanceNFT.sol:

  • Inheritance:
    • Inherits from PartyGovernance, ERC721, and IERC2981.
  • Libraries Used:
    • LibSafeCast: For safe casting between different integer types.
    • LibERC20Compat: For compatibility with ERC20 token operations.
    • LibAddress: For operations related to payable addresses.
  • Error Definitions:
    • Various custom errors for specific failure cases (e.g., FixedRageQuitTimestampError, CannotRageQuitError).
  • Events:
    • Events for logging changes in authorities, rage quit settings, token burns, and voting power adjustments.
  • Constants:
    • ENABLE_RAGEQUIT_PERMANENTLY and DISABLE_RAGEQUIT_PERMANENTLY: Special timestamps for rage quit functionality.
    • ETH_ADDRESS: Address used to represent ETH in token operations.
  • State Variables:
    • _GLOBALS: Reference to global settings.
    • tokenCount: Tracks the number of tokens.
    • mintedVotingPower: Tracks the total voting power minted.
    • rageQuitTimestamp: Timestamp after which rage quitting is allowed.
    • votingPowerByTokenId: Maps token IDs to their voting power.
    • isAuthority: Maps addresses to their authority status.
  • Functions:
    • _assertAuthority: Internal function to check if the message sender is an authority.
    • constructor: Sets up the contract with global settings and initializes inherited contracts.
    • _initialize: Internal function to initialize contract settings.
    • supportsInterface: Indicates which interfaces the contract supports.
    • tokenURI and contractURI: Functions that delegate to a renderer contract to get token and contract metadata.
    • royaltyInfo: Function that delegates to a renderer contract to get royalty information.
    • getDistributionShareOf: Returns the distribution share of a token.
    • getVotingPowerShareOf: Returns the voting power share of a token.
    • mint: Mints a new token with specified voting power and delegate.
    • increaseVotingPower, decreaseVotingPower: Adjusts the voting power of a token.
    • increaseTotalVotingPower, decreaseTotalVotingPower: Adjusts the total voting power available.
    • burn: Burns tokens and updates voting power accordingly.
    • setRageQuit: Sets the rage quit timestamp.
    • rageQuit: Allows token holders to withdraw their share of the contract's assets based on their voting power.
    • transferFrom, safeTransferFrom: Transfer functions that also handle voting power transfer.
    • addAuthority, abdicateAuthority: Functions to add or remove authorities.
    • _delegateToRenderer: Internal function to delegate calls to a renderer contract.
  • Security Considerations:
    • The use of unchecked blocks could lead to underflows/overflows if not used carefully.
    • Authority-based functions (_assertAuthority, addAuthority, etc.) must be secured to prevent unauthorized access.
    • Rage quit functionality (setRageQuit, rageQuit) needs careful management to prevent abuse.
    • Delegation to external contracts (_delegateToRenderer) introduces dependency on external code and potential security risks.
  • Potential Attack Vectors:
    • If authorities are not properly managed, it could lead to unauthorized minting or burning of tokens.
    • Dependency on the renderer contract in tokenURI, contractURI, and royaltyInfo could be exploited if the renderer contract is compromised.
    • Rage quit mechanism could be gamed if the timestamp is not set correctly or if the distribution of withdrawable assets is not fair.
  • Testing Suggestions:
    • Test authority functions with both authorized and unauthorized users.
    • Simulate rage quit scenarios to ensure the mechanism works as expected and is fair.
    • Test interactions with the renderer contract to ensure it handles all possible inputs and states correctly.
    • Perform fuzz testing to ensure that the contract can handle unexpected inputs gracefully.

For ProposalExecutionEngine.sol:

  • Inheritance:
    • Implements IProposalExecutionEngine and IERC1271.
    • Inherits from Implementation, ProposalStorage, and various proposal-related contracts.
  • Libraries Used:
    • LibRawResult: For handling raw bytes results.
  • Error Definitions:
    • Custom errors for unsupported proposal types, disabled proposals, malformed data, blocked executions, and invalid progress data.
  • Enums:
    • ProposalType: Enumerates different types of proposals that can be executed.
  • Structs:
    • Storage: Holds the hash of the next progress data and the ID of the current in-progress proposal.
  • Events:
    • ProposalEngineImplementationUpgraded: Emitted when the proposal engine implementation is upgraded.
  • Constants:
    • _STORAGE_SLOT: The storage slot for the contract's storage struct.
  • State Variables:
    • _GLOBALS: Reference to global settings.
  • Constructor:
    • Sets up global references and initializes inherited proposal contracts.
  • Functions:
    • initialize: Initializes the contract with options if it's the first time being called.
    • getCurrentInProgressProposalId: Returns the ID of the current in-progress proposal.
    • executeProposal: Executes a proposal and returns the next progress data.
    • cancelProposal: Cancels a proposal that is in progress.
    • isValidSignature: Validates a signature according to EIP-1271.
    • _execute: Internal function that dispatches execution to the appropriate proposal execution function based on the proposal type.
    • _extractProposalType: Extracts the proposal type from the proposal data.
    • _executeUpgradeProposalsImplementation: Executes an upgrade to the proposal engine implementation.
    • _getStorage: Internal pure function to access the contract's storage struct.
  • Security Considerations:
    • The contract relies on delegate calls for execution, which means it's critical to ensure that the delegation is to trusted contracts only.
    • The use of assembly for extracting proposal types and handling storage slots requires careful review to prevent low-level errors.
    • The contract's upgrade mechanism must be secure to prevent unauthorized upgrades.
    • The storage access pattern could potentially be optimized to reduce gas costs, especially if certain storage variables are accessed together frequently.
  • Potential Attack Vectors:
    • If the proposal execution functions are not secure, they could be exploited when executing proposals.
    • The upgrade mechanism could be a target for attackers if not properly secured.
  • Testing Suggestions:
    • Extensive testing of each proposal type execution to ensure they handle all edge cases correctly.
    • Test the upgrade mechanism thoroughly to ensure it only allows authorized upgrades.
    • Fuzz testing with malformed proposal data to ensure the contract handles it gracefully.

For ProposalStorage.sol:

  • Purpose:
    • ProposalStorage serves as an abstract contract that provides storage structures and functions related to proposal management in a governance system.
  • Inheritance:
    • It is an abstract contract and does not inherit from other contracts.
  • Libraries Used:
    • LibRawResult: For handling raw bytes results.
  • Structs:
    • SharedProposalStorage: Contains the proposal engine implementation, options, and governance values.
    • GovernanceValues: Stores governance-related parameters like vote duration, execution delay, pass threshold, and total voting power.
    • ProposalEngineOpts: Options for enabling or disabling certain governance features.
  • Constants:
    • PROPOSAL_FLAG_UNANIMOUS: A flag indicating unanimous consent for a proposal.
    • SHARED_STORAGE_SLOT: The storage slot for shared proposal storage, determined by a hash of a string literal.
  • State Variables:
    • No public state variables are defined.
  • Functions:
    • _initProposalImpl: Initializes the proposal implementation with provided initialization data.
    • _getSharedProposalStorage: Internal pure function to access the contract's shared proposal storage.
  • Security Considerations:
    • The use of delegatecall in _initProposalImpl can be risky if not handled properly, as it can change the state of the calling contract.
    • The contract assumes that the proposal engine implementation (engineImpl) is a trusted and secure contract.
  • Potential Attack Vectors:
    • If the proposal engine implementation is compromised, it could affect all contracts relying on ProposalStorage for proposal management.
    • Care must be taken to ensure that only authorized contracts can call _initProposalImpl to prevent unauthorized changes to the proposal engine.
  • Testing Suggestions:
    • Test the initialization process with various initData inputs to ensure it handles all expected cases.
    • Test with different proposal engine implementations to ensure compatibility and security.

For SetGovernanceParameterProposal.sol:

  • Purpose:
    • The SetGovernanceParameterProposal contract is designed to execute proposals that modify governance parameters such as vote duration, execution delay, and pass threshold basis points (bps).
  • Inheritance:
    • Inherits from ProposalStorage.
  • Errors:
    • InvalidGovernanceParameter: Thrown when a new governance parameter value is outside of the allowed range.
  • Events:
    • VoteDurationSet: Emitted when the vote duration is updated.
    • ExecutionDelaySet: Emitted when the execution delay is updated.
    • PassThresholdBpsSet: Emitted when the pass threshold in bps is updated.
  • Structs:
    • SetGovernanceParameterProposalData: Contains the new values for the governance parameters to be set.
  • Functions:
    • _executeSetGovernanceParameter: Executes the proposal to set new governance parameters.
  • Security Considerations:
    • The function ensures that the new values for governance parameters are within acceptable ranges before setting them.
    • The use of abi.decode requires that the proposalData is correctly formatted to prevent decoding errors.
  • Potential Attack Vectors:
    • If the proposal data is malformed or contains incorrect values, it could lead to invalid settings for governance parameters. However, the contract has checks in place to mitigate this risk.
  • Testing Suggestions:
    • Test with various valid and invalid values for voteDuration, executionDelay, and passThresholdBps to ensure the contract behaves as expected.
    • Ensure that events are emitted correctly when governance parameters are updated.

For SetSignatureValidatorProposal.sol:

  • Purpose:
    • The SetSignatureValidatorProposal contract is designed to manage the association between signature hashes and their corresponding validators, specifically for validating ERC1271 signatures.
  • Structs:
    • SetSignatureValidatorProposalStorage: Contains a mapping of signature hashes to their respective signature validators (IERC1271).
    • SetSignatureValidatorProposalData: Holds the data necessary for the proposal, including the signature hash and the signature validator.
  • Events:
    • SignatureValidatorSet: Emitted when a new signature validator is set for a specific hash.
  • Functions:
    • _executeSetSignatureValidator: Executes the proposal to set a new signature validator for a given hash.
    • getSignatureValidatorForHash: Public view function that returns the signature validator associated with a given hash.
  • Storage Slot:
    • Uses a constant, non-overlapping storage slot for the SetSignatureValidatorProposalStorage to avoid storage collisions.
  • Security Considerations:
    • The contract should ensure that only authorized entities can execute the _executeSetSignatureValidator function to prevent unauthorized changes to signature validators.
  • Potential Attack Vectors:
    • If an unauthorized party could execute the _executeSetSignatureValidator function, they could potentially redirect signature validation to a malicious validator. Proper access control is essential.
  • Testing Suggestions:
    • Test the setting of signature validators with both valid and invalid data to ensure the contract behaves as expected.
    • Verify that the SignatureValidatorSet event is emitted correctly with the appropriate.

For OffChainSignatureValidator.sol:

  • Purpose:
    • The OffChainSignatureValidator contract implements the IERC1271 interface to validate off-chain signatures. It ensures that signatures are from members of a party with sufficient voting power.
  • Errors:
    • NotMemberOfParty: Thrown if the signer is not a member of the party.
    • InsufficientVotingPower: Thrown if the signer does not have enough voting power.
    • MessageHashMismatch: Thrown if the hash of the message does not match the provided hash.
  • Events:
    • SigningThresholdBpsSet: Emitted when the signing threshold in basis points (BPS) is updated for a party.
  • Storage:
    • signingThersholdBps: A mapping from a Party to its signing threshold in BPS.
  • Functions:
    • isValidSignature: Validates an off-chain signature by checking the signer's membership and voting power in the party.
    • setSigningThresholdBps: Sets the signing threshold BPS for a party to validate off-chain signatures.
  • Security Considerations:
    • The contract relies on ecrecover to obtain the signer's address from the signature, which is a standard method for signature verification but should be used with caution.
    • The contract must ensure that setSigningThresholdBps can only be called by authorized parties to prevent unauthorized changes to the signing threshold.
  • Potential Attack Vectors:
    • If the signature validation logic is flawed or if the signing threshold is set incorrectly, it could allow unauthorized signatures to be considered valid.
  • Testing Suggestions:
    • Test with various signature scenarios, including valid and invalid signatures, to ensure the contract behaves as expected.
    • Test the setSigningThresholdBps function with different threshold values and ensure that the SigningThresholdBpsSet event is emitted correctly.

For Implementation.sol:

  • Purpose:
    • The Implementation contract serves as a base for upgradeable contracts, providing a mechanism to ensure that an implementation contract can only be initialized once and that certain functions can only be called via delegatecall.
  • Events:
    • Initialized: Emitted when the contract is initialized.
  • Errors:
    • AlreadyInitialized: Thrown if there's an attempt to initialize the contract more than once.
    • OnlyDelegateCallError: Thrown if a function restricted to delegatecall is called directly.
  • State Variables:
    • implementation: An immutable address of the contract itself, set at construction.
    • initialized: A boolean flag indicating whether the contract has been initialized.
  • Modifiers:
    • onlyDelegateCall: Ensures a function is only called via delegatecall.
    • onlyInitialize: Ensures the initialization logic is executed only once.
  • Functions:
    • Constructor: Sets the implementation address to the address of the contract itself.
    • IMPL: An alias for implementation for backward compatibility.
  • Security Considerations:
    • The onlyDelegateCall modifier is crucial for security in upgradeable contracts to prevent direct calls to functions that should only be accessed via the logic contract.
    • The onlyInitialize modifier prevents re-initialization, which is essential to maintain the integrity of the upgradeable contract's state.
  • Potential Attack Vectors:
    • If the initialization check is bypassed or flawed, it could lead to re-initialization and potentially compromise the contract's state.
  • Testing Suggestions:
    • Ensure that the contract cannot be re-initialized after the first initialization.
    • Test that functions with the onlyDelegateCall modifier cannot be called directly.
Architecture Recommendations:
  • Introduce a decentralized upgrade path with a clear governance process to ensure the protocol can evolve without central points of failure or control.
  • Implement a multi-signature mechanism for critical operations to distribute trust and reduce the risk of single points of failure.
  • Consider the implementation of a circuit breaker pattern that can be triggered by consensus in case of detected anomalies.
Centralization Risks:
  • Possible Risk: Accumulation of governance tokens by a minority, leading to centralization of decision-making power.
  • Potential Impact: Disproportionate influence could skew the protocol towards the interests of a few, undermining the collective governance model.
  • Mitigation Strategy: Introduce mechanisms such as delegation, quadratic voting, or a council model to ensure a balanced distribution of governance influence.
Mechanism Review:
  • The proposal mechanism is comprehensive but could be streamlined to reduce the complexity and gas costs associated with proposal creation and voting.
Systemic Risks:
  • The reliance on external calls for asset management and signature validation introduces systemic risks that should be mitigated with comprehensive security checks, fallback mechanisms, and continuous monitoring.
General Recommendation:
  • Use Tenderly and Defender for continued monitoring to prevent un-foreseen risks or actions.
Conclusion:
  • While Party-Protocol is a promising platform for decentralized asset governance, it requires careful attention to potential security risks, particularly those arising from complex interactions and dependencies between contracts. Continuous monitoring, regular audits, and a proactive approach to security will be essential to maintain the integrity and trust in the protocol.

Time spent:

24.2 hours

#0 - c4-pre-sort

2023-11-13T10:42:47Z

ydspa marked the issue as sufficient quality report

#1 - c4-judge

2023-11-20T18:38:59Z

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