Protocol Review

Users deposit underlying tokens through the SY wrapper, GenesisVaultSYToken. The wrapper calls deposit function on Vault module, where the logic records a cooldown timestamp for the caller address.

        // Update vault state
        vault.sharesOf[user] += shares;
        vault.totalShares += shares;
        vault.lastDepositTime[user] = block.timestamp;

When the SY wrapper deposits, msg.sender is the SY contract address, so the vault stores one lastDepositTime for the entire wrapper. When anyone later redeems via the SY wrapper, the wrapper calls withdraw on VaultModule contract. That path enforces the withdrawal delay against the caller address.

        // Check withdrawal delay
        uint256 nextWithdrawalTime = vault.lastDepositTime[user] +
            vault.withdrawalDelay;

        if (vault.withdrawalDelay > 0) {
            if (nextWithdrawalTime > block.timestamp) {
                revert Errors.WithdrawalDelayNotMet(
                    nextWithdrawalTime,
                    block.timestamp
                );
            }
        }

Because the cooldown is tracked once for the SY address (not per end‑user), any deposit via the SY wrapper resets lastDepositTime and pushes out the withdrawal window for every SY holder. An attacker can grief all SY redemptions by repeatedly depositing the minimum allowed amount just before the delay expires, creating a redemption DoS for all users who hold SY for that vault.

Move the cooldown enforcement to the SY wrapper on a per‑user basis. Track lastDepositTime[user] inside the SY contract and enforce the delay on redeem. Skip the further withdrawal delay validation for registered SY wrappers.

The protocol performs a check that the given user did not receive 0 shares and they have a configurable parameter to set a minDeposit amount for vaults. These security checks will decrease impact and likelihood but depending on the configs and the given situation a share price inflation attack may still be possible.

Be aware and consider to further mitigate against share price inflation attacks by implementing virtual shares or minting shares directly in the init flow of a new vault.

Vaults without a strategy route deposits to the Genesis Factory and price shares using getTotalAssets, which, in no‑strategy mode, returns the Factory’s entire balance of the vault’s liquidityToken.

    function getTotalAssets(
        Data storage vault
    ) internal view returns (uint256) {
        if (address(vault.strategy) == address(0)) {
            return IERC20(vault.liquidityToken).balanceOf(address(this));
        }

        return IGenesisStrategy(vault.strategy).totalAssets();
    }

Because this balance is shared across all no‑strategy vaults that use the same token, a deposit into Vault B increases the Factory’s balance and immediately inflates getTotalAssets for Vault A. Conversions then use contaminated AUM value, letting redemptions in one vault withdraw assets funded by another, and mispricing new mints.

Use per‑vault custody. Track each vault assets and use it in getTotalAssets instead of the Factory balance.

Users are able to drain the factory if multiple vaults share the same strategy. The reason for this is that they both use the same summed up total amount for the share price calculation but a different totalShare value.

For example:

• Vault A is connected to Strategy A and 100 WETH is deposited
• Vault B is created and connected to Strategy A
• Both vaults share the same totalAssets amount (balanceOf the strategy contract)
• Eve deposits 1 WETH into Vault B and therefore owns 100% of the shares of Vault B
• Eve withdraws all of her shares and by doing so receives: assetsOut = vaultBSharesIn * summedUpAssets / totalVaultBShares = 1e18 * 101e18 / 1e18 = 101e18

Consider to enforce a 1 to 1 relationship between vaults and strategies.

The emergencyRecover function in the GenesisVaultSYToken contract is unreachable as it has a onlyFactory modifier. But there is no contract in the factory to call it.

Consider adding a function to the factor to be able to call it.

The AUDIT_SCOPE.md document talks about a Configurable deposit/withdrawal limits in the Key Features of the vault. However only a configurable deposit limit was implemented.

Consider to implement a configurable withdrawal limit too or adjust the docs accordingly.

