Fixed Exchanges.sol reverting on deployment due to expecting sortedOracles report on Empty Address

packages/protocol/contracts/stability/Exchange.sol

@@ -54,6 +54,8 @@ contract Exchange is
   uint256 public updateFrequency;
   uint256 public minimumReports;
 
+  bytes32 public stableTokenRegistryId;
+
   modifier updateBucketsIfNecessary() {
     _updateBucketsIfNecessary();
     _;
@@ -64,7 +66,7 @@ contract Exchange is
    * @return The storage, major, minor, and patch version of the contract.
    */
   function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) {
-    return (1, 1, 1, 0);
+    return (1, 2, 0, 0);
   }
 
   /**
@@ -76,30 +78,39 @@ contract Exchange is
   /**
    * @notice Used in place of the constructor to allow the contract to be upgradable via proxy.
    * @param registryAddress The address of the registry core smart contract.
-   * @param stableToken Address of the stable token
    * @param _spread Spread charged on exchanges
    * @param _reserveFraction Fraction to commit to the gold bucket
    * @param _updateFrequency The time period that needs to elapse between bucket
    * updates
    * @param _minimumReports The minimum number of fresh reports that need to be
    * present in the oracle to update buckets
    * commit to the gold bucket
+   * @param stableTokenIdentifier String identifier of stabletoken in registry
    */
   function initialize(
     address registryAddress,
-    address stableToken,
+    string calldata stableTokenIdentifier,
     uint256 _spread,
     uint256 _reserveFraction,
     uint256 _updateFrequency,
     uint256 _minimumReports
   ) external initializer {
     _transferOwnership(msg.sender);
     setRegistry(registryAddress);
-    setStableToken(stableToken);
+    stableTokenRegistryId = keccak256(abi.encodePacked(stableTokenIdentifier));
     setSpread(_spread);
     setReserveFraction(_reserveFraction);
     setUpdateFrequency(_updateFrequency);
     setMinimumReports(_minimumReports);
+  }
+
+  /**
+   * @notice Ensures stable token address is set in storage and initializes buckets.
+   * @dev Will revert if stable token is not registered or does not have oracle reports.
+   */
+  function activateStable() external onlyOwner {
+    require(stable == address(0), "StableToken address already activated");
+    _setStableToken(registry.getAddressForOrDie(stableTokenRegistryId));
     _updateBucketsIfNecessary();
   }
 
@@ -276,8 +287,7 @@ contract Exchange is
     * @param newStableToken The new address for Stable Token
     */
   function setStableToken(address newStableToken) public onlyOwner {
-    stable = newStableToken;
-    emit StableTokenSet(newStableToken);
+    _setStableToken(newStableToken);
   }
 
   /**
@@ -299,6 +309,11 @@ contract Exchange is
     emit ReserveFractionSet(newReserveFraction);
   }
 
+  function _setStableToken(address newStableToken) internal {
+    stable = newStableToken;
+    emit StableTokenSet(newStableToken);
+  }
+
   /**
    * @notice Returns the buy token and sell token bucket sizes, in order. The ratio of
    * the two also represents the exchange rate between the two.

packages/protocol/contracts/stability/ExchangeEUR.sol

@@ -15,6 +15,6 @@ contract ExchangeEUR is Exchange {
   * @return The storage, major, minor, and patch version of the contract.
   */
   function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) {
-    return (1, 1, 0, 0);
+    return (1, 2, 0, 0);
   }
 }

packages/protocol/migrations/10_0_exchange_USD.ts

@@ -7,16 +7,12 @@ import {
 } from '@celo/protocol/lib/web3-utils'
 import { config } from '@celo/protocol/migrationsConfig'
 import { toFixed } from '@celo/utils/lib/fixidity'
-import { ExchangeInstance, FreezerInstance, StableTokenInstance } from 'types'
+import { ExchangeInstance, FreezerInstance } from 'types'
 
 const initializeArgs = async (): Promise<any[]> => {
-  const stableToken: StableTokenInstance = await getDeployedProxiedContract<StableTokenInstance>(
-    'StableToken',
-    artifacts
-  )
   return [
     config.registry.predeployedProxyAddress,
-    stableToken.address,
+    CeloContractName.StableToken,
     toFixed(config.exchange.spread).toString(),
     toFixed(config.exchange.reserveFraction).toString(),
     config.exchange.updateFrequency,
@@ -37,5 +33,6 @@ module.exports = deploymentForCoreContract<ExchangeInstance>(
       )
       await freezer.freeze(exchange.address)
     }
+    await exchange.activateStable()
   }
 )

packages/protocol/migrations/10_1_exchange_EUR.ts

@@ -7,21 +7,12 @@ import {
 } from '@celo/protocol/lib/web3-utils'
 import { config } from '@celo/protocol/migrationsConfig'
 import { toFixed } from '@celo/utils/lib/fixidity'
-import {
-  ExchangeEURInstance,
-  FreezerInstance,
-  ReserveInstance,
-  StableTokenEURInstance,
-} from 'types'
+import { ExchangeEURInstance, FreezerInstance, ReserveInstance } from 'types'
 
 const initializeArgs = async (): Promise<any[]> => {
-  const stableTokenEUR: StableTokenEURInstance = await getDeployedProxiedContract<StableTokenEURInstance>(
-    'StableTokenEUR',
-    artifacts
-  )
   return [
     config.registry.predeployedProxyAddress,
-    stableTokenEUR.address,
+    CeloContractName.StableTokenEUR,
     toFixed(config.exchange.spread).toString(),
     toFixed(config.exchange.reserveFraction).toString(),
     config.exchange.updateFrequency,
@@ -49,5 +40,6 @@ module.exports = deploymentForCoreContract<ExchangeEURInstance>(
     )
     // cUSD doesn't need to be added as it is currently harcoded in Reserve.sol
     await reserve.addExchangeSpender(exchange.address)
+    await exchange.activateStable()
   }
 )

packages/protocol/migrations/10_2_exchange_BRL.ts

@@ -7,21 +7,12 @@ import {
 } from '@celo/protocol/lib/web3-utils'
 import { config } from '@celo/protocol/migrationsConfig'
 import { toFixed } from '@celo/utils/lib/fixidity'
-import {
-  ExchangeBRLInstance,
-  FreezerInstance,
-  ReserveInstance,
-  StableTokenBRLInstance,
-} from 'types'
+import { ExchangeBRLInstance, FreezerInstance, ReserveInstance } from 'types'
 
 const initializeArgs = async (): Promise<any[]> => {
-  const stableTokenBRL: StableTokenBRLInstance = await getDeployedProxiedContract<StableTokenBRLInstance>(
-    'StableTokenBRL',
-    artifacts
-  )
   return [
     config.registry.predeployedProxyAddress,
-    stableTokenBRL.address,
+    CeloContractName.StableTokenBRL,
     toFixed(config.exchange.spread).toString(),
     toFixed(config.exchange.reserveFraction).toString(),
     config.exchange.updateFrequency,
@@ -49,5 +40,6 @@ module.exports = deploymentForCoreContract<ExchangeBRLInstance>(
     )
     // cUSD doesn't need to be added as it is currently harcoded in Reserve.sol
     await reserve.addExchangeSpender(exchange.address)
+    await exchange.activateStable()
   }
 )

packages/protocol/test/common/integration.ts

