Merge branch '3.1.0' into feat-rewardaccounting

contracts/p0/Broker.sol

@@ -92,7 +92,7 @@ contract BrokerP0 is ComponentP0, IBroker {
 
     /// Disable the broker until re-enabled by governance
     /// @custom:protected
-    function reportViolation() external notTradingPausedOrFrozen {
+    function reportViolation() external {
         require(trades[_msgSender()], "unrecognized trade contract");
         ITrade trade = ITrade(_msgSender());
         TradeKind kind = trade.KIND();

contracts/p1/Broker.sol

@@ -136,9 +136,9 @@ contract BrokerP1 is ComponentP1, IBroker {
 
     /// Disable the broker until re-enabled by governance
     /// @custom:protected
-    // checks: not paused (trading), not frozen, caller is a Trade this contract cloned
+    // checks: caller is a Trade this contract cloned
     // effects: disabled' = true
-    function reportViolation() external notTradingPausedOrFrozen {
+    function reportViolation() external {
         require(trades[_msgSender()], "unrecognized trade contract");
         ITrade trade = ITrade(_msgSender());
         TradeKind kind = trade.KIND();

contracts/plugins/assets/RTokenAsset.sol

@@ -19,12 +19,14 @@ contract RTokenAsset is IAsset, VersionedAsset, IRTokenOracle {
 
     // Component addresses are not mutable in protocol, so it's safe to cache these
     IMain public immutable main;
-    IBasketHandler public immutable basketHandler;
     IAssetRegistry public immutable assetRegistry;
     IBackingManager public immutable backingManager;
+    IBasketHandler public immutable basketHandler;
     IFurnace public immutable furnace;
+    IERC20 public immutable rsr;
+    IStRSR public immutable stRSR;
 
-    IERC20Metadata public immutable erc20;
+    IERC20Metadata public immutable erc20; // The RToken
 
     uint8 public immutable erc20Decimals;
 
@@ -39,10 +41,12 @@ contract RTokenAsset is IAsset, VersionedAsset, IRTokenOracle {
         require(maxTradeVolume_ > 0, "invalid max trade volume");
 
         main = erc20_.main();
-        basketHandler = main.basketHandler();
         assetRegistry = main.assetRegistry();
         backingManager = main.backingManager();
+        basketHandler = main.basketHandler();
         furnace = main.furnace();
+        rsr = main.rsr();
+        stRSR = main.stRSR();
 
         erc20 = IERC20Metadata(address(erc20_));
         erc20Decimals = erc20_.decimals();
@@ -79,18 +83,15 @@ contract RTokenAsset is IAsset, VersionedAsset, IRTokenOracle {
         assert(low <= high); // not obviously true
     }
 
-    // solhint-disable no-empty-blocks
     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();
+        if (msg.sender != address(assetRegistry)) assetRegistry.refresh();
 
         cachedOracleData.cachedAtTime = 0; // force oracle refresh
     }
 
-    // solhint-enable no-empty-blocks
-
     /// Should not revert
     /// @dev See `tryPrice` caveat about possible compounding error in calculating price
     /// @return {UoA/tok} The lower end of the price estimate
@@ -130,10 +131,15 @@ contract RTokenAsset is IAsset, VersionedAsset, IRTokenOracle {
 
     // solhint-enable no-empty-blocks
 
+    /// Force an update to the cache, including refreshing underlying assets
+    /// @dev Can revert if RToken is unpriced
     function forceUpdatePrice() external {
         _updateCachedPrice();
     }
 
+    /// @dev Can revert if RToken is unpriced
+    /// @return rTokenPrice {UoA/tok} The mean price estimate
+    /// @return updatedAt {s} The timestamp of the cache update
     function latestPrice() external returns (uint192 rTokenPrice, uint256 updatedAt) {
         // Situations that require an update, from most common to least common.
         if (
@@ -145,15 +151,17 @@ contract RTokenAsset is IAsset, VersionedAsset, IRTokenOracle {
             _updateCachedPrice();
         }
 
-        return (cachedOracleData.cachedPrice, cachedOracleData.cachedAtTime);
+        rTokenPrice = cachedOracleData.cachedPrice;
+        updatedAt = cachedOracleData.cachedAtTime;
     }
 
     // ==== Private ====
 
     // Update Oracle Data
     function _updateCachedPrice() internal {
-        (uint192 low, uint192 high) = price();
+        assetRegistry.refresh(); // will call furnace.melt()
 
+        (uint192 low, uint192 high) = price();
         require(low != 0 && high != FIX_MAX, "invalid price");
 
         cachedOracleData = CachedOracleData(
@@ -183,12 +191,12 @@ contract RTokenAsset is IAsset, VersionedAsset, IRTokenOracle {
             TradingContext memory ctx;
 
             ctx.basketsHeld = basketsHeld;
+            ctx.ar = assetRegistry;
             ctx.bm = backingManager;
             ctx.bh = basketHandler;
-            ctx.ar = assetRegistry;
-            ctx.stRSR = main.stRSR();
-            ctx.rsr = main.rsr();
-            ctx.rToken = main.rToken();
+            ctx.rsr = rsr;
+            ctx.rToken = IRToken(address(erc20));
+            ctx.stRSR = stRSR;
             ctx.minTradeVolume = backingManager.minTradeVolume();
             ctx.maxTradeSlippage = backingManager.maxTradeSlippage();
 

contracts/plugins/assets/curve/CurveStableRTokenMetapoolCollateral.sol

@@ -42,6 +42,11 @@ contract CurveStableRTokenMetapoolCollateral is CurveStableMetapoolCollateral {
         pairedAssetRegistry = IRToken(address(pairedToken)).main().assetRegistry();
     }
 
+    function refresh() public override {
+        pairedAssetRegistry.refresh(); // refresh all registered assets
+        super.refresh(); // already handles all necessary default checks
+    }
+
     /// Can revert, used by `_anyDepeggedOutsidePool()`
     /// Should not return FIX_MAX for low
     /// @return lowPaired {UoA/pairedTok} The low price estimate of the paired token

contracts/plugins/mocks/AssetMock.sol

@@ -4,6 +4,8 @@ pragma solidity 0.8.19;
 import "../assets/Asset.sol";
 
 contract AssetMock is Asset {
+    bool public stale;
+
     uint192 private lowPrice;
     uint192 private highPrice;
 
@@ -40,13 +42,18 @@ contract AssetMock is Asset {
             uint192
         )
     {
+        require(!stale, "stale price");
         return (lowPrice, highPrice, 0);
     }
 
     /// Should not revert
     /// Refresh saved prices
     function refresh() public virtual override {
-        // pass
+        stale = false;
+    }
+
+    function setStale(bool _stale) external {
+        stale = _stale;
     }
 
     function setPrice(uint192 low, uint192 high) external {

docs/pause-freeze-states.md

@@ -17,7 +17,7 @@ A :x: indicates it reverts.
 | `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:                     |
+| `Broker.reportViolation()`              | :heavy_check_mark: | :heavy_check_mark:      | :heavy_check_mark:      |
 | `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:      |

test/Broker.test.ts

@@ -574,28 +574,6 @@ describe(`BrokerP${IMPLEMENTATION} contract #fast`, () => {
       // Check nothing changed
       expect(await broker.batchTradeDisabled()).to.equal(false)
     })
-
-    it('Should not allow to report violation if paused or frozen', async () => {
-      // Check not disabled
-      expect(await broker.batchTradeDisabled()).to.equal(false)
-
-      await main.connect(owner).pauseTrading()
-
-      await expect(broker.connect(addr1).reportViolation()).to.be.revertedWith(
-        'frozen or trading paused'
-      )
-
-      await main.connect(owner).unpauseTrading()
-
-      await main.connect(owner).freezeShort()
-
-      await expect(broker.connect(addr1).reportViolation()).to.be.revertedWith(
-        'frozen or trading paused'
-      )
-
-      // Check nothing changed
-      expect(await broker.batchTradeDisabled()).to.equal(false)
-    })
   })
 
   describe('Trades', () => {

test/Revenues.test.ts

@@ -2667,11 +2667,133 @@ describe(`Revenues - P${IMPLEMENTATION}`, () => {
           },
         ])
 
+        // Check broker disabled (batch)
+        expect(await broker.batchTradeDisabled()).to.equal(true)
+
         // Check funds at destinations
         expect(await rsr.balanceOf(stRSR.address)).to.be.closeTo(minBuyAmt.sub(10), 50)
         expect(await rToken.balanceOf(furnace.address)).to.be.closeTo(minBuyAmtRToken.sub(10), 50)
       })
 
+      it('Should report violation even if paused or frozen', async () => {
+        // This test needs to be in this file and not Broker.test.ts because settleTrade()
+        // requires the BackingManager _actually_ started the trade
+
+        rewardAmountAAVE = bn('0.5e18')
+
+        // AAVE Rewards
+        await token2.setRewards(backingManager.address, rewardAmountAAVE)
+
+        // Collect revenue
+        // Expected values based on Prices between AAVE and RSR/RToken = 1 to 1 (for simplification)
+        const sellAmt: BigNumber = rewardAmountAAVE.mul(60).div(100) // due to f = 60%
+        const minBuyAmt: BigNumber = await toMinBuyAmt(sellAmt, fp('1'), fp('1'))
+
+        const sellAmtRToken: BigNumber = rewardAmountAAVE.sub(sellAmt) // Remainder
+        const minBuyAmtRToken: BigNumber = await toMinBuyAmt(sellAmtRToken, fp('1'), fp('1'))
+
+        // Claim rewards
+        await facadeTest.claimRewards(rToken.address)
+
+        // Check status of destinations at this point
+        expect(await rsr.balanceOf(stRSR.address)).to.equal(0)
+        expect(await rToken.balanceOf(furnace.address)).to.equal(0)
+
+        // Run auctions
+        await expectEvents(facadeTest.runAuctionsForAllTraders(rToken.address), [
+          {
+            contract: rsrTrader,
+            name: 'TradeStarted',
+            args: [anyValue, aaveToken.address, rsr.address, sellAmt, withinQuad(minBuyAmt)],
+            emitted: true,
+          },
+          {
+            contract: rTokenTrader,
+            name: 'TradeStarted',
+            args: [
+              anyValue,
+              aaveToken.address,
+              rToken.address,
+              sellAmtRToken,
+              withinQuad(minBuyAmtRToken),
+            ],
+            emitted: true,
+          },
+        ])
+
+        // Advance time till auction ended
+        await advanceTime(config.batchAuctionLength.add(100).toString())
+
+        // Perform Mock Bids for RSR and RToken (addr1 has balance)
+        // In order to force deactivation we provide an amount below minBuyAmt, this will represent for our tests an invalid behavior although in a real scenario would retrigger auction
+        // NOTE: DIFFERENT BEHAVIOR WILL BE OBSERVED ON PRODUCTION GNOSIS AUCTIONS
+        await rsr.connect(addr1).approve(gnosis.address, minBuyAmt)
+        await rToken.connect(addr1).approve(gnosis.address, minBuyAmtRToken)
+        await gnosis.placeBid(0, {
+          bidder: addr1.address,
+          sellAmount: sellAmt,
+          buyAmount: minBuyAmt.sub(10), // Forces in our mock an invalid behavior
+        })
+        await gnosis.placeBid(1, {
+          bidder: addr1.address,
+          sellAmount: sellAmtRToken,
+          buyAmount: minBuyAmtRToken.sub(10), // Forces in our mock an invalid behavior
+        })
+
+        // Freeze protocol
+        await main.connect(owner).freezeShort()
+
+        // Close auctions - Will end trades and also report violation
+        await expectEvents(facadeTest.runAuctionsForAllTraders(rToken.address), [
+          {
+            contract: broker,
+            name: 'BatchTradeDisabledSet',
+            args: [false, true],
+            emitted: true,
+          },
+          {
+            contract: rsrTrader,
+            name: 'TradeSettled',
+            args: [anyValue, aaveToken.address, rsr.address, sellAmt, minBuyAmt.sub(10)],
+            emitted: true,
+          },
+          {
+            contract: rTokenTrader,
+            name: 'TradeSettled',
+            args: [
+              anyValue,
+              aaveToken.address,
+              rToken.address,
+              sellAmtRToken,
+              minBuyAmtRToken.sub(10),
+            ],
+            emitted: true,
+          },
+          {
+            contract: rsrTrader,
+            name: 'TradeStarted',
+            emitted: false,
+          },
+          {
+            contract: rTokenTrader,
+            name: 'TradeStarted',
+            emitted: false,
+          },
+        ])
+
+        // Check broker disabled (batch)
+        expect(await broker.batchTradeDisabled()).to.equal(true)
+
+        // Funds are not distributed if paused or frozen
+        expect(await rsr.balanceOf(stRSR.address)).to.equal(0)
+        expect(await rsr.balanceOf(rsrTrader.address)).to.be.closeTo(minBuyAmt.sub(10), 50)
+        expect(await rToken.balanceOf(furnace.address)).to.equal(0)
+        expect(await rToken.balanceOf(rTokenTrader.address)).to.be.closeTo(
+          minBuyAmtRToken.sub(10),
+          50
+        )
+      })
+
       it('Should not report violation when Dutch Auction clears in geometric phase', async () => {
         // This test needs to be in this file and not Broker.test.ts because settleTrade()
         // requires the BackingManager _actually_ started the trade

test/plugins/individual-collateral/curve/crv/CrvStableRTokenMetapoolTestSuite.test.ts

@@ -241,6 +241,41 @@ const collateralSpecificStatusTests = () => {
     // refresh() should not revert
     await collateral.refresh()
   })
+
+  it('Regression test -- refreshes inner RTokenAsset on refresh()', async () => {
+    const [collateral] = await deployCollateral({})
+    const initialPrice = await collateral.price()
+    expect(initialPrice[0]).to.be.gt(0)
+    expect(initialPrice[1]).to.be.lt(MAX_UINT192)
+
+    // Swap out eUSD's RTokenAsset with a mock one
+    const AssetMockFactory = await ethers.getContractFactory('AssetMock')
+    const mockRTokenAsset = await AssetMockFactory.deploy(
+      bn('1'), // unused
+      ONE_ADDRESS, // unused
+      bn('1'), // unused
+      eUSD,
+      bn('1'), // unused
+      bn('1') // unused
+    )
+    const eUSDAssetRegistry = await ethers.getContractAt(
+      'IAssetRegistry',
+      '0x9B85aC04A09c8C813c37de9B3d563C2D3F936162'
+    )
+    await whileImpersonating('0xc8Ee187A5e5c9dC9b42414Ddf861FFc615446a2c', async (signer) => {
+      await eUSDAssetRegistry.connect(signer).swapRegistered(mockRTokenAsset.address)
+    })
+
+    // Set RTokenAsset price to stale
+    await mockRTokenAsset.setStale(true)
+    expect(await mockRTokenAsset.stale()).to.be.true
+
+    // Refresh CurveStableRTokenMetapoolCollateral
+    await collateral.refresh()
+
+    // Stale should be false again
+    expect(await mockRTokenAsset.stale()).to.be.false
+  })
 }
 
 /*