-
Notifications
You must be signed in to change notification settings - Fork 138
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
Remove consumer chain from provider #124
Changes from 10 commits
af06702
0d5c0a5
4bc3184
764ee6f
9c8f405
96437b3
4467937
4a1b169
80ad51a
04445da
0fa770d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -151,7 +151,21 @@ func (AppModule) ConsensusVersion() uint64 { return 1 } | |
|
||
// BeginBlock implements the AppModule interface | ||
// Set the VSC ID for the subsequent block to the same value as the current block | ||
// Panic if the provider's channel was established and then closed | ||
func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { | ||
mpoke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
channelID, found := am.keeper.GetProviderChannel(ctx) | ||
if found && am.keeper.IsChannelClosed(ctx, channelID) { | ||
// the CCV channel was established, but it was then closed; | ||
// the consumer chain is no longer safe | ||
|
||
// cleanup state | ||
am.keeper.DeleteProviderChannel(ctx) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may require cleaning more of the state, i.e., see all the setters in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jtremback Do we want to be able to restart the consumer chain afterwards? This will impact how we cleanup the state. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems unnecessary here if we're just going to stop the consumer chain? It will require off-chain coordination to restart the chain anyway. So we may as well leave it to off-chain coordination to decide what the new genesis should look like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AdityaSripal Yeah, but we should be cleaning the state as well as we can to facilitate the restart. Is there a reason not to do it? |
||
|
||
channelClosedMsg := fmt.Sprintf("CCV channel %q was closed - shutdown consumer chain since it is not secured anymore", channelID) | ||
ctx.Logger().Error(channelClosedMsg) | ||
panic(channelClosedMsg) | ||
sainoe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
blockHeight := uint64(ctx.BlockHeight()) | ||
vID := am.keeper.GetHeightValsetUpdateID(ctx, blockHeight) | ||
am.keeper.SetHeightValsetUpdateID(ctx, blockHeight+1, vID) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it okay here to return true if the channel ID is not found?
@AdityaSripal is channel state pruned? As long as the consumer chain has the
channedID
, thenchannelKeeper.GetChannel()
should always find the channel, right? If this is true, we should panic in case!found
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Channels are never pruned automatically. Of course, chains could choose to prune closed channels during a state migration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where and why would this be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In BeginBlock to check whether the CCV channel was closed.