Update / Add API to query for open orders with pagination

[abitmore]

IIRC we have 2 sets of API's related to query for open orders of one account in one market:

• get_full_accounts
• get_limit_orders + get_settle_orders + get_call_orders

However, both will return extra data and have some limitations, E.G. no pagination.

IMHO we need these API's:

• for limit_order
  • [ ] get_limit_orders_by_market: one market, for all accounts, with pagination
  • [x] get_limit_orders_by_account: all markets, for one account, with pagination
  • [x] get_limit_orders_by_account_market get_account_limit_orders: one market, for one account, with pagination
• for call_order
  • [ ] get_call_orders_by_asset: one asset, for all accounts, with pagination
  • [x] get_call_orders_by_account: all assets, for one account, with pagination - [ ] get_call_orders_by_account_asset: one asset, for one account, with pagination (note: get_call_orders_by_account can be used for this)
• for settle_order
  • [ ] get_settle_orders_by_asset: one asset, for all accounts, with pagination
  • [x] get_settle_orders_by_account: all assets, for one account, with pagination
  • [ ] get_settle_orders_by_account_asset: one asset, for one account, with pagination

[abitmore]

Related issue: https://github.com/bitshares/bitshares-ui/issues/393, which was closed by using get_full_accounts API. For common use case, get_full_accounts will be called once with subscription to receive future changes, so it should be good enough. So the feature described in this ticket is low priority thus can be postponed.

[oxarbitrage]

it will need to be by_asset in order to get the 3 orders types, by_market will not get the settlement orders as the force_settlement_object haves 1 asset only.

call should be added to the cli wallet.

[windycrypto]

Hi @ryanRfox, as talked in pr https://github.com/bitshares/bitshares-core/pull/849, neither the name nor the usage(except for get_limit_orders_by_account) of theses apis are confirmed. Maybe first refine requirements and prioritize them?

[ryanRfox]

I'll add my opinion here and rely on other Core Team members to comment further (looking at you: @abitmore @oxarbitrage @pmconrad ). I will also request the UI Team to review and add their thoughts on API naming and the dataset returned (@wmbutler @svk31 ).

The OP defines the API names in a format I support. I'm not sure if we have a naming convention defined, but this seems to work in this case: get_<this_data>_by_<filter_set> where <this_data> is the specific order type (limit, call, settle) and <filter_set> is an ordered list of various filters thereof (account, market, asset).

I will suggest to change the API names (but, please await others to comment).

As for the ordering of the parameters for each call, I feel they should begin with and iterate through the <filter_set>. It seems in this case that we have either _by_market or by_account_market, etc. Of course account requires only a single value, whereas market requires multiple parameters itself. In this case the parameter ordering is intuitive. However, we may have a <filter_set> where multiple filters require a set of parameters. That becomes a bit more difficult to intuit what order the parameters should be. As I believe it's out of scope for this Issue, let's table that discussion.

Note: One struggle I had (as the new guy here) with this Issue/PR was most of the refactoring/comments taking place within the PR rather than the Issue. My preference is for the Review to happen in the PR, while keeping the refinement dialog within the Issue, so those reviewing the Issue can more quickly be up to date on progress. Thanks.

[pmconrad]

Agree with the proposed APIs. One question:

get_call_orders_by_account_asset: one asset, for one account, with pagination

There can be at most one call_order per account and asset. Do we still want to return a list, with pagination, for symmetry?

[abitmore]

@pmconrad right now we don't have many MPA's or PM's in the system, so pagination for get_call_orders_by_account_asset API is not very useful. The pagination is to avoid changing it again in the future.

[pmconrad]

There can be at most one call_order per account and asset.

[abitmore]

There can be at most one call_order per account and asset.

Oh. Thought you're talking about another API. Yes, this one will return one or zero entry. I'm fine if change it to return an optional, and remove the pagination.

[abitmore]

Due to https://github.com/bitshares/bitshares-core/pull/1749, the get_limit_orders_by_account API is needed asap.

[abitmore]

Implemented get_limit_orders_by_account API via #2146 for 4.0 release. Moving the issue to the next release.