Fix reviews (#42)

* add code coverage script * update readme for code coverage * Check duplicates s119 (#38) * check for duplicate proposalIds in a standard funding slate * fix tests * fix typo * gas optimization * remove unused mapping * minor cleanups --------- Co-authored-by: Mike <mikehathaway@makerdao.com> * Sherlock 124 (#39) * expand comments * add tokens requestd check to executeExtraordinary; update fuzzExtraordinary test --------- Co-authored-by: Mike <mikehathaway@makerdao.com> * prevent transferFromWithPermit from being front run (#40) Co-authored-by: Mike <mikehathaway@makerdao.com> * Sherlock 64 (#43) * fix quadratic voting * fix most of the tests * add utility functions to calculate delegate rewards earned; fix delegate rewards test * add multi funding vote support * add funding vote multi function; support updating existing funding vote accumulator * fix voting power calculation; fix delegate rewards * fix tests * cleanups * add testQuadraticVotingTally * remove dead comment * add sqrt tests; add getFundingPowerVotes method and associated tests * rename sqrt to wsqrt; internalize sqrt scaling factor * rename variables; account for total cost of a proposal vote when revoting an existant proposal * fix quarterly distribution voting power accumulator * additional variable renaming * use account_ in castVote to stay consistent --------- Co-authored-by: Mike <mikehathaway@makerdao.com> * Fixes informational suggestions in Sherlock 9 and 10. (#41) * cleanup comments; remove magic numbers * additional comments; CONSTANT for snapshot blocks; cache array lengths in iterations * dmitri feedback * Styling cleanups of unchecked blocks --------- Co-authored-by: Mike <mikehathaway@makerdao.com> --------- Co-authored-by: Mike <mikehathaway@makerdao.com>
ajna-finance · Feb 13, 2023 · dcbdfea · dcbdfea
1 parent e99fb12
commit dcbdfea
Show file tree

Hide file tree

Showing 14 changed files with 803 additions and 246 deletions.
diff --git a/.env.example b/.env.example
@@ -1,2 +1,2 @@
 ## Ethereum node endpoint ##
-ETH_RPC_URL =
+ETH_RPC_URL=
diff --git a/src/grants/GrantFund.sol b/src/grants/GrantFund.sol
@@ -98,6 +98,43 @@ contract GrantFund is IGrantFund, ExtraordinaryFunding, StandardFunding {
     /*** Voting Functions ***/
     /************************/
 
+    /**
+     * @notice Cast an array of funding votes in one transaction.
+     * @dev    Calls out to StandardFunding._fundingVote().
+     * @dev    Only iterates through a maximum of 10 proposals that made it through the screening round.
+     * @dev    Counters incremented in an unchecked block due to being bounded by array length.
+     * @param voteParams_ The array of votes on proposals to cast.
+     * @return votesCast_ The total number of votes cast across all of the proposals.
+     */
+    function fundingVotesMulti(FundingVoteParams[] memory voteParams_) external returns (uint256 votesCast_) {
+        uint256 currentDistributionId = distributionIdCheckpoints.latest();
+        QuarterlyDistribution storage currentDistribution = distributions[currentDistributionId];
+        QuadraticVoter storage voter = quadraticVoters[currentDistribution.id][msg.sender];
+        uint256 screeningStageEndBlock = _getScreeningStageEndBlock(currentDistribution);
+
+        // this is the first time a voter has attempted to vote this period
+        if (voter.votingPower == 0) {
+            voter.votingPower          = Maths.wpow(_getVotesSinceSnapshot(msg.sender, screeningStageEndBlock - VOTING_POWER_SNAPSHOT_DELAY, screeningStageEndBlock), 2);
+            voter.remainingVotingPower = voter.votingPower;
+        }
+
+        uint256 numVotesCast = voteParams_.length;
+        for (uint256 i = 0; i < numVotesCast; ) {
+            Proposal storage proposal = standardFundingProposals[voteParams_[i].proposalId];
+
+            // cast each successive vote
+            votesCast_ += _fundingVote(
+                currentDistribution,
+                proposal,
+                msg.sender,
+                voter,
+                voteParams_[i]
+            );
+
+            unchecked { ++i; }
+        }
+    }
+
     /**
      * @notice Vote on a proposal in the screening or funding stage of the Distribution Period.
      * @dev Override channels all other castVote methods through here.
@@ -112,33 +149,32 @@ contract GrantFund is IGrantFund, ExtraordinaryFunding, StandardFunding {
         // standard funding mechanism
         if (mechanism == FundingMechanism.Standard) {
             Proposal storage proposal = standardFundingProposals[proposalId_];
-            QuarterlyDistribution memory currentDistribution = distributions[proposal.distributionId];
-            uint256 screeningPeriodEndBlock = currentDistribution.endBlock - 72000;
+            QuarterlyDistribution storage currentDistribution = distributions[proposal.distributionId];
+            uint256 screeningStageEndBlock = _getScreeningStageEndBlock(currentDistribution);
 
             // screening stage
-            if (block.number >= currentDistribution.startBlock && block.number <= screeningPeriodEndBlock) {
+            if (block.number >= currentDistribution.startBlock && block.number <= screeningStageEndBlock) {
                 uint256 votes = _getVotes(account_, block.number, bytes("Screening"));
 
                 votesCast_ = _screeningVote(account_, proposal, votes);
             }
 
             // funding stage
-            else if (block.number > screeningPeriodEndBlock && block.number <= currentDistribution.endBlock) {
+            else if (block.number > screeningStageEndBlock && block.number <= currentDistribution.endBlock) {
                 QuadraticVoter storage voter = quadraticVoters[currentDistribution.id][account_];
 
                 // this is the first time a voter has attempted to vote this period
-                if (voter.votingWeight == 0) {
-                    voter.votingWeight = Maths.wpow(_getVotesSinceSnapshot(account_, screeningPeriodEndBlock - 33, screeningPeriodEndBlock), 2);
-                    voter.budgetRemaining = int256(voter.votingWeight);
+                if (voter.votingPower == 0) {
+                    voter.votingPower          = Maths.wpow(_getVotesSinceSnapshot(account_, screeningStageEndBlock - VOTING_POWER_SNAPSHOT_DELAY, screeningStageEndBlock), 2);
+                    voter.remainingVotingPower = voter.votingPower;
                 }
 
-                // amount of quadratic budget to allocated to the proposal
-                int256 budgetAllocation = abi.decode(params_, (int256));
-
-                // check if the voter has enough budget remaining to allocate to the proposal
-                if (Maths.abs(budgetAllocation) > voter.budgetRemaining) revert InsufficientBudget();
+                // decode the amount of votes to allocated to the proposal
+                int256 votes = abi.decode(params_, (int256));
+                FundingVoteParams memory newVote = FundingVoteParams(proposalId_, votes);
 
-                votesCast_ = _fundingVote(proposal, account_, voter, budgetAllocation);
+                // allocate the votes to the proposal
+                votesCast_ = _fundingVote(currentDistribution, proposal, account_, voter, newVote);
             }
         }
 
@@ -162,20 +198,21 @@ contract GrantFund is IGrantFund, ExtraordinaryFunding, StandardFunding {
 
         // within screening period 1 token 1 vote
         if (keccak256(params_) == keccak256(bytes("Screening"))) {
-            // calculate voting weight based on the number of tokens held before the start of the distribution period
-            availableVotes_ = _getVotesSinceSnapshot(account_, currentDistribution.startBlock - 33, currentDistribution.startBlock);
+            // calculate voting weight based on the number of tokens held at the snapshot blocks of the screening stage
+            availableVotes_ = _getVotesSinceSnapshot(account_, currentDistribution.startBlock - VOTING_POWER_SNAPSHOT_DELAY, currentDistribution.startBlock);
         }
         // else if in funding period quadratic formula squares the number of votes
         else if (keccak256(params_) == keccak256(bytes("Funding"))) {
             QuadraticVoter memory voter = quadraticVoters[currentDistribution.id][account_];
 
             // voter has already allocated some of their budget this period
-            if (voter.votingWeight != 0) {
-                availableVotes_ = uint256(voter.budgetRemaining);
+            if (voter.votingPower != 0) {
+                availableVotes_ = voter.remainingVotingPower;
             }
-            // this is the first time a voter has attempted to vote this period
+            // voter hasn't yet called _castVote in this period
             else {
-                availableVotes_ = Maths.wpow(_getVotesSinceSnapshot(account_, currentDistribution.endBlock - 72033, currentDistribution.endBlock - 72000), 2);
+                uint256 screeningStageEndBlock = _getScreeningStageEndBlock(currentDistribution);
+                availableVotes_ = Maths.wpow(_getVotesSinceSnapshot(account_, screeningStageEndBlock - VOTING_POWER_SNAPSHOT_DELAY, screeningStageEndBlock), 2);
             }
         }
         else {
@@ -185,8 +222,9 @@ contract GrantFund is IGrantFund, ExtraordinaryFunding, StandardFunding {
 
                 // one token one vote for extraordinary funding
                 if (proposalId != 0) {
+                    // get the number of votes available to voters at the start of the proposal, and 33 blocks before the start of the proposal
                     uint256 startBlock = extraordinaryFundingProposals[proposalId].startBlock;
-                    availableVotes_ = _getVotesSinceSnapshot(account_, startBlock - 33, startBlock);
+                    availableVotes_ = _getVotesSinceSnapshot(account_, startBlock - VOTING_POWER_SNAPSHOT_DELAY, startBlock);
                 }
             }
             // voting is not possible for non-specified pathways
@@ -200,15 +238,17 @@ contract GrantFund is IGrantFund, ExtraordinaryFunding, StandardFunding {
      * @notice Retrieve the voting power of an account.
      * @dev    Voteing power is the minimum of the amount of votes available at a snapshot block 33 blocks prior to voting start, and at the vote starting block.
      * @param account_        The voting account.
-     * @param snapshot_       One of block numbers to retrieve the voting power at. 33 blocks prior to the vote starting block.
-     * @param voteStartBlock_ The block number the vote started at.
+     * @param snapshot_       One of the block numbers to retrieve the voting power at. 33 blocks prior to the block at which a proposal is available for voting.
+     * @param voteStartBlock_ The block number the proposal became available for voting.
      * @return                The voting power of the account.
      */
     function _getVotesSinceSnapshot(address account_, uint256 snapshot_, uint256 voteStartBlock_) internal view returns (uint256) {
+        // calculate the number of votes available at the snapshot block
         uint256 votes1 = token.getPastVotes(account_, snapshot_);
 
         // enable voting weight to be calculated during the voting period's start block
         voteStartBlock_ = voteStartBlock_ != block.number ? voteStartBlock_ : block.number - 1;
+        // calculate the number of votes available at the stage's start block
         uint256 votes2 = token.getPastVotes(account_, voteStartBlock_);
 
         return Maths.min(votes2, votes1);
@@ -232,21 +272,16 @@ contract GrantFund is IGrantFund, ExtraordinaryFunding, StandardFunding {
         if (mechanism == FundingMechanism.Standard) {
             Proposal memory proposal = standardFundingProposals[proposalId_]; 
             QuarterlyDistribution memory currentDistribution = distributions[proposal.distributionId];
-            uint256 screeningPeriodEndBlock = currentDistribution.endBlock - 72000;
+            uint256 screeningStageEndBlock = _getScreeningStageEndBlock(currentDistribution);
 
             // screening stage
-            if (block.number >= currentDistribution.startBlock && block.number <= screeningPeriodEndBlock) {
+            if (block.number >= currentDistribution.startBlock && block.number <= screeningStageEndBlock) {
                 hasVoted_ = hasVotedScreening[proposal.distributionId][account_];
             }
 
             // funding stage
-            else if (block.number > screeningPeriodEndBlock && block.number <= currentDistribution.endBlock) {
-                QuadraticVoter storage voter = quadraticVoters[currentDistribution.id][account_];
-
-                // Check if voter has voted
-                if (uint256(voter.budgetRemaining) < voter.votingWeight) {
-                    hasVoted_ = true;
-                }
+            else if (block.number > screeningStageEndBlock && block.number <= currentDistribution.endBlock) {
+                hasVoted_ = quadraticVoters[currentDistribution.id][account_].votesCast.length != 0;
             }
         }
         else {

diff --git a/src/grants/base/ExtraordinaryFunding.sol b/src/grants/base/ExtraordinaryFunding.sol
@@ -47,12 +47,12 @@ abstract contract ExtraordinaryFunding is Funding, IExtraordinaryFunding {
         }
 
         // check if the proposal has received more votes than minimumThreshold and tokensRequestedPercentage of all tokens
-        if (proposal.votesReceived >= proposal.tokensRequested + getSliceOfNonTreasury(_getMinimumThresholdPercentage())) {
-            proposal.succeeded = true;
-        } else {
-            proposal.succeeded = false;
+        if (proposal.votesReceived < proposal.tokensRequested + getSliceOfNonTreasury(_getMinimumThresholdPercentage()))
             revert ExecuteExtraordinaryProposalInvalid();
-        }
+        proposal.succeeded = true;
+
+        // check tokens requested are available for claiming from the treasury
+        if (proposal.tokensRequested > getSliceOfTreasury(Maths.WAD - _getMinimumThresholdPercentage())) revert ExtraordinaryFundingProposalInvalid();
 
         fundedExtraordinaryProposals.push(proposal.proposalId);
 
@@ -82,7 +82,7 @@ abstract contract ExtraordinaryFunding is Funding, IExtraordinaryFunding {
 
         uint256 totalTokensRequested = _validateCallDatas(targets_, values_, calldatas_);
 
-        // check tokens requested is within limits
+        // check tokens requested are available for claiming from the treasury
         if (totalTokensRequested > getSliceOfTreasury(Maths.WAD - _getMinimumThresholdPercentage())) revert ExtraordinaryFundingProposalInvalid();
 
         // store newly created proposal

diff --git a/src/grants/base/Funding.sol b/src/grants/base/Funding.sol
@@ -80,6 +80,12 @@ abstract contract Funding is Governor, ReentrancyGuard {
      */
     mapping(uint256 => mapping(address => bool)) internal hasVotedScreening;
 
+    /**
+     * @notice Number of blocks prior to a given voting stage to check an accounts voting power.
+     * @dev    Prevents flashloan attacks or duplicate voting with multiple accounts.
+     */
+    uint256 internal constant VOTING_POWER_SNAPSHOT_DELAY = 33;
+
     /**
      * @notice Total funds available for Funding Mechanism
     */
@@ -91,6 +97,7 @@ abstract contract Funding is Governor, ReentrancyGuard {
 
     /**
      * @notice Verifies proposal's targets, values, and calldatas match specifications.
+     * @dev    Counters incremented in an unchecked block due to being bounded by array length.
      * @param targets_         The addresses of the contracts to call.
      * @param values_          The amounts of ETH to send to each target.
      * @param calldatas_       The calldata to send to each target.
@@ -128,9 +135,7 @@ abstract contract Funding is Governor, ReentrancyGuard {
             // update tokens requested for additional calldata
             tokensRequested_ += tokensRequested;
 
-            unchecked {
-                ++i;
-            }
+            unchecked { ++i; }
         }
     }
 }