diff --git a/.gitignore b/.gitignore index 828920310..d95c54b2b 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ token.json credentials.json echidna/ validate/apikey.infura +ganache-blockchain-log.txt diff --git a/contracts/minting/Controller.sol b/contracts/minting/Controller.sol index 0e4e3791d..ad1099172 100644 --- a/contracts/minting/Controller.sol +++ b/contracts/minting/Controller.sol @@ -45,7 +45,7 @@ contract Controller is Ownable { */ modifier onlyController() { require(controllers[msg.sender] != address(0), - "The value of controllers[msg.sender] must be non-zero."); + "The value of controllers[msg.sender] must be non-zero"); _; } @@ -68,7 +68,7 @@ contract Controller is Ownable { returns (bool) { require(_controller != address(0), "Controller must be a non-zero address"); - require(_worker != address(0), "Worker must be a non-zero address."); + require(_worker != address(0), "Worker must be a non-zero address"); controllers[_controller] = _worker; emit ControllerConfigured(_controller, _worker); return true; @@ -85,6 +85,7 @@ contract Controller is Ownable { returns (bool) { require(_controller != address(0), "Controller must be a non-zero address"); + require(controllers[_controller] != address(0), "Worker must be a non-zero address"); controllers[_controller] = address(0); emit ControllerRemoved(_controller); return true; diff --git a/contracts/minting/MintController.sol b/contracts/minting/MintController.sol index 36a03d0d9..a7d16ed65 100644 --- a/contracts/minting/MintController.sol +++ b/contracts/minting/MintController.sol @@ -117,10 +117,10 @@ contract MintController is Controller { onlyController returns (bool) { - require(_allowanceIncrement > 0, "Allowance increment must be greater than 0."); + require(_allowanceIncrement > 0, "Allowance increment must be greater than 0"); address minter = controllers[msg.sender]; require(minterManager.isMinter(minter), - "Can only increment allowance for minters in minterManager."); + "Can only increment allowance for minters in minterManager"); uint256 currentAllowance = minterManager.minterAllowance(minter); uint256 newAllowance = currentAllowance.add(_allowanceIncrement); diff --git a/test/minting/MintP0_ArgumentTests.js b/test/minting/MintP0_ArgumentTests.js index c588d365f..9d08cef06 100644 --- a/test/minting/MintP0_ArgumentTests.js +++ b/test/minting/MintP0_ArgumentTests.js @@ -4,7 +4,7 @@ var MintController = artifacts.require('minting/MintController'); var MasterMinter = artifacts.require('minting/MasterMinter'); var FiatToken = artifacts.require('FiatTokenV1'); -var tokenUtils = require('./../TokenTestUtils.js'); +var tokenUtils = require('../TokenTestUtils.js'); var newBigNumber = tokenUtils.newBigNumber; var checkMINTp0 = tokenUtils.checkMINTp0; var expectRevert = tokenUtils.expectRevert; @@ -15,8 +15,8 @@ var maxAmount = tokenUtils.maxAmount; var clone = require('clone'); -var mintUtils = require('./../MintControllerUtils.js'); -var AccountUtils = require('./../AccountUtils.js'); +var mintUtils = require('../MintControllerUtils.js'); +var AccountUtils = require('../AccountUtils.js'); var Accounts = AccountUtils.Accounts; var getAccountState = AccountUtils.getAccountState; var MintControllerState = AccountUtils.MintControllerState; @@ -83,8 +83,9 @@ async function run_MINT_tests(newToken, MintControllerArtifact, accounts) { }); it('arg007 removeController(0) throws', async function () { - await expectError(mintController.removeController(zeroAddress, {from: Accounts.mintOwnerAccount}), - "Controller must be a non-zero address"); + // expect no changes + await expectError(mintController.removeController(zeroAddress, {from: Accounts.mintOwnerAccount}), "Controller must be a non-zero address"); + await checkMINTp0([token, mintController], [expectedTokenState, expectedMintControllerState]); }); it('arg008 setMinterManager(0) works', async function () { @@ -188,11 +189,20 @@ async function run_MINT_tests(newToken, MintControllerArtifact, accounts) { }); it('arg020 removeController(C) works', async function() { - // expect no changes - await mintController.removeController(Accounts.controller1Account, {from: Accounts.mintOwnerAccount}); + // make controller1Account a controller + await mintController.configureController(Accounts.controller1Account, Accounts.minterAccount, {from: Accounts.mintOwnerAccount}); + var actualMinter = await mintController.controllers(Accounts.controller1Account); + addressEquals(Accounts.minterAccount, actualMinter); + + // remove controller1Account + await mintController.removeController(Accounts.controller1Account, {from : Accounts.mintOwnerAccount}); await checkMINTp0([token, mintController], [expectedTokenState, expectedMintControllerState]); + actualMinter = await mintController.controllers(Accounts.controller1Account); + addressEquals(actualMinter, zeroAddress); + }); - // now make controller1Account a controller + it('arg021 removeController throws if worker is already address(0)', async function () { + // make controller1Account a controller await mintController.configureController(Accounts.controller1Account, Accounts.minterAccount, {from: Accounts.mintOwnerAccount}); var actualMinter = await mintController.controllers(Accounts.controller1Account); addressEquals(Accounts.minterAccount, actualMinter); @@ -202,7 +212,11 @@ async function run_MINT_tests(newToken, MintControllerArtifact, accounts) { await checkMINTp0([token, mintController], [expectedTokenState, expectedMintControllerState]); actualMinter = await mintController.controllers(Accounts.controller1Account); addressEquals(actualMinter, zeroAddress); - }); + + // attempting to remove the controller1Account again should throw because the worker is already set to address(0). + await expectError(mintController.removeController(Accounts.controller1Account, {from: Accounts.mintOwnerAccount}), + "Worker must be a non-zero address"); + }) } var testWrapper = require('./../TestWrapper'); diff --git a/test/minting2/MintControllerTests.js b/test/minting2/MintControllerTests.js index 5778e9c88..544f5a457 100644 --- a/test/minting2/MintControllerTests.js +++ b/test/minting2/MintControllerTests.js @@ -96,11 +96,11 @@ async function run_tests(newToken, accounts) { }); it('only controller removes a minter', async function () { - await expectError(mintController.removeMinter({from: Accounts.controller1Account}), "The value of controllers[msg.sender] must be non-zero."); + await expectError(mintController.removeMinter({from: Accounts.controller1Account}), "The value of controllers[msg.sender] must be non-zero"); }); it('only controller configures a minter', async function () { - await expectError(mintController.configureMinter(0, {from: Accounts.controller1Account}), "The value of controllers[msg.sender] must be non-zero."); + await expectError(mintController.configureMinter(0, {from: Accounts.controller1Account}), "The value of controllers[msg.sender] must be non-zero"); }); it('increment minter allowance', async function () { @@ -126,7 +126,7 @@ async function run_tests(newToken, accounts) { }); it('only controller increments allowance', async function () { - await expectError(mintController.incrementMinterAllowance(0, {from: Accounts.controller1Account}), "The value of controllers[msg.sender] must be non-zero."); + await expectError(mintController.incrementMinterAllowance(0, {from: Accounts.controller1Account}), "The value of controllers[msg.sender] must be non-zero"); }); it('only active minters can have allowance incremented', async function () { @@ -137,11 +137,11 @@ async function run_tests(newToken, accounts) { await checkMINTp0([token, mintController], [expectedTokenState, expectedMintControllerState]); // increment minter allowance - await expectError(mintController.incrementMinterAllowance(amount, {from: Accounts.controller1Account}), "Can only increment allowance for minters in minterManager."); + await expectError(mintController.incrementMinterAllowance(amount, {from: Accounts.controller1Account}), "Can only increment allowance for minters in minterManager"); }); } -var testWrapper = require('./../TestWrapper'); +var testWrapper = require('../TestWrapper'); testWrapper.execute('MintController_Tests', run_tests); module.exports = { diff --git a/test/minting2/MintP0_ABITests.js b/test/minting2/MintP0_ABITests.js index 90d8116ae..9cb9bfe18 100644 --- a/test/minting2/MintP0_ABITests.js +++ b/test/minting2/MintP0_ABITests.js @@ -2,7 +2,7 @@ var MintController = artifacts.require('minting/MintController'); var MasterMinter = artifacts.require('minting/MasterMinter'); var FiatToken = artifacts.require('FiatTokenV1'); -var tokenUtils = require('./../TokenTestUtils.js'); +var tokenUtils = require('../TokenTestUtils.js'); var checkMINTp0 = tokenUtils.checkMINTp0; var expectError = tokenUtils.expectError; var bigZero = tokenUtils.bigZero; @@ -11,8 +11,8 @@ var solidityErrors = tokenUtils.solidityErrors; var clone = require('clone'); -var mintUtils = require('./../MintControllerUtils.js'); -var AccountUtils = require('./../AccountUtils.js'); +var mintUtils = require('../MintControllerUtils.js'); +var AccountUtils = require('../AccountUtils.js'); var Accounts = AccountUtils.Accounts; var AccountPrivateKeys = AccountUtils.AccountPrivateKeys; var getAccountState = AccountUtils.getAccountState; @@ -20,7 +20,7 @@ var MintControllerState = AccountUtils.MintControllerState; var initializeTokenWithProxyAndMintController = mintUtils.initializeTokenWithProxyAndMintController; var checkMintControllerState = mintUtils.checkMintControllerState; -var abiUtils = require('./../ABIUtils.js'); +var abiUtils = require('../ABIUtils.js'); var makeRawTransaction = abiUtils.makeRawTransaction; var sendRawTransaction = abiUtils.sendRawTransaction; var msgData = abiUtils.msgData; diff --git a/test/minting3/MintP0_BasicTests.js b/test/minting3/MintP0_BasicTests.js index 21722cc1e..9f2d9a302 100644 --- a/test/minting3/MintP0_BasicTests.js +++ b/test/minting3/MintP0_BasicTests.js @@ -2,7 +2,7 @@ var MintController = artifacts.require('minting/MintController'); var MasterMinter = artifacts.require('minting/MasterMinter'); var FiatToken = artifacts.require('FiatTokenV1'); -var tokenUtils = require('./../TokenTestUtils.js'); +var tokenUtils = require('../TokenTestUtils.js'); var newBigNumber = tokenUtils.newBigNumber; var checkMINTp0 = tokenUtils.checkMINTp0; var expectRevert = tokenUtils.expectRevert; @@ -13,8 +13,8 @@ var maxAmount = tokenUtils.maxAmount; var clone = require('clone'); -var mintUtils = require('./../MintControllerUtils.js'); -var AccountUtils = require('./../AccountUtils.js'); +var mintUtils = require('../MintControllerUtils.js'); +var AccountUtils = require('../AccountUtils.js'); var Accounts = AccountUtils.Accounts; var getAccountState = AccountUtils.getAccountState; var MintControllerState = AccountUtils.MintControllerState; @@ -105,7 +105,7 @@ async function run_MINT_tests(newToken, MintControllerArtifact, accounts) { }); it('bt010 removeMinter reverts when msg.sender is not a controller', async function () { - await expectError(mintController.removeMinter({from: Accounts.controller1Account}), "The value of controllers[msg.sender] must be non-zero."); + await expectError(mintController.removeMinter({from: Accounts.controller1Account}), "The value of controllers[msg.sender] must be non-zero"); }); it('bt011 removeMinter sets minters[M] to 0', async function () { @@ -129,7 +129,7 @@ async function run_MINT_tests(newToken, MintControllerArtifact, accounts) { }); it('bt012 configureMinter reverts when msg.sender is not a controller', async function () { - await expectError(mintController.configureMinter(50, {from: Accounts.controller1Account}), "The value of controllers[msg.sender] must be non-zero."); + await expectError(mintController.configureMinter(50, {from: Accounts.controller1Account}), "The value of controllers[msg.sender] must be non-zero"); }); it('bt013 configureMinter works when controllers[msg.sender]=M', async function () { @@ -148,7 +148,7 @@ async function run_MINT_tests(newToken, MintControllerArtifact, accounts) { }); it('bt014 incrementMinterAllowance reverts if msg.sender is not a controller', async function () { - await expectError(mintController.incrementMinterAllowance(50, {from: Accounts.controller1Account}), "The value of controllers[msg.sender] must be non-zero."); + await expectError(mintController.incrementMinterAllowance(50, {from: Accounts.controller1Account}), "The value of controllers[msg.sender] must be non-zero"); }); it('bt015 incrementMinterAllowance works when controllers[msg.sender]=M', async function () { @@ -171,9 +171,9 @@ async function run_MINT_tests(newToken, MintControllerArtifact, accounts) { await checkMINTp0([token, mintController], [expectedTokenState, expectedMintControllerState]); }); - it('bt017 removeController does not revert when controllers[C] is 0', async function () { + it('bt017 removeController reverts when controllers[C] is 0', async function () { // "remove" a controller that does not exist - await mintController.removeController(Accounts.controller1Account, {from: Accounts.mintOwnerAccount}); + await expectError(mintController.removeController(Accounts.controller1Account, {from: Accounts.mintOwnerAccount}), "Worker must be a non-zero address"); await checkMINTp0([token, mintController], [expectedTokenState, expectedMintControllerState]); }); @@ -414,7 +414,7 @@ async function run_MINT_tests(newToken, MintControllerArtifact, accounts) { var isMinter = await minterManager.isMinter(Accounts.minterAccount); assert.isFalse(isMinter); - await expectError(mintController.incrementMinterAllowance(amount, {from: Accounts.controller1Account}), "Can only increment allowance for minters in minterManager."); + await expectError(mintController.incrementMinterAllowance(amount, {from: Accounts.controller1Account}), "Can only increment allowance for minters in minterManager"); expectedMintControllerState.controllers['controller1Account'] = Accounts.minterAccount; await checkMINTp0([token, mintController], [expectedTokenState, expectedMintControllerState]); }); @@ -607,6 +607,6 @@ async function run_MINT_tests(newToken, MintControllerArtifact, accounts) { }); } -var testWrapper = require('./../TestWrapper'); +var testWrapper = require('../TestWrapper'); testWrapper.execute('MINTp0_BasicTests MintController', run_tests_MintController); testWrapper.execute('MINTp0_BasicTests MasterMinter', run_tests_MasterMinter); diff --git a/verification/Spreadsheets/MINTp0_ArgumentTests.csv b/verification/Spreadsheets/MINTp0_ArgumentTests.csv index 0912288d0..6bb969e48 100644 --- a/verification/Spreadsheets/MINTp0_ArgumentTests.csv +++ b/verification/Spreadsheets/MINTp0_ArgumentTests.csv @@ -6,7 +6,7 @@ Controller.sol,configureController,newController == 0,arg003,"arg003 configureCo Controller.sol,configureController,newController == msg.sender,arg004,"arg004 configureController(msg.sender, M) works" Controller.sol,configureController,newController == newMinter,arg005,"arg005 configureController(M, M) works" Controller.sol,configureController,newMinter == 0,arg006,"arg006 configureController(C, 0) throws" -Controller.sol,removeController,newController == 0,arg007,arg007 removeController(0) throws +Controller.sol,removeController,controller == 0,arg007,arg007 removeController(0) throws MintController.sol,setMinterManager,newMinterManager = 0,arg008,arg008 setMinterManager(0) works MintController.sol,setMinterManager,newMinterManager == minterManager,arg009,arg009 setMinterManager(oldMinterManager) works MintController.sol,setMinterManager,newMinterManager == user,arg010,arg010 setMinterManager(user_account) works @@ -19,4 +19,5 @@ MintController.sol,incrementMinterAllowance,increment == oldAllowance,arg016,arg MintController.sol,incrementMinterAllowance,increment == 2^256-1,arg017,arg017 incrementMinterAllowance(MAX) throws MintController.sol,incrementMinterAllowance,increment + oldAllowance == 2^256 -1,arg018,arg018 incrementMinterAlllowance(BIG) throws Controller.sol,configureController,newController == 0,arg019,"arg019 configureController(0, 0) throws" -Controller.sol,removeController,newController == msg.sender,arg020,arg020 removeController(C) works +Controller.sol,removeController,controller == msg.sender,arg020,arg020 removeController(C) works +Controller.sol,removeController,controller where controllers[controller] == 0,arg021,arg021 removeController throws if worker is already address(0) \ No newline at end of file diff --git a/verification/Spreadsheets/MINTp0_BasicTests.csv b/verification/Spreadsheets/MINTp0_BasicTests.csv index 11f0e73dc..4bdfedca4 100644 --- a/verification/Spreadsheets/MINTp0_BasicTests.csv +++ b/verification/Spreadsheets/MINTp0_BasicTests.csv @@ -9,7 +9,7 @@ Controller.sol,configureController,controllers[C] == newM,bt023,"bt023 configure Controller.sol,constructor,controllers[ALL] = 0,bt016,bt016 Constructor sets all controllers to 0 Controller.sol,removeController,msg.sender == owner,bt008,bt008 removeController works when owner is msg.sender Controller.sol,removeController,msg.sender != owner,bt009,bt009 removeController reverts when owner is not msg.sender -Controller.sol,removeController,controllers[C]=0,bt017,bt017 removeController does not revert when controllers[C] is 0 +Controller.sol,removeController,controllers[C]=0,bt017,bt017 removeController reverts when controllers[C] is 0 Controller.sol,removeController,controllers[C]=M,bt018,bt018 removeController removes an arbitrary controller FiatTokenV1.sol,constructor,minterManager.isMinter[ALL] == false,bt045,bt045 constructor - minterManager.isMinter[ALL] is false FiatTokenV1.sol,constructor,minterManager.minterAllowance[ALL] == 0,bt046,bt046 constructor - minterManager.minterAllowance[ALL] = 0