Skip to content

Commit

Permalink
Safe casting (#50)
Browse files Browse the repository at this point in the history 
* use safeCast for integer casting

* fix overflow issue

* remove unnecessary using keyword; get contract size under limit

* pr feedback

---------

Co-authored-by: Mike <[email protected]>
  • Loading branch information
@MikeHathaway
MikeHathaway and Mike authored Mar 6, 2023
1 parent 43e8324 commit c22812d
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 23 deletions.
11 changes: 6 additions & 5 deletions src/grants/GrantFund.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

pragma solidity 0.8.16;

import { Governor } from "@oz/governance/Governor.sol";
import { IGovernor } from "@oz/governance/IGovernor.sol";
import { IVotes } from "@oz/governance/utils/IVotes.sol";
import { Governor } from "@oz/governance/Governor.sol";
import { IGovernor } from "@oz/governance/IGovernor.sol";
import { IVotes } from "@oz/governance/utils/IVotes.sol";
import { SafeCast } from "@oz/utils/math/SafeCast.sol";

import { Maths } from "./libraries/Maths.sol";

Expand Down Expand Up @@ -124,7 +125,7 @@ contract GrantFund is IGrantFund, ExtraordinaryFunding, StandardFunding {
// set initial voting power and remaining voting power
if (voter.votingPower == 0) {

uint128 newVotingPower = uint128(_getFundingStageVotingPower(msg.sender, screeningStageEndBlock));
uint128 newVotingPower = SafeCast.toUint128(_getFundingStageVotingPower(msg.sender, screeningStageEndBlock));

voter.votingPower = newVotingPower;
voter.remainingVotingPower = newVotingPower;
Expand Down Expand Up @@ -239,7 +240,7 @@ contract GrantFund is IGrantFund, ExtraordinaryFunding, StandardFunding {
// set initial voting power and remaining voting power
if (voter.votingPower == 0) {

uint128 newVotingPower = uint128(_getFundingStageVotingPower(msg.sender, screeningStageEndBlock));
uint128 newVotingPower = SafeCast.toUint128(_getFundingStageVotingPower(msg.sender, screeningStageEndBlock));

voter.votingPower = newVotingPower;
voter.remainingVotingPower = newVotingPower;
Expand Down
10 changes: 6 additions & 4 deletions src/grants/base/ExtraordinaryFunding.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

pragma solidity 0.8.16;

import { IERC20 } from "@oz/token/ERC20/IERC20.sol";
import { IERC20 } from "@oz/token/ERC20/IERC20.sol";
import { SafeCast } from "@oz/utils/math/SafeCast.sol";

import { Funding } from "./Funding.sol";

Expand Down Expand Up @@ -47,6 +48,7 @@ abstract contract ExtraordinaryFunding is Funding, IExtraordinaryFunding {

ExtraordinaryFundingProposal storage proposal = extraordinaryFundingProposals[proposalId_];

// since we are casting from uint128 to uint256, we can safely assume that the value will not overflow
uint256 tokensRequested = uint256(proposal.tokensRequested);

// revert if executed or if the proposal has received more votes than minimumThreshold and tokensRequestedPercentage of all tokens
Expand Down Expand Up @@ -95,8 +97,8 @@ abstract contract ExtraordinaryFunding is Funding, IExtraordinaryFunding {

// store newly created proposal
newProposal.proposalId = proposalId_;
newProposal.startBlock = uint128(block.number);
newProposal.endBlock = uint128(endBlock_);
newProposal.startBlock = SafeCast.toUint128(block.number);
newProposal.endBlock = SafeCast.toUint128(endBlock_);
newProposal.tokensRequested = totalTokensRequested;

emit ProposalCreated(
Expand Down Expand Up @@ -137,7 +139,7 @@ abstract contract ExtraordinaryFunding is Funding, IExtraordinaryFunding {

// check voting power at snapshot block
votes_ = _getVotes(account_, block.number, abi.encode(proposalId_));
proposal.votesReceived += uint112(votes_);
proposal.votesReceived += SafeCast.toUint112(votes_);

// record that voter has voted on this extraorindary funding proposal
hasVotedExtraordinary[proposalId_][account_] = true;
Expand Down
3 changes: 2 additions & 1 deletion src/grants/base/Funding.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity 0.8.16;

import { Governor } from "@oz/governance/Governor.sol";
import { ReentrancyGuard } from "@oz/security/ReentrancyGuard.sol";
import { SafeCast } from "@oz/utils/math/SafeCast.sol";

abstract contract Funding is Governor, ReentrancyGuard {

Expand Down Expand Up @@ -118,7 +119,7 @@ abstract contract Funding is Governor, ReentrancyGuard {
}

// update tokens requested for additional calldata
tokensRequested_ += uint128(tokensRequested);
tokensRequested_ += SafeCast.toUint128(tokensRequested);

unchecked { ++i; }
}
Expand Down
35 changes: 22 additions & 13 deletions src/grants/base/StandardFunding.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

pragma solidity 0.8.16;

import { IERC20 } from "@oz/token/ERC20/IERC20.sol";
import { SafeERC20 } from "@oz/token/ERC20/utils/SafeERC20.sol";
import { IERC20 } from "@oz/token/ERC20/IERC20.sol";
import { SafeCast } from "@oz/utils/math/SafeCast.sol";
import { SafeERC20 } from "@oz/token/ERC20/utils/SafeERC20.sol";

import { Funding } from "./Funding.sol";

Expand Down Expand Up @@ -128,7 +129,7 @@ abstract contract StandardFunding is Funding, IStandardFunding {
}

// set the distribution period to start at the current block
uint48 startBlock = uint48(block.number);
uint48 startBlock = SafeCast.toUint48(block.number);
uint48 endBlock = startBlock + DISTRIBUTION_PERIOD_LENGTH;

// set new value for currentDistributionId
Expand All @@ -140,7 +141,7 @@ abstract contract StandardFunding is Funding, IStandardFunding {
newDistributionPeriod.startBlock = startBlock;
newDistributionPeriod.endBlock = endBlock;
uint256 gbc = Maths.wmul(treasury, GLOBAL_BUDGET_CONSTRAINT);
newDistributionPeriod.fundsAvailable = uint128(gbc);
newDistributionPeriod.fundsAvailable = SafeCast.toUint128(gbc);

// decrease the treasury by the amount that is held for allocation in the new distribution period
treasury -= gbc;
Expand Down Expand Up @@ -180,6 +181,7 @@ abstract contract StandardFunding is Funding, IStandardFunding {

/**
* @notice Updates Treasury with surplus funds from distribution.
* @dev Counters incremented in an unchecked block due to being bounded by array length of at most 10.
* @param distributionId_ distribution Id of updating distribution
*/
function _updateTreasury(
Expand Down Expand Up @@ -319,7 +321,7 @@ abstract contract StandardFunding is Funding, IStandardFunding {
if (proposal.fundingVotesReceived < 0) return false;

// update counters
sum += uint128(proposal.fundingVotesReceived);
sum += uint128(proposal.fundingVotesReceived); // since we are converting from int128 to uint128, we can safely assume that the value will not overflow
totalTokensRequested += proposal.tokensRequested;

// check if slate of proposals exceeded budget constraint ( 90% of GBC )
Expand Down Expand Up @@ -452,14 +454,15 @@ abstract contract StandardFunding is Funding, IStandardFunding {
/**
* @notice Calculates the sum of funding votes allocated to a list of proposals.
* @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.
* @dev Counters incremented in an unchecked block due to being bounded by array length of at most 10.
* @param proposalIdSubset_ Array of proposal Ids to sum.
* @return sum_ The sum of the funding votes across the given proposals.
*/
function _sumProposalFundingVotes(
uint256[] memory proposalIdSubset_
) internal view returns (uint128 sum_) {
for (uint i = 0; i < proposalIdSubset_.length;) {
// since we are converting from int128 to uint128, we can safely assume that the value will not overflow
sum_ += uint128(standardFundingProposals[proposalIdSubset_[i]].fundingVotesReceived);

unchecked { ++i; }
Expand Down Expand Up @@ -505,6 +508,7 @@ abstract contract StandardFunding is Funding, IStandardFunding {

// voter had already cast a funding vote on this proposal
if (voteCastIndex != -1) {
// since we are converting from int256 to uint256, we can safely assume that the value will not overflow
FundingVoteParams storage existingVote = votesCast[uint256(voteCastIndex)];

// can't change the direction of a previous vote
Expand All @@ -524,7 +528,10 @@ abstract contract StandardFunding is Funding, IStandardFunding {
}

// calculate the cumulative cost of all votes made by the voter
uint128 cumulativeVotePowerUsed = uint128(_sumSquareOfVotesCast(votesCast));
// and check that attempted votes cast doesn't overflow uint128
uint256 sumOfTheSquareOfVotesCast = _sumSquareOfVotesCast(votesCast);
if (sumOfTheSquareOfVotesCast > type(uint128).max) revert InsufficientVotingPower();
uint128 cumulativeVotePowerUsed = SafeCast.toUint128(sumOfTheSquareOfVotesCast);

// check that the voter has enough voting power remaining to cast the vote
if (cumulativeVotePowerUsed > votingPower) revert InsufficientVotingPower();
Expand All @@ -533,17 +540,17 @@ abstract contract StandardFunding is Funding, IStandardFunding {
voter_.remainingVotingPower = votingPower - cumulativeVotePowerUsed;

// calculate the change in voting power used by the voter in this vote in order to accurately track the total voting power used in the funding stage
// since we are moving from uint128 to uint256, we can safely assume that the value will not overflow
uint256 incrementalVotingPowerUsed = uint256(cumulativeVotePowerUsed - voterPowerUsedPreVote);

// update accumulator for total voting power used in the funding stage in order to calculate delegate rewards
currentDistribution_.fundingVotePowerCast += incrementalVotingPowerUsed;

// update proposal vote tracking
proposal_.fundingVotesReceived += int128(voteParams_.votesUsed);
proposal_.fundingVotesReceived += SafeCast.toInt128(voteParams_.votesUsed);

// the incremental additional votes cast on the proposal
// used as a return value and emit value
incrementalVotesUsed_ = uint256(Maths.abs(voteParams_.votesUsed));
// the incremental additional votes cast on the proposal to be used as a return value and emit value
incrementalVotesUsed_ = SafeCast.toUint256(Maths.abs(voteParams_.votesUsed));

// emit VoteCast instead of VoteCastWithParams to maintain compatibility with Tally
// emits the amount of incremental votes cast for the proposal, not the voting power cost or total votes on a proposal
Expand Down Expand Up @@ -576,7 +583,7 @@ abstract contract StandardFunding is Funding, IStandardFunding {
uint256 proposalId = proposal_.proposalId;

// update proposal votes counter
proposal_.votesReceived += uint128(votes_);
proposal_.votesReceived += SafeCast.toUint128(votes_);

// check if proposal was already screened
int indexInArray = _findProposalIndex(proposalId, currentTopTenProposals);
Expand Down Expand Up @@ -659,6 +666,7 @@ abstract contract StandardFunding is Funding, IStandardFunding {
) internal pure returns (int256 index_) {
index_ = -1; // default value indicating proposalId not in the array

// since we are converting from uint256 to int256, we can safely assume that the value will not overflow
int256 numVotesCast = int256(voteParams_.length);
for (int256 i = 0; i < numVotesCast; ) {
//slither-disable-next-line incorrect-equality
Expand All @@ -675,6 +683,7 @@ abstract contract StandardFunding is Funding, IStandardFunding {
* @notice Sort the 10 proposals which will make it through screening and move on to the funding round.
* @dev Implements the descending insertion sort algorithm.
* @dev Counters incremented in an unchecked block due to being bounded by array length.
* @dev Since we are converting from int256 to uint256, we can safely assume that the values will not overflow.
* @param arr_ The array of proposals to sort by votes recieved.
*/
function _insertionSortProposalsByVotes(
Expand Down Expand Up @@ -713,7 +722,7 @@ abstract contract StandardFunding is Funding, IStandardFunding {
uint256 numVotesCast = votesCast_.length;

for (uint256 i = 0; i < numVotesCast; ) {
votesCastSumSquared_ += Maths.wpow(uint256(Maths.abs(votesCast_[i].votesUsed)), 2);
votesCastSumSquared_ += Maths.wpow(SafeCast.toUint256(Maths.abs(votesCast_[i].votesUsed)), 2);

unchecked { ++i; }
}
Expand Down

0 comments on commit c22812d

Please sign in to comment.