fix(providers): collect BlockState before constructing DB provider #11338
Conversation
|
tyty, will approve after trace cleanup |
| let anchor_hash = state.anchor().hash; | ||
| trace!(target: "providers::blockchain", ?anchor_hash, "Fetching historical provider for block hash"); | ||
| let latest_historical = self.database.history_by_block_hash(anchor_hash)?; |
There was a problem hiding this comment.
can there still be a race condition here between the moment anchor hash was set on BlockState and the moment when we actually retrieve the state provider? does it matter here?
There was a problem hiding this comment.
can there still be a race condition here between the moment anchor hash was set on BlockState and the moment when we actually retrieve the state provider? does it matter here?
I think so, although only for reorgs afaict. When the DB state only moves forward this should not be a problem. Take for example a reorg which removes one block from the DB. The race is:
- we fetch the state
- anchor is removed from DB, replaced with another block
- now we call
history_by_block_hashon something that does not exist
The only idea I can come up with to fix it, is to store ExecutedBlocks like we do TrieUpdates before finalization.
c743be2 to
32b3697
Compare
|
cc @rkrasiuk traces removed |
| @@ -177,7 +177,7 @@ impl<N: ProviderNodeTypes> BlockchainProvider2<N> { | |||
| let state = state.as_ref(); | |||
| let anchor_hash = state.anchor().hash; | |||
| let latest_historical = self.database.history_by_block_hash(anchor_hash)?; | |||
| Ok(self.canonical_in_memory_state.state_provider(state.hash(), latest_historical)) | |||
| Ok(self.canonical_in_memory_state.state_provider_from_state(state, latest_historical)) | |||
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
d72977d to
d9e663e
Compare
Changes from #11265 that fix a race condition in the builder
The race condition is between the builder and tree, with the following order of events:
NN+1from the in memory stateCanonicalInmemoryState::state_provider. In the current code this results in a DB provider for blockN, andBlockStateforN+2, ..., meaning there is a gap for blockN+1, which no longer exists inCanonicalInMemoryState.This happens because the anchor hash is determined before block
N+1is persisted. TheBlockStateis still valid, but we should not be re-fetching the state.This PR re-uses the
BlockStatethat is used to determine the anchor hash, and introduces a new methodstate_provider_from_statethat allows this reuse.