From 14aa56f3c73e43c128794c8c82e1190b886fba9a Mon Sep 17 00:00:00 2001 From: Yaroslav Savchuk Date: Fri, 13 Oct 2023 14:15:09 +0200 Subject: [PATCH 1/9] Fix TestIBCCallFromSmartContract to run relayer for itself fully --- integration-tests/ibc/ibc.go | 43 +++++++++++++++++++++++++++--- integration-tests/ibc/wasm_test.go | 11 ++++---- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/integration-tests/ibc/ibc.go b/integration-tests/ibc/ibc.go index ec6c27fe3..607cd56e2 100644 --- a/integration-tests/ibc/ibc.go +++ b/integration-tests/ibc/ibc.go @@ -4,6 +4,7 @@ package ibc import ( "context" + "errors" "fmt" "testing" "time" @@ -29,8 +30,9 @@ func ConvertToIBCDenom(channelID, denom string) string { ).IBCDenom() } -// CreateIBCChannelsAndConnect creates two new channels for the provided ports on provided chains and connects them. -func CreateIBCChannelsAndConnect( +// CreateOpenIBCChannelsAndStartRelaying creates two new channels for the provided ports on provided chains, +// connects them and starts relaying them. +func CreateOpenIBCChannelsAndStartRelaying( ctx context.Context, t *testing.T, srcChain integration.Chain, @@ -63,6 +65,7 @@ func CreateIBCChannelsAndConnect( } pathName := fmt.Sprintf("%s-%s", srcChain.ChainSettings.ChainID, dstChain.ChainSettings.ChainID) + require.NoError(t, relayerSrcChain.CreateOpenChannels( ctx, relayerDstChain, @@ -74,10 +77,42 @@ func CreateIBCChannelsAndConnect( "", pathName, )) - closerFunc := func() { + + relErrCh := cosmosrelayer.StartRelayer( + ctx, + log.With(zap.String("process", "relayer")), + map[string]*cosmosrelayer.Chain{ + srcChain.ChainSettings.ChainID: relayerSrcChain, + dstChain.ChainSettings.ChainID: relayerDstChain, + }, + []cosmosrelayer.NamedPath{ + {Name: pathName, Path: &cosmosrelayer.Path{ + Src: relayerSrcChain.PathEnd, + Dst: relayerDstChain.PathEnd, + Filter: cosmosrelayer.ChannelFilter{}, + }}, + }, + cosmosrelayer.DefaultMaxMsgLength, + "", + cosmosrelayer.DefaultClientUpdateThreshold, + cosmosrelayer.DefaultFlushInterval, + nil, + cosmosrelayer.ProcessorEvents, + 0, + nil, + ) + + go func() { + err := <-relErrCh + if !errors.Is(err, context.Canceled) { + return + } + require.NoError(t, err, "Cosmos Relayer start error") + }() + + return func() { require.NoError(t, relayerSrcChain.CloseChannel(ctx, relayerDstChain, 5, 5*time.Second, srcChain.ChainSettings.ChainID, srcChainPort, "", pathName)) } - return closerFunc } func setupRelayerChain( diff --git a/integration-tests/ibc/wasm_test.go b/integration-tests/ibc/wasm_test.go index 62558c34f..4652831fc 100644 --- a/integration-tests/ibc/wasm_test.go +++ b/integration-tests/ibc/wasm_test.go @@ -235,7 +235,7 @@ func TestIBCCallFromSmartContract(t *testing.T) { requireT.NotEmpty(osmosisIBCPort) t.Logf("Osmisis contrac IBC port:%s", osmosisIBCPort) - closerFunc := CreateIBCChannelsAndConnect( + closerFunc := CreateOpenIBCChannelsAndStartRelaying( ctx, t, coreumChain.Chain, @@ -251,21 +251,22 @@ func TestIBCCallFromSmartContract(t *testing.T) { osmosisToCoreumChannelID := osmosisChain.AwaitForIBCChannelID(ctx, t, osmosisIBCPort, coreumChain.ChainSettings.ChainID) t.Logf("Channels are ready coreum channel ID:%s, osmosis channel ID:%s", coreumToOsmosisChannelID, osmosisToCoreumChannelID) - t.Logf("Sendng two IBC transactions from coreum contract to osmosis contract") + // make sure that initial values are correct. awaitWasmCounterValue(ctx, t, coreumChain.Chain, coreumToOsmosisChannelID, coreumContractAddr, 0) awaitWasmCounterValue(ctx, t, osmosisChain, osmosisToCoreumChannelID, osmosisContractAddr, 0) - // execute coreum counter twice + t.Logf("Sendng two IBC-increment transactions from coreum contract to osmosis contract") executeWasmIncrement(ctx, requireT, coreumChain.Chain, coreumCaller, coreumToOsmosisChannelID, coreumContractAddr) executeWasmIncrement(ctx, requireT, coreumChain.Chain, coreumCaller, coreumToOsmosisChannelID, coreumContractAddr) // check that current state is expected // the order of assertion is important because we are waiting for the expected non-zero counter first to be sure - // that async operation is completed fully before the second assertion + // that async operations are completed fully before the second assertion. + // This is done to make sure that all IBC operations are finished before we make second assertion. awaitWasmCounterValue(ctx, t, osmosisChain, osmosisToCoreumChannelID, osmosisContractAddr, 2) awaitWasmCounterValue(ctx, t, coreumChain.Chain, coreumToOsmosisChannelID, coreumContractAddr, 0) - t.Logf("Sendng three IBC transactions from osmosis contract to coreum contract") + t.Logf("Sendng three IBC-increment transactions from osmosis contract to coreum contract") executeWasmIncrement(ctx, requireT, osmosisChain, osmosisCaller, osmosisToCoreumChannelID, osmosisContractAddr) executeWasmIncrement(ctx, requireT, osmosisChain, osmosisCaller, osmosisToCoreumChannelID, osmosisContractAddr) executeWasmIncrement(ctx, requireT, osmosisChain, osmosisCaller, osmosisToCoreumChannelID, osmosisContractAddr) From cbcfceac6f086c030df4b2212b9f0561d224f87f Mon Sep 17 00:00:00 2001 From: Yaroslav Savchuk Date: Fri, 13 Oct 2023 14:21:12 +0200 Subject: [PATCH 2/9] Improve comments --- integration-tests/ibc/ibc.go | 3 +++ integration-tests/ibc/wasm_test.go | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/integration-tests/ibc/ibc.go b/integration-tests/ibc/ibc.go index 607cd56e2..0dc1e2c1d 100644 --- a/integration-tests/ibc/ibc.go +++ b/integration-tests/ibc/ibc.go @@ -66,6 +66,9 @@ func CreateOpenIBCChannelsAndStartRelaying( pathName := fmt.Sprintf("%s-%s", srcChain.ChainSettings.ChainID, dstChain.ChainSettings.ChainID) + // port name samples: + // coreum: wasm.devcore14hj2tavq8fpesdwxxcu44rty3hh90vhujrvcmstl4zr3txmfvw9sd4f0ak + // osmosis: wasm.osmo14hj2tavq8fpesdwxxcu44rty3hh90vhujrvcmstl4zr3txmfvw9sq2r9g9 require.NoError(t, relayerSrcChain.CreateOpenChannels( ctx, relayerDstChain, diff --git a/integration-tests/ibc/wasm_test.go b/integration-tests/ibc/wasm_test.go index 4652831fc..1606a5bc3 100644 --- a/integration-tests/ibc/wasm_test.go +++ b/integration-tests/ibc/wasm_test.go @@ -157,10 +157,10 @@ func TestIBCTransferFromSmartContract(t *testing.T) { requireT.NoError(osmosisChain.AwaitForBalance(ctx, t, osmosisRecipient, expectedOsmosisRecipientBalance)) } -// TestIBCCallFromSmartContract tests the IBC contract calls. +// TestIBCCallFromSmartContract tests the WASM contract calls via WASM IBC channel. func TestIBCCallFromSmartContract(t *testing.T) { - // we don't enable the t.Parallel here since that test uses the config unseal hack because of the cosmos relayer - // implementation + // We don't enable the t.Parallel here since that test uses the config unseal hack. + // Config unseal is needed to run cosmos relayer which interacts with both chain SDK configs internally. restoreSDKConfig := unsealSDKConfig() defer restoreSDKConfig() From 31ba274276350b5bacec523de152d10634a135ef Mon Sep 17 00:00:00 2001 From: Yaroslav Savchuk Date: Fri, 13 Oct 2023 18:56:28 +0200 Subject: [PATCH 3/9] Temporary skip TestIBCCallFromSmartContract --- integration-tests/ibc/wasm_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/integration-tests/ibc/wasm_test.go b/integration-tests/ibc/wasm_test.go index 1606a5bc3..f3d5cd0cc 100644 --- a/integration-tests/ibc/wasm_test.go +++ b/integration-tests/ibc/wasm_test.go @@ -159,6 +159,9 @@ func TestIBCTransferFromSmartContract(t *testing.T) { // TestIBCCallFromSmartContract tests the WASM contract calls via WASM IBC channel. func TestIBCCallFromSmartContract(t *testing.T) { + // Temporary skip this test because it is incompatible with crust but crust PR needs changes from coreum. + t.SkipNow() + // We don't enable the t.Parallel here since that test uses the config unseal hack. // Config unseal is needed to run cosmos relayer which interacts with both chain SDK configs internally. restoreSDKConfig := unsealSDKConfig() From 135ff73668b7b46f71547d966433201ae67667e1 Mon Sep 17 00:00:00 2001 From: Yaroslav Savchuk Date: Mon, 16 Oct 2023 10:24:10 +0200 Subject: [PATCH 4/9] Fix --- integration-tests/ibc/ibc.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/integration-tests/ibc/ibc.go b/integration-tests/ibc/ibc.go index 0dc1e2c1d..ad946b57f 100644 --- a/integration-tests/ibc/ibc.go +++ b/integration-tests/ibc/ibc.go @@ -66,9 +66,6 @@ func CreateOpenIBCChannelsAndStartRelaying( pathName := fmt.Sprintf("%s-%s", srcChain.ChainSettings.ChainID, dstChain.ChainSettings.ChainID) - // port name samples: - // coreum: wasm.devcore14hj2tavq8fpesdwxxcu44rty3hh90vhujrvcmstl4zr3txmfvw9sd4f0ak - // osmosis: wasm.osmo14hj2tavq8fpesdwxxcu44rty3hh90vhujrvcmstl4zr3txmfvw9sq2r9g9 require.NoError(t, relayerSrcChain.CreateOpenChannels( ctx, relayerDstChain, @@ -107,7 +104,7 @@ func CreateOpenIBCChannelsAndStartRelaying( go func() { err := <-relErrCh - if !errors.Is(err, context.Canceled) { + if errors.Is(err, context.Canceled) { return } require.NoError(t, err, "Cosmos Relayer start error") From 373a0157fea78dbc9edf71012b05bc605fe6e9a5 Mon Sep 17 00:00:00 2001 From: Yaroslav Savchuk Date: Mon, 16 Oct 2023 10:51:03 +0200 Subject: [PATCH 5/9] Fix --- integration-tests/modules/legacy_nft_assetnft_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/integration-tests/modules/legacy_nft_assetnft_test.go b/integration-tests/modules/legacy_nft_assetnft_test.go index 78f0792c5..627e5f442 100644 --- a/integration-tests/modules/legacy_nft_assetnft_test.go +++ b/integration-tests/modules/legacy_nft_assetnft_test.go @@ -21,6 +21,8 @@ import ( // TestAssetNFTMintLegacyNFTClient tests legacy APIs in cnft are working. // we should remove this test after we remove cnf module. +// +//nolint:staticcheck // we are testing deprecated handlers func TestAssetNFTMintLegacyNFTClient(t *testing.T) { t.Parallel() @@ -93,7 +95,7 @@ func TestAssetNFTMintLegacyNFTClient(t *testing.T) { }, nftMintedEvent) // check that token is present in the nft module - nftRes, err := nftClient.NFT(ctx, &nft.QueryNFTRequest{ //nolint:staticcheck // we are testing deprecated handlers + nftRes, err := nftClient.NFT(ctx, &nft.QueryNFTRequest{ ClassId: classID, Id: nftMintedEvent.Id, }) @@ -112,7 +114,7 @@ func TestAssetNFTMintLegacyNFTClient(t *testing.T) { requireT.Equal(jsonData, data2.Data) // check the owner - ownerRes, err := nftClient.Owner(ctx, &nft.QueryOwnerRequest{ //nolint:staticcheck // we are testing deprecated handlers + ownerRes, err := nftClient.Owner(ctx, &nft.QueryOwnerRequest{ ClassId: classID, Id: nftMintedEvent.Id, }) @@ -145,7 +147,7 @@ func TestAssetNFTMintLegacyNFTClient(t *testing.T) { }, nftSentEvent) // check new owner - ownerRes, err = nftClient.Owner(ctx, &nft.QueryOwnerRequest{ //nolint:staticcheck // we are testing deprecated handlers + ownerRes, err = nftClient.Owner(ctx, &nft.QueryOwnerRequest{ ClassId: classID, Id: nftMintedEvent.Id, }) From bca6bbe8528c9a96d49a3ad10ffc0a25dad944c8 Mon Sep 17 00:00:00 2001 From: Yaroslav Savchuk Date: Mon, 16 Oct 2023 10:52:15 +0200 Subject: [PATCH 6/9] Fix 2 --- integration-tests/modules/legacy_nft_assetnft_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/integration-tests/modules/legacy_nft_assetnft_test.go b/integration-tests/modules/legacy_nft_assetnft_test.go index 627e5f442..6151844e4 100644 --- a/integration-tests/modules/legacy_nft_assetnft_test.go +++ b/integration-tests/modules/legacy_nft_assetnft_test.go @@ -21,8 +21,6 @@ import ( // TestAssetNFTMintLegacyNFTClient tests legacy APIs in cnft are working. // we should remove this test after we remove cnf module. -// -//nolint:staticcheck // we are testing deprecated handlers func TestAssetNFTMintLegacyNFTClient(t *testing.T) { t.Parallel() @@ -95,6 +93,7 @@ func TestAssetNFTMintLegacyNFTClient(t *testing.T) { }, nftMintedEvent) // check that token is present in the nft module + //nolint:staticcheck // we are testing deprecated handlers nftRes, err := nftClient.NFT(ctx, &nft.QueryNFTRequest{ ClassId: classID, Id: nftMintedEvent.Id, @@ -114,6 +113,7 @@ func TestAssetNFTMintLegacyNFTClient(t *testing.T) { requireT.Equal(jsonData, data2.Data) // check the owner + //nolint:staticcheck // we are testing deprecated handlers ownerRes, err := nftClient.Owner(ctx, &nft.QueryOwnerRequest{ ClassId: classID, Id: nftMintedEvent.Id, @@ -147,6 +147,7 @@ func TestAssetNFTMintLegacyNFTClient(t *testing.T) { }, nftSentEvent) // check new owner + //nolint:staticcheck // we are testing deprecated handlers ownerRes, err = nftClient.Owner(ctx, &nft.QueryOwnerRequest{ ClassId: classID, Id: nftMintedEvent.Id, From cb2c815f52f97a01bba33cd2504b51ff140d0d29 Mon Sep 17 00:00:00 2001 From: Yaroslav Savchuk Date: Mon, 16 Oct 2023 17:37:28 +0200 Subject: [PATCH 7/9] Run assertion for error returned by cosmos relayer in main routine --- integration-tests/ibc/ibc.go | 27 +++++++++++++++++---------- integration-tests/ibc/wasm_test.go | 6 +++--- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/integration-tests/ibc/ibc.go b/integration-tests/ibc/ibc.go index ad946b57f..4b9fca317 100644 --- a/integration-tests/ibc/ibc.go +++ b/integration-tests/ibc/ibc.go @@ -78,8 +78,9 @@ func CreateOpenIBCChannelsAndStartRelaying( pathName, )) - relErrCh := cosmosrelayer.StartRelayer( - ctx, + relayerCtx, relayerCtxCancelF := context.WithCancel(ctx) + relayerErrCh := cosmosrelayer.StartRelayer( + relayerCtx, log.With(zap.String("process", "relayer")), map[string]*cosmosrelayer.Chain{ srcChain.ChainSettings.ChainID: relayerSrcChain, @@ -102,16 +103,22 @@ func CreateOpenIBCChannelsAndStartRelaying( nil, ) - go func() { - err := <-relErrCh - if errors.Is(err, context.Canceled) { - return - } - require.NoError(t, err, "Cosmos Relayer start error") - }() - return func() { require.NoError(t, relayerSrcChain.CloseChannel(ctx, relayerDstChain, 5, 5*time.Second, srcChain.ChainSettings.ChainID, srcChainPort, "", pathName)) + + t.Logf("Stopping relayer") + relayerCtxCancelF() + + select { + case err := <-relayerErrCh: + if errors.Is(err, context.Canceled) { + t.Logf("Relayer exited gracefully") + return + } + require.NoError(t, err, "Cosmos Relayer run error") + case <-time.After(30 * time.Second): + require.FailNow(t, "Cosmos Relayer stop timeout") + } } } diff --git a/integration-tests/ibc/wasm_test.go b/integration-tests/ibc/wasm_test.go index f3d5cd0cc..e4cfd21a7 100644 --- a/integration-tests/ibc/wasm_test.go +++ b/integration-tests/ibc/wasm_test.go @@ -159,8 +159,8 @@ func TestIBCTransferFromSmartContract(t *testing.T) { // TestIBCCallFromSmartContract tests the WASM contract calls via WASM IBC channel. func TestIBCCallFromSmartContract(t *testing.T) { - // Temporary skip this test because it is incompatible with crust but crust PR needs changes from coreum. - t.SkipNow() + // FIXME: Temporary skip this test because it is incompatible with crust but crust PR needs changes from coreum. + //t.SkipNow() // We don't enable the t.Parallel here since that test uses the config unseal hack. // Config unseal is needed to run cosmos relayer which interacts with both chain SDK configs internally. @@ -258,7 +258,7 @@ func TestIBCCallFromSmartContract(t *testing.T) { awaitWasmCounterValue(ctx, t, coreumChain.Chain, coreumToOsmosisChannelID, coreumContractAddr, 0) awaitWasmCounterValue(ctx, t, osmosisChain, osmosisToCoreumChannelID, osmosisContractAddr, 0) - t.Logf("Sendng two IBC-increment transactions from coreum contract to osmosis contract") + t.Logf("Sending two IBC-increment transactions from coreum contract to osmosis contract") executeWasmIncrement(ctx, requireT, coreumChain.Chain, coreumCaller, coreumToOsmosisChannelID, coreumContractAddr) executeWasmIncrement(ctx, requireT, coreumChain.Chain, coreumCaller, coreumToOsmosisChannelID, coreumContractAddr) From 68dbed5d5c3e931bb1fc8407f44d54890c012c97 Mon Sep 17 00:00:00 2001 From: Yaroslav Savchuk Date: Mon, 16 Oct 2023 17:38:01 +0200 Subject: [PATCH 8/9] reset linter changes --- integration-tests/modules/legacy_nft_assetnft_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/integration-tests/modules/legacy_nft_assetnft_test.go b/integration-tests/modules/legacy_nft_assetnft_test.go index 6151844e4..78f0792c5 100644 --- a/integration-tests/modules/legacy_nft_assetnft_test.go +++ b/integration-tests/modules/legacy_nft_assetnft_test.go @@ -93,8 +93,7 @@ func TestAssetNFTMintLegacyNFTClient(t *testing.T) { }, nftMintedEvent) // check that token is present in the nft module - //nolint:staticcheck // we are testing deprecated handlers - nftRes, err := nftClient.NFT(ctx, &nft.QueryNFTRequest{ + nftRes, err := nftClient.NFT(ctx, &nft.QueryNFTRequest{ //nolint:staticcheck // we are testing deprecated handlers ClassId: classID, Id: nftMintedEvent.Id, }) @@ -113,8 +112,7 @@ func TestAssetNFTMintLegacyNFTClient(t *testing.T) { requireT.Equal(jsonData, data2.Data) // check the owner - //nolint:staticcheck // we are testing deprecated handlers - ownerRes, err := nftClient.Owner(ctx, &nft.QueryOwnerRequest{ + ownerRes, err := nftClient.Owner(ctx, &nft.QueryOwnerRequest{ //nolint:staticcheck // we are testing deprecated handlers ClassId: classID, Id: nftMintedEvent.Id, }) @@ -147,8 +145,7 @@ func TestAssetNFTMintLegacyNFTClient(t *testing.T) { }, nftSentEvent) // check new owner - //nolint:staticcheck // we are testing deprecated handlers - ownerRes, err = nftClient.Owner(ctx, &nft.QueryOwnerRequest{ + ownerRes, err = nftClient.Owner(ctx, &nft.QueryOwnerRequest{ //nolint:staticcheck // we are testing deprecated handlers ClassId: classID, Id: nftMintedEvent.Id, }) From b7241443dc73e6923ab73734d1397630a55a1857 Mon Sep 17 00:00:00 2001 From: Yaroslav Savchuk Date: Mon, 16 Oct 2023 17:38:43 +0200 Subject: [PATCH 9/9] uncomment skip now --- integration-tests/ibc/wasm_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/ibc/wasm_test.go b/integration-tests/ibc/wasm_test.go index e4cfd21a7..987384cc9 100644 --- a/integration-tests/ibc/wasm_test.go +++ b/integration-tests/ibc/wasm_test.go @@ -160,7 +160,7 @@ func TestIBCTransferFromSmartContract(t *testing.T) { // TestIBCCallFromSmartContract tests the WASM contract calls via WASM IBC channel. func TestIBCCallFromSmartContract(t *testing.T) { // FIXME: Temporary skip this test because it is incompatible with crust but crust PR needs changes from coreum. - //t.SkipNow() + t.SkipNow() // We don't enable the t.Parallel here since that test uses the config unseal hack. // Config unseal is needed to run cosmos relayer which interacts with both chain SDK configs internally.