Issue 8: Vault and Strategy Holdings #30

miguel567 · 2020-11-22T23:27:44Z

Issue: #8

Added ./abi folder as a volume to docker-compose.yml file to ease development.
calculated Vault holdings and Strategy holdings and Pools Balance (virtualPrice * Totalsupply)
Storing price_usd from coingecko for vault's want tokens.
Created a new table called holdings.

I'm following the approach from dudesahn as defined here: hackmd.io/@dudesahn/BkxKfTzqw
TODO: We believe we are missing to calculate the pool balance of yWBTC token.
This is the pre-requirement for calculating TVL (my next step).

miguel567 · 2020-11-22T23:35:55Z

I'm breaking APY/SAVE. need to fix it first.

miguel567 · 2020-11-23T18:48:54Z

fixed the APY save endpoint which was broken before and removed ethereum-blocks-by-date.js file as this is not needed.

ghost · 2020-11-23T23:26:47Z

services/vaults/apy/save/handler.js

@@ -276,7 +276,7 @@ const readVault = async (vault) => {
  return data;
 };

-module.exports.handler = async () => {
+const handler = async () => {


why change this? was it broken?

At the end of the file I have set:
module.exports = { handler, getVirtualPrice };

The reason is that I'm reusing the same getVirtualPrice function in : https://github.com/miguel567/yearn-api/blob/91c1696f5f5fa309126883fd17a51ee964a1448b/services/vaults/holdings/save/getHoldings.js#L94

so I had to modify the exports module to include both functions.

ghost · 2020-11-23T23:27:12Z

README.md

-
-## Stages
-
- Currently four stages are available


what is intention behind removing this README section?

I totally believe this is a mistake. Shoud I fix it and push again?

ghost · 2020-11-23T23:29:25Z

services/vaults/holdings/save/handler.js

+  }
+  try{
+    const holdings= await getHoldings(vault);
+    const priceFeed = await axios.get('https://api.coingecko.com/api/v3/coins/' + vault.price_id);


can we get price on-chain from uniswap instead perhaps? this way we don't have to maintain the API

OK, I need to investigate how to perform that. Any guidance?

Can we do this on an improvement ticket? Unless it is a blocker, I could make the other improvements easily, deliver the TVL endpoint and then focus on changing the price feed.

the "earn" products: yWBTC, yCRV and ybCRV are using getVirtualPrice = on-chain price.
Rest of tokens are using coingecko. I would merge as it is and create a new issue to move all pricing infro to on-chain using chainlink, for example. issue: #34

We should be good, we can just use Andre's keep3r price oracles, which already have all of the pairs we need. I updated with the relevant info on issue #34

ghost · 2020-11-23T23:30:02Z

services/vaults/holdings/save/vaults.js

+
+
+module.exports = [
+  {


Can we get vaults from vaults endpoint instead? The vaults endpoint pulls from the vault registry

ah yes, I saw you improved that part, I think I can do this easily.

Icannot use vaults from DB as the ABI is not stored. I could use:
const fetchAbi = async (address) => { const url =https://api.etherscan.io/api?module=contract&action=getabi&address=${address}&apikey=${etherscanApiKey}`;
const resp = await fetch(url).then((res) => res.json());
const abi = JSON.parse(resp.result);
await delay(delayTime);
return abi;
};
`
from services/vaults/holdings/save/handler.js, but we would rely on etherscan API.

As discussed in discord this cannot be done for now. We cannot fetch the ABI on-chain. Also, the services/vaults/holdings/save/vaults.js file is defining where the ABI is coming from. Also I cannot remove the vault.js file as, for instance the aLink and Link strategies have 6 decimals and there is no on-chain "decimals" method for these strategies for me to fetch the correct decimals. So for now we need to rely on the code in the files and not on-chain data :S

ghost · 2020-11-23T23:30:19Z

services/vaults/save/handler.js

 require('dotenv').config();
 const _ = require('lodash');
 const dynamodb = require('../../../utils/dynamoDb');
 const fetch = require('node-fetch');
 const Web3 = require('web3');
 const yRegistryAbi = require('../../../abi/yRegistry.json');
 const delay = require('delay');
-
+const db = dynamodb.doc;


nice fix, i spotted this as well

update

graham-u · 2020-11-30T22:20:35Z

services/vaults/holdings/save/getHoldings.js

+  const vaultContract = new web3.eth.Contract(vault.vaultContractABI, vault.vaultContractAddress);
+  const vaultHoldings = await vaultContract.methods.balance().call() / Math.pow(1,vault.decimals);
+  const strategyHoldings = await strategyContract.methods.balanceOf().call() / Math.pow(1,vault.decimals);
+    /* console.log(holdings); */


Can we remove commented out console.log?

Updated in the new push all the commented out code.

graham-u · 2020-11-30T22:21:44Z

services/vaults/holdings/save/getHoldings.js

+
+
+    return holdings;  
+}


Need an empty line at end of file.

graham-u · 2020-11-30T22:24:08Z

services/vaults/holdings/save/handler.js

+
+
+
+


Personal opinion but I think 4 blank lines here is a lot, better to have 1 or 2. Same above as well.

graham-u

There are quite a few instances of commented out code that I think we should remove unless it is there for a good reason.

miguel567 · 2020-12-05T16:53:00Z

I've reviewed your feedback @graham-u and @x48-crypto from the previous push, please check it above.

Also I've added to this push:

YFI staked holdings calculations
veCRV holdings calculations.
Added the compound Vault holdings calculations (I will add it to APY endpoint in this issue: Add compound vault to APY endpoint calculations #32)

FYI: I'm following Dudesahn's documented holdings calculation guide: https://hackmd.io/@dudesahn/BkxKfTzqw

miguel567 closed this Nov 22, 2020

miguel567 deleted the 8 branch November 22, 2020 23:36

miguel567 restored the 8 branch November 22, 2020 23:36

miguel567 reopened this Nov 23, 2020

miguel567 closed this Nov 23, 2020

miguel567 reopened this Nov 23, 2020

ghost reviewed Nov 23, 2020

View reviewed changes

Merge pull request #1 from iearn-finance/master

7d8d661

update

graham-u reviewed Nov 30, 2020

View reviewed changes

ghost approved these changes Dec 7, 2020

View reviewed changes

feat: implement vault holdings endpoints

8e749d4

ghost merged commit 5262e73 into yearn:master Dec 7, 2020

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 8: Vault and Strategy Holdings #30

Issue 8: Vault and Strategy Holdings #30

miguel567 commented Nov 22, 2020

miguel567 commented Nov 22, 2020

miguel567 commented Nov 23, 2020

ghost Nov 23, 2020

miguel567 Nov 24, 2020

ghost Nov 23, 2020

miguel567 Nov 24, 2020

miguel567 Dec 5, 2020

ghost Nov 23, 2020

miguel567 Nov 24, 2020

miguel567 Nov 24, 2020

miguel567 Dec 5, 2020

dudesahn Dec 5, 2020

ghost Nov 23, 2020

miguel567 Nov 24, 2020

miguel567 Nov 28, 2020

miguel567 Dec 5, 2020

ghost Nov 23, 2020

graham-u Nov 30, 2020

miguel567 Dec 5, 2020

graham-u Nov 30, 2020

miguel567 Dec 5, 2020

graham-u Nov 30, 2020

miguel567 Dec 5, 2020

graham-u left a comment

miguel567 commented Dec 5, 2020 •

edited

Loading

Issue 8: Vault and Strategy Holdings #30

Issue 8: Vault and Strategy Holdings #30

Conversation

miguel567 commented Nov 22, 2020

miguel567 commented Nov 22, 2020

miguel567 commented Nov 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graham-u left a comment

Choose a reason for hiding this comment

miguel567 commented Dec 5, 2020 • edited Loading

miguel567 commented Dec 5, 2020 •

edited

Loading