Skip to content

Commit ff8393d

Browse files
facuspagnuolofacuspagnuolo
committed
kill-switch: use highest instead of lowest allowed severities
1 parent 75c8a1d commit ff8393d

File tree

3 files changed

+87
-87
lines changed

3 files changed

+87
-87
lines changed

‎contracts/kill_switch/KillSwitch.sol‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,17 @@ import "./IssuesRegistry.sol";
66
contract KillSwitch is AragonApp {
77
bytes32 constant public SET_ISSUES_REGISTRY_ROLE = keccak256("SET_ISSUES_REGISTRY_ROLE");
88
bytes32 constant public SET_CONTRACT_ACTION_ROLE = keccak256("SET_CONTRACT_ACTION_ROLE");
9-
bytes32 constant public SET_LOWEST_ALLOWED_SEVERITY_ROLE = keccak256("SET_LOWEST_ALLOWED_SEVERITY_ROLE");
9+
bytes32 constant public SET_HIGHEST_ALLOWED_SEVERITY_ROLE = keccak256("SET_HIGHEST_ALLOWED_SEVERITY_ROLE");
1010

1111
enum ContractAction { Check, Ignore, Deny }
1212

1313
IssuesRegistry public issuesRegistry;
1414
mapping (address => ContractAction) internal contractActions;
15-
mapping (address => IssuesRegistry.Severity) internal lowestAllowedSeverityByContract;
15+
mapping (address => IssuesRegistry.Severity) internal highestAllowedSeverityByContract;
1616

1717
event IssuesRegistrySet(address issuesRegistry, address sender);
1818
event ContractActionSet(address contractAddress, ContractAction action);
19-
event LowestAllowedSeveritySet(address indexed _contract, IssuesRegistry.Severity severity);
19+
event HighestAllowedSeveritySet(address indexed _contract, IssuesRegistry.Severity severity);
2020

2121
function initialize(IssuesRegistry _issuesRegistry) external onlyInit {
2222
initialized();
@@ -31,12 +31,12 @@ contract KillSwitch is AragonApp {
3131
emit ContractActionSet(_contract, _action);
3232
}
3333

34-
function setLowestAllowedSeverity(address _contract, IssuesRegistry.Severity _severity)
34+
function setHighestAllowedSeverity(address _contract, IssuesRegistry.Severity _severity)
3535
external
36-
authP(SET_LOWEST_ALLOWED_SEVERITY_ROLE, arr(_contract, msg.sender))
36+
authP(SET_HIGHEST_ALLOWED_SEVERITY_ROLE, arr(_contract, msg.sender))
3737
{
38-
lowestAllowedSeverityByContract[_contract] = _severity;
39-
emit LowestAllowedSeveritySet(_contract, _severity);
38+
highestAllowedSeverityByContract[_contract] = _severity;
39+
emit HighestAllowedSeveritySet(_contract, _severity);
4040
}
4141

4242
function setIssuesRegistry(IssuesRegistry _issuesRegistry)
@@ -50,8 +50,8 @@ contract KillSwitch is AragonApp {
5050
return contractActions[_contract];
5151
}
5252

53-
function getLowestAllowedSeverity(address _contract) public view returns (IssuesRegistry.Severity) {
54-
return lowestAllowedSeverityByContract[_contract];
53+
function getHighestAllowedSeverity(address _contract) public view returns (IssuesRegistry.Severity) {
54+
return highestAllowedSeverityByContract[_contract];
5555
}
5656

5757
function isContractIgnored(address _contract) public view returns (bool) {
@@ -64,8 +64,8 @@ contract KillSwitch is AragonApp {
6464

6565
function isSeverityIgnored(address _contract) public view returns (bool) {
6666
IssuesRegistry.Severity severityFound = issuesRegistry.getSeverityFor(_contract);
67-
IssuesRegistry.Severity lowestAllowedSeverity = getLowestAllowedSeverity(_contract);
68-
return lowestAllowedSeverity >= severityFound;
67+
IssuesRegistry.Severity highestAllowedSeverity = getHighestAllowedSeverity(_contract);
68+
return highestAllowedSeverity >= severityFound;
6969
}
7070

7171
function shouldDenyCallingContract(address _base, address _instance, address _sender, bytes _data, uint256 _value) public returns (bool) {

‎test/kill_switch/KillSwitch.test.js‎

Lines changed: 67 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
4747
await killSwitch.initialize(issuesRegistry.address)
4848
const SET_CONTRACT_ACTION_ROLE = await killSwitchBase.SET_CONTRACT_ACTION_ROLE()
4949
await acl.createPermission(owner, killSwitch.address, SET_CONTRACT_ACTION_ROLE, root, { from: root })
50-
const SET_LOWEST_ALLOWED_SEVERITY_ROLE = await killSwitchBase.SET_LOWEST_ALLOWED_SEVERITY_ROLE()
51-
await acl.createPermission(owner, killSwitch.address, SET_LOWEST_ALLOWED_SEVERITY_ROLE, root, { from: root })
50+
const SET_HIGHEST_ALLOWED_SEVERITY_ROLE = await killSwitchBase.SET_HIGHEST_ALLOWED_SEVERITY_ROLE()
51+
await acl.createPermission(owner, killSwitch.address, SET_HIGHEST_ALLOWED_SEVERITY_ROLE, root, { from: root })
5252
})
5353

5454
beforeEach('create kill switched app', async () => {
@@ -128,15 +128,15 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
128128

129129
describe('isSeverityIgnored', function () {
130130
context('when there is no bug registered', () => {
131-
context('when there is no lowest allowed severity set for the contract being called', () => {
131+
context('when there is no highest allowed severity set for the contract being called', () => {
132132
it('returns true', async () => {
133133
assert.isTrue(await killSwitch.isSeverityIgnored(appBase.address))
134134
})
135135
})
136136

137-
context('when there is a lowest allowed severity set for the contract being called', () => {
138-
beforeEach('set lowest allowed severity', async () => {
139-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.LOW, { from: owner })
137+
context('when there is a highest allowed severity set for the contract being called', () => {
138+
beforeEach('set highest allowed severity', async () => {
139+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.LOW, { from: owner })
140140
})
141141

142142
it('returns true', async () => {
@@ -150,36 +150,36 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
150150
await issuesRegistry.setSeverityFor(appBase.address, SEVERITY.MID, { from: securityPartner })
151151
})
152152

153-
context('when there is no lowest allowed severity set for the contract being called', () => {
153+
context('when there is no highest allowed severity set for the contract being called', () => {
154154
it('returns false', async () => {
155155
assert.isFalse(await killSwitch.isSeverityIgnored(appBase.address))
156156
})
157157
})
158158

159-
context('when there is a lowest allowed severity set for the contract being called', () => {
160-
context('when the lowest allowed severity is under the reported bug severity', () => {
161-
beforeEach('set lowest allowed severity', async () => {
162-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.LOW, { from: owner })
159+
context('when there is a highest allowed severity set for the contract being called', () => {
160+
context('when the highest allowed severity is under the reported bug severity', () => {
161+
beforeEach('set highest allowed severity', async () => {
162+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.LOW, { from: owner })
163163
})
164164

165165
it('returns false', async () => {
166166
assert.isFalse(await killSwitch.isSeverityIgnored(appBase.address))
167167
})
168168
})
169169

170-
context('when the lowest allowed severity is equal to the reported bug severity', () => {
171-
beforeEach('set lowest allowed severity', async () => {
172-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.MID, { from: owner })
170+
context('when the highest allowed severity is equal to the reported bug severity', () => {
171+
beforeEach('set highest allowed severity', async () => {
172+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.MID, { from: owner })
173173
})
174174

175175
it('returns true', async () => {
176176
assert.isTrue(await killSwitch.isSeverityIgnored(appBase.address))
177177
})
178178
})
179179

180-
context('when the lowest allowed severity is greater than the reported bug severity', () => {
181-
beforeEach('set lowest allowed severity', async () => {
182-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.CRITICAL, { from: owner })
180+
context('when the highest allowed severity is greater than the reported bug severity', () => {
181+
beforeEach('set highest allowed severity', async () => {
182+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.CRITICAL, { from: owner })
183183
})
184184

185185
it('returns true', async () => {
@@ -190,35 +190,35 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
190190
})
191191
})
192192

193-
describe('setLowestAllowedSeverity', function () {
193+
describe('setHighestAllowedSeverity', function () {
194194
context('when the contract is the owner', function () {
195195
const from = owner
196196

197197
context('when there was no severity set', function () {
198-
it('sets the lowest allowed severity', async function () {
199-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.HIGH, { from })
198+
it('sets the highest allowed severity', async function () {
199+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.HIGH, { from })
200200

201-
assert.equal(await killSwitch.getLowestAllowedSeverity(appBase.address), SEVERITY.HIGH)
201+
assert.equal(await killSwitch.getHighestAllowedSeverity(appBase.address), SEVERITY.HIGH)
202202
})
203203
})
204204

205205
context('when there was a previous severity set', function () {
206-
beforeEach('set lowest allowed severity', async function () {
207-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.LOW, { from })
208-
assert.equal(await killSwitch.getLowestAllowedSeverity(appBase.address), SEVERITY.LOW)
206+
beforeEach('set highest allowed severity', async function () {
207+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.LOW, { from })
208+
assert.equal(await killSwitch.getHighestAllowedSeverity(appBase.address), SEVERITY.LOW)
209209
})
210210

211-
it('changes the lowest allowed severity', async function () {
212-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.MID, { from })
211+
it('changes the highest allowed severity', async function () {
212+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.MID, { from })
213213

214-
assert.equal(await killSwitch.getLowestAllowedSeverity(appBase.address), SEVERITY.MID)
214+
assert.equal(await killSwitch.getHighestAllowedSeverity(appBase.address), SEVERITY.MID)
215215
})
216216
})
217217
})
218218

219219
context('when the sender is not the owner', function () {
220220
it('reverts', async function () {
221-
await assertRevert(killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.MID))
221+
await assertRevert(killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.MID))
222222
})
223223
})
224224
})
@@ -247,13 +247,13 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
247247
}
248248

249249
context('when there is no bug registered', () => {
250-
context('when there is no lowest allowed severity set for the contract being called', () => {
250+
context('when there is no highest allowed severity set for the contract being called', () => {
251251
itExecutesTheCallEvenIfDenied()
252252
})
253253

254-
context('when there is a lowest allowed severity set for the contract being called', () => {
255-
beforeEach('set lowest allowed severity', async () => {
256-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.LOW, { from: owner })
254+
context('when there is a highest allowed severity set for the contract being called', () => {
255+
beforeEach('set highest allowed severity', async () => {
256+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.LOW, { from: owner })
257257
})
258258

259259
itExecutesTheCallEvenIfDenied()
@@ -265,26 +265,26 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
265265
await issuesRegistry.setSeverityFor(appBase.address, SEVERITY.MID, { from: securityPartner })
266266
})
267267

268-
context('when there is no lowest allowed severity set for the contract being called', () => {
268+
context('when there is no highest allowed severity set for the contract being called', () => {
269269
itExecutesTheCallEvenIfDenied()
270270
})
271271

272-
context('when there is a lowest allowed severity set for the contract being called', () => {
273-
context('when the lowest allowed severity is under the reported bug severity', () => {
272+
context('when there is a highest allowed severity set for the contract being called', () => {
273+
context('when the highest allowed severity is under the reported bug severity', () => {
274274
itExecutesTheCallEvenIfDenied()
275275
})
276276

277-
context('when the lowest allowed severity is equal to the reported bug severity', () => {
278-
beforeEach('set lowest allowed severity', async () => {
279-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.MID, { from: owner })
277+
context('when the highest allowed severity is equal to the reported bug severity', () => {
278+
beforeEach('set highest allowed severity', async () => {
279+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.MID, { from: owner })
280280
})
281281

282282
itExecutesTheCallEvenIfDenied()
283283
})
284284

285-
context('when the lowest allowed severity is greater than the reported bug severity', () => {
286-
beforeEach('set lowest allowed severity', async () => {
287-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.CRITICAL, { from: owner })
285+
context('when the highest allowed severity is greater than the reported bug severity', () => {
286+
beforeEach('set highest allowed severity', async () => {
287+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.CRITICAL, { from: owner })
288288
})
289289

290290
itExecutesTheCallEvenIfDenied()
@@ -330,13 +330,13 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
330330
}
331331

332332
context('when there is no bug registered', () => {
333-
context('when there is no lowest allowed severity set for the contract being called', () => {
333+
context('when there is no highest allowed severity set for the contract being called', () => {
334334
itExecutesTheCallWhenNotDenied()
335335
})
336336

337-
context('when there is a lowest allowed severity set for the contract being called', () => {
338-
beforeEach('set lowest allowed severity', async () => {
339-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.LOW, { from: owner })
337+
context('when there is a highest allowed severity set for the contract being called', () => {
338+
beforeEach('set highest allowed severity', async () => {
339+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.LOW, { from: owner })
340340
})
341341

342342
itExecutesTheCallWhenNotDenied()
@@ -349,7 +349,7 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
349349
})
350350

351351
context('when the bug was not fixed yet', () => {
352-
context('when there is no lowest allowed severity set for the contract being called', () => {
352+
context('when there is no highest allowed severity set for the contract being called', () => {
353353
context('when the contract being called is checked', () => {
354354
itDoesNotExecuteTheCall()
355355
})
@@ -371,10 +371,10 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
371371
})
372372
})
373373

374-
context('when there is a lowest allowed severity set for the contract being called', () => {
375-
context('when the lowest allowed severity is under the reported bug severity', () => {
376-
beforeEach('set lowest allowed severity', async () => {
377-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.LOW, { from: owner })
374+
context('when there is a highest allowed severity set for the contract being called', () => {
375+
context('when the highest allowed severity is under the reported bug severity', () => {
376+
beforeEach('set highest allowed severity', async () => {
377+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.LOW, { from: owner })
378378
})
379379

380380
context('when the contract being called is checked', () => {
@@ -398,17 +398,17 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
398398
})
399399
})
400400

401-
context('when the lowest allowed severity is equal to the reported bug severity', () => {
402-
beforeEach('set lowest allowed severity', async () => {
403-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.MID, { from: owner })
401+
context('when the highest allowed severity is equal to the reported bug severity', () => {
402+
beforeEach('set highest allowed severity', async () => {
403+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.MID, { from: owner })
404404
})
405405

406406
itExecutesTheCallWhenNotDenied()
407407
})
408408

409-
context('when the lowest allowed severity is greater than the reported bug severity', () => {
410-
beforeEach('set lowest allowed severity', async () => {
411-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.CRITICAL, { from: owner })
409+
context('when the highest allowed severity is greater than the reported bug severity', () => {
410+
beforeEach('set highest allowed severity', async () => {
411+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.CRITICAL, { from: owner })
412412
})
413413

414414
itExecutesTheCallWhenNotDenied()
@@ -421,30 +421,30 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
421421
await issuesRegistry.setSeverityFor(appBase.address, SEVERITY.NONE, { from: securityPartner })
422422
})
423423

424-
context('when there is no lowest allowed severity set for the contract being called', () => {
424+
context('when there is no highest allowed severity set for the contract being called', () => {
425425
itExecutesTheCallWhenNotDenied()
426426
})
427427

428-
context('when there is a lowest allowed severity set for the contract being called', () => {
429-
context('when the lowest allowed severity is under the reported bug severity', () => {
430-
beforeEach('set lowest allowed severity', async () => {
431-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.LOW, { from: owner })
428+
context('when there is a highest allowed severity set for the contract being called', () => {
429+
context('when the highest allowed severity is under the reported bug severity', () => {
430+
beforeEach('set highest allowed severity', async () => {
431+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.LOW, { from: owner })
432432
})
433433

434434
itExecutesTheCallWhenNotDenied()
435435
})
436436

437-
context('when the lowest allowed severity is equal to the reported bug severity', () => {
438-
beforeEach('set lowest allowed severity', async () => {
439-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.MID, { from: owner })
437+
context('when the highest allowed severity is equal to the reported bug severity', () => {
438+
beforeEach('set highest allowed severity', async () => {
439+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.MID, { from: owner })
440440
})
441441

442442
itExecutesTheCallWhenNotDenied()
443443
})
444444

445-
context('when the lowest allowed severity is greater than the reported bug severity', () => {
446-
beforeEach('set lowest allowed severity', async () => {
447-
await killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.CRITICAL, { from: owner })
445+
context('when the highest allowed severity is greater than the reported bug severity', () => {
446+
beforeEach('set highest allowed severity', async () => {
447+
await killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.CRITICAL, { from: owner })
448448
})
449449

450450
itExecutesTheCallWhenNotDenied()

0 commit comments

Comments
 (0)