diff --git a/contracts/DayLimit.sol b/contracts/DayLimit.sol index 49be5f194..7fae818e5 100644 --- a/contracts/DayLimit.sol +++ b/contracts/DayLimit.sol @@ -58,8 +58,9 @@ contract DayLimit { // simple modifier for daily limit. modifier limitedDaily(uint _value) { - if (underLimit(_value)) { - _; + if (!underLimit(_value)) { + throw; } + _; } } diff --git a/contracts/ownership/Claimable.sol b/contracts/ownership/Claimable.sol index 89a375d7f..a99ee5366 100644 --- a/contracts/ownership/Claimable.sol +++ b/contracts/ownership/Claimable.sol @@ -13,8 +13,10 @@ contract Claimable is Ownable { address public pendingOwner; modifier onlyPendingOwner() { - if (msg.sender == pendingOwner) - _; + if (msg.sender != pendingOwner) { + throw; + } + _; } function transferOwnership(address newOwner) onlyOwner { diff --git a/contracts/ownership/Ownable.sol b/contracts/ownership/Ownable.sol index d2a702889..8496320cf 100644 --- a/contracts/ownership/Ownable.sol +++ b/contracts/ownership/Ownable.sol @@ -15,9 +15,10 @@ contract Ownable { } modifier onlyOwner() { - if (msg.sender == owner) { - _; + if (msg.sender != owner) { + throw; } + _; } function transferOwnership(address newOwner) onlyOwner { diff --git a/contracts/ownership/Shareable.sol b/contracts/ownership/Shareable.sol index 57a6c21cd..dbcd2cfed 100644 --- a/contracts/ownership/Shareable.sol +++ b/contracts/ownership/Shareable.sol @@ -39,9 +39,10 @@ contract Shareable { // simple single-sig function modifier. modifier onlyOwner { - if (isOwner(msg.sender)) { - _; + if (!isOwner(msg.sender)) { + throw; } + _; } // multi-sig function modifier: the operation must have an intrinsic hash in order diff --git a/test/Claimable.js b/test/Claimable.js index da2aef1da..021213db2 100644 --- a/test/Claimable.js +++ b/test/Claimable.js @@ -1,4 +1,5 @@ 'use strict'; +const assertJump = require('./helpers/assertJump'); var Claimable = artifacts.require('../contracts/ownership/Claimable.sol'); @@ -23,17 +24,22 @@ contract('Claimable', function(accounts) { }); it('should prevent to claimOwnership from no pendingOwner', async function() { - claimable.claimOwnership({from: accounts[2]}); - let owner = await claimable.owner(); - - assert.isTrue(owner !== accounts[2]); + try { + await claimable.claimOwnership({from: accounts[2]}); + } catch(error) { + assertJump(error); + } }); it('should prevent non-owners from transfering', async function() { - await claimable.transferOwnership(accounts[2], {from: accounts[2]}); - let pendingOwner = await claimable.pendingOwner(); - - assert.isFalse(pendingOwner === accounts[2]); + const other = accounts[2]; + const owner = await claimable.owner.call(); + assert.isTrue(owner !== other); + try { + await claimable.transferOwnership(other, {from: other}); + } catch(error) { + assertJump(error); + } }); describe('after initiating a transfer', function () { diff --git a/test/DayLimit.js b/test/DayLimit.js index 6dfd55645..99c565701 100644 --- a/test/DayLimit.js +++ b/test/DayLimit.js @@ -1,4 +1,5 @@ 'use strict'; +const assertJump = require('./helpers/assertJump'); var DayLimitMock = artifacts.require('helpers/DayLimitMock.sol'); @@ -32,9 +33,11 @@ contract('DayLimit', function(accounts) { let spentToday = await dayLimit.spentToday(); assert.equal(spentToday, 8); - await dayLimit.attemptSpend(3); - spentToday = await dayLimit.spentToday(); - assert.equal(spentToday, 8); + try { + await dayLimit.attemptSpend(3); + } catch(error) { + assertJump(error); + } }); it('should allow spending if daily limit is reached and then set higher', async function() { @@ -45,7 +48,11 @@ contract('DayLimit', function(accounts) { let spentToday = await dayLimit.spentToday(); assert.equal(spentToday, 8); - await dayLimit.attemptSpend(3); + try { + await dayLimit.attemptSpend(3); + } catch(error) { + assertJump(error); + } spentToday = await dayLimit.spentToday(); assert.equal(spentToday, 8); @@ -63,7 +70,11 @@ contract('DayLimit', function(accounts) { let spentToday = await dayLimit.spentToday(); assert.equal(spentToday, 8); - await dayLimit.attemptSpend(3); + try { + await dayLimit.attemptSpend(3); + } catch(error) { + assertJump(error); + } spentToday = await dayLimit.spentToday(); assert.equal(spentToday, 8); diff --git a/test/Ownable.js b/test/Ownable.js index 6ec172423..0ad2fffcb 100644 --- a/test/Ownable.js +++ b/test/Ownable.js @@ -1,4 +1,5 @@ 'use strict'; +const assertJump = require('./helpers/assertJump'); var Ownable = artifacts.require('../contracts/ownership/Ownable.sol'); @@ -23,11 +24,14 @@ contract('Ownable', function(accounts) { }); it('should prevent non-owners from transfering', async function() { - let other = accounts[2]; - await ownable.transferOwnership(other, {from: accounts[2]}); - let owner = await ownable.owner(); - - assert.isFalse(owner === other); + const other = accounts[2]; + const owner = await ownable.owner.call(); + assert.isTrue(owner !== other); + try { + await ownable.transferOwnership(other, {from: other}); + } catch(error) { + assertJump(error); + } }); it('should guard ownership against stuck state', async function() {