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 APIs to query for general asset info #1896

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

abitmore
Copy link
Member

@abitmore abitmore commented Aug 10, 2019

PR for #1867. @sschiessl-bcp

Added APIs:

  • get_assets_general_info ( vector asset_symbols_or_ids )
  • list_assets_general_info ( optional lower_bound, optional limit, optional asset_type )

General asset info is:

   struct general_asset_info
   {                         
      asset_id_type           id;                ///< ID of this asset, i.e. "1.3.1"          
      string                  symbol;            ///< Ticker symbol for this asset, i.e. "USD"        
      uint8_t                 precision = 0;     ///< Maximum number of digits after the decimal point
      account_id_type         issuer;            ///< ID of the account which created this asset
      asset_type              type;              ///< Type of the asset                               
      optional<asset_id_type> backing_asset_id;  ///< ID of backing asset if this asset is a MPA or PM
   };

asset_type is

      enum asset_type                
      {                                     
         CORE,        ///< Core asset         
         UIA,         ///< User-issued asset  
         MPA,         ///< Market-pegged asset                       
         PM,          ///< Predition market                          
         CORE_OR_UIA, ///< CORE or UIA, to be used as query parameter
         MPA_OR_PM,   ///< MPA or PM, to be used as query parameter
         ALL          ///< ALL, to be used as query parameter
      };

Implementation details:

  • Add a symbol field in asset_bitasset_data_object
  • Add indices related to asset symbol

@abitmore
Copy link
Member Author

Other fields I thought that would be a bit frequently requested by clients are

  • when listing assets to be used to pay fees: CER, fee pool balance
  • when showing in dashboard: is_globally_settled (for whether can borrow)
  • when trying to transact with: info about white-listing

@sschiessl-bcp @startailcoon please comment.

@sschiessl-bcp
Copy link

sschiessl-bcp commented Aug 13, 2019

Currently we have list_assets which returns the full asset object.

Suggestion:

  1. Name of the new operation get_assets_general_info: Current method for getting list of assets is list_assets, thus I suggest same naming pattern. For example: list_assets_with_filter
  2. Add additional optional argument to get_assets_general_info, namely query_type. If query_type is set to full, then return the full asset object just like list_assets would. If query_type is set to info, return the general_asset_info like you suggested, including the information from your last comment, add current supply.
  3. Necessity of list_assets_general_info: Is this much more lightweight compared to get_assets?
  4. Structure of general_asset_info: Would it make sense to do compatibility format? i.e. it looks exactly like normal asset_object but only has the values mentioned filled in?

@abitmore
Copy link
Member Author

@sschiessl-bcp I fixed typos about API names in OP. The idea was to return only minimum required fields but not the full asset objects when querying for many assets, in order to reduce network traffic and improve UI loading speed. The returned fields are listed in OP. These fields should meet the needs of common use cases -- when showing a list in UI, we can get all needed data with one API call. For example, to show balance sheet or account activities, we only need the symbols and precision of related assets. Then, when need details of one asset or multiple assets, call get_assets API.

Since asset_type and supply weren't in asset_object, the result will never really be compatible. A possible solution is to change return type of get_assets and list_assets and perhaps list_assets_by_issuer API (we already made a step in #1836), and add more optional parameters including a type filter and a return_type indicator (we also already added one in #1868), so we don't need to add the 2 new APIs thus would reduce confusion. This would complex the implementation of related APIs, but I think it's worthwhile since the outcome could be very good.

For the return_type parameter, it seems that it's best if the client can always define what fields to retrieve, right? However, the API could be too complicated if it's too flexible, E.G. it would need a big request to get a bit more results. Please think about common use cases and proper workflows, then make suggestions. For example. CER is a big structure for clients (3-layer nested), there are at least 5 fields related to whitelisting, so I thought these aren't minimum required info when searching for assets. Anyway, in the worst case scenario, you can still use the old get_assets API.

@abitmore abitmore force-pushed the pr-database-api-code-refactory branch from c7d657a to fa42e70 Compare August 14, 2019 18:04
@abitmore abitmore force-pushed the pr-general-info-api branch from 36a5c28 to 7c31f3f Compare August 15, 2019 11:25
@abitmore abitmore force-pushed the pr-database-api-code-refactory branch from 98de678 to d51305c Compare August 15, 2019 12:33
@abitmore abitmore force-pushed the pr-general-info-api branch from 7c31f3f to 72b9860 Compare August 15, 2019 12:41
@abitmore abitmore force-pushed the pr-database-api-code-refactory branch from d51305c to 62fc622 Compare August 15, 2019 13:36
- Add a `symbol` field in asset_bitasset_data_object
- Add indices related to asset symbol
- Add `get_assets_general_info` API
- Add `list_assets_general_info` API
@abitmore abitmore force-pushed the pr-general-info-api branch from 72b9860 to 64eae6b Compare August 15, 2019 21:26
@abitmore abitmore changed the base branch from pr-database-api-code-refactory to develop August 15, 2019 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants