From 9e2cb03bfae44bce8a1d9bb157f9b6965a27a17f Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 28 May 2024 09:24:14 +0100 Subject: [PATCH 1/7] chore: adding v2 test cases for TestGetCallbackData --- .../apps/callbacks/types/callbacks_test.go | 254 ++++++++++++++++++ 1 file changed, 254 insertions(+) diff --git a/modules/apps/callbacks/types/callbacks_test.go b/modules/apps/callbacks/types/callbacks_test.go index be4098f9bc9..1deafcabc95 100644 --- a/modules/apps/callbacks/types/callbacks_test.go +++ b/modules/apps/callbacks/types/callbacks_test.go @@ -250,6 +250,260 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { types.CallbackData{}, types.ErrCallbackAddressNotFound, }, + + { + "success: source callback", + func() { + remainingGas = 2_000_000 + packetData = transfertypes.FungibleTokenPacketData{ + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, sender), + } + }, + types.CallbackData{ + CallbackAddress: sender, + SenderAddress: sender, + ExecutionGasLimit: 1_000_000, + CommitGasLimit: 1_000_000, + }, + nil, + }, + { + "success: destination callback", + func() { + callbackKey = types.DestinationCallbackKey + + remainingGas = 2_000_000 + packetData = transfertypes.FungibleTokenPacketData{ + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"dest_callback": {"address": "%s"}}`, sender), + } + }, + types.CallbackData{ + CallbackAddress: sender, + SenderAddress: "", + ExecutionGasLimit: 1_000_000, + CommitGasLimit: 1_000_000, + }, + nil, + }, + { + "success v2: destination callback with 0 user defined gas limit", + func() { + callbackKey = types.DestinationCallbackKey + + remainingGas = 2_000_000 + packetData = transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + }, + }, + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"dest_callback": {"address": "%s", "gas_limit":"0"}}`, sender), + } + }, + types.CallbackData{ + CallbackAddress: sender, + SenderAddress: "", + ExecutionGasLimit: 1_000_000, + CommitGasLimit: 1_000_000, + }, + nil, + }, + { + "success v2: source callback with gas limit < remaining gas < max gas", + func() { + packetData = transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + }, + }, + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "50000"}}`, sender), + } + + remainingGas = 100_000 + }, + types.CallbackData{ + CallbackAddress: sender, + SenderAddress: sender, + ExecutionGasLimit: 50_000, + CommitGasLimit: 50_000, + }, + nil, + }, + { + "success v2: source callback with remaining gas < gas limit < max gas", + func() { + remainingGas = 100_000 + packetData = transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + }, + }, + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "200000"}}`, sender), + } + }, + types.CallbackData{ + CallbackAddress: sender, + SenderAddress: sender, + ExecutionGasLimit: 100_000, + CommitGasLimit: 200_000, + }, + nil, + }, + { + "success v2: source callback with remaining gas < max gas < gas limit", + func() { + remainingGas = 100_000 + packetData = transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + }, + }, + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "2000000"}}`, sender), + } + }, + types.CallbackData{ + CallbackAddress: sender, + SenderAddress: sender, + ExecutionGasLimit: 100_000, + CommitGasLimit: 1_000_000, + }, + nil, + }, + { + "success v2: destination callback with remaining gas < max gas < gas limit", + func() { + callbackKey = types.DestinationCallbackKey + + remainingGas = 100_000 + packetData = transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + }, + }, + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"dest_callback": {"address": "%s", "gas_limit": "2000000"}}`, sender), + } + }, + types.CallbackData{ + CallbackAddress: sender, + SenderAddress: "", + ExecutionGasLimit: 100_000, + CommitGasLimit: 1_000_000, + }, + nil, + }, + { + "success v2: source callback with max gas < remaining gas < gas limit", + func() { + remainingGas = 2_000_000 + packetData = transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + }, + }, + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "3000000"}}`, sender), + } + }, + types.CallbackData{ + CallbackAddress: sender, + SenderAddress: sender, + ExecutionGasLimit: 1_000_000, + CommitGasLimit: 1_000_000, + }, + nil, + }, + { + "failure: packet data does not implement PacketDataProvider", + func() { + packetData = ibcmock.MockPacketData + }, + types.CallbackData{}, + types.ErrNotPacketDataProvider, + }, + { + "failure v2: empty memo", + func() { + packetData = transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + }, + }, + Sender: sender, + Receiver: receiver, + Memo: "", + } + }, + types.CallbackData{}, + types.ErrCallbackKeyNotFound, + }, + { + "failure v2: empty address", + func() { + packetData = transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"src_callback": {"address": ""}}`, + } + }, + types.CallbackData{}, + types.ErrCallbackAddressNotFound, + }, + { + "failure v2: space address", + func() { + packetData = transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"src_callback": {"address": " "}}`, + } + }, + types.CallbackData{}, + types.ErrCallbackAddressNotFound, + }, } for _, tc := range testCases { From 76d3a74ba82ff0fb4edcf3fa2495512de2cd060f Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 28 May 2024 09:34:15 +0100 Subject: [PATCH 2/7] chore: adding v2 tests for TestGetCallbackAddress --- .../apps/callbacks/types/callbacks_test.go | 123 +++++++++++++++++- 1 file changed, 122 insertions(+), 1 deletion(-) diff --git a/modules/apps/callbacks/types/callbacks_test.go b/modules/apps/callbacks/types/callbacks_test.go index 1deafcabc95..65eb538a3f1 100644 --- a/modules/apps/callbacks/types/callbacks_test.go +++ b/modules/apps/callbacks/types/callbacks_test.go @@ -2,6 +2,7 @@ package types_test import ( "fmt" + ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported" storetypes "cosmossdk.io/store/types" @@ -627,7 +628,7 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { testCases := []struct { name string - packetData transfertypes.FungibleTokenPacketData + packetData ibcexported.PacketDataProvider expAddress string }{ { @@ -718,6 +719,126 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { }, "", }, + { + "success v2: memo has callbacks in json struct and properly formatted src_callback_address which does not match packet sender", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, receiver), + }, + receiver, + }, + { + "success v2: valid src_callback address specified in memo that matches sender", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, sender), + }, + sender, + }, + { + "failure v2: memo is empty", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: "", + }, + "", + }, + { + "failure v2: memo is not json string", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: "memo", + }, + "", + }, + { + "failure v2: memo has empty src_callback object", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"src_callback": {}}`, + }, + "", + }, + { + "failure v2: memo does not have callbacks in json struct", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"Key": 10}`, + }, + "", + }, + { + "failure v2: memo has src_callback in json struct but does not have address key", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"src_callback": {"Key": 10}}`, + }, + "", + }, + { + "failure v2: memo has src_callback in json struct but does not have string value for address key", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"src_callback": {"address": 10}}`, + }, + "", + }, } for _, tc := range testCases { From f6c4c0dba3e66e3a61a0ecb88299971c9f01eb21 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 28 May 2024 09:40:38 +0100 Subject: [PATCH 3/7] chore: adding v2 tests to TestUserDefinedGasLimit --- .../apps/callbacks/types/callbacks_test.go | 137 +++++++++++++++++- 1 file changed, 136 insertions(+), 1 deletion(-) diff --git a/modules/apps/callbacks/types/callbacks_test.go b/modules/apps/callbacks/types/callbacks_test.go index 65eb538a3f1..55251534656 100644 --- a/modules/apps/callbacks/types/callbacks_test.go +++ b/modules/apps/callbacks/types/callbacks_test.go @@ -859,7 +859,7 @@ func (s *CallbacksTypesTestSuite) TestUserDefinedGasLimit() { testCases := []struct { name string - packetData transfertypes.FungibleTokenPacketData + packetData ibcexported.PacketDataProvider expUserGas uint64 }{ { @@ -961,6 +961,141 @@ func (s *CallbacksTypesTestSuite) TestUserDefinedGasLimit() { }, 0, }, + { + "success v2: memo is empty", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: "", + }, + 0, + }, + { + "success v2: memo has user defined gas limit", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"src_callback": {"gas_limit": "100"}}`, + }, + 100, + }, + { + "success v2: user defined gas limit is zero", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"src_callback": {"gas_limit": "0"}}`, + }, + 0, + }, + { + "failure v2: memo has empty src_callback object", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"src_callback": {}}`, + }, + 0, + }, + { + "failure v2: memo has user defined gas limit as json number", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"src_callback": {"gas_limit": 100}}`, + }, + 0, + }, + { + "failure v2: memo has user defined gas limit as negative", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"src_callback": {"gas_limit": "-100"}}`, + }, + 0, + }, + { + "failure v2: memo has user defined gas limit as string", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"src_callback": {"gas_limit": "invalid"}}`, + }, + 0, + }, + { + "failure v2: memo has user defined gas limit as empty string", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `{"src_callback": {"gas_limit": ""}}`, + }, + 0, + }, + { + "failure v2: malformed memo", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: denom, + Amount: amount, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: `invalid`, + }, + 0, + }, } for _, tc := range testCases { From 476399a7d14319fd8dfec255645f2b730aae4fd5 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 28 May 2024 10:11:36 +0100 Subject: [PATCH 4/7] chore: adding v2 tests for TestGetDestCallbackDataTransfer --- .../apps/callbacks/types/callbacks_test.go | 110 +++++++++++++----- 1 file changed, 80 insertions(+), 30 deletions(-) diff --git a/modules/apps/callbacks/types/callbacks_test.go b/modules/apps/callbacks/types/callbacks_test.go index 55251534656..6f8f32839a0 100644 --- a/modules/apps/callbacks/types/callbacks_test.go +++ b/modules/apps/callbacks/types/callbacks_test.go @@ -2,7 +2,7 @@ package types_test import ( "fmt" - ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported" + storetypes "cosmossdk.io/store/types" @@ -14,6 +14,7 @@ import ( transfertypes "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" + ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported" ibctesting "github.com/cosmos/ibc-go/v8/testing" ibcmock "github.com/cosmos/ibc-go/v8/testing/mock" ) @@ -578,46 +579,95 @@ func (s *CallbacksTypesTestSuite) TestGetSourceCallbackDataTransfer() { } func (s *CallbacksTypesTestSuite) TestGetDestCallbackDataTransfer() { - s.SetupTest() + // bytesProvider defines an interface that both packet data types implement in order to fetch the bytes. + type bytesProvider interface { + GetBytes() []byte + } sender := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() receiver := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() - packetData := transfertypes.FungibleTokenPacketData{ - Denom: ibctesting.TestCoin.Denom, - Amount: ibctesting.TestCoin.Amount.String(), - Sender: sender, - Receiver: receiver, - Memo: fmt.Sprintf(`{"dest_callback": {"address": "%s"}}`, sender), + testCases := []struct { + name string + packetData bytesProvider + expCallbackdata types.CallbackData + malleate func() + }{ + { + "success: v1", + transfertypes.FungibleTokenPacketData{ + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"dest_callback": {"address": "%s"}}`, sender), + }, + types.CallbackData{ + CallbackAddress: sender, + SenderAddress: "", + ExecutionGasLimit: 1_000_000, + CommitGasLimit: 1_000_000, + }, + func() { + s.path.EndpointA.ChannelConfig.Version = transfertypes.V1 + s.path.EndpointA.ChannelConfig.PortID = transfertypes.ModuleName + s.path.EndpointB.ChannelConfig.Version = transfertypes.V1 + s.path.EndpointB.ChannelConfig.PortID = transfertypes.ModuleName + }, + }, + { + "success: v2", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + }, + }, + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"dest_callback": {"address": "%s"}}`, sender), + }, + types.CallbackData{ + CallbackAddress: sender, + SenderAddress: "", + ExecutionGasLimit: 1_000_000, + CommitGasLimit: 1_000_000, + }, + func() { + s.path.EndpointA.ChannelConfig.Version = transfertypes.V2 + s.path.EndpointA.ChannelConfig.PortID = transfertypes.ModuleName + s.path.EndpointB.ChannelConfig.Version = transfertypes.V2 + s.path.EndpointB.ChannelConfig.PortID = transfertypes.ModuleName + }, + }, } - packetDataBytes := packetData.GetBytes() - expCallbackData := types.CallbackData{ - CallbackAddress: sender, - SenderAddress: "", - ExecutionGasLimit: 1_000_000, - CommitGasLimit: 1_000_000, - } + for _, tc := range testCases { + tc := tc + s.Run(tc.name, func() { + s.SetupTest() - s.path.EndpointA.ChannelConfig.Version = transfertypes.V1 - s.path.EndpointA.ChannelConfig.PortID = transfertypes.ModuleName - s.path.EndpointB.ChannelConfig.Version = transfertypes.V1 - s.path.EndpointB.ChannelConfig.PortID = transfertypes.ModuleName + tc.malleate() - transferStack, ok := s.chainA.App.GetIBCKeeper().PortKeeper.Route(transfertypes.ModuleName) - s.Require().True(ok) + packetDataBytes := tc.packetData.GetBytes() - packetUnmarshaler, ok := transferStack.(types.CallbacksCompatibleModule) - s.Require().True(ok) + transferStack, ok := s.chainA.App.GetIBCKeeper().PortKeeper.Route(transfertypes.ModuleName) + s.Require().True(ok) - s.path.Setup() + packetUnmarshaler, ok := transferStack.(types.CallbacksCompatibleModule) + s.Require().True(ok) - gasMeter := storetypes.NewGasMeter(2_000_000) - ctx := s.chainA.GetContext().WithGasMeter(gasMeter) - packet := channeltypes.NewPacket(packetDataBytes, 0, transfertypes.PortID, s.path.EndpointA.ChannelID, transfertypes.PortID, s.path.EndpointB.ChannelID, clienttypes.ZeroHeight(), 0) - callbackData, err := types.GetDestCallbackData(ctx, packetUnmarshaler, packet, 1_000_000) - s.Require().NoError(err) - s.Require().Equal(expCallbackData, callbackData) + s.path.Setup() + + gasMeter := storetypes.NewGasMeter(2_000_000) + ctx := s.chainA.GetContext().WithGasMeter(gasMeter) + packet := channeltypes.NewPacket(packetDataBytes, 0, transfertypes.PortID, s.path.EndpointA.ChannelID, transfertypes.PortID, s.path.EndpointB.ChannelID, clienttypes.ZeroHeight(), 0) + callbackData, err := types.GetDestCallbackData(ctx, packetUnmarshaler, packet, 1_000_000) + s.Require().NoError(err) + s.Require().Equal(tc.expCallbackdata, callbackData) + }) + } } func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { From 84a6a2aed89e68c1e563b9225c8bcbf0eeac4e4a Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 28 May 2024 10:25:46 +0100 Subject: [PATCH 5/7] chore: added v2 tests for TestGetSourceCallbackDataTransfer --- .../apps/callbacks/types/callbacks_test.go | 115 ++++++++++++------ 1 file changed, 78 insertions(+), 37 deletions(-) diff --git a/modules/apps/callbacks/types/callbacks_test.go b/modules/apps/callbacks/types/callbacks_test.go index 6f8f32839a0..0387b9ff47b 100644 --- a/modules/apps/callbacks/types/callbacks_test.go +++ b/modules/apps/callbacks/types/callbacks_test.go @@ -3,7 +3,6 @@ package types_test import ( "fmt" - storetypes "cosmossdk.io/store/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -533,57 +532,99 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { } } -func (s *CallbacksTypesTestSuite) TestGetSourceCallbackDataTransfer() { - s.SetupTest() +// bytesProvider defines an interface that both packet data types implement in order to fetch the bytes. +type bytesProvider interface { + GetBytes() []byte +} +func (s *CallbacksTypesTestSuite) TestGetSourceCallbackDataTransfer() { sender := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() receiver := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() - packetData := transfertypes.FungibleTokenPacketData{ - Denom: ibctesting.TestCoin.Denom, - Amount: ibctesting.TestCoin.Amount.String(), - Sender: sender, - Receiver: receiver, - Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, sender), + testCases := []struct { + name string + packetData bytesProvider + expCallbackData types.CallbackData + malleate func() + }{ + { + "success: v1", + transfertypes.FungibleTokenPacketData{ + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, sender), + }, + types.CallbackData{ + CallbackAddress: sender, + SenderAddress: sender, + ExecutionGasLimit: 1_000_000, + CommitGasLimit: 1_000_000, + }, + func() { + s.path.EndpointA.ChannelConfig.Version = transfertypes.V1 + s.path.EndpointA.ChannelConfig.PortID = transfertypes.ModuleName + s.path.EndpointB.ChannelConfig.Version = transfertypes.V1 + s.path.EndpointB.ChannelConfig.PortID = transfertypes.ModuleName + }, + }, + { + "success: v2", + transfertypes.FungibleTokenPacketDataV2{ + Tokens: transfertypes.Tokens{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + }, + }, + Sender: sender, + Receiver: receiver, + Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, sender), + }, + types.CallbackData{ + CallbackAddress: sender, + SenderAddress: sender, + ExecutionGasLimit: 1_000_000, + CommitGasLimit: 1_000_000, + }, + func() { + s.path.EndpointA.ChannelConfig.Version = transfertypes.V2 + s.path.EndpointA.ChannelConfig.PortID = transfertypes.ModuleName + s.path.EndpointB.ChannelConfig.Version = transfertypes.V2 + s.path.EndpointB.ChannelConfig.PortID = transfertypes.ModuleName + }, + }, } - packetDataBytes := packetData.GetBytes() - expCallbackData := types.CallbackData{ - CallbackAddress: sender, - SenderAddress: sender, - ExecutionGasLimit: 1_000_000, - CommitGasLimit: 1_000_000, - } + for _, tc := range testCases { + tc := tc + s.Run(tc.name, func() { + s.SetupTest() - s.path.EndpointA.ChannelConfig.Version = transfertypes.V1 - s.path.EndpointA.ChannelConfig.PortID = transfertypes.ModuleName - s.path.EndpointB.ChannelConfig.Version = transfertypes.V1 - s.path.EndpointB.ChannelConfig.PortID = transfertypes.ModuleName + tc.malleate() - transferStack, ok := s.chainA.App.GetIBCKeeper().PortKeeper.Route(transfertypes.ModuleName) - s.Require().True(ok) + packetDataBytes := tc.packetData.GetBytes() - packetUnmarshaler, ok := transferStack.(types.CallbacksCompatibleModule) - s.Require().True(ok) + transferStack, ok := s.chainA.App.GetIBCKeeper().PortKeeper.Route(transfertypes.ModuleName) + s.Require().True(ok) - s.path.Setup() + packetUnmarshaler, ok := transferStack.(types.CallbacksCompatibleModule) + s.Require().True(ok) - // Set up gas meter for context. - gasMeter := storetypes.NewGasMeter(2_000_000) - ctx := s.chainA.GetContext().WithGasMeter(gasMeter) + s.path.Setup() - packet := channeltypes.NewPacket(packetDataBytes, 0, transfertypes.PortID, s.path.EndpointA.ChannelID, transfertypes.PortID, s.path.EndpointB.ChannelID, clienttypes.ZeroHeight(), 0) - callbackData, err := types.GetSourceCallbackData(ctx, packetUnmarshaler, packet, 1_000_000) - s.Require().NoError(err) - s.Require().Equal(expCallbackData, callbackData) + gasMeter := storetypes.NewGasMeter(2_000_000) + ctx := s.chainA.GetContext().WithGasMeter(gasMeter) + packet := channeltypes.NewPacket(packetDataBytes, 0, transfertypes.PortID, s.path.EndpointA.ChannelID, transfertypes.PortID, s.path.EndpointB.ChannelID, clienttypes.ZeroHeight(), 0) + callbackData, err := types.GetSourceCallbackData(ctx, packetUnmarshaler, packet, 1_000_000) + s.Require().NoError(err) + s.Require().Equal(tc.expCallbackData, callbackData) + }) + } } func (s *CallbacksTypesTestSuite) TestGetDestCallbackDataTransfer() { - // bytesProvider defines an interface that both packet data types implement in order to fetch the bytes. - type bytesProvider interface { - GetBytes() []byte - } - sender := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() receiver := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() From a5f6cff53f4833150d5c2934f250fcde9e744db3 Mon Sep 17 00:00:00 2001 From: chatton Date: Wed, 29 May 2024 09:24:24 +0100 Subject: [PATCH 6/7] chore: appease the linting overlords and merge main --- modules/apps/callbacks/ibc_middleware_test.go | 36 +++++++------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index 74051f3942a..f859941a287 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -167,10 +167,8 @@ func (s *CallbacksTestSuite) TestSendPacket() { packetData = transfertypes.NewFungibleTokenPacketDataV2( []transfertypes.Token{ { - Denom: transfertypes.Denom{ - Base: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, - }, + Denom: ibctesting.TestCoin.GetDenom(), + Trace: []string{}, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -311,10 +309,8 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() { packetData = transfertypes.NewFungibleTokenPacketDataV2( []transfertypes.Token{ { - Denom: transfertypes.Denom{ - Base: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, - }, + Denom: ibctesting.TestCoin.GetDenom(), + Trace: []string{}, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -646,10 +642,8 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() { packetData = transfertypes.NewFungibleTokenPacketDataV2( []transfertypes.Token{ { - Denom: transfertypes.Denom{ - Base: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, - }, + Denom: ibctesting.TestCoin.GetDenom(), + Trace: []string{}, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -780,10 +774,8 @@ func (s *CallbacksTestSuite) TestWriteAcknowledgement() { packetData = transfertypes.NewFungibleTokenPacketDataV2( []transfertypes.Token{ { - Denom: transfertypes.Denom{ - Base: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, - }, + Denom: ibctesting.TestCoin.GetDenom(), + Trace: []string{}, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -1006,10 +998,8 @@ func (s *CallbacksTestSuite) TestUnmarshalPacketDataV1() { expPacketDataICS20V2 := transfertypes.FungibleTokenPacketDataV2{ Tokens: []transfertypes.Token{ { - Denom: transfertypes.Denom{ - Base: ibctesting.TestCoin.Denom, - Trace: nil, - }, + Denom: ibctesting.TestCoin.GetDenom(), + Trace: []string{}, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -1042,10 +1032,8 @@ func (s *CallbacksTestSuite) TestUnmarshalPacketDataV2() { expPacketDataICS20V2 := transfertypes.FungibleTokenPacketDataV2{ Tokens: []transfertypes.Token{ { - Denom: transfertypes.Denom{ - Base: ibctesting.TestCoin.Denom, - Trace: nil, - }, + Denom: ibctesting.TestCoin.GetDenom(), + Trace: []string{}, Amount: ibctesting.TestCoin.Amount.String(), }, }, From 22472040a2565434ae63cce60daad1a266888891 Mon Sep 17 00:00:00 2001 From: chatton Date: Wed, 29 May 2024 15:51:12 +0100 Subject: [PATCH 7/7] chore: use typed Denom --- modules/apps/callbacks/ibc_middleware_test.go | 36 ++++--- .../apps/callbacks/types/callbacks_test.go | 96 +++++++++++++------ 2 files changed, 90 insertions(+), 42 deletions(-) diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index f859941a287..98a31b79918 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -167,8 +167,10 @@ func (s *CallbacksTestSuite) TestSendPacket() { packetData = transfertypes.NewFungibleTokenPacketDataV2( []transfertypes.Token{ { - Denom: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.GetDenom(), + Trace: []string{}, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -309,8 +311,10 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() { packetData = transfertypes.NewFungibleTokenPacketDataV2( []transfertypes.Token{ { - Denom: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.GetDenom(), + Trace: []string{}, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -642,8 +646,10 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() { packetData = transfertypes.NewFungibleTokenPacketDataV2( []transfertypes.Token{ { - Denom: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.GetDenom(), + Trace: []string{}, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -774,8 +780,10 @@ func (s *CallbacksTestSuite) TestWriteAcknowledgement() { packetData = transfertypes.NewFungibleTokenPacketDataV2( []transfertypes.Token{ { - Denom: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.GetDenom(), + Trace: []string{}, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -998,8 +1006,10 @@ func (s *CallbacksTestSuite) TestUnmarshalPacketDataV1() { expPacketDataICS20V2 := transfertypes.FungibleTokenPacketDataV2{ Tokens: []transfertypes.Token{ { - Denom: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.GetDenom(), + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -1032,8 +1042,10 @@ func (s *CallbacksTestSuite) TestUnmarshalPacketDataV2() { expPacketDataICS20V2 := transfertypes.FungibleTokenPacketDataV2{ Tokens: []transfertypes.Token{ { - Denom: ibctesting.TestCoin.GetDenom(), - Trace: []string{}, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.GetDenom(), + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, diff --git a/modules/apps/callbacks/types/callbacks_test.go b/modules/apps/callbacks/types/callbacks_test.go index 0387b9ff47b..be01f892189 100644 --- a/modules/apps/callbacks/types/callbacks_test.go +++ b/modules/apps/callbacks/types/callbacks_test.go @@ -303,7 +303,10 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { packetData = transfertypes.FungibleTokenPacketDataV2{ Tokens: transfertypes.Tokens{ { - Denom: ibctesting.TestCoin.Denom, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.Denom, + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -326,7 +329,10 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { packetData = transfertypes.FungibleTokenPacketDataV2{ Tokens: transfertypes.Tokens{ { - Denom: ibctesting.TestCoin.Denom, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.Denom, + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -352,7 +358,10 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { packetData = transfertypes.FungibleTokenPacketDataV2{ Tokens: transfertypes.Tokens{ { - Denom: ibctesting.TestCoin.Denom, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.Denom, + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -376,7 +385,10 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { packetData = transfertypes.FungibleTokenPacketDataV2{ Tokens: transfertypes.Tokens{ { - Denom: ibctesting.TestCoin.Denom, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.Denom, + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -402,7 +414,10 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { packetData = transfertypes.FungibleTokenPacketDataV2{ Tokens: transfertypes.Tokens{ { - Denom: ibctesting.TestCoin.Denom, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.Denom, + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -426,7 +441,10 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { packetData = transfertypes.FungibleTokenPacketDataV2{ Tokens: transfertypes.Tokens{ { - Denom: ibctesting.TestCoin.Denom, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.Denom, + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -457,7 +475,10 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { packetData = transfertypes.FungibleTokenPacketDataV2{ Tokens: transfertypes.Tokens{ { - Denom: ibctesting.TestCoin.Denom, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.Denom, + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -475,7 +496,10 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { packetData = transfertypes.FungibleTokenPacketDataV2{ Tokens: transfertypes.Tokens{ { - Denom: ibctesting.TestCoin.Denom, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.Denom, + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -493,7 +517,10 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { packetData = transfertypes.FungibleTokenPacketDataV2{ Tokens: transfertypes.Tokens{ { - Denom: ibctesting.TestCoin.Denom, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.Denom, + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -574,7 +601,10 @@ func (s *CallbacksTypesTestSuite) TestGetSourceCallbackDataTransfer() { transfertypes.FungibleTokenPacketDataV2{ Tokens: transfertypes.Tokens{ { - Denom: ibctesting.TestCoin.Denom, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.Denom, + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -661,7 +691,10 @@ func (s *CallbacksTypesTestSuite) TestGetDestCallbackDataTransfer() { transfertypes.FungibleTokenPacketDataV2{ Tokens: transfertypes.Tokens{ { - Denom: ibctesting.TestCoin.Denom, + Denom: transfertypes.Denom{ + Base: ibctesting.TestCoin.Denom, + Trace: nil, + }, Amount: ibctesting.TestCoin.Amount.String(), }, }, @@ -712,7 +745,7 @@ func (s *CallbacksTypesTestSuite) TestGetDestCallbackDataTransfer() { } func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { - denom := ibctesting.TestCoin.Denom + denom := transfertypes.Denom{Base: ibctesting.TestCoin.Denom} amount := ibctesting.TestCoin.Amount.String() sender := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() receiver := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() @@ -725,7 +758,7 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { { "success: memo has callbacks in json struct and properly formatted src_callback_address which does not match packet sender", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -736,7 +769,7 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { { "success: valid src_callback address specified in memo that matches sender", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -747,7 +780,7 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { { "failure: memo is empty", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -758,7 +791,7 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { { "failure: memo is not json string", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -769,7 +802,7 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { { "failure: memo has empty src_callback object", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -780,7 +813,7 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { { "failure: memo does not have callbacks in json struct", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -791,7 +824,7 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { { "failure: memo has src_callback in json struct but does not have address key", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -802,7 +835,7 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { { "failure: memo has src_callback in json struct but does not have string value for address key", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -943,7 +976,10 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackAddress() { } func (s *CallbacksTypesTestSuite) TestUserDefinedGasLimit() { - denom := ibctesting.TestCoin.Denom + denom := transfertypes.Denom{ + Base: ibctesting.TestCoin.Denom, + Trace: nil, + } amount := ibctesting.TestCoin.Amount.String() sender := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() receiver := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() @@ -956,7 +992,7 @@ func (s *CallbacksTypesTestSuite) TestUserDefinedGasLimit() { { "success: memo is empty", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -967,7 +1003,7 @@ func (s *CallbacksTypesTestSuite) TestUserDefinedGasLimit() { { "success: memo has user defined gas limit", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -978,7 +1014,7 @@ func (s *CallbacksTypesTestSuite) TestUserDefinedGasLimit() { { "success: user defined gas limit is zero", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -989,7 +1025,7 @@ func (s *CallbacksTypesTestSuite) TestUserDefinedGasLimit() { { "failure: memo has empty src_callback object", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -1000,7 +1036,7 @@ func (s *CallbacksTypesTestSuite) TestUserDefinedGasLimit() { { "failure: memo has user defined gas limit as json number", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -1011,7 +1047,7 @@ func (s *CallbacksTypesTestSuite) TestUserDefinedGasLimit() { { "failure: memo has user defined gas limit as negative", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -1022,7 +1058,7 @@ func (s *CallbacksTypesTestSuite) TestUserDefinedGasLimit() { { "failure: memo has user defined gas limit as string", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -1033,7 +1069,7 @@ func (s *CallbacksTypesTestSuite) TestUserDefinedGasLimit() { { "failure: memo has user defined gas limit as empty string", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver, @@ -1044,7 +1080,7 @@ func (s *CallbacksTypesTestSuite) TestUserDefinedGasLimit() { { "failure: malformed memo", transfertypes.FungibleTokenPacketData{ - Denom: denom, + Denom: denom.Base, Amount: amount, Sender: sender, Receiver: receiver,