Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mkflow27/issue101 #157

Open
wants to merge 3 commits into
base: feat/add-conclusion-to-registry
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions rate-providers/AgEthRateProvider.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# Rate Provider: `AgEthRateProvider`

## Details
- Reviewed by: @mkflow27
- Checked by: @\<GitHub handle of secondary reviewer\>
- Deployed at:
- [ethereum:0xc276db339e551ecbe0ac323a7c4a5c6ca61813fe](https://etherscan.io/address/0xc276db339e551ecbe0ac323a7c4a5c6ca61813fe#code)

- Audit report(s):
- [August institutional loan audits](https://institutional-docs.fractalprotocol.org/o6pxRk8iuJtUnSajR9d6/smart-contracts/risks-and-audits)

## Context
The Airdrop Gain vault is designed to maximize your airdrops through a dual strategy: Earning Layer 2 (L2) airdrops and accessing mainnet DeFi opportunities. Here’s how it works:
Upon depositing, the vault issues a liquid token agETH that represents a claim on the deposited assets. This is an ERC-20 token which is a tradable and usable representation of your staked assets within the vault. The deposited assets are then bridged to partner L2 networks. This bridging allows the assets to be used in L2 ecosystems, which are eligible for various airdrop opportunities.

## Review Checklist: Bare Minimum Compatibility
Each of the items below represents an absolute requirement for the Rate Provider. If any of these is unchecked, the Rate Provider is unfit to use.

- [x] Implements the [`IRateProvider`](https://github.com/balancer/balancer-v2-monorepo/blob/bc3b3fee6e13e01d2efe610ed8118fdb74dfc1f2/pkg/interfaces/contracts/pool-utils/IRateProvider.sol) interface.
- [x] `getRate` returns an 18-decimal fixed point number (i.e., 1 == 1e18) regardless of underlying token decimals.

## Review Checklist: Common Findings
Each of the items below represents a common red flag found in Rate Provider contracts.

If none of these is checked, then this might be a pretty great Rate Provider! If any of these is checked, we must thoroughly elaborate on the conditions that lead to the potential issue. Decision points are not binary; a Rate Provider can be safe despite these boxes being checked. A check simply indicates that thorough vetting is required in a specific area, and this vetting should be used to inform a holistic analysis of the Rate Provider.

### Administrative Privileges
- [ ] The Rate Provider is upgradeable (e.g., via a proxy architecture or an `onlyOwner` function that updates the price source address).

- [x] Some other portion of the price pipeline is upgradeable (e.g., the token itself, an oracle, or some piece of a larger system that tracks the price).
- upgradeable component: `LendingPool` ([ethereum:0xe1B4d34E8754600962Cd944B535180Bd758E6c2e](https://etherscan.io/address/0xe1B4d34E8754600962Cd944B535180Bd758E6c2e))
- admin address: [ethereum:0x87ff94bB7709c70c6B2018FED12E4Ce0ABbf30Ce](https://etherscan.io/address/0x87ff94bB7709c70c6B2018FED12E4Ce0ABbf30Ce)
- admin type: multisig
- multisig threshold/signers: 3/5

- upgradeable component: `LRTOracle` ([ethereum:0x349A73444b1a310BAe67ef67973022020d70020d](https://etherscan.io/address/0x349A73444b1a310BAe67ef67973022020d70020d))
- admin address: [ethereum:0xb3696a817D01C8623E66D156B6798291fa10a46d](https://etherscan.io/address/0xb3696a817D01C8623E66D156B6798291fa10a46d#code)
- admin type: multisig
- multisig threshold/signers: 6/8
- timelock: 10 days

### Oracles
- [x] Price data is provided by an off-chain source (e.g., a Chainlink oracle, a multisig, or a network of nodes).
#### Lending Pool
- source: All whitelisted loans (as of this review 5) can call `notifyPrincipalRepayment` with `effectiveLoanAmount` & `principalRepaid`. Depending on what parameters are supplied, the `globalLoansAmount` is changed. This function does not have any token transfers associated with it.
- source address:
- [ethereum:0x0014F00f0C9cF179F932Ae15A62d095F8B41349F](https://etherscan.io/address/0x0014F00f0C9cF179F932Ae15A62d095F8B41349F)
- [ethereum:0xB3862123c98c96bd49Cd456604C45BFb347977De](https://etherscan.io/address/0xB3862123c98c96bd49Cd456604C45BFb347977De#code)
- [ethereum:0xEeE4414151de8BE6b537a33d57380461d63EE680](https://etherscan.io/address/0xEeE4414151de8BE6b537a33d57380461d63EE680)
- [ethereum:0x694BdB833224cE459c098915100ef776f2ab2BeB](https://etherscan.io/address/0x694BdB833224cE459c098915100ef776f2ab2BeB)
- [ethereum:0x38FcA3F85FD01b06b3555FAAac8C3fF38a50e56e](https://etherscan.io/address/0x38FcA3F85FD01b06b3555FAAac8C3fF38a50e56e)
- any protections? NO: currently these loan contracts are not verified

#### RsEthPriceOracle
- source: Custom RsEthPrice calculates price based on stakedAsset value received from Eigenlayer.
- Source address: [ethereum:0x349A73444b1a310BAe67ef67973022020d70020d](https://etherscan.io/address/0x349A73444b1a310BAe67ef67973022020d70020d)



- [ ] Price data is expected to be volatile (e.g., because it represents an open market price instead of a (mostly) monotonically increasing price).

### Common Manipulation Vectors
- [x] The Rate Provider is susceptible to donation attacks.
The rate can be changed by sending `_underlyingAsset` to the vault.
```solidity
function _getTotalAssets() internal view virtual override returns (uint256) {
// [Liquidity] + [the delta of all ACTIVE loans managed by this pool]
return globalLoansAmount + _underlyingAsset.balanceOf(address(this));
}
```

## Additional Findings
To save time, we do not bother pointing out low-severity/informational issues or gas optimizations (unless the gas usage is particularly egregious). Instead, we focus only on high- and medium-severity findings which materially impact the contract's functionality and could harm users.

## Conclusion
**Summary judgment: UNSAFE**
It is impossible to verify the functionality of this rate provider as an essential rate calculation component is unverified on Etherscan.

69 changes: 69 additions & 0 deletions rate-providers/HordeEthRateProvider.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# Rate Provider: `HETHRateProvider`

## Details
- Reviewed by: @mkflow27
- Checked by: @\<GitHub handle of secondary reviewer\>
- Deployed at:
- [ethereum:0x5A2295f0b8A1f2b9bF26b9549Ce808c68e1a3F5f](https://etherscan.io/address/0x5A2295f0b8A1f2b9bF26b9549Ce808c68e1a3F5f#readProxyContract)
- Audit report(s):
- [Horde Eth audits](https://docs.hord.fi/security/smart-contract-audits)

## Context
In doing so, you will be given a token called hETH. hETH is an ERC-20 standard token based on Ethereum. hETH represents both how much ETH you deposited and when you deposited it combined with rewards. The ratio includes rewards that Hord node operators earn from:
- The Beacon Chain itself
- Priority fees from block proposals
- MEV rewards from block proposals

More specifically, the value of hETH is determined by the following ratio:
hETH price in ETH = (amount of ETH deposited + ETH rewards generated) / amount of hETH minted.

## Review Checklist: Bare Minimum Compatibility
Each of the items below represents an absolute requirement for the Rate Provider. If any of these is unchecked, the Rate Provider is unfit to use.

- [x] Implements the [`IRateProvider`](https://github.com/balancer/balancer-v2-monorepo/blob/bc3b3fee6e13e01d2efe610ed8118fdb74dfc1f2/pkg/interfaces/contracts/pool-utils/IRateProvider.sol) interface.
- [x] `getRate` returns an 18-decimal fixed point number (i.e., 1 == 1e18) regardless of underlying token decimals.

## Review Checklist: Common Findings
Each of the items below represents a common red flag found in Rate Provider contracts.

If none of these is checked, then this might be a pretty great Rate Provider! If any of these is checked, we must thoroughly elaborate on the conditions that lead to the potential issue. Decision points are not binary; a Rate Provider can be safe despite these boxes being checked. A check simply indicates that thorough vetting is required in a specific area, and this vetting should be used to inform a holistic analysis of the Rate Provider.

### Administrative Privileges
- [x] The Rate Provider is upgradeable (e.g., via a proxy architecture or an `onlyOwner` function that updates the price source address).
- admin address: [ethereum:0x086A6d9FD61758096CF4F394AE7C1F9B6b4EEC14](https://etherscan.io/address/0x086A6d9FD61758096CF4F394AE7C1F9B6b4EEC14)
- admin type: multisig
- multisig threshold/signers: 2/3 (Hord governance)


- [x] Some other portion of the price pipeline is upgradeable (e.g., the token itself, an oracle, or some piece of a larger system that tracks the price).
- upgradeable component: `HordETHStakingManager` ([ethereum:0x5bBe36152d3CD3eB7183A82470b39b29EedF068B](https://etherscan.io/address/0x5bBe36152d3CD3eB7183A82470b39b29EedF068B))
- admin address: [ethereum:0x086A6d9FD61758096CF4F394AE7C1F9B6b4EEC14](https://etherscan.io/address/0x086A6d9FD61758096CF4F394AE7C1F9B6b4EEC14)
- admin type: multisig
- multisig threshold/signers: 2/3 (Hord governance)


### Oracles
- [ ] Price data is provided by an off-chain source (e.g., a Chainlink oracle, a multisig, or a network of nodes).
- source:
- source address:
- any protections?
- comment: impossible to verify as a downstream contract is not verified on Etherscan.

- [ ] Price data is expected to be volatile (e.g., because it represents an open market price instead of a (mostly) monotonically increasing price).
- description:
- should be:
- comment: impossible to verify as a downstream contract is not verified on Etherscan.


### Common Manipulation Vectors
- [ ] The Rate Provider is susceptible to donation attacks.
- comment: impossible to verify as a downstream contract is not verified on Etherscan.


## Additional Findings
To save time, we do not bother pointing out low-severity/informational issues or gas optimizations (unless the gas usage is particularly egregious). Instead, we focus only on high- and medium-severity findings which materially impact the contract's functionality and could harm users.

## Conclusion
**Summary judgment: UNSAFE**

It is impossible to verify the functionality of this rate provider as an essential rate calculation component is unverified on Etherscan.
35 changes: 35 additions & 0 deletions rate-providers/registry.json
Original file line number Diff line number Diff line change
Expand Up @@ -1736,6 +1736,41 @@
"upgradeableComponents": [],
"isAllowed": "true",
"conclusion": "This rate provider should work with Balancer pools. However due to the time-boxed nature of this review and the high complexity of the underlying Tokemak Autopool product, this review could not cover the total path of how the rate is computed. The approach used to rate calculation is the common totalAssets / totalSupply approach usually used by yield-bearing vault type products. One thing to mention is that for a user, there are essentially two ways to exit an Autopool. Either, by withdrawing the ERC4626 vault's `asset` or selling balETH into the the pool where this rate provider is used. Depending on withdraw size, the rate the pool uses can differ between the rate used on `withdraw` (hint:slippage) from the Vault. For more information also see the developer comments in `AutopoolDebt.sol:withdraw()`.Additionally During initial Autopool deployment rates are expected to be more dynamic. For more context see the developers comments> Right now the the balETH Autopool has done two rebalances and there is value loss associated with that... slippage and fee's swapping from WETH to wstETH/ETHx/rsETH. Since its so early in the deployment, it also hasn't performed any reward claiming and auto-compounds so it hasn't had a chance to make up lost value yet. We'd expect this on every Autopool for the first few days after the first rebalances.The suggestions is to revisit this rate provider review once the pool has gained traction and conduct a more thorough review of the underlying Tokemak system. "
},
"0x5A2295f0b8A1f2b9bF26b9549Ce808c68e1a3F5f": {
"asset": "0x5bBe36152d3CD3eB7183A82470b39b29EedF068B",
"name": "HETHRateProvider",
"summary": "unsafe",
"review": "./HordeEthRateProvider.md",
"warnings": {
"verification": "This rate provider does not have all relevant contracts to the rate provider verified on the respective chain explorer sites. Without contract verification, it is not possible to know the underlying mechanics which can control or manipulate the provided rate. Therefore no formal risk assessment can be completed on this contract related to the typical review process. Trust assumptions are ultimately the responsibility of the user."
},
"factory": "",
"upgradeableComponents": [],
"isAllowed": "true",
"conclusion": "It is impossible to verify the functionality of this rate provider as an essential rate calculation component is unverified on Etherscan."
},
"0xC276db339E551eCbe0ac323a7c4A5c6ca61813Fe": {
"asset": "0xe1B4d34E8754600962Cd944B535180Bd758E6c2e",
"name": "AgEthRateProvider",
"summary": "unsafe",
"review": "./AgEthRateProvider.md",
"warnings": {
"verification": "This rate provider does not have all relevant contracts to the rate provider verified on the respective chain explorer sites. Without contract verification, it is not possible to know the underlying mechanics which can control or manipulate the provided rate. Therefore no formal risk assessment can be completed on this contract related to the typical review process. Trust assumptions are ultimately the responsibility of the user."
},
"factory": "",
"upgradeableComponents": [
{
"entrypoint": "0xe1B4d34E8754600962Cd944B535180Bd758E6c2e",
"implementationReviewed": "0x1e367a83699119165cc748BD0714D8C2726973ce"
},
{
"entrypoint": "0x349A73444b1a310BAe67ef67973022020d70020d",
"implementationReviewed": "0xbABc1810DD38323bd63a68F562A9D031A6A7956C"
}
],
"isAllowed": "true",
"conclusion": "It is impossible to verify the functionality of this rate provider as an essential rate calculation component is unverified on Etherscan."
}
},
"fantom": {
Expand Down
1 change: 1 addition & 0 deletions test/schema.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const schema = {
only18decimals: { type: "string" },
eoaUpgradeable: { type: "string" },
chainlink: { type: "string" },
verification: { type: "string" },
},
additionalProperties: false,
minProperties: 0
Expand Down
Loading