Users can deposit directly into the Genesis vault (VaultModule.deposit) or via the SY wrapper (GenesisVaultSYToken.deposit). When depositing via SY, the Genesis factory mints vault shares to the SY contract, and the SY contract mints an equal amount of ERC-20 tokens to the user. The SY token exposes an exchangeRate function that indicates the number of underlying assets per SY token.

    function totalAssets() public view returns (uint256) {
        return IVaultModule(factory).totalAssets(vaultId);
    }

    /// @notice Get the current exchange rate (assets per SY token)
    /// @return rate Exchange rate scaled by 1e18
    function exchangeRate() public view override returns (uint256 rate) {
        uint256 _totalSupply = totalSupply();
        if (_totalSupply == 0) {
            return 1e18; // 1:1 for empty vault
        }

        uint256 _totalAssets = totalAssets();
        return (_totalAssets * 1e18) / _totalSupply;
    }

Because the vault allows direct deposits (EOAs mint vault shares that are not represented by SY supply), totalAssets includes all assets, while SY.totalSupply includes only SY-backed shares. As soon as non-SY shares are outstanding, the exchangeRate becomes inflated. Any off-chain valuation, UI or DEX logic that uses exchangeRate as a price proxy will be incorrect, which may open further attack scenarios.

Compute the SY exchange rate as the price per share, using the vault’s convertToAssets, so it reflects all outstanding shares (SY + direct holders).

function exchangeRate() public view override returns (uint256) {
    uint256 rate = IVaultModule(factory).convertToAssets(vaultId, 1e18);
    return rate == 0 ? 1e18 : rate;
}

A withdraw delay can be configured which reduces the likelihood but it may still be possible and profitable to front a yield distribution in the underlying vault of a strategy and deposit a lot of funds into the vault to gain most of the yield and exit right after the withdraw delay passed. So their may still be enough incentive for MEV here.

Be aware and consider one of these options:

• Always configure the withdraw delay accordingly
• Only pick vaults which do not have step-wise jumps in their share price value
• Implement a mechanism that distributes yield over time

Multiple functions which perform critical state changes do no emit an event:

• ConfigurationModule::transferOwnership
• StrategyModule::setPerformanceFee

Consider emitting events in all functions which perform critical state changes to follow best practices.

The harvest flow is the following:

• All of the accrued yield is withdrawn
• The performance fee is collected
• The rest of the yield is deposited back into the vault

This is very capital inefficient if there is a withdraw and or deposit fee in the given vault.

Be aware and consider to not use vaults with deposit or withdraw fees or rewrite the logic in that case to only withdraw the performance fee.

The validateStrategy function does not check if the given strategy contract uses address(this) as factory contract. Using a strategy with a different address set as factory could lead to DoS or other major issues.

Consider implementing this check.

The GenesisVaultSYToken emits events which do not strictly follow the ERC-5115 SY token standard: https://eips.ethereum.org/EIPS/eip-5115

Consider to follow best practices and implement the same events as specified in the ERC-5115 SY Token standard.

There is a unsafe transfer call in the collectPerformanceFee function as well as unsafe approve operations globally.

Consider always using safeTransfer and safeApprove functions to follow best practices.

There is unused code in multiple parts of the system.

Unused Errors:

• NoFeeRecipient
• NoFeeConfigured
• OnlyRouter
• InvalidAsset
• InvalidRouter
• InsufficientBalance

Unused Imports:

• IGenesisStrategy in the VaultModule contract
• Errors in the GenesisVaultSYToken contract

Consider to remove unused code.

In the GenesisVaultSYToken contract, the vaultId, factory, and asset variables assigned in the constructor are never modified afterward. There are no setter functions or internal assignments that change these values post-deployment. Keeping them as regular public variables causes unnecessary storage reads at runtime, which consume gas, whereas declaring them immutable embeds the values in bytecode at deployment, eliminating storage access costs.

Declare the mentioned fields as immutable.

The initialize function, implemented in ConfigurationModule is external and lacks access control. The first caller permanently sets GlobalStorage.owner, after which further calls revert. If deployment and initialization are not atomic, an attacker can front-run the initialization and set themselves as owner, forcing redeployment.

