doc: mention '--enable-trade'

It is limited, and mostly untested, I would need more explicit semantics
for the border cases to make it more robust.

README.md

@@ -0,0 +1,175 @@
+# Kraken technical assessment: matching engine
+
+## How to
+
+### Build
+
+This project was written on a Linux (NixOS) machine, using the `CMake` build
+system.
+
+To build the project you should run the following commands:
+
+```sh
+mkdir build
+cmake -B build
+cmake --build build
+```
+
+To run unit and integration tests you can use:
+
+```sh
+cmake --build build --target test
+```
+
+### Run
+
+The `kraken` binary should be built in `build/src/kraken`. You can see example
+inputs and their matching outputs in the `data/` directory at the root of the
+repository.
+
+`kraken` reads its input from standard input, and displays its results on
+the standard output. For example
+
+```sh
+kraken < ./data/inputs/balanced-book-1.in.csv
+```
+
+You can use `--enable-trade` to enable trade matching.
+
+## Architecture
+
+### Libraries
+
+The project is divided into small libraries in separate directories, each for
+a specific purpose:
+
+* `book`: defines the vocabulary types to quantify orders.
+* `csv`: reading and writing CSV files, in a very naive way.
+* `engine`: the matching engine proper, and a listener interface used to create
+  the expected output.
+* `parse`: parsing the raw CSV data into a list of orders.
+* `utils`: utility types, specifically a `StrongType` wrapper to ensure a `User`
+  is not mistaken for a `Quantity`.
+
+### A KISS architecture
+
+In each step of the project, the code was kept to its simplest, trying to solve
+the problem at hand in the simplest way possible, while keeping to the single
+responsibility principle. This is why for example:
+
+* The input is parsed at once, and processed in a single step, by different
+  components.
+* Almost no efforts were made to avoid superfluous copies, or such
+  optimizations.
+* The engine and the expected output are separated from each other through a
+  listener interface, instead of entangling both in the same class.
+
+### A test harness
+
+To allow for refactoring without fear, each library has a test-suite to ensure
+it is in working order.
+
+This allowed me to simplify cancelling orders from having to potentially look
+at *all currently active orders* to just a few orders on a given price level.
+
+### Reasonably extensible
+
+Given the focus on "events" (the engine processes each order separately, calling
+the listener at opportune times), it should be fairly simple to extend the core
+of this code to allow for online processing (i.e: the engine reads its input and
+displays its output as it comes along), etc...
+
+### What I would improve
+
+#### Matching trades
+
+The logic used when trade matching is enabled is pretty limited: it wasn't clear
+to me what to do when either of the orders have left-over quantities to be
+fulfilled. More explicit instructions on this point would lead to the removal of
+the final `FIXME`s in the code.
+
+Related to that point, the matching functionality should be tested further,
+rather than just using the two provided examples.
+
+#### Cancelling orders
+
+I do not like the way I have done the `cancel_reverse_info_` mapping: to
+simplify I use a `CancelOrder` value as a key instead of creating an
+`Engine`-specific type.
+
+#### Repetition
+
+I do not like the repetition that happens due to `asks_` and `bids_` being
+"mirror" of each other in the way they should be handled. This is mitigated
+somewhat by making use of helper lambda functions when the code is identical.
+
+#### Top of the book handling
+
+I feel like the `CallbackOnTopOfBookChange` is kind of a hack, even though it
+was very effective to solve the problem I had of:
+
+* I want to memorize the current top of the book
+* At the very end of my current operation, calculate the top of the book again
+* Then call the listener back to notify of a change if it happened.
+
+I think there might be a smarter way to go about this. To (mis-)quote Blaise
+Pascal:
+
+> If I had more time, I would have made it shorter.
+
+### Complexity analysis
+
+This will focus on the matching engine code, let's discard the complexity of
+the input pre-processing from this discussion.
+
+Given the use of `std::{multi_,}map` types, the space analysis is pretty simple:
+linear with respect to the number of active orders. Empty books are not removed
+and would therefore also consume a small amount of space: I am not accounting
+for this in this analysis.
+
+Let's focus on the time complexity.
+
+#### Flush orders
+
+The simplest to process, we just empty all the book information, in a time
+complexity linear in the number of active orders in the book.
+
+#### Cancel order
+
+The first version of this code would have a worst-case cost linear in the
+number of active orders in the book, simply iterating through each one in turn.
+
+Thanks to a reverse-mapping, the cancel cost is now the following:
+
+* Lookup in `cancel_reverse_info_`: logarithmic with respect to the number of
+  active orders across all instruments.
+* Lookup in `bids_`/`asks_` for the book on a given `Symbol`: logarithmic with
+  respect to the number of symbols.
+* Finding the bounds on the price range: logarithmic with respect to the number
+  of orders in the given book.
+* Iterating through that range: linear with respect to the number of orders
+  at the given price range.
+
+#### Trade order
+
+* Lookup on `bids_` and `asks_` for the given symbol: logarithmic to the number
+  of symbols.
+* Look for a cross of the book, ensure the book is not empty (constant time),
+  and look at the first value in the book: logarithmic to the number of orders
+  in the book
+* Inserting the order in the book: logarithmic to the number of orders in the
+  book.
+* Inserting into `cancel_reverse_info_` (for faster cancelling): logarithmic
+  to the number of orders across all instruments.
+
+#### Top-of-book handling
+
+For both trade orders and cancel orders, the `CallbackOnTopOfBookChange` does
+the following:
+
+* Lookup on `bids_` and `asks_` for the given symbol: logarithmic to the number
+  of symbols.
+* Check the size of the book (constant time) and look at the first order's
+  price: logarithmic to the number of orders in the book.
+* Find the price range: logarithmic yet again.
+* Iterating on the range: linear to the number of orders at the given price.

data/outputs/balanced-book-3.trades.out.csv

@@ -0,0 +1,10 @@
+A,1,1
+B,B,10,100
+A,1,2
+B,S,12,100
+A,2,101
+A,2,102
+B,S,11,100
+A,1,103
+T,1,103,2,102,11,100
+B,S,12,100

data/outputs/shallow-ask.trades.out.csv

@@ -0,0 +1,10 @@
+A,1,1
+B,B,10,100
+A,2,101
+A,2,102
+B,S,11,100
+A,1,2
+T,1,2,2,102,11,100
+B,S,-,-
+A,2,103
+B,S,11,100

src/engine/engine.cc

@@ -87,8 +87,9 @@ private:
     TopInfo top_info_{};
 };
 
-Engine::Engine(std::shared_ptr<EngineListener> listener)
-    : listener_(listener) {}
+Engine::Engine(std::shared_ptr<EngineListener> listener,
+               CrossBehaviour cross_behaviour)
+    : listener_(listener), cross_behaviour_(cross_behaviour) {}
 
 void Engine::process_orders(std::vector<Order> const& orders) {
     for (auto const& order : orders) {
@@ -101,6 +102,23 @@ void Engine::operator()(TradeOrder const& trade_order) {
     // Set-up automatic call-back in case top-of-book changes
     auto _ = CallbackOnTopOfBookChange(trade_order.symbol, *this);
 
+    // FIXME: assumes a single trade for the order
+    // FIXME: assumes no remaining orders on both sides
+    auto const do_cross = [this, &trade_order](auto const& bid_order,
+                                               auto const& ask_order,
+                                               Price matching_price) {
+        auto const matching_quantity
+            = std::min(bid_order.quantity, ask_order.quantity);
+
+        listener_->on_acknowledgement(trade_order.user, trade_order.id);
+        listener_->on_match(bid_order.user, bid_order.id, ask_order.user,
+                            ask_order.id, matching_price, matching_quantity);
+        assert(matching_quantity == bid_order.quantity
+               && "multiple matches not implemented");
+        assert(matching_quantity == ask_order.quantity
+               && "multiple matches not implemented");
+    };
+
     // NOTE: some amount of repetition/mirroring
     switch (trade_order.side) {
     case Side::ASK:
@@ -109,8 +127,16 @@ void Engine::operator()(TradeOrder const& trade_order) {
             auto& [_, bid_map] = *bid_map_it;
             if (bid_map.size() > 0
                 && bid_map.begin()->first >= trade_order.price) {
-                // FIXME: handle matching if enabled
-                listener_->on_rejection(trade_order.user, trade_order.id);
+                if (cross_behaviour_ == CrossBehaviour::REJECT) {
+                    listener_->on_rejection(trade_order.user, trade_order.id);
+                } else {
+                    auto matching = bid_map.begin();
+                    auto const& bid_order = matching->second;
+                    // NOTE: Pick the asking price for matching
+                    auto const matching_price = trade_order.price;
+                    do_cross(bid_order, trade_order, matching_price);
+                    bid_map.erase(matching);
+                }
                 return;
             }
         }
@@ -124,8 +150,15 @@ void Engine::operator()(TradeOrder const& trade_order) {
             auto& [_, ask_map] = *ask_map_it;
             if (ask_map.size() > 0
                 && ask_map.begin()->first <= trade_order.price) {
-                // FIXME: handle matching if enabled
-                listener_->on_rejection(trade_order.user, trade_order.id);
+                if (cross_behaviour_ == CrossBehaviour::REJECT) {
+                    listener_->on_rejection(trade_order.user, trade_order.id);
+                } else {
+                    auto matching = ask_map.begin();
+                    // NOTE: Pick the asking price for matching
+                    auto const& [matching_price, ask_order] = *matching;
+                    do_cross(trade_order, ask_order, matching_price);
+                    ask_map.erase(matching);
+                }
                 return;
             }
         }

src/engine/engine.hh

@@ -9,12 +9,21 @@
 
 namespace kraken::engine {
 
+/// Which behaviour on cross orders.
+enum class CrossBehaviour {
+    /// Reject the crossing order.
+    REJECT,
+    /// Make the trade with the matching order(s).
+    MATCH,
+};
+
 struct CallbackOnTopOfBookChange;
 struct EngineListener;
 
 /// Matching engine which processes orders and keeps the book up-to-date.
 struct Engine {
-    Engine(std::shared_ptr<EngineListener> listener);
+    Engine(std::shared_ptr<EngineListener> listener,
+           CrossBehaviour cross_behaviour = CrossBehaviour::REJECT);
 
     /// Process orders, triggerring the listener on each event.
     void process_orders(std::vector<Order> const& orders);
@@ -25,6 +34,7 @@ private:
     void operator()(FlushOrder const& flush_order);
 
     std::shared_ptr<EngineListener> listener_;
+    CrossBehaviour cross_behaviour_;
 
     // Symbol, price, side are implicit given the way the book is represented
     struct OrderMetaData {

src/kraken.cc

@@ -5,12 +5,21 @@
 #include "engine/engine.hh"
 #include "parse/parse.hh"
 
-int main() {
+int main(int argc, char** argv) {
+    auto cross_behaviour = kraken::engine::CrossBehaviour::REJECT;
+
+    if (argc > 1) {
+        using namespace std::literals;
+        if (argv[1] == "--enable-trade"sv) {
+            cross_behaviour = kraken::engine::CrossBehaviour::MATCH;
+        }
+    }
+
     auto const orders = kraken::parse::parse_orders(std::cin);
 
     auto listener = std::make_shared<kraken::engine::CsvEngineListener>();
 
-    auto engine = kraken::engine::Engine(listener);
+    auto engine = kraken::engine::Engine(listener, cross_behaviour);
 
     engine.process_orders(orders);
 

tests/integration/test-output.sh

@@ -31,6 +31,19 @@ test_file() {
     fi
 }
 
+test_file_trades() {
+    if ! diff="$(diff <("$KRAKEN" --enable-trade < "$DATA_DIR/inputs/$1.in.csv") "$DATA_DIR/outputs/$1.trades.out.csv")"; then
+        ((FAILURES += 1))
+        echo "$1 (trades): FAIL"
+        if [ -n "$VERBOSE" ]; then
+            printf '%s\n' "$diff"
+        fi
+    else
+        ((SUCCESSES += 1))
+        echo "$1 (trades): OK"
+    fi
+}
+
 for test_name in "$DATA_DIR"/inputs/*.in.csv; do
     test_name="$(basename "$test_name")"
     test_name="${test_name%%.in.csv}"
@@ -38,6 +51,13 @@ for test_name in "$DATA_DIR"/inputs/*.in.csv; do
     test_file "$test_name"
 done
 
+for test_name in "$DATA_DIR"/outputs/*.trades.out.csv; do
+    test_name="$(basename "$test_name")"
+    test_name="${test_name%%.trades.out.csv}"
+
+    test_file_trades "$test_name"
+done
+
 printf '\nSummary: %d successes, %d failures\n' "$SUCCESSES" "$FAILURES"
 
 exit "$((FAILURES != 0))"

tests/unit/engine.cc

@@ -16,20 +16,36 @@ using namespace kraken::parse;
 
 namespace {
 
+void do_compare(std::istream& input, std::istream& expected_output,
+                CrossBehaviour behaviour) {
+    auto listener = std::make_shared<CsvEngineListener>();
+    auto engine = Engine{listener, behaviour};
+
+    engine.process_orders(parse_orders(input));
+
+    auto expected = read_csv(expected_output, CsvHeader::KEEP);
+
+    ASSERT_EQ(listener->output(), expected);
+}
+
 void compare(std::string const& test_name) {
     using namespace std::literals;
     auto input = std::ifstream{CMAKE_SOURCE_DIR + "/data/inputs/"s + test_name
                                + ".in.csv"};
-    auto listener = std::make_shared<CsvEngineListener>();
-    auto engine = Engine{listener};
-
-    engine.process_orders(parse_orders(input));
-
     auto expected_output_file = std::ifstream{
         CMAKE_SOURCE_DIR + "/data/outputs/"s + test_name + ".out.csv"};
-    auto expected = read_csv(expected_output_file, CsvHeader::KEEP);
 
-    ASSERT_EQ(listener->output(), expected);
+    do_compare(input, expected_output_file, CrossBehaviour::REJECT);
+}
+
+void compare_trades(std::string const& test_name) {
+    using namespace std::literals;
+    auto input = std::ifstream{CMAKE_SOURCE_DIR + "/data/inputs/"s + test_name
+                               + ".in.csv"};
+    auto expected_output_file = std::ifstream{
+        CMAKE_SOURCE_DIR + "/data/outputs/"s + test_name + ".trades.out.csv"};
+
+    do_compare(input, expected_output_file, CrossBehaviour::MATCH);
 }
 
 } // namespace
@@ -97,3 +113,11 @@ TEST(engine, shallow_bid) {
 TEST(engine, tighten_spread_through_new_limit_orders) {
     compare("tighten-spread-through-new-limit-orders");
 }
+
+TEST(engine_trade, balanced_book_3) {
+    compare_trades("balanced-book-3");
+}
+
+TEST(engine_trade, shallow_ask) {
+    compare_trades("shallow-ask");
+}