Adamant-im / AIPs

ADAMANT Improvement Proposal repository
Creative Commons Zero v1.0 Universal
291 stars 11 forks source link

AIP 14: ChatRooms API #14

Open zyuhel opened 5 years ago

zyuhel commented 5 years ago

Here will go discussion regarding Chatrooms aip

zyuhel commented 5 years ago

https://github.com/Adamant-im/AIPs/blob/aip-chatrooms-api/AIPS/aip-14.md

MaaKut commented 5 years ago
  1. Please, provide a sample of the full /api/chatrooms response.
  2. In the /api/chatrooms/... response the last transaction details should be moved to a nested property:
    {
    lastTransaction: {
        id: ...,
        type: ....
    },
    participants: [...]
    }
  3. Semantics of the withPayments param is not clear: does it rely on type or amount > 0?
  4. BTW, do we really need withPayments? Transfers are the first class citizens in the chats now, so I can hardly imagine uses cases when withPayments needs to be false.
zyuhel commented 5 years ago
  1. It makes sense.
  2. withPayments semantics should add transfers, it doesn't matter how it will work on backend, we are discussing API AIP here.
  3. May be someone will want to make chat app without token transfers, because of some regulations, but your point is valid. Maybe we should have parameter withoutPayments
bludnic commented 5 years ago

orderBy - order by field, same as in transaction list API, by default = timestamp:desc&

The responses received from these endpoints are not equal: /api/chatrooms/U123456 /api/chatrooms/U123456?orderBy=timestamp:desc

bludnic commented 5 years ago

Missing property height.

/api/chatrooms/U111111/U222222 - ok /api/chatrooms/U111111 - no height

adamant-al commented 5 years ago

Still not clear what withPayments or withoutPayments does. Payments means ADM transfers (type 0), or messages with rich text of ETH/DOGE/etc (type 8).

I think public KVS records of participants (as public ETH and DOGE addresses) should also be returned with some kind of parameter like fetchKVS. It will help to make transaction lists with names (user friendly).

adamant-al commented 5 years ago

limit — limit field, same as in transactions list API, default HUI_HUI I think default should be 25.