triangular_arbitrage_bot/notes/audit.md

3.9 KiB

Code Audit — 2026-05-28

Verified against: commit 728f416 (evaluate.c paper-trade simulation) + all other files.


A. Evaluate vs Executor Cascade (Paper Mode)

These compare the paper-mode simulation in evaluate.c against the paper-mode simulation in executor.c. They must produce identical results for identical inputs.

ID Status Files Lines Description
A1 FIXED evaluate.c:320, executor.c:176 320, 176 Pre-fill apply_fee_hold on legs 1-2. Both now have it — matches.
A2 FIXED evaluate.c:359, executor.c:348 359, 348 Input apply_fee_hold removed from legs 1-2. Fee hold now applied only once per leg, at end-of-leg cascade.
A3 FIXED evaluate.c:334-335, executor.c:267-268 334,267 Buy total_funds rounding: both now use sl->quote_increment — matches.
A4 FIXED evaluate.c:354-356, executor.c:293-295 354,293 Fee formula: both now use output_amt * fee_rate — matches.
A5/A8 FIXED evaluate.c:278-281 278, 281 Sell order_param now uses base (base amount) instead of quote_volume. Fixes the dimension mismatch for sell-first triangles. Both sides consistent.

B. Other Bugs and Logic Errors

ID Status File Lines Description
B1 ACTIVE symbols_api.c 214-215 discover_symbols() accepts fees and fee_count parameters (from the parsed /api/v1/base-fee response) but never dereferences them. Fee rates are computed from fee_category * 0.001 * taker_fee_coeff instead. The fetched fee table is silently discarded.
B2 MINOR config.c 82 stats_interval_seconds is parsed but never read. Status interval is hardcoded to 30000ms in evaluate.c.
B3 MINOR config.h 37 executor_socket_path declared, never written, never read.
B4 ACTIVE ws_client.c 882-948 MAX_DIRTY_BATCH = 64. If a single ws_client_read burst processes more than 64 unique symbols, excess symbols are silently dropped from evaluation. No warning logged. With 400+ symbols split across connections, a burst on a busy connection can exceed 64.
B5 ACTIVE symbols_api.c 390-392 symbol_table_lookup returning -1 (symbol not found) causes index 0 to be assigned silently. Logged as a count but not per-symbol. Multiple unrelated triangles can silently map to the same wrong symbol at index 0.
B6 ACTIVE rest_client.c 86-88 SSL connection health check uses raw recv(fd, MSG_PEEK) on the underlying TCP socket, bypassing the SSL layer. SSL can buffer data in its own layer that has been consumed from the kernel socket buffer, causing recv() to return 0 or EAGAIN and falsely triggering a reconnect.
B7 FALSE rest_client.c 211-247 SSL_ERROR_WANT_READ is handled correctly with continue. Break only triggers when total >= need. No truncation possible.
B8 MINOR executor.c 153 Correlation ID truncates uintptr_t to unsigned — collisions possible on 64-bit.
B9 ACTIVE executor.c 471 sh->count-- has no bounds check. Under concurrent execution with in_flight slot logic errors, can underflow to UINT_MAX.
B10 MINOR ws_client.c 544 conn->state = WS_STATE_CONNECTED even on subscribe failure (return -1). Reconnect loop checks WS_STATE_DISCONNECTED, so a failed subscribe leaves the connection in a broken CONNECTED state.
D4 ACTIVE fill_handler.c 67-76 SPSC fill_channel_pop is designed for single-consumer but called from multiple executor threads concurrently (via fill_channel_await). Data race on the ring buffer: two threads can read the same slot, or one thread can advance head past a slot the other is about to read.

Summary

Category Count
Active bugs (B1, B4, B5, B6, B9, D4) 6
Minor/dead config (B2, B3, B8, B10) 4
Double fee hold — both sides match (A2) 1
Fixed in this commit (A1, A3, A4, A5/A8) 4
False positives (A6, A7, B7) 3