Refactor msg value to be passed down vs. direct reference (#1749)

TRileySchwarz · tinom9 · web-flow · commit a73a924c0a43 · 2025-10-07T10:47:47.000-07:00
Co-authored-by: Tino Martínez Molina &lt;tino.molina@layerzerolabs.org&gt;
diff --git a/packages/ovault-evm/contracts/VaultComposerSync.sol b/packages/ovault-evm/contracts/VaultComposerSync.sol
@@ -56,25 +56,14 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
     constructor(address _vault, address _assetOFT, address _shareOFT) {
         VAULT = IERC4626(_vault);
 
-        ASSET_OFT = _assetOFT;
         SHARE_OFT = _shareOFT;
-        SHARE_ERC20 = IOFT(SHARE_OFT).token();
+        ASSET_OFT = _assetOFT;
+
+        SHARE_ERC20 = _initializeShareToken();
+        ASSET_ERC20 = _initializeAssetToken();
 
         ENDPOINT = address(IOAppCore(ASSET_OFT).endpoint());
         VAULT_EID = ILayerZeroEndpointV2(ENDPOINT).eid();
-
-        if (SHARE_ERC20 != address(VAULT)) {
-            revert ShareTokenNotVault(SHARE_ERC20, address(VAULT));
-        }
-
-        /// @dev ShareOFT must be an OFT adapter. We can infer this by checking 'approvalRequired()'.
-        /// @dev burn() on tokens when a user sends changes totalSupply() which the asset:share ratio depends on.
-        if (!IOFT(SHARE_OFT).approvalRequired()) revert ShareOFTNotAdapter(SHARE_OFT);
-
-        /// @dev Approve the share adapter with the share tokens held by this contract
-        IERC20(SHARE_ERC20).approve(_shareOFT, type(uint256).max);
-
-        ASSET_ERC20 = _initializeAssetToken(_assetOFT, VAULT);
     }
 
     /**
@@ -111,7 +100,7 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
                 }
             }
 
-            _refund(_composeSender, _message, amount, tx.origin);
+            _refund(_composeSender, _message, amount, tx.origin, msg.value);
             emit Refunded(_guid);
         }
     }
@@ -141,9 +130,9 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
         if (msg.value < minMsgValue) revert InsufficientMsgValue(minMsgValue, msg.value);
 
         if (_oftIn == ASSET_OFT) {
-            _depositAndSend(_composeFrom, _amount, sendParam, tx.origin);
+            _depositAndSend(_composeFrom, _amount, sendParam, tx.origin, msg.value);
         } else {
-            _redeemAndSend(_composeFrom, _amount, sendParam, tx.origin);
+            _redeemAndSend(_composeFrom, _amount, sendParam, tx.origin, msg.value);
         }
     }
 
@@ -159,7 +148,13 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
         address _refundAddress
     ) external payable virtual nonReentrant {
         IERC20(ASSET_ERC20).safeTransferFrom(msg.sender, address(this), _assetAmount);
-        _depositAndSend(OFTComposeMsgCodec.addressToBytes32(msg.sender), _assetAmount, _sendParam, _refundAddress);
+        _depositAndSend(
+            OFTComposeMsgCodec.addressToBytes32(msg.sender),
+            _assetAmount,
+            _sendParam,
+            _refundAddress,
+            msg.value
+        );
     }
 
     /**
@@ -168,6 +163,7 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
      * @param _assetAmount The number of assets to deposit
      * @param _sendParam Parameter that defines how to send the shares
      * @param _refundAddress Address to receive excess payment of the LZ fees
+     * @param _msgValue The amount of native tokens sent with the transaction
      * @notice This function first deposits the assets to mint shares, validates the shares meet minimum slippage requirements,
      *         then sends the minted shares cross-chain using the OFT (Omnichain Fungible Token) protocol
      * @notice _sendParam.amountLD is set to the share amount minted, and minAmountLD is reset to 0 for send operation
@@ -176,15 +172,16 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
         bytes32 _depositor,
         uint256 _assetAmount,
         SendParam memory _sendParam,
-        address _refundAddress
+        address _refundAddress,
+        uint256 _msgValue
     ) internal virtual {
         uint256 shareAmount = _deposit(_depositor, _assetAmount);
         _assertSlippage(shareAmount, _sendParam.minAmountLD);
 
         _sendParam.amountLD = shareAmount;
         _sendParam.minAmountLD = 0;
 
-        _send(SHARE_OFT, _sendParam, _refundAddress);
+        _send(SHARE_OFT, _sendParam, _refundAddress, _msgValue);
         emit Deposited(_depositor, _sendParam.to, _sendParam.dstEid, _assetAmount, shareAmount);
     }
 
@@ -210,7 +207,13 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
         address _refundAddress
     ) external payable virtual nonReentrant {
         IERC20(SHARE_ERC20).safeTransferFrom(msg.sender, address(this), _shareAmount);
-        _redeemAndSend(OFTComposeMsgCodec.addressToBytes32(msg.sender), _shareAmount, _sendParam, _refundAddress);
+        _redeemAndSend(
+            OFTComposeMsgCodec.addressToBytes32(msg.sender),
+            _shareAmount,
+            _sendParam,
+            _refundAddress,
+            msg.value
+        );
     }
 
     /**
@@ -219,6 +222,7 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
      * @param _shareAmount The number of shares to redeem
      * @param _sendParam Parameter that defines how to send the assets
      * @param _refundAddress Address to receive excess payment of the LZ fees
+     * @param _msgValue The amount of native tokens sent with the transaction
      * @notice This function first redeems the specified share amount for the underlying asset,
      *         validates the received amount against slippage protection, then initiates a cross-chain
      *         transfer of the redeemed assets using the OFT (Omnichain Fungible Token) protocol
@@ -229,15 +233,16 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
         bytes32 _redeemer,
         uint256 _shareAmount,
         SendParam memory _sendParam,
-        address _refundAddress
+        address _refundAddress,
+        uint256 _msgValue
     ) internal virtual {
         uint256 assetAmount = _redeem(_redeemer, _shareAmount);
         _assertSlippage(assetAmount, _sendParam.minAmountLD);
 
         _sendParam.amountLD = assetAmount;
         _sendParam.minAmountLD = 0;
 
-        _send(ASSET_OFT, _sendParam, _refundAddress);
+        _send(ASSET_OFT, _sendParam, _refundAddress, _msgValue);
         emit Redeemed(_redeemer, _sendParam.to, _sendParam.dstEid, _shareAmount, assetAmount);
     }
 
@@ -303,12 +308,13 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
      * @param _oft The OFT contract address to use for sending
      * @param _sendParam The parameters for the send operation
      * @param _refundAddress Address to receive excess payment of the LZ fees
+     * @param _msgValue The amount of native tokens sent with the transaction
      */
-    function _send(address _oft, SendParam memory _sendParam, address _refundAddress) internal virtual {
+    function _send(address _oft, SendParam memory _sendParam, address _refundAddress, uint256 _msgValue) internal virtual {
         if (_sendParam.dstEid == VAULT_EID) {
-            _sendLocal(_oft, _sendParam, _refundAddress);
+            _sendLocal(_oft, _sendParam, _refundAddress, _msgValue);
         } else {
-            _sendRemote(_oft, _sendParam, _refundAddress);
+            _sendRemote(_oft, _sendParam, _refundAddress, _msgValue);
         }
     }
 
@@ -317,9 +323,11 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
      * @dev Transfers tokens directly without LayerZero messaging
      * @param _oft The OFT contract address to determine which token to transfer
      * @param _sendParam The parameters for the send operation
+     * @dev _refundAddress Address to receive excess payment of the LZ fees (unused for local transfers)
+     * @param _msgValue The amount of native tokens sent with the transaction (must be 0 for local transfers)
      */
-    function _sendLocal(address _oft, SendParam memory _sendParam, address /*_refundAddress*/) internal virtual {
-        if (msg.value > 0) revert NoMsgValueExpected();
+    function _sendLocal(address _oft, SendParam memory _sendParam, address /*_refundAddress*/, uint256 _msgValue) internal virtual {
+        if (_msgValue > 0) revert NoMsgValueExpected();
 
         /// @dev Can do this because _oft is validated before this function is called
         address erc20 = _oft == ASSET_OFT ? ASSET_ERC20 : SHARE_ERC20;
@@ -332,9 +340,10 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
      * @param _oft The OFT contract address to use for sending
      * @param _sendParam The parameters for the send operation
      * @param _refundAddress Address to receive excess payment of the LZ fees
+     * @param _msgValue The amount of native tokens sent with the transaction
      */
-    function _sendRemote(address _oft, SendParam memory _sendParam, address _refundAddress) internal virtual {
-        IOFT(_oft).send{ value: msg.value }(_sendParam, MessagingFee(msg.value, 0), _refundAddress);
+    function _sendRemote(address _oft, SendParam memory _sendParam, address _refundAddress, uint256 _msgValue) internal virtual {
+        IOFT(_oft).send{ value: _msgValue}(_sendParam, MessagingFee(_msgValue, 0), _refundAddress);
     }
 
     /**
@@ -343,36 +352,59 @@ contract VaultComposerSync is IVaultComposerSync, ReentrancyGuard {
      * @param _message The original message that was sent
      * @param _amount The amount of tokens to refund
      * @param _refundAddress Address to receive the refund
+     * @param _msgValue The amount of native tokens sent with the transaction
      */
-    function _refund(address _oft, bytes calldata _message, uint256 _amount, address _refundAddress) internal virtual {
+    function _refund(address _oft, bytes calldata _message, uint256 _amount, address _refundAddress, uint256 _msgValue) internal virtual {
         /// @dev Extracted from the _message header. Will always be part of the _message since it is created by lzReceive
         SendParam memory refundSendParam;
         refundSendParam.dstEid = OFTComposeMsgCodec.srcEid(_message);
         refundSendParam.to = OFTComposeMsgCodec.composeFrom(_message);
         refundSendParam.amountLD = _amount;
 
-        _sendRemote(_oft, refundSendParam, _refundAddress);
+        _sendRemote(_oft, refundSendParam, _refundAddress, _msgValue);
+    }
+
+    /**
+        * @dev Internal function to validate the share token compatibility
+        * @dev Validate part of the constructor in an overridable function due to differences in asset and OFT token
+        * @return shareERC20 The address of the share ERC20 token
+        * @notice Share token must be the vault itself
+        * @notice Share OFT must be an adapter (approvalRequired() returns true)
+     */
+    function _initializeShareToken() internal virtual returns (address shareERC20) {
+        shareERC20 = IOFT(SHARE_OFT).token();
+
+        if (shareERC20 != address(VAULT)) {
+            revert ShareTokenNotVault(shareERC20, address(VAULT));
+        }
+
+        /// @dev ShareOFT must be an OFT adapter. We can infer this by checking 'approvalRequired()'.
+        /// @dev burn() on tokens when a user sends changes totalSupply() which the asset:share ratio depends on.
+        if (!IOFT(SHARE_OFT).approvalRequired()) revert ShareOFTNotAdapter(SHARE_OFT);
+
+        /// @dev Approve the share adapter with the share tokens held by this contract
+        IERC20(shareERC20).approve(SHARE_OFT, type(uint256).max);
     }
 
     /**
      * @dev Internal function to validate the asset token compatibility
      * @dev Validate part of the constructor in an overridable function due to differences in asset and OFT token
      * @dev For example, in the case of VaultComposerSyncPoolNative, the asset token is WETH but the OFT token is native
-     * @param _assetOFT The address of the asset OFT
-     * @param _vault The address of the vault
+     * @return assetERC20 The address of the asset ERC20 token
+     * @notice Asset token should match the vault's underlying asset (overridable behavior)
      */
-    function _initializeAssetToken(address _assetOFT, IERC4626 _vault) internal virtual returns (address assetERC20) {
-        assetERC20 = IOFT(_assetOFT).token();
+    function _initializeAssetToken() internal virtual returns (address assetERC20) {
+        assetERC20 = IOFT(ASSET_OFT).token();
 
-        if (assetERC20 != address(_vault.asset())) {
-            revert AssetTokenNotVaultAsset(assetERC20, address(_vault.asset()));
+        if (assetERC20 != address(VAULT.asset())) {
+            revert AssetTokenNotVaultAsset(assetERC20, address(VAULT.asset()));
         }
 
-        /// @dev Approve the vault to spend the asset tokens held by this contract
-        IERC20(assetERC20).approve(address(_vault), type(uint256).max);
-
         /// @dev If the asset OFT is an adapter, approve it as well
-        if (IOFT(_assetOFT).approvalRequired()) IERC20(assetERC20).approve(_assetOFT, type(uint256).max);
+        if (IOFT(ASSET_OFT).approvalRequired()) IERC20(assetERC20).approve(ASSET_OFT, type(uint256).max);
+
+        /// @dev Approve the vault to spend the asset tokens held by this contract
+        IERC20(assetERC20).approve(address(VAULT), type(uint256).max);
     }
 
     receive() external payable virtual {}
diff --git a/packages/ovault-evm/contracts/VaultComposerSyncNative.sol b/packages/ovault-evm/contracts/VaultComposerSyncNative.sol
@@ -4,6 +4,7 @@ pragma solidity ^0.8.22;
 import { IERC4626 } from "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol";
 
 import { IOFT, SendParam, MessagingFee } from "@layerzerolabs/oft-evm/contracts/interfaces/IOFT.sol";
+import { OFTComposeMsgCodec } from "@layerzerolabs/oft-evm/contracts/libs/OFTComposeMsgCodec.sol";
 
 import { IVaultComposerSyncNative } from "./interfaces/IVaultComposerSyncNative.sol";
 import { IWETH } from "./interfaces/IWETH.sol";
@@ -20,6 +21,8 @@ import { VaultComposerSync } from "./VaultComposerSync.sol";
  * @dev Compatible with ERC4626 vaults and requires Share OFT to be an adapter
  */
 contract VaultComposerSyncNative is VaultComposerSync, IVaultComposerSyncNative {
+    using OFTComposeMsgCodec for bytes32;
+
     /**
      * @notice Initializes the VaultComposerSyncPoolNative contract with vault and OFT token addresses
      * @param _vault The address of the ERC4626 vault contract
@@ -48,8 +51,15 @@ contract VaultComposerSyncNative is VaultComposerSync, IVaultComposerSyncNative
         if (msg.value < _assetAmount) revert AmountExceedsMsgValue();
 
         IWETH(ASSET_ERC20).deposit{ value: _assetAmount }();
+
         /// @dev Reduce msg.value to the amount used as Fee for the lzSend operation
-        this.depositAndSend{ value: msg.value - _assetAmount }(_assetAmount, _sendParam, _refundAddress);
+        _depositAndSend(
+            OFTComposeMsgCodec.addressToBytes32(msg.sender),
+            _assetAmount,
+            _sendParam,
+            _refundAddress,
+            msg.value - _assetAmount
+        );
     }
 
     /**
@@ -59,9 +69,9 @@ contract VaultComposerSyncNative is VaultComposerSync, IVaultComposerSyncNative
      * @param _sendParam The parameters for the send operation
      * @param _refundAddress Address to receive tokens and native on Pool failure
      */
-    function _sendRemote(address _oft, SendParam memory _sendParam, address _refundAddress) internal override {
-        /// @dev msg.value passed in this call is used as LayerZero fee
-        uint256 msgValue = msg.value;
+    function _sendRemote(address _oft, SendParam memory _sendParam, address _refundAddress, uint256 _msgValue) internal override {
+        /// @dev _msgValue passed in this call is used as LayerZero fee
+        uint256 msgValue = _msgValue;
 
         /// @dev Safe because this is the only function in VaultComposerSync that calls oft.send()
         if (_oft == ASSET_OFT) {
@@ -73,20 +83,23 @@ contract VaultComposerSyncNative is VaultComposerSync, IVaultComposerSyncNative
             msgValue += _sendParam.amountLD;
         }
 
-        IOFT(_oft).send{ value: msgValue }(_sendParam, MessagingFee(msg.value, 0), _refundAddress);
+        IOFT(_oft).send{ value: msgValue }(_sendParam, MessagingFee(_msgValue, 0), _refundAddress);
     }
 
     /**
      * @dev Internal function to validate the asset token compatibility
      * @dev In VaultComposerSyncNative, the asset token is WETH but the OFT token is native (ETH)
-     * @param _assetOFT The address of the asset OFT (Omnichain Fungible Token) contract
-     * @param _vault The address of the vault contract
      * @return assetERC20 The address of the asset ERC20 token
      */
-    function _initializeAssetToken(address _assetOFT, IERC4626 _vault) internal override returns (address assetERC20) {
-        if (IOFT(_assetOFT).token() != address(0)) revert AssetOFTTokenNotNative();
-        assetERC20 = _vault.asset();
-        IWETH(assetERC20).approve(address(_vault), type(uint256).max);
+    function _initializeAssetToken() internal override returns (address assetERC20) {
+        assetERC20 = VAULT.asset();
+
+        if (IOFT(ASSET_OFT).token() != address(0)) revert AssetOFTTokenNotNative();
+
+        // @dev The asset OFT does NOT need approval since it operates in native ETH.
+        // if (IOFT(ASSET_OFT).approvalRequired()) IERC20(assetERC20).approve(ASSET_OFT, type(uint256).max);
+
+        IWETH(assetERC20).approve(address(VAULT), type(uint256).max);
     }
 
     /**
