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

Panic in core IBC Keeper if staking params are uninitialized #641

Closed
3 tasks
colin-axner opened this issue Dec 16, 2021 · 2 comments · Fixed by #1284
Closed
3 tasks

Panic in core IBC Keeper if staking params are uninitialized #641

colin-axner opened this issue Dec 16, 2021 · 2 comments · Fixed by #1284
Assignees
@catShaark
Labels
good first issue Good for newcomers type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Milestone
Q2-2022-backlog

Comments

@colin-axner
Copy link
Contributor

Summary

If the staking keeper passed to the core IBC Keeper does not have initialized params, our keeper should panic. We need the UnbondingTime to complete connection handshakes

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner mentioned this issue Dec 16, 2021
Closed
@crodriguezvega crodriguezvega added the type: refactor Architecture, code or CI improvements that may or may not tackle technical debt. label Jan 3, 2022
@crodriguezvega
Copy link
Contributor

We need to add the check in this function.

@crodriguezvega crodriguezvega added this to the Q2-2022-backlog milestone Apr 19, 2022
@crodriguezvega crodriguezvega added the good first issue Good for newcomers label Apr 19, 2022
@catShaark
Copy link
Contributor

catShaark commented Apr 21, 2022 •
edited
Loading

If the staking keeper passed to the core IBC Keeper does not have initialized params, our keeper should panic.

I think we should instead check if the passed in keepers is empty structs or not, cuz there's not a way to check in KVStore if the stakingKeeper's params is set without the code looking weird ( I'm talking about using sdk.Context in NewApp() ) and also, If the stakingKeeper's params is not set in the KVStore, there'll definitely be panic anyway.

@catShaark catShaark mentioned this issue Apr 21, 2022
Merged
9 tasks
CosmosCar pushed a commit to caelus-labs/ibc-go that referenced this issue Nov 6, 2023
@tzdybal
Integration with go-header (cosmos#685)
d8679cc 
Initial version of header interface is used to unblock work on go-header library.

Resolves cosmos#641.

<!-- 
Please provide an explanation of the PR, including the appropriate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue. 
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@catShaark catShaark

Labels
good first issue Good for newcomers type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Projects
ibc-go
Archived in project
Milestone
Q2-2022-backlog
Development

Successfully merging a pull request may close this issue.

add empty keepers checking in ibc NewKeeper notional-labs/ibc-go
3 participants
@crodriguezvega @colin-axner @catShaark