client/mm: CEX balance tracking and auto-rebalancing #2568

martonp · 2023-10-13T07:16:33Z

    multi: Return TxID from Send and add TransactionConfirmations
    
    Send and Withdraw are updated to return a transaction ID. Also,
    a TransactionConfirmations function is added to the wallet interface
    which allows callers to determine the amount of confirmations a wallet
    transaction has.

    client/mm/libxc: Add deposit/withdraw functionality to CEX interface
    
    - Deposit and Withdraw functionality is added to the CEX interface
    and is implemented in Binance.
    - The CEX interface is updated to take uint32 assetIDs instead of strings
    to represent assets.
    - The "weth.polygon" and "wbtc.polygon" assets are renamed to be
    "eth.polygon" and "btc.polygon". This makes it clear that "btc" and
    "btc.polygon" are the same asset, just on different networks.

    client/mm: Tracking of CEX balances
    
    Tracking of CEX balances for each bot is implemented in a similar fashion
    to tracking bot balances on the DEX. A `wrappedCEX` is created for each
    bot which behaves as if the entire balance of the CEX is the amount that
    is allocated for each bot.

    client/mm: Simple arb bot auto-rebalancing
    
    This implements auto-rebalancing in the simple arbitrage strategy. This is
    an optional functionality that allows the user to specify a minimum
    balance of each asset that should be on both the DEX and the CEX. If the
    balance dips below this amount, then either a deposit or withdrawal will
    be done to make the balances equal.

ukane-philemon · 2023-10-13T07:37:34Z

client/mm/wrapped_cex.go

+			ticker := time.NewTicker(time.Second * 20)
+			giveUp := time.NewTimer(time.Minute * 10)


Missed deferring Stop() for both tickers.

ukane-philemon · 2023-10-13T07:39:32Z

client/mm/wrapped_cex.go

+
+	w.tradesMtx.Lock()
+	defer w.tradesMtx.Unlock()
+
+	w.subscriptionIDMtx.RLock()
+	defer w.subscriptionIDMtx.RUnlock()


I think we'd want to avoid nested locks.

subscriptionID could be atomic actually.. but what's wrong with locking two things at once?

Nothing really wrong since they are protecting different data. But might lead to unexpected issues, I think.

Yeah it could cause a deadlock if in another location the locks are acquired in the reverse order.

client/mm/libxc/binance.go

client/asset/btc/btc.go

client/mm/mm_simple_arb.go

client/asset/interface.go

buck54321 · 2023-10-20T21:20:42Z

client/mm/config.go

@@ -63,6 +74,10 @@ func (c *BotConfig) requiresPriceOracle() bool {
 	return false
 }

+func (c *BotConfig) requiresCEX() bool {
+	return c.SimpleArbConfig != nil || c.MMWithCEXConfig != nil


Why not just make BotCEXCfg a field of, or embedded in, SimpleArbConfig and MMWithCEXConfig?

I did it like this because the strategy doesn't need to know anything about the CEX it's trading on. Creating the CEXes and allocating the balances is the purview of the level above the strategies. I was kind of split on this, thought it could go either way.

client/mm/libxc/binance.go

buck54321 · 2023-10-20T21:44:15Z

client/mm/mm.go

@@ -550,6 +633,56 @@ func (m *MarketMaker) botBalance(botID string, assetID uint32) uint64 {
 	return 0
 }

+func (m *MarketMaker) modifyBotCEXBalance(botID string, assetID uint32, amount uint64, increase bool) {


Delta-based value tracking tends to be fragile. Much better to find an idempotent pattern, such that we track the server's reported balance and pending deposits/withdraws to calculate balance.

The server's balance doesn't really tell us much because the balance of the CEX will be split between multiple bots. Also, not just deposits/withdrawals change the balance, but also trades. Maybe a better pattern would be to keep track of each of the actions which changed the balance and how much it changed it by. Then we could definitely avoid any double counting. Is this kind of what you're suggesting?

I'll leave it up to you. Just wanted to point out the scary pattern.

You're probably right, I'll think of something. The balance management for the DEX should be updated then too.

dex/bip-id.go

client/core/core.go

client/mm/config.go

client/asset/btc/btc.go

client/asset/interface.go

client/core/core.go

client/mm/libxc/binance.go

client/mm/wrapped_cex.go

JoeGruffins · 2023-10-25T08:31:04Z

client/mm/wrapped_cex.go

+					// Assign to balance to the bot as long as the wallet
+					// knows about the transaction.
+					w.mm.modifyBotBalance(w.botID, []*balanceMod{{balanceModIncrease, assetID, balTypeAvailable, withdrawnAmt}})
+					onConfirm()


Confirmed to exist from the point of view of our node but not confirmed really if 0 confs. Should this wait for some amount of confs?

JoeGruffins · 2023-10-25T11:49:56Z

client/mm/wrapped_cex.go

+	}
+
+	if !trade.sell && calc.BaseToQuote(trade.rate, trade.qty) > trade.quoteFilled {
+		unfilledQty := calc.BaseToQuote(trade.rate, trade.qty) - trade.quoteFilled


Could this trade have been completed at different rates? If taker maybe some quote was taken at a better rate?

The original reserved amount was calc.BaseToQuote(trade.rate, trade.qty). The amount of quote that was used is trade.quoteFilled. quoteFilled is the amount of quote that was used to buy the base in this trade, regardless of what the rate was.

Send and Withdraw are updated to return a transaction ID. Also, a TransactionConfirmations function is added to the wallet interface which allows callers to determine the amount of confirmations a wallet transaction has.

- Deposit and Withdraw functionality is added to the CEX interface and is implemented in Binance. - The CEX interface is updated to take uint32 assetIDs instead of strings to represent assets. - The "weth.polygon" and "wbtc.polygon" assets are renamed to be "eth.polygon" and "btc.polygon". This makes it clear that "btc" and "btc.polygon" are the same asset, just on different networks.

Tracking of CEX balances for each bot is implemented in a similar fashion to tracking bot balances on the DEX. A `wrappedCEX` is created for each bot which behaves as if the entire balance of the CEX is the amount that is allocated for each bot.

This implements auto-rebalancing in the simple arbitrage strategy. This is an optional functionality that allows the user to specify a minimum balance of each asset that should be on both the DEX and the CEX. If the balance dips below this amount, then either a deposit or withdrawal will be done to have an equal amount of the asset on both exchanges.

buck54321

Partial review. I'll get back into this tomorrow.

I'll just drop this diff here. buck54321/dcrdex@996b2a6...buck54321:dcrdex:cexbalances. Do with it what you will.

client/mm/libxc/binance.go

client/mm/libxc/interface.go

buck54321 · 2023-11-27T22:21:22Z

client/mm/mm_simple_arb.go

+	MinBaseAmt       uint64 `json:"minBaseAmt"`
+	MinBaseTransfer  uint64 `json:"minBaseTransfer"`
+	MinQuoteAmt      uint64 `json:"minQuoteAmt"`
+	MinQuoteTransfer uint64 `json:"minQuoteTransfer"`


I feel like none of these should need to be manually set. We should know when to trigger an auto-rebalance based on lot size and bot setttings. Min transfers too.

I think some people would want to be more aggressive with rebalancing and some people less. If someone allocates 4 lots on both sides of a market for simple arb, when should this be rebalanced? For Arb+mm, if there are a few matches in one direction, and there's not enough balance for all the placements on one side of the market, maybe someone would want to rebalance ASAP, while others would want to give it some time because the market may move in the other direction. I don't think there's a one size fits all.

client/mm/wrapped_cex.go

client/asset/interface.go

buck54321 · 2023-11-27T22:33:04Z

client/mm/mm_arb_market_maker.go

Arb+mm doesn't get auto-rebalance?

buck54321

I'm not going to add anything to this now. I know you were looking at some refactoring of the balance tracking anyway. If you can just address outstanding comments, we'll get this in.

martonp · 2023-12-01T11:50:46Z

@buck54321 I've made the changes. Adding rebalancing to mm+arb required some new logic, so that could still use a review.

ukane-philemon suggested changes Oct 13, 2023

View reviewed changes

buck54321 requested changes Oct 20, 2023

View reviewed changes

JoeGruffins approved these changes Oct 25, 2023

View reviewed changes

martonp and others added 7 commits November 26, 2023 19:35

multi: Return TxID from Send and add TransactionConfirmations

2d2ec5e

Send and Withdraw are updated to return a transaction ID. Also, a TransactionConfirmations function is added to the wallet interface which allows callers to determine the amount of confirmations a wallet transaction has.

client/mm: Tracking of CEX balances

e74ff54

Tracking of CEX balances for each bot is implemented in a similar fashion to tracking bot balances on the DEX. A `wrappedCEX` is created for each bot which behaves as if the entire balance of the CEX is the amount that is allocated for each bot.

Review updates

75cdf16

without new return args

2b1feea

Update wbtc and weth handling

92301e7

martonp force-pushed the cexRebalance branch 2 times, most recently from b96b239 to 6d22e0c Compare November 27, 2023 03:34

Joe review updates

996b2a6

martonp force-pushed the cexRebalance branch from 6d22e0c to 996b2a6 Compare November 27, 2023 04:22

buck54321 requested changes Nov 27, 2023

View reviewed changes

buck54321 reviewed Nov 29, 2023

View reviewed changes

martonp force-pushed the cexRebalance branch from 74b869f to 5316458 Compare December 1, 2023 12:04

Buck review updates

c3cb583

martonp force-pushed the cexRebalance branch from 5316458 to c3cb583 Compare December 1, 2023 12:18

buck54321 approved these changes Dec 2, 2023

View reviewed changes

buck54321 merged commit f9289bd into decred:master Dec 2, 2023
5 checks passed

buck54321 mentioned this pull request Dec 5, 2023

ui: arb + market maker ui #2615

Merged

martonp deleted the cexRebalance branch January 20, 2024 13:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client/mm: CEX balance tracking and auto-rebalancing #2568

client/mm: CEX balance tracking and auto-rebalancing #2568

martonp commented Oct 13, 2023

ukane-philemon Oct 13, 2023

ukane-philemon Oct 13, 2023

martonp Oct 13, 2023

ukane-philemon Oct 13, 2023

martonp Oct 13, 2023

buck54321 Oct 20, 2023 •

edited

Loading

martonp Oct 23, 2023

buck54321 Oct 20, 2023

martonp Oct 23, 2023

buck54321 Oct 24, 2023

martonp Oct 25, 2023

JoeGruffins Oct 25, 2023

JoeGruffins Oct 25, 2023

martonp Nov 27, 2023

buck54321 left a comment

buck54321 Nov 27, 2023

martonp Nov 28, 2023

buck54321 Nov 27, 2023

buck54321 left a comment

martonp commented Dec 1, 2023

		ticker := time.NewTicker(time.Second * 20)
		giveUp := time.NewTimer(time.Minute * 10)

client/mm: CEX balance tracking and auto-rebalancing #2568

client/mm: CEX balance tracking and auto-rebalancing #2568

Conversation

martonp commented Oct 13, 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

buck54321 Oct 20, 2023 • edited Loading

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

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

martonp commented Dec 1, 2023

buck54321 Oct 20, 2023 •

edited

Loading