Trust M-07: RTokenAsset.refresh() (#964)

reserve-protocol · Oct 18, 2023 · a007c47 · a007c47
1 parent 29c0f4c
commit a007c47
Show file tree

Hide file tree

Showing 30 changed files with 225 additions and 155 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,7 @@
 
 ### Upgrade Steps -- Required
 
-Upgrade `BackingManager`, `Broker`, and _all_ assets. ERC20s do not need to be upgraded.
+Upgrade all core contracts and _all_ assets. ERC20s do not need to be upgraded. Use `Deployer.deployRTokenAsset()` to create a new `RTokenAsset` instance. This asset should be swapped too.
 
 Then, call `Broker.cacheComponents()`.
 
@@ -16,6 +16,8 @@ Then, call `Broker.cacheComponents()`.
   - Remove `lotPrice()`
 - `Broker` [+1 slot]
   - Disallow starting dutch trades with non-RTokenAsset assets when `lastSave() != block.timestamp`
+- `Furnace`
+  - Allow melting while frozen
 
 ## Plugins
 

diff --git a/README.md b/README.md
@@ -22,6 +22,7 @@ For a much more detailed explanation of the economic design, including an hour-l
   - [Testing with Echidna](https://github.com/reserve-protocol/protocol/blob/master/docs/using-echidna.md): Notes so far on setup and usage of Echidna (which is decidedly an integration-in-progress!)
   - [Deployment](https://github.com/reserve-protocol/protocol/blob/master/docs/deployment.md): How to do test deployments in our environment.
 - [System Design](https://github.com/reserve-protocol/protocol/blob/master/docs/system-design.md): The overall architecture of our system, and some detailed descriptions about what our protocol is _intended_ to do.
+- [Pause and Freeze States](https://github.com/reserve-protocol/protocol/blob/master/docs/pause-freeze-states.md): An overview of which protocol functions are halted in the paused and frozen states.
 - [Deployment Variables](https://github.com/reserve-protocol/protocol/blob/master/docs/deployment-variables.md) A detailed description of the governance variables of the protocol.
 - [Our Solidity Style](https://github.com/reserve-protocol/protocol/blob/master/docs/solidity-style.md): Common practices, details, and conventions relevant to reading and writing our Solidity source code, estpecially where those go beyond standard practice.
 - [Writing Collateral Plugins](https://github.com/reserve-protocol/protocol/blob/master/docs/collateral.md): An overview of how to develop collateral plugins and the concepts / questions involved.

diff --git a/contracts/facade/FacadeRead.sol b/contracts/facade/FacadeRead.sol
@@ -33,7 +33,6 @@ contract FacadeRead is IFacadeRead {
 
         // Poke Main
         main.assetRegistry().refresh();
-        main.furnace().melt();
 
         // {BU}
         BasketRange memory basketsHeld = main.basketHandler().basketsHeldBy(account);
@@ -74,7 +73,6 @@ contract FacadeRead is IFacadeRead {
 
         // Poke Main
         reg.refresh();
-        main.furnace().melt();
 
         // Compute # of baskets to create `amount` qRTok
         uint192 baskets = (rTok.totalSupply() > 0) // {BU}
@@ -120,7 +118,6 @@ contract FacadeRead is IFacadeRead {
 
         // Poke Main
         main.assetRegistry().refresh();
-        main.furnace().melt();
 
         uint256 supply = rTok.totalSupply();
 
@@ -202,7 +199,7 @@ contract FacadeRead is IFacadeRead {
         IBasketHandler basketHandler = rToken.main().basketHandler();
 
         // solhint-disable-next-line no-empty-blocks
-        try rToken.main().furnace().melt() {} catch {}
+        try rToken.main().furnace().melt() {} catch {} // <3.1.0 RTokens may revert while frozen
 
         (erc20s, deposits) = basketHandler.quote(FIX_ONE, CEIL);
 
@@ -241,7 +238,6 @@ contract FacadeRead is IFacadeRead {
     {
         IMain main = rToken.main();
         main.assetRegistry().refresh();
-        main.furnace().melt();
 
         erc20s = main.assetRegistry().erc20s();
         balances = new uint256[](erc20s.length);

diff --git a/contracts/facade/FacadeTest.sol b/contracts/facade/FacadeTest.sol
@@ -97,7 +97,6 @@ contract FacadeTest is IFacadeTest {
 
         // Poke Main
         reg.refresh();
-        main.furnace().melt();
 
         address backingManager = address(main.backingManager());
         IERC20 rsr = main.rsr();

diff --git a/contracts/interfaces/IAssetRegistry.sol b/contracts/interfaces/IAssetRegistry.sol
@@ -34,7 +34,7 @@ interface IAssetRegistry is IComponent {
     function init(IMain main_, IAsset[] memory assets_) external;
 
     /// Fully refresh all asset state
-    /// @custom:interaction
+    /// @custom:refresher
     function refresh() external;
 
     /// Register `asset`

diff --git a/contracts/p0/BackingManager.sol b/contracts/p0/BackingManager.sol
@@ -86,7 +86,6 @@ contract BackingManagerP0 is TradingP0, IBackingManager {
     /// @custom:interaction
     function rebalance(TradeKind kind) external notTradingPausedOrFrozen {
         main.assetRegistry().refresh();
-        main.furnace().melt();
 
         // DoS prevention: unless caller is self, require 1 empty block between like-kind auctions
         require(
@@ -149,7 +148,6 @@ contract BackingManagerP0 is TradingP0, IBackingManager {
         require(ArrayLib.allUnique(erc20s), "duplicate tokens");
 
         main.assetRegistry().refresh();
-        main.furnace().melt();
 
         require(tradesOpen == 0, "trade open");
         require(main.basketHandler().isReady(), "basket not ready");

diff --git a/contracts/p0/Furnace.sol b/contracts/p0/Furnace.sol
@@ -36,7 +36,7 @@ contract FurnaceP0 is ComponentP0, IFurnace {
 
     /// Performs any melting that has vested since last call.
     /// @custom:refresher
-    function melt() public notFrozen {
+    function melt() public {
         if (uint48(block.timestamp) < uint64(lastPayout) + PERIOD) return;
 
         // # of whole periods that have passed since lastPayout
@@ -58,15 +58,9 @@ contract FurnaceP0 is ComponentP0, IFurnace {
     /// Ratio setting
     /// @custom:governance
     function setRatio(uint192 ratio_) public governance {
-        if (lastPayout > 0) {
-            // solhint-disable-next-line no-empty-blocks
-            try this.melt() {} catch {
-                uint48 numPeriods = uint48((block.timestamp) - lastPayout) / PERIOD;
-                lastPayout += numPeriods * PERIOD;
-                lastPayoutBal = main.rToken().balanceOf(address(this));
-            }
-        }
         require(ratio_ <= MAX_RATIO, "invalid ratio");
+        melt(); // cannot revert
+
         // The ratio can safely be set to 0, though it is not recommended
         emit RatioSet(ratio, ratio_);
         ratio = ratio_;

diff --git a/contracts/p0/Main.sol b/contracts/p0/Main.sol
@@ -37,8 +37,7 @@ contract MainP0 is Versioned, Initializable, Auth, ComponentRegistry, IMain {
 
     /// @custom:refresher
     function poke() external {
-        assetRegistry.refresh();
-        if (!frozen()) furnace.melt();
+        assetRegistry.refresh(); // runs furnace.melt()
         stRSR.payoutRewards();
         // NOT basketHandler.refreshBasket
     }

diff --git a/contracts/p1/AssetRegistry.sol b/contracts/p1/AssetRegistry.sol
@@ -57,6 +57,8 @@ contract AssetRegistryP1 is ComponentP1, IAssetRegistry {
     //   tracks basket status on basketHandler
     function refresh() public {
         // It's a waste of gas to require notPausedOrFrozen because assets can be updated directly
+        // Assuming an RTokenAsset is registered, furnace.melt() will also be called
+
         uint256 length = _erc20s.length();
         for (uint256 i = 0; i < length; ++i) {
             assets[IERC20(_erc20s.at(i))].refresh();

diff --git a/contracts/p1/BackingManager.sol b/contracts/p1/BackingManager.sol
@@ -113,7 +113,6 @@ contract BackingManagerP1 is TradingP1, IBackingManager {
     function rebalance(TradeKind kind) external nonReentrant notTradingPausedOrFrozen {
         // == Refresh ==
         assetRegistry.refresh();
-        furnace.melt();
 
         // DoS prevention: unless caller is self, require 1 empty block between like-kind auctions
         require(
@@ -184,7 +183,6 @@ contract BackingManagerP1 is TradingP1, IBackingManager {
         require(ArrayLib.allUnique(erc20s), "duplicate tokens");
 
         assetRegistry.refresh();
-        furnace.melt();
 
         BasketRange memory basketsHeld = basketHandler.basketsHeldBy(address(this));
 

diff --git a/contracts/p1/Furnace.sol b/contracts/p1/Furnace.sol
@@ -71,7 +71,7 @@ contract FurnaceP1 is ComponentP1, IFurnace {
     // actions:
     //   rToken.melt(payoutAmount), paying payoutAmount to RToken holders
 
-    function melt() external notFrozen {
+    function melt() public {
         if (uint48(block.timestamp) < uint64(lastPayout) + PERIOD) return;
 
         // # of whole periods that have passed since lastPayout
@@ -90,15 +90,9 @@ contract FurnaceP1 is ComponentP1, IFurnace {
     /// Ratio setting
     /// @custom:governance
     function setRatio(uint192 ratio_) public governance {
-        if (lastPayout > 0) {
-            // solhint-disable-next-line no-empty-blocks
-            try this.melt() {} catch {
-                uint48 numPeriods = uint48((block.timestamp) - lastPayout) / PERIOD;
-                lastPayout += numPeriods * PERIOD;
-                lastPayoutBal = rToken.balanceOf(address(this));
-            }
-        }
         require(ratio_ <= MAX_RATIO, "invalid ratio");
+        melt(); // cannot revert
+
         // The ratio can safely be set to 0 to turn off payouts, though it is not recommended
         emit RatioSet(ratio, ratio_);
         ratio = ratio_;

diff --git a/contracts/p1/Main.sol b/contracts/p1/Main.sol
@@ -43,10 +43,9 @@ contract MainP1 is Versioned, Initializable, Auth, ComponentRegistry, UUPSUpgrad
     /// @dev Not intended to be used in production, only for equivalence with P0
     function poke() external {
         // == Refresher ==
-        assetRegistry.refresh();
+        assetRegistry.refresh(); // runs furnace.melt()
 
         // == CE block ==
-        if (!frozen()) furnace.melt();
         stRSR.payoutRewards();
     }
 

diff --git a/contracts/p1/RToken.sol b/contracts/p1/RToken.sol
@@ -108,7 +108,6 @@ contract RTokenP1 is ComponentP1, ERC20PermitUpgradeable, IRToken {
         // == Refresh ==
 
         assetRegistry.refresh();
-        furnace.melt();
 
         // == Checks-effects block ==
 
@@ -182,8 +181,6 @@ contract RTokenP1 is ComponentP1, ERC20PermitUpgradeable, IRToken {
     function redeemTo(address recipient, uint256 amount) public notFrozen {
         // == Refresh ==
         assetRegistry.refresh();
-        // solhint-disable-next-line no-empty-blocks
-        try furnace.melt() {} catch {} // nice for the redeemer, but not necessary
 
         // == Checks and Effects ==
 
@@ -254,8 +251,6 @@ contract RTokenP1 is ComponentP1, ERC20PermitUpgradeable, IRToken {
     ) external notFrozen {
         // == Refresh ==
         assetRegistry.refresh();
-        // solhint-disable-next-line no-empty-blocks
-        try furnace.melt() {} catch {} // nice for the redeemer, but not necessary
 
         // == Checks and Effects ==
 

diff --git a/contracts/p1/RevenueTrader.sol b/contracts/p1/RevenueTrader.sol
@@ -134,10 +134,8 @@ contract RevenueTraderP1 is TradingP1, IRevenueTrader {
         IAsset assetToBuy = assetRegistry.toAsset(tokenToBuy);
 
         // Refresh everything if RToken is involved
-        if (involvesRToken) {
-            assetRegistry.refresh();
-            furnace.melt();
-        } else {
+        if (involvesRToken) assetRegistry.refresh();
+        else {
             // Otherwise: refresh just the needed assets and nothing more
             for (uint256 i = 0; i < len; ++i) {
                 assetRegistry.toAsset(erc20s[i]).refresh();

diff --git a/contracts/plugins/assets/RTokenAsset.sol b/contracts/plugins/assets/RTokenAsset.sol
@@ -22,6 +22,7 @@ contract RTokenAsset is IAsset, VersionedAsset, IRTokenOracle {
     IBasketHandler public immutable basketHandler;
     IAssetRegistry public immutable assetRegistry;
     IBackingManager public immutable backingManager;
+    IFurnace public immutable furnace;
 
     IERC20Metadata public immutable erc20;
 
@@ -41,6 +42,7 @@ contract RTokenAsset is IAsset, VersionedAsset, IRTokenOracle {
         basketHandler = main.basketHandler();
         assetRegistry = main.assetRegistry();
         backingManager = main.backingManager();
+        furnace = main.furnace();
 
         erc20 = IERC20Metadata(address(erc20_));
         erc20Decimals = erc20_.decimals();
@@ -81,6 +83,9 @@ contract RTokenAsset is IAsset, VersionedAsset, IRTokenOracle {
     function refresh() public virtual override {
         // No need to save lastPrice; can piggyback off the backing collateral's saved prices
 
+        if (msg.sender != address(assetRegistry)) assetRegistry.refresh();
+        furnace.melt();
+
         cachedOracleData.cachedAtTime = 0; // force oracle refresh
     }
 

diff --git a/docs/pause-freeze-states.md b/docs/pause-freeze-states.md
@@ -0,0 +1,73 @@
+# Pause Freeze States
+
+Some protocol functions may be halted while the protocol is either (i) issuance-paused; (ii) trading-paused; or (iii) frozen. Below is a table that shows which protocol interactions (`@custom:interaction`) and refreshers (`@custom:refresher`) execute during paused/frozen states, as of the 3.1.0 release.
+
+All governance functions (`@custom:governance`) remain enabled during all paused/frozen states. They are not mentioned here.
+
+A :heavy_check_mark: indicates the function still executes in this state.
+A :x: indicates it reverts.
+
+| Function                                | Issuance-Paused    | Trading-Paused          | Frozen                  |
+| --------------------------------------- | ------------------ | ----------------------- | ----------------------- |
+| `BackingManager.claimRewards()`         | :heavy_check_mark: | :x:                     | :x:                     |
+| `BackingManager.claimRewardsSingle()`   | :heavy_check_mark: | :x:                     | :x:                     |
+| `BackingManager.grantRTokenAllowance()` | :heavy_check_mark: | :heavy_check_mark:      | :x:                     |
+| `BackingManager.forwardRevenue()`       | :heavy_check_mark: | :x:                     | :x:                     |
+| `BackingManager.rebalance()`            | :heavy_check_mark: | :x:                     | :x:                     |
+| `BackingManager.settleTrade()`          | :heavy_check_mark: | :heavy_check_mark:      | :heavy_check_mark:      |
+| `BasketHandler.refreshBasket()`         | :heavy_check_mark: | :x: (unless governance) | :x: (unless governance) |
+| `Broker.openTrade()`                    | :heavy_check_mark: | :heavy_check_mark:      | :heavy_check_mark:      |
+| `Broker.reportViolation()`              | :heavy_check_mark: | :x:                     | :x:                     |
+| `Distributor.distribute()`              | :heavy_check_mark: | :heavy_check_mark:      | :heavy_check_mark:      |
+| `Furnace.melt()`                        | :heavy_check_mark: | :heavy_check_mark:      | :heavy_check_mark:      |
+| `Main.poke()`                           | :heavy_check_mark: | :heavy_check_mark:      | :heavy_check_mark:      |
+| `RevenueTrader.claimRewards()`          | :heavy_check_mark: | :x:                     | :x:                     |
+| `RevenueTrader.claimRewardsSingle()`    | :heavy_check_mark: | :x:                     | :x:                     |
+| `RevenueTrader.distributeTokenToBuy()`  | :heavy_check_mark: | :x:                     | :x:                     |
+| `RevenueTrader.manageTokens()`          | :heavy_check_mark: | :x:                     | :x:                     |
+| `RevenueTrader.returnTokens()`          | :heavy_check_mark: | :x:                     | :x:                     |
+| `RevenueTrader.settleTrade()`           | :heavy_check_mark: | :heavy_check_mark:      | :heavy_check_mark:      |
+| `RToken.issue()`                        | :x:                | :heavy_check_mark:      | :x:                     |
+| `RToken.issueTo()`                      | :x:                | :heavy_check_mark:      | :x:                     |
+| `RToken.redeem()`                       | :heavy_check_mark: | :heavy_check_mark:      | :x:                     |
+| `RToken.redeemTo()`                     | :heavy_check_mark: | :heavy_check_mark:      | :x:                     |
+| `RToken.redeemCustom()`                 | :heavy_check_mark: | :heavy_check_mark:      | :x:                     |
+| `StRSR.cancelUnstake()`                 | :heavy_check_mark: | :heavy_check_mark:      | :x:                     |
+| `StRSR.payoutRewards()`                 | :heavy_check_mark: | :heavy_check_mark:      | :heavy_check_mark:      |
+| `StRSR.stake()`                         | :heavy_check_mark: | :heavy_check_mark:      | :heavy_check_mark:      |
+| `StRSR.seizeRSR()`                      | :heavy_check_mark: | :x:                     | :x:                     |
+| `StRSR.unstake()`                       | :heavy_check_mark: | :x:                     | :x:                     |
+| `StRSR.withdraw()`                      | :heavy_check_mark: | :x:                     | :x:                     |
+
+## Issuance-pause
+
+The issuance-paused states indicates that RToken issuance should be paused, and _only_ that. It is a narrow control knob that is designed solely to protect against a case where bad debt is being injected into the protocol, say, because default detection for an asset has a false negative.
+
+## Trading-pause
+
+The trading-paused state has significantly more scope than the issuance-paused state. It is designed to prevent against cases where the protocol may trade unneccesarily. Many other functions in addition to just `BackingManager.rebalance()` and `RevenueTrader.manageTokens()` are halted. In general anything that manages the backing and revenue for an RToken is halted. This may become neccessary to use due to (among other things):
+
+- An asset's `price()` malfunctions or is manipulated
+- A collateral's default detection has a false positive or negative
+
+## Freezing
+
+The scope of freezing is the largest, and it should be used least frequently. Nearly all protocol interactions (`@custom:interaction`) are halted. Any refreshers (`@custom:refresher`) remain enabled, as well as `StRSR.stake()` and the "wrap up" routine `*.settleTrade()`.
+
+An important function of freezing is to provide a finite time for governance to push through a repair proposal an RToken in the event that a 0-day is discovered that requires a contract upgrade.
+
+### `Furnace.melt()`
+
+It is necessary for `Furnace.melt()` to remain emabled in order to allow `RTokenAsset.refresh()` to update its `price()`. Any revenue RToken that has already accumulated at the Furnace will continue to be melted, but the flow of new revenue RToken into the contract is halted.
+
+### `StRSR.payoutRewards()`
+
+It is necessary for `StRSR.payoutRewards()` to remain enabled in order for `StRSR.stake()` to use the up-to-date StRSR-RSR exchange rate. If it did not, then in the event of freezing there would be an unfair benefit to new stakers. Any revenue RSR that has already accumulated at the StRSR contract will continue to be paid out, but the flow of new revenue RSR into the contract is halted.
+
+### `StRSR.stake()`
+
+It is important for `StRSR.stake()` to remain emabled while frozen in order to allow honest RSR to flow into an RToken to vote against malicious governance proposals.
+
+### `*.settleTrade()`
+
+The settleTrade functionality must remain enabled in order to maintain the property that dutch auctions will discover the optimal price. If settleTrade were halted, it could become possible for a dutch auction to clear at a much lower price than it should have, simply because bidding was disabled during the earlier portion of the auction.