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

Create Parent Genesis and Tests #418

Merged
merged 5 commits into from
Sep 22, 2021

Conversation

AdityaSripal
Copy link
Member

@AdityaSripal AdityaSripal commented Sep 16, 2021

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.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • 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

@lgtm-com
Copy link

lgtm-com bot commented Sep 16, 2021

This pull request introduces 1 alert when merging 8117883 into 6e5ba90 - view on LGTM.com

new alerts:

  • 1 for Impossible interface nil check

@lgtm-com
Copy link

lgtm-com bot commented Sep 22, 2021

This pull request introduces 1 alert when merging c92c55c into 6e5ba90 - view on LGTM.com

new alerts:

  • 1 for Impossible interface nil check

@AdityaSripal AdityaSripal enabled auto-merge (squash) September 22, 2021 10:07
@lgtm-com
Copy link

lgtm-com bot commented Sep 22, 2021

This pull request introduces 1 alert when merging d990260 into 6e5ba90 - view on LGTM.com

new alerts:

  • 1 for Impossible interface nil check

@AdityaSripal AdityaSripal enabled auto-merge (squash) September 22, 2021 10:54
@AdityaSripal AdityaSripal merged commit 42b7b3f into cross-chain-validation Sep 22, 2021
@AdityaSripal AdityaSripal deleted the aditya/ccv-parent-genesis branch September 22, 2021 12:08
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()))
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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)),
Copy link
Contributor

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)) {
Copy link
Contributor

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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

godoc

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