Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add NFT functionality to Cosmos SDK #23279

Closed
wants to merge 1 commit into from
Closed

Add NFT functionality to Cosmos SDK #23279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions x/nft/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,75 @@ func NewKeeper(env appmodule.Environment,
ac: ak.AddressCodec(),
}
}

// MsgNewClass creates a new NFT class
func (k Keeper) MsgNewClass(ctx context.Context, msg *nft.MsgNewClass) (*nft.MsgNewClassResponse, error) {
class := nft.Class{
Id: msg.ClassId,
Name: msg.Name,
Symbol: msg.Symbol,
Description: msg.Description,
Uri: msg.Uri,
UriHash: msg.UriHash,
Data: msg.Data,
}
if err := k.SaveClass(ctx, class); err != nil {
return nil, err
}
return &nft.MsgNewClassResponse{}, nil
}
Comment on lines +37 to +52
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation and authorization checks for class creation.

The function needs the following improvements:

  1. Add input validation for required fields (ClassId, Name, Symbol).
  2. Add authorization check to control who can create NFT classes.
  3. Emit an event when a class is created for better observability.

Apply this diff to add the suggested improvements:

 func (k Keeper) MsgNewClass(ctx context.Context, msg *nft.MsgNewClass) (*nft.MsgNewClassResponse, error) {
+    if msg.ClassId == "" {
+        return nil, nft.ErrEmptyClassID
+    }
+    if msg.Name == "" {
+        return nil, nft.ErrEmptyClassName
+    }
+    if msg.Symbol == "" {
+        return nil, nft.ErrEmptyClassSymbol
+    }
+
+    sender, err := k.ac.StringToBytes(msg.Sender)
+    if err != nil {
+        return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid sender address (%s)", msg.Sender)
+    }
+
     class := nft.Class{
         Id:          msg.ClassId,
         Name:        msg.Name,
         Symbol:      msg.Symbol,
         Description: msg.Description,
         Uri:         msg.Uri,
         UriHash:     msg.UriHash,
         Data:        msg.Data,
     }
     if err := k.SaveClass(ctx, class); err != nil {
         return nil, err
     }
+
+    if err = k.EventService.EventManager(ctx).Emit(&nft.EventNewClass{
+        ClassId:     msg.ClassId,
+        Sender:      msg.Sender,
+    }); err != nil {
+        return nil, err
+    }
+
     return &nft.MsgNewClassResponse{}, nil
 }

Committable suggestion skipped: line range outside the PR's diff.


// MsgUpdateClass updates an existing NFT class
func (k Keeper) MsgUpdateClass(ctx context.Context, msg *nft.MsgUpdateClass) (*nft.MsgUpdateClassResponse, error) {
class := nft.Class{
Id: msg.ClassId,
Name: msg.Name,
Symbol: msg.Symbol,
Description: msg.Description,
Uri: msg.Uri,
UriHash: msg.UriHash,
Data: msg.Data,
}
if err := k.UpdateClass(ctx, class); err != nil {
return nil, err
}
return &nft.MsgUpdateClassResponse{}, nil
}
Comment on lines +54 to +69
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation and authorization checks for class updates.

The function needs the following improvements:

  1. Add input validation for required fields (ClassId).
  2. Add authorization check to control who can update NFT classes.
  3. Emit an event when a class is updated for better observability.

Apply this diff to add the suggested improvements:

 func (k Keeper) MsgUpdateClass(ctx context.Context, msg *nft.MsgUpdateClass) (*nft.MsgUpdateClassResponse, error) {
+    if msg.ClassId == "" {
+        return nil, nft.ErrEmptyClassID
+    }
+
+    sender, err := k.ac.StringToBytes(msg.Sender)
+    if err != nil {
+        return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid sender address (%s)", msg.Sender)
+    }
+
+    // Verify sender is authorized to update the class
+    if !k.IsClassEditor(ctx, msg.ClassId, sender) {
+        return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "%s is not authorized to update class %s", msg.Sender, msg.ClassId)
+    }
+
     class := nft.Class{
         Id:          msg.ClassId,
         Name:        msg.Name,
         Symbol:      msg.Symbol,
         Description: msg.Description,
         Uri:         msg.Uri,
         UriHash:     msg.UriHash,
         Data:        msg.Data,
     }
     if err := k.UpdateClass(ctx, class); err != nil {
         return nil, err
     }
+
+    if err = k.EventService.EventManager(ctx).Emit(&nft.EventUpdateClass{
+        ClassId:     msg.ClassId,
+        Sender:      msg.Sender,
+    }); err != nil {
+        return nil, err
+    }
+
     return &nft.MsgUpdateClassResponse{}, nil
 }

Committable suggestion skipped: line range outside the PR's diff.


// MsgMintNFT mints a new NFT
func (k Keeper) MsgMintNFT(ctx context.Context, msg *nft.MsgMintNFT) (*nft.MsgMintNFTResponse, error) {
nft := nft.NFT{
ClassId: msg.ClassId,
Id: msg.Id,
Uri: msg.Uri,
UriHash: msg.UriHash,
Data: msg.Data,
}
if err := k.Mint(ctx, nft, msg.Receiver); err != nil {
return nil, err
}
return &nft.MsgMintNFTResponse{}, nil
}
Comment on lines +71 to +84
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation and authorization checks for NFT minting.

The function needs the following improvements:

  1. Add input validation for required fields (ClassId, Id, Receiver).
  2. Add authorization check to control who can mint NFTs.
  3. Emit an event when an NFT is minted for better observability.

Apply this diff to add the suggested improvements:

 func (k Keeper) MsgMintNFT(ctx context.Context, msg *nft.MsgMintNFT) (*nft.MsgMintNFTResponse, error) {
+    if msg.ClassId == "" {
+        return nil, nft.ErrEmptyClassID
+    }
+    if msg.Id == "" {
+        return nil, nft.ErrEmptyNFTID
+    }
+    if msg.Receiver == "" {
+        return nil, nft.ErrEmptyReceiver
+    }
+
+    sender, err := k.ac.StringToBytes(msg.Sender)
+    if err != nil {
+        return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid sender address (%s)", msg.Sender)
+    }
+
+    // Verify sender is authorized to mint NFTs for this class
+    if !k.IsMinter(ctx, msg.ClassId, sender) {
+        return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "%s is not authorized to mint NFTs for class %s", msg.Sender, msg.ClassId)
+    }
+
+    receiver, err := k.ac.StringToBytes(msg.Receiver)
+    if err != nil {
+        return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid receiver address (%s)", msg.Receiver)
+    }
+
     nft := nft.NFT{
         ClassId: msg.ClassId,
         Id:      msg.Id,
         Uri:     msg.Uri,
         UriHash: msg.UriHash,
         Data:    msg.Data,
     }
-    if err := k.Mint(ctx, nft, msg.Receiver); err != nil {
+    if err := k.Mint(ctx, nft, receiver); err != nil {
         return nil, err
     }
+
+    if err = k.EventService.EventManager(ctx).Emit(&nft.EventMintNFT{
+        ClassId:     msg.ClassId,
+        Id:          msg.Id,
+        Sender:      msg.Sender,
+        Receiver:    msg.Receiver,
+    }); err != nil {
+        return nil, err
+    }
+
     return &nft.MsgMintNFTResponse{}, nil
 }

Committable suggestion skipped: line range outside the PR's diff.


// MsgBurnNFT burns an existing NFT
func (k Keeper) MsgBurnNFT(ctx context.Context, msg *nft.MsgBurnNFT) (*nft.MsgBurnNFTResponse, error) {
if err := k.Burn(ctx, msg.ClassId, msg.Id); err != nil {
return nil, err
}
return &nft.MsgBurnNFTResponse{}, nil
}
Comment on lines +86 to +92
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation and authorization checks for NFT burning.

The function needs the following improvements:

  1. Add input validation for required fields (ClassId, Id).
  2. Add authorization check to ensure only the NFT owner can burn it.
  3. Emit an event when an NFT is burned for better observability.

Apply this diff to add the suggested improvements:

 func (k Keeper) MsgBurnNFT(ctx context.Context, msg *nft.MsgBurnNFT) (*nft.MsgBurnNFTResponse, error) {
+    if msg.ClassId == "" {
+        return nil, nft.ErrEmptyClassID
+    }
+    if msg.Id == "" {
+        return nil, nft.ErrEmptyNFTID
+    }
+
+    sender, err := k.ac.StringToBytes(msg.Sender)
+    if err != nil {
+        return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid sender address (%s)", msg.Sender)
+    }
+
+    // Verify sender owns the NFT
+    owner := k.GetOwner(ctx, msg.ClassId, msg.Id)
+    if !bytes.Equal(owner, sender) {
+        return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "%s is not the owner of NFT %s", msg.Sender, msg.Id)
+    }
+
     if err := k.Burn(ctx, msg.ClassId, msg.Id); err != nil {
         return nil, err
     }
+
+    if err = k.EventService.EventManager(ctx).Emit(&nft.EventBurnNFT{
+        ClassId: msg.ClassId,
+        Id:      msg.Id,
+        Sender:  msg.Sender,
+    }); err != nil {
+        return nil, err
+    }
+
     return &nft.MsgBurnNFTResponse{}, nil
 }

Committable suggestion skipped: line range outside the PR's diff.


// MsgUpdateNFT updates an existing NFT
func (k Keeper) MsgUpdateNFT(ctx context.Context, msg *nft.MsgUpdateNFT) (*nft.MsgUpdateNFTResponse, error) {
nft := nft.NFT{
ClassId: msg.ClassId,
Id: msg.Id,
Uri: msg.Uri,
UriHash: msg.UriHash,
Data: msg.Data,
}
if err := k.Update(ctx, nft); err != nil {
return nil, err
}
return &nft.MsgUpdateNFTResponse{}, nil
}
Comment on lines +94 to +107
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation and authorization checks for NFT updates.

The function needs the following improvements:

  1. Add input validation for required fields (ClassId, Id).
  2. Add authorization check to ensure only the NFT owner can update it.
  3. Emit an event when an NFT is updated for better observability.

Apply this diff to add the suggested improvements:

 func (k Keeper) MsgUpdateNFT(ctx context.Context, msg *nft.MsgUpdateNFT) (*nft.MsgUpdateNFTResponse, error) {
+    if msg.ClassId == "" {
+        return nil, nft.ErrEmptyClassID
+    }
+    if msg.Id == "" {
+        return nil, nft.ErrEmptyNFTID
+    }
+
+    sender, err := k.ac.StringToBytes(msg.Sender)
+    if err != nil {
+        return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid sender address (%s)", msg.Sender)
+    }
+
+    // Verify sender owns the NFT
+    owner := k.GetOwner(ctx, msg.ClassId, msg.Id)
+    if !bytes.Equal(owner, sender) {
+        return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "%s is not the owner of NFT %s", msg.Sender, msg.Id)
+    }
+
     nft := nft.NFT{
         ClassId: msg.ClassId,
         Id:      msg.Id,
         Uri:     msg.Uri,
         UriHash: msg.UriHash,
         Data:    msg.Data,
     }
     if err := k.Update(ctx, nft); err != nil {
         return nil, err
     }
+
+    if err = k.EventService.EventManager(ctx).Emit(&nft.EventUpdateNFT{
+        ClassId: msg.ClassId,
+        Id:      msg.Id,
+        Sender:  msg.Sender,
+    }); err != nil {
+        return nil, err
+    }
+
     return &nft.MsgUpdateNFTResponse{}, nil
 }

Committable suggestion skipped: line range outside the PR's diff.

168 changes: 168 additions & 0 deletions x/nft/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,3 +399,171 @@ func (s *TestSuite) TestInitGenesis() {
s.Require().True(has)
s.Require().EqualValues(expNFT, actNFT)
}

func (s *TestSuite) TestMsgNewClass() {
msg := &nft.MsgNewClass{
ClassId: testClassID,
Name: testClassName,
Symbol: testClassSymbol,
Description: testClassDescription,
Uri: testClassURI,
UriHash: testClassURIHash,
}

_, err := s.nftKeeper.MsgNewClass(s.ctx, msg)
s.Require().NoError(err)

actual, has := s.nftKeeper.GetClass(s.ctx, testClassID)
s.Require().True(has)
s.Require().EqualValues(nft.Class{
Id: testClassID,
Name: testClassName,
Symbol: testClassSymbol,
Description: testClassDescription,
Uri: testClassURI,
UriHash: testClassURIHash,
}, actual)
}

func (s *TestSuite) TestMsgUpdateClass() {
class := nft.Class{
Id: testClassID,
Name: testClassName,
Symbol: testClassSymbol,
Description: testClassDescription,
Uri: testClassURI,
UriHash: testClassURIHash,
}
err := s.nftKeeper.SaveClass(s.ctx, class)
s.Require().NoError(err)

msg := &nft.MsgUpdateClass{
ClassId: testClassID,
Name: "Updated Name",
Symbol: "Updated Symbol",
Description: "Updated Description",
Uri: "Updated URI",
UriHash: "Updated URI Hash",
}

_, err = s.nftKeeper.MsgUpdateClass(s.ctx, msg)
s.Require().NoError(err)

actual, has := s.nftKeeper.GetClass(s.ctx, testClassID)
s.Require().True(has)
s.Require().EqualValues(nft.Class{
Id: testClassID,
Name: "Updated Name",
Symbol: "Updated Symbol",
Description: "Updated Description",
Uri: "Updated URI",
UriHash: "Updated URI Hash",
}, actual)
}

