Skip to content

Commit 713b4e2

Browse files
committed
refactor: drop operation.positionManager, key active on fund + bound PM
The operation always runs against the instance-bound position manager, which cannot change while active, so storing it on the operation record was pure duplication. Remove the field: - fund becomes the operation-active flag (both start paths set it, only clearOperation clears it, and no fund can be the zero address since setFund contract-checks it). isActive, _checkStart and setPositionManager's active guard key on fund; checkActive keys on fund and returns the bound PM. - consume/authorizeMinting/resolve read the bound PM from assets storage; the operation() view derives its positionManager (bound PM when active, else zero) instead of returning a stored field, keeping the same ABI and semantics. RetargetterOperation repacks to 7 slots (request now shares slot 0, fund + the order mode share slot 1); the storage-layout and stray-order tests are updated. Full suite 2347 passing, 74 invariants passing; the fund active flag is mutation-checked. Retargetter runtime 23,953 B on the ci profile, 623 B under EIP-170.
1 parent 1ec46bd commit 713b4e2

4 files changed

Lines changed: 66 additions & 61 deletions

File tree

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

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,15 @@ struct RetargetterWhitelists {
4141
}
4242

4343
/// @notice The one in-flight operation (namespace "retargetter.operation").
44-
/// @dev `positionManager != address(0)` is the operation-active flag. An ASYNC operation uses
45-
/// every field and is zeroed at resolve; a SYNC flash-loan window populates only the
46-
/// addresses the steps need (`positionManager`, `fund`) and zeroes them at window close,
47-
/// so `request == address(0)` inside a window is what gates the ASYNC-only steps. The
48-
/// stored order is partial: `owner` and `receiver` are always the Retargetter (enforced
49-
/// at create) so {LibStorage.order} rebuilds the full Order in memory.
50-
/// @param positionManager The operation's position manager
44+
/// @dev `fund != address(0)` is the operation-active flag: both start paths set it and only
45+
/// {clearOperation} clears it, and no fund can be the zero address (setFund contract-
46+
/// checks it). The position manager an operation runs against is the instance-bound one
47+
/// ({RetargetterAssets.positionManager}), which cannot change while active, so it is not
48+
/// duplicated here. An ASYNC operation uses every field and is zeroed at resolve; a SYNC
49+
/// flash-loan window populates only `fund` and zeroes it at window close, so
50+
/// `request == address(0)` inside a window is what gates the ASYNC-only steps. The stored
51+
/// order is partial: `owner` and `receiver` are always the Retargetter (enforced at
52+
/// create) so {LibStorage.order} rebuilds the full Order in memory.
5153
/// @param startedAt The loan clock origin, set at the first consume or nonzero mint
5254
/// authorization (0 until then); rewound to 0 by a revocation that leaves the
5355
/// operation with no PT, no YT and no pending authorization
@@ -74,7 +76,6 @@ struct RetargetterWhitelists {
7476
/// eagerly, a completed mint is pruned lazily by the next write-path computation, and
7577
/// resolve empties the set
7678
struct RetargetterOperation {
77-
address positionManager;
7879
uint40 startedAt;
7980
uint16 operationMaxYieldBps;
8081
bool consumptionClosed;
@@ -135,12 +136,13 @@ library LibStorage {
135136
/* OPERATION GATES */
136137
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
137138

138-
/// @dev Reverts unless an operation is active (ASYNC or SYNC window); returns its position
139-
/// manager.
139+
/// @dev Reverts unless an operation is active (ASYNC or SYNC window), keyed on the fund flag;
140+
/// returns the instance-bound position manager the operation runs against (always set
141+
/// while active, since it was required at start and cannot be unbound mid-operation).
140142
/// @param self The storage pointer to the operation
141143
function checkActive(RetargetterOperation storage self) internal view returns (address positionManager) {
142-
positionManager = self.positionManager;
143-
if (positionManager == address(0)) revert LibRetargetterErrors.NoActiveOperation();
144+
if (self.fund == address(0)) revert LibRetargetterErrors.NoActiveOperation();
145+
positionManager = assetsStorage().positionManager;
144146
}
145147

146148
/// @dev Reverts unless an ASYNC operation is active (SYNC windows carry no Request);
@@ -305,7 +307,6 @@ library LibStorage {
305307
/// each removal a plain pop); the order fields are cleared separately as orders end.
306308
/// @param self The storage pointer to the operation
307309
function clearOperation(RetargetterOperation storage self) internal {
308-
self.positionManager = address(0);
309310
self.startedAt = 0;
310311
self.operationMaxYieldBps = 0;
311312
self.consumptionClosed = false;

‎src/manager/rebalancer/Retargetter.sol‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,8 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
234234
0
235235
);
236236

237-
operation_.positionManager = positionManager;
237+
// fund is the operation-active flag (see {RetargetterOperation}); the position manager it
238+
// runs against is the instance-bound one, not duplicated here
238239
operation_.operationMaxYieldBps = effectiveYieldCap;
239240
operation_.request = request;
240241
operation_.repaymentDeadline = repaymentDeadline;
@@ -275,7 +276,9 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
275276
operation_.checkYield(offer.amount, offer.expectedReturn);
276277
operation_.checkConsumptionWindow();
277278
ytAmount = IRequest(request).consume(offer, signature, ptAmount);
278-
operation_.checkPrincipalCap(request, 0, _maxPrincipal(operation_.positionManager, operation_.operationMaxYieldBps));
279+
operation_.checkPrincipalCap(
280+
request, 0, _maxPrincipal(LibStorage.assetsStorage().positionManager, operation_.operationMaxYieldBps)
281+
);
279282
emit OfferConsumed(request, offer.maker, ptAmount, ytAmount);
280283
}
281284

@@ -306,7 +309,7 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
306309
ptAmount,
307310
ytAmount,
308311
ptAmount,
309-
_maxPrincipal(operation_.positionManager, operation_.operationMaxYieldBps)
312+
_maxPrincipal(LibStorage.assetsStorage().positionManager, operation_.operationMaxYieldBps)
310313
);
311314
// The capacity bound keeps every loop over the set within gas reach; see the constant
312315
accounts.add(to, MAX_AUTHORIZED_ACCOUNTS);
@@ -388,7 +391,7 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
388391
function resolve() external onlyOwnerOrRebalancer nonReentrant {
389392
RetargetterOperation storage operation_ = LibStorage.operationStorage();
390393
address request = operation_.checkRequest();
391-
address positionManager = operation_.positionManager;
394+
address positionManager = LibStorage.assetsStorage().positionManager;
392395
if (!IRequest(request).syncRepaidStatus()) revert LibRetargetterErrors.RequestNotRepaid();
393396
operation_.checkNoPendingOrder(operation_.fund);
394397
_checkResidual();
@@ -595,15 +598,15 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
595598
revert LibRetargetterErrors.ModuleNotWhitelisted();
596599
}
597600

598-
// Open the window: the operation storage carries the addresses the steps read (and locks
599-
// out nested starts); the module slot authenticates the callback and is zeroed on its
600-
// entry (single-shot). The payload digest and the pre-loan debt balance bind the callback
601-
// to this exact payload and to the full delivery of the principal, so a whitelisted
602-
// module can neither substitute its own step calls nor collect the repayment approval
603-
// for funds it never delivered. Step authority is not handed out here: the window slot
604-
// is set inside the callback around the payload alone, so the module holds no authority
605-
// in its own frame and cannot source the delivery from the position manager itself
606-
operation_.positionManager = positionManager;
601+
// Open the window: the operation's fund both records the venue the steps read and flags
602+
// the operation active (locking out nested starts). The module slot authenticates the
603+
// callback and is zeroed on its entry (single-shot). The payload digest and the pre-loan
604+
// debt balance bind the callback to this exact payload and to the full delivery of the
605+
// principal, so a whitelisted module can neither substitute its own step calls nor collect
606+
// the repayment approval for funds it never delivered. Step authority is not handed out
607+
// here: the window slot is set inside the callback around the payload alone, so the module
608+
// holds no authority in its own frame and cannot source the delivery from the position
609+
// manager itself
607610
operation_.fund = fund;
608611
address debtAsset = LibStorage.assetsStorage().debtAsset;
609612
bytes memory payload = abi.encode(data);
@@ -730,7 +733,7 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
730733
/// PositionManagerNotBound meanwhile). Every start-gate and principal-cap read trusts
731734
/// the bound address, which is why the binding is owner-curated, not caller-supplied.
732735
function setPositionManager(address positionManager) external onlyOwner nonReentrant {
733-
if (LibStorage.operationStorage().positionManager != address(0)) revert LibRetargetterErrors.OperationActive();
736+
if (LibStorage.operationStorage().fund != address(0)) revert LibRetargetterErrors.OperationActive();
734737
_bindPositionManager(positionManager);
735738
}
736739

@@ -763,8 +766,10 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
763766
)
764767
{
765768
RetargetterOperation storage operation_ = LibStorage.operationStorage();
769+
// The operation runs against the instance-bound position manager while active (fund set),
770+
// and zero when idle: derived rather than stored on the operation
766771
return (
767-
operation_.positionManager,
772+
operation_.fund != address(0) ? LibStorage.assetsStorage().positionManager : address(0),
768773
operation_.request,
769774
operation_.fund,
770775
operation_.startedAt,
@@ -780,7 +785,7 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
780785

781786
/// @inheritdoc IRetargetter
782787
function isActive() external view returns (bool) {
783-
return LibStorage.operationStorage().positionManager != address(0);
788+
return LibStorage.operationStorage().fund != address(0);
784789
}
785790

786791
/// @inheritdoc IRetargetter
@@ -882,7 +887,7 @@ contract Retargetter is IRetargetter, IFlashLoanReceiver, OwnableRoles, Initiali
882887
view
883888
returns (address positionManager)
884889
{
885-
if (operation_.positionManager != address(0)) revert LibRetargetterErrors.OperationActive();
890+
if (operation_.fund != address(0)) revert LibRetargetterErrors.OperationActive();
886891
if (operation_.orderLive) revert LibRetargetterErrors.OrderActive();
887892
positionManager = LibStorage.assetsStorage().positionManager;
888893
if (positionManager == address(0)) revert LibRetargetterErrors.PositionManagerNotBound();

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

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,30 +1062,29 @@ contract RetargetterTest is RetargetterBaseTest {
10621062
assertEq(accounts.length, 1, "one registered account");
10631063
assertEq(accounts[0], broker, "broker registered");
10641064

1065-
// Slot A: positionManager (bits 0..159) | startedAt (160..199) | operationMaxYieldBps
1066-
// (200..215) | consumptionClosed (216..223) | horizon (224..255): full, no wasted bits
1065+
// Slot A: startedAt (bits 0..39) | operationMaxYieldBps (40..55) | consumptionClosed
1066+
// (56..63) | horizon (64..95) | request (96..255): full, no wasted bits (no stored
1067+
// position manager: it is the instance-bound one, and fund is the active flag)
10671068
uint256 slotA = uint256(vm.load(address(retargetter), OPERATION_STORAGE_SLOT));
1068-
assertEq(address(uint160(slotA)), address(positionManager), "position manager bits");
1069-
assertEq(uint40(slotA >> 160), uint40(startTime + 1 days), "startedAt bits");
1070-
assertEq(uint16(slotA >> 200), 100, "operation max yield bits");
1071-
assertEq(uint8(slotA >> 216), 0, "consumption closed bit clear");
1072-
assertEq(slotA >> 224, DEFAULT_HORIZON, "horizon bits");
1073-
1074-
// Slot B: request (bits 0..159) | repaymentDeadline (160..199) | tickDuration (200..223)
1075-
// | tickThreshold (224..247)
1069+
assertEq(uint40(slotA), uint40(startTime + 1 days), "startedAt bits");
1070+
assertEq(uint16(slotA >> 40), 100, "operation max yield bits");
1071+
assertEq(uint8(slotA >> 56), 0, "consumption closed bit clear");
1072+
assertEq(uint32(slotA >> 64), DEFAULT_HORIZON, "horizon bits");
1073+
assertEq(address(uint160(slotA >> 96)), request, "request bits");
1074+
1075+
// Slot B: repaymentDeadline (bits 0..39) | tickDuration (40..63) | tickThreshold (64..87)
1076+
// | fund (88..247) | orderMode (248..255): full
10761077
uint256 slotB = uint256(vm.load(address(retargetter), bytes32(uint256(OPERATION_STORAGE_SLOT) + 1)));
1077-
assertEq(address(uint160(slotB)), request, "request bits");
1078-
assertEq(uint40(slotB >> 160), uint40(startTime + 90 days), "repayment deadline bits");
1079-
assertEq(uint24(slotB >> 200), DEFAULT_TICK_DURATION, "tick duration bits");
1080-
assertEq(uint24(slotB >> 224), DEFAULT_TICK_THRESHOLD, "tick threshold bits");
1081-
assertEq(slotB >> 248, 0, "slot B upper bits clean");
1078+
assertEq(uint40(slotB), uint40(startTime + 90 days), "repayment deadline bits");
1079+
assertEq(uint24(slotB >> 40), DEFAULT_TICK_DURATION, "tick duration bits");
1080+
assertEq(uint24(slotB >> 64), DEFAULT_TICK_THRESHOLD, "tick threshold bits");
1081+
assertEq(address(uint160(slotB >> 88)), address(fund), "fund bits");
1082+
assertEq(uint8(slotB >> 248), uint8(Mode.REDEEM), "order mode bits");
10821083

1083-
// Slot C: fund (bits 0..159) | orderMode (160..167) | orderLive (168..175)
1084+
// Slot C: orderLive (bits 0..7)
10841085
uint256 slotC = uint256(vm.load(address(retargetter), bytes32(uint256(OPERATION_STORAGE_SLOT) + 2)));
1085-
assertEq(address(uint160(slotC)), address(fund), "fund bits");
1086-
assertEq(uint8(slotC >> 160), uint8(Mode.REDEEM), "order mode bits");
1087-
assertEq(uint8(slotC >> 168), 1, "order live bits");
1088-
assertEq(slotC >> 176, 0, "slot C upper bits clean");
1086+
assertEq(uint8(slotC), 1, "order live bits");
1087+
assertEq(slotC >> 8, 0, "slot C upper bits clean");
10891088

10901089
// Slots D to F: the stored order's input, output and salt
10911090
assertEq(
@@ -1098,15 +1097,15 @@ contract RetargetterTest is RetargetterBaseTest {
10981097
vm.load(address(retargetter), bytes32(uint256(OPERATION_STORAGE_SLOT) + 5)), bytes32(uint256(42)), "order salt"
10991098
);
11001099

1101-
// Pulling funds flips the consumption-closed bit (216..223) and nothing else in slot A
1100+
// Pulling funds flips the consumption-closed bit (56..63) and nothing else in slot A
11021101
vm.prank(rebalancer);
11031102
retargetter.pullRequestFunds(1);
11041103
slotA = uint256(vm.load(address(retargetter), OPERATION_STORAGE_SLOT));
1105-
assertEq(address(uint160(slotA)), address(positionManager), "position manager bits unchanged");
1106-
assertEq(uint40(slotA >> 160), uint40(startTime + 1 days), "startedAt bits unchanged");
1107-
assertEq(uint16(slotA >> 200), 100, "operation max yield bits unchanged");
1108-
assertEq(uint8(slotA >> 216), 1, "consumption closed bit set");
1109-
assertEq(slotA >> 224, DEFAULT_HORIZON, "horizon bits unchanged by the pull");
1104+
assertEq(uint40(slotA), uint40(startTime + 1 days), "startedAt bits unchanged");
1105+
assertEq(uint16(slotA >> 40), 100, "operation max yield bits unchanged");
1106+
assertEq(uint8(slotA >> 56), 1, "consumption closed bit set");
1107+
assertEq(uint32(slotA >> 64), DEFAULT_HORIZON, "horizon bits unchanged by the pull");
1108+
assertEq(address(uint160(slotA >> 96)), request, "request bits unchanged by the pull");
11101109
}
11111110

11121111
function test_storageLayout_assetsPacking() public view {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,13 @@ contract RetargetterSmokeTest is RetargetterBaseTest {
8585
assertApproxEqAbs(_currentLtv(), 6875e14, 1e14, "post LTV (11000/16000)");
8686
}
8787

88-
/// @dev Plants a stray live order directly in storage. The orderLive flag packs into the
89-
/// operation namespace's third slot at bit 168 (after the fund address and the mode);
90-
/// no reachable flow leaves it set without an active operation, so the defensive
91-
/// OrderActive guards at both start entry points need this to be exercised.
88+
/// @dev Plants a stray live order directly in storage. The orderLive flag sits alone at bit
89+
/// 0 of the operation namespace's third slot (the fund address and the order mode moved
90+
/// into the second slot); no reachable flow leaves it set without an active operation,
91+
/// so the defensive OrderActive guards at both start entry points need this exercised.
9292
function _plantStrayOrder() internal {
9393
bytes32 slot = bytes32(uint256(OPERATION_STORAGE_SLOT) + 2);
94-
vm.store(address(retargetter), slot, bytes32(uint256(1) << 168));
94+
vm.store(address(retargetter), slot, bytes32(uint256(1)));
9595
}
9696

9797
function test_startRetargetting_revertsOrderActiveOnStrayOrder() public {

0 commit comments

Comments
 (0)