refactor MT #263
Merged
refactor MT #263
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # modules/mt/keeper/keeper.go
zhangyelong
reviewed
Feb 19, 2022
modules/mt/keeper/keeper.go
Outdated
| denomID, tokenID string, | ||
| amout uint64, | ||
| data []byte, | ||
| owner sdk.AccAddress, |
zhangyelong
reviewed
Feb 19, 2022
modules/mt/keeper/keeper.go
Outdated
| types.MT{ | ||
| Id: mt.GetID(), | ||
| Supply: amout + mt.GetSupply(), | ||
| Data: owner, |
zhangyelong
reviewed
Feb 19, 2022
modules/mt/keeper/keeper.go
Outdated
| ctx, denomID, | ||
| types.MT{ | ||
| Id: mt.GetID(), | ||
| Supply: amout + mt.GetSupply(), |
zhangyelong
reviewed
Feb 19, 2022
modules/mt/keeper/keeper.go
Outdated
| ) error { | ||
|
|
||
| // TODO create if not exists, mint if exists | ||
| if k.HasMT(ctx, denomID, tokenID) { |
There was a problem hiding this comment.
Have you checked that denom exists or not?
zhangyelong
reviewed
Feb 19, 2022
modules/mt/keeper/keeper.go
Outdated
| if denom.Owner != owner.String() { | ||
| return sdkerrors.Wrapf(types.ErrUnauthorized, "Denom is owned by %s", denom.Owner) | ||
| } | ||
|
|
||
| mt, err := k.Authorize(ctx, denomID, tokenID, owner) |
There was a problem hiding this comment.
Authorize checks if the sender is the owner of the given MT, but not of the given denom, so I guess here is a bug when the denom owner is not the MT owner.
Maybe we do not need the owner property in the MT structure, because the MT is owned by the denom
zhangyelong
suggested changes
Feb 19, 2022
modules/mt/keeper/keeper.go
Outdated
| mt, err := k.Authorize(ctx, denomID, tokenID, srcOwner) | ||
| if err != nil { | ||
| return err | ||
| srcOwnerAmount := k.getOwner(ctx, denomID, tokenID, srcOwner) |
There was a problem hiding this comment.
I prefer to use getBalance instead of getOwner
modules/mt/keeper/keeper.go
Outdated
| return err | ||
| srcOwnerAmount := k.getOwner(ctx, denomID, tokenID, srcOwner) | ||
| if srcOwnerAmount < amount { | ||
| return sdkerrors.Wrapf(types.ErrInvalidCollection, "Lack of mt: %", srcOwnerAmount) |
modules/mt/keeper/keeper.go
Outdated
|
|
||
| k.setMT(ctx, denomID, mt) | ||
| k.swapOwner(ctx, denomID, tokenID, srcOwner, dstOwner) | ||
| k.swapOwner(ctx, denomID, tokenID, amount, srcOwner, dstOwner) |
There was a problem hiding this comment.
Rename the swapOwner to transferMT
modules/mt/keeper/keeper.go
Outdated
| k.deleteMT(ctx, denomID, mt) | ||
| k.deleteOwner(ctx, denomID, tokenID, owner) | ||
| k.decreaseSupply(ctx, denomID) | ||
| srcOwnerAmount := k.getOwner(ctx, denomID, tokenID, owner) |
modules/mt/keeper/keeper.go
Outdated
| k.decreaseSupply(ctx, denomID) | ||
| srcOwnerAmount := k.getOwner(ctx, denomID, tokenID, owner) | ||
| if srcOwnerAmount < amount { | ||
| return sdkerrors.Wrapf(types.ErrInvalidCollection, "Lack of mt: %", srcOwnerAmount) |
| ctx.EventManager().EmitEvents(sdk.Events{ | ||
| sdk.NewEvent( | ||
| types.EventTypeMintMT, | ||
| sdk.NewAttribute(types.AttributeKeyTokenID, msg.Id), | ||
| sdk.NewAttribute(types.AttributeKeyDenomID, msg.DenomId), | ||
| sdk.NewAttribute(types.AttributeKeyRecipient, msg.Recipient), | ||
| sdk.NewAttribute(types.AttributeKeySupply, strconv.FormatUint(mt.GetSupply(), 10)), |
There was a problem hiding this comment.
All of the events should be redesigned. Let's do that in another PR.
| sender, | ||
| recipient, | ||
| ); err != nil { | ||
| if err := m.Keeper.TransferOwner(ctx, msg.DenomId, msg.Id, msg.Amount, sender, recipient); err != nil { |
There was a problem hiding this comment.
ditto, rename TransferOwner to Transfer or TransferMT
modules/mt/keeper/owners.go
Outdated
| "github.com/irisnet/irismod/modules/mt/types" | ||
| ) | ||
|
|
||
| func (k Keeper) deleteOwner(ctx sdk.Context, denomID, tokenID string, owner sdk.AccAddress) { | ||
| func (k Keeper) deleteOwner(ctx sdk.Context, |
modules/mt/keeper/owners.go
Outdated
| store.Set(types.KeyOwner(owner, denomID, tokenID), bz) | ||
| } | ||
|
|
||
| func (k Keeper) swapOwner(ctx sdk.Context, denomID, tokenID string, srcOwner, dstOwner sdk.AccAddress) { | ||
| func (k Keeper) swapOwner(ctx sdk.Context, |
There was a problem hiding this comment.
ditto, this is not a swap operation, just a transfer that combines subMT from sender and addMT to recipient.
| @@ -16,7 +16,8 @@ var ( | |||
| AttributeKeyRecipient = "recipient" | |||
| AttributeKeyOwner = "owner" | |||
| AttributeKeyTokenID = "token_id" | |||
There was a problem hiding this comment.
AttributeKeyMTID = "mt_id"
Refactor MT Module
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.