From 3306ad7ff1ab6d1a20423276f7fd55fa4d594e94 Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Mon, 21 Aug 2023 20:26:00 +0800 Subject: [PATCH 1/3] Update go.work.example (#4397) --- go.work.example | 1 + 1 file changed, 1 insertion(+) diff --git a/go.work.example b/go.work.example index 655938729b8..e09d8e1fcd1 100644 --- a/go.work.example +++ b/go.work.example @@ -3,5 +3,6 @@ go 1.20 use ( ./ ./modules/capability + ./modules/applications/callbacks ./e2e ) From b7cb1ebe37f666d0830a14156071b21f1b0740d3 Mon Sep 17 00:00:00 2001 From: srdtrk <59252793+srdtrk@users.noreply.github.com> Date: Mon, 21 Aug 2023 17:28:43 +0300 Subject: [PATCH 2/3] refactor(ica): packet data unmarshaling logic refactored (#4232) * refactor(ica): refactored packet data's unmarshal logic * fix(ica_test): made tests pass * imp(ica): changed to UnmarshalJSON api * docs(ica): added godocs to iapd's 'UnmarshalJSON' method --- .../controller/ibc_middleware.go | 8 +++---- .../host/keeper/relay.go | 4 ++-- .../27-interchain-accounts/types/packet.go | 5 +++++ .../types/packet_test.go | 21 +++++++++++++++++++ 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 1ccd5206604..5bfc6ca16c9 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -261,10 +261,10 @@ func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) // into an InterchainAccountPacketData. This function implements the optional // PacketDataUnmarshaler interface required for ADR 008 support. func (IBCMiddleware) UnmarshalPacketData(bz []byte) (interface{}, error) { - var packetData icatypes.InterchainAccountPacketData - if err := icatypes.ModuleCdc.UnmarshalJSON(bz, &packetData); err != nil { + var data icatypes.InterchainAccountPacketData + err := data.UnmarshalJSON(bz) + if err != nil { return nil, err } - - return packetData, nil + return data, nil } diff --git a/modules/apps/27-interchain-accounts/host/keeper/relay.go b/modules/apps/27-interchain-accounts/host/keeper/relay.go index 74dd7c6f832..a91a89a185d 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/relay.go +++ b/modules/apps/27-interchain-accounts/host/keeper/relay.go @@ -18,8 +18,8 @@ import ( // If the transaction is successfully executed, the transaction response bytes will be returned. func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) ([]byte, error) { var data icatypes.InterchainAccountPacketData - - if err := icatypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { + err := data.UnmarshalJSON(packet.GetData()) + if err != nil { // UnmarshalJSON errors are indeterminate and therefore are not wrapped and included in failed acks return nil, errorsmod.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain account packet data") } diff --git a/modules/apps/27-interchain-accounts/types/packet.go b/modules/apps/27-interchain-accounts/types/packet.go index 3091e6ba33c..f4e0efc9958 100644 --- a/modules/apps/27-interchain-accounts/types/packet.go +++ b/modules/apps/27-interchain-accounts/types/packet.go @@ -57,6 +57,11 @@ func (iapd InterchainAccountPacketData) GetBytes() []byte { return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&iapd)) } +// UnmarshalJSON unmarshals raw JSON bytes into an InterchainAccountPacketData. +func (iapd *InterchainAccountPacketData) UnmarshalJSON(bz []byte) error { + return ModuleCdc.UnmarshalJSON(bz, iapd) +} + // GetBytes returns the JSON marshalled interchain account CosmosTx. func (ct CosmosTx) GetBytes() []byte { return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&ct)) diff --git a/modules/apps/27-interchain-accounts/types/packet_test.go b/modules/apps/27-interchain-accounts/types/packet_test.go index c1ff4753652..6708ff2adaf 100644 --- a/modules/apps/27-interchain-accounts/types/packet_test.go +++ b/modules/apps/27-interchain-accounts/types/packet_test.go @@ -180,3 +180,24 @@ func (suite *TypesTestSuite) TestPacketDataProvider() { suite.Require().Equal(tc.expCustomData, customData) } } + +func (suite *TypesTestSuite) TestPacketDataUnmarshalerInterface() { + expPacketData := types.InterchainAccountPacketData{ + Type: types.EXECUTE_TX, + Data: []byte("data"), + Memo: "some memo", + } + + var packetData types.InterchainAccountPacketData + err := packetData.UnmarshalJSON(expPacketData.GetBytes()) + suite.Require().NoError(err) + suite.Require().Equal(expPacketData, packetData) + + // test invalid packet data + invalidPacketDataBytes := []byte("invalid packet data") + + var invalidPacketData types.InterchainAccountPacketData + err = packetData.UnmarshalJSON(invalidPacketDataBytes) + suite.Require().Error(err) + suite.Require().Equal(types.InterchainAccountPacketData{}, invalidPacketData) +} From 48a7baf57653ebb94c575f9e528dcc9f4a979fdd Mon Sep 17 00:00:00 2001 From: srdtrk <59252793+srdtrk@users.noreply.github.com> Date: Mon, 21 Aug 2023 17:48:44 +0300 Subject: [PATCH 3/3] test(callbacks): checking that processCallback consumes gas (#4381) * imp(callbacks_test): checking that processCallback consumes gas from callback execution * imp(callbacks_test): simplified success test case a bit --- modules/apps/callbacks/ibc_middleware_test.go | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index 83383eee239..c995450ce50 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -741,6 +741,7 @@ func (s *CallbacksTestSuite) TestProcessCallback() { callbackData types.CallbackData ctx sdk.Context callbackExecutor func(sdk.Context) error + expGasConsumed uint64 ) callbackError := fmt.Errorf("callbackExecutor error") @@ -761,6 +762,7 @@ func (s *CallbacksTestSuite) TestProcessCallback() { "success: callbackExecutor panic, but not out of gas", func() { callbackExecutor = func(cachedCtx sdk.Context) error { + cachedCtx.GasMeter().ConsumeGas(expGasConsumed, "callbackExecutor gas consumption") panic("callbackExecutor panic") } }, @@ -771,8 +773,9 @@ func (s *CallbacksTestSuite) TestProcessCallback() { "success: callbackExecutor oog panic, but retry is not allowed", func() { executionGas := callbackData.ExecutionGasLimit + expGasConsumed = executionGas callbackExecutor = func(cachedCtx sdk.Context) error { - cachedCtx.GasMeter().ConsumeGas(executionGas+1, "callbackExecutor oog panic") + cachedCtx.GasMeter().ConsumeGas(expGasConsumed+1, "callbackExecutor gas consumption") return nil } }, @@ -783,6 +786,7 @@ func (s *CallbacksTestSuite) TestProcessCallback() { "failure: callbackExecutor error", func() { callbackExecutor = func(cachedCtx sdk.Context) error { + cachedCtx.GasMeter().ConsumeGas(expGasConsumed, "callbackExecutor gas consumption") return callbackError } }, @@ -794,6 +798,7 @@ func (s *CallbacksTestSuite) TestProcessCallback() { func() { callbackType = types.CallbackTypeSendPacket callbackExecutor = func(cachedCtx sdk.Context) error { + cachedCtx.GasMeter().ConsumeGas(expGasConsumed, "callbackExecutor gas consumption") panic("callbackExecutor panic") } }, @@ -805,6 +810,7 @@ func (s *CallbacksTestSuite) TestProcessCallback() { func() { executionGas := callbackData.ExecutionGasLimit callbackData.CommitGasLimit = executionGas + 1 + expGasConsumed = executionGas callbackExecutor = func(cachedCtx sdk.Context) error { cachedCtx.GasMeter().ConsumeGas(executionGas+1, "callbackExecutor oog panic") return nil @@ -823,18 +829,22 @@ func (s *CallbacksTestSuite) TestProcessCallback() { // set a callback data that does not allow retry callbackData = types.CallbackData{ CallbackAddress: s.chainB.SenderAccount.GetAddress().String(), - ExecutionGasLimit: 1000000, + ExecutionGasLimit: 1_000_000, SenderAddress: s.chainB.SenderAccount.GetAddress().String(), - CommitGasLimit: 600000, + CommitGasLimit: 600_000, } // this only makes a difference if it is SendPacket callbackType = types.CallbackTypeReceivePacket + // expGasConsumed can be overwritten in malleate + expGasConsumed = 300_000 + ctx = s.chainB.GetContext() - // set a callback executor that will always succeed + // set a callback executor that will always succeed after consuming expGasConsumed callbackExecutor = func(cachedCtx sdk.Context) error { + cachedCtx.GasMeter().ConsumeGas(expGasConsumed, "callbackExecutor gas consumption") return nil } @@ -862,6 +872,8 @@ func (s *CallbacksTestSuite) TestProcessCallback() { processCallback() s.Require().ErrorIs(tc.expValue.(error), err) } + + s.Require().Equal(expGasConsumed, ctx.GasMeter().GasConsumed()) }) } }