Liquidity Module for Swagger documentation QA (2.2.1) #206
Conversation
There was a problem hiding this comment.
@dongsam so awesome to see these files in entirety, thank you so much. I think that we can continue to improve this content by clarifying the intention of each message and pinning down the language that accurately describes each message. I made some assumptions based on the light paper (which I should have started with, I think I'm starting to understand little bit now). I can use help in the many places I wasn't sure
There was a problem hiding this comment.
@barriebyron Thanks for taking your time to provide this valuable feedback. It is really helpful. I left some comments to give you my opinion and some answers to questions. Check out the links that I provide in the comments and if you still feel lost or curious about anything, feel free to let me know. This concept took me a while to understand as well. I am also learning some of the new concepts that are coming up in the next version.
|
|
||
| option go_package = "github.com/tendermint/liquidity/x/liquidity/types"; | ||
|
|
||
| // Msg defines the liquidity Msg service. |
There was a problem hiding this comment.
Since this liquidity module will be included in Cosmos SDK, we try to follow how they comment service.
References:
| // Withdraw submit to the batch from the Liquidity pool with the specified `pool_id`, `pool_coin` of the pool | ||
| // this requests are stacked in the batch of the liquidity pool, not immediately processed and | ||
| // processed in the `endblock` at once with other requests. |
There was a problem hiding this comment.
All requests (deposit, withdraw, swap) are accumulated in what is called batch and they are processed at the end of each block.
In Cosmos SDK, there are BeginBlocker and EndBlocker. They will be triggered at the beginning and at the end of each block respectively. In this liquidity module, we add ExecutePoolBatch logic to execute whatever requests (deposit, withdraw, swap) are accumulated in batch. I hope this makes sense? If you need more explanation, let me know :)
|
@barriebyron Thanks for the review and suggestion! Except for a few things we're still discussing, we've applied and committed them all. If the discussions are resolved, we will merge them to the develop branch |
|
I just open new PR #214 to apply the suggestions using to cherry-pick for minimizing conflicts I think all discussions are resolved, and If there is no further suggestion, we will close the PR. |
Okay, so we're gonna keep this PR going, and if get suggestions, I'll do it like #214 |
There was a problem hiding this comment.
thanks for your thoughtful comments @defihyung and welcome to Tendermint! See my new questions and suggestions
|
|
||
| option go_package = "github.com/tendermint/liquidity/x/liquidity/types"; | ||
|
|
||
| // Msg defines the liquidity Msg service. |
| // Withdraw submit to the batch from the Liquidity pool with the specified `pool_id`, `pool_coin` of the pool | ||
| // this requests are stacked in the batch of the liquidity pool, not immediately processed and | ||
| // processed in the `endblock` at once with other requests. |
There was a problem hiding this comment.
Very good explanation, I think the suggested change still makes sense.
| // Withdraw submit to the batch from the Liquidity pool with the specified `pool_id`, `pool_coin` of the pool | ||
| // this requests are stacked in the batch of the liquidity pool, not immediately processed and | ||
| // processed in the `endblock` at once with other requests. |
There was a problem hiding this comment.
@defihyung are BeginBlocker and EndBlocker the operations that begin and end a block?
per https://github.com/cosmos/cosmos-sdk/blob/master/docs/spec/SPEC-SPEC.md
If I understand correctly, we now have a new operation category for batch. Would it make sense to put the batch operations in a new file XX_batch.md?
dongsam
force-pushed
the
suggestions-swagger
branch
from
80b49bb
to
e5ef323
Compare
barriebyron
changed the title
Description
It is PR to get the swagger documentation QA suggestions based on the sources, continues to after #172, #188
The commit of this PR consists of a .proto, swagger file diff to receive reviews and suggestions comments and is not for merge purposes, so please ignore the confilts
In 2.2.1 version, #173 (comment) feedbacks was applied and a description of the structure was added and modified.