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.

Sign up for GitHub

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

Jump to bottom

Empty ICA state causes a panic on ExportGenesis #2151

Closed
3 tasks
technicallyty opened this issue Aug 31, 2022 · 3 comments · Fixed by #2160
Closed
3 tasks

Empty ICA state causes a panic on ExportGenesis #2151

technicallyty opened this issue Aug 31, 2022 · 3 comments · Fixed by #2160
Assignees
@colin-axner
Labels
27-interchain-accounts
Milestone
v5.0.0

Comments

@technicallyty
Copy link
Contributor

technicallyty commented Aug 31, 2022 •
edited
Loading

Summary of Bug

If no initial state is given to the module, calling ExportGenesis on the module, or the app in general, will cause a panic, due to this line, which gets the params from the paramKeeper subspace. subspace.Get causes a panic if the parameter does not yet exist.

Expected Behaviour

ExportGenesis should not panic if the module has no state. It might be more preferable to use GetIfExists when getting the parameters for exporting state to prevent such errors.

Version

v5

Steps to Reproduce

  • add ICA to new chain
  • do not import any state or set any params
  • export genesis on the chain

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@technicallyty technicallyty mentioned this issue Aug 31, 2022
Merged
19 tasks
@colin-axner
Copy link
Contributor

Hi @technicallyty, thanks for opening the issue. Based on your comment this looks like a duplicate issue of #1352. Is that correct?

I believe it is expected that a panic will occur if an SDK module does not have its state initialized

This was referenced Aug 31, 2022
Merged
Merged
Merged
mergify bot pushed a commit that referenced this issue Aug 31, 2022
@colin-axner
feat: add decoder store to ics27 simulations (#2158)
23a7515 
## Description



ref: #1352, #2151 

---

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 31, 2022
@colin-axner @mergify
feat: add decoder store to ics27 simulations (#2158)
a3a3365 
## Description

ref: #1352, #2151

---

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 23a7515)
mergify bot pushed a commit that referenced this issue Aug 31, 2022
@colin-axner @mergify
feat: add decoder store to ics27 simulations (#2158)
397cda3 
## Description

ref: #1352, #2151

---

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 23a7515)
mergify bot pushed a commit that referenced this issue Aug 31, 2022
@colin-axner
feat: add genesis simulation generation for ics27 (#2154)
eb508b5 
## Description



ref: #1352, #2151

---

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.

- [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] 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 31, 2022
@colin-axner @mergify
feat: add genesis simulation generation for ics27 (#2154)
6c4b11c 
## Description

ref: #1352, #2151

---

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.

- [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] 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 eb508b5)

# Conflicts:
#	CHANGELOG.md
#	modules/apps/27-interchain-accounts/module.go
mergify bot pushed a commit that referenced this issue Aug 31, 2022
@colin-axner @mergify
feat: add genesis simulation generation for ics27 (#2154)
70d734e 
## Description

ref: #1352, #2151

---

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.

- [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] 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 eb508b5)

# Conflicts:
#	CHANGELOG.md
#	modules/apps/27-interchain-accounts/module.go
@technicallyty
Copy link
Contributor Author

technicallyty commented Aug 31, 2022 •
edited
Loading

Hi @technicallyty, thanks for opening the issue. Based on your comment this looks like a duplicate issue of #1352. Is that correct?

sort of - i'm moreso concerned with the panic on empty state during ExportGenesis, if that gets fixed by changing the param get method used or implementing sims -- either would suffice for me

I believe it is expected that a panic will occur if an SDK module does not have its state initialized

modules in the SDK commonly have panic-safe code when getting params for ExportGenesis. see: x/bank, x/auth, x/distribution, x/staking etc

damiannolan pushed a commit that referenced this issue Aug 31, 2022
@mergify @colin-axner
feat: add decoder store to ics27 simulations (#2158) (#2163)
8045fcc 
## Description

ref: #1352, #2151

---

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 23a7515)

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
damiannolan pushed a commit that referenced this issue Aug 31, 2022
@mergify @colin-axner
feat: add decoder store to ics27 simulations (#2158) (#2164)
3a9b253 
## Description

ref: #1352, #2151

---

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 23a7515)

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
@crodriguezvega crodriguezvega added this to the v5.0.0 milestone Sep 6, 2022
@colin-axner
Copy link
Contributor

We will implement sims for >= v5 and then second concern will be addressed when we migrate our params to be self managed #2010

I believe the SDK traditionally panic'd on unset params (this is from v0.46.1) and that this behaviour changed when it migrated to self managed params. I agree this behaviour is incorrect as one should not cause ExportGenesis to panic. It seems like a very unlikely edge case as the app.go would have to be misconfigured (not calling app.mm.InitGenesis) so I don't see reason to prioritize this change

colin-axner added a commit that referenced this issue Sep 7, 2022
@mergify @colin-axner
feat: add genesis simulation generation for ics27 (backport #2154) (#…
5f10b75 
…2168)

* feat: add genesis simulation generation for ics27 (#2154)

## Description

ref: #1352, #2151

---

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.

- [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] 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 eb508b5)

# Conflicts:
#	CHANGELOG.md
#	modules/apps/27-interchain-accounts/module.go

* fix conflicts

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
colin-axner added a commit that referenced this issue Sep 8, 2022
@mergify @colin-axner
feat: add genesis simulation generation for ics27 (backport #2154) (#…
d3f4b54 
…2169)

* feat: add genesis simulation generation for ics27 (#2154)

## Description

ref: #1352, #2151

---

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.

- [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] 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 eb508b5)

# Conflicts:
#	CHANGELOG.md
#	modules/apps/27-interchain-accounts/module.go

* fix conflicts

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@colin-axner colin-axner

Labels
27-interchain-accounts
Projects
ibc-go
Status: Done 🥳
Milestone
v5.0.0
Development

Successfully merging a pull request may close this issue.

feat: add params for ics27 simulation cosmos/ibc-go
3 participants
@crodriguezvega @colin-axner @technicallyty