Merge pull request #109 from argentlabs/revert-95-feature/token-provider-remove-kyber

Revert "Remove Kyber price syncing from TokenPriceProvider"

Author: elenadimitrova

contracts/infrastructure/TokenPriceProvider.sol

@@ -17,12 +17,24 @@ pragma solidity ^0.5.4;
 import "../../lib/utils/SafeMath.sol";
 import "../../lib/other/ERC20.sol";
 import "../base/Managed.sol";
+import "../../lib/other/KyberNetwork.sol";
 
 contract TokenPriceProvider is Managed {
+
+    // Mock token address for ETH
+    address constant internal ETH_TOKEN_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
+
     using SafeMath for uint256;
 
     mapping(address => uint256) public cachedPrices;
 
+    // Address of the KyberNetwork contract
+    KyberNetwork public kyberNetwork;
+
+    constructor(KyberNetwork _kyberNetwork) public {
+        kyberNetwork = _kyberNetwork;
+    }
+
     function setPrice(ERC20 _token, uint256 _price) public onlyManager {
         cachedPrices[address(_token)] = _price;
     }
@@ -44,4 +56,26 @@ contract TokenPriceProvider is Managed {
         uint256 price = cachedPrices[_token];
         return price.mul(_amount).div(10**decimals);
     }
+
+    //
+    // The following is added to be backward-compatible with Argent's old backend
+    //
+
+    function setKyberNetwork(KyberNetwork _kyberNetwork) external onlyManager {
+        kyberNetwork = _kyberNetwork;
+    }
+
+    function syncPrice(ERC20 _token) external {
+        require(address(kyberNetwork) != address(0), "Kyber sync is disabled");
+        (uint256 expectedRate,) = kyberNetwork.getExpectedRate(_token, ERC20(ETH_TOKEN_ADDRESS), 10000);
+        cachedPrices[address(_token)] = expectedRate;
+    }
+
+    function syncPriceForTokenList(ERC20[] calldata _tokens) external {
+        require(address(kyberNetwork) != address(0), "Kyber sync is disabled");
+        for (uint16 i = 0; i < _tokens.length; i++) {
+            (uint256 expectedRate,) = kyberNetwork.getExpectedRate(_tokens[i], ERC20(ETH_TOKEN_ADDRESS), 10000);
+            cachedPrices[address(_tokens[i])] = expectedRate;
+        }
+    }
 }

deployment/2_deploy_contracts.js

