sclem / pfcrypto

chome extension to automatically update cryptocurrency holdings in personal capital
https://chrome.google.com/webstore/detail/pfcrypto/ceepigemlmlbphjhffddplfecgedhoeb
MIT License
48 stars 11 forks source link

code cleanup/refactor #16

Closed sclem closed 4 years ago

sclem commented 4 years ago

Refactor to use es6 classes and async/await instead of callbacks.

This will make it easier to switch to a different ticker api in the future if needed.

More useful console logging added to help users debug updates.

didyouexpectthat commented 4 years ago

On #15 , I tried to create a brand new manual holdings and wow, it started updating... so going to transfer over each of my items. While testing this PR, the name of $BAT is "Basic Attention Token" but pfcrypto said it was going to skip this one.
skipping account named: 'BASIC ATTENTION TOKEN' image I can switch to $BAT but not sure if its expected behavior to skip over it

[edit] The instructions say to use the full name, which I did. It skips over anything with a space. For example, "Bitcoin Cash" not accepted so I changed it to the id (bitcoin-cash) and it's updating those items now.

[edit2] It took wards of 12 seconds to update each coin...

fetching pc holdings and coins...
personalcapital.js:230 mapping crypto prices to pc holdings...
personalcapital.js:241 skipping account named: ''
personalcapital.js:249 mapping wallet balances for 13 accounts...
personalcapital.js:279 retrieved balance for 0 accounts
personalcapital.js:287 attempting to update ETHEREUM-CLASSIC to $5.03, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating ETHEREUM-CLASSIC to $5.03, took 10.88 seconds
personalcapital.js:287 attempting to update BASIC-ATTENTION-TOKEN to $0.12, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating BASIC-ATTENTION-TOKEN to $0.12, took 10.56 seconds
personalcapital.js:287 attempting to update BITCOIN-SV to $124.28, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating BITCOIN-SV to $124.28, took 10.38 seconds
personalcapital.js:287 attempting to update GOLEM-NETWORK-TOKENS to $0.03, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating GOLEM-NETWORK-TOKENS to $0.03, took 12.03 seconds
personalcapital.js:287 attempting to update BITCOIN to $5351.71, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating BITCOIN to $5351.71, took 11.78 seconds
personalcapital.js:287 attempting to update RIPPLE to $0.15, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating RIPPLE to $0.15, took 11.12 seconds
personalcapital.js:287 attempting to update STELLAR to $0.04, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating STELLAR to $0.04, took 10.18 seconds
personalcapital.js:287 attempting to update BITCOIN-CASH to $180.47, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating BITCOIN-CASH to $180.47, took 10.98 seconds
personalcapital.js:287 attempting to update 0X to $0.16, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating 0X to $0.16, took 11.26 seconds
personalcapital.js:287 attempting to update LITECOIN to $36.34, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating LITECOIN to $36.34, took 11.78 seconds
personalcapital.js:287 attempting to update ZCASH to $25.81, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating ZCASH to $25.81, took 11.68 seconds
personalcapital.js:287 attempting to update AUGUR to $7.52, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating AUGUR to $7.52, took 11.26 seconds
personalcapital.js:287 attempting to update ETHEREUM to $123.87, shares XXX. price updated: true, shares updated: false...
personalcapital.js:293 success updating ETHEREUM to $123.87, took 12.53 seconds
personalcapital.js:302 done updating holdings.
sclem commented 4 years ago

It searches for id or symbol, not "name" in the https://api.alternative.me/v1/ticker/?limit=0 endpoint.

so you would want basic-attention-token or BAT. In the v2 endpoint example you shared (not used) id seems to now be website_slug

didyouexpectthat commented 4 years ago

Suggest changing this in readme.md. Add a holding in the account edit page. Enter the full name of the crypto currency, or the ticker symbol. Example: "BITCOIN" or "BTC". Enter the number of coins you hold (decimal is fine) and any price. Only under Troubleshooting does it say to use the id or symbol.

sclem commented 4 years ago

Updated the readme and added support to lookup by name, so Basic Attention Token should now work. To prevent collisions in the map, a coin won't overwrite another so you may have to fall back to 'symbol', then 'name' in that order if it gives incorrect data.

sclem commented 4 years ago

It takes my login 8-10 seconds per coin too, that's all on personal capital tho.

didyouexpectthat commented 4 years ago

I tested trying to put receive addresses for my wallets in the descriptions.

This happened. After it happened, it stopped the update.

mapping crypto prices to pc holdings...
personalcapital.js:241 skipping account named: ''
personalcapital.js:249 mapping wallet balances for 14 accounts...
personalcapital.js:258 attempting to find balance for account BITCOIN-CASH
personalcapital.js:258 attempting to find balance for account ETHEREUM
personalcapital.js:258 attempting to find balance for account BITCOIN
personalcapital.js:158 Uncaught (in promise) TypeError: Cannot destructure property 'key' of 'BlockCypherAPI.balanceMap[symbol]' as it is undefined.
    at Function.getBalance (personalcapital.js:158)
    at personalcapital.js:266
    at Array.map (<anonymous>)
    at main (personalcapital.js:253)
getBalance @ personalcapital.js:158
(anonymous) @ personalcapital.js:266
main @ personalcapital.js:253
async function (async)
main @ personalcapital.js:207
(anonymous) @ personalcapital.js:313
postMessage (async)
(anonymous) @ VM197:1
(anonymous) @ personalcapital.js:326
sclem commented 4 years ago

@didyouexpectthat thank you for all the testing. Added a note to the readme for the coins that wallet balance is supported on (BTC, ETH, LTC, DOGE, erc20 tokens) and fixed the nonexistent key.

didyouexpectthat commented 4 years ago

Thank you! My balance always go to 0. I use Coinbase mainly so not sure exactly which address to try since it generates a new address for every transaction... when I query my receive addresses on ethplorer and blockcypher, I can confirm they all show 0 balance as well... so I will look into why that is... but otherwise, no major issues seen in latest refactor.

On my setup, I have personalcapital.js:241 skipping account named: '' but zero idea where a blank account name is at. Is it possible to add which investment account in PC this is coming from in the console so I can delete/recreate or review/fix? like skipping account named '' from '$investmentaccountname' ?

sclem commented 4 years ago

I have the same empty ticker in my accounts, its a hidden cash account. I added better logging there to show the details on the account.

No idea about coinbase, my only advice is: if you don't own your private key, you don't own your coins. You can skip the wallet address in the description and set your shares manually.

didyouexpectthat commented 4 years ago

Ahhh, the cash account. Adding the details helps clarify that, too. Thanks! That being said, I've tested up to what I have available for me to test right now. I am not having any issues with it updating the coins anymore. 🙇 🙇

I hear you about the coin ownership. I will eventually migrate off of Coinbase and keep them local.

Thanks again.