Hu-Fi / Mr.Market

Mr. Market is the exchange oracle of HuFi, and a CeFi crypto bot on Mixin Messenger
https://mr-market-one.vercel.app
GNU Affero General Public License v3.0
1 stars 6 forks source link

104 #121

Closed mj-blockydevs closed 3 months ago

mj-blockydevs commented 3 months ago

@zed-wong

https://github.com/Hu-Fi/Mr.Market/blob/main/server/src/modules/mixin/rebalance/rebalance.service.ts#L158

Because of the previous conditions:

((a + b) / 2) - a

will always return a value lower than 0. We calculate the average value of 2 variables and then subtract a bigger value from it.

Shouldn't we use abs here: https://github.com/Hu-Fi/Mr.Market/blob/main/server/src/modules/mixin/rebalance/rebalance.service.ts#L158 ?

The same goes for rebalancing from Mixin to the exchange. We won't fulfill this condition: https://github.com/Hu-Fi/Mr.Market/blob/main/server/src/modules/mixin/rebalance/rebalance.service.ts#L224

after succeeding in this one: https://github.com/Hu-Fi/Mr.Market/blob/main/server/src/modules/mixin/rebalance/rebalance.service.ts#L113

Shouldn't we check

if (!balanceBN.isLessThanOrEqualTo(minBalance)) {

instead in this line: https://github.com/Hu-Fi/Mr.Market/blob/main/server/src/modules/mixin/rebalance/rebalance.service.ts#L224 ?

vercel[bot] commented 3 months ago

@arianejasuwienas is attempting to deploy a commit to the hufi Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] commented 3 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
mr-market ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 8, 2024 10:47am
zed-wong commented 3 months ago

@zed-wong

https://github.com/Hu-Fi/Mr.Market/blob/main/server/src/modules/mixin/rebalance/rebalance.service.ts#L158

Because of the previous conditions:

* Mixin amount lower than the amount from DB

* Total Exchange Amount greater than minAmount from DB
  we know that exchange amount has to be greater than Mixin amount. And for a > b:

((a + b) / 2) - a

will always return a value lower than 0. We calculate the average value of 2 variables and then subtract a bigger value from it.

Shouldn't we use abs here: https://github.com/Hu-Fi/Mr.Market/blob/main/server/src/modules/mixin/rebalance/rebalance.service.ts#L158 ?

The same goes for rebalancing from Mixin to the exchange. We won't fulfill this condition: https://github.com/Hu-Fi/Mr.Market/blob/main/server/src/modules/mixin/rebalance/rebalance.service.ts#L224

after succeeding in this one: https://github.com/Hu-Fi/Mr.Market/blob/main/server/src/modules/mixin/rebalance/rebalance.service.ts#L113

Shouldn't we check

if (!balanceBN.isLessThanOrEqualTo(minBalance)) {

instead in this line: https://github.com/Hu-Fi/Mr.Market/blob/main/server/src/modules/mixin/rebalance/rebalance.service.ts#L224 ?

@mj-blockydevs Yes, these are bugs, I'm sorry, please fix them

mj-blockydevs commented 3 months ago

I'm not sure if I didn't miss anything. And if I understood correctly how it SHOULD work. I believe it still has to be double-checked.

zed-wong commented 3 months ago

Also I wonder if it's possible to have it tested in real environment with at least one exchange (like okx). It would decide if the whole thing works.