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

martonp · 2023-04-30T18:50:56Z

New configurations are added to the market maker config specifying what part of the wallet's balance each market maker will have control over. Using this config, the MarketMaker sets initial balances for each of the bots, and this is increased and decreased as the bot makes trades.

A wrappedCore is also added. This wraps the core methods with additional logic that checks and updates each bot's balances of each asset. A wrappedCore is passed to each bot, and the core methods behave as if the entire balances of the wallets are only what has been allocated to that bot.

client/asset/interface.go

client/asset/dcr/dcr.go

client/mm/balance_handler.go

client/asset/btc/btc.go

client/mm/balance_handler.go

client/asset/btc/btc.go

martonp · 2023-07-17T14:13:57Z

I've moved the balance tracking to be part of MarketMaker instead of a separate balanceHandler struct. This is more convenient because the orderInfos are also useful for creating the history of all actions a bot took and tracking the P/L.

New configurations are added to the market maker config specifying what part of the wallet's balance each market maket will have control over. Using this config, a `balanceHandler` object is created, which tracks each of the orders made by the market makers, decreasing and increasing their balances as needed over the lifecycle of the order. The `balanceHandler` is not called directly by the market makers, instead a `coreWithSegregatedBalance` is created which wraps the core methods so that from the point of view of the bot, they behave as if the entire balance of the wallet is only what has been allocated to that bot by the `balanceHandler`.

This does not change any behavior. It will be used to display on the UI.

buck54321 · 2023-07-22T18:17:19Z

client/mm/wrapped_core.go

+	fundingFees, err := c.core.MaxFundingFees(quote, 1, options)
+	if err != nil {
+		return 0, err
+	}
+
+	swapFees, redeemFees, err := c.SingleLotFees(&core.SingleLotFeesForm{
+		Host:          host,
+		Base:          base,
+		Quote:         quote,
+		UseMaxFeeRate: true,
+	})


The market maker's rebalancing uses MaxBuy, which in this implementation is always based on estimates using a constant number of inputs. Why not get accurate values when the wallets are connected? Do we really need offline estimates to begin with? Where are we presenting order estimates before wallets are connected?

I actually stopped using MaxBuy in the updates to the market maker. If we're getting accurate values, then there would need to be some locking to make sure that MaxBuy -> MultiTrade calls are atomic, if multiple bots are trading with the same asset. If they are not atomic, then the UTXOs when estimating the order would be different than the UTXOs when making the trade. I think a conservative estimate would be quicker and still safe.

buck54321 · 2023-07-22T18:28:02Z

client/core/core.go

+		dc.assetsMtx.Lock()
+		swapAsset := dc.assets[wallets.fromWallet.AssetID]


The assetsMtx only protects the dc.assets field, and the stored map[uint32]*dex.Asset is read-only, so you can just

dc.assetsMtx.RLock() swapAsset, redeemAsset := dc.assets[wallets.fromWallet.AssetID], dc.assets[wallets.toWallet.AssetID] dc.assetsMtx.RUnlock() ... and avoid all the unlocking below.

buck54321 · 2023-07-22T19:38:20Z

client/mm/mm.go

