pegnet / pegnetd

The pegnet daemon to track txs, conversions, etc
Other
13 stars 14 forks source link

Limit conversions #111

Closed mberry closed 3 years ago

mberry commented 4 years ago

Adds a conversion command that only triggers when certain conditions are met. By default this is when the source is below the limit amount. Optional flags include using the destination asset as the trigger instead and/or being above a certain amount. Additionally adds a dryrun flag which will go through the functions logic but will exit before completing the conversion.

Emyrk commented 4 years ago

Hmm. I'm not a huge fan of including this in the reference.

First the limit is not guaranteed. If I run this with a XBT limit of 9K, it will trigger when the rates go over 9K (or under). But that is not the rate I get, I get the next rate. So this limit is just a best attempt. I worry if someone uses this, they would assume the limit is actually enforced.

The other thing is the sleeping. I don't think that behavior is expected. To run the command and potentially wait forever. I feel like this should be implemented in a daemon, not a client.

Ideally, we could craft actual limit txs. I took a stab at it, but realized there is a "feature" in pegnetd that prevents this. We check if the address has enough balance before each tx in a batch before we execute: https://github.com/pegnet/pegnetd/blob/master/node/sync.go#L445-L447. If we fix this, then we can implement limits with the batchs.

mberry commented 4 years ago

Yeah fair call guys, do understand. like the idea of a limit type conversion, but wondering how you would guarantee the limit regardless? If conversions can only happen at the prescribed rate, it seems quite messy otherwise to attempt some method of averaging it out amongst participants per block or enforcing the strict limits.

Renaming it away from the traditional terminology does seem prudent. I have this naming convention in the rough python lib and trigger is far more apt, will fix that when it gets reworked.

Certainly these calls can run forever, being stacked in the daemon is obviously a cleaner solution but those orders themselves can potentially be locked up unwillingly for a long time and then forgotten with little user feedback on the matter, someone running pegnetd may not realise they made a trade 3 weeks ago they not got filled, things do change quickly for plenty of assets we currently track. Perhaps offering and/or enforcing a block limit on conversions is the way.

WhoSoup commented 4 years ago

but wondering how you would guarantee the limit regardless?

The thing emyrk mentioned is more of a neat trick than an intended feature. Basically if you want to guarantee a conversion rate, you can create a batch transaction containing two conversions. One that converts from A to B, one that converts from B to C. If the rate of B is too low then the first conversion won't be able to cover the cost of the second conversion and the entire thing will fail. However, at the moment there's a bug where the first transaction inside a batch isn't credited to the balance that's used to check the second transaction, so you can't chain them.

It's an edge case that would only work under certain circumstances, not the same thing you're proposing

mberry commented 4 years ago

Basically if you want to guarantee a conversion rate, you can create a batch transaction containing two conversions. One that converts from A to B, one that converts from B to C.

Is this only limited to a second conversion or will it have to be chained into other conversions?

Otherwise ongoing cvt batching left unchecked could create a DOS situation and the system keeps creating evermore batch conversions to keep up. Obviously that's an extreme and hopefully there'd be some sanity checks enforced. The main gist is "what do pegnet traders actually want from a conversion?"

Hypothetical: Alice creates a trigger order for $9000 pUSD/pBTC and the price balloons well over that in one block incapable of ever being converted regardless of the basket and number of batch conversions allowed.

Would Alice prefer it to have been triggered at the last price of $9100 rather than being outright cancelled? Perhaps something that should be an option. Seems ripe for user angst, missing out on big moves by cancelled orders seems to be the one big complaint you'll see across all markets.

Edit: Upon a reread that last sentence suggests it won't always work which I misinterpreted, the other parts still stand, should give people a chance to choose.