bitshares / bitshares-core

BitShares Blockchain node and command-line wallet
https://bitshares.github.io/
Other
1.17k stars 647 forks source link

add pagination and limit to get_key_references and get_account_references #1753

Open oxarbitrage opened 5 years ago

oxarbitrage commented 5 years ago

UI team requesting this at https://github.com/bitshares/bitshares-core/pull/1749#discussion_r282520956

By the way, please fix the duplicate result issue (#1209), see the example below. Perhaps change the return data type from vector<vector> to vector<set> or vector<flat_set>.

sschiessl-bcp commented 5 years ago

In what ways could response be sorted? What information does the backend have, if for.example, 10 accounts have the same key?

abitmore commented 5 years ago

@sschiessl-bcp current behavior looks like this:

get_key_references [
  BTS7qQkK8vcMZVxXtxb66gHnMPnXAqzZGzYPa3QsLUoSBotjTZt47,
  BTS4x8AS3sifAQZ7m3y3xB1Wv3Pw1nF3wwfB6xUWu2RXf1Q7pvLTg,
  BTS7WqnrS4XFENL4XxQTL7mmQyDqsx5xopwV3ExqHZgKDh1uatwTx]

[[
    "1.2.1151419",
    "1.2.1147979",
    "1.2.1151419"
  ],[
    "1.2.1147979"
  ],[
    "1.2.1147979",
    "1.2.1211831",
    "1.2.1211836",
    "1.2.1211838",
    "1.2.1211841",
    "1.2.1211843",
    "1.2.1212184",
    "1.2.1212186",
    "1.2.1212187",
    "1.2.1212353",
    "1.2.1212802",
    "1.2.1212806",
    "1.2.1212810",
    "1.2.1212819",
    "1.2.1213978",
    "1.2.1213979",
    "1.2.1219177",
    "1.2.1219178",
    "1.2.1219235",
    "1.2.1220248",
    "1.2.1234827",
    "1.2.1234833",
    "1.2.1260698",
    "1.2.1261064",
    "1.2.1273867",
    "1.2.1279104",
    "1.2.1282180",
    "1.2.1327264",
    "1.2.1436504",
    "1.2.1507910",
    "1.2.1509783",
    "1.2.1147979",
    "1.2.1211802",
    "1.2.1211831",
    "1.2.1211836",
    "1.2.1211838",
    "1.2.1211841",
    "1.2.1211843",
    "1.2.1212184",
    "1.2.1212186",
    "1.2.1212187",
    "1.2.1212353",
    "1.2.1212802",
    "1.2.1212806",
    "1.2.1212810",
    "1.2.1212819",
    "1.2.1213978",
    "1.2.1213979",
    "1.2.1219177",
    "1.2.1219178",
    "1.2.1219235",
    "1.2.1220248",
    "1.2.1234827",
    "1.2.1234833",
    "1.2.1260698",
    "1.2.1261064",
    "1.2.1273867",
    "1.2.1279104",
    "1.2.1282180",
    "1.2.1327264",
    "1.2.1436504",
    "1.2.1507910",
    "1.2.1509783"
  ]
]

2 accounts have the first key (one is duplicate, IMHO is a bug), 1 account has the second key, lots of accounts have the last key (with duplicates as well, and pagination would be useful for this case).

sschiessl-bcp commented 5 years ago

Let's take the account with lots of key as example.

Comments:

  1. Since get_key/account_references subscribes, this call pretty much breaks the UI if too many references are present. Default behavior should be changed IMO Reasoning:

    • This is a lookup feature here, afterwards you select which account is the "active" one in the UI and do the subscription
    • This can be used to attack nodes
    • I don't have a use-case in my head where subscriptions would be beneficial
    • When this is paginated (see this issue OP), subscriptions makes even less sense
  2. Take the third example with massive amount of keys. Does the backend have any information on the accounts? For example: Which one of the accounts displayed was the originally registered account?

  3. Duplicates should be removed and sorted by id (smallest first)

abitmore commented 5 years ago

@sschiessl-bcp

  1. actually get_key/account_references doesn't subscribe (see https://github.com/bitshares/bitshares-core/issues/777);
  2. "any information on the accounts" can be obtained via get_accounts API;
  3. duplicates can be easily removed and should be done, already mentioned in OP.
sschiessl-bcp commented 5 years ago
  1. "any information on the accounts" can be obtained via get_accounts API;

What I meant is what information has the backend available to use for sorting the return value. As I understand it nothing except account id and public key.