Sample trading bot code review report

Sample format, fictional bot, real run. Bot A, the mock of Exchange A and the script reproduce.py are in the flawed_demo folder of the order gateway download, so you can check each finding yourself. The output blocks are pasted unedited from python reproduce.py, run on November 5, 2026 with Python 3.11.3 on Windows 11. The same run with Python 3.12.13 on Debian 13 printed the same text. Text in [square brackets] would be filled in for a real report. This is the written report from the trading bot code review service.

1. Summary

Bot A's normal path works: one decision makes one order, and its requests are signed the way Exchange A expects. Its problems appear when something goes wrong. The run reproduced all 14 findings. Six of them can send unintended orders or expose the account, whatever rules the bot runs. A real report would recommend fixing those six before the bot's next live run, and that decision would be Client A's.

14 of 14 defects reproduced. 2 of 2 checks of what works passed.
Bot A and Exchange A are fictional, and the mock is not a market.
This output says nothing about whether any trading rules make money.

This review covers the software, never whether the rules will make money. It does not say whether any trading rules are good, and it gives no advice on strategies or parameters. Bot A has no trading rules at all: the script hands it each decision directly.

2. What was reviewed, and how

• Read: every file in bot_a, including its settings.
• Traced: the path of an order from decision to Exchange A and back, and what happens on each failure.
• Reproduced: each finding, with reproduce.py against exchange_a_mock.py, a mock of Exchange A on 127.0.0.1. The mock has no order book, no prices and no liquidity. An order rests until the script calls fill(), and each failure happens because the script asked for it. The script prints no times, ports or signatures, so its output is the same on every run. It exits with 0 when every defect shows up.
• Not done: nothing was run against a real exchange or testnet. Under each finding, "What the run showed" summarizes the script's output, which follows it unedited, and "What can happen" is what would follow in a real deployment.

3. How findings are ranked

Findings are ranked by what can happen to the account, then by how likely that is. Likelihood matters because failures cluster: a slow exchange can bring timeouts, rate-limit answers and dropped connections together, so defects in those paths can fire at the same time.

4. Findings at a glance

5. Priority 1 findings

F01 API key and secret in the source; withdrawal permission demanded

Where: bot_a/config.py, lines 3 to 5, and Bot.start in bot_a/bot.py, lines 36 to 40.

API_KEY = "demo-key-not-a-real-secret"
API_SECRET = "demo-secret-not-a-real-secret"
REQUIRED_PERMISSIONS = {"read", "trade", "withdraw"}

What the run showed: the bot refuses to start with a key that can only read and trade, and starts once the key can withdraw. It never calls a withdrawal route.

== F01 [P1] API key and secret in the source; withdrawal permission demanded
  bot_a/config.py line 3: API_KEY = "demo-key-not-a-real-secret"
  bot_a/config.py line 4: API_SECRET = "demo-secret-not-a-real-secret"
  start with a read and trade key: refused: API key must also have: withdraw
  start with a key that can withdraw: started
  routes Bot A calls: 4, of which withdrawal routes: 0
  REPRODUCED

What can happen: anyone with a copy of the code, or of its repository history, can sign requests as Client A. Because the bot insists on withdrawal permission it never uses, a leaked key can move funds out of the account, not only trade. Deleting the lines does not help once they are in version history.

Fix: revoke the key on the exchange first, because changing a file does not revoke it. Create a new key with read and trade permissions only, withdrawal off, and IP restriction where the platform supports it. Remove withdraw from the required permissions. Keep secrets out of the source, in storage on the server that only the bot can read, and add secret scanning before commits.

F02 Order retry can place duplicate orders

Where: ExchangeClient.place_order in bot_a/exchange.py, lines 64 to 71, with MAX_ATTEMPTS = 5 and a timeout of 0.5 seconds in bot_a/config.py.

def place_order(self, symbol, side, qty, price):
    params = {"symbol": symbol, "side": side, "qty": qty, "price": price}
    for attempt in range(config.MAX_ATTEMPTS):
        try:
            return self._signed("POST", "/v1/order", params)
        except ExchangeError:
            time.sleep(config.RETRY_SLEEP_SECONDS)
    return None

What the run showed: when Exchange A takes an order but the answer never arrives, or arrives as a 503, the loop sends a new order each time. One decision became three orders in one scenario and five in two others, while the bot knew about one order or none. A plain refusal was sent five times.

== F02 [P1] Order retry can place duplicate orders
  one decision, and the answers to the first 2 POST attempts never arrive:
    POST attempts 3, orders at Exchange A 3, orders Bot A knows 1
  one decision, and no answer to any POST attempt:
    POST attempts 5, orders at Exchange A 5, orders Bot A knows 0
  one decision, and Exchange A takes each POST but answers 503:
    POST attempts 5, orders at Exchange A 5, orders Bot A knows 0
  one decision, and Exchange A refuses it:
    POST attempts 5, answers: 5 x 400 insufficient_balance
  client order ID on an order Bot A sends: None
  REPRODUCED

What can happen: on a slow exchange, one decision can become up to five live orders, and the bot can believe it holds none of them. A timeout or a 5xx answer does not say whether the order was placed, and the loop treats it as if it was not.

Fix: derive a client order ID once, save it with the order before the first send, and reuse it. After a timeout or a 5xx, record the order as unknown and read its status by that ID. Send again only after the platform confirms the order does not exist, and only on a platform that enforces unique client order IDs. Never retry a clear refusal. The order gateway in the same download shows one way to do this.

F03 Risk check fails open

Where: Bot.on_decision in bot_a/bot.py, lines 58 to 65.

def on_decision(self, decision):
    try:
        if not self.risk.allowed(decision):
            log.info("blocked by risk")
            return
    except Exception:
        log.debug("risk check error")
    self.submit(decision)

What the run showed: the limit blocks an oversized order while the balance call works. When the balance call answers 503, the same order goes out, and the only trace is a debug line.

== F03 [P1] Risk check fails open
  order with notional 900 against a limit of 100, balance call working:
    orders at Exchange A 0, log: INFO blocked by risk
  the same order, with the balance call answering 503:
    orders at Exchange A 1, log says only: DEBUG risk check error
  REPRODUCED

What can happen: the limits switch off in the conditions where they are needed most: an exchange under load, a rate-limit answer or a network fault.

Fix: fail closed. If a limit cannot be evaluated, block the order and alert someone. Move the check into the only path that reaches the exchange, as F11 describes.

F04 A restart forgets open orders and the position

Where: Bot.__init__ in bot_a/bot.py, lines 31 to 34. Nothing is written to disk, and nothing is read from Exchange A at start-up.

def __init__(self, exchange, risk):
    self.exchange, self.risk = exchange, risk
    self.position = 0.0
    self.open_orders = {}

What the run showed: a first process placed an order and was killed with os._exit, so it could not clean up. Exchange A filled part of the order while nothing was running. A second process started flat, never asked Exchange A what was open, and sent a new order that its own limit check allowed.

== F04 [P1] A restart forgets open orders and the position
  first process: order for 4 placed, killed with os._exit (exit code 70)
  Exchange A fills 3 of it while no process is running
  Exchange A now: position 3, open orders 1
  second process at start: position 0.0, open orders known 0
  calls to the open orders route, in either process: 0
  second process takes a decision for 4, and rules.entry_allowed says yes
  Exchange A now: position 3, open orders 2
  position if every open order fills: 8, against a limit of 5
  REPRODUCED

What can happen: after any crash and automatic restart, the bot can open a second position on top of the first, and orders it no longer knows about can still fill.

Fix: write each order to durable storage before sending it. At start-up, accept no new orders until open orders, fills and balances have been read from the exchange and compared with what was stored.

F05 Control endpoint has no authentication

Where: ControlHandler.do_GET in bot_a/control.py, lines 9 to 16. In this demo the endpoint listens on 127.0.0.1 only, which is a limit of the demo, so the run shows the missing check, not exposure to a network.

def do_GET(self):
    if self.path == "/close_all":
        self.server.bot.close_all_positions(self.server.symbol)
        self.send_response(200)
        self.end_headers()
        self.wfile.write(b"ok")
    else:
        self.send_error(404)

What the run showed: one plain GET, with no key, token or login, canceled the bot's open order and sent a new order to sell its position. Nothing in the bot's log records the call.

== F05 [P1] Control endpoint has no authentication
  before: Bot A position 2.0, open orders at Exchange A 1
  GET /close_all with no key, no token and no login: HTTP 200, body 'ok'
  after: buy order canceled, and new orders at Exchange A: SELL 2
  log lines that mention the call or who made it: 0
  REPRODUCED

What can happen: deployed on a reachable address, anyone who finds the port can trade the account with one request. Because it is a GET, a link preview, a crawler or a web page the operator opens can send it too.

Fix: require authentication, use POST for anything that changes state, log each call with who made it, and keep the endpoint on localhost or a private network. Remove it if nobody needs it.

F06 API key and signed requests written to logs

Where: ExchangeClient._signed in bot_a/exchange.py, line 52, with LOG_LEVEL = "DEBUG" in bot_a/config.py, line 7.

log.debug("%s %s headers=%s", method, target, headers)

What the run showed: one decision wrote the API key and a valid signature into the log twice. Replaying the logged POST, using nothing but the log file, created a second order. The script hides the timestamp and signature in its own output; the log file holds them in full.

== F06 [P1] API key and signed requests written to logs
  LOG_LEVEL = 'DEBUG'. One decision wrote 3 log lines
  lines holding the API key: 2, lines holding a valid signature: 2
  one logged request, with this script hiding the time and signature:
    POST /v1/order?symbol=DEMO-USD&side=BUY&qty=1&price=20
      &timestamp=<ms>&time_window_ms=60000&signature=<64 hex>
      headers={'X-API-KEY': 'demo-key-not-a-real-secret'}
  that POST sent again, using nothing but the log file: HTTP 200
  orders at Exchange A: 2, from one decision
  REPRODUCED

What can happen: anyone who can read the logs, including a log service they are shipped to, gets the key and can replay a logged order until its time window closes. F12 makes that window as wide as Exchange A allows. Fixing F01 does not close this leak, because a new key would be logged the same way.

Fix: mask keys and signatures in a logging filter, keep debug logging off in production, and add a test that searches the logs for secrets. Make this change before the new key from F01 goes into use, and treat any key that has appeared in a log as exposed.

6. Priority 2 findings

F07 Partial fills counted as full fills

Where: Bot.on_order_event in bot_a/bot.py, lines 75 to 81.

def on_order_event(self, event):
    order = self.open_orders[event["order_id"]]
    if event["status"] in ("PARTIALLY_FILLED", "FILLED"):
        qty = float(order["qty"])
        self.position += qty if order["side"] == "BUY" else -qty
    if event["status"] == "FILLED":
        del self.open_orders[event["order_id"]]

What the run showed: an order for 1 that filled in three parts, with one event delivered twice, left the bot believing it held 4. An event for an order the bot did not know raised KeyError and stopped the stream thread, so a later fill was never applied.

== F07 [P2] Partial fills counted as full fills
  order for 1, fill of 0.4: Bot A position 1.0, Exchange A 0.4
  same event delivered again: Bot A position 2.0, Exchange A 0.4
  fill of 0.3: Bot A position 3.0, Exchange A 0.7
  fill of 0.3, order complete: Bot A position 4.0, Exchange A 1
  event for an order Bot A does not know: KeyError, the stream thread stops
  the next fill: not applied. Bot A position 4.0, Exchange A 3
  REPRODUCED

What can happen: the position the bot reports, and every limit and closing order based on it, is wrong. An order placed by hand on the same account, or one the bot forgot after a restart (F04), silently stops all further event handling.

Fix: take the position from the cumulative filled quantity in each event, ignore an execution ID already applied, and treat an event for an unknown order as a reason to stop new orders and reconcile, not as a crash.

F08 A reconnect loses events

Where: AccountStream in bot_a/stream.py: on_close, lines 36 and 37, reconnects at once, and the subscription is sent only in start, line 17.

def on_close(self):
    self.connect()

What the run showed: after a dropped connection the bot tried again five times, less than half a second apart, never subscribed again, and missed both fills that happened after the drop. A connection that went quiet without closing was never noticed.

== F08 [P2] A reconnect loses events
  connection dropped, and Exchange A closes the next 4 connections at once
  attempts to reconnect: 5, longest gap between attempts under 0.5 s: True
  subscribe messages sent after the drop: 0
  fills at Exchange A after the drop: 2, events that reached Bot A: 0
  calls that could have fetched the gap, to the open orders route: 0
  connection stays open but goes quiet, and a fill happens
  events that reached Bot A: 0, reconnects: 0, errors: 0, thread alive: True
  REPRODUCED

What can happen: after a short outage the bot keeps running with no account events at all and no error, and an exchange outage can turn into a loop of connection attempts.

Fix: reconnect with exponential backoff and jitter, subscribe again on every connect, set a heartbeat deadline for quiet connections, and read open orders and fills over the REST API before allowing new orders.

F09 Rate-limit responses ignored

Where: Bot.run in bot_a/bot.py, lines 42 to 49, and ExchangeClient._call in bot_a/exchange.py, lines 30 to 44, which turns every answer other than 200 into the same error.

def run(self, symbol, polls):
    for _ in range(polls):
        try:
            self.exchange.ticker(symbol)
            self.exchange.balance()
        except ExchangeError as exc:
            log.debug("poll failed: %s", exc)
        time.sleep(config.POLL_SECONDS)

What the run showed: Exchange A answered 429 with Retry-After: 30. The bot kept polling, was banned, and then could not place an order to close a position.

== F09 [P2] Rate-limit responses ignored
  Exchange A answers 429 with Retry-After: 30
  requests Bot A sent during that wait: 7
  answers: 6 x 429, then 2 x 403 banned
  mentions of Retry-After in bot_a: 0
  order to close a position, after the ban: 5 POSTs, 5 x 403 banned
  orders at Exchange A: 0
  REPRODUCED

What can happen: a ban blocks every request from the bot's server, including the order that would reduce risk. Exchange A's ban rule is part of this fictional mock, but many platforms publish rate limits and penalties for ignoring them, so read your platform's documentation.

Fix: wait the time a 429 states, keep a request budget below the published limits, give order traffic priority over reads, and read market data from a stream where the platform offers one. Do not try to get around a limit with extra keys or IP addresses.

F10 Quantities use floats and fixed precision

Where: size_order in bot_a/rules.py, lines 10 and 11, with QTY_DECIMALS = 3, and the float position in Bot.

def size_order(notional, price):
    return round(notional / price, config.QTY_DECIMALS)

What the run showed: three decimals did not fit a step size of 0.01, so the order was refused, five times. Rounding to the nearest step rounded up, to a cost above the balance, so the bot's own check blocked the order and the decision was dropped. Two fills of 0.1 and 0.2 added up to 0.30000000000000004 in the bot, so its order to close the position was refused too.

== F10 [P2] Quantities use floats and fixed precision
  step size 0.01: size_order(100, 30) = 3.333
    answers: 5 x 400 qty_not_multiple_of_step
  calls to the symbol rules route: 0
  balance 10: size_order(10, 2.9997) = 3.334, which costs 10.0009998
    log: INFO blocked by risk, orders at Exchange A: 0
  full fills of 0.1 and 0.2
    Bot A position 0.30000000000000004, Exchange A 0.3
  order to close Bot A's position: 5 x 400 qty_not_multiple_of_step
  REPRODUCED

What can happen: orders for some symbols are always refused, decisions are dropped, and a position cannot be closed by the code that opened it.

Fix: use decimals for prices and quantities, read tick size, step size and minimum order value from the exchange, round quantities down to the step, and do not send an order below the minimum.

F11 Limits sit inside the rule path, and there is no emergency stop

Where: entry_allowed in bot_a/rules.py, lines 6 and 7, called only from Bot.enter. Bot.reverse, bot_a/bot.py lines 55 and 56, never calls it.

def entry_allowed(position, qty):
    return position + qty <= config.MAX_POSITION

What the run showed: the entry path blocked a buy over the limit, but the reversal path sent a sell of 16 and took the position to -12. The check also ignores side, so a further sell passed. The bot has no stop, and after close_all_positions the next decision still sends an order.

== F11 [P2] Limits sit inside the rule path, and there is no emergency stop
  limit 5, position 4: enter buy 2 sends 0 orders, so the entry path checks
  reverse with sell 16: sent. Position at Exchange A -12
  enter sell 2: rules.entry_allowed says yes. Position at Exchange A -14
  methods on Bot that could stop it: 0
  after close_all_positions, the next decision still sends 1 order
  REPRODUCED

What can happen: a position several times the intended limit, in either direction, and no way to stop new orders except killing the process, which leaves resting orders open on the exchange.

Fix: move limits out of the rules, into one gate in the only path to the exchange, checking the absolute position after the order on either side, with values set by Client A and no defaults. Add a stop that blocks new orders and works when the rest of the code does not. Client A decides what the stop does to open orders and positions.

7. Priority 3 and 4 findings

F12 Widest request window hides clock problems

Where: bot_a/config.py, line 9: TIME_WINDOW_MS = 60000 # stops the timestamp errors. Exchange A's default window is 5,000 ms and its largest is 60,000 ms.

What the run showed: with the clock 40 seconds off, orders were still accepted. The default window would have refused them. At 70 seconds off every order failed, and nothing in the log mentioned the clock or was logged as a warning or an error.

== F12 [P3] Widest request window hides clock problems
  clock 40000 ms off, Bot A's window of 60000 ms: orders at Exchange A 1
  same clock, default window of 5000 ms: 400 timestamp_outside_window
  calls to the server time route: 0
  log lines that mention the clock: 0
  clock 70000 ms off: POST answers: 5 x 400 timestamp_outside_window
  log lines that mention the clock: 0, warnings or errors: 0
  REPRODUCED

What can happen: the setting removes the error without removing the cause. A drifting clock goes unnoticed until it passes the widest window and every order fails, and a delayed request stays valid long after the decision that produced it.

Fix: run the host under time synchronization, measure the offset against the exchange's server time, alert when it grows, and use the default window or a smaller one.

F13 Failures are silent, and decisions cannot be traced

Where: throughout bot_a. Errors are logged at debug level or not at all, no ID links a decision to its order, and there is no alert code.

What the run showed: three decisions, one normal, one with the balance call failing and one with every POST answered 503, produced no warning and no error. No log line names a decision, and no order carries anything that links it to one.

== F13 [P3] Failures are silent, and decisions cannot be traced
  3 decisions: one normal, one with the balance call failing,
  one with every POST answered 503
  log lines by level: DEBUG 11, INFO 2, WARNING 0, ERROR 0
  log lines that name a decision: 0
  orders at Exchange A: 2, with anything linking them to a decision: 0
  mentions of alert, notify, webhook or smtp in bot_a: 0
  REPRODUCED

What can happen: after an incident nobody can say why an order went out or what the exchange answered, and a bot that has stopped working can go unnoticed for as long as nobody looks.

Fix: write structured logs with UTC timestamps and one ID carried from decision to order to fill, log failures as warnings or errors, and send alerts on repeated errors, restarts and a missing heartbeat.

F14 No tests

Where: the bot_a folder has no test files.

== F14 [P4] No tests
  Python files in bot_a: 7, lines: 271, test files: 0
  REPRODUCED

What can happen: every change, including the fixes above, gets tried on a live account first.

Fix: add tests against a mock that fails on demand, starting with the cases above. reproduce.py is not that test suite: it exits with 1 once the defects are fixed, by design.

8. What works

A report that lists only faults is hard to trust. These two checks passed in the same run.

== C1 The normal path works
  one decision, no failures: 1 POST, 1 order
  side SELL, quantity 2, price 30, status OPEN
  PASSED

== C2 Request signing follows Exchange A's scheme
  signed requests from Bot A: 2, answers: 2 x 200
  one signed request with its quantity changed: 401 bad_signature
  PASSED

9. Suggested order of repair

1. First: revoke the key and remove the withdrawal permission (F01), turn off debug logging, and replace any key that has appeared in a log (F06).
2. Before the next live run: F02, F03, F04 and F05.
3. Before raising any limit: F07 to F11.
4. Next: F12 and F13, then F14.

Fixes are quoted separately if you want Fincode to make them, and they follow the same written scope and test stages as any other project. You can also take this report to another developer. Cost and dates for repairs: [in a separate written proposal].

10. What this review does not cover

• Whether the rules will make money.
• Advice on strategies or parameters. Those decisions are Client A's.
• Running the bot or watching positions.
• The security of the server and account credentials, beyond what the code shows.
• The internals of third-party libraries, and any penetration test.
• A security certificate. This report states findings, and it does not certify the software.

Sample of the format for a fictional bot. Client A, Bot A and Exchange A do not exist, Bot A was written with defects on purpose, and the mock is not a market. Fixing every finding would make the software behave more predictably, and it would say nothing about future results. Trades can lose money. Fincode is a software company and does not provide investment advice, portfolio management or crypto asset services.