func (s *TestSuite) TestMsgMintNFT() {
class := nft.Class{
Id: testClassID,
Name: testClassName,
Symbol: testClassSymbol,
Description: testClassDescription,
Uri: testClassURI,
UriHash: testClassURIHash,
}
err := s.nftKeeper.SaveClass(s.ctx, class)
s.Require().NoError(err)

msg := &nft.MsgMintNFT{
ClassId: testClassID,
Id: testID,
Uri: testURI,
UriHash: testURIHash,
Receiver: s.encodedAddrs[0],
}

_, err = s.nftKeeper.MsgMintNFT(s.ctx, msg)
s.Require().NoError(err)

actual, has := s.nftKeeper.GetNFT(s.ctx, testClassID, testID)
s.Require().True(has)
s.Require().EqualValues(nft.NFT{
ClassId: testClassID,
Id: testID,
Uri: testURI,
UriHash: testURIHash,
}, actual)
}

func (s *TestSuite) TestMsgBurnNFT() {
class := nft.Class{
Id: testClassID,
Name: testClassName,
Symbol: testClassSymbol,
Description: testClassDescription,
Uri: testClassURI,
UriHash: testClassURIHash,
}
err := s.nftKeeper.SaveClass(s.ctx, class)
s.Require().NoError(err)

nft := nft.NFT{
ClassId: testClassID,
Id: testID,
Uri: testURI,
UriHash: testURIHash,
}
err = s.nftKeeper.Mint(s.ctx, nft, s.addrs[0])
s.Require().NoError(err)

msg := &nft.MsgBurnNFT{
ClassId: testClassID,
Id: testID,
}

_, err = s.nftKeeper.MsgBurnNFT(s.ctx, msg)
s.Require().NoError(err)

_, has := s.nftKeeper.GetNFT(s.ctx, testClassID, testID)
s.Require().False(has)
}

func (s *TestSuite) TestMsgUpdateNFT() {
class := nft.Class{
Id: testClassID,
Name: testClassName,
Symbol: testClassSymbol,
Description: testClassDescription,
Uri: testClassURI,
UriHash: testClassURIHash,
}
err := s.nftKeeper.SaveClass(s.ctx, class)
s.Require().NoError(err)

nft := nft.NFT{
ClassId: testClassID,
Id: testID,
Uri: testURI,
UriHash: testURIHash,
}
err = s.nftKeeper.Mint(s.ctx, nft, s.addrs[0])
s.Require().NoError(err)

msg := &nft.MsgUpdateNFT{
ClassId: testClassID,
Id: testID,
Uri: "Updated URI",
UriHash: "Updated URI Hash",
}

_, err = s.nftKeeper.MsgUpdateNFT(s.ctx, msg)
s.Require().NoError(err)

actual, has := s.nftKeeper.GetNFT(s.ctx, testClassID, testID)
s.Require().True(has)
s.Require().EqualValues(nft.NFT{
ClassId: testClassID,
Id: testID,
Uri: "Updated URI",
UriHash: "Updated URI Hash",
}, actual)
}
Comment on lines +403 to +569
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve test coverage for error cases and validation.

The test functions need the following improvements:

  1. Add test cases for input validation errors (empty IDs, invalid addresses).
  2. Add test cases for authorization errors (unauthorized sender).
  3. Add test cases for event emission.

Example test cases for TestMsgNewClass:

func (s *TestSuite) TestMsgNewClass() {
    testCases := []struct {
        name    string
        msg     *nft.MsgNewClass
        expErr  bool
        errMsg  string
    }{
        {
            name: "empty class id",
            msg: &nft.MsgNewClass{
                ClassId: "",
                Name:    testClassName,
            },
            expErr: true,
            errMsg: "empty class id",
        },
        {
            name: "unauthorized sender",
            msg: &nft.MsgNewClass{
                ClassId: testClassID,
                Name:    testClassName,
                Sender:  "unauthorized",
            },
            expErr: true,
            errMsg: "unauthorized",
        },
        // Add more test cases
    }

    for _, tc := range testCases {
        s.Run(tc.name, func() {
            _, err := s.nftKeeper.MsgNewClass(s.ctx, tc.msg)
            if tc.expErr {
                s.Require().Error(err)
                s.Require().Contains(err.Error(), tc.errMsg)
            } else {
                s.Require().NoError(err)
                // Verify event emission
                events := s.ctx.EventManager().Events()
                s.Require().Len(events, 1)
                s.Require().Equal("new_class", events[0].Type)
            }
        })
    }
}
🧰 Tools
🪛 golangci-lint (1.62.2)

