Skip to content

Commit 4979873

Browse files
committed
fix: measure bridge value on the whole book and scope window authority to the payload
The value-conservation gate read the position manager's totalAssets, which drops any module whose debt exceeds its collateral, so it read flat while bridge capital was poured into an underwater module. It now measures quoted collateral minus debt summed over every module (PositionValueIncreased). Window step authority moved from the whole flash-loan call into the callback around the committed payload, so a module holds no authority in its own frame and cannot source the delivery check's balance growth from the position manager itself.
1 parent e412c0f commit 4979873

7 files changed

Lines changed: 183 additions & 55 deletions

File tree

‎src/libs/manager/rebalancer/LibRetargetterConstants.sol‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,13 @@ bytes32 constant OPERATION_STORAGE_SLOT = 0x79211724a6a2d25fad538c732b7f9fee62ff
116116
/* TRANSIENT STORAGE */
117117
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
118118

119-
/// @dev Transient slot for the flash-loan window: holds the module driving the open window,
120-
/// the only address besides the owner and rebalancer role holders that passes the step
121-
/// authorization (zero when no window is open). Distinct from MODULE_TSLOT, which
122-
/// authenticates the callback and is zeroed on callback entry to make it single-shot,
123-
/// while this slot keeps the window's authority until the window closes.
119+
/// @dev Transient slot for the executing payload: holds the module whose committed step
120+
/// calls are running, the only address besides the owner and rebalancer role holders
121+
/// that passes the step authorization (zero otherwise). Set and cleared around the
122+
/// payload inside the callback, so the module holds no authority in its own frame and
123+
/// cannot act on the Retargetter before the delivery check or after the steps end.
124+
/// Distinct from MODULE_TSLOT, which authenticates the callback and is zeroed on
125+
/// callback entry to make it single-shot.
124126
/// Computed as: bytes32(uint256(keccak256("retargetter.transient.window")) - 1)
125127
bytes32 constant WINDOW_TSLOT = 0xd6afb5b9c2dc000141f8a98b95795a2e10b482782ed7428b0e955204ff9c2c83;
126128

