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

backport: gov reopen/close to v1.2.5 #349

Closed
wants to merge 6 commits into from

Conversation

aljo242
Copy link

@aljo242 aljo242 commented Mar 14, 2023

1. Summary

Fixes QCK-174

  • Backport GovReopenChannel and GovCloseChannel

  • Add InterchainStakingKeeper.SetEpochsKeeper() to prevent a panic

Note: we should add test for the msg_server functionality in the future.

@aljo242 aljo242 changed the title generate backport: gov reopen/close to v1.2.5 Mar 14, 2023
@@ -588,14 +589,17 @@ func (s *KeeperTestSuite) TestHandleRewardsCallbackEmptyResponse() {

respbz, err := app.AppCodec().Marshal(&response)
s.Require().NoError(err)

epoch := app.EpochsKeeper.GetEpochInfo(ctx, "epoch")
fmt.Println(epoch)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug

@@ -634,3 +639,7 @@ func DetermineAllocationsForRebalancing(currentAllocations map[string]math.Int,

return out
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are adding setEpochsKeeper, do we need to remove epochkeeper from NewKeeper (I added in v1.2.5)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we should remove. We need to use setEpochsKeeper to ensure the order of keeper init is valid

func (k msgServer) GovReopenChannel(goCtx context.Context, msg *types.MsgGovReopenChannel) (*types.MsgGovReopenChannelResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)

// checking msg authority is the gov module address
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer a proposal, so can remove these commented lines

Copy link
Contributor

@joe-bowman joe-bowman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of comments - can you also add the cli tx command for reopen

@ajansari95 ajansari95 mentioned this pull request Mar 15, 2023
2 tasks
@aljo242 aljo242 closed this Mar 15, 2023
@aljo242 aljo242 deleted the backport/gov-channel branch March 15, 2023 11:59
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

Successfully merging this pull request may close these issues.

2 participants