From ef37dfef182cdbafbfec2deed6fbe4e93d9522be Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 13 Jul 2022 15:45:50 +0100 Subject: [PATCH] remove spurious `TestABCICodeDeterminism` tests (#1695) (#1697) Thanks @colin-axner for noticing this. closes: #XXXX --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing) - [ ] Updated relevant documentation (`docs/`) or specification (`x//spec/`) - [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [x] Re-reviewed `Files changed` in the Github PR explorer - [ ] Review `Codecov Report` in the comment section below once CI passes (cherry picked from commit 1e6af489ed57450f8fa187816cfdd61d4fd4f19e) Co-authored-by: Carlos Rodriguez --- .../host/ibc_module_test.go | 63 --------------- .../host/types/ack_test.go | 81 ------------------- 2 files changed, 144 deletions(-) delete mode 100644 modules/apps/27-interchain-accounts/host/types/ack_test.go diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index 8071f5fb371..3ad7f961a04 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -10,9 +10,6 @@ import ( "github.com/Finschia/ostracon/crypto" "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/suite" - abcitypes "github.com/tendermint/tendermint/abci/types" - tmprotostate "github.com/tendermint/tendermint/proto/tendermint/state" - tmstate "github.com/tendermint/tendermint/state" "github.com/cosmos/ibc-go/v4/modules/apps/27-interchain-accounts/host/types" icatypes "github.com/cosmos/ibc-go/v4/modules/apps/27-interchain-accounts/types" @@ -692,63 +689,3 @@ func (suite *InterchainAccountsTestSuite) TestControlAccountAfterChannelClose() hasBalance = suite.chainB.GetSimApp().BankKeeper.HasBalance(suite.chainB.GetContext(), icaAddr, sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(0)}) suite.Require().True(hasBalance) } - -// The safety of including SDK MsgResponses in the acknowledgement rests -// on the inclusion of the abcitypes.ResponseDeliverTx.Data in the -// abcitypes.ResposneDeliverTx hash. If the abcitypes.ResponseDeliverTx.Data -// gets removed from consensus they must no longer be used in the packet -// acknowledgement. -// -// This test acts as an indicator that the abcitypes.ResponseDeliverTx.Data -// may no longer be deterministic. -func (suite *InterchainAccountsTestSuite) TestABCICodeDeterminism() { - msgResponseBz, err := proto.Marshal(&channeltypes.MsgChannelOpenInitResponse{}) - suite.Require().NoError(err) - - msgData := &sdk.MsgData{ - MsgType: sdk.MsgTypeURL(&channeltypes.MsgChannelOpenInit{}), - Data: msgResponseBz, - } - - txResponse, err := proto.Marshal(&sdk.TxMsgData{ - Data: []*sdk.MsgData{msgData}, - }) - suite.Require().NoError(err) - - deliverTx := abcitypes.ResponseDeliverTx{ - Data: txResponse, - } - responses := tmprotostate.ABCIResponses{ - DeliverTxs: []*abcitypes.ResponseDeliverTx{ - &deliverTx, - }, - } - - differentMsgResponseBz, err := proto.Marshal(&channeltypes.MsgRecvPacketResponse{}) - suite.Require().NoError(err) - - differentMsgData := &sdk.MsgData{ - MsgType: sdk.MsgTypeURL(&channeltypes.MsgRecvPacket{}), - Data: differentMsgResponseBz, - } - - differentTxResponse, err := proto.Marshal(&sdk.TxMsgData{ - Data: []*sdk.MsgData{differentMsgData}, - }) - suite.Require().NoError(err) - - differentDeliverTx := abcitypes.ResponseDeliverTx{ - Data: differentTxResponse, - } - - differentResponses := tmprotostate.ABCIResponses{ - DeliverTxs: []*abcitypes.ResponseDeliverTx{ - &differentDeliverTx, - }, - } - - hash := tmstate.ABCIResponsesResultsHash(&responses) - differentHash := tmstate.ABCIResponsesResultsHash(&differentResponses) - - suite.Require().NotEqual(hash, differentHash) -} diff --git a/modules/apps/27-interchain-accounts/host/types/ack_test.go b/modules/apps/27-interchain-accounts/host/types/ack_test.go deleted file mode 100644 index e1227f4cab2..00000000000 --- a/modules/apps/27-interchain-accounts/host/types/ack_test.go +++ /dev/null @@ -1,81 +0,0 @@ -package types_test - -import ( - "testing" - - sdkerrors "github.com/Finschia/finschia-sdk/types/errors" - "github.com/stretchr/testify/suite" - abcitypes "github.com/tendermint/tendermint/abci/types" - tmprotostate "github.com/tendermint/tendermint/proto/tendermint/state" - tmstate "github.com/tendermint/tendermint/state" - - ibctesting "github.com/cosmos/ibc-go/v4/testing" -) - -const ( - gasUsed = uint64(100) - gasWanted = uint64(100) -) - -type TypesTestSuite struct { - suite.Suite - - coordinator *ibctesting.Coordinator - - chainA *ibctesting.TestChain - chainB *ibctesting.TestChain -} - -func (suite *TypesTestSuite) SetupTest() { - suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2) - - suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1)) - suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2)) -} - -func TestTypesTestSuite(t *testing.T) { - suite.Run(t, new(TypesTestSuite)) -} - -// The safety of including ABCI error codes in the acknowledgement rests -// on the inclusion of these ABCI error codes in the abcitypes.ResposneDeliverTx -// hash. If the ABCI codes get removed from consensus they must no longer be used -// in the packet acknowledgement. -// -// This test acts as an indicator that the ABCI error codes may no longer be deterministic. -func (suite *TypesTestSuite) TestABCICodeDeterminism() { - // same ABCI error code used - err := sdkerrors.Wrap(sdkerrors.ErrOutOfGas, "error string 1") - errSameABCICode := sdkerrors.Wrap(sdkerrors.ErrOutOfGas, "error string 2") - - // different ABCI error code used - errDifferentABCICode := sdkerrors.ErrNotFound - - deliverTx := sdkerrors.ResponseDeliverTx(err, gasUsed, gasWanted, false) - responses := tmprotostate.ABCIResponses{ - DeliverTxs: []*abcitypes.ResponseDeliverTx{ - &deliverTx, - }, - } - - deliverTxSameABCICode := sdkerrors.ResponseDeliverTx(errSameABCICode, gasUsed, gasWanted, false) - responsesSameABCICode := tmprotostate.ABCIResponses{ - DeliverTxs: []*abcitypes.ResponseDeliverTx{ - &deliverTxSameABCICode, - }, - } - - deliverTxDifferentABCICode := sdkerrors.ResponseDeliverTx(errDifferentABCICode, gasUsed, gasWanted, false) - responsesDifferentABCICode := tmprotostate.ABCIResponses{ - DeliverTxs: []*abcitypes.ResponseDeliverTx{ - &deliverTxDifferentABCICode, - }, - } - - hash := tmstate.ABCIResponsesResultsHash(&responses) - hashSameABCICode := tmstate.ABCIResponsesResultsHash(&responsesSameABCICode) - hashDifferentABCICode := tmstate.ABCIResponsesResultsHash(&responsesDifferentABCICode) - - suite.Require().Equal(hash, hashSameABCICode) - suite.Require().NotEqual(hash, hashDifferentABCICode) -}