Change multicall error validation behaviour

Change error validation behaviour to prevent entire observable stream from erroring giving errorred values an opportunity to update with valid values at a later time

packages/dai-plugin-mcd/src/schemas/_validators.js

@@ -8,7 +8,13 @@ export const tag = (strings, ...keys) => (...values) => {
 };
 
 export const validateAddress = (...args) => address =>
-  !/^0x[0-9a-fA-F]{40}$/.test(address) && tag(...args)({ address });
+  (!/^0x[0-9a-fA-F]{40}$/.test(address) ||
+    address === '0x0000000000000000000000000000000000000000') &&
+  tag(...args)({ address: address === null ? '(null)' : address });
 
-export const validatProxyAddressResult = (...args) => (proxyAddress, [address]) =>
-  proxyAddress === '0x0000000000000000000000000000000000000000' && tag(...args)({ address }); // prettier-ignore
+export const validateVaultId = id =>
+  !/^\d+$/.test(id) &&
+  `Invalid vault id: must be a positive integer. Received ${id}`;
+
+export const validateVaultTypeResult = vaultType =>
+  !vaultType && 'Vault does not exist';

packages/dai-plugin-mcd/src/schemas/cdpManager.js

@@ -7,6 +7,7 @@ import {
   VAULTS_CREATED,
   VAULT_OWNER
 } from './_constants';
+import { validateVaultId, validateVaultTypeResult } from './_validators';
 
 export const cdpManagerUrns = {
   generate: id => ({
@@ -17,13 +18,6 @@ export const cdpManagerUrns = {
   returns: [VAULT_ADDRESS]
 };
 
-const validateVaultId = id =>
-  !/^\d+$/.test(id) &&
-  `Invalid vault id: must be a positive integer. Received ${id}`;
-
-const validateVaultTypeResult = vaultType =>
-  !vaultType && 'Vault does not exist';
-
 export const cdpManagerIlks = {
   generate: id => ({
     id: `CDP_MANAGER.ilks(${id})`,

packages/dai-plugin-mcd/src/schemas/computed.js

@@ -54,7 +54,7 @@ import {
   COLLATERAL_TYPE_COLLATERALIZATION,
   COLLATERAL_TYPE_DATA
 } from './_constants';
-import { validateAddress } from './_validators';
+import { validateAddress, validateVaultId } from './_validators';
 
 export const collateralTypePrice = {
   generate: collateralTypeName => ({
@@ -106,8 +106,7 @@ export const vaultTypeAndAddress = {
 
 export const vaultExternalOwner = {
   generate: id => ({
-    dependencies: [[PROXY_OWNER, [VAULT_OWNER, id]]],
-    // TODO: throw error if no owner (DSProxy contract doesn't exist)
+    dependencies: [[PROXY_OWNER, [VAULT_OWNER, id]], [VAULT_OWNER, id]],
     computed: owner => owner
   })
 };
@@ -381,7 +380,10 @@ export const vault = {
           .toNumber();
       }
     })
-  })
+  }),
+  validate: {
+    args: validateVaultId
+  }
 };
 
 export const daiLockedInDsr = {
@@ -395,7 +397,7 @@ export const daiLockedInDsr = {
     }
   }),
   validate: {
-    args: validateAddress`Invalid address: ${'address'}`
+    args: validateAddress`Invalid address for daiLockedInDsr: ${'address'}`
   }
 };
 
@@ -461,7 +463,7 @@ export const savings = {
     })
   }),
   validate: {
-    args: validateAddress`Invalid address: ${'address'}`
+    args: validateAddress`Invalid address for savings: ${'address'}`
   }
 };
 
@@ -491,7 +493,7 @@ export const userVaultsList = {
       )
   }),
   validate: {
-    args: validateAddress`Invalid address: ${'address'}`
+    args: validateAddress`Invalid address for userVaultsList: ${'address'}`
   }
 };
 

packages/dai-plugin-mcd/src/schemas/getCdps.js

@@ -3,6 +3,7 @@ import {
   USER_VAULT_ADDRESSES,
   USER_VAULT_TYPES
 } from './_constants';
+import { validateAddress } from './_validators';
 import { bytesToString } from '../utils';
 
 export const getCdps = {
@@ -17,6 +18,10 @@ export const getCdps = {
       proxyAddress
     ]
   }),
+  validate: {
+    args: (_, address) =>
+      validateAddress`Invalid address for getCdps: ${'address'}`(address)
+  },
   returns: [
     [USER_VAULT_IDS, v => v.map(n => n.toNumber())],
     [USER_VAULT_ADDRESSES],

packages/dai-plugin-mcd/src/schemas/proxyRegistry.js

@@ -1,16 +1,19 @@
 import { PROXY_ADDRESS } from './_constants';
-// import { validateAddress, validatProxyAddressResult } from './_validators';
+import { validateAddress } from './_validators';
 
 export const proxyRegistryProxies = {
   generate: address => ({
     id: `PROXY_REGISTRY.proxies(${address})`,
     contract: 'PROXY_REGISTRY',
-    call: ['proxies(address)(address)', address]
+    call: ['proxies(address)(address)', address],
+    transforms: {
+      [PROXY_ADDRESS]: v =>
+        v === '0x0000000000000000000000000000000000000000' ? null : v
+    }
   }),
-  // validate: {
-  //   args: validateAddress`Invalid address: ${'address'}`,
-  //   [PROXY_ADDRESS]: validatProxyAddressResult`No proxy found for account address: ${'address'}`
-  // },
+  validate: {
+    args: validateAddress`Invalid address for proxyAddress: ${'address'}`
+  },
   returns: [[PROXY_ADDRESS]]
 };
 

packages/dai-plugin-mcd/src/schemas/token.js

@@ -2,6 +2,7 @@ import { getMcdToken } from '../utils';
 import BigNumber from 'bignumber.js';
 
 import { TOKEN_BALANCE, TOKEN_ALLOWANCE_BASE } from './_constants';
+import { validateAddress } from './_validators';
 
 export const ALLOWANCE_AMOUNT = BigNumber(
   '0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff'
@@ -76,7 +77,16 @@ export const tokenAllowance = {
         : [TOKEN_ALLOWANCE_BASE, address, proxyAddress, symbol]
     ],
     computed: v => v
-  })
+  }),
+  validate: {
+    args: (address, proxyAddress) =>
+      validateAddress`Invalid address for tokenAllowance: ${'address'}`(
+        address
+      ) ||
+      validateAddress`Invalid proxy address for tokenAllowance: ${'address'}`(
+        proxyAddress
+      )
+  }
 };
 
 export const adapterBalance = {

packages/dai-plugin-mcd/test/schemas/cat.spec.js

@@ -33,9 +33,8 @@ test(LIQUIDATOR_ADDRESS, async () => {
   const address = await maker.latest(LIQUIDATOR_ADDRESS, 'ETH-A');
   expect(address.toLowerCase()).toEqual(expected);
 
-  expect(() => {
-    maker.latest(LIQUIDATOR_ADDRESS, null);
-  }).toThrow(/invalid/i);
+  const promise = maker.latest(LIQUIDATOR_ADDRESS, null);
+  await expect(promise).rejects.toThrow(/invalid/i);
 });
 
 test(LIQUIDATION_PENALTY, async () => {

packages/dai-plugin-mcd/test/schemas/cdpManager.spec.js

@@ -73,9 +73,8 @@ test(VAULT_TYPE, async () => {
   await expect(vaultType2).rejects.toThrow(/does not exist/i);
 
   const cdpId3 = -9000;
-  expect(() => {
-    maker.latest(VAULT_TYPE, cdpId3);
-  }).toThrow(/invalid vault id/i);
+  const vaultType3 = maker.latest(VAULT_TYPE, cdpId3);
+  await expect(vaultType3).rejects.toThrow(/invalid vault id/i);
 });
 
 test(VAULTS_CREATED, async () => {

packages/dai-plugin-mcd/test/schemas/computed.spec.js

@@ -57,7 +57,13 @@ import computedSchemas from '../../src/schemas/computed';
 
 import { createCurrencyRatio } from '@makerdao/currency';
 
-let maker, snapshotData, address, address2, proxyAddress, expectedVaultAddress;
+let maker,
+  multicall,
+  snapshotData,
+  address,
+  address2,
+  proxyAddress,
+  expectedVaultAddress;
 
 const ETH_A_COLLATERAL_AMOUNT = ETH(1);
 const ETH_A_DEBT_AMOUNT = MDAI(1);
@@ -78,9 +84,9 @@ beforeAll(async () => {
   });
   address = maker.service('web3').currentAddress();
   address2 = TestAccountProvider.nextAccount().address;
-
-  maker.service('multicall').createWatcher();
-  maker.service('multicall').registerSchemas({
+  multicall = maker.service('multicall');
+  multicall.createWatcher();
+  multicall.registerSchemas({
     vatIlks,
     vatUrns,
     vatGem,
@@ -101,7 +107,7 @@ beforeAll(async () => {
     getCdps,
     ...computedSchemas
   });
-  maker.service('multicall').start();
+  multicall.start();
 
   await setupCollateral(maker, 'ETH-A', {
     price: ETH_A_PRICE
@@ -134,6 +140,42 @@ afterAll(async () => {
   await restoreSnapshot(snapshotData, maker);
 });
 
+test(DAI_LOCKED_IN_DSR, async () => {
+  const daiLockedInDsr = await maker.latest(DAI_LOCKED_IN_DSR, address);
+  expect(daiLockedInDsr.symbol).toEqual('DSR-DAI');
+  expect(daiLockedInDsr.toNumber()).toBeCloseTo(1, 18);
+});
+
+test(`${DAI_LOCKED_IN_DSR} using invalid account address`, async () => {
+  const promise = maker.latest(DAI_LOCKED_IN_DSR, '0xfoobar');
+  await expect(promise).rejects.toThrow(/invalid/i);
+});
+
+test(`${DAI_LOCKED_IN_DSR} before and after account has proxy`, async () => {
+  const nextAccount = TestAccountProvider.nextAccount();
+  await maker.addAccount({ ...nextAccount, type: 'privateKey' });
+  maker.useAccount(nextAccount.address);
+
+  const promise = maker.latest(DAI_LOCKED_IN_DSR, nextAccount.address);
+  await expect(promise).rejects.toThrow(/invalid/i);
+
+  await maker.service('proxy').ensureProxy();
+  await mineBlocks(maker.service('token'), 1);
+
+  const daiLockedInDsr = await maker.latest(
+    DAI_LOCKED_IN_DSR,
+    nextAccount.address
+  );
+  expect(daiLockedInDsr.symbol).toEqual('DSR-DAI');
+  expect(daiLockedInDsr.toNumber()).toEqual(0);
+});
+
+test(TOTAL_DAI_LOCKED_IN_DSR, async () => {
+  const totalDaiLockedInDsr = await maker.latest(TOTAL_DAI_LOCKED_IN_DSR);
+  expect(totalDaiLockedInDsr.symbol).toEqual('DSR-DAI');
+  expect(totalDaiLockedInDsr.toNumber()).toBeCloseTo(1, 18);
+});
+
 test(COLLATERAL_TYPE_PRICE, async () => {
   const ethAPrice = await maker.latest(COLLATERAL_TYPE_PRICE, 'ETH-A');
   expect(ethAPrice.toNumber()).toEqual(180);
@@ -333,32 +375,8 @@ test(`${VAULT} with non-existent id`, async () => {
 
 test(`${VAULT} with invalid id`, async () => {
   const cdpId = -9000;
-  expect(() => {
-    maker.latest(VAULT, cdpId);
-  }).toThrow(/invalid vault id/i);
-});
-
-test(DAI_LOCKED_IN_DSR, async () => {
-  const daiLockedInDsr = await maker.latest(DAI_LOCKED_IN_DSR, address);
-  expect(daiLockedInDsr.symbol).toEqual('DSR-DAI');
-  expect(daiLockedInDsr.toNumber()).toBeCloseTo(1, 18);
-});
-
-test.skip(`${DAI_LOCKED_IN_DSR} using invalid account address`, async () => {
-  expect(() => {
-    maker.latest(DAI_LOCKED_IN_DSR, '0xfoobar');
-  }).toThrow(/invalid/i);
-});
-
-test.skip(`${DAI_LOCKED_IN_DSR} using account with no proxy`, async () => {
-  const promise = maker.latest(DAI_LOCKED_IN_DSR, address2);
-  await expect(promise).rejects.toThrow();
-});
-
-test(TOTAL_DAI_LOCKED_IN_DSR, async () => {
-  const totalDaiLockedInDsr = await maker.latest(TOTAL_DAI_LOCKED_IN_DSR);
-  expect(totalDaiLockedInDsr.symbol).toEqual('DSR-DAI');
-  expect(totalDaiLockedInDsr.toNumber()).toBeCloseTo(1, 18);
+  const vault = maker.latest(VAULT, cdpId);
+  await expect(vault).rejects.toThrow(/invalid vault id/i);
 });
 
 test(BALANCE, async () => {
@@ -423,6 +441,16 @@ test(ALLOWANCE, async () => {
   maker.useAccount('default');
 });
 
+test(`${ALLOWANCE} using invalid account address`, async () => {
+  const promise = maker.latest(ALLOWANCE, 'BAT', null);
+  await expect(promise).rejects.toThrow(/invalid address/i);
+});
+
+test(`${ALLOWANCE} for account with no proxy`, async () => {
+  const promise = maker.latest(ALLOWANCE, 'BAT', address2);
+  await expect(promise).rejects.toThrow(/invalid proxy/i);
+});
+
 test(USER_VAULTS_LIST, async () => {
   const [batVault, ethVault] = await maker.latest(USER_VAULTS_LIST, address);
 
@@ -440,6 +468,16 @@ test(USER_VAULTS_LIST, async () => {
   );
 });
 
+test(`${USER_VAULTS_LIST} for account with no proxy`, async () => {
+  const promise = maker.latest(USER_VAULTS_LIST, address2);
+  await expect(promise).rejects.toThrow(/invalid/i);
+});
+
+test(`${USER_VAULTS_LIST} for account with no proxy`, async () => {
+  const promise = maker.latest(USER_VAULTS_LIST, address2);
+  await expect(promise).rejects.toThrow(/invalid/i);
+});
+
 test(PROXY_OWNER, async () => {
   const proxyOwner = await maker.latest(PROXY_OWNER, proxyAddress);
   expect(isValidAddressString(proxyOwner)).toEqual(true);

packages/dai-plugin-mcd/test/schemas/pot.spec.js

@@ -11,7 +11,6 @@ import {
   ANNUAL_DAI_SAVINGS_RATE,
   DATE_EARNINGS_LAST_ACCRUED
 } from '../../src/schemas';
-
 import potSchemas from '../../src/schemas/pot';
 
 let maker, snapshotData, cdpMgr, saveService;
@@ -69,6 +68,17 @@ test(SAVINGS_DAI, async () => {
   expect(savingsDai.toNumber()).toBeCloseTo(0.99995);
 });
 
+test(`${SAVINGS_DAI} using invalid account address`, async () => {
+  const promise1 = maker.latest(SAVINGS_DAI, '0xfoobar');
+  await expect(promise1).rejects.toThrow(/invalid/i);
+
+  const promise2 = maker.latest(
+    SAVINGS_DAI,
+    '0x0000000000000000000000000000000000000000'
+  );
+  await expect(promise2).rejects.toThrow(/invalid/i);
+});
+
 test(DAI_SAVINGS_RATE, async () => {
   const daiSavingsRate = await maker.latest(DAI_SAVINGS_RATE);
   expect(daiSavingsRate).toEqual(BigNumber('1.000000000315522921573372069'));

packages/dai-plugin-mcd/test/schemas/proxyRegistry.spec.js

@@ -40,16 +40,15 @@ test(PROXY_ADDRESS, async () => {
   expect(proxy2).toEqual(proxyAddress2);
 });
 
-test.skip(`${PROXY_ADDRESS} using invalid account address`, async () => {
-  expect(() => {
-    maker.latest(PROXY_ADDRESS, '0xfoobar');
-  }).toThrow(/invalid/i);
+test(`${PROXY_ADDRESS} using invalid account address`, async () => {
+  const promise = maker.latest(PROXY_ADDRESS, '0xfoobar');
+  await expect(promise).rejects.toThrow(/invalid/i);
 });
 
-test.skip(`${PROXY_ADDRESS} using account with no proxy`, async () => {
-  const promise = maker.latest(
+test(`${PROXY_ADDRESS} using account with no proxy`, async () => {
+  const proxy = await maker.latest(
     PROXY_ADDRESS,
     '0x1111111111111111111111111111111111111111'
   );
-  await expect(promise).rejects.toThrow(/no proxy found/i);
+  expect(proxy).toBeNull();
 });