search

celestiaorg / celestia-core

A fork of CometBFT
Apache License 2.0
481 stars 264 forks source link

refactor!: remove v0 mempool #1388

Open ninabarbakadze opened 3 months ago

ninabarbakadze commented 3 months ago

Description

Fixes #1387

cmwaters commented 3 months ago

Hmm I want this to only be apart of v3. Which might be okay since v0.34.x is the branch for v1 and v2.

I would ideally like to be sure of our v3 release strategy i.e. will v3 have a v2 of celestia-core. Will we diverge from cometbft in our release naming because we already released a v1, will this be compatible with the SDK and our ambitions of no longer needing a fork etc...

cc @evan-forbes @rootulp

In any case it's probably fine to still go ahead and deprecate this. I just want to give ample time to double check. We should be in no rush to have to remove v0

ninabarbakadze commented 3 months ago

Hmm I want this to only be apart of v3. Which might be okay since v0.34.x is the branch for v1 and v2.

I would ideally like to be sure of our v3 release strategy i.e. will v3 have a v2 of celestia-core. Will we diverge from cometbft in our release naming because we already released a v1, will this be compatible with the SDK and our ambitions of no longer needing a fork etc...

cc @evan-forbes @rootulp

In any case it's probably fine to still go ahead and deprecate this. I just want to give ample time to double check. We should be in no rush to have to remove v0

should i convert this into a draft?

evan-forbes commented 3 months ago

we just need to avoid backporting it, and can keep all of our changes for v3 in main

evan-forbes commented 3 months ago

I personally haven't been convinced that we should deprecate and remove the v0 mempool. Is there anything I can read about how difficult it is to maintain?

my understanding is that we just wanted to save some time implementing bestTx things multiple times, and no one is using the v0 mempool. This is a good point though. When we look upstream, we see that they exclusively use v0, so this might make backporting certain test require further changes.