hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

Core smart contracts of Velvet Capital
Other
0 stars 1 forks source link

`Rebalancing::removePortfolioToken` is unable to remove tokens #43

Open hats-bug-reporter[bot] opened 3 months ago

hats-bug-reporter[bot] commented 3 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x2e409c32dc9c95c7cfe5cbd253ddbf8a1f65fca3434b561556227bb8927fc8c6 Severity: medium

Description: Description

Rebalancing::removePortfolioToken() removes an portfolio token from the portfolio. which can be called by the asset manager

Proof of Concept (PoC) File

  function removePortfolioToken(
    address _token
  ) external onlyAssetManager nonReentrant {
    if (!_isPortfolioToken(_token)) revert ErrorLibrary.NotPortfolioToken();

    // Generate a new token list excluding the token to be removed
    address[] memory currentTokens = _getCurrentTokens();
    uint256 tokensLength = currentTokens.length;
    address[] memory newTokens = new address[](tokensLength - 1);
    uint256 j = 0;
    for (uint256 i; i < tokensLength; i++) {
      address token = currentTokens[i];
      if (token != _token) {
        newTokens[j++] = token;
      }
    }

    portfolio.updateTokenList(newTokens);

    uint256 tokenBalance = IERC20Upgradeable(_token).balanceOf(_vault);
    _tokenRemoval(_token, tokenBalance);
  }

for removing the token, it generates new token list excluding the token to be removed and calls portfolio.updateTokenList(newTokens) for updating the token list

  function updateTokenList(
    address[] calldata _tokens
  ) external onlyRebalancerContract {
    uint256 _assetLimit = protocolConfig().assetLimit();
    uint256 tokenLength = _tokens.length;

    if (tokenLength > _assetLimit)
      revert ErrorLibrary.TokenCountOutOfLimit(_assetLimit);

    for (uint256 i; i < tokenLength; i++) {
      address token = _tokens[i];
      _beforeInitCheck(token);
      if (_previousToken[token]) {
        revert ErrorLibrary.TokenAlreadyExist();
      }
      _previousToken[token] = true;
    }
    _resetPreviousTokenList(_tokens);
    tokens = _tokens;
  }

But the issue is,updateTokenList() will revert with TokenAlreadyExist() error since every token is part of previousToken and hence for those tokens _previousToken[token] will be true

langnavina97 commented 3 months ago

The previous tokens mapping is reset at the end of the function. The function specifically verifies whether there are any duplicate tokens in the provided token list and updates the token list if there are no duplicates.