Platform: Code4rena
Start Date: 12/09/2022
Pot Size: $75,000 USDC
Total HM: 19
Participants: 110
Period: 7 days
Judge: HardlyDifficult
Total Solo HM: 9
Id: 160
League: ETH
Rank: 23/110
Findings: 2
Award: $199.60
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Lambda
Also found by: 0x1f8b, 0x4non, 0x52, 0x5rings, 0xDanielC, 0xNazgul, 0xSmartContract, 0xbepresent, Anth3m, Aymen0909, B2, CRYP70, CertoraInc, Ch_301, Chom, ChristianKuri, CodingNameKiki, Deivitto, Funen, JC, JansenC, Jeiwan, KIntern_NA, MasterCookie, MiloTruck, Olivierdem, PaludoX0, R2, RaymondFam, ReyAdmirado, StevenL, The_GUILD, Tomo, Trust, V_B, __141345__, asutorufos, ayeslick, bin2chen, brgltd, bulej93, c3phas, cccz, ch0bu, cryptphi, csanuragjain, d3e4, delfin454000, djxploit, erictee, fatherOfBlocks, gogo, hansfriese, indijanc, ladboy233, leosathya, lukris02, malinariy, martin, pedr02b2, pfapostol, rvierdiiev, slowmoses, smiling_heretic, tnevler, wagmi
94.5684 USDC - $94.57
File: /contracts/globals/Globals.sol 24: multiSig = multiSig_; 28: multiSig = newMultiSig;
The call() functions return a boolean indicating if the call succeeded or failed. Thus these functions have a simple caveat, in that the transaction that executes these functions will not revert if the external call (initialised by call() or send()) fails, rather the call() or send() will simply return false.
File: /contracts/party/PartyGovernance.sol 783: function emergencyExecute( 784: address targetAddress, 785: bytes calldata targetCallData, 786: uint256 amountEth 787: ) 788: external 789: payable 790: onlyPartyDao 791: onlyWhenEmergencyExecuteAllowed 792: onlyDelegateCall 793: returns (bool success) 794: { 795: (success, ) = targetAddress.call{value: amountEth}(targetCallData); 796: }
In the above, a return value is defined but the status are not checked.
Always check the return value of low level calls
The cases where ERC-721 tokens are transferred are always using transferFrom and not safeTransferFrom. This means users need to be aware their tokens may get locked up in an unprepared contract recipient.
Should a ERC-721 compatible token be transferred to an unprepared contract, it would end up being locked up there.
From openzeppelin. >> Note that the caller is responsible to confirm that the recipient is capable of receiving ERC721 or else they may be permanently lost. Usage of safeTransferFrom prevents loss, though the caller must understand this adds an external call which potentially creates a reentrancy vulnerability.
File: /contracts/crowdfund/Crowdfund.sol 301: preciousTokens[i].transferFrom(address(this), address(party_), preciousTokenIds[i]);
Recommend consider changing transferFrom to safeTransferFrom .
There are several occurrences of literal values with unexplained meaning .Literal values in the codebase without an explained meaning make the code harder to read, understand and maintain, thus hindering the experience of developers, auditors and external contributors alike.
Developers should define a constant variable for every magic value used , giving it a clear and self-explanatory name.
File: /contracts/party/PartyGovernanceNFT.sol //@audit: 1e18 112: return votingPowerByTokenId[tokenId] * 1e18 / _getTotalVotingPower();
File: /contracts/proposals/ArbitraryCallsProposal.sol //@audit: 68 208: if (callData.length < 68) { 226: if (callData.length < 68) {
File: /contracts/distribution/TokenDistributor.sol //@audit: 1e18 261: + (1e18 - 1) //@audit: 1e18 263: / 1e18 //@audit: 1e4 335: if (args.feeBps > 1e4) { //@audit: 1e4 352: uint128 fee = supply * args.feeBps / 1e4;
File: /contracts/crowdfund/Crowdfund.sol //@audit: 1e4 129: if (opts.governanceOpts.feeBps > 1e4) { //@audit: 1e4 132: if (opts.governanceOpts.passThresholdBps > 1e4) { //@audit: 1e4 135: if (opts.splitBps > 1e4) { //@audit: 1e4 370: votingPower = ((1e4 - splitBps_) * ethUsed) / 1e4; //@audit: 1e4 374: votingPower += (splitBps_ * totalEthUsed + (1e4 - 1)) / 1e4; // round up
File: /contracts/party/PartyGovernance.sol //@audit: 1e4 280: if (opts.feeBps > 1e4) { //@audit: 1e4 283: if (opts.passThresholdBps > 1e4) { //@audit: 1e4 1062: uint256 acceptanceRatio = (totalVotes * 1e4) / totalVotingPower; //@audit: 0.9999e4 1066: return acceptanceRatio >= 0.9999e4; //@audit: 1e4 1078: return uint256(voteCount) * 1e4
_mint() is discouraged in favor of _safeMint() which ensures that the recipient is either an EOA or implements IERC721Receiver. Both open OpenZeppelin and solmate have versions of this function so that NFTs aren't lost if they're minted to contracts that cannot transfer them back out.
File: /contracts/party/PartyGovernanceNFT.sol 132: _mint(owner, tokenId);
assembly { size := extcodesize() } => uint256 size = address().code.length
We can rewrite the code without the need for assembly here .
File: /contracts/utils/Implementation.sol 22: modifier onlyConstructor() { 23: uint256 codeSize; 24: assembly { codeSize := extcodesize(address()) } 25: if (codeSize != 0) { 26: revert OnlyConstructorError(); 27: } 28: _; 29: }
Contracts are allowed to override their parents' functions and change the visibility from external to public.
File: /contracts/crowdfund/CrowdfundFactory.sol 35: function createBuyCrowdfund( 36: BuyCrowdfund.BuyCrowdfundOptions memory opts, 37: bytes memory createGateCallData 38: ) 39: public 40: payable 41: returns (BuyCrowdfund inst) 42: {
File: /contracts/crowdfund/CrowdfundFactory.sol 61: function createAuctionCrowdfund( 62: AuctionCrowdfund.AuctionCrowdfundOptions memory opts, 63: bytes memory createGateCallData 64: ) 65: public
File: /contracts/crowdfund/CrowdfundFactory.sol 87: function createCollectionBuyCrowdfund( 88: CollectionBuyCrowdfund.CollectionBuyCrowdfundOptions memory opts, 89: bytes memory createGateCallData 90: ) 91: public
File: /contracts/crowdfund/Crowdfund.sol 167: function burn(address payable contributor) 168: public 169: { 191: function contribute(address delegate, bytes memory gateData) 192: public
Contracts should be deployed with the same compiler version and flags that they have been tested the most with. Locking the pragma helps ensure that contracts do not accidentally get deployed using, for example, the latest compiler which may have higher risks of undiscovered bugs. Contracts may also be deployed by others and the pragma indicates the compiler version intended by the original authors.
File: /contracts/utils/EIP165.sol 2: pragma solidity ^0.8;
https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/tokens/ERC1155Receiver.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/tokens/ERC721Receiver.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/utils/Proxy.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/utils/ReadOnlyDelegateCall.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/party/Party.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/party/PartyFactory.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/proposals/ProposalStorage.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/proposals/FractionalizeProposal.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/globals/Globals.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/BuyCrowdfund.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/CollectionBuyCrowdfund.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/CrowdfundFactory.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/CrowdfundNFT.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/proposals/ListOnZoraProposal.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/AuctionCrowdfund.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/proposals/ArbitraryCallsProposal.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/proposals/ProposalExecutionEngine.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/distribution/TokenDistributor.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/utils/Implementation.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/BuyCrowdfundBase.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/proposals/ListOnOpenseaProposal.sol#L2 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/party/PartyGovernance.sol#L2
Solidity contracts can use a special form of comments to provide rich documentation for functions, return variables and more.It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). see official docs
File: /contracts/utils/ReadOnlyDelegateCall.sol o //@audit: Missing @param impl, @param callData 18: function delegateCallAndRevert(address impl, bytes memory callData) external {
File: /contracts/party/PartyFactory.sol //@audit: Missing @param authority,@param opts,@param preciousTokens,@param preciousTokenIds,@return party 26: function createParty( 27: address authority, 28: Party.PartyOptions memory opts, 29: IERC721[] memory preciousTokens, 30: uint256[] memory preciousTokenIds 31: ) 32: external 33: returns (Party party) 34: {
File: /contracts/crowdfund/CrowdfundFactory.sol //@audit: Missing @return inst 35: function createBuyCrowdfund( 36: BuyCrowdfund.BuyCrowdfundOptions memory opts, 37: bytes memory createGateCallData 38: ) 39: public 40: payable 41: returns (BuyCrowdfund inst)
File: /contracts/crowdfund/CrowdfundFactory.sol //@audit: Missing @return inst 61: function createAuctionCrowdfund( 62: AuctionCrowdfund.AuctionCrowdfundOptions memory opts, 63: bytes memory createGateCallData 64: ) 65: public 66: payable 67: returns (AuctionCrowdfund inst) //@audit: Missing @return inst 87: function createCollectionBuyCrowdfund(
File: /contracts/party/PartyGovernanceNFT.sol //@audit: Missing @params, @returns 101: function royaltyInfo(uint256, uint256) 102: external 103: view 104: returns (address, uint256) { //@audit: Missing @param tokenId,@return 111: function getDistributionShareOf(uint256 tokenId) external view returns (uint256) {
require()/revert() statements should have descriptive reason strings
File: /contracts/utils/ReadOnlyDelegateCall.sol 20: require(msg.sender == address(this));
File: /contracts/proposals/ProposalExecutionEngine.sol 246: require(proposalType != ProposalType.Invalid); 247: require(uint8(proposalType) < uint8(ProposalType.NumProposalTypes));
Using both named returns and a return statement isn’t necessary in a function.To improve code quality, consider using only one of those.
File: /contracts/proposals/FractionalizeProposal.sol //@audit: nextProgressData has not been used anywhere in this function 39: function _executeFractionalize( 40: IProposalExecutionEngine.ExecuteProposalParams memory params 41: ) 42: internal 43: returns (bytes memory nextProgressData
File: /contracts/crowdfund/CrowdfundFactory.sol //@audit: newGateKeeperId has not been used anywhere in this function 107: function _prepareGate( 108: IGateKeeper gateKeeper, 109: bytes12 gateKeeperId, 110: bytes memory createGateCallData 111: ) 112: private 113: returns (bytes12 newGateKeeperId)
File: /contracts/crowdfund/CrowdfundNFT.sol //@audit: numTokens has not been used anywhere in this function 133: function balanceOf(address owner) external view returns (uint256 numTokens) { 134: return _doesTokenExistFor(owner) ? 1 : 0; 135: }
File: /contracts/proposals/ListOnZoraProposal.sol //@audit: sold not used anywhere in this function 164: function _settleZoraAuction( .... 169: ) 170: internal 171: override 172: returns (bool sold)
File: /contracts/proposals/ArbitraryCallsProposal.sol //@audit: nextProgressData not used in this function 37: function _executeArbitraryCalls( 38: IProposalExecutionEngine.ExecuteProposalParams memory params 39: ) 40: internal 41: returns (bytes memory nextProgressData) 42: {
File: /contracts/party/PartyGovernance.sol //@audit: votingPower 347: function getVotingPowerAt(address voter, uint40 timestamp) 348: external 349: view 350: returns (uint96 votingPower) 351: { 352: return getVotingPowerAt(voter, timestamp, type(uint256).max); 353: }
File: /contracts/party/PartyGovernance.sol //@audit: votingPower not used 361: function getVotingPowerAt(address voter, uint40 timestamp, uint256 snapIndex) 362: public 363: view 364: returns (uint96 votingPower) 365: { 366: VotingPowerSnapshot memory snap = _getVotingPowerSnapshotAt(voter, timestamp, snapIndex); 367: return (snap.isDelegated ? 0 : snap.intrinsicVotingPower) + snap.delegatedVotingPower; 368: }
File: /contracts/party/PartyGovernance.sol //@audit: named gv returned _governanceValues 385: function getGovernanceValues() external view returns (GovernanceValues memory gv) { 386: return _governanceValues; 387: }
File: /contracts/party/PartyGovernance.sol //@audit: named totalVotes returned values.votes 562: function accept(uint256 proposalId, uint256 snapIndex) 563: public 564: onlyDelegateCall 565: returns (uint256 totalVotes)
File: /contracts/party/PartyGovernance.sol //@audit: named completed, returned nextProgressData.length == 0; 804: function _executeProposal( 812: ) 813: private 814: returns (bool completed) 815: { 844: return nextProgressData.length == 0; 845: }
File: /contracts/proposals/ListOnZoraProposal.sol //@audit: exit instead of exist 60: // keccak256(abi.encodeWithSignature('Error(string)', "Auction doesn't exit"))
File: /contracts/crowdfund/CrowdfundFactory.sol //@audit: purchases instead of purchase 29: /// @notice Create a new crowdfund to purchases a specific NFT (i.e., with a //@audit: purchases instead of purchase 81: /// @notice Create a new crowdfund to purchases any NFT from a collection
File: /contracts/utils/ReadOnlyDelegateCall.sol //@audit: performs instead of perform 13: // Inherited by contracts to performs read-only delegate calls.
File: /contracts/party/PartyGovernance.sol //@audit: set the it to themself ? set it to themself?, maybe 896: // If `oldDelegate` is zero, `voter` never delegated, set the it to 897: // themself.
Events help non-contract tools to track changes, and events prevent users from being surprised by changes
File: /contracts/crowdfund/Crowdfund.sol 124: function _initialize(CrowdfundOptions memory opts)
File: /contracts/crowdfund/BuyCrowdfundBase.sol 70: function _initialize(BuyCrowdfundBaseOptions memory opts)
File: /contracts/crowdfund/AuctionCrowdfund.sol 110: function initialize(AuctionCrowdfundOptions memory opts)
File: /contracts/party/PartyGovernanceNFT.sol 49: function _initialize(
File: /contracts/party/Party.sol 33: function initialize(PartyInitData memory initData)
File: /contracts/crowdfund/BuyCrowdfund.sol 68: function initialize(BuyCrowdfundOptions memory opts)
File: /contracts/crowdfund/CollectionBuyCrowdfund.sol 83: function initialize(CollectionBuyCrowdfundOptions memory opts)
Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.
File: /contracts/crowdfund/Crowdfund.sol 82: event Burned(address contributor, uint256 ethUsed, uint256 ethOwed, uint256 votingPower); 83: event Contributed(address contributor, uint256 amount, address delegate, uint256 previousTotalContributions);
File: /contracts/crowdfund/CrowdfundFactory.sol 16: event BuyCrowdfundCreated(BuyCrowdfund crowdfund, BuyCrowdfund.BuyCrowdfundOptions opts); 17: event AuctionCrowdfundCreated(AuctionCrowdfund crowdfund, AuctionCrowdfund.AuctionCrowdfundOptions opts); 18: event CollectionBuyCrowdfundCreated(CollectionBuyCrowdfund crowdfund,CollectionBuyCrowdfund.CollectionBuyCrowdfundOptions opts);
File: /contracts/proposals/ListOnZoraProposal.sol 45: event ZoraAuctionCreated( 46: uint256 auctionId, 47: IERC721 token, 48: uint256 tokenId, 49: uint256 startingPrice, 50: uint40 duration, 51: uint40 timeoutTime 52: ); 53: event ZoraAuctionExpired(uint256 auctionId, uint256 expiry); 54: event ZoraAuctionSold(uint256 auctionId); 55: event ZoraAuctionFailed(uint256 auctionId);
File: /contracts/proposals/ArbitraryCallsProposal.sol 35: event ArbitraryCallExecuted(uint256 proposalId, uint256 idx, uint256 count);
File: /contracts/proposals/ProposalExecutionEngine.sol 66: event ProposalEngineImplementationUpgraded(address oldImpl, address newImpl);
File: /contracts/crowdfund/BuyCrowdfundBase.sol 52: event Won(Party party, IERC721 token, uint256 tokenId, uint256 settledPrice);
File: /contracts/proposals/ListOnOpenseaProposal.sol event OpenseaOrderListed( IOpenseaExchange.OrderParameters orderParams, bytes32 orderHash, IERC721 token, uint256 tokenId, uint256 listPrice, uint256 expiry ); event OpenseaOrderSold( bytes32 orderHash, IERC721 token, uint256 tokenId, uint256 listPrice ); event OpenseaOrderExpired( bytes32 orderHash, IERC721 token, uint256 tokenId, uint256 expiry ); // Coordinated event w/OS team to track on-chain orders. event OrderValidated( bytes32 orderHash, address indexed offerer, address indexed zone, IOpenseaExchange.OfferItem[] offer, IOpenseaExchange.ConsiderationItem[] consideration, IOpenseaExchange.OrderType orderType, uint256 startTime, uint256 endTime, bytes32 zoneHash, uint256 salt, bytes32 conduitKey, uint256 counter );
File: /contracts/party/PartyGovernance.sol 151: event Proposed( 152: uint256 proposalId, 153: address proposer, 154: Proposal proposal 155: ); 156: event ProposalAccepted( 157: uint256 proposalId, 158: address voter, 159: uint256 weight 160: );
Use a solidity version of at least 0.8.2 to get simple compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value
The entire codebase seems to be using the following
pragma solidity ^0.8;
File: /contracts/party/PartyGovernance.sol 238: modifier onlyPartyDao() { 239: { 240: address partyDao = _GLOBALS.getAddress(LibGlobals.GLOBAL_DAO_WALLET); 241: if (msg.sender != partyDao) { 242: revert OnlyPartyDaoError(msg.sender, partyDao); 243: } 244: } 245: _; 246: }
The above is only used on Line 790
File: /contracts/party/PartyGovernance.sol 249: modifier onlyPartyDaoOrHost() { 250: address partyDao = _GLOBALS.getAddress(LibGlobals.GLOBAL_DAO_WALLET); 251: if (msg.sender != partyDao && !isHost[msg.sender]) { 252: revert OnlyPartyDaoOrHostError(msg.sender, partyDao); 253: } 254: _; 255: }
The above modifier is only used on Line 800
File: /contracts/party/PartyGovernance.sol 258: modifier onlyWhenEmergencyExecuteAllowed() { 259: if (emergencyExecuteDisabled) { 260: revert OnlyWhenEmergencyActionsAllowedError(); 261: } 262: _; 263: }
The above is only used on Line 791
🌟 Selected for report: CertoraInc
Also found by: 0x1f8b, 0x4non, 0x5rings, 0x85102, 0xNazgul, 0xSmartContract, 0xkatana, Amithuddar, Aymen0909, B2, Bnke0x0, CRYP70, Chom, ChristianKuri, CodingNameKiki, Deivitto, Diraco, Fitraldys, Funen, IgnacioB, JAGADESH, JC, Lambda, LeoS, Matin, Metatron, MiloTruck, Noah3o6, Ocean_Sky, Olivierdem, PaludoX0, RaymondFam, ReyAdmirado, Rohan16, Rolezn, Saintcode_, Sm4rty, SnowMan, StevenL, Tomio, Tomo, V_B, Waze, __141345__, ajtra, asutorufos, aysha, brgltd, bulej93, c3phas, ch0bu, d3e4, delfin454000, dharma09, djxploit, erictee, fatherOfBlocks, francoHacker, gianganhnguyen, gogo, got_targ, ignacio, jag, karanctf, ladboy233, leosathya, lukris02, m_Rassska, malinariy, martin, natzuu, pashov, peanuts, peiw, pfapostol, prasantgupta52, robee, simon135, slowmoses, sryysryy, tnevler
105.0279 USDC - $105.03
NB: Some functions have been truncated where neccessary to just show affected parts of the code
Throught the report some places might be denoted with audit tags to show the actual place affected.
The code can be optimized by minimizing the number of SLOADs.
SLOADs are expensive (100 gas after the 1st one) compared to MLOADs/MSTOREs (3 gas each). Storage values read multiple times should instead be cached in memory the first time (costing 1 SLOAD) and then read from this cache to avoid multiple SLOADs.
File: /contracts/party/PartyGovernanceNFT.sol 35: modifier onlyMinter() { 36: if (msg.sender != mintAuthority) { //@audit: mintAuthority SLOAD 1 37: revert OnlyMintAuthorityError(msg.sender, mintAuthority); //@audit: mintAuthority SLOAD 2 38: } 39: _; 40: }
As the above is a modifer , everytime the modifier is used in a function, we read the storage value mintAuthority two times inside the modifier only
The above modifer has been used two times in this contract , at Line 126 and Line 169, which means 4 SLOADS
File: /contracts/crowdfund/AuctionCrowdfund.sol 149: function bid() external onlyDelegateCall { 162: if (market.isFinalized(auctionId_)) { //@audit: market SLOAD 1 166: if (market.getCurrentHighestBidder(auctionId_) == address(this)) { //@audit: market SLOAD 2 169: // Get the minimum necessary bid to be the highest bidder. 170: uint96 bidAmount = market.getMinimumBid(auctionId_).safeCastUint256ToUint96(); //@audit: market SLOAD 3 178: (bool s, bytes memory r) = address(market).delegatecall(abi.encodeCall( //@audit: market SLOAD 4 179: IMarketWrapper.bid, 180: (auctionId_, bidAmount) 181: )); 188: }
File: /contracts/crowdfund/AuctionCrowdfund.sol 149: function bid() external onlyDelegateCall { 171: // Make sure the bid is less than the maximum bid. 172: if (bidAmount > maximumBid) { //@audit: maximumBid SLOAD 1 173: revert ExceedsMaximumBidError(bidAmount, maximumBid); //@audit: maximumBid SLOAD 2 174: } 188: }
File: /contracts/crowdfund/AuctionCrowdfund.sol 196: function finalize(FixedGovernanceOpts memory governanceOpts) ... 215: if (!market.isFinalized(auctionId_)) { //@audit: market SLOAD 1 216: // Note that even if this crowdfund has expired but the auction is still 217: // ongoing, this call can fail and block finalization until the auction ends. 218: (bool s, bytes memory r) = address(market).call(abi.encodeCall( //@audit: market SLOAD 2 219: IMarketWrapper.finalize, 220: auctionId_ 221: )); ... 259: }
File: /contracts/crowdfund/AuctionCrowdfund.sol 196: function finalize(FixedGovernanceOpts memory governanceOpts) 232: // Are we now in possession of the NFT? 233: if (nftContract.safeOwnerOf(nftTokenId) == address(this)) {//@audit: nftContract SLOAD 1,//@audit: nftTokenId SLOAD 1 243: // Create a governance party around the NFT. 244: party_ = _createParty( 245: _getPartyFactory(), 246: governanceOpts, 247: nftContract, //@audit: nftContract SLOAD 2 248: nftTokenId //@audit: nftTokenId SLOAD 2 249: ); 259: }
File: /contracts/crowdfund/BuyCrowdfundBase.sol 90: function _buy( 116: uint96 maximumPrice_ = maximumPrice; //@audit: maximumPrice SLOAD 1 117: if (maximumPrice_ != 0 && callValue > maximumPrice_) { 118: revert MaximumPriceError(callValue, maximumPrice); //@audit: maximumPrice SLOAD 2(use cached value) 119: } 147: }
In the above function , as the value of maximumPrice is already cached as maximumPrice_ , we should use the cached value in the revert statement
File: /contracts/crowdfund/Crowdfund.sol 262: function _createParty( 271: if (party != Party(payable(0))) { //@audit: party SLOAD 1 272: revert PartyAlreadyExistsError(party);//@audit: party SLOAD 2 273: } 303: }
File: /contracts/crowdfund/Crowdfund.sol 275: bytes16 governanceOptsHash_ = _hashFixedGovernanceOpts(governanceOpts); 276: if (governanceOptsHash_ != governanceOptsHash) { //@audit: governanceOptsHash SLOAD 1 277: revert InvalidGovernanceOptionsError(governanceOptsHash_, governanceOptsHash);//@audit: SLOAD 2 278: }
File: /contracts/crowdfund/Crowdfund.sol 378: function _contribute( 392: if (gateKeeper != IGateKeeper(address(0))) { //@audit: gateKeeper SLOAD 1 393: if (!gateKeeper.isAllowed(contributor, gateKeeperId, gateData)) { //@audit: gateKeeper SLOAD 2 , //gateKeeperId SLOAD 1 394: revert NotAllowedByGateKeeperError( 395: contributor, 396: gateKeeper, //@audit: gateKeeper SLOAD 3 397: gateKeeperId, //@audit: gateKeeperId SLOAD 2 398: gateData 399: ); 442: }
File: /contracts/crowdfund/Crowdfund.sol 444: function _burn(address payable contributor, CrowdfundLifecycle lc, Party party_) 457: if (contributor == splitRecipient) { //@audit: splitRecipient SLOAD 1 464: if (splitRecipient != contributor || _doesTokenExistFor(contributor)) { //@audit: splitRecipient SLOAD 2
File: /contracts/party/PartyGovernance.sol 562: function accept(uint256 proposalId, uint256 snapIndex) 600: if (values.passedTime == 0 && _areVotesPassing( 601: values.votes, 602: _governanceValues.totalVotingPower, //@audit: _governanceValues SLOAD 1 603: _governanceValues.passThresholdBps)) //@audit: _governanceValues SLOAD 2 609: }
Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
File: /contracts/proposals/ArbitraryCallsProposal.sol 93: function _executeSingleArbitraryCall( 94: uint256 idx, 95: ArbitraryCall memory call, 96: IERC721[] memory preciousTokens, 97: uint256[] memory preciousTokenIds, 98: bool isUnanimous, 99: uint256 ethAvailable 100: ) 101: private 102: {
The above is only called once on Line 63
File: /contracts/proposals/ArbitraryCallsProposal.sol 142: function _isCallAllowed( 143: ArbitraryCall memory call, 144: bool isUnanimous, 145: IERC721[] memory preciousTokens, 146: uint256[] memory preciousTokenIds 147: ) 148: private 149: view 150: returns (bool isAllowed) 151: {
The above is only called once on Line 104
203: function _decodeApproveCallDataArgs(bytes memory callData) 204: private 205: pure 206: returns (address operator, uint256 tokenId) 207: {
The above is only called once on Line 175
File: /contracts/proposals/ArbitraryCallsProposal.sol 221: function _decodeSetApprovalForAllCallDataArgs(bytes memory callData) 222: private 223: pure 224: returns (address operator, bool isApproved) 225: {
The above is only called once on Line 187
File: /contracts/proposals/ProposalExecutionEngine.sol 228: function _extractProposalType(bytes memory proposalData) 229: private 230: pure 231: returns (ProposalType proposalType, bytes memory offsetProposalData) 232: {
The above is only called once on Line 169
File: /contracts/proposals/ProposalExecutionEngine.sol 251: function _executeUpgradeProposalsImplementation(bytes memory proposalData) 252: private 253: {
The above is only called once on Line 219
File: /contracts/party/PartyGovernance.sol 804: function _executeProposal( 805: uint256 proposalId, 806: Proposal memory proposal, 807: IERC721[] memory preciousTokens, 808: uint256[] memory preciousTokenIds, 809: uint256 flags, 810: bytes memory progressData, 811: bytes memory extraData 812: ) 813: private 814: returns (bool completed) 815: {
The above is only called once on Line 697
File: /contracts/party/PartyGovernance.sol 916: function _getTotalVotingPower() internal view returns (uint256) {
File: /contracts/party/PartyGovernance.sol 922: function _rebalanceDelegates( 923: address voter, 924: address oldDelegate, 925: address newDelegate, 926: VotingPowerSnapshot memory oldSnap, 927: VotingPowerSnapshot memory newSnap 928: ) 929: private 930: {
The above is only called once on Line 913
File: /contracts/party/PartyGovernance.sol 1000: function _getProposalFlags(ProposalStateValues memory pv) 1001: private 1002: view 1003: returns (uint256) 1004: {
The above is only called once on Line 702
File: /contracts/party/PartyGovernance.sol 1069: function _areVotesPassing( 1070: uint96 voteCount, 1071: uint96 totalVotingPower, 1072: uint16 passThresholdBps 1073: ) 1074: private
The above is only called once on Line 600
File: /contracts/party/PartyGovernance.sol 1082: function _setPreciousList( 1083: IERC721[] memory preciousTokens, 1084: uint256[] memory preciousTokenIds 1085: ) 1086: private 1087: {
The above is only called once on Line 304
File: /contracts/party/PartyGovernance.sol 1094: function _isPreciousListCorrect(
File: /contracts/proposals/ArbitraryCallsProposal.sol 37: function _executeArbitraryCalls( 38: IProposalExecutionEngine.ExecuteProposalParams memory params 39: ) 40: internal 41: returns (bytes memory nextProgressData) 42: {
The above is called once on a child contract on Line 217
File: /contracts/party/PartyGovernance.sol 848: function _getVotingPowerSnapshotAt(address voter, uint40 timestamp, uint256 hintIndex) 849: internal 850: view 851: returns (VotingPowerSnapshot memory snap) 852: {
The above is only called once on Line 366
Caching a mapping's value in a local storage or calldata variable when the value is accessed multiple times saves ~42 gas per access due to not having to perform the same offset calculation every time.
To help the optimizer,declare a storage type variable and use it instead of repeatedly fetching the reference in a map or an array.
As an example, instead of repeatedly calling someMap[someIndex], save its reference like this: SomeStruct storage someStruct = someMap[someIndex] and use it.
File: /contracts/crowdfund/CrowdfundNFT.sol 141: function _mint(address owner) internal returns (uint256 tokenId) 142: { 143: tokenId = uint256(uint160(owner)); 144: if (_owners[tokenId] != owner) { //@audit: Cache _owners[tokenId] in local storage 145: _owners[tokenId] = owner; /@audit: _owners[tokenId] should use the cached value 146: emit Transfer(address(0), owner, tokenId); 147: } 148: }
File: /contracts/crowdfund/CrowdfundNFT.sol 150: function _burn(address owner) internal { 152: if (_owners[tokenId] == owner) { //@audit: _owners[tokenId] should be cached in local storage 153: _owners[tokenId] = address(0); //@audit: _owners[tokenId] should use the cached variable to update the value
File: /contracts/distribution/TokenDistributor.sol 331: function _createDistribution(CreateDistributionArgs memory args) 341: supply = (args.currentTokenBalance - _storedBalances[balanceId]) //@audit: Initial access 342: .safeCastUint256ToUint128(); 347: // Update stored balance. 348: _storedBalances[balanceId] = args.currentTokenBalance; //@audit: Second access 349: }
File: /contracts/party/PartyGovernance.sol 526: function propose(Proposal memory proposal, uint256 latestSnapIndex) 535: _proposalStateByProposalId[proposalId].values, //@audit: _proposalStateByProposalId[proposalId] 536: _proposalStateByProposalId[proposalId].hash //@audit: _proposalStateByProposalId[proposalId] 549: }
When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution. Note that even if an interface defines a function as having memory arguments, it's still valid for implementation contracs to use calldata arguments instead.
If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it's still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one
Note that I've also flagged instances where the function is public but can be marked as external since it's not called by the contract, and cases where a constructor is involved
File: /contracts/utils/ReadOnlyDelegateCall.sol //@audit: bytes memory callData should be declared as bytes calldata callData 18: function delegateCallAndRevert(address impl, bytes memory callData) external { 19: // Attempt to gate to only `_readOnlyDelegateCall()` invocations. 20: require(msg.sender == address(this)); 21: (bool s, bytes memory r) = impl.delegatecall(callData); 22: // Revert with success status and return data. 23: abi.encode(s, r).rawRevert(); 24: }
File: /contracts/party/PartyFactory.sol //@audit: use calldata on opts,preciousTokens and preciousTokenIds as they are readonly in this function 26: function createParty( 27: address authority, 28: Party.PartyOptions memory opts, 29: IERC721[] memory preciousTokens, 30: uint256[] memory preciousTokenIds 31: ) 32: external 33: returns (Party party) 34: {
File: /contracts/crowdfund/AuctionCrowdfund.sol //@audit: opts should use calldata as it's readonly in this function 110: function initialize(AuctionCrowdfundOptions memory opts) 111: external 112: payable 113: onlyConstructor 114: {
File: /contracts/crowdfund/AuctionCrowdfund.sol //@audit: use calldata on governanceOpts 196: function finalize(FixedGovernanceOpts memory governanceOpts) 197: external 198: onlyDelegateCall 199: returns (Party party_) 200: {
File: /contracts/party/PartyGovernance.sol //@audit: use calldata on proposal as it's readonly in this function 526: function propose(Proposal memory proposal, uint256 latestSnapIndex) 527: external 528: onlyActiveMember 529: onlyDelegateCall 530: returns (uint256 proposalId) 531: {
File: /contracts/party/PartyGovernance.sol //@audit: use calldata on proposal,preciousTokens and preciousTokenIds 653: function execute( 654: uint256 proposalId, 655: Proposal memory proposal, 656: IERC721[] memory preciousTokens, 657: uint256[] memory preciousTokenIds, 658: bytes calldata progressData, 659: bytes calldata extraData 660: ) 661: external
Prior to 0.8.10 the compiler inserted extra code, including EXTCODESIZE (100 gas), to check for contract existence for external calls. In more recent solidity versions, the compiler will not insert these checks if the external call has a return value
File: /contracts/party/PartyGovernance.sol 240: address partyDao = _GLOBALS.getAddress(LibGlobals.GLOBAL_DAO_WALLET); 250: address partyDao = _GLOBALS.getAddress(LibGlobals.GLOBAL_DAO_WALLET); 744: uint256 globalMaxCancelDelay = 745: _GLOBALS.getUint256(LibGlobals.GLOBAL_PROPOSAL_MAX_CANCEL_DURATION);
File: /contracts/proposals/ListOnOpenseaProposal.sol 202: uint40 minDuration = uint40(_GLOBALS.getUint256(LibGlobals.GLOBAL_OS_MIN_ORDER_DURATION)); 203: uint40 maxDuration = uint40(_GLOBALS.getUint256(LibGlobals.GLOBAL_OS_MAX_ORDER_DURATION)); 254: bytes32 conduitKey = _GLOBALS.getBytes32(LibGlobals.GLOBAL_OPENSEA_CONDUIT_KEY); 255: (address conduit,) = CONDUIT_CONTROLLER.getConduit(conduitKey); 265: orderParams.zone = _GLOBALS.getAddress(LibGlobals.GLOBAL_OPENSEA_ZONE); 346: orderHash = SEAPORT.getOrderHash(orderComps); 359: (,, uint256 totalFilled,) = SEAPORT.getOrderStatus(orderHash);
The above are just some of the instances that would benefit from this.
While the DIV / MUL opcode uses 5 gas, the SHR / SHL opcode only uses 3 gas. Furthermore, beware that Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting. Eventually, overflow checks are never performed for shift operations as they are done for arithmetic operations. Instead, the result is always truncated, so the calculation can be unchecked in Solidity version 0.8+
Use >> 1 instead of / 2
File: /contracts/party/PartyGovernance.sol 434: uint256 mid = (low + high) / 2;
File: /contracts/crowdfund/Crowdfund.sol 411: totalContributions += amount;
The above should be modified to
totalContributions = totalContributions + amount;
File: /contracts/distribution/TokenDistributor.sol 381: _storedBalances[balanceId] -= amount;
When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.
https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed
File: /contracts/crowdfund/BuyCrowdfund.sol //@audit: uint96 callValue 98:function buy( 99: address payable callTarget, 100: uint96 callValue, 101: bytes calldata callData, 102: FixedGovernanceOpts memory governanceOpts 103: )
File: /contracts/crowdfund/CollectionBuyCrowdfund.sol //@audit: uint96 callValue 113: function buy( 114: uint256 tokenId, 115: address payable callTarget, 116: uint96 callValue, 117: bytes calldata callData, 118: FixedGovernanceOpts memory governanceOpts 119: )
File: /contracts/proposals/ListOnZoraProposal.sol 94: uint40 minDuration = uint40(_GLOBALS.getUint256(LibGlobals.GLOBAL_ZORA_MIN_AUCTION_DURATION)); 95: uint40 maxDuration = uint40(_GLOBALS.getUint256(LibGlobals.GLOBAL_ZORA_MAX_AUCTION_DURATION)); 102: uint40 maxTimeout = uint40(_GLOBALS.getUint256(LibGlobals.GLOBAL_ZORA_MAX_AUCTION_TIMEOUT));
File: /contracts/proposals/ListOnZoraProposal.sol //@audit: uint40 timeout,uint40 duration 129: function _createZoraAuction( 130: // The minimum bid. 131: uint256 listPrice, 132: // How long the auction must wait for the first bid. 133: uint40 timeout, 134: // How long the auction will run for once a bid has been placed. 135: uint40 duration, 136: IERC721 token, 137: uint256 tokenId 138: )
File: /contracts/proposals/ListOnZoraProposal.sol //@audit: uint40 minExpiry 164: function _settleZoraAuction( 165: uint256 auctionId, 166: uint40 minExpiry, 167: IERC721 token, 168: uint256 tokenId 169: )
File: /contracts/proposals/ListOnOpenseaProposal.sol 151: uint40 zoraTimeout = 152: uint40(_GLOBALS.getUint256(LibGlobals.GLOBAL_OS_ZORA_AUCTION_TIMEOUT)); 153: uint40 zoraDuration = 202: uint40 minDuration = uint40(_GLOBALS.getUint256(LibGlobals.GLOBAL_OS_MIN_ORDER_DURATION)); 203: uint40 maxDuration = uint40(_GLOBALS.getUint256(LibGlobals.GLOBAL_OS_MAX_ORDER_DURATION));
File: /contracts/distribution/TokenDistributor.sol //@audit: uint16 feeBps 98: function createNativeDistribution( 99: ITokenDistributorParty party, 100: address payable feeRecipient, 101: uint16 feeBps 102: )
File: /contracts/distribution/TokenDistributor.sol //@audit: uint16 feeBps 118: function createErc20Distribution( 119: IERC20 token, 120: ITokenDistributorParty party, 121: address payable feeRecipient, 122: uint16 feeBps 123: ) 338: uint128 supply; 352: uint128 fee = supply * args.feeBps / 1e4; 353: uint128 memberSupply = supply - fee;
File: /contracts/crowdfund/BuyCrowdfundBase.sol //@audit: uint96 callValue 90: function _buy( 91: IERC721 token, 92: uint256 tokenId, 93: address payable callTarget, 94: uint96 callValue, 95: bytes calldata callData, 96: FixedGovernanceOpts memory governanceOpts 97: ) 114: uint96 settledPrice_; 116: uint96 maximumPrice_ = maximumPrice;
File: /contracts/crowdfund/Crowdfund.sol //@audit: uint96 amount,uint96 previousTotalContributions 378: function _contribute( 379: address contributor, 380: uint96 amount, 381: address delegate, 382: uint96 previousTotalContributions, 383: bytes memory gateData 384: )
File: /contracts/party/PartyGovernance.sol //@audit: uint40 timestamp 347: function getVotingPowerAt(address voter, uint40 timestamp) //@audit: uint40 timestamp 361: function getVotingPowerAt(address voter, uint40 timestamp, uint256 snapIndex) //@audit: uint40 timestamp 422: function findVotingPowerSnapshotIndex(address voter, uint40 timestamp) //@audit: uint40 timestamp 848: function _getVotingPowerSnapshotAt(address voter, uint40 timestamp, uint256 hintIndex) //@audit: uint96 totalVotes, uint96 totalVotingPower 1057: function _isUnanimousVotes(uint96 totalVotes, uint96 totalVotingPower)
File: contracts/party/PartyGovernance.sol //@audit: uint96 voteCount,uint96 totalVotingPower,uint16 passThresholdBps 1069: function _areVotesPassing( 1070: uint96 voteCount, 1071: uint96 totalVotingPower, 1072: uint16 passThresholdBps 1073: )
Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block see resource https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/distribution/TokenDistributor.sol#L170
File: /contracts/distribution/TokenDistributor.sol 170: state.remainingMemberSupply = remainingMemberSupply - amountClaimed;
The above operation cannot underflow due to the check on Line 167 that ensures that remainingMemberSupply is greater than amountClaimed
File: /contracts/crowdfund/Crowdfund.sol 424: Contribution memory lastContribution = contributions[numContributions - 1]; 428: contributions[numContributions - 1] = lastContribution;
The operation numContributions - 1 cannot underflow due to the check on Line 423 that ensures that numContributions is greater or equal to 1
File: /contracts/party/PartyGovernance.sol 445: return high == 0 ? type(uint256).max : high - 1;
The above cannot underflow due to the check on high == 0? which is just a shorthand that will ensure that high - 1 would only be evaluated if high > 0
File: /contracts/party/PartyGovernance.sol 862: (hintIndex == snapsLength - 1 || snaps[hintIndex+1].timestamp > timestamp)
The operation snapsLength - 1 cannot underflow due to the check on Line 855 that ensures that snapsLength is greater or equal to 1
File: /contracts/party/PartyGovernance.sol 979: if (n != 0) { 980: VotingPowerSnapshot memory lastSnap = voterSnaps[n - 1]; 981: if (lastSnap.timestamp == snap.timestamp) { 982: voterSnaps[n - 1] = snap;
The operation n - 1 cannot unerflow due to the check on Line 979 that ensures that n is greater than or equal to 1 before perfoming the subtraction arithmetic
File: /contracts/party/PartyGovernance.sol 996: if (n != 0) { 997: snap = voterSnaps[n - 1]; 998: }
The operation n - 1 cannot unerflow due to the check on Line 996 that ensures that n is greater than or equal to 1 before perfoming the subtraction arithmetic
The majority of Solidity for loops increment a uint256 variable that starts at 0. These increment operations never need to be checked for over/underflow because the variable will never reach the max number of uint256 (will run out of gas long before that happens). The default over/underflow check wastes gas in every iteration of virtually every for loop
Affected code
File: /contracts/crowdfund/CollectionBuyCrowdfund.sol 62: for (uint256 i; i < hosts.length; i++) { 63: if (hosts[i] == msg.sender) { 64: isHost = true; 65: break; 66: } 67: }
The above should be modified to:
for (uint256 i; i < hosts.length;) { if (hosts[i] == msg.sender) { isHost = true; break; } unchecked { ++i; } }
Other Instances to modify https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/proposals/ListOnOpenseaProposal.sol#L291
File: /contracts/proposals/ListOnOpenseaProposal.sol 291: for (uint256 i = 0; i < fees.length; ++i) {
File: /contracts/proposals/LibProposal.sol 14: for (uint256 i = 0; i < preciousTokens.length; ++i) { 32: for (uint256 i = 0; i < preciousTokens.length; ++i) {
Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.
The solidity compiler will always read the length of the array during each iteration. That is,
1.if it is a storage array, this is an extra sload operation (100 additional extra gas (EIP-2929 2) for each iteration except for the first), 2.if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first), 3.if it is a calldata array, this is an extra calldataload operation (3 additional gas for each iteration except for the first)
This extra costs can be avoided by caching the array length (in stack): When reading the length of an array, sload or mload or calldataload operation is only called once and subsequently replaced by a cheap dupN instruction. Even though mload , calldataload and dupN have the same gas cost, mload and calldataload needs an additional dupN to put the offset in the stack, i.e., an extra 3 gas. which brings this to 6 gas
Here, I suggest storing the array’s length in a variable before the for-loop, and use it instead:
File: /contracts/crowdfund/CollectionBuyCrowdfund.sol 62: for (uint256 i; i < hosts.length; i++) {
The above should be modified to
uint256 length = hosts.length; for (uint256 i; i < length; i++) {
Other instances to modify https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/proposals/ArbitraryCallsProposal.sol#L50-L52
File: /contracts/proposals/ArbitraryCallsProposal.sol 50: bool[] memory hadPreciouses = new bool[](params.preciousTokenIds.length); 51: if (!isUnanimous) { 52: for (uint256 i = 0; i < hadPreciouses.length; ++i) {
File: /contracts/proposals/ArbitraryCallsProposal.sol 61: for (uint256 i = 0; i < calls.length; ++i) { 78: for (uint256 i = 0; i < hadPreciouses.length; ++i) {
File: /contracts/distribution/TokenDistributor.sol 230: for (uint256 i = 0; i < infos.length; ++i) { 239: for (uint256 i = 0; i < infos.length; ++i) {
File: /contracts/crowdfund/Crowdfund.sol 180: for (uint256 i = 0; i < contributors.length; ++i) { 300: for (uint256 i = 0; i < preciousTokens.length; ++i) {
File: /contracts/party/PartyGovernance.sol 306: for (uint256 i=0; i < opts.hosts.length; ++i) {
File: /contracts/proposals/ListOnOpenseaProposal.sol 291: for (uint256 i = 0; i < fees.length; ++i) {
File: /contracts/proposals/LibProposal.sol 14: for (uint256 i = 0; i < preciousTokens.length; ++i) { 32: for (uint256 i = 0; i < preciousTokens.length; ++i) {
++i costs less gas compared to i++ or i += 1 for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.
i++ increments i and returns the initial value of i. Which means:
uint i = 1; i++; // == 1 but i == 2
But ++i returns the actual incremented value:
uint i = 1; ++i; // == 2 and i == 2 too, so no need for a temporary variable
In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2
Instances include:
File: /contracts/crowdfund/CollectionBuyCrowdfund.sol 62: for (uint256 i; i < hosts.length; i++) {
// Booleans are more expensive than uint256 or any type that takes up a full // word because each write operation emits an extra SLOAD to first read the // slot's contents, replace the bits taken up by the boolean, and then write // back. This is the compiler's defense against contract upgrades and // pointer aliasing, and it cannot be disabled.
See source Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past
Instances affected include https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/globals/Globals.sol#L12
File: /contracts/globals/Globals.sol 12: mapping(uint256 => mapping(bytes32 => bool)) private _includedWordValues;
File: /contracts/distribution/TokenDistributor.sol 62: bool public emergencyActionsDisabled;
File: /contracts/crowdfund/Crowdfund.sol 106: bool private _splitRecipientHasBurned;
File: /contracts/party/PartyGovernance.sol 197: bool public emergencyExecuteDisabled; 207: mapping(address => bool) public isHost;
File: /contracts/party/PartyGovernance.sol 238: modifier onlyPartyDao() { 239: { 240: address partyDao = _GLOBALS.getAddress(LibGlobals.GLOBAL_DAO_WALLET); 241: if (msg.sender != partyDao) { 242: revert OnlyPartyDaoError(msg.sender, partyDao); 243: } 244: } 245: _; 246: }
The above is only used on Line 790
File: /contracts/party/PartyGovernance.sol 249: modifier onlyPartyDaoOrHost() { 250: address partyDao = _GLOBALS.getAddress(LibGlobals.GLOBAL_DAO_WALLET); 251: if (msg.sender != partyDao && !isHost[msg.sender]) { 252: revert OnlyPartyDaoOrHostError(msg.sender, partyDao); 253: } 254: _; 255: }
The above modifier is only used on Line 800
File: /contracts/party/PartyGovernance.sol 258: modifier onlyWhenEmergencyExecuteAllowed() { 259: if (emergencyExecuteDisabled) { 260: revert OnlyWhenEmergencyActionsAllowedError(); 261: } 262: _; 263: }
The above is only used on Line 791
Changing abi.encode function to abi.encodePacked can save gas since the abi.encode function pads extra null bytes at the end of the call data, which is unnecessary. Also, in general, abi.encodePacked is more gas-efficient see Solidity-Encode-Gas-Comparison.
Consider using abi.encodePacked() here:
File: /contracts/utils/ReadOnlyDelegateCall.sol 23: abi.encode(s, r).rawRevert();
Use a solidity version of at least 0.8.2 to get simple compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value
The entire codebase seems to be using the following
pragma solidity ^0.8;