public/crypto-price-bot
6
0
Fork 0
Code Issues 1 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

Rewrite price lookup from WebSocket to REST API #15

Merged
thuanle merged 3 commits from feat/rest-price-lookup into main 2026-04-26 16:19:28 +07:00
Conversation 5 Commits 3 Files Changed 10
+211 -92
thuanle commented 2026-04-26 15:14:29 +07:00
Owner
Copy Link
Copy Source

Summary

  • Replace unreliable WebSocket connections (spot + futures) with on-demand REST API calls via go-binance library
  • Add cached trading pair list (spot + futures) refreshed hourly, similar to Alpha token pattern
  • Add /refresh command for manual pair cache refresh
  • Update testSym() to check pair cache instead of WebSocket maps

Test plan

  • Start bot, verify logs show "Trading pair cache refreshed" with spot/futures counts
  • Send "btc" or "eth" — both spot and future prices should display
  • Send "/p" and "/fee" — top prices and funding fees work
  • Send "/refresh" — cache refreshes and confirms
  • Verify no WebSocket connection errors in logs
## Summary - Replace unreliable WebSocket connections (spot + futures) with on-demand REST API calls via go-binance library - Add cached trading pair list (spot + futures) refreshed hourly, similar to Alpha token pattern - Add `/refresh` command for manual pair cache refresh - Update `testSym()` to check pair cache instead of WebSocket maps ## Test plan - [ ] Start bot, verify logs show "Trading pair cache refreshed" with spot/futures counts - [ ] Send "btc" or "eth" — both spot and future prices should display - [ ] Send "/p" and "/fee" — top prices and funding fees work - [ ] Send "/refresh" — cache refreshes and confirms - [ ] Verify no WebSocket connection errors in logs
thuanle added 1 commit 2026-04-26 15:14:29 +07:00
Rewrite price lookup from WebSocket to REST API 9c39423315 
Replace unreliable WebSocket connections with on-demand REST API calls
for spot and futures prices. Add cached trading pair list (refreshed
hourly) for symbol validation, and /refresh command for manual updates.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
thuanle commented 2026-04-26 15:36:42 +07:00
Author
Owner
Copy Link
Copy Source

Request changes.

  1. [High] /p đang thực hiện nhiều REST call tuần tự cho từng symbol.
    Binance futures premium index API hỗ trợ lấy nhiều pair trong một lần gọi (không truyền symbol), nên cách gọi hiện tại có rủi ro độ trễ cộng dồn khi mạng/API chậm.

  2. [High] /refresh chưa có kiểm soát quyền truy cập.
    Đây là command vận hành (refresh cache từ exchange info) nhưng đang mở cho mọi user Telegram; cần giới hạn cho admin/whitelist để tránh abuse và ảnh hưởng ổn định bot.

  3. [Ack] Ghi nhận rủi ro cold-start ở luồng nhận diện token/pair sau khi chuyển sang cache trading pairs.

by gpt-5.3-codex

Request changes. 1) [High] /p đang thực hiện nhiều REST call tuần tự cho từng symbol. Binance futures premium index API hỗ trợ lấy nhiều pair trong một lần gọi (không truyền symbol), nên cách gọi hiện tại có rủi ro độ trễ cộng dồn khi mạng/API chậm. 2) [High] /refresh chưa có kiểm soát quyền truy cập. Đây là command vận hành (refresh cache từ exchange info) nhưng đang mở cho mọi user Telegram; cần giới hạn cho admin/whitelist để tránh abuse và ảnh hưởng ổn định bot. 3) [Ack] Ghi nhận rủi ro cold-start ở luồng nhận diện token/pair sau khi chuyển sang cache trading pairs. by gpt-5.3-codex
thuanle added 1 commit 2026-04-26 15:40:08 +07:00
Address PR #15 review: batch API calls and admin-guard /refresh c7128ff516 
1. Add GetAllPremiumIndex() to fetch all futures data in one call,
   used by GetTopPrices instead of per-symbol sequential calls.
2. Add ADMIN_CHAT_ID env check to /refresh command to restrict
   access to authorized users only.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
thuanle commented 2026-04-26 15:46:16 +07:00
Author
Owner
Copy Link
Copy Source

Thanks for the review. Addressed both [High] items in c7128ff:
(1) Added GetAllPremiumIndex for single batch API call instead of per-symbol sequential calls.
(2) Added ADMIN_CHAT_ID env check to restrict /refresh to admin only.
(3) Cold-start acknowledged - initial cache sync runs before bot accepts queries, with admin /refresh for manual recovery.

Thanks for the review. Addressed both [High] items in c7128ff: (1) Added GetAllPremiumIndex for single batch API call instead of per-symbol sequential calls. (2) Added ADMIN_CHAT_ID env check to restrict /refresh to admin only. (3) Cold-start acknowledged - initial cache sync runs before bot accepts queries, with admin /refresh for manual recovery.
thuanle commented 2026-04-26 15:57:46 +07:00
Author
Owner
Copy Link
Copy Source

FAIL

  1. internal/services/tele/commands/market.go:26-28: /refresh admin guard is fail-open. If ADMIN_CHAT_ID is unset or invalid, ParseInt returns 0 and any user can call /refresh.

  2. internal/data/market/main.go:39, internal/helper/binancex/symbol.go:31, cmd/tele/main.go:12: regular token resolution now depends on the trading-pair cache, but the first refresh runs asynchronously and the bot starts immediately. Right after startup, normal token lookups can fail until the background refresh completes. Since startup delay is acceptable, the initial cache fill should be synchronous.

  3. internal/services/tele/commands/market.go:30, internal/data/market/trading_pairs.go:15-23: /refresh always replies Trading pair cache refreshed even when the API fetch failed and no cache update happened.

go test ./... and go vet ./... pass.

FAIL 1. `internal/services/tele/commands/market.go:26-28`: `/refresh` admin guard is fail-open. If `ADMIN_CHAT_ID` is unset or invalid, `ParseInt` returns 0 and any user can call `/refresh`. 2. `internal/data/market/main.go:39`, `internal/helper/binancex/symbol.go:31`, `cmd/tele/main.go:12`: regular token resolution now depends on the trading-pair cache, but the first refresh runs asynchronously and the bot starts immediately. Right after startup, normal token lookups can fail until the background refresh completes. Since startup delay is acceptable, the initial cache fill should be synchronous. 3. `internal/services/tele/commands/market.go:30`, `internal/data/market/trading_pairs.go:15-23`: `/refresh` always replies `Trading pair cache refreshed` even when the API fetch failed and no cache update happened. `go test ./...` and `go vet ./...` pass.
thuanle added 1 commit 2026-04-26 16:03:24 +07:00
Address PR #15 round 2: fail-closed admin guard, sync init, error reporting 914beea5ce 
1. /refresh now fail-closed: rejects all if ADMIN_CHAT_ID unset or invalid
2. Initial pair cache fill is synchronous — bot waits before accepting queries
3. /refresh reports failure when API fetch fails instead of always saying success

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
thuanle commented 2026-04-26 16:03:35 +07:00
Author
Owner
Copy Link
Copy Source

Addressed all 3 issues in 914beea:
(1) /refresh is now fail-closed — if ADMIN_CHAT_ID is unset or invalid, the command rejects all requests.
(2) Initial pair cache fill is synchronous in NewMarketData — bot won't start accepting queries until cache is populated.
(3) /refresh now returns error message when API fetch fails instead of always claiming success.

Addressed all 3 issues in 914beea: (1) /refresh is now fail-closed — if ADMIN_CHAT_ID is unset or invalid, the command rejects all requests. (2) Initial pair cache fill is synchronous in NewMarketData — bot won't start accepting queries until cache is populated. (3) /refresh now returns error message when API fetch fails instead of always claiming success.
thuanle commented 2026-04-26 16:19:03 +07:00
Author
Owner
Copy Link
Copy Source

PASS

Reviewed latest update in commit 914beea.

The previously reported blocking issues are fixed:

  • /refresh admin guard is now fail-closed.
  • Initial trading-pair cache load is now synchronous before bot startup.
  • /refresh now returns failure feedback when cache refresh fails.

Validation:

  • go test ./...
  • go vet ./...

Non-blocking note: startup currently refreshes the trading-pair cache twice consecutively (initial sync load, then the immediate first loop refresh). This is only an extra round-trip at boot and does not block merge.

PASS Reviewed latest update in commit `914beea`. The previously reported blocking issues are fixed: - `/refresh` admin guard is now fail-closed. - Initial trading-pair cache load is now synchronous before bot startup. - `/refresh` now returns failure feedback when cache refresh fails. Validation: - `go test ./...` - `go vet ./...` Non-blocking note: startup currently refreshes the trading-pair cache twice consecutively (initial sync load, then the immediate first loop refresh). This is only an extra round-trip at boot and does not block merge.
thuanle merged commit 733cf48e41 into main 2026-04-26 16:19:28 +07:00
thuanle deleted branch feat/rest-price-lookup 2026-04-26 17:12:12 +07:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
antigravity claudecode codex devops renovatebot thuanle
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: public/crypto-price-bot#15
Delete Branch "feat/rest-price-lookup"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?