From 6c24125d84f7d0212cd63df4d982dfc98485705f Mon Sep 17 00:00:00 2001 From: Unique Divine <51418232+Unique-Divine@users.noreply.github.com> Date: Mon, 2 Oct 2023 03:27:43 -0500 Subject: [PATCH] feat(ante)!: Ante handler to add a maximum commission rate of 25% for validators. (#1615) * feat(ante)!: Ante handler to add a maximum commission rate of 25% for validators. * test: fix broken tests --------- Co-authored-by: Matthias <97468149+matthiasmatt@users.noreply.github.com> --- CHANGELOG.md | 4 +- app/ante.go | 1 + app/ante/commission.go | 37 ++++++ app/ante/commission_test.go | 138 +++++++++++++++++++++++ app/ante/errors.go | 24 ++++ app/ante/fixed_gas.go | 5 +- app/ante/fixed_gas_test.go | 18 +-- app/ante/testutil_test.go | 6 +- x/common/testutil/cli/network.go | 2 +- x/tokenfactory/keeper/grpc_query_test.go | 4 +- x/tokenfactory/keeper/keeper_test.go | 12 +- 11 files changed, 226 insertions(+), 25 deletions(-) create mode 100644 app/ante/commission.go create mode 100644 app/ante/commission_test.go create mode 100644 app/ante/errors.go diff --git a/CHANGELOG.md b/CHANGELOG.md index acb38e507..c680916df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#1609](https://github.com/NibiruChain/nibiru/pull/1609) - refactor(app)!: Remove x/stablecoin module. * [#1613](https://github.com/NibiruChain/nibiru/pull/1613) - feat(app)!: enforce min commission by changing default and genesis validation +* [#1615](https://github.com/NibiruChain/nibiru/pull/1613) - feat(ante)!: Ante + handler to add a maximum commission rate of 25% for validators. ### Improvements @@ -666,4 +668,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Testing * [#695](https://github.com/NibiruChain/nibiru/pull/695) Add `OpenPosition` integration tests. -* [#692](https://github.com/NibiruChain/nibiru/pull/692) Add test coverage for Perp MsgServer methods. \ No newline at end of file +* [#692](https://github.com/NibiruChain/nibiru/pull/692) Add test coverage for Perp MsgServer methods. diff --git a/app/ante.go b/app/ante.go index c80c7d071..a4d4a7861 100644 --- a/app/ante.go +++ b/app/ante.go @@ -61,6 +61,7 @@ func NewAnteHandler(options AnteHandlerOptions) (sdk.AnteHandler, error) { sdkante.NewTxTimeoutHeightDecorator(), sdkante.NewValidateMemoDecorator(options.AccountKeeper), ante.NewPostPriceFixedPriceDecorator(), + ante.AnteDecoratorStakingCommission{}, sdkante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper), // Replace fee ante from cosmos auth with a custom one. sdkante.NewDeductFeeDecorator( diff --git a/app/ante/commission.go b/app/ante/commission.go new file mode 100644 index 000000000..c0de8f0ae --- /dev/null +++ b/app/ante/commission.go @@ -0,0 +1,37 @@ +package ante + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" +) + +func MAX_COMMISSION() sdk.Dec { return sdk.MustNewDecFromStr("0.25") } + +var _ sdk.AnteDecorator = (*AnteDecoratorStakingCommission)(nil) + +// AnteDecoratorStakingCommission: Implements sdk.AnteDecorator, enforcing the +// maximum staking commission for validators on the network. +type AnteDecoratorStakingCommission struct{} + +func (a AnteDecoratorStakingCommission) AnteHandle( + ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler, +) (newCtx sdk.Context, err error) { + for _, msg := range tx.GetMsgs() { + switch msg := msg.(type) { + case *stakingtypes.MsgCreateValidator: + rate := msg.Commission.Rate + if rate.GT(MAX_COMMISSION()) { + return ctx, NewErrMaxValidatorCommission(rate) + } + case *stakingtypes.MsgEditValidator: + rate := msg.CommissionRate + if rate != nil && msg.CommissionRate.GT(MAX_COMMISSION()) { + return ctx, NewErrMaxValidatorCommission(*rate) + } + default: + continue + } + } + + return next(ctx, tx, simulate) +} diff --git a/app/ante/commission_test.go b/app/ante/commission_test.go new file mode 100644 index 000000000..693cc39b2 --- /dev/null +++ b/app/ante/commission_test.go @@ -0,0 +1,138 @@ +package ante_test + +import ( + "testing" + + sdkclienttx "github.com/cosmos/cosmos-sdk/client/tx" + sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + + "github.com/NibiruChain/nibiru/app" + "github.com/NibiruChain/nibiru/app/ante" + "github.com/NibiruChain/nibiru/x/common/testutil" +) + +func (s *AnteTestSuite) TestAnteDecoratorStakingCommission() { + // nextAnteHandler: A no-op next handler to make this a unit test. + var nextAnteHandler sdk.AnteHandler = func( + ctx sdk.Context, tx sdk.Tx, simulate bool, + ) (newCtx sdk.Context, err error) { + return ctx, nil + } + + mockDescription := stakingtypes.Description{ + Moniker: "mock-moniker", + Identity: "mock-identity", + Website: "mock-website", + SecurityContact: "mock-security-contact", + Details: "mock-details", + } + + valAddr := sdk.ValAddress(testutil.AccAddress()).String() + commissionRatePointer := new(sdk.Dec) + *commissionRatePointer = sdk.NewDecWithPrec(10, 2) + happyMsgs := []sdk.Msg{ + &stakingtypes.MsgCreateValidator{ + Description: mockDescription, + Commission: stakingtypes.CommissionRates{ + Rate: sdk.NewDecWithPrec(6, 2), // 6% + MaxRate: sdk.NewDec(420), + MaxChangeRate: sdk.NewDec(420), + }, + MinSelfDelegation: sdk.NewInt(1), + DelegatorAddress: testutil.AccAddress().String(), + ValidatorAddress: valAddr, + Pubkey: &codectypes.Any{}, + Value: sdk.NewInt64Coin("unibi", 1), + }, + &stakingtypes.MsgEditValidator{ + Description: mockDescription, + ValidatorAddress: valAddr, + CommissionRate: commissionRatePointer, // 10% + MinSelfDelegation: nil, + }, + } + + createSadMsgs := func() []sdk.Msg { + sadMsgCreateVal := new(stakingtypes.MsgCreateValidator) + *sadMsgCreateVal = *(happyMsgs[0]).(*stakingtypes.MsgCreateValidator) + sadMsgCreateVal.Commission.Rate = sdk.NewDecWithPrec(26, 2) + + sadMsgEditVal := new(stakingtypes.MsgEditValidator) + *sadMsgEditVal = *(happyMsgs[1]).(*stakingtypes.MsgEditValidator) + newCommissionRate := new(sdk.Dec) + *newCommissionRate = sdk.NewDecWithPrec(26, 2) + sadMsgEditVal.CommissionRate = newCommissionRate + + return []sdk.Msg{ + sadMsgCreateVal, + sadMsgEditVal, + } + } + sadMsgs := createSadMsgs() + + for _, tc := range []struct { + name string + txMsgs []sdk.Msg + wantErr string + }{ + { + name: "happy blank", + txMsgs: []sdk.Msg{}, + wantErr: "", + }, + { + name: "happy msgs", + txMsgs: []sdk.Msg{ + happyMsgs[0], + happyMsgs[1], + }, + wantErr: "", + }, + { + name: "sad: max commission on create validator", + txMsgs: []sdk.Msg{ + sadMsgs[0], + happyMsgs[1], + }, + wantErr: ante.ErrMaxValidatorCommission.Error(), + }, + { + name: "sad: max commission on edit validator", + txMsgs: []sdk.Msg{ + happyMsgs[0], + sadMsgs[1], + }, + wantErr: ante.ErrMaxValidatorCommission.Error(), + }, + } { + s.T().Run(tc.name, func(t *testing.T) { + txGasCoins := sdk.NewCoins( + sdk.NewCoin("unibi", sdk.NewInt(1_000)), + sdk.NewCoin("utoken", sdk.NewInt(500)), + ) + + encCfg := app.MakeEncodingConfigAndRegister() + txBuilder, err := sdkclienttx.Factory{}. + WithFees(txGasCoins.String()). + WithChainID(s.ctx.ChainID()). + WithTxConfig(encCfg.TxConfig). + BuildUnsignedTx(tc.txMsgs...) + s.NoError(err) + + anteDecorator := ante.AnteDecoratorStakingCommission{} + simulate := true + s.ctx, err = anteDecorator.AnteHandle( + s.ctx, txBuilder.GetTx(), simulate, nextAnteHandler, + ) + + if tc.wantErr != "" { + s.ErrorContains(err, tc.wantErr) + return + } + s.NoError(err) + }) + } +} diff --git a/app/ante/errors.go b/app/ante/errors.go new file mode 100644 index 000000000..5e5ef2bdc --- /dev/null +++ b/app/ante/errors.go @@ -0,0 +1,24 @@ +package ante + +import ( + sdkerrors "cosmossdk.io/errors" + sdk "github.com/cosmos/cosmos-sdk/types" +) + +var errorCodeIdx uint32 = 1 + +func registerError(errMsg string) *sdkerrors.Error { + errorCodeIdx += 1 + return sdkerrors.Register("ante-nibiru", errorCodeIdx, errMsg) +} + +// app/ante "sentinel" errors +var ( + ErrOracleAnte = registerError("oracle ante error") + ErrMaxValidatorCommission = registerError("validator commission rate is above max") +) + +func NewErrMaxValidatorCommission(gotCommission sdk.Dec) error { + return ErrMaxValidatorCommission.Wrapf( + "got (%s), max rate is (%s)", gotCommission, MAX_COMMISSION()) +} diff --git a/app/ante/fixed_gas.go b/app/ante/fixed_gas.go index 179ed0569..d1f4ca7ad 100644 --- a/app/ante/fixed_gas.go +++ b/app/ante/fixed_gas.go @@ -3,7 +3,6 @@ package ante import ( sdkerrors "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/errors" oracletypes "github.com/NibiruChain/nibiru/x/oracle/types" ) @@ -41,13 +40,13 @@ func (gd EnsureSinglePostPriceMessageDecorator) AnteHandle( if hasOracleVoteMsg && hasOraclePreVoteMsg { if len(msgs) > 2 { - return ctx, sdkerrors.Wrap(errors.ErrInvalidRequest, "a transaction cannot have more than a single oracle vote and prevote message") + return ctx, sdkerrors.Wrap(ErrOracleAnte, "a transaction cannot have more than a single oracle vote and prevote message") } ctx = ctx.WithGasMeter(NewFixedGasMeter(OracleMessageGas)) } else if hasOraclePreVoteMsg || hasOracleVoteMsg { if len(msgs) > 1 { - return ctx, sdkerrors.Wrap(errors.ErrInvalidRequest, "a transaction that includes an oracle vote or prevote message cannot have more than those two messages") + return ctx, sdkerrors.Wrap(ErrOracleAnte, "a transaction that includes an oracle vote or prevote message cannot have more than those two messages") } ctx = ctx.WithGasMeter(NewFixedGasMeter(OracleMessageGas)) diff --git a/app/ante/fixed_gas_test.go b/app/ante/fixed_gas_test.go index 6583ca9a4..30d9f0bef 100644 --- a/app/ante/fixed_gas_test.go +++ b/app/ante/fixed_gas_test.go @@ -3,9 +3,9 @@ package ante_test import ( "testing" - "github.com/cosmos/cosmos-sdk/types/errors" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - sdkerrors "cosmossdk.io/errors" + sdkioerrors "cosmossdk.io/errors" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" @@ -68,7 +68,7 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { }, }, expectedGas: 1042, - expectedErr: sdkerrors.Wrap(errors.ErrInvalidRequest, "a transaction that includes an oracle vote or prevote message cannot have more than those two messages"), + expectedErr: sdkioerrors.Wrap(ante.ErrOracleAnte, "a transaction that includes an oracle vote or prevote message cannot have more than those two messages"), }, { name: "Two messages in a transaction, one of them is an oracle vote message should fail (with MsgAggregateExchangeRatePrevote) permutation 2", @@ -85,7 +85,7 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { }, }, expectedGas: 1042, - expectedErr: sdkerrors.Wrap(errors.ErrInvalidRequest, "a transaction that includes an oracle vote or prevote message cannot have more than those two messages"), + expectedErr: sdkioerrors.Wrap(ante.ErrOracleAnte, "a transaction that includes an oracle vote or prevote message cannot have more than those two messages"), }, { name: "Two messages in a transaction, one of them is an oracle vote message should fail (with MsgAggregateExchangeRateVote)", @@ -103,7 +103,7 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { }, }, expectedGas: 1042, - expectedErr: sdkerrors.Wrap(errors.ErrInvalidRequest, "a transaction that includes an oracle vote or prevote message cannot have more than those two messages"), + expectedErr: sdkioerrors.Wrap(ante.ErrOracleAnte, "a transaction that includes an oracle vote or prevote message cannot have more than those two messages"), }, { name: "Two messages in a transaction, one of them is an oracle vote message should fail (with MsgAggregateExchangeRateVote) permutation 2", @@ -121,7 +121,7 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { }, }, expectedGas: 1042, - expectedErr: sdkerrors.Wrap(errors.ErrInvalidRequest, "a transaction that includes an oracle vote or prevote message cannot have more than those two messages"), + expectedErr: sdkioerrors.Wrap(ante.ErrOracleAnte, "a transaction that includes an oracle vote or prevote message cannot have more than those two messages"), }, { name: "Two messages in a transaction, one is oracle vote, the other oracle pre vote: should work with fixed price", @@ -180,7 +180,7 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { }, }, expectedGas: 1042, - expectedErr: sdkerrors.Wrap(errors.ErrInvalidRequest, "a transaction cannot have more than a single oracle vote and prevote message"), + expectedErr: sdkioerrors.Wrap(ante.ErrOracleAnte, "a transaction cannot have more than a single oracle vote and prevote message"), }, { name: "Other two messages", @@ -196,7 +196,7 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { Amount: sdk.NewCoins(sdk.NewInt64Coin(app.BondDenom, 200)), }, }, - expectedGas: 62504, + expectedGas: 62288, expectedErr: nil, }, } @@ -249,7 +249,7 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { func (s *AnteTestSuite) ValidateTx(tx signing.Tx, t *testing.T) { memoTx, ok := tx.(sdk.TxWithMemo) if !ok { - s.Fail(sdkerrors.Wrap(errors.ErrTxDecode, "invalid transaction type").Error(), "memoTx: %t", memoTx) + s.Fail(sdkioerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type").Error(), "memoTx: %t", memoTx) } params := s.app.AccountKeeper.GetParams(s.ctx) diff --git a/app/ante/testutil_test.go b/app/ante/testutil_test.go index 6ed01b2eb..d18aa8ffb 100644 --- a/app/ante/testutil_test.go +++ b/app/ante/testutil_test.go @@ -19,7 +19,7 @@ import ( tmproto "github.com/cometbft/cometbft/proto/tendermint/types" "github.com/NibiruChain/nibiru/app" - feeante "github.com/NibiruChain/nibiru/app/ante" + nibiruante "github.com/NibiruChain/nibiru/app/ante" "github.com/NibiruChain/nibiru/x/common/testutil/genesis" "github.com/NibiruChain/nibiru/x/common/testutil/testapp" ) @@ -38,6 +38,7 @@ type AnteTestSuite struct { // SetupTest setups a new test, with new app, context, and anteHandler. func (suite *AnteTestSuite) SetupTest() { // Set up base app and ctx + testapp.EnsureNibiruPrefix() encodingConfig := genesis.TEST_ENCODING_CONFIG suite.app = testapp.NewNibiruTestApp(app.NewDefaultGenesisState(encodingConfig.Marshaler)) chainId := "test-chain-id" @@ -66,7 +67,8 @@ func (suite *AnteTestSuite) SetupTest() { ante.NewValidateBasicDecorator(), ante.NewTxTimeoutHeightDecorator(), ante.NewValidateMemoDecorator(suite.app.AccountKeeper), - feeante.NewPostPriceFixedPriceDecorator(), + nibiruante.NewPostPriceFixedPriceDecorator(), + nibiruante.AnteDecoratorStakingCommission{}, ante.NewConsumeGasForTxSizeDecorator(suite.app.AccountKeeper), ante.NewDeductFeeDecorator(suite.app.AccountKeeper, suite.app.BankKeeper, suite.app.FeeGrantKeeper, nil), // Replace fee ante from cosmos auth with a custom one. diff --git a/x/common/testutil/cli/network.go b/x/common/testutil/cli/network.go index 7a693f3fe..1d8a436cd 100644 --- a/x/common/testutil/cli/network.go +++ b/x/common/testutil/cli/network.go @@ -371,7 +371,7 @@ func New(logger Logger, baseDir string, cfg Config) (*Network, error) { genBalances = append(genBalances, banktypes.Balance{Address: addr.String(), Coins: balances.Sort()}) genAccounts = append(genAccounts, authtypes.NewBaseAccount(addr, nil, 0, 0)) - commission, err := sdk.NewDecFromStr("0.5") + commission, err := sdk.NewDecFromStr("0.05") if err != nil { return nil, err } diff --git a/x/tokenfactory/keeper/grpc_query_test.go b/x/tokenfactory/keeper/grpc_query_test.go index 652f0e8de..8e47da052 100644 --- a/x/tokenfactory/keeper/grpc_query_test.go +++ b/x/tokenfactory/keeper/grpc_query_test.go @@ -5,7 +5,7 @@ import ( ) func (s *TestSuite) TestQueryModuleParams() { - res, err := s.queryClient.Params(s.GoCtx(), &types.QueryParamsRequest{}) + res, err := s.querier.Params(s.GoCtx(), &types.QueryParamsRequest{}) s.NoError(err) - s.Equal(*res, types.DefaultModuleParams()) + s.Equal(res.Params, types.DefaultModuleParams()) } diff --git a/x/tokenfactory/keeper/keeper_test.go b/x/tokenfactory/keeper/keeper_test.go index 438820430..d8d5aac85 100644 --- a/x/tokenfactory/keeper/keeper_test.go +++ b/x/tokenfactory/keeper/keeper_test.go @@ -12,7 +12,6 @@ import ( "github.com/NibiruChain/nibiru/app" "github.com/NibiruChain/nibiru/x/common/testutil/testapp" - "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -24,9 +23,8 @@ type TestSuite struct { ctx sdk.Context app *app.NibiruApp - keeper tfkeeper.Keeper - queryClient tftypes.QueryClient - // msgServer tftypes.MsgServer // TODO when txs are added. + keeper tfkeeper.Keeper + querier tfkeeper.Querier genesis tftypes.GenesisState } @@ -44,9 +42,9 @@ func (s *TestSuite) SetupTest() { s.keeper = s.app.TokenFactoryKeeper s.genesis = *tftypes.DefaultGenesis() - queryGrpcHelper := baseapp.NewQueryServerTestHelper( - s.ctx, s.app.InterfaceRegistry()) - s.queryClient = tftypes.NewQueryClient(queryGrpcHelper) + s.querier = tfkeeper.Querier{ + Keeper: s.app.TokenFactoryKeeper, + } } func (s *TestSuite) GoCtx() context.Context { return sdk.WrapSDKContext(s.ctx) }