Frontier DB block mapping one-to-many

[tgmichel]

On equivocation state_root is the same between blocks, and so is the Ethereum header/hash. Because we are currently mapping one-to-one in the frontier db, if the future canonical block is imported first, it will be overriden by the orphan block hash.

To avoid that, this PR proposes going back to one-to-many for Ethereum<>Substrate block hash mapping + required data migration. Migration includes schema change for block mapping column (now holds a Vec<Block::Hash> instead Block::Hash) + retroactive orphan substrate block hash substitution.

Note on parity-db

For the migration we need to iter the block mapping column to retrieve the existing keys. In contrast to RocksDb, ParityDb apparently needs the column to be indexed to do that so I changed the block mapping column configuration to be btree_index = true.

I'm not really sure if existing data for that column will be indexed when we opening the db, so I asked in paritytech/parity-db#129

** Update on parity-db **

Apparently is not possible to iterate over column data that was configured with default values, so we cannot get the keys of a hash-indexed column of an existing db. @sorpaas so either we set the BLOCK_MAPPING column to btree_index = true to support future migrations - being btree index generally slower than default hash index - or we don't support data migration for paritydb at all until an api is provided. In any case this PR will require a re-sync for any paritydb live dbs.

[koushiro]

Hi, @tgmichel I upgrade the substrate in the #830 and found the tests(commitment_create, commitment_update and mapping_read_works) of frontier_db_cmd crashed.

I added some debug log, error message like below:

mapping_db query: id = BlockId::Hash(0x3e473eb1eca2d98fa7c3f58331fb746a3f93ccd9d809977181eef6bd80dd0643)
Err(Application(Execution(Wasmi(Function("Module doesn't have export EthereumRuntimeRPCApi_current_transaction_statuses")))))
thread 'frontier_db_cmd::tests::tests::commitment_create' panicked at 'assertion failed: cmd(format!(\"{:?}\", ethereum_block_hash), Some(test_value_path.clone()),\n            Operation::Create,\n            Column::Block).run(Arc::clone(&client), backend.clone()).is_ok()', client/cli/src/frontier_db_cmd/tests.rs:575:9

So is it wrong to use substrate_test_runtime_client here?
Because the client uses thesubstrate_test_runtime that does not implement EthereumRuntimeRPCApi.
Hmm, but I don't know why the tests will pass in the master branch :(

[tgmichel]

@koushiro I see yes, thank you. It's a good question and I will need to dig into it. For this PR I will just use the test runtime api.

[sorpaas]

or we don't support data migration for paritydb at all until an api is provided. In any case this PR will require a re-sync for any paritydb live dbs.

That's okay. We've done that in Substrate in the past as well (requiring paritydb to resync).

Eventually those migration code will be removed anyway.

[sorpaas]

Looks like C is only used in DB migrations. In that case, it may be better to remove this generic from Backend and add it to Backend::new.

Suggested change

	pub fn new(client: Arc<C>, config: &DatabaseSettings) -> Result<Self, String> {
	pub fn new<C: HeaderBackend<...>>(client: Arc<C>, config: &DatabaseSettings) -> Result<Self, String> {

[sorpaas]

Grumble on the generic, and otherwise LGTM.