Loopring / DEX-API

The API of Loopring DEX
1 stars 3 forks source link

[3.6 API refine] Complete Swagger #72

Open luzzif opened 3 years ago

luzzif commented 3 years ago

You can find the refined Swagger definition, complete af all APIs here.

Would be super helpful if you could go over all APIs again and give me feedback on each one (well, at least when there are doubts/things to change).

Updated Swagger 1 here. Updated Swagger 2 here. Updated Swagger 3 here.

yueawang commented 3 years ago

Seems the file does not include all APIs, could you double check?

One more question is, do you have comments on ws API??

luzzif commented 3 years ago

Updated the Swagger. A couple APIs were indeed missing. As of now I left out Ethereum-related APIs, get relayer time, get user registration and password reset transactions, which if I'm not mistaken we agreed to leave out.

luzzif commented 3 years ago

Updated the OpenAPI spec.

A question: in the exit pool API we have a fee parameter in the body. By who is it decided and how is it paid exactly? By how is it paid I mean which token is used to pay the fee?

luzzif commented 3 years ago

Updated the OpenAPI spec:

That should be it for AMM-related APIs. For the ticker one, as you said, is pretty much the same as the one that we have for standard order-book-based markets, so I think that can be taken directly without me adding it to the Swagger. As far as websocket goes, I think the current API is pretty good, plus, not having much experience with websocket APIs, I wouldn't give too many advices on how that could be improved.

yueawang commented 3 years ago

A question: in the exit pool API we have a fee parameter in the body. By who is it decided and how is it paid exactly? By how is it paid I mean which token is used to pay the fee?

It's a general exit fee, the fee token is hardcoded to the quote token of the pool, i.e., ETH for LRC-ETH pool.

luzzif commented 3 years ago

A question: in the exit pool API we have a fee parameter in the body. By who is it decided and how is it paid exactly? By how is it paid I mean which token is used to pay the fee?

It's a general exit fee, the fee token is hardcoded to the quote token of the pool, i.e., ETH for LRC-ETH pool.

What for pools with more than 2 assets?

yueawang commented 3 years ago

Actually, it is hardcoded to the last token in the token list.

luzzif commented 3 years ago

Ok, I will integrate that in the doc. That's all for the API structure to me. I'll start working on the written documentation next.

yueawang commented 3 years ago

Thanks We will start integration soon after the withdrawal mining promotion begins. Have you registered for the new wallet APP :)

yueawang commented 3 years ago

Hi, @luzzif

As I started working on the new API. there is 1 thing I want to double check. As we proposed to combine related members into one structure, for example, we now have a PublicKey structure which holds the x y as below:

      struct PublicKey {
        string x,
        string y,
    }

So, the previous input in the key-value pair

   {
        publicKeyX: 0x1234
        publicKeyY: 0x5678
    }

becomes 2 level key-value pair

   {
        PublicKey: {
            x: 0x1234,
            y: 0x5678
        }
    }

Is that better than the plain key-value we used before? For response, it might be clear, but for request, I think it's a little bit harder to construct such data. Actually, I did not find any wildly used API (binance/okex/etc) has such data structure. So the question is: If we don't use combined structure in the request, should we keep using it in the response??

Another case is the tokenInfo, in some response data, the server replies a tokenInfo structure, as the example below:

    TokenInfo{
        tokenId: 0
        volume: "12341000000"
    }

I tried to use it in those offchain requests. The new request looks like this:

    TransferRequest {
        transfer_token_info : {
            tokenId: 0,
            volume: "130000000000"
        },
        fee_token_info : {
            tokenId: 0,
            volume: "1300000000"
        },
    }

However, without extra description, people don't know the "volume" in fee tokenInfo is the maximum fee amount of this request. From this point of view, I think the previous key-value pair, as illustrated below, is better than above, as its member explains itself.

    TransferRequest {
            tokenId: 0,
            volume: "130000000000"
            feeTokenId: 0,
            maxFeeAmount: "1300000000"
    }

What's your opinion?? Thanks!

luzzif commented 3 years ago

Hi Yue.

Is that better than the plain key-value we used before? For response, it might be clear, but for request, I think it's a little bit harder to construct such data. Actually, I did not find any wildly used API (binance/okex/etc) has such data structure. So the question is: If we don't use combined structure in the request, should we keep using it in the response??

I personally think it's pretty self explanatory, mainly because the documentation will be updated and include hopefully better and more clear explanations about what certain data structures are and what certain members mean, but also from a pure data structure standpoint. Also, I've had a look at the current transfer API and there's no maxFeeAmount parameter in any request/response (there's feeAmount and amountF). Anyway, I can see Loopring having one or multiple official API clients that can abstract away some of the API mechanics which are "complex" and make the user's life easier (I already worked on this, there's my loopring-lightcone library which Rails is currently using in production that does just that).

P.S.: have we decided to use snake_case in the new API?

yueawang commented 3 years ago

In 3.6, most fees in offchain requests are max fees, which is different than 3.1 transfer. A similar case in 3.1 is the order, but order uses maxFeeBips rather than maxFeeAmount.

P.S.: have we decided to use snake_case in the new API?

What is snake_case?

luzzif commented 3 years ago

In 3.6, most fees in offchain requests are max fees, which is different than 3.1 transfer. A similar case in 3.1 is the order, but order uses maxFeeBips rather than maxFeeAmount.

I think the structure will make sense if the naming is updated then. So the key for the structure should become maxFeeInfo or something like that.

What is snake_case?

A way of neming things. Snake case is the one generally used in C for example, with all lowercase letters divided by underscores (the one you used above). JSON uses camelCase to name attributes.

yueawang commented 3 years ago

I think the structure will make sense if the naming is updated then. So the key for the structure should become maxFeeInfo or something like that.

OK for me.

A way of neming things. Snake case is the one generally used in C for example, with all lowercase letters divided by underscores (the one you used above). JSON uses camelCase to name attributes.

No, as we accept JSON, I think follow JSON format is better.

Another thing is: as we have both ecdsa & eddsa & potential approved_hash in those offchain requests, do you think a structure called authentication is a better way to organize the request?? for example: Current request:

    transfer {
    ...
    ecdsaSignature: "0x123456",
    eddsaSignature: None, # or eddsaSignature only, i.e., ecdsaSignature is None.
    ...
    }

After reorg:

    transfer {
    ...
    authentication: {
        type : "ECDSA",  # or "EDDSA" / "APPROVED_HASH"
        signature: "0x134567" 
    },
    ...
    }

What's your opinion?

luzzif commented 3 years ago

I think it's possible to have three different properties for the auth (eddsa, ecdsa and approval hash), all at the same level, without enclosing struct, but the user doesn't necessarily need to specify all three, with two not even specified in the body and only one actually used.

So a user would have wither a eddsaSignature, ecdsaSignature or approvalHash specified in a given request body. It's the server role to perform authentication based on whT the user says then.

What do you think?

yueawang commented 3 years ago

Yes, currently we use 3 in body style, the server could choose which way if the user writes more than 1 sig, actually server reports error directly. I was think if type+signature is better for this scenario, but so far all in the body is OK for me.

luzzif commented 3 years ago

Personally, I think it's good enough! The only thing to make sure of, is that every one of the 3 properties is named correctly in order to avoid confusion, then we should be good.

yueawang commented 3 years ago

Yes, i think name in body insures that.

BTW: we want to release our documents by 15th DEC, please pay more attention to the technical writing, for swagger, I think so far it's ok, and if something missed, we can check case by case. Please write more explanation on those important APIs (at least transfer/amm related join&exit), to make sure user know what is storageID, why there are odd & even, how to sign request and pass authentic check, and how to join/swap/exit AMM pool. Thanks!