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.

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(inflation): add burn method #1823

Merged
merged 4 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 1 addition & 1 deletion app/keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ func ModuleAccPerms() map[string][]string {
return map[string][]string{
authtypes.FeeCollectorName: nil,
distrtypes.ModuleName: nil,
inflationtypes.ModuleName: {authtypes.Minter},
inflationtypes.ModuleName: {authtypes.Minter, authtypes.Burner},
stakingtypes.BondedPoolName: {authtypes.Burner, authtypes.Staking},
stakingtypes.NotBondedPoolName: {authtypes.Burner, authtypes.Staking},
govtypes.ModuleName: {authtypes.Burner},
Expand Down
31 changes: 21 additions & 10 deletions proto/nibiru/inflation/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,25 @@ package nibiru.inflation.v1;
import "gogoproto/gogo.proto";
import "google/api/annotations.proto";
import "nibiru/inflation/v1/inflation.proto";
import "cosmos/base/v1beta1/coin.proto";

option go_package = "github.com/NibiruChain/nibiru/x/inflation/types";


service Msg {
// ToggleInflation defines a method to enable or disable inflation.
rpc ToggleInflation(MsgToggleInflation) returns (MsgToggleInflationResponse) {
option (google.api.http).post = "/nibiru/inflation/v1/toggle";
};

// EditInflationParams defines a method to edit the inflation params.
rpc EditInflationParams(MsgEditInflationParams)
returns (MsgEditInflationParamsResponse) {
rpc EditInflationParams(MsgEditInflationParams)
returns (MsgEditInflationParamsResponse) {
option (google.api.http).post = "/nibiru/inflation/edit-inflation-params";
};
};

rpc Burn(MsgBurn) returns (MsgBurnResponse) {
option (google.api.http).post = "/nibiru/inflation/v1/burn";
};
}

// MsgToggleInflation defines a message to enable or disable inflation.
Expand All @@ -33,16 +37,16 @@ message MsgToggleInflation {
message MsgEditInflationParams {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

string sender = 1;
bool inflation_enabled = 2;
repeated string polynomial_factors = 3[
repeated string polynomial_factors = 3 [
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
(gogoproto.nullable) = true
];
InflationDistribution inflation_distribution = 4
[ (gogoproto.nullable) = true ];

string epochs_per_period = 5 [
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
(gogoproto.nullable) = true
Expand All @@ -57,8 +61,15 @@ message MsgEditInflationParams {
];
}

message MsgToggleInflationResponse {
}
message MsgToggleInflationResponse {}

message MsgEditInflationParamsResponse {
message MsgEditInflationParamsResponse {}

// MsgBurn: allows burning of any token
message MsgBurn {
string sender = 1 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
cosmos.base.v1beta1.Coin coin = 2
[ (gogoproto.moretags) = "yaml:\"coin\"", (gogoproto.nullable) = false ];
}

message MsgBurnResponse {}
10 changes: 10 additions & 0 deletions x/inflation/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,13 @@ func NewKeeper(
func (k Keeper) Logger(ctx sdk.Context) log.Logger {
return ctx.Logger().With("module", "x/"+types.ModuleName)
}

func (k Keeper) Burn(ctx sdk.Context, coins sdk.Coins, sender sdk.AccAddress) error {
if err := k.bankKeeper.SendCoinsFromAccountToModule(
ctx, sender, types.ModuleName, coins,
); err != nil {
return err
}

return k.bankKeeper.BurnCoins(ctx, types.ModuleName, coins)
Comment on lines +93 to +100
Copy link
Contributor

Choose a reason for hiding this comment

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

The Burn method in the Keeper struct correctly transfers coins from an account to the module before burning them. This method should ensure that the coins are indeed transferred to a "burn" account or somehow marked as burned to prevent their reuse. Additionally, consider adding more detailed logging for auditability, especially for operations like burning coins which are critical to the monetary policy.

Enhance the Burn method with detailed logging to improve traceability and auditability of burn operations.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there logging done in the bank keeper so this can be picked up by heart monitor?

Copy link
Member

@Unique-Divine Unique-Divine Mar 19, 2024

Choose a reason for hiding this comment

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

We can use the query of the supply from the bank module

nibid q bank total

Copy link
Member

Choose a reason for hiding this comment

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

That is one thing missing from the tests. They don't check if the total supply response changes after burning

}
48 changes: 48 additions & 0 deletions x/inflation/keeper/keeper_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package keeper_test

import (
"fmt"
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/require"

"github.com/NibiruChain/nibiru/x/common/testutil"
"github.com/NibiruChain/nibiru/x/common/testutil/testapp"
"github.com/NibiruChain/nibiru/x/inflation/types"
)

func init() {
testapp.EnsureNibiruPrefix()
}

func TestBurn(t *testing.T) {
testCases := []struct {
name string
sender sdk.AccAddress
burnCoin sdk.Coin
expectedErr error
}{
{
name: "pass",
sender: testutil.AccAddress(),
burnCoin: sdk.NewCoin("nibiru", sdk.NewInt(100)),
expectedErr: nil,
},
}
for _, tc := range testCases {
t.Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) {
nibiruApp, ctx := testapp.NewNibiruTestAppAndContext()
nibiruApp.BankKeeper.MintCoins(ctx, types.ModuleName, sdk.NewCoins(tc.burnCoin))
nibiruApp.BankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, tc.sender, sdk.NewCoins(tc.burnCoin))

// Burn coins
err := nibiruApp.InflationKeeper.Burn(ctx, sdk.NewCoins(tc.burnCoin), tc.sender)
if tc.expectedErr != nil {
require.EqualError(t, err, tc.expectedErr.Error())
} else {
require.NoError(t, err)
}
})
}
}
k-yang marked this conversation as resolved.
Show resolved Hide resolved
14 changes: 14 additions & 0 deletions x/inflation/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,17 @@ func (ms msgServer) ToggleInflation(
resp = &types.MsgToggleInflationResponse{}
return resp, err
}

func (ms msgServer) Burn(
goCtx context.Context, msg *types.MsgBurn,
) (resp *types.MsgBurnResponse, err error) {
ctx := sdk.UnwrapSDKContext(goCtx)

sender, err := sdk.AccAddressFromBech32(msg.Sender)
if err != nil {
return nil, err
}

err = ms.Keeper.Burn(ctx, sdk.NewCoins(msg.Coin), sender)
return &types.MsgBurnResponse{}, err
}
17 changes: 17 additions & 0 deletions x/inflation/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,20 @@ func TestMsgEditInflationParams(t *testing.T) {
params = app.InflationKeeper.GetParams(ctx)
require.EqualValues(t, params.EpochsPerPeriod, 42)
}

func TestMsgBurn(t *testing.T) {
app, ctx := testapp.NewNibiruTestAppAndContext()
sender := testutil.AccAddress()
app.BankKeeper.MintCoins(ctx, types.ModuleName, sdk.NewCoins(sdk.NewCoin("unibi", sdk.NewInt(100))))
app.BankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, sender, sdk.NewCoins(sdk.NewCoin("unibi", sdk.NewInt(100))))

msgServer := keeper.NewMsgServerImpl(app.InflationKeeper)

msg := types.MsgBurn{
Sender: sender.String(),
Coin: sdk.NewCoin("unibi", sdk.NewInt(100)),
}

_, err := msgServer.Burn(ctx, &msg)
require.NoError(t, err)
}
k-yang marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions x/inflation/types/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ type BankKeeper interface {
GetAllBalances(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins
SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
SendCoinsFromModuleToModule(ctx sdk.Context, senderModule, recipientModule string, amt sdk.Coins) error
SendCoinsFromAccountToModule(
ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins,
) error
Comment on lines +24 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

The addition of SendCoinsFromAccountToModule to the BankKeeper interface is crucial for the burn functionality. However, the method lacks documentation. It's important for interface methods, especially those that are part of public APIs, to have clear, concise documentation explaining their purpose, parameters, and expected behavior.

Add documentation to the SendCoinsFromAccountToModule method to improve code readability and maintainability.

MintCoins(ctx sdk.Context, name string, amt sdk.Coins) error
BurnCoins(ctx sdk.Context, name string, amt sdk.Coins) error
HasSupply(ctx sdk.Context, denom string) bool
Expand Down
62 changes: 54 additions & 8 deletions x/inflation/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,30 @@ import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
)

// ensure Msg interface compliance at compile time
var (
_ sdk.Msg = &MsgEditInflationParams{}
_ sdk.Msg = &MsgToggleInflation{}
_ legacytx.LegacyMsg = &MsgEditInflationParams{}
_ legacytx.LegacyMsg = &MsgToggleInflation{}
_ legacytx.LegacyMsg = &MsgBurn{}
)

// oracle message types
const (
TypeMsgEditInflationParams = "edit_inflation_params"
TypeMsgToggleInflation = "toggle_inflation"
TypeMsgBurn = "msg_burn"
)

// Route implements sdk.Msg
// Route implements legacytx.LegacyMsg
func (msg MsgEditInflationParams) Route() string { return RouterKey }

// Type implements sdk.Msg
// Type implements legacytx.LegacyMsg
func (msg MsgEditInflationParams) Type() string { return TypeMsgEditInflationParams }

// GetSignBytes implements sdk.Msg
// GetSignBytes implements legacytx.LegacyMsg
func (msg MsgEditInflationParams) GetSignBytes() []byte {
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}
Expand Down Expand Up @@ -75,13 +78,13 @@ func (m MsgEditInflationParams) ValidateBasic() error {

// -------------------------------------------------
// MsgToggleInflation
// Route implements sdk.Msg
// Route implements legacytx.LegacyMsg
func (msg MsgToggleInflation) Route() string { return RouterKey }

// Type implements sdk.Msg
// Type implements legacytx.LegacyMsg
func (msg MsgToggleInflation) Type() string { return TypeMsgToggleInflation }

// GetSignBytes implements sdk.Msg
// GetSignBytes implements legacytx.LegacyMsg
func (msg MsgToggleInflation) GetSignBytes() []byte {
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}
Expand All @@ -102,3 +105,46 @@ func (m MsgToggleInflation) ValidateBasic() error {
}
return nil
}

// -------------------------------------------------
// MsgBurn
// Route implements legacytx.LegacyMsg
func (msg MsgBurn) Route() string { return RouterKey }

// Type implements legacytx.LegacyMsg
func (msg MsgBurn) Type() string { return TypeMsgBurn }

// GetSignBytes implements legacytx.LegacyMsg
func (msg MsgBurn) GetSignBytes() []byte {
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}

// GetSigners implements legacytx.LegacyMsg
func (msg MsgBurn) GetSigners() []sdk.AccAddress {
feeder, err := sdk.AccAddressFromBech32(msg.Sender)
if err != nil {
panic(err)
}

return []sdk.AccAddress{feeder}
}

func (m MsgBurn) ValidateBasic() error {
if _, err := sdk.AccAddressFromBech32(m.Sender); err != nil {
return err
}

if m.Coin.Denom == "" {
return fmt.Errorf("coin denom should not be empty")
}

if m.Coin.Amount.IsZero() {
return fmt.Errorf("coin amount should not be zero")
}

if m.Coin.IsNil() {
return fmt.Errorf("coin should not be nil")
}

return nil
}
k-yang marked this conversation as resolved.
Show resolved Hide resolved
Loading
Loading