CoinRoster / slotmachine

fruitgame
0 stars 0 forks source link

admin page - balance sheet #44

Closed RiskingTime closed 6 years ago

RiskingTime commented 6 years ago

see explanation #41

@Patrick-Bay if necessary, can you provide details about how you will complete this along with an estimated time-frame?

Patrick-Bay commented 6 years ago

I've revised my initial estimate for this task somewhat to 3 hours. To recap:

Approach

This will require a number of updates to the client (browser) code, including:

RiskingTime commented 6 years ago

also to note, maybe it would be better (easier) not to use blockcypher API to query the value of bitcoins on the associated addresses? maybe we could use another method, so as to avoid looking in the same place?

for the addresses that will end up being displayed, it will look similar to what is currently displayed, except instead, our "balance sheet" will show the current value of each bitcoin address according to the blockchain, and not by referencing our own database

I plan on clearing out the addresses on a regular basis (probably daily as required), so I'd like the balance sheet addresses not to show the addresses with zero balances.

Patrick-Bay commented 6 years ago

Understood. I'll use an alternative to BlockCypher (perhaps https://www.blocktrail.com/tBTC or https://testnet.blockexplorer.com/), to retrieve the addresses and I'll make a note of where we can update this API reference should we wish to use a different one later.

Regarding 0 balances, I'll have to write an additional API function to make a note in the database to exclude them from future retrievals (for the admin page), otherwise we will not have a way to differentiate which addresses to retrieve and as the list grows, checking each in turn will take an increasingly long amount of time. (0 balances can exist for additional reasons besides a full withdrawal)

This will probably increase this task to the original 4 hour estimate as it'll probably affect other sections of the game server (when deposits and withdrawals are made).

Patrick-Bay commented 6 years ago

The balance sheet update has been committed (https://github.com/CoinRoster/slotmachine/commit/f45358855e4065abae09d7c9095dcc22bd9400d9), has been pulled, and is currently active on the development server (http://myfruitgame.com/admin).

I ended up going with the blockchain.info API since Block Explorer's seems to be misreporting unconfirmed transactions (always 0). Nevertheless, reverting is simply a matter of uncommenting and commenting the desired API address in the admin page JavaScript client (browser) code:

https://github.com/CoinRoster/slotmachine/blob/f45358855e4065abae09d7c9095dcc22bd9400d9/html/admin/js/index.js#L21-L24

... and if we ever want to support additional APIs we simply need to add a custom parser to the API return handler:

https://github.com/CoinRoster/slotmachine/blob/f45358855e4065abae09d7c9095dcc22bd9400d9/html/admin/js/index.js#L280-L293

RiskingTime commented 6 years ago

so the next thing to add to the balance sheet is other "hard coded" addresses to the asset list, such as the "cash register address" and other cold storage addresses that have bitcoins on them. at first glance it looks like we are in a deficit, the total amount showing on the stats page is more than the sum of the three addresses with bitcoins on them

Patrick-Bay commented 6 years ago

Update has been made to the admin page as per our last discussion:

The update is available for review on the development server right now.

RiskingTime commented 6 years ago

hey @Patrick-Bay the balance sheet looks good! wondering if the address in the asset table labelled "live wallet" is this the hot money wallet (connected from our system by way of blockcypher api) that I usually refer to as the "cash register address"?

RiskingTime commented 6 years ago

I also noticed that the "total investment balance" in the liabilities table is the cost base of the investment, but it should actually show the market value of the investment, since the market value is what the users actually have and can withdraw, can you make this change?

RiskingTime commented 6 years ago

also wondering if the buttons for "transfer all accounts funds" and "transfer all funds" for each addresses work? I haven't tested yet, since I don't want to push the buttons and mess up the data if the buttons are not ready to be used yet. just a reminder that if the "transfer target address" is the same address that is listed in another part of the balance sheet (say its the cold storage address) then I'd like the transfer to be reflected immediately as the transaction is made (even if it requires a browser refresh thats ok)

Patrick-Bay commented 6 years ago

To answer your questions (in order):

  1. The "live wallet" is the same as the "cash register address", which can be found in the game server config as "withdrawal account":

https://github.com/CoinRoster/slotmachine/blob/c3e4b93f831698274eb2cd60a9e5c000f7cd0c2c/node/game_server_config.js#L38-L54

  1. I'll have another look at the total investment balance.

  2. The transfer buttons should be fully functional. However, the balances in the list are not updated, only the transfer buttons are (I believe they show "transfer completed" or something similar).

  3. I don't recall any special handling of the transfer target address but I will make this change. We won't be able to refresh the browser immediately as that can mess up any pending transfers but we can queue a refresh once all transfers are complete. Would this be acceptable? Alternatively, we can update the field when it's updated but this may not be the blockchain balance since we can't yet check it (since other transfers may still be pending). Basically our current options are: update immediately (probably correct but not verified with blockchain), update after all transfers (not immediate but correct).

RiskingTime commented 6 years ago

regarding point 3, so if I push a transfer button, the transaction will be triggered, but how might the table change? will the address which I transferred from which now has a zero balance still show in the table after a browser refresh?

Patrick-Bay commented 6 years ago

Currently the only thing you'll see is the transfer button change to "Transfer pending..." and then "Transfer Complete", but nothing else will update (so yes, the address will remain along with it previous balance). The admin page will remain this way until the page is refreshed.

That being said, we can zero out the balance immediately (just simply subtract in the browser), or we can attempt a balance re-check as part of the process (transfer -> re-check/update -> next account).

Patrick-Bay commented 6 years ago

Please advise on what you would like to see here.

Patrick-Bay commented 6 years ago

Thanks for chatting with me about this yesterday. To sum up, we will simply leave the page as-is when funds are transferred out and we will need to refresh the page to see updated balances (or to remove 0-balance addresses altogether).

Patrick-Bay commented 6 years ago

The latest updates have been committed and pulled to the game server where they may be reviewed immediately.

One note on transfers: when testing extremely low transfer fees (e.g. 1 satoshi), I noticed that the transaction gets posted to BlockCypher's systems but does not appear to be reflected immediately elsewhere (e.g. blockchain.info or block explorer). I suspect that this is due to the fee which may be interpreted as an invalid transaction, or there may be some propagation delays. Nevertheless, this is something we should discuss as it can cause emptied addresses to re-appear in the list on the Admin page.

Currently we're applying only a 1 Satoshi miner's fee (BlockCypher rejects 0-fee transactions), but we can easily update this:

https://github.com/CoinRoster/slotmachine/blob/f615d706bca12764a6956ee8358fa546187007b1/node/admin_plugin.js#L242-L246

https://github.com/CoinRoster/slotmachine/blob/f615d706bca12764a6956ee8358fa546187007b1/node/admin_plugin.js#L336