Code coverage improvements (TokenPriceProvider and base contracts). Disabling the KyberNetwork

deployment/7_upgrade_1_6.js

@@ -8,6 +8,7 @@ const Upgrader = require("../build/SimpleUpgrader");
 const DeployManager = require("../utils/deploy-manager.js");
 const MultisigExecutor = require("../utils/multisigexecutor.js");
 const LegacyUpgrader = require("../build/LegacyUpgrader");
+const TokenPriceProvider = require("../build/TokenPriceProvider");
 
 const utils = require("../utils/utilities.js");
 
@@ -35,6 +36,7 @@ const deploy = async (network) => {
   const deploymentWallet = deployer.signer;
   const { config } = configurator;
 
+  const TokenPriceProviderWrapper = await deployer.wrapDeployedContract(TokenPriceProvider, config.contracts.TokenPriceProvider);
   const ModuleRegistryWrapper = await deployer.wrapDeployedContract(ModuleRegistry, config.contracts.ModuleRegistry);
   const MultiSigWrapper = await deployer.wrapDeployedContract(MultiSig, config.contracts.MultiSigWallet);
   const multisigExecutor = new MultisigExecutor(MultiSigWrapper, deploymentWallet, config.multisig.autosign);
@@ -93,6 +95,18 @@ const deploy = async (network) => {
       [wrapper.contractAddress, utils.asciiToBytes32(wrapper._contract.contractName)]);
   }
 
+  // ////////////////////////////////////////////////////
+  // Disable KyberNetwork on the TokenPriceProvider
+  // ////////////////////////////////////////////////////
+
+  const TokenPriceProviderAddManagerTx = await TokenPriceProviderWrapper.contract.addManager(deploymentWallet);
+  await TokenPriceProviderWrapper.verboseWaitForTransaction(TokenPriceProviderAddManagerTx,
+    `Set ${deploymentWallet} as the manager of the TokenPriceProvider`);
+
+  const TokenPriceProviderSetKyberNetworkTx = await TokenPriceProviderWrapper.contract.setKyberNetwork("0x0000000000000000000000000000000000000000");
+  await TokenPriceProviderWrapper.verboseWaitForTransaction(TokenPriceProviderSetKyberNetworkTx,
+    "Disable the KyberNetwork on the TokenPriceProvider");
+
   // //////////////////////////////////
   // Deploy and Register upgraders
   // //////////////////////////////////

package.json

@@ -12,7 +12,7 @@
     "test": "npx etherlime test --skip-compilation",
     "ctest": "npm run compile && npm run test",
     "provision:lib:artefacts": "bash ./scripts/provision_lib_artefacts.sh",
-    "test:coverage": "bash ./scripts/provision_lib_artefacts.sh & npx etherlime coverage --solcVersion 0.5.4 && istanbul check-coverage --statements 74 --branches 62 --functions 77 --lines 74",
+    "test:coverage": "bash ./scripts/provision_lib_artefacts.sh & npx etherlime coverage --solcVersion 0.5.4 && istanbul check-coverage --statements 75 --branches 63 --functions 79 --lines 75",
     "lint:contracts": "npx ethlint --dir .",
     "lint:contracts:staged": "bash ./scripts/ethlint.sh",
     "test:deployment": "./scripts/deploy.sh ganache `seq 1 7`",

test/baseContracts.js

