From 9519370efdf237faaf3f103c575e96a44b5a6b9d Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Wed, 8 Jun 2022 18:49:51 +0800 Subject: [PATCH 1/8] add batch operation for x/nft module --- x/nft/keeper/nft_batch.go | 106 ++++++++++ x/nft/keeper/nft_batch_test.go | 363 +++++++++++++++++++++++++++++++++ 2 files changed, 469 insertions(+) create mode 100644 x/nft/keeper/nft_batch.go create mode 100644 x/nft/keeper/nft_batch_test.go diff --git a/x/nft/keeper/nft_batch.go b/x/nft/keeper/nft_batch.go new file mode 100644 index 000000000000..5eba4272c714 --- /dev/null +++ b/x/nft/keeper/nft_batch.go @@ -0,0 +1,106 @@ +package keeper + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/x/nft" +) + +// BatchMint defines a method for minting a batch of nfts +func (k Keeper) BatchMint(ctx sdk.Context, + tokens []nft.NFT, + receiver sdk.AccAddress, +) error { + classIDs := make(map[string]bool, len(tokens)) + for _, token := range tokens { + if !classIDs[token.ClassId] && !k.HasClass(ctx, token.ClassId) { + return sdkerrors.Wrap(nft.ErrClassNotExists, token.ClassId) + } + + if k.HasNFT(ctx, token.ClassId, token.Id) { + return sdkerrors.Wrap(nft.ErrNFTExists, token.Id) + } + + k.setNFT(ctx, token) + k.setOwner(ctx, token.ClassId, token.Id, receiver) + k.incrTotalSupply(ctx, token.ClassId) + + ctx.EventManager().EmitTypedEvent(&nft.EventMint{ + ClassId: token.ClassId, + Id: token.Id, + Owner: receiver.String(), + }) + classIDs[token.ClassId] = true + } + return nil +} + +// BatchBurn defines a method for burning a batch of nfts from a specific classID. +// Note: When the upper module uses this method, it needs to authenticate nft +func (k Keeper) BatchBurn(ctx sdk.Context, classID string, nftIDs []string) error { + if !k.HasClass(ctx, classID) { + return sdkerrors.Wrap(nft.ErrClassNotExists, classID) + } + + for _, nftID := range nftIDs { + if !k.HasNFT(ctx, classID, nftID) { + return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) + } + + owner := k.GetOwner(ctx, classID, nftID) + nftStore := k.getNFTStore(ctx, classID) + nftStore.Delete([]byte(nftID)) + + k.deleteOwner(ctx, classID, nftID, owner) + ctx.EventManager().EmitTypedEvent(&nft.EventBurn{ + ClassId: classID, + Id: nftID, + Owner: owner.String(), + }) + } + k.updateTotalSupply( + ctx, + classID, + k.GetTotalSupply(ctx, classID)-uint64(len(nftIDs)), + ) + return nil +} + +// BatchUpdate defines a method for updating a batch of exist nfts +// Note: When the upper module uses this method, it needs to authenticate nft +func (k Keeper) BatchUpdate(ctx sdk.Context, tokens []nft.NFT) error { + for _, token := range tokens { + if !k.HasClass(ctx, token.ClassId) { + return sdkerrors.Wrap(nft.ErrClassNotExists, token.ClassId) + } + + if !k.HasNFT(ctx, token.ClassId, token.Id) { + return sdkerrors.Wrap(nft.ErrNFTNotExists, token.Id) + } + k.setNFT(ctx, token) + } + return nil +} + +// BatchTransfer defines a method for sending a batch of nfts from one account to another account from a specific classID. +// Note: When the upper module uses this method, it needs to authenticate nft +func (k Keeper) BatchTransfer(ctx sdk.Context, + classID string, + nftIDs []string, + receiver sdk.AccAddress, +) error { + for _, nftID := range nftIDs { + if !k.HasClass(ctx, classID) { + return sdkerrors.Wrap(nft.ErrClassNotExists, classID) + } + + if !k.HasNFT(ctx, classID, nftID) { + return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) + } + + owner := k.GetOwner(ctx, classID, nftID) + k.deleteOwner(ctx, classID, nftID, owner) + k.setOwner(ctx, classID, nftID, receiver) + } + return nil +} diff --git a/x/nft/keeper/nft_batch_test.go b/x/nft/keeper/nft_batch_test.go new file mode 100644 index 000000000000..3fc4dc806233 --- /dev/null +++ b/x/nft/keeper/nft_batch_test.go @@ -0,0 +1,363 @@ +package keeper_test + +import ( + "fmt" + "math/rand" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/nft" +) + +func (s *TestSuite) TestBatchMint() { + receiver := s.addrs[0] + testCases := []struct { + msg string + malleate func([]nft.NFT) + tokens []nft.NFT + expPass bool + }{ + { + "success with empty nft", + func(tokens []nft.NFT) { + s.saveClass(tokens) + }, + []nft.NFT{}, + true, + }, + { + "success with single nft", + func(tokens []nft.NFT) { + s.saveClass(tokens) + }, + []nft.NFT{ + {ClassId: "classID1", Id: "nftID1"}, + }, + true, + }, + { + "success with multiple nft", + func(tokens []nft.NFT) { + s.saveClass(tokens) + }, + []nft.NFT{ + {ClassId: "classID1", Id: "nftID1"}, + {ClassId: "classID1", Id: "nftID2"}, + }, + true, + }, + { + "success with multiple class and multiple nft", + func(tokens []nft.NFT) { + s.saveClass(tokens) + }, + []nft.NFT{ + {ClassId: "classID1", Id: "nftID1"}, + {ClassId: "classID1", Id: "nftID2"}, + {ClassId: "classID2", Id: "nftID1"}, + {ClassId: "classID2", Id: "nftID2"}, + }, + true, + }, + { + "faild with repeated nft", + func(tokens []nft.NFT) { + s.saveClass(tokens) + }, + []nft.NFT{ + {ClassId: "classID1", Id: "nftID1"}, + {ClassId: "classID1", Id: "nftID1"}, + {ClassId: "classID2", Id: "nftID2"}, + }, + false, + }, + { + "faild with not exist class", + func(tokens []nft.NFT) { + //do nothing + }, + []nft.NFT{ + {ClassId: "classID1", Id: "nftID1"}, + {ClassId: "classID1", Id: "nftID1"}, + {ClassId: "classID2", Id: "nftID2"}, + }, + false, + }, + { + "faild with exist nft", + func(tokens []nft.NFT) { + s.saveClass(tokens) + idx := rand.Intn(len(tokens)) + s.app.NFTKeeper.Mint(s.ctx, tokens[idx], receiver) + }, + []nft.NFT{ + {ClassId: "classID1", Id: "nftID1"}, + {ClassId: "classID1", Id: "nftID2"}, + {ClassId: "classID2", Id: "nftID2"}, + }, + false, + }, + } + + for _, tc := range testCases { + s.Run(fmt.Sprintf("Case %s", tc.msg), func() { + s.SetupTest() // reset + tc.malleate(tc.tokens) + + err := s.app.NFTKeeper.BatchMint(s.ctx, tc.tokens, receiver) + if tc.expPass { + s.Require().NoError(err) + + classMap := groupByClassID(tc.tokens) + for classID, tokens := range classMap { + for _, token := range tokens { + actNFT, has := s.app.NFTKeeper.GetNFT(s.ctx, token.ClassId, token.Id) + s.Require().True(has) + s.Require().EqualValues(token, actNFT) + + owner := s.app.NFTKeeper.GetOwner(s.ctx, token.ClassId, token.Id) + s.Require().True(receiver.Equals(owner)) + } + + actNFTs := s.app.NFTKeeper.GetNFTsOfClass(s.ctx, classID) + s.Require().EqualValues(tokens, actNFTs) + + actNFTs = s.app.NFTKeeper.GetNFTsOfClassByOwner(s.ctx, classID, receiver) + s.Require().EqualValues(tokens, actNFTs) + + balance := s.app.NFTKeeper.GetBalance(s.ctx, classID, receiver) + s.Require().EqualValues(len(tokens), balance) + + supply := s.app.NFTKeeper.GetTotalSupply(s.ctx, classID) + s.Require().EqualValues(len(tokens), supply) + } + return + } + s.Require().Error(err) + }) + } +} + +func (s *TestSuite) TestBatchBurn() { + receiver := s.addrs[0] + tokens := []nft.NFT{ + {ClassId: "classID1", Id: "nftID1"}, + {ClassId: "classID1", Id: "nftID2"}, + {ClassId: "classID2", Id: "nftID1"}, + {ClassId: "classID2", Id: "nftID2"}, + } + + testCases := []struct { + msg string + malleate func() + classID string + nftIDs []string + expPass bool + }{ + { + "success", + func() { + s.saveClass(tokens) + s.app.NFTKeeper.BatchMint(s.ctx, tokens, receiver) + }, + "classID1", + []string{"nftID1", "nftID2"}, + true, + }, + { + "failed with not exist classID", + func() {}, + "classID1", + []string{"nftID1", "nftID2"}, + false, + }, + { + "failed with not exist nftID", + func() { + s.saveClass(tokens) + }, + "classID1", + []string{"nftID1", "nftID2"}, + false, + }, + } + + for _, tc := range testCases { + s.Run(fmt.Sprintf("Case %s", tc.msg), func() { + s.SetupTest() // reset + tc.malleate() + + err := s.app.NFTKeeper.BatchBurn(s.ctx, tc.classID, tc.nftIDs) + if tc.expPass { + s.Require().NoError(err) + for _, nftID := range tc.nftIDs { + s.Require().False(s.app.NFTKeeper.HasNFT(s.ctx, tc.classID, nftID)) + } + return + } + s.Require().Error(err) + }) + } + +} + +func (s *TestSuite) TestBatchUpdate() { + receiver := s.addrs[0] + tokens := []nft.NFT{ + {ClassId: "classID1", Id: "nftID1"}, + {ClassId: "classID1", Id: "nftID2"}, + {ClassId: "classID2", Id: "nftID1"}, + {ClassId: "classID2", Id: "nftID2"}, + } + testCases := []struct { + msg string + malleate func() + tokens []nft.NFT + expPass bool + }{ + { + "success", + func() { + s.saveClass(tokens) + s.app.NFTKeeper.BatchMint(s.ctx, tokens, receiver) + }, + []nft.NFT{ + {ClassId: "classID1", Id: "nftID1", Uri: "nftID1_URI"}, + {ClassId: "classID2", Id: "nftID2", Uri: "nftID2_URI"}, + }, + true, + }, + { + "failed with not exist classID", + func() {}, + []nft.NFT{ + {ClassId: "classID1", Id: "nftID1", Uri: "nftID1_URI"}, + {ClassId: "classID2", Id: "nftID2", Uri: "nftID2_URI"}, + }, + false, + }, + { + "failed with not exist nftID", + func() { + s.saveClass(tokens) + }, + []nft.NFT{ + {ClassId: "classID1", Id: "nftID1", Uri: "nftID1_URI"}, + {ClassId: "classID2", Id: "nftID2", Uri: "nftID2_URI"}, + }, + false, + }, + } + + for _, tc := range testCases { + s.Run(fmt.Sprintf("Case %s", tc.msg), func() { + s.SetupTest() // reset + tc.malleate() + + err := s.app.NFTKeeper.BatchUpdate(s.ctx, tc.tokens) + if tc.expPass { + s.Require().NoError(err) + for _, token := range tc.tokens { + actToken, found := s.app.NFTKeeper.GetNFT(s.ctx, token.ClassId, token.Id) + s.Require().True(found) + s.Require().EqualValues(token, actToken) + } + return + } + s.Require().Error(err) + }) + } + +} + +func (s *TestSuite) TestBatchTransfer() { + owner := s.addrs[0] + receiver := s.addrs[1] + tokens := []nft.NFT{ + {ClassId: "classID1", Id: "nftID1"}, + {ClassId: "classID1", Id: "nftID2"}, + {ClassId: "classID2", Id: "nftID1"}, + {ClassId: "classID2", Id: "nftID2"}, + } + testCases := []struct { + msg string + malleate func() + classID string + nftIDs []string + expPass bool + }{ + { + "success", + func() { + s.saveClass(tokens) + s.app.NFTKeeper.BatchMint(s.ctx, tokens, owner) + }, + "classID1", + []string{"nftID1", "nftID2"}, + true, + }, + { + "failed with not exist classID", + func() { + s.saveClass(tokens) + s.app.NFTKeeper.BatchMint(s.ctx, tokens, receiver) + }, + "classID3", + []string{"nftID1", "nftID2"}, + false, + }, + { + "failed with not exist nftID", + func() { + s.saveClass(tokens) + }, + "classID1", + []string{"nftID1", "nftID2"}, + false, + }, + } + + for _, tc := range testCases { + s.Run(fmt.Sprintf("Case %s", tc.msg), func() { + s.SetupTest() // reset + tc.malleate() + + err := s.app.NFTKeeper.BatchTransfer(s.ctx, tc.classID, tc.nftIDs, receiver) + if tc.expPass { + s.Require().NoError(err) + for _, nftID := range tc.nftIDs { + actOwner := s.app.NFTKeeper.GetOwner(s.ctx, tc.classID, nftID) + s.Require().EqualValues(receiver, actOwner) + } + return + } + s.Require().Error(err) + }) + } + +} + +func groupByClassID(tokens []nft.NFT) map[string][]nft.NFT { + classMap := make(map[string][]nft.NFT, len(tokens)) + for _, token := range tokens { + if _, ok := classMap[token.ClassId]; !ok { + classMap[token.ClassId] = make([]nft.NFT, 0) + } + classMap[token.ClassId] = append(classMap[token.ClassId], token) + } + return classMap +} + +func (s *TestSuite) saveClass(tokens []nft.NFT) { + classMap := groupByClassID(tokens) + for classID := range classMap { + err := s.app.NFTKeeper.SaveClass(s.ctx, nft.Class{Id: classID}) + s.Require().NoError(err) + } +} + +func (s *TestSuite) mintNFT(tokens []nft.NFT, receiver sdk.AccAddress) { + for _, token := range tokens { + err := s.app.NFTKeeper.Mint(s.ctx, token, receiver) + s.Require().NoError(err) + } +} From cc5fc3d1545ef51a025830bf418def8bc1bbfe9c Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Wed, 8 Jun 2022 20:08:27 +0800 Subject: [PATCH 2/8] refactor code --- x/nft/keeper/nft_batch.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/x/nft/keeper/nft_batch.go b/x/nft/keeper/nft_batch.go index 5eba4272c714..8cce72affca8 100644 --- a/x/nft/keeper/nft_batch.go +++ b/x/nft/keeper/nft_batch.go @@ -69,8 +69,9 @@ func (k Keeper) BatchBurn(ctx sdk.Context, classID string, nftIDs []string) erro // BatchUpdate defines a method for updating a batch of exist nfts // Note: When the upper module uses this method, it needs to authenticate nft func (k Keeper) BatchUpdate(ctx sdk.Context, tokens []nft.NFT) error { + classIDs := make(map[string]bool, len(tokens)) for _, token := range tokens { - if !k.HasClass(ctx, token.ClassId) { + if !classIDs[token.ClassId] && !k.HasClass(ctx, token.ClassId) { return sdkerrors.Wrap(nft.ErrClassNotExists, token.ClassId) } @@ -89,11 +90,11 @@ func (k Keeper) BatchTransfer(ctx sdk.Context, nftIDs []string, receiver sdk.AccAddress, ) error { - for _, nftID := range nftIDs { - if !k.HasClass(ctx, classID) { - return sdkerrors.Wrap(nft.ErrClassNotExists, classID) - } + if !k.HasClass(ctx, classID) { + return sdkerrors.Wrap(nft.ErrClassNotExists, classID) + } + for _, nftID := range nftIDs { if !k.HasNFT(ctx, classID, nftID) { return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) } From b9065127c4ad3161e747960ca9b6b7e6c624fd5d Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Sun, 12 Jun 2022 09:33:14 +0800 Subject: [PATCH 3/8] apply comment from github --- x/nft/keeper/nft_batch.go | 60 +++++---------------------------------- 1 file changed, 7 insertions(+), 53 deletions(-) diff --git a/x/nft/keeper/nft_batch.go b/x/nft/keeper/nft_batch.go index 8cce72affca8..0ed58efa8a29 100644 --- a/x/nft/keeper/nft_batch.go +++ b/x/nft/keeper/nft_batch.go @@ -11,26 +11,10 @@ func (k Keeper) BatchMint(ctx sdk.Context, tokens []nft.NFT, receiver sdk.AccAddress, ) error { - classIDs := make(map[string]bool, len(tokens)) for _, token := range tokens { - if !classIDs[token.ClassId] && !k.HasClass(ctx, token.ClassId) { - return sdkerrors.Wrap(nft.ErrClassNotExists, token.ClassId) + if err := k.Mint(ctx, token, receiver); err != nil { + return err } - - if k.HasNFT(ctx, token.ClassId, token.Id) { - return sdkerrors.Wrap(nft.ErrNFTExists, token.Id) - } - - k.setNFT(ctx, token) - k.setOwner(ctx, token.ClassId, token.Id, receiver) - k.incrTotalSupply(ctx, token.ClassId) - - ctx.EventManager().EmitTypedEvent(&nft.EventMint{ - ClassId: token.ClassId, - Id: token.Id, - Owner: receiver.String(), - }) - classIDs[token.ClassId] = true } return nil } @@ -43,42 +27,20 @@ func (k Keeper) BatchBurn(ctx sdk.Context, classID string, nftIDs []string) erro } for _, nftID := range nftIDs { - if !k.HasNFT(ctx, classID, nftID) { - return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) + if err := k.Burn(ctx, classID, nftID); err != nil { + return err } - - owner := k.GetOwner(ctx, classID, nftID) - nftStore := k.getNFTStore(ctx, classID) - nftStore.Delete([]byte(nftID)) - - k.deleteOwner(ctx, classID, nftID, owner) - ctx.EventManager().EmitTypedEvent(&nft.EventBurn{ - ClassId: classID, - Id: nftID, - Owner: owner.String(), - }) } - k.updateTotalSupply( - ctx, - classID, - k.GetTotalSupply(ctx, classID)-uint64(len(nftIDs)), - ) return nil } // BatchUpdate defines a method for updating a batch of exist nfts // Note: When the upper module uses this method, it needs to authenticate nft func (k Keeper) BatchUpdate(ctx sdk.Context, tokens []nft.NFT) error { - classIDs := make(map[string]bool, len(tokens)) for _, token := range tokens { - if !classIDs[token.ClassId] && !k.HasClass(ctx, token.ClassId) { - return sdkerrors.Wrap(nft.ErrClassNotExists, token.ClassId) + if err := k.Update(ctx, token); err != nil { + return err } - - if !k.HasNFT(ctx, token.ClassId, token.Id) { - return sdkerrors.Wrap(nft.ErrNFTNotExists, token.Id) - } - k.setNFT(ctx, token) } return nil } @@ -90,18 +52,10 @@ func (k Keeper) BatchTransfer(ctx sdk.Context, nftIDs []string, receiver sdk.AccAddress, ) error { - if !k.HasClass(ctx, classID) { - return sdkerrors.Wrap(nft.ErrClassNotExists, classID) - } - for _, nftID := range nftIDs { - if !k.HasNFT(ctx, classID, nftID) { + if err := k.Transfer(ctx, classID, nftID, receiver); err != nil { return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) } - - owner := k.GetOwner(ctx, classID, nftID) - k.deleteOwner(ctx, classID, nftID, owner) - k.setOwner(ctx, classID, nftID, receiver) } return nil } From a5286d394cc2d3ee0d43f5cfe4b108140491ebf5 Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Sun, 12 Jun 2022 09:34:57 +0800 Subject: [PATCH 4/8] Remove redundant checksums --- x/nft/keeper/nft_batch.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/x/nft/keeper/nft_batch.go b/x/nft/keeper/nft_batch.go index 0ed58efa8a29..102277cf1392 100644 --- a/x/nft/keeper/nft_batch.go +++ b/x/nft/keeper/nft_batch.go @@ -22,10 +22,6 @@ func (k Keeper) BatchMint(ctx sdk.Context, // BatchBurn defines a method for burning a batch of nfts from a specific classID. // Note: When the upper module uses this method, it needs to authenticate nft func (k Keeper) BatchBurn(ctx sdk.Context, classID string, nftIDs []string) error { - if !k.HasClass(ctx, classID) { - return sdkerrors.Wrap(nft.ErrClassNotExists, classID) - } - for _, nftID := range nftIDs { if err := k.Burn(ctx, classID, nftID); err != nil { return err From d114913884cf0755b9abec074c5aca93fc7af8f5 Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Mon, 13 Jun 2022 22:43:23 +0800 Subject: [PATCH 5/8] add noCheck method --- x/nft/keeper/nft.go | 38 ++++++++++++++++++++++++++++++++++++-- x/nft/keeper/nft_batch.go | 39 +++++++++++++++++++++++++++++++++------ 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/x/nft/keeper/nft.go b/x/nft/keeper/nft.go index 336aff41b677..9f453a6a6c14 100644 --- a/x/nft/keeper/nft.go +++ b/x/nft/keeper/nft.go @@ -17,6 +17,14 @@ func (k Keeper) Mint(ctx sdk.Context, token nft.NFT, receiver sdk.AccAddress) er return sdkerrors.Wrap(nft.ErrNFTExists, token.Id) } + k.MintWithNoCheck(ctx, token, receiver) + return nil +} + +// MintWithNoCheck defines a method for minting a new nft +// Note: this method does not check whether the class already exists in nft. +// The upper-layer application needs to check it when it needs to use it. +func (k Keeper) MintWithNoCheck(ctx sdk.Context, token nft.NFT, receiver sdk.AccAddress) { k.setNFT(ctx, token) k.setOwner(ctx, token.ClassId, token.Id, receiver) k.incrTotalSupply(ctx, token.ClassId) @@ -26,7 +34,6 @@ func (k Keeper) Mint(ctx sdk.Context, token nft.NFT, receiver sdk.AccAddress) er Id: token.Id, Owner: receiver.String(), }) - return nil } // Burn defines a method for burning a nft from a specific account. @@ -40,6 +47,14 @@ func (k Keeper) Burn(ctx sdk.Context, classID string, nftID string) error { return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) } + k.BurnWithNoCheck(ctx, classID, nftID) + return nil +} + +// BurnWithNoCheck defines a method for burning a nft from a specific account. +// Note: this method does not check whether the class already exists in nft. +// The upper-layer application needs to check it when it needs to use it +func (k Keeper) BurnWithNoCheck(ctx sdk.Context, classID string, nftID string) error { owner := k.GetOwner(ctx, classID, nftID) nftStore := k.getNFTStore(ctx, classID) nftStore.Delete([]byte(nftID)) @@ -64,10 +79,17 @@ func (k Keeper) Update(ctx sdk.Context, token nft.NFT) error { if !k.HasNFT(ctx, token.ClassId, token.Id) { return sdkerrors.Wrap(nft.ErrNFTNotExists, token.Id) } - k.setNFT(ctx, token) + k.UpdateWithNoCheck(ctx, token) return nil } +// Update defines a method for updating an exist nft +// Note: this method does not check whether the class already exists in nft. +// The upper-layer application needs to check it when it needs to use it +func (k Keeper) UpdateWithNoCheck(ctx sdk.Context, token nft.NFT) { + k.setNFT(ctx, token) +} + // Transfer defines a method for sending a nft from one account to another account. // Note: When the upper module uses this method, it needs to authenticate nft func (k Keeper) Transfer(ctx sdk.Context, @@ -83,6 +105,18 @@ func (k Keeper) Transfer(ctx sdk.Context, return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) } + k.TransferWithNoCheck(ctx, classID, nftID, receiver) + return nil +} + +// Transfer defines a method for sending a nft from one account to another account. +// Note: this method does not check whether the class already exists in nft. +// The upper-layer application needs to check it when it needs to use it +func (k Keeper) TransferWithNoCheck(ctx sdk.Context, + classID string, + nftID string, + receiver sdk.AccAddress, +) error { owner := k.GetOwner(ctx, classID, nftID) k.deleteOwner(ctx, classID, nftID, owner) k.setOwner(ctx, classID, nftID, receiver) diff --git a/x/nft/keeper/nft_batch.go b/x/nft/keeper/nft_batch.go index 102277cf1392..4c312b3e48a5 100644 --- a/x/nft/keeper/nft_batch.go +++ b/x/nft/keeper/nft_batch.go @@ -11,10 +11,18 @@ func (k Keeper) BatchMint(ctx sdk.Context, tokens []nft.NFT, receiver sdk.AccAddress, ) error { + checked := make(map[string]bool, len(tokens)) for _, token := range tokens { - if err := k.Mint(ctx, token, receiver); err != nil { - return err + if !checked[token.ClassId] && !k.HasClass(ctx, token.ClassId) { + return sdkerrors.Wrap(nft.ErrClassNotExists, token.ClassId) + } + + if k.HasNFT(ctx, token.ClassId, token.Id) { + return sdkerrors.Wrap(nft.ErrNFTExists, token.Id) } + + checked[token.ClassId] = true + k.MintWithNoCheck(ctx, token, receiver) } return nil } @@ -22,8 +30,14 @@ func (k Keeper) BatchMint(ctx sdk.Context, // BatchBurn defines a method for burning a batch of nfts from a specific classID. // Note: When the upper module uses this method, it needs to authenticate nft func (k Keeper) BatchBurn(ctx sdk.Context, classID string, nftIDs []string) error { + if !k.HasClass(ctx, classID) { + return sdkerrors.Wrap(nft.ErrClassNotExists, classID) + } for _, nftID := range nftIDs { - if err := k.Burn(ctx, classID, nftID); err != nil { + if !k.HasNFT(ctx, classID, nftID) { + return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) + } + if err := k.BurnWithNoCheck(ctx, classID, nftID); err != nil { return err } } @@ -33,10 +47,17 @@ func (k Keeper) BatchBurn(ctx sdk.Context, classID string, nftIDs []string) erro // BatchUpdate defines a method for updating a batch of exist nfts // Note: When the upper module uses this method, it needs to authenticate nft func (k Keeper) BatchUpdate(ctx sdk.Context, tokens []nft.NFT) error { + checked := make(map[string]bool, len(tokens)) for _, token := range tokens { - if err := k.Update(ctx, token); err != nil { - return err + if !checked[token.ClassId] && !k.HasClass(ctx, token.ClassId) { + return sdkerrors.Wrap(nft.ErrClassNotExists, token.ClassId) + } + + if !k.HasNFT(ctx, token.ClassId, token.Id) { + return sdkerrors.Wrap(nft.ErrNFTNotExists, token.Id) } + checked[token.ClassId] = true + k.UpdateWithNoCheck(ctx, token) } return nil } @@ -48,8 +69,14 @@ func (k Keeper) BatchTransfer(ctx sdk.Context, nftIDs []string, receiver sdk.AccAddress, ) error { + if !k.HasClass(ctx, classID) { + return sdkerrors.Wrap(nft.ErrClassNotExists, classID) + } for _, nftID := range nftIDs { - if err := k.Transfer(ctx, classID, nftID, receiver); err != nil { + if !k.HasNFT(ctx, classID, nftID) { + return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) + } + if err := k.TransferWithNoCheck(ctx, classID, nftID, receiver); err != nil { return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) } } From 953275ebbe88ab26dbf2dc0e33f562283a14174b Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Tue, 14 Jun 2022 09:09:35 +0800 Subject: [PATCH 6/8] private noCheck method --- x/nft/keeper/nft.go | 20 ++++++++++---------- x/nft/keeper/nft_batch.go | 8 ++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/x/nft/keeper/nft.go b/x/nft/keeper/nft.go index 9f453a6a6c14..c4b6e58bb43c 100644 --- a/x/nft/keeper/nft.go +++ b/x/nft/keeper/nft.go @@ -17,14 +17,14 @@ func (k Keeper) Mint(ctx sdk.Context, token nft.NFT, receiver sdk.AccAddress) er return sdkerrors.Wrap(nft.ErrNFTExists, token.Id) } - k.MintWithNoCheck(ctx, token, receiver) + k.mintWithNoCheck(ctx, token, receiver) return nil } -// MintWithNoCheck defines a method for minting a new nft +// mintWithNoCheck defines a method for minting a new nft // Note: this method does not check whether the class already exists in nft. // The upper-layer application needs to check it when it needs to use it. -func (k Keeper) MintWithNoCheck(ctx sdk.Context, token nft.NFT, receiver sdk.AccAddress) { +func (k Keeper) mintWithNoCheck(ctx sdk.Context, token nft.NFT, receiver sdk.AccAddress) { k.setNFT(ctx, token) k.setOwner(ctx, token.ClassId, token.Id, receiver) k.incrTotalSupply(ctx, token.ClassId) @@ -47,14 +47,14 @@ func (k Keeper) Burn(ctx sdk.Context, classID string, nftID string) error { return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) } - k.BurnWithNoCheck(ctx, classID, nftID) + k.burnWithNoCheck(ctx, classID, nftID) return nil } -// BurnWithNoCheck defines a method for burning a nft from a specific account. +// burnWithNoCheck defines a method for burning a nft from a specific account. // Note: this method does not check whether the class already exists in nft. // The upper-layer application needs to check it when it needs to use it -func (k Keeper) BurnWithNoCheck(ctx sdk.Context, classID string, nftID string) error { +func (k Keeper) burnWithNoCheck(ctx sdk.Context, classID string, nftID string) error { owner := k.GetOwner(ctx, classID, nftID) nftStore := k.getNFTStore(ctx, classID) nftStore.Delete([]byte(nftID)) @@ -79,14 +79,14 @@ func (k Keeper) Update(ctx sdk.Context, token nft.NFT) error { if !k.HasNFT(ctx, token.ClassId, token.Id) { return sdkerrors.Wrap(nft.ErrNFTNotExists, token.Id) } - k.UpdateWithNoCheck(ctx, token) + k.updateWithNoCheck(ctx, token) return nil } // Update defines a method for updating an exist nft // Note: this method does not check whether the class already exists in nft. // The upper-layer application needs to check it when it needs to use it -func (k Keeper) UpdateWithNoCheck(ctx sdk.Context, token nft.NFT) { +func (k Keeper) updateWithNoCheck(ctx sdk.Context, token nft.NFT) { k.setNFT(ctx, token) } @@ -105,14 +105,14 @@ func (k Keeper) Transfer(ctx sdk.Context, return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) } - k.TransferWithNoCheck(ctx, classID, nftID, receiver) + k.transferWithNoCheck(ctx, classID, nftID, receiver) return nil } // Transfer defines a method for sending a nft from one account to another account. // Note: this method does not check whether the class already exists in nft. // The upper-layer application needs to check it when it needs to use it -func (k Keeper) TransferWithNoCheck(ctx sdk.Context, +func (k Keeper) transferWithNoCheck(ctx sdk.Context, classID string, nftID string, receiver sdk.AccAddress, diff --git a/x/nft/keeper/nft_batch.go b/x/nft/keeper/nft_batch.go index 4c312b3e48a5..936d2074e28f 100644 --- a/x/nft/keeper/nft_batch.go +++ b/x/nft/keeper/nft_batch.go @@ -22,7 +22,7 @@ func (k Keeper) BatchMint(ctx sdk.Context, } checked[token.ClassId] = true - k.MintWithNoCheck(ctx, token, receiver) + k.mintWithNoCheck(ctx, token, receiver) } return nil } @@ -37,7 +37,7 @@ func (k Keeper) BatchBurn(ctx sdk.Context, classID string, nftIDs []string) erro if !k.HasNFT(ctx, classID, nftID) { return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) } - if err := k.BurnWithNoCheck(ctx, classID, nftID); err != nil { + if err := k.burnWithNoCheck(ctx, classID, nftID); err != nil { return err } } @@ -57,7 +57,7 @@ func (k Keeper) BatchUpdate(ctx sdk.Context, tokens []nft.NFT) error { return sdkerrors.Wrap(nft.ErrNFTNotExists, token.Id) } checked[token.ClassId] = true - k.UpdateWithNoCheck(ctx, token) + k.updateWithNoCheck(ctx, token) } return nil } @@ -76,7 +76,7 @@ func (k Keeper) BatchTransfer(ctx sdk.Context, if !k.HasNFT(ctx, classID, nftID) { return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) } - if err := k.TransferWithNoCheck(ctx, classID, nftID, receiver); err != nil { + if err := k.transferWithNoCheck(ctx, classID, nftID, receiver); err != nil { return sdkerrors.Wrap(nft.ErrNFTNotExists, nftID) } } From 9fa5c2e913c8f1ded876ce3fc4a29c8d67457bb5 Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Tue, 14 Jun 2022 17:23:56 +0800 Subject: [PATCH 7/8] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 968f73d40b1e..614c1d98d95f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#12089](https://github.com/cosmos/cosmos-sdk/pull/12089) Mark the `TipDecorator` as beta, don't include it in simapp by default. * [#12153](https://github.com/cosmos/cosmos-sdk/pull/12153) Add a new `NewSimulationManagerFromAppModules` constructor, to simplify simulation wiring. +* [#12187](https://github.com/cosmos/cosmos-sdk/pull/12187) Add batch operation for x/nft module. ### API Breaking Changes From dd666e82f2606c164dece7b39c0e58bc62a7b4b5 Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Wed, 15 Jun 2022 08:41:24 +0800 Subject: [PATCH 8/8] fix test error --- x/nft/keeper/nft_batch_test.go | 40 +++++++++++++++++----------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/x/nft/keeper/nft_batch_test.go b/x/nft/keeper/nft_batch_test.go index 3fc4dc806233..d9561106e5ee 100644 --- a/x/nft/keeper/nft_batch_test.go +++ b/x/nft/keeper/nft_batch_test.go @@ -87,7 +87,7 @@ func (s *TestSuite) TestBatchMint() { func(tokens []nft.NFT) { s.saveClass(tokens) idx := rand.Intn(len(tokens)) - s.app.NFTKeeper.Mint(s.ctx, tokens[idx], receiver) + s.nftKeeper.Mint(s.ctx, tokens[idx], receiver) }, []nft.NFT{ {ClassId: "classID1", Id: "nftID1"}, @@ -103,31 +103,31 @@ func (s *TestSuite) TestBatchMint() { s.SetupTest() // reset tc.malleate(tc.tokens) - err := s.app.NFTKeeper.BatchMint(s.ctx, tc.tokens, receiver) + err := s.nftKeeper.BatchMint(s.ctx, tc.tokens, receiver) if tc.expPass { s.Require().NoError(err) classMap := groupByClassID(tc.tokens) for classID, tokens := range classMap { for _, token := range tokens { - actNFT, has := s.app.NFTKeeper.GetNFT(s.ctx, token.ClassId, token.Id) + actNFT, has := s.nftKeeper.GetNFT(s.ctx, token.ClassId, token.Id) s.Require().True(has) s.Require().EqualValues(token, actNFT) - owner := s.app.NFTKeeper.GetOwner(s.ctx, token.ClassId, token.Id) + owner := s.nftKeeper.GetOwner(s.ctx, token.ClassId, token.Id) s.Require().True(receiver.Equals(owner)) } - actNFTs := s.app.NFTKeeper.GetNFTsOfClass(s.ctx, classID) + actNFTs := s.nftKeeper.GetNFTsOfClass(s.ctx, classID) s.Require().EqualValues(tokens, actNFTs) - actNFTs = s.app.NFTKeeper.GetNFTsOfClassByOwner(s.ctx, classID, receiver) + actNFTs = s.nftKeeper.GetNFTsOfClassByOwner(s.ctx, classID, receiver) s.Require().EqualValues(tokens, actNFTs) - balance := s.app.NFTKeeper.GetBalance(s.ctx, classID, receiver) + balance := s.nftKeeper.GetBalance(s.ctx, classID, receiver) s.Require().EqualValues(len(tokens), balance) - supply := s.app.NFTKeeper.GetTotalSupply(s.ctx, classID) + supply := s.nftKeeper.GetTotalSupply(s.ctx, classID) s.Require().EqualValues(len(tokens), supply) } return @@ -157,7 +157,7 @@ func (s *TestSuite) TestBatchBurn() { "success", func() { s.saveClass(tokens) - s.app.NFTKeeper.BatchMint(s.ctx, tokens, receiver) + s.nftKeeper.BatchMint(s.ctx, tokens, receiver) }, "classID1", []string{"nftID1", "nftID2"}, @@ -186,11 +186,11 @@ func (s *TestSuite) TestBatchBurn() { s.SetupTest() // reset tc.malleate() - err := s.app.NFTKeeper.BatchBurn(s.ctx, tc.classID, tc.nftIDs) + err := s.nftKeeper.BatchBurn(s.ctx, tc.classID, tc.nftIDs) if tc.expPass { s.Require().NoError(err) for _, nftID := range tc.nftIDs { - s.Require().False(s.app.NFTKeeper.HasNFT(s.ctx, tc.classID, nftID)) + s.Require().False(s.nftKeeper.HasNFT(s.ctx, tc.classID, nftID)) } return } @@ -218,7 +218,7 @@ func (s *TestSuite) TestBatchUpdate() { "success", func() { s.saveClass(tokens) - s.app.NFTKeeper.BatchMint(s.ctx, tokens, receiver) + s.nftKeeper.BatchMint(s.ctx, tokens, receiver) }, []nft.NFT{ {ClassId: "classID1", Id: "nftID1", Uri: "nftID1_URI"}, @@ -253,11 +253,11 @@ func (s *TestSuite) TestBatchUpdate() { s.SetupTest() // reset tc.malleate() - err := s.app.NFTKeeper.BatchUpdate(s.ctx, tc.tokens) + err := s.nftKeeper.BatchUpdate(s.ctx, tc.tokens) if tc.expPass { s.Require().NoError(err) for _, token := range tc.tokens { - actToken, found := s.app.NFTKeeper.GetNFT(s.ctx, token.ClassId, token.Id) + actToken, found := s.nftKeeper.GetNFT(s.ctx, token.ClassId, token.Id) s.Require().True(found) s.Require().EqualValues(token, actToken) } @@ -289,7 +289,7 @@ func (s *TestSuite) TestBatchTransfer() { "success", func() { s.saveClass(tokens) - s.app.NFTKeeper.BatchMint(s.ctx, tokens, owner) + s.nftKeeper.BatchMint(s.ctx, tokens, owner) }, "classID1", []string{"nftID1", "nftID2"}, @@ -299,7 +299,7 @@ func (s *TestSuite) TestBatchTransfer() { "failed with not exist classID", func() { s.saveClass(tokens) - s.app.NFTKeeper.BatchMint(s.ctx, tokens, receiver) + s.nftKeeper.BatchMint(s.ctx, tokens, receiver) }, "classID3", []string{"nftID1", "nftID2"}, @@ -321,11 +321,11 @@ func (s *TestSuite) TestBatchTransfer() { s.SetupTest() // reset tc.malleate() - err := s.app.NFTKeeper.BatchTransfer(s.ctx, tc.classID, tc.nftIDs, receiver) + err := s.nftKeeper.BatchTransfer(s.ctx, tc.classID, tc.nftIDs, receiver) if tc.expPass { s.Require().NoError(err) for _, nftID := range tc.nftIDs { - actOwner := s.app.NFTKeeper.GetOwner(s.ctx, tc.classID, nftID) + actOwner := s.nftKeeper.GetOwner(s.ctx, tc.classID, nftID) s.Require().EqualValues(receiver, actOwner) } return @@ -350,14 +350,14 @@ func groupByClassID(tokens []nft.NFT) map[string][]nft.NFT { func (s *TestSuite) saveClass(tokens []nft.NFT) { classMap := groupByClassID(tokens) for classID := range classMap { - err := s.app.NFTKeeper.SaveClass(s.ctx, nft.Class{Id: classID}) + err := s.nftKeeper.SaveClass(s.ctx, nft.Class{Id: classID}) s.Require().NoError(err) } } func (s *TestSuite) mintNFT(tokens []nft.NFT, receiver sdk.AccAddress) { for _, token := range tokens { - err := s.app.NFTKeeper.Mint(s.ctx, token, receiver) + err := s.nftKeeper.Mint(s.ctx, token, receiver) s.Require().NoError(err) } }