-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactored usage of storage traits #583
Conversation
25b042a
to
b336c22
Compare
Refactored all tables to implement new storage traits(Also refactored all tests). Replaced all `<Self as Storage<K, V>>::method` with `self.storage::<Table>().method`. The same for `MerkleRootStorage`. Also refactored all tests. Instead of constants for columns, use the `Column` enum to manage tables columns(Use `Column` everywhere instead of `ColumnId`). In the future, we can use more advanced naming for fields, instead of "column-{i}" because now we have names of enum=) In some places, removed unnecessary usage of `Vec<u8>`(in `put`, `insert` etc methods). Removed usage of `Clone` and `Copy` for some `value` in the same methods. Prepared the code for the next refactoring: - to simplify the implementation of storage-related traits - to use only read/write traits in the places where it is really required - performance improvements to work with types more optimal Update stable rust to 1.63 to support enum-iterator
cc6bbf9
to
bedddb5
Compare
# Conflicts: # fuel-core-interfaces/Cargo.toml # fuel-core/src/database/storage/message.rs
Updated `fuel-vm` to `0.18`. Renamed all columns to be according with the table. Added description to each `Column`. Added more description to tables.
# Conflicts: # fuel-client/Cargo.toml # fuel-core-interfaces/Cargo.toml # fuel-core-interfaces/src/db.rs # fuel-core-interfaces/src/relayer.rs # fuel-core-interfaces/src/txpool.rs # fuel-core/Cargo.toml
…o feature/split-storage-trait
@@ -3,7 +3,7 @@ use async_trait::async_trait; | |||
use thiserror::Error; | |||
|
|||
/// Dummy signer that will be removed in next pull request. | |||
/// TODO do not use. | |||
/// TODO: Do not use. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this PR as well, but isn't a bit weird that we have a public trait that should not be used? Shouldn't we make it private or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall very nice refactor! I have a few minor outstanding feedback items, but I don't think those are as urgent or blocking as getting fuel-core synced with the VM.
Refactored all tables to implement new storage traits(Also refactored all tests). Created types for all tables with descriptions and re-used tables defined in the
fuel-vm
.Replaced all
<Self as Storage<K, V>>::method
withself.storage::<Table>().method
.The same for
MerkleRootStorage
and tests.Instead of constants for columns, use the
Column
enum to manage tables columns(UseColumn
everywhere instead of
ColumnId
). In the future, we can use more advanced naming for fields,instead of "column-{i}" because now we have names of cloumns=)
In some places, removed unnecessary usage of
Vec<u8>
(input
,insert
etc methods).Removed usage of
Clone
andCopy
for somevalue
in the same methods.Prepared the code for the next refactoring: