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

ibc: ⛅ hoist ics02 validation out of client_counter #3599

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

cratelyn
Copy link
Contributor

@cratelyn cratelyn commented Jan 10, 2024

nb: this branch strictly contains plain code motion. it does not make any
changes to the code being moved.

this branch performs two simple transformations, broken into two commits:

⛅ hoist ics02 validation out of client_counter

ics02_validation contains some important code for interfacing with
tendermint/cometbft. while this submodule lives beneath the client counter
logic, its contents are used in our message handling code related to e.g.
misbehavior.

this hoists this submodule up one level, to be a child of
penumbra_ibc::component.

⛅ place validate_penumbra_client_state in ics02_validation

while validate_penumbra_client_state was not originally placed in the ics02_validation submodule,
it feels like it is more connected to that validation logic than the
surrounding client counter types.

this bears out to be true; it is pub, the client counter code isn't changed,
and affected imports are in the message handler area.

nb: this commit strictly contains plain code motion. it does not make any
changes to the code being moved.

`ics02_validation` contains some important code for interfacing with
tendermint/cometbft. while this submodule lives beneath the client
counter logic, its contents are used in our message handling code
related to e.g. misbehavior.

this hoists this submodule up one level, to be a child of
`penumbra_ibc::component`.
nb: this commit strictly contains plain code motion. it does not make any
changes to the code being moved.

while this was not originally placed in the ics02_validation submodule,
it feels like it is more connected to that validation logic than the
surrounding client counter types.

this bears out to be true; it is `pub`, the client counter code isn't
changed, and affected imports are in the message handler area.
@cratelyn cratelyn self-assigned this Jan 10, 2024
@cratelyn cratelyn added the A-IBC Area: IBC integration with Penumbra label Jan 10, 2024
@cratelyn cratelyn force-pushed the katie/hoist-ics02-validation branch from 800434b to 83f95f1 Compare January 10, 2024 14:27
@cratelyn cratelyn marked this pull request as ready for review January 10, 2024 14:33
@cratelyn cratelyn requested a review from avahowell January 10, 2024 16:18
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.

Did a review pass, and looks good to me. let's wait for @avahowell's +1 before merging

@cratelyn
Copy link
Contributor Author

Did a review pass, and looks good to me. let's wait for @avahowell's +1 before merging

agree. this will conflict slightly with #3598, so do feel free to merge this on my behalf today 🙂

@erwanor erwanor merged commit 5adb302 into main Jan 11, 2024
7 checks passed
@erwanor erwanor deleted the katie/hoist-ics02-validation branch January 11, 2024 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-IBC Area: IBC integration with Penumbra
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants