AttemptFill tests are passing

go.mod

@@ -15,6 +15,8 @@ require (
 	github.com/magiconair/properties v1.8.7 // indirect
 	github.com/mitchellh/mapstructure v1.5.0 // indirect
 	github.com/pelletier/go-toml/v2 v2.0.6 // indirect
+	github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5 // indirect
+	github.com/sasha-s/go-deadlock v0.3.1 // indirect
 	github.com/spf13/afero v1.9.3 // indirect
 	github.com/spf13/cast v1.5.0 // indirect
 	github.com/spf13/jwalterweatherman v1.1.0 // indirect

go.sum

@@ -142,6 +142,8 @@ github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyua
 github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
 github.com/pelletier/go-toml/v2 v2.0.6 h1:nrzqCb7j9cDFj2coyLNLaZuJTLjWjlaz6nvTvIwycIU=
 github.com/pelletier/go-toml/v2 v2.0.6/go.mod h1:eumQOmlWiOPt5WriQQqoM5y18pDHwha2N+QD+EUNTek=
+github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5 h1:q2e307iGHPdTGp0hoxKjt1H5pDo6utceo3dQVK3I5XQ=
+github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5/go.mod h1:jvVRKCrJTQWu0XVbaOlby/2lO20uSCHEMzzplHXte1o=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/sftp v1.13.1/go.mod h1:3HaPG6Dq1ILlpPZRO0HVMrsydcdLt6HRDccSgb87qRg=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
@@ -150,6 +152,8 @@ github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:
 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
 github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k=
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
+github.com/sasha-s/go-deadlock v0.3.1 h1:sqv7fDNShgjcaxkO0JNcOAlr8B9+cV5Ey/OB71efZx0=
+github.com/sasha-s/go-deadlock v0.3.1/go.mod h1:F73l+cr82YSh10GxyRI6qZiCgK64VaZjwesgfQ1/iLM=
 github.com/spf13/afero v1.9.3 h1:41FoI0fD7OR7mGcKE/aOiLkGreyf8ifIOQmJANWogMk=
 github.com/spf13/afero v1.9.3/go.mod h1:iUV7ddyEEZPO5gA3zD4fJt6iStLlL+Lg4m2cihcDf8Y=
 github.com/spf13/cast v1.5.0 h1:rj3WzYc11XZaIZMPKmwP96zkFEnnAmV8s6XbB2aY32w=

pkg/orderbook/orderbook.go

@@ -8,7 +8,8 @@ import (
 	"fmt"
 	"log"
 	"sort"
-	"sync"
+
+	"github.com/sasha-s/go-deadlock"
 
 	"github.com/dylanlott/orderbook/pkg/accounts"
 )
@@ -47,17 +48,21 @@ type Order struct {
 	Metadata  map[string]string
 }
 
-// Match holds a buy and a sell side order
+// Match holds a buy and a sell side order at a quantity per price.
+// Matches can be made for any type of order, including limit or market orders.
 type Match struct {
-	Buy  *Order
-	Sell *Order
-	Tx   accounts.Transaction
+	Buy      *Order
+	Sell     *Order
+	Price    uint64 // at what price was each unit purchased by the buyer from the seller
+	Quantity uint64 // how many units were transferred from seller to buyer
+	Total    uint64 // total = price * quantity
 }
 
 // Book holds buy and sell side orders. OpRead and OpWrite are applied to
 // to the book. Buy and sell side orders are binary trees of order lists.
 type Book struct {
-	sync.RWMutex
+	// sync.RWMutex
+	deadlock.Mutex
 
 	buy  *Node
 	sell *Node
@@ -99,7 +104,6 @@ func Start(
 		}
 	}()
 
-	// TODO: factor out into a handleFills function
 	for {
 		select {
 		case <-ctx.Done():
@@ -109,69 +113,84 @@ func Start(
 			if w.Order.Side == "buy" {
 				o := &w.Order
 				book.buy.Insert(o)
-				go attemptFill(book, accts, o, matches, errs)
+				go AttemptFill(book, accts, o, matches, errs)
 				w.Result <- WriteResult{
 					Order: *o,
 					Err:   nil,
 				}
 			} else {
 				o := &w.Order
 				book.sell.Insert(o)
-				go attemptFill(book, accts, o, matches, errs)
+				go AttemptFill(book, accts, o, matches, errs)
 				w.Result <- WriteResult{
 					Order: *o,
 					Err:   nil,
 				}
 			}
-		default:
-			book.buy.Print()
-			book.sell.Print()
 		}
 	}
 }
 
-func attemptFill(
+// AttemptFill attempts to fill an order until it's completed.
+// * For simplicity, AttemptFill controls the book mutex.
+// It loops until the order is filled.
+func AttemptFill(
 	book *Book,
 	acc accounts.AccountManager,
 	fillorder *Order,
 	matches chan Match,
 	errs chan error,
 ) {
-	// Loop as long as the order is not filled
-	for fillorder.Filled < fillorder.Open {
-		book.RWMutex.Lock()
+	for {
+		book.Lock()
+
+		if fillorder.Filled < fillorder.Open {
+			log.Printf("NOT FILLED")
+		} else {
+			log.Printf("FILLED: %+v", fillorder)
+		}
 
 		if fillorder.Side == "buy" {
 			// match to sell
+			log.Printf("[buy order]: %+v", fillorder)
 			low := book.sell.FindMin()
 			if len(low.Orders) == 0 {
+				book.Unlock()
 				continue
 			}
 
-			bookorder := low.Orders[0]
+			bookorder := low.Orders[0] // select highest time priority by first price-valid match
 			available := bookorder.Open - bookorder.Filled
+
 			wanted := fillorder.Open - fillorder.Filled
 
 			match := &Match{
 				Buy:  fillorder,
 				Sell: bookorder,
 			}
 
-			if wanted > available {
+			switch {
+			case wanted > available:
 				greedy(book, acc, match, matches, errs)
-			}
-
-			if wanted < available {
+			case wanted < available:
 				humble(book, acc, match, matches, errs)
-			}
-
-			if wanted == available {
+			default:
 				exact(book, acc, match, matches, errs)
 			}
+
+		} else {
+			// sell
+			log.Printf("[sell order]: %+v", fillorder)
 		}
+
+		book.Unlock()
 	}
 }
 
+// greedy, humble, and exact are the three order handlers for different scenarios
+// of supply and demand between a match on price. These functions shouldn't handle
+// locking or unlocking, that should all be handled in the AttemptFill function.
+
 // exact is a buy order that wants the exact available amount from the sell order
 func exact(book *Book, acc accounts.AccountManager, match *Match, matchCh chan Match, errs chan error) {
 	available := match.Sell.Open - match.Sell.Filled
@@ -180,20 +199,21 @@ func exact(book *Book, acc accounts.AccountManager, match *Match, matchCh chan M
 		log.Fatalf("should not happen, this is a bug - match: %+v", match)
 	}
 
-	// amount is calculated from price and available quantity
 	amount := float64((available * match.Sell.Price) / 100)
 
-	_, err := acc.Tx(match.Buy.AccountID, match.Sell.AccountID, amount)
+	balances, err := acc.Tx(match.Buy.AccountID, match.Sell.AccountID, amount)
 	if err != nil {
 		errs <- fmt.Errorf("failed to transfer: %v", err)
 		return
 	}
+	log.Printf("balances: %+v", balances)
 
-	// mark both as filled
 	match.Buy.Filled += available
 	match.Sell.Filled += available
 
-	// remove it from books,
+	match.Buy.History = append(match.Buy.History, *match)
+	match.Sell.History = append(match.Sell.History, *match)
+
 	if ok := book.buy.RemoveOrder(match.Buy); !ok {
 		errs <- fmt.Errorf("failed to remove over from tree %+v", match.Buy)
 	}
@@ -214,7 +234,6 @@ func humble(
 ) {
 	// we know it's a humble fill, so we're taking less than the total available.
 	wanted := match.Buy.Open - match.Buy.Filled
-	// amount is calculated from price and available quantity
 	amount := float64((wanted * match.Sell.Price) / 100)
 	balances, err := acc.Tx(match.Buy.AccountID, match.Sell.AccountID, amount)
 	if err != nil {
@@ -223,16 +242,16 @@ func humble(
 	}
 	log.Printf("[TX] updated balances: %+v", balances)
 
-	// update order quantities
 	match.Buy.Filled += wanted
 	match.Sell.Filled += wanted
 
-	// remove the order from the buyside books
+	match.Buy.History = append(match.Buy.History, *match)
+	match.Sell.History = append(match.Sell.History, *match)
+
 	if ok := book.buy.RemoveOrder(match.Buy); !ok {
 		errs <- fmt.Errorf("failed to remove order from buy side: %+v", match.Buy)
 	}
 
-	// mark as filled
 	matchCh <- *match
 }
 
@@ -247,28 +266,33 @@ func greedy(
 	// a greedy fill takes all that's available.
 	available := match.Sell.Open - match.Sell.Filled
 
-	// amount is calculated from price and available quantity
 	amount := float64((available * match.Sell.Price) / 100)
 
-	// transfer from buyer to seller the agreed upon amount
-	balances, err := acc.Tx(match.Buy.AccountID, match.Sell.AccountID, amount)
+	_, err := acc.Tx(match.Buy.AccountID, match.Sell.AccountID, amount)
 	if err != nil {
 		errs <- fmt.Errorf("failed to transfer: %v", err)
 		return
 	}
-	log.Printf("[TX] updated balances: %+v", balances)
 
-	// fill both sides
 	match.Sell.Filled += available
 	match.Buy.Filled += available
 
+	match.Price = match.Sell.Price
+	match.Quantity = available
+
+	match.Buy.History = append(match.Buy.History, *match)
+	match.Sell.History = append(match.Sell.History, *match)
+
 	if ok := book.sell.RemoveOrder(match.Sell); !ok {
 		errs <- fmt.Errorf("failed to remove sell order from the books %+v", match.Sell)
+		return
 	}
 
 	matchCh <- *match
 }
 
+// TODO: write another set of tests using MatchOrders instead and then benchmark them.
+//
 // MatchOrders is an alternative approach to order matching that
 // works by aligning two opposing sorted slices of Orders.
 func MatchOrders(buyOrders []Order, sellOrders []Order) []Order {

pkg/orderbook/orderbook_test.go

@@ -86,7 +86,7 @@ func TestRun(t *testing.T) {
 	wg.Wait()
 }
 
-func Test_attemptFill(t *testing.T) {
+func TestAttemptFill(t *testing.T) {
 	acc := &accounts.InMemoryManager{
 		Accounts: map[string]*accounts.UserAccount{
 			"foo@test.com": {
@@ -161,19 +161,89 @@ func Test_attemptFill(t *testing.T) {
 				},
 				acc:       acc,
 				fillorder: fillorder,
-				matches:   make(chan Match),
-				errs:      make(chan error),
+				matches:   make(chan Match, 1000),
+				errs:      make(chan error, 1000),
+			},
+		},
+		{
+			name: "should fill greedy",
+			args: args{
+				book: &Book{
+					buy: &Node{
+						Price:  10,
+						Orders: []*Order{},
+						Left:   &Node{},
+						Right: &Node{
+							Price: 11,
+							Orders: []*Order{
+								{
+									Price:     11,
+									ID:        "foo",
+									Side:      "buy",
+									Filled:    0,
+									Open:      20,
+									AccountID: "foo@test.com",
+									Kind:      "market",
+									History:   make([]Match, 0),
+								},
+							},
+							Left:  &Node{},
+							Right: &Node{},
+						},
+					},
+					sell: &Node{
+						Price:  10,
+						Orders: []*Order{},
+						Left: &Node{
+							Price: 9,
+							Orders: []*Order{
+								{
+									Price:     9,
+									ID:        "bar",
+									Side:      "sell",
+									Filled:    0,
+									Open:      10,
+									AccountID: "bar@test.com",
+									Kind:      "market",
+									History:   make([]Match, 0),
+								},
+								{
+									Price:     9,
+									ID:        "baz",
+									Side:      "sell",
+									Filled:    0,
+									Open:      10,
+									AccountID: "baz@test.com",
+									Kind:      "market",
+									History:   make([]Match, 0),
+								},
+								{
+									Price:     9,
+									ID:        "baz",
+									Side:      "sell",
+									Filled:    0,
+									Open:      10,
+									AccountID: "baz@test.com",
+									Kind:      "market",
+									History:   make([]Match, 0),
+								},
+							},
+						},
+						Right: &Node{},
+					},
+				},
+				acc:       acc,
+				fillorder: fillorder,
+				matches:   make(chan Match, 1000),
+				errs:      make(chan error, 1000),
 			},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			go attemptFill(tt.args.book, tt.args.acc, fillorder, tt.args.matches, tt.args.errs)
+			go AttemptFill(tt.args.book, tt.args.acc, fillorder, tt.args.matches, tt.args.errs)
 			got := <-tt.args.matches
-			fmt.Printf("got: %v\n", got)
-			fmt.Printf("tt.args.book: %v\n", tt.args.book)
-			fmt.Printf("tt.args.book.buy.FindMax(): %v\n", tt.args.book.buy.FindMax())
-			fmt.Printf("tt.args.book.sell.FindMin(): %v\n", tt.args.book.sell.FindMin())
+			t.Logf("[got]: %+v", got)
 		})
 	}
 }