-
Notifications
You must be signed in to change notification settings - Fork 656
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
Create Parent Genesis and Tests #418
Create Parent Genesis and Tests #418
Conversation
This pull request introduces 1 alert when merging 8117883 into 6e5ba90 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging c92c55c into 6e5ba90 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging d990260 into 6e5ba90 - view on LGTM.com new alerts:
|
data ccv.ValidatorSetChangePacketData | ||
) | ||
if err := ccv.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { | ||
ack = channeltypes.NewErrorAcknowledgement(fmt.Sprintf("cannot unmarshal CCV packet data: %s", err.Error())) | ||
errAck := channeltypes.NewErrorAcknowledgement(fmt.Sprintf("cannot unmarshal CCV packet data: %s", err.Error())) |
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.
The error acknowledgement message cannot contain the unmarshal json error string
@@ -322,24 +322,21 @@ func (am AppModule) OnRecvPacket( | |||
_ sdk.AccAddress, | |||
) ibcexported.Acknowledgement { | |||
var ( | |||
ack ibcexported.Acknowledgement | |||
ack *channeltypes.Acknowledgement |
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.
why is this a pointer?
data ccv.ValidatorSetChangePacketData | ||
) | ||
if err := ccv.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { | ||
ack = channeltypes.NewErrorAcknowledgement(fmt.Sprintf("cannot unmarshal CCV packet data: %s", err.Error())) | ||
errAck := channeltypes.NewErrorAcknowledgement(fmt.Sprintf("cannot unmarshal CCV packet data: %s", err.Error())) | ||
ack = &errAck |
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.
why is errAck
initialized instead of being assigned directly to ack
} | ||
|
||
ctx.EventManager().EmitEvent( | ||
sdk.NewEvent( | ||
ccv.EventTypePacket, | ||
sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), | ||
sdk.NewAttribute(ccv.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success())), | ||
sdk.NewAttribute(ccv.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack != nil)), |
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.
This returns true even on an error acknowledgement
} | ||
// IterateChannelToChain iterates over the channel to chain mappings and calls the provided callback until the iteration ends | ||
// or the callback returns stop=true | ||
func (k Keeper) IterateChannelToChain(ctx sdk.Context, cb func(ctx sdk.Context, channelID, chainID string) (stop bool)) { |
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.
this func needs a test
} | ||
} | ||
|
||
func (k Keeper) ExportGenesis(ctx sdk.Context) types.ParentGenesisState { |
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.
godoc
Description
closes: #431
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.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes