support Binance paper trading for sync sub-command #1605
Conversation
lanphan
commented
Mar 26, 2024
lanphan
commented
- Enable Paper trading in sync command
- Refactor pkg/util
- take advantage of adshao/go-binance/v2 in order to set URL for paper trading
- Sync for Binance paper trading works now
- Run with Binance paper trading works with user data stream synced
|
Welcome back! @lanphan, This pull request may get 267 BBG. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1605 +/- ##
==========================================
- Coverage 22.63% 22.61% -0.02%
==========================================
Files 621 621
Lines 44903 44913 +10
==========================================
- Hits 10164 10159 -5
- Misses 34009 34022 +13
- Partials 730 732 +2
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
Re-estimated karma: this pull request may get 283 BBG |
pkg/exchange/binance/exchange.go
Outdated
| const BinanceUSWebSocketURL = "wss://stream.binance.us:9443" | ||
| const WebSocketURL = "wss://stream.binance.com:9443" | ||
| const WebSocketTestURL = "wss://testnet.binance.vision" | ||
| const FutureTestBaseURL = "https://testnet.binancefuture.com" |
There was a problem hiding this comment.
these are still required because we want to replace adshao/binance with the requestgen api
There was a problem hiding this comment.
all constants containing test url are kept, done
| if paperTrade() { | ||
| client.BaseURL = BinanceTestBaseURL | ||
| futuresClient.BaseURL = FutureTestBaseURL | ||
| } |
There was a problem hiding this comment.
no need
my new code is added before create new binance client as followings
if util.IsPaperTrade() {
binance.UseTestnet = true
}
var client = binance.NewClient(key, secret)
it'll help to set BaseURL of binance client to its test base url. I did run it and it works.
pkg/service/sync.go
Outdated
| @@ -88,6 +89,10 @@ func (s *SyncService) SyncRewardHistory(ctx context.Context, exchange types.Exch | |||
| } | |||
|
|
|||
| log.Infof("syncing %s reward records...", exchange.Name()) | |||
| if util.IsPaperTrade() { | |||
| log.Info("Reward is not supported in paper trading") | |||
There was a problem hiding this comment.
such check is done by checking the interface, you can check pkg/types/exchange.go
There was a problem hiding this comment.
there is no check there
In fact, when I don't have that code, sync progress is corrupted with error below
[0001] INFO syncing binance reward records...
Error: request failed with status code: 404, body: ""
[0002] FATAL cannot execute command error=request failed with status code: 404, body: ""
There was a problem hiding this comment.
inside the QueryRewardHistory method, return empty slice for paper trading? and actually you can turn off the reward history sync ;-)
There was a problem hiding this comment.
inside the QueryRewardHistory method --> there is no method with name QueryRewardHistory
and actually you can turn off the reward history sync --> you mean there is env to on/off this? ah, I found it, rewardHistory: true
At last, I think this code is ok at this time, just because it has the same pattern with deposit / withdrawn methods
|
Re-estimated karma: this pull request may get 364 BBG |
|
Re-estimated karma: this pull request may get 363 BBG |
|
Re-estimated karma: this pull request may get 378 BBG |
pkg/exchange/binance/stream.go
Outdated
| @@ -304,7 +304,7 @@ func (s *Stream) getEndpointUrl(listenKey string) string { | |||
|
|
|||
| if s.IsFutures { | |||
| url = FuturesWebSocketURL + "/ws" | |||
| } else if isBinanceUs() { | |||
| } else if util.IsBinanceUs() { | |||
There was a problem hiding this comment.
isBinanceUs should only be used inside the binance package, only common functions could be moved into the util package
There was a problem hiding this comment.
ok, moved back, done
pkg/service/sync.go
Outdated
| @@ -109,6 +119,11 @@ func (s *SyncService) SyncDepositHistory(ctx context.Context, exchange types.Exc | |||
|
|
|||
| func (s *SyncService) SyncWithdrawHistory(ctx context.Context, exchange types.Exchange, startTime time.Time) error { | |||
| log.Infof("syncing %s withdraw records...", exchange.Name()) | |||
| if util.IsPaperTrade() { | |||
| log.Info("Withdraw is not supported in paper trading") | |||
There was a problem hiding this comment.
log messages start with lower case :-)
pkg/service/sync.go
Outdated
| @@ -97,6 +102,11 @@ func (s *SyncService) SyncRewardHistory(ctx context.Context, exchange types.Exch | |||
|
|
|||
| func (s *SyncService) SyncDepositHistory(ctx context.Context, exchange types.Exchange, startTime time.Time) error { | |||
| log.Infof("syncing %s deposit records...", exchange.Name()) | |||
| if util.IsPaperTrade() { | |||
| log.Info("Deposit is not supported in paper trading") | |||
There was a problem hiding this comment.
ditto, log messages start with lower case
|
Re-estimated karma: this pull request may get 443 BBG |
|
Hi @lanphan, Well done! 458 BBG has been sent to your polygon wallet. Please check the following tx: https://polygonscan.com/tx/0x898a7a144065e12bacbf304b2e0f62707123e97a57a7a0230cc288c2f77c6ba0 Thank you for your contribution! |