Initialize the ConfigurationModule in the Genesis Factory’s constructor during deployment to prevent front-running.

The validatePerformanceFee function only rejects rate greater than 10000 BPS, so a 100% performance fee is currently permitted.

This is an overly permissive configuration that poses unexpected risk if set accidentally.

Enforce a sensible hard cap below 100% (e.g. 2000–3000 bps), and consider applying a time lock for fee increases.

The vault assumes standard ERC-20 behaviour - exact amount is received on transfer, balances change only by that amount and math can safely rely on the requested value. If the underlying token deviates from these assumptions, internal accounting can drift and downstream calls may operate on incorrect balances. Examples include fee-on-transfer or deflationary tokens that deliver fewer tokens than requested, rebasing tokens whose balances change asynchronously or ERC-777 hooks that may introduce the reentrancy risk.

Because the implementation mints shares from the requested amount instead of the observed balance delta, it can over-mint shares and attempt to deposit more into the strategy than it actually holds, breaking internal accounting.

Accept only standard ERC-20 tokens or update the vault logic to correctly handle non-standard behaviours. Note that even commonly used tokens such as USDT or USDC could enable transfer fees in the future.

The switchStrategy flow calls the unsetStrategy function to unset the old strategy. However this function also resets the performanceFeeRate and the feeRecipient.

This may lead to lost revenue for the protocol.

Consider to not reset the performance fee configs or add a boolean if this is wished or not.

There is a withdrawal delay based on when a user was the recipient of a deposit the last time. However an arbitrary address can be set as recipient of a deposit.

This allows users to DoS other users from withdrawing their funds by depositing a small amount of funds and set the victim as recipient.

As long as the minDeposit value is high enough the chance that this happens is unlikely. But be aware and consider to make changes if necessary.

The withdraw delay can be avoided by transferring the tokens to another EOA and withdrawing with this account instead as the check is performed for the last deposit of the given msg.sender.

Here are two possible solutions:

• Make withdrawals a 2 step process with a delay in between
• Increase the lastDepositTime of the recipient during a transfer and add a minTransfer amount to mitigate against another griefing DoS vector

The emergencyWithdraw function in the AAVEStrategy contract is unreachable as it has a onlyFactory modifier. But there is no contract in the factory to call it.

Consider adding a function to the factor to be able to call it.

The migration specification requires a 3-day waiting period before execution, but the implementation sets MIN_MIGRATION_DELAY = 1 days and applies migrationDelay == 0 ? MIN_MIGRATION_DELAY : migrationDelay. Additionally, GlobalStorage comments state the default is 3 days. This inconsistency can allow migrations to execute earlier than stakeholders expect, reducing the intended review/exit window.

Align code and documentation by setting MIN_MIGRATION_DELAY to 3 days.

The claimStrategyIncentives function will pass and emit an StrategyIncentivesClaimed event even if no tokens were claimed.

Consider to revert in that case.

There is unused code in multiple parts of the system:

• IERC165 import in the ConfigurationModule
• validateVaultInteraction function in the GlobalStorage library
• InsufficientBalance error in the AAVEStrategy contract
• safeTransfer in the catch flow of the AAVEStrategy deposit function is redundant as the state will reset anyway on a revert

Consider to remove redundant code.

AAVEStrategy contract uses try/catch around Aave calls in deposit, withdraw, and withdrawAll functions, where failures should revert the whole transaction. This masks root causes by replacing Aave’s revert data with generic errors and adds bytecode size and complexity with no functional benefit.

Consider removing try/catch pattern from deposit, withdraw and withdrawAll functions.

In AAVEStrategy.deposit, after pulling tokens from the factory, the catch block calls safeTransfer and then reverts. Since the revert rolls back all state changes in this transaction, the transfer is redundant and adds an unnecessary external call.

Remove the safeTransfer in the catch block and revert directly.