client: Move market making out of core #2320

martonp · 2023-04-21T23:19:07Z

 client: Remove Bot from Core

In preparation for moving the bot logic outside of core, all of
the previous bot logic is deleted. The UI is also removed as it
will no longer function without major modification.

 client: Put market making logic outside of core

This moves the market making logic outside of core. There is a new
`MarketMaker` type which calls into core. When running the `MarketMaker`,
one strategy can be defined for each dex market. Since the Gap strategy
was originally designed to have multiple bots running on a market, the
strategy will have to be modified, but this diff only deals with
refactoring the logic outside of core.

The RPCServer is updated with startMarketMaking and stopMarketMaking
commands. Market making can be started by providing a path to a config
file.

New Core methods are also added to support the MarketMaker: `ExchangeMarket`
and `SingleLotFees`.

martonp · 2023-04-22T00:36:18Z

I'm thinking to first build out all the features, including balance handling, profit/loss reporting and the three different strategy types, then restore the UI once everything is mostly finalized.

chappjc · 2023-04-24T14:51:47Z

I'm thinking to first build out all the features, including balance handling, profit/loss reporting and the three different strategy types, then restore the UI once everything is mostly finalized.

Makes sense. I guess main would create something like a BotMgr instance that is given to webserver and rpcserver.

Maybe we don't need to rm mm.{ts,scss,htmpl} though since when it gets plugged back in the current page basically has the right controls and all? While I think the strongest use case for a bot is a cli app with config file/rpc controls, we want to continue offering the GUI controls. We wouldn't even want to do the next major release without it, even if all the advanced bot features aren't yet complete.

What are the main hurdles in retaining the existing UI (at least create/start/stop)? Looks like it would be the hypothetical BotMgr.

martonp · 2023-04-24T21:42:21Z

There doesn't need to be a complete overhaul to the UI, but there are significant changes that need to be made. Instead of starting and stopping individual bot programs, it would be one stop and start button for the entire system. The calls made to the webserver will also have to change. Also, there are a few more settings that will need to be added to this strategy before it's complete. I was just removing it so that there is no unusable code in the repo, and I didn't want to fix the UI until at least this one strategy is finalized. I can add it back though so that the PR for updating the UI is smaller though if that's what you're worried about.

chappjc · 2023-04-24T21:44:59Z

I can add it back though so that the PR for updating the UI is smaller though if that's what you're worried about.

No, just that with git you might lose the ability to track line-by-line changes over time if it gets rm'd and created fresh later. If you think the end UI will be very different then it sounds like there's no point to that.

chappjc · 2023-04-30T23:02:05Z

There doesn't need to be a complete overhaul to the UI, but there are significant changes that need to be made. .... I was just removing it so that there is no unusable code in the repo, and I didn't want to fix the UI until at least this one strategy is finalized. I can add it back though so that the PR for updating the UI is smaller though if that's what you're worried about.

As per my last comment, it's just that with git you might lose the ability to track line-by-line changes over time if it gets rm'd and created fresh later. At this point, I think we should leave the big frontend source codes, but have it inaccessible from the frontend. This bot project is going to introduce a lot of unusable code until this project is complete, as you say. The webserver mm page route could simply be inaccessible (e.g. redirect to /markets) until at least the new webserver route handlers are created and the basic conceptual changes (start/stop bot, not bots, update programs, etc.) have been adapted to.

Anyway, I say we move forward with the refactor of mm to a core consumer (this PR), but we should really get the UI elements back asap, even if some of the controls are inactive. You could mock it up as if everything is done, and I think that would help shed light on the end goal. If we want to release a 1.0 in the near term, which is something we want before starting major changes for mesh, we can't have the MM controls vanish.

martonp · 2023-05-01T01:39:56Z

OK I'll add it back and update the UI ASAP. I think it would be nice to have #2332 in before updating the UI though.

chappjc · 2023-05-01T02:13:21Z

OK I'll add it back and update the UI ASAP. I think it would be nice to have #2332 in before updating the UI though.

I'm sure we can get these two in pretty quick. I scanned the balance PR and it make sense in concept.

chappjc

Needs resolve, but LGTM. I'm very excited about this project. Can't wait to see it all come together.

client/core/core.go

client/asset/btc/btc.go

client/mm/config.go

client/mm/mm.go

martonp · 2023-05-04T07:21:54Z

Fixed @JoeGruffins's review comments, added the UI code back, and the MarketMaking button links to the markets page now.

In preparation for moving the bot logic outside of core, all of the previous bot logic is deleted. The UI is also removed as it will no longer function without major modification.

This moves the market making logic outside of core. There is a new `MarketMaker` type which calls into core. When running the `MarketMaker`, one strategy can be defined for each dex market. Since the Gap strategy was originally designed to have multiple bots running on a market, the strategy will have to be modified, but this diff only deals with refactoring the logic outside of core. The RPCServer is updated with startMarketMaking and stopMarketMaking commands. Market making can be started by providing a path to a config file. New Core methods are also added to support the MarketMaker: `ExchangeMarket` and `SingleLotFees`.

buck54321

I like where this is headed.

buck54321 · 2023-05-04T12:38:12Z

client/asset/dcr/dcr.go

+	const numInputs = 10
+	txSize := dexdcr.InitTxSizeBase + (numInputs * dexdcr.P2PKHInputSize)


Can you comment on the choice of 10 inputs?

It was kind of an arbitrary choice.. I just wanted to get a reasonably safe estimate without having to actually look inside the wallet. Do you think I should pick a higher number?

I'll update to 12, same as what the bonds fee buffer uses.

buck54321 · 2023-05-04T12:42:37Z

client/asset/dcr/dcr.go

+	const numInputs = 10
+	txSize := dexdcr.InitTxSizeBase + (numInputs * dexdcr.P2PKHInputSize)
+
+	var splitTxSize int


You can just make txSize and splitTxSize uint64s, and skip the conversion when you sum below.

buck54321 · 2023-05-04T13:52:41Z

client/core/bot_test.go

No migrations for TestBasisPrice, TestBreakEvenHalfSpread, or TestRebalance? They all seem to have analogues.

I'm bringing those back in another PR I'm working on right now that updates the strategy a bit. Instead of just putting just one order on each side of the market there will be an array of orders at different distances from the midgap.

buck54321 · 2023-05-04T14:10:21Z

client/mm/price_oracle.go

+	parts := strings.Split(u.Host, ".")
+	if len(parts) < 2 {
+		return "", fmt.Errorf("not enough URL parts: %q", u.Host)
+	}
+	return parts[len(parts)-2] + "." + parts[len(parts)-1], nil


We found this approach insufficient for handling IP addresses in eth/multirpc.go. I don't expect you to fix this here, just noting.

martonp force-pushed the removeBotCore branch from 17ca5e2 to 426f378 Compare April 21, 2023 23:19

martonp mentioned this pull request Apr 30, 2023

client/{mm,core}: Market maker balance segregation #2332

Merged

chappjc approved these changes May 3, 2023

View reviewed changes

client/core/core.go Show resolved Hide resolved

JoeGruffins approved these changes May 4, 2023

View reviewed changes

client/asset/btc/btc.go Outdated Show resolved Hide resolved

client/mm/config.go Show resolved Hide resolved

client/mm/mm.go Show resolved Hide resolved

martonp force-pushed the removeBotCore branch 2 times, most recently from 23606be to b89e3d9 Compare May 4, 2023 07:19

martonp added 3 commits May 4, 2023 03:45

client: Remove Bot from Core

5ba6c6e

In preparation for moving the bot logic outside of core, all of the previous bot logic is deleted. The UI is also removed as it will no longer function without major modification.

Joe review fixes

bcaa232

martonp force-pushed the removeBotCore branch from b89e3d9 to bcaa232 Compare May 4, 2023 07:52

buck54321 approved these changes May 4, 2023

View reviewed changes

Buck review fixes

d70b60e

chappjc merged commit de0d52c into decred:master May 7, 2023

martonp deleted the removeBotCore branch January 20, 2024 13:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client: Move market making out of core #2320

client: Move market making out of core #2320

martonp commented Apr 21, 2023

martonp commented Apr 22, 2023

chappjc commented Apr 24, 2023 •

edited

Loading

martonp commented Apr 24, 2023

chappjc commented Apr 24, 2023

chappjc commented Apr 30, 2023

martonp commented May 1, 2023

chappjc commented May 1, 2023

chappjc left a comment

martonp commented May 4, 2023

buck54321 left a comment

buck54321 May 4, 2023

martonp May 5, 2023

martonp May 5, 2023

buck54321 May 4, 2023

buck54321 May 4, 2023

martonp May 5, 2023

buck54321 May 4, 2023

		const numInputs = 10
		txSize := dexdcr.InitTxSizeBase + (numInputs * dexdcr.P2PKHInputSize)

client: Move market making out of core #2320

client: Move market making out of core #2320

Conversation

martonp commented Apr 21, 2023

martonp commented Apr 22, 2023

chappjc commented Apr 24, 2023 • edited Loading

martonp commented Apr 24, 2023

chappjc commented Apr 24, 2023

chappjc commented Apr 30, 2023

martonp commented May 1, 2023

chappjc commented May 1, 2023

chappjc left a comment

Choose a reason for hiding this comment

martonp commented May 4, 2023

buck54321 left a comment

Choose a reason for hiding this comment

buck54321 May 4, 2023

Choose a reason for hiding this comment

martonp May 5, 2023

Choose a reason for hiding this comment

martonp May 5, 2023

Choose a reason for hiding this comment

buck54321 May 4, 2023

Choose a reason for hiding this comment

buck54321 May 4, 2023

Choose a reason for hiding this comment

martonp May 5, 2023

Choose a reason for hiding this comment

buck54321 May 4, 2023

Choose a reason for hiding this comment

chappjc commented Apr 24, 2023 •

edited

Loading