Omniscia 0xPhase Audit

BalancerAccountingFacet Manual Review Findings

BalancerAccountingFacet Manual Review Findings

BAF-01M: Inefficient Deposit Flow

TypeSeverityLocation
Logical FaultBalancerAccountingFacet.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:

balancer/diamond/BalancerAccountingFacet.sol
17/// @inheritdoc IBalancerAccounting
18/// @custom:protected onlyRole(BalancerConstants.VAULT_ROLE)
19function deposit(
20 IERC20 asset,
21 uint256 user,
22 uint256 amount
23) 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 - amount
32 );
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.