Skip to content

feat: upgrade ibc-go v2 to v4 - #1

Open
atheeshp wants to merge 14 commits into
masterfrom
ap/ibc-v2-to-v4
Open

feat: upgrade ibc-go v2 to v4#1
atheeshp wants to merge 14 commits into
masterfrom
ap/ibc-v2-to-v4

Conversation

@atheeshp

@atheeshp atheeshp commented Mar 7, 2023

Copy link
Copy Markdown
Owner

No description provided.

Comment thread x/ibctransfer/module.go Outdated
func (am AppModule) OnAcknowledgementPacket(ctx sdk.Context, packet types.Packet, acknowledgement []byte, relayer sdk.AccAddress) error {
return am.cosmosAppModule.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
}
// func (am AppModule) OnAcknowledgementPacket(ctx sdk.Context, packet types.Packet, acknowledgement []byte, relayer sdk.AccAddress) error {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we still need to implement these methods. Now they are in the separate type IBCModule.

Comment thread app/app.go
app.GetSubspace(disptypes.ModuleName),
)
mockModule := ibcmock.NewAppModule(scopedIBCMockKeeper, &app.IBCKeeper.PortKeeper)
mockModule := ibcmock.NewAppModule(&app.IBCKeeper.PortKeeper)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe I'm missing something but I don't quite understand why is the mock needed....

@snobbee snobbee Oct 27, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@faneaatiku here is an explanation:
https://github.com/atheeshp/sifnode/blob/master/app/app.go#L308-L309

	// NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do
	// note replicate if you do not need to test core IBC or light clients.

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.

6 participants