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(app): 🎣 add missing Dex component hook #4247

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

cratelyn
Copy link
Contributor

@cratelyn cratelyn commented Apr 19, 2024

see #3739, and #4246.

while wiring up a pair of missing component hooks for the Sct component, i found another (noöp) hook that we do not include in the App's control flow.

this isn't currently a bug, since <Dex as Component>::begin_block() is a noöps but could easily lead to a surprising outcome in the future, if more logic was introduced to that trait method.

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:

    while this does change the control flow of consensus-critical logic, this does not
    change the execution of incoming data. the hooks introduced here are noöps.

see #3799, and #4246.

while wiring up a pair of missing component hooks for the `Sct`
component, i found another (noöp) hook that we do not include in the
`App`'s control flow.

this isn't currently a bug, since `<Dex as Component>::begin_block()`
is a noöps but could easily lead to a surprising outcome in the future, if
more logic was introduced to that trait method.
@cratelyn cratelyn added E-easy Effort: Easy A-dex Area: Relates to the dex labels Apr 19, 2024
@cratelyn cratelyn added this to the Sprint 4 milestone Apr 19, 2024
@cratelyn cratelyn self-assigned this Apr 19, 2024
@cratelyn
Copy link
Contributor Author

#[instrument(name = "dex", skip(_state, _begin_block))]
async fn begin_block<S: StateWrite + 'static>(
_state: &mut Arc<S>,
_begin_block: &abci::request::BeginBlock,
) {
}

for reference:

@cratelyn cratelyn marked this pull request as ready for review April 19, 2024 17:39
@cratelyn cratelyn merged commit 7e8ff55 into main Apr 19, 2024
8 checks passed
@cratelyn cratelyn deleted the kate/add-missing-dex-component-hook branch April 19, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dex Area: Relates to the dex E-easy Effort: Easy
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants