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

Channel capability not found after chain_id change #613

Closed
3 tasks
V-Staykov opened this issue Dec 9, 2021 · 14 comments
Closed
3 tasks

Channel capability not found after chain_id change #613

V-Staykov opened this issue Dec 9, 2021 · 14 comments

Comments

@V-Staykov
Copy link

V-Staykov commented Dec 9, 2021

Summary of Bug

After I do an ibv-upgrade with changed chain_id to rev 2 and starting height from block 1, I get the following error:

{
    "height": "27",
    "txhash": "F3C47BDE2CFFF22E3FF9CC782F74E7CEABE4B570FDE7E36BAC88063088B032A7",
    "codespace": "channel",
    "code": 9,
    "data": "",
    "raw_log": "failed to execute message; message index: 0: module does not own channel capability: channel capability not found",
    "logs": [],
    "info": "",
    "gas_wanted": "200000",
    "gas_used": "43092",
    "tx": null,
    "timestamp": ""
}

I found a simmilar issue here.

In my case though, creating a new channel and transferring over it works fine.

Version

Cosmos sdk v0.44.3, Ibc-go v1.1, Hermes relayer v0.9

Steps to Reproduce

  1. Get 2 chains and a relayer up.
  2. Create clients and channel between them.
  3. Make an ibc-upgrade proposal, with client state that is exported from chain b and has the following fields edited:
  • chain_id to rev 2
  • trusting_period set to 0
  • frozen_height revision_height to 1 block before the upgrade block
  • latest_height revision_number to 2 and revision_height to 1
  1. Vote and pass the upgrade
  2. When chain a halts, upgrade client from the Hermes relayer
  3. Replace chain a binary with one that has a basic upgrade handler for the upgrade:
	app.UpgradeKeeper.SetUpgradeHandler("val", func(ctx sdk.Context, plan upgradetypes.Plan, a module.VersionMap) (module.VersionMap, error) {
		return a, nil
	})
  1. Export the genesis, change the chain_id and starting-height and replace the old one with it.
  2. unsafe-reset-all and start chain a
  3. Try to send token transfer from chain a to chain b over the existing channel.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner
Copy link
Contributor

Thanks @V-Staykov for raising this issue. I think the issue at hand is unrelated to the linked issue since new channels can be created without error. I have a few questions which will hopefully allow us to narrow in on the possible problem

The ibc-upgrade proposal part looks correct to me, but what I don't follow is why you do a genesis state restart and set the upgrade handler. The upgrade handler only needs to be set if an in-place upgrade is to occur (otherwise you can remove the upgrade proposal after exporting genesis). Also, is there a particular reason you choose to do a genesis dump state restart as opposed to using the in-place upgrade?

The binary is unchanged except for the upgrade handler added?

What is interesting is that the error returned is unrelated to the client upgrade. If there was an issue with the ibc-upgrade proposal, we would get an error on chain B. Thus it sounds like there might be an issue with exporting capabilities. Are you certain the same channel id is being used for the transfer? Can you show me your app.go file? Is capabilities the first module to be run on begin blocker after upgrade?

@V-Staykov
Copy link
Author

V-Staykov commented Dec 9, 2021

Hi @colin-axner , thanks for the fast reply.
I am setting the upgrade handles out of a habit, because it requires it to start the chain after an upgrade, but you are right - I don't need it. I actually forgot to mention that I delete the proposal from the new genesis anyways.

I am doing a genesis dump state restart, because I wanted to test the process basically.

The binary is unchanged, except for the upgrade handler added.

Yes I make sure I use the same channel id, there is no other anyways. The capabilities is NOT on begin blocker before and after the upgrade.

Here is a link to the app.go.

@colin-axner
Copy link
Contributor

Could you add the capabilities begin blocker after upgrade and retry to send the packet? No need to do the whole process again, just start the chain with the updated binary and attempt to send the packet

@V-Staykov
Copy link
Author

Tried it - it is the same.

@V-Staykov
Copy link
Author

V-Staykov commented Dec 10, 2021

As @colin-axner mentiont it is most probably an issue with exporting/importing capabilities. I just tried just to export genesis, unsafe reset all and start the chain again, without any changes to it and the same problem occurs.

Also accidently found out that after this erro occurs, if I just stop and start again the node, the error disappears and the tx passes...

@colin-axner
Copy link
Contributor

@V-Staykov can you link me the genesis file after exporting?

Also accidently found out that after this erro occurs, if I just stop and start again the node, the error disappears and the tx passes...

Very interesting, sounds like the capabilities are not being initialized in memory, but then when you restart they are via BeginBlock. Why they aren't being initialized in InitGenesis is beyond me.

@colin-axner
Copy link
Contributor

Also can you give me the exact ibc-go version? ibc-go v1.1.0 does not use v0.44.3 of the SDK

@V-Staykov
Copy link
Author

V-Staykov commented Dec 10, 2021

Yes, here is the genesis. I did check it and the capabilities there seem fine. Also I started printing some stuff in the GetCapability in capability's keeper.go and found out that the indexBytes in this code returns [] after the first start. Other than that the name is correct and all the data from the genesis seems correctly imported.

	memStore := ctx.KVStore(sk.memKey)
	key := types.RevCapabilityKey(sk.module, name)
	indexBytes := memStore.Get(key)
        fmt.Println(indexBytes)
	index := sdk.BigEndianToUint64(indexBytes)

The ibc-go version is indeed v1.1.0, I meant we use 0.44.3 of the sdk for our modules. Maybe that is a problem? I could try to convert ibc-go to v2 and try like that, although it will take me some time. Also the Hermes doesn't yer work with V2, so I am reluctant to do it.

@colin-axner
Copy link
Contributor

colin-axner commented Dec 10, 2021

The link appears to be incorrect. That is the JSON for the upgraded client state in the upgrade proposal, I think. It isn't a genesis file

What is the value of index in the genesis file?

The ibc-go version is indeed v1.1.0, I meant we use 0.44.3 of the sdk for our modules. Maybe that is a problem? I could try to convert ibc-go to v2 and try like that, although it will take me some time. Also the Hermes doesn't yer work with V2, so I am reluctant to do it.

No this is fine, it just means v0.44.0 is being used by ibc-go - I'm fairly certain.

@V-Staykov
Copy link
Author

Sry about the link, I did fix it.

@colin-axner
Copy link
Contributor

Thanks for fixing the link. I see two entries for the same capability:

"owners": [{
                "index": "1",
                "index_owners": {
                    "owners": [{
                        "module": "ibc",
                        "name": "ports/transfer"
                    }, {
                        "module": "transfer",
                        "name": "ports/transfer"
                    }]
                }
            },
            ...
             {
                "index": "3",
                "index_owners": {
                    "owners": [{
                        "module": "ibc",
                        "name": "ports/transfer"
                    }, {
                        "module": "transfer",
                        "name": "ports/transfer"
                    }]
                }
            }]

Could you try removing the index: 3? Did you modify the genesis file at all? It seems problematic to have duplicate entries

@V-Staykov
Copy link
Author

I tried but it's the same. I did test though - exporting the genesis just adds this another time, no matter if it exists already. So it was in this file basically because I had done export a few times already.

@colin-axner
Copy link
Contributor

Hmm, this is very odd, but seems to be related to capability issues we have run into in the past. I will try to reproduce this issue on Monday using the simapp in ibc-go. If it works using our simapp, I'll try to reproduce using your binary

@V-Staykov
Copy link
Author

Okay, so I managed to find the problem. Basically it was a problem on our side. On SDK v0.42.9 a bug was fixed with InitializeAndSeal method and the initialization was moved to InitGenezis and BeginBlocker. On our side we didn't correctly migrate and basically manually replased the InitializeAndSeal in this code in app.go

if loadLatest {
		if err := app.LoadLatestVersion(); err != nil {
			tmos.Exit(err.Error())
		}

		// Initialize and seal the capability keeper so all persistent capabilities
		// are loaded in-memory and prevent any further modules from creating scoped
		// sub-keepers.
		// This must be done during creation of baseapp rather than in InitChain so
		// that in-memory capabilities get regenerated on app restart.
		// Note that since this reads from the store, we can only perform it when
		// `loadLatest` is set to true.
		ctx := app.BaseApp.NewUncachedContext(true, tmproto.Header{})
		app.CapabilityKeeper.InitMemStore(ctx)
		app.CapabilityKeeper.Seal()
	}

What happend was that the InitMemStore was setting an initialized flag to true, before the owners were set in init genesis. Then the init genesis was setting the owners, but when trying to init the mem store, it already had it's flag set to initialized.

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

No branches or pull requests

2 participants