Use DB backend in test client

[svyatonik]

closes #1603

Also fixed two issues, revealed by this migration:

1. wrong assumptions about API safety in changes trie storage implementation;
2. fixed simultaneous import of block B + finalization of its ancestors (i.e. Instant finality doesn't work #1602).

This PR also introduced test_client::new_light to be used in integration tests. The test_client::LightFetcher is currently a stub, because I'm not sure yet about its implementation (it will be fixed as a part of #1669).

[bkchr]

Can we not then drop client/in_mem.rs?

[svyatonik]

Not sure, but imo it still will be useful for some small, low-level tests which should simulate abnormal cases. Original idea was to use DB backend in all tests where test_client is involved (i.e. relatively high level tests), because the client behavior sometimes is completely different when using in_mem vs db backends.

Also I'm using it here to compare DB backends (i.e. db_backend1.as_in_memory_backend().compare_to(db_backend2.as_in_memory_backend())). This could be reimplemented of course, but still some in-memory structure would be useful (especially when comparing light vs full databases).

[bkchr]

But the kvdb in memory implementation is also just in memory? In my opinion this just requires additional code that is only used in tests and we don't test the real code that interacts with the db interface.
We should maybe improve the implementation of the kvdb in memory to support stuff like comparing etc.

[svyatonik]

But the kvdb in memory implementation is also just in memory?

Comparing kvdb = comparing db-level representation (columns, ...) even if we improve it. Also it could be affected by some irrelevant factors (insertion order - idk?). To compare two client backends we need to compare the set of headers + the set of chain (best + last finalized + ...) + ... => we'll need several HashMaps + insertion ops anyway (which is in_mem::*).

In my opinion this just requires additional code that is only used in tests and we don't test the real code that interacts with the db interface.

Exactly. We have a lot of code that is only used in tests (helpers/stubs/mock objects/...). Consider in_mem::* as another one helper. Like we do not tend to use wasm executor in every test, though there are some tests that are testing it.

[svyatonik]

That said, I'm ready to give up if you'll find more supporters on your side :) But would prefer to separate "Use DB backend in test-client" and "Get rid of in-memory backend" tasks.

[bkchr]

I don't want you to give up^^
It is just that kvdb-memorydb uses a hashmap internally: https://github.com/paritytech/parity-common/blob/9abaf267d1319ed65e2fdd90ee55432dc4540f78/kvdb-memorydb/src/lib.rs#L29

In #1609 we have the situation that we need to implement a completely different interface for in_mem. Thus, the real interface will not be tested by unit tests. So, we maybe do not catch bugs in tests.

[svyatonik]

I'd say that it is probably better to rebase #1609 on this PR && add unimplemented!() in in_mem::Blockchain::children() until some specific implementation is actually required for some low-level tests. I could see that right now all new tests there are using test_client::new() => DB backend will be used.

[bkchr]

Okay, sounds like a plan :)

[bkchr]

Looks good, but the test interface should use kvdb-inmemory.