fix: buy debt and role addition

contracts/VaultFactory.vy

@@ -52,7 +52,7 @@ event UpdateCustomProtocolFee:
 event RemovedCustomProtocolFee:
     vault: indexed(address)
 
-event FactoryShutdown():
+event FactoryShutdown:
     pass
 
 event UpdateGovernance:

contracts/VaultV3.vy

@@ -597,7 +597,7 @@ def _mint(sender: address, recipient: address, shares: uint256) -> uint256:
 
     assets: uint256 = self._convert_to_assets(shares, Rounding.ROUND_UP)
 
-    assert assets > 0, "cannot mint zero"
+    assert assets > 0, "cannot deposit zero"
     assert self._total_assets() + assets <= self.deposit_limit, "exceed deposit limit"
 
     # Transfer the tokens to the vault first.
@@ -1273,15 +1273,45 @@ def _enforce_role(account: address, role: Roles):
 @external
 def set_role(account: address, role: Roles):
     """
-    @notice Set the role of an account.
+    @notice Set the roles for an account.
+    @dev This will fully override an accounts current roles
+     so it should include all roles the account should hold.
     @param account The account to set the role for.
-    @param role The role to set.
+    @param role The roles the account should hold.
     """
     assert msg.sender == self.role_manager
     self.roles[account] = role
 
     log RoleSet(account, role)
 
+@external
+def add_role(account: address, role: Roles):
+    """
+    @notice Add a new role to an address.
+    @dev This will add a new role to the account
+     without effecting any of the previously held roles.
+    @param account The account to add a role to.
+    @param role The new role to add to account.
+    """
+    assert msg.sender == self.role_manager
+    self.roles[account] = self.roles[account] | role
+
+    log RoleSet(account, self.roles[account])
+
+@external
+def remove_role(account: address, role: Roles):
+    """
+    @notice Remove a single role from an account.
+    @dev This will leave all other roles for the 
+     account unchanged.
+    @param account The account to remove a Role from.
+    @param role The Role to remove.
+    """
+    assert msg.sender == self.role_manager
+    self.roles[account] = self.roles[account] & ~role
+
+    log RoleSet(account, self.roles[account])
+
 @external
 def set_open_role(role: Roles):
     """
@@ -1390,9 +1420,8 @@ def buy_debt(strategy: address, amount: uint256):
     @dev This should only ever be used in an emergency in place
     of force revoking a strategy in order to not report a loss.
     It allows the DEBT_PURCHASER role to buy the strategies debt
-    for an equal amount of `asset`. It's important to note that 
-    this does rely on the strategies `convertToShares` function to
-    determine the amount of shares to buy.
+    for an equal amount of `asset`. 
+
     @param strategy The strategy to buy the debt for
     @param amount The amount of debt to buy from the vault.
     """
@@ -1401,35 +1430,37 @@ def buy_debt(strategy: address, amount: uint256):
 
     # Cache the current debt.
     current_debt: uint256 = self.strategies[strategy].current_debt
-
-    assert current_debt > 0, "nothing to buy"
-    assert amount > 0, "nothing to buy with"
+    _amount: uint256 = amount
 
-    # Get the current shares value for the amount.
-    shares: uint256 = IStrategy(strategy).convertToShares(amount)
+    assert current_debt > 0, "nothing to buy"
+    assert _amount > 0, "nothing to buy with"
+
+    if _amount > current_debt:
+        _amount = current_debt
 
-    assert shares > 0, "can't buy 0"
-    assert shares <= IStrategy(strategy).balanceOf(self), "not enough shares"
+    # We get the proportion of the debt that is being bought and
+    # transfer the equivalent shares. We assume this is being used
+    # due to strategy issues so won't rely on its conversion rates.
+    shares: uint256 = IStrategy(strategy).balanceOf(self) * _amount / current_debt
 
-    self._erc20_safe_transfer_from(ASSET.address, msg.sender, self, amount)
+    assert shares > 0, "cannot buy zero"
 
-    # Adjust if needed to not underflow on math
-    bought: uint256 = min(current_debt, amount)
+    self._erc20_safe_transfer_from(ASSET.address, msg.sender, self, _amount)
 
     # Lower strategy debt
-    self.strategies[strategy].current_debt -= bought
+    self.strategies[strategy].current_debt -= _amount
     # lower total debt
-    self.total_debt -= bought
+    self.total_debt -= _amount
     # Increase total idle
-    self.total_idle += bought
+    self.total_idle += _amount
 
     # log debt change
-    log DebtUpdated(strategy, current_debt, current_debt - bought)
+    log DebtUpdated(strategy, current_debt, current_debt - _amount)
 
     # Transfer the strategies shares out.
     self._erc20_safe_transfer(strategy, msg.sender, shares)
 
-    log DebtPurchased(strategy, bought)
+    log DebtPurchased(strategy, _amount)
 
 ## STRATEGY MANAGEMENT ##
 @external

tests/unit/vault/test_buy_debt.py

@@ -17,13 +17,13 @@ def set_role(vault, gov):
 
 
 def test_buy_debt__strategy_not_active__reverts(
-    gov, asset, vault, mint_and_deposit_into_vault, fish_amount, create_strategy
+    gov, asset, vault, mint_and_deposit_into_vault, fish, fish_amount, create_strategy
 ):
     amount = fish_amount
 
     strategy = create_strategy(vault)
 
-    mint_and_deposit_into_vault(vault)
+    mint_and_deposit_into_vault(vault, fish, amount)
 
     # Approve vault to pull funds.
     asset.mint(gov.address, amount, sender=gov)
