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

feat(bank): add MintCoinsToAddress #9620

Closed
wants to merge 1 commit into from
Closed

feat(bank): add MintCoinsToAddress #9620

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
49 changes: 33 additions & 16 deletions x/bank/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,37 +364,54 @@ func (k BaseKeeper) UndelegateCoinsFromModuleToAccount(
// MintCoins creates new coins from thin air and adds it to the module account.
// It will panic if the module account does not exist or is unauthorized.
func (k BaseKeeper) MintCoins(ctx sdk.Context, moduleName string, amounts sdk.Coins) error {
acc := k.ak.GetModuleAccount(ctx, moduleName)
if acc == nil {
panic(sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", moduleName))
}
return k.mint(ctx, moduleName, nil, amounts)
}

if !acc.HasPermission(authtypes.Minter) {
panic(sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to mint tokens", moduleName))
}
// MintCoinsToAddress creates new coins from thin air using module account and puts them in
// receiver balance. It will panic if the module account does not exist or is unauthorized.
func (k BaseKeeper) MintCoinsToAddress(ctx sdk.Context, moduleName string, receiver sdk.AccAddress, amounts sdk.Coins) error {
return k.mint(ctx, moduleName, receiver, amounts)
}

err := k.addCoins(ctx, acc.GetAddress(), amounts)
if err != nil {
// mint is a common function for minting new coins. It mints from module to receiver if receiver
// is not nil. Otherwise the coins are minted to the module itself.
func (k BaseKeeper) mint(ctx sdk.Context, moduleName string, receiver sdk.AccAddress, amounts sdk.Coins) error {
mAcc := k.assertModuleIsMinter(ctx, moduleName).GetAddress()
destination := receiver
if receiver == nil {
destination = mAcc
}
if err := k.addCoins(ctx, destination, amounts); err != nil {
return err
}

for _, amount := range amounts {
supply := k.GetSupply(ctx, amount.GetDenom())
supply = supply.Add(amount)
k.setSupply(ctx, supply)
k.setSupply(ctx, supply.Add(amount))
}

logger := k.Logger(ctx)
logger.Info("minted coins from module account", "amount", amounts.String(), "from", moduleName)
k.Logger(ctx).
Info("minted coins from module account",
"amount", amounts.String(), "from", moduleName, "to", destination)

// emit mint event
ctx.EventManager().EmitEvent(
types.NewCoinMintEvent(acc.GetAddress(), amounts),
types.NewCoinMintEvent(mAcc, amounts, receiver),
)

return nil
}

func (k BaseKeeper) assertModuleIsMinter(ctx sdk.Context, moduleName string) authtypes.ModuleAccountI {
acc := k.ak.GetModuleAccount(ctx, moduleName)
if acc == nil {
panic(sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", moduleName))
}

if !acc.HasPermission(authtypes.Minter) {
panic(sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to mint tokens", moduleName))
}
return acc
}

// BurnCoins burns coins deletes coins from the balance of the module account.
// It will panic if the module account does not exist or is unauthorized.
func (k BaseKeeper) BurnCoins(ctx sdk.Context, moduleName string, amounts sdk.Coins) error {
Expand Down
11 changes: 10 additions & 1 deletion x/bank/types/events.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,16 @@ func NewCoinReceivedEvent(receiver sdk.AccAddress, amount sdk.Coins) sdk.Event {

// NewCoinMintEvent construct a new coin minted sdk.Event
// nolint: interfacer
func NewCoinMintEvent(minter sdk.AccAddress, amount sdk.Coins) sdk.Event {
func NewCoinMintEvent(minter sdk.AccAddress, amount sdk.Coins, receiver sdk.AccAddress) sdk.Event {
if len(receiver) > 0 {
return sdk.NewEvent(
EventTypeCoinMint,
sdk.NewAttribute(AttributeKeyMinter, minter.String()),
sdk.NewAttribute(sdk.AttributeKeyAmount, amount.String()),
sdk.NewAttribute(AttributeKeyReceiver, receiver.String()),
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

do you think it make sense to have variable list of attributes for the same event type?

Copy link
Contributor

Choose a reason for hiding this comment

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

why can the receiver be empty? a module has to mint coins somewhere, either to itself or to an address?

Copy link
Member

@aaronc aaronc Jul 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Let's not diverge from the existing event definitions. For minting and sending let's just have two events and not add receiver to the mint event. Better to be consistent with existing usage where possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@fdymylja - receiver is empty to be compatible with the current behavior when minting to a module.
@aaronc - the idea was to not send and have a functionality and event to mint to an account.

Re event: maybe we can add a third argument in both cases? Will it break any client? Do we know how this events are parsed?

Copy link
Contributor

Choose a reason for hiding this comment

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

receiver is empty to be compatible with the current behavior when minting to a module.

What does this mean exactly? In what cases is it empty?

the idea was to not send and have a functionality and event to mint to an account.

I don't follow this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@robert-zaremba, if the current events are mantained as they are then there's no need.

The current implementation uses the mint event to signal a coinbase
Then the addCoins function creates a CoinReceived event. This combo allows rosetta to be aware of balance changes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@fdymylja - my question was rather related to the changes proposed here: keeping same event with one extra argument. I guess, currently Rosetta assumes that all minting is done to a module only, right?

Copy link
Contributor

@fdymylja fdymylja Jul 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I guess, currently Rosetta assumes that all minting is done to a module only, right?

Nope, when there's a mint, a coinbase event is produced, which identifies how much the supply was inflated, and who is the minter. This event does not make rosetta add coins to the balance of the minter.
What adds coins to the balance of an address is the event produced by addCoins bank keeper function which is called in the Mint function alongside SendCoins etc...

Copy link
Member

Choose a reason for hiding this comment

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

I think for consistency we should stick with the current events for now and MintCoinsToAddress would then emit two events - one for minting, one for sending. Does that seem workable @robert-zaremba ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, so let's do this:

  1. We don't create a new event nor extend the current one.
  2. All mint events will not have recipient.
  3. For Rosetta, to account a new balance, we will use CoinReceivedEvent -- triggered in addCoins


}
return sdk.NewEvent(
EventTypeCoinMint,
sdk.NewAttribute(AttributeKeyMinter, minter.String()),
Expand Down