Description:
To allow maximum flexibility when the vault deployer is choosing its flavour, the vault would benefit from the possibility of leaving permissioned functions callable by anyone.

This means that the permissioned functions are initially closed (as no roles have been granted yet and roles are not open) but can be open by the role_manager (if it has not been burned setting it to 0).

The granularity of control for permissioned functions is per role (meaning that full roles can be opened to the public but that means that all the functions with that role assigned will be open).

For that, we need to replace _enforce_role (https://github.com/jmonteer/yearn-vaults-v3/blob/fdf9b73ed690897c717174a71c410121140bf2d7/contracts/VaultV3.vy#L852)
for something like
assert (role in self.roles[account]) or self.open_roles[role] # dev: not allowed

set_open_role function should be added too

tests checking that functions can be called by others should be open too

Dependencies:
NA

Acceptance criteria:

• code in VaultV3.vy
• tests

Payment Trigger:
PR is merged
Category:
DEV
Level:
0

We have different options:

• Implement at core level (difficult due to interface)
• Implement at wrapper / router level

Implement Share Management Functions

Description:
Implement deposit
Implement withdraw
Implement share value functions
Implement ERC20 functions

Dependencies:
NA
Acceptance criteria:
Required functions are implemented
Payment Trigger:
PR is merged
Category:
DEV
Level:
1

Create tests for strategy management functions

Description:
We need tests for strategy management functions. There's a short list here:
#20
Dependencies:
NA
Acceptance criteria:
Tests are written and passing
Payment Trigger:
PR is merged
Category:
DEV
Level:
1

Problem

code generating problems on CI is:

chain.pending_timestamp = 1
chain.mine(timestamp=chain.pending_timestamp)

which easies understanding tests but generates problems when running tests on CI.

Example error:

Example CI:
https://github.com/jmonteer/yearn-vaults-v3/runs/7994352545?check_suite_focus=true

SOLUTION

Save initial timestamp at test start up and use it to sum it every time we mine/advance in time

Description:

• implement processReport in the vault and write corresponding tests.

processReport should handle strategy updates to the vault regarding any gains or losses it incurs. the vault should update its own registry of the strategy's debt, gains and losses.

• write tests to handle processReport as well.

Dependencies:
N/A

Excluded:

• healthcheck is not implemented or handled
• feemanager module is not implemented or handled

Acceptance criteria:

• implement processReport
• implement unit tests associated with processReport

Payment Trigger:
PR is merged
Category:
<>
Level:
<>

Implement Strategy Management Functions

Description:
Implement addStrategy
Implement removeStrategy
Implement migrateStrategy
Implement right structure of strategies

Dependencies:
NA
Acceptance criteria:
Required functions are implemented and testing
Payment Trigger:
PR is merged
Category:
DEV
Level:
1

DECIMALS max value is 256 but that should be much less.

Description:

• implement depositLimit, a way to cap deposits into a vault.

Dependencies:
N/A

Acceptance criteria:

• introduce new variable depositLimit into the vault
• add a function availableDepositLimit to indicate how much you can deposit into the vault
• add setter for depositLimit
• modify deposit to account for depositLimit
• add test for depositLimit setter
• update deposit test for depositLimit
• update e2e test for depositLimit

Payment Trigger:
PR is merged
Category:
DEV
Level:
1

Description:
Due to profit distribution being done in discrete events (calls to processReport function), there is a MEV opportunity. The profit could be annotated immediately, allowing a depositor to deposit a big amount of tokens in the vault just before the processReport and take the majority of the profit, then immediately withdraw. There is a similar situation with losses being frontrunnable.

In v2, a patch was implemented that locked profit for some hours. It solved the issue but it's still not the optimal solution for the long term.

For V3, the best option is to design a profit distribution mechanism that works as a profit buffer that is releasing the profit gradually over a certain period of time, specified at deployment time.

For example, if a vault has a 7 day profit buffer, the profit will be fully unlocked 7 days after the last processReport call, at a linear rate. All the profit will go to that buffer and be smoothly distributed.

The buffer will be distributed per second at profitUnlockedPerSecondPerToken.

Each processReport call will add profit into the buffer, changing profitUnlockedPerSecondPerToken.

Withdrawals and deposits change the amount of totalSupply, so will change profitUnlockedPerSecondPerToken too.

Losses would be immediately removed from the buffer ensuring that, if the TIME_TO_UNLOCK_BUFFER is correctly selected, it will be a lot more difficult to avoid losses by front running a processReport (as profits won't have been distributed earlier)

Dependencies:
NA

Acceptance criteria:

• economic scenarios analysis and simulations is attached to this issue
• code is implemented in vault
• tests are implemented and running

Payment Trigger:
PR is merged

Category:
DEV

Level:
2

Implement BaseStrategy (Solidity)

Description:
Create BaseStrategy contract so that writing a strategy is as simple as possible.

BaseStrategy is an abstract contract that is inherited by actual strategies.

Use BaseStrategy from the v2 rewrite in solidity: https://github.com/saltyfacu/vaults-hardhat/blob/develop/contracts/core/BaseStrategy.sol

This version should leave Access Control references as:
# TODO: permissioned: ROLE to indicate that, when the AccessControlRegistry contract is created, strategies should reference to it

Create several TestStrategyFLAVOR.sol file in test folder that implements the simplest possible instance of a strategy (by just moving tokens) so it can be used during tests.

Flavors of strategies to test:

• regular liquid strategy
• strategy that locks liquidity (not possible to withdraw until some time passes)
• strategy that losses money when withdrawing only (similar to slippage when getting out, or withdrawal fee)

Dependencies:
NA

Acceptance criteria:

• contracts/BaseStrategy.sol is created
• contracts/test/TestStrategyLiquid.sol is created and tested
• contracts/test/TestStrategyLocked.sol is created and tested
• contracts/test/TestStrategyLossy.sol is created and tested

Payment Trigger:
PR is merged
Category:
DEV
Level:
1

permit follows the same abi as the one used on vault v2 <0.4.4.

On 0.4.4 permit follows eip-2612. This should be changed: https://github.com/jmonteer/yearn-vaults-v3/blob/master/contracts/VaultV3.vy#L930

Tests for Role based access

Description:
Ensure that all permissioned functions are under role enforcement in one of the following three roles:

• DEBT_MANAGER: updateDebt and setters
• STRATEGY_MANAGER: add remove and migrate strategies and their setters
• ACCOUNTING_MANAGER: process reports

Also, ensure that the governance role can do role changes successfully.

Dependencies:
Bounty #44 is done

Acceptance criteria:
All tests are implemented and running.
¡
Payment Trigger:
PR is merged
Category:
DEV
Level:
1

During a process_report call function, we add the gain or loss recorded into the StrategyParams struct.

https://github.com/jmonteer/yearn-vaults-v3/blob/2095b9c6931c854b57c746ee3d1868dfdcbf5bdc/contracts/VaultV3.vy#L620
https://github.com/jmonteer/yearn-vaults-v3/blob/2095b9c6931c854b57c746ee3d1868dfdcbf5bdc/contracts/VaultV3.vy#L597

While reviewing this function, I realised it's just for logging purposes as it is not used elsewhere.

Should we remove it?

v1

Our legacy registry is a single contract that allows to create and endorse a new vault cloning a specific vault version. It holds a registry of vaults version and a registry of token<>vault.

Only governance can endorse vault owned by governance.
You can only have one vault per token<>version

v2

The second version of the registry I have recently deployed is two contracts a VaultRegistry and a ReleaseRegistry.

• VaultRegistry: token<>Vault
• ReleaseRegistry: version<>Vault implementation.

This new version of the registry comes with more flexibility, it is possible to have more than one address in charge of endorsing vaults and vault can be owned by different whitelisted addresses.

The vaults are typed and we can have more than one vault for a token<>version pair.

v3

V3 will have to have a different ReleaseRegistry from v2 since we won't be cloning but using a vyper factory.

Description:
Tests should be parameterised with different tokens.

examples:
USDT has special cases around not complying exactly with ERC20
USDC 6 decimals
WBTC 8 decimals
some other with 2 decimals

vault should be 100% compatible with at least:
USDC, USDT, DAI, WBTC, WETH and YFI

Dependencies:
NA

Acceptance criteria:

• tests running for different flavors of ERC20 tokens

Payment Trigger:
PR is merged
Category:
DEV
Level:
2

Description:

• update withdraw in the vault to handle withdrawals from different strategies

Withdraw should iterate through the strategies, withdrawing and freeing funds where appropriate.

• write corresponding tests that test the base cases (i.e. handle cases where withdrawals > totalIdle)

Dependencies:
N/A

Acceptance criteria:

• update withdraw implementation
• add unit tests to handle withdrawing from strategies, including combinations from the three mocked strategies we have (e.g. queue containing liquid only, locked + liquid) etc

Payment Trigger:
PR is merged
Category:
DEV
Level:
1

Vault's Emergency Shutdown

Description:
Vault should implement a state of emergency for those situations that are not expected.

An emergency shutdown implies that:

• no new deposits will be accepted, only withdraws are available
• all the updateDebt calls will try their best to reduce debt to 0 (i.e. all maxDebtPerStrategies will be replaced with 0)
• updateDebtEmergency can be called (to force strategies to return funds to vault, even if they need to take losses)
• the EMERGENCY_MANAGER account will be granted all permissions, for this account to be able to call updateDebt and

The emergencyShutdown parameter will be set by a specific EMERGENCY_MANAGER role. This is expected to be a smart contract so that it can be triggered by depositors if the other roles misbehave / stop doing their roles.

Dependencies:
NA

Acceptance criteria:

• Emergency shutdown setter
• Emergency shutdown changes of behavior are implemented
• Tests

Payment Trigger:
PR is merged
Category:
DEV
Level:
1

• test permit
• permit2d
• test withdrawing from unrealised losses strategies
• test withdrawing from broken strategies (!)
• migration: should we call the strategy function?
• protocol fees: should we add a small protocol fee? 0.25% anual
• roles for keepers?

Implement FeeManager

Description:
FeeManager is a smart module whose responsibility is to assess fees and split them between recipients.

It implements:

• assessFees(adddress strategy, uint256 gain) : receives the last gain amount
• distribute(address owner) sends owner's accrued fees to owner

Dependencies:
core vault code needs to be finished

Acceptance criteria:

• FeeManager module is implemented and can replicate V2 fee system
• Tests

Payment Trigger:
PR is merged
Category:
DEV
Level:
1

Add a minTotalIdle to ensure a minimum amount of liquidity at vault

Description:
To ensure that vault manager can decide how much liquidity must be available at vault level, we have to add a new storage variable that is set by the DEBT_MANAGER role.

As an example of how to use this, a Smart Module that manages liquidity requests could be implemented and will be "plugged" as debt manager. Another option is that this functionality is voted by tokenholders. Initial functionality will be a vault management multisig managing this number.

This variable is minimumTotalIdle and is just used to ensure a minimum amount of liquidity during debt rebalances.

This means that, when updateDebt is called:

• the strategy will return funds (if available) to meet minimumTotalIdle liquidity
• then, if minimumTotalIdle is respected, the strategy will return or take funds from the vault as desired

During a withdrawal, minTotalIdle should not be enforced. This means that the only way totalIdle is lower than minTotalIdle is by serving withdrawal requests.

This issue requires tests.

Dependencies:
NA
Acceptance criteria:

• setter function protected by DEBT_MANAGER role
• required changes in updateDebt
• tests ensuring right behavior

Payment Trigger:
PR is merged
Category:
DEV
Level:
1

Description:
Background:
#73

Add target_debt to the _update_debt function as an argument.

The vault will try to match the strategy's debt to the target_debt. Right now, we are using strategy.max_debt as target_debt (implicitely). This means that we need to add a check to ensure that target_debt <= max_debt Worth thinking if this should be a min or an assert, taking into account the implications.

Dependencies:
NA

Acceptance criteria:

• code in VaultV3.vy
• tests

Payment Trigger:
PR is merged
Category:
DEV
Level:
0

Implement a mechanism that replicates OpenZeppelin's functionality for Access Control (https://docs.openzeppelin.com/contracts/4.x/access-control) for Vyper.

https://github.com/jmonteer/yearn-vaults-v3/blob/master/contracts/VaultV3.vy#L949-L959

1. The erc20_safe_transfer uses ASSET.address as address paramater no matter what adress is passed to the funtion.
2. V3 issues shares to itself that don't actually belong to itself while profit is unlocking. So I believe there should be a check of "assert token != self"

chore: replace utils/actions.py with fixtures

Description:
As discussed here: #47 (comment) and during a call, we agreed to use fixtures instead of a utils folder for specific actions like depositing in a vault or withdrawing from it. These actions can be customised with specific tests.

Dependencies:
NA
Acceptance criteria:
utils/actions.py is replaced with fixtures

Payment Trigger:
PR is merged
Category:
DEV
Level:
0

problem

we initialize the domain separator once on init. if a new network splits from the one the contract is deployed on, permits could potentially be replayable, even if the new network uses a distinct chain id.

solution

compute the domain separator dynamically instead of reading it from storage.

references

• https://eips.ethereum.org/EIPS/eip-1344
• https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol#L162-L164

Description:
Tests should be parameterised to test combinations of periphery modules:

• Accountant
• Whitelister
• ...

So we are sure the vault runs both with them enabled and disabled.

Dependencies:
NA

Acceptance criteria:

• tests running for all possible scenarios

Payment Trigger:
PR is merged
Category:
DEV
Level:
0

Description:
In order to make it possible for the core code to fully work with more complex add ons like Junior Tranches (and other risk management mechanisms, strategy registries, ...) we need to enable plugging in logic to react to strategies' P&L reports. The simplest way is to let the vault manager specify a report receiver.

For this, and to keep the complexity as low as possible, the solution would be to replace fee_manager module with an accountant module.

This accountant module will receive a report of the P&L (gain and loss) for the processed strategy.

The call to this new contract should have the following form:
(total_fees, total_refunds) = accountant.report(strategy, gain, loss)

This module will return total_fees (for fees charged in any concept) and total_refunds (for insurance or hedges payments, other incentives, ...)

This will enable junior tranches to pay in case there are losses. It also allows fees to be minted in a different way, taking into account profits and losses.

Question: the accountant could send the funds directly to the strategy or send them to the vault and let the vault being the one sending the funds and noting that for profits and losses.
Question: should the vault re-check total assets in case of payments?

The fee_manager module needs to be renamed and adapted but there is no need to change its internal logic.

Dependencies:
NA

Acceptance criteria:

• code in VaultV3.vy
• tests

Payment Trigger:
PR is merged
Category:
DEV
Level:
1

Description:
Following issue #66, remove all the references of total_loss and total_gain from core code and tests

Dependencies:
NA

Acceptance criteria:

Payment Trigger:
PR is merged
Category:
DEV
Level:
0

Please add protocols and their beauty here so we can learn from them.

Important that they bring something we can use.

Tests for ERC4626 (incl. ERC20)

Description:
Due to the fact that we use our own implementation of ERC20 and ERC4626, we need to test that everything works as expected.

This includes:
ERC4626 according to the standard.
ERC20 according to the standard
EIP-712

Dependencies:
Merged #30
Acceptance criteria:
Tests include testing for the behavior as described in the respective standards.
Payment Trigger:
PR is merged
Category:
DEV
Level:
1

When revoking a strategy, it is required that it has 0 debt.

Should we add a forcing parameter that allows to take that debt as a loss (reducing price per share)?

The main problem is that the loss would be irrecoverable and opens the door to strategy rugs.

My current position is that we shouldn't but sharing here for more info.

ref: https://a16zcrypto.com/generalized-property-tests-for-erc4626-vaults/

Description:
Implement a sweep function that allows airdrops and mistakenly sent asset tokens to be swept out by the function caller. Asset tokens swept out should respect the existing total_idle in the vault.

Dependencies:
NA

Acceptance criteria:

• Implement vault sweep for tokens & assets
• Include tests

Payment Trigger:
PR is merged
Category:
DEV
Level:
0

see title:

even if we choose to implement erc4626 separately, having the vault call it "amount" and "shares" vs. erc4626 calling it "asset" and "shares" adds cognitive burden to people reading/reviewing the code.

i propose changing reference to amount to asset in the vault code.

Set up ApeWorx

Short Description:

To be able to use ApeWorx, we need to set up the framework for tests to be written. This includes choosing the right test structure and writting fixtures for the tests to be easy to write. An utils folder is also important to be correctly thought so we can reuse as much code as possible.

Details

See:

• V2 version of tests: https://github.com/yearn/yearn-vaults/tree/main/tests
• V3 first version: https://github.com/saltyfacu/vaults-hardhat/tree/develop/test
• fancy things like parametrized fixtures, utils, actions, ... (Joint strategy repo) https://github.com/fp-crypto/joint-strategy/tree/fp-univ3/tests

The objective is to create a folder structure that complies with our plans:

• e2e testing
• unit testing
• utils (these may be specific to e2e or unit testing) so may make sense to split inside or move into the other folders

In the utils folders, it may make sense to separate in:

• utils: the functions to do accounting, conversions, ...
• actions: user / permissioned accounts actions that are used in all the tests: deposits, withdraws, transfers, ...
• checks: checks that are used several times: check that pps is up / down, check a vault is withdrawable, ...

Each folder should have its own conftest.py with the right fixtures within it.

Dependencies:

NA

Acceptance criteria:

• Folder structure with tests
• Implemented utils / actions / checks folders
• Created conftest.py for e2e and unit tests
• Created basic user-facing unit tests we know how they should behave: deposit and withdraw
• Created basic happy path e2e test

Payment Trigger:

PR is merged

Category:

DEV

Level:**

2

Unlike v2 we are expressing the number of tokens a strategy can borrow in a number of tokens.
This is not a great approach since the amount of debt a vault will old will greatly change.
A strategy could be allowed to borrow 100% of the funds in the vault after a vault total assets significantly decreased. Or some funds could be left not managed if the total asset in the vault significantly increased.

We should reintroduce a ratio mechanism in percentage. See:
https://github.com/yearn/yearn-vaults/blob/main/contracts/Vault.vy#L85

Description:
Background:
#74

During initialisation, the instance deployer should have the possibility to specify a smart contract address where the vault will check for whitelisting.

This allows instance deployer to release their own guest selection mechanism (e.g. veYFI holders, cryptopunk holders, ...)

This would include a immutable variable that holds that address. If the address is set to 0 at initialization, the deposits will be open for anyone.

The variable is immutable because it will save a lot of gas and makes impossible to enable censorship once instantiated permissionless.

Withdraws will never be permissioned in any case.

Dependencies:
NA

Acceptance criteria:

• code in VaultV3.vy
• tests

Payment Trigger:
PR is merged
Category:
DEV
Level:
1

Description:
Before deploying any code and entering into "test in prod phase", we need to build an extensive suite of E2E tests.

E2E tests need to cover from basic behavior to complex one

To achieve good test coverage at the lowest cost possible we will split test writting in two weeks.

1 week of tests (from today 21st of July to next Thursday 28th of July)

• all the tests proposed to be included will be rewarded with a small PM bounty
• for every 5 tests implemented, there will be a dev-lvl-0

1 week of more complex tests (from 28th July to 4th August):

• all the test proposed to be included will be rewarded with a small PM bounty
• for every 7 tests implemented there will be a dev-lvl-1

The objective is that contributors are incentivised to create tests ASAP in the first week to receive payment, tackling first the most simple ones but also doing complex ones at the less expensive rate.

Tests must be proposed in a comment in this issue: 1 test - 1 comment
The format will be
*test__doing_whatever_by_role_X__reverts*: description of the test

Approved tests will have a reaction of 👍 by a yearn contributor
Rejected tests will have nothing and might be commented for them to be amended or improved

IMPORTANT: tests should have a bare minimum quality and complexity. Those that are not complex enough but need to be implemented will be put together in a separate issue and be implemented in a batch as a dev-lvl-0 bounty. Feel free to suggest whatever test though

Dependencies:
NA

Acceptance criteria:

• tests must be approved by a yearn contributor to be eligible for bounty
• tests must be implemented and merged into master

Payment Trigger:
PR is merged

Category:
DEV

Level:
NA

Right now, update_debt function will make the current_debt of the strategy to the max_debt specified by the DEBT_ALLOCATOR role.

This means that the vault will effectively be using max_debt as target_debt.

To avoid storing the target_debt every time we change it, I think we should consider allowing an input for the DEBT_ALLOCATOR role to fill target debt.
Result:
def _update_debt(strategy: address, target_debt: address)

Target debt would be capped to max_debt in any case (as stored in the strategy params).

This allows to put a limit on the debt as guardrails but also use interesting ways to calculate debt allocation on/off-chain that does not require saving it, at let the DEBT_ALLOCATOR decide (in a way that can be implemented in the future).

I am thinking some vaults could take great advantage of being able to allow deposits only to a subset of users.

This module, if set, would return a true or false when asked. For example, a module might check if the deposit is coming from a cryptopunk holder. Or from a veYFI tokenholder. Or from a KYC'd account. Or in a simple whitelist.

Thoughts?

Description:
Vault should implement a healthcheck for strategies that can determine whether or not strategies are allowed to report.
Implement forced process strategy report + tests.

A healthcheck allows:

• maintains different profit/loss limits for different strategies
• checks whether a strategy is "healthy" by checking whether the gain/loss is within the limit set by the profit/loss limit ratios. if not, the strategy is unhealthy
• can enable/disable the healthcheck

Dependencies:
NA

Acceptance criteria:

• Implementation for healthcheck
• Implement tests for healthcheck
• Implement force process strategy report
• Implement tests for force process strategy report

Payment Trigger:
PR is merged
Category:
DEV
Level:
1

Set up CI/CD

Description:
We need to setup CI/CD that:

• runs tests
• checks commit lint
• checks vyper lint
• checks solidity lint
• checks python lint

We also need github hooks that lint before commiting to be sure this is not painful when developing

Dependencies:
NA
Acceptance criteria:
CI/CD is set up.
Lint is chosen and set up for all languages involved
Github hooks are set up

Payment Trigger:
PR is merged
Category:
DEV
Level:
0

Description:

• implement updateDebt in the vault and write corresponding tests.

updateDebt should handle how the vault manages the debt of the strategy. If there is room to increase the debt, then it should push funds from the vault into the strategy. If the debt the strategy can handle has decreased, it should pull funds from the strategy.

• write tests to handle updateDebt as well.

Dependencies:
N/A

Acceptance criteria:

• implement updateDebt
• implement unit tests associated with updateDebt

Payment Trigger:
PR is merged
Category:
<>
Level:
<>

https://github.com/jmonteer/yearn-vaults-v3/blob/master/contracts/VaultV3.vy#L870
https://github.com/jmonteer/yearn-vaults-v3/blob/master/contracts/VaultV3.vy#L538

We need help writing the tickets for issues / bounties.

Bounty tickets will receive a payment for its creation. It will only be paid when it receives the to-do tag.

Each bounty needs:

• Short description: An explanation of what this bounty will achieve.
• If required, longer description + references / docs to understand the underlying problem better.
• Dependencies: other bounties / issues / tasks that need to be done before the bounty can be completed
• Acceptance Criteria: a short list of deliverables / clear checks to be sure the objective is accomplished. Answer to these must be either Completed or Non-completed
• Payment trigger: the action that will queue the payment (e.g. the PR is merged)
• Category: PM or DEV
• Difficulty: Estimated level (0-3) of difficulty or time it will take

Different difficulty levels will receive different payment amounts.
The lower the level, the smaller amount of effort is needed and the lower the payment.

As an example:

Bounty 0: Writing tickets for project's bounties

Description:
This project uses bounties for external contributors. These bounties need to be written to be specific enough.
Dependencies:
NA
Acceptance criteria:
Bounty is created as a ticket in the Issues section of the yearn-vaults-v3 repo.
Payment Trigger:
Ticket receives the to-do tag
Category:
PM
Level:
0