Skip to content

Commit

Permalink
Merge pull request #12 from bgd-labs/feat/improved-tests
Browse files Browse the repository at this point in the history
Feat/improved tests
  • Loading branch information
kyzia551 authored Aug 16, 2024
2 parents 0698a73 + 9cf1cc0 commit c6befb0
Show file tree
Hide file tree
Showing 20 changed files with 1,206 additions and 1,158 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ abstract contract ERC4626StataTokenUpgradeable is ERC4626Upgradeable, IERC4626St
SignatureParams memory sig,
bool depositToAave
) public returns (uint256) {
// TODO: add tests
ERC4626StataTokenStorage storage $ = _getERC4626StataTokenStorage();
IERC20Permit assetToDeposit = IERC20Permit(depositToAave ? asset() : address($._aToken));
IERC20Permit assetToDeposit = IERC20Permit(
depositToAave ? asset() : address(_getERC4626StataTokenStorage()._aToken)
);

try
assetToDeposit.permit(_msgSender(), address(this), assets, deadline, sig.v, sig.r, sig.s)
Expand Down
67 changes: 21 additions & 46 deletions src/periphery/contracts/static-a-token/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,71 +37,46 @@ For this project, the security procedures applied/being finished are:
- The test suite of the codebase itself.
- Certora audit/property checking for all the dynamics of the `stataToken`, including respecting all the specs of [EIP-4626](https://eips.ethereum.org/EIPS/eip-4626).

## Upgrade Notes Umbrella
## Upgrade Notes StataTokenV2

### Inheritance

Interface inheritance has been changed so that `IStaticATokenLM` implements `IERC4626`, making it easier for integrators to work with the interface.
The current `Initializable` has been removed in favor of the new [Initializable](https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/9a47a37c4b8ce2ac465e8656f31d32ac6fe26eaa/contracts/proxy/utils/Initializable.sol) following the [`ERC-7201`](https://eips.ethereum.org/EIPS/eip-7201) standard.
To account for the shift in storage, a new `DeprecationGap` has been introduced to maintain the remaining storage at the current position.
The `StaticATokenLM`(v1) was based on solmate.
To allow more flexibility the new `StataTokenV2`(v2) is based on [open-zeppelin-upgradeable](https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable) which relies on [`ERC-7201`](https://eips.ethereum.org/EIPS/eip-7201) which isolates storage per contract.

### Misc
The `StataTokenV2` is seperated in 3 different contracts, where `StataTokenV2` inherits `ERC4626StataToken` and `ERC20AaveLM`.

Permit params have been excluded from the METADEPOSIT_TYPEHASH as they are not necessary.
Potential frontrunning of the permit via mempool observation is unavoidable, but due to wrapping the permit execution in a `try..catch` griefing is impossible.
- `ERC20AaveLM` is an abstract contract implementing the forwarding of liquidity mining from an underlying AaveERC20 - an ERC20 implementing `scaled` functions - to a wrapper contract.
- `ERC4626StataToken` is an abstract contract implementing the [EIP-4626](https://eips.ethereum.org/EIPS/eip-4626) methods for an underlying aToken. In addition it adds a `latestAnswer`.
- `StataTokenV2` is the main contract stritching things together, while adding `Pausability`, `Rescuability`, `Permit` and the actual initialization.

### Features
### MetaTransactions

MetaTransactions have been removed as there was no clear use-case besides permit based deposits ever used.
To account for that specific use-case a dedicated `depositWithPermit` was added.

### Direct AToken Interaction

In v1 deposit was overleaded to allow underlying & aToken deposits.
While this appraoch was fine it seemed unclean and caused some confusion with integrators.
Therefore v2 introduces dedicated `depositATokens` and `redeemATokens` methods.

#### Rescuable

[Rescuable](https://github.com/bgd-labs/solidity-utils/blob/main/src/contracts/utils/Rescuable.sol) has been applied to
the `StaticATokenLM` which will allow the ACL_ADMIN of the corresponding `POOL` to rescue any tokens on the contract.
the `StataTokenV2` which will allow the ACL_ADMIN of the corresponding `POOL` to rescue any tokens on the contract.

#### Pausability

The `StaticATokenLM` implements the [PausableUpgradeable](https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/9a47a37c4b8ce2ac465e8656f31d32ac6fe26eaa/contracts/utils/PausableUpgradeable.sol) allowing any emergency admin to pause the vault in case of an emergency.
The `StataTokenV2` implements the [PausableUpgradeable](https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/9a47a37c4b8ce2ac465e8656f31d32ac6fe26eaa/contracts/utils/PausableUpgradeable.sol) allowing any emergency admin to pause the vault in case of an emergency.
As long as the vault is paused, minting, burning, transfers and claiming of rewards is impossible.

#### LatestAnswer

While there are already mechanisms to price the `StaticATokenLM` implemented by 3th parties for improved UX/DX the `StaticATokenLM` now exposes `latestAnswer`.
`latestAnswer` returns the asset price priced as `underlying_price * excahngeRate`.
While there are already mechanisms to price the `StataTokenV2` implemented by 3th parties for improved UX/DX the `StataTokenV2` now exposes `latestAnswer`.
`latestAnswer` returns the asset price priced as `underlying_price * exchangeRate`.
It is important to note that:

- `underlying_price` is fetched from the AaveOracle, which means it is subject to mechanisms implemented by the DAO on top of the Chainlink price feeds.
- the `latestAnswer` is a scaled response returning the price in the same denomination as `underlying_price` which means the sprice can be undervalued by up to 1 wei
- while this should be obvious deviations in the price - even when limited to 1 wei per share - will compound per full share

### Storage diff

```
git checkout main
forge inspect src/periphery/contracts/static-a-token/Stata4626LM.sol:Stata4626LM storage-layout --pretty > reports/StaticATokenStorageBefore.md
git checkout project-a
forge inspect src/periphery/contracts/static-a-token/Stata4626LM.sol:Stata4626LM storage-layout --pretty > reports/StaticATokenStorageAfter.md
make git-diff before=reports/StaticATokenStorageBefore.md after=reports/StaticATokenStorageAfter.md out=StaticATokenStorageDiff
```

```diff
diff --git a/reports/StaticATokenStorageBefore.md b/reports/StaticATokenStorageAfter.md
index a7e3105..89e0967 100644
--- a/reports/StaticATokenStorageBefore.md
+++ b/reports/StaticATokenStorageAfter.md
@@ -1,7 +1,6 @@
| Name | Type | Slot | Offset | Bytes | Contract |
| ------------------ | ------------------------------------------------------------------------------ | ---- | ------ | ----- | ------------------------------------------------------------------------ |
-| \_initialized | uint8 | 0 | 0 | 1 | src/periphery/contracts/static-a-token/Stata4626LM.sol:Stata4626LM |
-| \_initializing | bool | 0 | 1 | 1 | src/periphery/contracts/static-a-token/Stata4626LM.sol:Stata4626LM |
+| \_\_deprecated | uint256 | 0 | 0 | 32 | src/periphery/contracts/static-a-token/Stata4626LM.sol:Stata4626LM |
| name | string | 1 | 0 | 32 | src/periphery/contracts/static-a-token/Stata4626LM.sol:Stata4626LM |
| symbol | string | 2 | 0 | 32 | src/periphery/contracts/static-a-token/Stata4626LM.sol:Stata4626LM |
| decimals | uint8 | 3 | 0 | 1 | src/periphery/contracts/static-a-token/Stata4626LM.sol:Stata4626LM |
```

### Umbrella upgrade plan

The upgrade can be performed independent(before) from any umbrella changes as it has no dependencies.
The upgrade will need to:

- upgrade the `StaticATokenFactory` with a new version, replacing the `STATIC_A_TOKEN_IMPL`.
- upgrade existing stata tokens via `upgradeToAndCall` to the new implementation. While the tokens are already initialized, due to changing the `Initializable` the corresponding storage is lost.
41 changes: 0 additions & 41 deletions src/periphery/contracts/static-a-token/StataOracle.sol

This file was deleted.

1 change: 1 addition & 0 deletions src/periphery/contracts/static-a-token/StataTokenV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ contract StataTokenV2 is
) ERC20AaveLMUpgradeable(rewardsController) ERC4626StataTokenUpgradeable(pool) {
_disableInitializers();
}

modifier onlyPauseGuardian() {
if (!canPause(_msgSender())) revert OnlyPauseGuardian(_msgSender());
_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,18 @@ contract StaticATokenFactory is Initializable, IStaticATokenFactory {
revert NotListedUnderlying(reserveData.aTokenAddress);
bytes memory symbol = abi.encodePacked(
'stat',
IERC20Metadata(reserveData.aTokenAddress).symbol()
IERC20Metadata(reserveData.aTokenAddress).symbol(),
'v2'
);
address staticAToken = TRANSPARENT_PROXY_FACTORY.createDeterministic(
STATIC_A_TOKEN_IMPL,
PROXY_ADMIN,
abi.encodeWithSelector(
StataTokenV2.initialize.selector,
reserveData.aTokenAddress,
string(abi.encodePacked('Static ', IERC20Metadata(reserveData.aTokenAddress).name())),
string(
abi.encodePacked('Static ', IERC20Metadata(reserveData.aTokenAddress).name(), ' v2')
),
string(symbol)
),
bytes32(uint256(uint160(underlyings[i])))
Expand Down
31 changes: 0 additions & 31 deletions src/periphery/contracts/static-a-token/interfaces/IStataOracle.sol

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

interface IStataTokenV2 {
import {IERC4626StataToken} from './IERC4626StataToken.sol';
import {IERC20AaveLM} from './IERC20AaveLM.sol';

interface IStataTokenV2 is IERC4626StataToken, IERC20AaveLM {
/**
* @notice Checks if the passed actor is permissioned emergency admin.
* @param actor The reward to claim
Expand Down
Loading

0 comments on commit c6befb0

Please sign in to comment.