refactor: per-offer revoke auth via registry boolean read, hoist marketId, silence cast lints

Review follow-ups on the offer lifecycle hardening:
- revokeOffers resolves the registry guardian/owner power once as a boolean
  (canRevokeOffer replaces the reverting checkCanRevokeOffer) and the per-offer
  proposer check moves into LibBorrowOffers.removeOffer; the Unauthorized error
  keeps Solady's selector so external behavior is unchanged
- _positionLtvFrame and _consumeOffers read marketId from storage once
- _checkLtvReduced NatSpec records why the cross-multiplication needs
  fullMulDiv's 512-bit intermediate (price-scaled values can overflow 256 bits)
- forge 1.5.1 unsafe-typecast lints in LibBorrowOffers silenced with justified
  disable comments
- new tests: empty revoke batch no-op, guardian revoke of an active offer,
  per-offer OfferRevoked events in a batch

Author: maxencerb

src/borrow/BorrowOffersRegistry.sol

@@ -171,8 +171,8 @@ contract BorrowOffersRegistry is IBorrowOffersRegistry, Initializable, OwnableRo
   }
 
   /// @inheritdoc IBorrowOffersRegistry
-  function checkCanRevokeOffer(address account) external view override {
-    _checkHasRolesOrOwner(account, GUARDIAN_ROLE);
+  function canRevokeOffer(address account) external view override returns (bool) {
+    return hasAnyRole(account, GUARDIAN_ROLE) || account == owner();
   }
 
   /// @dev Reverts with Solady's `Unauthorized` unless `account` holds any of `roles` or is the

src/borrow/MorphoBorrowPosition.sol

@@ -465,19 +465,23 @@ contract MorphoBorrowPosition is IBorrowPosition, IBorrowOffers, Initializable,
   ///      {_isHealthy}). Called by {_offerPreLiquidate} before the consume and after the repay
   ///      settles, so the two frames differ only by the fill's real effect on Morpho's totals.
   function _positionLtvFrame(uint256 price) internal view returns (uint256 debtValue, uint256 collateralValue) {
-    BorrowPositionStorage storage _storage = _borrowPositionStorage();
-    Position memory position = MORPHO.position(_storage.marketId, address(this));
-    Market memory market = MORPHO.market(_storage.marketId);
+    Id _marketId = _borrowPositionStorage().marketId;
+    Position memory position = MORPHO.position(_marketId, address(this));
+    Market memory market = MORPHO.market(_marketId);
     debtValue =
       uint256(position.borrowShares).toAssetsUp(uint256(market.totalBorrowAssets), uint256(market.totalBorrowShares));
     collateralValue = uint256(position.collateral).mulDiv(price, ORACLE_PRICE_SCALE);
   }
 
   /// @dev Reverts {LibBorrowErrors.LtvNotReduced} unless the settled position's LTV is strictly
   ///      below the entry frame's: `exitDebt / exitCollateral < entryDebt / entryCollateral`,
-  ///      compared exactly by cross-multiplication (`fullMulDiv` floors, so
-  ///      `floor(a / d) < y <=> a < y * d` makes the comparison exact). `entryDebtValue > 0` in
-  ///      the band (the position has debt, rounded up).
+  ///      cross-multiplied to `exitDebt * entryCollateral < entryDebt * exitCollateral` so the
+  ///      comparison is exact. The product goes through `fullMulDiv`'s 512-bit intermediate
+  ///      rather than a plain multiplication: the collateral values are price-scaled (`price` is
+  ///      an unbounded oracle value at 1e36 scale) and can exceed 128 bits, so the direct product
+  ///      can overflow 256 bits and checked multiplication would turn a legitimate fill into a
+  ///      spurious revert. Flooring keeps it exact (`floor(a * b / d) >= c <=> a * b >= c * d`
+  ///      for integers). `entryDebtValue > 0` in the band (the position has debt, rounded up).
   function _checkLtvReduced(uint256 price, uint256 entryDebtValue, uint256 entryCollateralValue) internal view {
     (uint256 exitDebtValue, uint256 exitCollateralValue) = _positionLtvFrame(price);
     if (FixedPointMathLib.fullMulDiv(exitDebtValue, entryCollateralValue, entryDebtValue) >= exitCollateralValue) {
@@ -493,9 +497,9 @@ contract MorphoBorrowPosition is IBorrowPosition, IBorrowOffers, Initializable,
     internal
     returns (uint256 totalSeized, uint256 totalDebtShares)
   {
-    BorrowPositionStorage storage _storage = _borrowPositionStorage();
-    Position memory position = MORPHO.position(_storage.marketId, borrower);
-    Market memory market = MORPHO.market(_storage.marketId);
+    Id _marketId = _borrowPositionStorage().marketId;
+    Position memory position = MORPHO.position(_marketId, borrower);
+    Market memory market = MORPHO.market(_marketId);
 
     LibBorrowOffers.BorrowOffersStorage storage o = LibBorrowOffers.borrowOffersStorage();
     LibBorrowOffers.ConsumeInput memory input = LibBorrowOffers.ConsumeInput({
@@ -960,25 +964,17 @@ contract MorphoBorrowPosition is IBorrowPosition, IBorrowOffers, Initializable,
   /// @inheritdoc IBorrowOffers
   /// @dev Authorization is per offer: the recorded proposer may revoke its own offer at any time
   ///      (a mistaken offer must not stand just because no guardian reacted before `activeAt`);
-  ///      any other caller must pass {IBorrowOffersRegistry.checkCanRevokeOffer} (a guardian, or
-  ///      the registry owner), the veto inside the timelock window and the kill switch for a bad
-  ///      standing offer at any later stage. Liveness is checked before the proposer read so an
-  ///      unknown id reverts {LibBorrowErrors.OfferNotFound} for every caller; an empty batch
-  ///      performs no authorization check and is a no-op.
+  ///      any other caller needs the registry's revoke power
+  ///      ({IBorrowOffersRegistry.canRevokeOffer}: a guardian, or the registry owner), read once
+  ///      for the whole batch and enforced per offer inside {LibBorrowOffers.removeOffer}. An
+  ///      empty batch is a no-op.
   function revokeOffers(uint8[] calldata ids) external override {
     LibBorrowOffers.BorrowOffersStorage storage o = LibBorrowOffers.borrowOffersStorage();
-    bool checkedCanRevokeAny; // the registry guardian/owner check passes at most once
-    // Cache the calldata length once, and each id into a stack variable, so the loop performs a
-    // single CALLDATALOAD per iteration instead of re-reading `ids.length` and `ids[i]` twice each.
+    bool isGuardian = OFFERS_REGISTRY.canRevokeOffer(msg.sender);
     uint256 length = ids.length;
     for (uint256 i; i < length; ++i) {
       uint8 id = ids[i];
-      if (!o.isLive(id)) revert LibBorrowErrors.OfferNotFound();
-      if (o.slab[id].proposer != msg.sender && !checkedCanRevokeAny) {
-        OFFERS_REGISTRY.checkCanRevokeOffer(msg.sender);
-        checkedCanRevokeAny = true;
-      }
-      o.removeOffer(id);
+      o.removeOffer(id, msg.sender, isGuardian);
       emit OfferRevoked(id, msg.sender);
     }
   }

src/interfaces/borrow/IBorrowOffers.sol

@@ -96,9 +96,12 @@ interface IBorrowOffers {
   function proposeOffer(uint128 collateral, uint128 debtShares, uint40 expiresAt) external returns (uint8 id);
 
   /// @notice Revokes one or more offers (batch). Authorization is per offer: the recorded
-  ///         proposer may revoke its own offer at any time; any other caller must pass
-  ///         {IBorrowOffersRegistry.checkCanRevokeOffer} (a guardian, or the registry owner).
-  /// @dev Reverts {LibBorrowErrors.OfferNotFound} if any id is not a currently-live offer.
+  ///         proposer may revoke its own offer at any time; any other caller needs the registry's
+  ///         revoke power ({IBorrowOffersRegistry.canRevokeOffer}: a guardian, or the registry
+  ///         owner).
+  /// @dev Reverts {LibBorrowErrors.OfferNotFound} if any id is not a currently-live offer, and
+  ///      {LibBorrowErrors.Unauthorized} if any offer in the batch belongs to another proposer
+  ///      while the caller lacks the registry's revoke power.
   /// @param ids The slab ids to revoke.
   function revokeOffers(uint8[] calldata ids) external;
 

src/interfaces/borrow/IBorrowOffersRegistry.sol

@@ -49,11 +49,13 @@ interface IBorrowOffersRegistry {
   ///      a role, so an ownership handover never leaves the new owner without powers.
   function checkCanCreateOffer(address account) external view;
 
-  /// @notice Reverts unless `account` may revoke offers (a guardian, or the registry owner): the
-  ///         veto power inside the timelock window, and the kill switch for a bad standing offer.
-  /// @dev The authorization check used by {MorphoBorrowPosition.revokeOffers}; reverts with
-  ///      Solady's `Unauthorized()`.
-  function checkCanRevokeOffer(address account) external view;
+  /// @notice Returns whether `account` may revoke any offer regardless of its proposer (a
+  ///         guardian, or the registry owner): the veto power inside the timelock window, and the
+  ///         kill switch for a bad standing offer.
+  /// @dev Read once per batch by {MorphoBorrowPosition.revokeOffers}, which enforces the
+  ///      per-offer proposer check itself for callers without this power. A boolean read rather
+  ///      than a reverting check: a proposer revoking its own offer is authorized without it.
+  function canRevokeOffer(address account) external view returns (bool);
 
   /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
   /*                       CONFIGURATION                        */

src/libs/borrow/LibBorrowErrors.sol

@@ -78,6 +78,12 @@ library LibBorrowErrors {
   /// @notice Thrown when the callback is called by an address other than Morpho.
   error NotMorpho();
 
+  /// @notice Thrown when the caller of {IBorrowOffers.revokeOffers} is neither the offer's
+  ///         recorded proposer nor a registry guardian/owner. Deliberately the same selector as
+  ///         Solady's `Unauthorized()`, which the registry's own authorization checks revert
+  ///         with, so callers see one selector for every authorization failure.
+  error Unauthorized();
+
   /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
   /*                          OFFERS                            */
   /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

src/libs/borrow/LibBorrowOffers.sol

@@ -143,6 +143,17 @@ library LibBorrowOffers {
     delete s.slab[id];
   }
 
+  /// @notice Authorized removal for {IBorrowOffers.revokeOffers}: `isGuardian` callers (the
+  ///         registry's guardian/owner revoke power, resolved once per batch by the caller) may
+  ///         remove any offer; any other caller must be the offer's recorded proposer.
+  /// @dev Liveness resolves first, so an unknown id reverts {LibBorrowErrors.OfferNotFound} for
+  ///      every caller; an unauthorized caller then reverts {LibBorrowErrors.Unauthorized}.
+  function removeOffer(BorrowOffersStorage storage s, uint8 id, address caller, bool isGuardian) internal {
+    if (!isLive(s, id)) revert LibBorrowErrors.OfferNotFound();
+    if (!isGuardian && s.slab[id].proposer != caller) revert LibBorrowErrors.Unauthorized();
+    removeOffer(s, id);
+  }
+
   /// @dev Allocates the lowest free slab index and marks it live, or reverts
   ///      {LibBorrowErrors.TooManyOffers} when all `MAX_OFFERS` slots hold unexpired offers.
   ///      Expired offers are pruned first (bit cleared, slot zeroed; housekeeping only, mirroring
@@ -162,6 +173,8 @@ library LibBorrowOffers {
     uint256 freeBits = ~liveBits & ((uint256(1) << MAX_OFFERS) - 1);
     if (freeBits == 0) revert LibBorrowErrors.TooManyOffers();
     id = uint8(LibBit.ffs(freeBits));
+    // Only bits below `MAX_OFFERS` (32) survive the mask above, so the cast cannot truncate.
+    // forge-lint: disable-next-line(unsafe-typecast)
     s.liveBits = uint32(liveBits | (uint256(1) << id));
   }
 
@@ -224,6 +237,8 @@ library LibBorrowOffers {
       }
       emit IBorrowOffers.OfferConsumed(walkOffer.id, walkOffer.filledCollateral, walkOffer.filledShares, exhausted);
     }
+    // Clearing bits of an already-`uint32` value cannot truncate.
+    // forge-lint: disable-next-line(unsafe-typecast)
     if (clearBits != 0) s.liveBits = uint32(s.liveBits & ~clearBits);
   }
 
@@ -263,6 +278,8 @@ library LibBorrowOffers {
       }
       if (block.timestamp < offer.activeAt) continue;
       WalkOffer memory walkOffer = offers[walkLength];
+      // `id` indexes a set bit of the 32-bit live bitmap, so the cast cannot truncate.
+      // forge-lint: disable-next-line(unsafe-typecast)
       walkOffer.id = uint8(id);
       walkOffer.remainingCollateral = offer.remainingCollateral;
       walkOffer.remainingDebtShares = offer.remainingDebtShares;
@@ -331,10 +348,12 @@ library LibBorrowOffers {
 
       // CONSUME. `fillCollateral <= remainingCollateral` and `fillShares <= remainingDebtShares`
       // by construction, so the uint128 casts and subtractions cannot overflow/underflow.
+      // forge-lint: disable-start(unsafe-typecast)
       walkOffer.remainingCollateral -= uint128(fillCollateral);
       walkOffer.remainingDebtShares -= uint128(fillShares);
       walkOffer.filledCollateral = uint128(fillCollateral);
       walkOffer.filledShares = uint128(fillShares);
+      // forge-lint: disable-end(unsafe-typecast)
       totalSeized += fillCollateral;
       totalDebtShares += fillShares;
 

test/borrow/BorrowOffersRegistry.t.sol

@@ -162,37 +162,34 @@ contract BorrowOffersRegistryTest is Test {
     registry.checkCanCreateOffer(proposer);
   }
 
-  function test_CheckCanRevokeOffer_GuardianRoundTrip() public {
-    // Not a guardian yet: rejected
-    vm.expectRevert(Unauthorized.selector);
-    registry.checkCanRevokeOffer(guardian);
+  function test_CanRevokeOffer_GuardianRoundTrip() public {
+    // Not a guardian yet: no revoke power
+    assertFalse(registry.canRevokeOffer(guardian), "non-guardian should have no revoke power");
 
-    // Enabled: passes (no revert)
+    // Enabled: revoke power granted
     vm.prank(owner);
     registry.setGuardian(guardian, true);
-    registry.checkCanRevokeOffer(guardian);
+    assertTrue(registry.canRevokeOffer(guardian), "guardian should have revoke power");
     assertTrue(registry.hasAnyRole(guardian, registry.GUARDIAN_ROLE()), "guardian role bit should be set");
 
-    // Disabled again: rejected
+    // Disabled again: revoke power revoked
     vm.prank(owner);
     registry.setGuardian(guardian, false);
-    vm.expectRevert(Unauthorized.selector);
-    registry.checkCanRevokeOffer(guardian);
+    assertFalse(registry.canRevokeOffer(guardian), "removed guardian should lose revoke power");
   }
 
   function test_Checks_OwnerPassesWithoutRoles() public view {
     // The owner is authorized by derivation, without holding any role bit
     assertEq(registry.rolesOf(owner), 0, "owner should hold no explicit role");
     registry.checkCanCreateOffer(owner);
-    registry.checkCanRevokeOffer(owner);
+    assertTrue(registry.canRevokeOffer(owner), "owner should have revoke power by derivation");
   }
 
   function test_Checks_StrangerRejected() public {
     vm.expectRevert(Unauthorized.selector);
     registry.checkCanCreateOffer(stranger);
 
-    vm.expectRevert(Unauthorized.selector);
-    registry.checkCanRevokeOffer(stranger);
+    assertFalse(registry.canRevokeOffer(stranger), "stranger should have no revoke power");
   }
 
   function test_Checks_RoleSeparation() public {
@@ -201,9 +198,8 @@ contract BorrowOffersRegistryTest is Test {
     registry.setGuardian(guardian, true);
     vm.stopPrank();
 
-    // A proposer cannot revoke offers
-    vm.expectRevert(Unauthorized.selector);
-    registry.checkCanRevokeOffer(proposer);
+    // A proposer holds no batch-wide revoke power (it may still revoke its own offers)
+    assertFalse(registry.canRevokeOffer(proposer), "proposer should have no revoke power");
 
     // A guardian cannot create offers
     vm.expectRevert(Unauthorized.selector);
@@ -525,13 +521,12 @@ contract BorrowOffersRegistryTest is Test {
 
     // The new owner is authorized by derivation
     registry.checkCanCreateOffer(newOwner);
-    registry.checkCanRevokeOffer(newOwner);
+    assertTrue(registry.canRevokeOffer(newOwner), "new owner should have revoke power");
 
     // The old owner has no lingering powers
     vm.expectRevert(Unauthorized.selector);
     registry.checkCanCreateOffer(owner);
-    vm.expectRevert(Unauthorized.selector);
-    registry.checkCanRevokeOffer(owner);
+    assertFalse(registry.canRevokeOffer(owner), "old owner should lose revoke power");
 
     vm.prank(owner);
     vm.expectRevert(Unauthorized.selector);

test/borrow/MorphoBorrowPositionOffers.t.sol

@@ -1255,6 +1255,57 @@ contract MorphoBorrowPositionOffersTest is Test {
     assertEq(pos.offerCount(), 0, "guardian revoked both");
   }
 
+  /// @notice An empty batch is a no-op for every caller class: no revert, no state change and no
+  ///         events (the registry revoke-power read is a state-neutral view).
+  function test_revokeOffers_emptyBatchIsNoOp() public {
+    _enterBand(0.7e18);
+    _proposeAtPrice(_positionShares() / 4, 1.2e18);
+
+    uint8[] memory none = new uint8[](0);
+    vm.recordLogs();
+    vm.prank(makeAddr("rando"));
+    pos.revokeOffers(none);
+    vm.prank(guardian);
+    pos.revokeOffers(none);
+    assertEq(pos.offerCount(), 1, "book untouched by empty batches");
+    assertEq(vm.getRecordedLogs().length, 0, "no events emitted");
+  }
+
+  /// @notice The guardian power is not limited to the timelock window: a standing (active,
+  ///         consumable) offer can still be pulled at any later stage.
+  function test_revokeOffers_guardianRevokesActiveOffer() public {
+    _enterBand(0.7e18);
+    uint8 id = _proposeAtPrice(_positionShares() / 4, 1.2e18);
+    _warpActive();
+    assertTrue(pos.isConsumable(id), "offer is live and consumable");
+
+    uint8[] memory ids = new uint8[](1);
+    ids[0] = id;
+    vm.prank(guardian);
+    pos.revokeOffers(ids);
+    assertEq(pos.offerCount(), 0, "guardian revoked the active offer");
+  }
+
+  /// @notice A multi-offer batch emits one {OfferRevoked} per removed offer, each attributed to
+  ///         the batch caller.
+  function test_revokeOffers_batchEmitsPerOffer() public {
+    _enterBand(0.7e18);
+    uint256 shares = _positionShares() / 8;
+    uint8 a = _proposeAtPrice(shares, 1.2e18);
+    uint8 b = _proposeAtPrice(shares, 1.3e18);
+
+    uint8[] memory ids = new uint8[](2);
+    ids[0] = a;
+    ids[1] = b;
+    vm.expectEmit(true, true, false, false, address(pos));
+    emit IBorrowOffers.OfferRevoked(a, guardian);
+    vm.expectEmit(true, true, false, false, address(pos));
+    emit IBorrowOffers.OfferRevoked(b, guardian);
+    vm.prank(guardian);
+    pos.revokeOffers(ids);
+    assertEq(pos.offerCount(), 0, "both revoked");
+  }
+
   function test_slab_recyclesFreedSlots() public {
     _enterBand(0.7e18);
     uint256 shares = _positionShares() / 8;