@@ -34,15 +34,15 @@ def test_buy_debt__strategy_not_active__reverts(
 
 
 def test_buy_debt__no_debt__reverts(
-    gov, asset, vault, mint_and_deposit_into_vault, fish_amount, create_strategy
+    gov, asset, vault, mint_and_deposit_into_vault, fish_amount, create_strategy, fish
 ):
     amount = fish_amount
 
     strategy = create_strategy(vault)
 
     vault.add_strategy(strategy.address, sender=gov)
 
-    mint_and_deposit_into_vault(vault)
+    mint_and_deposit_into_vault(vault, fish, amount)
 
     # Approve vault to pull funds.
     asset.mint(gov.address, amount, sender=gov)
@@ -60,10 +60,11 @@ def test_buy_debt__no_amount__reverts(
     fish_amount,
     strategy,
     add_debt_to_strategy,
+    fish,
 ):
     amount = fish_amount
 
-    mint_and_deposit_into_vault(vault, gov, amount)
+    mint_and_deposit_into_vault(vault, fish, amount)
 
     add_debt_to_strategy(gov, strategy, vault, amount)
 
@@ -75,27 +76,50 @@ def test_buy_debt__no_amount__reverts(
         vault.buy_debt(strategy, 0, sender=gov)
 
 
-def test_buy_debt__to_many_shares__reverts(
+def test_buy_debt__more_than_available__withdraws_current_debt(
     gov,
     asset,
     vault,
     mint_and_deposit_into_vault,
     fish_amount,
     strategy,
     add_debt_to_strategy,
+    fish,
 ):
     amount = fish_amount
 
-    mint_and_deposit_into_vault(vault, gov, amount)
+    mint_and_deposit_into_vault(vault, fish, amount)
 
     add_debt_to_strategy(gov, strategy, vault, amount)
 
     # Approve vault to pull funds.
     asset.mint(gov.address, amount, sender=gov)
     asset.approve(vault.address, amount, sender=gov)
 
-    with ape.reverts("not enough shares"):
-        vault.buy_debt(strategy, amount * 2, sender=gov)
+    before_balance = asset.balanceOf(gov)
+    before_shares = strategy.balanceOf(gov)
+
+    tx = vault.buy_debt(strategy, amount * 2, sender=gov)
+
+    logs = list(tx.decode_logs(vault.DebtPurchased))[0]
+
+    assert logs.strategy == strategy.address
+    assert logs.amount == amount
+
+    logs = list(tx.decode_logs(vault.DebtUpdated))
+
+    assert len(logs) == 1
+    assert logs[0].strategy == strategy.address
+    assert logs[0].current_debt == amount
+    assert logs[0].new_debt == 0
+
+    assert vault.totalIdle() == amount
+    assert vault.totalDebt() == 0
+    assert vault.pricePerShare() == 10 ** asset.decimals()
+    assert vault.strategies(strategy)["current_debt"] == 0
+    # assert shares got moved
+    assert asset.balanceOf(gov) == before_balance - amount
+    assert strategy.balanceOf(gov) == before_shares + amount
 
 
 def test_buy_debt__full_debt(
@@ -106,10 +130,11 @@ def test_buy_debt__full_debt(
     fish_amount,
     strategy,
     add_debt_to_strategy,
+    fish,
 ):
     amount = fish_amount
 
-    mint_and_deposit_into_vault(vault, gov, amount)
+    mint_and_deposit_into_vault(vault, fish, amount)
 
     add_debt_to_strategy(gov, strategy, vault, amount)
 
@@ -120,7 +145,19 @@ def test_buy_debt__full_debt(
     before_balance = asset.balanceOf(gov)
     before_shares = strategy.balanceOf(gov)
 
-    vault.buy_debt(strategy, amount, sender=gov)
+    tx = vault.buy_debt(strategy, amount, sender=gov)
+
+    logs = list(tx.decode_logs(vault.DebtPurchased))[0]
+
+    assert logs.strategy == strategy.address
+    assert logs.amount == amount
+
+    logs = list(tx.decode_logs(vault.DebtUpdated))
+
+    assert len(logs) == 1
+    assert logs[0].strategy == strategy.address
+    assert logs[0].current_debt == amount
+    assert logs[0].new_debt == 0
 
     assert vault.totalIdle() == amount
     assert vault.totalDebt() == 0
@@ -139,10 +176,11 @@ def test_buy_debt__half_debt(
     fish_amount,
     strategy,
     add_debt_to_strategy,
+    fish,
 ):
     amount = fish_amount
 
-    mint_and_deposit_into_vault(vault, gov, amount)
+    mint_and_deposit_into_vault(vault, fish, amount)
 
     add_debt_to_strategy(gov, strategy, vault, amount)
 
@@ -155,7 +193,19 @@ def test_buy_debt__half_debt(
     before_balance = asset.balanceOf(gov)
     before_shares = strategy.balanceOf(gov)
 
-    vault.buy_debt(strategy, to_buy, sender=gov)
+    tx = vault.buy_debt(strategy, to_buy, sender=gov)
+
+    logs = list(tx.decode_logs(vault.DebtPurchased))[0]
+
+    assert logs.strategy == strategy.address
+    assert logs.amount == to_buy
+
+    logs = list(tx.decode_logs(vault.DebtUpdated))
+
+    assert len(logs) == 1
+    assert logs[0].strategy == strategy.address
+    assert logs[0].current_debt == amount
+    assert logs[0].new_debt == amount - to_buy
 
     assert vault.totalIdle() == to_buy
     assert vault.totalDebt() == amount - to_buy