Migration

Locally, the public depositAndSend only checks that composeMsg is non-empty; any EOA on the hub can call it with arbitrary MigrationData, mark a (vaultId, sourceEid) as processed, and register an allocation in NLPClaim, blocking the real migration.

Cross-chain, lzCompose only verifies the sender is ASSET_OFT/SHARE_OFT; it never validates composeFrom/srcEid against a trusted migrator allowlist, so any source-chain user can craft an OFT send with fake MigrationData and achieve the same result.

This will result in a cheap, irreversible DoS/poisoning of migrations; attacker-controlled totalShares/nlpAmount skew claims

Restrict depositAndSend to trusted LayerZero compose flows; add (srcEid => trusted migrator) checks on composeFrom/srcEid; validate MigrationData against factory state or signatures before registering allocations.

Before claiming, NLPClaim uses lzRead to fetch the share balance of the user on the source chain. When the state is read and the result is confirmed by the DVN, NLPClaim._lzReceive() will be executed.

There, the following check is performed:

        if (origin.srcEid != request.sourceEid) {
            revert MigrationErrors.ChainMismatch(request.sourceEid, origin.srcEid);
        }

For lzRead the origin.srcEid is equal to the READ_CHANNEL. The app is set up in a way that the peer for READ_CHANNEL is the app itself.

    function setReadChannel(uint32 _channelId, bool _active) public virtual onlyOwner {
        _setPeer(_channelId, _active ? AddressCast.toBytes32(address(this)) : bytes32(0));
    }

In contrast, request.sourceEid will be equal to the chain the state is read from.

        // Build and send read request
        bytes memory cmd = _buildReadCommand(vaultId, user, sourceEid);
        bytes memory options = this.combineOptions(READ_CHANNEL, CLAIM_MSG_TYPE, hex"");

        MessagingReceipt memory receipt = _lzSend(
            READ_CHANNEL,
            cmd,
            options,
            MessagingFee(msg.value, 0),
            payable(msg.sender)
        );

        // Store request data for response routing
        claimRequests[receipt.guid] = ClaimRequest({
            user: user,
            vaultId: vaultId,
            sourceEid: sourceEid
        });

Because of this, _lzReceive will always revert and users won't be able to claim their assets. The tokens will have to be recovered by calling NLPClaim.recoverTokens() and distributing them manually after that.

POC

A contract using lzRead was deployed on Arbitrum Sepolia. The contract emits the following event in its _lzReceive() function.

emit OriginSrcEidReceived(origin.srcEid);

The contract reads WETH.decimals() from Ethereum Sepolia. The LZ message is successfully delivered. Exploring the transaction, it can be observed that the event emits the value 4294967295, which is the read channel id.

Compare origin.srcEid against READ_CHANNEL, not request.sourceEid.

-      if (origin.srcEid != request.sourceEid) {
+      if (origin.srcEid != READ_CHANNEL) {
-           revert MigrationErrors.ChainMismatch(request.sourceEid, origin.srcEid);
+           revert MigrationErrors.ChainMismatch(READ_CHANNEL, origin.srcEid);
        }

When a migration is executed through the VaultMigrator, all of the assets held by the vault are swapped to the desired stablecoin through curve.

        uint256 expectedOut = curvePool.get_dy(fromIndex, toIndex, amountIn);
        uint256 minAmountOut = (expectedOut * (10000 - maxSlippageBps)) / 10000;

        // Approve Curve pool to spend input asset
        IERC20(assetIn).forceApprove(address(curvePool), amountIn);

        // Execute swap
        amountOut = curvePool.exchange(fromIndex, toIndex, amountIn, minAmountOut);

The maxSlippageBps variable is applied to the result of get_dy()to determine the minimum amount of assets the contract is willing to accept. However, get_dy() returns a result based on the current chain state, so it will return the exact amount that will be received. Applying a discount to this value will always result in a smaller value than the tokens received, nullifying the slippage parameter. This means the swap is susceptible to being sandwiched in order to profit at the expense of the vault share holders.

Instead of applying the slippage to the result of get_dy(), consider having an exact value for minAmountOut passed when migrating.

In VaultMigrator._sendOFT the migration uses amountLD and minAmountLD equal to the raw asset amount with no slippage buffer.

If the target stable’s local decimals exceed the OFT’s shared decimals (e.g., an 18‑dec token like USD.e wrapped with an OFT set to 6 shared decimals), OFTCore will truncate “dust” when converting to shared units. The resulting amount credited on the destination is slightly less than amountLD, causing the send to revert against minAmountLD with error SlippageExceeded.

Therefore, any migrated value that is not a multiple of 10**(token decimals-shared decimals) can thus stall migrations.

Similarly, when claiming NLP, the cross chain flow uses the same value for amountLD and minAmountLD . If the SHARE_ERC20 decimals is greater than the SHARE_OFT shared decimals (6 by default), the transfer will revert due to slippage.

Before sending, account for shared/local decimal differences: set minAmountLD to 0 or to the OFT’s amountReceivedLD from quoteSend. Add an explicit check of token.decimals() vs IOFT.sharedDecimals() and adjust slippage accordingly.

Keep in mind that this will leave some dust behind due to the truncation.

During claim verification, an lzRead is initiated to read the vault shares in the target chain. This read flow will call the getUserSharesForClaim function in the configured factory contract for given chain.

The current VaultModule.getUserSharesForClaim will return the share balance of users that deposited using the factory contract. Therefore, SY Vault users will have zero shares recorded in the factory, as the SY Vault itself is the share's owner.

Additionally, the SY Token vault mint ERC20 tokens to users. Even if NLPClaim can read the burnedBalance mapping in vault, user will only be able to claim once, as the _hasClaimed flag is set to true after claiming. In case any user receives more SY tokens, they won't be able to burn or claim, so they will need to transfer them to a different address.

Consider creating a separate initiateClaimSY function that allows users to claim their shares from the SY Vaults, by reading the vault.getUserSharesForClaim which returns the user's burned balance.

Additionally, consider pausing SY transfers once the vault is migrated. Alternatively, document this scenario to users so they are aware of NLP claiming for SY holders.

The quoteMigration() encodes composeMsg using MigrationData passing it to quoteSend, while executeVaultMigration sends a much larger payload: abi.encode(SendParam(innerParam with composeMsg=migrationData), uint256(0)).

This underquotes the native fee, causing executeVaultMigration to revert at the send step if the operator uses the quoted fee.

Build the compose payload in quoteMigration exactly as in executeVaultMigration: wrap MigrationData inside an inner SendParam and then ABI-encode (innerParam, minMsgValue) for the outer compose. Then use that exact payload with IOFT.quoteSend

NLPClaim._buildReadCommand() hardcodes the block confirmations for the lzRead to 15 for all chains. While this configuration may work for some chains, it can be too low or too high for others.

For example, reading from a chain more susceptible to reorgs increases the risk of the NLP contract receiving state from a block that was removed.

If we take a look at the LayerZeroV1 default configurations for sending messages, we can see popular chains like Optimism and Arbitrum use 20.

In addition, if the oApp configuration changes in the future, it won't be possible to sync the NLP contract with the current approach.

Add a configurable mapping with block confirmations for each target chain and use that instead of hardcoding the blocks.

There is an OFT for the shares token of the vault on the hub chain. These ofts can be redeemed cross chain via the lzCompose flow. In case that handleCompose() fails, the tokens are refunded back to the sender on the source chain.

        try
            this.handleCompose{ value: msg.value }(_composeSender, composeFrom, composeMsg, amount)
        {
            emit Sent(_guid);
        } catch (bytes memory _err) {
            /// @dev A revert where the msg.value passed is lower than the min expected msg.value is handled separately
            /// This is because it is possible to re-trigger from the endpoint the compose operation with the right msg.value
            if (bytes4(_err) == InsufficientMsgValue.selector) {
                assembly {
                    revert(add(32, _err), mload(_err))
                }
            }

            _refund(_composeSender, _message, amount, tx.origin);
            emit Refunded(_guid);
        }

It's expected that msg.value is a positive value, otherwise the _refund call will fail since it calls OFT.send(). That msg.value is passed as value to handleCompose().

Then the flow is handleCompose() -> _redeemAndSend() -> _send().

In the _send() function there is a check that requires msg.value to be 0 if the desired chain is the VAULT_EID. It's meant to protect users from sending value unexpectedly for same chain redeems, but in the case described, this check will cause the transaction to fail.

        if (_sendParam.dstEid == VAULT_EID) {
            /// @dev Can do this because _oft is validated before this function is called
            address erc20 = _oft == ASSET_OFT ? ASSET_ERC20 : SHARE_ERC20;

            if (msg.value > 0) revert NoMsgValueExpected();
            IERC20(erc20).safeTransfer(_sendParam.to.bytes32ToAddress(), _sendParam.amountLD);
        }

Because of that, crosschain redeems where the destination chain is the hub will be refunded.

You can move the NoMsgValueExpected check in the public redeemAndSend and depositAndSend functions or remove the if(msg.value > 0) revert NoMsgValueExpected() check and adding appropriate warnings to the frontend so users don't send funds when they don't have to.

When SHARE_OFT is sent from a source chain to the hub chain, lzCompose will try calling handleCompose() and if it fails, the share tokens will be refunded back on the source chain.

        try
            this.handleCompose{ value: msg.value }(_composeSender, composeFrom, composeMsg, amount)
        {
            emit Sent(_guid);
        } catch (bytes memory _err) {
            /// @dev A revert where the msg.value passed is lower than the min expected msg.value is handled separately
            /// This is because it is possible to re-trigger from the endpoint the compose operation with the right msg.value
            if (bytes4(_err) == InsufficientMsgValue.selector) {
                assembly {
                    revert(add(32, _err), mload(_err))
                }
            }

            _refund(_composeSender, _message, amount, tx.origin);
            emit Refunded(_guid);
        }

Inside _handleCompose(), the _redeemAndSend() function will refund any surplus msg.value to tx.origin. This can be weaponized to grief the redeem:

• A user delegates code to their account with a type 4 transaction
• The user hijacks the call to lzCompose()
• The users sends more value than needed in order to trigger the refund
• The user reverts the first refund
• The catch block will be entered and the share asset will be sent back to the original owner on the source chain.

In result the malicious user successfully griefed the redemption of the share assets at the expense of the send fee.

Consider refunding to address(this) instead of tx.origin in handleCompose(). After handleCompose() finishes you can measure the change in the balance of the contract and if it's less than msg.value, refund the difference to tx.origin.

Cooldown is copied to recipients only when balanceOf(to) == 0. An attacker can pre-seed an address with a dust balance older than the delay, then transfer a large amount to that address; the recipient’s lastDepositTime is not updated, letting them redeem immediately and bypass the intended withdrawal delay.

On every transfer propagate the max of sender/recipient cooldown (or use a balance-weighted accumulator), not just for zero-balance recipients, to prevent cooldown reset/bypass via pre-seeded addresses.

The _sendOFT function hardcodes the refund address to address(this) and the contract exposes no method to withdraw native currency. Any overpayment of msg.value, LayerZero refund, or direct ETH transfers become trapped, leaving migration operators unable to recover funds.

Forward refunds to the caller/owner or add a payable withdraw function so native refunds are reclaimable.

When a compose message is received by GenesisVaultComposer, the msg.value sent should be positive if the end chain is different than the hub and 0 otherwise.

If the OFT has enforced options for all composed messages to require msg.value sent, users would end up overpaying for the cases they don't need to send native tokens.

If there aren't enforced options, users should carefully pass their own extra options, increasing the risk of failures.

Consider how should compose options be handled. If you decide to stick with enforced options, a refund mechanism for the unused native tokens may be helpful.

During migration, all of the assets of the vaults are converted to a desired stablecoin and sent to the hub chain. Then each user can claim part of the shares of the vault on the hub chain. The amount they are eligible to claim depends on how much shares are they holding from the migrated vault. While this approach works for EOAs, it may fail for contracts. For example, a contract on chain A may not have the same address on chain B. This can happen for different reasons, one of which is the contract being deployed with the CREATE opcode after which the deployer's nonce is used to deploy another contract. In this scenario, the tokens of these contracts will be lost.

This can be mitigated if all of the contracts that don't have access to their address on the hub chain redeem their shares before the migration happens. This is possible because when a migration is initiated in MigrationModule, only deposits are disabled, but withdrawals can still happen until the delay of minimum one day passes and the migration is started. However, accounts holding less value than the minimum withdrawable amount will not be able to get their funds out because of the following check in VaultModule

if (assets < vault.minWithdraw) {
revert Errors.BelowMinimumWithdraw(vault.minWithdraw, assets);
}

Furthermore, if the same address is controlled by different entities on the two chains, one of them may claim the assets of the other.

Before you initiate a migration, make it clear that such risks for contracts exists and set a long enough migration delay so users are able to successfully withdraw. Also consider skipping the minWithdraw check if the migration has already been initialized.

During migrations,VaultMigrator sends tokens to the hub chain through the OFT and marks the vault as migrated migrated[vaultId]. Any further attempt to migrate the same vault will revert because it has already been migrated.

On the hub chain, the OFTs will be received and handleCompose() will be executed. If handleCompose() reverts, tokens will be refunded back to the source chain. This can happen for different reasons, one of which is a failure to deposit assets to the vault. In this case, the assets would be returned to the VaultMigrator, but the vault they correspond to will not be eligible for migration.

Consider if the refund functionality is desired for migrations.

The VaultMigrator will approve the stable asset to be spent by the OFT adapter. This will allow the OFT.send to pull assets from the migrator contract.

However, not all stable tokens use the OFTAdapter flow. For example, USD.e is an OFT token, which means send will actually burn tokens from the caller, and does not use transferFrom.

Therefore, the approval for certain tokens is unnecessary, and allowance will never be used.

Consider calling OFT.approvalRequired to determine if a forceApprove is needed for given token.

Some functions are marked as payable as it may involve a LZ fee to be paid in native currency.

The NLPClaim.claim function does not require any msg.value if destEid == VAULT_EID. However, it does not enforce zero value sent, no refund at the end, and no ownable function to recover native currency in the contract.

Consider adding the same validation as in composer when destEid == VAULT_EID:

if (msg.value > 0) revert NoMsgValueExpected();

The setVaultFactoryAddress function updates critical configuration but does not emit an event, making it harder to track configuration changes off-chain.

Add an event VaultFactoryAddressSet(uint32 indexed eid, address vaultFactoryAddress) and emit it when the factory address is updated.

The quoteVerification uses options to calculate the quote, but the actual initiateClaim flow has empty options.

Additionally, this function also uses msg.sender as the user address, which is not ideal in view functions.

Consider removing the options param, and use empty bytes, just like in the initiateClaim flow, and instead add an address param for the user.

NLPClaim.initiateClaim() can be executed with user == address(0). This will result in a message that always fails in the _lzReceive check due to the following constraint

if (request.user == address(0)) {
revert MigrationErrors.InvalidGuid(guid);
}

Consider reverting if initiateClaim is called for address(0)

The VaultMigrator.destEid state variable is only set during deployment, just like the sourceEid and genesisFactory. However, destEid is not marked as immutable.

Set destEid as an immutable param in VaultMigrator.