proposal: capability addition to ibc-go structure

[tac0turtle]

Description

This is a proposal on how to adopt capability. In the near future we will be able to remove the dependency of the sdk from this module making it standalone. The reason for the separate go.mod is that this module will most likely hit v1 and not iterate further with the plans of the sdk to introduce internal message routers. Tying it to an ibc-go release could be done as well, but didnt see the point since there wont be iteration on this work for a while.

before going further wanted to double check if this is acceptable

closes: #3076

Commit Message / Changelog Entry

feat: migrate capability module from cosmos-sdk to ibc-go

see the guidelines for commit messages. (view raw markdown for examples)

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Targeted PR against correct branch (see CONTRIBUTING.md).
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the module structure standards and Go style guide.
• Wrote unit and integration tests.
• Updated relevant documentation (docs/) or specification (x/<module>/spec/).
• Added relevant godoc comments.
• Provide a commit message to be used for the changelog entry in the PR description for review.
• Re-reviewed Files changed in the Github PR explorer.
• Review Codecov Report in the comment section below once CI passes.

[tac0turtle]

any chance i can get feedback on this? would like to move forward while its still fresh in my mind

[crodriguezvega]

any chance i can get feedback on this? would like to move forward while its still fresh in my mind

Thank you for helping us with this work, @tac0turtle. We'll try to review this or next week, if that's ok.

[damiannolan]

We can take over this PR and make any changes required to get it merged.

• I think we need to update errors to use errorsmod instead of sdkerrors.
• Copy over protos and regenerate (I think the current .pb.go files are copied directly)

I'd like to suggest we move capability from the modules directory to the root of the repo, and then tag as: capability/v1.0.0

edit: we decided to keep it under modules

[chatton]

We made the followup updates to this PR:

• Added proto files (and regenerated them).
• Moved capability module up one directory as @damiannolan suggested.
• Updated sdk to tip of rc2 release branch.
• Updated tendermint references to point at cometbft

We can't fully test/merge this until this PR is merged (which will be updating interchain test to cometbft) and then we can update ibc-go to use cometbft as well, which will then let us update references to this new module and run our E2E.

[chatton]

Myself and @damiannolan did some more work to push this forward, we ran into a lot of issues which need further debugging but we were able to get everything building and tests passing using a local pin of the new go module.

1. Because we are using a local pin, we needed to modify the Dockerfile to ADD the modules directory before calling go mod download, this can be reverted when we are not on a local pin.
2. Capabilities errors were failing to register as the duplicate module/code pairs are already registered in the 0.47-rc3 tag of the sdk. As a workaround ,we have simply changed the codes.
3. We needed to pin the iavl module to v0.20.0-alpha4. (down from v0.21.0-alpha.1), without this pin, we get the following error

go build ./...
# github.com/cosmos/cosmos-sdk/store/iavl
vendor/github.com/cosmos/cosmos-sdk/store/iavl/tree.go:11:11: cannot use (*immutableTree)(nil) (value of type *immutableTree) as type Tree in variable declaration:
        *immutableTree does not implement Tree (wrong type for Iterator method)
                have Iterator(start []byte, end []byte, ascending bool) ("github.com/cosmos/cosmos-db".Iterator, error)
                want Iterator(start []byte, end []byte, ascending bool) ("github.com/cometbft/cometbft-db".Iterator, error)
vendor/github.com/cosmos/cosmos-sdk/store/iavl/tree.go:12:11: cannot use (*iavl.MutableTree)(nil) (value of type *"github.com/cosmos/iavl".MutableTree) as type Tree in variable declaration:
        *"github.com/cosmos/iavl".MutableTree does not implement Tree (wrong type for Iterator method)
                have Iterator(start []byte, end []byte, ascending bool) ("github.com/cosmos/cosmos-db".Iterator, error)
                want Iterator(start []byte, end []byte, ascending bool) ("github.com/cometbft/cometbft-db".Iterator, error)
vendor/github.com/cosmos/cosmos-sdk/store/iavl/store.go:55:43: cannot use db (variable of type "github.com/cometbft/cometbft-db".DB) as type "github.com/cosmos/cosmos-db".DB in argument to iavl.NewMutableTreeWithOpts:
        "github.com/cometbft/cometbft-db".DB does not implement "github.com/cosmos/cosmos-db".DB (wrong type for Iterator method)
                have Iterator(start []byte, end []byte) ("github.com/cometbft/cometbft-db".Iterator, error)
                want Iterator(start []byte, end []byte) ("github.com/cosmos/cosmos-db".Iterator, error)
vendor/github.com/cosmos/cosmos-sdk/store/iavl/store.go:90:11: cannot use tree (variable of type *"github.com/cosmos/iavl".MutableTree) as type Tree in struct literal:
        *"github.com/cosmos/iavl".MutableTree does not implement Tree (wrong type for Iterator method)
                have Iterator(start []byte, end []byte, ascending bool) ("github.com/cosmos/cosmos-db".Iterator, error)
                want Iterator(start []byte, end []byte, ascending bool) ("github.com/cometbft/cometbft-db".Iterator, error)
