From a5212a154e23143a713691031547a64710d0ca1b Mon Sep 17 00:00:00 2001 From: Brian Le Date: Fri, 21 Oct 2022 22:51:23 -0400 Subject: [PATCH] refactors: QAs + gas optimizations (#142) * refactors + optimizations * style: tests * refactor * rebase Co-authored-by: Lawrence Forman --- contracts/crowdfund/AuctionCrowdfund.sol | 51 ++++++++++++------- contracts/crowdfund/BuyCrowdfund.sol | 1 - contracts/crowdfund/BuyCrowdfundBase.sol | 3 +- contracts/crowdfund/Crowdfund.sol | 49 +++++++++--------- contracts/crowdfund/CrowdfundFactory.sol | 4 +- contracts/crowdfund/CrowdfundNFT.sol | 8 +++ contracts/distribution/ITokenDistributor.sol | 8 +-- contracts/distribution/TokenDistributor.sol | 4 +- contracts/globals/Globals.sol | 15 +++++- contracts/party/PartyFactory.sol | 2 +- contracts/party/PartyGovernance.sol | 35 +++++++------ contracts/party/PartyGovernanceNFT.sol | 5 +- .../proposals/ArbitraryCallsProposal.sol | 6 +-- contracts/proposals/LibProposal.sol | 4 +- contracts/proposals/ListOnOpenseaProposal.sol | 2 +- contracts/proposals/ListOnZoraProposal.sol | 2 +- .../proposals/ProposalExecutionEngine.sol | 7 +-- contracts/proposals/ProposalStorage.sol | 3 +- contracts/tokens/ERC1155Receiver.sol | 3 +- contracts/tokens/ERC721Receiver.sol | 3 +- contracts/utils/EIP165.sol | 2 +- contracts/utils/LibAddress.sol | 2 + contracts/utils/PartyHelpers.sol | 6 +-- contracts/utils/ReadOnlyDelegateCall.sol | 4 +- contracts/vendor/solmate/ERC1155.sol | 8 +-- deploy/deploy.sol | 2 +- docs/governance.md | 2 +- sol-tests/crowdfund/BuyCrowdfund.t.sol | 2 +- sol-tests/crowdfund/Crowdfund.t.sol | 2 +- sol-tests/distribution/TokenDistributor.t.sol | 2 +- .../distribution/TokenDistributorUnit.t.sol | 12 ++--- .../gatekeepers/AllowListGateKeeper.t.sol | 2 +- sol-tests/party/PartyFactory.t.sol | 2 +- sol-tests/party/PartyGovernanceUnit.t.sol | 2 +- .../proposals/ArbitraryCallsProposal.t.sol | 16 +++--- .../ListOnOpenseaProposalForked.t.sol | 4 +- sol-tests/proposals/OpenseaTestUtils.sol | 5 +- 37 files changed, 163 insertions(+), 127 deletions(-) diff --git a/contracts/crowdfund/AuctionCrowdfund.sol b/contracts/crowdfund/AuctionCrowdfund.sol index 9b1cbbd5..de1b22e9 100644 --- a/contracts/crowdfund/AuctionCrowdfund.sol +++ b/contracts/crowdfund/AuctionCrowdfund.sol @@ -84,6 +84,7 @@ contract AuctionCrowdfund is Crowdfund { error AuctionFinalizedError(uint256 auctionId); error AlreadyHighestBidderError(); error ExceedsMaximumBidError(uint256 bidAmount, uint256 maximumBid); + error MinimumBidExceedsMaximumBidError(uint256 bidAmount, uint256 maximumBid); error NoContributionsError(); error AuctionNotExpiredError(); @@ -151,6 +152,12 @@ contract AuctionCrowdfund is Crowdfund { { revert InvalidAuctionIdError(); } + + // Check that the minimum bid is less than the maximum bid. + uint256 minimumBid = market.getMinimumBid(opts.auctionId); + if (minimumBid > opts.maximumBid) { + revert MinimumBidExceedsMaximumBidError(minimumBid, opts.maximumBid); + } } /// @notice Accept naked ETH, e.g., if an auction needs to return ETH to us. @@ -196,18 +203,19 @@ contract AuctionCrowdfund is Crowdfund { _bidStatus = AuctionCrowdfundStatus.Busy; // Make sure the auction is not finalized. + IMarketWrapper market_ = market; uint256 auctionId_ = auctionId; - if (market.isFinalized(auctionId_)) { + if (market_.isFinalized(auctionId_)) { revert AuctionFinalizedError(auctionId_); } // Only bid if we are not already the highest bidder. - if (market.getCurrentHighestBidder(auctionId_) == address(this)) { + if (market_.getCurrentHighestBidder(auctionId_) == address(this)) { revert AlreadyHighestBidderError(); } // Get the minimum necessary bid to be the highest bidder. - uint96 bidAmount = market.getMinimumBid(auctionId_).safeCastUint256ToUint96(); + uint96 bidAmount = market_.getMinimumBid(auctionId_).safeCastUint256ToUint96(); // Prevent unaccounted ETH from being used to inflate the bid and // create "ghost shares" in voting power. uint96 totalContributions_ = totalContributions; @@ -215,14 +223,15 @@ contract AuctionCrowdfund is Crowdfund { revert ExceedsTotalContributionsError(bidAmount, totalContributions_); } // Make sure the bid is less than the maximum bid. - if (bidAmount > maximumBid) { - revert ExceedsMaximumBidError(bidAmount, maximumBid); + uint96 maximumBid_ = maximumBid; + if (bidAmount > maximumBid_) { + revert ExceedsMaximumBidError(bidAmount, maximumBid_); } lastBid = bidAmount; // No need to check that we have `bidAmount` since this will attempt to // transfer `bidAmount` ETH to the auction platform. - (bool s, bytes memory r) = address(market).delegatecall(abi.encodeCall( + (bool s, bytes memory r) = address(market_).delegatecall(abi.encodeCall( IMarketWrapper.bid, (auctionId_, bidAmount) )); @@ -253,23 +262,27 @@ contract AuctionCrowdfund is Crowdfund { // Mark as busy to prevent burn(), bid(), and contribute() // getting called because this will result in a `CrowdfundLifecycle.Busy`. _bidStatus = AuctionCrowdfundStatus.Busy; - - uint256 auctionId_ = auctionId; - // Finalize the auction if it isn't finalized. - if (!market.isFinalized(auctionId_)) { - // Note that even if this crowdfund has expired but the auction is still - // ongoing, this call can fail and block finalization until the auction ends. - (bool s, bytes memory r) = address(market).call(abi.encodeCall( - IMarketWrapper.finalize, - auctionId_ - )); - if (!s) { - r.rawRevert(); + { + uint256 auctionId_ = auctionId; + IMarketWrapper market_ = market; + // Finalize the auction if it isn't finalized. + if (!market_.isFinalized(auctionId_)) { + // Note that even if this crowdfund has expired but the auction is still + // ongoing, this call can fail and block finalization until the auction ends. + (bool s, bytes memory r) = address(market_).call(abi.encodeCall( + IMarketWrapper.finalize, + auctionId_ + )); + if (!s) { + r.rawRevert(); + } } } + IERC721 nftContract_ = nftContract; + uint256 nftTokenId_ = nftTokenId; if ( // Are we now in possession of the NFT? - nftContract.safeOwnerOf(nftTokenId) == address(this) && + nftContract_.safeOwnerOf(nftTokenId_) == address(this) && // And it wasn't acquired for free or "gifted" to us? address(this).balance < totalContributions ) { diff --git a/contracts/crowdfund/BuyCrowdfund.sol b/contracts/crowdfund/BuyCrowdfund.sol index 2ae5a8d7..583857ab 100644 --- a/contracts/crowdfund/BuyCrowdfund.sol +++ b/contracts/crowdfund/BuyCrowdfund.sol @@ -5,7 +5,6 @@ pragma solidity 0.8.17; import "../tokens/IERC721.sol"; import "../party/Party.sol"; import "../utils/LibSafeERC721.sol"; -import "../utils/LibRawResult.sol"; import "../globals/IGlobals.sol"; import "../gatekeepers/IGateKeeper.sol"; diff --git a/contracts/crowdfund/BuyCrowdfundBase.sol b/contracts/crowdfund/BuyCrowdfundBase.sol index 058f4248..735894c5 100644 --- a/contracts/crowdfund/BuyCrowdfundBase.sol +++ b/contracts/crowdfund/BuyCrowdfundBase.sol @@ -29,9 +29,8 @@ abstract contract BuyCrowdfundBase is Crowdfund { // How long this crowdfund has to bid on the NFT, in seconds. uint40 duration; // Maximum amount this crowdfund will pay for the NFT. - // If zero, no maximum. uint96 maximumPrice; - // An address that receieves an extra share of the final voting power + // An address that receives an extra share of the final voting power // when the party transitions into governance. address payable splitRecipient; // What percentage (in bps) of the final total voting power `splitRecipient` diff --git a/contracts/crowdfund/Crowdfund.sol b/contracts/crowdfund/Crowdfund.sol index 77537c60..a0323187 100644 --- a/contracts/crowdfund/Crowdfund.sol +++ b/contracts/crowdfund/Crowdfund.sol @@ -193,7 +193,7 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT { function batchBurn(address payable[] calldata contributors) external { Party party_ = party; CrowdfundLifecycle lc = getCrowdfundLifecycle(); - for (uint256 i = 0; i < contributors.length; ++i) { + for (uint256 i; i < contributors.length; ++i) { _burn(contributors[i], lc, party_); } } @@ -277,9 +277,9 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT { ) { CrowdfundLifecycle lc = getCrowdfundLifecycle(); - Contribution[] storage contributions = _contributionsByContributor[contributor]; + Contribution[] memory contributions = _contributionsByContributor[contributor]; uint256 numContributions = contributions.length; - for (uint256 i = 0; i < numContributions; ++i) { + for (uint256 i; i < numContributions; ++i) { ethContributed += contributions[i].amount; } if (lc == CrowdfundLifecycle.Won || lc == CrowdfundLifecycle.Lost) { @@ -288,7 +288,7 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT { } /// @notice Get the current lifecycle of the crowdfund. - function getCrowdfundLifecycle() public virtual view returns (CrowdfundLifecycle); + function getCrowdfundLifecycle() public virtual view returns (CrowdfundLifecycle lifecycle); // Get the final sale price of the bought assets. This will also be the total // voting power of the governance party. @@ -349,7 +349,7 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT { } // Can be called after a party has won. - // Deploys and initializes a a `Party` instance via the `PartyFactory` + // Deploys and initializes a `Party` instance via the `PartyFactory` // and transfers the bought NFT to it. // After calling this, anyone can burn CF tokens on a contributor's behalf // with the `burn()` function. @@ -393,7 +393,7 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT { preciousTokenIds ); // Transfer the acquired NFTs to the new party. - for (uint256 i = 0; i < preciousTokens.length; ++i) { + for (uint256 i; i < preciousTokens.length; ++i) { preciousTokens[i].transferFrom(address(this), address(party_), preciousTokenIds[i]); } } @@ -435,7 +435,7 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT { assembly { // Replace the address[] hosts field with its hash temporarily. let oldHostsFieldValue := mload(opts) - mstore(opts, keccak256(add(mload(opts), 0x20), mul(mload(mload(opts)), 32))) + mstore(opts, keccak256(add(oldHostsFieldValue, 0x20), mul(mload(oldHostsFieldValue), 32))) // Hash the entire struct. h := keccak256(opts, 0xC0) // Restore old hosts field value. @@ -450,9 +450,9 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT { { uint256 totalEthUsed = _getFinalPrice(); { - Contribution[] storage contributions = _contributionsByContributor[contributor]; + Contribution[] memory contributions = _contributionsByContributor[contributor]; uint256 numContributions = contributions.length; - for (uint256 i = 0; i < numContributions; ++i) { + for (uint256 i; i < numContributions; ++i) { Contribution memory c = contributions[i]; if (c.previousTotalContributions >= totalEthUsed) { // This entire contribution was not used. @@ -496,11 +496,12 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT { revert InvalidDelegateError(); } // Must not be blocked by gatekeeper. - if (gateKeeper != IGateKeeper(address(0))) { - if (!gateKeeper.isAllowed(contributor, gateKeeperId, gateData)) { + IGateKeeper _gateKeeper = gateKeeper; + if (_gateKeeper != IGateKeeper(address(0))) { + if (!_gateKeeper.isAllowed(contributor, gateKeeperId, gateData)) { revert NotAllowedByGateKeeperError( contributor, - gateKeeper, + _gateKeeper, gateKeeperId, gateData ); @@ -514,9 +515,6 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT { // OK to contribute with zero just to update delegate. if (amount != 0) { - // Increase total contributions. - totalContributions += amount; - // Only allow contributions while the crowdfund is active. { CrowdfundLifecycle lc = getCrowdfundLifecycle(); @@ -524,6 +522,8 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT { revert WrongLifecycleError(lc); } } + // Increase total contributions. + totalContributions += amount; // Create contributions entry for this contributor. Contribution[] storage contributions = _contributionsByContributor[contributor]; uint256 numContributions = contributions.length; @@ -562,15 +562,18 @@ abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT { revert WrongLifecycleError(lc); } // Split recipient can burn even if they don't have a token. - if (contributor == splitRecipient) { - if (_splitRecipientHasBurned) { - revert SplitRecipientAlreadyBurnedError(); + { + address splitRecipient_ = splitRecipient; + if (contributor == splitRecipient_) { + if (_splitRecipientHasBurned) { + revert SplitRecipientAlreadyBurnedError(); + } + _splitRecipientHasBurned = true; + } + // Revert if already burned or does not exist. + if (splitRecipient_ != contributor || _doesTokenExistFor(contributor)) { + CrowdfundNFT._burn(contributor); } - _splitRecipientHasBurned = true; - } - // Revert if already burned or does not exist. - if (splitRecipient != contributor || _doesTokenExistFor(contributor)) { - CrowdfundNFT._burn(contributor); } // Compute the contributions used and owed to the contributor, along // with the voting power they'll have in the governance stage. diff --git a/contracts/crowdfund/CrowdfundFactory.sol b/contracts/crowdfund/CrowdfundFactory.sol index cb9eded5..98944a64 100644 --- a/contracts/crowdfund/CrowdfundFactory.sol +++ b/contracts/crowdfund/CrowdfundFactory.sol @@ -28,7 +28,7 @@ contract CrowdfundFactory { _GLOBALS = globals; } - /// @notice Create a new crowdfund to purchases a specific NFT (i.e., with a + /// @notice Create a new crowdfund to purchase a specific NFT (i.e., with a /// known token ID) listing for a known price. /// @param opts Options used to initialize the crowdfund. These are fixed /// and cannot be changed later. @@ -80,7 +80,7 @@ contract CrowdfundFactory { emit AuctionCrowdfundCreated(inst, opts); } - /// @notice Create a new crowdfund to purchases any NFT from a collection + /// @notice Create a new crowdfund to purchase any NFT from a collection /// (i.e. any token ID) from a collection for a known price. /// @param opts Options used to initialize the crowdfund. These are fixed /// and cannot be changed later. diff --git a/contracts/crowdfund/CrowdfundNFT.sol b/contracts/crowdfund/CrowdfundNFT.sol index 9deae2b2..7b171449 100644 --- a/contracts/crowdfund/CrowdfundNFT.sol +++ b/contracts/crowdfund/CrowdfundNFT.sol @@ -11,6 +11,7 @@ import "../renderers/RendererStorage.sol"; /// @notice NFT functionality for crowdfund types. This NFT is soulbound and read-only. contract CrowdfundNFT is IERC721, EIP165, ReadOnlyDelegateCall { + error AlreadyMintedError(address owner, uint256 tokenId); error AlreadyBurnedError(address owner, uint256 tokenId); error InvalidTokenError(uint256 tokenId); error InvalidAddressError(); @@ -59,6 +60,7 @@ contract CrowdfundNFT is IERC721, EIP165, ReadOnlyDelegateCall { /// Attempting to call this function will always fail. function transferFrom(address, address, uint256) external + pure alwaysRevert {} @@ -66,6 +68,7 @@ contract CrowdfundNFT is IERC721, EIP165, ReadOnlyDelegateCall { /// Attempting to call this function will always fail. function safeTransferFrom(address, address, uint256) external + pure alwaysRevert {} @@ -73,6 +76,7 @@ contract CrowdfundNFT is IERC721, EIP165, ReadOnlyDelegateCall { /// Attempting to call this function will always fail. function safeTransferFrom(address, address, uint256, bytes calldata) external + pure alwaysRevert {} @@ -80,6 +84,7 @@ contract CrowdfundNFT is IERC721, EIP165, ReadOnlyDelegateCall { /// Attempting to call this function will always fail. function approve(address, uint256) external + pure alwaysRevert {} @@ -87,6 +92,7 @@ contract CrowdfundNFT is IERC721, EIP165, ReadOnlyDelegateCall { /// Attempting to call this function will always fail. function setApprovalForAll(address, bool) external + pure alwaysRevert {} @@ -155,6 +161,8 @@ contract CrowdfundNFT is IERC721, EIP165, ReadOnlyDelegateCall { if (_owners[tokenId] != owner) { _owners[tokenId] = owner; emit Transfer(address(0), owner, tokenId); + } else { + revert AlreadyMintedError(owner, tokenId); } } diff --git a/contracts/distribution/ITokenDistributor.sol b/contracts/distribution/ITokenDistributor.sol index 411dab4d..e2b90acf 100644 --- a/contracts/distribution/ITokenDistributor.sol +++ b/contracts/distribution/ITokenDistributor.sol @@ -59,7 +59,7 @@ interface ITokenDistributor { /// @param party The party whose members can claim the distribution. /// @param feeRecipient Who can claim `fee`. /// @param feeBps Percentage (in bps) of the distribution `feeRecipient` receives. - /// @param info Information on the created distribution. + /// @return info Information on the created distribution. function createNativeDistribution( ITokenDistributorParty party, address payable feeRecipient, @@ -78,7 +78,7 @@ interface ITokenDistributor { /// @param party The party whose members can claim the distribution. /// @param feeRecipient Who can claim `fee`. /// @param feeBps Percentage (in bps) of the distribution `feeRecipient` receives. - /// @param info Information on the created distribution. + /// @return info Information on the created distribution. function createErc20Distribution( IERC20 token, ITokenDistributorParty party, @@ -93,7 +93,7 @@ interface ITokenDistributor { /// must own this token. /// @param info Information on the distribution being claimed. /// @param partyTokenId The ID of the party token to claim for. - /// @param amountClaimed The amount of the distribution claimed. + /// @return amountClaimed The amount of the distribution claimed. function claim(DistributionInfo calldata info, uint256 partyTokenId) external returns (uint128 amountClaimed); @@ -108,7 +108,7 @@ interface ITokenDistributor { /// @notice Batch version of `claim()`. /// @param infos Information on the distributions being claimed. /// @param partyTokenIds The ID of the party tokens to claim for. - /// @param amountsClaimed The amount of the distributions claimed. + /// @return amountsClaimed The amount of the distributions claimed. function batchClaim(DistributionInfo[] calldata infos, uint256[] calldata partyTokenIds) external returns (uint128[] memory amountsClaimed); diff --git a/contracts/distribution/TokenDistributor.sol b/contracts/distribution/TokenDistributor.sol index 64ec547e..66d80fae 100644 --- a/contracts/distribution/TokenDistributor.sol +++ b/contracts/distribution/TokenDistributor.sol @@ -41,8 +41,9 @@ contract TokenDistributor is ITokenDistributor { uint16 feeBps; } + event EmergencyExecute(address target, bytes data); + error OnlyPartyDaoError(address notDao, address partyDao); - error OnlyPartyDaoAuthorityError(address notDaoAuthority); error InvalidDistributionInfoError(DistributionInfo info); error DistributionAlreadyClaimedByPartyTokenError(uint256 distributionId, uint256 partyTokenId); error DistributionFeeAlreadyClaimedError(uint256 distributionId); @@ -315,6 +316,7 @@ contract TokenDistributor is ITokenDistributor { if (!success) { res.rawRevert(); } + emit EmergencyExecute(targetAddress, targetCallData); } function _createDistribution(CreateDistributionArgs memory args) diff --git a/contracts/globals/Globals.sol b/contracts/globals/Globals.sol index 63a56382..75f65656 100644 --- a/contracts/globals/Globals.sol +++ b/contracts/globals/Globals.sol @@ -7,12 +7,14 @@ import "./IGlobals.sol"; /// @notice Contract storing global configuration values. contract Globals is IGlobals { address public multiSig; + address public pendingMultiSig; // key -> word value mapping(uint256 => bytes32) private _wordValues; // key -> word value -> isIncluded mapping(uint256 => mapping(bytes32 => bool)) private _includedWordValues; error OnlyMultiSigError(); + error OnlyPendingMultiSigError(); error InvalidBooleanValueError(uint256 key, uint256 value); modifier onlyMultisig() { @@ -22,12 +24,23 @@ contract Globals is IGlobals { _; } + modifier onlyPendingMultisig() { + if (msg.sender != pendingMultiSig) { + revert OnlyPendingMultiSigError(); + } + _; + } constructor(address multiSig_) { multiSig = multiSig_; } function transferMultiSig(address newMultiSig) external onlyMultisig { - multiSig = newMultiSig; + pendingMultiSig = newMultiSig; + } + + function acceptMultiSig() external onlyPendingMultisig { + multiSig = pendingMultiSig; + delete pendingMultiSig; } function getBytes32(uint256 key) external view returns (bytes32) { diff --git a/contracts/party/PartyFactory.sol b/contracts/party/PartyFactory.sol index 15d3f404..0c62030e 100644 --- a/contracts/party/PartyFactory.sol +++ b/contracts/party/PartyFactory.sol @@ -11,7 +11,7 @@ import "../renderers/RendererStorage.sol"; import "./Party.sol"; import "./IPartyFactory.sol"; -/// @notice Factory used to deploys new proxified `Party` instances. +/// @notice Factory used to deploy new proxified `Party` instances. contract PartyFactory is IPartyFactory { error InvalidAuthorityError(address authority); diff --git a/contracts/party/PartyGovernance.sol b/contracts/party/PartyGovernance.sol index 8fe18399..0cc75f51 100644 --- a/contracts/party/PartyGovernance.sol +++ b/contracts/party/PartyGovernance.sol @@ -159,6 +159,7 @@ abstract contract PartyGovernance is address voter, uint256 weight ); + event EmergencyExecute(address target, bytes data, uint256 amountEth); event ProposalPassed(uint256 indexed proposalId); event ProposalVetoed(uint256 indexed proposalId, address host); @@ -167,12 +168,11 @@ abstract contract PartyGovernance is event DistributionCreated(ITokenDistributor.TokenType tokenType, address token, uint256 tokenId); event VotingPowerDelegated(address indexed owner, address indexed delegate); event HostStatusTransferred(address oldHost, address newHost); + event EmergencyExecuteDisabled(); error MismatchedPreciousListLengths(); error BadProposalStatusError(ProposalStatus status); - error ProposalExistsError(uint256 proposalId); error BadProposalHashError(bytes32 proposalHash, bytes32 actualHash); - error ProposalHasNoVotesError(uint256 proposalId); error ExecutionTimeExceededError(uint40 maxExecutableTime, uint40 timestamp); error OnlyPartyHostError(); error OnlyActiveMemberError(); @@ -188,7 +188,7 @@ abstract contract PartyGovernance is error InvalidBpsError(uint16 bps); uint256 constant private UINT40_HIGH_BIT = 1 << 39; - uint96 constant private VETO_VALUE = uint96(int96(-1)); + uint96 constant private VETO_VALUE = type(uint96).max; // The `Globals` contract storing global configuration values. This contract // is immutable and it’s address will never change. @@ -520,9 +520,11 @@ abstract contract PartyGovernance is ); emit DistributionCreated(tokenType, token, tokenId); // Create a native token distribution. + address payable feeRecipient_ = feeRecipient; + uint16 feeBps_ = feeBps; if (tokenType == ITokenDistributor.TokenType.Native) { return distributor.createNativeDistribution - { value: address(this).balance }(this, feeRecipient, feeBps); + { value: address(this).balance }(this, feeRecipient_, feeBps_); } // Otherwise must be an ERC20 token distribution. assert(tokenType == ITokenDistributor.TokenType.Erc20); @@ -533,8 +535,8 @@ abstract contract PartyGovernance is return distributor.createErc20Distribution( IERC20(token), this, - feeRecipient, - feeBps + feeRecipient_, + feeBps_ ); } @@ -735,13 +737,12 @@ abstract contract PartyGovernance is } /// @notice Cancel a (probably stuck) InProgress proposal. - /// @dev proposal.cancelDelay seconds must have passed since it was first - /// executed for this to be valid. - /// The currently active proposal will simply be yeeted out of existence - /// so another proposal can execute. - /// This is intended to be a last resort and can leave the party - /// in a broken state. Whenever possible, active proposals should be - /// allowed to complete their lifecycle. + /// @dev `proposal.cancelDelay` seconds must have passed since it was first + /// executed for this to be valid. The currently active proposal will + /// simply be yeeted out of existence so another proposal can execute. + /// This is intended to be a last resort and can leave the party in a + /// broken state. Whenever possible, active proposals should be + /// allowed to complete their lifecycle. /// @param proposalId The ID of the proposal to cancel. /// @param proposal The details of the proposal to cancel. function cancel(uint256 proposalId, Proposal calldata proposal) @@ -819,12 +820,14 @@ abstract contract PartyGovernance is if (!success) { res.rawRevert(); } + emit EmergencyExecute(targetAddress, targetCallData, amountEth); } /// @notice Revoke the DAO's ability to call emergencyExecute(). /// @dev Either the DAO or the party host can call this. function disableEmergencyExecute() external onlyPartyDaoOrHost onlyDelegateCall { emergencyExecuteDisabled = true; + emit EmergencyExecuteDisabled(); } function _executeProposal( @@ -919,8 +922,8 @@ abstract contract PartyGovernance is VotingPowerSnapshot memory oldSnap = _getLastVotingPowerSnapshotForVoter(voter); address oldDelegate = delegationsByVoter[voter]; - // If `oldDelegate` is zero, `voter` never delegated, set the it to - // themself. + // If `oldDelegate` is zero and `voter` never delegated, then have + // `voter` delegate to themself. oldDelegate = oldDelegate == address(0) ? voter : oldDelegate; // If the new `delegate` is zero, use the current (old) delegate. delegate = delegate == address(0) ? oldDelegate : delegate; @@ -1056,7 +1059,7 @@ abstract contract PartyGovernance is return ProposalStatus.Complete; } // Vetoed. - if (pv.votes == uint96(int96(-1))) { + if (pv.votes == type(uint96).max) { return ProposalStatus.Defeated; } uint40 t = uint40(block.timestamp); diff --git a/contracts/party/PartyGovernanceNFT.sol b/contracts/party/PartyGovernanceNFT.sol index 2c52b876..ded4e403 100644 --- a/contracts/party/PartyGovernanceNFT.sol +++ b/contracts/party/PartyGovernanceNFT.sol @@ -35,8 +35,9 @@ contract PartyGovernanceNFT is mapping (uint256 => uint256) public votingPowerByTokenId; modifier onlyMinter() { - if (msg.sender != mintAuthority) { - revert OnlyMintAuthorityError(msg.sender, mintAuthority); + address minter = mintAuthority; + if (msg.sender != minter) { + revert OnlyMintAuthorityError(msg.sender, minter); } _; } diff --git a/contracts/proposals/ArbitraryCallsProposal.sol b/contracts/proposals/ArbitraryCallsProposal.sol index 7971876d..8ced652b 100644 --- a/contracts/proposals/ArbitraryCallsProposal.sol +++ b/contracts/proposals/ArbitraryCallsProposal.sol @@ -54,7 +54,7 @@ contract ArbitraryCallsProposal { // so we can check that we still have them later. bool[] memory hadPreciouses = new bool[](params.preciousTokenIds.length); if (!isUnanimous) { - for (uint256 i = 0; i < hadPreciouses.length; ++i) { + for (uint256 i; i < hadPreciouses.length; ++i) { hadPreciouses[i] = _getHasPrecious( params.preciousTokens[i], params.preciousTokenIds[i] @@ -63,7 +63,7 @@ contract ArbitraryCallsProposal { } // Can only forward ETH attached to the call. uint256 ethAvailable = msg.value; - for (uint256 i = 0; i < calls.length; ++i) { + for (uint256 i; i < calls.length; ++i) { // Execute an arbitrary call. _executeSingleArbitraryCall( i, @@ -80,7 +80,7 @@ contract ArbitraryCallsProposal { // If not a unanimous vote and we had a precious beforehand, // ensure that we still have it now. if (!isUnanimous) { - for (uint256 i = 0; i < hadPreciouses.length; ++i) { + for (uint256 i; i < hadPreciouses.length; ++i) { if (hadPreciouses[i]) { if (!_getHasPrecious(params.preciousTokens[i], params.preciousTokenIds[i])) { revert PreciousLostError( diff --git a/contracts/proposals/LibProposal.sol b/contracts/proposals/LibProposal.sol index c55a0e9c..95cdbba0 100644 --- a/contracts/proposals/LibProposal.sol +++ b/contracts/proposals/LibProposal.sol @@ -12,7 +12,7 @@ library LibProposal { pure returns (bool) { - for (uint256 i = 0; i < preciousTokens.length; ++i) { + for (uint256 i; i < preciousTokens.length; ++i) { if (token == preciousTokens[i]) { return true; } @@ -30,7 +30,7 @@ library LibProposal { pure returns (bool) { - for (uint256 i = 0; i < preciousTokens.length; ++i) { + for (uint256 i; i < preciousTokens.length; ++i) { if (token == preciousTokens[i] && tokenId == preciousTokenIds[i]) { return true; } diff --git a/contracts/proposals/ListOnOpenseaProposal.sol b/contracts/proposals/ListOnOpenseaProposal.sol index 0614ced0..3b8b33c0 100644 --- a/contracts/proposals/ListOnOpenseaProposal.sol +++ b/contracts/proposals/ListOnOpenseaProposal.sol @@ -290,7 +290,7 @@ abstract contract ListOnOpenseaProposal is ZoraHelpers { cons.identifierOrCriteria = 0; cons.startAmount = cons.endAmount = listPrice; cons.recipient = payable(address(this)); - for (uint256 i = 0; i < fees.length; ++i) { + for (uint256 i; i < fees.length; ++i) { cons = orderParams.consideration[1 + i]; cons.itemType = IOpenseaExchange.ItemType.NATIVE; cons.token = address(0); diff --git a/contracts/proposals/ListOnZoraProposal.sol b/contracts/proposals/ListOnZoraProposal.sol index a89dd0a6..ce9d1117 100644 --- a/contracts/proposals/ListOnZoraProposal.sol +++ b/contracts/proposals/ListOnZoraProposal.sol @@ -156,7 +156,7 @@ contract ListOnZoraProposal is ZoraHelpers { token, tokenId, listPrice, - uint40(duration), + duration, uint40(block.timestamp + timeout) ); } diff --git a/contracts/proposals/ProposalExecutionEngine.sol b/contracts/proposals/ProposalExecutionEngine.sol index c2ffe8ba..5be882da 100644 --- a/contracts/proposals/ProposalExecutionEngine.sol +++ b/contracts/proposals/ProposalExecutionEngine.sol @@ -11,7 +11,6 @@ import "./ListOnOpenseaProposal.sol"; import "./ListOnZoraProposal.sol"; import "./FractionalizeProposal.sol"; import "./ArbitraryCallsProposal.sol"; -import "./LibProposal.sol"; import "./ProposalStorage.sol"; /// @notice Upgradable implementation of proposal execution logic for parties that use it. @@ -39,9 +38,7 @@ contract ProposalExecutionEngine is ListOnZora, Fractionalize, ArbitraryCalls, - UpgradeProposalEngineImpl, - // Append new proposal types here. - NumProposalTypes + UpgradeProposalEngineImpl } // Explicit storage bucket for "private" state owned by the `ProposalExecutionEngine`. @@ -245,7 +242,7 @@ contract ProposalExecutionEngine is offsetProposalData := add(proposalData, 4) } require(proposalType != ProposalType.Invalid); - require(uint8(proposalType) < uint8(ProposalType.NumProposalTypes)); + require(uint8(proposalType) <= uint8(type(ProposalType).max)); } // Upgrade implementation to the latest version. diff --git a/contracts/proposals/ProposalStorage.sol b/contracts/proposals/ProposalStorage.sol index d038392c..3c224682 100644 --- a/contracts/proposals/ProposalStorage.sol +++ b/contracts/proposals/ProposalStorage.sol @@ -4,12 +4,11 @@ pragma solidity 0.8.17; import "./IProposalExecutionEngine.sol"; import "../utils/LibRawResult.sol"; -import "../tokens/IERC721.sol"; // The storage bucket shared by `PartyGovernance` and the `ProposalExecutionEngine`. // Read this for more context on the pattern motivating this: // https://github.com/dragonfly-xyz/useful-solidity-patterns/tree/main/patterns/explicit-storage-buckets -contract ProposalStorage { +abstract contract ProposalStorage { using LibRawResult for bytes; struct SharedProposalStorage { diff --git a/contracts/tokens/ERC1155Receiver.sol b/contracts/tokens/ERC1155Receiver.sol index 3289d473..bb4f9cd0 100644 --- a/contracts/tokens/ERC1155Receiver.sol +++ b/contracts/tokens/ERC1155Receiver.sol @@ -4,8 +4,7 @@ pragma solidity ^0.8; import "../vendor/solmate/ERC1155.sol"; import "../utils/EIP165.sol"; -contract ERC1155Receiver is EIP165, ERC1155TokenReceiverBase { - +abstract contract ERC1155Receiver is EIP165, ERC1155TokenReceiverBase { /// @inheritdoc EIP165 function supportsInterface(bytes4 interfaceId) public diff --git a/contracts/tokens/ERC721Receiver.sol b/contracts/tokens/ERC721Receiver.sol index cf8f1f95..a5b42d45 100644 --- a/contracts/tokens/ERC721Receiver.sol +++ b/contracts/tokens/ERC721Receiver.sol @@ -8,8 +8,7 @@ import "../vendor/solmate/ERC721.sol"; /// @notice Mixin for contracts that want to receive ERC721 tokens. /// @dev Use this instead of solmate's ERC721TokenReceiver because the /// compiler has issues when overriding EIP165/IERC721Receiver functions. -contract ERC721Receiver is IERC721Receiver, EIP165, ERC721TokenReceiver { - +abstract contract ERC721Receiver is IERC721Receiver, EIP165, ERC721TokenReceiver { /// @inheritdoc IERC721Receiver function onERC721Received(address, address, uint256, bytes memory) public diff --git a/contracts/utils/EIP165.sol b/contracts/utils/EIP165.sol index 633c3e62..9a008c86 100644 --- a/contracts/utils/EIP165.sol +++ b/contracts/utils/EIP165.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity 0.8.17; -contract EIP165 { +abstract contract EIP165 { /// @notice Query if a contract implements an interface. /// @param interfaceId The interface identifier, as specified in ERC-165 /// @return `true` if the contract implements `interfaceId` and diff --git a/contracts/utils/LibAddress.sol b/contracts/utils/LibAddress.sol index 08d5c8d6..ed4f27b4 100644 --- a/contracts/utils/LibAddress.sol +++ b/contracts/utils/LibAddress.sol @@ -8,6 +8,8 @@ library LibAddress { function transferEth(address payable receiver, uint256 amount) internal { + if (amount == 0) return; + (bool s, bytes memory r) = receiver.call{value: amount}(""); if (!s) { revert EthTransferFailed(receiver, r); diff --git a/contracts/utils/PartyHelpers.sol b/contracts/utils/PartyHelpers.sol index 7c2e98cc..79fa5ec5 100644 --- a/contracts/utils/PartyHelpers.sol +++ b/contracts/utils/PartyHelpers.sol @@ -62,7 +62,7 @@ contract PartyHelpers { { Party p = Party(payable(party)); membersAndDelegates = new MemberAndDelegate[](members.length); - for (uint256 i = 0; i < members.length; i++) { + for (uint256 i; i < members.length; ++i) { membersAndDelegates[i] = MemberAndDelegate({ member: members[i], delegate: p.delegationsByVoter(members[i]) @@ -83,7 +83,7 @@ contract PartyHelpers { { Party p = Party(payable(party)); memberAndVotingPower = new MemberAndVotingPower[](voters.length); - for (uint256 i = 0; i < voters.length; i++) { + for (uint256 i; i < voters.length; ++i) { memberAndVotingPower[i] = MemberAndVotingPower({ member: voters[i], votingPower: p.getVotingPowerAt(voters[i], timestamp, indexes[i]) @@ -113,7 +113,7 @@ contract PartyHelpers { nftInfos = new NftInfo[](count); - for (uint256 i = 0; i < count; i++) { + for (uint256 i; i < count; ++i) { uint256 currIndex = startTokenId + i; address owner = p.ownerOf(currIndex); uint256 intrinsicVotingPower = p.votingPowerByTokenId(currIndex); diff --git a/contracts/utils/ReadOnlyDelegateCall.sol b/contracts/utils/ReadOnlyDelegateCall.sol index e2ab2879..332ea26c 100644 --- a/contracts/utils/ReadOnlyDelegateCall.sol +++ b/contracts/utils/ReadOnlyDelegateCall.sol @@ -10,8 +10,8 @@ interface IReadOnlyDelegateCall { view; } -// Inherited by contracts to performs read-only delegate calls. -contract ReadOnlyDelegateCall { +// Inherited by contracts to perform read-only delegate calls. +abstract contract ReadOnlyDelegateCall { using LibRawResult for bytes; // Delegatecall into implement and revert with the raw result. diff --git a/contracts/vendor/solmate/ERC1155.sol b/contracts/vendor/solmate/ERC1155.sol index dd6a0f2b..5dc82cf0 100644 --- a/contracts/vendor/solmate/ERC1155.sol +++ b/contracts/vendor/solmate/ERC1155.sol @@ -77,7 +77,7 @@ abstract contract ERC1155 is IERC1155 { uint256 id; uint256 amount; - for (uint256 i = 0; i < ids.length; ) { + for (uint256 i; i < ids.length; ) { id = ids[i]; amount = amounts[i]; @@ -115,7 +115,7 @@ abstract contract ERC1155 is IERC1155 { // Unchecked because the only math done is incrementing // the array index counter which cannot possibly overflow. unchecked { - for (uint256 i = 0; i < owners.length; ++i) { + for (uint256 i; i < owners.length; ++i) { balances[i] = balanceOf[owners[i]][ids[i]]; } } @@ -165,7 +165,7 @@ abstract contract ERC1155 is IERC1155 { require(idsLength == amounts.length, "LENGTH_MISMATCH"); - for (uint256 i = 0; i < idsLength; ) { + for (uint256 i; i < idsLength; ) { balanceOf[to][ids[i]] += amounts[i]; // An array can't have a total length @@ -195,7 +195,7 @@ abstract contract ERC1155 is IERC1155 { require(idsLength == amounts.length, "LENGTH_MISMATCH"); - for (uint256 i = 0; i < idsLength; ) { + for (uint256 i; i < idsLength; ) { balanceOf[from][ids[i]] -= amounts[i]; // An array can't have a total length diff --git a/deploy/deploy.sol b/deploy/deploy.sol index 61fac2d7..50a6a872 100644 --- a/deploy/deploy.sol +++ b/deploy/deploy.sol @@ -549,7 +549,7 @@ contract DeployScript is Script, Deploy { returns (string memory) { string memory vals = ""; - for (uint256 i = 0; i < parts.length; ++i) { + for (uint256 i; i < parts.length; ++i) { string memory newValue = string.concat( '"', parts[i].key, diff --git a/docs/governance.md b/docs/governance.md index 99f53aa3..de267d98 100644 --- a/docs/governance.md +++ b/docs/governance.md @@ -294,7 +294,7 @@ abi.encodeWithSelector( This proposal is atomic, always completing in one step/execute. - Each call is executed in the order declared. -- ETH to attach to each call must be provided by the caller of the `Party.execute()` call. If the the sum of all successful calls try to consume more than `msg.value`, the entire proposal will revert. +- ETH to attach to each call must be provided by the caller of the `Party.execute()` call. If the sum of all successful calls try to consume more than `msg.value`, the entire proposal will revert. - If a call has a non-zero `expectedResultHash` then the result of the call will be hashed and matched against this value. If they do not match, then the entire proposal will revert. - If the call is to the `Party` itself, the entire proposal will revert. - If the call is to the `IERC721.onERC721Received()` function, the entire proposal will revert. diff --git a/sol-tests/crowdfund/BuyCrowdfund.t.sol b/sol-tests/crowdfund/BuyCrowdfund.t.sol index e9f0e7fa..ac2ba48b 100644 --- a/sol-tests/crowdfund/BuyCrowdfund.t.sol +++ b/sol-tests/crowdfund/BuyCrowdfund.t.sol @@ -442,7 +442,7 @@ contract BuyCrowdfundTest is Test, TestUtils { assertTrue(cf.getCrowdfundLifecycle() == Crowdfund.CrowdfundLifecycle.Active); } - function testGettingNFTForFreeTriggersLostToRefund() public { + function testGettingNFTForFreeTriggersLossToRefund() public { DummyERC721 token = erc721Vault.token(); uint256 tokenId = 1; // Create a BuyCrowdfund instance. diff --git a/sol-tests/crowdfund/Crowdfund.t.sol b/sol-tests/crowdfund/Crowdfund.t.sol index 0c62cb77..4895fdc2 100644 --- a/sol-tests/crowdfund/Crowdfund.t.sol +++ b/sol-tests/crowdfund/Crowdfund.t.sol @@ -105,7 +105,7 @@ contract CrowdfundTest is Test, TestUtils { { tokens = new IERC721[](count); tokenIds = new uint256[](count); - for (uint256 i = 0; i < count; ++i) { + for (uint256 i; i < count; ++i) { DummyERC721 t = new DummyERC721(); tokens[i] = IERC721(t); tokenIds[i] = t.mint(owner); diff --git a/sol-tests/distribution/TokenDistributor.t.sol b/sol-tests/distribution/TokenDistributor.t.sol index bd88f7f1..519c4f0e 100644 --- a/sol-tests/distribution/TokenDistributor.t.sol +++ b/sol-tests/distribution/TokenDistributor.t.sol @@ -29,7 +29,7 @@ contract TokenDistributorTest is Test, TestUtils { distributor = new TokenDistributor(globals, uint40(block.timestamp) + 365 days); // Reset addresses used in tests (can be non-zero when running forked tests) - for (uint160 i; i < 10; i++) { + for (uint160 i; i < 10; ++i) { vm.deal(address(i), 0); } } diff --git a/sol-tests/distribution/TokenDistributorUnit.t.sol b/sol-tests/distribution/TokenDistributorUnit.t.sol index fcf7d44b..406cee8d 100644 --- a/sol-tests/distribution/TokenDistributorUnit.t.sol +++ b/sol-tests/distribution/TokenDistributorUnit.t.sol @@ -395,7 +395,7 @@ contract TokenDistributorUnitTest is Test, TestUtils { // Deal more ETH to the distributor to attempt to steal. vm.deal(address(distributor), address(distributor).balance + supply); uint256 balanceBeforeClaim = address(distributor).balance; - for (uint256 i = 0; i < members.length; ++i) { + for (uint256 i; i < members.length; ++i) { vm.prank(members[i]); distributor.claim(di, memberTokenIds[i]); } @@ -414,7 +414,7 @@ contract TokenDistributorUnitTest is Test, TestUtils { distributor.createNativeDistribution(party, payable(0), 0); assertEq(di.memberSupply, supply); // Claim. - for (uint256 i = 0; i < members.length; ++i) { + for (uint256 i; i < members.length; ++i) { vm.prank(members[i]); distributor.claim(di, memberTokenIds[i]); } @@ -476,7 +476,7 @@ contract TokenDistributorUnitTest is Test, TestUtils { assertEq(abi.encode(di).length / 32, 7); // Try replacing each field and claiming. - for (uint256 i = 0; i < 8; ++i) { + for (uint256 i; i < 8; ++i) { ITokenDistributor.DistributionInfo memory di_ = di; if (i == 0) { di_.tokenType = ITokenDistributor.TokenType.Erc20; @@ -710,13 +710,13 @@ contract TokenDistributorUnitTest is Test, TestUtils { tokenIds = new uint256[](count); shares = new uint256[](count); uint256 sharesSum = 0; - for (uint256 i = 0; i < count; ++i) { + for (uint256 i; i < count; ++i) { sharesSum += shares[i] = _randomUint256() % 1e18; } - for (uint256 i = 0; i < count; ++i) { + for (uint256 i; i < count; ++i) { shares[i] = shares[i] * total / sharesSum; } - for (uint256 i = 0; i < count; ++i) { + for (uint256 i; i < count; ++i) { owners[i] = _randomAddress(); tokenIds[i] = _randomUint256(); party.mintShare(owners[i], tokenIds[i], shares[i]); diff --git a/sol-tests/gatekeepers/AllowListGateKeeper.t.sol b/sol-tests/gatekeepers/AllowListGateKeeper.t.sol index f1f4fdb6..74cf9469 100644 --- a/sol-tests/gatekeepers/AllowListGateKeeper.t.sol +++ b/sol-tests/gatekeepers/AllowListGateKeeper.t.sol @@ -11,7 +11,7 @@ contract AllowListGateKeeperTest is Test, TestUtils { // Generates a randomized 4-member allow list. function _randomAllowList() private view returns (address[4] memory allowList) { - for (uint i = 0; i < 4; i++) { + for (uint i = 0; i < 4; ++i) { allowList[i] = _randomAddress(); } } diff --git a/sol-tests/party/PartyFactory.t.sol b/sol-tests/party/PartyFactory.t.sol index 4f799fde..93a9cf79 100644 --- a/sol-tests/party/PartyFactory.t.sol +++ b/sol-tests/party/PartyFactory.t.sol @@ -44,7 +44,7 @@ contract PartyFactoryTest is Test, TestUtils { { preciousTokens = new IERC721[](count); preciousTokenIds = new uint256[](count); - for (uint256 i = 0; i < count; ++i) { + for (uint256 i; i < count; ++i) { preciousTokens[i] = IERC721(_randomAddress()); preciousTokenIds[i] = _randomUint256(); } diff --git a/sol-tests/party/PartyGovernanceUnit.t.sol b/sol-tests/party/PartyGovernanceUnit.t.sol index 4b3b62fa..1cf75001 100644 --- a/sol-tests/party/PartyGovernanceUnit.t.sol +++ b/sol-tests/party/PartyGovernanceUnit.t.sol @@ -319,7 +319,7 @@ contract PartyGovernanceUnitTest is Test, TestUtils { { tokens = new IERC721[](count); tokenIds = new uint256[](count); - for (uint256 i = 0; i < count; ++i) { + for (uint256 i; i < count; ++i) { // Doesn't actually have to be real tokens for these tests. tokens[i] = IERC721(_randomAddress()); tokenIds[i] = _randomUint256(); diff --git a/sol-tests/proposals/ArbitraryCallsProposal.t.sol b/sol-tests/proposals/ArbitraryCallsProposal.t.sol index ae1516bd..2653c958 100644 --- a/sol-tests/proposals/ArbitraryCallsProposal.t.sol +++ b/sol-tests/proposals/ArbitraryCallsProposal.t.sol @@ -85,7 +85,7 @@ contract ArbitraryCallsProposalTest is uint256[] preciousTokenIds; constructor() { - for (uint256 i = 0; i < 2; ++i) { + for (uint256 i; i < 2; ++i) { DummyERC721 t = new DummyERC721(); preciousTokens.push(t); preciousTokenIds.push(t.mint(address(testContract))); @@ -135,7 +135,7 @@ contract ArbitraryCallsProposalTest is new ArbitraryCallsProposal.ArbitraryCall[](count); callArgs = new bytes32[](count); bytes[] memory callResults = new bytes[](count); - for (uint256 i = 0; i < count; ++i) { + for (uint256 i; i < count; ++i) { callArgs[i] = _randomBytes32(); callResults[i] = shouldCallsReturnData ? abi.encode(_randomBytes32()) : bytes(''); @@ -159,7 +159,7 @@ contract ArbitraryCallsProposalTest is ) = _createSimpleCalls(1, false); IProposalExecutionEngine.ExecuteProposalParams memory prop = _createTestProposal(calls); - for (uint256 i = 0; i < calls.length; ++i) { + for (uint256 i; i < calls.length; ++i) { _expectNonIndexedEmit(); emit ArbitraryCallTargetSuccessCalled(address(testContract), 0, callArgs[i]); _expectNonIndexedEmit(); @@ -175,7 +175,7 @@ contract ArbitraryCallsProposalTest is ) = _createSimpleCalls(2, false); IProposalExecutionEngine.ExecuteProposalParams memory prop = _createTestProposal(calls); - for (uint256 i = 0; i < calls.length; ++i) { + for (uint256 i; i < calls.length; ++i) { _expectNonIndexedEmit(); emit ArbitraryCallTargetSuccessCalled(address(testContract), 0, callArgs[i]); _expectNonIndexedEmit(); @@ -191,7 +191,7 @@ contract ArbitraryCallsProposalTest is ) = _createSimpleCalls(1, true); IProposalExecutionEngine.ExecuteProposalParams memory prop = _createTestProposal(calls); - for (uint256 i = 0; i < calls.length; ++i) { + for (uint256 i; i < calls.length; ++i) { _expectNonIndexedEmit(); emit ArbitraryCallTargetSuccessCalled(address(testContract), 0, callArgs[i]); _expectNonIndexedEmit(); @@ -225,7 +225,7 @@ contract ArbitraryCallsProposalTest is calls[0].value = 1e18; IProposalExecutionEngine.ExecuteProposalParams memory prop = _createTestProposal(calls); - for (uint256 i = 0; i < calls.length; ++i) { + for (uint256 i; i < calls.length; ++i) { _expectNonIndexedEmit(); emit ArbitraryCallTargetSuccessCalled(address(testContract), calls[i].value, callArgs[i]); _expectNonIndexedEmit(); @@ -243,7 +243,7 @@ contract ArbitraryCallsProposalTest is calls[1].value = 0.5e18; IProposalExecutionEngine.ExecuteProposalParams memory prop = _createTestProposal(calls); - for (uint256 i = 0; i < calls.length; ++i) { + for (uint256 i; i < calls.length; ++i) { _expectNonIndexedEmit(); emit ArbitraryCallTargetSuccessCalled(address(testContract), calls[i].value, callArgs[i]); _expectNonIndexedEmit(); @@ -260,7 +260,7 @@ contract ArbitraryCallsProposalTest is calls[0].value = 1e18; IProposalExecutionEngine.ExecuteProposalParams memory prop = _createTestProposal(calls); - for (uint256 i = 0; i < calls.length; ++i) { + for (uint256 i; i < calls.length; ++i) { _expectNonIndexedEmit(); emit ArbitraryCallTargetSuccessCalled(address(testContract), calls[i].value, callArgs[i]); _expectNonIndexedEmit(); diff --git a/sol-tests/proposals/ListOnOpenseaProposalForked.t.sol b/sol-tests/proposals/ListOnOpenseaProposalForked.t.sol index acd61c34..316f2ba1 100644 --- a/sol-tests/proposals/ListOnOpenseaProposalForked.t.sol +++ b/sol-tests/proposals/ListOnOpenseaProposalForked.t.sol @@ -110,7 +110,7 @@ contract ListOnOpenseaProposalForkedTest is { tokens = new IERC721[](count); tokenIds = new uint256[](count); - for (uint256 i = 0; i < count; ++i) { + for (uint256 i; i < count; ++i) { DummyERC721 t = new DummyERC721(); tokens[i] = t; tokenIds[i] = t.mint(owner); @@ -188,7 +188,7 @@ contract ListOnOpenseaProposalForkedTest is cons.identifierOrCriteria = 0; cons.startAmount = cons.endAmount = data.listPrice; cons.recipient = payable(address(impl)); - for (uint256 i = 0; i < data.fees.length; ++i) { + for (uint256 i; i < data.fees.length; ++i) { cons = orderParams.consideration[1 + i]; cons.itemType = IOpenseaExchange.ItemType.NATIVE; cons.token = address(0); diff --git a/sol-tests/proposals/OpenseaTestUtils.sol b/sol-tests/proposals/OpenseaTestUtils.sol index 3d77c059..9df755fc 100644 --- a/sol-tests/proposals/OpenseaTestUtils.sol +++ b/sol-tests/proposals/OpenseaTestUtils.sol @@ -7,7 +7,6 @@ import "../../contracts/proposals/vendor/IOpenseaExchange.sol"; import "../../contracts/tokens/IERC721.sol"; contract OpenseaTestUtils is Test { - IOpenseaExchange private immutable SEAPORT; constructor(IOpenseaExchange seaport) { @@ -49,7 +48,7 @@ contract OpenseaTestUtils is Test { internal { uint256 totalValue = params.listPrice; - for (uint256 i = 0; i < fees.length; ++i) { + for (uint256 i; i < fees.length; ++i) { totalValue += fees[i]; } vm.deal(params.buyer, address(params.buyer).balance + totalValue); @@ -92,7 +91,7 @@ contract OpenseaTestUtils is Test { considerations[0].identifierOrCriteria = 0; considerations[0].startAmount = considerations[0].endAmount = params.listPrice; considerations[0].recipient = params.maker; - for (uint256 i = 0; i < fees.length; ++i) { + for (uint256 i; i < fees.length; ++i) { considerations[1 + i].itemType = IOpenseaExchange.ItemType.NATIVE; considerations[1 + i].token = address(0); considerations[1 + i].identifierOrCriteria = 0;