Skip to content

Commit 5b0ec99

Browse files
barrassobarrasso
committed
internal feedback
1 parent a944156 commit 5b0ec99

File tree

3 files changed

+189
-14
lines changed

3 files changed

+189
-14
lines changed

‎contracts/DynamicSynthRedeemer.sol

+45-8
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ contract DynamicSynthRedeemer is Owned, IDynamicSynthRedeemer, MixinResolver {
2323
bytes32 public constant CONTRACT_NAME = "DynamicSynthRedeemer";
2424

2525
uint public discountRate;
26+
bool public redemptionActive;
2627

2728
bytes32 internal constant sUSD = "sUSD";
2829

@@ -33,6 +34,8 @@ contract DynamicSynthRedeemer is Owned, IDynamicSynthRedeemer, MixinResolver {
3334
discountRate = SafeDecimalMath.unit();
3435
}
3536

37+
/* ========== RESOLVER CONFIG ========== */
38+
3639
function resolverAddressesRequired() public view returns (bytes32[] memory addresses) {
3740
addresses = new bytes32[](2);
3841
addresses[0] = CONTRACT_ISSUER;
@@ -47,27 +50,29 @@ contract DynamicSynthRedeemer is Owned, IDynamicSynthRedeemer, MixinResolver {
4750
return IExchangeRates(requireAndGetAddress(CONTRACT_EXRATES));
4851
}
4952

53+
function redeemingActive() internal view {
54+
require(redemptionActive, "Redemption deactivated");
55+
}
56+
57+
/* ========== VIEWS ========== */
58+
5059
function getDiscountRate() external view returns (uint) {
5160
return discountRate;
5261
}
5362

54-
function setDiscountRate(uint _newRate) external onlyOwner {
55-
require(_newRate >= 0 && _newRate <= SafeDecimalMath.unit(), "Invalid rate");
56-
discountRate = _newRate;
57-
emit DiscountRateUpdated(_newRate);
58-
}
63+
/* ========== MUTATIVE FUNCTIONS ========== */
5964

60-
function redeemAll(address[] calldata synthProxies) external {
65+
function redeemAll(address[] calldata synthProxies) external requireRedemptionActive {
6166
for (uint i = 0; i < synthProxies.length; i++) {
6267
_redeem(synthProxies[i], IERC20(synthProxies[i]).balanceOf(msg.sender));
6368
}
6469
}
6570

66-
function redeem(address synthProxy) external {
71+
function redeem(address synthProxy) external requireRedemptionActive {
6772
_redeem(synthProxy, IERC20(synthProxy).balanceOf(msg.sender));
6873
}
6974

70-
function redeemPartial(address synthProxy, uint amountOfSynth) external {
75+
function redeemPartial(address synthProxy, uint amountOfSynth) external requireRedemptionActive {
7176
// technically this check isn't necessary - Synth.burn would fail due to safe sub,
7277
// but this is a useful error message to the user
7378
require(IERC20(synthProxy).balanceOf(msg.sender) >= amountOfSynth, "Insufficient balance");
@@ -76,6 +81,7 @@ contract DynamicSynthRedeemer is Owned, IDynamicSynthRedeemer, MixinResolver {
7681

7782
function _redeem(address synthProxy, uint amountOfSynth) internal {
7883
bytes32 currencyKey = ISynth(IProxy(synthProxy).target()).currencyKey();
84+
require(currencyKey != sUSD, "Cannot redeem sUSD");
7985
// Discount rate applied to chainlink price for dynamic redemptions
8086
uint rateToRedeem = exchangeRates().rateForCurrency(currencyKey).multiplyDecimalRound(discountRate);
8187
require(rateToRedeem > 0, "Synth not redeemable");
@@ -86,6 +92,37 @@ contract DynamicSynthRedeemer is Owned, IDynamicSynthRedeemer, MixinResolver {
8692
emit SynthRedeemed(address(synthProxy), msg.sender, amountOfSynth, amountInsUSD);
8793
}
8894

95+
/* ========== MODIFIERS ========== */
96+
97+
modifier requireRedemptionActive() {
98+
redeemingActive();
99+
_;
100+
}
101+
102+
/* ========== RESTRICTED FUNCTIONS ========== */
103+
104+
function setDiscountRate(uint _newRate) external onlyOwner {
105+
require(_newRate >= 0 && _newRate <= SafeDecimalMath.unit(), "Invalid rate");
106+
discountRate = _newRate;
107+
emit DiscountRateUpdated(_newRate);
108+
}
109+
110+
function suspendRedemption() external onlyOwner {
111+
require(redemptionActive, "Redemption suspended");
112+
redemptionActive = false;
113+
emit RedemptionSuspended();
114+
}
115+
116+
function resumeRedemption() external onlyOwner {
117+
require(!redemptionActive, "Redemption not suspended");
118+
redemptionActive = true;
119+
emit RedemptionResumed();
120+
}
121+
122+
/* ========== EVENTS ========== */
123+
124+
event RedemptionSuspended();
125+
event RedemptionResumed();
89126
event DiscountRateUpdated(uint discountRate);
90127
event SynthRedeemed(address synth, address account, uint amountOfSynth, uint amountInsUSD);
91128
}

‎contracts/interfaces/IDynamicSynthRedeemer.sol

+4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ pragma solidity >=0.4.24;
33
import "./IERC20.sol";
44

55
interface IDynamicSynthRedeemer {
6+
function suspendRedemption() external;
7+
8+
function resumeRedemption() external;
9+
610
// Rate applied to chainlink price for redemptions
711
function getDiscountRate() external view returns (uint);
812

‎test/contracts/DynamicSynthRedeemer.js

+140-6
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,13 @@ contract('DynamicSynthRedeemer', async accounts => {
2121
const [, owner, , , account1] = accounts;
2222

2323
let instance;
24-
let addressResolver, dynamicSynthRedeemer, exchangeRates, issuer, proxysETH;
24+
let addressResolver,
25+
dynamicSynthRedeemer,
26+
exchangeRates,
27+
issuer,
28+
proxysETH,
29+
proxysUSD,
30+
proxySynthetix;
2531

2632
before(async () => {
2733
({
@@ -30,6 +36,8 @@ contract('DynamicSynthRedeemer', async accounts => {
3036
ExchangeRates: exchangeRates,
3137
Issuer: issuer,
3238
ProxyERC20sETH: proxysETH,
39+
ProxyERC20sUSD: proxysUSD,
40+
ProxySynthetix: proxySynthetix,
3341
} = await setupAllContracts({
3442
accounts,
3543
synths,
@@ -41,6 +49,7 @@ contract('DynamicSynthRedeemer', async accounts => {
4149
'Issuer',
4250
'Liquidator',
4351
'LiquidatorRewards',
52+
'ProxyERC20',
4453
'RewardEscrowV2',
4554
],
4655
}));
@@ -55,7 +64,14 @@ contract('DynamicSynthRedeemer', async accounts => {
5564
ensureOnlyExpectedMutativeFunctions({
5665
abi: dynamicSynthRedeemer.abi,
5766
ignoreParents: ['Owned', 'MixinResolver'],
58-
expected: ['redeem', 'redeemAll', 'redeemPartial', 'setDiscountRate'],
67+
expected: [
68+
'redeem',
69+
'redeemAll',
70+
'redeemPartial',
71+
'setDiscountRate',
72+
'resumeRedemption',
73+
'suspendRedemption',
74+
],
5975
});
6076
});
6177

@@ -69,16 +85,101 @@ contract('DynamicSynthRedeemer', async accounts => {
6985
assert.equal(await instance.resolver(), addressResolver.address);
7086
});
7187

88+
it('should set default discount rate', async () => {
89+
assert.bnEqual(await instance.getDiscountRate(), toUnit('1'));
90+
});
91+
92+
it('should not be active for redemption', async () => {
93+
assert.equal(await instance.redemptionActive(), false);
94+
});
95+
7296
it('should access its dependencies via the address resolver', async () => {
7397
assert.equal(await addressResolver.getAddress(toBytes32('Issuer')), issuer.address);
7498
assert.equal(
7599
await addressResolver.getAddress(toBytes32('ExchangeRates')),
76100
exchangeRates.address
77101
);
78102
});
103+
});
79104

80-
it('should set default discount rate', async () => {
81-
assert.bnEqual(await instance.getDiscountRate(), toUnit('1'));
105+
describe('suspendRedemption', () => {
106+
describe('failure modes', () => {
107+
beforeEach(async () => {
108+
// first resume redemptions
109+
await instance.resumeRedemption({ from: owner });
110+
});
111+
112+
it('reverts when not invoked by the owner', async () => {
113+
await onlyGivenAddressCanInvoke({
114+
fnc: instance.suspendRedemption,
115+
args: [],
116+
accounts,
117+
reason: 'Only the contract owner may perform this action',
118+
address: owner,
119+
});
120+
});
121+
122+
it('reverts when redemption is already suspended', async () => {
123+
await instance.suspendRedemption({ from: owner });
124+
await assert.revert(instance.suspendRedemption({ from: owner }), 'Redemption suspended');
125+
});
126+
});
127+
128+
describe('when invoked by the owner', () => {
129+
let txn;
130+
beforeEach(async () => {
131+
// first resume redemptions
132+
await instance.resumeRedemption({ from: owner });
133+
txn = await instance.suspendRedemption({ from: owner });
134+
});
135+
136+
it('and redemptionActive is false', async () => {
137+
assert.equal(await instance.redemptionActive(), false);
138+
});
139+
140+
it('and a RedemptionSuspended event is emitted', async () => {
141+
assert.eventEqual(txn, 'RedemptionSuspended', []);
142+
});
143+
});
144+
});
145+
146+
describe('resumeRedemption', () => {
147+
describe('failure modes', () => {
148+
it('reverts when not invoked by the owner', async () => {
149+
await onlyGivenAddressCanInvoke({
150+
fnc: instance.resumeRedemption,
151+
args: [],
152+
accounts,
153+
reason: 'Only the contract owner may perform this action',
154+
address: owner,
155+
});
156+
});
157+
158+
it('reverts when redemption is not suspended', async () => {
159+
await instance.resumeRedemption({ from: owner });
160+
await assert.revert(instance.resumeRedemption({ from: owner }), 'Redemption not suspended');
161+
});
162+
});
163+
164+
describe('when redemption is suspended', () => {
165+
it('redemptionActive is false', async () => {
166+
assert.equal(await instance.redemptionActive(), false);
167+
});
168+
169+
describe('when invoked by the owner', () => {
170+
let txn;
171+
beforeEach(async () => {
172+
txn = await instance.resumeRedemption({ from: owner });
173+
});
174+
175+
it('redemptions are active again', async () => {
176+
assert.equal(await instance.redemptionActive(), true);
177+
});
178+
179+
it('a RedemptionResumed event is emitted', async () => {
180+
assert.eventEqual(txn, 'RedemptionResumed', []);
181+
});
182+
});
82183
});
83184
});
84185

@@ -103,6 +204,20 @@ contract('DynamicSynthRedeemer', async accounts => {
103204

104205
describe('redemption', () => {
105206
describe('redeem()', () => {
207+
beforeEach(async () => {
208+
await instance.resumeRedemption({ from: owner });
209+
});
210+
211+
it('reverts when redemption is suspended', async () => {
212+
await instance.suspendRedemption({ from: owner });
213+
await assert.revert(
214+
instance.redeem(proxysETH.address, {
215+
from: account1,
216+
}),
217+
'Redemption deactivated'
218+
);
219+
});
220+
106221
it('reverts when discount rate is set to zero', async () => {
107222
await instance.setDiscountRate(toUnit('0'), { from: owner });
108223
await assert.revert(
@@ -113,8 +228,7 @@ contract('DynamicSynthRedeemer', async accounts => {
113228
);
114229
});
115230

116-
it('redemption reverts when user has no balance', async () => {
117-
await instance.setDiscountRate(toUnit('1'), { from: owner });
231+
it('reverts when user has no balance', async () => {
118232
await assert.revert(
119233
instance.redeem(proxysETH.address, {
120234
from: account1,
@@ -123,9 +237,29 @@ contract('DynamicSynthRedeemer', async accounts => {
123237
);
124238
});
125239

240+
it('reverts when user attempts to redeem sUSD', async () => {
241+
await assert.revert(
242+
instance.redeem(proxysUSD.address, {
243+
from: account1,
244+
}),
245+
'Cannot redeem sUSD'
246+
);
247+
});
248+
249+
it('reverts when user attempts to redeem a non-synth token', async () => {
250+
await assert.revert(
251+
instance.redeem(proxySynthetix.address, {
252+
from: account1,
253+
})
254+
);
255+
});
256+
126257
describe('when the user has a synth balance', () => {
127258
let userBalanceBefore;
128259
beforeEach(async () => {
260+
// TODO: check owner ETH balance
261+
// if got ETH, wrap it using ETH wrapper to get sETH and transfer to account1
262+
129263
// await proxysETH.transfer(account1, toUnit('5'), { from: owner });
130264
userBalanceBefore = await proxysETH.balanceOf(account1);
131265
console.log(userBalanceBefore);

0 commit comments

Comments
 (0)