New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bugfixes: Backtester credentials, Binance ExecutionLimits, Mock recording #1539
bugfixes: Backtester credentials, Binance ExecutionLimits, Mock recording #1539
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1539 +/- ##
==========================================
- Coverage 36.03% 35.91% -0.13%
==========================================
Files 412 409 -3
Lines 177779 177453 -326
==========================================
- Hits 64060 63729 -331
- Misses 105868 105871 +3
- Partials 7851 7853 +2
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested. ACK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
backtester/engine/setup.go
Outdated
base.API.AuthenticatedSupport = validated | ||
if !validated { | ||
return fmt.Errorf("%v %w", base.GetName(), errInvalidCredentials) | ||
creds, err := base.GetCredentials(context.TODO()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
creds, err := base.GetCredentials(context.TODO()) | |
_, err := base.GetCredentials(context.TODO()) | |
if err != nil { | |
return err | |
} |
And can drop the VerifyAPICredentials below since in this context GetCredentials
will check them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making those changes!
PR Description
David on Slack has been informing us of issues with the Backtester. This fix uses up-to-date credential checking functions instead of returning an error.
Before I could fix that I found Binance changed the json for SPOT/MARGIN execution limits to use [][]permissionSets
Upon fixing that I went to update our json mock data, and that showed that #1444 introduced a compatibility change with recording to the new path
I also came across duplicate errors right next to eachother and have addressed that
To test the Backtester
/backtester/config/strategyexamples/dca-candles-live.strat
real-orders
totrue
maximum-total
to a tiny figure/backtester
executego run .
/backtester/btcli/
executego run . executestrategyfromfile -p="config/strategyexamples/dca-candles-live.strat"
Fixes # (issue)
How has this been tested