feat: triedb.Config support for arbitrary backend implementations
#70
Conversation
| @@ -36,6 +36,8 @@ type Config struct { | |||
| IsVerkle bool // Flag whether the db is holding a verkle tree | |||
| HashDB *hashdb.Config // Configs for hash-based scheme | |||
| PathDB *pathdb.Config // Configs for experimental path-based scheme | |||
|
|
|||
| DBOverride BackendConstructor // Injects an arbitrary backend implementation | |||
There was a problem hiding this comment.
nit: IMO we can call this BackendConstructor as well (or a different name that refers to Backend)
There was a problem hiding this comment.
I agree that they should have the same name. What do you think of these changes?
triedb/database.go
Outdated
| @@ -119,13 +121,16 @@ func NewDatabase(diskdb ethdb.Database, config *Config) *Database { | |||
| } | |||
| db.backend = hashdb.New(diskdb, config.HashDB, resolver) | |||
| } | |||
| db.overrideBackend(diskdb, config) | |||
There was a problem hiding this comment.
if we handle this here then if config.IsVerkle is set the hashDB path will try to initialize but fail
There was a problem hiding this comment.
this is an example (non-recommended) fix ethereum@753d1f5 but probably a different order of checking whether the optional config is specified
There was a problem hiding this comment.
Nice catch, thank you. I was too focussed on mechanistic rules for where changes are made (in this case, "last thing before the return") so we can one day define how to resolve conflicts.
I've moved it to immediately after db is declared and the override now returns a bool that signals an early return. This also removes the smell of having to close a just-opened backend.
| // along with the go-ethereum library. If not, see | ||
| // <http://www.gnu.org/licenses/>. | ||
|
|
||
| package triedb |
There was a problem hiding this comment.
nit: not sure if the triedb/database would be a better package to define this
There was a problem hiding this comment.
I put it here to avoid having to replicate the backend interface anywhere; it also fits nicely with the "accept interfaces" idiom as well as "Go interfaces generally belong in the package that consumes values of the interface type".
Only constructor implementations would need to import them and, since the constructor is injected, there won't be an import loop.
Why this should be merged
Allow
ava-labs/corethto use arbitrarytriedbdatabase implementations.How this works
Introduces
HashBackendandPathBackendinterfaces thattriedb.Databasetype-asserts to instead ofhashdb.Databaseandpathdb.Backendrespectively. Other interfaces are introduced to avoid having to modify{hash,path}db.Database.Reader()return values.The explicit
DBOverridefield means that we don't have to modify any of thetriedbconstructor beyond adding a single line immediately before the return. This leaves all modifications of original files entirely mechanistic. This is, however, at the expense of compile-time guarantees of the overriding database being either aHashBackendor aPathBackend.How this was tested
Unit test demonstrating override + plumbing.