Add query NFT classes with migration.

[dzmitryhil]

This change is 

[wojtek-coreum]

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

a discussion (no related file):
It would be nice to have a full upgrade integration test

---

---

x/asset/nft/keeper/keeper_test.go line 69 at r1 (raw file):

	requireT.True(types.ErrInvalidInput.Is(err))

	// try to get non-invalid class

non-invalid?

---

x/asset/nft/keeper/keeper_test.go line 73 at r1 (raw file):

	requireT.ErrorIs(err, types.ErrInvalidInput)

	//// try to get nonexistent class

//// -> //

---

x/asset/nft/keeper/keeper_test.go line 128 at r1 (raw file):

	for i := range classes {
		requireClassSettingsEqualClass(requireT, allSettings[i], classes[i])

can't you compare slices using built-in functions?

---

x/asset/nft/legacy/v1/store.go line 32 at r1 (raw file):

	}

	return oldStoreIter.Close()

Close() should be called in defer` present immediately after creating the iterator

---

x/asset/nft/types/keys.go line 45 at r1 (raw file):

// CreateIssuerClassPrefix constructs the key for the non-fungible token class for the specific issuer.
func CreateIssuerClassPrefix(issuer sdk.AccAddress) []byte {
	return store.JoinKeys(NFTClassKeyPrefix, issuer)

I think we should use length here. This is what comos sdk always does when address is part of the key

[dzmitryhil]

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

a discussion (no related file):

Previously, wojtek-coreum (Wojtek) wrote…

It would be nice to have a full upgrade integration test

It will be, I'm blocked by muti step upgrade task.

---

---

x/asset/nft/keeper/keeper_test.go line 69 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

non-invalid?

Done.

---

x/asset/nft/keeper/keeper_test.go line 73 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

//// -> //

Done.

---

x/asset/nft/keeper/keeper_test.go line 128 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

can't you compare slices using built-in functions?

Nope, since we compare the Class and ClassSettings

---

x/asset/nft/legacy/v1/store.go line 32 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

Close() should be called in defer` present immediately after creating the iterator

But in that case you won't get an error. Or you will have to use the naming returns.

---

x/asset/nft/types/keys.go line 45 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

I think we should use length here. This is what comos sdk always does when address is part of the key

In that case we won't be able to search by address.

[wojtek-coreum]

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

---

x/asset/nft/legacy/v1/store.go line 32 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

But in that case you won't get an error. Or you will have to use the naming returns.

But in this implementation iterator stays open if error is returned in the loop

---

x/asset/nft/types/keys.go line 45 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

In that case we won't be able to search by address.

why? during the search you just add the length to the prefix too

[dzmitryhil]

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

---

x/asset/nft/legacy/v1/store.go line 32 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

But in this implementation iterator stays open if error is returned in the loop

Agree, update to defer

---

x/asset/nft/types/keys.go line 45 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

why? during the search you just add the length to the prefix too

Missed that, agree, updated.

[wojtek-coreum]

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

[miladz68]

Reviewed 24 of 28 files at r1, 1 of 1 files at r2, 4 of 4 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @silverspase and @ysv)

[miladz68]

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

[wojtek-coreum]

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

[wojtek-coreum]

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