From a953aa4de486012af029e4d52bc12a5c9e288b87 Mon Sep 17 00:00:00 2001 From: Yurist-85 Date: Tue, 30 Jan 2024 17:44:43 +0800 Subject: [PATCH] v1.7.1 - Fix vesting ante (#277) * fix: delegate vesting coins * chore: minor changes * fix: remove redundant check * test: add some tests for vesting delegation * chore: add upgrade handler and bump version --- Makefile | 2 +- app/ante/cosmos/eip712.go | 6 ++--- app/ante/cosmos/fees.go | 2 +- app/ante/cosmos/min_price_test.go | 4 +-- app/ante/cosmos/vesting.go | 19 +++++++------- app/ante/evm/ante_test.go | 24 +++++++++--------- app/ante/evm/fee_checker.go | 8 +++--- app/ante/evm/fees.go | 4 +-- app/ante/handler_options.go | 7 +++--- app/ante/sigverify.go | 2 +- app/app.go | 7 ++++++ app/upgrades/v1.7.1/constants.go | 6 +++++ app/upgrades/v1.7.1/upgrades.go | 20 +++++++++++++++ x/vesting/keeper/integration_test.go | 37 +++++++++++++++++++++++++--- x/vesting/keeper/utils_test.go | 2 +- 15 files changed, 106 insertions(+), 44 deletions(-) create mode 100644 app/upgrades/v1.7.1/constants.go create mode 100644 app/upgrades/v1.7.1/upgrades.go diff --git a/Makefile b/Makefile index 852123ff..39cc1097 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ DIFF_TAG=$(shell git rev-list --tags="v*" --max-count=1 --not $(shell git rev-li DEFAULT_TAG=$(shell git rev-list --tags="v*" --max-count=1) # VERSION ?= $(shell echo $(shell git describe --tags $(or $(DIFF_TAG), $(DEFAULT_TAG))) | sed 's/^v//') -VERSION := "1.7.0" +VERSION := "1.7.1" CBFTVERSION := $(shell go list -m github.com/cometbft/cometbft | sed 's:.* ::') COMMIT := $(shell git log -1 --format='%H') LEDGER_ENABLED ?= true diff --git a/app/ante/cosmos/eip712.go b/app/ante/cosmos/eip712.go index b3ce722e..7980b01d 100644 --- a/app/ante/cosmos/eip712.go +++ b/app/ante/cosmos/eip712.go @@ -23,12 +23,12 @@ import ( evmtypes "github.com/haqq-network/haqq/x/evm/types" ) -var evmosCodec codec.ProtoCodecMarshaler +var protoCodec codec.ProtoCodecMarshaler func init() { registry := codectypes.NewInterfaceRegistry() types.RegisterInterfaces(registry) - evmosCodec = codec.NewProtoCodec(registry) + protoCodec = codec.NewProtoCodec(registry) } // Deprecated: LegacyEip712SigVerificationDecorator Verify all signatures for a tx and return an error if any are invalid. Note, @@ -223,7 +223,7 @@ func VerifySignature( FeePayer: feePayer, } - typedData, err := eip712.LegacyWrapTxToTypedData(evmosCodec, extOpt.TypedDataChainID, msgs[0], txBytes, feeDelegation) + typedData, err := eip712.LegacyWrapTxToTypedData(protoCodec, extOpt.TypedDataChainID, msgs[0], txBytes, feeDelegation) if err != nil { return errorsmod.Wrap(err, "failed to create EIP-712 typed data from tx") } diff --git a/app/ante/cosmos/fees.go b/app/ante/cosmos/fees.go index 02b06c25..a1ed7638 100644 --- a/app/ante/cosmos/fees.go +++ b/app/ante/cosmos/fees.go @@ -127,7 +127,7 @@ func (dfd DeductFeeDecorator) deductFee(ctx sdk.Context, sdkTx sdk.Tx, fees sdk. // deduct the fees if err := deductFeesFromBalanceOrUnclaimedStakingRewards(ctx, dfd, deductFeesFromAcc, fees); err != nil { - return fmt.Errorf("insufficient funds and failed to claim sufficient staking rewards to pay for fees: %w", err) + return fmt.Errorf("%q has insufficient funds and failed to claim sufficient staking rewards to pay for fees: %w", deductFeesFrom.String(), err) } events := sdk.Events{ diff --git a/app/ante/cosmos/min_price_test.go b/app/ante/cosmos/min_price_test.go index d55ff9cf..ef24f7cc 100644 --- a/app/ante/cosmos/min_price_test.go +++ b/app/ante/cosmos/min_price_test.go @@ -23,8 +23,8 @@ var execTypes = []struct { func (suite *AnteTestSuite) TestMinGasPriceDecorator() { denom := utils.BaseDenom testMsg := banktypes.MsgSend{ - FromAddress: "evmos1x8fhpj9nmhqk8z9kpgjt95ck2xwyue0ptzkucp", - ToAddress: "evmos1dx67l23hz9l0k9hcher8xz04uj7wf3yu26l2yn", + FromAddress: "haqq1tjdjfavsy956d25hvhs3p0nw9a7pfghqm0up92", + ToAddress: "haqq1hdr0lhv75vesvtndlh78ck4cez6esz8u2lk0hq", Amount: sdk.Coins{sdk.Coin{Amount: sdkmath.NewInt(10), Denom: denom}}, } diff --git a/app/ante/cosmos/vesting.go b/app/ante/cosmos/vesting.go index 036178e2..5e1a3a76 100644 --- a/app/ante/cosmos/vesting.go +++ b/app/ante/cosmos/vesting.go @@ -18,14 +18,16 @@ import ( type VestingDelegationDecorator struct { ak evmtypes.AccountKeeper sk vestingtypes.StakingKeeper + bk evmtypes.BankKeeper cdc codec.BinaryCodec } // NewVestingDelegationDecorator creates a new VestingDelegationDecorator -func NewVestingDelegationDecorator(ak evmtypes.AccountKeeper, sk vestingtypes.StakingKeeper, cdc codec.BinaryCodec) VestingDelegationDecorator { +func NewVestingDelegationDecorator(ak evmtypes.AccountKeeper, sk vestingtypes.StakingKeeper, bk evmtypes.BankKeeper, cdc codec.BinaryCodec) VestingDelegationDecorator { return VestingDelegationDecorator{ ak: ak, sk: sk, + bk: bk, cdc: cdc, } } @@ -91,19 +93,18 @@ func (vdd VestingDelegationDecorator) validateMsg(ctx sdk.Context, msg sdk.Msg) // error if bond amount is > vested coins bondDenom := vdd.sk.BondDenom(ctx) - coins := clawbackAccount.GetVestedOnly(ctx.BlockTime()) - if coins == nil || coins.Empty() { - return errorsmod.Wrap( - vestingtypes.ErrInsufficientVestedCoins, - "account has no vested coins", - ) + balance := vdd.bk.GetBalance(ctx, addr, bondDenom) + unvestedOnly := clawbackAccount.GetUnvestedOnly(ctx.BlockTime()) + spendable, hasNeg := sdk.Coins{balance}.SafeSub(unvestedOnly...) + if hasNeg { + spendable = sdk.NewCoins() } - vested := coins.AmountOf(bondDenom) + vested := spendable.AmountOf(bondDenom) if vested.LT(delegateMsg.Amount.Amount) { return errorsmod.Wrapf( vestingtypes.ErrInsufficientVestedCoins, - "cannot delegate unvested coins. coins vested < delegation amount (%s < %s)", + "cannot delegate unvested coins. coins stakable < delegation amount (%s < %s)", vested, delegateMsg.Amount.Amount, ) } diff --git a/app/ante/evm/ante_test.go b/app/ante/evm/ante_test.go index d8b97ec8..11bd22f4 100644 --- a/app/ante/evm/ante_test.go +++ b/app/ante/evm/ante_test.go @@ -547,7 +547,7 @@ func (suite *AnteTestSuite) TestAnteHandler() { from := acc.GetAddress() gas := uint64(200000) amount := sdk.NewCoins(sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewInt(100*int64(gas)))) - txBuilder, err := suite.CreateTestEIP712TxBuilderMsgSend(from, privKey, "evmos_9002-1", gas, amount) + txBuilder, err := suite.CreateTestEIP712TxBuilderMsgSend(from, privKey, "haqq_121799-1", gas, amount) suite.Require().NoError(err) return txBuilder.GetTx() }, false, false, false, @@ -571,7 +571,7 @@ func (suite *AnteTestSuite) TestAnteHandler() { from := acc.GetAddress() gas := uint64(200000) amount := sdk.NewCoins(sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewInt(100*int64(gas)))) - txBuilder, err := suite.CreateTestEIP712TxBuilderMsgSend(from, privKey, "evmos_9001-1", gas, amount) + txBuilder, err := suite.CreateTestEIP712TxBuilderMsgSend(from, privKey, "haqq_121799-1", gas, amount) suite.Require().NoError(err) return txBuilder.GetTx() }, false, false, false, @@ -640,7 +640,7 @@ func (suite *AnteTestSuite) TestAnteHandler() { addr[:], sdk.NewCoins( sdk.NewCoin( - "evmos", + "ISLM", sdk.NewInt(1), ), ), @@ -670,7 +670,7 @@ func (suite *AnteTestSuite) TestAnteHandler() { addr[:], sdk.NewCoins( sdk.NewCoin( - "evmos", + "ISLM", sdk.NewInt(1), ), ), @@ -700,7 +700,7 @@ func (suite *AnteTestSuite) TestAnteHandler() { addr[:], sdk.NewCoins( sdk.NewCoin( - "evmos", + "ISLM", sdk.NewInt(1), ), ), @@ -755,7 +755,7 @@ func (suite *AnteTestSuite) TestAnteHandler() { addr[:], sdk.NewCoins( sdk.NewCoin( - "evmos", + "ISLM", sdk.NewInt(1), ), ), @@ -765,7 +765,7 @@ func (suite *AnteTestSuite) TestAnteHandler() { privKeys, signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, msg, - "evmos_9005-1", + "haqq_121799-1", 2000000, "mixed", ) @@ -785,7 +785,7 @@ func (suite *AnteTestSuite) TestAnteHandler() { addr[:], sdk.NewCoins( sdk.NewCoin( - "evmos", + "ISLM", sdk.NewInt(1), ), ), @@ -815,7 +815,7 @@ func (suite *AnteTestSuite) TestAnteHandler() { addr[:], sdk.NewCoins( sdk.NewCoin( - "evmos", + "ISLM", sdk.NewInt(1), ), ), @@ -845,7 +845,7 @@ func (suite *AnteTestSuite) TestAnteHandler() { addr[:], sdk.NewCoins( sdk.NewCoin( - "evmos", + "ISLM", sdk.NewInt(1), ), ), @@ -879,7 +879,7 @@ func (suite *AnteTestSuite) TestAnteHandler() { addr[:], sdk.NewCoins( sdk.NewCoin( - "evmos", + "ISLM", sdk.NewInt(1), ), ), @@ -909,7 +909,7 @@ func (suite *AnteTestSuite) TestAnteHandler() { addr[:], sdk.NewCoins( sdk.NewCoin( - "evmos", + "ISLM", sdk.NewInt(1), ), ), diff --git a/app/ante/evm/fee_checker.go b/app/ante/evm/fee_checker.go index 61094a50..d6a9e129 100644 --- a/app/ante/evm/fee_checker.go +++ b/app/ante/evm/fee_checker.go @@ -11,7 +11,7 @@ import ( anteutils "github.com/haqq-network/haqq/app/ante/utils" haqqtypes "github.com/haqq-network/haqq/types" - "github.com/haqq-network/haqq/x/evm/types" + evmtypes "github.com/haqq-network/haqq/x/evm/types" ) // NewDynamicFeeChecker returns a `TxFeeChecker` that applies a dynamic fee to @@ -69,7 +69,7 @@ func NewDynamicFeeChecker(k DynamicFeeEVMKeeper) anteutils.TxFeeChecker { } // calculate the effective gas price using the EIP-1559 logic. - effectivePrice := sdkmath.NewIntFromBigInt(types.EffectiveGasPrice(baseFeeInt.BigInt(), feeCap.BigInt(), maxPriorityPrice.BigInt())) + effectivePrice := sdkmath.NewIntFromBigInt(evmtypes.EffectiveGasPrice(baseFeeInt.BigInt(), feeCap.BigInt(), maxPriorityPrice.BigInt())) // NOTE: create a new coins slice without having to validate the denom effectiveFee := sdk.Coins{ @@ -79,7 +79,7 @@ func NewDynamicFeeChecker(k DynamicFeeEVMKeeper) anteutils.TxFeeChecker { }, } - bigPriority := effectivePrice.Sub(baseFeeInt).Quo(types.DefaultPriorityReduction) + bigPriority := effectivePrice.Sub(baseFeeInt).Quo(evmtypes.DefaultPriorityReduction) priority := int64(math.MaxInt64) if bigPriority.IsInt64() { @@ -127,7 +127,7 @@ func getTxPriority(fees sdk.Coins, gas int64) int64 { for _, fee := range fees { gasPrice := fee.Amount.QuoRaw(gas) - amt := gasPrice.Quo(types.DefaultPriorityReduction) + amt := gasPrice.Quo(evmtypes.DefaultPriorityReduction) p := int64(math.MaxInt64) if amt.IsInt64() { diff --git a/app/ante/evm/fees.go b/app/ante/evm/fees.go index db2ccd1e..a1e40206 100644 --- a/app/ante/evm/fees.go +++ b/app/ante/evm/fees.go @@ -98,8 +98,8 @@ func (empd EthMinGasPriceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul if fee.LT(requiredFee) { return ctx, errorsmod.Wrapf( errortypes.ErrInsufficientFee, - "provided fee < minimum global fee (%d < %d). Please increase the priority tip (for EIP-1559 txs) or the gas prices (for access list or legacy txs)", //nolint:lll - fee.TruncateInt().Int64(), requiredFee.TruncateInt().Int64(), + "provided fee < minimum global fee (%s < %s). Please increase the priority tip (for EIP-1559 txs) or the gas prices (for access list or legacy txs)", //nolint:lll + fee.TruncateInt().String(), requiredFee.TruncateInt().String(), ) } } diff --git a/app/ante/handler_options.go b/app/ante/handler_options.go index 80b4670f..d91ea234 100644 --- a/app/ante/handler_options.go +++ b/app/ante/handler_options.go @@ -20,8 +20,7 @@ import ( vestingtypes "github.com/haqq-network/haqq/x/vesting/types" ) -// HandlerOptions defines the list of module keepers required to run the Evmos -// AnteHandler decorators. +// HandlerOptions defines the list of module keepers required to run the Haqq AnteHandler decorators. type HandlerOptions struct { Cdc codec.BinaryCodec AccountKeeper evmtypes.AccountKeeper @@ -115,7 +114,7 @@ func newCosmosAnteHandler(options HandlerOptions) sdk.AnteHandler { cosmosante.NewMinGasPriceDecorator(options.FeeMarketKeeper, options.EvmKeeper), ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper), cosmosante.NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.DistributionKeeper, options.FeegrantKeeper, options.StakingKeeper, options.TxFeeChecker), - cosmosante.NewVestingDelegationDecorator(options.AccountKeeper, options.StakingKeeper, options.Cdc), + cosmosante.NewVestingDelegationDecorator(options.AccountKeeper, options.StakingKeeper, options.BankKeeper, options.Cdc), // SetPubKeyDecorator must be called before all signature verification decorators ante.NewSetPubKeyDecorator(options.AccountKeeper), ante.NewValidateSigCountDecorator(options.AccountKeeper), @@ -142,7 +141,7 @@ func newLegacyCosmosAnteHandlerEip712(options HandlerOptions) sdk.AnteHandler { ante.NewValidateMemoDecorator(options.AccountKeeper), ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper), cosmosante.NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.DistributionKeeper, options.FeegrantKeeper, options.StakingKeeper, options.TxFeeChecker), - cosmosante.NewVestingDelegationDecorator(options.AccountKeeper, options.StakingKeeper, options.Cdc), + cosmosante.NewVestingDelegationDecorator(options.AccountKeeper, options.StakingKeeper, options.BankKeeper, options.Cdc), // SetPubKeyDecorator must be called before all signature verification decorators ante.NewSetPubKeyDecorator(options.AccountKeeper), ante.NewValidateSigCountDecorator(options.AccountKeeper), diff --git a/app/ante/sigverify.go b/app/ante/sigverify.go index e86adb22..8da1f5d2 100644 --- a/app/ante/sigverify.go +++ b/app/ante/sigverify.go @@ -21,7 +21,7 @@ const ( Secp256k1VerifyCost uint64 = 21000 ) -// SigVerificationGasConsumer is the Evmos implementation of SignatureVerificationGasConsumer. It consumes gas +// SigVerificationGasConsumer is the Haqq implementation of SignatureVerificationGasConsumer. It consumes gas // for signature verification based upon the public key type. The cost is fetched from the given params and is matched // by the concrete type. // The types of keys supported are: diff --git a/app/app.go b/app/app.go index c3acd901..6ea119c7 100644 --- a/app/app.go +++ b/app/app.go @@ -156,6 +156,7 @@ import ( v163 "github.com/haqq-network/haqq/app/upgrades/v1.6.3" v164 "github.com/haqq-network/haqq/app/upgrades/v1.6.4" v170 "github.com/haqq-network/haqq/app/upgrades/v1.7.0" + v171 "github.com/haqq-network/haqq/app/upgrades/v1.7.1" // NOTE: override ICS20 keeper to support IBC transfers of ERC20 tokens "github.com/haqq-network/haqq/x/ibc/transfer" @@ -1189,6 +1190,12 @@ func (app *Haqq) setupUpgradeHandlers() { ), ) + // v1.7.1 Fix Vesting AnteHandler + app.UpgradeKeeper.SetUpgradeHandler( + v171.UpgradeName, + v171.CreateUpgradeHandler(app.mm, app.configurator), + ) + // When a planned update height is reached, the old binary will panic // writing on disk the height and name of the update that triggered it // This will read that value, and execute the preparations for the upgrade. diff --git a/app/upgrades/v1.7.1/constants.go b/app/upgrades/v1.7.1/constants.go new file mode 100644 index 00000000..e07a354d --- /dev/null +++ b/app/upgrades/v1.7.1/constants.go @@ -0,0 +1,6 @@ +package v171 + +const ( + // UpgradeName is the shared upgrade plan name for mainnet and testnet + UpgradeName = "v1.7.1" +) diff --git a/app/upgrades/v1.7.1/upgrades.go b/app/upgrades/v1.7.1/upgrades.go new file mode 100644 index 00000000..0fa63a2d --- /dev/null +++ b/app/upgrades/v1.7.1/upgrades.go @@ -0,0 +1,20 @@ +package v171 + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/module" + upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" +) + +// CreateUpgradeHandler creates an SDK upgrade handler for v1.7.1 +func CreateUpgradeHandler( + mm *module.Manager, + configurator module.Configurator, +) upgradetypes.UpgradeHandler { + return func(ctx sdk.Context, _ upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) { + logger := ctx.Logger() + logger.Info("run migration v1.7.1") + + return mm.RunMigrations(ctx, configurator, vm) + } +} diff --git a/x/vesting/keeper/integration_test.go b/x/vesting/keeper/integration_test.go index 2d0bef3c..34dc080b 100644 --- a/x/vesting/keeper/integration_test.go +++ b/x/vesting/keeper/integration_test.go @@ -150,15 +150,28 @@ var _ = Describe("Clawback Vesting Accounts", Ordered, func() { vested := clawbackAccount.GetVestedOnly(s.ctx.BlockTime()) unlocked := clawbackAccount.GetUnlockedOnly(s.ctx.BlockTime()) zeroCoins := sdk.NewCoins(sdk.NewCoin(stakeDenom, sdk.ZeroInt())) - s.Require().Equal(zeroCoins, vested) - s.Require().Equal(zeroCoins, unlocked) + s.Require().Equal(zeroCoins.String(), vested.String()) + s.Require().Equal(zeroCoins.String(), unlocked.String()) }) - It("cannot delegate tokens", func() { - err := delegate(clawbackAccount, math.NewInt(100), s.validator.GetOperator().String()) + It("cannot delegate unvested tokens", func() { + // Try to delegate more than available spendable balance + // All vesting tokens are unvested at this point + // Only 1e16 aISLM is available for gas coverage + err := delegate(clawbackAccount, math.NewInt(1e18), s.validator.GetOperator().String()) + // Must fail due to insufficient balance Expect(err).ToNot(BeNil()) }) + It("can delegate spendable tokens", func() { + // Try to delegate available spendable balance + // All vesting tokens are unvested at this point + // Only 1e16 aISLM is available for gas coverage + err := delegate(clawbackAccount, math.NewInt(1e15), s.validator.GetOperator().String()) + // Must succeed as spendable balance is sufficient + Expect(err).To(BeNil()) + }) + It("can transfer spendable tokens", func() { account := testAccounts[0] // Fund account with new spendable tokens @@ -215,6 +228,7 @@ var _ = Describe("Clawback Vesting Accounts", Ordered, func() { // Check if some, but not all tokens are vested vested = clawbackAccount.GetVestedOnly(s.ctx.BlockTime()) + unvested = clawbackAccount.GetUnvestedOnly(s.ctx.BlockTime()) expVested := sdk.NewCoins(sdk.NewCoin(stakeDenom, amt.Mul(sdk.NewInt(cliff)))) s.Require().NotEqual(vestingAmtTotal, vested) s.Require().Equal(expVested, vested) @@ -225,6 +239,21 @@ var _ = Describe("Clawback Vesting Accounts", Ordered, func() { Expect(err).To(BeNil()) }) + It("can delegate vested and own tokens", func() { + // Try to delegate more than vested amount + // 1e16 aISLM is available for gas coverage + // So there's enough to delegate vested and own tokens + vestedIslm := vested.AmountOf(stakeDenom) + unvestedIslm := unvested.AmountOf(stakeDenom) + // Fund account with new spendable tokens + err := testutil.FundAccount(s.ctx, s.app.BankKeeper, clawbackAccount.GetAddress(), unvested) + Expect(err).To(BeNil()) + + err = delegate(clawbackAccount, vestedIslm.Add(unvestedIslm), s.validator.GetOperator().String()) + // Must succeed as spendable balance is sufficient + Expect(err).To(BeNil()) + }) + It("cannot delegate unvested tokens", func() { err := delegate(clawbackAccount, vestingAmtTotal.AmountOf(stakeDenom), s.validator.GetOperator().String()) Expect(err).ToNot(BeNil()) diff --git a/x/vesting/keeper/utils_test.go b/x/vesting/keeper/utils_test.go index 1d7d22e2..90c2ee08 100644 --- a/x/vesting/keeper/utils_test.go +++ b/x/vesting/keeper/utils_test.go @@ -222,7 +222,7 @@ func delegate(clawbackAccount *types.ClawbackVestingAccount, amount sdkmath.Int, s.Require().NoError(err) delegateMsg := stakingtypes.NewMsgDelegate(addr, val, sdk.NewCoin(s.app.StakingKeeper.BondDenom(s.ctx), amount)) - dec := cosmosante.NewVestingDelegationDecorator(s.app.AccountKeeper, s.app.StakingKeeper, types.ModuleCdc) + dec := cosmosante.NewVestingDelegationDecorator(s.app.AccountKeeper, s.app.StakingKeeper, s.app.BankKeeper, types.ModuleCdc) err = testutil.ValidateAnteForMsgs(s.ctx, dec, delegateMsg) return err }