From f3906a9c315ff11874ab2114bb11ed6550541e37 Mon Sep 17 00:00:00 2001 From: NisTun Date: Mon, 8 Jul 2024 23:58:10 +0700 Subject: [PATCH 1/4] add ForwardingTestSuite --- modules/apps/transfer/keeper/keeper_test.go | 4 +-- .../transfer/keeper/relay_forwarding_test.go | 35 ++++++++++++------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/modules/apps/transfer/keeper/keeper_test.go b/modules/apps/transfer/keeper/keeper_test.go index f816dbe4740..b37592d85d2 100644 --- a/modules/apps/transfer/keeper/keeper_test.go +++ b/modules/apps/transfer/keeper/keeper_test.go @@ -30,15 +30,13 @@ type KeeperTestSuite struct { chainA *ibctesting.TestChain chainB *ibctesting.TestChain chainC *ibctesting.TestChain - chainD *ibctesting.TestChain } func (suite *KeeperTestSuite) SetupTest() { - suite.coordinator = ibctesting.NewCoordinator(suite.T(), 4) + suite.coordinator = ibctesting.NewCoordinator(suite.T(), 3) suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1)) suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2)) suite.chainC = suite.coordinator.GetChain(ibctesting.GetChainID(3)) - suite.chainD = suite.coordinator.GetChain(ibctesting.GetChainID(4)) queryHelper := baseapp.NewQueryServerTestHelper(suite.chainA.GetContext(), suite.chainA.GetSimApp().InterfaceRegistry()) types.RegisterQueryServer(queryHelper, suite.chainA.GetSimApp().TransferKeeper) diff --git a/modules/apps/transfer/keeper/relay_forwarding_test.go b/modules/apps/transfer/keeper/relay_forwarding_test.go index fbe3526ac90..e0294655489 100644 --- a/modules/apps/transfer/keeper/relay_forwarding_test.go +++ b/modules/apps/transfer/keeper/relay_forwarding_test.go @@ -17,9 +17,20 @@ import ( clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/v8/testing" + testifysuite "github.com/stretchr/testify/suite" ) -func (suite *KeeperTestSuite) setupForwardingPaths() (pathAtoB, pathBtoC *ibctesting.Path) { +type ForwardingTestSuite struct { + testifysuite.Suite + + // testing chains used for convenience and readability + chainA *ibctesting.TestChain + chainB *ibctesting.TestChain + chainC *ibctesting.TestChain + chainD *ibctesting.TestChain +} + +func (suite *ForwardingTestSuite) setupForwardingPaths() (pathAtoB, pathBtoC *ibctesting.Path) { pathAtoB = ibctesting.NewTransferPath(suite.chainA, suite.chainB) pathBtoC = ibctesting.NewTransferPath(suite.chainB, suite.chainC) pathAtoB.Setup() @@ -34,7 +45,7 @@ const ( balance ) -func (suite *KeeperTestSuite) assertAmountOnChain(chain *ibctesting.TestChain, balanceType amountType, amount sdkmath.Int, denom string) { +func (suite *ForwardingTestSuite) assertAmountOnChain(chain *ibctesting.TestChain, balanceType amountType, amount sdkmath.Int, denom string) { var total sdk.Coin switch balanceType { case escrow: @@ -51,7 +62,7 @@ func (suite *KeeperTestSuite) assertAmountOnChain(chain *ibctesting.TestChain, b // from chain A to chain B is stored after when the packet is received on chain B // and then forwarded to chain C, and checks the balance of the escrow accounts // in chain A nad B. -func (suite *KeeperTestSuite) TestStoredForwardedPacketAndEscrowAfterFirstHop() { +func (suite *ForwardingTestSuite) TestStoredForwardedPacketAndEscrowAfterFirstHop() { /* Given the following topology: chain A (channel 0) -> (channel-0) chain B (channel-1) -> (channel-0) chain A @@ -112,7 +123,7 @@ func (suite *KeeperTestSuite) TestStoredForwardedPacketAndEscrowAfterFirstHop() } // TestSuccessfulForward tests a successful transfer from A to C through B. -func (suite *KeeperTestSuite) TestSuccessfulForward() { +func (suite *ForwardingTestSuite) TestSuccessfulForward() { /* Given the following topology: chain A (channel 0) -> (channel-0) chain B (channel-1) -> (channel-0) chain C @@ -225,7 +236,7 @@ func (suite *KeeperTestSuite) TestSuccessfulForward() { } // TestSuccessfulForwardWithMemo tests a successful transfer from A to C through B with a memo that should arrive at C. -func (suite *KeeperTestSuite) TestSuccessfulForwardWithMemo() { +func (suite *ForwardingTestSuite) TestSuccessfulForwardWithMemo() { /* Given the following topology: chain A (channel 0) -> (channel-0) chain B (channel-1) -> (channel-0) chain C @@ -366,7 +377,7 @@ func (suite *KeeperTestSuite) TestSuccessfulForwardWithMemo() { // TestSuccessfulForwardWithNonCosmosAccAddress tests that a packet is successfully forwarded with a non-Cosmos account address. // The test stops before verifying the final receive, because we don't have a non-cosmos chain to test with. -func (suite *KeeperTestSuite) TestSuccessfulForwardWithNonCosmosAccAddress() { +func (suite *ForwardingTestSuite) TestSuccessfulForwardWithNonCosmosAccAddress() { /* Given the following topology: chain A (channel 0) -> (channel-0) chain B (channel-1) -> (channel-0) chain C @@ -445,7 +456,7 @@ func (suite *KeeperTestSuite) TestSuccessfulForwardWithNonCosmosAccAddress() { // TestSuccessfulUnwind tests unwinding of tokens sent from A -> B -> C by // forwarding the tokens back from C -> B -> A. -func (suite *KeeperTestSuite) TestSuccessfulUnwind() { +func (suite *ForwardingTestSuite) TestSuccessfulUnwind() { /* Given the following topolgy: chain A (channel 0) -> (channel-0) chain B (channel-1) -> (channel-0) chain C @@ -590,7 +601,7 @@ func (suite *KeeperTestSuite) TestSuccessfulUnwind() { // middle chain is native source when receiving and sending the packet. In other words, the middle chain's native // token has been sent to chain C, and the multi-hop transfer from C -> B -> A has chain B being the source of // the token both when receiving and forwarding (sending). -func (suite *KeeperTestSuite) TestAcknowledgementFailureWithMiddleChainAsNativeTokenSource() { +func (suite *ForwardingTestSuite) TestAcknowledgementFailureWithMiddleChainAsNativeTokenSource() { /* Given the following topology: chain A (channel 0) -> (channel-0) chain B (channel-1) -> (channel-0) chain C @@ -758,7 +769,7 @@ func (suite *KeeperTestSuite) TestAcknowledgementFailureWithMiddleChainAsNativeT // TestAcknowledgementFailureWithMiddleChainAsNotBeingTokenSource tests a failure in the last hop where the middle chain // is not source of the token when receiving or sending the packet. In other words, the middle chain's is sent // (and forwarding) someone else's native token (in this case chain C). -func (suite *KeeperTestSuite) TestAcknowledgementFailureWithMiddleChainAsNotBeingTokenSource() { +func (suite *ForwardingTestSuite) TestAcknowledgementFailureWithMiddleChainAsNotBeingTokenSource() { /* Given the following topology: chain A (channel 0) <- (channel-0) chain B (channel-1) <- (channel-0) chain C @@ -878,7 +889,7 @@ func (suite *KeeperTestSuite) TestAcknowledgementFailureWithMiddleChainAsNotBein // TestOnTimeoutPacketForwarding tests the scenario in which a packet goes from // A to C, using B as a forwarding hop. The packet times out when going to C // from B and we verify that funds are properly returned to A. -func (suite *KeeperTestSuite) TestOnTimeoutPacketForwarding() { +func (suite *ForwardingTestSuite) TestOnTimeoutPacketForwarding() { pathAtoB, pathBtoC := suite.setupForwardingPaths() amount := sdkmath.NewInt(100) @@ -1018,7 +1029,7 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacketForwarding() { // TestForwardingWithMoreThanOneHop tests the scenario in which we // forward with more than one forwarding hop. -func (suite *KeeperTestSuite) TestForwardingWithMoreThanOneHop() { +func (suite *ForwardingTestSuite) TestForwardingWithMoreThanOneHop() { // Setup A->B->C->D coinOnA := ibctesting.TestCoin @@ -1131,7 +1142,7 @@ func (suite *KeeperTestSuite) TestForwardingWithMoreThanOneHop() { suite.Require().NoError(err) } -func (suite *KeeperTestSuite) TestMultihopForwardingErrorAcknowledgement() { +func (suite *ForwardingTestSuite) TestMultihopForwardingErrorAcknowledgement() { // Setup A->B->C->D coinOnA := ibctesting.TestCoin From 284831204159d399f3c86df05c5b2e7c3173e54c Mon Sep 17 00:00:00 2001 From: NisTun Date: Tue, 9 Jul 2024 10:08:53 +0700 Subject: [PATCH 2/4] lint --- modules/apps/transfer/keeper/relay_forwarding_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/keeper/relay_forwarding_test.go b/modules/apps/transfer/keeper/relay_forwarding_test.go index e0294655489..ccca2b14c55 100644 --- a/modules/apps/transfer/keeper/relay_forwarding_test.go +++ b/modules/apps/transfer/keeper/relay_forwarding_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/cosmos/gogoproto/proto" + testifysuite "github.com/stretchr/testify/suite" sdkmath "cosmossdk.io/math" @@ -17,7 +18,6 @@ import ( clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/v8/testing" - testifysuite "github.com/stretchr/testify/suite" ) type ForwardingTestSuite struct { From 604bf26a7f8ec585f8e6a4fa86071448b94df83c Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Tue, 9 Jul 2024 10:36:20 +0300 Subject: [PATCH 3/4] chore: add initial test to trigger tests. --- .../transfer/keeper/relay_forwarding_test.go | 73 ++++++++++++------- 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/modules/apps/transfer/keeper/relay_forwarding_test.go b/modules/apps/transfer/keeper/relay_forwarding_test.go index ccca2b14c55..3de788c6885 100644 --- a/modules/apps/transfer/keeper/relay_forwarding_test.go +++ b/modules/apps/transfer/keeper/relay_forwarding_test.go @@ -2,6 +2,7 @@ package keeper_test import ( "fmt" + "testing" "time" "github.com/cosmos/gogoproto/proto" @@ -20,14 +21,38 @@ import ( ibctesting "github.com/cosmos/ibc-go/v8/testing" ) -type ForwardingTestSuite struct { - testifysuite.Suite +const ( + escrow amountType = iota + balance +) + +type ( + ForwardingTestSuite struct { + testifysuite.Suite + + coordinator *ibctesting.Coordinator - // testing chains used for convenience and readability - chainA *ibctesting.TestChain - chainB *ibctesting.TestChain - chainC *ibctesting.TestChain - chainD *ibctesting.TestChain + // testing chains used for convenience and readability + chainA *ibctesting.TestChain + chainB *ibctesting.TestChain + chainC *ibctesting.TestChain + chainD *ibctesting.TestChain + } + + amountType int +) + +func TestForwardingTestSuite(t *testing.T) { + testifysuite.Run(t, new(ForwardingTestSuite)) +} + +func (suite *ForwardingTestSuite) SetupTest() { + suite.coordinator = ibctesting.NewCoordinator(suite.T(), 4) + + suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1)) + suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2)) + suite.chainC = suite.coordinator.GetChain(ibctesting.GetChainID(3)) + suite.chainD = suite.coordinator.GetChain(ibctesting.GetChainID(4)) } func (suite *ForwardingTestSuite) setupForwardingPaths() (pathAtoB, pathBtoC *ibctesting.Path) { @@ -35,27 +60,8 @@ func (suite *ForwardingTestSuite) setupForwardingPaths() (pathAtoB, pathBtoC *ib pathBtoC = ibctesting.NewTransferPath(suite.chainB, suite.chainC) pathAtoB.Setup() pathBtoC.Setup() - return pathAtoB, pathBtoC -} -type amountType int - -const ( - escrow amountType = iota - balance -) - -func (suite *ForwardingTestSuite) assertAmountOnChain(chain *ibctesting.TestChain, balanceType amountType, amount sdkmath.Int, denom string) { - var total sdk.Coin - switch balanceType { - case escrow: - total = chain.GetSimApp().TransferKeeper.GetTotalEscrowForDenom(chain.GetContext(), denom) - case balance: - total = chain.GetSimApp().BankKeeper.GetBalance(chain.GetContext(), chain.SenderAccounts[0].SenderAccount.GetAddress(), denom) - default: - suite.Fail("invalid amountType %s", balanceType) - } - suite.Require().Equal(amount, total.Amount, fmt.Sprintf("Chain %s: got balance of %d, wanted %d", chain.Name(), total.Amount, amount)) + return pathAtoB, pathBtoC } // TestStoredForwardedPacketAndEscrowAfterFirstHop tests that the forwarded packet @@ -1278,3 +1284,16 @@ func parseAckFromTransferEvents(events []abci.Event) (string, error) { return "", fmt.Errorf("acknowledgement event attribute not found") } + +func (suite *ForwardingTestSuite) assertAmountOnChain(chain *ibctesting.TestChain, balanceType amountType, amount sdkmath.Int, denom string) { + var total sdk.Coin + switch balanceType { + case escrow: + total = chain.GetSimApp().TransferKeeper.GetTotalEscrowForDenom(chain.GetContext(), denom) + case balance: + total = chain.GetSimApp().BankKeeper.GetBalance(chain.GetContext(), chain.SenderAccounts[0].SenderAccount.GetAddress(), denom) + default: + suite.Fail("invalid amountType %s", balanceType) + } + suite.Require().Equal(amount, total.Amount, fmt.Sprintf("Chain %s: got balance of %d, wanted %d", chain.Name(), total.Amount, amount)) +} From dedf741ddbf77bd03ed6d37c0f2f208ed7d720fa Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Tue, 9 Jul 2024 12:34:23 +0300 Subject: [PATCH 4/4] chore: split grouping to keep struct def separate. --- .../transfer/keeper/relay_forwarding_test.go | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/modules/apps/transfer/keeper/relay_forwarding_test.go b/modules/apps/transfer/keeper/relay_forwarding_test.go index 3de788c6885..9f499ec0f59 100644 --- a/modules/apps/transfer/keeper/relay_forwarding_test.go +++ b/modules/apps/transfer/keeper/relay_forwarding_test.go @@ -26,21 +26,19 @@ const ( balance ) -type ( - ForwardingTestSuite struct { - testifysuite.Suite +type ForwardingTestSuite struct { + testifysuite.Suite - coordinator *ibctesting.Coordinator + coordinator *ibctesting.Coordinator - // testing chains used for convenience and readability - chainA *ibctesting.TestChain - chainB *ibctesting.TestChain - chainC *ibctesting.TestChain - chainD *ibctesting.TestChain - } + // testing chains used for convenience and readability + chainA *ibctesting.TestChain + chainB *ibctesting.TestChain + chainC *ibctesting.TestChain + chainD *ibctesting.TestChain +} - amountType int -) +type amountType int func TestForwardingTestSuite(t *testing.T) { testifysuite.Run(t, new(ForwardingTestSuite))