‎src/libs/manager/rebalancer/LibRetargetterErrors.sol‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,14 @@ library LibRetargetterErrors {
7272
/// @param position The offending borrow module
7373
error PositionAboveTarget(address position);
7474

75-
/// @notice Thrown when a rebalance grows the position manager's totalAssets while the
75+
/// @notice Thrown when a rebalance grows the position manager's net value while the
7676
/// operation's Request is unrepaid and short of its deadline (the growth would be
7777
/// bridge capital left in the position manager with no shares backing it, exposed
7878
/// to capture by any LP exit before the Request is repaid).
79-
/// @param totalAssetsBefore The totalAssets snapshot before the rebalance
80-
/// @param totalAssetsAfter The totalAssets value after the rebalance
81-
error TotalAssetsIncreased(uint256 totalAssetsBefore, uint256 totalAssetsAfter);
79+
/// @param valueBefore The net value snapshot before the rebalance (quoted collateral minus
80+
/// debt, summed over every module including underwater ones, so it may be negative)
81+
/// @param valueAfter The net value after the rebalance, under the same convention
82+
error PositionValueIncreased(int256 valueBefore, int256 valueAfter);
8283

8384
/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
8485
/* ORDERS */

‎src/manager/rebalancer/Retargetter.sol‎

Lines changed: 58 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,10 @@ import {FixedPointMathLib} from "lib/solady/src/utils/FixedPointMathLib.sol";
6262
///
6363
/// Trust model: fully trusted owner, semi-trusted rebalancer boxed in by the guardrails
6464
/// (direction checks, the bridge value-conservation gate, principal and yield caps,
65-
/// whitelists, residual settlement gates), untrusted everyone else. The Retargetter holds no value at rest: `resolve` and the end
66-
/// of the flash-loan window both require its balances of the two bound assets to stay
67-
/// within the configured residual tolerance (exact zero by default).
65+
/// whitelists, residual settlement gates), untrusted everyone else. The Retargetter
66+
/// holds no value at rest: `resolve` and the end of the flash-loan window both require
67+
/// its balances of the two bound assets to stay within the configured residual
68+
/// tolerance (exact zero by default).
6869
///
6970
/// Derive, do not store: everything readable from the composed contracts (direction,
7071
/// consumed principal, order liveness progress, repaid status) is recomputed fresh at
@@ -95,13 +96,14 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
9596
/* MODIFIERS */
9697
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
9798

98-
/// @dev Passes when the caller is the module driving an open flash-loan window (the
99+
/// @dev Passes when the caller is the module whose committed payload is executing (the
99100
/// payload steps are delegatecalled from the callback, so `msg.sender` there is the
100101
/// module), when the caller is the owner, or when the caller holds the rebalancer
101-
/// role. Outside a window the slot reads the zero address, which never matches a
102-
/// caller, and a third party gaining execution control inside a window stays an
103-
/// ordinary unauthorized caller. The body lives in a function so the modifier costs
104-
/// a jump at each of its many use sites instead of an inlined copy (contract size).
102+
/// role. Outside the payload the slot reads the zero address, which never matches a
103+
/// caller, so the module has no authority in its own frame either side of the
104+
/// callback, and a third party gaining execution control mid-window stays an ordinary
105+
/// unauthorized caller. The body lives in a function so the modifier costs a jump at
106+
/// each of its many use sites instead of an inlined copy (contract size).
105107
modifier onlyOwnerOrRebalancer() {
106108
_checkOwnerOrRebalancer();
107109
_;
@@ -437,9 +439,9 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
437439
/// one-atom improvement flooring flattens to an equal value, passes), and no module
438440
/// may end with debt against zero collateral. LTV convention throughout: zero when
439441
/// debt is zero (idle modules and an emptied position pass), the max sentinel for bad
440-
/// debt. While the operation's Request is unrepaid and short of its deadline,
441-
/// totalAssets must not grow across the call, for every caller including the owner;
442-
/// see the value-conservation comment in the body. The position manager's own loss,
442+
/// debt. While the operation's Request is unrepaid and short of its deadline, the
443+
/// position's net value must not grow across the call, for every caller including the
444+
/// owner; see {_bridgeValueSnapshot} and {_checkBridgeValue}. The position manager's own loss,
443445
/// cooldown and safe-LTV checks apply underneath. A sentinel resolving to zero (an
444446
/// empty balance, or a REPAY leg on a debt-free module) produces a zero-amount leg,
445447
/// which the borrow modules reject; the whole call reverts atomically.
@@ -481,7 +483,7 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
481483

482484
// Bridge value conservation: the snapshot is armed only while the operation's Request
483485
// is unrepaid and short of its deadline; see {_bridgeValueSnapshot}/{_checkBridgeValue}
484-
uint256 totalAssetsBefore = _bridgeValueSnapshot(positionManager);
486+
int256 valueBefore = _bridgeValueSnapshot(positionManager);
485487

486488
// Snapshot the goalposts: target, aggregate LTV and per-module LTVs
487489
(uint256 target,) = IPositionManager(positionManager).config();
@@ -500,7 +502,7 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
500502
if (resolved.collateral > 0) collateralAsset.safeApprove(positionManager, 0);
501503
if (resolved.debt > 0) debtAsset.safeApprove(positionManager, 0);
502504

503-
_checkBridgeValue(positionManager, totalAssetsBefore);
505+
_checkBridgeValue(positionManager, valueBefore);
504506

505507
// Direction checks; owner bypass is evaluated on msg.sender only, never through the
506508
// window (inside a window msg.sender is the flash-loan module). The aggregate must
@@ -552,17 +554,17 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
552554
}
553555

554556
// Open the window: the operation storage carries the addresses the steps read (and locks
555-
// out nested starts); the window slot hands step authority to the module alone for the
556-
// whole window, while the module slot authenticates the callback and is zeroed on its
557-
// entry (single-shot), which is why the module address lives in both. The payload digest
558-
// and the pre-loan debt balance bind the callback to this exact payload and to the full
559-
// delivery of the principal, so a whitelisted module can neither substitute its own step
560-
// calls nor collect the repayment approval for funds it never delivered
557+
// out nested starts); the module slot authenticates the callback and is zeroed on its
558+
// entry (single-shot). The payload digest and the pre-loan debt balance bind the callback
559+
// to this exact payload and to the full delivery of the principal, so a whitelisted
560+
// module can neither substitute its own step calls nor collect the repayment approval
561+
// for funds it never delivered. Step authority is not handed out here: the window slot
562+
// is set inside the callback around the payload alone, so the module holds no authority
563+
// in its own frame and cannot source the delivery from the position manager itself
561564
operation_.positionManager = positionManager;
562565
operation_.fund = fund;
563566
address debtAsset = LibStorage.assetsStorage().debtAsset;
564567
bytes memory payload = abi.encode(data);
565-
WINDOW_TSLOT.tStoreAddress(flashLoanModule);
566568
MODULE_TSLOT.tStoreAddress(flashLoanModule);
567569
AMOUNT_TSLOT.tStoreUint(flashLoanAmount);
568570
PAYLOAD_TSLOT.tStoreBytes32(keccak256(payload));
@@ -571,7 +573,6 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
571573
IFlashLoanModule(flashLoanModule).flashLoan(debtAsset, flashLoanAmount, payload);
572574

573575
// The module has pulled its repayment; close the window and scrub any leftover approval
574-
WINDOW_TSLOT.tStoreAddress(address(0));
575576
MODULE_TSLOT.tStoreAddress(address(0));
576577
AMOUNT_TSLOT.tStoreUint(0);
577578
PAYLOAD_TSLOT.tStoreBytes32(bytes32(0));
@@ -622,7 +623,13 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
622623
calls.offset := add(arrayOffset, 0x20)
623624
calls.length := calldataload(arrayOffset)
624625
}
626+
// Step authority lasts exactly as long as the committed payload: the module is
627+
// authorized while its own steps run (they are delegatecalled, so `msg.sender` is the
628+
// module) and holds none of it before or after, which is what keeps the delivery check
629+
// above honest
630+
WINDOW_TSLOT.tStoreAddress(module);
625631
_multicall(calls);
632+
WINDOW_TSLOT.tStoreAddress(address(0));
626633
// The module pulls its repayment through this allowance after the callback returns
627634
debtAsset.safeApproveWithRetry(module, amount);
628635
}
@@ -903,38 +910,53 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
903910
}
904911

905912
/// @dev Arms the rebalance value-conservation gate: returns the position manager's current
906-
/// totalAssets while the operation's Request is unrepaid and short of its deadline,
907-
/// and the max sentinel (gate disarmed) otherwise. Once the Request is repaid, or
913+
/// net value while the operation's Request is unrepaid and short of its deadline, and
914+
/// the max sentinel (gate disarmed) otherwise. Once the Request is repaid, or
908915
/// auto-expired past its deadline with holders redeeming its remaining balance,
909916
/// folding residual value back into the position must reopen, which is why the gate
910917
/// keys on the effective repaid state rather than the operation being active.
911-
function _bridgeValueSnapshot(address positionManager) internal view returns (uint256) {
918+
function _bridgeValueSnapshot(address positionManager) internal view returns (int256) {
912919
RetargetterOperation storage operation_ = LibStorage.operationStorage();
913920
address request = operation_.request;
914921
if (
915922
request == address(0) || block.timestamp >= operation_.repaymentDeadline
916923
|| IRequestInteractions(request).isRepaid()
917924
) {
918-
return type(uint256).max;
925+
return type(int256).max;
919926
}
920-
return IPositionManager(positionManager).totalAssets();
927+
return _positionValue(positionManager);
921928
}
922929

923930
/// @dev Value-conservation gate, applied to every caller (unlike the direction checks,
924931
/// this guards Request lenders rather than the steering): with the gate armed (a
925-
/// non-sentinel snapshot), totalAssets must not have grown across the rebalance.
926-
/// Growth would be Request capital parked in the position with no shares backing it,
927-
/// exposed to capture by any LP exit before the Request is repaid; pairing each input
928-
/// leg with an equivalent output leg in the same call (SUPPLY with BORROW, REPAY with
929-
/// WITHDRAW) is what keeps the bridge whole.
930-
function _checkBridgeValue(address positionManager, uint256 totalAssetsBefore) internal view {
931-
if (totalAssetsBefore == type(uint256).max) return;
932-
uint256 totalAssetsAfter = IPositionManager(positionManager).totalAssets();
933-
if (totalAssetsAfter > totalAssetsBefore) {
934-
revert LibRetargetterErrors.TotalAssetsIncreased(totalAssetsBefore, totalAssetsAfter);
932+
/// non-sentinel snapshot), the position manager's net value must not have grown
933+
/// across the rebalance. Growth would be Request capital parked in the position with
934+
/// no shares backing it, exposed to capture by any LP exit before the Request is
935+
/// repaid; pairing each input leg with an equivalent output leg in the same call
936+
/// (SUPPLY with BORROW, REPAY with WITHDRAW) is what keeps the bridge whole. Equality
937+
/// passes, so an operator sizing the output leg must cover any quoting round-up on
938+
/// the input leg; a leg one atom short of neutral is rejected rather than tolerated.
939+
function _checkBridgeValue(address positionManager, int256 valueBefore) internal view {
940+
if (valueBefore == type(int256).max) return;
941+
int256 valueAfter = _positionValue(positionManager);
942+
if (valueAfter > valueBefore) {
943+
revert LibRetargetterErrors.PositionValueIncreased(valueBefore, valueAfter);
935944
}
936945
}
937946

947+
/// @dev Net value of the whole position: quoted collateral minus debt summed over every
948+
/// module, negative when the book is underwater. Deliberately not the position
949+
/// manager's totalAssets, which drops any module whose debt exceeds its collateral
950+
/// and so reads flat while value is poured into an underwater module; the gate above
951+
/// must see that value, hence the bad-debt-inclusive aggregates. Both operands are
952+
/// sums of token amounts far below the signed range.
953+
function _positionValue(address positionManager) internal view returns (int256) {
954+
uint256 collateralQuoted = IPositionManager(positionManager).collateralAmountQuoted();
955+
uint256 debt = IPositionManager(positionManager).debtAmount();
956+
// forge-lint: disable-next-line(unsafe-typecast)
957+
return int256(collateralQuoted) - int256(debt);
958+
}
959+
938960
/// @dev Aggregate position manager LTV under the snapshot convention.
939961
function _positionManagerLtv(address positionManager) internal view returns (uint256) {
940962
return

‎test/manager/rebalancer/RetargetterAsync.t.sol‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,7 +1103,7 @@ contract RetargetterAsyncTest is RetargetterBaseTest {
11031103
/// been converted to collateral, supplying it without borrowing the repayment back
11041104
/// out reverts on the value-conservation gate, so existing position manager
11051105
/// shareholders can never absorb Request-funded collateral.
1106-
function test_async_supplyWithoutBorrow_revertsTotalAssetsIncreased() public {
1106+
function test_async_supplyWithoutBorrow_revertsPositionValueIncreased() public {
11071107
_seedPosition(10_000e18, 5_000e18);
11081108
address request = _startAsync(6_000e18, 100);
11091109
_consume(request, 6_000e18, 60e18, 6_000e18);
@@ -1121,7 +1121,7 @@ contract RetargetterAsyncTest is RetargetterBaseTest {
11211121

11221122
// The supply-only tail: no BORROW leg pulls the repayment back out of the position
11231123
vm.prank(rebalancer);
1124-
vm.expectPartialRevert(LibRetargetterErrors.TotalAssetsIncreased.selector);
1124+
vm.expectPartialRevert(LibRetargetterErrors.PositionValueIncreased.selector);
11251125
retargetter.rebalance(_rebalancingData(MAX_SENTINEL, 0, RebalancingOperationType.SUPPLY, MAX_SENTINEL));
11261126
}
11271127

0 commit comments

Comments
 (0)