Skip to content

client/core: monotonic OrderBook state updates #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
norwnd wants to merge 1 commit into
base: master
Choose a base branch
from
Open

client/core: monotonic OrderBook state updates #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion client/core/core_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9502,9 +9502,10 @@ func TestPreOrder(t *testing.T) {
var baseFeeRate uint64 = 5
var quoteFeeRate uint64 = 10

const seedSeq = 1
err := book.Sync(&msgjson.OrderBook{
MarketID: tDcrBtcMktName,
Seq: 1,
Seq: seedSeq,
Epoch: 1,
Orders: []*msgjson.BookOrderNote{sellNote, &buyNote},
BaseFeeRate: baseFeeRate,
Expand Down Expand Up @@ -9580,10 +9581,12 @@ func TestPreOrder(t *testing.T) {

// Market orders have to have a market to make estimates.
book.Unbook(&msgjson.UnbookOrderNote{
Seq: seedSeq + 1,
MarketID: tDcrBtcMktName,
OrderID: sellNote.OrderID,
})
book.Unbook(&msgjson.UnbookOrderNote{
Seq: seedSeq + 2,
MarketID: tDcrBtcMktName,
OrderID: buyNote.OrderID,
})
Expand Down
11 changes: 6 additions & 5 deletions client/orderbook/epochqueue_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ import (

var tLogger = dex.NewLogger("TBOOK", dex.LevelTrace, os.Stdout)

func makeEpochOrderNote(mid string, oid order.OrderID, side uint8, rate uint64, qty uint64, commitment order.Commitment, epoch uint64) *msgjson.EpochOrderNote {
func makeEpochOrderNote(seq uint64, mid string, oid order.OrderID, side uint8, rate uint64, qty uint64, commitment order.Commitment, epoch uint64) *msgjson.EpochOrderNote {
return &msgjson.EpochOrderNote{
Commit: commitment[:],
BookOrderNote: msgjson.BookOrderNote{
OrderNote: msgjson.OrderNote{
Seq: seq,
MarketID: mid,
OrderID: oid[:],
},
Expand Down Expand Up @@ -69,21 +70,21 @@ func TestEpochQueue(t *testing.T) {
copy(n1Pimg[:], n1PimgB)
n1Commitment := n1Pimg.Commit() // aba75140b1f6edf26955a97e1b09d7b17abdc9c0b099fc73d9729501652fbf66
n1OrderID := [32]byte{'a'}
n1 := makeEpochOrderNote(mid, n1OrderID, msgjson.BuyOrderNum, 1, 2, n1Commitment, epoch)
n1 := makeEpochOrderNote(1, mid, n1OrderID, msgjson.BuyOrderNum, 1, 2, n1Commitment, epoch)

n2PimgB, _ := hex.DecodeString("8e6c140071db1eb2f7a18194f1a045a94c078835c75dff2f3e836180baad9e95")
var n2Pimg order.Preimage
copy(n2Pimg[:], n2PimgB)
n2Commitment := n2Pimg.Commit() // 0f4bc030d392cef3f44d0781870ab7fcb78a0cda36c73e50b88c741b4f851600
n2OrderID := [32]byte{'b'}
n2 := makeEpochOrderNote(mid, n2OrderID, msgjson.BuyOrderNum, 1, 2, n2Commitment, epoch)
n2 := makeEpochOrderNote(2, mid, n2OrderID, msgjson.BuyOrderNum, 1, 2, n2Commitment, epoch)

n3PimgB, _ := hex.DecodeString("e1f796fa0fc16ba7bb90be2a33e87c3d60ab628471a420834383661801bb0bfd")
var n3Pimg order.Preimage
copy(n3Pimg[:], n3PimgB)
n3Commitment := n3Pimg.Commit() // aba75140b1f6edf26955a97e1b09d7b17abdc9c0b099fc73d9729501652fbf66
n3OrderID := [32]byte{'c'}
n3 := makeEpochOrderNote(mid, n3OrderID, msgjson.BuyOrderNum, 1, 2, n3Commitment, epoch)
n3 := makeEpochOrderNote(3, mid, n3OrderID, msgjson.BuyOrderNum, 1, 2, n3Commitment, epoch)

// This csum matches the server-side tests.
wantCSum, _ := hex.DecodeString("8c743c3225b89ffbb50b5d766d3e078cd8e2658fa8cb6e543c4101e1d59a8e8e")
Expand Down Expand Up @@ -257,7 +258,7 @@ func benchmarkGenerateMatchProof(c int, b *testing.B) {
preimages := make([]order.Preimage, 0, c)
for i := range notes {
pi := randPreimage()
notes[i] = makeEpochOrderNote("mkt", randOrderID(),
notes[i] = makeEpochOrderNote(uint64(i), "mkt", randOrderID(),
msgjson.BuyOrderNum, 1, 2, blake256.Sum256(pi[:]), 10)
preimages = append(preimages, pi)
}
Expand Down
83 changes: 70 additions & 13 deletions client/orderbook/orderbook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,17 +140,35 @@ func (ob *OrderBook) isSynced() bool {
return ob.synced
}

// setSeq should be called whenever a sequenced message is received. If seq is
// out of sequence, an error is logged.
func (ob *OrderBook) setSeq(seq uint64) {
// tryApplySeq should be called whenever a sequenced message is received, to
// verify it's seq number is in valid range and apply it if it is. It returns:
// - warn = "" and error = nil when seq number is good and state notification can
// be applied
// - warn != "" and error = nil when seq number is in expected range, but we shouldn't
// be applying corresponding notification state on top of current OrderBook
// - warn = "" and error != nil error when seq number is obviously bad, and we shouldn't
// be applying corresponding notification state on top of current OrderBook
func (ob *OrderBook) tryApplySeq(seq uint64) (warn string, err error) {
ob.seqMtx.Lock()
defer ob.seqMtx.Unlock()
if seq != ob.seq+1 {
ob.log.Errorf("notification received out of sync. %d != %d - 1", ob.seq, seq)
if seq <= ob.seq {
// Due to asynchronous nature of client-server order book we might receive
// notification from previous subscription, previous
// meaning it will have seq number less than or equal to OrderBook.seq
// (which is seq in order book snapshot we got when resubscribing),
// and it's Ok to drop these outdated transactions. Still, log this
// event just in case we observe something unexpected.
return fmt.Sprintf("got sequence number from the past %d, want exactly "+
"%d + 1", seq, ob.seq), nil
}
if seq > ob.seq {
ob.seq = seq
if seq != ob.seq+1 {
// This is an error (could be a bug), it's gotta be strictly +1. So just
// log it and move on.
return "", fmt.Errorf("got sequence number from the future %d, want "+
"exactly %d + 1", seq, ob.seq)
}
ob.seq = seq
return "", nil
}

// cacheOrderNote caches an order note.
Expand Down Expand Up @@ -224,7 +242,7 @@ func (ob *OrderBook) processCachedNotes() error {
}

// Sync updates a client tracked order book with an order book snapshot. It is
// an error if the the OrderBook is already synced.
// an error if the OrderBook is already synced.
func (ob *OrderBook) Sync(snapshot *msgjson.OrderBook) error {
if ob.isSynced() {
return fmt.Errorf("order book is already synced")
Expand All @@ -236,7 +254,9 @@ func (ob *OrderBook) Sync(snapshot *msgjson.OrderBook) error {
// snapshot. This resets the sequence.
// TODO: eliminate this and half of the mutexes!
func (ob *OrderBook) Reset(snapshot *msgjson.OrderBook) error {
// Don't use setSeq here, since this message is the seed and is not expected
ob.log.Tracef("Resetting order book from server snapshot seq: %d", snapshot.Seq)

// Don't use tryApplySeq here, since this message is the seed and is not expected
// to be 1 more than the current seq value.
ob.seqMtx.Lock()
ob.seq = snapshot.Seq
Expand Down Expand Up @@ -314,7 +334,16 @@ func (ob *OrderBook) book(note *msgjson.BookOrderNote, cached bool) error {
}
}

ob.setSeq(note.Seq)
warn, err := ob.tryApplySeq(note.Seq)
if err != nil {
ob.log.Errorf("Unexpected book_order notification encountered: %+v, err: %v", note, err)
return nil // log for investigation and skip
}
if warn != "" {
ob.log.Tracef("Undesirable book_order notification encountered: %+v, reason: %s", note, warn)
return nil // log for debugging and skip
}
// Otherwise, safe to apply.

if len(note.OrderID) != order.OrderIDSize {
return fmt.Errorf("expected order id length of %d, got %d",
Expand Down Expand Up @@ -370,7 +399,16 @@ func (ob *OrderBook) updateRemaining(note *msgjson.UpdateRemainingNote, cached b
}
}

ob.setSeq(note.Seq)
warn, err := ob.tryApplySeq(note.Seq)
if err != nil {
ob.log.Errorf("Unexpected update_remaining notification encountered: %+v, err: %v", note, err)
return nil // log for investigation and skip
}
if warn != "" {
ob.log.Tracef("Undesirable update_remaining notification encountered: %+v, reason: %s", note, warn)
return nil // log for debugging and skip
}
// Otherwise, safe to apply.

if len(note.OrderID) != order.OrderIDSize {
return fmt.Errorf("expected order id length of %d, got %d",
Expand Down Expand Up @@ -423,7 +461,16 @@ func (ob *OrderBook) unbook(note *msgjson.UnbookOrderNote, cached bool) error {
}
}

ob.setSeq(note.Seq)
warn, err := ob.tryApplySeq(note.Seq)
if err != nil {
ob.log.Errorf("Unexpected unbook_order notification encountered: %+v, err: %v", note, err)
return nil // log for investigation and skip
}
if warn != "" {
ob.log.Tracef("Undesirable unbook_order notification encountered: %+v, reason: %s", note, warn)
return nil // log for debugging and skip
}
// Otherwise, safe to apply.

if len(note.OrderID) != order.OrderIDSize {
return fmt.Errorf("expected order id length of %d, got %d",
Expand Down Expand Up @@ -493,7 +540,17 @@ func (ob *OrderBook) Orders() ([]*Order, []*Order, []*Order) {

// Enqueue appends the provided order note to the corresponding epoch's queue.
func (ob *OrderBook) Enqueue(note *msgjson.EpochOrderNote) error {
ob.setSeq(note.Seq)
warn, err := ob.tryApplySeq(note.Seq)
if err != nil {
ob.log.Errorf("Unexpected epoch_order notification encountered: %+v, err: %v", note, err)
return nil // log for investigation and skip
}
if warn != "" {
ob.log.Tracef("Undesirable epoch_order notification encountered: %+v, reason: %s", note, warn)
return nil // log for debugging and skip
}
// Otherwise, safe to apply.

idx := note.Epoch
ob.epochMtx.Lock()
defer ob.epochMtx.Unlock()
Expand Down
Loading