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: 43/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
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
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.
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
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);
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;
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);
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.
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
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
File: proposals/ProposalExecutionEngine.sol 313 require(proposalType != ProposalType.Invalid); 314 require(uint8(proposalType) <= uint8(type(ProposalType).max));
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
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.
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.
uint256 private constant _STORAGE_SLOT = uint256(keccak256("ProposalExecutionEngine.Storage"));
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
File: proposals/ProposalExecutionEngine.sol 104 uint256 private constant _STORAGE_SLOT = uint256(keccak256("ProposalExecutionEngine.Storage"));
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
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);
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
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));
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
File: party/PartyGovernance.sol 198 address payable public feeRecipient;
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
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
File: party/PartyGovernance.sol 286 abi.encode(proposalEngineOpts)
https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol
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
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) {
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
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
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
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
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.
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
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 )
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 )
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
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;
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
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
File: crowdfund/InitialETHCrowdfund.sol 358 (bool s, bytes memory r) = address(this).call( 382 authorities[authoritiesLength - 1] = address(this);
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
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
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;
Mappings provide direct element access without bounds checks, improving performance versus arrays when the use case allows their semantics.
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
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);
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
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
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 }),
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
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
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(
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.
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
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 );
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);
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);
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
File: crowdfund/InitialETHCrowdfund.sol 367 amounts[i] = abi.decode(r, (uint96));
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));
File: proposals/SetGovernanceParameterProposal.sol 28 SetGovernanceParameterProposalData memory proposalData = abi.decode(
File: proposals/SetSignatureValidatorProposal.sol 31 SetSignatureValidatorProposalData memory data = abi.decode(
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.
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:
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
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.
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
🌟 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
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.
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.
Some privileged roles exercise powers over the Controller contract:
modifier onlyPartyDao() { { address partyDao = _GLOBALS.getAddress(LibGlobals.GLOBAL_DAO_WALLET); if (msg.sender != partyDao) { revert NotAuthorized(); } } _; }
modifier onlyPartyDaoOrHost() { address partyDao = _GLOBALS.getAddress(LibGlobals.GLOBAL_DAO_WALLET); if (msg.sender != partyDao && !isHost[msg.sender]) { revert NotAuthorized(); } _; }
modifier onlyOwner() { if (msg.sender != OWNER) revert OnlyOwner(); _; }
modifier onlyMultisig() { if (msg.sender != multiSig) { revert OnlyMultiSigError(); } _; }
modifier onlyPendingMultisig() { if (msg.sender != pendingMultiSig) { revert OnlyPendingMultiSigError(); } _; }
The system defines different roles, including Members, Hosts, and Authorities.
Members have voting power within the party, either through holding a membership NFT or receiving delegated votes.
Hosts are special roles with veto power over proposals and the ability to configure Rage Quit.
Authorities can perform sensitive operations like minting and burning new memberships and are usually trusted contracts or users.
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.
Accordingly, I analyzed and audited the subject in the following steps;
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.
Documentation Review:
Then went to Review This Docs for a more detailed and technical explanation of Party protocol.
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 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.
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.
"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.
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.
Contracts in the "gatekeepers" directory, such as "AllowListGateKeeper" and "TokenGateKeeper," manage access control and gatekeeping functions.
Market wrapper contracts in the "market-wrapper" directory integrate with external markets such as Koans and Nouns.
Contracts in the "operators" directory handle various operator functionalities, including batch buys and ERC20 swaps.
The "OffChainSignatureValidator.sol" contract is used for validating off-chain signatures associated with parties.
Contracts in the "tokens" directory define ERC721 and ERC1155 receivers and interfaces for tokens.
Contracts in the "renderers" directory deal with metadata rendering and providers for NFTs.
The "utils" directory contains utility contracts for handling EIP165, safe casting, and other general-purpose functions.
Vendor contracts integrate with external systems and markets, like Koans, Nouns, and Solmate.
Shared dependencies like storage are centralized rather than duplicated across contracts.
Integrate oracles to provide reliable and tamper-resistant external data for proposals, particularly if proposals require real-world information.
Implement an upgradability mechanism, such as a proxy contract, to allow for the protocol's smart contracts to be upgraded without disrupting existing operations.
Enhance the vote delegation mechanism to enable users to delegate their votes to multiple recipients if needed.
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 Categories | Comments |
---|---|
Code Maintainability and Reliability | The 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 Comments | The 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. |
Documentation | The 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. |
Testing | The audit scope of the contracts to be audited is 62% and it should be aimed to be 100%. |
Code Structure and Formatting | The 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. |
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:
No having, fuzzing and invariant tests could open the door to future vulnerabilities.
If the crowdfund fails in the InitialETHCrowdfund contract, there is no clear fallback and funds/assets could be lost.
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.
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.
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.
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.
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.
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.
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.
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.
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