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

Merged
merged 4 commits into from
Mar 19, 2024
Merged

feat(inflation): add burn method #1823

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1553012
feat(inflation): add burn method
k-yang Mar 19, 2024
595c976
Update CHANGELOG.md
k-yang Mar 19, 2024
2e99e3f
chore: linter
Unique-Divine Mar 19, 2024
d91a509
refactor: use coin.Validate()
Unique-Divine Mar 19, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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 •
edited
Loading

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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