binance: Retry keep alive.

[JoeGruffins]

trafficstars

Untested. Attempting to solve a problem with stuck books.

[JoeGruffins]

Looking into querying the subscriptions.

[JoeGruffins]

Sorry I have not tested this on testnet/mainnet yet, but I should be able to use on testnet correct?

[buck54321]

Sorry I have not tested this on testnet/mainnet yet, but I should be able to use on testnet correct?

Maybe? I haven't found testing mm on testnet very useful.

[buck54321]

Maybe a better technique would be if an orderbook has not received an update within a certain period, maybe 30 seconds, then resync the orderbook. Currently we will only know that an orderbook is stale every time checkSubs is run, so it could remain stale for 30 mins.

This would result in tons of unnecessary reloads on low-volume markets. checkSubs interval can be much shorter though.

[JoeGruffins]

@martonp does the order book desync look ok? https://github.com/decred/dcrdex/compare/1a223db8b8ae83b3e4da70b6ddd1adbf421ef027..e3cf2a8358ff0ac4a6e6e61507439a2b1478f56b

[JoeGruffins]

I don't think this test fail is due to my changes....

[martonp]

There were a few issues I fixed.. consider these changes: https://github.com/decred/dcrdex/commit/68008b363d9e28fb49d616b757a6f1585392c6cc

You can test using the TestVWAP in the diff I posted above: https://github.com/decred/dcrdex/pull/2958#pullrequestreview-2319691032

I think another good change would be to trigger a reconnect if the list subscriptions request fails, because this probably means that the connection is broken.

[martonp]

I made some updated to that commit: a15e541309bc9d0aadd9827fab9445a76bba1058

The changes in wsconn.go, why not just add the reconnect timer to keepAlive? As it is now we have to wait for a read or a write for the scheduled reconnect to happen. So, it will happen after we want it to, or not at all if we never have another read or write.

You're right, but I don't think the timer should be in keepAlive, because we need the read loop to end. I've updated it so that we don't have to wait for a read.

[JoeGruffins]

Thanks those changes look good to me. Added them. They replaced the last commit.

[JoeGruffins]

testbinance panic:

2024-10-10 17:10:45.032 [ERR] TB: Error getting deposit confirmations for ETH -> 0x41e71205021391160dc30bd5840f8a93e9d2bdcaf2fbe5c60f50c75d2fc7cc7a: TransactionReceipt error: not found
2024-10-10 17:25:17.298 [ERR] TB: Error getting deposit confirmations for BTC -> f7e8cb5a3ba91149970f97d3d8c9b9b645442514849a55a139d01b3d8495f129: gettransaction error with output = "error code: -5\nerror message:\nInvalid or non-wallet transaction id\n", err = exit status 5
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x8016f8]

goroutine 70 [running]:
main.(*fakeBinance).updateOrderBalances(0xc0001243c0, {0xc000144df6?, 0xd659c0?}, 0x0, 0x3fb999999999999a, 0x3fa43731c574e9b7, 0x2)
        /home/joe/git/dcrdex/client/cmd/testbinance/main.go:968 +0xf8
main.(*fakeBinance).run.func2()
        /home/joe/git/dcrdex/client/cmd/testbinance/main.go:408 +0x4b3
created by main.(*fakeBinance).run in goroutine 1
        /home/joe/git/dcrdex/client/cmd/testbinance/main.go:369 +0xd1

[JoeGruffins]

Removed the wsconn changes for now. They should maybe be a separate pr. Attempting to fix the other issues pointed out by @martonp https://github.com/decred/dcrdex/compare/d089412d7cd1f74acae443b4728d581dad35efb3..1ea349eed8446916cd75718efdf265400db9925d