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

feat(utils): implement v038 for tower-trace #4160

Merged
merged 1 commit into from
Apr 4, 2024
Merged

Conversation

noot
Copy link
Collaborator

@noot noot commented Apr 4, 2024

Describe your changes

implement support for ABCI v038 for the tower-trace util crate. required for eventual upgrade to ABCI v038 :)

Issue ticket number and link

n/a

Checklist before requesting a review

  • If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason:

it only affects the tower-trace util crate

Copy link
Member

@erwanor erwanor left a comment

Choose a reason for hiding this comment

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

LGTM, every consensus request variant is covered and this is using the right v0_38 messages. I will merge on green CI.

@erwanor erwanor merged commit 8b06546 into main Apr 4, 2024
7 checks passed
@erwanor erwanor deleted the noot/tower-trace branch April 4, 2024 22:23
@erwanor erwanor added this to the Sprint 3 milestone Apr 7, 2024
@erwanor erwanor added the C-enhancement Category: an enhancement to the codebase label Apr 7, 2024
github-merge-queue bot pushed a commit to astriaorg/astria that referenced this pull request Apr 17, 2024
## Summary

updates the sequencer app to use ABCI v0.38, which replaces
`begin_block`/`deliver_tx`/`end_block` with one call, `finalize_block`.

relevant penumbra PRs:

- update penumbra-tower-trace to re-add instrumentation to the ABCI
methods (done in penumbra-zone/penumbra#4160)
- update cnidarium to be able to get the root hash of the state without
calling `commit` (done in
penumbra-zone/penumbra#4122)

## Background
this was inevitable, also it cleans up the block execution flow inside
the sequencer application. (removed `processed_txs` and
`current_sequencer_block_builder` from the app)

## Changes
- implement `finalize_block`; for the most part, I left the
`begin_block`/`deliver_tx`/`end_block` as they are but called them
inside `finalize_block`
- however the block execution flow is cleaned up as we no longer need to
track how many txs have been delivered (for the commitments) and we can
construct the `SequencerBlock` at the end of `finalize_block` without
needing to track things between calls

## Testing
unit tests 

ran it with cometbft release
[v0.38.6](https://github.com/cometbft/cometbft/releases/tag/v0.38.6)

## Breaking Changelist
- the sequencer app will now only work with a cometbft version that
supports ABCI v0.38

## Related Issues

closes #679
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: an enhancement to the codebase
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants