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

Add the ability to inject message routers that support ADR 031 #2044

Closed
3 tasks
Taztingo opened this issue Aug 18, 2022 · 1 comment · Fixed by #2058
Closed
3 tasks

Add the ability to inject message routers that support ADR 031 #2044

Taztingo opened this issue Aug 18, 2022 · 1 comment · Fixed by #2058
Assignees
Labels
27-interchain-accounts change: api breaking Issues or PRs that break Go API (need to be release in a new major version) type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Milestone

Comments

@Taztingo
Copy link
Contributor

Taztingo commented Aug 18, 2022

Summary

The ibc-go submodule keepers do not allow custom message routing, and we are forced to use baseapp.MsgServiceRouter. It would be very helpful to allow injection of custom routers that support ADR 031 request type routing.

Problem Definition

This is an issue for projects that need use their own router. ICA is unusable to them, and is currently blocking them

Proposal

Allow users to inject routers that support ADR 031 into icacontroller.keeper and icahost.keeper. A great example of this is how wasmd added the MessageRouter interface. CosmWasm/wasmd#717


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@Taztingo Taztingo changed the title Injectable message routers that support ADR 031 Add Injectable message routers that support ADR 031 Aug 18, 2022
@Taztingo Taztingo changed the title Add Injectable message routers that support ADR 031 Add the ability to inject message routers that support ADR 031 Aug 18, 2022
@crodriguezvega crodriguezvega added 27-interchain-accounts type: refactor Architecture, code or CI improvements that may or may not tackle technical debt. change: api breaking Issues or PRs that break Go API (need to be release in a new major version) labels Aug 19, 2022
@colin-axner
Copy link
Contributor

Ah yes! Nice suggestion, sorry we didn't implement this from the start!

@crodriguezvega crodriguezvega added this to the v5.0.0 milestone Aug 22, 2022
@mergify mergify bot closed this as completed in #2058 Aug 23, 2022
mergify bot pushed a commit that referenced this issue Aug 23, 2022
## Description

Updated the controller and host keeper to take in an interface for the router. This allows systems that implement ADR 031, but do not use the default baseapp.router to use ICA.

closes: #2044 

---

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](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes
mergify bot pushed a commit that referenced this issue Aug 23, 2022
## Description

Updated the controller and host keeper to take in an interface for the router. This allows systems that implement ADR 031, but do not use the default baseapp.router to use ICA.

closes: #2044

---

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](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes

(cherry picked from commit 397b88c)

# Conflicts:
#	CHANGELOG.md
#	modules/apps/27-interchain-accounts/controller/keeper/keeper.go
#	modules/apps/27-interchain-accounts/host/keeper/keeper.go
mergify bot pushed a commit that referenced this issue Aug 23, 2022
## Description

Updated the controller and host keeper to take in an interface for the router. This allows systems that implement ADR 031, but do not use the default baseapp.router to use ICA.

closes: #2044

---

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](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes

(cherry picked from commit 397b88c)

# Conflicts:
#	CHANGELOG.md
#	modules/apps/27-interchain-accounts/controller/keeper/keeper.go
#	modules/apps/27-interchain-accounts/host/keeper/keeper.go
damiannolan added a commit that referenced this issue Aug 24, 2022
#2058) (#2088)

* Added interface to accept any type of ADR 031 message router.  (#2058)

## Description

Updated the controller and host keeper to take in an interface for the router. This allows systems that implement ADR 031, but do not use the default baseapp.router to use ICA.

closes: #2044

---

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](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes

(cherry picked from commit 397b88c)

# Conflicts:
#	CHANGELOG.md
#	modules/apps/27-interchain-accounts/controller/keeper/keeper.go
#	modules/apps/27-interchain-accounts/host/keeper/keeper.go

* fix conflict

* fix conflict

* fix conflict

* Update keeper.go

* Update keeper.go

Co-authored-by: Matt Witkowski <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Damian Nolan <[email protected]>
damiannolan added a commit that referenced this issue Aug 24, 2022
#2058) (#2087)

* Added interface to accept any type of ADR 031 message router.  (#2058)

## Description

Updated the controller and host keeper to take in an interface for the router. This allows systems that implement ADR 031, but do not use the default baseapp.router to use ICA.

closes: #2044

---

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](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes

(cherry picked from commit 397b88c)

# Conflicts:
#	CHANGELOG.md
#	modules/apps/27-interchain-accounts/controller/keeper/keeper.go
#	modules/apps/27-interchain-accounts/host/keeper/keeper.go

* fix conflict

* fix conflict

* Update keeper.go

* Update keeper.go

* Update keeper.go

Co-authored-by: Matt Witkowski <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Damian Nolan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
27-interchain-accounts change: api breaking Issues or PRs that break Go API (need to be release in a new major version) type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants