QA API

[minkyraccoon]

Purpose of this issue to QA the API review

Background:

• API has already been reviewed by Jordan.
• Dongsam has made changes based on this feedback.

Next Steps:

• New version is available and needs to be checked
  • such that Jordan's recommendations are done
  • no issues are remaining

Outcomes

• PR with proposed changes
• Associated documentation detailing any bigger changes or deficiencies

[fdymylja]

types mismatch:
favor string over bytes, for readability during json encoded queries:

• https://github.com/tendermint/liquidity/blob/develop/proto/liquidity.proto#L98 - should be type string
• https://github.com/tendermint/liquidity/blob/develop/proto/liquidity.proto#L108 - should be type string
• https://github.com/tendermint/liquidity/blob/develop/proto/liquidity.proto#L118 - should be type string

in responses instead of repeating the type descriptor, nest the type:

• https://github.com/tendermint/liquidity/blob/develop/proto/liquidity.proto#L215 - is a clone of PoolMetadata, the correct way to deal with that would be:

 message PoolMetadataResponse { 
      PoolMetadata pool_metadata = 1;
 }

• https://github.com/tendermint/liquidity/blob/develop/proto/liquidity.proto#L292 - same a above

missing documentation:

• https://github.com/tendermint/liquidity/blob/develop/proto/liquidity.proto#L338
• https://github.com/tendermint/liquidity/blob/develop/proto/liquidity.proto#L376
• https://github.com/tendermint/liquidity/blob/develop/proto/liquidity.proto#L414

documentation fix:

• https://github.com/tendermint/liquidity/blob/develop/proto/msg.proto#L14

not clear:

• https://github.com/tendermint/liquidity/blob/develop/proto/liquidity.proto#L16 - can you clarify the role of this ID?
• https://github.com/tendermint/liquidity/blob/develop/proto/liquidity.proto#L215 - where is this used?
• https://github.com/tendermint/liquidity/blob/develop/proto/liquidity.proto#L292 - where is this used?
• https://github.com/tendermint/liquidity/blob/develop/proto/liquidity.proto#L338 - could you clarify the role of MsgState?
• https://github.com/tendermint/liquidity/blob/develop/proto/msg.proto - what is the role of this file? we have tx.proto and msg.proto
• https://github.com/tendermint/liquidity/blob/develop/proto/tx.proto#L61 - whats the role of base_req, where does it come from?
• https://github.com/tendermint/liquidity/blob/develop/proto/tx.proto#L69 - what's the role of StdTx ? where does it come from?
• https://github.com/tendermint/liquidity/blob/develop/proto/tx.proto#L272 - why do we alias the sdk tx type here?
• https://github.com/tendermint/liquidity/blob/develop/proto/tx.proto#L420 - same as point above.

proto conventions:

• please version your API package, such as done in https://github.com/cosmos/cosmos-sdk/blob/master/proto/cosmos/bank/v1beta1/authz.proto#L2 for better backwards compatibility handling.

[dongsam]

Hi, @fdymylja Thanks for your feedback,

I just applied that from #193, #202

• types mismatch:favor string over bytes, for readability during json encoded queries:
  fixed type from bytes to string #193
  - order_price
  - swap_fee_rate
  - withdraw_fee_rate
  - max_order_amount_ratio

• in responses instead of repeating the type descriptor, nest the type:

  → removed nested response #193

• proto conventions:
  → add version to package path tendermint.liquidity.v1beta1 #193

• not clear

  • removed legacy, unofficial base_req, msg.proto #193
  • remove unofficial StdTx for Msg{*}Response #202
  • remove unofficial StdTx for Msg{*}Response #202
  • remove unused legacy PoolBatchResponse #202

• documentation #202

  • clarify the role of pool_type_id
  • clarify the role of MsgStates

you can see updated swagger v2.2.1