Skip to content
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

WIP add: Calculate fees on backtest #282

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ c.symbols = [
symbol: 'ETHUSDT',
exchange: 'binance_futures',
periods: ['1m', '15m', '1h'],
feesPerTrade: 0.04,
trade: {
currency_capital: 10,
strategies: [
Expand Down Expand Up @@ -369,6 +370,7 @@ Per pair you can set used margin before orders are created; depending on exchang
c.symbols.push({
'symbol': 'BTCUSD',
'exchange': 'bitmex',
'feesPerTrade': 0.05,
'extra': {
'bitmex_leverage': 5,
},
Expand All @@ -377,12 +379,24 @@ Per pair you can set used margin before orders are created; depending on exchang
c.symbols.push({
'symbol': 'EOSUSD',
'exchange': 'bybit',
'feesPerTrade': 0.075,
'extra': {
'bybit_leverage': 5,
},
})
```

### Fees

Fees can be configured on `instance.js` file including the field `feesPerTrade`, it is important to
mention that prices of fees are for Taker, not for Maker

For example:

Binance futures: has a Maker/Taker 0.0200%/0.0400%.

'feesPerTrade': 0.04,

## Tools

### Fill data
Expand Down
4 changes: 4 additions & 0 deletions instance.js.dist
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ c.init = () => {
'periods': ['1m', '15m', '1h'],
'exchange': 'binance',
'state': 'watch',
'feesPerTrade': 0.04,
'watchdogs': [
{
'name': 'stoploss_watch',
Expand Down Expand Up @@ -144,6 +145,7 @@ y.forEach((pair) => {
'symbol': pair,
'periods': ['1m', '15m', '1h'],
'exchange': 'bitmex',
'feesPerTrade': 0.1,
'state': 'watch',
'extra': {
'bitmex_leverage': 5,
Expand Down Expand Up @@ -193,6 +195,7 @@ l.forEach((pair) => {
'symbol': pair,
'periods': ['1m', '15m', '1h'],
'exchange': 'bitmex_testnet',
'feesPerTrade': 0.1,
'state': 'watch',
'watchdogs': [
{
Expand Down Expand Up @@ -235,6 +238,7 @@ z.forEach((pair) => {
'symbol': pair,
'periods': ['1m', '15m', '1h'],
'exchange': 'binance',
'feesPerTrade': 0.1,
'state': 'watch',
'strategies': [
{
Expand Down
25 changes: 25 additions & 0 deletions src/command/backtest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
const services = require('../modules/services');

process.on('message', async msg => {
const p = msg.pair.split('.');

services.boot(msg.projectDir);

const results = await services
.getBacktest()
.getBacktestResult(
msg.tickIntervalInMinutes,
msg.hours,
msg.strategy,
msg.candlePeriod,
p[0],
p[1],
msg.options,
msg.initialCapital,
msg.projectDir
);

process.send({
results: results
});
});
64 changes: 43 additions & 21 deletions src/modules/backtest.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,20 @@ module.exports = class Backtest {
});
}

getBacktestResult(tickIntervalInMinutes, hours, strategy, candlePeriod, exchange, pair, options, initial_capital) {
getBacktestResult(
tickIntervalInMinutes,
hours,
strategy,
candlePeriod,
exchange,
pair,
options,
initialCapital,
projectDir
) {
if (projectDir) {
this.projectDir = projectDir;
}
return new Promise(async resolve => {
const start = moment()
.startOf('hour')
Expand Down Expand Up @@ -186,7 +199,10 @@ module.exports = class Backtest {
};
});

const backtestSummary = await this.getBacktestSummary(signals, initial_capital);
const instance = this.instances.symbols.filter(i => i.symbol === pair && i.exchange === exchange)[0];
const fees = instance.feesPerTrade === undefined ? 0 : instance.feesPerTrade;

const backtestSummary = await this.getBacktestSummary(signals, initialCapital, fees);
resolve({
summary: backtestSummary,
rows: rows.slice().reverse(),
Expand All @@ -205,10 +221,10 @@ module.exports = class Backtest {
});
}

getBacktestSummary(signals, initial_capital) {
return new Promise(async resolve => {
const initialCapital = Number(initial_capital); // 1000 $ Initial Capital
let workingCapital = initialCapital; // Capital that changes after every trade
getBacktestSummary(signals, initialCapital, feesPerTrade) {
return new Promise(resolve => {
const initialCapitalNumber = Number(initialCapital); // 1000 $ Initial Capital
let workingCapital = initialCapitalNumber; // Capital that changes after every trade

let lastPosition; // Holds Info about last action

Expand All @@ -222,47 +238,52 @@ module.exports = class Backtest {
};

let cumulativePNLPercent = 0; // Sum of all the PNL Percentages
let cumulativeNetFees = 0;
const pnlRateArray = []; // Array of all PNL Percentages of all the trades

// Iterate over all the signals
for (let s = 0; s < signals.length; s++) {
const signalObject = signals[s];
const signalType = signalObject.result._signal; // Can be long,short,close
const signalType = signalObject.result.getSignal(); // Can be long,short,close

// When a trade is closed
if (signalType == 'close') {
if (signalType === 'close') {
// Increment the total trades counter
trades.total += 1;

// Entry Position Details
const entrySignalType = lastPosition.result._signal; // Long or Short
const entryPrice = lastPosition.price; // Price during the trade entry
const tradedQuantity = Number((workingCapital / entryPrice)); // Quantity

const tradedQuantity = Number(workingCapital / entryPrice); // Quantity
const entrySignalType = lastPosition.result.getSignal(); // Long or Short
const entryValue = Number((tradedQuantity * entryPrice).toFixed(2)); // Price * Quantity
const entryFee = (entryValue * feesPerTrade) / 100;
// Exit Details
const exitPrice = signalObject.price; // Price during trade exit
const exitValue = Number((tradedQuantity * exitPrice).toFixed(2)); // Price * Quantity
const exitFee = (exitValue * feesPerTrade) / 100;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If those are fees per Trade, then why is it multiplied with the exit Value?
Shouldn't it be named relativeFees or something then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is my understanding that the fees are paid when we buy and then when we sell depending on each value, I took the info from: https://www.binance.com/en/support/faq/360033544231
I'm thinking that this code doesn't consider if for example the trade starts as a limit order and ends as a market one, simply applies a taker fee to the whole process. I wonder if the UI backtesting would need to be updated to include those options


const totalFee = entryFee + exitFee;
cumulativeNetFees += totalFee;

// Trade Details
let pnlValue = 0; // Profit or Loss Value

// When the position is Long
if (entrySignalType == 'long') {
if (exitPrice > entryPrice) {
if (entrySignalType === 'long') {
if (exitValue - totalFee > entryValue) {
// Long Trade is Profitable
trades.profitableCount += 1;
}

// Set the PNL
pnlValue = exitValue - workingCapital;
} else if (entrySignalType == 'short') {
if (exitPrice < entryPrice) {
pnlValue = exitValue - totalFee - workingCapital;
} else if (entrySignalType === 'short') {
if (exitValue - totalFee < entryValue) {
// Short Trade is Profitable
trades.profitableCount += 1;
}

// Set the PNL
pnlValue = -(exitValue - workingCapital);
pnlValue = -(exitValue - totalFee - workingCapital);
}

// Percentage Return
Expand All @@ -276,7 +297,7 @@ module.exports = class Backtest {

// Update Working Cap
workingCapital += pnlValue;
} else if (signalType == 'long' || signalType == 'short') {
} else if (signalType === 'long' || signalType === 'short') {
// Enter into a position
lastPosition = signalObject;
}
Expand Down Expand Up @@ -309,15 +330,16 @@ module.exports = class Backtest {
// -- End of Sharpe Ratio Calculation

// Net Profit
const netProfit = Number((((workingCapital - initialCapital) / initialCapital) * 100).toFixed(2));
const netProfit = Number((((workingCapital - initialCapitalNumber) / initialCapitalNumber) * 100).toFixed(2));

trades.profitabilityPercent = Number(((trades.profitableCount * 100) / trades.total).toFixed(2));

const summary = {
sharpeRatio: sharpeRatio,
averagePNLPercent: averagePNLPercent,
netProfit: netProfit,
initialCapital: initialCapital,
netFees: cumulativeNetFees,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

netFees would imply there to also to be grossFees

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, I'm not familiar with the financial term, am I missing more information to be shown in the screen? Thanks!

initialCapital: initialCapitalNumber,
finalCapital: Number(workingCapital.toFixed(2)),
trades: trades
};
Expand Down
71 changes: 52 additions & 19 deletions src/modules/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@ const auth = require('basic-auth');
const cookieParser = require('cookie-parser');
const crypto = require('crypto');
const moment = require('moment');
const { fork } = require('child_process');
const OrderUtil = require('../utils/order_util');

const backtestPendingPairs = {};
const backtestResults = {};

module.exports = class Http {
constructor(
systemUtil,
Expand Down Expand Up @@ -92,7 +96,13 @@ module.exports = class Http {
strict_variables: false
});

app.use(express.urlencoded({ limit: '12mb', extended: true, parameterLimit: 50000 }));
app.use(
express.urlencoded({
limit: '12mb',
extended: true,
parameterLimit: 50000
})
);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a big fan of refactoring code if it doesn't have anything to do with the feature.
Please create another pull request for this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be the prettier plugin

app.use(cookieParser());
app.use(compression());
app.use(express.static(`${this.projectDir}/web/static`, { maxAge: 3600000 * 24 }));
Expand Down Expand Up @@ -136,29 +146,52 @@ module.exports = class Http {
pairs = [pairs];
}

const asyncs = pairs.map(pair => {
return async () => {
const p = pair.split('.');
const key = moment().unix();

backtestPendingPairs[key] = [];
backtestResults[key] = [];

pairs.forEach(pair => {
backtestPendingPairs[key].push(pair);

const forked = fork('src/command/backtest.js');

return {
forked.send({
pair,
tickIntervalInMinutes: parseInt(req.body.ticker_interval, 10),
hours: req.body.hours,
strategy: req.body.strategy,
candlePeriod: req.body.candle_period,
options: req.body.options ? JSON.parse(req.body.options) : {},
initialCapital: req.body.initial_capital,
projectDir: this.projectDir
});

forked.on('message', msg => {
backtestPendingPairs[key].splice(backtestPendingPairs[key].indexOf(pair), 1);
backtestResults[key].push({
pair: pair,
result: await this.backtest.getBacktestResult(
parseInt(req.body.ticker_interval, 10),
req.body.hours,
req.body.strategy,
req.body.candle_period,
p[0],
p[1],
req.body.options ? JSON.parse(req.body.options) : {},
req.body.initial_capital
)
};
};
result: msg.results
});
});
});

const backtests = await Promise.all(asyncs.map(fn => fn()));
res.render('../templates/backtest-pending-results.html.twig', {
key: key
});
});

app.get('/backtest/:backtestKey', async (req, res) => {
res.send({
ready:
backtestPendingPairs[req.params.backtestKey] === undefined
? false
: backtestPendingPairs[req.params.backtestKey].length === 0
});
});

// single details view
app.get('/backtest/result/:backtestKey', (req, res) => {
const backtests = backtestResults[req.params.backtestKey];
if (backtests.length === 1) {
res.render('../templates/backtest_submit.html.twig', backtests[0].result);
return;
Expand Down
2 changes: 1 addition & 1 deletion src/modules/services.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ module.exports = {
myDb.pragma('journal_mode = WAL');

myDb.pragma('SYNCHRONOUS = 1;');
myDb.pragma('LOCKING_MODE = EXCLUSIVE;');
// myDb.pragma('LOCKING_MODE = EXCLUSIVE;');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I specified it on the commit message bit it got lost in the log :) It's basically cos sqlite3 is for some reason not allowing multiple connections to the database, one from the fork and from the main app. Probably worth locking the queries instead of the connection to the db. Happy to change to for a different approach if you have any ideas


return (db = myDb);
},
Expand Down
Loading