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

ica host: genesis import does not persist the bounded port id #4626

Closed
3 tasks
alpe opened this issue Sep 12, 2023 · 1 comment · Fixed by #4640
Closed
3 tasks

ica host: genesis import does not persist the bounded port id #4626

alpe opened this issue Sep 12, 2023 · 1 comment · Fixed by #4640
Assignees
Labels
27-interchain-accounts type: bug Something isn't working as expected
Milestone

Comments

@alpe
Copy link
Contributor

alpe commented Sep 12, 2023

Summary of Bug

ica host does not restore the bind port when capabilities were set.

The simulations runs are failing in our Wasmd sdk-50/ ibc-v8 integration branch due to a store mismatch on import.
The decoded key is port/icahost.
After some debugging, I have identified that the bind port is not set in InitGenesis() when the capability exists (due to import order).

Expected Behaviour

Bind port should always be persisted on genesis import

Version

v8.0.0-alpha.1

Steps to Reproduce

See linked CircleCI run output for the command

Wishlist

Please add simulations to your project, too


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner added the type: bug Something isn't working as expected label Sep 12, 2023
@colin-axner colin-axner added this to the v8.0.0 milestone Sep 12, 2023
@colin-axner
Copy link
Contributor

Thanks for reporting the bug @alpe! Apologies that this issue slipped through and caused you debugging time

For implementer:

I would recommend breaking apart BindPort into setPort and a direct call to k.portKeeper.BindPort. We ran into an issue because the function is handling persisted state and in memory state. The capability keeper is populating the in memory state which is being used to check if the function should be called, but the persisted state should be restored by 27-interchain-accounts regardless.

So we can do something like this:

diff --git a/modules/apps/27-interchain-accounts/controller/keeper/genesis.go b/modules/apps/27-interchain-accounts/controller/keeper/genesis.go
index f41155e8c..3e190a129 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/genesis.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/genesis.go
@@ -12,8 +12,11 @@ import (
 // InitGenesis initializes the interchain accounts controller application state from a provided genesis state
 func InitGenesis(ctx sdk.Context, keeper Keeper, state genesistypes.ControllerGenesisState) {
        for _, portID := range state.Ports {
+               keeper.setPort(ctx, portID)
+
+               // generate port capability if it does not already exist
                if !keeper.hasCapability(ctx, portID) {
-                       capability := keeper.BindPort(ctx, portID)
+                       capability := keeper.portKeeper.BindPort(ctx, portID)
                        if err := keeper.ClaimCapability(ctx, capability, host.PortPath(portID)); err != nil {
                                panic(fmt.Sprintf("could not claim port capability: %v", err))
                        }

I also recommend just calling keeper.portKeeper.BindPort as it is more explicit that the port keeper is generating the capability first and then interchain accounts is claiming the generated capability

Same bug exists in controller, can be done in the same pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
27-interchain-accounts type: bug Something isn't working as expected
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants