Strategy Vault Updates

ProtocolVault.updateUnClaimed credits users with claimable assets and increments a per‑user fee accumulator:

// ProtocolVault.updateUnClaimed(...)
userClaimedById[userId][USDC_HASH].unClaimedAssets += userClaimInfos[i].assets;
crossChainFee[userId] += ccFee;

However, the non‑payable claim() path does not consult crossChainFee at all, it simply transfers the full amount and never clears any pending fee:

function claim(ClaimParams memory claimParams) external whenNotPaused {
    // no check on crossChainFee[id]
    uint256 amount = userClaimedById[id][USDC_HASH].unClaimedAssets;
    userClaimedById[id][USDC_HASH].unClaimedAssets = 0;
    delete userClaimedById[id][USDC_HASH].requestIds;
    SafeTransferLib.safeTransfer(ERC20(claimParams.token), msg.sender, amount);
}

By contrast, claimWithFee() does enforce and clear the fee:

function claimWithFee(ClaimParams memory claimParams) external payable whenNotPaused {
    require(msg.value == crossChainFee[id], "NotEnoughCCFee()");
    ...
    delete crossChainFee[id];
}

This allows a user with crossChainFee[id] > 0 to call claim() and receive their assets without paying the intended cross‑chain fee. The fee then “dangles” in storage and can continue to accumulate across updates, creating an inconsistent state and a systematic loss of expected fee revenue. If a UI later uses claimWithFee(), exact‑equality on the accumulated crossChainFee[id] may also cause confusing reverts when there is no remaining unClaimedAssets.

Therefore, users can repeatedly avoid paying cross‑chain fees while still receiving their assets, causing protocol fee leakage and stale fee balances.

Enforce fee settlement whenever a fee is pending. The simplest hardening is to block the free path when a fee exists:

// In claim():
require(crossChainFee[id] == 0, "Fee pending — use claimWithFee");

Alternatively, merge both flows into a single payable claim() that conditionally requires and clears the fee:

if (crossChainFee[id] > 0) { require(msg.value == crossChainFee[id], "fee mismatch"); delete crossChainFee[id]; }

Also consider clearing crossChainFee[id] only upon successful claims and documenting whether fees are per‑claim or per‑user to avoid future ambiguity.

In handleDexRequests, each item is flagged as handled before the contract knows whether the state change succeeded. The function sets isDexRequestHandled[requestId] = true and then calls _handleRequest. When _handleRequest returns false (typical for withdrawals if there aren’t enough pendingShares or pendingStrategyProviderShares to freeze), the function only emits DexWithdrawNotEnough(requestId) but leaves the “handled” flag set. Because the request is now considered consumed, resubmitting the same dexRequestId later, after balances are updated by the period pipeline, will be rejected with an AlreadyCalled error and the withdrawal can never progress.

Minimal excerpt showing the ordering problem:

isDexRequestHandled[requestId] = true; // set too early
if (_handleRequest(request.dexRequestData.payloadType, request.id, tokenHash, request.dexRequestData.amount) {
    emit DexRequestHandled(request);
} else {
    emit DexWithdrawNotEnough(requestId); // but requestId already consumed
}

This affects LP_WITHDRAW and SP_WITHDRAW flows, where _handleRequest checks _checkWithdraw against pending share balances and returns false when the operator has not yet staged enough pending shares for the id.

The main impact of this issue is that valid withdrawals can be permanently dropped due to timing, causing operational DoS and cross‑chain accounting mismatches.

Consider only marking a DEX request as handled after a successful state change, e.g.:

if (_handleRequest(...)) { isDexRequestHandled[requestId] = true; }

In LedgerCoreImpl.allocateToFunds, the multi‑strategy deposit path distributes pendingLpDepositAssets pro‑rata to existing main vault exposure only, computed from each strategy’s mainShares converted to assets. When a strategy is added later (e.g., at period N) it starts with mainShares == 0. The allocation loop first sums the current main exposure:

totalMainAssetsInFund += LedgerUtils._convertToAssets(
    strategyFundToken.mainShares,
    strategyFundToken.fundAssetsAfterFee,
    strategyFundToken.totalShares,
    Math.Rounding.Floor
);

and then assigns each strategy’s share:

uint256 mainAssetsInFund = LedgerUtils._convertToAssets(
    strategyFundToken.mainShares,
    strategyFundToken.fundAssetsAfterFee,
    strategyFundToken.totalShares,
    Math.Rounding.Floor
);
uint256 distributeDepositAssets =
    depositAssets.mulDiv(mainAssetsInFund, totalMainAssetsInFund, Math.Rounding.Floor);

A newly added strategy therefore receives zero from LP deposits because mainAssetsInFund == 0. Self‑funding via SP_DEPOSIT does not help: _handleSPDeposit mints strategy‑provider shares (pendingStrategyProviderShares) and updates pendingTotalShares/Assets, but does not increase pendingMainShares, so the strategy’s weight in future LP splits remains zero. Since allocateToFunds is allowed only once per period, operators cannot do a “seed then allocate” pattern in the same period.

Incorporate a method for the operator to allocate a desired seed amount of main vault capital to a newly added strategy (e.g., “rebalance from existing strategies” to ensure the newcomer starts with a non-zero main share).

LedgerCoreImpl.allocateToFunds() distributes all processed deposits and withdrawals to specified strategies. In case there is more than one strategy, funds will be split proportionally. The problem arises when there aren't any funds in the strategies, either because it's the start of the vault or because they have been naturally depleted in a previous period.

In that case, totalMainAssetsInFund will be computed as 0 and the following division will revert:

uint256 distributeDepositAssets = depositAssets.mulDiv(mainAssetsInFund, totalMainAssetsInFund, Math.Rounding.Floor);

In result, allocating the funds to multiple strategies won't be possible and the Orderly operator will be forced to choose only one strategy for the given period.

Split all the funds evenly between the strategies if totalMainAssetsInFund == 0

Signatures in the Ledger contract don't have deadline parameter which means they may be executed in any point in time. This can lead to unfavorable actions being executed, especially for dex requests.

Consider adding deadline checks.

VaultCrossChainManager._lzReceive accepts two ledger‑originated message types, ASSETS_DISTRIBUTION and UPDATE_USER_CLAIMand immediately decodes and acts on them without asserting that they actually came from the ledger chain.

In the ASSETS_DISTRIBUTION branch the code decodes the payload and then calls into ProtocolVault.depositToStrategy. Similarly, in UPDATE_USER_CLAIM the code decodes and updates claims.

Neither branch validates that strategyVaultCCmessage.srcChainId corresponds to the configured ledger chain (nor cross‑checks it against the LayerZero Origin.srcEid). While the inherited receiver already enforces that the message comes from a configured peer for a given EID, the contract may still be configured with multiple peers/EIDs. Without an explicit “ledger chain only” check here, any other configured peer could send these payload types and the handler would accept them, and the handler trusts the chain IDs embedded inside the message for decimal conversion. Impact: a misconfigured or compromised non‑ledger peer could trigger unintended strategy deposits or mutate claim balances, and incorrect src/dst IDs inside the message could skew amount conversions.

Defensively assert that these branches only execute for messages whose source is the ledger chain and, optionally, that the embedded dstChainId matches block.chainid. A minimal fix is to add a source‑chain check that uses either a stored ledgerChainId or the EID mapping together with the LayerZero Origin:

require(chainIdToEid[strategyVaultCCmessage.srcChainId] == _origin.srcEid, "invalid ledger source");

Apply the same check in both ASSETS_DISTRIBUTION and UPDATE_USER_CLAIM branches. Optionally also assert strategyVaultCCmessage.dstChainId == block.chainid to prevent forged destination IDs affecting decimal conversion.

In LedgerExtension._verifySolRequest, Solana DEX requests always validate the identifier (request.id) as an LP account ID, irrespective of the payloadType. The code path is:

function _verifySolRequest(DexRequest calldata request) internal view {
    DexRequestData calldata data = request.dexRequestData;

    // Verify id (always as Account ID)
    if (!VaultUtils.validateAccountId(data.receiver, vaultBroker[data.vaultId], request.id)) {
        revert InvalidId();
    }

    Signature.verifySOLSig(data, request.r, request.s, request.chainId, data.receiver);
}

For SP_DEPOSIT or SP_WITHDRAW, this means a legitimate strategy provider ID will be rejected because it is checked as an account ID. Conversely, a caller can submit an SP request using a valid account ID and pass _verifySolRequest. Execution then proceeds to _handleRequest; for example, the SP‑withdraw branch:

} else if (payloadType == PayloadType.SP_WITHDRAW) {
    if (_checkWithdraw(amount, strategyFundToken.frozenShares,
                       strategyFundToken.pendingState.pendingStrategyProviderShares)) {
        strategyFundToken.frozenShares += amount;
    } else {
        return false;
    }
}

Because the identifier that passed validation is an account ID (not a strategy provider ID), the SP share bucket is zero and _checkWithdraw fails; however, in handleDexRequests the request is marked as handled before invoking _handleRequest:

isDexRequestHandled[requestId] = true;  // set before attempting the state change
if (_handleRequest(...)) { ... } else { ... }  // failure does not unset the flag

This combination produces two failures: first, legitimate SP operations on Solana are blocked by incorrect on‑chain ID validation; second, a mismatched SP request that slips through the validator burns the dexRequestId (since it is flagged as handled even when _handleRequest returns false), preventing a corrected re‑submission with the same ID and forcing off‑chain remediation.

Validate the identifier according to payloadType and only mark isDexRequestHandled after a successful state transition. Concretely, in _verifySolRequest branch on the payload:

if (data.payloadType == PayloadType.LP_DEPOSIT || data.payloadType == PayloadType.LP_WITHDRAW) {
    require(VaultUtils.validateAccountId(data.receiver, vaultBroker[data.vaultId], request.id), "InvalidId");
} else if (data.payloadType == PayloadType.SP_DEPOSIT || data.payloadType == PayloadType.SP_WITHDRAW) {
    require(VaultUtils.validateSPId(protocolVault /* or SP namespace */, vaultBroker[data.vaultId], request.id), "InvalidId");
} else {
    revert InvalidType();
}

Then, in handleDexRequests, assign isDexRequestHandled[requestId] = true only if _handleRequest returns true; if it returns false, revert (or store a separate “failed” status) so the DEX can correct and resubmit without losing the request ID.

In the multi‑fund branch of LedgerCoreImpl.allocateToFunds, the contract splits deposits and withdrawals proportionally across strategies using different rounding directions per fund and does not perform any balancing step afterward. For deposits, the per‑fund split uses Math.Rounding.Floor:

uint256 distributeDepositAssets =
    depositAssets.mulDiv(mainAssetsInFund, totalMainAssetsInFund, Math.Rounding.Floor);

Every slice is rounded down, so the sum of all distributeDepositAssets is ≤ pendingLpDepositAssets, leaving unassigned “dust” that never reaches any strategy’s pendingTotalAssets in that period. For withdrawals, the split uses Math.Rounding.Ceil:

uint256 distributeWithdrawAssets =
    withdrawAssets.mulDiv(mainAssetsInFund, totalMainAssetsInFund, Math.Rounding.Ceil);

Every slice is rounded up, so the sum of all distributeWithdrawAssets is ≥ pendingLpWithdrawAssets and the contract subtracts more from strategies’ pendingTotalAssets than the requested withdrawal. Over time, repeated floors on deposits and ceils on withdrawals create a systematic downward drift in per‑fund asset totals, which can bias NAV/share pricing. As a simple illustration with three equal funds and 100 units: floor on deposits yields 33+33+33=99 (1 unit not allocated), while ceil on withdrawals yields 34+34+34=102 (2 units over‑removed). In edge cases with many funds and very small amounts, the per‑fund over‑subtractions can even trigger underflow reverts.

Use an exact‑sum allocation scheme so that per‑fund amounts add up exactly to the total for both deposits and withdrawals. Two robust approaches are:

• Largest‑remainder method: compute exact rationals, take floors for all, then assign the remaining “dust” to funds with largest fractional remainders.
• Balance‑on‑last: floor the first N‑1 allocations and set the last allocation to total − sumFloors. For example, for the last fund: distributeAssets_last = totalAssets - sumAllocatedSoFar;

LedgerCoreImpl.updateUnclaimed() quotes LayerZero for the native fee that must be paid on the Orderly chain. It will then encode the amount in the crosschain message sent to the Vault side where it will be recorded in crossChainFee and will be paid by the user upon claim. However, if the vault chain has a different native token than the Orderly chain, this action will result in accounting mismatch. For example, if the contracts are deployed on Polygon PoS, the fee recorded will be in MATIC instead of ETH.

Add additional logic that handles these cases if you decide to deploy to such chains.

LedgerExtension.handleDexRequests() doesn't check if the token provided is USDC, which makes it possible to use the system with other tokens even if they are not supported.

Skip any requests if the token they are working with is not USDC.

LedgerExtension is designed to be used only via delegatecall from ProtocolVaultLedger, but it exposes public/external entry points without enforcing that call context. If someone calls the implementation address of LedgerExtension directly, its functions execute against the implementation’s own storage, not the ledger’s storage, and can still emit “successful” events (e.g., OperationHandled, DexRequestHandled) and update mappings like isDexRequestHandled within the implementation contract. This does not mutate the real ledger state but can pollute on‑chain telemetry and generally widens the surface for future mistakes. The risk is currently limited to observability and monitoring confusion because state changes remain isolated to the implementation contract.

Add an only‑delegatecall guard to every external entry point in LedgerExtension (and mirror it in other implementation modules) so that calls revert unless executed via the proxy. A minimal pattern is to record the implementation’s own address once and require that address(this) differs when invoked (i.e., reject direct calls): require(address(this) != IMPLEMENTATION_SELF, "impl: only delegatecall");.

In the LedgerCoreImpl contract's updateUnclaimed function, the cross-chain fee ccFee obtained from the IVaultCrossChainManager's quoteClaim function is divided by the number of valid unclaimed requests len before encoding it into the StrategyVaultCCMessage payload as abi.encode(periodId, ccFee / len, userClaimInfos). Solidity performs integer division on uint256 types by default flooring the result and discarding any remainder, meaning that for non-divisible values the computed per-claim fee will be lower than intended. For instance, if ccFee equals 10 and len equals 3, then ccFee / len evaluates to 3 with a remainder of 1 lost, resulting in the payload implying a total fee of 3 multiplied by 3 equaling 9 which is less than the originally quoted 10. This truncation happens because the division operator in Solidity does not round up or preserve fractions, leading to a systematic underestimation when the fee is not perfectly divisible by the batch size. The code snippet illustrating this is:

(ccFee,) = IVaultCrossChainManager(crossChainManager).quoteClaim(chainId, message);
message = _createCCMessage(
    PayloadType.UPDATE_USER_CLAIM, chainId, abi.encode(periodId, ccFee / len, userClaimInfos)
);

Here, the quoted ccFee is for the entire batch message, but the encoded per-claim value may cause the destination chain's processing logic to reconstruct a total that is insufficient if it multiplies back by len. The impact is potential underfunding of cross-chain messages leading to delivery failures, reverts.

Replace the division with a ceiling calculation using (ccFee + len - 1) / len to ensure the total implied fee when multiplied back meets or exceeds the quoted amount, and add a check to revert if the per-claim fee is zero for non-empty batches.

LedgerCoreImpl.updateUnclaimed() is designed to handle duplicates in the requestIds passed by the BE. This becomes evident from the second check for a valid request.

// Ignore if handled
if (_isValidRequestId(requestIds[i])) {
    userClaimInfos[index] = userClaimInfo[requestIds[i]];
    isUserClaimHandled[requestIds[i]] = true;
    index++;
    delete userClaimInfo[requestIds[i]];
}

Even though these requests won't be included in the userClaimInfos array, its length has already been set to len which includes all the duplicates.

In result, userClaimInfos will contain empty slots instead of duplicates, but there is a bigger problem - the fee calculation: ccFee / len

Since len includes the empty slots, the individual fee paid for each request will be diluted. Nobody will pay the fees for the empty slots which means they have to be covered by the Orderly protocol.

In addition, on the EVM side the userClaimedById and crossChainFee mappings will be populated for address(0) because of these empty slots.

The simplest solution is to compute the individual fee to be paid as ccFee / index instead of ccFee / len. While this won't stop the mappings being incorrectly populated, the fee dilution problem will be solved.

If you want to fix both issues, you should redesign the function.

Additional check that ensures fee cannot be changed before NAV is updated was added to the setFeeRate() function in response to the following finding from a previous audit round -

However, in the latest commit, the check was modified to work for the previous period, not the current one

        if (latestPeriodId != 0 && !isUpdateStrategyFundAssets[latestPeriodId - 1]) {
            revert NotAllowedTime();
        }

For the vault to work correctly, once updatePeriodId() is called, it means updateStrategyFundAssets() has already been called for the previous period. This means the check in setFeeRate() will be always passing, reintroducing the issue from the original report. An exception is if setFeeRate() is called in period 1 because updateStrategyFundAssets() may have not been called in period 0. In that case, the fee can't be changed until period 1 ends.

Check for the current period, not the previous one.

-        if (latestPeriodId != 0 && !isUpdateStrategyFundAssets[latestPeriodId - 1]) {
+        if (latestPeriodId != 0 && !isUpdateStrategyFundAssets[latestPeriodId]) {
            revert NotAllowedTime();
        }

LedgerExtension.handleDexRequests() performs validation to confirm the account id to be modified is derived by the user address and the brokerHash. The brokerHash is fetched from vaultBroker[data.vaultId] using the data.vaultId field. However, there is no further validation applied to brokerHash. For example, it may be empty. This will yield invalid account id which is still controlled by the user and can be claimed by them, but is not derived from a valid broker. In result, users are able to bypass the accounts system and operate with invalid accounts.

It's also possible that vaultBroker for that vaultId was changed to address(0) by calling setVaultBroker(). One would expect accounts with that broker won't be used anymore, but the issue shows the opposite.

Ensure brokerHash is not empty after fetching it in _verifyEVMRequest and _verifySolRequest()

All of the three contracts - ProtocolVaultLedger, LedgerCoreImpl and LedgerExtension - must have the same storage layout because the first one delegate calls the other two. The current layout is fine because none of the other contracts ProtocolVaultLedger inherits from has any state.

Be careful with future updates since they can corrupt the Ledger storage layout.

In VaultAdapter._processDeposit, the tokenAmount field of VaultDepositFE is assigned using an explicit cast:

VaultDepositFE memory depositData = VaultDepositFE({
    accountId: id,
    brokerHash: brokerHash,
    tokenHash: adapterDeposit.tokenHash,
    tokenAmount: uint128(adapterDeposit.amount) // amount is already uint128
});

If adapterDeposit.amount is indeed uint128, this cast is redundant and slightly reduces readability while adding a negligible cost.

Remove the cast as adapterDeposit.amount is already uint128.

In _handleRequest, the function pre‑derives both storage pointers:

AccountToken storage accountToken = accountTokenInfo[id][tokenHash];
StrategyFundToken storage strategyFundToken = strategyFundTokenInfo[id][tokenHash];

irrespective of payloadType. In the LP_* branches only accountToken is used; in the SP_* branches only strategyFundToken is used. While this does not perform an SLOAD by itself, deriving a nested‑mapping storage pointer requires one or more keccak256 computations to obtain the slot. Computing the unused pointer on every call therefore incurs avoidable gas for each request (e.g., ~two extra keccak256 operations per call in typical paths), with no functional benefit.

Compute the storage pointer lazily inside the relevant branch only. For example, declare AccountToken storage a = accountTokenInfo[id][tokenHash]; only in LP_* branches and StrategyFundToken storage s = strategyFundTokenInfo[id][tokenHash]; only in SP_* branches. This avoids deriving unused mapping slots and saves gas on every request.

VaultAdapter.depositNative forwards native currency to the DEX vault:

function depositNative(AdapterDeposit memory adapterDeposit, bytes calldata signature) external onlyOperator {
    (uint256 fee, VaultDepositFE memory depositData) = _processDeposit(adapterDeposit, signature);
    uint256 totalValue = adapterDeposit.amount + fee;
    IDexVault(dexVault).depositTo{value: totalValue}(adapterDeposit.receiver, depositData);
    emit DepositFromCeffu(adapterDeposit, true);
}

However, the function is not marked payable. As a result, the caller cannot attach ETH to this transaction to fund totalValue. The call to depositTo{value: totalValue} will revert if the adapter contract’s balance is insufficient, effectively causing a denial of service for native deposits unless the contract is pre-funded out-of-band via receive(). This contradicts the function’s intent (“Deposit native token (ETH) to the specified receiver”) and makes operational flows brittle, since success depends on prior top‑ups and exact balance management.

Moreover, this forces the top-ups and deposits to be executed atomically otherwise a different operator could make use through another deposit call of the native assets that were sent but not yet consumed by the contracts.

Consider declaring the function payable and validate the provided value against the required total:

function depositNative(AdapterDeposit memory adapterDeposit, bytes calldata signature) external payable onlyOperator {
    (uint256 fee, VaultDepositFE memory depositData) = _processDeposit(adapterDeposit, signature);
    uint256 totalValue = adapterDeposit.amount + fee;
    require(msg.value == totalValue, "Incorrect msg.value");
    IDexVault(dexVault).depositTo{value: msg.value}(adapterDeposit.receiver, depositData);
}

If you prefer to allow overpayment, require msg.value >= totalValue and refund the excess before calling depositTo.

After internal DEX withdraw happens, handleDexRequests() will increase the user's frozenShares, then updateLPAndStrategyFund will finalize the withdrawal, but it will also populate userClaimInfo for that requestId.

The BE should differentiate between normal requests and DEX requests and not call updateUnclaimed() for the DEX requests, but it's still technically possible. If this happens users will receive their funds twice - once by internal transfer and once by claiming from the ProtocolVault on the EVM chain.

To avoid the risk completely add a check to updateLPAndStrategyFund() that skips writing to userClaimInfo if the current request is a DEX one.

The ledger’s critical state transitions depend on values computed and orchestrated off‑chain by privileged actors (the engine, operator, DEX signers and the cross‑chain manager). On‑chain code primarily verifies signatures and roles, not the economic correctness of the supplied data. For example, in LedgerCoreImpl.updateStrategyFundAssets the engine signs the per‑strategy totalAssets (NAV) array that drives performance‑fee minting and share‑price updates; the contract only calls Signature.verifyUpdateFundAssets(...) and then computes fees and fundAssetsAfterFee from those off‑chain NAVs. If the NAVs are stale or manipulated, performance fees and main/share accounting are affected with no on‑chain way to self‑correct. In LedgerCoreImpl.updateLPAndStrategyFund the engine submits the list of operations (LP/SP deposits/withdrawals, amounts and requestIds). The contract verifies the signature and applies conversions and freezes, assuming the off‑chain stream correctly reflects real deposits/withdrawals observed on other chains; misuse or bugs can mis‑credit unAllocatedAssets or freeze the wrong amounts until later settlement. In LedgerCoreImpl.allocateToFunds the operator chooses which strategy IDs participate in the split for the period; the on‑chain logic does proportional math but does not enforce any fairness or inclusion invariants, so a strategy can be starved or over‑favored by off‑chain selection. In LedgerCoreImpl.distributeAssets the engine dictates the cross‑chain distribution tuple (chainId, assets) per strategy and the ledger blindly constructs and sends cross‑chain messages through the configured manager; there is no on‑chain reconciliation that the distribution matches pending balances. Finally, in LedgerCoreImpl.updateUnclaimed the engine provides the set of requestIds to finalize and the contract clears and forwards them; omission or mis‑ordering off‑chain leaves claims pending indefinitely. Similar dependence exists in LedgerExtension.handleDexRequests, where the contract trusts DEX‑provided signatures and request formatting to mutate unAllocatedAssets or frozenShares. These flows are role‑ and signature‑gated, but economic truth (correct NAVs, operation completeness, fair inclusion, and accurate distributions) is externalized to off‑chain processes.

Merely informative. Note that any error in the offchain components of the protocol can have a severe impact in the protocol.

LedgerExtension.rebalance directly edits the final per‑fund fields (totalAssets, totalShares, mainShares) but does not update pendingState.* or fundAssetsAfterFee. Later, the period pipeline calls settleMainAndStrategyFunds, which commits final fields from the pending state, thereby clobbering the earlier rebalance edits. During the window between rebalance and settleMainAndStrategyFunds, other flows (notably allocateToFunds) read a mixed source of truth. Updated mainShares/totalShares paired with stale fundAssetsAfterFee, which can misweight allocations and make conversions inconsistent with the intended post‑rebalance basis.

Settle overwrites final fields from pending:

// LedgerCoreImpl.settleMainAndStrategyFunds(...)
strategyFundToken.totalShares = pendingState.pendingTotalShares;
strategyFundToken.totalAssets = pendingState.pendingTotalAssets;
strategyFundToken.mainShares  = pendingState.pendingMainShares;

Allocation uses an inconsistent triplet because rebalance doesn’t touch fundAssetsAfterFee or pendingState:

// LedgerCoreImpl.allocateToFunds(...)
mainAssetsInFund = LedgerUtils._convertToAssets(
    strategyFundToken.mainShares,         // updated by rebalance()
    strategyFundToken.fundAssetsAfterFee, // NOT updated by rebalance()
    strategyFundToken.totalShares,        // updated by rebalance()
    ...
);

Because of this, the intended cross‑fund redistribution performed by rebalance does not persist across settlement and before settlement it can skew per‑fund weights and conversions.

Consider applying rebalance deltas to pendingState.pendingTotalAssets, pendingState.pendingTotalShares and pendingState.pendingMainShares (and, if your pricing depends on it, reconcile fundAssetsAfterFee).

During a rebalance() , the fund.lastSharePrice variable is used to calculate the main shares to be added or removed.

uint256 sharePrice = fund.lastSharePrice;
...
deltaShares = deltaAssets * 10 ** USDC_DECIMAL / sharePrice;

This price is modified at the beginning of the period when uploading the NAV via updateStrategyFundAssets()

strategyFundToken.lastSharePrice = (fundAssets - performanceFee) * 10 ** USDC_DECIMAL / fundShares;

However, _handleSPDeposit converts the deposited assets to shares using the current price of the SP fund, not the cached one.

        uint256 depositShares = LedgerUtils._convertToShares(
            amount, strategyFundToken.fundAssetsAfterFee, strategyFundToken.totalShares, Math.Rounding.Floor
        );

In case fundAssetsAfterFee and totalShares are 0, shares and assets are priced 1:1. This discrepancy between the SP share price used in rebalance and the SP share price in _handleSPDeposit will cause accounting mismatch and lead to unfair funds distribution. For example:

1. LP deposits 100 USDC and receives 100 shares
2. SP deposits 100 USDC to SP_B and receives 100 shares
3. The LP funds are allocated to SP_B (mainShares = 100, totalShares = 200, assets = 200)
4. At the beginning of the next period the NAV is updated and SP_B reports a double increase, i.e assets = 400 and lastSharePrice = 2
5. The SP withdraws all their shares from SP_B and receives 200 USDC (mainShares = 100, totalShares = 100, assets = 200)
6. Rebalance moves all of the LP’s funds to SP_A . This leaves SP_B empty
7. In the next period the SP deposits again 100 USDC to SP_B. Since it’s empty, they receive 100 shares.
8. Rebalance moves all of the LP’s funds to SP_B. It uses lastSharePrice = 2 and converts 200 USDC to 100 shares.
9. Now the LP and SP have both 100 shares, or 150 USDC to withdraw.
10. In the end, the SP stole 50 USDC from the LP.

Consider if caching the lastSharePrice is needed instead of using the same ratio as spDeposit and spWithdraw

In _lzReceive of VaultCrossChainManager, the incoming OperationData.amount is always passed through decimal conversion whenever isSpecialDecimal[tokenHash][srcChainId] is set, regardless of the operation type. This is correct for deposits (where amount is in asset units) but wrong for withdrawals, where amount represents shares, a dimensionless value that must not be scaled by token decimals.

Relevant code (current behavior):

// _lzReceive(...)
OperationData memory operationData = abi.decode(payload, (OperationData));
uint256 srcChainId = strategyVaultCCmessage.srcChainId;
if (isSpecialDecimal[operationData.tokenHash][srcChainId]) {
    operationData.amount = _convertAmount(
        operationData.amount,
        tokenDecimals[operationData.tokenHash][srcChainId],
        ledgerDecimal
    );
}
IProtocolVaultLedger(ledger).handleOpFromVault(payloadType, srcChainId, operationData);

For LP_WITHDRAW/SP_WITHDRAW, this scaled value is then treated as shares by the ledger:

• Intake freezes that (now wrong) amount of shares:

// LedgerExtension._handleRequest
if (payloadType == PayloadType.LP_WITHDRAW) {
    if (_checkWithdraw(amount, accountToken.frozenShares, accountToken.pendingShares)) {
        accountToken.frozenShares += amount; // amount is assumed to be shares
    } else {
        return false;
    }
}

• Settlement later requires the amount to be <= frozenShares and uses it to compute assets:

// LedgerCoreImpl._handleLpWithdraw
LedgerUtils.requireEnoughFrozenShares(amount, accountToken.frozenShares);
uint256 withdrawAssets = LedgerUtils._convertToAssets(amount, mainAssetsAfterFee, mainShares, Math.Rounding.Floor);

Because _lzReceive has down‑ or up‑scaled shares, the ledger freezes the wrong number of shares. In practice, operators will typically settle an amount that matches what the ledger froze to avoid reverts, which produces silent under‑withdrawals (e.g., user intended 600 shares; only 6 were frozen and settled). If the operator instead tries to settle the original amount, settlement reverts with NotEnoughFrozenShare, creating a DoS for that request. In short, the unit mismatch corrupts withdraw flows and can either underpay users or block settlement.

Only perform decimal conversion for deposits (asset units). Do not convert operationData.amount for LP_WITHDRAW/SP_WITHDRAW, since it represents shares. A minimal fix is to gate the conversion:

// Pseudocode inside _lzReceive before calling handleOpFromVault
if ((payloadType == LP_DEPOSIT || payloadType == SP_DEPOSIT) &&
    isSpecialDecimal[operationData.tokenHash][srcChainId]) {
    operationData.amount = _convertAmount(operationData.amount,
        tokenDecimals[operationData.tokenHash][srcChainId],
        ledgerDecimal);
}
// For withdraws, forward amount unchanged.

When the owner requests a rebalance, the function is expected to clear the previous request and store the new one.

bytes32 rebalanceId = keccak256(abi.encode(vaultId, targetRatios, block.timestamp));

// Clear and push rebalance request
delete rebalanceRequest[rebalanceId];

However, the rebalanceId of a request is computed as keccak256(abi.encode(vaultId, targetRatios, block.timestamp)), meaning each request will have a different ID unless all parameters are exactly the same. As a result, the same vault and strategy providers can have multiple valid rebalance requests concurrently, since the previous request is not actually cleared.

Additionally, the rebalance request creation timestamp is neither emitted in an event nor stored in storedRatios, and the rebalance function does not take it into account. As a result, an earlier request can be executed after a later one, effectively overriding the most recent rebalance request.

For example:

• The owner creates a rebalance request with a 30:70 ratio for 2 SPs in the vault.
• Later, the owner creates a new rebalance request with a 40:60 ratio for the same 2 SPs, assuming it would clear the previous request.
• The previous request has not been cleared, so both requests remain valid.
• The operator executes the second request first, then executes the first request.
• As a result, the funds are rebalanced based on the 30:70 ratio.

Consider having requestIds based only on vaultId and SP IDs, without including the ratios or block.timestamp. This way, there will be only one active request for the same vault–SP pair, even if the intended ratios change.

To disable a strategyProvider , first isAllowedStrategyProvider must be disabled in the ProtocolVault contract, then all in-flight messages must be executed and finally disable the SP in the ProtocolVaultLedger contract.

Otherwise the in-flight messages will be executed and the OperationHandled event will be emitted with a disabled SP which will lead to the reverting of updateLPAndStrategyFund() and DOS the strategy vault, since there the SP is checked against the allowed ones.

The rebalance() function treats the last SP as a fund that receives all of the assets leftovers after the previous SPs have been allocated to.

        for (uint256 i = 0; i < len; i++) {
            if (i < len - 1) {
                targetAssets[i] = totalMainAssets.mulDiv(targetRatios[i].ratio, REBALANCE_BASE, Math.Rounding.Floor);
                allocatedAssets += targetAssets[i];
            } else {
                targetAssets[i] = totalMainAssets - allocatedAssets;
            }
        }

Since the targetAssets are rounded down for each fund, the leftover assets will be assigned to the last SP even if it’s targetRatio = 0

To avoid assigning funds to an SP with targetRatio = 0, make sure the last SP in the rebalanceRequest array has a positive targetRatio. This can also be enforced in the ProtocolVaultLedger.requestRebalance() function.

When LedgerExtension rebalances the assets in the SP funds, it modifies the totalAssets , mainShares and totalShares.

deltaAssets = currentMainAssets - targetAssets[i];
deltaShares = deltaAssets * 10 ** USDC_DECIMAL / sharePrice;
fund.totalAssets -= deltaAssets;
fund.totalShares -= deltaShares;
fund.mainShares -= deltaShares;

Since deltaShares are rounded down, mainShares can end up being slightly larger than expected. This can become problematic if there are no SP shares because it will result in the fund not having any assets - because they were fully subtracted, but still having dust shares. This will also impact the hwm because it won’t enter the zero case during settlement

        if (totalShares == 0) {
            return 10 ** decimal;
        }

If targetAssets[i] == 0, execute the following logic instead

fund.totalAssets -= currentMainAssets;
fund.totalShares -= fund.mainShares;
fund.mainShares = 0;

In the VaultCrossChainManager contract, quoteClaim(uint256 chainId, StrategyVaultCCMessage message) derives the destination endpoint ID (EID) from the function parameter chainId, whereas the actual send path sendMessage derives the destination from message.dstChainId. If a caller supplies a chainId that differs from message.dstChainId, the quoted LayerZero fee will be computed for one destination but the send will target another, producing incorrect fees.

Illustrative code:

// quoting path
function quoteClaim(uint256 chainId, StrategyVaultCCMessage memory message) public view returns (MessagingFee memory) {
    bytes memory lzMessage = abi.encode(message);
    uint32 dstEid = chainIdToEid[chainId]; // uses param
    MessagingFee memory fee = _quote(dstEid, lzMessage, _getOptions(message.payloadType), false);
    return fee;
}

// send path
function sendMessage(StrategyVaultCCMessage memory message) external onlyLedger {
    bytes memory lzMessage = abi.encode(message);
    uint32 dstEid = chainIdToEid[message.dstChainId]; // uses message field
    MessagingFee memory fee = _quote(dstEid, lzMessage, _getOptions(message.payloadType), false);
    _lzSend(dstEid, lzMessage, _getOptions(message.payloadType), fee, payable(address(this)));
}

When the two values diverge, operators may underfund the send (leading to NotEnoughNative and failed delivery) or overfund. While current usage appears to keep them equal, the API remains a footgun that can surface as intermittent delivery failures or unnecessary balance management overhead.

Consider removing the chainId parameter from quoteClaim and derive the destination solely from message.dstChainId.

ProtocolVaultLedger exposes several operator entry points that must be run in a specific per‑period order for accounting and custody to stay consistent. The contract only enforces “once per period” flags (e.g., isAllocatedToFunds, isAssetDistributed) and period equality (_check(periodId)) on some functions, but it does not enforce the required sequencing between them. Several calls accept any periodId and can be executed out of order without reverting.

Examples that compile and run but produce inconsistent state:

• allocate before NAV: Calling allocateToFunds before updateStrategyFundAssets uses stale fundAssetsAfterFee/lastSharePrice for per‑fund conversions, mispricing shares.
• distribute without period gating: distributeAssets has no _check(periodId) and can be called for any period (including “after” updatePeriodId), decoupling custody movement from the accounting window.
• …

Because of this, operators can unintentionally execute valid transactions in the wrong order, causing mispricing, inconsistent books vs custody and operational foot‑guns that are hard to detect and fix.

Enforce an explicit per‑period state machine and add ordering preconditions:

• Add a stage enum per period (e.g., None → NavUploaded → OpsProcessed → Allocated → Distributed → FundsSettled → AccountsSettled → Closed).
• Gate each function with required stage(s).