Ajna Finance (Governance) security record

Delegate rewards are given when a user with delegated votes has voted in both screening and funding rounds.`_getDelegateReward()` function calculates the number of rewards the user should receive. It uses the following function: $$ \begin{array}{ll} rewards = \dfrac{{TotalFundsAvailable * UserUsedVotingPower}}{TotalCastedFundingVotes} * 0.1 \end{array} $$ This logic works correctly if it is assumed that everyone has also voted in the screening round, which is not necessarily the case. If the user has not voted in the screening round, the function would revert. Hence the rewards are distributed amongst all users that have participated and have not participated in the screening round in the current logic, and some reward amounts will not be accessible to anyone.

A new standard funding distribution period starts through the execution of `startNewDistributionPeriod()` , , resulting in the automatic deduction of the global budget constraint (GBC) from the available balance in the `treasury`. When a distribution period expires and `startNewDistributionPeriod()` is called to start a new distribution period, `_updateTreasury()` will add back to `treasury` all the unused funds of the previous periods. However, the contract does not maintain a record of the funds allocated for delegation rewards (10% of the GBC intended for rewarding eligible voters via the `claimDelegateReward()` function). `_updateTreasury()` will add this 10% back to the treasury, independently of if the rewards were claimed or not. As a result, the `treasury` variable surpasses the contract balance, leading to an inaccurate calculation of the GBC.

AJNA tokens may become locked in this contract in the case of a Standard Proposal failing to execute. `Funding._execute()` will revert if transferring AJNA tokens to any of the target addresses included in a proposal fails. In `StandardFunding._updateTreasury()` the treasury is updated by only considering the tokens requested for a proposal, regardless of the status of the proposal execution. As a result, upon the failed execution of a Standard Proposal, the treasury will not be updated to include these funds which could not be sent.

In the Ajna Finance Whitepaper, one of the conditions for an extraordinary funding proposal's success is that the threshold number of required votes is the sum of the "minimum threshold" and the "proposal threshold", or in the terms of the whitepaper: > *When submitting a proposal the proposer must include the exact number of tokens they would like to extract, proposal threshold. This proposal threshold is added to the minimum threshold to provide a required votes received threshold. If the vote fails to reach this threshold it will fail and no tokens will be distributed.* The example given treats both the minimum threshold and the proposal threshold as determined in terms of the non-treasury tokens. > a. *A proposer requests tokens equivalent to 10% of the treasury.* >> i. *50% + 10% = 60%* >> ii. *If 65% of non-treasury tokens vote affirmatively, 10% of the treasury is released* >> iii. *If 59.9% of non-treasury tokens vote affirmatively, 0% of the treasury is released* However, as implemented, the proposal threshold is being replaced with the number of tokens requested, as can be seen at `ExtraordinaryFunding.sol#173`: ``` (votesReceived >= tokensRequested_ + _getSliceOfNonTreasury(minThresholdPercentage)) ``` This does not agree with the example from the whitepaper. Take the following scenario for example: 1. The treasury contains 300 million tokens. 2. There are 200 million non-treasury tokens. 3. The extraordinary funding proposal requests 10% of the treasury (30 million). 4. The number of votes required based on the description is 120 million votes. 5. The number of votes required based on the implementation is 130 million votes. <br> There is another ambiguity regarding extraordinary funding proposal success. The following condition is described in the whitepaper: > *Also, a proposal must not exceed the minimum threshold in what it withdraws. For example, if a proposal specifies that it would like to take 100,000,000 tokens from the treasury, which represents 60% of the treasury supply, and the minimum threshold is 55%, this proposal will automatically fail.* However, the example that follows is inconsistent with this description: > *EF2 should now fail since 120,000,000 > ((1 - minimum threshold) \* treasury)* That is, the example demonstrates that a proposal must not exceed (1 - minimum threshold) in the withdrawal amount, whereas the preceding paragraph states that the withdrawal amount must not exceed the minimum threshold. The code implements the example's calculation: ``` (tokensRequested_ <= _getSliceOfTreasury(Maths.WAD - minThresholdPercentage)) ``` Finally, point 3 defines the minimum threshold as a proportion of non-treasury tokens, whereas the example in point 5 regards the minimum threshold as a proportion of treasury tokens. > *This mechanism works by allowing up to the percentage of non-treasury tokens, minimum threshold ...* > *EF2 should now fail since 120,000,000 > ((1 - minimum threshold) \* treasury)*

`StandardFunding.claimDelegateReward()` checks if the challenge stage of the distribution has ended before transferring the delegation rewards in this statement: ``` // Check if Challenge Period is still active if(block.number < _getChallengeStageEndBlock(currentDistribution.endBlock)) revert ChallengePeriodNotEnded(); ``` For that logic, the end block of the challenge stage will be valid, which is not consistent with the validations done in other parts of the code. To follow the patterns in the codebase, users should not be able to claim rewards in the end block of the challenge stage, as it is still in progress.

It is important to validate inputs, even if they only come from trusted addresses, to avoid human error. Missing input validation was found in the following scenarios: 1. During the funding stage, users that have AJNA tokens can vote in a quadratic manner. It is possible to cast a vote of 0 power. This can be done accidentally or intentionally. Accidental input of 0 has a worse outcome as due to the current design of `_fundingVote()`, a secondary vote cannot change the direction of the previous vote, and 0 counts as a `support=1` vote. So, if the user wanted to vote again against the proposal, it will not be possible anymore. 1. In `proposeStandard()` `description_` can be an empty string. 1. `proposeExtraordinary()` allows a user to propose an extraordinary vote for a certain proposal. This is only possible to do nine times as the minimum threshold for votes goes up by 5% every time. It is possible to make a proposal with zero tokens requested which will force an extra one passed proposal (the proposal has to be voted on and eligible to be executed). After that `proposeExtraordinary()` will revert on any call as `Maths.WAD - _getMinimumThresholdPercentage()` will overflow.

After the funding part of the voting in Standard Proposal Voting is over, one can submit a winner slate. A winner slate is a selection of winning proposals in such a way that the total funds needed are lower than 90% of GBC. These slates can be submitted only if they are better than the previous one. The ranking is done in a way of the total number of votes in the slate. This leads to a classic optimization problem called 0-1 Knapsack, which can be defined as follows: $$ \begin{array}{ll} \operatorname{maximize} & \sum_{i=1}^{10} v_i x_i \end{array} $$ $$ \begin{array}{ll} \text {subject to} & \sum_{i=1}^{10} w_i x_i \leq GBC \text { and } x_i \in\{0,1\} \text{ and } w_i \gt 0 \end{array} $$ As the field is not huge, it is possible to brute force an optimized solution. The problem arises if an optimum solution (i.e. slate with a max number of votes) has multiple combinations. This means there is now another level of selection where the first person to submit an optimal slate is chosen.

The contracts are currently set to be compiled with Solidity version `0.8.16` according to their `pragma` statements. However, an [important bug](https://blog.soliditylang.org/2022/09/08/solidity-0.8.17-release-announcement/) was fixed in `0.8.17`.

`GrantFund.hashProposal()` allow users to calculate the proposal ID passing the proposal data as parameters. However, it does not include the prefixes added to the description, and the NatSpec comment about `descriptionHash_` can be confusing as it is not updated for this code commit.

The `executeExtraordinary()` function evaluates if the proposal has passed and is ready to be executed. `_extraordinaryProposalSucceeded()` is the function that actually whether a proposal succeeded or not. The calculation is based on the current state of the treasury and it is verified that at least a certain threshold of the current holdings of the treasury are met. This means that from the moment of proposal to execution, the state of the treasury can vary significantly. Though no direct vulnerability has been identified, this behavior can lead to unexpected outcomes.