Wasm IBC integration tests

[dzmitryhil]

• Refactor integration tests to make the wasm chain agnostic.
• Add smart contract with simple IBC transfer and integration test for it.
• Add go.mod for the integration tests.
• Implement the IBC call contract and test for it.
• Integrate the go IBC relayer to create/connect the channels in runtime.

---

This change is 

[miladz68]

Reviewed 38 of 38 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil, @silverspase, @wojtek-coreum, and @ysv)

---

integration-tests/go.mod line 1 at r1 (raw file):

module github.com/CoreumFoundation/coreum/integration-tests

I guess I missed the reason for this separate go.mod

---

integration-tests/wasm.go line 28 at r1 (raw file):

// Wasm provides wasm client for the testing.
type Wasm struct {

I don't think this struct is an improvement. let's keep these methods as functions, just like they used to be.

[dzmitryhil]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @miladz68, @silverspase, @wojtek-coreum, and @ysv)

---

integration-tests/go.mod line 1 at r1 (raw file):

Previously, miladz68 (milad) wrote…

I guess I missed the reason for this separate go.mod

The reason is the importing of the cosmos relayer to the dependencies.

---

integration-tests/wasm.go line 28 at r1 (raw file):

Previously, miladz68 (milad) wrote…

I don't think this struct is an improvement. let's keep these methods as functions, just like they used to be.

See, the idea is that some chains might support it some - not. Save as Faucet. So if a chain supports it will have the Wasm there is not then not.

[miladz68]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @silverspase, @wojtek-coreum, and @ysv)

[wojtek-coreum]

Reviewed 24 of 38 files at r1, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @dzmitryhil, @silverspase, and @ysv)

a discussion (no related file):
I reviewed everything except rust and integration test. Will continue once we discuss merging two contracts into single one.

---

---

integration-tests/chain_ibc.go line 30 at r1 (raw file):

)

// FIXME add/check that linter works with integration tests since now it's a separate module

Yes, it should work. Crust automatically detects all the modules in the repo and lints each one separately

---

integration-tests/chain_ibc.go line 32 at r1 (raw file):

// FIXME add/check that linter works with integration tests since now it's a separate module
// FIXME update the crust with new run args
// FIXME update the crust to build the integration tests since now it's a separate module

I think it should work out of the box

---

integration-tests/chain_ibc.go line 233 at r1 (raw file):

	t.Helper()

	log := zaptest.NewLogger(t)

In meantime we decided to use testing logger

---

integration-tests/chain_ibc.go line 292 at r1 (raw file):

	relayerSrcChainProvider, err := relayerSrcChainConfig.NewProvider(log, t.TempDir(), false, chain.ChainSettings.ChainID)
	require.NoError(t, err)
	relayerSrcChainKeyInfo, err := relayerSrcChainProvider.AddKey(relayerKeyName, chain.ChainSettings.CoinType)

What does it mean? Why is coin type stored under relayer-key key?

---

integration-tests/ibc/asset_ft_test.go line 54 at r1 (raw file):

	})

	gaiaChain.Faucet.FundAccounts(ctx, t, integrationtests.FundedAccount{

Why is it needed now?

---

integration-tests/ibc/testdata/wasm/.gitignore line 2 at r1 (raw file):

target
# FIXME(dzmitryhil) add artifacts to the ignore once we integrate the crust

?
In the modules tests artifacts are already ignored

---

integration-tests/ibc/testdata/wasm/README.md line 1 at r1 (raw file):

# Overview

Do we want to keep this file? We build artifacts with crust

---

integration-tests/ibc/testdata/wasm/ibc-transfer/Cargo.toml line 1 at r1 (raw file):

[package]

I don't see benefits of having two contracts for this. IMO it would be better to merge them into a single one, then create two instances and call one from the other.
On the other case I see benefits of having single one: less data to download, less caching in CI, single compilation. less bytecode to embed in the integration test binary.

@miladz68

let's discuss

---

integration-tests/modules/wasm_test.go line 1336 at r1 (raw file):

// isWASMContractPinned returns true if smart contract is pinned.
func isWASMContractPinned(ctx context.Context, clientCtx client.Context, codeID uint64) (bool, error) {

Maybe move this as well? If we have separate scope for wasm functions, then let's put everything there.

[dzmitryhil]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @miladz68, @silverspase, and @ysv)

---

integration-tests/ibc/testdata/wasm/ibc-transfer/Cargo.toml line 1 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

I don't see benefits of having two contracts for this. IMO it would be better to merge them into a single one, then create two instances and call one from the other.
On the other case I see benefits of having single one: less data to download, less caching in CI, single compilation. less bytecode to embed in the integration test binary.

@miladz68

let's discuss

But they are complitelty different in tearms of the business logic. One just calls the IBC transfer, and another implements the IBC fully. IMO merging them is same as merging of all wasm test contracts.

[miladz68]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @dzmitryhil, @silverspase, @wojtek-coreum, and @ysv)

---

integration-tests/ibc/testdata/wasm/ibc-transfer/Cargo.toml line 1 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

But they are complitelty different in tearms of the business logic. One just calls the IBC transfer, and another implements the IBC fully. IMO merging them is same as merging of all wasm test contracts.

let's discuss real fast.

[wojtek-coreum]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @dzmitryhil, @miladz68, @silverspase, and @ysv)

