mm: bot panics (trying to placeMultiTrade)

[norwnd]

MM bot panic (note, I'm running my own fork at https://github.com/norwnd/dcrdex/commits/master - but this is probably relevant for thus upstream repo too):

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x8 pc=0x102a4c268]

goroutine 4980 [running]:
decred.org/dcrdex/client/mm.(*unifiedExchangeAdaptor).placeMultiTrade(0x14006ba6600, {0x1400b60a0a0, 0x1, 0x10383541c?}, 0x1)
	/Users/norwnd/dcrdex/client/mm/exchange_adaptor.go:970 +0x438
decred.org/dcrdex/client/mm.(*unifiedExchangeAdaptor).multiTrade(0x14006ba6600, {0x1400b60a088, 0x1, 0x1400b608150?}, 0x1, 0x0, 0x6e4f8d3)
	/Users/norwnd/dcrdex/client/mm/exchange_adaptor.go:1343 +0xc60
decred.org/dcrdex/client/mm.(*basicMarketMaker).rebalance(0x140087e20c0, 0x6e4f8d3)
	/Users/norwnd/dcrdex/client/mm/mm_basic.go:590 +0x274
decred.org/dcrdex/client/mm.(*basicMarketMaker).botLoop.func1()
	/Users/norwnd/dcrdex/client/mm/mm_basic.go:634 +0x294
created by decred.org/dcrdex/client/mm.(*basicMarketMaker).botLoop in goroutine 3828
	/Users/norwnd/dcrdex/client/mm/mm_basic.go:619 +0x26c
make: *** [a] Error 2

I think MultiTrade func needs to be adjusted like this:

// MultiTrade is used to place multiple standing limit orders on the same
// side of the same market simultaneously.
func (c *Core) MultiTrade(pw []byte, form *MultiTradeForm) []*MultiTradeResult {
	results := make([]*MultiTradeResult, 0, len(form.Placements))
	reqs, err := c.prepareMultiTradeRequests(pw, form)
	if err != nil {
		for range form.Placements {
			results = append(results, &MultiTradeResult{Error: err})
		}
		return results
	}

	for _, req := range reqs {
		var corder *Order
		corder, err = c.sendTradeRequest(req)
		if err != nil {
			results = append(results, &MultiTradeResult{Error: err})
			continue
		}
		results = append(results, &MultiTradeResult{Order: corder})
	}

	return results
}

there is a related "sanity-check" in MM code after MultiTrade is called (but it's always gonna pass with master code version):

	if len(placements) != len(results) {
		u.log.Errorf("unexpected number of results. expected %d, got %d", len(placements), len(results))
		return results
	}

not sure if all that ^ means there is a deeper underlying issue, but panic is certainly not a desired outcome.

[ancow]

I'm getting what seems like the same error on a vanilla decred v2.0.5 install:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0xaae6ed]

goroutine 11569 [running]:
decred.org/dcrdex/client/mm.(*unifiedExchangeAdaptor).placeMultiTrade(0xc00270b000, {0xc01f18aae0, 0x3, 0xc000dd9860?}, 0x1)
        decred.org/dcrdex@v1.0.2/client/mm/exchange_adaptor.go:970 +0x52d
decred.org/dcrdex/client/mm.(*unifiedExchangeAdaptor).multiTrade(0xc00270b000, {0xc00ed492d8, 0x3, 0xc001012bc0?}, 0x1, 0x3f60624dd2f1a9fc, 0x6e59a8d)
        decred.org/dcrdex@v1.0.2/client/mm/exchange_adaptor.go:1342 +0x104c
decred.org/dcrdex/client/mm.(*basicMarketMaker).rebalance(0xc000c1df88?, 0x6e59a8d)
        decred.org/dcrdex@v1.0.2/client/mm/mm_basic.go:392 +0x26d
decred.org/dcrdex/client/mm.(*basicMarketMaker).botLoop.func1()
        decred.org/dcrdex@v1.0.2/client/mm/mm_basic.go:429 +0x185
created by decred.org/dcrdex/client/mm.(*basicMarketMaker).botLoop in goroutine 11466
        decred.org/dcrdex@v1.0.2/client/mm/mm_basic.go:421 +0x275

My DCRDEX score keeps suffering because of this...

[dev-warrior777]

I'm getting what seems like the same error on a vanilla decred v2.0.5 install:

Thanks .. may I know what coins you were trading when you hit the panic? .. I would like to reproduce on master first.

[norwnd]

Thanks .. may I know what coins you were trading when you hit the panic? .. I would like to reproduce on master first.

I'm hitting this with DCR-BTC MM bot at the moment, and I also noticed a related log line:

FundMultiOrder only funded 0 orders out of 1 (options = map[multisplit:false multisplitbuffer:5])

followed by log line (since I added some code to bypass that panic as I mentioned above):

unexpected number of results. expected 1, got 0

So fromWallet.FundMultiOrder returns empty allCoins and no error at the same time (I think in my case fromWallet is BTC and could be it tried to split UTXO but I configured MM bot to forbid it to save on fees; and I definitely have enough balance to fund that MM order since I've configured bot to use only fraction of my total BTC balance)

[dev-warrior777]

Yes I saw your feerate changes in bison-lean repo. The insights are valuable.

• I have several diffs now
  • bison-lean against master .. major differences feeding into the problem area
  • v1.0.2 against master .. which we know panics from @ancow
  • bison-lean against v1.0.2 .. maybe can be ignored except for useful insights
• And we have
  • git showing unknownErr changes 2 months ago - Not in v1.0.2
  • git showing epochReporting 3 months ago - already in v1.0.2

I have not managed to panic yet on master -- @martonp any ideas on this one?

[martonp]

This should be fixed by this PR: #3099. It's not part of the releases yet though.

[norwnd]

This should be fixed by this PR: #3099. It's not part of the releases yet though.

I've just rebased onto latest master (of https://github.com/decred/dcrdex repo), I can confirm that it does resolve panic but it doesn't really solve my problem of Bot not being able to place DCR-BTC trade,

which I think is due to:

So fromWallet.FundMultiOrder returns empty allCoins and no error at the same time (I think in my case fromWallet is BTC and could be it tried to split UTXO but I configured MM bot to forbid it to save on fees; and I definitely have enough balance to fund that MM order since I've configured bot to use only fraction of my total BTC balance)

and the only way I can even see that "error" is due to me adding the following logging - https://github.com/norwnd/bison-lean/blob/faf1d782ce1e6aa1d8593dfb9e26b2e492c7df9a/client/mm/exchange_adaptor.go#L970-L973

2025-01-18 14:09:30.243 [TRC] CORE[btc][SPV]: Bals: spendable = 0.1******* BTC (0.1******* BTC trusted, 0 BTC untrusted, 0 BTC assumed locked), immature = 0 BTC
2025-01-18 14:09:30.248 [ERR] MM[MM-dex.decred.org:7232-42-0]: incomplete multi-trade, couldn't make a placement: wallet unable to fund order

which means:

• you probably want to log this error in a similar way to how I did it
• there probably needs to be a clear error for BTC wallet to return (instead of just returning nothing)
• and/or if it is indeed due to "the need to split UTXO" I guess it should be handled automatically without user even knowing about it (meaning, if MM bot can't really work without Allow multi split setting enabled in some cases - perhaps it shouldn't be a setting in the first place, and just be mandatory thing instead)

update 1: when I enable Allow multi split (with Multi split buffer of 10%) I now get the following error (not empty error) from BTC wallet:

2025-01-18 14:40:30.016 [ERR] MM[MM-dex.decred.org:7232-42-0]: incomplete multi-trade, couldn't make a placement: FundMultiOrder error for btc: cannot fund all with split

update 2: and when I've changed Multi split buffer to be 1% it went ahead and issued a BTC split-transaction - I haven't looked at whether or not MM bot immediately placed a trade for me after that, but when I've restarted Bison wallet (disabling Allow multi split before I started MM bot for DCR-BTC market) now bot is placing my trade(s) just fine (btw even though split-transaction is still pending in mempool),

so, it seems there was some sort of parameters mismatch passed into btc.fundMultiSplitTx

But either way, even though I haven't thought it all the way through, issuing split-transactions just to be able to place trade(s) seems like a huge waste and for some reason it's MM bot-specific requirement since I was able to place DCR-BTC trades manually just fine even before that split-transactions was made.

update 3: encountering same "wallet unable to fund order" issue again with MM bot (but can place trades of arbitrary size manually just fine), I guess prices changed since yesterday and outputs of that split-transaction I mentioned above no longer fit some MM bot requirements ? Which means not only split-transactions are wasteful but they don't even work in all cases.

[norwnd]

update 4:

so, it seems there was some sort of parameters mismatch passed into btc.fundMultiSplitTx

I guess my hunch was correct on this ^ since this work-around seemingly lets MM bot to keep placing DCR-BTC orders (with disabled Allow multi split),

and I still think MM bot should function to the best of its abilities without Allow multi split and I would recommend to disable multi-split flag by default (currently it's enabled) since it's pretty costly for BTC and similar assets.

[martonp]

and I still think MM bot should function to the best of its abilities without Allow multi split and I would recommend to disable multi-split flag by default (currently it's enabled) since it's pretty costly for BTC and similar assets.

The dex protocol requires that each trade has proof of funds, and UTXOs cannot be reused between trades. If multi-split is disabled, and your wallet only has one UTXO, then only one placement can be made.

The BTC wallet code expects that you have enough funds to place all of the placements in the FundMultiOrder parameters. The problem is that the market making code was not taking into accoun the multi-split buffer. This is an easy fix.

Your work around is fine as long as you are running only one bot. I have a PR that's still work in progress that will allow you to manually specify the balance of a bot (#3081). You will be able to allocate the entire wallet balance to a bot if you like, which will be the equivalent of your workaround.

[norwnd]

Thanks for clarifying all that,

The dex protocol requires that each trade has proof of funds, and UTXOs cannot be reused between trades. If multi-split is disabled, and your wallet only has one UTXO, then only one placement can be made.

Isn't it better to have multi-split disabled by default (like with any other dangerous or semi-dangerous settings) ?

The BTC wallet code expects that you have enough funds to place all of the placements in the FundMultiOrder parameters. The problem is that the market making code was not taking into accoun the multi-split buffer. This is an easy fix.

That would be my guess, glad to know there is a proper fix coming (I'll revert mine after that)

[martonp]

Isn't it better to have multi-split disabled by default (like with any other dangerous or semi-dangerous settings) ?

I don't think this is a dangerous setting.. without it, the bot will generally not work properly.

[norwnd]

I don't think this is a dangerous setting..

I guess it's subjective thing,

for somebody (a user) who doesn't understand how/why this works the way it works (the trade-off between having it on/off) - when he starts MM bot and those split-transactions show up in his wallet (each spending 1$ of his money) for seemingly no reason ... it's not a good introduction to BTC market-making