search

DeXter-on-Radix / website

Community built order book dex on Radix.
https://dexteronradix.com/
Other
13 stars 21 forks source link

Order details feature #56

Closed maelsar closed 1 year ago

maelsar commented 1 year ago

Feature for MVP from #1

Todo:

EvgeniiaVak commented 1 year ago

the backend connection is already done basically - we get most of the data from AlphaDEX (see contexts.ts file and how it's used), if something is missing in the AdexStateContext, you can try to use the wallet connection, but that's on generate_orders branch, not in main yet

maelsar commented 1 year ago

thank you. i have been looking into that. i was even thinking of branching from generate_orders branch moving forward. in the meantime i am doing some reading on the api documentation. I will post some updates when i have more work done.

EngineerCharlie commented 1 year ago

To briefly summarize in case you weren't aware: If you need information about the user (address, persona) you'll need to use the RDT. If you want information about assets/NFTs, you'll need to use the gateway.

If you need to use the gateway, then it's implemented poorly on the generate orders branch, but it's much cleaner on the order-button-on-v0.3.

EvgeniiaVak commented 1 year ago

@maelsar could you provide more details about "Cancel unfilled limit order" - what this item stands for?

UPD: ah, it's just copied from #1, so the question is for @steferr actually

maelsar commented 1 year ago

yes, that is a copy from issue #1 . If i am mistaken, making this feature should also fix "partially filled orders".

EvgeniiaVak commented 1 year ago

I'm thinking if we need to merge generate_orders or order-button-on-v0.3 into main to unblock @maelsar , or should adex context be enough? cc @SmashingBumpkin

maelsar commented 1 year ago

I am able to view order history from adex, but eventually it would be nice to be able to test it live after an order has been placed. No rush. i can work with what i have until it is ready. will give updates in a day or two.

EvgeniiaVak commented 1 year ago

Ok, cool, so we could skip the v0.2 and resume #35 further when AlphaDEX for RCNet v0.3 is ready, @fliebenberg please ping us when it happens

EngineerCharlie commented 1 year ago

As mentioned im reluctant to push either, but if it makes life easier we can do. Mael won't be able to push orders on either branch, as they will only get access to the v0.3 app, which isn't live with alphadex just yet.

maelsar commented 1 year ago

I am still working from the v0.2 from main. I will modify accordingly when v0.3. Would love to hear your opinions.

Still a work in progress. This is currently showing Order History. Just wondering why is there negative price @fliebenberg. table is rendering the SdkResult from getAccountOrders. Thanks! Image

EngineerCharlie commented 1 year ago

Still a work in progress. This is currently showing Order History. Just wondering why is there negative price @fliebenberg. table is rendering the SdkResult from getAccountOrders. Thanks! Image

It looks like rhe negative price is because it's a market order. When submitting an order, if it's market you specify an amount and a slippage, with price set to -1. If it's limit you specify amount and price, with slippage set to -1. Perhaps fred put the executed price in a different parameter of the return object?

steferr commented 1 year ago

@maelsar could you provide more details about "Cancel unfilled limit order" - what this item stands for?

UPD: ah, it's just copied from #1, so the question is for @steferr actually

When a user places a limit order it gets placed both in the "open orders" and in the "orders history" tabs. When the order is completely filled, it is deleted from "open orders". Until then, whether it's completely unfilled or partially filled, user should be able to cancel it.

See Figma

fliebenberg commented 1 year ago

Hi. Yes, the negative price is because it is a market order and therefore there is no specified price. I can't remember offhand now, but there should be another field in the order receipt that shows the average executed price I think. If it is not there then I will add it as it is very easy to calculate (token1 units received/sent) divided by (Token2 units received/sent). Will check on Sunday when I get home. It also depends what we want to show in the history. Do we show the specified price for the order or the actual price traded. These could be different even for a limit order. Eg if you send a limit order to buy at a price of 10, but there are tokens available at 9, then your order will be executed at the better price of 9.

On Thu, 03 Aug 2023, 2:51 PM SmashingBumpkin, @.***> wrote:

Still a work in progress. This is currently showing Order History. Just wondering why is there negative price @fliebenberg https://github.com/fliebenberg. table is rendering the SdkResult from getAccountOrders. Thanks! [image: Image] https://user-images.githubusercontent.com/107124432/258106422-4ef7f9fc-8437-486f-9288-6dfad34878ce.PNG

It looks like rhe negative price is because it's a market order. When submitting an order, if it's market you specify an amount and a slippage, with price set to -1. If it's limit you specify amount and price, with slippage set to -1. Perhaps fred put the executed price in a different parameter of the return object?

— Reply to this email directly, view it on GitHub https://github.com/DeXter-on-Radix/website/issues/56#issuecomment-1663928632, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHGDSQK2A6UFA6DMMNMF23LXTONGLANCNFSM6AAAAAA26HT45I . You are receiving this because you were mentioned.Message ID: @.***>

maelsar commented 1 year ago

Yes, thank you. i have been looking for the executed price and i cant find it on the response.
"Do we show the specified price for the order or the actual price traded. These could be different even for a limit order." - i think both specified price and avg executed price would be useful for some traders (market/ filled-limit/ partially-filled).

May i suggest also, that under the OrderReceipt to also add another field for pair name. I am not sure if my approach is correct, but currently i need to make another call to pairList and match the pairAddress to the pairList to get the name.

@fliebenberg - this may not be necessary for the MVP but what happens to the order receipts of a trading pair if in the future it has been delisted? i would assume that the resource address becomes null but then we could no longer reference the name of the pair in trade history.

maelsar commented 1 year ago

updates for MVP:

post features:

Image Image Image

fliebenberg commented 1 year ago

Yes, thank you. i have been looking for the executed price and i cant find it on the response. "Do we show the specified price for the order or the actual price traded. These could be different even for a limit order." - i think both specified price and avg executed price would be useful for some traders (market/ filled-limit/ partially-filled).

I will look at this today to see what is available or how to best implement.

May i suggest also, that under the OrderReceipt to also add another field for pair name. I am not sure if my approach is correct, but currently i need to make another call to pairList and match the pairAddress to the pairList to get the name.

I try to keep the data we store on-ledger to the minimum as it all adds to the cost of a transaction. You already have all the pair data stored in memory in the SDK, so it should be very quick to just look that info up.

@fliebenberg - this may not be necessary for the MVP but what happens to the order receipts of a trading pair if in the future it has been delisted? i would assume that the resource address becomes null but then we could no longer reference the name of the pair in trade history. Because all data on the ledger is permanent, it actually never gets completely deleted. The way I deal with it is to mark a pair as either active/inactive. So the pair and all its info will always be available, even when it it has been "delisted".

fliebenberg commented 1 year ago

I have added pairName to the orders data , so need to look it up anymore. I also added an amountFilled field that indicated how much of the specified amount has been filled already (similar to completedPerc) and also added a list of trade ids that relates to the order in a field called trades. This info can be used to show the user only their trades (rather than all trades for the pair). You need to update to the latest version of the SDK to get these fields.

fliebenberg commented 1 year ago

I'm thinking if we need to merge generate_orders or order-button-on-v0.3 into main to unblock @maelsar , or should adex context be enough? cc @SmashingBumpkin

Have approved @EvgeniiaVak's PR and merged "order-button-on-v0.3" into main.

EvgeniiaVak commented 1 year ago

fixed by #89