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

Productionize blockchain reactor v2 [Tracking Issue] #5681

Closed
7 tasks
tessr opened this issue Nov 17, 2020 · 11 comments
Closed
7 tasks

Productionize blockchain reactor v2 [Tracking Issue] #5681

tessr opened this issue Nov 17, 2020 · 11 comments
Assignees
Labels
C:sync Component: Fast Sync, State Sync T:tracking Tracking issue for other issues
Milestone

Comments

@tessr
Copy link
Contributor

tessr commented Nov 17, 2020

The blockchain reactor v2 has been around for almost a year, but it hasn't been fully tested, and there are a still a number of outstanding follow-ups around it:

Perhaps:

(I think there are more; will add to this tracking issue as I find them.)

We'd like to make v2 the default blockchain reactor in Tendermint 1.0, but we need to handle these outstanding issues first.

@tessr tessr added the C:sync Component: Fast Sync, State Sync label Nov 17, 2020
@tessr tessr added this to the Tendermint 1.0 milestone Nov 17, 2020
@tessr tessr added the T:tracking Tracking issue for other issues label Nov 17, 2020
@alexanderbez
Copy link
Contributor

I would be happy to tackle these :)

@tessr
Copy link
Contributor Author

tessr commented Nov 17, 2020

OK, nice! I will put your face on it, speculatively

@cmwaters
Copy link
Contributor

I think we should also consider light client verification for fast sync: #4457

@melekes
Copy link
Contributor

melekes commented Jan 21, 2021

#5496 is probably the most important issue

@tessr
Copy link
Contributor Author

tessr commented Jan 21, 2021

Thanks, will add these both ^

@tac0turtle
Copy link
Contributor

I'll be the one to say it.

Let's remove v2. We can focus on v0 and look into a simplification of v0 if need be.

@alexanderbez
Copy link
Contributor

I support that. Unless someone really wants to take ownership and gain domain expertise over v2, I say we should be OK with removing it.

@tac0turtle
Copy link
Contributor

Seems like there is general agreement for removing the protocol. I would propose we close all issues, mark v2 as deprecated in 0.35 and remove it in 0.36?

@ebuchman, as your team spent many months on this work, I would love to have your support in this work.

@ebuchman
Copy link
Contributor

What is the motivating reason for abandoning v2 for v0? It seems there is some missing context between the comment about #5496 being the biggest issue (a protocol issue which i believe also exists in v0 and would be easier to solve in v2), and the comment about removing it and "simplifying" v0. Is there an issue somewhere that can fill in this context? Maybe something about problems/reliability issues using it to sync on a real network?

@tac0turtle
Copy link
Contributor

there have been a few discussions within the tendermint team throughout the past year. Back when anton and erik were still working on the project, they agreed v2 is unnecessarily complex, does not add performance or understanding to the protocol. V0 is simple and has fewer issues than v2 in the time it has been in the repo. We have had many messages from users that they are running into issues with v2, while with v0 we still get messages, but way less frequent.

This led us to removing users the ability to use v2 (merged in 0.35), shortly after that a few team members started thinking what if we removed v2 and there seem to be agreement around it.

@tac0turtle
Copy link
Contributor

V2 has been removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:sync Component: Fast Sync, State Sync T:tracking Tracking issue for other issues
Projects
None yet
Development

No branches or pull requests

6 participants