Addresses Provider can be removed from the constructor in the GhoAToken and instead a call can be made to the pool to retrieve the addresses provider.

using the immutable may be cheaper since there is not external call. Check gas here.

Reported By

• Openzeppelin

Scope
Throughout the codebase there are many places where "wadray" math is used to scale and descale balances, amounts, discounts, etc. In some cases, variable names include the word "scaled" to explicitly state that the value is a scaled value while in other places it does not even though the value is indeed scaled.

To ensure clarity and easy readability, consider explicitly stating whether an amount is scaled in the variable name.

What data should be included in the mint and burn events?

Should this continue with the V2 pattern or adopt the V3 pattern?
Should discounts be included in the mint and burn events or used in separate events?

Description
The _removeFacilitator() function does not implement checks for non-existent facilitators which causes wrong
events to be emitted.
When removing a facilitator, the remove() function of the EnumerableSet library will check if the facilitator exists
prior to be removed, but the return value is not used. This will cause wrong events to be emitted.

Recommendations
One solution could be to add checks that require that the facilitator to be removed exists in the facilitators mapping
by checking the length of its label. Another solution could be to handle the return value of the remove() function to
revert when the facilitator to be removed does not exist.

Review solidity versions

Reported By

• Openzeppelin

Scope
The current lack of explicit casting when handling unsigned integer variables used in the GhoToken contract, considering the different types of unsigned integers in use (e.g. uint256 , and uint128 ), hinders code readability, making it more error-prone and hard to maintain.

In particular, the below events emitted by the GhoToken contract use uint256 parameters for capacities and levels, while those values are stored as uint128 integers. Casting is implicit where the following events are called:

• FacilitatorAdded
• FacilitatorBucketCapacityUpdated
• BucketLevelChanged

Consider explicitly casting all integer values to their expected type when sending them as parameters of functions and events. It is advisable to review the entire codebase and apply this recommendation to all segments of code where the issue is found.

The repo is setup to work with both Hardhat and Foundry.

By running a local forked node via hardhat, you can deploy contracts with older versions of solidity (which forge struggles with). This is needed to deploy Antei and create and Antei Reserve in Aave. You can then run a task in hardhat to configure the Antei Reserve after the contracts are deployed.

Then you should be able to run forge tests against the local network, and it should take into account the state from the hardhat deployments and the hardhat configuration scripts - so that you can then run forge solidity tests on the configured antei reserves.

From my testing, this seems not to be working. It seems like the forge tests take into account the state updates on the local network of the deployments, but does not take into account the state from the configuration scripts.

This issue is solved when you can run a forge test against the Aave market with an appropriately configured reserve.

Reported By

• Openzeppelin

Scope

GHO can support nearly limitless facilitators. However, at launch, in the audited commit, Aave
will be the only GHO facilitator. As such, GHO can only be sourced by borrowing it through Aave's lending pool. Users pay interest to borrow GHO, at a rate set depending on their staking status. When repaying GHO debt via the LendingPool , users need to transfer the principal borrowed plus interest on the debt using the underlying asset (i.e., GHO).

Since GHO can only be sourced by borrowing, and assuming the interest rate is above zero, it becomes impossible for a GHO loan to be repaid without another GHO loan first being originated. The additional loan originated is necessary to create a secondary market where the first borrower can source GHO to pay for accrued interest. This can have a domino effect on how GHO liquidity is borrowed and repaid depending on several factors, such as interest rates on borrowed GHO and supplied collateral, discounted interest rates for stakers borrowing GHO, and minting capacity limits.

This can be mitigated with non-interest-baring GHO liquidity in the market. Consider implementing additional facilitators with alternative ways to source GHO.

Reported By
OpenZeppelin

Scope
The setFacilitatorBucketCapacity function in the GhoToken contract allows the owner of the contract to either increase or decrease a given facilitator's bucket's maxCapacity . This storage variable represents the total amount of GHO tokens that a facilitator can mint. In the case of the Aave pool, the first facilitator, these tokens are lent to borrowers, and the amount of GHO tokens that can be borrowed is limited to this
maxCapacity .

The Aave facilitator also works as a price stabilizer, since users can borrow, repay and liquidate
GHO at 1 USD, which creates an arbitrage opportunity for borrowers:

• if the price is below the peg, borrowers can purchase GHO at a lower price in a parallel market, and then repay their debt using those tokens in the pool, making a profit.

• if the price is above the peg, users can borrow GHO at 1 USD and sell it in the parallel market.

The issue lies in the fact that when the maxCapacity is reached, under a scenario of high buy pressure in the market, arbitrageurs will not be able to borrow additional GHO from the Aave pool, and therefore not be able to sell and stabilize market prices around 1 USD. Likewise, if the maxCapacity is reached, it becomes impossible to source GHO from the facilitator to pay for accrued interest, unless bought in the parallel market, increasing the buy pressure and increasing its price further.

Given the necessary speed of response by the DAO in the case of worsening market conditions and potential GHO depegging, consider further research and implementing an algorithmic variable interest rate with a DAO permissioned backstop that can override the interest rate.

** Reported By **

• Openzeppelin

Scope

Within GhoVariableDebtToken.sol there are several functions that try to inherit docstrings but
are not in the correct comment format. For instance:
• updateDiscountDistribution on line 258
• getDiscountPercent on line 317
• getBalanceFromInterest on line 322
• decreaseBalanceFromInterest on line 327 • rebalanceUserDiscountPercent on line 333 • updateDiscountLockPeriod on line 365
• getDiscountLockPeriod on line 372
• getUserRebalanceTimestamp on line 377

Suggestion
To ensure documentation is generated correctly, consider updating the comment format to adhere to the Solidity Style Guide on documentation style.

Reported By

• Openzeppelin

Scope
In the contract StakedTokenV2Rev4 , the event RewardsAccrued has an address parameter, user , that is not indexed.

To enable off chain monitoring, consider making the parameter indexed

discountToken can be confusing

stakedToken is not the best replacement

Consider another name to apply throughout.

Reported By

• Openzeppelin

Scope
Since solidity version 0.8.4, custom errors provide a cleaner, consistent, and more cost-
efficient way to explain to users why an operation failed.

Multiple instances of hardcoded require messages were found throughout the codebase. For example:

• In GhoDebtTokenBase.sol on lines 115, 127, 133, 144, 155, and 166
• In GhoIncentivizedERC20.sol on lines 181, 182, 201, 217, 237, and 238 • In GhoAToken.sol on lines 292, 294, 303, and 349
• In GhoVariableDebtToken.sol on lines 67, 75, 280, and 395
• In GhoToken.sol on lines 34, 38, 80, 107, 119, 120, 121, and 136-139

Additionally, there are instances where an Errors library is used. If this is not needed for backward compatibility, we suggest updating these cases as well.

For conciseness, consistency, and gas savings, consider replacing hardcoded require and revert messages with custom errors.

Reported By

• Openzeppelin

Scope
Throughout the codebase there are functions that have unused parameters.

Suggestions
• The calculateInterestRates function's params parameter.
• The mint function's caller , onBehalfOf , amount , and index parameters.
• The burn function's from , receiverOfUnderlying , amount , and index
parameters.
• The mintToTreasury function's amount and index parameters.
• The transferOnLiquidation function's from , to , and value parameters.
• The permit function's owner , spender , value , deadline , v , r , and s
parameters.
• The _transfer function's from , to , amount , and validate parameters.
• The _transfer function's from , to , and amount parameters.

To provide clarity that these parameters are not used, consider removing the names of the parameters and keeping the types to retain the function signatures.

Note: See report for links

The oracle was updated to meet the requirements for v3 - 8 decimal, USD price feeds.

Miguel updated the new version and made it far cleaner

Maybe worth still having an AnswerUpdated event in the constructor if any integrators depend on that event.

The initial _ghoTreasury can be set in the GhoAToken initialization params.

Reported By

• Openzeppelin

Scope

When a given user performs their first borrow, the execution flow does not differ from subsequent borrows. Around 1% gas savings can be achieved if the first borrow is handled separately, since there is no need to accrue interest and most user state values are already set to zero.

Suggestion
To reduce the initial GHO borrowing gas consumption for a user, consider updating the relevant code to be more performant.

Reported By

• Openzeppelin

Scope
In the contract GhoAToken.sol, the following functions are implemented but have no use, since
it is not possible to mint or transfer the GhoAToken:

• balanceOf
• scaledBalanceOf
• getScaledUserBalanceAndSupply
• totalSupply
• scaledTotalSupply
• permit
• _transfer
• _transfer

For clarity and conciseness, consider replacing the function bodies with a dev comment or revert statement as in mint and burn . In the cases of balances and token supply, a hard coded value might be more appropriate.

** Reported By **

• openzeppelin

scope
Throughout the codebase there are Solidity pragma statements that use an outdated or unpinned version of Solidity. For instance:
• The pragma statement on line 2 of GhoDiscountRateStrategy.sol .
• The pragma statement on line 2 of GhoOracle.sol .

• The pragma statement on line 2 of GhoAToken.sol .
• The pragma statement on line 2 of GhoVariableDebtToken.sol .
• The pragma statement on line 2 of ScaledBalanceTokenBase.sol . • The pragma statement on line 2 of IGhoAToken.sol .
• The pragma statement on line 2 of IGhoVariableDebtToken.sol .

Suggestion

• Consider taking advantage of the latest Solidity version to improve the overall readability and security of the codebase. Regardless of which version of Solidity is used, consider pinning the version consistently throughout the codebase to prevent bugs due to incompatible future releases.

So users may get very heavily discounts by taking positions early with relatively small stkAave sizes. These users should be rewarded highly, but maybe only for 6 months or a year, not indefinitley.

This could be a public rebalance discount function that users can call. It would check the last index of a the rebalanced user and if it was more than X time ago - the user being rebalanced would have their interest accrue and a new workingBalance could be recalculated.

Reported By

• Openzeppelin

Scope
The NatSpec comments in IGhoDiscountRateStrategy.sol and GhoDiscountRateStrategy.sol define the parameters debtBalance and discountTokenBalance as "The address of the reserve" and "The liquidity available in the
reserve" respectively, when they should be defined as "The debt balance of the user" and "The discount token balance of the user" respectively as they are in refreshDiscountPercent .

For clarity and correctness, consider updating the NatSpec definitions to properly define the parameters. It is advisable to review the entire codebase and apply this recommendation to all NatSpec comments.

Reported By

• Openzeppelin

Scope
The mint and burn functions cast the newBucketValue variable, defined as uint256 , to uint128 . In the mint function, the newBucketLevel variable could potentially be greater
than the size of a uint128 , but it is being implicitly caught by this require statement.

To reduce the potential for errors in the future, consider using SafeCast functions like toUint128 when down-casting integers throughout the codebase to revert in case the
integer is too large.

Reported By

• Openzeppelin

Scope
In GhoToken contract, the variable _facilitatorsCount is defined but never used. For
code cleaniness and clarity, consider removing any unused variables.

Using a seperate _ghoTreasury rather than the default _treasury value in the GhoAToken.

The _treasury in all other reserves is generally the same, while the _ghoTreasury can be different. That being said, the _treasury variable is not used in the GhoAToken, so can be used to reference the _ghoTreasury (which may even be the same as the treasury for other assets).

Reported By

• Openzeppelin

Scope
In the contract GhoAToken.sol, there are functions that are labeled as virtual but the contract is
not meant to be inherited:
• On line 47
• On line 102
• On line 112
• On line 134
• On line 142
• On line 163
• On line 169

Suggestion
For clarity and code cleanliness, consider removing the virtual label from functions that are not intended to be overridden.

Reported By

• Openzeppelin

Scope
Different from other events in the system where both the old and new values set are emitted as parameters, the VariableDebtTokenSet event in the GhoAToken contract and the ATokenSet event in the GhoVariableDebtToken contract only emit the new value.

Suggestion
To favor consistency, consider emitting both the old and new values in these and any other events.

Review LendingPool Revision 2 vs Revision 3

Mainnet is v3 and the aave repo is v2

The highest level v3 contract is the same as the v2 contract. There maybe differences within the dependency contracts.

The discount strategy uses requires a minimum amount of debt in order to avoid the complexity of giving discounts to users with dust balances.

Reported By

• Openzeppelin

Scope
To provide more clarity on the price oracle answer being fixed to 1 USD with 8 decimals, consider changing the mutability of the latestAnswer function in GhoOracle.sol from
view to pure.

Rather than having entities with mint and burn addresses privileged to mint and burn on demand, we can shift to governance minting entities ASD up to their mint limit when they an entity is added or when an entities mint limit increases.

This avoids some issues brought about by the Aave V2 implementation. It will also help avoid additional transfers brought about by always minting and burning on demand.

This mean requires updates to the AnteiStableDollarEntities contract. Entities no longer need minter[] and burner[] arrays and the logic that revolves around these structures can be removed from the contract.

This is a temporary hybrid implementation so that the repay and discount functions can still be built out.
#17

uint256 discountScaled = (discount * WadRayMath.RAY) / index;

All contracts should be migrated to the latest solidity version, pending more extended analysis to ensure compatibility with the V2 market.

** Reported By **

• Openzeppelin

Scope
Within GhoVariableDebtToken.sol there are several functions that do not have docstrings. For
instance:
• allowance on line 189
• approve on line 193
• transferFrom on line 201
• increaseAllowance on line 205 • decreaseAllowance on line 209

Suggestion
To improve documentation generation and readability, consider adding docstrings for each function individually.

Reported By

• Openzeppelin

Scope
The interface IAaveIncentivesController.sol is compiled using a solidity version, specifically 0.8.10, where the pragma experimental ABIEncoderV2 is deprecated and the ABIEncoderV2 is enabled by default.

To adhere to the solidity documentation, consider either removing the deprecated pragma or
adding the recommended explicit pragma pragma abicoder v2; instead.

Setting the interest rate as immutable in the interest rate strategy saves 2100 gas on average for borrows and repayments. This requires a new contract to be deployed whenever the interest rate is updated, but I think the gas savings is worth it.

Reported By

• Openzeppelin

Scope
The whitepaper mentions that the total supply of GHO in a certain time t is given by the following formula

Where:

• SGHOt is the total supply of GHO at t
• CBi is the capacity of the bucket Bi

However, this does not hold in all cases of bucket capacity and level values. The setFacilitatorBucketCapacity function in the GhoToken contract allows the owner to change the bucket max capacity to values below the current bucket level.

This means that if the owner of the contract updates the bucket's capacity from P to M with a current bucket level of N (where M < N <= P), then the bucket would be virtually overflowed, and the inequation mentioned above will not hold, since the total circulating supply could be greater than the sum of all bucket's capacities.

Consider updating the whitepaper accordingly. Additionally, consider checking that the total supply SGHOt is not being used as the total supply in other out-of-scope sections of the code,
to avoid miscalculations.

Repaying GHO on behalf of another user records interest as paid for the sender, not the bor- rower

Reported By

• Openzeppelin

Scope

• Custom error messages

Suggesstions
Since Solidity version 0.8.4, custom errors provide a more consistent, cleaner, and more cost-
efficient way to explain to users why an operation failed. Throughout the codebase, instances of revert and/or require messages were found:

• In the GhoAToken.sol contract:
• the revert statement with the message OPERATION_NOT_PERMITTED .
• the revert statement with the message OPERATION_NOT_PERMITTED .
• the revert statement with the message OPERATION_NOT_PERMITTED .
• the revert statement with the message OPERATION_NOT_PERMITTED .
• the revert statement with the message OPERATION_NOT_PERMITTED .
• the revert statement with the message OPERATION_NOT_PERMITTED .
• the revert statement with the message OPERATION_NOT_PERMITTED .
• the require statement with the message VARIABLE_DEBT_TOKEN_ALREADY_SET .
• the require statement with the message Errors.POOL_ADDRESSES_DO_NOT_MATCH . • the require statement with the message Errors.UNDERLYING_CANNOT_BE_RESCUED . • In the GhoVariableDebtToken.sol contract:
• the require statement with the message CALLER_NOT_DISCOUNT_TOKEN .
• the require statement with the message CALLER_NOT_A_TOKEN .
• the require statement with the message ATOKEN_ALREADY_SET .
• the require statement with the message
DISCOUNT_PERCENT_REBALANCE_CONDITION_NOT_MET .

• the require statement with the message Errors.POOL_ADDRESSES_DO_NOT_MATCH . • the revert statement with the message Errors.OPERATION_NOT_SUPPORTED .
• the revert statement with the message Errors.OPERATION_NOT_SUPPORTED .
• the revert statement with the message Errors.OPERATION_NOT_SUPPORTED .
• the revert statement with the message Errors.OPERATION_NOT_SUPPORTED . • the revert statement with the message Errors.OPERATION_NOT_SUPPORTED . • the revert statement with the message Errors.OPERATION_NOT_SUPPORTED . • the require statement with the message Errors.INVALID_MINT_AMOUNT .
• the require statement with the message Errors.INVALID_BURN_AMOUNT . • In the ScaledBalanceTokenBase.sol contract:
• the require statement with the message Errors.INVALID_MINT_AMOUNT . • the require statement with the message Errors.INVALID_BURN_AMOUNT .

For consistency, consider replacing hard coded require and revert messages with the use of the Errors library. Additionally, for conciseness and gas savings, consider replacing the
Errors library with custom errors.

Initializing the reserve and adding ASD as an entity will be done through executing payload contracts called from aave governance.

This first should:
Initialized the asd reserve
enable asd borrowing
configure the asd oracle

The second should:
configure Aave as an entity within the ASD contract

This can be done in one if we can pre-determine the ASD aToken and variableDebtToken addresses.

Merge asdConfiguration and aave-v2-addresses into one file with separate config objects exported

in the mint function

  _burn(onBehalfOf, discountScaled.add(amountScaled));

should subtract amountScaled instead of add it.

Reported By

• Openzeppelin

Scope
Within GhoVariableDebtToken.sol there is a require statement on line 335 that requires multiple
conditions to be satisfied.

Suggestion
To simplify the codebase and to raise the most helpful error messages for failing require statements, consider having a single require statement per condition.

Review all licenses

Reported By

• Openzeppelin

Scope
The rescueTokens function allows the pool admin to rescue any mistakenly sent tokens from the GhoAToken contract. For regular Aave markets, adding a validation to prevent rescuing the underlying token address is necessary to avoid sweeping all users' deposits from the contract, but for the GHO token, it is not necessary since the supply side of the pool does not exist.
Consider allowing the pool admin to rescue GHO tokens that were mistakenly sent to the GhoAToken contract by removing this require statement.

Suggestion
Consider allowing the pool admin to rescue GHO tokens that were mistakenly sent to the GhoAToken contract by removing this require statement.

The discount token address is included in the discount strategy contract, but not used.

** Submitted By **

• OpenZeppelin

Scope
The rebalanceUserDiscountPercent function cannot be called until a certain period of time has passed, in order to allow a user to enjoy the applicable discount rate for a minimum amount of time.

However, anyone can bypass this timelock and force a user's discount rate to be updated by transferring any amount of stkAAVE into the their address, since that will trigger an automatic call to the updateDiscountDistribution function, which will execute the same actions as rebalanceUserDiscountPercent on both the sender and the recipient .

As an example, a user with a large active GHO principal debt who is entitled to a full discount rate for a given lock period, could effectively have it revoked in a scenario where governance decides to lower the discount rate by issuing a new discount rate strategy and anyone transferring stkAAVE to that user's address. Anyone with a vested interest over the GHO treasury would be incentivized to do so, since it would result in more interest paid to the treasury. The user themselves could inadvertently hurt their discount rate by simply staking additional AAVE or purchasing more stkAAVE .

Suggestion
In order to allow users entitled to a discount rate for a locked period to enjoy the full period, consider adding additional protections in the situations mentioned above to prevent their discount rate from being rebalanced.

Reported By

• Openzeppelin

Scope

The GhoToken.sol file is unlicensed. Consider choosing an open source license for the project and updating the SPDX-License-Identifier accordingly.

Add unchecked math were there is not the opportunity for errors

Both the AToken and the debt token need references to each other.

These are currently set via setter functions however they can be set as immutables when deploying the contracts.

The addresses of the AToken proxy and Variable Debt Token Proxy can be calculated before the reserve is created and those addresses should be used when deploying the AToken amd VariableDebtToken implementations.