Summary
The current way of connecting markets to vaults is that the owner calls connectVaultsAndMarkets, the function recalculates the vaults' creadit capacities and then connects every vault with every market and vice versa.
function connectVaultsAndMarkets(uint256[] calldata marketIds, uint256[] calldata vaultIds) external onlyOwner {
...
for (uint128 i; i < vaultIds.length; i++) {
if (Vault.load(vaultIds[i].toUint128()).connectedMarkets.length > 0) {
vaultIdToRecalculate[0] = vaultIds[i];
Vault.recalculateVaultsCreditCapacity(vaultIdToRecalculate);
}
}
for (uint256 i; i < marketIds.length; i++) {
_configureMarketConnectedVaults(marketIds[i].toUint128(), vaultIds);
}
for (uint256 i; i < vaultIds.length; i++) {
_configureVaultConnectedMarkets(vaultIds[i].toUint128(), marketIds);
}
}
The issue is that this approach is highly inefficient in terms of gas usage. The `recalculateVaultsCreditCapacity` function has complex functionality and is iterated many times. It is used not only in `connectVaultsAndMarkets` but also in `_configureMarketConnectedVaults`. The number of vaults and markets provided as input can reach a level that could hit the gas limit, resulting in an Out Of Gas (OOG) error.
Impact
Impossible to execute certain transactions due to OOG. High gas costs.
Tools Used
Manual
Recommendations
Add restrictions on vault and market counts, and remove unnecessary code. The first loop is not useful because recalculateVaultsCreditCapacity already performs the same check. Iterating through each vaultId duplicates gas usage.
function connectVaultsAndMarkets(uint256[] calldata marketIds, uint256[] calldata vaultIds) external onlyOwner {
// revert if no marketIds are provided
if (marketIds.length == 0) {
revert Errors.ZeroInput("connectedMarketIds");
}
// revert if no vaultIds are provided
if (vaultIds.length == 0) {
revert Errors.ZeroInput("connectedVaultIds");
}
- // define array that will contain a single vault id to recalculate credit for
- uint256[] memory vaultIdToRecalculate = new uint256[](1);
- // iterate over vault ids
- for (uint128 i; i < vaultIds.length; i++) {
- // if vault has connected markets
- if (Vault.load(vaultIds[i].toUint128()).connectedMarkets.length > 0) {
- // cache vault id //
- vaultIdToRecalculate[0] = vaultIds[i];
- // recalculate the credit capacity of the vault
- Vault.recalculateVaultsCreditCapacity(vaultIdToRecalculate);
- }
- }
+ // recalculate the credit capacity of the vault
+ Vault.recalculateVaultsCreditCapacity(vaultIds);
for (uint256 i; i < marketIds.length; i++) {
_configureMarketConnectedVaults(marketIds[i].toUint128(), vaultIds);
}
// perform state updates for markets connected to each market id
for (uint256 i; i < vaultIds.length; i++) {
_configureVaultConnectedMarkets(vaultIds[i].toUint128(), marketIds);
}
}
This line market.connectedVaults.push();creates new empty array that ignores the previously used vaults.
Delete the second if statement in _configureMarketConnectedVaults as these vaults won't be used anymore so it makes no sense checking their credit capacity here.
function _configureMarketConnectedVaults(uint128 marketId, uint256[] calldata vaultIds) internal {
...
- // if market has connected vaults
- if (market.connectedVaults.length > 0) {
- // Make sure that vaults connected to markets that are updated but not passed as an argument
- uint256[] memory vaultIdToRecalculate = market.connectedVaults[market.connectedVaults.length - 1].values();
- // recalculate the credit capacity for current market vaults
- Vault.recalculateVaultsCreditCapacity(vaultIdToRecalculate);
- }
// push new array of connected markets
market.connectedVaults.push();
// add markets ids to connected markets
for (uint256 i; i < vaultIds.length; i++) {
// use [vault.connectedVaults.length - 1] to get the last connected markets array
market.connectedVaults[market.connectedVaults.length - 1].add(vaultIds[i]);
}
// emit event LogConfigureMarketConnectedVaults
emit LogConfigureMarketConnectedVaults(marketId, vaultIds);
}
