[cxxt][2/2] Add columns for slippage detection

[xmatthias]

Summary

As mentioned in #606, i think having a slippage detection would help determine good exchanges.

Solve the issue: #606

Quick changelog

• add new columns open_rate_requested and close_rate_requested.

What's new?

• Add columns to allow slippage-anlaysis to determine if a market / exchange is a good choice.

The currently available columns open_rate and close_rate are updated once the trade is executed. This means, we loose the information of the bot buy decision rate, only saving the rate the trade was executed at.

As explained in #606 - this is not necessarily related to changes to ccxt - however i think this is a good time for this due to the database-changes required for #623 - so once ccxt is merged into develop the database needs to be recreated only once.

Important note
Existing database-files are currently incompatible due to the change in the schema.
A migration can be facilitated - however I'm not sure if a custom check / add is a good choice, or if sqlalchemy-migrate or alembic should be considered (i haven't used any of the 2 so far).

[gcarq]

I also think it is a good idea to do both schema changes in one. I would prefer a migration script over a custom check, maybe PR #101 can be used for that.

[xmatthias]

quickly looking at #101, it appears break usage in docker containers (it renames the DB - but with Docker containers it's a mount into the container - so the "updated" database would not be persisted anymore).

From looking at this - i don't really see any benefit to using caribou - it's a script recreating / altering the table as needed if the filename is wrong - but i'll leave a comment there explaining what i mean.

my idea for this was to detect if all tables are present, and either recreate the new version (but using sqlalchemy's feature to do so) - or to alter the table to have all columns - depending on the required change.

A quick draft (untested - probably not flake8 compatible) is here

Not sure this is the best way - but #101 duplicates the "create table" statements (it's in the class already - why need a explicit statement??)

[xmatthias]

I just added a script to automate the database conversation.

I think this should be merged before #652 to avoid problems for users simply running git pull and then having an incompatible, existing Database.

[gcarq]

That was fast :-) I will take a look

[xmatthias]

i've had most of it since a week without time to finish testing ... :)

[gcarq]

I did some tests and it looks very good, but we should also take care of the pair format.
After migrating a very old db file with some open trades I got this warning:

...
WARNING - get_ticker() returned exception: "Could not load ticker history due to ExchangeError. Message: bittrex No market symbol BTC_OMG"
...

I'm not sure yet if there are other implications for open trades.

[gcarq]

LGTM, but I think we should add some migration logging and also cover the pair format conversion from (BTC_XYZ -> XYZ/BTC) within this PR.

[xmatthias]

Great catch, didn't think about this anymore. Added a 2nd test for this, to make sure both cases (old format, new format) are tested properly.

Also added a conversion to the exchange now - ccxt saves it all lowercase, while previously it was 'BITTREX' - so i guess it makes sense to convert the exchange-name to lowercase.
It's not a functional change (both ways will work technically) - but it makes the older trades "stand out" in the database.

[gcarq]

Nice, thanks a lot. Good catch with the exchange name. LGTM