Platform: Code4rena
Start Date: 03/05/2023
Pot Size: $60,500 USDC
Total HM: 25
Participants: 114
Period: 8 days
Judge: Picodes
Total Solo HM: 6
Id: 234
League: ETH
Rank: 11/114
Findings: 3
Award: $912.95
🌟 Selected for report: 2
🚀 Solo Findings: 0
187.2333 USDC - $187.23
https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L236-L265 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L274-L293 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L119-L164 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L197-L220
When block.number
is more than the end block of the challenge stage for the current distribution period, the following StandardFunding.claimDelegateReward
function can be called to claim the delegate reward for the corresponding voter. Since the StandardFunding._getDelegateReward
function below executes rewards_ = Maths.wdiv(Maths.wmul(currentDistribution_.fundsAvailable, votingPowerAllocatedByDelegatee), currentDistribution_.fundingVotePowerCast) / 10
, the delegate reward is part of fundsAvailable
for the current distribution period.
function claimDelegateReward( uint24 distributionId_ ) external override returns(uint256 rewardClaimed_) { // Revert if delegatee didn't vote in screening stage if(screeningVotesCast[distributionId_][msg.sender] == 0) revert DelegateRewardInvalid(); QuarterlyDistribution memory currentDistribution = _distributions[distributionId_]; // Check if Challenge Period is still active if(block.number < _getChallengeStageEndBlock(currentDistribution.endBlock)) revert ChallengePeriodNotEnded(); // check rewards haven't already been claimed if(hasClaimedReward[distributionId_][msg.sender]) revert RewardAlreadyClaimed(); QuadraticVoter memory voter = _quadraticVoters[distributionId_][msg.sender]; // calculate rewards earned for voting rewardClaimed_ = _getDelegateReward(currentDistribution, voter); hasClaimedReward[distributionId_][msg.sender] = true; ... // transfer rewards to delegatee IERC20(ajnaTokenAddress).safeTransfer(msg.sender, rewardClaimed_); }
function _getDelegateReward( QuarterlyDistribution memory currentDistribution_, QuadraticVoter memory voter_ ) internal pure returns (uint256 rewards_) { // calculate the total voting power available to the voter that was allocated in the funding stage uint256 votingPowerAllocatedByDelegatee = voter_.votingPower - voter_.remainingVotingPower; // if none of the voter's voting power was allocated, they receive no rewards if (votingPowerAllocatedByDelegatee == 0) return 0; // calculate reward // delegateeReward = 10 % of GBC distributed as per delegatee Voting power allocated rewards_ = Maths.wdiv( Maths.wmul( currentDistribution_.fundsAvailable, votingPowerAllocatedByDelegatee ), currentDistribution_.fundingVotePowerCast ) / 10; }
At the same time when block.number
is more than the end block of the challenge stage for the current distribution period, the following StandardFunding.startNewDistributionPeriod
function can also be called to start a new distribution period. Because block.number > _getChallengeStageEndBlock(currentDistributionEndBlock)
is true, _updateTreasury(currentDistributionId)
would be executed.
function startNewDistributionPeriod() external override returns (uint24 newDistributionId_) { uint24 currentDistributionId = _currentDistributionId; uint256 currentDistributionEndBlock = _distributions[currentDistributionId].endBlock; // check that there isn't currently an active distribution period if (block.number <= currentDistributionEndBlock) revert DistributionPeriodStillActive(); // update Treasury with unused funds from last two distributions { // Check if any last distribution exists and its challenge stage is over if (currentDistributionId > 0 && (block.number > _getChallengeStageEndBlock(currentDistributionEndBlock))) { // Add unused funds from last distribution to treasury _updateTreasury(currentDistributionId); } // checks if any second last distribution exist and its unused funds are not added into treasury if (currentDistributionId > 1 && !_isSurplusFundsUpdated[currentDistributionId - 1]) { // Add unused funds from second last distribution to treasury _updateTreasury(currentDistributionId - 1); } } // set the distribution period to start at the current block uint48 startBlock = SafeCast.toUint48(block.number); uint48 endBlock = startBlock + DISTRIBUTION_PERIOD_LENGTH; // set new value for currentDistributionId newDistributionId_ = _setNewDistributionId(); // create QuarterlyDistribution struct QuarterlyDistribution storage newDistributionPeriod = _distributions[newDistributionId_]; newDistributionPeriod.id = newDistributionId_; newDistributionPeriod.startBlock = startBlock; newDistributionPeriod.endBlock = endBlock; uint256 gbc = Maths.wmul(treasury, GLOBAL_BUDGET_CONSTRAINT); newDistributionPeriod.fundsAvailable = SafeCast.toUint128(gbc); // decrease the treasury by the amount that is held for allocation in the new distribution period treasury -= gbc; ... }
When calling the following StandardFunding._updateTreasury
function for the current distribution period, treasury += (fundsAvailable - totalTokensRequested)
would be executed so the difference between fundsAvailable
and total tokens requested by the proposals to be executed for the current distribution period is added back to treasury
. However, such difference still includes the voters' delegate rewards that are also part of fundsAvailable
for the current distribution period. This causes the accounting for treasury
to be incorrect in which treasury
becomes more than it should be; after voters claim their delegate rewards for the current distribution period, treasury
would become more than the GrantFund
contract's AJNA balance.
function _updateTreasury( uint24 distributionId_ ) private { bytes32 fundedSlateHash = _distributions[distributionId_].fundedSlateHash; uint256 fundsAvailable = _distributions[distributionId_].fundsAvailable; uint256[] memory fundingProposalIds = _fundedProposalSlates[fundedSlateHash]; uint256 totalTokensRequested; uint256 numFundedProposals = fundingProposalIds.length; for (uint i = 0; i < numFundedProposals; ) { Proposal memory proposal = _standardFundingProposals[fundingProposalIds[i]]; totalTokensRequested += proposal.tokensRequested; unchecked { ++i; } } // readd non distributed tokens to the treasury treasury += (fundsAvailable - totalTokensRequested); _isSurplusFundsUpdated[distributionId_] = true; }
Back in the StandardFunding.startNewDistributionPeriod
function, executing uint256 gbc = Maths.wmul(treasury, GLOBAL_BUDGET_CONSTRAINT)
and newDistributionPeriod.fundsAvailable = SafeCast.toUint128(gbc)
would cause fundsAvailable
for the new distribution period to be more than it should be because treasury
is inflated already. Because proposals' number of requested tokens and voters' delegate rewards are proportions of fundsAvailable
for the new distribution period, these proposals can request more tokens and these voters can claim more delegate rewards than they are allowed to. The extra number of tokens requested by the proposals and extra delegate rewards claimed by the voters that are more than they are entitled to are essentially lost in the treasury's perspective.
The following steps can occur for the described scenario.
9e18
, the total delegate rewards that can be claimed by the voters can be 1e18
, fundsAvailable
can be 10e18
, and treasury
can be 333,333,333,333,333,333,334 - 10e18 = 323,333,333,333,333,333,334
.block.number
is more than the end block of the challenge stage for the current distribution period, the StandardFunding.startNewDistributionPeriod
function is called to start the new distribution period.StandardFunding.startNewDistributionPeriod
function calls the StandardFunding._updateTreasury
function, treasury
is increased by 10e18 - 9e18 = 1e18
to become 323,333,333,333,333,333,334 + 1e18 = 324,333,333,333,333,333,334
.treasury
should not be increased at all because the increment of 1e18
was actually the voters' delegate rewards for the current distribution period. If these voters claim these delegate rewards, the GrantFund
contract's AJNA balance would become 333,333,333,333,333,333,334 - 9e18 - 1e18 = 323,333,333,333,333,333,334
but treasury
is increased to 324,333,333,333,333,333,334
in step 3, which is more than the GrantFund
contract's AJNA balance. Thus, the accounting for treasury
becomes incorrect.StandardFunding.startNewDistributionPeriod
function executes uint256 gbc = Maths.wmul(treasury, GLOBAL_BUDGET_CONSTRAINT)
and newDistributionPeriod.fundsAvailable = SafeCast.toUint128(gbc)
, fundsAvailable
for the new distribution period is calculated to 324,333,333,333,333,333,334 * 0.03 = 9,730,000,000,000,000,000.02 = 9,730,000,000,000,000,000
.treasury
was not increased by the voters' delegate rewards for the current distribution period in step 3, fundsAvailable
for the new distribution period would be calculated to 323,333,333,333,333,333,334 * 0.03 = 9,700,000,000,000,000,000.02 = 9,700,000,000,000,000,000
, which is 3e16
less than the incorrectly inflated fundsAvailable
for the new distribution period shown in step 5.VSCode
The StandardFunding._updateTreasury
function can be updated to not add the voters' total delegate rewards for the corresponding distribution period back to treasury
.
Error
#0 - c4-sponsor
2023-05-19T19:16:47Z
MikeHathaway marked the issue as sponsor confirmed
#1 - dmitriia
2023-05-21T18:15:13Z
Looks like dup of #263
#2 - c4-judge
2023-05-30T18:10:31Z
Picodes marked the issue as duplicate of #263
#3 - c4-judge
2023-05-30T18:14:46Z
Picodes marked the issue as satisfactory
🌟 Selected for report: rbserver
Also found by: deadrxsezzz, rvierdiiev
329.7645 USDC - $329.76
https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L519-L569 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L891-L910 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/Funding.sol#L76-L93 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L572-L596 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L698-L753 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L872-L881 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L131-L157 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L246-L256
Because of if (block.number <= screeningStageEndBlock || block.number > endBlock) revert InvalidVote()
, the following StandardFunding.fundingVote
function can only execute uint128 newVotingPower = SafeCast.toUint128(_getVotesFunding(msg.sender, votingPower, voter.remainingVotingPower, screeningStageEndBlock))
when block.number
is bigger than screeningStageEndBlock
.
function fundingVote( FundingVoteParams[] memory voteParams_ ) external override returns (uint256 votesCast_) { uint24 currentDistributionId = _currentDistributionId; QuarterlyDistribution storage currentDistribution = _distributions[currentDistributionId]; QuadraticVoter storage voter = _quadraticVoters[currentDistributionId][msg.sender]; uint256 endBlock = currentDistribution.endBlock; uint256 screeningStageEndBlock = _getScreeningStageEndBlock(endBlock); // check that the funding stage is active if (block.number <= screeningStageEndBlock || block.number > endBlock) revert InvalidVote(); uint128 votingPower = voter.votingPower; // if this is the first time a voter has attempted to vote this period, // set initial voting power and remaining voting power if (votingPower == 0) { // calculate the voting power available to the voting power in this funding stage uint128 newVotingPower = SafeCast.toUint128(_getVotesFunding(msg.sender, votingPower, voter.remainingVotingPower, screeningStageEndBlock)); voter.votingPower = newVotingPower; voter.remainingVotingPower = newVotingPower; } ... }
When the StandardFunding.fundingVote
function calls the following StandardFunding._getVotesFunding
function, screeningStageEndBlock
would be used as the voteStartBlock_
input for calling the Funding._getVotesAtSnapshotBlocks
function below. Because block.number
would always be bigger than screeningStageEndBlock
, voteStartBlock_
would always be screeningStageEndBlock
in the Funding._getVotesAtSnapshotBlocks
function. This means that the Funding._getVotesAtSnapshotBlocks
function would always return the same voting power for the same voter at any block.number
that is bigger than screeningStageEndBlock
during the funding phase.
function _getVotesFunding( address account_, uint256 votingPower_, uint256 remainingVotingPower_, uint256 screeningStageEndBlock_ ) internal view returns (uint256 votes_) { // voter has already allocated some of their budget this period if (votingPower_ != 0) { votes_ = remainingVotingPower_; } // voter hasn't yet called _castVote in this period else { votes_ = Maths.wpow( _getVotesAtSnapshotBlocks( account_, screeningStageEndBlock_ - VOTING_POWER_SNAPSHOT_DELAY, screeningStageEndBlock_ ), 2); } }
https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/Funding.sol#L76-L93
function _getVotesAtSnapshotBlocks( address account_, uint256 snapshot_, uint256 voteStartBlock_ ) internal view returns (uint256) { IVotes token = IVotes(ajnaTokenAddress); // 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); }
However, because of if (block.number < currentDistribution.startBlock || block.number > _getScreeningStageEndBlock(currentDistribution.endBlock)) revert InvalidVote()
, calling the following StandardFunding.screeningVote
function would not revert when block.number
equals the current distribution period's start block.
function screeningVote( ScreeningVoteParams[] memory voteParams_ ) external override returns (uint256 votesCast_) { QuarterlyDistribution memory currentDistribution = _distributions[_currentDistributionId]; // check screening stage is active if (block.number < currentDistribution.startBlock || block.number > _getScreeningStageEndBlock(currentDistribution.endBlock)) revert InvalidVote(); uint256 numVotesCast = voteParams_.length; for (uint256 i = 0; i < numVotesCast; ) { Proposal storage proposal = _standardFundingProposals[voteParams_[i].proposalId]; // check that the proposal is part of the current distribution period if (proposal.distributionId != currentDistribution.id) revert InvalidVote(); uint256 votes = voteParams_[i].votes; // cast each successive vote votesCast_ += votes; _screeningVote(msg.sender, proposal, votes); unchecked { ++i; } } }
When the StandardFunding.screeningVote
function calls the following StandardFunding._screeningVote
function, if (screeningVotesCast[distributionId][account_] + votes_ > _getVotesScreening(distributionId, account_)) revert InsufficientVotingPower()
is executed in which the StandardFunding._getVotesScreening
function below would use the current distribution period's start block as the voteStartBlock_
input for calling the Funding._getVotesAtSnapshotBlocks
function. Since the Funding._getVotesAtSnapshotBlocks
function executes voteStartBlock_ = voteStartBlock_ != block.number ? voteStartBlock_ : block.number - 1
, voteStartBlock_
would be 1 block prior to the current distribution period's start block when block.number
equals the current distribution period's start block, and voteStartBlock_
would be the current distribution period's start block when block.number
is bigger than the current distribution period's start block. However, it is possible that the same voter has different available votes at 1 block prior to the current distribution period's start block and at the current distribution period's start block. This is unlike the StandardFunding._getVotesFunding
function that would always return the same voting power for the same voter when calling the StandardFunding.fundingVote
function during the funding phase. Since calling the StandardFunding._getVotesScreening
function when block.number
equals the current distribution period's start block and when block.number
is bigger than the current distribution period's start block during the screening phase can return different available votes for the same voter, this voter would call the StandardFunding.screeningVote
function at a chosen block.number
that would provide the highest votes.
This should not be allowed because _getVotesScreening(distributionId, account_)
needs to return the same number of votes across all blocks during the screening phase to make if (screeningVotesCast[distributionId][account_] + votes_ > _getVotesScreening(distributionId, account_)) revert InsufficientVotingPower()
effective in the StandardFunding._screeningVote
function. For example, a voter who has no available votes at 1 block prior to the current distribution period's start block can mint many AJNA tokens at the current distribution period's start block and call the StandardFunding.screeningVote
function at block.number
that is bigger than the current distribution period's start block during the screening phase to use her or his available votes at current distribution period's start block. For another example, a voter who has available votes at 1 block prior to the current distribution period's start block can call the StandardFunding.screeningVote
function when block.number
equals the current distribution period's start block and then sell all of her or his AJNA tokens at the same block.number
. Such voters' actions are unfair to other voters who maintain the same number of available votes at 1 block prior to the current distribution period's start block and at the current distribution period's start block for the screening stage voting.
function _screeningVote( address account_, Proposal storage proposal_, uint256 votes_ ) internal { uint24 distributionId = proposal_.distributionId; // check that the voter has enough voting power to cast the vote if (screeningVotesCast[distributionId][account_] + votes_ > _getVotesScreening(distributionId, account_)) revert InsufficientVotingPower(); ... }
function _getVotesScreening(uint24 distributionId_, address account_) internal view returns (uint256 votes_) { uint256 startBlock = _distributions[distributionId_].startBlock; // calculate voting weight based on the number of tokens held at the snapshot blocks of the screening stage votes_ = _getVotesAtSnapshotBlocks( account_, startBlock - VOTING_POWER_SNAPSHOT_DELAY, startBlock ); }
Please note that calling the following ExtraordinaryFunding.voteExtraordinary
function when block.number
equals _extraordinaryFundingProposals[proposalId_].startBlock
also does not revert, and the ExtraordinaryFunding._getVotesExtraordinary
function below also uses _extraordinaryFundingProposals[proposalId_].startBlock
as the voteStartBlock_
input for calling the Funding._getVotesAtSnapshotBlocks
function. Hence, the same issue also applies to the ExtraordinaryFunding.voteExtraordinary
function.
function voteExtraordinary( uint256 proposalId_ ) external override returns (uint256 votesCast_) { // revert if msg.sender already voted on proposal if (hasVotedExtraordinary[proposalId_][msg.sender]) revert AlreadyVoted(); ExtraordinaryFundingProposal storage proposal = _extraordinaryFundingProposals[proposalId_]; // revert if proposal is inactive if (proposal.startBlock > block.number || proposal.endBlock < block.number || proposal.executed) { revert ExtraordinaryFundingProposalInactive(); } // check voting power at snapshot block and update proposal votes votesCast_ = _getVotesExtraordinary(msg.sender, proposalId_); proposal.votesReceived += SafeCast.toUint120(votesCast_); // record that voter has voted on this extraordinary funding proposal hasVotedExtraordinary[proposalId_][msg.sender] = true; ... }
function _getVotesExtraordinary(address account_, uint256 proposalId_) internal view returns (uint256 votes_) { if (proposalId_ == 0) revert ExtraordinaryFundingProposalInactive(); uint256 startBlock = _extraordinaryFundingProposals[proposalId_].startBlock; votes_ = _getVotesAtSnapshotBlocks( account_, startBlock - VOTING_POWER_SNAPSHOT_DELAY, startBlock ); }
The following steps can occur for the described scenario.
StandardFunding.startNewDistributionPeriod
transaction that would end the previous distribution period and starts the current distribution period in the mempool, Alice backruns that transaction by minting a lot of AJNA tokens at the current distribution period's start block.block.number
becomes bigger than the current distribution period's start block during the screening phase, Alice calls the StandardFunding.screeningVote
function to successfully use all of her available votes at the current distribution period's start block for a proposal.VSCode
https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L578 can be updated to the following code:
if (block.number <= currentDistribution.startBlock || block.number > _getScreeningStageEndBlock(currentDistribution.endBlock)) revert InvalidVote();
and
https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/ExtraordinaryFunding.sol#L139-L141 can be updated to the following code:
if (proposal.startBlock >= block.number || proposal.endBlock < block.number || proposal.executed) { revert ExtraordinaryFundingProposalInactive(); }
Timing
#0 - c4-judge
2023-05-18T16:00:57Z
Picodes marked the issue as duplicate of #355
#1 - c4-judge
2023-05-18T16:01:05Z
Picodes marked the issue as selected for report
#2 - c4-sponsor
2023-05-19T19:16:15Z
MikeHathaway marked the issue as sponsor disputed
#3 - MikeHathaway
2023-05-19T19:16:36Z
Different voting mechanisms have different voting specifications.
#4 - c4-judge
2023-05-31T14:02:42Z
Picodes marked the issue as satisfactory
#5 - Picodes
2023-05-31T14:03:57Z
@MikeHathaway I think this issue is valid: it is not about different phases but about what happens at voteStartBlock_ == block.number
versus voteStartBlock_ == block.number + 1
#6 - 0xRobocop
2023-05-31T19:52:43Z
I think this issue was miss-judged
First of all the warden jumps from talking about the funding vote and then talks about the screening vote, which have different voting specifications.
Second and most important, is that warden's assumptions and examples are simply invalid. All the examples and assumptions starts that some user has no AJNA tokens 1 block prior the start block, but when the user sees that a new distribution will start, then he gets the tokens. This scenario will yield a zero voting power for the user because the voting power always uses the minimum balance of the user between block numbers. Where startBlock - VOTING_POWER_SNAPSHOT_DELAY
is always one of them. The current delay is 33 blocks.
Function _getVotesScreening(uint24 distributionId_, address account_) internal view returns (uint256 votes_) { uint256 startBlock = _distributions[distributionId_].startBlock; // calculate voting weight based on the number of tokens held at the snapshot blocks of the screening stage votes_ = _getVotesAtSnapshotBlocks( account_, startBlock - VOTING_POWER_SNAPSHOT_DELAY, startBlock ); }
function _getVotesAtSnapshotBlocks( address account_, uint256 snapshot_, uint256 voteStartBlock_ ) internal view returns (uint256) { IVotes token = IVotes(ajnaTokenAddress); // 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); }
For example, Warden's proof of concept is wrong, Alice cannot get her ajna tokens at the start block and then use those tokens as voting power.
#7 - rbserver
2023-05-31T22:09:16Z
Sorry for any confusion. At 1 block prior to the current distribution period's start block, Alice has no available votes at all
in the POC section means that Alice has no available votes at that particular block that is 1 block prior to the current distribution period's start block; yet, she can still have available votes at blocks before that particular block. When Alice has available votes at the block that is 33 blocks prior to the current distribution period's start block, it is possible that she uses all of her available votes at the current distribution period's start block for the scenario described in the POC section.
#8 - MikeHathaway
2023-06-01T20:46:01Z
If Alice has no voting power 1 block prior to start, than at the start block she will have no voting power if she doesn't transfer and delegate tokens to that address. Prior voting power won't matter, as we take the minimum of the prior snapshot and the current snapshot.
#9 - rbserver
2023-06-01T21:25:24Z
If Alice mints or receives AJNA tokens at the current distribution period's start block, can she gain available votes at the current distribution period's start block? If so, she can have voting power at the current distribution period's start block while has 0 available votes at 1 block prior to the current distribution period's start block. Please correct me if I am understanding this incorrect.
#10 - MikeHathaway
2023-06-01T21:53:37Z
That is incorrect, the snapshot system requires that the balance be > 0 for the entire 33 blocks prior to the distribution period's start. If any block is 0, their power is 0.
This can be verified with a quick unit test:
function testVotingPowerAtDistributionStart() external { // 14 tokenholders self delegate their tokens to enable voting on the proposals _selfDelegateVoters(_token, _votersArr); // roll 32 blocks forward to the block before the distribution period starts vm.roll(_startBlock + 32); // _tokenHolder1 transfers their tokens away address nonVotingAddress = makeAddr("nonVotingAddress"); changePrank(_tokenHolder1); _token.transfer(nonVotingAddress, 25_000_000 * 1e18); vm.roll(_startBlock + 33); // nonVotingAddress returns the funds one block later changePrank(nonVotingAddress); _token.transfer(_tokenHolder1, 25_000_000 * 1e18); // start distribution period _startDistributionPeriod(_grantFund); // check voting power of _tokenHolder1 is 0 uint256 votingPower = _getScreeningVotes(_grantFund, _tokenHolder1); assertEq(votingPower, 0); votingPower = _getScreeningVotes(_grantFund, nonVotingAddress); assertEq(votingPower, 0); }
#11 - rbserver
2023-06-01T23:36:08Z
Thanks for your reply and unit test!
The thing is that, to vote in the current distribution period, Alice does not have to vote at the current distribution period's start block and can vote at an eligible block after the current distribution period's start block as stated in Step 3 of the POC section. When voting at such eligible block, Alice can utilize the available votes that she gained at the start block. To demonstrate this, I've made some modifications to your unit test below, which does pass. Please see @audit
for the places of modification. For this test, at _startBlock + 33
, which is the start block, _tokenHolder1
's voting power is 0; however, at _startBlock + 34
and _startBlock + 35
, _tokenHolder1
's voting power becomes 25_000_000 * 1e18. The inconsistency between the voting power when voting at the start block and when voting at an eligible block after the start block is the main point of this report.
function testVotingPowerAtDistributionStart() external { // 14 tokenholders self delegate their tokens to enable voting on the proposals _selfDelegateVoters(_token, _votersArr); // roll 32 blocks forward to the block before the distribution period starts vm.roll(_startBlock + 32); // _tokenHolder1 transfers their tokens away address nonVotingAddress = makeAddr("nonVotingAddress"); changePrank(_tokenHolder1); // @audit transfer _token.balanceOf(_tokenHolder1) so _tokenHolder1 does not have any AJNA at _startBlock + 32 _token.transfer(nonVotingAddress, _token.balanceOf(_tokenHolder1)); vm.roll(_startBlock + 33); // nonVotingAddress returns the funds one block later changePrank(nonVotingAddress); _token.transfer(_tokenHolder1, 25_000_000 * 1e18); // start distribution period _startDistributionPeriod(_grantFund); // check voting power of _tokenHolder1 is 0 uint256 votingPower = _getScreeningVotes(_grantFund, _tokenHolder1); assertEq(votingPower, 0); votingPower = _getScreeningVotes(_grantFund, nonVotingAddress); assertEq(votingPower, 0); // @audit at _startBlock + 34, _tokenHolder1's voting power becomes 25_000_000 * 1e18, which is not 0 vm.roll(_startBlock + 34); votingPower = _getScreeningVotes(_grantFund, _tokenHolder1); assertEq(votingPower, 25_000_000 * 1e18); // @audit at _startBlock + 35, _tokenHolder1's voting power is still 25_000_000 * 1e18, which is not 0 vm.roll(_startBlock + 35); votingPower = _getScreeningVotes(_grantFund, _tokenHolder1); assertEq(votingPower, 25_000_000 * 1e18); }
#12 - MikeHathaway
2023-06-01T23:53:52Z
My mistake, you are correct. This is a real issue. Thank you for the report and the extra assertions!
🌟 Selected for report: rbserver
Also found by: 0xnev, ABAIKUNANBAEV, Audit_Avengers, Aymen0909, BGSecurity, BRONZEDISC, Bason, DadeKuma, GG_Security, Jerry0x, Jorgect, MohammedRizwan, REACH, Sathish9098, Shogoki, T1MOH, UniversalCrypto, aviggiano, ayden, berlin-101, bytes032, codeslide, descharre, fatherOfBlocks, hals, kaveyjoe, kodyvim, lfzkoala, lukris02, nadin, naman1778, patitonar, pontifex, sakshamguruji, squeaky_cactus, teawaterwire, wonjun, yjrwkk
395.9568 USDC - $395.96
Issue | |
---|---|
[01] | CHALLENGE_PERIOD_LENGTH , DISTRIBUTION_PERIOD_LENGTH , FUNDING_PERIOD_LENGTH , AND MAX_EFM_PROPOSAL_LENGTH ARE HARDCODED BASED ON 7200 BLOCKS PER DAY |
[02] | AMBIGUITY IN StandardFunding._standardProposalState FUNCTION |
[03] | ExtraordinaryFundingProposal.votesReceived IN ExtraordinaryFunding CONTRACT IS uint120 INSTEAD OF uint128 |
[04] | CALLING ExtraordinaryFunding.proposeExtraordinary AND StandardFunding.proposeStandard FUNCTIONS CAN REVERT AND WASTE GAS |
[05] | ajnaTokenAddress IS HARDCODED |
[06] | CODE COMMENT IN ExtraordinaryFunding._extraordinaryProposalSucceeded FUNCTION CAN BE INCORRECT |
[07] | CODE COMMENT FOR CHALLENGE_PERIOD_LENGTH CAN BE MORE ACCURATE |
[08] | MISSING address(0) CHECKS FOR CRITICAL CONSTRUCTOR INPUTS |
[09] | SOLIDITY VERSION 0.8.19 CAN BE USED |
[10] | SETTING support TO 1 WHEN voteParams_.votesUsed < 0 IS FALSE IN StandardFunding._fundingVote FUNCTION IS REDUNDANT |
[11] | REDUNDANT EXECUTION OF if (sumOfTheSquareOfVotesCast > type(uint128).max) revert InsufficientVotingPower() IN StandardFunding._fundingVote FUNCTION |
[12] | InvalidVote ERROR CAN BE MORE DESCRIPTIVE |
[13] | CONSTANTS CAN BE USED INSTEAD OF MAGIC NUMBERS |
[14] | UNDERSCORES CAN BE ADDED FOR NUMBERS |
[15] | uint256 CAN BE USED INSTEAD OF uint |
[16] | SPACES CAN BE ADDED FOR BETTER READABILITY |
CHALLENGE_PERIOD_LENGTH
, DISTRIBUTION_PERIOD_LENGTH
, FUNDING_PERIOD_LENGTH
, AND MAX_EFM_PROPOSAL_LENGTH
ARE HARDCODED BASED ON 7200 BLOCKS PER DAYThe following CHALLENGE_PERIOD_LENGTH
, DISTRIBUTION_PERIOD_LENGTH
, FUNDING_PERIOD_LENGTH
, and MAX_EFM_PROPOSAL_LENGTH
are hardcoded and assume that the number of blocks per day is 7200 and the number of seconds per block is 12. Yet, it is possible that the number of seconds per block is more or less than 12 due to network traffic and future chain upgrades. When the number of seconds per block is no longer 12, the durations corresponding to CHALLENGE_PERIOD_LENGTH
, DISTRIBUTION_PERIOD_LENGTH
, FUNDING_PERIOD_LENGTH
, and MAX_EFM_PROPOSAL_LENGTH
are no longer 7, 90, 10, and 30 days, which break the duration specifications for various phases. This can cause unexpectedness to users; for example, when the duration for FUNDING_PERIOD_LENGTH
becomes less than 10 days, a user, who expects that she or he could vote on the 10th day, can fail to vote unexpectedly on that 10th day. To avoid unexpectedness and disputes, please consider using block.timestamp
instead of blocks for defining durations for various phases in the StandardFunding
and ExtraordinaryFunding
contracts.
/** * @notice Length of the challengephase of the distribution period in blocks. * @dev Roughly equivalent to the number of blocks in 7 days. * @dev The period in which funded proposal slates can be checked in updateSlate. */ uint256 internal constant CHALLENGE_PERIOD_LENGTH = 50400;
/** * @notice Length of the distribution period in blocks. * @dev Roughly equivalent to the number of blocks in 90 days. */ uint48 internal constant DISTRIBUTION_PERIOD_LENGTH = 648000;
/** * @notice Length of the funding phase of the distribution period in blocks. * @dev Roughly equivalent to the number of blocks in 10 days. */ uint256 internal constant FUNDING_PERIOD_LENGTH = 72000;
/** * @notice The maximum length of a proposal's voting period, in blocks. */ uint256 internal constant MAX_EFM_PROPOSAL_LENGTH = 216_000; // number of blocks in one month
StandardFunding._standardProposalState
FUNCTIONWhen block.number
is bigger than _distributions[proposal.distributionId].endBlock
, it is possible that the proposal is in the challenge phase. In the challenge phase, calling the following StandardFunding._standardProposalState
function, which further calls the StandardFunding._standardFundingVoteSucceeded
function below, can return ProposalState.Succeeded
if the proposal is found in _fundedProposalSlates[_distributions[distributionId].fundedSlateHash]
at that moment. However, during the challenge phase, the StandardFunding.updateSlate
function below can be called to update _fundedProposalSlates
for the mentioned _distributions[distributionId].fundedSlateHash]
. Such update can exclude the same proposal from _fundedProposalSlates[_distributions[distributionId].fundedSlateHash]
; calling the StandardFunding._standardProposalState
function again would then return ProposalState.Defeated
for such proposal. Hence, the StandardFunding._standardProposalState
function cannot properly determine the status of the corresponding proposal in the challenge phase. To prevent users from being misled, please update the StandardFunding._standardProposalState
function to return a state, which is not Succeeded
or Defeated
, to indicate that the proposal's success state is to be determined when the time is in the challenge phase.
function _standardProposalState(uint256 proposalId_) internal view returns (ProposalState) { Proposal memory proposal = _standardFundingProposals[proposalId_]; if (proposal.executed) return ProposalState.Executed; else if (_distributions[proposal.distributionId].endBlock >= block.number) return ProposalState.Active; else if (_standardFundingVoteSucceeded(proposalId_)) return ProposalState.Succeeded; else return ProposalState.Defeated; }
function _standardFundingVoteSucceeded( uint256 proposalId_ ) internal view returns (bool) { uint24 distributionId = _standardFundingProposals[proposalId_].distributionId; return _findProposalIndex(proposalId_, _fundedProposalSlates[_distributions[distributionId].fundedSlateHash]) != -1; }
function updateSlate( uint256[] calldata proposalIds_, uint24 distributionId_ ) external override returns (bool newTopSlate_) { ... // if slate of proposals is new top slate, update state if (newTopSlate_) { uint256[] storage existingSlate = _fundedProposalSlates[newSlateHash]; for (uint i = 0; i < numProposalsInSlate; ) { // update list of proposals to fund existingSlate.push(proposalIds_[i]); unchecked { ++i; } } // update hash to point to the new leading slate of proposals currentDistribution.fundedSlateHash = newSlateHash; emit FundedSlateUpdated( distributionId_, newSlateHash ); } }
ExtraordinaryFundingProposal.votesReceived
IN ExtraordinaryFunding
CONTRACT IS uint120
INSTEAD OF uint128
In the StandardFunding
contract, Proposal.votesReceived
is uint128
.
struct Proposal { ... uint128 votesReceived; // accumulator of screening votes received by a proposal ... }
Yet, in the ExtraordinaryFunding
contract, ExtraordinaryFundingProposal.votesReceived
is uint120
. Calling the ExtraordinaryFunding.voteExtraordinary
function below by a user, who has the voting power being more than type(uint120).max
, can revert, which causes such user to waste gas and fail to vote for the corresponding proposal. This degrades the user experience because such user, who is familiar with Proposal.votesReceived
in the StandardFunding
contract, could think that ExtraordinaryFundingProposal.votesReceived
in the ExtraordinaryFunding
contract would be uint128
as well. To be more consistent and prevent disputes, please consider using ExtraordinaryFundingProposal.votesReceived
as uint128
instead of uint120
in the ExtraordinaryFunding
contract.
struct ExtraordinaryFundingProposal { ... uint120 votesReceived; // Total votes received for this proposal. ... }
function voteExtraordinary( uint256 proposalId_ ) external override returns (uint256 votesCast_) { ... // check voting power at snapshot block and update proposal votes votesCast_ = _getVotesExtraordinary(msg.sender, proposalId_); proposal.votesReceived += SafeCast.toUint120(votesCast_); ... }
ExtraordinaryFunding.proposeExtraordinary
AND StandardFunding.proposeStandard
FUNCTIONS CAN REVERT AND WASTE GASCalling the following ExtraordinaryFunding.proposeExtraordinary
and StandardFunding.proposeStandard
functions will call the Funding._validateCallDatas
function below. Calling the Funding._validateCallDatas
function will revert if targets_[i] != ajnaTokenAddress || values_[i] != 0
is true. Hence, the ExtraordinaryFunding.proposeExtraordinary
and StandardFunding.proposeStandard
functions cannot be used to create proposals for calling addresses other than ajnaTokenAddress
or sending ETH. When users are unaware of this, they could believe that proposals for general purposes can be created and executed; yet, because calling ExtraordinaryFunding.proposeExtraordinary
and StandardFunding.proposeStandard
functions with targets_
being not ajnaTokenAddress
or values_
being positive revert, these users would waste their gas for nothing. To avoid confusion and disputes, please consider updating the ExtraordinaryFunding.proposeExtraordinary
and StandardFunding.proposeStandard
functions so targets_
and values_
would only be ajnaTokenAddress
and 0 and not be specified by users.
function proposeExtraordinary( uint256 endBlock_, address[] memory targets_, uint256[] memory values_, bytes[] memory calldatas_, string memory description_) external override returns (uint256 proposalId_) { ... uint128 totalTokensRequested = _validateCallDatas(targets_, values_, calldatas_); ... }
function proposeStandard( address[] memory targets_, uint256[] memory values_, bytes[] memory calldatas_, string memory description_ ) external override returns (uint256 proposalId_) { ... newProposal.tokensRequested = _validateCallDatas(targets_, values_, calldatas_); // check proposal parameters are valid and update tokensRequested ... }
function _validateCallDatas( address[] memory targets_, uint256[] memory values_, bytes[] memory calldatas_ ) internal view returns (uint128 tokensRequested_) { // check params have matching lengths if (targets_.length == 0 || targets_.length != values_.length || targets_.length != calldatas_.length) revert InvalidProposal(); for (uint256 i = 0; i < targets_.length;) { // check targets and values params are valid if (targets_[i] != ajnaTokenAddress || values_[i] != 0) revert InvalidProposal(); ... } }
ajnaTokenAddress
IS HARDCODEDThe following ajnaTokenAddress
is hardcoded. It is not AJNA's address on chains like Polygon, Arbitrum, and Optimism. This means that the functionalities that rely on ajnaTokenAddress
will break on these chains in which the GrantFund
contract, which inherits the Funding
contract, cannot be used on these chains. To be more future-proofed, please consider adding a function that is only callable by the trusted admin for updating ajnaTokenAddress
instead of relying on a hardcoded ajnaTokenAddress
.
https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/Funding.sol#L21
address public immutable ajnaTokenAddress = 0x9a96ec9B57Fb64FbC60B423d1f4da7691Bd35079;
ExtraordinaryFunding._extraordinaryProposalSucceeded
FUNCTION CAN BE INCORRECTIn the following ExtraordinaryFunding._extraordinaryProposalSucceeded
function, the comment for (votesReceived >= tokensRequested_ + _getSliceOfNonTreasury(minThresholdPercentage))
is succeeded if proposal's votes received doesn't exceed the minimum threshold required
. However, (votesReceived >= tokensRequested_ + _getSliceOfNonTreasury(minThresholdPercentage))
can only be true when the proposal's votes received meet or exceed such minimum threshold, which is the opposite of the comment. To prevent confusion, please consider updating the comment to match the corresponding code.
function _extraordinaryProposalSucceeded( uint256 proposalId_, uint256 tokensRequested_ ) internal view returns (bool) { uint256 votesReceived = uint256(_extraordinaryFundingProposals[proposalId_].votesReceived); uint256 minThresholdPercentage = _getMinimumThresholdPercentage(); return // succeeded if proposal's votes received doesn't exceed the minimum threshold required (votesReceived >= tokensRequested_ + _getSliceOfNonTreasury(minThresholdPercentage)) && // succeeded if tokens requested are available for claiming from the treasury (tokensRequested_ <= _getSliceOfTreasury(Maths.WAD - minThresholdPercentage)) ; }
CHALLENGE_PERIOD_LENGTH
CAN BE MORE ACCURATEThe challenge phase starts after the time for DISTRIBUTION_PERIOD_LENGTH
is finished. The time for CHALLENGE_PERIOD_LENGTH
is more of an addition to the distribution period instead of part of the distribution period. Thus, describing CHALLENGE_PERIOD_LENGTH
as Length of the challengephase of the distribution period
given that DISTRIBUTION_PERIOD_LENGTH
is described as Length of the distribution period
is somewhat misleading. To avoid confusion, the comment for CHALLENGE_PERIOD_LENGTH
can be updated to indicate that CHALLENGE_PERIOD_LENGTH
is not included in DISTRIBUTION_PERIOD_LENGTH
.
/** * @notice Length of the challengephase of the distribution period in blocks. * @dev Roughly equivalent to the number of blocks in 7 days. * @dev The period in which funded proposal slates can be checked in updateSlate. */ uint256 internal constant CHALLENGE_PERIOD_LENGTH = 50400;
/** * @notice Length of the distribution period in blocks. * @dev Roughly equivalent to the number of blocks in 90 days. */ uint48 internal constant DISTRIBUTION_PERIOD_LENGTH = 648000;
address(0)
CHECKS FOR CRITICAL CONSTRUCTOR INPUTSTo prevent unintended behaviors, critical constructor inputs should be checked against address(0)
.
address(0)
checks are missing for erc20Factory_
and erc721Factory_
in the following constructor. Please consider checking them.
https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L116-L122
constructor( ERC20PoolFactory erc20Factory_, ERC721PoolFactory erc721Factory_ ) PermitERC721("Ajna Positions NFT-V1", "AJNA-V1-POS", "1") { erc20PoolFactory = erc20Factory_; erc721PoolFactory = erc721Factory_; }
address(0)
check is missing for positionManager_
in the following constructor. Please consider checking it.
https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/RewardsManager.sol#L95-L100
constructor(address ajnaToken_, IPositionManager positionManager_) { if (ajnaToken_ == address(0)) revert DeployWithZeroAddress(); ajnaToken = ajnaToken_; positionManager = positionManager_; }
0.8.19
CAN BE USEDUsing the more updated version of Solidity can enhance security. As described in https://github.com/ethereum/solidity/releases, Version 0.8.19
is the latest version of Solidity, which "contains a fix for a long-standing bug that can result in code that is only used in creation code to also be included in runtime bytecode". To be more secured and more future-proofed, please consider using Version 0.8.19
for the following contracts.
ajna-core\src\PositionManager.sol 3: pragma solidity 0.8.14; ajna-core\src\RewardsManager.sol 3: pragma solidity 0.8.14; ajna-grants\src\grants\GrantFund.sol 3: pragma solidity 0.8.16; ajna-grants\src\grants\base\ExtraordinaryFunding.sol 3: pragma solidity 0.8.16; ajna-grants\src\grants\base\Funding.sol 3: pragma solidity 0.8.16; ajna-grants\src\grants\base\StandardFunding.sol 3: pragma solidity 0.8.16; ajna-grants\src\grants\libraries\Maths.sol 2: pragma solidity 0.8.16;
support
TO 1
WHEN voteParams_.votesUsed < 0
IS FALSE IN StandardFunding._fundingVote
FUNCTION IS REDUNDANTWhen calling the following StandardFunding._fundingVote
function, uint8 support = 1
is executed before voteParams_.votesUsed < 0 ? support = 0 : support = 1
. Therefore, when voteParams_.votesUsed < 0
is false, support
does not need to be set to 1
again. Please consider refactoring the code to only update support
to 0
when voteParams_.votesUsed < 0
is true.
function _fundingVote( QuarterlyDistribution storage currentDistribution_, Proposal storage proposal_, address account_, QuadraticVoter storage voter_, FundingVoteParams memory voteParams_ ) internal returns (uint256 incrementalVotesUsed_) { uint8 support = 1; uint256 proposalId = proposal_.proposalId; // determine if voter is voting for or against the proposal voteParams_.votesUsed < 0 ? support = 0 : support = 1; ... }
if (sumOfTheSquareOfVotesCast > type(uint128).max) revert InsufficientVotingPower()
IN StandardFunding._fundingVote
FUNCTIONThe following StandardFunding._fundingVote
function executes if (sumOfTheSquareOfVotesCast > type(uint128).max) revert InsufficientVotingPower()
before uint128 cumulativeVotePowerUsed = SafeCast.toUint128(sumOfTheSquareOfVotesCast)
. Because calling the SafeCast.toUint128
function below would revert when sumOfTheSquareOfVotesCast > type(uint128).max
is true, executing if (sumOfTheSquareOfVotesCast > type(uint128).max) revert InsufficientVotingPower()
becomes redundant. Please consider removing if (sumOfTheSquareOfVotesCast > type(uint128).max) revert InsufficientVotingPower()
from the StandardFunding._fundingVote
function.
function _fundingVote( QuarterlyDistribution storage currentDistribution_, Proposal storage proposal_, address account_, QuadraticVoter storage voter_, FundingVoteParams memory voteParams_ ) internal returns (uint256 incrementalVotesUsed_) { ... // calculate the cumulative cost of all votes made by the voter // 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); ... }
function toUint128(uint256 value) internal pure returns (uint128) { require(value <= type(uint128).max, "SafeCast: value doesn't fit in 128 bits"); return uint128(value); }
InvalidVote
ERROR CAN BE MORE DESCRIPTIVEThe following StandardFunding.fundingVote
and StandardFunding.screeningVote
functions can revert with the InvalidVote
error for various reasons. Yet, executing revert InvalidVote()
does not describe the specific reason. To be more descriptive and user-friendly, please consider updating the InvalidVote
error so it can provide the reason why calling the corresponding function reverts.
function fundingVote( FundingVoteParams[] memory voteParams_ ) external override returns (uint256 votesCast_) { ... uint256 endBlock = currentDistribution.endBlock; uint256 screeningStageEndBlock = _getScreeningStageEndBlock(endBlock); // check that the funding stage is active if (block.number <= screeningStageEndBlock || block.number > endBlock) revert InvalidVote(); ... for (uint256 i = 0; i < numVotesCast; ) { Proposal storage proposal = _standardFundingProposals[voteParams_[i].proposalId]; // check that the proposal is part of the current distribution period if (proposal.distributionId != currentDistributionId) revert InvalidVote(); // check that the proposal being voted on is in the top ten screened proposals if (_findProposalIndex(voteParams_[i].proposalId, _topTenProposals[currentDistributionId]) == -1) revert InvalidVote(); ... } }
function screeningVote( ScreeningVoteParams[] memory voteParams_ ) external override returns (uint256 votesCast_) { QuarterlyDistribution memory currentDistribution = _distributions[_currentDistributionId]; // check screening stage is active if (block.number < currentDistribution.startBlock || block.number > _getScreeningStageEndBlock(currentDistribution.endBlock)) revert InvalidVote(); uint256 numVotesCast = voteParams_.length; for (uint256 i = 0; i < numVotesCast; ) { Proposal storage proposal = _standardFundingProposals[voteParams_[i].proposalId]; // check that the proposal is part of the current distribution period if (proposal.distributionId != currentDistribution.id) revert InvalidVote(); ... } }
( Please note that the following instances are not found in https://gist.github.com/CloudEllie/a4655b833548ed9a86a63eb7292bcc0f#n04-constants-should-be-defined-rather-than-using-magic-numbers. )
To improve readability and maintainability, a constant can be used instead of the magic number. Please consider replacing the magic numbers, such as 2
, used in the following code with constants.
ajna-grants\src\grants\base\StandardFunding.sol 292: ) / 10; 849: votesCastSumSquared_ += Maths.wpow(SafeCast.toUint256(Maths.abs(votesCast_[i].votesUsed)), 2); 908: ), 2);
It is a common practice to separate each 3 digits in a number by an underscore to improve code readability. Unlike MAX_EFM_PROPOSAL_LENGTH
below, the following CHALLENGE_PERIOD_LENGTH
, DISTRIBUTION_PERIOD_LENGTH
, and FUNDING_PERIOD_LENGTH
do not use underscores; please consider adding underscores for these numbers.
uint256 internal constant CHALLENGE_PERIOD_LENGTH = 50400;
uint48 internal constant DISTRIBUTION_PERIOD_LENGTH = 648000;
uint256 internal constant FUNDING_PERIOD_LENGTH = 72000;
uint256 internal constant MAX_EFM_PROPOSAL_LENGTH = 216_000; // number of blocks in one month
uint256
CAN BE USED INSTEAD OF uint
Both uint
and uint256
are used in the protocol's code. In favor of explicitness, please consider using uint256
instead of uint
in the following code.
ajna-grants\src\grants\base\StandardFunding.sol 208: for (uint i = 0; i < numFundedProposals; ) { 324: for (uint i = 0; i < numProposalsInSlate; ) { 434: for (uint i = 0; i < numProposalsInSlate_; ) { 468: for (uint i = 0; i < numProposals; ) { 469: for (uint j = i + 1; j < numProposals; ) { 491: for (uint i = 0; i < proposalIdSubset_.length;) {
For better readability, spaces can be added in code where appropriate.
A space can be added between challenge
and phase
in the following comment.
https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L30
* @notice Length of the challengephase of the distribution period in blocks.
A space can be added between returns
and (uint256
in the following code.
https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L238
) external override returns(uint256 rewardClaimed_) {
A space can be added between currentSlateHash
and !=
in the following code.
https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L318
(currentSlateHash!= 0 && sum > _sumProposalFundingVotes(_fundedProposalSlates[currentSlateHash]));
#0 - c4-judge
2023-05-18T18:59:30Z
Picodes marked the issue as grade-a
#1 - c4-judge
2023-05-31T14:30:54Z
Picodes marked the issue as selected for report
#2 - Picodes
2023-06-29T05:43:02Z
5, 8, 9, 13 can be removed for the final report as they don't add much value. It'd be interesting to add #296, #286 and #290 to the report as they are interesting Low Sev findings.
#3 - Picodes
2023-06-29T05:43:11Z
5, 8, 9, 13 can be removed for the final report as they don't add much value. It'd be interesting to add #296, #286 and #290 to the report as they are interesting Low Sev findings.