Yield-limit retry cache stores consumed shares as pending shares

[chenshj73]

Summary

Controller_YieldLimitExecHook appears to invert the share accounting for partially pending destination mints. When pendingUnderlying > 0, the code calculates consumedShares using pendingUnderlying, then stores pendingShares = totalShares - consumedShares as the amount to retry later. This records the consumed portion as pending and emits the pending portion as consumed.

Reviewed commit: 24551fb53934

Why this matters

This is a cross-chain lifecycle bug in the partial-fill/retry path. If the destination-side receive is only partially allowed by limits, the hook should:

release/mint the consumed portion now
cache the pending portion for retry

The current code can cache the wrong amount. A later retry(...) uses the cached value as totalPendingShares, so users can receive too little or the pending cache can be cleared against the wrong quantity.

Affected code

contracts/hooks/Controller_YieldLimitExecHook.sol

dstPreHookCall(...) computes the consumed and pending underlying amounts and mints the full share amount to the hook contract when some underlying is pending.

124         (uint256 consumedUnderlying, uint256 pendingUnderlying) = _limitDstHook(
125             params_.connector,
126             params_.transferInfo.amount
127         );
128         uint256 sharesToMint = yieldToken__.calculateMintAmount(
129             params_.transferInfo.amount
130         );
131 
132         postHookData = abi.encode(
133             consumedUnderlying,
134             pendingUnderlying,
135             params_.transferInfo.amount,
136             params_.transferInfo.receiver
137         );
138 
139         transferInfo = params_.transferInfo;
140         if (pendingUnderlying != 0) transferInfo.receiver = address(this);
141         transferInfo.amount = sharesToMint;
142         transferInfo.extraData = payload;

So by the time dstPostHookCall(...) runs, params_.transferInfo.amount is the total minted shares for depositUnderlying, while consumedUnderlying and pendingUnderlying describe how much of the original underlying amount should be released now versus cached for retry.

However, dstPostHookCall(...) uses pendingUnderlying to compute consumedShares.

158         (
159             uint256 consumedUnderlying,
160             uint256 pendingUnderlying,
161             uint256 depositUnderlying,
162             address receiver
163         ) = abi.decode(
164                 params_.postHookData,
165                 (uint256, uint256, uint256, address)
166             );
167         bytes memory execPayload = params_.transferInfo.extraData;
168 
169         uint256 connectorPendingShares = _getConnectorPendingAmount(
170             params_.connectorCache
171         );
172 
173         uint256 pendingShares;
174         if (pendingUnderlying > 0) {
175             // totalShares * consumedU / totalU
176             uint256 consumedShares = (params_.transferInfo.amount *
177                 pendingUnderlying) / depositUnderlying;
178 
179             pendingShares = params_.transferInfo.amount - consumedShares;
180 
181             cacheData.identifierCache = abi.encode(
182                 params_.transferInfo.receiver,
183                 pendingShares,
184                 params_.connector,
185                 execPayload
186             );
187             yieldToken__.transfer(receiver, consumedUnderlying);
188 
189             emit TokensPending(
190                 params_.connector,
191                 params_.transferInfo.receiver,
192                 consumedShares,
193                 pendingShares,
194                 params_.messageId
195             );

The comment says totalShares * consumedU / totalU, but line 177 uses pendingUnderlying. Then line 179 subtracts that value and stores the remainder as pendingShares.

With a 1:1 share price:

depositUnderlying = 100
consumedUnderlying = 40
pendingUnderlying = 60
params_.transferInfo.amount = 100 shares

current code:
  consumedShares = 100 * 60 / 100 = 60
  pendingShares = 100 - 60 = 40
  cache stores 40 shares as pending

expected:
  consumedShares = 100 * 40 / 100 = 40
  pendingShares = 60
  cache stores 60 shares as pending

Retry consumes the cached value as the pending balance

The cached value is not just an event/display problem. preRetryHook(...) decodes it as totalPendingShares and uses it as the amount subject to the next receive limit.

246         (
247             address receiver,
248             uint256 totalPendingShares,
249             address connector,
250 
251         ) = abi.decode(
252                 params_.cacheData.identifierCache,
253                 (address, uint256, address, bytes)
254             );
255 
256         if (connector != params_.connector) revert InvalidConnector();
257 
258         (uint256 consumedShares, uint256 pendingShares) = _limitDstHook(
259             params_.connector,
260             totalPendingShares
261         );
262 
263         postHookData = abi.encode(receiver, consumedShares, pendingShares);
264         uint256 consumedUnderlying = yieldToken__.convertToAssets(
265             consumedShares
266         );
267         yieldToken__.transfer(receiver, consumedUnderlying);

So if the first receive cached 40 shares instead of the expected 60, retry can only release/check limits for those 40 shares. The remaining 20 shares from the example above are no longer represented by identifierCache.

The bridge retry path depends on these hook values

Controller.retry(...) calls the hook, mints/transfers the returned TransferInfo, and then writes the hook's updated cache.

93     function retry(
94         address connector_,
95         bytes32 messageId_
96     ) external nonReentrant {
97         (
98             bytes memory postHookData,
99             TransferInfo memory transferInfo
100         ) = _beforeRetry(connector_, messageId_);
101         _mint(transferInfo.receiver, transferInfo.amount);
102         totalMinted += transferInfo.amount;
103 
104         _afterRetry(connector_, messageId_, postHookData);
105     }

and the base hook cache update writes whatever postRetryHook(...) returns:

301     function _afterRetry(
302         address connector_,
303         bytes32 messageId_,
304         bytes memory postHookData
305     ) internal {
306         CacheData memory cacheData = CacheData(
307             identifierCache[messageId_],
308             connectorCache[connector_]
309         );
310 
311         (cacheData) = hook__.postRetryHook(
312             PostRetryHookCallParams(
313                 connector_,
314                 messageId_,
315                 postHookData,
316                 cacheData
317             )
318         );
319         identifierCache[messageId_] = cacheData.identifierCache;
320         connectorCache[connector_] = cacheData.connectorCache;
321     }

That makes the initial inverted pendingShares calculation persistent across the retry lifecycle.

Suggested fix

Compute pending shares from pendingUnderlying, or consumed shares from consumedUnderlying, but keep the variable names and cache semantics aligned. For example:

uint256 pendingShares =
    (params_.transferInfo.amount * pendingUnderlying) / depositUnderlying;
uint256 consumedShares = params_.transferInfo.amount - pendingShares;

Depending on the intended rounding direction, it may be safer to round pendingShares up so the retry cache does not understate the user's remaining claim.

It would also be worth adding a test where pendingUnderlying != 0 and consumedUnderlying != pendingUnderlying, then asserting that identifierCache stores the share amount corresponding to the pending underlying amount, not the consumed amount.