-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feat: Vault Yield Implementation with Aave #6
Open
banasa44
wants to merge
15
commits into
openfort-xyz:feat/aave_vault
Choose a base branch
from
banasa44:feat/carles-vault-yield
base: feat/aave_vault
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Feat: Vault Yield Implementation with Aave #6
banasa44
wants to merge
15
commits into
openfort-xyz:feat/aave_vault
from
banasa44:feat/carles-vault-yield
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
banasa44
changed the title
Feat/carles vault yield
Feat: Vault Yield Implementation with Aave
Dec 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feat: Vault Yield Implementation
Summary
This work was done for OpenFort during a testing week, therefor this PR will include an extensive report of all the work done, and all the decision-making will be described.
In this pull request, you will find an enhanced functionality for OpenFort Vaults, particularly, the support for Aave yield protocols to generate interest for the users within the vaults.
The work has been done trying to adapt to existing functionalities as much as possible. However other implementations will be considered in this report.
How to Install and Test
Follow these steps to install and test the changes in this branch:
Prerequisites
Install Foundry (if not already installed):
curl -L https://foundry.paradigm.xyz | bash foundryup
Installation and build
git clone https://github.com/banasa44/fork-openfort-chain-abstraction.git cd openfort-chain-abstraction git checkout feat/carles-vault-yield
Testing
For this test, you will need to fork a testnet to recreate the interaction with Aave protocol Smart Contracts deployed on an Aave-supported testnet.
Sepolia may be a nice choice.
Run all tests with the Testnet fork enabled:
To run individual tests, use the --match-contract or --match-test flags:
Notes for testing
I provided you an
example.env
file with the necessary environment variables to make the test work.I left the Aave Sepolia addresses so you don't need to change them, however, if you prefer to test on another testnet or use another token you can check on the Aave address-book repository:
https://github.com/bgd-labs/aave-address-book/tree/main/src
For the test to work the address doing the deposit (owner), has to have token balance.
Notes for reviewers
Files Changed/Added
deployChainAbstractionSetup
, added functionality to deploy and register AaveVaults.CheckAaveTokenStatus
auxiliary script to check if some token is supported by Aave protocol.aaveVault
vault supplying tokens to Aave protocol.AaveVault.t.sol
E2E test for AaveVault.deployChainAbstractionSetup
Added the deployment and register in the VaultManager of AaveVaults. To do so, an auxiliary script was used, to check if the the underlying token is supported by the Aave Protocol. Additionally, I added a function to retrieve the aToken associated with the underlying token.
AaveVault
The core piece of the PR, this AaveVault inherits from BaseVault and overrides/adds:
initialize
function_afterDeposit
_previewDeposit
_afterWithdraw
_totalAssets
1. Custom
initialize
Functioninitialize
function is implemented to handle additional parameters during smart contract deployment.aToken
for the underlying token (automated via theCheckAaveTokenStatus
script)..env
file for each specific chain.super.initialize(_vaultManager, _underlyingToken);
to leverage the parent contract's initialization logic.2.
_afterDeposit
_beforeDeposit
could arguably suffice in this specific case,_afterDeposit
was chosen for clarity and consistency in handling post-deposit logic.3.
_previewDeposit
_afterDeposit
necessitated changes to_previewDeposit
:newShares
usingtotalAssets
.totalAssets
fetches theaToken.balanceOf(address(this))
(which updates only after supply), subtractingamount
fromvirtualPriorTokenBalance
became incorrect.4.
_afterWithdraw
aTokens
and retrieve the underlying tokens (plus proportional interest) for the user.5.
_totalAssets
aToken
associated with the Vault instead of the underlying token.aToken
.Considerations and Decision Making
IYieldVault vs IVault
To keep things simple and fit with the existing protocol, I decided not to use the
IYieldVault
interface. The flexibility of thebefore
andafter
hooks was enough to enhance the deposit and withdrawal functionalities.The overridden hooks only added the necessary functionality to work with Aave while keeping the input arguments the same. This was enough for this specific use case.
That said, Aave has specific methods for L2 pools:
Aave L2 Pool Documentation
These methods require function arguments to be passed as byte-encoded inputs. This reduces transaction costs on L2s. If we were to use these L2-specific methods, we would need to add specific deposit and withdrawal functions in the
VaultManager
andAaveL2Vault
. AnL2Encoder
could be used by OpenFort's backend to properly call these functions on theVaultManager
. In this case, using theIYieldVault
interface would make more sense.Additionally, if we plan to add features like calculating the interest earned by each user or similar functions, using
IYieldVault
would become more valuable. It would provide a clearer and more structured way to manage these advanced features.Having a variety of Vaults
Managing different types of vaults can create organizational challenges. Currently, all vaults are registered like this:
I believe this could be improved by updating the mapping to store an array of structs for each ERC20 token. Or adding an additional mapping form VaultAddress to VaultType (maybe this is a cheaper and easier approach).
Why Chose Aave for OpenFort Vaults
I've chosen Aave for the yielding vaults because it offers simplicity, safety, and instant access to funds. It works with over 30 assets, including key stablecoins like USDC, DAI, and USDT, and supports most Layer 2s such as Polygon, Arbitrum, Optimism, and Avalanche.
Compared to alternatives like Compound, which supports fewer chains, or MakerDAO, which is limited to DAI, Aave stands out for its broader coverage and multi-chain flexibility.
Summary Table: Aave Alternatives
ToDo
I would have loved to make tests more robust.
The current test just checks E2E, if a user deposits, and time passes, the user will get back more money.
I added another test to see the distribution of shares.
However, more robust and precise tests ensuring precise workflow of deposits, yielding and withdrawals should be done (properly calculating the yield generated for each user (using shares), and the amount withdrawn any time even if the user doesn't withdraw all the amount he processes in the vault).
Sometimes there's a 1wei unit discrepancy while running the tests, it doesn't happen every time.
Sorry for writing such a loooong book 🙈🙈
Have a nice holiday!!!