Added RevertibleStorage to handle storage that does not need to be verified using Merkle Tries
#27
Conversation
… skiping first 5 bites
| &mut self, | ||
| requested_id: ChangeID, | ||
| ) -> Result<(), BonsaiStorageError<DB::DatabaseError>> { | ||
| let kv = &mut self.db; |
There was a problem hiding this comment.
Is it possible to make a function with this code and the one from revert_to (that is not tied to any of the storage) and call it here instead of duplicating the code ?
There was a problem hiding this comment.
I'm generally a bit on the fence when it comes to refactoring small bits of code like this, as I tend to prefer having functions explicitly state their inner workings. In this case, I'm not sure having a single code source for revert_to between RevertibleStorage and BonsaiStorage is a good idea, especially since these two structs each have their own unique approach to storage. I think it might be a better to keep them separate for now, as the logic isn't much complicated.
| ChangeID: id::Id, | ||
| { | ||
| db: KeyValueDB<DB, ChangeID>, | ||
| cache_storage_modified: HashMap<TrieKey, InsertOrRemove<Vec<u8>>>, |
There was a problem hiding this comment.
You don't need any temporary storage in this structure KeyValueDB already do it for you and provide the features to store changes etc. You should called its insert remove etc in your function and just call commit on it in the commit
There was a problem hiding this comment.
Doesn't KeyValue also write to the db during insertion if a batch isn't provided? See here.
|
Closing this as it is not relevent anymore according to the upcoming optimizations |
Pull Request type
What is the current behavior?
Bonsai lib currently only handles verifiable, consensus-critical storage, with each new commit resulting in recursive hashing to compute a root hash. This can be a problem since not all data stored in Madara or Deoxys is essential to the state of the chain. This is for example the case for contract classes, where only the contract class hash is used to compute the state root for a given block.
Currently, storage that is not consensus-critical is still handled using Substrate storage, but this is also overkill since runtime storage in Substrate also uses Merkle trees for state verification. This is not recommended for large storage, as is currently the case.
A solution to this is to implement simple key-value based revertible storage using the abstractions already provided by the Bonsai lib. This would allow for the easy storage of data that is not consensus critical yet still needs to be reverted to a previous state in the chain in case of a reorg or rpc call on a previous block.
On the long run, this could allow us to migrate away from Substrate as a whole towards our own custom storage solutions.
What is the new behavior?
This pr introduces a new type of storage,
RevertibleStorage, which functions similarly toBonsaiStoragebut which does not use Merkle tries to compute the state of the storage. This means it is not possible to compute the storage state forRevertibleStorage. This results in simpler, smaller storage for data that is not consensus-critical.Does this introduce a breaking change?
No. While
RevertibleStoragefunctions similarly toBonsaiStorage, it exists completely apart from it and does not otherwise change it's inner workings.Other information
As much as possible,
RevertibleStoragehas the same behavior asBonsaiStorage. I wonder though if it is still pertinent to retain identifiers for storage access for the sake of feature parity. Let me know your thoughts on that.