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

EPIC: ABCI++ #12272

Closed
38 of 42 tasks
tac0turtle opened this issue Jun 15, 2022 · 8 comments · Fixed by #16315
Closed
38 of 42 tasks

EPIC: ABCI++ #12272

tac0turtle opened this issue Jun 15, 2022 · 8 comments · Fixed by #16315

Comments

@tac0turtle
Copy link
Member

tac0turtle commented Jun 15, 2022

Summary

Tendermint team has been working on a refactor of the abci layer to enable more use cases. This has been a requested feature by many users and will be a focus of the sdk team once a release of 0.36 is cut and SDK team has completed app wiring migration.

More Information can be found on the design here: https://github.com/tendermint/tendermint/tree/master/spec/abci%2B%2B

Phase 1

Phase 1 of baseapp++ will be focused on prepare proposal and process proposal.

Phase 2

Phase 2 of baseapp++ will commence once tendermint releases a version of abci with vote extensions and finalise block

  • abci 2.0: write ADR #14674 (@alexanderbez)
  • Implement vote extensions (@alexanderbez)
  • Implement finalize block (@alexanderbez)
  • Update remaining components and tests of BaseApp
  • Update modules (this might need to be broken down by module in case burden is too high) (@alexanderbez)
  • Update simapp
  • Update store (this includes possibly revamping streaming APIs) (@tac0turtle)
  • Update runtime
  • Update server
  • Update types
  • Update client
  • Audit BaseApp ABCI methods to ensure errors are only returned where necessary
  • BaseApp tests for FinalizeBlock, ExtendVote, and VerifyVoteExtension

Phase 3

Phase 3 is all about testing

Phase 4

  • add changelog and upgrading.md for changes

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@p0mvn
Copy link
Member

p0mvn commented Jun 26, 2022

Is the plan to switch to the newly introduced "same block execution" or keep using "next block execution"?

Asking to learn - why would we prefer one over the other in the SDK?

@alexanderbez
Copy link
Contributor

I think the idea here @p0mvn is to refactor/update the BaseApp implementation to implement the new ABCI++ interface and deprecate or remove the old methods.

@tac0turtle
Copy link
Member Author

Is the plan to switch to the newly introduced "same block execution" or keep using "next block execution"?

Asking to learn - why would we prefer one over the other in the SDK?

We will continue to use next-block execution because the sdk is built with the off by one approach of delayed execution (next-block execution), if we move to immediate execution (same-block execution) we would need to refactor quite a few things

@tac0turtle tac0turtle added the T:Epic Epics label Jul 28, 2022
@tac0turtle tac0turtle moved this from 📝 Todo to 💪 In Progress in Cosmos-SDK Aug 8, 2022
@itsdevbear
Copy link
Contributor

The ability to do same block execution would be nice, how bad would the refactor be @marbar3778 ?

@tac0turtle
Copy link
Member Author

immediate execution would be a fairly hefty rewrite, I believe, as staking and other modules assume H+1. Optimistic execution, where we start execution in the process proposal phase is a lot lighter of a lift

@alexanderbez
Copy link
Contributor

alexanderbez commented Oct 2, 2022

Yeah it's not trivial because as @marbar3778 pointed out there are various places in the SDK where we have this "off by 1" assumption, so it's not just a few changes to baseapp :-/

In any case, we're proposing the execute the entire block in ProcessProposal, where apps (if they decide) can actually execute this in parallel with Tendermint's voting rounds.

@itsdevbear
Copy link
Contributor

Immediate execution seems like the "more correct" way to do things long term though? Should improve UX a lot since chain will feel "snappier" when using. Less important in a world of sub second block times, but still seems important?

I know this would mess with the distributor for sure, but I presume also evidence crisis etc. would need a big refactor.
Would optimistic processing get close to similar performance?

@alexanderbez
Copy link
Contributor

Long term? Yes. It's not on our immediate roadmap though as ABCI++ integration is more important along with store improvements as that is the biggest pain point atm. People don't really mind the current execution model, granted it is annoying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants