Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API History commands #329

Closed
grctest opened this issue Jul 16, 2017 · 13 comments
Closed

API History commands #329

grctest opened this issue Jul 16, 2017 · 13 comments
Labels

Comments

@grctest
Copy link

grctest commented Jul 16, 2017

Hey,

The graphene documentation makes reference to functions which are not available within the CLI wallet, such as:
get_trade_history

vector<market_trade> graphene::app::database_api::get_trade_history(const string &base, const string &quote, fc::time_point_sec start, fc::time_point_sec stop, unsigned limit = 100)

unlocked >>> get_trade_history
get_trade_history
10 assert_exception: Assert Exception
itr != _by_name.end(): no method with name 'get_trade_history'
th_a  api_connection.hpp:109 call

And get_fill_order_history:
vector<order_history_object> graphene::app::history_api::get_fill_order_history(asset_id_type a, asset_id_type b, uint32_t limit) const

unlocked >>> get_fill_order_history
get_fill_order_history
10 assert_exception: Assert Exception
itr != _by_name.end(): no method with name 'get_fill_order_history'
th_a  api_connection.hpp:109 call

Is this functionality disabled in BTS?

Thanks

Reference thread: https://bitsharestalk.org/index.php/topic,24622.msg309020.html#msg309020

@pmconrad
Copy link
Contributor

There's a difference between the CLI API and the node API. get_fill_order_history is a node API method. The CLI does not expose all node API commands, but adds some convenience methods and wallet handling stuff that's not part of the node API.

I don't know what the rationale is/was for adding access to node API methods as CLI commands. I suppose the basic question is - why would someone want to do that on the command line? AFAICT from the linked thread, the goal is to programmatically call that method. In that case the solution is to simply call the node API instead of the CLI API.

@oxarbitrage
Copy link
Member

they are node api calls, correct. they can be used like this following the curl examples in the forum thread:

curl --data '{"jsonrpc": "2.0", "method": "get_trade_history", "params": ["USD", "BTS", "2017-07-16T12:16:15", "2017-07-16T11:16:15", 100], "id": 1}' http://127.0.0.1:8090/rpc

curl --data '{"jsonrpc": "2.0", "method": "get_trade_history", "params": ["USD", "BTS", "2017-07-02T12:16:15", "2017-07-01T11:16:15", 100], "id": 1}' http://127.0.0.1:8090/rpc

or with wscat:

root@alfredo:~# wscat -c localhost:8090
connected (press CTRL+C to quit)
> {"jsonrpc": "2.0", "method": "get_trade_history", "params": ["USD", "BTS", "2017-07-16T12:16:15", "2017-07-16T11:16:15", 100], "id": 1}

as @pmconrad describes it seems this could be started by manual cli_wallet users or lazy client side devs that wanted to do everything from the cli_wallet :)
whatever is the reason we need to consider the individuals that are using the cli_wallet to transfer/trade assets manually, make daily basics tasks(payments) and other stuff.
when a front end app like openledger can't do something(add a worker proposal for example) the next natural level for the user is the cli_wallet. i am sure we have a lot of people in the cli_wallet level, definitely used manually.

maybe a call_node_api for them ?

unlocked >>>call_node_api '{"jsonrpc": "2.0", "method": "get_trade_history", "params": ["USD", "BTS", "2017-07-16T12:16:15", "2017-07-16T11:16:15", 100], "id": 1}'

@grctest
Copy link
Author

grctest commented Jul 16, 2017

get_trade_history
{"date":"2017-07-16T11:59:09","price":"0.08899984149627516","amount":"1.26180000000000003","value":"0.11230000000000000"}

Damn, it doesn't include IDs of the users involved in the trade 😞

Attempting get_fill_order_history against a full node doesn't work neither

 curl --data '{"jsonrpc": "2.0", "method": "get_fill_order_history", "params": ["USD", "BTS", "2017-07-16T12:16:15", "2017-07-16T11:16:15", 100], "id": 1}' http://IPADDRESS:8092/rpc
{"id":1,"jsonrpc":"2.0","error":{"code":1,"message":"Assert Exception: itr != _by_name.end(): no method with name 'get_fill_order_history'","data".....

Right, so back to scanning every account for assets held then scanning their trade history to the filter for filled orders.. unless there's a get_asset_holders or a retrievable asset_holder object?

I could scrape cryptofresh's API for asset holders, however it isn't always online.

@oxarbitrage
Copy link
Member

the following calls are in the asset_api, you need to be logged. here you can get the count of asset_holders:

> {"id":2,"method":"call","params":[1,"login",["",""]]}
< {"id":2,"jsonrpc":"2.0","result":true}
> {"id":2,"method":"call","params":[1,"asset",[]]}
< {"id":2,"jsonrpc":"2.0","result":2}
> {"id":1, "method":"call", "params":[2,"get_asset_holders_count",["1.3.0"]]}
< {"id":1,"jsonrpc":"2.0","result":24085}
> 

here a call to get the top 10 holders of BTS:

> {"id":1, "method":"call", "params":[2,"get_asset_holders",["1.3.0", 0, 10]]}
< {"id":1,"jsonrpc":"2.0","result":[{"name":"poloniexcoldstorage","account_id":"1.2.24484","amount":"29000120286608"},{"name":"chbts","account_id":"1.2.224015","
amount":"21323905140061"},{"name":"yunbi-cold-wallet","account_id":"1.2.29211","amount":"14000000042195"},{"name":"bitcc-bts-cold","account_id":"1.2.152313","amo
unt":"10943523959939"},{"name":"yunbi-granary","account_id":"1.2.170580","amount":"10000000048617"},{"name":"jubi-bts","account_id":"1.2.253310","amount":"699215
7568429"},{"name":"bittrex-deposit","account_id":"1.2.22583","amount":"6843227690310"},{"name":"btschbtc","account_id":"1.2.224081","amount":"5000098977059"},{"n
ame":"bterdeposit","account_id":"1.2.9173","amount":"2195728656599"},{"name":"aloha","account_id":"1.2.10946","amount":"2061578333527"}]}
> 

not sure if it is what you need, hope it helps!

@grctest
Copy link
Author

grctest commented Jul 16, 2017

Cool, so I could do get_asset_holders_count then forward the count into get_asset_holders to retrieve a full list of account holders to then scan for transaction histories.

Thanks for the help guys 👍

@grctest grctest closed this as completed Jul 16, 2017
@grctest grctest reopened this Jul 27, 2017
@grctest
Copy link
Author

grctest commented Jul 27, 2017

curl --data '{"jsonrpc": "2.0", "method": "get_trade_history", "params": ["USD", "BTS", "2017-07-16T12:16:15", "2017-07-16T11:16:15", 100], "id": 1}' http://127.0.0.1:8090/rpc

get_trade_history
{"date":"2017-07-16T11:59:09","price":"0.08899984149627516","amount":"1.26180000000000003","value":"0>  .11230000000000000"}

Damn, it doesn't include IDs of the users involved in the trade 😞

Rather than querying the quantity of holders, grabbing their account names then dumping their individual overall transaction history for a time period then filtering for filled orders against the trading pair I'm interested in, it would be far simpler if the 'get_trade_history' command included user id's for the buyer and seller.

Would it be possible to add this data to the get_trade_history command, or was it left out for increased privacy on the BTS DEX?

@oxarbitrage
Copy link
Member

i don't think it will be any problem for us if we add the account_id to the get_trade_history results.

the following pull request do just that: #344

new output:

alfredo@alfredo-Inspiron-5559 ~/CLionProjects/issue329 $ curl --data '{"jsonrpc": "2.0", "method": "get_trade_history", "params": ["USD", "BTS", "2017-07-16T12:16:15", "2017-07-16T12:10:15", 100], "id": 1}' http://127.0.0.1:8090/rpc | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   776  100   641  100   135   117k  25285 --:--:-- --:--:-- --:--:--  125k
{
  "id": 1,
  "jsonrpc": "2.0",
  "result": [
    {
      "date": "2017-07-16T12:14:54",
      "price": "0.08156146875468195",
      "amount": "70804.16387999999278691",
      "value": "5774.89159999999992579",
      "account_id": "1.2.165970"
    },
    {
      "date": "2017-07-16T12:14:42",
      "price": "0.08156146882158860",
      "amount": "3396.21151999999983673",
      "value": "277.00000000000000000",
      "account_id": "1.2.280721"
    },
    {
      "date": "2017-07-16T12:11:36",
      "price": "0.08169505097852067",
      "amount": "12.36304999999999943",
      "value": "1.01000000000000001",
      "account_id": "1.2.280721"
    },
    {
      "date": "2017-07-16T12:11:06",
      "price": "0.08841733411374239",
      "amount": "81.92284999999999684",
      "value": "7.24340000000000028",
      "account_id": "1.2.117806"
    }
  ]
}
alfredo@alfredo-Inspiron-5559 ~/CLionProjects/issue329 $ 

@abitmore
Copy link
Member

abitmore commented Aug 1, 2017

A trade has two participants. Best if we add two fields, a maker and a taker. Otherwise it would confuse people.

By the way, IMHO that CLI API has other flaws. For example, it uses double, so the amounts are inaccurate. For example, the price may be off due to rounding issues.

@grctest
Copy link
Author

grctest commented Aug 1, 2017

I agree with @abitmore, having both maker&taker account_id's would be beneficial. Thank you very much for working on this improvement @oxarbitrage 👍

@abitmore Will the changes being discussed in #342 fix the rounding issues in the CLI or will additional changes be required within the cli wallet? Thanks

@oxarbitrage
Copy link
Member

added the account of the 2 sides of the trade to pull request, output is now:

lfredo@alfredo-Inspiron-5559 ~/CLionProjects/issue329 $ curl --data '{"jsonrpc": "2.0", "method": "get_trade_history", "params": ["USD", "BTS", "2017-07-16T12:16:15", "2017-07-16T12:10:15", 100], "id": 1}' http://127.0.0.1:8090/rpc | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   928  100   793  100   135  43163   7348 --:--:-- --:--:-- --:--:-- 44055
{
  "id": 1,
  "jsonrpc": "2.0",
  "result": [
    {
      "date": "2017-07-16T12:14:54",
      "price": "0.08156146875468195",
      "amount": "70804.16387999999278691",
      "value": "5774.89159999999992579",
      "side1_account_id": "1.2.165970",
      "side2_account_id": "1.2.111823"
    },
    {
      "date": "2017-07-16T12:14:42",
      "price": "0.08156146882158860",
      "amount": "3396.21151999999983673",
      "value": "277.00000000000000000",
      "side1_account_id": "1.2.280721",
      "side2_account_id": "1.2.111823"
    },
    {
      "date": "2017-07-16T12:11:36",
      "price": "0.08169505097852067",
      "amount": "12.36304999999999943",
      "value": "1.01000000000000001",
      "side1_account_id": "1.2.280721",
      "side2_account_id": "1.2.111823"
    },
    {
      "date": "2017-07-16T12:11:06",
      "price": "0.08841733411374239",
      "amount": "81.92284999999999684",
      "value": "7.24340000000000028",
      "side1_account_id": "1.2.117806",
      "side2_account_id": "1.2.111823"
    }
  ]
}

for each trade 2 fill orders are created for each side of the it. the iteration is incremented by 2 in the call to avoid the duplicates but the new code addition will go over the skipped itr to get the second account id involved.

i was not able to know if the first in the sequence is the maker and the next is the taker, or if the first is the taker and the second the maker or if it is not guaranteed to keep this order.
this is why i used side1_account_id and side2_account_id instead of something more convenient like maker_account_id and taker_account_id.

@oxarbitrage
Copy link
Member

merged the changes to the develop branch and closing the issue.

@grctest
Copy link
Author

grctest commented Dec 24, 2017

@oxarbitrage Hey, has this change made it into the production Bishares-Core client yet?

I've requested that this functionality is extended to the python-bitshares library, since i'm working on the HUG REST API right now.

Cheers 👍

@oxarbitrage
Copy link
Member

yes, this change is at master already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants