diff --git a/contracts/controller/Controller.sol b/contracts/controller/Controller.sol index 16b4303f..7ddf13f0 100644 --- a/contracts/controller/Controller.sol +++ b/contracts/controller/Controller.sol @@ -329,18 +329,18 @@ contract Controller is Initializable { onlyUpgradingScheme returns(bool) { - require(newController == address(0)); // so the upgrade could be done once for a contract. - require(_newController != address(0)); + require(newController == address(0), "this controller was already upgraded"); // so the upgrade could be done once for a contract. + require(_newController != address(0), "new controller cannot be 0"); newController = _newController; avatar.transferOwnership(_newController); - require(avatar.owner() == _newController); + require(avatar.owner() == _newController, "failed to transfer avatar ownership to the new controller"); if (nativeToken.owner() == address(this)) { nativeToken.transferOwnership(_newController); - require(nativeToken.owner() == _newController); + require(nativeToken.owner() == _newController, "failed to transfer token ownership to the new controller"); } if (nativeReputation.owner() == address(this)) { nativeReputation.transferOwnership(_newController); - require(nativeReputation.owner() == _newController); + require(nativeReputation.owner() == _newController, "failed to transfer reputation ownership to the new controller"); } emit UpgradeController(address(this), newController); return true; diff --git a/contracts/utils/DAOFactory.sol b/contracts/utils/DAOFactory.sol index 65e27613..4a26d715 100644 --- a/contracts/utils/DAOFactory.sol +++ b/contracts/utils/DAOFactory.sol @@ -99,7 +99,7 @@ contract DAOFactory is Initializable { require(locks[address(_avatar)].sender == msg.sender, "sender is not holding the lock"); // Mint token and reputation for founders: for (uint256 i = 0; i < _founders.length; i++) { - require(_founders[i] != address(0)); + require(_founders[i] != address(0), "founder address cannot be 0"); if (_foundersTokenAmount[i] > 0) { Controller( _avatar.owner()).mintTokens(_foundersTokenAmount[i], _founders[i]); diff --git a/test/app.js b/test/app.js index 8d4737c6..12105359 100644 --- a/test/app.js +++ b/test/app.js @@ -90,8 +90,37 @@ contract('App', accounts => { it("addVersion", async function() { await setup(); - var tx = await registration.packageInstance.addVersion([1,0,0],registration.implementationDirectory.address,helpers.NULL_HASH); + var tx = await registration.packageInstance.addVersion([1,1,0],registration.implementationDirectory.address,helpers.NULL_HASH); assert.equal(tx.logs[0].event, "VersionAdded"); assert.equal(tx.logs[0].args.semanticVersion[0], 1); + + try { + await registration.packageInstance.addVersion([1,2,0],helpers.NULL_ADDRESS,helpers.NULL_HASH); + assert(false, "contract address cannot be 0"); + } catch(error) { + helpers.assertVMException(error); + } + try { + await registration.packageInstance.addVersion([1,1,0],registration.implementationDirectory.address,helpers.NULL_HASH); + assert(false, "cannot register same version twice"); + } catch(error) { + helpers.assertVMException(error); + } + try { + await registration.packageInstance.addVersion([0,0,0],registration.implementationDirectory.address,helpers.NULL_HASH); + assert(false, "version cannot be 0"); + } catch(error) { + helpers.assertVMException(error); + } + + tx = await registration.packageInstance.addVersion([1,0,0],registration.implementationDirectory.address,helpers.NULL_HASH); + assert.equal(tx.logs[0].event, "VersionAdded"); + assert.equal(tx.logs[0].args.semanticVersion[0], 1); + assert.equal(tx.logs[0].args.semanticVersion[1], 0); + + let latest = await registration.packageInstance.getLatest(); + assert.equal(latest.semanticVersion[0], 1); + assert.equal(latest.semanticVersion[1], 1); + assert.equal(latest.semanticVersion[2], 0); }); }); diff --git a/test/controller.js b/test/controller.js index d8e68055..7a1a50c1 100644 --- a/test/controller.js +++ b/test/controller.js @@ -62,6 +62,17 @@ contract('Controller', accounts => { assert.equal(rep,amountToMint); }); + it("mint reputation - only registered scheme", async () => { + controller = await setup(accounts); + await reputation.transferOwnership(controller.address); + try { + await controller.mintReputation(amountToMint,accounts[0],{from:accounts[1]}); + assert(false, 'only registered scheme can mint reputation'); + } catch (ex) { + helpers.assertVMException(ex); + } + }); + it("burn reputation via controller", async () => { controller = await setup(accounts); await reputation.transferOwnership(controller.address); @@ -214,6 +225,17 @@ contract('Controller', accounts => { assert.equal(count[0], 1); //pre assert.equal(count[1], 1); //post }); + + it("globalConstraint - cannot register without sufficient permissions", async () => { + controller = await setup(accounts, "0x00000001"); + try { + await controller.addGlobalConstraint(helpers.NULL_ADDRESS); + assert(false, 'only scheme with sufficient permission can add global constraint'); + } catch (ex) { + helpers.assertVMException(ex); + } + }); + it("removeGlobalConstraint ", async () => { const zeroBytes32 = "0x0000000000000000000000000000000000000000"; controller = await setup(accounts); @@ -255,11 +277,43 @@ contract('Controller', accounts => { assert.equal(gcCount[1],3); }); - it("upgrade controller ", async () => { + it("upgrade controller", async () => { controller = await setup(accounts); await reputation.transferOwnership(controller.address); await token.transferOwnership(controller.address); + + try { + await controller.upgradeController(accounts[1]); + assert(false, 'cannot upgrade controller if it does not ow the avatar'); + } catch (ex) { + helpers.assertVMException(ex); + } + await avatar.transferOwnership(controller.address); + + try { + await controller.upgradeController(helpers.NULL_ADDRESS); + assert(false, 'new controller cannot be 0'); + } catch (ex) { + helpers.assertVMException(ex); + } + + var tx = await controller.upgradeController(accounts[1]); + assert.equal(tx.logs.length, 1); + assert.equal(tx.logs[0].event, "UpgradeController"); + + try { + await controller.upgradeController(accounts[2]); + assert(false, 'cannot upgrade twice the same controller contract'); + } catch (ex) { + helpers.assertVMException(ex); + } + }); + + it("upgrade controller - no native token and reputation", async () => { + controller = await setup(accounts); + await avatar.transferOwnership(controller.address); + var tx = await controller.upgradeController(accounts[1]); assert.equal(tx.logs.length, 1); assert.equal(tx.logs[0].event, "UpgradeController"); @@ -310,6 +364,22 @@ contract('Controller', accounts => { assert.equal(result[1], 14); }); + it("generic call - only generic call scheme", async () => { + controller = await setup(accounts,'0x00000001'); + await avatar.transferOwnership(controller.address); + let actionMock = await ActionMock.new(); + let a = 7; + let b = actionMock.address; + let c = "0x1234"; + const encodeABI = await new web3.eth.Contract(actionMock.abi).methods.test(a,b,c).encodeABI(); + try { + await controller.genericCall.call(actionMock.address,encodeABI,0); + assert(false, 'only registered scheme can mint reputation'); + } catch (ex) { + helpers.assertVMException(ex); + } + }); + it("generic call withoutReturnValue", async () => { controller = await setup(accounts,'0x00000010'); await avatar.transferOwnership(controller.address); @@ -623,5 +693,14 @@ contract('Controller', accounts => { }); - + it("metaData - cannot set metaData without sufficient permissions", async () => { + controller = await setup(accounts, "0x00000001"); + await avatar.transferOwnership(controller.address); + try { + await controller.metaData('newMetadata'); + assert(false, 'only scheme with sufficient permission can set metadata'); + } catch (ex) { + helpers.assertVMException(ex); + } + }); }); diff --git a/test/daofactory.js b/test/daofactory.js index 65e74698..bc095617 100644 --- a/test/daofactory.js +++ b/test/daofactory.js @@ -228,6 +228,14 @@ contract('DaoFactory', function(accounts) { helpers.assertVMException(ex); } + try { + await registration.daoFactory.forgeOrg("testOrg",nativeTokenData,[accounts[0]],[],[11],[0,0,0],{gas:constants.ARC_GAS_LIMIT}); + assert(false,"should revert because token array size is 0"); + } + catch(ex){ + helpers.assertVMException(ex); + } + try { await registration.daoFactory.forgeOrg("testOrg", nativeTokenData,[accounts[0], @@ -236,7 +244,7 @@ contract('DaoFactory', function(accounts) { [amountToMint,amountToMint], [0,0,0], {gas:constants.ARC_GAS_LIMIT}); - assert(false,"should revert because account is 0"); + assert(false,"should revert because account is 0"); } catch(ex){ helpers.assertVMException(ex); @@ -287,6 +295,63 @@ contract('DaoFactory', function(accounts) { assert.equal(tx.logs[2].args._avatar, avatar.address); }); + + it("setSchemes to SchemeMock and addFounders wrong lengths", async function() { + var amountToMint = 10; + await setup(accounts,amountToMint,amountToMint); + var foundersArray = []; + var founderReputation = []; + var founderToken = []; + + try { + await registration.daoFactory.addFounders(avatar.address,foundersArray,founderReputation,founderToken,{gas:constants.ARC_GAS_LIMIT}); + assert(false, "should revert because founders list is empty"); + } + catch(ex){ + helpers.assertVMException(ex); + } + + var i; + var numberOfFounders = 60; + for (i=0;i { assert.equal(tx.logs[0].args._sender,accounts[0]); assert.equal(await testSetup.org.reputation.balanceOf(accounts[0]),1000); assert.equal(await testSetup.org.reputation.balanceOf(accounts[1]),expected); + + try { + await testSetup.reputationFromToken.redeem(accounts[1]); + assert(false, "cannot redeem twice"); + } catch(error) { + helpers.assertVMException(error); + } }); it("redeemWithSignature", async () => { @@ -225,6 +232,16 @@ contract('ReputationFromToken and RepAllocation', accounts => { assert.equal(await testSetup.org.reputation.balanceOf(accounts[1]),0); }); + it("cannot redeem before initialize", async () => { + try { + let reputationFromToken = await ReputationFromToken.new(); + await reputationFromToken.redeem(accounts[1]); + assert(false, "cannot redeem before initialize"); + } catch(error) { + helpers.assertVMException(error); + } + }); + it("cannot initialize twice", async () => { let testSetup = await setup(accounts); try { diff --git a/test/schemeregistrar.js b/test/schemeregistrar.js index fd56a18e..a4b9c933 100644 --- a/test/schemeregistrar.js +++ b/test/schemeregistrar.js @@ -109,6 +109,26 @@ contract('SchemeRegistrar', accounts => { assert.equal(tx.logs[0].event, "RemoveSchemeProposal"); }); + it("proposeScheme cannot be 0", async function() { + var testSetup = await setup(accounts); + try { + await testSetup.schemeRegistrar.proposeScheme( + helpers.NULL_ADDRESS, + "0x00000000", + helpers.NULL_HASH); + } catch(ex) { + helpers.assertVMException(ex); + } + }); + + it("proposeToRemoveScheme cannot be 0", async function() { + var testSetup = await setup(accounts); + try { + await testSetup.schemeRegistrar.proposeToRemoveScheme(helpers.NULL_ADDRESS, helpers.NULL_HASH); + } catch(ex) { + helpers.assertVMException(ex); + } + }); it("execute proposeScheme and execute -yes - fee > 0 ", async function() { var testSetup = await setup(accounts); diff --git a/test/voteinorganization.js b/test/voteinorganization.js index 15d9f4ec..408011b4 100644 --- a/test/voteinorganization.js +++ b/test/voteinorganization.js @@ -94,28 +94,94 @@ contract('VoteInOrganizationScheme', accounts => { helpers.etherForEveryone(accounts); }); - it("proposeVote log", async function() { - var testSetup = await setup(accounts); + it("proposeVote log", async function() { + var testSetup = await setup(accounts); - var anotherTestSetup = await setup(accounts); - var absoluteVoteExecuteMock = await AbsoluteVoteExecuteMock.new(); - await absoluteVoteExecuteMock.initialize(testSetup.org.reputation.address, - anotherTestSetup.voteInOrganizationParams.votingMachine.absoluteVote.address); + var anotherTestSetup = await setup(accounts); + var absoluteVoteExecuteMock = await AbsoluteVoteExecuteMock.new(); + await absoluteVoteExecuteMock.initialize(testSetup.org.reputation.address, + anotherTestSetup.voteInOrganizationParams.votingMachine.absoluteVote.address); - var tx = await absoluteVoteExecuteMock.propose(2, - anotherTestSetup.voteInOrganizationParams.votingMachine.params, - anotherTestSetup.org.avatar.address, - accounts[0],helpers.NULL_ADDRESS); + var tx = await absoluteVoteExecuteMock.propose(2, + anotherTestSetup.voteInOrganizationParams.votingMachine.params, + anotherTestSetup.org.avatar.address, + accounts[0],helpers.NULL_ADDRESS); - const proposalId = await helpers.getProposalId(tx,anotherTestSetup.voteInOrganizationParams.votingMachine.absoluteVote, 'NewProposal'); - tx = await testSetup.voteInOrganization.proposeVote( - anotherTestSetup.voteInOrganizationParams.votingMachine.absoluteVote.address, - proposalId,1,helpers.NULL_HASH); - assert.equal(tx.logs.length, 1); - assert.equal(tx.logs[0].event, "NewVoteProposal"); - assert.equal(tx.logs[0].args._vote, 1); - }); + const proposalId = await helpers.getProposalId(tx,anotherTestSetup.voteInOrganizationParams.votingMachine.absoluteVote, 'NewProposal'); + tx = await testSetup.voteInOrganization.proposeVote( + anotherTestSetup.voteInOrganizationParams.votingMachine.absoluteVote.address, + proposalId,1,helpers.NULL_HASH); + assert.equal(tx.logs.length, 1); + assert.equal(tx.logs[0].event, "NewVoteProposal"); + assert.equal(tx.logs[0].args._vote, 1); + }); + + it("proposeVote vote not an option", async function() { + var testSetup = await setup(accounts); + + var anotherTestSetup = await setup(accounts); + var absoluteVoteExecuteMock = await AbsoluteVoteExecuteMock.new(); + await absoluteVoteExecuteMock.initialize(testSetup.org.reputation.address, + anotherTestSetup.voteInOrganizationParams.votingMachine.absoluteVote.address); + + var tx = await absoluteVoteExecuteMock.propose(2, + anotherTestSetup.voteInOrganizationParams.votingMachine.params, + anotherTestSetup.org.avatar.address, + accounts[0],helpers.NULL_ADDRESS); + + const proposalId = await helpers.getProposalId(tx,anotherTestSetup.voteInOrganizationParams.votingMachine.absoluteVote, 'NewProposal'); + try { + await testSetup.voteInOrganization.proposeVote( + anotherTestSetup.voteInOrganizationParams.votingMachine.absoluteVote.address, + proposalId,3,helpers.NULL_HASH + ); + assert(false, "vote not an option"); + } catch(error) { + helpers.assertVMException(error); + } + }); + + it("proposeVote vote not in range", async function() { + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,0,true,standardTokenMock.address); + + var anotherTestSetup = await setup(accounts,0,true,standardTokenMock.address); + var reputation = await Reputation.new(); + await reputation.initialize(accounts[0]); + await reputation.mint(testSetup.org.avatar.address,100); + var genesisProtocolCallbacksMock = await GenesisProtocolCallbacksMock.new(); + await genesisProtocolCallbacksMock.initialize(reputation.address, + standardTokenMock.address, + anotherTestSetup.voteInOrganizationParams.votingMachine.genesisProtocol.address); + await reputation.transferOwnership(genesisProtocolCallbacksMock.address); + var tx = await genesisProtocolCallbacksMock.propose(2, + anotherTestSetup.voteInOrganizationParams.votingMachine.params, + anotherTestSetup.org.avatar.address, + accounts[0], + helpers.NULL_ADDRESS); + var originalProposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + try { + await testSetup.voteInOrganization.proposeVote( + anotherTestSetup.voteInOrganizationParams.votingMachine.genesisProtocol.address, + originalProposalId,3,helpers.NULL_HASH + ); + assert(false, "vote out of range (too high)"); + } catch(error) { + helpers.assertVMException(error); + } + + try { + await testSetup.voteInOrganization.proposeVote( + anotherTestSetup.voteInOrganizationParams.votingMachine.genesisProtocol.address, + originalProposalId,0,helpers.NULL_HASH + ); + assert(false, "vote out of range (too low)"); + } catch(error) { + helpers.assertVMException(error); + } + }); + it("execute proposeVote -no decision - proposal data delete", async function() { var testSetup = await setup(accounts);