@@ -627,7 +627,7 @@ contract('Integration: Adding StableToken', (accounts: string[]) => {
       )
       await exchangeAbc.initialize(
         registry.address,
-        stableTokenAbc.address,
+        'StableTokenABC',
         '5000000000000000000000', // spread, matches mainnet for cUSD and cEUR
         '1300000000000000000000', // reserveFraction, matches mainnet for cEUR
         '300', // updateFrequency, matches mainnet for cUSD and cEUR
@@ -638,9 +638,6 @@ contract('Integration: Adding StableToken', (accounts: string[]) => {
     it(`should be impossible to sell CELO`, async () => {
       await goldToken.approve(exchangeAbc.address, sellAmount)
       await assertRevert(exchangeAbc.sell(sellAmount, minBuyAmount, true))
-      // This last case is not relevant, but the test is meant to warn in case the behavior ever changes
-      await goldToken.approve(exchangeAbc.address, sellAmount)
-      await exchangeAbc.sell(sellAmount, 0, true)
     })
 
     it(`should be impossible to sell stable token`, async () => {
@@ -691,6 +688,9 @@ contract('Integration: Adding StableToken', (accounts: string[]) => {
       await reserve.addExchangeSpender(exchangeAbc.address)
       await freezer.unfreeze(stableTokenAbc.address)
       await freezer.unfreeze(exchangeAbc.address)
+
+      // activate stable during mento-activation proposal
+      await exchangeAbc.activateStable()
       // Fee currency can't be tested here, but keep this line for reference
       await feeCurrencyWhitelist.addToken(stableTokenAbc.address)
     })

packages/protocol/test/stability/exchange.ts

@@ -1,3 +1,4 @@
+import { NULL_ADDRESS } from '@celo/base'
 import { CeloContractName } from '@celo/protocol/lib/registry-utils'
 import {
   addressMinedLatestBlock,
@@ -24,6 +25,7 @@ import {
   StableTokenContract,
   StableTokenInstance,
 } from 'types'
+import { keccak256 } from 'web3-utils'
 import { SECONDS_IN_A_WEEK } from '../constants'
 
 const Exchange: ExchangeContract = artifacts.require('Exchange')
@@ -126,7 +128,7 @@ contract('Exchange', (accounts: string[]) => {
       SECONDS_IN_A_WEEK,
       [],
       [],
-      'Exchange' // USD
+      CeloContractName.Exchange // USD
     )
 
     mockSortedOracles = await MockSortedOracles.new()
@@ -140,12 +142,14 @@ contract('Exchange', (accounts: string[]) => {
     exchange = await Exchange.new(true)
     await exchange.initialize(
       registry.address,
-      stableToken.address,
+      CeloContractName.StableToken,
       spread,
       reserveFraction,
       updateFrequency,
       minimumReports
     )
+
+    await registry.setAddressFor(CeloContractName.StableToken, stableToken.address)
     await registry.setAddressFor(CeloContractName.Exchange, exchange.address)
   })
 
@@ -155,11 +159,16 @@ contract('Exchange', (accounts: string[]) => {
       assert.equal(expectedOwner, accounts[0])
     })
 
+    it('should set stable token identfier', async () => {
+      const identifier = await exchange.stableTokenRegistryId()
+      assert.equal(identifier, keccak256(CeloContractName.StableToken))
+    })
+
     it('should not be callable again', async () => {
       await assertRevert(
         exchange.initialize(
           registry.address,
-          stableToken.address,
+          CeloContractName.StableToken,
           spread,
           reserveFraction,
           updateFrequency,
@@ -169,6 +178,36 @@ contract('Exchange', (accounts: string[]) => {
     })
   })
 
+  describe('#activateStable', () => {
+    it('should emit a StableTokenSet event', async () => {
+      const tx = await exchange.activateStable()
+      assertLogMatches2(tx.logs[0], {
+        event: 'StableTokenSet',
+        args: {
+          stable: stableToken.address,
+        },
+      })
+    })
+
+    it('should set the stable storage address', async () => {
+      const stableBefore = await exchange.stable()
+      assert.equal(stableBefore, NULL_ADDRESS)
+      await exchange.activateStable()
+      const stableAfter = await exchange.stable()
+      assert.equal(stableAfter, stableToken.address)
+    })
+
+    it('should not allow a non-owner to activate', async () => {
+      const nonOwner = accounts[1]
+      await assertRevert(exchange.activateStable({ from: nonOwner }))
+    })
+
+    it('should not be callable again', async () => {
+      await exchange.activateStable()
+      await assertRevert(exchange.activateStable())
+    })
+  })
+
   describe('#setUpdateFrequency', () => {
     const newUpdateFrequency = new BigNumber(60 * 30)
 
@@ -318,6 +357,10 @@ contract('Exchange', (accounts: string[]) => {
   })
 
   describe('#getBuyAndSellBuckets', () => {
+    beforeEach(async () => {
+      await exchange.activateStable()
+    })
+
     it('should return the correct amount of buy and sell token', async () => {
       const [buyBucketSize, sellBucketSize] = await exchange.getBuyAndSellBuckets(true)
       assertEqualBN(sellBucketSize, initialGoldBucket)
@@ -355,6 +398,7 @@ contract('Exchange', (accounts: string[]) => {
 
       it(`should return the same value if updateFrequency seconds haven't passed yet`, async () => {
         await mockSortedOracles.setMedianTimestampToNow(stableToken.address)
+
         const [buyBucketSize, sellBucketSize] = await exchange.getBuyAndSellBuckets(true)
 
         assertEqualBN(sellBucketSize, initialGoldBucket)
@@ -374,6 +418,10 @@ contract('Exchange', (accounts: string[]) => {
   })
 
   describe('#getBuyTokenAmount', () => {
+    beforeEach(async () => {
+      await exchange.activateStable()
+    })
+
     it('should return the correct amount of buyToken', async () => {
       const amount = 10
       const buyAmount = await exchange.getBuyTokenAmount(amount, true)
@@ -389,6 +437,10 @@ contract('Exchange', (accounts: string[]) => {
   })
 
   describe('#getSellTokenAmount', () => {
+    beforeEach(async () => {
+      await exchange.activateStable()
+    })
+
     it('should return the correct amount of sellToken', async () => {
       const buyAmount = 10
       const sellAmount = await exchange.getSellTokenAmount(buyAmount, true)
@@ -409,6 +461,10 @@ contract('Exchange', (accounts: string[]) => {
 
   for (const sellFunctionName of sellFunctionNames) {
     describe(`#${sellFunctionName}`, () => {
+      beforeEach(async () => {
+        await exchange.activateStable()
+      })
+
       const user = accounts[1]
 
       // This test is run for both the `sell` and `exchange` functions
@@ -924,6 +980,10 @@ contract('Exchange', (accounts: string[]) => {
   }
 
   describe('#buy', () => {
+    beforeEach(async () => {
+      await exchange.activateStable()
+    })
+
     const user = accounts[1]
 
     describe('when buying stable with gold', () => {