Trie-based execution proof

[svyatonik]

Previously light nodes were fetching the whole state as an execution proof.
Brief overview:

• renamed client/db/DbState to state_machine/TrieBackend;
• added TryIntoTrieBackend trait which 'converts' state into trie state (implemented by db and in-mem backends and NOT implemented (here: return None) by light backend);
• added state backend ProvingBackend, which tracks all accessed trie nodes;
• added fn create_proof_check_backend, which creates in-memory TrieBackend, filled with execution proof;
• added prove_at_state to CallExecutor (fails on light node).

[svyatonik]

This expect is not valid anymore, afaiu. Like when we're trying to access missing value from proof check backend, based on invalid proof. I'm willing to change return type of fn storage() to Result, but not sure - why it isn't Result already? Maybe I'm missing some logic behind this? Could anyone clarify?

[svyatonik]

depends on: openethereum/parity-ethereum#8726

[gavofyork]

The expect asserts that if you are to run e.g. execute_block in the runtime, you will have all storage items required to do so. It's actually only a "soft" panic - in that it doesn't propagate and take down the application since it happens within the runtime. Switching it to Result would mean ensuring that any runtime asking for it should early exit before control is returned to the runtime since runtimes should not need to be able to deal with storage entries not being available.

[svyatonik]

Ok, thanks. So if it is 'soft panic', probably better to leave it as is.

[gavofyork]

Would be nice to have some unit tests for this code.

[rphmeier]

(looks like they've been added)

[gavofyork]

Could do with some finer-grained tests?

[gavofyork]

Some unit tests for this module?

[gavofyork]

Code looks good, but some additional tests would be nice

[svyatonik]

@gavofyork done

[rphmeier]

let's block this on #169 (which should already be far enough along to base off of)

[gavofyork]

Will still need resolving...

[svyatonik]

@gavofyork There are no conflicts and tests are failing because of LLVM ERROR: IO failure on output stream: No space left on device. I can't restart job - access denied.

[rphmeier]

perhaps a RefCell is enough?

[rphmeier]

I think all this TrieLookupRecorder stuff is redundant with the patricia_trie::Query trait, isn't it?

[rphmeier]

i.e. instead of using the Ephemeral DB at all, we just replace the call to TrieDB::get to TrieDB::get_with and use a patricia_trie::Recorder

[svyatonik]

Yeah - I see, sorry - missed that method.

[svyatonik]

changed to use get_with with Recorder

[svyatonik]

...also removed Clone requirement for state backend, since Recorder is not cloneable - not sure why we need all states to be cloneable. If this is required, could revert + add Clone implementation for Recorder.

[rphmeier]

@svyatonik moving conversation here since the other is hidden now

I don't think we need states to be cloneable right now. That requirement stems a bit from my idea that:

• state handles should prevent pruning while live
• and should be easily shared

but I'm not sure we will end up going in that direction. We can reinstate as necessary.

[svyatonik]

Notes for future PRs:

1. block digest won't include flag of ":code" change => execution proof must also include the ":code" proof
2. there should be some optimization of this (guess it will be based on Protocol: Light-client friendly storage tracking #131 ) so that the whole ":code" isn't fetched with every call
3. (questionable) since (after optimization) we'll store the ":code" locally, there could be a ":code" cache for recent blocks (of the same structure as "authorities_at" cache)

requested tests were added