Skip to content

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

Merged
sergio-mena merged 10 commits intomasterfrom
sergio/remove_obsolete_abci_methods
May 30, 2022
Merged

Remove obsolete abci methods, no longer called by ABCI++ Tendermint#8633
sergio-mena merged 10 commits intomasterfrom
sergio/remove_obsolete_abci_methods

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

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.

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

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
@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

None yet

Development

Successfully merging this pull request may close these issues.

3 participants