mm: Individually Start Bots / Live Updates #2738
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
buck54321
requested changes
Apr 30, 2024
| t.Helper() | ||
| var err error | ||
| for i := 0; i < 20; i++ { | ||
| time.Sleep(100 * time.Millisecond) |
There was a problem hiding this comment.
I know it's not new, but what are we synchronizing here?
There was a problem hiding this comment.
storeEvent is async.
client/mm/exchange_adaptor.go
Outdated
| // placementIndex/counterTradeRate are used by MultiTrade to know | ||
| // which orders to place/cancel. | ||
| placementIndex uint64 | ||
| counterTradeRate uint64 | ||
| } | ||
|
|
||
| // updateState should only be called with txMtx write locked. |
There was a problem hiding this comment.
Is your mutex comment outdated?
client/mm/exchange_adaptor.go
Outdated
| } | ||
|
|
||
| func (u *unifiedExchangeAdaptor) updateConfig(cfg *BotConfig, balanceDiffs *BotBalanceDiffs) { | ||
| cfgUpdated := !reflect.DeepEqual(cfg, u.botCfg.Load().(*BotConfig)) |
There was a problem hiding this comment.
No DeepEqual in production please. Why are we grouping config and inventory updates. They seem like separate events.
There was a problem hiding this comment.
What are you thinking for the UI? I was thinking that when you update the config, you would also have the option to update balance in the same operation. Then there would also be a separate screen for just updating balances. I could add a separate function only for balance updates.
client/mm/mm.go
Outdated
Comment on lines
674
to
840
| ctx, die := context.WithCancel(m.ctx) | ||
| adaptorUpdateManager, botUpdateManager := cfgUpdateManagerPair() | ||
| mktID := dexMarketID(botCfg.Host, botCfg.BaseID, botCfg.QuoteID) | ||
| logger := m.botSubLogger(botCfg) | ||
| exchangeAdaptor := unifiedExchangeAdaptorForBot(&exchangeAdaptorCfg{ | ||
| botID: mktID, | ||
| market: mkt, | ||
| baseDexBalances: allocation.DEX, | ||
| baseCexBalances: allocation.CEX, | ||
| core: m.core, | ||
| cex: cex, | ||
| log: logger, | ||
| botCfg: botCfg, | ||
| eventLogDB: m.eventLogDB, | ||
| cfgUpdateManager: adaptorUpdateManager, | ||
| }) | ||
| if err := exchangeAdaptor.run(ctx); err != nil { | ||
| die() | ||
| return err | ||
| } |
There was a problem hiding this comment.
This context pattern is what we have dex.ConnectionMaster for.
client/mm/mm_arb_market_maker.go
Outdated
Comment on lines
500
to
503
| pauseEpochs <- struct{}{} | ||
| pauseCEXUpdates <- struct{}{} | ||
| pauseDEXUpdates <- struct{}{} | ||
| updateManager.botPaused <- struct{}{} |
There was a problem hiding this comment.
This seems racy. These are zero-capacity channels. If the context is canceled after the updateBot message but before one of these structs{}{} can be received, e.g. while processDEXOrderUpdate is running, this will deadlock.
There was a problem hiding this comment.
It's purposely zero-capacity because I wanted it to block, but I see your point about the context getting cancelled.
client/mm/price_oracle.go
Outdated
Comment on lines
288
to
293
| func coinpapSlug(symbol, name string) string { | ||
| slug := fmt.Sprintf("%s-%s", symbol, name) | ||
| // Special handling for asset names with multiple space, e.g Bitcoin Cash. | ||
| return strings.ToLower(strings.ReplaceAll(slug, " ", "-")) | ||
| } | ||
|
|
This diff adds the ability to start/stop bots individually and also to perform live updates to bot settings and the amount of funds controlled by bots. All balance fields are removed from the BotConfig. These are now specified when a bot is started. To perform updates to a running bot, an “update manager” consisting of three channels is passed to both the `unifiedExchangeAdaptor` and the bot First, the adaptor signals that it wants do an update. The bot pauses all operations, and signals to the adaptor that it is paused. Then, the bot and the adaptor both update their configurations. When the adaptor is finished, it signals to the bot that it can proceed. To perform a balance update, we need to know the exact amount that is available in the wallets and on the CEX that is not currently reserved by a running bot. To do this, we first check the available amounts according to the wallet/cex, then we sync the state of all pending trades, deposits, and withdrawals, and then we recheck the available amounts. If the first check is the same as the last, we know nothing has changed and we have the correct amounts, so we can proceed. In order for this to work properly, the `WalletTransaction` function of wallets must return `Confirmed == true` if and only if the any incoming funds from that transaction are part of the available balance. The priceOracle is also refactored. Instead of having a “synced” priceOracle used for markets on which a bot is running, and an “unsynced” one for any other markets, there is now only one priceOracle. `startAutoSyncingMarket` and `stopAutoSyncingMarket` are called whenever bots are started / stopped. A testing program is added that tracks the available balances in wallets/cexes that are unused by any bots. This amount should not change unless the user start, stops, or updates a bot. If there are any unexpected changes, this means there is a bug in the balance tracking.
If a bond creation transaction makes the bots have more balance than is available in the wallet, decrease the bot balances.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This diff adds the ability to start/stop bots individually and also to perform live updates to bot settings and the amount of funds controlled by bots.
All balance fields are removed from the BotConfig. These are now specified when a bot is started.
To perform updates to a running bot, an “update manager” consisting of three channels is passed to both the
unifiedExchangeAdaptorand the bot First, the adaptor signals that it wants do an update. The bot pauses all operations, and signals to the adaptor that it is paused. Then, the bot and the adaptor both update their configurations. When the adaptor is finished, it signals to the bot that it can proceed.To perform a balance update, we need to know the exact amount that is available in the wallets and on the CEX that is not currently reserved by a running bot. To do this, we first check the available amounts according to the wallet/cex, then we sync the state of all pending trades, deposits, and withdrawals, and then we recheck the available amounts. If the first check is the same as the last, we know nothing has changed and we have the correct amounts, so we can proceed. In order for this to work properly, the
WalletTransactionfunction of wallets must returnConfirmed == trueif and only if the any incoming funds from that transaction are part of the available balance.The priceOracle is also refactored. Instead of having a “synced” priceOracle used for markets on which a bot is running, and an “unsynced” one for any other markets, there is now only one priceOracle.
startAutoSyncingMarketandstopAutoSyncingMarketare called whenever bots are started / stopped.A testing program is added that tracks the available balances in wallets/cexes that are unused by any bots. This amount should not change unless the user start, stops, or updates a bot. If there are any unexpected changes, this means there is a bug in the balance tracking.
*This is still in draft as the UI and profit calculations need to be updated based on this work. The new RPC functions can
be used to test.