@@ -0,0 +1,105 @@
+/* global accounts */
+const ethers = require("ethers");
+const Managed = require("../build/Managed");
+
+const TestManager = require("../utils/test-manager");
+
+describe("Test Token Price Provider", () => {
+  const manager = new TestManager();
+
+  const infrastructure = accounts[0].signer;
+  const manager1 = accounts[1].signer;
+  const manager2 = accounts[2].signer;
+  const nonOwner = accounts[3].signer;
+
+  let deployer;
+  let managed;
+
+  before(async () => {
+    deployer = manager.newDeployer();
+  });
+
+  beforeEach(async () => {
+    managed = await deployer.deploy(Managed);
+  });
+
+  describe("Owned contract logic", () => {
+    it("should set owner to caller", async () => {
+      const owner = await managed.owner();
+      assert.equal(owner, infrastructure.address);
+    });
+
+    it("should be able to change owner", async () => {
+      const newOwner = accounts[1].signer;
+      await managed.changeOwner(newOwner.address);
+      const owner = await managed.owner();
+      assert.equal(owner, newOwner.address);
+    });
+
+    it("should not be able to change owner to zero address", async () => {
+      await assert.revertWith(managed.changeOwner(ethers.constants.AddressZero), "Address must not be null");
+    });
+  });
+
+  describe("Managed contract logic", () => {
+    it("should be able to add manager", async () => {
+      // Ensure the manager test accounts are not managers to start with
+      let isManager1 = await managed.managers(manager1.address);
+      assert.isFalse(isManager1);
+      let isManager2 = await managed.managers(manager2.address);
+      assert.isFalse(isManager2);
+
+      // Add managers
+      await managed.addManager(manager1.address);
+      await managed.addManager(manager2.address);
+
+      isManager1 = await managed.managers(manager1.address);
+      assert.isTrue(isManager1);
+      isManager2 = await managed.managers(manager2.address);
+      assert.isTrue(isManager2);
+    });
+
+    it("should not be able to add manager if not called by owner", async () => {
+      await assert.revertWith(managed.from(nonOwner.address).addManager(manager1.address), "Must be owner");
+    });
+
+    it("should not be able to set manager to zero address", async () => {
+      await assert.revertWith(managed.addManager(ethers.constants.AddressZero), "M: Address must not be null");
+    });
+
+    it("should be able to set manager twice without error", async () => {
+      // Set manager once
+      await managed.addManager(manager1.address);
+      let isManager1 = await managed.managers(manager1.address);
+      assert.isTrue(isManager1);
+
+      // Set manager twice
+      await managed.addManager(manager1.address);
+      isManager1 = await managed.managers(manager1.address);
+      assert.isTrue(isManager1);
+    });
+
+    it("should be able to revoke manager", async () => {
+      // Add managers
+      await managed.addManager(manager1.address);
+      await managed.addManager(manager2.address);
+
+      // Revoke only the second manager
+      await managed.revokeManager(manager2.address);
+
+      const isManager1 = await managed.managers(manager1.address);
+      assert.isTrue(isManager1);
+      const isManager2 = await managed.managers(manager2.address);
+      assert.isFalse(isManager2);
+    });
+
+    it("should not be able to revoke manager if not called by owner", async () => {
+      await managed.addManager(manager1.address);
+      await assert.revertWith(managed.from(nonOwner.address).revokeManager(manager1.address), "Must be owner");
+    });
+
+    it("should not be able to revoke a nonexisting managerr", async () => {
+      await assert.revertWith(managed.revokeManager(manager2.address), "M: Target must be an existing manager");
+    });
+  });
+});

test/tokenPriceProvider.js

@@ -0,0 +1,61 @@
+/* global accounts */
+
+const ERC20 = require("../build/TestERC20");
+const KyberNetwork = require("../build/KyberNetworkTest");
+const TokenPriceProvider = require("../build/TokenPriceProvider");
+
+const TestManager = require("../utils/test-manager");
+
+describe("Test Token Price Provider", () => {
+  const manager = new TestManager();
+
+  const infrastructure = accounts[0].signer;
+  const priceProviderManager = accounts[1].signer;
+
+  let deployer;
+  let kyber;
+  let priceProvider;
+  let erc20First;
+  let erc20Second;
+
+  before(async () => {
+    deployer = manager.newDeployer();
+    kyber = await deployer.deploy(KyberNetwork);
+  });
+
+  beforeEach(async () => {
+    priceProvider = await deployer.deploy(TokenPriceProvider, {}, kyber.contractAddress);
+    await priceProvider.addManager(priceProviderManager.address);
+    erc20First = await deployer.deploy(ERC20, {}, [infrastructure.address], 10000000, 18);
+    erc20Second = await deployer.deploy(ERC20, {}, [infrastructure.address], 10000000, 18);
+  });
+
+  it("should be able to set the Kyber network contract", async () => {
+    const kyberNew = await deployer.deploy(KyberNetwork);
+    await priceProvider.from(priceProviderManager).setKyberNetwork(kyberNew.contractAddress);
+    const kyberContract = await priceProvider.kyberNetwork();
+    assert.equal(kyberNew.contractAddress, kyberContract);
+  });
+
+  describe("Reading and writing token prices", () => {
+    it("should set token price correctly", async () => {
+      await priceProvider.from(priceProviderManager).setPrice(erc20First.contractAddress, 1800);
+      const tokenPrice = await priceProvider.cachedPrices(erc20First.contractAddress);
+      assert.equal(tokenPrice.toNumber(), 1800);
+    });
+
+    it("should set multiple token prices correctly", async () => {
+      await priceProvider.from(priceProviderManager).setPriceForTokenList([erc20First.contractAddress, erc20Second.contractAddress], [1800, 1900]);
+      const tokenPrice1 = await priceProvider.cachedPrices(erc20First.contractAddress);
+      assert.equal(tokenPrice1.toNumber(), 1800);
+      const tokenPrice2 = await priceProvider.cachedPrices(erc20Second.contractAddress);
+      assert.equal(tokenPrice2.toNumber(), 1900);
+    });
+
+    it("should be able to get the ether value of a given amount of tokens", async () => {
+      await priceProvider.from(priceProviderManager).setPrice(erc20First.contractAddress, 1800);
+      const etherValue = await priceProvider.getEtherValue("15000000000000000000", erc20First.contractAddress);
+      assert.isTrue(etherValue.eq(1800 * 15));
+    });
+  });
+});