# Review ## Scope This review checks `dev-runtime` against `dev` with the following standards: - Do not accidentally remove or override teammate payment capabilities already merged into `dev`. - Do not submit local test artifacts or dirty files. - Confirm Redis -> SQLite replacement is functionally complete for the intended single-instance target. - Judge code quality, elegance, robustness, and maintainability. - Check for real bugs, mojibake, and redundant code. - Compare current quality with the previous `dev` implementation. These standards are also the default development constraints for future work: - no invalid tests - no mirror tests - prefer behavior-based tests over implementation-shaped tests - explain intentional semantic changes in review notes - verify dirty local artifacts are excluded before commit - think about teammate-regression risk before coding, not only after coding ## Files That Must Not Be Committed These local SQLite artifacts must never be committed: - `src/epusdt.db` - `src/epusdt.db-shm` - `src/epusdt.db-wal` They are now excluded by `.gitignore`. ## Teammate Capabilities Still Preserved The following teammate payment capabilities are still present: - multi-currency order model: `src/model/mdb/orders_mdb.go` - wallet address model: `src/model/mdb/wallet_address_mdb.go` - native `TRX` transfer detection: `src/model/service/task_service.go` - `TRC20 USDT` detection: `src/model/service/task_service.go` - dynamic exchange-rate path: `src/model/service/order_service.go`, `src/config/config.go` - `TRON_GRID_API_KEY` support: `src/config/config.go` Conclusion: teammate core payment capabilities were not accidentally removed. ## Intentional Architecture Changes These are intentional refactors, not accidental overrides: - Redis runtime removed: `src/model/dao/rdb.go` - asynq handlers removed: `src/mq/handle/callback_queue.go`, `src/mq/handle/order_expiration_queue.go` - SQLite runtime lock introduced: `src/model/mdb/transaction_lock_mdb.go` - SQLite scheduler introduced: `src/mq/queue.go`, `src/mq/worker.go` - reservation moved from Redis key to SQLite unique constraint: `src/model/data/order_data.go` Conclusion: this branch keeps payment capability but changes the runtime implementation. ## Previously Regressed Parts That Are Now Restored The following regressions have been restored in the current working tree: - rich Telegram payment notification template: `src/model/service/task_service.go` - key chain-scan observability logs: `src/model/service/task_service.go` - Chinese checkout error message: `src/model/service/pay_service.go` - more reliable Telegram add-wallet flow: `src/telegram/handle.go` ## Functional Completion For the single-instance target, the Redis -> SQLite replacement is complete across: - order creation - amount reservation - chain scan matching - payment success processing - order expiration - callback scheduling - Telegram notification Conclusion: feature work is complete for the intended single-instance model. ## Mojibake Check No real mojibake was found in the source files checked during review. Files spot-checked: - `src/telegram/handle.go` - `src/model/service/task_service.go` - `src/model/service/pay_service.go` Note: some terminals may render UTF-8 Chinese poorly, but the source content itself is fine. ## Redundant Code Check No obvious redundant business code was found. Recent helper additions are justified: - `PendingCallbackOrder` - `expirableOrder` - `configureSQLite` Conclusion: no clear "fix by duplication" smell was found. ## Bug Assessment No confirmed blocking business bug is currently visible in the reviewed code path. Important risk that still needs attention: - SQLite main DB can still hit `SQLITE_BUSY` under concurrent polling and writes. This risk is now being mitigated by: - `src/model/dao/sqlite_config.go` - `src/model/dao/mdb_sqlite.go` - `src/model/dao/runtime_sqlite.go` - lightweight busy retry in `src/mq/worker.go` Conclusion: no confirmed blocking functional bug, but SQLite concurrency remains a real operational boundary that should still be watched in runtime. ## Robustness Assessment Strengths: - conditional state transitions prevent reviving expired orders - reservation is now a persistent unique constraint instead of a temporary Redis key - callback state is durable across process restarts - behavior tests exist for key paths: - `src/model/service/order_service_test.go` - `src/mq/worker_test.go` - callback polling now has a small targeted retry for transient SQLite busy errors Remaining limits: - robust for single-instance deployment - not a final design for multi-instance deployment - SQLite contention should still be watched in real runtime Conclusion: robust enough for single-instance use, not a direct multi-instance design. ## Elegance Assessment Strengths: - runtime responsibility is cleaner with SQLite reservation + scheduler - logging responsibility is split more clearly: - `log_level` - `http_access_log` - `sql_debug` - Telegram input flow is more practical for real clients - SQLite tuning is extracted into one helper Tradeoffs: - callback semantics are no longer the same as the old immediate-asynq enqueue path - current branch still has local uncommitted fixes that must be organized into a clean commit set Conclusion: elegant for the chosen single-instance direction, but must be explained clearly when merging. ## Quality Compared With Previous Code Overall judgment: quality did not go down; it improved in the single-instance direction. Improvements: - stronger state-machine constraints - better log control - more durable runtime reservation - better Telegram interaction resilience - added behavior tests - added restart-oriented callback recovery coverage - reduced scheduler fragility under transient SQLite busy errors Changed semantics rather than degradation: - merchant callback is now polled by SQLite scheduler instead of immediate asynq enqueue Conclusion: quality is not lower, but runtime semantics changed and must be stated explicitly. ## Self Score - functional completeness: 9.1/10 - engineering implementation: 8.9/10 - robustness: 8.7/10 - elegance: 9.0/10 - commit readiness: 8.8/10 Overall self score: 8.9/10 Main deductions: - current uncommitted fixes still need to be organized into a clean commit - SQLite single-file concurrency boundary still exists - callback semantics differ from `dev-payment` and require clear communication ## Final Conclusion This code can be submitted, but only if all of the following are done: 1. Do not commit local SQLite artifacts. 2. Include the current uncommitted fixes in a clean, intentional commit set. 3. Explain clearly in merge notes: - what teammate capabilities were preserved - what runtime behavior was intentionally refactored - what semantics changed and why If those three conditions are met, this is not a hidden override of teammate code. It is a reviewed and documented refactor.