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

remove url escape icq #1068

Merged
merged 2 commits into from
Jan 11, 2024
Merged

remove url escape icq #1068

merged 2 commits into from
Jan 11, 2024

Conversation

asalzmann
Copy link
Contributor

@asalzmann asalzmann commented Jan 11, 2024

Summary
Upgrading from ibc-go 7.2.0 -> 7.3.1 caused ICQs to fail with the following error

dockernet-stride1-1  | 7:35PM INF |   GAIA          |  VALIDATOR ICQCALLBACK  |  Inclusion proof validated - QueryId cd2662154e6d76b2b2b92e70c0cac3ccf534f9b74eb5b89819ec509083d00a50 module=x/interchainquery

The issue is in modules/core/23-commitment/types/merkle.go. Looking at the diff in merkle.go between those releases, we see that GetKey no long URL escapes merkle paths.

cosmos/ibc-go@v7.2.0...v7.3.1#diff-dd94ec1dde9b047c0cdfba204e30dad74a81de202e3b09ac5b42f493153811afR119

So, we also have to remove url escaping from Stride.

Relevant information from ibc-go: changelog and PR.

Also fixes a typo in the integration tests (@sampocs)

Test plan
Run integration tests

 ✓ [INTEGRATION-BASIC-GAIA] host zones successfully registered
 ✓ [INTEGRATION-BASIC-GAIA] ibc transfer
 ✓ [INTEGRATION-BASIC-GAIA] liquid stake mint and transfer
 ✓ [INTEGRATION-BASIC-GAIA] packet forwarding automatically liquid stake and ibc transfer stAsset to original network
 ✓ [INTEGRATION-BASIC-GAIA] delegation on GAIA
 ✓ [INTEGRATION-BASIC-GAIA] LSM liquid stake
 ✓ [INTEGRATION-BASIC-GAIA] LSM liquid stake with slash query
 ✓ [INTEGRATION-BASIC-GAIA] autopilot liquid stake
 ✓ [INTEGRATION-BASIC-GAIA] autopilot liquid stake and transfer
 ✓ [INTEGRATION-BASIC-GAIA] autopilot redeem stake

@asalzmann asalzmann requested a review from sampocs January 11, 2024 19:47
@asalzmann asalzmann merged commit c55aa61 into main Jan 11, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants