core: protect epoch checksum with its own mutex #2977
Merged
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
JoeGruffins
reviewed
Sep 18, 2024
There was a problem hiding this comment.
Looks good but trying it out all of my cancels fail.
2024-09-18 08:07:17.441 [INF] CORE: Cancel order d208ee4fa7f536ffb2025bf6c1b31d1fbe9af6c506d3332fb70478ab3cac7e24 targeting order 7f0eec29050cd41ed20ce16aa5aec94c08bfb4cf6aeb8ef60b9c483478c078ab at 127.0.0.1:17273 has been placed
2024-09-18 08:07:17.441 [DBG] CORE: notify: |POKE| (order) Cancelling order - A cancel order has been submitted for order {{{order|7f0eec29050cd41ed20ce16aa5aec94c08bfb4cf6aeb8ef60b9c483478c078ab}}} - Order: 7f0eec29050cd41ed20ce16aa5aec94c08bfb4cf6aeb8ef60b9c483478c078ab
....
2024-09-18 08:07:30.000 [DBG] CORE: Received preimage request for order d208ee4fa7f536ffb2025bf6c1b31d1fbe9af6c506d3332fb70478ab3cac7e24 with known commitment 753dae1f98bd897467f8754912c82533b36612288f448f9dbefb472085a35fe7
2024-09-18 08:07:30.000 [DBG] CORE: async processPreimageRequest for d208ee4fa7f536ffb2025bf6c1b31d1fbe9af6c506d3332fb70478ab3cac7e24 succeeded
2024-09-18 08:07:30.000 [ERR] CORE[wss://127.0.0.1:17273/ws]: No handler found for response: {"type":2,"id":8886,"payload":{"result":null,"error":{"code":45,"message":"preimage hash f655dda0147785bf309308f63fe2c50a548f5ccecd864b91932db04823f23069 does not match order commitment 753dae1f98bd897467f8754912c82533b36612288f448f9dbefb472085a35fe7"}},"sig":""}
2024-09-18 08:07:30.011 [DBG] CORE: Score changed at 127.0.0.1:17273. New score is 13 / 60, tier = 10, penalties = 0
2024-09-18 08:07:30.011 [WRN] CORE: Deleting failed cancel order d208ee4fa7f536ffb2025bf6c1b31d1fbe9af6c506d3332fb70478ab3cac7e24 that targeted trade order 7f0eec29050cd41ed20ce16aa5aec94c08bfb4cf6aeb8ef60b9c483478c078ab
2024-09-18 08:07:30.013 [WRN] CORE: notify: |WARNING| (order) Failed cancel - Cancel order for order 7f0eec29050cd41ed20ce16aa5aec94c08bfb4cf6aeb8ef60b9c483478c078ab failed and is now deleted. - Order: 7f0eec29050cd41ed20ce16aa5aec94c08bfb4cf6aeb8ef60b9c483478c078ab
I'm not able to reproduce this at all. I'm running an mm+arb bot with full market swings with and without loadbot sniper. All of my cancels are successful. |
|
hmm, tried again today. Make a new simnet and new bisonw. Send funds. Make an order then click the |
|
Yeah I don't know what the heck I was looking at. Should be fixed now. |
JoeGruffins
approved these changes
Oct 11, 2024
dev-warrior777
approved these changes
Oct 11, 2024
There was a problem hiding this comment.
Tested before and after latest commit.
latest: cancel_logs.zip
Looks good.
buck54321
added a commit
to buck54321/dcrdex
that referenced
this pull request
Oct 17, 2024
* match checksum to separate mutex and add dc cancel tracking
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.
The
trackedTrade.csumfield was protected under the very broadtrackedTrade.mtx. This created lock contention when preimage requests came in for cancel orders while we had long running ticks, sinceacceptCsumhad to lock that mutex. With this change, we now protect the checksum fields for trades and cancels under a different mutex.Additionally, there were two places where the
trackedTrade.cancelfield was being read unsafely. A solution for that is implemented.Closes #2964
Supercedes #2973