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

ui: arb + market maker ui #2615

Merged
merged 4 commits into from
Feb 5, 2024
Merged

ui: arb + market maker ui #2615

merged 4 commits into from
Feb 5, 2024

Conversation

buck54321
Copy link
Member

Implement arb + mm ui. This required a fair amount refactoring on both the front and back ends. For the back end, the primary changes are about when and how cexes are loaded. Since we need to know what markets are available before the bot it started, we need to call (CEX).Markets() during initialization, before the CEX is running. This occurs either during startup (MarketMaker is now a dex.Connector), or when the cex is added (through the new UpdateCEXConfig method).

In order to facilitate ui testing, I've made the MarketMaker an interface for use by WebServer (like clientCore), and then implemented the interface in live_test.go.

On the front end, most effort is focused around mmsettings, and there was a lot of refactoring done to facilitate the changes and clean things up.

I have attempted to test on testnet, but can't seem to get a data stream, so the book is empty (and timed out since book.latestUpdate is never set). We may want to consider writing a mock server for all simnet and testnet http and ws endpoints, rather than just wallet endpoints.

I'm not going for perfection in the UX yet, but this is a big step forward and I'll keep iterating.

image
image
image

Copy link
Collaborator

@martonp martonp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI is starting to look really great!

My main question is how will we set the Simple Arb strategy with this design?
I think it may be better if there's another popup after selecting the market that allows you to pick the strategy. Then once you're on the settings page, you just information relevant to that strategy, and a button that shows the strategy selection popup again to change the strategy. Once we add more CEXes, there will be many buttons, and it's irrelevant when you're doing market making without arb. Also there may be more strategies in the future.

I see a few issues also:

  • When in mm-arb mode, the reset button doesn't clear the yellow border on the new settings.
  • In mm-arb mode there is no way to set the balances.
  • It should show the CEX balances on the /mm page as well. Looks like it'll be cluttered though, maybe have two lines each for base and quote balance, one for local balance and one for CEX.
  • Should we still show the oracles/fiat rates when we're doing MM-Arb? Those are not involved in the bot's decision making. Maybe show some info about the CEX that will be arbed. This could be in another PR though, it's a larger job.
  • I don't see any way to change the API key once I set it.

@@ -1200,6 +1209,7 @@ type bnMarket struct {
QuoteAsset string `json:"quoteAsset"`
QuoteAssetPrecision int `json:"quoteAssetPrecision"`
OrderTypes []string `json:"orderTypes"`
Permissions []string `json:"permissions"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why add Permissions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. I was just poking around trying to figure out the testnet websocket stream thing. I forgot to delete.

}

/*
* config returns the curret MarketMakingConfig. config will fetch the
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* config returns the curret MarketMakingConfig. config will fetch the
* config returns the current MarketMakingConfig. config will fetch the

* config returns the curret MarketMakingConfig. config will fetch the
* MarketMakingConfig once. Future updates occur in updateCEXConfig and
* updateBotConfig. after a page handler calls config during intitialization,
* the the config can be accessed directly thr MM.cfg to avoid the async
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* the the config can be accessed directly thr MM.cfg to avoid the async
* the the config can be accessed directly the MM.cfg to avoid the async

@buck54321
Copy link
Member Author

In mm-arb mode there is no way to set the balances.

I actually thought balance tracking wasn't enabled until #2568 was merged. My bad.

It should show the CEX balances on the /mm page as well. Looks like it'll be cluttered though, maybe have two lines each for base and quote balance, one for local balance and one for CEX.

Definitely. Just like cex markets, we need to grab cex balances before Connect so that we can math.

I don't see any way to change the API key once I set it.

Haven't gotten that far yet. Just need a button, really.

client/mm/mm.go Outdated
Comment on lines 420 to 499
if cfg.ArbMarketMakerConfig != nil {
continue
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not for the arb bot?

@@ -447,6 +448,7 @@ var EnUS = map[string]string{
"base_asset_balance_tooltip": "The amount of the base asset to allocate for this bot. If the upper end of this range is < 100%, it means that the rest of the balance was allocated to other bots.",
"quote_asset_balance_tooltip": "The amount of the quote asset to allocate for this bot. If the upper end of this range is < 100%, it means that the rest of the balance was allocated to other bots.",
"drift_tolerance_tooltip": "How far from the ideal price orders are allowed to drift before they are cancelled and re-booked.",
"order_persistence_tooltip": "How long CEX orders that don't immediately match be are allowed to remain booked",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"order_persistence_tooltip": "How long CEX orders that don't immediately match be are allowed to remain booked",
"order_persistence_tooltip": "How long CEX orders that don't immediately match are allowed to remain booked",

Copy link
Collaborator

@martonp martonp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The back button on the mmsettings page doesn't work if a market has not been selected. Would it be better if it just redirected back to the mm page if the popup is closed without selecting a market and popup?

client/app/config.go Outdated Show resolved Hide resolved
client/mm/libxc/binance.go Outdated Show resolved Hide resolved
client/mm/mm.go Outdated Show resolved Hide resolved
client/mm/mm.go Outdated Show resolved Hide resolved
client/mm/mm.go Outdated Show resolved Hide resolved
client/mm/mm.go Outdated Show resolved Hide resolved
Comment on lines 1434 to 1435
if (baseBalance === 0) setError(page.baseBalanceErr, intl.ID_NO_ZERO)
if (quoteBalance === 0) setError(page.quoteBalanceErr, intl.ID_NO_ZERO)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we allow empty balance for one of the assets? They could start off with 0, then after the first trade have balance on that asset.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Removing.

I think there are some additional validation steps that can be done during configuration, but it'll be a project of its own. I plan to follow up with some "quick configuration" options that will help the user get their settings right without having to manually adjust everything.

client/webserver/site/src/js/mmsettings.ts Outdated Show resolved Hide resolved
client/mm/mm.go Show resolved Hide resolved
client/webserver/site/src/js/mmsettings.ts Outdated Show resolved Hide resolved
@buck54321 buck54321 force-pushed the binance-ui branch 2 times, most recently from b0780bf to 898014f Compare January 22, 2024 12:38
Copy link
Collaborator

@martonp martonp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's having some issues when I run dexc without the testbinance script running. I go to the page where it's showing the connection error. Then I run the testbinance runserver script, and when I press the submit button to reconnect, I get a javascript error. Also, if it wasn't able to connect the first time, then try to I reconnect, dexc cannot shut down. Seems to be some deadlock, but I cannot find the culprit yet.
Screenshot 2024-01-25 at 4 43 14 PM

Also, the connection error is sticking out of the box:
Screenshot 2024-01-25 at 4 42 14 PM

One other issue is that if my screen is smaller, I cannot scroll vertically on the mm settings page.

client/webserver/site/src/html/mmsettings.tmpl Outdated Show resolved Hide resolved
Implement arbmm ui. This required a fair amount refactoring on
both the front and back ends. For the back end, the primary
changes are about when and how cexes are loaded. Since we need
to know what markets are available before the bot it started,
we need to call Markets() on cex initialization, before the CEX
is running. This occurs either during startup (MarketMaker is
now a dex.Connector), or when the cex is added (through the new
UpdateCEXConfig method).

In order to facilitate ui testing, I've made the MarketMaker an
interface for use by WebServer (like clientCore), and then
implemented the interface in live_test.go.

On the front end, most effort is focused around mmsettings, and
there was a lot of refactoring done to facilitate the changes
and clean things up.
Adapt to new configuration style and new rebalance settings.
Copy link
Collaborator

@martonp martonp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@buck54321 buck54321 merged commit d77fd7d into decred:master Feb 5, 2024
5 checks passed
peterzen pushed a commit to peterzen/dcrdex that referenced this pull request Feb 25, 2024
* ui: arb + market maker ui

Implement arbmm ui. This required a fair amount refactoring on
both the front and back ends. For the back end, the primary
changes are about when and how cexes are loaded. Since we need
to know what markets are available before the bot it started,
we need to call Markets() on cex initialization, before the CEX
is running. This occurs either during startup (MarketMaker is
now a dex.Connector), or when the cex is added (through the new
UpdateCEXConfig method).

In order to facilitate ui testing, I've made the MarketMaker an
interface for use by WebServer (like clientCore), and then
implemented the interface in live_test.go.

On the front end, most effort is focused around mmsettings, and
there was a lot of refactoring done to facilitate the changes
and clean things up.
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

Successfully merging this pull request may close these issues.

3 participants