vendor/github.com/cosmos/cosmos-sdk/store/iavl/store.go:103:9: cannot use tree (variable of type *"github.com/cosmos/iavl".MutableTree) as type Tree in struct literal:
        *"github.com/cosmos/iavl".MutableTree does not implement Tree (wrong type for Iterator method)
                have Iterator(start []byte, end []byte, ascending bool) ("github.com/cosmos/cosmos-db".Iterator, error)
                want Iterator(start []byte, end []byte, ascending bool) ("github.com/cometbft/cometbft-db".Iterator, error)
vendor/github.com/cosmos/cosmos-sdk/store/iavl/store.go:123:9: cannot use &immutableTree{…} (value of type *immutableTree) as type Tree in struct literal:
        *immutableTree does not implement Tree (wrong type for Iterator method)
                have Iterator(start []byte, end []byte, ascending bool) ("github.com/cosmos/cosmos-db".Iterator, error)
                want Iterator(start []byte, end []byte, ascending bool) ("github.com/cometbft/cometbft-db".Iterator, error)
vendor/github.com/cosmos/cosmos-sdk/store/iavl/store.go:277:14: impossible type assertion: istore.tree.(*immutableTree)
        *immutableTree does not implement Tree (wrong type for Iterator method)
                have Iterator(start []byte, end []byte, ascending bool) ("github.com/cosmos/cosmos-db".Iterator, error)
                want Iterator(start []byte, end []byte, ascending bool) ("github.com/cometbft/cometbft-db".Iterator, error)
vendor/github.com/cosmos/cosmos-sdk/store/iavl/store.go:286:14: impossible type assertion: st.tree.(*iavl.MutableTree)
        *"github.com/cosmos/iavl".MutableTree does not implement Tree (wrong type for Iterator method)
                have Iterator(start []byte, end []byte, ascending bool) ("github.com/cosmos/cosmos-db".Iterator, error)
                want Iterator(start []byte, end []byte, ascending bool) ("github.com/cometbft/cometbft-db".Iterator, error)

4. We needed to pin ledger-cosmos-go to v0.12.1 (the version sdk 47 uses). This was being indirectly pulled in at version v0.13.0
   The error we got before pinning was

make build
go build -mod=readonly -tags "netgo ledger" -ldflags '-X github.com/cosmos/cosmos-sdk/version.Name=sim -X github.com/cosmos/cosmos-sdk/version.AppName=simd -X github.com/cosmos/cosmos-sdk/version.Version=02-client-refactor-beta1-171-gd9774d95d -X github.com/cosmos/cosmos-sdk/version.Commit=d9774d95dd61720cb9327c2d0630e879062cf822 -X "github.com/cosmos/cosmos-sdk/version.BuildTags=netgo,ledger" -w -s' -trimpath -o /Users/chatton/checkouts/ibc-go/build/ ./...
# github.com/cosmos/cosmos-sdk/crypto/ledger
../../go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.47.0-rc3/crypto/ledger/ledger_real.go:20:10: cannot use device (variable of type *ledger_cosmos_go.LedgerCosmos) as type SECP256K1 in return statement:
        *ledger_cosmos_go.LedgerCosmos does not implement SECP256K1 (wrong type for SignSECP256K1 method)
                have SignSECP256K1(bip32Path []uint32, transaction []byte, p2 byte) ([]byte, error)
                want SignSECP256K1([]uint32, []byte) ([]byte, error)
make: *** [build] Error 2

5. Some of the unit tests are failing for unknown reasons and have been commented out. These need to be debugged and fixed.

[damiannolan]

LGTM. This looks good to go to me.

App v2 wiring has been ripped out and test code has been adjusted to use the ibc-go simapp. I've also added the fix cosmos/cosmos-sdk#15030

CI is green, new tasks added for build/test capability module. All E2E's look good with simapp wired with capability from this PR. Linting is failing but addressed in #3311 (maybe we'll need to lint the capability code also, but that will come or could be ignored in linter config).

Thanks @tac0turtle @chatton.

[chatton]

Changes seem fine to me, but I will leave final approval to someone else on the team who was not as involved in adding it!

Thanks @damiannolan / @tac0turtle

[colin-axner]

Thank you @damiannolan and @chatton!!! Excellent work! 💯

I have reviewed the diffs against SDK v0.47 release and I think I found a few merge conflicts that got duplicated. Otherwise everything looks great to me. I think it'd be nice if the go.mod didn't have to pin core IBC just for testing (will we remove this for releases?). I think this could be addressed by making simapp its own go.mod

[colin-axner]

Why does capability module reference ibc-go? I guess for tests? Will this cause versioning issues?

[chatton]

it is just for tests, so when a chain imports the module, it won't be pulled in as a dependency

To test this I created a go.mod file that just imports the capability module.

require github.com/cosmos/ibc-go/modules/capability v1.0.0-alpha1

require (
	// no sign of ibc-go
)

Go does some clever things with dependences that are only used in test code.

[colin-axner]

Nice job y'all! Everything looks good to go to me!