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

Submit misbehaviour messages using the CCV consumer id (Permissionless ICS) #4182

Merged
merged 39 commits into from
Nov 6, 2024

Conversation

romac
Copy link
Member

@romac romac commented Sep 5, 2024

Closes: #4153

Depends on: informalsystems/tendermint-rs#1467

Description


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

@ljoss17 ljoss17 marked this pull request as ready for review October 25, 2024 07:38
crates/relayer-cli/src/commands/evidence.rs Outdated Show resolved Hide resolved
crates/relayer-cli/src/commands/evidence.rs Outdated Show resolved Hide resolved
crates/relayer-cli/src/commands/evidence.rs Outdated Show resolved Hide resolved
crates/relayer-cli/src/commands/evidence.rs Outdated Show resolved Hide resolved
crates/relayer/src/chain/cosmos.rs Outdated Show resolved Hide resolved
crates/relayer/src/foreign_client.rs Outdated Show resolved Hide resolved
crates/relayer/src/foreign_client.rs Show resolved Hide resolved
@ancazamfir
Copy link
Collaborator

ancazamfir commented Oct 29, 2024

If I run double_sign_test.sh with ics v6.3.0 I get the following error (changed the script to print the raw_log) :

30
consumer creation failed with code: "failed to execute message; message index: 0: cannot set consumer initialization parameters: invalid initial height for consumer id (0): chain ID (consumer) doesn't match revision number (1): invalid consumer initialization parameters"

Same happens with light_client_attack_freeze_test.sh and light_client_attack_test.sh.

Scripts work with v6.2.0

@ancazamfir
Copy link
Collaborator

Also, do we have any tests where we test a 3rd-party chain connecting to a consumer chain, in addition to the provider. And the test should check both 3rd party and the provider chains freeze their clients, and the provider gets the extra ICS message. (this is for light client attack)

@ljoss17
Copy link
Contributor

ljoss17 commented Oct 30, 2024

If I run double_sign_test.sh with ics v6.3.0 I get the following error (changed the script to print the raw_log) :

30
consumer creation failed with code: "failed to execute message; message index: 0: cannot set consumer initialization parameters: invalid initial height for consumer id (0): chain ID (consumer) doesn't match revision number (1): invalid consumer initialization parameters"

Same happens with light_client_attack_freeze_test.sh and light_client_attack_test.sh.

Scripts work with v6.2.0

I will look into this

@ljoss17
Copy link
Contributor

ljoss17 commented Oct 30, 2024

Also, do we have any tests where we test a 3rd-party chain connecting to a consumer chain, in addition to the provider. And the test should check both 3rd party and the provider chains freeze their clients, and the provider gets the extra ICS message. (this is for light client attack)

I think it would be best to migrate from the .sh scripts to the integration-tests in order to easily do this

@ljoss17
Copy link
Contributor

ljoss17 commented Oct 31, 2024

If I run double_sign_test.sh with ics v6.3.0 I get the following error (changed the script to print the raw_log) :

30
consumer creation failed with code: "failed to execute message; message index: 0: cannot set consumer initialization parameters: invalid initial height for consumer id (0): chain ID (consumer) doesn't match revision number (1): invalid consumer initialization parameters"

Same happens with light_client_attack_freeze_test.sh and light_client_attack_test.sh.

Scripts work with v6.2.0

It should now be fixed with 40bc893

@ancazamfir
Copy link
Collaborator

I think it would be best to migrate from the .sh scripts to the integration-tests in order to easily do this

Ok, sounds good. Let's do it in a separate PR.

Copy link
Collaborator

@ancazamfir ancazamfir left a comment

Choose a reason for hiding this comment

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

Great work @ljoss17 and @romac 🚀

@ljoss17
Copy link
Contributor

ljoss17 commented Nov 6, 2024

I think it would be best to migrate from the .sh scripts to the integration-tests in order to easily do this

Ok, sounds good. Let's do it in a separate PR.

Opened the issue #4238

@ljoss17 ljoss17 added this pull request to the merge queue Nov 6, 2024
Merged via the queue into master with commit c3105e9 Nov 6, 2024
30 checks passed
@ljoss17 ljoss17 deleted the romac/permissionless-ics branch November 6, 2024 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Submit misbehaviour messages using the consumer id
3 participants