Omniscia 0xPhase Audit
BalancerAccountingFacet Manual Review Findings
BalancerAccountingFacet Manual Review Findings
BAF-01M: Inefficient Deposit Flow
Type | Severity | Location |
---|---|---|
Logical Fault | BalancerAccountingFacet.sol:L76-L96 |
Description:
The BalancerAccountingFacet::deposit
flow is highly inefficient as it iterates all offsets twice redundantly and will distribute the full deposit to a single active yield source at the end regardless of how it is performing.
Impact:
The current distribution mechanism is inefficient in both gas cost and capital efficiency.
Example:
17/// @inheritdoc IBalancerAccounting18/// @custom:protected onlyRole(BalancerConstants.VAULT_ROLE)19function deposit(20 IERC20 asset,21 uint256 user,22 uint256 amount23) external override onlyRole(BalancerConstants.VAULT_ROLE) {24 BalancerStorage storage s = _s();25 Asset storage ast = s.asset[asset];26 uint256 total = _getters().totalBalance(asset);27
28 uint256 shares = ShareLib.calculateShares(29 amount,30 ast.totalShares,31 total - amount32 );33
34 ast.shares[user] += shares;35 ast.totalShares += shares;36
37 emit Deposit(asset, user, amount, shares);38
39 (Offset[] memory arr, , ) = _calculations().offsets(asset);40
41 if (arr.length == 0) {42 return;43 }44
45 uint256 acc = asset.balanceOf(address(this));46
47 for (uint256 i = 0; i < arr.length; ) {48 if (acc == 0) return;49
50 Offset memory offset = arr[i];51 Yield storage yield = s.yield[offset.yieldSrc];52
53 if (offset.isPositive || !yield.state) {54 unchecked {55 i++;56 }57
58 continue;59 }60
61 _updateAPR(offset.yieldSrc);62
63 uint256 yieldAmount = MathLib.min(offset.offset, acc);64
65 asset.safeTransfer(address(offset.yieldSrc), yieldAmount);66 offset.yieldSrc.deposit(yieldAmount);67
68 yield.lastDeposit = offset.yieldSrc.totalBalance();69
70 unchecked {71 acc -= yieldAmount;72 i++;73 }74 }75
76 for (uint256 i = 0; i < arr.length; ) {77 Offset memory offset = arr[i];78 Yield storage yield = s.yield[offset.yieldSrc];79
80 if (!yield.state) {81 unchecked {82 i++;83 }84
85 continue;86 }87
88 _updateAPR(offset.yieldSrc);89
90 asset.safeTransfer(address(offset.yieldSrc), acc);91 offset.yieldSrc.deposit(acc);92
93 yield.lastDeposit = offset.yieldSrc.totalBalance();94
95 break;96 }97}
Recommendation:
We advise the code of BalancerAccountingFacet::deposit
to iterate through the yield sources with a deficit once. To achieve this, the first for
loop should be retained. When performing a deposit, instead of depositing the minimum between the desirable offset
and the available balance (acc
), the code should deposit the input amount
proportionately to the yield sources at a deficit.
The BalancerCalculationsFacet::offsets
function already yields a totalNegative
value that should be utilized in the code as the full percentage of all negative offsets. This enables a deposit of amount * offset / totalOffsets
to be performed (i.e. amount * offset.offset / totalNegative
), ensuring that the deposited amount is proportionately distributed to the underperforming yield sources.
Alleviation:
The deposit flow of the balancer has been optimized as advised, distributing the deposit capital to the negative offset strategies and thus maximizing the contract's capital efficiency.