Lack of pausable feature on rebalance or setting weight open for front-run issues

[hats-bug-reporter[bot]]

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

Description: Description:

Rebalancing operations are essential for maintaining the desired asset allocation in a portfolio. In Velvet, this rebalance will update weight, which also includes a swap on enso handler.

Currently the pause is only in protocol level, not in each portfolio level. Thus when asset manager want to rebalance, this open for a front-run issue.

When users deposit or withdraw tokens, the vault needs to adjust the underlying holdings to maintain the desired weights. This can lead to arbitrage opportunities where users exploit temporary price discrepancies between the portfolio token and its individual token assets.

  function _updateWeights(
    address[] calldata _sellTokens,
    address[] memory _newTokens,
    uint256[] calldata _sellAmounts,
    address _handler,
    bytes memory _callData
  ) private {
    if (protocolConfig.isProtocolPaused())
      revert ErrorLibrary.ProtocolIsPaused();

...
    // Execute the swap using the handler.
    address[] memory ensoBuyTokens = IIntentHandler(_handler)
      .multiTokenSwapAndTransfer(_vault, _callData);

    // Verify that all specified sell tokens have been completely sold by the handler.
    for (uint256 i; i < sellTokenLength; i++) {
      if (_getTokenBalanceOf(_sellTokens[i], _handler) != 0)
        revert ErrorLibrary.BalanceOfHandlerShouldBeZero();
    }
...
  }

Scenario:

Initial weight: 40% ETH, 30% USDC, 30% WBTC Rebalancing goal: 35% ETH, 35% USDC, 30% WBTC

1. AssetManager starts rebalancing.
2. Velvet contract prepare to sells ETH to buy USDC
3. Bob notices and predicts ETH price will drop in the vault.
4. Bob front-run:
   • Deposits USDC, gets PortfolioTokens at old price.
   • Withdraws ETH as price drops
5. Alice deposits ETH, unaware of rebalancing. Gets fewer PortfolioTokens due to lower ETH price.
6. Rebalancing finishes, but vault is imbalanced again due to Bob's actions
7. Bob sells his extra ETH for profit on the market.
8. Alice withdraws, receiving less ETH than she deposited.

Result:

• Bob profits unfairly.
• Alice may loses money.
• Imbalanced weight as intended

With pause/unpause:

• AssetManager would pause before rebalancing.
• Bob and Alice couldn't interact during rebalancing
• Rebalancing would complete smoothly as intended.
• Everyone would trade fairly after unpausing.

This shows how pause/unpause prevents exploitation and protects users during rebalancing.

Impact:

lack of a pause/unpause on rebalancing can lead to unfair advantages, user losses, and weight may imbalanced

Mitigation:

Implement a pausing mechanism which temporarily halts all deposits and withdrawals, allowing the rebalancing operation to proceed. Once the rebalancing is complete and the system is stable, the pause can be lifted, and normal operations can resume

[langnavina97]

Asset managers can use a private RPC to avoid front-running.

[chainNue]

IMO, having a private RPC is not a solution in general practice, as asset manager may not want to or don't know to use this private RPC. Having a pause mechanism is a common practice, not sure why this is rejected and assuming every Asset Manager will use private RPC. Please reconsider this issue @langnavina97

Also, in previous version of Velvet, the pause is exist, which also still showing on the current docs on Velvet website.

The asset manager can pause or unpause the IndexSwap contract's functionalities using the setPause function. This can be useful during rebalancing to make sure there’s no deposits/withdrawals during rebalancing.

The system can be paused or unpaused during the rebalancing to avoid incorrect deposit/withdrawal calculations