Omniscia Sova Network Audit

ManagedWithdrawRWA Code Style Findings

ManagedWithdrawRWA Code Style Findings

MWR-01C: Inefficient Hook Reads

Description:

The ManagedWithdrawRWA::batchRedeemShares function will repetitively read the opHooks from storage for each redemption being processed.

Example:

src/token/ManagedWithdrawRWA.sol
105// Cache the OP_WITHDRAW hooks once to save gas / avoid stack depth issues
106HookInfo[] storage opHooks = operationHooks[OP_WITHDRAW];
107

108// Execute each individual withdrawal (includes hooks)
109for (uint256 i; i < len; ++i) {
110    uint256 userShares = shares[i];
111    uint256 userAssets = assets[i];
112    address recipient = to[i];
113    address shareOwner = owner[i];
114

115    // Call hooks (same logic as in _withdraw)
116    for (uint256 j; j < opHooks.length; ++j) {
117        IHook.HookOutput memory hookOut =
118            opHooks[j].hook.onBeforeWithdraw(address(this), strategy, userAssets, recipient, shareOwner);
119        if (!hookOut.approved) revert HookCheckFailed(hookOut.reason);
120        opHooks[j].hasProcessedOperations = true;
121    }
122

123    // Accounting and transfers (mirrors _withdraw logic sans nonReentrant)
124    if (strategy != shareOwner) _spendAllowance(shareOwner, strategy, userShares);
125    _beforeWithdraw(userAssets, userShares);
126    _burn(shareOwner, userShares);
127

128    SafeTransferLib.safeTransfer(asset(), recipient, userAssets);
129    emit Withdraw(strategy, recipient, shareOwner, userAssets, userShares);
130}

Recommendation:

We advise all hooks to be loaded into memory once, ensuring that they can be re-used as needed per redemption efficiently.

Alleviation (e4d6885477438291b0d3ca477fab7e088522967b):

The code was updated to load all hooks into memory efficiently, addressing this exhibit.

MWR-02C: Potentially Incorrect Allowance Requirement

Description:

The ManagedWithdrawRWA contract will require an approval by any member that wishes to perform a withdrawal toward the strategy which we consider non-standard and an inefficient approach as we can assume the strategy has performed adequate validation to ensure its caller is authorized by the member the withdrawal is being performed from.

Example:

src/token/ManagedWithdrawRWA.sol
67/**
68 * @notice Process a batch of user-requested withdrawals with minimum assets check
69 * @param shares The amount of shares to redeem
70 * @param to The address to send the assets to
71 * @param owner The owner of the shares
72 * @param minAssets The minimum amount of assets for each withdrawal
73 * @return assets The amount of assets received
74 */
75function batchRedeemShares(
76    uint256[] calldata shares,
77    address[] calldata to,
78    address[] calldata owner,
79    uint256[] calldata minAssets
80) external onlyStrategy nonReentrant returns (uint256[] memory assets) {
81    // Validate array lengths
82    uint256 len = shares.length;
83    if (len != to.length || len != owner.length || len != minAssets.length) {
84        revert InvalidArrayLengths();
85    }
86

87    // Prepare memory array and accumulate total assets required
88    assets = new uint256[](len);
89    uint256 totalAssets;
90

91    for (uint256 i; i < len; ++i) {
92        // Validate share amount for each owner
93        if (shares[i] > maxRedeem(owner[i])) revert RedeemMoreThanMax();
94

95        uint256 amt = previewRedeem(shares[i]);
96        if (amt < minAssets[i]) revert InsufficientOutputAssets();
97

98        assets[i] = amt;
99        totalAssets += amt;
100    }
101

102    // Pull all assets from the strategy in a single transfer
103    _collect(totalAssets);
104

105    // Cache the OP_WITHDRAW hooks once to save gas / avoid stack depth issues
106    HookInfo[] storage opHooks = operationHooks[OP_WITHDRAW];
107

108    // Execute each individual withdrawal (includes hooks)
109    for (uint256 i; i < len; ++i) {
110        uint256 userShares = shares[i];
111        uint256 userAssets = assets[i];
112        address recipient = to[i];
113        address shareOwner = owner[i];
114

115        // Call hooks (same logic as in _withdraw)
116        for (uint256 j; j < opHooks.length; ++j) {
117            IHook.HookOutput memory hookOut =
118                opHooks[j].hook.onBeforeWithdraw(address(this), strategy, userAssets, recipient, shareOwner);
119            if (!hookOut.approved) revert HookCheckFailed(hookOut.reason);
120            opHooks[j].hasProcessedOperations = true;
121        }
122

123        // Accounting and transfers (mirrors _withdraw logic sans nonReentrant)
124        if (strategy != shareOwner) _spendAllowance(shareOwner, strategy, userShares);
125        _beforeWithdraw(userAssets, userShares);
126        _burn(shareOwner, userShares);
127

128        SafeTransferLib.safeTransfer(asset(), recipient, userAssets);
129        emit Withdraw(strategy, recipient, shareOwner, userAssets, userShares);
130    }
131}

Recommendation:

We advise the withdrawal requirement to be omitted, optimizing the code's gas cost as well as user experience.

Alleviation (e4d6885477438291b0d3ca477fab7e088522967b):

The allowance requirement has been omitted as advised, optimizing the code's gas cost.

MWR-03C: Redundant Withdrawal Override

Description:

The ManagedWithdrawRWA::_withdraw function override is redundant as it executes the same statements as its parent tRWA::_withdraw implementation sans the tRWA::_collect call which the ManagedWithdrawRWA replicates manually.

Example:

src/token/ManagedWithdrawRWA.sol
146/**
147 * @notice Redeem shares from the strategy - must be called by the manager
148 * @param shares The amount of shares to redeem
149 * @param to The address to send the assets to
150 * @param owner The owner of the shares
151 * @return assets The amount of assets received
152 */
153function redeem(uint256 shares, address to, address owner) public override onlyStrategy returns (uint256 assets) {
154    if (shares > maxRedeem(owner)) revert RedeemMoreThanMax();
155    assets = previewRedeem(shares);
156

157    // Collect assets from strategy
158    _collect(assets);
159

160    // User must token-approve strategy for withdrawal
161    _withdraw(strategy, to, owner, assets, shares);
162}
163

164/**
165 * @notice Override _withdraw to skip transferAssets since we already collected
166 * @param by Address initiating the withdrawal
167 * @param to Address receiving the assets
168 * @param owner Address that owns the shares
169 * @param assets Amount of assets to withdraw
170 * @param shares Amount of shares to burn
171 */
172function _withdraw(address by, address to, address owner, uint256 assets, uint256 shares)
173    internal
174    override
175    nonReentrant
176{
177    if (by != owner) _spendAllowance(owner, by, shares);
178    _beforeWithdraw(assets, shares);
179    _burn(owner, shares);
180

181    // Call hooks after state changes but before final transfer
182    HookInfo[] storage opHooks = operationHooks[OP_WITHDRAW];
183    for (uint256 i = 0; i < opHooks.length;) {
184        IHook.HookOutput memory hookOutput = opHooks[i].hook.onBeforeWithdraw(address(this), by, assets, to, owner);
185        if (!hookOutput.approved) {
186            revert HookCheckFailed(hookOutput.reason);
187        }
188        // Mark hook as having processed operations
189        opHooks[i].hasProcessedOperations = true;
190

191        unchecked {
192            ++i;
193        }
194    }
195

196    // Transfer the assets to the recipient
197    SafeTransferLib.safeTransfer(asset(), to, assets);
198

199    emit Withdraw(by, to, owner, assets, shares);
200}

Recommendation:

We advise the code to omit the manual tRWA::_collect calls and to remove the ManagedWithdrawRWA::_withdraw function implementation, optimizing the code's gas cost.

Alleviation (e4d6885477438291b0d3ca477fab7e088522967b):

The code was revised as advised, minimizing code redundancy.

Prev

LibRoleManaged.sol (LRM-C)

Next

ManagedWithdrawRWAStrategy.sol (MWW-C)

On this page