Skip to content
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

Move more provider traits into storage-api crate #12478

Open
mattsse opened this issue Nov 12, 2024 · 7 comments
Open

Move more provider traits into storage-api crate #12478

mattsse opened this issue Nov 12, 2024 · 7 comments
Assignees
Labels
C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started

Comments

@mattsse
Copy link
Collaborator

mattsse commented Nov 12, 2024

Describe the feature

we still have a few traits defined in the reth-provider crate that are independent of reth-provider internals, like

pub trait HistoryWriter: Send + Sync {

all of those should be moved to

name = "reth-storage-api"

ideally, trait by trait to make review easy.
can be re-exported from reth-provider to not break a ton of stuff

Additional context

No response

@PanGan21
Copy link
Contributor

Hi @mattsse started with #12480 and I can continue on this depending on the recommended length of each pr!

@mattsse
Copy link
Collaborator Author

mattsse commented Nov 12, 2024

cool, ty!

@PanGan21
Copy link
Contributor

Which other traits do you think are relevant to this task?

@mattsse
Copy link
Collaborator Author

mattsse commented Nov 13, 2024

this one we can move as well

pub trait HeaderSyncGapProvider: Send + Sync {

@PanGan21
Copy link
Contributor

I had a look again on this and it seems it is used here:

impl<TX: DbTx + 'static, N: NodeTypes> HeaderSyncGapProvider for DatabaseProvider<TX, N> {

Do you still consider that it should be moved to storage-api crate?

@mattsse
Copy link
Collaborator Author

mattsse commented Nov 16, 2024

hmm, actually not sure about this one because this is kinda p2p, so we can keep this particular one for now

@PanGan21
Copy link
Contributor

PanGan21 commented Nov 16, 2024

hmm, actually not sure about this one because this is kinda p2p, so we can keep this particular one for now

Cool. What else do you think is necessary to resolve this ticket?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started
Projects
Status: Todo
Development

No branches or pull requests

2 participants