mirror of
https://github.com/GMWalletApp/epusdt.git
synced 2026-07-07 02:06:16 +00:00
refactor: harden sqlite runtime and packaged startup flow
- refine config loading with --config and current-directory .env - improve sqlite busy handling and runtime DB tuning - restore telegram payment notification and wallet input flow - add behavior-based config and callback recovery tests
This commit is contained in:
alphago9
@@ -0,0 +1,208 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user