@@ -43,7 +43,7 @@ const deploy = async (network) => {
   // Deploy the MultiSig
   const MultiSigWrapper = await deployer.deploy(MultiSig, {}, newConfig.multisig.threshold, newConfig.multisig.owners);
   // Deploy TokenPriceProvider
-  const TokenPriceProviderWrapper = await deployer.deploy(TokenPriceProvider);
+  const TokenPriceProviderWrapper = await deployer.deploy(TokenPriceProvider, {}, newConfig.Kyber.contract);
   // Deploy Module Registry
   const ModuleRegistryWrapper = await deployer.deploy(ModuleRegistry);
   // Deploy Compound Registry

deployment/7_upgrade_1_6.js

@@ -63,9 +63,7 @@ const deploy = async (network) => {
   await MakerRegistryWrapper.verboseWaitForTransaction(addCollateralTransaction, `Adding join adapter ${wethJoinAddress} to the MakerRegistry`);
   const changeMakerRegistryOwnerTx = await MakerRegistryWrapper.contract.changeOwner(config.contracts.MultiSigWallet, { gasPrice });
   await MakerRegistryWrapper.verboseWaitForTransaction(changeMakerRegistryOwnerTx, "Set the MultiSig as the owner of the MakerRegistry");
-
-  // Deploy new TokenPriceProvider instance
-  const TokenPriceProviderWrapper = await deployer.deploy(TokenPriceProvider);
+  const TokenPriceProviderWrapper = await deployer.wrapDeployedContract(TokenPriceProvider, config.contracts.TokenPriceProvider);
 
   // //////////////////////////////////
   // Deploy new modules
@@ -131,7 +129,6 @@ const deploy = async (network) => {
 
   configurator.updateInfrastructureAddresses({
     MakerRegistry: MakerRegistryWrapper.contractAddress,
-    TokenPriceProvider: TokenPriceProviderWrapper.contractAddress,
   });
 
   const gitHash = childProcess.execSync("git rev-parse HEAD").toString("utf8").replace(/\n$/, "");
@@ -144,7 +141,6 @@ const deploy = async (network) => {
     abiUploader.upload(MakerV2ManagerWrapper, "modules"),
     abiUploader.upload(TransferManagerWrapper, "modules"),
     abiUploader.upload(MakerRegistryWrapper, "contracts"),
-    abiUploader.upload(TokenPriceProviderWrapper, "contracts"),
   ]);
 
   // //////////////////////////////////
@@ -157,20 +153,6 @@ const deploy = async (network) => {
       [wrapper.contractAddress, utils.asciiToBytes32(wrapper._contract.contractName)]);
   }
 
-  // ////////////////////////////////////////////////////
-  // Change the owner of TokenPriceProvider
-  // ////////////////////////////////////////////////////
-
-  for (const idx in config.backend.accounts) {
-    const account = config.backend.accounts[idx];
-    const TokenPriceProviderAddManagerTx = await TokenPriceProviderWrapper.contract.addManager(account, { gasPrice });
-    await TokenPriceProviderWrapper.verboseWaitForTransaction(TokenPriceProviderAddManagerTx,
-      `Set ${account} as the manager of the TokenPriceProvider`);
-  }
-
-  const changeOwnerTx = await TokenPriceProviderWrapper.contract.changeOwner(config.contracts.MultiSigWallet, { gasPrice });
-  await TokenPriceProviderWrapper.verboseWaitForTransaction(changeOwnerTx, "Set the MultiSig as the owner of TokenPriceProvider");
-
   // //////////////////////////////////
   // Deploy and Register upgraders
   // //////////////////////////////////

package.json

@@ -14,7 +14,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 & COVERAGE=1 npx etherlime coverage --solcVersion 0.5.4 && istanbul check-coverage --statements 71 --branches 62 --functions 73 --lines 71",
+    "test:coverage": "bash ./scripts/provision_lib_artefacts.sh & COVERAGE=1 npx etherlime coverage --solcVersion 0.5.4 && istanbul check-coverage --statements 71 --branches 61 --functions 73 --lines 71",
     "lint:contracts": "npx ethlint --dir .",
     "lint:contracts:staged": "bash ./scripts/ethlint.sh",
     "test:deployment": "./scripts/deploy.sh ganache `seq 1 7`",

test/approvedTransfer.js

@@ -6,6 +6,7 @@ const Registry = require("../build/ModuleRegistry");
 const GuardianStorage = require("../build/GuardianStorage");
 const GuardianManager = require("../build/GuardianManager");
 const ApprovedTransfer = require("../build/ApprovedTransfer");
+const KyberNetwork = require("../build/KyberNetworkTest");
 const TokenPriceProvider = require("../build/TokenPriceProvider");
 const ERC20 = require("../build/TestERC20");
 const TestContract = require("../build/TestContract");
@@ -14,7 +15,7 @@ const TestManager = require("../utils/test-manager");
 const { sortWalletByAddress, parseRelayReceipt, ETH_TOKEN } = require("../utils/utilities.js");
 
 const DECIMALS = 12; // number of decimal for TOKN contract
-const TOKEN_RATE = 51 * 10 ** 13; // 1 TOKN = 0.00051 ETH
+const KYBER_RATE = 51 * 10 ** 13; // 1 TOKN = 0.00051 ETH
 
 const ZERO_BYTES32 = ethers.constants.HashZero;
 
@@ -38,24 +39,26 @@ describe("Approved Transfer", function () {
   let guardianManager;
   let approvedTransfer;
   let priceProvider;
+  let kyber;
   let erc20;
   const amountToTransfer = 10000;
 
   before(async () => {
     deployer = manager.newDeployer();
     const registry = await deployer.deploy(Registry);
     const guardianStorage = await deployer.deploy(GuardianStorage);
-    priceProvider = await deployer.deploy(TokenPriceProvider);
+    kyber = await deployer.deploy(KyberNetwork);
+    priceProvider = await deployer.deploy(TokenPriceProvider, {}, kyber.contractAddress);
     await priceProvider.addManager(infrastructure.address);
     guardianManager = await deployer.deploy(GuardianManager, {}, registry.contractAddress, guardianStorage.contractAddress, 24, 12);
     approvedTransfer = await deployer.deploy(ApprovedTransfer, {}, registry.contractAddress, guardianStorage.contractAddress);
   });
-
   beforeEach(async () => {
     wallet = await deployer.deploy(Wallet);
     await wallet.init(owner.address, [approvedTransfer.contractAddress, guardianManager.contractAddress]);
     erc20 = await deployer.deploy(ERC20, {}, [infrastructure.address, wallet.contractAddress], 10000000, DECIMALS); // TOKN contract with 10M tokens (5M TOKN for wallet and 5M TOKN for account[0])
-    await priceProvider.setPrice(erc20.contractAddress, TOKEN_RATE);
+    await kyber.addToken(erc20.contractAddress, KYBER_RATE, DECIMALS);
+    await priceProvider.syncPrice(erc20.contractAddress);
     await infrastructure.sendTransaction({ to: wallet.contractAddress, value: 50000000 });
   });
 

test/makerV2Manager_loan.js

@@ -100,7 +100,7 @@ describe("MakerV2 Vaults", function () {
     );
 
     // Deploy TransferManager
-    const priceProvider = await deployer.deploy(TokenPriceProvider);
+    const priceProvider = await deployer.deploy(TokenPriceProvider, {}, AddressZero);
     const transferStorage = await deployer.deploy(TransferStorage);
     transferManager = await deployer.deploy(TransferManager, {},
       AddressZero,

test/tokenExchanger.js

@@ -11,7 +11,7 @@ const ERC20 = require("../build/TestERC20");
 const { ETH_TOKEN } = require("../utils/utilities.js");
 
 const DECIMALS = 12; // number of decimal for TOKN contract
-const TOKEN_RATE = 51 * 10 ** 13; // 1 TOKN = 0.00051 ETH
+const KYBER_RATE = 51 * 10 ** 13; // 1 TOKN = 0.00051 ETH
 const FEE_RATIO = 30;
 
 const TestManager = require("../utils/test-manager");
@@ -43,7 +43,7 @@ describe("Token Exchanger", function () {
     wallet = await deployer.deploy(Wallet);
     await wallet.init(owner.address, [exchanger.contractAddress]);
     erc20 = await deployer.deploy(ERC20, {}, [kyber.contractAddress, wallet.contractAddress], 10000000, DECIMALS); // TOKN contract with 10M tokens (5M TOKN for wallet and 5M TOKN for kyber)
-    await kyber.addToken(erc20.contractAddress, TOKEN_RATE, DECIMALS);
+    await kyber.addToken(erc20.contractAddress, KYBER_RATE, DECIMALS);
     await infrastructure.sendTransaction({ to: wallet.contractAddress, value: 50000000 });
     await infrastructure.sendTransaction({ to: kyber.contractAddress, value: 50000000 });
   });

test/tokenPriceProvider.js

@@ -1,6 +1,7 @@
 /* global accounts */
 
 const ERC20 = require("../build/TestERC20");
+const KyberNetwork = require("../build/KyberNetworkTest");
 const TokenPriceProvider = require("../build/TokenPriceProvider");
 
 const TestManager = require("../utils/test-manager");
@@ -12,21 +13,30 @@ describe("Token Price Provider", () => {
   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);
+    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);

test/transferManager.js

@@ -7,6 +7,7 @@ const TransferStorage = require("../build/TransferStorage");
 const GuardianStorage = require("../build/GuardianStorage");
 const TransferModule = require("../build/TransferManager");
 const LegacyTransferManager = require("../build/LegacyTransferManager");
+const KyberNetwork = require("../build/KyberNetworkTest");
 const TokenPriceProvider = require("../build/TokenPriceProvider");
 const ERC20 = require("../build/TestERC20");
 const TestContract = require("../build/TestContract");
@@ -17,7 +18,7 @@ const ETH_LIMIT = 1000000;
 const SECURITY_PERIOD = 2;
 const SECURITY_WINDOW = 2;
 const DECIMALS = 12; // number of decimal for TOKN contract
-const TOKEN_RATE = ethers.utils.bigNumberify(51 * 10 ** 13); // 1 TOKN = 0.00051 ETH
+const KYBER_RATE = ethers.utils.bigNumberify(51 * 10 ** 13); // 1 TOKN = 0.00051 ETH
 const ZERO_BYTES32 = ethers.constants.HashZero;
 
 const ACTION_TRANSFER = 0;
@@ -36,6 +37,7 @@ describe("TransferManager", function () {
   const spender = accounts[4].signer;
 
   let deployer;
+  let kyber;
   let registry;
   let priceProvider;
   let transferStorage;
@@ -48,9 +50,8 @@ describe("TransferManager", function () {
   before(async () => {
     deployer = manager.newDeployer();
     registry = await deployer.deploy(Registry);
-    priceProvider = await deployer.deploy(TokenPriceProvider);
-    await priceProvider.addManager(infrastructure.address);
-
+    kyber = await deployer.deploy(KyberNetwork);
+    priceProvider = await deployer.deploy(TokenPriceProvider, {}, kyber.contractAddress);
     transferStorage = await deployer.deploy(TransferStorage);
     guardianStorage = await deployer.deploy(GuardianStorage);
 
@@ -81,7 +82,8 @@ describe("TransferManager", function () {
     wallet = await deployer.deploy(Wallet);
     await wallet.init(owner.address, [transferModule.contractAddress]);
     erc20 = await deployer.deploy(ERC20, {}, [infrastructure.address, wallet.contractAddress], 10000000, DECIMALS); // TOKN contract with 10M tokens (5M TOKN for wallet and 5M TOKN for account[0])
-    await priceProvider.setPrice(erc20.contractAddress, TOKEN_RATE);
+    await kyber.addToken(erc20.contractAddress, KYBER_RATE, DECIMALS);
+    await priceProvider.syncPrice(erc20.contractAddress);
     await infrastructure.sendTransaction({ to: wallet.contractAddress, value: ethers.utils.bigNumberify("1000000000000000000") });
   });
 

utils/config/ganache.json

@@ -1 +1 @@
-{"ENS":{"deployOwnRegistry":true,"ensRegistry":"0x9d6CfEc16741f08991654Bc36d1045A1D241F29e","domain":"argent.xyz"},"backend":{"accounts":["0xD9995BAE12FEe327256FFec1e3184d492bD94C31"]},"multisig":{"owners":["0xD9995BAE12FEe327256FFec1e3184d492bD94C31"],"threshold":1,"autosign":true},"settings":{"deployer":{"type":"ganache"},"lockPeriod":480,"recoveryPeriod":480,"securityPeriod":240,"securityWindow":240,"feeRatio":15,"defaultLimit":"1000000000000000000"},"Kyber":{"deployOwn":true,"contract":"0x81A1198d28a7491F1257B22DaB444a73ABCf9298"},"CryptoKitties":{"contract":"0x0000000000000000000000000000000000000000"},"defi":{"maker":{"deployOwn":true,"tub":"0x0000000000000000000000000000000000000000","pot":"0x0000000000000000000000000000000000000000","jug":"0x0000000000000000000000000000000000000000","migration":"0x09ce60Bd50333a737e03db957bd7f845b08DB095"},"uniswap":{"deployOwn":true,"factory":"0xb5d7Ef06dd742b62d71eb2CaBa47d320b4D2A578"},"compound":{"comptroller":"0x0000000000000000000000000000000000000000","markets":{"0x0000000000000000000000000000000000000000":"0x0000000000000000000000000000000000000000"}}},"contracts":{"MultiSigWallet":"0xE6192099D3489211a92be2645aDe339ad4998a8c","WalletFactory":"0x0d326c46DCb8a85242C80d3c2A1a426CD2328401","ENSResolver":"0xD629AF06612B631c5b370055dA1cB3FB46A1c392","ENSManager":"0xb7358663Ed8f0E0aF7b05482AF9E8016DE3b554F","TokenPriceProvider":"0x1C2878ABd6a189D5449e797D735545A9C08d9234","ModuleRegistry":"0x0c1bCf5DF51D47ebD980cBb870dF35dCE283e771","BaseWallet":"0xa9442f66E6bEF32c3d196B71e943406e4722326b","CompoundRegistry":"0x93e33b71aF68a10ACd863A5DF1D5D81719Ac15B2","MakerRegistry":"0x4576CA92833f8A1E99A33E1595cce9E5349dD19d"},"modules":{"GuardianStorage":"0xE9c77f070a5671fA32381ac826036eE3FDF5Ad56","TransferStorage":"0x73C62c291621e20799CA5D6Bf7b114F678ebE9Cb","GuardianManager":"0xd85C69EA74b1DBaBb40Eea240E5Cd2Ce4426856F","LockManager":"0x0eA33b7525189c49e8Da46AD99278480811fcA64","RecoveryManager":"0xC07f212AaEeF5Dc948BBC5302cB46D9D799417Ec","ApprovedTransfer":"0x84c01919c85CE8c0a43F7A6b95FC5B3cf6230D0a","TokenExchanger":"0x88A8e4a07fed276B6EeDB33449bfE9b0b5448B1F","NftTransfer":"0xD41288d45765e93ef63F58ac11CAa271BdA2bC21","MakerManager":"0x8A90941A8fB0B386eEEbFb2E1E96b54Af59ec29B","TransferManager":"0x39dCCe15d1a2c17c61B757C34cbe6aacF813Edc1","CompoundManager":"0xDCD324973848077F310E884CA7207b1Ecc67B493","MakerV2Manager":"0xFc31F3f732e9F1293ca3eb587871691bC8b28b82"},"gitCommit":"be87cf3131b90214644372a9331d20e64622ce03"}
+{"ENS":{"deployOwnRegistry":true,"ensRegistry":"0x274C3fB4B9A3f5836612C169ADEE957be8E1CA49","domain":"argent.xyz"},"backend":{"accounts":["0xD9995BAE12FEe327256FFec1e3184d492bD94C31"]},"multisig":{"owners":["0xD9995BAE12FEe327256FFec1e3184d492bD94C31"],"threshold":1,"autosign":true},"settings":{"deployer":{"type":"ganache"},"lockPeriod":480,"recoveryPeriod":480,"securityPeriod":240,"securityWindow":240,"feeRatio":15,"defaultLimit":"1000000000000000000"},"Kyber":{"deployOwn":true,"contract":"0xCE7A2788C4f14A3fe43c9d66eAe5C5b8ffd40240"},"CryptoKitties":{"contract":"0x0000000000000000000000000000000000000000"},"defi":{"maker":{"deployOwn":true,"tub":"0x0000000000000000000000000000000000000000","pot":"0x0000000000000000000000000000000000000000","jug":"0x0000000000000000000000000000000000000000","migration":"0xD2877b6409313680d2f7186f11CB584961135A62"},"uniswap":{"deployOwn":true,"factory":"0xB2533a6e92F0d638302aB6808FE3Ba6023cFEE08"},"compound":{"comptroller":"0x0000000000000000000000000000000000000000","markets":{"0x0000000000000000000000000000000000000000":"0x0000000000000000000000000000000000000000"}}},"contracts":{"MultiSigWallet":"0x118b5e4FD0aF29800CFeEC3B2E037fA7172a5A1C","WalletFactory":"0xe22c9dE7D0c258Fe64BcA9B19AF6ad9557C0a405","ENSResolver":"0xEf80C55AF72e8df28daed66B71137C7532B5c8Ea","ENSManager":"0x45a2Fe7F67a58fAEcc6109aB99304d7009B4D7F5","TokenPriceProvider":"0xdDD61b72d8019FEA5459ebD47d6bAc243e514fA2","ModuleRegistry":"0xf4DF8fe8c2b3882E9f5cC88008eE98e225c9753C","BaseWallet":"0x7A0E7cCae904066DD64968aE5fC857413D293e9d","CompoundRegistry":"0xeFf03c08398f4E744b365507D95355305B82ceab","MakerRegistry":"0x355d26E5f04255F60E910F1aA1251DE5D10996E8"},"modules":{"GuardianStorage":"0x5945751727ae5923AA39dC8Bd6a658925C53d300","TransferStorage":"0xebaf86d5dDdb586E19065e2D29dD5Ad9a31a8F31","GuardianManager":"0x2202723Bb717f362f64b6e41086A2cf52d9Ee28f","LockManager":"0xc1DA53758396f84198e78f6318904C2E666fC4Ea","RecoveryManager":"0xD6aC219C2b10a85D565ef8911e075464B5D53807","ApprovedTransfer":"0x11d3467345Ed6Ab494D131d42A45e469b2F7eB51","TokenExchanger":"0xA64fEeD186483380ad10496C4C9fADd89494633C","NftTransfer":"0x85D24B941A22D5DA687c607769173F01Abc61A56","MakerManager":"0x2718cAce241213B18F67C6d66B245EE77DB0Ce2a","TransferManager":"0xB58188A994fD291e2F97644b7FDb87372D27b2fD","CompoundManager":"0x1362D309a42165bC6dC00Be56D32625dF2C2c517","MakerV2Manager":"0x43f082E392cF13492a2660f932eFe22504Ab7517"},"gitCommit":"301c40ae9091b66f4faa30bbc5d20ae735febd3f"}