TRST-M-1 mit review: trackCollateralization() inside trackStatus() (#…

…1088)

contracts/interfaces/IBasketHandler.sol

@@ -97,14 +97,10 @@ interface IBasketHandler is IComponent {
     /// @custom:interaction
     function refreshBasket() external;
 
-    /// Track the basket status changes
+    /// Track basket status and collateralization changes
     /// @custom:refresher
     function trackStatus() external;
 
-    /// Track when last collateralized
-    /// @custom:refresher
-    function trackCollateralization() external;
-
     /// @return If the BackingManager has sufficient collateral to redeem the entire RToken supply
     function fullyCollateralized() external view returns (bool);
 
@@ -175,6 +171,8 @@ interface IBasketHandler is IComponent {
 }
 
 interface TestIBasketHandler is IBasketHandler {
+    function lastCollateralized() external view returns (uint48);
+
     function warmupPeriod() external view returns (uint48);
 
     function setWarmupPeriod(uint48 val) external;

contracts/p0/BackingManager.sol

@@ -82,8 +82,6 @@ contract BackingManagerP0 is TradingP0, IBackingManager {
                 if (errData.length == 0) revert(); // solhint-disable-line reason-string
             }
         }
-
-        main.basketHandler().trackCollateralization();
     }
 
     /// Apply the overall backing policy using the specified TradeKind, taking a haircut if unable

contracts/p0/BasketHandler.sol

@@ -218,7 +218,7 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler {
         trackStatus();
     }
 
-    /// Track basket status changes if they ocurred
+    /// Track basket status and collateralization changes
     // effects: lastStatus' = status(), and lastStatusTimestamp' = current timestamp
     /// @custom:refresher
     function trackStatus() public {
@@ -228,13 +228,9 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler {
             lastStatus = currentStatus;
             lastStatusTimestamp = uint48(block.timestamp);
         }
-    }
 
-    /// Track when last collateralized
-    // effects: lastCollateralized' = nonce if nonce > lastCollateralized && fullyCapitalized
-    /// @custom:refresher
-    function trackCollateralization() external {
-        if (nonce > lastCollateralized && fullyCollateralized()) {
+        // Invalidate old nonces if fully collateralized
+        if (reweightable && nonce > lastCollateralized && fullyCollateralized()) {
             emit LastCollateralizedChanged(lastCollateralized, nonce);
             lastCollateralized = nonce;
         }

contracts/p1/BackingManager.sol

@@ -107,8 +107,6 @@ contract BackingManagerP1 is TradingP1, IBackingManager {
                 if (errData.length == 0) revert(); // solhint-disable-line reason-string
             }
         }
-
-        basketHandler.trackCollateralization();
     }
 
     /// Apply the overall backing policy using the specified TradeKind, taking a haircut if unable

contracts/p1/BasketHandler.sol

@@ -160,23 +160,22 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler {
         trackStatus();
     }
 
-    /// Track basket status changes if they ocurred
+    /// Track basket status and collateralization changes
     // effects: lastStatus' = status(), and lastStatusTimestamp' = current timestamp
     /// @custom:refresher
     function trackStatus() public {
+        // Historical context: This is not the ideal naming for this function but it allowed
+        // reweightable RTokens introduced in 3.2.0 to be a minor update as opposed to major
+
         CollateralStatus currentStatus = status();
         if (currentStatus != lastStatus) {
             emit BasketStatusChanged(lastStatus, currentStatus);
             lastStatus = currentStatus;
             lastStatusTimestamp = uint48(block.timestamp);
         }
-    }
 
-    /// Track when last collateralized
-    // effects: lastCollateralized' = nonce if nonce > lastCollateralized && fullyCapitalized
-    /// @custom:refresher
-    function trackCollateralization() external {
-        if (nonce > lastCollateralized && fullyCollateralized()) {
+        // Invalidate old nonces if fully collateralized
+        if (reweightable && nonce > lastCollateralized && fullyCollateralized()) {
             emit LastCollateralizedChanged(lastCollateralized, nonce);
             lastCollateralized = nonce;
         }
@@ -479,9 +478,13 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler {
                 basketNonces[i] >= lastCollateralized && basketNonces[i] <= nonce,
                 "invalid basketNonce"
             );
-            Basket storage b = basketHistory[basketNonces[i]];
+            // Known limitation: During an ongoing rebalance it may possible to redeem
+            // on a previous basket nonce for _more_ UoA value than the current basket.
+            // This can only occur for index RTokens, and the risk has been mitigated
+            // by updating `lastCollateralized` on every assetRegistry.refresh().
 
             // Add-in refAmts contribution from historical basket
+            Basket storage b = basketHistory[basketNonces[i]];
             for (uint256 j = 0; j < b.erc20s.length; ++j) {
                 // untestable:
                 //     previous baskets erc20s do not contain the zero address