From 82aac04333cea6b9261bbc73307483bc800f057c Mon Sep 17 00:00:00 2001 From: Martin Date: Sat, 2 Nov 2019 00:56:44 +0100 Subject: [PATCH] Allow a specified address to disable/enable the Exchange (#1467) --- .../protocol/contracts/baklava/Freezable.sol | 43 +++++++++ .../contracts/baklava/test/FreezableTest.sol | 20 ++++ .../protocol/contracts/stability/Exchange.sol | 11 ++- .../stability/interfaces/IExchange.sol | 1 + packages/protocol/migrations/09_exchange.ts | 5 +- packages/protocol/test/baklava/freezable.ts | 91 +++++++++++++++++++ packages/protocol/test/stability/exchange.ts | 13 +++ 7 files changed, 182 insertions(+), 2 deletions(-) create mode 100644 packages/protocol/contracts/baklava/Freezable.sol create mode 100644 packages/protocol/contracts/baklava/test/FreezableTest.sol create mode 100644 packages/protocol/test/baklava/freezable.ts diff --git a/packages/protocol/contracts/baklava/Freezable.sol b/packages/protocol/contracts/baklava/Freezable.sol new file mode 100644 index 00000000000..2d0a57a5c87 --- /dev/null +++ b/packages/protocol/contracts/baklava/Freezable.sol @@ -0,0 +1,43 @@ +pragma solidity ^0.5.3; + + +contract Freezable { + bool public frozen; + address public freezer; + + // onlyFreezer functions can only be called by the specified `freezer` address + modifier onlyFreezer() { + require(msg.sender == freezer); + _; + } + + // onlyWhenNotFrozen functions can only be called when `frozen` is false, otherwise they will + // revert. + modifier onlyWhenNotFrozen() { + require(!frozen); + _; + } + + /** + * @notice Sets the address that is allowed to freeze/unfreeze the contract. + * @param _freezer The address that is allowed to freeze/unfree the contract + * @dev This function is `internal` and leaves its permissioning up to the inheriting contract. + */ + function _setFreezer(address _freezer) internal { + freezer = _freezer; + } + + /** + * @notice Freezes the contract, disabling `onlyWhenNotFrozen` functions. + */ + function freeze() external onlyFreezer { + frozen = true; + } + + /** + * @notice Unreezes the contract, enabling `onlyWhenNotFrozen` functions. + */ + function unfreeze() external onlyFreezer { + frozen = false; + } +} diff --git a/packages/protocol/contracts/baklava/test/FreezableTest.sol b/packages/protocol/contracts/baklava/test/FreezableTest.sol new file mode 100644 index 00000000000..23c5baf55fa --- /dev/null +++ b/packages/protocol/contracts/baklava/test/FreezableTest.sol @@ -0,0 +1,20 @@ +pragma solidity ^0.5.8; + +import "../Freezable.sol"; + + +contract FreezableTest is Freezable { + event FunctionCalled(); + + function setFreezer(address _freezer) external { + _setFreezer(_freezer); + } + + function freezableFunction() external onlyWhenNotFrozen { + emit FunctionCalled(); + } + + function nonfreezableFunction() external { + emit FunctionCalled(); + } +} diff --git a/packages/protocol/contracts/stability/Exchange.sol b/packages/protocol/contracts/stability/Exchange.sol index 5e0e457422e..0291998717e 100644 --- a/packages/protocol/contracts/stability/Exchange.sol +++ b/packages/protocol/contracts/stability/Exchange.sol @@ -10,6 +10,7 @@ import "./interfaces/IStableToken.sol"; import "../common/FractionUtil.sol"; import "../common/Initializable.sol"; import "../common/FixidityLib.sol"; +import "../baklava/Freezable.sol"; import "../common/UsingRegistry.sol"; @@ -17,7 +18,7 @@ import "../common/UsingRegistry.sol"; * @title Contract that allows to exchange StableToken for GoldToken and vice versa * using a Constant Product Market Maker Model */ -contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, ReentrancyGuard { +contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, ReentrancyGuard, Freezable { using SafeMath for uint256; using FractionUtil for FractionUtil.Fraction; using FixidityLib for FixidityLib.Fraction; @@ -89,6 +90,7 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc */ function initialize( address registryAddress, + address _freezer, address stableToken, uint256 _spread, uint256 _reserveFraction, @@ -99,6 +101,7 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc initializer { _transferOwnership(msg.sender); + setFreezer(_freezer); setRegistry(registryAddress); setStableToken(stableToken); setSpread(_spread); @@ -116,6 +119,7 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc * transaction to succeed * @param sellGold `true` if gold is the sell token * @return The amount of buyToken that was transfered + * @dev This function can be frozen using the Freezable interface. */ function exchange( uint256 sellAmount, @@ -123,6 +127,7 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc bool sellGold ) external + onlyWhenNotFrozen updateBucketsIfNecessary nonReentrant returns (uint256) @@ -254,6 +259,10 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc emit MinimumReportsSet(newMininumReports); } + function setFreezer(address freezer) public onlyOwner { + _setFreezer(freezer); + } + /** * @notice Allows owner to set the Stable Token address * @param newStableToken The new address for Stable Token diff --git a/packages/protocol/contracts/stability/interfaces/IExchange.sol b/packages/protocol/contracts/stability/interfaces/IExchange.sol index 8be29102eb3..b70d8250ecc 100644 --- a/packages/protocol/contracts/stability/interfaces/IExchange.sol +++ b/packages/protocol/contracts/stability/interfaces/IExchange.sol @@ -4,6 +4,7 @@ pragma solidity ^0.5.3; interface IExchange { function initialize( + address, address, address, uint256, diff --git a/packages/protocol/migrations/09_exchange.ts b/packages/protocol/migrations/09_exchange.ts index 07d78d5a295..b0e1f4b97ef 100644 --- a/packages/protocol/migrations/09_exchange.ts +++ b/packages/protocol/migrations/09_exchange.ts @@ -8,14 +8,17 @@ import { import { config } from '@celo/protocol/migrationsConfig' import { toFixed } from '@celo/utils/lib/fixidity' import { ExchangeInstance, ReserveInstance, StableTokenInstance } from 'types' +const truffle = require('@celo/protocol/truffle-config.js') -const initializeArgs = async (): Promise => { +const initializeArgs = async (networkName: string): Promise => { + const network: any = truffle.networks[networkName] const stableToken: StableTokenInstance = await getDeployedProxiedContract( 'StableToken', artifacts ) return [ config.registry.predeployedProxyAddress, + network.from, stableToken.address, toFixed(config.exchange.spread).toString(), toFixed(config.exchange.reserveFraction).toString(), diff --git a/packages/protocol/test/baklava/freezable.ts b/packages/protocol/test/baklava/freezable.ts new file mode 100644 index 00000000000..1c7431a80f6 --- /dev/null +++ b/packages/protocol/test/baklava/freezable.ts @@ -0,0 +1,91 @@ +import { assertSameAddress, assertLogMatches2, assertRevert } from '@celo/protocol/lib/test-utils' +import { FreezableTestInstance } from 'types' + +contract('Freezable', (accounts: string[]) => { + const FreezableTest = artifacts.require('FreezableTest') + let freezableTest: FreezableTestInstance + + beforeEach(async () => { + freezableTest = await FreezableTest.new() + freezableTest.setFreezer(accounts[0]) + }) + + describe('_setFreezer', () => { + it('should allow owner to change the freezer', async () => { + // _setFreezer is internal in Freezable, FreezableTest wraps around it with setFreezer + await freezableTest.setFreezer(accounts[1]) + const freezer = await freezableTest.freezer() + assertSameAddress(freezer, accounts[1]) + }) + }) + + describe('when the contract is not frozen', () => { + it('should allow freezable functions to be called', async () => { + const resp = await freezableTest.freezableFunction() + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertLogMatches2(log, { + event: 'FunctionCalled', + args: {}, + }) + }) + + it('should allow non-freezable functions to be called', async () => { + const resp = await freezableTest.nonfreezableFunction() + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertLogMatches2(log, { + event: 'FunctionCalled', + args: {}, + }) + }) + }) + + describe('freeze', () => { + it('should allow freezer to freeze the contract', async () => { + await freezableTest.freeze() + const frozen = await freezableTest.frozen() + assert.isTrue(frozen) + }) + + it('should not allow a non-freezer to freeze the contract', async () => { + await assertRevert(freezableTest.freeze({ from: accounts[1] })) + }) + }) + + describe('unfreeze', () => { + beforeEach(async () => { + freezableTest.freeze() + }) + + it('should allow freezer to unfreeze the contract', async () => { + await freezableTest.unfreeze() + const frozen = await freezableTest.frozen() + assert.isFalse(frozen) + }) + + it('should not allow a non-freezer to unfreeze the contract', async () => { + await assertRevert(freezableTest.freeze({ from: accounts[1] })) + }) + }) + + describe('when the contract is frozen', () => { + beforeEach(async () => { + await freezableTest.freeze() + }) + + it('should revert a freezable function', async () => { + await assertRevert(freezableTest.freezableFunction()) + }) + + it('should not affect a non-freezable function', async () => { + const resp = await freezableTest.nonfreezableFunction() + assert.equal(resp.logs.length, 1) + const log = resp.logs[0] + assertLogMatches2(log, { + event: 'FunctionCalled', + args: {}, + }) + }) + }) +}) diff --git a/packages/protocol/test/stability/exchange.ts b/packages/protocol/test/stability/exchange.ts index 36be55415bc..b9f2e81b1a9 100644 --- a/packages/protocol/test/stability/exchange.ts +++ b/packages/protocol/test/stability/exchange.ts @@ -122,6 +122,7 @@ contract('Exchange', (accounts: string[]) => { exchange = await Exchange.new() await exchange.initialize( registry.address, + accounts[0], stableToken.address, spread, reserveFraction, @@ -141,6 +142,7 @@ contract('Exchange', (accounts: string[]) => { await assertRevert( exchange.initialize( registry.address, + accounts[0], stableToken.address, spread, reserveFraction, @@ -794,5 +796,16 @@ contract('Exchange', (accounts: string[]) => { }) }) }) + + describe('when the contract is frozen', () => { + beforeEach(async () => { + await exchange.freeze() + }) + + it('should revert', async () => { + await goldToken.approve(exchange.address, 1000) + await assertRevert(exchange.exchange(1000, 1, true)) + }) + }) }) })