Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow a specified address to disable/enable the Exchange #1467

Merged
merged 16 commits into from
Nov 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions packages/protocol/contracts/baklava/Freezable.sol
Original file line number Diff line number Diff line change
@@ -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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? I would consider cutting this function and renaming onlyWhenNotFrozenOrThrow to onlyWhenNotFrozen

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have cleared this up - this is already looking forward to the next change, where distributeEpochPayments will need to be frozen. By not reverting when that function is frozen, I think we'll be able to contain this temporary change for Baklava to smart contracts.

I could see an argument for reverting being the default semantic, happy to rename this one to onlyWhenNotFrozenNoThrow.

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure we don't want to revert when we freeze epoch rewards for TGCSO? Reverting would prevent any proof-of-stake action from happening in Finalize, e.g. validator score changes, payments, etc.

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;
}
}
20 changes: 20 additions & 0 deletions packages/protocol/contracts/baklava/test/FreezableTest.sol
Original file line number Diff line number Diff line change
@@ -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();
}
}
11 changes: 10 additions & 1 deletion packages/protocol/contracts/stability/Exchange.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@ import "./interfaces/IStableToken.sol";
import "../common/FractionUtil.sol";
import "../common/Initializable.sol";
import "../common/FixidityLib.sol";
import "../baklava/Freezable.sol";
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;
Expand Down Expand Up @@ -89,6 +90,7 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc
*/
function initialize(
address registryAddress,
address _freezer,
address stableToken,
uint256 _spread,
uint256 _reserveFraction,
Expand All @@ -99,6 +101,7 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc
initializer
{
_transferOwnership(msg.sender);
setFreezer(_freezer);
setRegistry(registryAddress);
setStableToken(stableToken);
setSpread(_spread);
Expand All @@ -116,13 +119,15 @@ 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,
uint256 minBuyAmount,
bool sellGold
)
external
onlyWhenNotFrozen
updateBucketsIfNecessary
nonReentrant
returns (uint256)
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity ^0.5.3;
interface IExchange {

function initialize(
address,
address,
address,
uint256,
Expand Down
5 changes: 4 additions & 1 deletion packages/protocol/migrations/09_exchange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<any[]> => {
const initializeArgs = async (networkName: string): Promise<any[]> => {
const network: any = truffle.networks[networkName]
const stableToken: StableTokenInstance = await getDeployedProxiedContract<StableTokenInstance>(
'StableToken',
artifacts
)
return [
config.registry.predeployedProxyAddress,
network.from,
stableToken.address,
toFixed(config.exchange.spread).toString(),
toFixed(config.exchange.reserveFraction).toString(),
Expand Down
91 changes: 91 additions & 0 deletions packages/protocol/test/baklava/freezable.ts
Original file line number Diff line number Diff line change
@@ -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: {},
})
})
})
})
13 changes: 13 additions & 0 deletions packages/protocol/test/stability/exchange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ contract('Exchange', (accounts: string[]) => {
exchange = await Exchange.new()
await exchange.initialize(
registry.address,
accounts[0],
stableToken.address,
spread,
reserveFraction,
Expand All @@ -141,6 +142,7 @@ contract('Exchange', (accounts: string[]) => {
await assertRevert(
exchange.initialize(
registry.address,
accounts[0],
stableToken.address,
spread,
reserveFraction,
Expand Down Expand Up @@ -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))
})
})
})
})