celestiaorg / go-header

Go library with all the services needed to request, sync and store blockchain headers.
Apache License 2.0
17 stars 16 forks source link

store: drop `Init` function #187

Closed cristaloleg closed 1 month ago

cristaloleg commented 1 month ago

Looks like this function is just a test helper (used only in 1 place BTW). But also it looks orthogonal to the Store interface which already contains Init method.

I'm proposing to drop this function in favour of NewStoreWithHead which does the initialization.

cristaloleg commented 1 month ago

The only thing is that we've 1 usage in celestia-node see nodebuilder/header/constructors.go but this can be easily addressed.

cristaloleg commented 1 month ago

After a deeper look looks like we do not use Store.Init properly. By example in celestia-node in cmd/cel-shed/header.go we initialize store manually however Init func can do the same.

I think this issues should be extended to something: store: drop Init function and use Store.Init where appropriate.

WDYT ?

Wondertan commented 1 month ago

Why not to keep this function for reuse purposes? We already use in one place and as you mentioned we could use it one more place?

cristaloleg commented 1 month ago

Yep, after hour of jumping back and forth all over the code looks like there is nothing we can do. We have a way to initialize store via header, via height. Better to leave it as it's for now.

Closing.