From 5849d5a11837310c98612792586eebb9c20519e8 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 23 Mar 2021 18:02:49 -0400 Subject: [PATCH 01/10] make changes and start fixing tests --- .../apps/transfer/keeper/mbt_relay_test.go | 2 +- modules/apps/transfer/keeper/relay.go | 22 ++-- modules/apps/transfer/keeper/relay_test.go | 2 +- modules/apps/transfer/module.go | 8 +- .../core/03-connection/keeper/verify_test.go | 8 +- modules/core/04-channel/keeper/packet_test.go | 123 +++++++++--------- .../core/04-channel/keeper/timeout_test.go | 54 ++++---- modules/core/05-port/types/module.go | 5 +- modules/core/keeper/msg_server.go | 8 +- modules/core/keeper/msg_server_test.go | 57 ++++---- .../07-tendermint/types/client_state_test.go | 8 +- testing/chain.go | 5 +- testing/mock/mock.go | 16 ++- 13 files changed, 168 insertions(+), 150 deletions(-) diff --git a/modules/apps/transfer/keeper/mbt_relay_test.go b/modules/apps/transfer/keeper/mbt_relay_test.go index 4130845c8a7..345d00dbb8a 100644 --- a/modules/apps/transfer/keeper/mbt_relay_test.go +++ b/modules/apps/transfer/keeper/mbt_relay_test.go @@ -345,7 +345,7 @@ func (suite *KeeperTestSuite) TestModelBasedRelay() { 0) } case "OnRecvPacket": - err = suite.chainB.App.TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, tc.packet.Data) + _, err = suite.chainB.App.TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, tc.packet.Data) case "OnTimeoutPacket": registerDenom() err = suite.chainB.App.TransferKeeper.OnTimeoutPacket(suite.chainB.GetContext(), packet, tc.packet.Data) diff --git a/modules/apps/transfer/keeper/relay.go b/modules/apps/transfer/keeper/relay.go index a4ce016de30..d29cedeae32 100644 --- a/modules/apps/transfer/keeper/relay.go +++ b/modules/apps/transfer/keeper/relay.go @@ -183,20 +183,20 @@ func (k Keeper) SendTransfer( // and sent to the receiving address. Otherwise if the sender chain is sending // back tokens this chain originally transferred to it, the tokens are // unescrowed and sent to the receiving address. -func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData) error { +func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData) (revert bool, err error) { // validate packet data upon receiving if err := data.ValidateBasic(); err != nil { - return err + return false, err } if !k.GetReceiveEnabled(ctx) { - return types.ErrReceiveDisabled + return false, types.ErrReceiveDisabled } // decode the receiver address receiver, err := sdk.AccAddressFromBech32(data.Receiver) if err != nil { - return err + return false, err } labels := []metrics.Label{ @@ -237,7 +237,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t // counterparty module. The bug may occur in bank or any part of the code that allows // the escrow address to be drained. A malicious counterparty module could drain the // escrow address by allowing more tokens to be sent back then were escrowed. - return sdkerrors.Wrap(err, "unable to unescrow tokens, this may be caused by a malicious counterparty module or a bug: please open an issue on counterparty module") + return false, sdkerrors.Wrap(err, "unable to unescrow tokens, this may be caused by a malicious counterparty module or a bug: please open an issue on counterparty module") } defer func() { @@ -256,7 +256,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t ) }() - return nil + return false, nil } // sender chain is the source, mint vouchers @@ -289,14 +289,18 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t if err := k.bankKeeper.MintCoins( ctx, types.ModuleName, sdk.NewCoins(voucher), ); err != nil { - return err + // error returned by bank keeper, revert all state changes by callback + // and continue tx processing + return true, err } // send to receiver if err := k.bankKeeper.SendCoinsFromModuleToAccount( ctx, types.ModuleName, receiver, sdk.NewCoins(voucher), ); err != nil { - panic(fmt.Sprintf("unable to send coins from module to account despite previously minting coins to module account: %v", err)) + // error returned by bank keeper, revert all state changes by callback + // and continue tx processing + return true, err } defer func() { @@ -315,7 +319,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t ) }() - return nil + return false, nil } // OnAcknowledgementPacket responds to the the success or failure of a packet diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index 2f754e60597..ff4d06565d2 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -219,7 +219,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.Uint64(), suite.chainA.SenderAccount.GetAddress().String(), receiver) packet := channeltypes.NewPacket(data.GetBytes(), seq, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) - err = suite.chainB.App.TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, data) + _, err = suite.chainB.App.TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, data) if tc.expPass { suite.Require().NoError(err) diff --git a/modules/apps/transfer/module.go b/modules/apps/transfer/module.go index f4620ee9283..4476cb0e7e8 100644 --- a/modules/apps/transfer/module.go +++ b/modules/apps/transfer/module.go @@ -322,15 +322,15 @@ func (am AppModule) OnChanCloseConfirm( func (am AppModule) OnRecvPacket( ctx sdk.Context, packet channeltypes.Packet, -) (*sdk.Result, []byte, error) { +) (*sdk.Result, []byte, bool, error) { var data types.FungibleTokenPacketData if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { - return nil, nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error()) + return nil, nil, false, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error()) } acknowledgement := channeltypes.NewResultAcknowledgement([]byte{byte(1)}) - err := am.keeper.OnRecvPacket(ctx, packet, data) + revert, err := am.keeper.OnRecvPacket(ctx, packet, data) if err != nil { acknowledgement = channeltypes.NewErrorAcknowledgement(err.Error()) } @@ -349,7 +349,7 @@ func (am AppModule) OnRecvPacket( // NOTE: acknowledgement will be written synchronously during IBC handler execution. return &sdk.Result{ Events: ctx.EventManager().Events().ToABCIEvents(), - }, acknowledgement.GetBytes(), nil + }, acknowledgement.GetBytes(), revert, nil } // OnAcknowledgementPacket implements the IBCModule interface diff --git a/modules/core/03-connection/keeper/verify_test.go b/modules/core/03-connection/keeper/verify_test.go index d11db9d77ef..85e14143271 100644 --- a/modules/core/03-connection/keeper/verify_test.go +++ b/modules/core/03-connection/keeper/verify_test.go @@ -283,7 +283,7 @@ func (suite *KeeperTestSuite) TestVerifyPacketCommitment() { connection.ClientId = ibctesting.InvalidID } - packet := channeltypes.NewPacket(ibctesting.TestHash, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, defaultTimeoutHeight, 0) + packet := channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, defaultTimeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -344,7 +344,7 @@ func (suite *KeeperTestSuite) TestVerifyPacketAcknowledgement() { } // send and receive packet - packet := channeltypes.NewPacket(ibctesting.TestHash, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, defaultTimeoutHeight, 0) + packet := channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, defaultTimeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -412,7 +412,7 @@ func (suite *KeeperTestSuite) TestVerifyPacketReceiptAbsence() { } // send, only receive if specified - packet := channeltypes.NewPacket(ibctesting.TestHash, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, defaultTimeoutHeight, 0) + packet := channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, defaultTimeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -481,7 +481,7 @@ func (suite *KeeperTestSuite) TestVerifyNextSequenceRecv() { } // send and receive packet - packet := channeltypes.NewPacket(ibctesting.TestHash, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, defaultTimeoutHeight, 0) + packet := channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, defaultTimeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index aa9bc3167c3..2ba823c9379 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -14,7 +14,6 @@ import ( ) var ( - validPacketData = []byte("VALID PACKET DATA") disabledTimeoutTimestamp = uint64(0) disabledTimeoutHeight = clienttypes.ZeroHeight() timeoutHeight = clienttypes.NewHeight(0, 100) @@ -39,23 +38,23 @@ func (suite *KeeperTestSuite) TestSendPacket() { testCases := []testCase{ {"success: UNORDERED channel", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, true}, {"success: ORDERED channel", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, true}, {"sending packet out of order on UNORDERED channel", func() { // setup creates an unordered channel _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 5, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 5, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"sending packet out of order on ORDERED channel", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 5, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 5, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"packet basic validation failed, empty packet data", func() { @@ -66,12 +65,12 @@ func (suite *KeeperTestSuite) TestSendPacket() { {"channel not found", func() { // use wrong channel naming _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"channel closed", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) err := suite.coordinator.SetChannelClosed(suite.chainA, suite.chainB, channelA) suite.Require().NoError(err) @@ -79,13 +78,13 @@ func (suite *KeeperTestSuite) TestSendPacket() { {"packet dest port ≠ channel counterparty port", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong port for dest - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"packet dest channel ID ≠ channel counterparty channel ID", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong channel for dest - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"connection not found", func() { @@ -97,7 +96,7 @@ func (suite *KeeperTestSuite) TestSendPacket() { channelA.PortID, channelA.ID, types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelB.PortID, channelB.ID), []string{connIDA}, channelA.Version), ) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) suite.chainA.CreateChannelCapability(channelA.PortID, channelA.ID) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, @@ -109,7 +108,7 @@ func (suite *KeeperTestSuite) TestSendPacket() { connection.ClientId = ibctesting.InvalidID suite.chainA.App.IBCKeeper.ConnectionKeeper.SetConnection(suite.chainA.GetContext(), connA.ID, connection) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"client state is frozen", func() { @@ -124,7 +123,7 @@ func (suite *KeeperTestSuite) TestSendPacket() { cs.FrozenHeight = clienttypes.NewHeight(0, 1) suite.chainA.App.IBCKeeper.ClientKeeper.SetClientState(suite.chainA.GetContext(), connection.ClientId, cs) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, @@ -132,7 +131,7 @@ func (suite *KeeperTestSuite) TestSendPacket() { clientA, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use client state latest height for timeout clientState := suite.chainA.GetClientState(clientA) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clientState.GetLatestHeight().(clienttypes.Height), disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clientState.GetLatestHeight().(clienttypes.Height), disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"timeout timestamp passed", func() { @@ -143,14 +142,14 @@ func (suite *KeeperTestSuite) TestSendPacket() { timestamp, err := suite.chainA.App.IBCKeeper.ConnectionKeeper.GetTimestampAtHeight(suite.chainA.GetContext(), connection, clientState.GetLatestHeight()) suite.Require().NoError(err) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, disabledTimeoutHeight, timestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, disabledTimeoutHeight, timestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"next sequence send not found", func() { _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint) channelA := suite.chainA.NextTestChannel(connA, ibctesting.TransferPort) channelB := suite.chainB.NextTestChannel(connB, ibctesting.TransferPort) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) // manually creating channel prevents next sequence from being set suite.chainA.App.IBCKeeper.ChannelKeeper.SetChannel( suite.chainA.GetContext(), @@ -162,13 +161,13 @@ func (suite *KeeperTestSuite) TestSendPacket() { }, false}, {"next sequence wrong", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) suite.chainA.App.IBCKeeper.ChannelKeeper.SetNextSequenceSend(suite.chainA.GetContext(), channelA.PortID, channelA.ID, 5) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"channel capability not found", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = capabilitytypes.NewCapability(5) }, false}, } @@ -204,7 +203,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() { testCases := []testCase{ {"success: ORDERED channel", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) @@ -212,7 +211,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() { {"success UNORDERED channel", func() { // setup uses an UNORDERED channel _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) @@ -220,13 +219,13 @@ func (suite *KeeperTestSuite) TestRecvPacket() { {"success with out of order packet: UNORDERED channel", func() { // setup uses an UNORDERED channel _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) // send 2 packets err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) // set sequence to 2 - packet = types.NewPacket(validPacketData, 2, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 2, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) err = suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) // attempts to receive packet 2 without receiving packet 1 @@ -234,13 +233,13 @@ func (suite *KeeperTestSuite) TestRecvPacket() { }, true}, {"out of order packet failure with ORDERED channel", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) // send 2 packets err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) // set sequence to 2 - packet = types.NewPacket(validPacketData, 2, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 2, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) err = suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) // attempts to receive packet 2 without receiving packet 1 @@ -249,12 +248,12 @@ func (suite *KeeperTestSuite) TestRecvPacket() { {"channel not found", func() { // use wrong channel naming _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"channel not open", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) err := suite.coordinator.SetChannelClosed(suite.chainB, suite.chainA, channelB) suite.Require().NoError(err) @@ -262,7 +261,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() { }, false}, {"capability cannot authenticate", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) channelCap = capabilitytypes.NewCapability(3) @@ -270,13 +269,13 @@ func (suite *KeeperTestSuite) TestRecvPacket() { {"packet source port ≠ channel counterparty port", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong port for dest - packet = types.NewPacket(validPacketData, 1, ibctesting.InvalidID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, ibctesting.InvalidID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"packet source channel ID ≠ channel counterparty channel ID", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong port for dest - packet = types.NewPacket(validPacketData, 1, channelA.PortID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"connection not found", func() { @@ -288,7 +287,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() { channelB.PortID, channelB.ID, types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelA.PortID, channelA.ID), []string{connIDB}, channelB.Version), ) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) suite.chainB.CreateChannelCapability(channelB.PortID, channelB.ID) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, @@ -306,18 +305,18 @@ func (suite *KeeperTestSuite) TestRecvPacket() { channelB.PortID, channelB.ID, types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelA.PortID, channelA.ID), []string{connB.ID}, channelB.Version), ) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) suite.chainB.CreateChannelCapability(channelB.PortID, channelB.ID) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"timeout height passed", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"timeout timestamp passed", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, disabledTimeoutHeight, uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, disabledTimeoutHeight, uint64(suite.chainB.GetContext().BlockTime().UnixNano())) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"next receive sequence is not found", func() { @@ -332,17 +331,17 @@ func (suite *KeeperTestSuite) TestRecvPacket() { types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelA.PortID, channelA.ID), []string{connB.ID}, channelB.Version), ) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) // manually set packet commitment - suite.chainA.App.IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), channelA.PortID, channelA.ID, packet.GetSequence(), ibctesting.TestHash) + suite.chainA.App.IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), channelA.PortID, channelA.ID, packet.GetSequence(), ibctesting.MockPacketData) suite.chainB.CreateChannelCapability(channelB.PortID, channelB.ID) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"receipt already stored", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.chainB.App.IBCKeeper.ChannelKeeper.SetPacketReceipt(suite.chainB.GetContext(), channelB.PortID, channelB.ID, 1) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) @@ -350,7 +349,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() { {"validation failed", func() { // packet commitment not set resulting in invalid proof _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, } @@ -403,8 +402,8 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { "success", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - ack = ibctesting.TestHash + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + ack = ibctesting.MockAcknowledgement channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, true, @@ -412,14 +411,14 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { {"channel not found", func() { // use wrong channel naming _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) - ack = ibctesting.TestHash + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) + ack = ibctesting.MockAcknowledgement channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, false}, {"channel not open", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - ack = ibctesting.TestHash + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + ack = ibctesting.MockAcknowledgement err := suite.coordinator.SetChannelClosed(suite.chainB, suite.chainA, channelB) suite.Require().NoError(err) @@ -429,8 +428,8 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { "capability authentication failed", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - ack = ibctesting.TestHash + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + ack = ibctesting.MockAcknowledgement channelCap = capabilitytypes.NewCapability(3) }, false, @@ -439,8 +438,8 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { "no-op, already acked", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) - ack = ibctesting.TestHash + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + ack = ibctesting.MockAcknowledgement suite.chainB.App.IBCKeeper.ChannelKeeper.SetPacketAcknowledgement(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ack) channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, @@ -450,7 +449,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { "empty acknowledgement", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) ack = nil channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID) }, @@ -487,7 +486,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { testCases := []testCase{ {"success on ordered channel", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) // create packet commitment err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -501,7 +500,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { {"success on unordered channel", func() { // setup uses an UNORDERED channel clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) // create packet commitment err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) @@ -516,11 +515,11 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { {"channel not found", func() { // use wrong channel naming _, _, _, _, _, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) }, false}, {"channel not open", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) err := suite.coordinator.SetChannelClosed(suite.chainA, suite.chainB, channelA) suite.Require().NoError(err) @@ -528,7 +527,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { }, false}, {"capability authentication failed", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) // create packet commitment err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -542,13 +541,13 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { {"packet destination port ≠ channel counterparty port", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong port for dest - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"packet destination channel ID ≠ channel counterparty channel ID", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong channel for dest - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"connection not found", func() { @@ -560,7 +559,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { channelB.PortID, channelB.ID, types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelA.PortID, channelA.ID), []string{connIDB}, channelB.Version), ) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) suite.chainA.CreateChannelCapability(channelA.PortID, channelA.ID) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, @@ -578,20 +577,20 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { channelA.PortID, channelA.ID, types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelB.PortID, channelB.ID), []string{connA.ID}, channelA.Version), ) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) suite.chainA.CreateChannelCapability(channelA.PortID, channelA.ID) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"packet hasn't been sent", func() { // packet commitment never written _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"packet ack verification failed", func() { // ack never written _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) // create packet commitment suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) @@ -601,7 +600,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint) channelA := suite.chainA.NextTestChannel(connA, ibctesting.TransferPort) channelB := suite.chainB.NextTestChannel(connB, ibctesting.TransferPort) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) // manually creating channel prevents next sequence acknowledgement from being set suite.chainA.App.IBCKeeper.ChannelKeeper.SetChannel( suite.chainA.GetContext(), @@ -609,16 +608,16 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelB.PortID, channelB.ID), []string{connA.ID}, channelA.Version), ) // manually set packet commitment - suite.chainA.App.IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), channelA.PortID, channelA.ID, packet.GetSequence(), ibctesting.TestHash) + suite.chainA.App.IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), channelA.PortID, channelA.ID, packet.GetSequence(), ibctesting.MockPacketData) // manually set packet acknowledgement and capability - suite.chainB.App.IBCKeeper.ChannelKeeper.SetPacketAcknowledgement(suite.chainB.GetContext(), channelB.PortID, channelB.ID, packet.GetSequence(), ibctesting.TestHash) + suite.chainB.App.IBCKeeper.ChannelKeeper.SetPacketAcknowledgement(suite.chainB.GetContext(), channelB.PortID, channelB.ID, packet.GetSequence(), ibctesting.MockAcknowledgement) suite.chainA.CreateChannelCapability(channelA.PortID, channelA.ID) channelCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"next ack sequence mismatch", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) // create packet commitment err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) diff --git a/modules/core/04-channel/keeper/timeout_test.go b/modules/core/04-channel/keeper/timeout_test.go index 4c286690f94..94c4b6a008e 100644 --- a/modules/core/04-channel/keeper/timeout_test.go +++ b/modules/core/04-channel/keeper/timeout_test.go @@ -26,7 +26,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { ordered = true clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) // need to update chainA's client representing chainB to prove missing ack suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, exported.Tendermint) @@ -35,7 +35,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { ordered = false clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) // need to update chainA's client representing chainB to prove missing ack suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, exported.Tendermint) @@ -43,11 +43,11 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { {"channel not found", func() { // use wrong channel naming _, _, _, _, _, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) }, false}, {"channel not open", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) err := suite.coordinator.SetChannelClosed(suite.chainA, suite.chainB, channelA) suite.Require().NoError(err) @@ -55,12 +55,12 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { {"packet destination port ≠ channel counterparty port", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong port for dest - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) }, false}, {"packet destination channel ID ≠ channel counterparty channel ID", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong channel for dest - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) }, false}, {"connection not found", func() { channelA := ibctesting.TestChannel{PortID: portID, ID: channelIDA} @@ -71,11 +71,11 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { channelA.PortID, channelA.ID, types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelB.PortID, channelB.ID), []string{connIDA}, channelA.Version), ) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) }, false}, {"timeout", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, exported.Tendermint) }, false}, @@ -84,13 +84,13 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { nextSeqRecv = 2 clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, uint64(suite.chainB.GetContext().BlockTime().UnixNano())) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, exported.Tendermint) }, false}, {"packet hasn't been sent", func() { clientA, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, uint64(suite.chainB.GetContext().BlockTime().UnixNano())) suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, exported.Tendermint) }, false}, {"next seq receive verification failed", func() { @@ -98,7 +98,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { ordered = false clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, exported.Tendermint) }, false}, @@ -107,7 +107,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { ordered = true clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, exported.Tendermint) }, false}, @@ -156,7 +156,7 @@ func (suite *KeeperTestSuite) TestTimeoutExecuted() { testCases := []testCase{ {"success ORDERED", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) chanCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) @@ -164,11 +164,11 @@ func (suite *KeeperTestSuite) TestTimeoutExecuted() { {"channel not found", func() { // use wrong channel naming _, _, _, _, _, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) }, false}, {"incorrect capability", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) chanCap = capabilitytypes.NewCapability(100) @@ -209,7 +209,7 @@ func (suite *KeeperTestSuite) TestTimeoutOnClose() { {"success: ORDERED", func() { ordered = true clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.coordinator.SetChannelClosed(suite.chainB, suite.chainA, channelB) // need to update chainA's client representing chainB to prove missing ack @@ -220,7 +220,7 @@ func (suite *KeeperTestSuite) TestTimeoutOnClose() { {"success: UNORDERED", func() { ordered = false clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.coordinator.SetChannelClosed(suite.chainB, suite.chainA, channelB) // need to update chainA's client representing chainB to prove missing ack @@ -231,18 +231,18 @@ func (suite *KeeperTestSuite) TestTimeoutOnClose() { {"channel not found", func() { // use wrong channel naming _, _, _, _, _, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) }, false}, {"packet dest port ≠ channel counterparty port", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong port for dest - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, ibctesting.InvalidID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) chanCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"packet dest channel ID ≠ channel counterparty channel ID", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) // use wrong channel for dest - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp) chanCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"connection not found", func() { @@ -254,7 +254,7 @@ func (suite *KeeperTestSuite) TestTimeoutOnClose() { channelA.PortID, channelA.ID, types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelB.PortID, channelB.ID), []string{connIDA}, channelA.Version), ) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp) // create chancap suite.chainA.CreateChannelCapability(channelA.PortID, channelA.ID) @@ -262,14 +262,14 @@ func (suite *KeeperTestSuite) TestTimeoutOnClose() { }, false}, {"packet hasn't been sent", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) chanCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, {"packet already received", func() { nextSeqRecv = 2 ordered = true clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.coordinator.SetChannelClosed(suite.chainB, suite.chainA, channelB) // need to update chainA's client representing chainB to prove missing ack @@ -280,7 +280,7 @@ func (suite *KeeperTestSuite) TestTimeoutOnClose() { {"channel verification failed", func() { ordered = true _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) chanCap = suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) }, false}, @@ -288,7 +288,7 @@ func (suite *KeeperTestSuite) TestTimeoutOnClose() { // set ordered to false providing the wrong proof for ORDERED case ordered = false clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.coordinator.SetChannelClosed(suite.chainB, suite.chainA, channelB) suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, exported.Tendermint) @@ -298,7 +298,7 @@ func (suite *KeeperTestSuite) TestTimeoutOnClose() { // set ordered to true providing the wrong proof for UNORDERED case ordered = true clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.UNORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.coordinator.SetChannelClosed(suite.chainB, suite.chainA, channelB) suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, exported.Tendermint) @@ -307,7 +307,7 @@ func (suite *KeeperTestSuite) TestTimeoutOnClose() { {"channel capability not found", func() { ordered = true clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, types.ORDERED) - packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = types.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.coordinator.SetChannelClosed(suite.chainB, suite.chainA, channelB) // need to update chainA's client representing chainB to prove missing ack diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index 91ee642f002..c9489fcd578 100644 --- a/modules/core/05-port/types/module.go +++ b/modules/core/05-port/types/module.go @@ -60,10 +60,13 @@ type IBCModule interface { // OnRecvPacket must return the acknowledgement bytes // In the case of an asynchronous acknowledgement, nil should be returned. + // OnRecvPacket must also return a revert boolean. If true, the state changes on callback + // are reverted, the rest of the transaction is still processed. Thus, the packet is successfully + // received and any returned acknowledgement is written. OnRecvPacket( ctx sdk.Context, packet channeltypes.Packet, - ) (*sdk.Result, []byte, error) + ) (*sdk.Result, []byte, bool, error) OnAcknowledgementPacket( ctx sdk.Context, diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index d931abedb36..02714df7e78 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -443,10 +443,16 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke } // Perform application logic callback - _, ack, err := cbs.OnRecvPacket(ctx, msg.Packet) + // Cache context so that we may discard state changes from callback if revert is true. + cacheCtx, writeFn := ctx.CacheContext() + _, ack, revert, err := cbs.OnRecvPacket(cacheCtx, msg.Packet) if err != nil { return nil, sdkerrors.Wrap(err, "receive packet callback failed") } + // If revert is false, persist callback state changes by writing to underlying parent store. + if !revert { + writeFn() + } // Set packet acknowledgement only if the acknowledgement is not nil. // NOTE: IBC applications modules may call the WriteAcknowledgement asynchronously if the diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index 18830d79f94..34af8470741 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/suite" sdk "github.com/cosmos/cosmos-sdk/types" + upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" clienttypes "github.com/cosmos/ibc-go/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" commitmenttypes "github.com/cosmos/ibc-go/modules/core/23-commitment/types" @@ -15,7 +16,6 @@ import ( ibctmtypes "github.com/cosmos/ibc-go/modules/light-clients/07-tendermint/types" ibctesting "github.com/cosmos/ibc-go/testing" ibcmock "github.com/cosmos/ibc-go/testing/mock" - upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" ) const height = 10 @@ -66,14 +66,14 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { }{ {"success: ORDERED", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) }, true}, {"success: UNORDERED", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -84,7 +84,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { // attempts to receive packet with sequence 10 without receiving packet with sequence 1 for i := uint64(1); i < 10; i++ { - packet = channeltypes.NewPacket(ibctesting.MockCommitment, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -95,7 +95,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { // attempts to receive packet with sequence 10 without receiving packet with sequence 1 for i := uint64(1); i < 10; i++ { - packet = channeltypes.NewPacket(ibctesting.MockCommitment, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -107,11 +107,11 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { }, false}, {"packet not sent", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) }, false}, {"ORDERED: packet already received (replay)", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -122,7 +122,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { {"UNORDERED: packet already received (replay)", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -184,7 +184,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { }{ {"success: ORDERED", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -194,7 +194,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { }, true}, {"success: UNORDERED", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -208,7 +208,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { // attempts to acknowledge ack with sequence 10 without acknowledging ack with sequence 1 (removing packet commitment) for i := uint64(1); i < 10; i++ { - packet = channeltypes.NewPacket(ibctesting.MockCommitment, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -222,7 +222,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { // attempts to acknowledge ack with sequence 10 without acknowledging ack with sequence 1 (removing packet commitment for i := uint64(1); i < 10; i++ { - packet = channeltypes.NewPacket(ibctesting.MockCommitment, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -237,14 +237,14 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { }, false}, {"packet not received", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) }, false}, {"ORDERED: packet already acknowledged (replay)", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -252,13 +252,13 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) - err = suite.coordinator.AcknowledgePacket(suite.chainA, suite.chainB, clientB, packet, ibctesting.TestHash) + err = suite.coordinator.AcknowledgePacket(suite.chainA, suite.chainB, clientB, packet, ibctesting.MockAcknowledgement) suite.Require().NoError(err) }, false}, {"UNORDERED: packet already acknowledged (replay)", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) @@ -266,7 +266,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) - err = suite.coordinator.AcknowledgePacket(suite.chainA, suite.chainB, clientB, packet, ibctesting.TestHash) + err = suite.coordinator.AcknowledgePacket(suite.chainA, suite.chainB, clientB, packet, ibctesting.MockAcknowledgement) suite.Require().NoError(err) }, false}, } @@ -276,7 +276,6 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { suite.Run(tc.name, func() { suite.SetupTest() // reset - ibctesting.TestHash = ibctesting.MockAcknowledgement tc.malleate() @@ -323,7 +322,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { }{ {"success: ORDERED", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) // create packet commitment err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) @@ -336,7 +335,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { }, true}, {"success: UNORDERED", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano())) // create packet commitment err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) @@ -354,7 +353,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { // attempts to timeout the last packet sent without timing out the first packet // packet sequences begin at 1 for i := uint64(1); i < maxSequence; i++ { - packet = channeltypes.NewPacket(ibctesting.MockCommitment, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), 0) // create packet commitment err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) @@ -370,7 +369,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { // attempts to timeout the last packet sent without timing out the first packet // packet sequences begin at 1 for i := uint64(1); i < maxSequence; i++ { - packet = channeltypes.NewPacket(ibctesting.MockCommitment, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), 0) // create packet commitment err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) @@ -389,7 +388,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() { }, false}, {"UNORDERED: packet not sent", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) }, false}, } @@ -445,7 +444,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { }{ {"success: ORDERED", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) counterpartyChannel = ibctesting.TestChannel{ PortID: channelB.PortID, ID: channelB.ID, @@ -466,7 +465,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { }, true}, {"success: UNORDERED", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) counterpartyChannel = ibctesting.TestChannel{ PortID: channelB.PortID, ID: channelB.ID, @@ -497,7 +496,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { // attempts to timeout the last packet sent without timing out the first packet // packet sequences begin at 1 for i := uint64(1); i < maxSequence; i++ { - packet = channeltypes.NewPacket(ibctesting.MockCommitment, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) // create packet commitment err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) @@ -521,7 +520,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { // attempts to timeout the last packet sent without timing out the first packet // packet sequences begin at 1 for i := uint64(1); i < maxSequence; i++ { - packet = channeltypes.NewPacket(ibctesting.MockCommitment, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, i, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) // create packet commitment err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) @@ -542,7 +541,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { }, false}, {"UNORDERED: packet not sent", func() { clientA, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) packetKey = host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) counterpartyChannel = ibctesting.TestChannel{ PortID: channelB.PortID, @@ -555,7 +554,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() { }, false}, {"ORDERED: channel not closed", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) - packet = channeltypes.NewPacket(ibctesting.MockCommitment, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) counterpartyChannel = ibctesting.TestChannel{ PortID: channelB.PortID, ID: channelB.ID, diff --git a/modules/light-clients/07-tendermint/types/client_state_test.go b/modules/light-clients/07-tendermint/types/client_state_test.go index 914851fa9e8..bf0811ebbaa 100644 --- a/modules/light-clients/07-tendermint/types/client_state_test.go +++ b/modules/light-clients/07-tendermint/types/client_state_test.go @@ -435,7 +435,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketCommitment() { // setup testing conditions clientA, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet := channeltypes.NewPacket(ibctesting.TestHash, 1, channelB.PortID, channelB.ID, channelA.PortID, channelA.ID, clienttypes.NewHeight(0, 100), 0) + packet := channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelB.PortID, channelB.ID, channelA.PortID, channelA.ID, clienttypes.NewHeight(0, 100), 0) err := suite.coordinator.SendPacket(suite.chainB, suite.chainA, packet, clientA) suite.Require().NoError(err) @@ -534,7 +534,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgement() { // setup testing conditions clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet := channeltypes.NewPacket(ibctesting.TestHash, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) + packet := channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) // send packet err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) @@ -638,7 +638,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketReceiptAbsence() { // setup testing conditions clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) - packet := channeltypes.NewPacket(ibctesting.TestHash, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) + packet := channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) // send packet, but no recv err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) @@ -741,7 +741,7 @@ func (suite *TendermintTestSuite) TestVerifyNextSeqRecv() { // setup testing conditions clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) - packet := channeltypes.NewPacket(ibctesting.TestHash, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) + packet := channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) // send packet err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) diff --git a/testing/chain.go b/testing/chain.go index 6f1f4e781aa..3516dd700f5 100644 --- a/testing/chain.go +++ b/testing/chain.go @@ -65,7 +65,6 @@ var ( // Default params variables used to create a TM client DefaultTrustLevel ibctmtypes.Fraction = ibctmtypes.DefaultTrustLevel - TestHash = tmhash.Sum([]byte("TESTING HASH")) TestCoin = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)) UpgradePath = []string{"upgrade", "upgradedIBCState"} @@ -73,7 +72,7 @@ var ( ConnectionVersion = connectiontypes.ExportedVersionsToProto(connectiontypes.GetCompatibleVersions())[0] MockAcknowledgement = mock.MockAcknowledgement - MockCommitment = mock.MockCommitment + MockPacketData = mock.MockPacketData ) // TestChain is a testing struct that wraps a simapp with the last TM Header, the current ABCI @@ -897,7 +896,7 @@ func (chain *TestChain) WriteAcknowledgement( channelCap := chain.GetChannelCapability(packet.GetDestPort(), packet.GetDestChannel()) // no need to send message, acting as a handler - err := chain.App.IBCKeeper.ChannelKeeper.WriteAcknowledgement(chain.GetContext(), channelCap, packet, TestHash) + err := chain.App.IBCKeeper.ChannelKeeper.WriteAcknowledgement(chain.GetContext(), channelCap, packet, MockAcknowledgement) if err != nil { return err } diff --git a/testing/mock/mock.go b/testing/mock/mock.go index 8a709fba764..13b00c774f5 100644 --- a/testing/mock/mock.go +++ b/testing/mock/mock.go @@ -1,6 +1,7 @@ package mock import ( + "bytes" "encoding/json" "github.com/cosmos/cosmos-sdk/types/module" @@ -27,8 +28,10 @@ const ( ) var ( - MockAcknowledgement = []byte("mock acknowledgement") - MockCommitment = []byte("mock packet commitment") + MockAcknowledgement = []byte("mock acknowledgement") + MockFailAcknowledgement = []byte("mock failed acknowledgement") + MockPacketData = []byte("mock packet data") + MockFailPacketData = []byte("mock failed packet data") ) // AppModuleBasic is the mock AppModuleBasic. @@ -171,8 +174,13 @@ func (am AppModule) OnChanCloseConfirm(sdk.Context, string, string) error { } // OnRecvPacket implements the IBCModule interface. -func (am AppModule) OnRecvPacket(sdk.Context, channeltypes.Packet) (*sdk.Result, []byte, error) { - return nil, MockAcknowledgement, nil +func (am AppModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) (*sdk.Result, []byte, bool, error) { + // set state by claiming capability to check if revert happens return + am.scopedKeeper.NewCapability(ctx, "test") + if bytes.Equal(MockPacketData, packet.GetData()) { + return nil, MockAcknowledgement, false, nil + } + return nil, MockFailAcknowledgement, true, nil } // OnAcknowledgementPacket implements the IBCModule interface. From c4ea50fbb7d457b5be6cd4cc24040b638aef6193 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 23 Mar 2021 18:31:51 -0400 Subject: [PATCH 02/10] add msg server test --- modules/core/keeper/msg_server_test.go | 38 ++++++++++++++++++-------- testing/chain.go | 6 ++-- testing/mock/mock.go | 11 ++++---- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index 34af8470741..890c84f2dbc 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -60,9 +60,10 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { ) testCases := []struct { - name string - malleate func() - expPass bool + name string + malleate func() + expPass bool + expRevert bool }{ {"success: ORDERED", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) @@ -70,14 +71,14 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - }, true}, + }, true, false}, {"success: UNORDERED", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) - }, true}, + }, true, false}, {"success: UNORDERED out of order packet", func() { // setup uses an UNORDERED channel _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) @@ -89,7 +90,14 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) } - }, true}, + }, true, false}, + {"success: OnRecvPacket callback returns revert=true", func() { + _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) + packet = channeltypes.NewPacket(ibctesting.MockFailPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + + err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) + suite.Require().NoError(err) + }, true, true}, {"failure: ORDERED out of order packet", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) @@ -100,15 +108,15 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) } - }, false}, + }, false, false}, {"channel does not exist", func() { // any non-nil value of packet is valid suite.Require().NotNil(packet) - }, false}, + }, false, false}, {"packet not sent", func() { _, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) - }, false}, + }, false, false}, {"ORDERED: packet already received (replay)", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) @@ -118,7 +126,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) - }, false}, + }, false, false}, {"UNORDERED: packet already received (replay)", func() { clientA, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) @@ -129,7 +137,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { err = suite.coordinator.RecvPacket(suite.chainA, suite.chainB, clientA, packet) suite.Require().NoError(err) - }, false}, + }, false, false}, } for _, tc := range testCases { @@ -156,6 +164,14 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { _, err := keeper.Keeper.RecvPacket(*suite.chainB.App.IBCKeeper, sdk.WrapSDKContext(suite.chainB.GetContext()), msg) suite.Require().Error(err) + // check that callback state was handled correctly + _, exists := suite.chainB.App.ScopedIBCMockKeeper.GetCapability(suite.chainB.GetContext(), ibctesting.MockCanaryCapabilityName) + if tc.expRevert { + suite.Require().False(exists, "capability exists in store even after callback reverted") + } else { + suite.Require().True(exists, "callback state not persisted when revert is false") + } + // verify ack was written ack, found := suite.chainB.App.IBCKeeper.ChannelKeeper.GetPacketAcknowledgement(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) suite.Require().NotNil(ack) diff --git a/testing/chain.go b/testing/chain.go index 3516dd700f5..9fd7b234920 100644 --- a/testing/chain.go +++ b/testing/chain.go @@ -71,8 +71,10 @@ var ( ConnectionVersion = connectiontypes.ExportedVersionsToProto(connectiontypes.GetCompatibleVersions())[0] - MockAcknowledgement = mock.MockAcknowledgement - MockPacketData = mock.MockPacketData + MockAcknowledgement = mock.MockAcknowledgement + MockPacketData = mock.MockPacketData + MockFailPacketData = mock.MockFailPacketData + MockCanaryCapabilityName = mock.MockCanaryCapabilityName ) // TestChain is a testing struct that wraps a simapp with the last TM Header, the current ABCI diff --git a/testing/mock/mock.go b/testing/mock/mock.go index 13b00c774f5..c7ab7426389 100644 --- a/testing/mock/mock.go +++ b/testing/mock/mock.go @@ -28,10 +28,11 @@ const ( ) var ( - MockAcknowledgement = []byte("mock acknowledgement") - MockFailAcknowledgement = []byte("mock failed acknowledgement") - MockPacketData = []byte("mock packet data") - MockFailPacketData = []byte("mock failed packet data") + MockAcknowledgement = []byte("mock acknowledgement") + MockFailAcknowledgement = []byte("mock failed acknowledgement") + MockPacketData = []byte("mock packet data") + MockFailPacketData = []byte("mock failed packet data") + MockCanaryCapabilityName = "mock canary capability name" ) // AppModuleBasic is the mock AppModuleBasic. @@ -176,7 +177,7 @@ func (am AppModule) OnChanCloseConfirm(sdk.Context, string, string) error { // OnRecvPacket implements the IBCModule interface. func (am AppModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) (*sdk.Result, []byte, bool, error) { // set state by claiming capability to check if revert happens return - am.scopedKeeper.NewCapability(ctx, "test") + am.scopedKeeper.NewCapability(ctx, MockCanaryCapabilityName) if bytes.Equal(MockPacketData, packet.GetData()) { return nil, MockAcknowledgement, false, nil } From 00abadcbf196bff4494ec465534ba4dd59941658 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 23 Mar 2021 19:37:39 -0400 Subject: [PATCH 03/10] change to revert on SendCoins fail --- modules/apps/transfer/keeper/relay.go | 3 ++- modules/apps/transfer/keeper/relay_test.go | 17 ++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/modules/apps/transfer/keeper/relay.go b/modules/apps/transfer/keeper/relay.go index d29cedeae32..927cbc27006 100644 --- a/modules/apps/transfer/keeper/relay.go +++ b/modules/apps/transfer/keeper/relay.go @@ -237,7 +237,8 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t // counterparty module. The bug may occur in bank or any part of the code that allows // the escrow address to be drained. A malicious counterparty module could drain the // escrow address by allowing more tokens to be sent back then were escrowed. - return false, sdkerrors.Wrap(err, "unable to unescrow tokens, this may be caused by a malicious counterparty module or a bug: please open an issue on counterparty module") + // Revert callback state changes if SendCoins returns an error. + return true, sdkerrors.Wrap(err, "unable to unescrow tokens, this may be caused by a malicious counterparty module or a bug: please open an issue on counterparty module") } defer func() { diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index ff4d06565d2..debeea227aa 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -151,27 +151,28 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { malleate func() recvIsSource bool // the receiving chain is the source of the coin originally expPass bool + expRevert bool }{ - {"success receive on source chain", func() {}, true, true}, - {"success receive with coin from another chain as source", func() {}, false, true}, + {"success receive on source chain", func() {}, true, true, false}, + {"success receive with coin from another chain as source", func() {}, false, true, false}, {"empty coin", func() { trace = types.DenomTrace{} amount = sdk.ZeroInt() - }, true, false}, + }, true, false, false}, {"invalid receiver address", func() { receiver = "gaia1scqhwpgsmr6vmztaa7suurfl52my6nd2kmrudl" - }, true, false}, + }, true, false, false}, // onRecvPacket // - coin from chain chainA {"failure: mint zero coin", func() { amount = sdk.ZeroInt() - }, false, false}, + }, false, false, false}, // - coin being sent back to original chain (chainB) {"tries to unescrow more tokens than allowed", func() { amount = sdk.NewInt(1000000) - }, true, false}, + }, true, false, true}, } for _, tc := range testCases { @@ -219,12 +220,14 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.Uint64(), suite.chainA.SenderAccount.GetAddress().String(), receiver) packet := channeltypes.NewPacket(data.GetBytes(), seq, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) - _, err = suite.chainB.App.TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, data) + revert, err := suite.chainB.App.TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, data) if tc.expPass { suite.Require().NoError(err) + suite.Require().False(revert) } else { suite.Require().Error(err) + suite.Require().Equal(tc.expRevert, revert) } }) } From 1149f3c93108ec458ff837b2470f8a34a385ae5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 7 Apr 2021 13:01:33 +0200 Subject: [PATCH 04/10] update interfaces and fix tests --- modules/apps/transfer/handler_test.go | 6 +-- .../apps/transfer/keeper/mbt_relay_test.go | 2 +- modules/apps/transfer/keeper/relay.go | 19 ++++----- modules/apps/transfer/keeper/relay_test.go | 19 ++++----- modules/apps/transfer/module.go | 41 ++++++++++--------- .../core/03-connection/keeper/verify_test.go | 4 +- modules/core/04-channel/keeper/packet_test.go | 2 +- .../core/04-channel/types/acknowledgement.go | 20 ++++++--- .../04-channel/types/acknowledgement_test.go | 17 +++++--- modules/core/05-port/types/module.go | 3 +- modules/core/exported/channel.go | 7 ++++ modules/core/keeper/msg_server.go | 13 +++--- modules/core/keeper/msg_server_test.go | 3 +- .../07-tendermint/types/client_state_test.go | 2 +- testing/chain.go | 2 +- testing/mock/mock.go | 26 ++++++------ 16 files changed, 102 insertions(+), 84 deletions(-) diff --git a/modules/apps/transfer/handler_test.go b/modules/apps/transfer/handler_test.go index 5d4d95d7aa8..d797c6560ca 100644 --- a/modules/apps/transfer/handler_test.go +++ b/modules/apps/transfer/handler_test.go @@ -52,7 +52,7 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() { fungibleTokenPacket := types.NewFungibleTokenPacketData(coinToSendToB.Denom, coinToSendToB.Amount.Uint64(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String()) packet := channeltypes.NewPacket(fungibleTokenPacket.GetBytes(), 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)}) - err = suite.coordinator.RelayPacket(suite.chainA, suite.chainB, clientA, clientB, packet, ack.GetBytes()) + err = suite.coordinator.RelayPacket(suite.chainA, suite.chainB, clientA, clientB, packet, ack.Acknowledgement()) suite.Require().NoError(err) // relay committed // check that voucher exists on chain B @@ -77,7 +77,7 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() { fullDenomPath := types.GetPrefixedDenom(channelOnCForB.PortID, channelOnCForB.ID, voucherDenomTrace.GetFullDenomPath()) fungibleTokenPacket = types.NewFungibleTokenPacketData(voucherDenomTrace.GetFullDenomPath(), coinSentFromAToB.Amount.Uint64(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainC.SenderAccount.GetAddress().String()) packet = channeltypes.NewPacket(fungibleTokenPacket.GetBytes(), 1, channelOnBForC.PortID, channelOnBForC.ID, channelOnCForB.PortID, channelOnCForB.ID, timeoutHeight, 0) - err = suite.coordinator.RelayPacket(suite.chainB, suite.chainC, clientOnBForC, clientOnCForB, packet, ack.GetBytes()) + err = suite.coordinator.RelayPacket(suite.chainB, suite.chainC, clientOnBForC, clientOnCForB, packet, ack.Acknowledgement()) suite.Require().NoError(err) // relay committed coinSentFromBToC := sdk.NewInt64Coin(types.ParseDenomTrace(fullDenomPath).IBCDenom(), 100) @@ -100,7 +100,7 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() { // NOTE: fungible token is prefixed with the full trace in order to verify the packet commitment fungibleTokenPacket = types.NewFungibleTokenPacketData(fullDenomPath, coinSentFromBToC.Amount.Uint64(), suite.chainC.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String()) packet = channeltypes.NewPacket(fungibleTokenPacket.GetBytes(), 1, channelOnCForB.PortID, channelOnCForB.ID, channelOnBForC.PortID, channelOnBForC.ID, timeoutHeight, 0) - err = suite.coordinator.RelayPacket(suite.chainC, suite.chainB, clientOnCForB, clientOnBForC, packet, ack.GetBytes()) + err = suite.coordinator.RelayPacket(suite.chainC, suite.chainB, clientOnCForB, clientOnBForC, packet, ack.Acknowledgement()) suite.Require().NoError(err) // relay committed balance = suite.chainB.App.BankKeeper.GetBalance(suite.chainB.GetContext(), suite.chainB.SenderAccount.GetAddress(), coinSentFromAToB.Denom) diff --git a/modules/apps/transfer/keeper/mbt_relay_test.go b/modules/apps/transfer/keeper/mbt_relay_test.go index 345d00dbb8a..4130845c8a7 100644 --- a/modules/apps/transfer/keeper/mbt_relay_test.go +++ b/modules/apps/transfer/keeper/mbt_relay_test.go @@ -345,7 +345,7 @@ func (suite *KeeperTestSuite) TestModelBasedRelay() { 0) } case "OnRecvPacket": - _, err = suite.chainB.App.TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, tc.packet.Data) + err = suite.chainB.App.TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, tc.packet.Data) case "OnTimeoutPacket": registerDenom() err = suite.chainB.App.TransferKeeper.OnTimeoutPacket(suite.chainB.GetContext(), packet, tc.packet.Data) diff --git a/modules/apps/transfer/keeper/relay.go b/modules/apps/transfer/keeper/relay.go index 927cbc27006..ca4a4575e1e 100644 --- a/modules/apps/transfer/keeper/relay.go +++ b/modules/apps/transfer/keeper/relay.go @@ -183,20 +183,20 @@ func (k Keeper) SendTransfer( // and sent to the receiving address. Otherwise if the sender chain is sending // back tokens this chain originally transferred to it, the tokens are // unescrowed and sent to the receiving address. -func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData) (revert bool, err error) { +func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData) error { // validate packet data upon receiving if err := data.ValidateBasic(); err != nil { - return false, err + return err } if !k.GetReceiveEnabled(ctx) { - return false, types.ErrReceiveDisabled + return types.ErrReceiveDisabled } // decode the receiver address receiver, err := sdk.AccAddressFromBech32(data.Receiver) if err != nil { - return false, err + return err } labels := []metrics.Label{ @@ -237,8 +237,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t // counterparty module. The bug may occur in bank or any part of the code that allows // the escrow address to be drained. A malicious counterparty module could drain the // escrow address by allowing more tokens to be sent back then were escrowed. - // Revert callback state changes if SendCoins returns an error. - return true, sdkerrors.Wrap(err, "unable to unescrow tokens, this may be caused by a malicious counterparty module or a bug: please open an issue on counterparty module") + return sdkerrors.Wrap(err, "unable to unescrow tokens, this may be caused by a malicious counterparty module or a bug: please open an issue on counterparty module") } defer func() { @@ -257,7 +256,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t ) }() - return false, nil + return nil } // sender chain is the source, mint vouchers @@ -292,7 +291,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t ); err != nil { // error returned by bank keeper, revert all state changes by callback // and continue tx processing - return true, err + return err } // send to receiver @@ -301,7 +300,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t ); err != nil { // error returned by bank keeper, revert all state changes by callback // and continue tx processing - return true, err + return err } defer func() { @@ -320,7 +319,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t ) }() - return false, nil + return nil } // OnAcknowledgementPacket responds to the the success or failure of a packet diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index debeea227aa..71504bcbd6f 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -151,28 +151,27 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { malleate func() recvIsSource bool // the receiving chain is the source of the coin originally expPass bool - expRevert bool }{ - {"success receive on source chain", func() {}, true, true, false}, - {"success receive with coin from another chain as source", func() {}, false, true, false}, + {"success receive on source chain", func() {}, true, true}, + {"success receive with coin from another chain as source", func() {}, false, true}, {"empty coin", func() { trace = types.DenomTrace{} amount = sdk.ZeroInt() - }, true, false, false}, + }, true, false}, {"invalid receiver address", func() { receiver = "gaia1scqhwpgsmr6vmztaa7suurfl52my6nd2kmrudl" - }, true, false, false}, + }, true, false}, // onRecvPacket // - coin from chain chainA {"failure: mint zero coin", func() { amount = sdk.ZeroInt() - }, false, false, false}, + }, false, false}, // - coin being sent back to original chain (chainB) {"tries to unescrow more tokens than allowed", func() { amount = sdk.NewInt(1000000) - }, true, false, true}, + }, true, false}, } for _, tc := range testCases { @@ -199,7 +198,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { fungibleTokenPacket := types.NewFungibleTokenPacketData(coinFromBToA.Denom, coinFromBToA.Amount.Uint64(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) packet := channeltypes.NewPacket(fungibleTokenPacket.GetBytes(), 1, channelB.PortID, channelB.ID, channelA.PortID, channelA.ID, clienttypes.NewHeight(0, 110), 0) ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)}) - err = suite.coordinator.RelayPacket(suite.chainB, suite.chainA, clientB, clientA, packet, ack.GetBytes()) + err = suite.coordinator.RelayPacket(suite.chainB, suite.chainA, clientB, clientA, packet, ack.Acknowledgement()) suite.Require().NoError(err) // relay committed seq++ @@ -220,14 +219,12 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.Uint64(), suite.chainA.SenderAccount.GetAddress().String(), receiver) packet := channeltypes.NewPacket(data.GetBytes(), seq, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) - revert, err := suite.chainB.App.TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, data) + err = suite.chainB.App.TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, data) if tc.expPass { suite.Require().NoError(err) - suite.Require().False(revert) } else { suite.Require().Error(err) - suite.Require().Equal(tc.expRevert, revert) } }) } diff --git a/modules/apps/transfer/module.go b/modules/apps/transfer/module.go index 4476cb0e7e8..a8080aad194 100644 --- a/modules/apps/transfer/module.go +++ b/modules/apps/transfer/module.go @@ -7,13 +7,6 @@ import ( "math" "math/rand" - "github.com/grpc-ecosystem/grpc-gateway/runtime" - - "github.com/gorilla/mux" - "github.com/spf13/cobra" - - abci "github.com/tendermint/tendermint/abci/types" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" @@ -22,6 +15,11 @@ import ( "github.com/cosmos/cosmos-sdk/types/module" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" + "github.com/gorilla/mux" + "github.com/grpc-ecosystem/grpc-gateway/runtime" + "github.com/spf13/cobra" + abci "github.com/tendermint/tendermint/abci/types" + "github.com/cosmos/ibc-go/modules/apps/transfer/client/cli" "github.com/cosmos/ibc-go/modules/apps/transfer/keeper" "github.com/cosmos/ibc-go/modules/apps/transfer/simulation" @@ -29,6 +27,7 @@ import ( channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/modules/core/05-port/types" host "github.com/cosmos/ibc-go/modules/core/24-host" + ibcexported "github.com/cosmos/ibc-go/modules/core/exported" ) var ( @@ -318,21 +317,27 @@ func (am AppModule) OnChanCloseConfirm( return nil } -// OnRecvPacket implements the IBCModule interface +// OnRecvPacket implements the IBCModule interface. A successful acknowledgement +// is returned if the packet data is succesfully decoded and the receive application +// logic returns without error. func (am AppModule) OnRecvPacket( ctx sdk.Context, packet channeltypes.Packet, -) (*sdk.Result, []byte, bool, error) { +) ibcexported.Acknowledgement { + ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)}) + var data types.FungibleTokenPacketData if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { - return nil, nil, false, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error()) + ack = channeltypes.NewErrorAcknowledgement(fmt.Sprintf("cannot unmarshal ICS-20 transfer packet data: %s", err.Error())) } - acknowledgement := channeltypes.NewResultAcknowledgement([]byte{byte(1)}) - - revert, err := am.keeper.OnRecvPacket(ctx, packet, data) - if err != nil { - acknowledgement = channeltypes.NewErrorAcknowledgement(err.Error()) + // only attempt the application logic if the packet data + // was successfully decoded + if ack.Success() { + err := am.keeper.OnRecvPacket(ctx, packet, data) + if err != nil { + ack = channeltypes.NewErrorAcknowledgement(err.Error()) + } } ctx.EventManager().EmitEvent( @@ -342,14 +347,12 @@ func (am AppModule) OnRecvPacket( sdk.NewAttribute(types.AttributeKeyReceiver, data.Receiver), sdk.NewAttribute(types.AttributeKeyDenom, data.Denom), sdk.NewAttribute(types.AttributeKeyAmount, fmt.Sprintf("%d", data.Amount)), - sdk.NewAttribute(types.AttributeKeyAckSuccess, fmt.Sprintf("%t", err != nil)), + sdk.NewAttribute(types.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success())), ), ) // NOTE: acknowledgement will be written synchronously during IBC handler execution. - return &sdk.Result{ - Events: ctx.EventManager().Events().ToABCIEvents(), - }, acknowledgement.GetBytes(), revert, nil + return ack } // OnAcknowledgementPacket implements the IBCModule interface diff --git a/modules/core/03-connection/keeper/verify_test.go b/modules/core/03-connection/keeper/verify_test.go index 85e14143271..9afd4816943 100644 --- a/modules/core/03-connection/keeper/verify_test.go +++ b/modules/core/03-connection/keeper/verify_test.go @@ -360,12 +360,12 @@ func (suite *KeeperTestSuite) TestVerifyPacketAcknowledgement() { ack := ibcmock.MockAcknowledgement if tc.changeAcknowledgement { - ack = []byte(ibctesting.InvalidID) + ack = ibcmock.MockFailAcknowledgement } err = suite.chainA.App.IBCKeeper.ConnectionKeeper.VerifyPacketAcknowledgement( suite.chainA.GetContext(), connection, malleateHeight(proofHeight, tc.heightDiff), proof, - packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ack, + packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ack.Acknowledgement(), ) if tc.expPass { diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index 2ba823c9379..9a503e34b4d 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -641,7 +641,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) proof, proofHeight := suite.chainB.QueryProof(packetKey) - err := suite.chainA.App.IBCKeeper.ChannelKeeper.AcknowledgePacket(suite.chainA.GetContext(), channelCap, packet, ack, proof, proofHeight) + err := suite.chainA.App.IBCKeeper.ChannelKeeper.AcknowledgePacket(suite.chainA.GetContext(), channelCap, packet, ack.Acknowledgement(), proof, proofHeight) pc := suite.chainA.App.IBCKeeper.ChannelKeeper.GetPacketCommitment(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) channelA, _ := suite.chainA.App.IBCKeeper.ChannelKeeper.GetChannel(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel()) diff --git a/modules/core/04-channel/types/acknowledgement.go b/modules/core/04-channel/types/acknowledgement.go index a3f677ab441..e71fcb5da9e 100644 --- a/modules/core/04-channel/types/acknowledgement.go +++ b/modules/core/04-channel/types/acknowledgement.go @@ -1,6 +1,7 @@ package types import ( + "reflect" "strings" sdk "github.com/cosmos/cosmos-sdk/types" @@ -27,11 +28,6 @@ func NewErrorAcknowledgement(err string) Acknowledgement { } } -// GetBytes is a helper for serialising acknowledgements -func (ack Acknowledgement) GetBytes() []byte { - return sdk.MustSortJSON(SubModuleCdc.MustMarshalJSON(&ack)) -} - // ValidateBasic performs a basic validation of the acknowledgement func (ack Acknowledgement) ValidateBasic() error { switch resp := ack.Response.(type) { @@ -43,8 +39,22 @@ func (ack Acknowledgement) ValidateBasic() error { if strings.TrimSpace(resp.Error) == "" { return sdkerrors.Wrap(ErrInvalidAcknowledgement, "acknowledgement error cannot be empty") } + default: return sdkerrors.Wrapf(ErrInvalidAcknowledgement, "unsupported acknowledgement response field type %T", resp) } return nil } + +// Success implements the Acknowledgement interface. The acknowledgement is +// considered successful if it is a ResultAcknowledgement. Otherwise it is +// considered a failed acknowledgement if it is an ErrorAcknowledgement. +func (ack Acknowledgement) Success() bool { + return reflect.TypeOf(ack.Response) == reflect.TypeOf(((*Acknowledgement_Result)(nil))) +} + +// Acknowledgement implements the Acknowledgement interface. It returns the +// acknowledgement serialised using JSON. +func (ack Acknowledgement) Acknowledgement() []byte { + return sdk.MustSortJSON(SubModuleCdc.MustMarshalJSON(&ack)) +} diff --git a/modules/core/04-channel/types/acknowledgement_test.go b/modules/core/04-channel/types/acknowledgement_test.go index fa286d066ba..92d546a8164 100644 --- a/modules/core/04-channel/types/acknowledgement_test.go +++ b/modules/core/04-channel/types/acknowledgement_test.go @@ -5,29 +5,34 @@ import "github.com/cosmos/ibc-go/modules/core/04-channel/types" // tests acknowledgement.ValidateBasic and acknowledgement.GetBytes func (suite TypesTestSuite) TestAcknowledgement() { testCases := []struct { - name string - ack types.Acknowledgement - expPass bool + name string + ack types.Acknowledgement + expSuccess bool // indicate if this is a success or failed ack + expPass bool }{ { "valid successful ack", types.NewResultAcknowledgement([]byte("success")), true, + true, }, { "valid failed ack", types.NewErrorAcknowledgement("error"), + false, true, }, { "empty successful ack", types.NewResultAcknowledgement([]byte{}), + true, false, }, { "empty faied ack", types.NewErrorAcknowledgement(" "), false, + false, }, { "nil response", @@ -35,6 +40,7 @@ func (suite TypesTestSuite) TestAcknowledgement() { Response: nil, }, false, + false, }, } @@ -54,10 +60,11 @@ func (suite TypesTestSuite) TestAcknowledgement() { // expect all acks to be able to be marshaled suite.NotPanics(func() { - bz := tc.ack.GetBytes() + bz := tc.ack.Acknowledgement() suite.Require().NotNil(bz) }) + + suite.Require().Equal(tc.expSuccess, tc.ack.Success()) }) } - } diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index c9489fcd578..ec10c01591d 100644 --- a/modules/core/05-port/types/module.go +++ b/modules/core/05-port/types/module.go @@ -5,6 +5,7 @@ import ( capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" + "github.com/cosmos/ibc-go/modules/core/exported" ) // IBCModule defines an interface that implements all the callbacks @@ -66,7 +67,7 @@ type IBCModule interface { OnRecvPacket( ctx sdk.Context, packet channeltypes.Packet, - ) (*sdk.Result, []byte, bool, error) + ) exported.Acknowledgement OnAcknowledgementPacket( ctx sdk.Context, diff --git a/modules/core/exported/channel.go b/modules/core/exported/channel.go index 6a0d542c1ee..f6393393513 100644 --- a/modules/core/exported/channel.go +++ b/modules/core/exported/channel.go @@ -30,3 +30,10 @@ type PacketI interface { GetData() []byte ValidateBasic() error } + +// Acknowledgement defines the interface used to return +// acknowledgements in the OnRecvPacket callback. +type Acknowledgement interface { + Success() bool + Acknowledgement() []byte +} diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 02714df7e78..9b16cd26ddf 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -443,14 +443,11 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke } // Perform application logic callback - // Cache context so that we may discard state changes from callback if revert is true. + // Cache context so that we may discard state changes from callback if the acknowledgement is unsuccessful. cacheCtx, writeFn := ctx.CacheContext() - _, ack, revert, err := cbs.OnRecvPacket(cacheCtx, msg.Packet) - if err != nil { - return nil, sdkerrors.Wrap(err, "receive packet callback failed") - } - // If revert is false, persist callback state changes by writing to underlying parent store. - if !revert { + ack := cbs.OnRecvPacket(cacheCtx, msg.Packet) + if ack.Success() { + // write application state changes writeFn() } @@ -458,7 +455,7 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke // NOTE: IBC applications modules may call the WriteAcknowledgement asynchronously if the // acknowledgement is nil. if ack != nil { - if err := k.ChannelKeeper.WriteAcknowledgement(ctx, cap, msg.Packet, ack); err != nil { + if err := k.ChannelKeeper.WriteAcknowledgement(ctx, cap, msg.Packet, ack.Acknowledgement()); err != nil { return nil, err } } diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index 890c84f2dbc..bad7d30b7b9 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -154,7 +154,6 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { msg := channeltypes.NewMsgRecvPacket(packet, proof, proofHeight, suite.chainB.SenderAccount.GetAddress()) - // ante-handle RecvPacket _, err := keeper.Keeper.RecvPacket(*suite.chainB.App.IBCKeeper, sdk.WrapSDKContext(suite.chainB.GetContext()), msg) if tc.expPass { @@ -298,7 +297,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() { packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) proof, proofHeight := suite.chainB.QueryProof(packetKey) - msg := channeltypes.NewMsgAcknowledgement(packet, ibcmock.MockAcknowledgement, proof, proofHeight, suite.chainA.SenderAccount.GetAddress()) + msg := channeltypes.NewMsgAcknowledgement(packet, ibcmock.MockAcknowledgement.Acknowledgement(), proof, proofHeight, suite.chainA.SenderAccount.GetAddress()) _, err := keeper.Keeper.Acknowledgement(*suite.chainA.App.IBCKeeper, sdk.WrapSDKContext(suite.chainA.GetContext()), msg) diff --git a/modules/light-clients/07-tendermint/types/client_state_test.go b/modules/light-clients/07-tendermint/types/client_state_test.go index bf0811ebbaa..2838ebb49aa 100644 --- a/modules/light-clients/07-tendermint/types/client_state_test.go +++ b/modules/light-clients/07-tendermint/types/client_state_test.go @@ -562,7 +562,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgement() { currentTime := uint64(suite.chainA.GetContext().BlockTime().UnixNano()) err = clientState.VerifyPacketAcknowledgement( store, suite.chainA.Codec, proofHeight, currentTime, delayPeriod, &prefix, proof, - packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ibcmock.MockAcknowledgement, + packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), ibcmock.MockAcknowledgement.Acknowledgement(), ) if tc.expPass { diff --git a/testing/chain.go b/testing/chain.go index 9fd7b234920..7e9e2b75e84 100644 --- a/testing/chain.go +++ b/testing/chain.go @@ -71,7 +71,7 @@ var ( ConnectionVersion = connectiontypes.ExportedVersionsToProto(connectiontypes.GetCompatibleVersions())[0] - MockAcknowledgement = mock.MockAcknowledgement + MockAcknowledgement = mock.MockAcknowledgement.Acknowledgement() MockPacketData = mock.MockPacketData MockFailPacketData = mock.MockFailPacketData MockCanaryCapabilityName = mock.MockCanaryCapabilityName diff --git a/testing/mock/mock.go b/testing/mock/mock.go index c7ab7426389..2df5022e25f 100644 --- a/testing/mock/mock.go +++ b/testing/mock/mock.go @@ -4,23 +4,21 @@ import ( "bytes" "encoding/json" - "github.com/cosmos/cosmos-sdk/types/module" - - "github.com/grpc-ecosystem/grpc-gateway/runtime" - - "github.com/gorilla/mux" - "github.com/spf13/cobra" - - abci "github.com/tendermint/tendermint/abci/types" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/module" capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" + "github.com/gorilla/mux" + "github.com/grpc-ecosystem/grpc-gateway/runtime" + "github.com/spf13/cobra" + abci "github.com/tendermint/tendermint/abci/types" + channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/modules/core/24-host" + "github.com/cosmos/ibc-go/modules/core/exported" ) const ( @@ -28,8 +26,8 @@ const ( ) var ( - MockAcknowledgement = []byte("mock acknowledgement") - MockFailAcknowledgement = []byte("mock failed acknowledgement") + MockAcknowledgement = channeltypes.NewResultAcknowledgement([]byte("mock acknowledgement")) + MockFailAcknowledgement = channeltypes.NewErrorAcknowledgement("mock failed acknowledgement") MockPacketData = []byte("mock packet data") MockFailPacketData = []byte("mock failed packet data") MockCanaryCapabilityName = "mock canary capability name" @@ -175,13 +173,13 @@ func (am AppModule) OnChanCloseConfirm(sdk.Context, string, string) error { } // OnRecvPacket implements the IBCModule interface. -func (am AppModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) (*sdk.Result, []byte, bool, error) { +func (am AppModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) exported.Acknowledgement { // set state by claiming capability to check if revert happens return am.scopedKeeper.NewCapability(ctx, MockCanaryCapabilityName) if bytes.Equal(MockPacketData, packet.GetData()) { - return nil, MockAcknowledgement, false, nil + return MockAcknowledgement } - return nil, MockFailAcknowledgement, true, nil + return MockFailAcknowledgement } // OnAcknowledgementPacket implements the IBCModule interface. From 0ba44bc3de42138a93ea1e57e2bf3fa5aa71ec94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 7 Apr 2021 13:26:04 +0200 Subject: [PATCH 05/10] self review fixes --- modules/apps/transfer/keeper/relay.go | 4 ---- modules/core/05-port/types/module.go | 8 +++---- modules/core/keeper/msg_server.go | 4 ++-- modules/core/keeper/msg_server_test.go | 31 +++++++++++++++++++++++--- testing/mock/mock.go | 4 ++++ 5 files changed, 38 insertions(+), 13 deletions(-) diff --git a/modules/apps/transfer/keeper/relay.go b/modules/apps/transfer/keeper/relay.go index ca4a4575e1e..7e9bdf37214 100644 --- a/modules/apps/transfer/keeper/relay.go +++ b/modules/apps/transfer/keeper/relay.go @@ -289,8 +289,6 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t if err := k.bankKeeper.MintCoins( ctx, types.ModuleName, sdk.NewCoins(voucher), ); err != nil { - // error returned by bank keeper, revert all state changes by callback - // and continue tx processing return err } @@ -298,8 +296,6 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t if err := k.bankKeeper.SendCoinsFromModuleToAccount( ctx, types.ModuleName, receiver, sdk.NewCoins(voucher), ); err != nil { - // error returned by bank keeper, revert all state changes by callback - // and continue tx processing return err } diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index ec10c01591d..10a756bbe03 100644 --- a/modules/core/05-port/types/module.go +++ b/modules/core/05-port/types/module.go @@ -59,11 +59,11 @@ type IBCModule interface { channelID string, ) error - // OnRecvPacket must return the acknowledgement bytes + // OnRecvPacket must return an acknowledgement that implements the Acknowledgement interface. // In the case of an asynchronous acknowledgement, nil should be returned. - // OnRecvPacket must also return a revert boolean. If true, the state changes on callback - // are reverted, the rest of the transaction is still processed. Thus, the packet is successfully - // received and any returned acknowledgement is written. + // If the acknowledgement returned is successful, the state changes on callback are written, + // otherwise the application state changes are discarded. In either case the packet is received + // and the acknowledgement is written (in synchronous cases). OnRecvPacket( ctx sdk.Context, packet channeltypes.Packet, diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 9b16cd26ddf..a64cb2ec86b 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -446,8 +446,8 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke // Cache context so that we may discard state changes from callback if the acknowledgement is unsuccessful. cacheCtx, writeFn := ctx.CacheContext() ack := cbs.OnRecvPacket(cacheCtx, msg.Packet) - if ack.Success() { - // write application state changes + if ack == nil || ack.Success() { + // write application state changes for asynchronous and successful acknowledgements writeFn() } diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index bad7d30b7b9..7c0f656628b 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -57,6 +57,7 @@ func TestIBCTestSuite(t *testing.T) { func (suite *KeeperTestSuite) TestHandleRecvPacket() { var ( packet channeltypes.Packet + async bool // indicate no ack written ) testCases := []struct { @@ -98,6 +99,22 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) suite.Require().NoError(err) }, true, true}, + {"success: ORDERED - async acknowledgement", func() { + async = true + _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) + packet = channeltypes.NewPacket(ibcmock.MockAsyncPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + + err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) + suite.Require().NoError(err) + }, true, false}, + {"success: UNORDERED - async acknowledgement", func() { + async = true + _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) + packet = channeltypes.NewPacket(ibcmock.MockAsyncPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, 0) + + err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB) + suite.Require().NoError(err) + }, true, false}, {"failure: ORDERED out of order packet", func() { _, clientB, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.ORDERED) @@ -145,6 +162,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { suite.Run(tc.name, func() { suite.SetupTest() // reset + async = false // reset tc.malleate() @@ -171,10 +189,17 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { suite.Require().True(exists, "callback state not persisted when revert is false") } - // verify ack was written + // verify if ack was written ack, found := suite.chainB.App.IBCKeeper.ChannelKeeper.GetPacketAcknowledgement(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) - suite.Require().NotNil(ack) - suite.Require().True(found) + + if async { + suite.Require().Nil(ack) + suite.Require().False(found) + + } else { + suite.Require().NotNil(ack) + suite.Require().True(found) + } } else { suite.Require().Error(err) } diff --git a/testing/mock/mock.go b/testing/mock/mock.go index 2df5022e25f..1ac33f8f392 100644 --- a/testing/mock/mock.go +++ b/testing/mock/mock.go @@ -30,6 +30,7 @@ var ( MockFailAcknowledgement = channeltypes.NewErrorAcknowledgement("mock failed acknowledgement") MockPacketData = []byte("mock packet data") MockFailPacketData = []byte("mock failed packet data") + MockAsyncPacketData = []byte("mock async packet data") MockCanaryCapabilityName = "mock canary capability name" ) @@ -178,7 +179,10 @@ func (am AppModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) ex am.scopedKeeper.NewCapability(ctx, MockCanaryCapabilityName) if bytes.Equal(MockPacketData, packet.GetData()) { return MockAcknowledgement + } else if bytes.Equal(MockAsyncPacketData, packet.GetData()) { + return nil } + return MockFailAcknowledgement } From bbd1e38a0f573ee2c9d796335f33e7251360b3f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 7 Apr 2021 13:26:24 +0200 Subject: [PATCH 06/10] Update modules/core/04-channel/types/acknowledgement.go --- modules/core/04-channel/types/acknowledgement.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/04-channel/types/acknowledgement.go b/modules/core/04-channel/types/acknowledgement.go index e71fcb5da9e..cfc088ab0c9 100644 --- a/modules/core/04-channel/types/acknowledgement.go +++ b/modules/core/04-channel/types/acknowledgement.go @@ -48,7 +48,7 @@ func (ack Acknowledgement) ValidateBasic() error { // Success implements the Acknowledgement interface. The acknowledgement is // considered successful if it is a ResultAcknowledgement. Otherwise it is -// considered a failed acknowledgement if it is an ErrorAcknowledgement. +// considered a failed acknowledgement. func (ack Acknowledgement) Success() bool { return reflect.TypeOf(ack.Response) == reflect.TypeOf(((*Acknowledgement_Result)(nil))) } From f97e6f44dc91993b6e0e0e373a40ce6f63806841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 7 Apr 2021 13:32:46 +0200 Subject: [PATCH 07/10] Add Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 234a38be084..c30c8d979d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +### API Breaking Changes + +* (modules) [\#107](https://github.com/cosmos/ibc-go/pull/107) Modify OnRecvPacket callback to return an acknowledgement which indicates if it is successful or not. Application state changes are discarded for unsuccessful acknowledgements only. + ### State Machine Breaking * (modules/light-clients/07-tendermint) [\#99](https://github.com/cosmos/ibc-go/pull/99) Enforce maximum chain-id length for tendermint client. From 9da6a1668514e2bc67c6ff1339319b63b2b3ac86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 7 Apr 2021 17:28:19 +0200 Subject: [PATCH 08/10] update docs --- docs/custom.md | 51 ++++++++++++---------------- docs/migrations/ibc-migration-043.md | 8 +++++ 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/docs/custom.md b/docs/custom.md index a5f75aad08b..1afe61e0495 100644 --- a/docs/custom.md +++ b/docs/custom.md @@ -295,49 +295,42 @@ invoked by the IBC module after the packet has been proved valid and correctly p keepers. Thus, the `OnRecvPacket` callback only needs to worry about making the appropriate state changes given the packet data without worrying about whether the packet is valid or not. -Modules may return an acknowledgement as a byte string and return it to the IBC handler. +Modules may return to the IBC handler an acknowledgement which implements the Acknowledgement interface. The IBC handler will then commit this acknowledgement of the packet so that a relayer may relay the acknowledgement back to the sender module. +The state changes that occurred during this callback will only be written if: +- the acknowledgement was successful as indicated by the `Success()` function of the acknowledgement +- if the acknowledgement returned is nil indicating that an asynchronous process is occurring + ```go OnRecvPacket( ctx sdk.Context, packet channeltypes.Packet, -) (res *sdk.Result, ack []byte, abort error) { +) ibcexported.Acknowledgement { // Decode the packet data packetData := DecodePacketData(packet.Data) - // do application state changes based on packet data - // and return result, acknowledgement and abortErr - // Note: abortErr is only not nil if we need to abort the entire receive packet, and allow a replay of the receive. - // If the application state change failed but we do not want to replay the packet, - // simply encode this failure with relevant information in ack and return nil error - res, ack, abortErr := processPacket(ctx, packet, packetData) - - // if we need to abort the entire receive packet, return error - if abortErr != nil { - return nil, nil, abortErr - } + // do application state changes based on packet data and return the acknowledgement + // NOTE: The acknowledgement will indicate to the IBC handler if the application + // state changes should be written via the `Success()` function. Application state + // changes are only written if the acknowledgement is successful or the acknowledgement + // returned is nil indicating that an asynchronous acknowledgement will occur. + ack := processPacket(ctx, packet, packetData) - // Encode the ack since IBC expects acknowledgement bytes - ackBytes := EncodeAcknowledgement(ack) - - return res, ackBytes, nil + return ack } ``` -::: warning -`OnRecvPacket` should **only** return an error if we want the entire receive packet execution -(including the IBC handling) to be reverted. This will allow the packet to be replayed in the case -that some mistake in the relaying caused the packet processing to fail. - -If some application-level error happened while processing the packet data, in most cases, we will -not want the packet processing to revert. Instead, we may want to encode this failure into the -acknowledgement and finish processing the packet. This will ensure the packet cannot be replayed, -and will also allow the sender module to potentially remediate the situation upon receiving the -acknowledgement. An example of this technique is in the `ibc-transfer` module's -[`OnRecvPacket`](https://github.com/cosmos/ibc-go/tree/main/modules/apps/transfer/module.go). -::: +The Acknowledgement interface: +```go +// Acknowledgement defines the interface used to return +// acknowledgements in the OnRecvPacket callback. +type Acknowledgement interface { + Success() bool + Acknowledgement() []byte +} +``` ### Acknowledgements diff --git a/docs/migrations/ibc-migration-043.md b/docs/migrations/ibc-migration-043.md index 93afc79ea9c..6d6b9cff9e9 100644 --- a/docs/migrations/ibc-migration-043.md +++ b/docs/migrations/ibc-migration-043.md @@ -91,3 +91,11 @@ REST routes are not supported for these proposals. ## Proto file changes The gRPC querier service endpoints have changed slightly. The previous files used `v1beta1`, this has been updated to `v1`. + +## IBC callback changes + +### OnRecvPacket + +Application developers need to update their `OnRecvPacket` callback logic. + +The `OnRecvPacket` callback has been modified to only return the acknowledgement. The acknowledgement returned must implement the `Acknowledgement` interface. The acknowledgement should indicate if it represents a successful processing of a packet by returning true on `Success()` and false in all other cases. A return value of false on `Success()` will result in all state changes which occurred in the callback being discarded. More information can be found in the [documentation](https://github.com/cosmos/ibc-go/blob/main/docs/custom.md#receiving-packets). From 2b00c1bd90b882976d23bb649843b1fbde70127d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 12 Apr 2021 12:02:34 +0200 Subject: [PATCH 09/10] Update CHANGELOG.md Co-authored-by: Aditya --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c30c8d979d3..2770a226146 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,7 +38,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes -* (modules) [\#107](https://github.com/cosmos/ibc-go/pull/107) Modify OnRecvPacket callback to return an acknowledgement which indicates if it is successful or not. Application state changes are discarded for unsuccessful acknowledgements only. +* (modules) [\#107](https://github.com/cosmos/ibc-go/pull/107) Modify OnRecvPacket callback to return an acknowledgement which indicates if it is successful or not. Callback state changes are discarded for unsuccessful acknowledgements only. ### State Machine Breaking From 3111c1d4fbe9844d993d9eb318793788cd3e1b98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 12 Apr 2021 12:14:08 +0200 Subject: [PATCH 10/10] add note for async acks --- docs/custom.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/custom.md b/docs/custom.md index 1afe61e0495..64d857f281e 100644 --- a/docs/custom.md +++ b/docs/custom.md @@ -303,6 +303,10 @@ The state changes that occurred during this callback will only be written if: - the acknowledgement was successful as indicated by the `Success()` function of the acknowledgement - if the acknowledgement returned is nil indicating that an asynchronous process is occurring +NOTE: Applications which process asynchronous acknowledgements must handle reverting state changes +when appropriate. Any state changes that occurred during the `OnRecvPacket` callback will be written +for asynchronous acknowledgements. + ```go OnRecvPacket( ctx sdk.Context,