Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dex] DEX-locked funds awareness #3838

Open
norwnd opened this issue Nov 23, 2022 · 7 comments
Open

[dex] DEX-locked funds awareness #3838

norwnd opened this issue Nov 23, 2022 · 7 comments

Comments

@norwnd
Copy link

norwnd commented Nov 23, 2022

That might be something that was already mentioned/discussed somewhere, but I didn't find and decided it is worth mentioning,

When I'm placing an order on DEX-window through Decrediton (on testnet), it isn't reflected in Decrediton itself (an order is already booked, locked balance in Decrediton = 0):

image

that results in a bit of confusion when using same wallet for other things like sending transactions, because it shows all the funds on my "Balance" when I'm trying to send one:

image

but when I try to fill in all the info I get the error below, and I can only guess that it happens because I have 60 DCR locked in an order on DEX out of 119.something shown on screenshot above in my mixed acc1 (canceling DEX order removes this issue, that's why I'm pretty confident it is the root cause for Insufficient funds error I'm seeing; specifying lower amount to send also removes the error):

image

@norwnd
Copy link
Author

norwnd commented Nov 23, 2022

I've tried to reproduce this issue and stumbled upon 2nd type of it, I think. Basically,

  • sometimes I get the Insufficient funds error like I've described above
  • other times I'm actually able to send transaction successfully (it got mined) while still having my DEX order booked (meaning, it will fail if counter party tries to accept it because I no longer have enough DCR since I just sent somewhere else)

Also, I think DEX devs have fixed some issues related to locking/unlocking funds recently (or maybe still working them out), so that could contribute to what I'm observing here as well.

@chappjc
Copy link
Member

chappjc commented Nov 23, 2022

  • other times I'm actually able to send transaction successfully (it got mined) while still having my DEX order booked (meaning, it will fail if counter party tries to accept it because I no longer have enough DCR since I just sent somewhere else)

It should not have spent the locked UTXOs in this case. Have you verified that this transaction you sent from Decrediton spent the coins that were reserved by a DEX order?

Also, I think DEX devs have fixed some issues related to locking/unlocking funds recently (or maybe still working them out), so that could contribute to what I'm observing here as well.

Everything is solid with UTXO coin locking/unlocking. I can't say what is possible for a Decrediton user to manually do to override it though (unlocking coins somehow?). When trades are running, the user shouldn't be spending funds from the used account from within Decrediton.

@alexlyp
Copy link
Member

alexlyp commented Nov 23, 2022

I have a rough idea of how it could happen, I'll have to try it out on testnet myself.

@norwnd
Copy link
Author

norwnd commented Nov 24, 2022

It should not have spent the locked UTXOs in this case. Have you verified that this transaction you sent from Decrediton spent the coins that were reserved by a DEX order?

I didn't check the exact coins used simply because I had: booked-order-value + sent-value > total balance in Decrediton,

I tried to reproduce it again just so we get the complete picture, so I create limit sell order on DEX with the following funding coins:

image

and then I when I don't see any funds being locked in Decrediton (I had/have same ~80 DCR in my Decrediton wallet, in mixed account, before and after my DEX order for selling 60 DCR is booked; with 0 DCR locked before/after order is booked):

image

then I close DEX window and try to send 60 DCR and I get Insufficient funds error (like I mentioned above) - so I'm unable to spend funds locked in my DEX limit order, but I'm not informed about why I can't (because they are locked in DEX order),

then I close Decrediton app and open it again (basically restarting my Decrediton wallet), and while I can still see that 60 DCR sell-limit-order on DEX in "booked" status (I don't open DEX window, and I don't log in here - just looking at the DEX-server order-book from another separate browser-client to make sure the order is still there) I try to send 60 DCR and I now I succeed (maybe if I log in Insufficient funds error will appear again ?), so the behavior changed from error to allowing me to spend funds locked in DEX order after restart spending those same outputs locked in my DEX order.

Note, in both cases (before and after restart) I'm trying to send to my own address (from that same Decrediton wallet), but that shouldn't matter I think ? Becase that funding coin got spent on-chain anyway.

Observing this with Decrediton:release-v1.7.6 testnet.

@chappjc
Copy link
Member

chappjc commented Nov 24, 2022

then I close DEX window and try to send 60 DCR and I get Insufficient funds error (like I mentioned above) - so I'm unable to spend funds locked in my DEX limit order, but I'm not informed about why I can't (because they are locked in DEX order),

In the above case, this is all working as intended except for Decrediton messaging and locked balance display, as you say. Decrediton can be changed to check listunspent and listlockunspent (frequently) to discern the correct locked balance amount.

then I close Decrediton app and open it again (basically restarting my Decrediton wallet), and while I can still see that 60 DCR sell-limit-order on DEX in "booked" status (I don't open DEX window, and I don't log in here - just looking at the DEX-server order-book from another separate browser-client to make sure the order is still there). I try to send 60 DCR and I now I succeed

Well that was an important detail. Yeah, when you restart dcrwallet (that Decrediton restart does), the locked coins are unlocked. Only when you log back into DEX will it re-lock the coins. I think there's ample messaging not to do this, and if you do, to log back in immediately or orders may fail.

@norwnd
Copy link
Author

norwnd commented Nov 24, 2022

In the above case, this is all working as intended except for Decrediton messaging and locked balance display, as you say. Decrediton can be changed to check listunspent and listlockunspent (frequently) to discern the correct locked balance amount.

Yeah, messaging is what I think could be improved there.

Only when you log back into DEX will it re-lock the coins. I think there's ample messaging not to do this, and if you do, to log back in immediately or orders may fail.

Maybe messaging for this could be improved in Decrediton itself: when Decrediton wallet is started and there are open orders on Decrediton-embedded-DEX it could prompt the user to login directly transferring him onto DEX login page, any reason for not to do so ?

@chappjc
Copy link
Member

chappjc commented Nov 28, 2022

Maybe messaging for this could be improved in Decrediton itself: when Decrediton wallet is started and there are open orders on Decrediton-embedded-DEX it could prompt the user to login directly transferring him onto DEX login page, any reason for not to do so ?

Broadly speaking, Decrediton has no clue what DEX is doing, except indirectly by observing the affects on dcrwallet, which include balance and locked UTXO changes. For Decrediton to figure out if there are active orders, it presently requires logging into DEX (and updating the core adapter with a method to get the exchange map with orders, the easy part). We can probably make some changes to client/core.Core to make this info available to consumers like Decrediton prior to logging in. We can look at that after decred/dcrdex#1903

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants
@alexlyp @chappjc @norwnd and others