-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat(core): switch database to Redb #1351
Conversation
@@ -580,6 +609,45 @@ impl Encodable for ChainDataIndex { | |||
} | |||
} | |||
|
|||
impl redb::Value for ChainDataIndex { |
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.
We shouldn't have code necessary for the redb impl in a module gated behind the libmdbx
feature, maybe we could move ChainDataIndex
outside this module ( to engine or engine/utils) and only keep implementation-specific code here
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.
Agreed, I moved the ChainDataIndex
enum to its own utils
module under engines
here dbf7bdd
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.
In the storage module we have tests that run the full test suite against the different engine implementations, we should add one for redb too. We just need to add this test:
#[test]
fn test_redb_store() {
test_store_suite(EngineType::Redb);
}
Nice!, added the test here 404f388 |
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.
LGTM
Motivation
Description
Closes #738