NFT issuer does not need whitelisting

[wojtek-coreum]

This change is 

[miladz68]

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

[miladz68]

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

[miladz68]

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

[ysv]

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

---

x/asset/nft/keeper/keeper.go line 469 at r2 (raw file):

	}

	if classDefinition.Issuer == account.String() {

IMO this check should be moved to CheckFeatureAllowed method
I think it is implemented in the same way in FT

[wojtek-coreum]

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

---

x/asset/nft/keeper/keeper.go line 469 at r2 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

IMO this check should be moved to CheckFeatureAllowed method
I think it is implemented in the same way in FT

Nope, in FT it done in SetWhitelistedBalance:

	if def.IsIssuer(addr) {
		return sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "issuer's balance can't be whitelisted")
	}

I followed that convention in NFT

[ysv]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dzmitryhil and @silverspase)

[ysv]

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

---

x/asset/nft/keeper/keeper.go line 469 at r2 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

Nope, in FT it done in SetWhitelistedBalance:

	if def.IsIssuer(addr) {
		return sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "issuer's balance can't be whitelisted")
	}

I followed that convention in NFT

Then IMO it is logical to move this logic to CheckFeature allowed in both FT & NFT (freezing & whitelisting) to have centralised place controlling permissions of these features

[dzmitryhil]

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

---

x/asset/nft/keeper/keeper.go line 469 at r2 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

Then IMO it is logical to move this logic to CheckFeature allowed in both FT & NFT (freezing & whitelisting) to have centralised place controlling permissions of these features

Agree, but we have similar check in the Is Feature Allowed so probably that func would be better.

[wojtek-coreum]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @miladz68, @silverspase, and @ysv)

---

x/asset/nft/keeper/keeper.go line 469 at r2 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Agree, but we have similar check in the Is Feature Allowed so probably that func would be better.

This is not possible. CheckFeatureAllowed is called by all the features and it acts based on issuer/sender account only. Here the decision is made based on the account address. Passing this address to that function doesn't make sense because it is specific to whitelisting only.

[dzmitryhil]

Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @miladz68, @silverspase, and @ysv)

[ysv]

Reviewed 2 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @silverspase and @wojtek-coreum)

---

x/asset/nft/keeper/keeper.go line 469 at r2 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

This is not possible. CheckFeatureAllowed is called by all the features and it acts based on issuer/sender account only. Here the decision is made based on the account address. Passing this address to that function doesn't make sense because it is specific to whitelisting only.

I see what you mean
Decission in usually based on sender but here it should be based on destination

ok resolving it

---

x/asset/nft/types/token.go line 120 at r4 (raw file):

// CheckFeatureAllowed returns error if feature isn't allowed for the address.
func (nftd ClassDefinition) CheckFeatureAllowed(addr sdk.AccAddress, feature ClassFeature) error {
	switch {

I personally prefer if version

[wojtek-coreum]

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

---

x/asset/nft/types/token.go line 120 at r4 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

I personally prefer if version

Done.

[ysv]

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

[miladz68]

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