[WIP] More tests (#744)

* DAOfactory test coverage

* lint

* Update reputationfromtoken.js

* vote in organization test

* App tests

* Fix voteinorganization

* More controller tests

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;

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]);

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);
     });
 });

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);
+      }
+     });
 });

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<numberOfFounders;i++) {
+        foundersArray[i] = accounts[1];
+        founderReputation[i] = 1;
+        founderToken[i] = 1;
+
+      }
+      founderToken[i] = 1;
+      try {
+        await registration.daoFactory.addFounders(avatar.address,foundersArray,founderReputation,founderToken,{gas:constants.ARC_GAS_LIMIT});
+        assert(false, "should revert because founders list is shorter than tokens");
+      } catch(ex) {
+        helpers.assertVMException(ex);
+      }
+      founderToken.pop();
+      founderReputation[i] = 1;
+      try {
+        await registration.daoFactory.addFounders(avatar.address,foundersArray,founderReputation,founderToken,{gas:constants.ARC_GAS_LIMIT});
+        assert(false, "should revert because founders list is shorter than reputations");
+      } catch(ex) {
+        helpers.assertVMException(ex);
+      }
+      founderReputation.pop();
+      foundersArray[i-1] = helpers.NULL_ADDRESS;
+      try {
+        await registration.daoFactory.addFounders(avatar.address,foundersArray,founderReputation,founderToken,{gas:constants.ARC_GAS_LIMIT});
+        assert(false, "should revert because founder address cannot be 0");
+      } catch(ex) {
+        helpers.assertVMException(ex);
+      }
+      foundersArray[i-1] = accounts[1];
+      founderReputation[i-1] = 0;
+      founderToken[i-1] = 0;
+      await registration.daoFactory.addFounders(avatar.address,foundersArray,founderReputation,founderToken,{gas:constants.ARC_GAS_LIMIT});
+      var rep = await reputation.balanceOf(accounts[1],{from:accounts[1]});
+      assert.equal(rep.toNumber(),numberOfFounders - 1);
+      var founderBalance = await daoToken.balanceOf(accounts[1],{from:accounts[1]});
+      assert.equal(founderBalance.toNumber(),numberOfFounders - 1);
+    });
+
       it("forgeOrg different version", async function() {
           var amountToMint = 10;
           await setup(accounts,amountToMint,amountToMint);

test/reputationfromtoken.js

@@ -192,6 +192,13 @@ contract('ReputationFromToken and RepAllocation', accounts => {
       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 {

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);