haiVELO V2 review

The implementation of migrateV1toV2() transfers V1 tokens to a burn address without adjusting the supply, while new V2 tokens are minted.

WrappedTokenV2.sol#L130

Informational.

Ensure no component depends on the total supply of the old token, and that the burn address balance is accounted for when calculating the circulating supply.

Acknowledged. No fix needed.

Even though the implementation should fail on transferring a duplicate token ID, the depositNFTs() function could validate that there are no repeated IDs in the _tokenIds array.

WrappedTokenV2.sol#L104

Informational.

Validate token IDs are not repeated in the _tokenIds array.

    uint256 _balance = 0;
    for (uint256 i = 0; i < _tokenIds.length; i++) {
      _balance += uint256(uint128(BASE_TOKEN_NFT.locked(_tokenIds[i]).amount));
+     if (i > 0 && _tokenIds[i-1] >= _tokenIds[i]) {
+       revert WrappedTokenV2_DuplicateTokenId();
+     }
    }

Fixed in https://github.com/hai-on-op/core/commit/74b2ffcfbc3e9ceda0a63fc5e21a707676bc60ca.

HaiVelo tokens maintain a 1:1 backing with VELO tokens through either pure VELO tokens or veVELO NFTs. However, during the minting process, the backing VELO tokens are transferred to the baseTokenManager, which is a Safe multisig controlled by the Hai team.

This introduces a trust assumption where any party with access to the Safe could mint additional HaiVelo tokens without proper backing, breaking the 1:1 peg.

Informational.

Store the backing VELO tokens and veVELO NFTs in an immutable smart contract that enforces the 1:1 backing mechanically, removing the need for trusted custodians.

Acknowledged. We are in the process of writing a contract to handle this.

The depositNFTs() function transfers the tokens before minting haiVELO, potentially leading to an incorrect supply if read from the NFT transfer callback.

WrappedTokenV2.sol#L104

Informational.

Take into account this desynchronization in future manager implementations. Another alternative would be to follow a CEI approach and mint before transferring.

Fixed in https://github.com/hai-on-op/core/commit/64d57eb84b67bb088e221a6158a54740e64ef6ca.

The events should notify about the user depositing or migrating, but are instead emitted with the receiver.

As an example, in the WrappedTokenV2Deposit event, the _account argument refers to the user depositing the base tokens.

20:   /**
21:    * @notice Emitted when a user deposits tokens and mints wrapped tokens
22:    * @param _account Address of the user depositing the base tokens
23:    * @param _wad Amount of tokens deposited
24:    */
25:   event WrappedTokenV2Deposit(address indexed _account, uint256 _wad);

However, the event is then emitted with the _account parameter, the receiver, which may differ from the caller.

100:     emit WrappedTokenV2Deposit(_account, _wad);

A second event should be emitting the amount of Velo per NFT locked but is emitting the sum of all Velo locked instead.

The interface:

27:  /**
28:   * @notice Emitted when a user deposits a veNFT and mints wrapped tokens
29:   * @param _account Address of the user depositing the base tokens
30:   * @param _tokenId ID of the veNFT being deposited
31:   * @param _wad Amount of locked velo in veNFT being deposited
32:   */
33:  event WrappedTokenV2NFTDeposit(address indexed _account, uint256 _tokenId, uint256 _wad);

However inside the function, it uses the sum of all NFTs locked instead of the amount per tokenId.

110: _balance += uint256(uint128(BASE_TOKEN_NFT.locked(_tokenIds[i]).amount));
...
119: emit WrappedTokenV2NFTDeposit(_account, _tokenIds[i], _balance);

Informational.

Change event arguments to msg.sender, or change the associated documentation to note that this address is the token recipient and not the originator of the action.

Fixed in https://github.com/hai-on-op/core/commit/0fde26277968341029a57ac77fb8596153996312 & https://github.com/hai-on-op/core/commit/757e12b42236c4f8859853fee113f1f22780c25e & https://github.com/hai-on-op/core/commit/937ab3752c43f08ab6ae1d173b78b04d7732f7f2.

The BURN_ADDRESS is known at compilation time and can be defined as constant.

• WrappedTokenV2.sol#L51

Gas savings.

Change the variable to constant.

Fixed in https://github.com/hai-on-op/core/commit/be1e037bd0b571bcb940d789ecbbe471e073b363.