---

integration-tests/ibc/testdata/wasm/ibc-transfer/Cargo.toml line 1 at r1 (raw file):

Previously, miladz68 (milad) wrote…

let's discuss real fast.

But they can't live alone. The single purpose of their existence is testing. Do we really need a separate smart contract for single command sending "pings"?

[dzmitryhil]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @silverspase, @wojtek-coreum, and @ysv)

a discussion (no related file):

Previously, wojtek-coreum (Wojtek) wrote…

I reviewed everything except rust and integration test. Will continue once we discuss merging two contracts into single one.

Ok

---

---

integration-tests/chain_ibc.go line 30 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

Yes, it should work. Crust automatically detects all the modules in the repo and lints each one separately

Removed all fixme, the PR with all required changes is ready in the crust.

---

integration-tests/chain_ibc.go line 32 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

I think it should work out of the box

Removed all fixme, the PR with all required changes is ready in the crust.

---

integration-tests/chain_ibc.go line 233 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

In meantime we decided to use testing logger

Actually - not. The zap.Logger is a cosmos bridge dependency.

---

integration-tests/chain_ibc.go line 292 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

What does it mean? Why is coin type stored under relayer-key key?

That's how the cosmos relayer interface accept it.

---

integration-tests/ibc/asset_ft_test.go line 54 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

Why is it needed now?

We need it, since I've changes the fee from zero to minimum. That change is required to let the bridge work.

---

integration-tests/ibc/testdata/wasm/.gitignore line 2 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

?
In the modules tests artifacts are already ignored

Updated the crust. And added ignore here.
The module uses the different root, so that ignore should live separately.

---

integration-tests/ibc/testdata/wasm/README.md line 1 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

Do we want to keep this file? We build artifacts with crust

Removed.

---

integration-tests/ibc/testdata/wasm/ibc-transfer/Cargo.toml line 1 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

But they can't live alone. The single purpose of their existence is testing. Do we really need a separate smart contract for single command sending "pings"?

IMO we don't we shouldn't mix them.

---

integration-tests/modules/wasm_test.go line 1336 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

Maybe move this as well? If we have separate scope for wasm functions, then let's put everything there.

Done.

[wojtek-coreum]

Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @silverspase and @ysv)

[dzmitryhil]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @silverspase, @wojtek-coreum, and @ysv)

---

integration-tests/ibc/testdata/wasm/ibc-transfer/Cargo.toml line 1 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

IMO we don't we shouldn't mix them.

Decided to keep it.

[wojtek-coreum]

Reviewed 14 of 38 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @silverspase and @ysv)

[miladz68]

Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @silverspase and @ysv)

[wojtek-coreum]

Reviewed 30 of 30 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @silverspase and @ysv)

[wojtek-coreum]

Reviewed 14 of 14 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil, @silverspase, and @ysv)

a discussion (no related file):
I propose to disable funlen completely - it's sooo annoying

---

---

integration-tests/ibc/wasm_test.go line 166 at r4 (raw file):

// TestIBCCallFromSmartContract tests the IBC contract calls.
//
//nolint:funlen // there are many tests cases

test cases

[wojtek-coreum]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil, @silverspase, and @ysv)

[dzmitryhil]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @silverspase, @wojtek-coreum, and @ysv)

a discussion (no related file):

Previously, wojtek-coreum (Wojtek) wrote…

I propose to disable funlen completely - it's sooo annoying

Agree, but let's difable the funlen for the tests. And in separate task.

---

---

integration-tests/ibc/wasm_test.go line 166 at r4 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

test cases

Done.

[dzmitryhil]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @silverspase, @wojtek-coreum, and @ysv)

a discussion (no related file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Agree, but let's difable the funlen for the tests. And in separate task.

Create a task for it.

---

[wojtek-coreum]

Reviewed 14 of 14 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @silverspase and @ysv)

[wojtek-coreum]

Reviewed 3 of 3 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @miladz68, @silverspase, and @ysv)

[miladz68]

Reviewed 24 of 30 files at r3, 1 of 14 files at r4, 1 of 2 files at r5, 13 of 14 files at r6, 3 of 3 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @silverspase and @ysv)