404-404: undefined: nft.MsgNewClass

(typecheck)

72 changes: 72 additions & 0 deletions x/nft/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,75 @@ func (k Keeper) Send(ctx context.Context, msg *nft.MsgSend) (*nft.MsgSendRespons

return &nft.MsgSendResponse{}, nil
}

// NewClass implements NewClass method of the types.MsgServer.
func (k Keeper) NewClass(ctx context.Context, msg *nft.MsgNewClass) (*nft.MsgNewClassResponse, error) {
class := nft.Class{
Id: msg.ClassId,
Name: msg.Name,
Symbol: msg.Symbol,
Description: msg.Description,
Uri: msg.Uri,
UriHash: msg.UriHash,
Data: msg.Data,
}
if err := k.SaveClass(ctx, class); err != nil {
return nil, err
}
return &nft.MsgNewClassResponse{}, nil
}

// UpdateClass implements UpdateClass method of the types.MsgServer.
func (k Keeper) UpdateClass(ctx context.Context, msg *nft.MsgUpdateClass) (*nft.MsgUpdateClassResponse, error) {
class := nft.Class{
Id: msg.ClassId,
Name: msg.Name,
Symbol: msg.Symbol,
Description: msg.Description,
Uri: msg.Uri,
UriHash: msg.UriHash,
Data: msg.Data,
}
if err := k.UpdateClass(ctx, class); err != nil {
return nil, err
}
return &nft.MsgUpdateClassResponse{}, nil
}

// MintNFT implements MintNFT method of the types.MsgServer.
func (k Keeper) MintNFT(ctx context.Context, msg *nft.MsgMintNFT) (*nft.MsgMintNFTResponse, error) {
nft := nft.NFT{
ClassId: msg.ClassId,
Id: msg.Id,
Uri: msg.Uri,
UriHash: msg.UriHash,
Data: msg.Data,
}
if err := k.Mint(ctx, nft, msg.Receiver); err != nil {
return nil, err
}
return &nft.MsgMintNFTResponse{}, nil
}

// BurnNFT implements BurnNFT method of the types.MsgServer.
func (k Keeper) BurnNFT(ctx context.Context, msg *nft.MsgBurnNFT) (*nft.MsgBurnNFTResponse, error) {
if err := k.Burn(ctx, msg.ClassId, msg.Id); err != nil {
return nil, err
}
return &nft.MsgBurnNFTResponse{}, nil
}

// UpdateNFT implements UpdateNFT method of the types.MsgServer.
func (k Keeper) UpdateNFT(ctx context.Context, msg *nft.MsgUpdateNFT) (*nft.MsgUpdateNFTResponse, error) {
nft := nft.NFT{
ClassId: msg.ClassId,
Id: msg.Id,
Uri: msg.Uri,
UriHash: msg.UriHash,
Data: msg.Data,
}
if err := k.Update(ctx, nft); err != nil {
return nil, err
}
return &nft.MsgUpdateNFTResponse{}, nil
}
Comment on lines +56 to +126
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remove code duplication between keeper.go and msg_server.go.

The functions in this file duplicate the logic from keeper.go. To improve maintainability and reduce the risk of inconsistencies, consider one of these approaches:

  1. Move the validation and authorization logic to keeper.go and have the message server functions simply delegate to the keeper functions.
  2. Move all the logic to the message server and have the keeper functions delegate to the message server.

Here's an example of approach #1 for the NewClass function:

 func (k Keeper) NewClass(ctx context.Context, msg *nft.MsgNewClass) (*nft.MsgNewClassResponse, error) {
-    class := nft.Class{
-        Id:          msg.ClassId,
-        Name:        msg.Name,
-        Symbol:      msg.Symbol,
-        Description: msg.Description,
-        Uri:         msg.Uri,
-        UriHash:     msg.UriHash,
-        Data:        msg.Data,
-    }
-    if err := k.SaveClass(ctx, class); err != nil {
-        return nil, err
-    }
-    return &nft.MsgNewClassResponse{}, nil
+    return k.MsgNewClass(ctx, msg)
 }

Committable suggestion skipped: line range outside the PR's diff.

Loading
Loading