Add IBC hooks for every IBC method #2296
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| package ibc_hooks | ||
|
|
||
| import ( | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" | ||
| channeltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types" | ||
| ibcexported "github.com/cosmos/ibc-go/v5/modules/core/exported" | ||
| ) | ||
|
|
||
| type IBCAppHooks interface { | ||
|
There was a problem hiding this comment. Wondering if we could shave off the |
||
| } | ||
|
|
||
| type IBCAppHooksOnChanOpenInitOverride interface { | ||
| OnChanOpenInitOverride(im IBCMiddleware, ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID string, channelID string, channelCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string) (string, error) | ||
|
There was a problem hiding this comment. nit: do you mind to put each argument on a new line (like we do here)? I think it's easier for readability. There was a problem hiding this comment. changed it to only one line specifically for readability (so it's easy to scan which methods are defined on each interface, considering the arguments are the same as for the functions they override), but happy to change it back to one arg per line |
||
| } | ||
| type IBCAppHooksOnChanOpenInitBefore interface { | ||
| OnChanOpenInitBeforeHook(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID string, channelID string, channelCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string) | ||
| } | ||
| type IBCAppHooksOnChanOpenInitAfter interface { | ||
| OnChanOpenInitAfterHook(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID string, channelID string, channelCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string, result string, err error) | ||
| } | ||
|
|
||
| // OnChanOpenTry Hooks | ||
| type IBCAppHooksOnChanOpenTryOverride interface { | ||
| OnChanOpenTryOverride(im IBCMiddleware, ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID, channelID string, channelCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, counterpartyVersion string) (string, error) | ||
| } | ||
| type IBCAppHooksOnChanOpenTryBefore interface { | ||
| OnChanOpenTryBeforeHook(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID, channelID string, channelCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, counterpartyVersion string) | ||
| } | ||
| type IBCAppHooksOnChanOpenTryAfter interface { | ||
| OnChanOpenTryAfterHook(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID, channelID string, channelCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, counterpartyVersion string, version string, err error) | ||
| } | ||
|
|
||
| // OnChanOpenAck Hooks | ||
| type IBCAppHooksOnChanOpenAckOverride interface { | ||
| OnChanOpenAckOverride(im IBCMiddleware, ctx sdk.Context, portID, channelID string, counterpartyChannelID string, counterpartyVersion string) error | ||
| } | ||
| type IBCAppHooksOnChanOpenAckBefore interface { | ||
| OnChanOpenAckBeforeHook(ctx sdk.Context, portID, channelID string, counterpartyChannelID string, counterpartyVersion string) | ||
| } | ||
| type IBCAppHooksOnChanOpenAckAfter interface { | ||
| OnChanOpenAckAfterHook(ctx sdk.Context, portID, channelID string, counterpartyChannelID string, counterpartyVersion string, err error) | ||
| } | ||
|
|
||
| // OnChanOpenConfirm Hooks | ||
| type IBCAppHooksOnChanOpenConfirmOverride interface { | ||
| OnChanOpenConfirmOverride(im IBCMiddleware, ctx sdk.Context, portID, channelID string) error | ||
| } | ||
| type IBCAppHooksOnChanOpenConfirmBefore interface { | ||
| OnChanOpenConfirmBeforeHook(ctx sdk.Context, portID, channelID string) | ||
| } | ||
| type IBCAppHooksOnChanOpenConfirmAfter interface { | ||
| OnChanOpenConfirmAfterHook(ctx sdk.Context, portID, channelID string, err error) | ||
| } | ||
|
|
||
| // OnChanCloseInit Hooks | ||
| type IBCAppHooksOnChanCloseInitOverride interface { | ||
| OnChanCloseInitOverride(im IBCMiddleware, ctx sdk.Context, portID, channelID string) error | ||
| } | ||
| type IBCAppHooksOnChanCloseInitBefore interface { | ||
| OnChanCloseInitBeforeHook(ctx sdk.Context, portID, channelID string) | ||
| } | ||
| type IBCAppHooksOnChanCloseInitAfter interface { | ||
| OnChanCloseInitAfterHook(ctx sdk.Context, portID, channelID string, err error) | ||
| } | ||
|
|
||
| // OnChanCloseConfirm Hooks | ||
| type IBCAppHooksOnChanCloseConfirmOverride interface { | ||
| OnChanCloseConfirmOverride(im IBCMiddleware, ctx sdk.Context, portID, channelID string) error | ||
| } | ||
| type IBCAppHooksOnChanCloseConfirmBefore interface { | ||
| OnChanCloseConfirmBeforeHook(ctx sdk.Context, portID, channelID string) | ||
| } | ||
| type IBCAppHooksOnChanCloseConfirmAfter interface { | ||
| OnChanCloseConfirmAfterHook(ctx sdk.Context, portID, channelID string, err error) | ||
| } | ||
|
|
||
| // OnAcknowledgementPacket Hooks | ||
|
There was a problem hiding this comment. I think I would change the order and put the |
||
| type IBCAppHooksOnAcknowledgementPacketOverride interface { | ||
| OnAcknowledgementPacketOverride(im IBCMiddleware, ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error | ||
| } | ||
| type IBCAppHooksOnAcknowledgementPacketBefore interface { | ||
| OnAcknowledgementPacketBeforeHook(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) | ||
| } | ||
| type IBCAppHooksOnAcknowledgementPacketAfter interface { | ||
| OnAcknowledgementPacketAfterHook(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, err error) | ||
| } | ||
|
|
||
| // OnTimeoutPacket Hooks | ||
| type IBCAppHooksOnTimeoutPacketOverride interface { | ||
| OnTimeoutPacketOverride(im IBCMiddleware, ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) error | ||
| } | ||
| type IBCAppHooksOnTimeoutPacketBefore interface { | ||
| OnTimeoutPacketBeforeHook(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) | ||
| } | ||
| type IBCAppHooksOnTimeoutPacketAfter interface { | ||
| OnTimeoutPacketAfterHook(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, err error) | ||
| } | ||
|
|
||
| // OnRecvPacket Hooks | ||
| type IBCAppHooksOnRecvPacketOverride interface { | ||
| OnRecvPacketOverride(im IBCMiddleware, ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) ibcexported.Acknowledgement | ||
| } | ||
| type IBCAppHooksOnRecvPacketBefore interface { | ||
| OnRecvPacketBeforeHook(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) | ||
| } | ||
| type IBCAppHooksOnRecvPacketAfter interface { | ||
| OnRecvPacketAfterHook(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, ack ibcexported.Acknowledgement) | ||
| } | ||
|
|
||
| // SendPacket Hooks | ||
| type IBCAppHooksSendPacketOverride interface { | ||
| SendPacketOverride(im IBCMiddleware, ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI) error | ||
| } | ||
| type IBCAppHooksSendPacketBefore interface { | ||
| SendPacketBeforeHook(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI) | ||
| } | ||
| type IBCAppHooksSendPacketAfter interface { | ||
| SendPacketAfterHook(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, err error) | ||
| } | ||
|
|
||
| // WriteAcknowledgement Hooks | ||
| type IBCAppHooksWriteAcknowledgementOverride interface { | ||
| WriteAcknowledgementOverride(im IBCMiddleware, ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, ack ibcexported.Acknowledgement) error | ||
| } | ||
| type IBCAppHooksWriteAcknowledgementBefore interface { | ||
| WriteAcknowledgementBeforeHook(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, ack ibcexported.Acknowledgement) | ||
| } | ||
| type IBCAppHooksWriteAcknowledgementAfter interface { | ||
| WriteAcknowledgementAfterHook(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, ack ibcexported.Acknowledgement, err error) | ||
| } | ||
|
|
||
| // GetAppVersion Hooks | ||
| type IBCAppHooksGetAppVersionOverride interface { | ||
| GetAppVersionOverride(im IBCMiddleware, ctx sdk.Context, portID, channelID string) (string, bool) | ||
| } | ||
| type IBCAppHooksGetAppVersionBefore interface { | ||
| GetAppVersionBeforeHook(ctx sdk.Context, portID, channelID string) | ||
| } | ||
| type IBCAppHooksGetAppVersionAfter interface { | ||
| GetAppVersionAfterHook(ctx sdk.Context, portID, channelID string, result string, success bool) | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| package ibc_hooks_test | ||
|
|
||
| import ( | ||
| "cosmossdk.io/math" | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| testutils "github.com/cosmos/ibc-go/v5/modules/apps/hooks/testutils" | ||
| transfertypes "github.com/cosmos/ibc-go/v5/modules/apps/transfer/types" | ||
| clienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types" | ||
| channeltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types" | ||
| ibctesting "github.com/cosmos/ibc-go/v5/testing" | ||
| ibcmock "github.com/cosmos/ibc-go/v5/testing/mock" | ||
|
|
||
| "github.com/stretchr/testify/suite" | ||
| "testing" | ||
| ) | ||
|
|
||
| type HooksTestSuite struct { | ||
| suite.Suite | ||
|
|
||
| coordinator *ibctesting.Coordinator | ||
|
|
||
| chainA *ibctesting.TestChain | ||
| chainB *ibctesting.TestChain | ||
| chainC *ibctesting.TestChain | ||
|
|
||
| path *ibctesting.Path | ||
| pathAToC *ibctesting.Path | ||
|
There was a problem hiding this comment. I think There was a problem hiding this comment. yes! that was left over from copy pasting a transfer test. Removed. |
||
| } | ||
|
|
||
| func (suite *HooksTestSuite) SetupTest() { | ||
| suite.coordinator = ibctesting.NewCoordinator(suite.T(), 3) | ||
| suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1)) | ||
| suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2)) | ||
| suite.chainC = suite.coordinator.GetChain(ibctesting.GetChainID(3)) | ||
| } | ||
|
|
||
| func TestIBCHooksTestSuite(t *testing.T) { | ||
| suite.Run(t, new(HooksTestSuite)) | ||
| } | ||
|
|
||
| func (suite *HooksTestSuite) CreateMockPacket() channeltypes.Packet { | ||
| return channeltypes.NewPacket( | ||
| ibcmock.MockPacketData, | ||
| suite.chainA.SenderAccount.GetSequence(), | ||
| suite.path.EndpointA.ChannelConfig.PortID, | ||
| suite.path.EndpointA.ChannelID, | ||
| suite.path.EndpointB.ChannelConfig.PortID, | ||
| suite.path.EndpointB.ChannelID, | ||
| clienttypes.NewHeight(0, 100), | ||
| 0, | ||
| ) | ||
| } | ||
|
|
||
| func NewTransferPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path { | ||
| path := ibctesting.NewPath(chainA, chainB) | ||
| path.EndpointA.ChannelConfig.PortID = ibctesting.TransferPort | ||
| path.EndpointB.ChannelConfig.PortID = ibctesting.TransferPort | ||
| path.EndpointA.ChannelConfig.Version = transfertypes.Version | ||
| path.EndpointB.ChannelConfig.Version = transfertypes.Version | ||
|
|
||
| return path | ||
| } | ||
|
|
||
| func (suite *HooksTestSuite) TestOnRecvPacketHooks() { | ||
| var ( | ||
| trace transfertypes.DenomTrace | ||
| amount math.Int | ||
| receiver string | ||
| status testutils.Status | ||
| ) | ||
|
|
||
| testCases := []struct { | ||
| msg string | ||
| malleate func(*testutils.Status) | ||
| recvIsSource bool // the receiving chain is the source of the coin originally | ||
|
There was a problem hiding this comment. Do you actually need to test the scenario of the receiving chain being the original source of the tokens? If not, maybe this can be removed to simplify the logic of the test. There was a problem hiding this comment. Good call! simplifying. |
||
| expPass bool | ||
| }{ | ||
| {"override", func(status *testutils.Status) { | ||
| suite.chainB.GetSimApp().HooksMiddleware.Hooks = testutils.TestRecvBeforeAfterHooks{Status: status} | ||
nicolaslara marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }, true, true}, | ||
| {"before and after", func(status *testutils.Status) { | ||
| suite.chainB.GetSimApp().HooksMiddleware.Hooks = testutils.TestRecvBeforeAfterHooks{Status: status} | ||
| }, false, true}, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| tc := tc | ||
| suite.Run(tc.msg, func() { | ||
| suite.SetupTest() // reset | ||
|
|
||
| path := NewTransferPath(suite.chainA, suite.chainB) | ||
| suite.coordinator.Setup(path) | ||
| receiver = suite.chainB.SenderAccount.GetAddress().String() // must be explicitly changed in malleate | ||
| status = testutils.Status{} | ||
|
|
||
| amount = sdk.NewInt(100) // must be explicitly changed in malleate | ||
| seq := uint64(1) | ||
|
|
||
| if tc.recvIsSource { | ||
| // send coin from chainB to chainA, receive them, acknowledge them, and send back to chainB | ||
| coinFromBToA := sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)) | ||
| transferMsg := transfertypes.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, coinFromBToA, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 110), 0) | ||
| res, err := suite.chainB.SendMsgs(transferMsg) | ||
| suite.Require().NoError(err) // message committed | ||
|
|
||
| packet, err := ibctesting.ParsePacketFromEvents(res.GetEvents()) | ||
| suite.Require().NoError(err) | ||
|
|
||
| err = path.RelayPacket(packet) | ||
| suite.Require().NoError(err) // relay committed | ||
|
|
||
| seq++ | ||
|
|
||
| // NOTE: trace must be explicitly changed in malleate to test invalid cases | ||
| trace = transfertypes.ParseDenomTrace(transfertypes.GetPrefixedDenom(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.DefaultBondDenom)) | ||
| } else { | ||
| trace = transfertypes.ParseDenomTrace(sdk.DefaultBondDenom) | ||
| } | ||
|
|
||
| // send coin from chainA to chainB | ||
| transferMsg := transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.NewCoin(trace.IBCDenom(), amount), suite.chainA.SenderAccount.GetAddress().String(), receiver, clienttypes.NewHeight(1, 110), 0) | ||
| _, err := suite.chainA.SendMsgs(transferMsg) | ||
| suite.Require().NoError(err) // message committed | ||
|
|
||
| tc.malleate(&status) | ||
|
|
||
| data := transfertypes.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.String(), suite.chainA.SenderAccount.GetAddress().String(), receiver) | ||
| packet := channeltypes.NewPacket(data.GetBytes(), seq, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(1, 100), 0) | ||
|
|
||
| ack := suite.chainB.GetSimApp().HooksMiddleware.OnRecvPacket(suite.chainB.GetContext(), packet, suite.chainC.SenderAccount.GetAddress()) | ||
|
|
||
| if tc.expPass { | ||
| suite.Require().True(ack.Success()) | ||
| } else { | ||
| suite.Require().False(ack.Success()) | ||
| } | ||
|
|
||
| if _, ok := suite.chainB.GetSimApp().HooksMiddleware.Hooks.(testutils.TestRecvOverrides); ok { | ||
| suite.Require().True(status.OverrideRan) | ||
| suite.Require().False(status.BeforeRan) | ||
| suite.Require().False(status.AfterRan) | ||
| } | ||
|
|
||
| if _, ok := suite.chainB.GetSimApp().HooksMiddleware.Hooks.(testutils.TestRecvBeforeAfterHooks); ok { | ||
| suite.Require().False(status.OverrideRan) | ||
| suite.Require().True(status.BeforeRan) | ||
| suite.Require().True(status.AfterRan) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Would it make sense to move this file inside the
typesfolder?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.
I think that would create an import cycle, as ibc_module.go depends on the hooks and the hooks on the middleware struct defined in that file