client/multi: Refactor MM Balance Handling #2641

martonp · 2023-12-26T15:54:15Z

This PR makes market making dependent on the WalletHistorian interface, which will need to be implemented for all wallets. Currently only BTC/clone SPV wallets and ETH/Polygon wallets can be used.

client/{asset,core,rpcserver}: Add WalletTransaction

- This adds a WalletTransaction function to the WalletHistorian interface.
It returns information about a transaction which the wallet made, or one
in which the wallet received funds. The TransactionConfirmations function
is removed, as it will no longer be needed once all wallets support
WalletTransaction.

- A `PartOfActiveBalance` field is added to the `WalletTransaction` type.
This is required for market making to know when it is safe to consider the
funds from an incoming transaction available for use.

- A `ParentAssetLockedAmt` field is added to the `core.Order` type. This is
needed in order to know the amount of token's fee asset is locked for an
order.

client/mm: Refactor bot balance tracking

- This diff updates the balance tracking for market making bots. Previously,
there was a fragile diff based accounting technique. Now, on each bot
action, including trades, withdrawals, and deposits, a pending action is
stored. This pending action is updated until it is complete, at which time
it is removed, and the "base" balance of the bot is updated. The bot
balances are calculated by adding the base balances of the bot to the
effects of all pending actions.

- The `wrappedCore` and `wrappedCEX` types are removed, and their functionalities
are combined into a `unifiedExchangeAdaptor`. This is useful because
pending deposits and pending withdrawals have balance effects on both
CEX and DEX balances. Each `unifiedExchangeAdaptor` listens to a core
notification stream, and the `MarketMaker` type no longer needs to. In
a future refactor, the bots themselves will no longer listen to the
core notification functionality, and all common functionalities can be
moved to the `unifiedExchangeAdaptor`.

- Previously, the fee assets of tokens were not taken into account, but
now they are.

- The Binance library now uses a different endpoint, `/api/v3/balances`,
to get balances. This is supported by the Binance testnet api, so
testbinance is no longer required for balances.

JoeGruffins

Just running the basic bot without arb. Do not need to fix now but see these errors with cancel orders pop up:

2024-01-02 05:17:00.021 [ERR] MM[MarketMaker-127.0.0.1:17273-42-0]: Error canceling order 9d70534d61d934e097e00c37ce8b0959b5b3b2cded5947b6c4bbf049fc549149: order 9d70534d61d934e097e00c37ce8b0959b5b3b2cded5947b6c4bbf049fc549149 - only one cancel order can be submitted per order per epoch. still waiting on cancel order 068a0d0b27a7a8c666f68f7c7da84f54374034fb40fed3b2dda6e1dcdeec284c to match

2024-01-02 05:44:00.036 [ERR] MM[MarketMaker-127.0.0.1:17273-42-0]: Error canceling order 4b9b95ee7d8b02899a220d3a5547be362ab35c6706f7543b319ea3cae4f4ea8f: order 4b9b95ee7d8b02899a220d3a5547be362ab35c6706f7543b319ea3cae4f4ea8f not cancellable in status canceled

client/asset/eth/eth.go

client/asset/interface.go

client/mm/exchange_adaptor.go

client/asset/btc/btc.go

client/asset/interface.go

client/core/wallet.go

client/mm/exchange_adaptor.go

client/mm/exchange_adaptor_test.go

buck54321 · 2024-01-12T12:47:33Z

client/mm/libxc/binance.go

+// setBalances queries binance for the user's balances and stores them in the
+// balances map.
+func (bnc *binance) setBalances(ctx context.Context) error {
+	var resp struct {
+		Balances []struct {
+			Asset  string  `json:"asset"`
+			Free   float64 `json:"free,string"`
+			Locked float64 `json:"locked,string"`
+		} `json:"balances"`
+	}
+	err := bnc.getAPI(ctx, "/api/v3/account", nil, true, true, &resp)


Hmm. I made some changes in #2615 that might be relevant to these changes too. I think we should just expect to be connected to configured cexes, or connect on-the-fly when we need info.

This change just updates the API used to get the balances. I think your changes over there are still good.

I think we should just expect to be connected to configured cexes, or connect on-the-fly when we need info.

I think the use should make some action before connecting the the CEX. It shouldn't automatically connect at start-up even if the user doesn't intend to use market making at all. They may want to set up some configurations before connecting.

buck54321 · 2024-01-30T10:48:13Z

client/asset/eth/eth.go

+		return nil, err
+	}
+	if *tx.To() != w.netToken.Address {
+		w.log.Infof("from token address")


This doesn't seem very useful.

buck54321 · 2024-01-30T10:51:38Z

client/asset/eth/fundingcoin.go

@@ -110,6 +113,10 @@ func (c *tokenFundingCoin) Value() uint64 {
 	return c.amt
 }

+func (c *tokenFundingCoin) ParentValue() uint64 {


How about just Fees?

buck54321 · 2024-01-30T11:03:39Z

client/cmd/testbinance/main.go

+	// Returning configs for eth, btc, ltc, bch, zec, usdc, matic.
+	// Balances do not use a sapi endpoint, so they do not need to be handled
+	// here.
+	resp := `[


resp := []byte(`[

buck54321 · 2024-01-30T11:06:28Z

client/core/trade.go

+// NOTE: This amount only applies to the wallet from which swaps are sent. This
+// is the BASE asset wallet for a SELL order and the QUOTE asset wallet for a
+// BUY order.
+// lockedAmount should be called with the mtx >= RLocked.


lockedAmount -> parentLockedAmt

buck54321 · 2024-01-30T11:37:00Z

client/mm/exchange_adaptor.go

+		pendingDeposit.mtx.RLock()
+		if pendingDeposit.assetID == assetID {
+			totalAvailableDiff -= int64(pendingDeposit.amtSent)
+		}
+
+		token := asset.TokenInfo(pendingDeposit.assetID)
+		if token != nil && token.ParentID == assetID {
+			totalAvailableDiff -= int64(pendingDeposit.fee)
+		} else if token == nil && pendingDeposit.assetID == assetID {
+			totalAvailableDiff -= int64(pendingDeposit.fee)
+		}
+		pendingDeposit.mtx.RUnlock()


fee is the only protected field, so you might as well

pendingDeposit.mtx.RLock() fee := int64(pendingDeposit.fee) pendingDeposit.mtx.RUnlock()

buck54321 · 2024-01-30T12:00:33Z

client/mm/exchange_adaptor.go

+	u.balancesMtx.Lock()
+	defer u.balancesMtx.Unlock()
+
+	if trade.Complete {


Fine, but the Trade returned from (CEX).Trade is never Complete.

buck54321 · 2024-01-30T12:03:02Z

client/mm/exchange_adaptor.go

+	u.balancesMtx.RLock()
+	pendingOrder, found := u.pendingDEXOrders[orderID]
+	if !found {
+		u.balancesMtx.RUnlock()
+		return
+	}
+	u.balancesMtx.RUnlock()


u.balancesMtx.RLock() pendingOrder, found := u.pendingDEXOrders[orderID] u.balancesMtx.RUnlock() if !found { return }

buck54321 · 2024-01-30T12:16:18Z

client/mm/exchange_adaptor.go

+	for txID := range swaps {
+		if _, found := pendingOrder.swaps[txID]; !found {
+			pendingOrder.swaps[txID] = &asset.WalletTransaction{}
+		}
+	}
+	for txID := range redeems {
+		if _, found := pendingOrder.redeems[txID]; !found {
+			pendingOrder.redeems[txID] = &asset.WalletTransaction{}
+		}
+	}
+	for txID := range refunds {
+		if _, found := pendingOrder.refunds[txID]; !found {
+			pendingOrder.refunds[txID] = &asset.WalletTransaction{}
+		}
+	}
+
+	// Query the wallet regarding all unconfirmed transactions
+	for txID, oldTx := range pendingOrder.swaps {
+		if oldTx.Confirmed {
+			continue
+		}
+		tx, err := u.clientCore.WalletTransaction(fromAsset, txID)
+		if err != nil {
+			u.log.Errorf("Error getting swap tx %s: %v", txID, err)
+			continue
+		}
+		pendingOrder.swaps[txID] = tx
+	}
+	for txID, oldTx := range pendingOrder.redeems {
+		if oldTx.Confirmed {
+			continue
+		}
+		tx, err := u.clientCore.WalletTransaction(toAsset, txID)
+		if err != nil {
+			u.log.Errorf("Error getting redeem tx %s: %v", txID, err)
+			continue
+		}
+		pendingOrder.redeems[txID] = tx
+	}
+	for txID, oldTx := range pendingOrder.refunds {
+		if oldTx.Confirmed {
+			continue
+		}
+		tx, err := u.clientCore.WalletTransaction(fromAsset, txID)
+		if err != nil {
+			u.log.Errorf("Error getting refund tx %s: %v", txID, err)
+			continue
+		}
+		pendingOrder.refunds[txID] = tx
+	}


processTxs := func(assetID uint32, m map[string]*asset.WalletTransaction, txs map[string]bool) { // Add new txs to tx cache for txID := range txs { if _, found := m[txID]; !found { m[txID] = &asset.WalletTransaction{} } } // Query the wallet regarding all unconfirmed transactions for txID, oldTx := range m { if oldTx.Confirmed { continue } tx, err := u.clientCore.WalletTransaction(assetID, txID) if err != nil { u.log.Errorf("Error getting tx %s: %v", txID, err) continue } m[txID] = tx } } processTxs(fromAsset, pendingOrder.swaps, swaps) processTxs(toAsset, pendingOrder.redeems, redeems) processTxs(fromAsset, pendingOrder.refunds, refunds)

buck54321 · 2024-01-30T13:28:39Z

client/mm/exchange_adaptor.go

+			dex.BipIDSymbol(feeAsset), u.baseDexBalances[feeAsset], deposit.fee)
+		u.baseDexBalances[feeAsset] = 0
+	} else {
+		u.baseDexBalances[feeAsset] -= deposit.fee


Unlike other balances, we only subtract from the fee balance. This means a bot will run out of allocated fee funding at some point, even if they're have profits to compensate.

If the asset is not a token, then this is not a problem, they are not tracked separately. If it is a token, there's nothing we can do other than buying the fee asset on another market. I think for now it'll just require a manual intervention every once in a while.

The operator's choice with this scheme is a lose-lose. They can either 1) Allocate a lot of funding for fees, possibly preventing them from making other markets, or they can 2) allocate a smaller amount, and risk their bot going down when things get hot, missing out on profit.

The bot should just ensure some minimum level of token fee funding during startup and periodically check the status of fee funding balances so that it can issue a notification is funding is low.

Let's just roll with this for now. Will revisit later.

The bot should just ensure some minimum level of token fee funding during startup and periodically check the status of fee funding balances so that it can issue a notification is funding is low.

Yeah it can definitely do a notification. Other than that it can do some sort of reallocations between bots.

buck54321 · 2024-01-30T13:58:43Z

client/cmd/testbinance/main.go

+		{
+			"coin": "MATIC",
+			"depositAllEnable": true,
+			"free": "0",


Not setting a balance?

No, we're not using this request to get the balances anymore. Using /api/v3/account which is supported by the Binance testnet.

buck54321 · 2024-02-02T15:24:33Z

Rebase please.

- This adds a WalletTransaction function to the WalletHistorian interface. It returns information about a transaction which the wallet made, or one in which the wallet received funds. The TransactionConfirmations function is removed, as it will no longer be needed once all wallets support WalletTransaction. - A `PartOfActiveBalance` field is added to the `WalletTransaction` type. This is required for market making to know when it is safe to consider the funds from an incoming transaction available for use. - A `ParentAssetLockedAmt` field is added to the `core.Order` type. This is needed in order to know the amount of token's fee asset is locked for an order.

- This diff updates the balance tracking for market making bots. Previously, there was a fragile diff based accounting technique. Now, on each bot action, including trades, withdrawals, and deposits, a pending action is stored. This pending action is updated until it is complete, at which time it is removed, and the "base" balance of the bot is updated. The bot balances are calculated by adding the base balances of the bot to the effects of all pending actions. - The `wrappedCore` and `wrappedCEX` types are removed, and their functionalities are combined into a `unifiedExchangeAdaptor`. This is useful because pending deposits and pending withdrawals have balance effects on both CEX and DEX balances. Each `unifiedExchangeAdaptor` listens to a core notification stream, and the `MarketMaker` type no longer needs to. In a future refactor, the bots themselves will no longer listen to the core notification functionality, and all common functionalities can be moved to the `unifiedExchangeAdaptor`. - Previously, the fee assets of tokens were not taken into account, but now they are. - The Binance library now uses a different endpoint, `/api/v3/balances`, to get balances. This is supported by the Binance testnet api, so testbinance is no longer required for balances.

martonp force-pushed the refactorMMBalanceHandling branch 2 times, most recently from c503c16 to 2410dbe Compare December 28, 2023 18:22

JoeGruffins approved these changes Jan 2, 2024

View reviewed changes

buck54321 requested changes Jan 12, 2024

View reviewed changes

martonp force-pushed the refactorMMBalanceHandling branch 2 times, most recently from 3dc39bc to 656f88a Compare January 16, 2024 16:37

martonp mentioned this pull request Jan 20, 2024

client/mm: Move shared logic to unifiedExchangeAdaptor #2678

Merged

buck54321 requested changes Jan 30, 2024

View reviewed changes

martonp force-pushed the refactorMMBalanceHandling branch from 656f88a to 72a4835 Compare January 30, 2024 16:44

buck54321 approved these changes Feb 2, 2024

View reviewed changes

martonp force-pushed the refactorMMBalanceHandling branch from 72a4835 to 55e2dd9 Compare February 2, 2024 18:01

martonp added 7 commits February 16, 2024 11:48

Buck/Joe reivew fixes

8cd6300

Make WalletTransaction take a string txID

3a0233f

Buck review fixes

91f0110

Update DCR WalletTransaction signature

78005ff

Fix variable names

2b1d2b1

martonp force-pushed the refactorMMBalanceHandling branch from 55e2dd9 to 2b1d2b1 Compare February 16, 2024 19:24

buck54321 merged commit 72a51e6 into decred:master Feb 19, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client/multi: Refactor MM Balance Handling #2641

client/multi: Refactor MM Balance Handling #2641

martonp commented Dec 26, 2023 •

edited

Loading

JoeGruffins left a comment

buck54321 Jan 12, 2024

martonp Jan 12, 2024

buck54321 Jan 30, 2024

buck54321 Jan 30, 2024

buck54321 Jan 30, 2024

buck54321 Jan 30, 2024

buck54321 Jan 30, 2024

buck54321 Jan 30, 2024

buck54321 Jan 30, 2024

buck54321 Jan 30, 2024

buck54321 Jan 30, 2024

martonp Jan 30, 2024

buck54321 Jan 30, 2024

buck54321 Feb 2, 2024

martonp Feb 2, 2024

buck54321 Jan 30, 2024

martonp Jan 30, 2024

buck54321 commented Feb 2, 2024

client/multi: Refactor MM Balance Handling #2641

client/multi: Refactor MM Balance Handling #2641

Conversation

martonp commented Dec 26, 2023 • edited Loading

JoeGruffins left a 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

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 commented Feb 2, 2024

martonp commented Dec 26, 2023 •

edited

Loading