+		var balanceMods []*balanceMod
+		if orderInfo.order.Sell {
+			balanceMods = []*balanceMod{
+				{false, orderInfo.order.BaseID, balTypeFundingOrder, match.Qty},


Would be helpful to assign these bools to consts.

const ( balanceModIncrease = true balanceModDecrease = false )

buck54321 · 2023-07-30T03:18:02Z

client/mm/wrapped_core.go

+
+	mm    *MarketMaker
+	botID string
+	core  clientCore


Looks like you're embedding now. Can drop this field?

No, because sometimes the wrapped_core needs to call a function with the same name in the actual core. In this case the field needs to be used, otherwise there will be an infinite loop.

You can call methods on an embedded field by using the type name.

mkt, err := c.clientCore.ExchangeMarket(form.Host, form.Base, form.Quote)

buck54321 · 2023-07-30T03:39:41Z

client/mm/mm.go

+		switch mod.typ {
+		case balTypeAvailable:
+			if mod.increase {
+				assetBalance.Available += mod.amount
+			} else {
+				if assetBalance.Available < mod.amount {
+					m.log.Errorf("modifyBotBalance: bot %s has insufficient balance for asset %d. "+
+						"balance: %d, amount: %d", botID, mod.assetID, assetBalance.Available, mod.amount)
+					assetBalance.Available = 0
+					return
+				}
+				assetBalance.Available -= mod.amount
+			}
+		case balTypeFundingOrder:
+			if mod.increase {
+				assetBalance.FundingOrder += mod.amount
+			} else {
+				if assetBalance.FundingOrder < mod.amount {
+					m.log.Errorf("modifyBotBalance: bot %s has insufficient funding order for asset %d. "+
+						"balance: %d, amount: %d", botID, mod.assetID, assetBalance.FundingOrder, mod.amount)
+					assetBalance.FundingOrder = 0
+					return
+				}
+				assetBalance.FundingOrder -= mod.amount
+			}
+		case balTypePendingRedeem:
+			if mod.increase {
+				assetBalance.PendingRedeem += mod.amount
+			} else {
+				if assetBalance.PendingRedeem < mod.amount {
+					m.log.Errorf("modifyBotBalance: bot %s has insufficient pending redeem for asset %d. "+
+						"balance: %d, amount: %d", botID, mod.assetID, assetBalance.PendingRedeem, mod.amount)
+					assetBalance.PendingRedeem = 0
+					return
+				}
+				assetBalance.PendingRedeem -= mod.amount
+			}
+		case balTypePendingRefund:
+			if mod.increase {
+				assetBalance.PendingRefund += mod.amount
+			} else {
+				if assetBalance.PendingRefund < mod.amount {
+					m.log.Errorf("modifyBotBalance: bot %s has insufficient pending refund for asset %d. "+
+						"balance: %d, amount: %d", botID, mod.assetID, assetBalance.PendingRefund, mod.amount)
+					assetBalance.PendingRefund = 0
+					return
+				}
+				assetBalance.PendingRefund -= mod.amount
+			}
+		}


newFieldValue := func(balanceType string, initialValue uint64) uint64 { if mod.increase { return initialValue + mod.amount } else { if assetBalance.Available < mod.amount { m.log.Errorf("modifyBotBalance: bot %s has insufficient %s for asset %d. "+ "balance: %d, amount: %d", botID, balanceType, mod.assetID, initialValue, mod.amount) return 0 } return initialValue - mod.amount } } switch mod.typ { case balTypeAvailable: assetBalance.Available = newFieldValue("available balance", assetBalance.Available) case balTypeFundingOrder: assetBalance.FundingOrder = newFieldValue("funding order", assetBalance.FundingOrder) case balTypePendingRedeem: assetBalance.PendingRedeem = newFieldValue("pending redeem", assetBalance.PendingRedeem) case balTypePendingRefund: assetBalance.PendingRefund = newFieldValue("pending refund", assetBalance.PendingRefund) }

buck54321 · 2023-07-30T03:58:58Z

client/mm/wrapped_core.go

+	}
+
+	var totalLots uint64
+	remainingBalance := quoteBalance - fundingFees


Need to switch on sell here too.

buck54321 · 2023-07-30T04:32:21Z

client/mm/mm.go

+	unusedLockedFundsReturned bool
+	excessFeesReturned        bool
+	matchesSeen               map[order.MatchID]struct{}
+	matchesRefunded           map[order.MatchID]struct{}


This is also used to indicate MatchComplete status without a refund. How about naming it matchesSettled or something.

buck54321 · 2023-07-30T04:48:03Z

client/mm/mm.go

@@ -139,11 +794,11 @@ func (m *MarketMaker) Stop() {
 	}
 }

-// NewMarketMaker creates a new MarketMaker.
-func NewMarketMaker(core *core.Core, log dex.Logger) (*MarketMaker, error) {
+func NewMarketMaker(c clientCore, log dex.Logger) (*MarketMaker, error) {


Please move this up right under the MarketMaker type declaration.

client/mm/mm.go

buck54321 · 2023-07-30T05:13:02Z

client/mm/mm.go

+		if cfg.BaseBalanceType == Percentage {
+			baseRequired = baseBalance.balanceAvailable * cfg.BaseBalance / 100
+		} else {
+			baseRequired = cfg.BaseBalance
+		}


Do we need to validate the balance types across []*BotConfig somehow? What if the user is trading on a couple markets with the same base asset, and they give Percentage values that sum to > 100? What happens if they provide mixed Percentage and Amount balance types for the same asset?

This is being validated in setupBalances.

Ah. I guess you mean because it would trigger an error below at

if baseRequired > baseBalance.balanceAvailable-baseBalance.balanceReserved { ...

I still wonder if it wouldn't be easy to screw up. As an example, say the balance of xyz is 2. We run one bot that reserves 1 xyz, and one bot that reserves 50% of xyz. If the balance of xyz changes to 1.5 for whatever reason, then those allocations cannot both be satisfied.

OK for now, but just wanted to point it out.

Yeah, you'd have to change your settings in that case before the run starts.

buck54321

I still have some questions about how some things are going to work practically, but I'm comfortable with these changes and I want to get this in so we can get #2416 rebased and out of draft.

buck54321 · 2023-08-05T02:14:47Z

client/asset/btc/btc_test.go

+	maxFundingFees := wallet.MaxFundingFees(3, useSplitOptions)
+	expectedFees := feeRateLimit * (inputSize*12 + outputSize*4 + dexbtc.MinimumTxOverhead)


consts instead of magic numbers is better.

const maxSwaps = 3 const numInputs = 12 maxFundingFees := wallet.MaxFundingFees(maxSwaps, useSplitOptions) expectedFees := feeRateLimit * (inputSize*numInputs + outputSize*(maxSwaps+1) + dexbtc.MinimumTxOverhead)

martonp mentioned this pull request May 1, 2023

client: Move market making out of core #2320

Merged

martonp force-pushed the balanceHandling branch from d68165e to d88fd81 Compare May 5, 2023 09:55

martonp force-pushed the balanceHandling branch from d88fd81 to 3e434d2 Compare May 22, 2023 09:47

chappjc reviewed May 22, 2023

View reviewed changes

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

chappjc reviewed May 22, 2023

View reviewed changes

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

martonp force-pushed the balanceHandling branch from 3e434d2 to 10064d6 Compare May 26, 2023 11:02

chappjc reviewed May 31, 2023

View reviewed changes

martonp force-pushed the balanceHandling branch 2 times, most recently from 893ec48 to 2130d83 Compare June 2, 2023 10:00

martonp mentioned this pull request Jun 3, 2023

client/{core, btc}: MultiTrade #2362

Merged

martonp force-pushed the balanceHandling branch from 2130d83 to 80eedea Compare June 27, 2023 08:34

martonp mentioned this pull request Jun 27, 2023

client/mm: Update Basic Market Maker #2416

Merged

JoeGruffins approved these changes Jun 28, 2023

View reviewed changes

martonp force-pushed the balanceHandling branch 2 times, most recently from 87b2e50 to 0eb37ac Compare July 17, 2023 14:43

martonp added 5 commits July 19, 2023 05:56

Chappjc review fixes

5de3cc6

Joe review updates

9684df0

Move balance handling code into MarketMaker

2d4d151

Track pending/locked bot balances

057fc87

This does not change any behavior. It will be used to display on the UI.

martonp force-pushed the balanceHandling branch from 7989be1 to 057fc87 Compare July 19, 2023 09:57

buck54321 requested changes Jul 30, 2023

View reviewed changes

buck54321 approved these changes Aug 5, 2023

View reviewed changes

Buck review fixes

bdd260f

martonp force-pushed the balanceHandling branch from 2e960d3 to bdd260f Compare August 6, 2023 07:29

buck54321 merged commit de675a6 into decred:master Aug 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

martonp commented Apr 30, 2023 •

edited

Loading

martonp commented Jul 17, 2023

buck54321 Jul 22, 2023

martonp Aug 3, 2023

buck54321 Jul 22, 2023

buck54321 Jul 22, 2023

buck54321 Jul 30, 2023

martonp Aug 2, 2023

buck54321 Aug 2, 2023

buck54321 Jul 30, 2023

buck54321 Jul 30, 2023

buck54321 Jul 30, 2023

buck54321 Jul 30, 2023

buck54321 Jul 30, 2023

martonp Aug 2, 2023

buck54321 Aug 2, 2023

martonp Aug 3, 2023

buck54321 left a comment

buck54321 Aug 5, 2023

		dc.assetsMtx.Lock()
		swapAsset := dc.assets[wallets.fromWallet.AssetID]

		maxFundingFees := wallet.MaxFundingFees(3, useSplitOptions)
		expectedFees := feeRateLimit * (inputSize12 + outputSize4 + dexbtc.MinimumTxOverhead)

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

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

Conversation

martonp commented Apr 30, 2023 • edited Loading

martonp commented Jul 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

buck54321 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martonp commented Apr 30, 2023 •

edited

Loading