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

add pagination and limit to get_key_references and get_account_references #1753

Open
oxarbitrage opened this issue May 10, 2019 · 5 comments
Labels
6 API Impact flag identifying the application programing interface (API)

Comments

@oxarbitrage
Copy link
Member

oxarbitrage commented May 10, 2019

UI team requesting this at #1749 (comment)

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
Copy link

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

@abitmore
Copy link
Member

@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
Copy link

sschiessl-bcp commented May 13, 2019

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
Copy link
Member

@sschiessl-bcp

  1. actually get_key/account_references doesn't subscribe (see Review subscription API #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
Copy link

  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.

@abitmore abitmore added this to the Future Feature Release milestone May 20, 2019
@abitmore abitmore added the 6 API Impact flag identifying the application programing interface (API) label May 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6 API Impact flag identifying the application programing interface (API)
Projects
None yet
Development

No branches or pull requests

3 participants