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
Rank: 42/65
Findings: 2
Award: $83.82
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0xVolcano
Also found by: 0x11singh99, 0xAnah, 0xhacksmithh, 0xhex, 0xta, DavidGiladi, K42, Topmark, arjun16, dharma09, hunter_w3b, lsaudit, pavankv, tabriz, tala7985, ybansal2403
23.8054 USDC - $23.81
Possible Optimization 1 =
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); }
SSTORE
operations.Possible Optimization 2 =
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)); }
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 Optimization 1 =
memory
to avoid multiple storage reads.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 ... }
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 ... }
Possible Optimization 1 =
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; }
Possible Optimization 2 =
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; }
Possible Optimization 3 =
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]; }
Possible Optimization 1 =
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; }
Possible Optimization 2 =
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]; }
Possible Optimization 3 =
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 ... }
withdrawTokens
array has multiple entries for the native ETH token, this could save 800 gas for each additional entry after the first.Possible Optimization 1 =
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 ... }
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 =
revert
if it does not match the proposalId. This check can be moved to a modifier
to reduce code duplication across functions that require the same check.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; }
Possible Optimization 3 =
mapping
to cache the results of getSignatureValidatorForHash calls.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 ... }
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
🌟 Selected for report: catellatech
Also found by: 0xSmartContract, 0xbrett8571, Bauchibred, K42, Myd, SAAJ, ZanyBonzy, clara, foxb868, hunter_w3b, kaveyjoe, pavankv
60.0107 USDC - $60.01
_contribute
: Central function for handling contributions, with checks for gatekeeping and delegate validation.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.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.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.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.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.InvalidDelegateError
and NotAllowedByGateKeeperError
to provide clear feedback on failed operations.safeCastUint256ToUint96
for potential edge cases where casting could lead to unexpected behavior.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:
CrowdfundLifecycle enum
to manage the state of the crowdfunding process._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.getCrowdfundLifecycle
: Determines the current state of the crowdfund based on time and contribution thresholds._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.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.sendFundingSplit
: Sends the configured portion of the total contributions to the funding split recipient and marks the split as paid.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.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.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
UINT40_HIGH_BIT
and VETO_VALUE
.ERC1155
and ERC721
tokens received, and delegates other calls to a proposal engine.Security Considerations
Testing Suggestions
PartyGovernance
, ERC721
, and IERC2981
.LibSafeCast
: For safe casting between different integer types.LibERC20Compat
: For compatibility with ERC20
token operations.LibAddress
: For operations related to payable addresses.FixedRageQuitTimestampError
, CannotRageQuitError
).ENABLE_RAGEQUIT_PERMANENTLY
and DISABLE_RAGEQUIT_PERMANENTLY
: Special timestamps for rage quit functionality.ETH_ADDRESS
: Address used to represent ETH
in token operations._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._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._assertAuthority
, addAuthority
, etc.) must be secured to prevent unauthorized access.setRageQuit
, rageQuit
) needs careful management to prevent abuse._delegateToRenderer
) introduces dependency on external code and potential security risks.tokenURI
, contractURI
, and royaltyInfo
could be exploited if the renderer contract is compromised.For ProposalExecutionEngine.sol:
IProposalExecutionEngine
and IERC1271
.Implementation
, ProposalStorage
, and various proposal-related contracts.LibRawResult
: For handling raw bytes results.ProposalType
: Enumerates different types of proposals that can be executed.Storage
: Holds the hash of the next progress data and the ID of the current in-progress proposal.ProposalEngineImplementationUpgraded
: Emitted when the proposal engine implementation is upgraded._STORAGE_SLOT
: The storage slot for the contract's storage struct._GLOBALS
: Reference to global settings.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.For ProposalStorage.sol:
ProposalStorage
serves as an abstract contract that provides storage structures and functions related to proposal management in a governance system.LibRawResult
: For handling raw bytes results.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.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._initProposalImpl
: Initializes the proposal implementation with provided initialization data._getSharedProposalStorage
: Internal pure function to access the contract's shared proposal storage.delegatecall
in _initProposalImpl
can be risky if not handled properly, as it can change the state of the calling contract.engineImpl
) is a trusted and secure contract.ProposalStorage
for proposal management._initProposalImpl
to prevent unauthorized changes to the proposal engine.initData
inputs to ensure it handles all expected cases.For SetGovernanceParameterProposal.sol:
SetGovernanceParameterProposal
contract is designed to execute proposals that modify governance parameters such as vote duration, execution delay, and pass threshold basis points (bps
).ProposalStorage
.InvalidGovernanceParameter
: Thrown when a new governance parameter value is outside of the allowed range.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.SetGovernanceParameterProposalData
: Contains the new values for the governance parameters to be set._executeSetGovernanceParameter
: Executes the proposal to set new governance parameters.abi.decode
requires that the proposalData
is correctly formatted to prevent decoding errors.voteDuration
, executionDelay
, and passThresholdBps
to ensure the contract behaves as expected.For SetSignatureValidatorProposal.sol:
SetSignatureValidatorProposal
contract is designed to manage the association between signature hashes and their corresponding validators, specifically for validating ERC1271
signatures.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.SignatureValidatorSet
: Emitted when a new signature validator is set for a specific hash._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.SetSignatureValidatorProposalStorage
to avoid storage collisions._executeSetSignatureValidator
function to prevent unauthorized changes to signature validators._executeSetSignatureValidator
function, they could potentially redirect signature validation to a malicious validator. Proper access control is essential.SignatureValidatorSet
event is emitted correctly with the appropriate.For OffChainSignatureValidator.sol:
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.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.SigningThresholdBpsSet
: Emitted when the signing threshold in basis points (BPS
) is updated for a party.signingThersholdBps
: A mapping from a Party to its signing threshold in BPS
.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.ecrecover
to obtain the signer's address from the signature, which is a standard method for signature verification but should be used with caution.setSigningThresholdBps
can only be called by authorized parties to prevent unauthorized changes to the signing threshold.setSigningThresholdBps
function with different threshold values and ensure that the SigningThresholdBpsSet event is emitted correctly.For Implementation.sol:
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.Initialized
: Emitted when the contract is initialized.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.implementation
: An immutable address of the contract itself, set at construction.initialized
: A boolean flag indicating whether the contract has been initialized.onlyDelegateCall
: Ensures a function is only called via delegatecall
.onlyInitialize
: Ensures the initialization logic is executed only once.Constructor
: Sets the implementation address to the address of the contract itself.IMPL
: An alias for implementation for backward compatibility.onlyDelegateCall
modifier is crucial for security in upgradeable contracts to prevent direct calls to functions that should only be accessed via the logic contract.onlyInitialize
modifier prevents re-initialization, which is essential to maintain the integrity of the upgradeable contract's state.onlyDelegateCall
modifier cannot be called directly.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