This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
Set StateBackend::Transaction to PrefixedMemoryDB
#14612
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e-transaction-cache
bkchr
added
A0-please_review
bkchr
added a commit
to paritytech/cumulus
that referenced
this pull request
Jul 21, 2023
melekes
approved these changes
Jul 24, 2023
| } | ||
| } | ||
|
|
||
| impl<H: Hasher> Clone for OverlayedChanges<H> { |
davxy
approved these changes
Jul 31, 2023
| @@ -142,7 +188,7 @@ pub struct StorageChanges<Transaction, H: Hasher> { | |||
| /// [`main_storage_changes`](StorageChanges::main_storage_changes) and from | |||
| /// [`child_storage_changes`](StorageChanges::child_storage_changes). | |||
| /// [`offchain_storage_changes`](StorageChanges::offchain_storage_changes). | |||
| pub transaction: Transaction, | |||
| pub transaction: PrefixedMemoryDB<H>, | |||
There was a problem hiding this comment.
Nit. Replace PrefixedMemoryDB<H> in the overall file with the alias BackendTransaction<H> (import crate::backend::BackendTransaction).
I know it's just an alias, but for sure is more explicit about the usage intent
| /// Storage transactions are calculated as part of the `storage_root`. | ||
| /// These transactions can be reused for importing the block into the | ||
| /// storage. So, we cache them to not require a recomputation of those transactions. | ||
| pub struct StorageTransactionCache<Transaction, H: Hasher> { | ||
| pub struct StorageTransactionCache<H: Hasher> { |
There was a problem hiding this comment.
Suggested change
| pub struct StorageTransactionCache<H: Hasher> { | |
| struct StorageTransactionCache<H: Hasher> { |
If only used here maybe we can keep it private?
| @@ -176,7 +177,7 @@ mod execution { | |||
| pub type DefaultHandler<E> = fn(CallResult<E>, CallResult<E>) -> CallResult<E>; | |||
|
|
|||
| /// Trie backend with in-memory storage. | |||
| pub type InMemoryBackend<H> = TrieBackend<MemoryDB<H>, H>; | |||
| pub type InMemoryBackend<H> = TrieBackend<PrefixedMemoryDB<H>, H>; | |||
There was a problem hiding this comment.
Not really related to this PR, but since there is some overhaul in progress...
Can't we define the InMemoryBackend in in_memory_backend.rs (maybe as a newtype) and replace the function new_in_mem with a new constructor in the impl block for that newtype?
Co-authored-by: Davide Galassi <davxy@datawok.net>
…e-transaction-cache
bkchr
added
the
D3-trivial 🧸
andresilva
approved these changes
Aug 16, 2023
|
bot merge |
paritytech-processbot
bot
deleted the
bkchr-rework-storage-transaction-cache
branch
paritytech-processbot bot
pushed a commit
to paritytech/cumulus
that referenced
this pull request
Aug 17, 2023
* Companion for Substrate#14612 paritytech/substrate#14612 * Remove patch * Cargo.lock * Fix * Fix compilation * Fix Fix * ... * :face_palm: * ................. * update lockfile for {"polkadot", "substrate"} * ".git/.scripts/commands/fmt/fmt.sh" --------- Co-authored-by: parity-processbot <>
Ank4n
pushed a commit
that referenced
this pull request
Aug 20, 2023
* Yep * Try to get it working everywhere * Make `from_raw_storage` start with an empty db * More fixes! * Make everything compile * Fix `child_storage_root` * Fix after merge * Cleanups * Update primitives/state-machine/src/overlayed_changes/mod.rs Co-authored-by: Davide Galassi <davxy@datawok.net> * Review comments * Fix issues * Silence warning * FMT * Clippy --------- Co-authored-by: Davide Galassi <davxy@datawok.net>
13 tasks
14 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main change this pull request doing is the removal of
Transactionassociated type fromsp_state_machine::Backend. The transaction is then fixed toPrefixedMemoryDB. BasicallyPrefixedMemoryDBwas always the transaction type being used in Substrate. This removes a lot of generics which simplifies the code in a lot of places. Another important change this pull request is doing is to move the so called "storage transaction cache" into theOverlayedChanges. There is no real need to have this cache out of theOverlayedChangesand thus, we can also simplify some things here and there.This is works towards: paritytech/polkadot-sdk#27
Downstream Code changes
TransactionFororStateBackend::Transactionwas used in where bounds or similar can just be removed to make the code compile again.BlockImportParamswas also changed to only take one generic argument, the block type and not anymore the transaction type as the second generic parameter.DefaultImportQueueis also only taking one generic argument after this pr, the block type.polkadot companion: paritytech/polkadot#7536
cumulus companion: paritytech/cumulus#2923