added query to list burnt NFTs

[miladz68]

This change is 

[dzmitryhil]

Reviewable status: 0 of 9 files reviewed, 3 unresolved discussions (waiting on @miladz68, @silverspase, @wojtek-coreum, and @ysv)

---

proto/coreum/asset/nft/v1/query.proto line 129 at r1 (raw file):

}

message QueryBurntNFTsInClassRequest {

@wojtek-coreum @ysv
Do we need them both or we can keep only QueryBurntNFTs with both class_id and nft_id (optional) attributes? So if you query for both and the result is empty then it isn't burnt.

---

x/asset/nft/keeper/grpc_query.go line 23 at r1 (raw file):

	IsWhitelisted(ctx sdk.Context, classID, nftID string, account sdk.AccAddress) (bool, error)
	GetWhitelistedAccountsForNFT(ctx sdk.Context, classID, nftID string, q *query.PageRequest) ([]string, *query.PageResponse, error)
	ListBurnt(ctx sdk.Context, classID string, q *query.PageRequest) (*query.PageResponse, []string, error)

The name is different from the style we followed before.
I propose:
ListBurnt - >GetBurnt

---

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

// CreateClassBurningKey constructs the key for the burning of non-fungible token.
func CreateClassBurningKey(classID string) ([]byte, error) {

You can use it in the CreateBurningKey as well.

[dzmitryhil]

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

[miladz68]

Reviewable status: 7 of 9 files reviewed, 3 unresolved discussions (waiting on @dzmitryhil, @silverspase, @wojtek-coreum, and @ysv)

---

proto/coreum/asset/nft/v1/query.proto line 129 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

@wojtek-coreum @ysv
Do we need them both or we can keep only QueryBurntNFTs with both class_id and nft_id (optional) attributes? So if you query for both and the result is empty then it isn't burnt.

We already discussed this and decided to go with separate queries.

---

x/asset/nft/keeper/grpc_query.go line 23 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

The name is different from the style we followed before.
I propose:
ListBurnt - >GetBurnt

Done.

---

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

Previously, dzmitryhil (Dzmitry Hil) wrote…

You can use it in the CreateBurningKey as well.

Doing that will append key length twice. we will need to introduce another helper function to do that properly, which is overkill.

[dzmitryhil]

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

[ysv]

Reviewed 7 of 9 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @miladz68, @silverspase, and @wojtek-coreum)

---

proto/coreum/asset/nft/v1/query.proto line 120 at r2 (raw file):

}

message QueryBurntNFTRequest {

What do you think about naming it QueryIsNFTBurntRequest and name it accordingly everywhere: IsNFTBurnt including query service etc

if you disagree I will resolve & approve

[miladz68]

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

---

proto/coreum/asset/nft/v1/query.proto line 120 at r2 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

What do you think about naming it QueryIsNFTBurntRequest and name it accordingly everywhere: IsNFTBurnt including query service etc

if you disagree I will resolve & approve

I don't have a strong opinion here, I also thought about it before and also here and I kind of agree with it. But I in the past I used IsWhitelisted and IsFrozen in previous queries and the result of the discussion was not to use Is prefix. So to follow on the same style I did not use Is Prefix.

[miladz68]

Reviewable status: 4 of 9 files reviewed, 1 unresolved discussion (waiting on @dzmitryhil, @silverspase, @wojtek-coreum, and @ysv)

---

proto/coreum/asset/nft/v1/query.proto line 120 at r2 (raw file):

Previously, miladz68 (milad) wrote…

I don't have a strong opinion here, I also thought about it before and also here and I kind of agree with it. But I in the past I used IsWhitelisted and IsFrozen in previous queries and the result of the discussion was not to use Is prefix. So to follow on the same style I did not use Is Prefix.

I also removed is prefix from the field to completely match the previous style.

[dzmitryhil]

Reviewable status: 4 of 9 files reviewed, 1 unresolved discussion (waiting on @silverspase, @wojtek-coreum, and @ysv)

[ysv]

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