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.

Sign up for GitHub

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

Jump to bottom

tests: ❓ implement an IBC handshake using mock consensus engine #3758

Closed
cratelyn opened this issue Feb 6, 2024 · 1 comment · Fixed by #4797
Closed

tests: ❓ implement an IBC handshake using mock consensus engine #3758

cratelyn opened this issue Feb 6, 2024 · 1 comment · Fixed by #4797
Assignees
@zbuc
Labels
A-IBC Area: IBC integration with Penumbra A-mock-consensus Area: Relates to the mock consensus engine A-testing Area: Relates to testing of Penumbra _P-medium Medium priority
Milestone
Sprint 10

Comments

@cratelyn
Copy link
Contributor

cratelyn commented Feb 6, 2024

stub issue.

see #3588.

once we have the basics of our consensus engine working, we should proceed to provide facilities to test IBC functionality. let's start by writing a test that an IBC handshake works properly between two chains.
@cratelyn cratelyn added the A-testing Area: Relates to testing of Penumbra label Feb 6, 2024
@github-project-automation github-project-automation bot moved this to 🗄️ Backlog in Penumbra Feb 6, 2024
@github-project-automation github-project-automation bot moved this to Future in Testnets Feb 6, 2024
@cratelyn cratelyn mentioned this issue Feb 6, 2024
Closed
@cratelyn cratelyn added A-IBC Area: IBC integration with Penumbra A-mock-consensus Area: Relates to the mock consensus engine labels Feb 8, 2024
@hdevalence hdevalence added the _P-medium Medium priority label Feb 9, 2024
@cratelyn cratelyn mentioned this issue Mar 6, 2024
Open
@cratelyn cratelyn mentioned this issue Mar 21, 2024
Open
@aubrika aubrika added the _P-V1 Priority: slated for V1 release label Apr 3, 2024
@aubrika aubrika added this to the Sprint 4 milestone Apr 8, 2024
@aubrika aubrika moved this from Backlog to Todo in Penumbra Apr 8, 2024
@cratelyn
Copy link
Contributor Author

cratelyn commented Apr 9, 2024

this wasn't assigned during spring planning, so i am going to remove it from the current sprint.

@cratelyn cratelyn removed this from the Sprint 4 milestone Apr 9, 2024
@aubrika aubrika moved this from Todo to Backlog in Penumbra Apr 16, 2024
@aubrika aubrika added this to the Sprint 5 milestone Apr 22, 2024
@aubrika aubrika moved this from Backlog to Todo in Penumbra Apr 22, 2024
@aubrika aubrika moved this from Todo to Backlog in Penumbra Apr 22, 2024
@aubrika aubrika removed this from the Sprint 5 milestone Apr 22, 2024
@aubrika aubrika removed the _P-V1 Priority: slated for V1 release label May 30, 2024
@zbuc zbuc mentioned this issue Aug 15, 2024
Merged
1 task
@zbuc zbuc moved this from Backlog to In progress in Penumbra Aug 16, 2024
@zbuc zbuc added this to the Sprint 10 milestone Aug 16, 2024
conorsch pushed a commit that referenced this issue Aug 22, 2024
@zbuc @avahowell
Basic IBC Handshake Test (#4797)
8ee733c 
## Describe your changes

This adds a basic IBC handshake test, using the existing mock client.
Some opportunistic refactoring is also included.

This PR adds a new `tests::common::ibc_tests` module, which contains a
`MockRelayer` that can be extended later on.

Follow-up tasks should be basic transfer testing, transfer timeout
testing, and testing with malformed requests.

While debugging this test, bugs were found in the various IBC query
APIs, specifically that the `proof_height` was consistently being
returned one lower than the height whose header would contain the
app_hash necessary for validating the proof. The Go relayer is
unaffected because it uses the ABCI RPC query interface instead, and
Hermes uses the affected APIs but discards the affected `proof_height`
fields and uses its own internal mechanisms for height tracking instead.
Fixes were included for the affected APIs.

## Issue

Closes #3758

## 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:

> There are changes to TendermintProxy and IBC RPC responses however
anyone using the affected RPCs would quickly run into the issues we saw
in testing that revealed the bugs in the RPC responses, and the chain
would properly block any requests made based on the incorrect response
values. The changeset also affects the test code heavily however there
should be nothing that affects consensus or state.

---------

Co-authored-by: Ava Howell <ava@avahowell.me>
@github-project-automation github-project-automation bot moved this from In progress to Done in Penumbra Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zbuc zbuc

Labels
A-IBC Area: IBC integration with Penumbra A-mock-consensus Area: Relates to the mock consensus engine A-testing Area: Relates to testing of Penumbra _P-medium Medium priority
Projects
Penumbra
Archived in project
Testnets
Status: Future
Milestone
Sprint 10
Development

Successfully merging a pull request may close this issue.

Basic IBC Handshake Test penumbra-zone/penumbra
4 participants
@hdevalence @zbuc @cratelyn @aubrika