Define basic interfaces for using Merkle tree #39
Conversation
| _x: PhantomData<S>, | ||
| } | ||
|
|
||
| impl<S: KVStorage> MerkleTree<S> { |
There was a problem hiding this comment.
Maybe we need a sort of contain(value) -> bool method?
There was a problem hiding this comment.
no, for the same reason with get().
merkle-tree/src/lib.rs
Outdated
|
|
||
| /// A state machine that interpret the given storage as a Merkle tree rooted by a specific key. | ||
| /// | ||
| /// Note that there is no `get()` method as leaf nodes (which the user of this struct would like to read) |
There was a problem hiding this comment.
I don't understand this part. Isn't it that the MerkleTree is an upper-level abstraction upon KVstorage? In that case, I believe get() method is still needed unless our PBC code holds both KVstorage and MerkleTree.
There was a problem hiding this comment.
No it's not. MerkleTree is just one way of interpreting the given storage. We still need to access to the underlying storage for the following cases.
- We need to access to entries that are not part of the Merkle tree. (e.g., past blocks)
- We need to use the 'checkpoint' functionalities provided by the storage.
That makes it natural not to provideget()method that would just invoke anotherget()of the underlying storage.
There was a problem hiding this comment.
I add a commit about this. Please check a6d9a65
|
|
||
| /// TODO: Provide error types. | ||
| /// | ||
| /// Note: This trait is quite subject to change. | ||
| #[async_trait] | ||
| pub trait KVStore { | ||
| pub trait KVStorage { |
simperby/src/lib.rs
Outdated
| @@ -16,7 +16,7 @@ pub trait BlockExecutor { | |||
| type Transaction; | |||
| async fn execute( | |||
| &self, | |||
| store: &mut dyn KVStore, | |||
| store: &mut dyn KVStorage, | |||
There was a problem hiding this comment.
it's better to change from "store" to "storage"
kv-storage/src/lib.rs
Outdated
| /// Retrieves the value associated with the key. If not exists, it will return `None`. | ||
| async fn get(&self, key: Hash256) -> Result<Option<Vec<u8>>, Error>; | ||
| /// Checks whether the given item exists in the storage. | ||
| async fn contain(&self, key: Hash256) -> Result<(), Error>; |
There was a problem hiding this comment.
how contain fn notify its result?
I thk, contain(val) -> bool is better.
There was a problem hiding this comment.
@Milchstra3e Same as get
i.e., Ok(()) if exists, Error if
- not exists
- DB error
Error variants will be added later.
kv-storage/src/lib.rs
Outdated
| /// Removes a key-value pair from the storage. If not exists, it will fail. | ||
| async fn remove(&mut self, key: Hash256) -> Result<(), Error>; | ||
| /// Retrieves the value associated with the key. If not exists, it will return `None`. | ||
| async fn get(&self, key: Hash256) -> Result<Option<Vec<u8>>, Error>; |
There was a problem hiding this comment.
get returns None if the item of the given key doesn't exist, while contain returns Err in the same case.
That might be confusing.
merkle-tree/src/lib.rs
Outdated
| unimplemented!() | ||
| } | ||
|
|
||
| /// Returns a reference to the underlting storage. |
| /// Records the current state to the persistent storage. | ||
| async fn commit_checkpoint(&mut self) -> Result<(), Error>; | ||
| /// Reverts all the changes made since the last checkpoint. | ||
| async fn revert_to_latest_checkpoint(&mut self) -> Result<(), Error>; |
There was a problem hiding this comment.
Can we commit checkpoint two times in a row and then revert to latest two times?
In other words, will the commits stack up if we call commit_checkpoint multiple times?
No description provided.