Skip to content

Commit

Permalink
Apply whitelisting rules when minting NFTs (#675)
Browse files Browse the repository at this point in the history
# Description

This PR replaces previous one:
#674

Approach taken there was a complete failure because to check whitelisting nft must be minted first so it was a chicken & egg problem.

# Reviewers checklist:
- [ ] Try to write more meaningful comments with clear actions to be taken.
- [ ] Nit-picking should be unblocking. Focus on core issues.

# Authors checklist
- [x] Provide a concise and meaningful description
- [x] Review the code yourself first, before making the PR.
- [x] Annotate your PR in places that require explanation.
- [x] Think and try to split the PR to smaller PR if it is big.
  • Loading branch information
wojtek-coreum authored Oct 20, 2023
1 parent 360b1dc commit 84503b3
Show file tree
Hide file tree
Showing 9 changed files with 197 additions and 27 deletions.
5 changes: 3 additions & 2 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ import (
"github.com/pkg/errors"
"github.com/spf13/cast"

"github.com/cosmos/cosmos-sdk/x/nft"
nftkeeper "github.com/cosmos/cosmos-sdk/x/nft/keeper"

"github.com/CoreumFoundation/coreum/v3/app/openapi"
appupgrade "github.com/CoreumFoundation/coreum/v3/app/upgrade"
appupgradev1 "github.com/CoreumFoundation/coreum/v3/app/upgrade/v1"
Expand Down Expand Up @@ -141,8 +144,6 @@ import (
"github.com/CoreumFoundation/coreum/v3/x/wnft"
wnftkeeper "github.com/CoreumFoundation/coreum/v3/x/wnft/keeper"
"github.com/CoreumFoundation/coreum/v3/x/wstaking"
"github.com/cosmos/cosmos-sdk/x/nft"
nftkeeper "github.com/cosmos/cosmos-sdk/x/nft/keeper"
)

const (
Expand Down
88 changes: 88 additions & 0 deletions integration-tests/modules/assetnft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,94 @@ func TestAssetNFTIssueClassInvalidFeatures(t *testing.T) {
requireT.ErrorContains(err, "non-existing class feature provided")
}

// TestAssetNFTMintAndWhitelisting tests non-fungible token minting when whitelisting is required.
func TestAssetNFTMintAndWhitelisting(t *testing.T) {
t.Parallel()

ctx, chain := integrationtests.NewCoreumTestingContext(t)

requireT := require.New(t)
issuer := chain.GenAccount()
recipient := chain.GenAccount()

nftClient := nft.NewQueryClient(chain.ClientContext)
chain.FundAccountWithOptions(ctx, t, issuer, integration.BalancesOptions{
Messages: []sdk.Msg{
&assetnfttypes.MsgIssueClass{},
&assetnfttypes.MsgMint{},
&assetnfttypes.MsgAddToClassWhitelist{},
&assetnfttypes.MsgMint{},
},
Amount: chain.QueryAssetNFTParams(ctx, t).MintFee.Amount,
})

// issue new NFT class
issueMsg := &assetnfttypes.MsgIssueClass{
Issuer: issuer.String(),
Symbol: "NFTClassSymbol",
Features: []assetnfttypes.ClassFeature{
assetnfttypes.ClassFeature_whitelisting,
},
}
_, err := client.BroadcastTx(
ctx,
chain.ClientContext.WithFromAddress(issuer),
chain.TxFactory().WithGas(chain.GasLimitByMsgs(issueMsg)),
issueMsg,
)
requireT.NoError(err)

classID := assetnfttypes.BuildClassID(issueMsg.Symbol, issuer)

// mint new token in that class - should fail
mintMsg := &assetnfttypes.MsgMint{
Sender: issuer.String(),
Recipient: recipient.String(),
ID: "id-1",
ClassID: classID,
URI: "https://my-class-meta.invalid/1",
URIHash: "content-hash",
}
_, err = client.BroadcastTx(
ctx,
chain.ClientContext.WithFromAddress(issuer),
chain.TxFactory().WithGas(chain.GasLimitByMsgs(mintMsg)),
mintMsg,
)
requireT.ErrorIs(err, cosmoserrors.ErrUnauthorized)

// whitelist class
msgAddToWhitelist := &assetnfttypes.MsgAddToClassWhitelist{
Sender: issuer.String(),
ClassID: classID,
Account: recipient.String(),
}
_, err = client.BroadcastTx(
ctx,
chain.ClientContext.WithFromAddress(issuer),
chain.TxFactory().WithGas(chain.GasLimitByMsgs(msgAddToWhitelist)),
msgAddToWhitelist,
)
requireT.NoError(err)

// mint again - should succeed
_, err = client.BroadcastTx(
ctx,
chain.ClientContext.WithFromAddress(issuer),
chain.TxFactory().WithGas(chain.GasLimitByMsgs(mintMsg)),
mintMsg,
)
requireT.NoError(err)

// check the owner
ownerRes, err := nftClient.Owner(ctx, &nft.QueryOwnerRequest{
ClassId: classID,
Id: mintMsg.ID,
})
requireT.NoError(err)
requireT.Equal(recipient.String(), ownerRes.Owner)
}

// TestAssetNFTMint tests non-fungible token minting.
func TestAssetNFTMint(t *testing.T) {
t.Parallel()
Expand Down
14 changes: 0 additions & 14 deletions x/asset/nft/keeper/before_transfer.go

This file was deleted.

34 changes: 30 additions & 4 deletions x/asset/nft/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,16 @@ func (k Keeper) Mint(ctx sdk.Context, settings types.MintSettings) error {
return sdkerrors.Wrapf(cosmoserrors.ErrUnauthorized, "address %q is unauthorized to perform the mint operation", settings.Sender.String())
}

if definition.IsFeatureEnabled(types.ClassFeature_whitelisting) && !definition.IsIssuer(settings.Recipient) {
isWhitelisted, err := k.isClassWhitelisted(ctx, settings.ClassID, settings.Recipient)
if err != nil {
return err
}
if !isWhitelisted {
return sdkerrors.Wrapf(cosmoserrors.ErrUnauthorized, "due to enabled whitelisting only the issuer can receive minted NFT, %s is not the issuer", settings.Recipient.String())
}
}

if !k.nftKeeper.HasClass(ctx, settings.ClassID) {
return sdkerrors.Wrapf(types.ErrInvalidInput, "classID %q not found", settings.ClassID)
}
Expand Down Expand Up @@ -569,17 +579,33 @@ func (k Keeper) IsWhitelisted(ctx sdk.Context, classID, nftID string, account sd
return false, sdkerrors.Wrapf(types.ErrFeatureDisabled, `feature "whitelisting" is disabled`)
}

if !k.nftKeeper.HasNFT(ctx, classID, nftID) {
return false, sdkerrors.Wrapf(types.ErrNFTNotFound, "nft with classID:%s and ID:%s not found", classID, nftID)
isWhitelisted, err := k.isTokenWhitelisted(ctx, classID, nftID, account)
if err != nil {
return false, err
}
if isWhitelisted {
return true, nil
}

return k.isClassWhitelisted(ctx, classID, account)
}

func (k Keeper) isClassWhitelisted(ctx sdk.Context, classID string, account sdk.AccAddress) (bool, error) {
if !k.nftKeeper.HasClass(ctx, classID) {
return false, sdkerrors.Wrapf(types.ErrNFTNotFound, "nft class with classID:%s not found", classID)
}

classKey, err := types.CreateClassWhitelistingKey(classID, account)
if err != nil {
return false, err
}

if bytes.Equal(ctx.KVStore(k.storeKey).Get(classKey), types.StoreTrue) {
return true, nil
return bytes.Equal(ctx.KVStore(k.storeKey).Get(classKey), types.StoreTrue), nil
}

func (k Keeper) isTokenWhitelisted(ctx sdk.Context, classID, nftID string, account sdk.AccAddress) (bool, error) {
if !k.nftKeeper.HasNFT(ctx, classID, nftID) {
return false, sdkerrors.Wrapf(types.ErrNFTNotFound, "nft with classID:%s and ID:%s not found", classID, nftID)
}

key, err := types.CreateWhitelistingKey(classID, nftID, account)
Expand Down
50 changes: 49 additions & 1 deletion x/asset/nft/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func TestKeeper_MintWithRecipient(t *testing.T) {
URIHash: "content-hash",
}

// mint first NFT
// mint NFT
err = nftKeeper.Mint(ctx, settings)
requireT.NoError(err)

Expand All @@ -282,6 +282,54 @@ func TestKeeper_MintWithRecipient(t *testing.T) {
requireT.Equal(sdkmath.ZeroInt().String(), balance.Amount.String())
}

func TestKeeper_MintWithRecipientAndWhitelisting(t *testing.T) {
requireT := require.New(t)
testApp := simapp.New()
ctx := testApp.NewContext(false, tmproto.Header{})
nftKeeper := testApp.AssetNFTKeeper

nftParams := types.Params{
MintFee: sdk.NewInt64Coin(constant.DenomDev, 10_000_000),
}
requireT.NoError(nftKeeper.SetParams(ctx, nftParams))

addr := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address())
randomAddr := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address())
requireT.NoError(testApp.FundAccount(ctx, addr, sdk.NewCoins(nftParams.MintFee)))
classSettings := types.IssueClassSettings{
Issuer: addr,
Symbol: "symbol",
Features: []types.ClassFeature{
types.ClassFeature_whitelisting,
},
}

classID, err := nftKeeper.IssueClass(ctx, classSettings)
requireT.NoError(err)
requireT.EqualValues(classSettings.Symbol+"-"+addr.String(), classID)

settings := types.MintSettings{
Sender: addr,
Recipient: randomAddr,
ClassID: classID,
ID: "my-id",
URI: "https://my-nft-meta.invalid/1",
URIHash: "content-hash",
}

// mint NFT - should fail because recipient is not whitelisted
requireT.ErrorIs(nftKeeper.Mint(ctx, settings), cosmoserrors.ErrUnauthorized)

// whitelist for class
requireT.NoError(nftKeeper.AddToClassWhitelist(ctx, classID, addr, randomAddr))

// now minting should work
requireT.NoError(nftKeeper.Mint(ctx, settings))

// verify ownership
requireT.Equal(randomAddr, testApp.NFTKeeper.GetOwner(ctx, classID, settings.ID))
}

func TestKeeper_Burn(t *testing.T) {
requireT := require.New(t)
testApp := simapp.New()
Expand Down
22 changes: 22 additions & 0 deletions x/asset/nft/keeper/transfer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package keeper

import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

// Transfer wraps the original transfer function of the nft keeper to include our custom interceptor.
func (k Keeper) Transfer(ctx sdk.Context, classID, nftID string, receiver sdk.AccAddress) error {
if err := k.beforeTransfer(ctx, classID, nftID, receiver); err != nil {
return err
}

return k.nftKeeper.Transfer(ctx, classID, nftID, receiver)
}

func (k Keeper) beforeTransfer(ctx sdk.Context, classID, nftID string, receiver sdk.AccAddress) error {
if err := k.isNFTSendable(ctx, classID, nftID); err != nil {
return err
}

return k.isNFTReceivable(ctx, classID, nftID, receiver)
}
3 changes: 3 additions & 0 deletions x/asset/nft/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
)

// NFTKeeper defines the expected NFT interface.
//
//nolint:interfacebloat
type NFTKeeper interface {
SaveClass(ctx sdk.Context, class nft.Class) error
GetClass(ctx sdk.Context, classID string) (nft.Class, bool)
Expand All @@ -18,6 +20,7 @@ type NFTKeeper interface {
Burn(ctx sdk.Context, classID, nftID string) error
Update(ctx sdk.Context, n nft.NFT) error
GetOwner(ctx sdk.Context, classID, nftID string) sdk.AccAddress
Transfer(ctx sdk.Context, classID string, nftID string, receiver sdk.AccAddress) error
}

// BankKeeper defines the expected bank interface.
Expand Down
6 changes: 1 addition & 5 deletions x/wnft/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,5 @@ func (wk Wrapper) Send(goCtx context.Context, msg *nft.MsgSend) (*nft.MsgSendRes

// Transfer overwrites the original transfer function to include our custom interceptor.
func (wk Wrapper) Transfer(ctx sdk.Context, classID, nftID string, receiver sdk.AccAddress) error {
if err := wk.nonFungibleTokenProvider.BeforeTransfer(ctx, classID, nftID, receiver); err != nil {
return err
}

return wk.Keeper.Transfer(ctx, classID, nftID, receiver)
return wk.nonFungibleTokenProvider.Transfer(ctx, classID, nftID, receiver)
}
2 changes: 1 addition & 1 deletion x/wnft/types/exptected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ import sdk "github.com/cosmos/cosmos-sdk/types"

// NonFungibleTokenProvider defines the interface to intercept within nft method calls.
type NonFungibleTokenProvider interface {
BeforeTransfer(ctx sdk.Context, classID, nftID string, receiver sdk.AccAddress) error
Transfer(ctx sdk.Context, classID string, nftID string, receiver sdk.AccAddress) error
}

0 comments on commit 84503b3

Please sign in to comment.