Vault Wrapper

In the registerVault function, the vaultABI parameter is provided to represent the target function that is callable on the vault.

However this vaultABI parameter is only emitted in the VaultRegistered event and is not used in the logic or storage resulting from the registerVault function.

When a vault is configured to be used with the VaultWrapper system any function on it may be invoked through the VaultWrapper, even those that are not intending to be called through the expected execution flow.

Given that the VaultWrapper is intended to work with arbitrary vault implementations, it would be prudent to limit the callable function signatures to the expected set for a vault.

It may be expected that the vaultABI string is only emitted in the VaultRegistered event. However it would be prudent to consider storing a set of whitelisted bytes4 function selectors for each vault to explicitly define the expected interaction with a supported vault.

The VaultDepositManager assigns a FLAG_ALLOWS_NATIVE flag for the entire vault, which may have multiple functions that are immediately supported when a vault is registered.

However not all of these functions that are now callable on a vault may support native payment. For example, often vaults have a deposit and a depositNative type function, where the deposit function would not be compatible with native Ether.

Be aware of this generalization that the configuration makes. The native support could be indicated for each supported selector for a vault, however this may introduce more complexity than such a validation is worth.

The expected flow for any vault functions that accept native payment is to include a direct Ether transfer to the VaultWrapper contract in the multicall before the relevant deposit action.

Relay protocol allows calls to be configured with a allowFailure value, where when true, if the call fails as a part of the multicall, it continues with execution of the batch.

However in the case of the VaultWrapper’s native use-case this could result in native funds being trapped if allowFailure is used for the invocation of the VaultWrapper.deposit function which then fails for any reason at the Fun wrapper level or at the underlying vault level.

In the offchain UI configuration of these actions do not allow allowFailure to be configured as true. Additionally, consider documenting this risk for integrators.

The mint function of the ERC4626 standard will not always transfer the entire input amount from the depositor, but instead the amount that is necessary to mint the specified shares.

The VaultWrapper system should be considered incompatible with any mint or mint-like functions which do not take an explicit deposit input amount since they will leave input tokens in the VaultWrapper contract which are susceptible to being stolen by the next depositor.

Be aware and clearly document that any mint function or function that does not accept and use an explicit input token amount is strictly incompatible with the VaultWrapper system.

The replacer always assumes that values are left padded to fit a 32 byte increment and that their value will be interpreted using the whole 32 byte word (as is the case with static types such as address uint etc…).

However this is often not the case with dynamic types which will not be left padded and are instead interpreted using their length entry.

For example, consider the following function calldata for a function “deposit(bytes amount)”, where the amount is encoded in a bytes object and those bytes are expected to be a uint64 (less than a full word).

bytes memory amount = hex"FF"; // Assume amount is represented by the 1 byte 0xFF for demonstration
├ Hex (Memory):
├─ Length ([0x00:0x20]): 0x0000000000000000000000000000000000000000000000000000000000000001
├─ Contents ([0x20:..]): 0xff00000000000000000000000000000000000000000000000000000000000000

➜ abi.encodeWithSignature("deposit(bytes)", amount)
Type: dynamic bytes
├ Hex (Memory):
├─ Length ([0x00:0x20]): 0x0000000000000000000000000000000000000000000000000000000000000064
├─ Contents ([0x20:..]): 0x98b1e06a00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001ff0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

The calldata is: 0x98b1e06a00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001ff00000000000000000000000000000000000000000000000000000000000000

As you can see the ff value is left aligned rather than right aligned. If this amount value is replaced with the placeholder, the replace function will replace the bytes assuming that the value is right aligned and uses the entire word. Which will provide an invalid amount of 0 given the length value will not reach a right aligned value.

Be aware that the replacer is not compatible with any dynamic type encodings. These should be rare in any vault implementation and therefore are not worth supporting given the complexity. The VaultWrapper should only be used with vaults that use a static type encoding for fields that ought to be replaced.

Upon production deployment of the VaultWrapper, the owner is transferred twice in the same deployment transaction when the initial owner is a multisig and will not be the deployer.

This is to support the logic where if the initialOwner address is zero then the owner can default to the msg.sender. However a more efficient way to achieve this is to simply use a ternary case in the constructor invocation of the Ownable constructor.

In the constructor of the VaultDepositManager contract, consider removing the initialOwner != address(0) case and instead passing the initial owner address as such to the Ownable constructor:

constructor(address initialOwner) Ownable(initialOwner == address(0) ? msg.sender : initialOwner)

This way avoiding duplicating _transferOwnership calls.

In the VaultDepositManager contract the only function which exposes configuration for the vaultConfigs mapping is the registerVault function.

However this function can only be called once, when the vault is initially being configured and not again until the vault has been removed.

It may however be useful to update vault configuration if for example later it is desired to support native functions with a vault.

Be aware that a vault will have to be removed to be reconfigured. Consider adding an update configuration function in case it is needed.

The VaultRegistered event includes all of the details around the configuration that was made for the corresponding vault except the allowsNative value that was configured.

Consider including the allowsNative value in the VaultRegistered event that is emitted.

For native value deposits the VaultWrapper system expects that the Relay router has transferred the native funds to the VaultWrapper contract ahead of the deposit invocation.

The only compatible way to do this for e.g. a swap is if the swap uses the VaultWrapper as the output address. Otherwise if the relay router receives the output tokens, like in the example transaction provided where USDC is swapped for USDT, then there is no way to be sure you can transfer all Ether to the VaultWrapper contract since the ether amount would have to be encoded in one of the call objects ahead of time.

For some esoteric systems, it may not be possible to specify a receiver and instead the native output may be forced to go to the Relay router address thus breaking the use-case of the fun VaultWrapper contract.

Simply be aware of this limitation of support for Native deposits and consider it when evaluating which swap venues to use.

When a vault is configured with the FLAG_CONVERT_TO_18_DECIMALS flag, the tokens are converted to 18 decimals for the deposit calldata. However only tokens that have less than 18 decimals are converted up to 18.

Tokens with larger than 18 decimals, such as YAM v2 which have 24 decimals will not be converted to 18 decimals. For any vault that requires 18 decimal representations this will break the integration.

Consider explicitly disallowing tokens that have larger than 18 decimals in the FLAG_CONVERT_TO_18_DECIMALS handling.

In the registerVault function there is no validation that the vault contract houses bytecode. As a result, when a deposit is executed, the .call will return true even though no deposit was made. And the transaction will silently complete while leaving user tokens in the VaultWrapper contract.

Consider validating that the byte code of the vault address is nonzero when registering to avoid any mistakes that can cause loss of funds.

The contract compares minAmountOut to IERC20(token).balanceOf(msg.sender) before transfer. For fee-on-transfer or negatively rebasing tokens, the wrapper will receive fewer tokens than the sender’s reported balance, but the deposit still proceeds, undermining slippage protection. The vault may then deposit a smaller net amount than the user intended.

Be aware that fee on transfer tokens are not supported with the VaultWrapper.