Acting on TODOs from previous commit

client/core/bookie.go

@@ -1168,16 +1168,7 @@ func handleEpochReportMsg(_ *Core, dc *dexConnection, msg *msgjson.Message) erro
 	if err != nil {
 		return fmt.Errorf("epoch report note unmarshal error: %w", err)
 	}
-	// Acquiring dc.booksMtx here in dc.bookie serves as a barrier for notifications that
-	// come after we sent new subscription request (e.i. notes that need to be applied
-	// on top of snapshot we get from new subscription request will wait on that mutex
-	// here). Note, we don't acquire dc.booksMtx for the whole handleBookOrderMsg
-	// duration; that leaves some room for notifications from previous subscription to
-	// come and apply concurrently with us applying snapshot we just got from new
-	// subscription request.
-	// TODO: I'm not sure at the moment whether is OK to execute this notification from
-	//  old subscription while Resetting order book concurrently. Could this result into
-	//  mixed OrderBook state (for orders, candles cache, or something else) ?
+	// Locking dc.booksMtx to just get the bookie, nothing more.
 	book := dc.bookie(note.MarketID)
 	if book == nil {
 		return fmt.Errorf("no order book found with market id %q", note.MarketID)
@@ -1205,9 +1196,10 @@ func handleEpochOrderMsg(_ *Core, dc *dexConnection, msg *msgjson.Message) error
 	// duration; that leaves some room for notifications from previous subscription to
 	// come and apply concurrently with us applying snapshot we just got from new
 	// subscription request.
-	// TODO: I'm not sure at the moment whether is OK to execute this notification from
-	//  old subscription while Resetting order book concurrently. Could this result into
-	//  mixed OrderBook state (for orders, candles cache, or something else) ?
+	// TODO: for epoch_order notification this ^ is only necessary because we currently
+	//  update bookie.OrderBook.seq here below (if we don't wait on this barrier then
+	//  bookie.OrderBook.seq monotonicity might get corrupted. There is no reason to do
+	//  it and we can stop it with https://github.com/norwnd/dcrdex/issues/8.
 	book := dc.bookie(note.MarketID)
 	if book == nil {
 		return fmt.Errorf("no order book found with market id %q", note.MarketID)

client/orderbook/epochqueue.go

@@ -52,7 +52,7 @@ func (eq *EpochQueue) Enqueue(note *msgjson.EpochOrderNote) error {
 	// Ensure the provided epoch order is not already queued.
 	_, ok := eq.orders[oid]
 	if ok {
-		return fmt.Errorf("%s is already queued", oid)
+		return fmt.Errorf("%s is already queued", oid) // should never happen
 	}
 
 	var commitment order.Commitment

client/orderbook/orderbook.go

@@ -504,6 +504,7 @@ func (ob *OrderBook) Enqueue(note *msgjson.EpochOrderNote) error {
 		if idx > ob.currentEpoch {
 			ob.currentEpoch = idx
 		} else {
+			// This should never happen.
 			ob.log.Errorf("epoch order note received for epoch %d but current epoch is %d", idx, ob.currentEpoch)
 		}
 	}
@@ -532,8 +533,15 @@ func (ob *OrderBook) ValidateMatchProof(note msgjson.MatchProofNote) error {
 		if noteSize == 0 || firstProof {
 			return nil, nil
 		}
-		return nil, fmt.Errorf("epoch %d match proof note references %d orders, but local epoch queue is empty",
-			idx, noteSize)
+		// Due to asynchronous nature of client-server communication, the way it's
+		// currently implemented (epochQueues, proofedEpoch and currentEpoch can fall
+		// out of sync because these are changed in non-atomic manner with respect to
+		// each other) we can't differentiate between server sending invalid match
+		// proof note and client's view on epoch orders being out of sync (non-atomic).
+		// Hopefully we can change it in the future, for now we don't want to treat
+		// it as error, logging in case it's useful for debugging and moving on.
+		ob.log.Tracef("epoch %d match proof note references %d orders, but local epoch queue is empty", idx, noteSize)
+		return nil, nil
 	}
 	eq, err := extractEpochQueue()
 	if eq == nil /* includes err != nil */ {
@@ -548,10 +556,18 @@ func (ob *OrderBook) ValidateMatchProof(note msgjson.MatchProofNote) error {
 		if firstProof && localSize < noteSize {
 			return nil // we only saw part of the epoch
 		}
-		// Since match_proof lags epoch close by up to preimage request timeout,
-		// this can still happen for multiple proofs after (re)connect.
-		return fmt.Errorf("epoch %d match proof note references %d orders, but local epoch queue has %d",
-			idx, noteSize, localSize)
+		// Due to asynchronous nature of client-server communication, the way it's
+		// currently implemented (epochQueues, proofedEpoch and currentEpoch can fall
+		// out of sync because these are changed in non-atomic manner with respect to
+		// each other) we can't differentiate between server sending invalid match
+		// proof note and client's view on epoch orders being out of sync (non-atomic).
+		// Hopefully we can change it in the future, for now we don't want to treat
+		// it as error, logging in case it's useful for debugging and moving on.
+		ob.log.Tracef(
+			"epoch %d match proof note references %d orders, but local epoch queue has %d",
+			idx, noteSize, localSize,
+		)
+		return nil
 	}
 	if len(note.Preimages) == 0 {
 		return nil