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

Remove obsolete abci methods, no longer called by ABCI++ Tendermint #8633

Merged
merged 10 commits into from
May 30, 2022

Conversation

sergio-mena
Copy link
Contributor

@sergio-mena sergio-mena commented May 27, 2022

At a first stage, we had decided to leave BeginBlock, DeliverTx, EndBlock in the ".proto" file, marked as deprecated. However, since Tendermint will not be calling them, an App developer might be tempted to implement them, which would result in dead code.

This PR removes those ABCI methods from the ".proto" file.

The PR also adds a link to ABCI++ spec in the "UPGRADING.md" file, as a preliminary step to documenting the upgrade process.

@creachadair
Copy link
Contributor

The reasoning here makes sense, but I wonder: Do we need to worry about being able to read stored responses in order to validate old blocks? I seem to recall we do rely on some of the ABCI response data being persisted in order to deal with synchronization.

ABCI++ automation moved this from In progress to Reviewer approved May 27, 2022
Copy link
Contributor

@tychoish tychoish left a comment

Choose a reason for hiding this comment

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

seems good but fix lint

@sergio-mena
Copy link
Contributor Author

The reasoning here makes sense, but I wonder: Do we need to worry about being able to read stored responses in order to validate old blocks? I seem to recall we do rely on some of the ABCI response data being persisted in order to deal with synchronization.

Are you thinking of ResponseDeliverTx?
If yes, the first version of this patch was indeed nuking it, but then I realized it was used elsewhere (I saw it was used by RPC-related code). So I decided to leave it there (check the 4th commit of this PR, the one with an unfinished commit message 😊).

The rest of the protobuf data structures deleted in this PR are just not used elsewhere in the "manually generated" code.

@creachadair
Copy link
Contributor

Are you thinking of ResponseDeliverTx?

Yes, and I wasn't sure if there were others.

@sergio-mena sergio-mena merged commit 571f26b into master May 30, 2022
ABCI++ automation moved this from Reviewer approved to Done May 30, 2022
@sergio-mena sergio-mena deleted the sergio/remove_obsolete_abci_methods branch May 30, 2022 06:41
@sergio-mena sergio-mena mentioned this pull request Jul 22, 2022
35 tasks
@sergio-mena sergio-mena mentioned this pull request Dec 19, 2022
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants