8000 Conform consensus state to Tendermint technical specification · Issue #1171 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Conform consensus state to Tendermint technical specification #1171

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.

Sign up for GitHub

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

Jump to bottom
Closed
BrendanChou opened this issue Jul 26, 2023 · 3 comments
Closed

Conform consensus state to Tendermint technical specification #1171

BrendanChou opened this issue Jul 26, 2023 · 3 comments
Assignees
@sergio-mena
Labels
bug Something isn't working consensus
Milestone
2023-Q3

Comments

@BrendanChou
Copy link
Contributor
BrendanChou commented Jul 26, 2023 •
edited
Loading

Bug Report

Setup

CometBFT version:
v0.37.2 and before

Have you tried the latest version:
Yes

ABCI app:
N/A

Environment:
N/A

node command runtime flags:
N/A

Config

N/A

What happened?

Discrepancies between the Tendermint implementation code and whitepaper were found
tendermint/tendermint#6849
tendermint/tendermint#6850

What did you expect to happen?

These were fixed in Tendermint core in 2021, these changes did not seem to make it to CometBFT somehow, perhaps due to a code rollback of some sort

How to reproduce it

N/A

Anything else we need to know

These fixes are considered to block larger, previously-accepted consensus changes such as Proposer-Based Timestamps tendermint/tendermint#6942
@BrendanChou BrendanChou added bug Something isn't working needs-triage This issue/PR has not yet been triaged by the team. labels Jul 26, 2023
@sergio-mena sergio-mena self-assigned this Jul 27, 2023
@BrendanChou BrendanChou mentioned this issue Jul 27, 2023
Merged
3 tasks
@thanethomson thanethomson added consensus and removed needs-triage This issue/PR has not yet been triaged by the team. labels Jul 31, 2023
@thanethomson thanethomson moved this to In Progress in CometBFT 2023 Jul 31, 2023
@thanethomson thanethomson added this to the 2023-Q3 milestone Jul 31, 2023
@sergio-mena
Copy link
Contributor

Taking a close look at tendermint/tendermint#6849 and tendermint/tendermint#6850 before forward-porting them to main (as a first step... then back to the relevant branches: possibly v0.38.x and v0.37.x)

@BrendanChou
Copy link
Contributor Author
BrendanChou commented Jul 31, 2023 •
edited
Loading

The main difference for tendermint/tendermint#6850 is that it was written before ProcessProposal existed. Therefore, the fall-through-case (the case when reaching the end of the function) is now different from when it was written, requiring a rewrite.

tendermint/tendermint#6849 can be ported largely unchanged, but also includes some minor cleanup which I consider unnecessary to include in the same PR. I have ported it here #1175

This was referenced Aug 3, 2023
Merged
Closed
@sergio-mena
Copy link
Contributor

Closed by #1175, #1207, and #1203

@github-project-automation github-project-automation bot moved this from In Progress to Done in CometBFT 2023 Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sergio-mena sergio-mena

Labels
bug Something isn't working consensus
Projects
No open projects
CometBFT 2023
Status: Done
Milestone
2023-Q3
Development

No branches or pull requests
3 participants
@thanethomson @BrendanChou @sergio-mena
0