(test) Add tests for callback execution when forwarding a packet #6805
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,182 @@ | ||
| package ibccallbacks_test | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/suite" | ||
|
|
||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| "github.com/cosmos/ibc-go/modules/apps/callbacks/testing/simapp" | ||
| callbacktypes "github.com/cosmos/ibc-go/modules/apps/callbacks/types" | ||
| "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" | ||
| clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" | ||
| channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" | ||
| ibctesting "github.com/cosmos/ibc-go/v8/testing" | ||
| ) | ||
|
|
||
| func init() { | ||
| ibctesting.DefaultTestingAppInit = SetupTestingApp | ||
| } | ||
|
|
||
| type CallbacksForwardingTestSuite struct { | ||
| suite.Suite | ||
|
|
||
| coordinator *ibctesting.Coordinator | ||
|
|
||
| chainA *ibctesting.TestChain | ||
| chainB *ibctesting.TestChain | ||
| chainC *ibctesting.TestChain | ||
|
|
||
| pathAtoB *ibctesting.Path | ||
| pathBtoC *ibctesting.Path | ||
| } | ||
|
|
||
| // setupChains sets up a coordinator with 3 test chains. | ||
| func (s *CallbacksForwardingTestSuite) setupChains() { | ||
| s.coordinator = ibctesting.NewCoordinator(s.T(), 3) | ||
| s.chainA = s.coordinator.GetChain(ibctesting.GetChainID(1)) | ||
| s.chainB = s.coordinator.GetChain(ibctesting.GetChainID(2)) | ||
| s.chainC = s.coordinator.GetChain(ibctesting.GetChainID(3)) | ||
|
|
||
| s.pathAtoB = ibctesting.NewTransferPath(s.chainA, s.chainB) | ||
| s.pathBtoC = ibctesting.NewTransferPath(s.chainB, s.chainC) | ||
| } | ||
|
|
||
| func (s *CallbacksForwardingTestSuite) SetupTest() { | ||
| s.setupChains() | ||
|
|
||
| s.pathAtoB.Setup() | ||
| s.pathBtoC.Setup() | ||
| } | ||
|
|
||
| func TestIBCCallbacksForwardingTestsuite(t *testing.T) { | ||
| suite.Run(t, new(CallbacksForwardingTestSuite)) | ||
| } | ||
|
|
||
| // TestForwardingWithMemoCallback tests that, when forwarding a packet with memo from A to B to C, | ||
| // the callback is executed only on the final hop. | ||
| // NOTE: this does not test the full forwarding behaviour (assert on amounts, packets, acks etc) | ||
| // as this is covered in other forwarding tests. | ||
| func (suite *CallbacksForwardingTestSuite) TestForwardingWithMemoCallback() { | ||
| testCases := []struct { | ||
| name string | ||
| testMemo string | ||
| expCallbackMapOnChainB map[callbacktypes.CallbackType]int | ||
|
There was a problem hiding this comment. We discussed this with @chatton and our understanding is that when the memo specifies If that's not the case, please let me know! I was surprised to see some callbacks executed on B but after some reasoning it started to make sense. There was a problem hiding this comment. With the current implementation it makes sense that this would be the case.. but it is a little weird imo. Wonder if we should have a testcase for ensuring that the send_packet callback is never invoked on chainA There was a problem hiding this comment. yea very strange, but I guess not much we can do. Would be great to add into the docs somewhere? With multi packetdata, it should become more explicit when and where callbacks are executed (hopefully 😅) There was a problem hiding this comment. I opened #6883 to ensure we document this properly |
||
| expCallbackMapOnChainC map[callbacktypes.CallbackType]int | ||
| }{ | ||
| { | ||
| name: "no memo", | ||
| expCallbackMapOnChainB: map[callbacktypes.CallbackType]int{}, | ||
| expCallbackMapOnChainC: map[callbacktypes.CallbackType]int{}, | ||
| }, | ||
| { | ||
| name: "recv callback", | ||
|
There was a problem hiding this comment. It's still not super clear to me what's the mapping between Memo content and which callbacks are executed. Does anyone have a pointer to learn this better? There was a problem hiding this comment. the main thing is that the special keys in the callback data specify on which chain the callback should be executed. Which is why that in this test case, it is only being executed on the |
||
| testMemo: fmt.Sprintf(`{"dest_callback": {"address": "%s"}}`, simapp.SuccessContract), | ||
| expCallbackMapOnChainB: map[callbacktypes.CallbackType]int{}, | ||
| expCallbackMapOnChainC: map[callbacktypes.CallbackType]int{callbacktypes.CallbackTypeReceivePacket: 1}, | ||
| }, | ||
| { | ||
| name: "ack callback", | ||
| testMemo: fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, simapp.SuccessContract), | ||
| expCallbackMapOnChainB: map[callbacktypes.CallbackType]int{callbacktypes.CallbackTypeAcknowledgementPacket: 1}, | ||
| expCallbackMapOnChainC: map[callbacktypes.CallbackType]int{}, | ||
| }, | ||
|
Comment on lines
+80
to
+85
There was a problem hiding this comment. might be worth adding a memo that contains both
There was a problem hiding this comment. {
name: "ack and recv callback",
testMemo: fmt.Sprintf(`{"src_callback": {"address": "%s"}, "dest_callback": {"address": "%s"}}`, simapp.SuccessContract, simapp.SuccessContract),
expCallbackMapOnChainB: map[callbacktypes.CallbackType]int{callbacktypes.CallbackTypeAcknowledgementPacket: 1},
expCallbackMapOnChainC: map[callbacktypes.CallbackType]int{callbacktypes.CallbackTypeReceivePacket: 1},
},There was a problem hiding this comment. Good one thank you! added! |
||
| { | ||
| name: "ack callback with low gas (error)", | ||
| testMemo: fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, simapp.OogErrorContract), | ||
| expCallbackMapOnChainB: map[callbacktypes.CallbackType]int{callbacktypes.CallbackTypeAcknowledgementPacket: 1}, | ||
| expCallbackMapOnChainC: map[callbacktypes.CallbackType]int{}, | ||
| }, | ||
| { | ||
| name: "recv callback with low gas (error)", | ||
| testMemo: fmt.Sprintf(`{"dest_callback": {"address": "%s"}}`, simapp.OogErrorContract), | ||
| expCallbackMapOnChainB: map[callbacktypes.CallbackType]int{}, | ||
| expCallbackMapOnChainC: map[callbacktypes.CallbackType]int{callbacktypes.CallbackTypeReceivePacket: 1}, | ||
| }, | ||
| } | ||
| for _, tc := range testCases { | ||
| suite.Run(tc.name, func() { | ||
| suite.SetupTest() | ||
|
|
||
| coinOnA := ibctesting.TestCoin | ||
| sender := suite.chainA.SenderAccounts[0].SenderAccount | ||
| receiver := suite.chainC.SenderAccounts[0].SenderAccount | ||
| forwarding := types.NewForwarding(false, types.NewHop( | ||
| suite.pathBtoC.EndpointA.ChannelConfig.PortID, | ||
| suite.pathBtoC.EndpointA.ChannelID, | ||
| )) | ||
| successAck := channeltypes.NewResultAcknowledgement([]byte{byte(1)}) | ||
|
|
||
| transferMsg := types.NewMsgTransfer( | ||
| suite.pathAtoB.EndpointA.ChannelConfig.PortID, | ||
| suite.pathAtoB.EndpointA.ChannelID, | ||
| sdk.NewCoins(coinOnA), | ||
| sender.GetAddress().String(), | ||
| receiver.GetAddress().String(), | ||
| clienttypes.ZeroHeight(), | ||
| suite.chainA.GetTimeoutTimestamp(), | ||
| tc.testMemo, | ||
| forwarding, | ||
| ) | ||
|
|
||
| result, err := suite.chainA.SendMsgs(transferMsg) | ||
| suite.Require().NoError(err) // message committed | ||
|
|
||
| // parse the packet from result events and recv packet on chainB | ||
| packetFromAtoB, err := ibctesting.ParsePacketFromEvents(result.Events) | ||
| suite.Require().NoError(err) | ||
| suite.Require().NotNil(packetFromAtoB) | ||
|
|
||
| err = suite.pathAtoB.EndpointB.UpdateClient() | ||
| suite.Require().NoError(err) | ||
|
|
||
| result, err = suite.pathAtoB.EndpointB.RecvPacketWithResult(packetFromAtoB) | ||
| suite.Require().NoError(err) | ||
| suite.Require().NotNil(result) | ||
|
|
||
| packetFromBtoC, err := ibctesting.ParsePacketFromEvents(result.Events) | ||
| suite.Require().NoError(err) | ||
| suite.Require().NotNil(packetFromBtoC) | ||
|
|
||
| err = suite.pathBtoC.EndpointA.UpdateClient() | ||
| suite.Require().NoError(err) | ||
|
|
||
| err = suite.pathBtoC.EndpointB.UpdateClient() | ||
| suite.Require().NoError(err) | ||
|
|
||
| result, err = suite.pathBtoC.EndpointB.RecvPacketWithResult(packetFromBtoC) | ||
| suite.Require().NoError(err) | ||
| suite.Require().NotNil(result) | ||
|
|
||
| packetOnC, err := ibctesting.ParseRecvPacketFromEvents(result.Events) | ||
| suite.Require().NoError(err) | ||
| suite.Require().NotNil(packetOnC) | ||
|
|
||
| // Ack back to B | ||
| err = suite.pathBtoC.EndpointB.UpdateClient() | ||
| suite.Require().NoError(err) | ||
|
|
||
| err = suite.pathBtoC.EndpointA.AcknowledgePacket(packetFromBtoC, successAck.Acknowledgement()) | ||
| suite.Require().NoError(err) | ||
|
|
||
| // Ack back to A | ||
| err = suite.pathAtoB.EndpointA.UpdateClient() | ||
| suite.Require().NoError(err) | ||
|
|
||
| err = suite.pathAtoB.EndpointA.AcknowledgePacket(packetFromAtoB, successAck.Acknowledgement()) | ||
| suite.Require().NoError(err) | ||
|
|
||
| // We never expect chainA to have executed any callback | ||
| suite.Require().Empty(GetSimApp(suite.chainA).MockContractKeeper.Counters, "chainA's callbacks counter map is not empty") | ||
|
|
||
| // We expect chainB to have executed callbacks when the memo is of type `src_callback` | ||
| chainBCallbackMap := GetSimApp(suite.chainB).MockContractKeeper.Counters | ||
| suite.Require().Equal(tc.expCallbackMapOnChainB, chainBCallbackMap, "chainB: expected callback counters map to be %v, got %v instead", tc.expCallbackMapOnChainB, chainBCallbackMap) | ||
|
|
||
| // We expect chainC to have executed callbacks when the memo is of type `dest_callback` | ||
| chainCCallbackMap := GetSimApp(suite.chainC).MockContractKeeper.Counters | ||
| suite.Require().Equal(tc.expCallbackMapOnChainC, chainCCallbackMap, "chainC: expected callback counters map to be %v, got %v instead", tc.expCallbackMapOnChainC, chainCCallbackMap) | ||
| }) | ||
| } | ||
| } | ||
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.
I had to add a new file here in the callbacks module because it's the only module in which we can check the callbacks counter. This required setting up a new, small test suite that creates 3 chains.