Dual-Finance / risk-manager

Risk management for Dual Finance protocol DIP positions
MIT License
12 stars 4 forks source link

Wait to subscribe until after checking for MM #172

Closed DonDuala closed 1 year ago

DonDuala commented 1 year ago

Pretty stuck here. This is the best behavior I've been able to control for. This will handle startup cleanly & a single dip deposit. This will still run multiple RMs whose hedges can collide if there a multiple successive quick deposits. Still this is better than whats currently run and could be merged if we don't want to entirely rework routing immediately

DonDuala commented 1 year ago

Could try to create an unsubscribe Poller function to be run during any checkMM

brittcyr commented 1 year ago

Could try to create an unsubscribe Poller function to be run during any checkMM

Is there a problem with routing, or is the issue that we are running too many scalpers? The scalper should be changed to be threadsafe by adding locking. If done correctly, would that solve most of the issues? Would also have to hold the lock during sending to MM, but seems like that would fix it.

DonDuala commented 1 year ago

Could try to create an unsubscribe Poller function to be run during any checkMM

Is there a problem with routing, or is the issue that we are running too many scalpers? The scalper should be changed to be threadsafe by adding locking. If done correctly, would that solve most of the issues? Would also have to hold the lock during sending to MM, but seems like that would fix it.

I'm viewing multiple scalpers running as a symptom of incorrect routing. We should only route when we've confirmed there is a net position change, but the poller triggers a callback on any AccountChangeCallback. So in the event the protocol buys and then sells immediately to MM, routing tries to run RM twice. We can either stop the poller during those checkMM periods or do make it threadsafe. Not sure where to begin on the threadsafe portion if you have a reference

DonDuala commented 1 year ago

@brittcyr This is cleanly handling DIP deposits at startup & on the fly. Worth running

brittcyr commented 1 year ago

@brittcyr This is cleanly handling DIP deposits at startup & on the fly. Worth running

Is it possible to strip out the unrelated stuff and just make this change be for handling routing?

DonDuala commented 1 year ago

@brittcyr This is cleanly handling DIP deposits at startup & on the fly. Worth running

Is it possible to strip out the unrelated stuff and just make this change be for handling routing?

Ya not sure why the merge did that.. Let me fiddle with it