Skip to content

Commit

Permalink
kill-switch: use highest instead of lowest allowed severities
Browse files Browse the repository at this point in the history
  • Loading branch information
@facuspagnuolo
facuspagnuolo committed Apr 30, 2019
1 parent 75c8a1d commit ff8393d
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 87 deletions.
22 changes: 11 additions & 11 deletions contracts/kill_switch/KillSwitch.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ import "./IssuesRegistry.sol";
contract KillSwitch is AragonApp {
bytes32 constant public SET_ISSUES_REGISTRY_ROLE = keccak256("SET_ISSUES_REGISTRY_ROLE");
bytes32 constant public SET_CONTRACT_ACTION_ROLE = keccak256("SET_CONTRACT_ACTION_ROLE");
bytes32 constant public SET_LOWEST_ALLOWED_SEVERITY_ROLE = keccak256("SET_LOWEST_ALLOWED_SEVERITY_ROLE");
bytes32 constant public SET_HIGHEST_ALLOWED_SEVERITY_ROLE = keccak256("SET_HIGHEST_ALLOWED_SEVERITY_ROLE");

enum ContractAction { Check, Ignore, Deny }

IssuesRegistry public issuesRegistry;
mapping (address => ContractAction) internal contractActions;
mapping (address => IssuesRegistry.Severity) internal lowestAllowedSeverityByContract;
mapping (address => IssuesRegistry.Severity) internal highestAllowedSeverityByContract;

event IssuesRegistrySet(address issuesRegistry, address sender);
event ContractActionSet(address contractAddress, ContractAction action);
event LowestAllowedSeveritySet(address indexed _contract, IssuesRegistry.Severity severity);
event HighestAllowedSeveritySet(address indexed _contract, IssuesRegistry.Severity severity);

function initialize(IssuesRegistry _issuesRegistry) external onlyInit {
initialized();
Expand All @@ -31,12 +31,12 @@ contract KillSwitch is AragonApp {
emit ContractActionSet(_contract, _action);
}

function setLowestAllowedSeverity(address _contract, IssuesRegistry.Severity _severity)
function setHighestAllowedSeverity(address _contract, IssuesRegistry.Severity _severity)
external
authP(SET_LOWEST_ALLOWED_SEVERITY_ROLE, arr(_contract, msg.sender))
authP(SET_HIGHEST_ALLOWED_SEVERITY_ROLE, arr(_contract, msg.sender))
{
lowestAllowedSeverityByContract[_contract] = _severity;
emit LowestAllowedSeveritySet(_contract, _severity);
highestAllowedSeverityByContract[_contract] = _severity;
emit HighestAllowedSeveritySet(_contract, _severity);
}

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

function getLowestAllowedSeverity(address _contract) public view returns (IssuesRegistry.Severity) {
return lowestAllowedSeverityByContract[_contract];
function getHighestAllowedSeverity(address _contract) public view returns (IssuesRegistry.Severity) {
return highestAllowedSeverityByContract[_contract];
}

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

function isSeverityIgnored(address _contract) public view returns (bool) {
IssuesRegistry.Severity severityFound = issuesRegistry.getSeverityFor(_contract);
IssuesRegistry.Severity lowestAllowedSeverity = getLowestAllowedSeverity(_contract);
return lowestAllowedSeverity >= severityFound;
IssuesRegistry.Severity highestAllowedSeverity = getHighestAllowedSeverity(_contract);
return highestAllowedSeverity >= severityFound;
}

function shouldDenyCallingContract(address _base, address _instance, address _sender, bytes _data, uint256 _value) public returns (bool) {
Expand Down
134 changes: 67 additions & 67 deletions test/kill_switch/KillSwitch.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
await killSwitch.initialize(issuesRegistry.address)
const SET_CONTRACT_ACTION_ROLE = await killSwitchBase.SET_CONTRACT_ACTION_ROLE()
await acl.createPermission(owner, killSwitch.address, SET_CONTRACT_ACTION_ROLE, root, { from: root })
const SET_LOWEST_ALLOWED_SEVERITY_ROLE = await killSwitchBase.SET_LOWEST_ALLOWED_SEVERITY_ROLE()
await acl.createPermission(owner, killSwitch.address, SET_LOWEST_ALLOWED_SEVERITY_ROLE, root, { from: root })
const SET_HIGHEST_ALLOWED_SEVERITY_ROLE = await killSwitchBase.SET_HIGHEST_ALLOWED_SEVERITY_ROLE()
await acl.createPermission(owner, killSwitch.address, SET_HIGHEST_ALLOWED_SEVERITY_ROLE, root, { from: root })
})

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

describe('isSeverityIgnored', function () {
context('when there is no bug registered', () => {
context('when there is no lowest allowed severity set for the contract being called', () => {
context('when there is no highest allowed severity set for the contract being called', () => {
it('returns true', async () => {
assert.isTrue(await killSwitch.isSeverityIgnored(appBase.address))
})
})

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

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

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

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

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

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

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

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

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

describe('setLowestAllowedSeverity', function () {
describe('setHighestAllowedSeverity', function () {
context('when the contract is the owner', function () {
const from = owner

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

assert.equal(await killSwitch.getLowestAllowedSeverity(appBase.address), SEVERITY.HIGH)
assert.equal(await killSwitch.getHighestAllowedSeverity(appBase.address), SEVERITY.HIGH)
})
})

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

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

assert.equal(await killSwitch.getLowestAllowedSeverity(appBase.address), SEVERITY.MID)
assert.equal(await killSwitch.getHighestAllowedSeverity(appBase.address), SEVERITY.MID)
})
})
})

context('when the sender is not the owner', function () {
it('reverts', async function () {
await assertRevert(killSwitch.setLowestAllowedSeverity(appBase.address, SEVERITY.MID))
await assertRevert(killSwitch.setHighestAllowedSeverity(appBase.address, SEVERITY.MID))
})
})
})
Expand Down Expand Up @@ -247,13 +247,13 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
}

context('when there is no bug registered', () => {
context('when there is no lowest allowed severity set for the contract being called', () => {
context('when there is no highest allowed severity set for the contract being called', () => {
itExecutesTheCallEvenIfDenied()
})

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

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

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

context('when there is a lowest allowed severity set for the contract being called', () => {
context('when the lowest allowed severity is under the reported bug severity', () => {
context('when there is a highest allowed severity set for the contract being called', () => {
context('when the highest allowed severity is under the reported bug severity', () => {
itExecutesTheCallEvenIfDenied()
})

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

itExecutesTheCallEvenIfDenied()
})

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

itExecutesTheCallEvenIfDenied()
Expand Down Expand Up @@ -330,13 +330,13 @@ contract('KillSwitch', ([_, root, owner, securityPartner]) => {
}

context('when there is no bug registered', () => {
context('when there is no lowest allowed severity set for the contract being called', () => {
context('when there is no highest allowed severity set for the contract being called', () => {
itExecutesTheCallWhenNotDenied()
})

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

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

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

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

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

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

itExecutesTheCallWhenNotDenied()
})

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

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

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

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

itExecutesTheCallWhenNotDenied()
})

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

itExecutesTheCallWhenNotDenied()
})

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

itExecutesTheCallWhenNotDenied()
Expand Down
Loading

0 comments on commit ff8393d

Please sign in to comment.