DebtRatio problem

[zgfzgf]

yearn-vaults/contracts/Vault.vy

Lines 1441 to 1445 in ec7d2e3

	strategy_debtLimit: uint256 = (
	self.strategies[strategy].debtRatio
	* self.totalDebt
	/ self.debtRatio
	)

yearn-vaults/contracts/Vault.vy

Line 1478 in ec7d2e3

strategy_debtLimit: uint256 = self.strategies[strategy].debtRatio * vault_totalAssets / MAX_BPS

above. compute strategy_debtLimit is different

yearn-vaults/contracts/Vault.vy

Line 1476 in ec7d2e3

vault_debtLimit: uint256 = self.debtRatio * vault_totalAssets / MAX_BPS

should add var debtLimit
vault_debtLimit: uint256 = self.debtLimit * vault_totalAssets / MAX_BPS
self.debtLimit <= MAX_BPS

No more judgment self.debtRatio<=MAX_BPS

compute strategy_debtLimit
self.strategies[strategy].debtRatio * self.totalDebt / self.debtRatio
add strategy
self.debtRatio += self.strategies[strategy].debtRatio

[zgfzgf]

now use var self.debtRatio controller vault_debtLimit. It's hard to control and understand.

[ghost]

What you are suggesting is the code we use to have prior to the changes.
a5c51b2#diff-b583ba5c4d7b67e56b9084eff9fe883e12a2cdd94a2ea25029322c6f3568d807L1140

The original design was based on totalAssets() but this can be user manipulated.

A potential fraudulent strategy that doesn't want to return back his assets after gains could fraud by depositing assets into the vault, call harvest it debtRatio compared to total assets would not change and then the strategist can withdraw the funds.

We tried to prevent that with our latest changes but that introduced another issue:
#375

[ghost]

What you are suggesting is the code we use to have prior to the changes.
a5c51b2#diff-b583ba5c4d7b67e56b9084eff9fe883e12a2cdd94a2ea25029322c6f3568d807L1140

[zgfzgf]

I think. add var debtLimit controller available amout. vault 1 code:
// Custom logic in here for how much the vault allows to be borrowed
// Sets minimum required on-hand to keep small withdrawals cheap
function available() public view returns (uint) {
return token.balanceOf(address(this)).mul(min).div(max);
}
a5c51b2#diff-b583ba5c4d7b67e56b9084eff9fe883e12a2cdd94a2ea25029322c6f3568d807L1140
there is no such function here.
add var debtLimit.
We don't need that judgment
self.debtRatio<=MAX_BPS
I think it's better and easier to understand. @steffenix @fubuloubu

[ghost]

sorry, I do not understand your suggestion.
Can you propose some changes?

[zgfzgf]

Well, I'll change the code tomorrow, and we'll discuss it. You can better understand my idea from the code.

[ghost]

Ok I will discuss with @fubuloubu
Thanks!

[zgfzgf]

a5c51b2#diff-b583ba5c4d7b67e56b9084eff9fe883e12a2cdd94a2ea25029322c6f3568d807L1140
def addStrategy(
strategy: address,
debtLimit: uint256,
rateLimit: uint256,
performanceFee: uint256,
) before debtLimit is good. it controller strategy amount. why delete it now?

[ghost]

Because a strategy owner can reduce or bypass debt reduction.

The computation of the ratio change depends on total assets, which are computed using the balance of the vault. However, a strategy owner can temporarily increase this value with a flash loan, with the following sequences of calls in a single transaction:

1. Take a flash loan
2. Make deposit
3. Call report
4. Make a withdrawal
5. Repay the flash loan

Since the value of the _totalAssets() will be increased (up to the deposit limit), the amount of ratio_change can be reduced, even to zero, avoiding the debt change.

The updates I made were incorrect and we have to find a way around that.

[zgfzgf]

thank you.
I suggest add strategy debtLimit. not add self.debtLimit. add var self.available controller use asset.
for example:
self.available=9500. max may use asset = 9500*vault_totalAssets/MAX_BPS
strategy 1 debtLimit = 100 strategy 1 may use asset <= 100.
strategy 2 debtLimit = 200 strategy 2 may use asset <= 200.

[ghost]

To my understanding, your suggestion is also prone to the manipulation described above.

[zgfzgf]

Because a strategy owner can reduce or bypass debt reduction.

The computation of the ratio change depends on total assets, which are computed using the balance of the vault. However, a strategy owner can temporarily increase this value with a flash loan, with the following sequences of calls in a single transaction:

1. Take a flash loan
2. Make deposit
3. Call report
4. Make a withdrawal
5. Repay the flash loan

Since the value of the _totalAssets() will be increased (up to the deposit limit), the amount of ratio_change can be reduced, even to zero, avoiding the debt change.

The updates I made were incorrect and we have to find a way around that.

To solve this problem, we estimate that we should lock deposit amount.
like _calculateLockedProfit function

[zgfzgf]

for example:
strategy 1 strategy 2 parameters are the same. debtLimit = 100.
strategy 1 debt = 100 and strategy 2 debt = 100
when withdraw 80.
strategy 1 debt = 20 and strategy 2 debt = 100
normal： report
strategy 1 debt = 60 and strategy 2 debt = 60
if strategy 2 do above flash loan
strategy 1 debt = 20 and strategy 2 debt = 100 There is nothing wrong with that. debt<debtLimit. strategy 2 may max use 100.
if control strategy 2 debt. update strategy debtLimit =60.