Skip to content

fix!: TimeoutInfo proto break #2646

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
Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

fix!: TimeoutInfo proto break #2646

wants to merge 4 commits into from

Conversation

@rootulp
Copy link
Collaborator

@rootulp rootulp commented Nov 16, 2025 •
edited
Loading

Closes #2645

Tested in celestia-app: celestiaorg/celestia-app#6203

@rootulp rootulp self-assigned this Nov 16, 2025
evan-forbes
evan-forbes approved these changes Nov 16, 2025
View reviewed changes
proto/tendermint/abci/types.proto
message TimeoutInfo {
google.protobuf.Duration timeout_propose = 1 [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
google.protobuf.Duration timeout_propose_delta = 2 [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
google.protobuf.Duration timeout_commit = 2 [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
Copy link
Member

@evan-forbes evan-forbes Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

damn! I could swear I purposefully avoided modifying the proto in a breaking way, idk how it slipped through

@rootulp
Copy link
Collaborator Author

rootulp commented Nov 16, 2025

I can't test this PR in celestia-app because

celestia-app rp/fix-consensus-timeouts $ celestia-appd version
panic: proto: duplicate enum registered: tendermint.abci.CheckTxType

goroutine 1 [running]:
github.com/gogo/protobuf/proto.RegisterEnum(...)
	/Users/rootulp/go/pkg/mod/github.com/gogo/[email protected]/proto/properties.go:520
github.com/tendermint/tendermint/abci/types.init.0()
	/Users/rootulp/go/pkg/mod/github.com/celestiaorg/[email protected]/abci/types/types.pb.go:3574 +0xc64

I'm reasonably confident the make proto-gen isn't working as expected b/c it changed some improts

@rootulp
Copy link
Collaborator Author

rootulp commented Nov 16, 2025

Fixed by pinning some buf dependencies

This was referenced Nov 16, 2025
Draft
Closed
@rootulp rootulp marked this pull request as ready for review November 16, 2025 21:13
@rootulp
Copy link
Collaborator Author

rootulp commented Nov 17, 2025

Discussed with Rachid and Evan. We plan on not merging until v7.x.

@rootulp rootulp added breaking WS: v7 issues related to v7 labels Nov 17, 2025
@ninabarbakadze
Copy link
Member

ninabarbakadze commented Nov 17, 2025

Discussed with Rachid and Evan. We plan on not merging until v7.x.

should we convert to draft for the meantime?

@rootulp rootulp marked this pull request as draft November 17, 2025 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evan-forbes evan-forbes evan-forbes approved these changes

@rach-id rach-id Awaiting requested review from rach-id rach-id is a code owner

@tzdybal tzdybal Awaiting requested review from tzdybal tzdybal is a code owner

@ninabarbakadze ninabarbakadze Awaiting requested review from ninabarbakadze ninabarbakadze is a code owner

Assignees

@rootulp rootulp

Labels

breaking WS: v7 issues related to v7

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TimeoutInfo proto breakage

3 participants

@rootulp @ninabarbakadze @evan-forbes