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: reduce gas for first vote by validators #2163

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ preparation for a second audit starting in November 2024.
- [#1755](https://github.com/NibiruChain/nibiru/pull/1755) - feat(oracle): Add more events on validator's performance
- [#1764](https://github.com/NibiruChain/nibiru/pull/1764) - fix(perp): make updateswapinvariant aware of total short supply to avoid panics
- [#1710](https://github.com/NibiruChain/nibiru/pull/1710) - refactor(perp): Clean and organize module errors for x/perp
- [#2163](https://github.com/NibiruChain/nibiru/pull/1710) - feat: reduce gas for first vote by validators

### Non-breaking/Compatible Improvements

Expand Down
3 changes: 3 additions & 0 deletions app/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (

authante "github.com/cosmos/cosmos-sdk/x/auth/ante"

oracleante "github.com/NibiruChain/nibiru/v2/x/oracle/ante"

"github.com/NibiruChain/nibiru/v2/app/ante"
"github.com/NibiruChain/nibiru/v2/app/evmante"
devgasante "github.com/NibiruChain/nibiru/v2/x/devgas/v1/ante"
Expand Down Expand Up @@ -71,6 +73,7 @@ func NewAnteHandlerNonEVM(
// ticket: https://github.com/NibiruChain/nibiru/issues/1915
authante.NewExtensionOptionsDecorator(opts.ExtensionOptionChecker),
authante.NewValidateBasicDecorator(),
oracleante.NewVoteFeeDiscountDecorator(opts.OracleKeeper, opts.StakingKeeper),
authante.NewTxTimeoutHeightDecorator(),
authante.NewValidateMemoDecorator(opts.AccountKeeper),
ante.AnteDecoratorEnsureSinglePostPriceMessage{},
Expand Down
6 changes: 6 additions & 0 deletions app/ante/handler_opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ import (
authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
ibckeeper "github.com/cosmos/ibc-go/v7/modules/core/keeper"

stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"

devgasante "github.com/NibiruChain/nibiru/v2/x/devgas/v1/ante"
devgaskeeper "github.com/NibiruChain/nibiru/v2/x/devgas/v1/keeper"
evmkeeper "github.com/NibiruChain/nibiru/v2/x/evm/keeper"
oraclekeeper "github.com/NibiruChain/nibiru/v2/x/oracle/keeper"
)

type AnteHandlerOptions struct {
Expand All @@ -23,6 +26,9 @@ type AnteHandlerOptions struct {
EvmKeeper *evmkeeper.Keeper
AccountKeeper authkeeper.AccountKeeper

OracleKeeper oraclekeeper.Keeper
StakingKeeper stakingkeeper.Keeper

TxCounterStoreKey types.StoreKey
WasmConfig *wasmtypes.WasmConfig
MaxTxGasWanted uint64
Expand Down
9 changes: 6 additions & 3 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,10 @@ func NewNibiruApp(
app.SetBeginBlocker(app.BeginBlocker)
anteHandler := NewAnteHandler(app.AppKeepers, ante.AnteHandlerOptions{
HandlerOptions: authante.HandlerOptions{
AccountKeeper: app.AccountKeeper,
BankKeeper: app.BankKeeper,
FeegrantKeeper: app.FeeGrantKeeper,
AccountKeeper: app.AccountKeeper,
BankKeeper: app.BankKeeper,
FeegrantKeeper: app.FeeGrantKeeper,

SignModeHandler: encodingConfig.TxConfig.SignModeHandler(),
SigGasConsumer: authante.DefaultSigVerificationGasConsumer,
ExtensionOptionChecker: func(*codectypes.Any) bool { return true },
Expand All @@ -228,6 +229,8 @@ func NewNibiruApp(
WasmConfig: &wasmConfig,
DevGasKeeper: &app.DevGasKeeper,
DevGasBankKeeper: app.BankKeeper,
StakingKeeper: *app.StakingKeeper,
OracleKeeper: app.OracleKeeper,
// TODO: feat(evm): enable app/server/config flag for Evm MaxTxGasWanted.
MaxTxGasWanted: DefaultMaxTxGasWanted,
EvmKeeper: app.EvmKeeper,
Expand Down
38 changes: 38 additions & 0 deletions x/common/testutil/testapp/testapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/cometbft/cometbft/libs/log"
tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
"github.com/cosmos/cosmos-sdk/testutil/sims"
sdk "github.com/cosmos/cosmos-sdk/types"
auth "github.com/cosmos/cosmos-sdk/x/auth/types"
Expand Down Expand Up @@ -156,6 +157,43 @@ func NewNibiruTestApp(gen app.GenesisState, baseAppOptions ...func(*baseapp.Base
return app
}

// AddTestAddrsIncremental generates `numAddrs` new addresses in a simple
// incremental manner, adds them as BaseAccounts in the AccountKeeper, and
// optionally funds them with `coins`. It returns their addresses as
// `[]sdk.ValAddress`, which you can treat as potential validators if needed.
func AddTestAddrsIncremental(
nibiruApp *app.NibiruApp,
ctx sdk.Context,
numAddrs int,
coins sdk.Coins,
) []sdk.ValAddress {
var valAddrs []sdk.ValAddress

for i := 0; i < numAddrs; i++ {
// 1. Generate a new private key
pk := secp256k1.GenPrivKey()

// 2. Derive an account address from the pubkey
accAddr := sdk.AccAddress(pk.PubKey().Address())

// 3. Create a new base account object
acc := nibiruApp.AccountKeeper.NewAccountWithAddress(ctx, accAddr)
nibiruApp.AccountKeeper.SetAccount(ctx, acc)

// 4. If the function call passed a non-empty `coins`, fund the new account
if !coins.IsZero() {
if err := FundAccount(nibiruApp.BankKeeper, ctx, accAddr, coins); err != nil {
panic(err)
}
}

// 5. Convert to ValAddress for convenience
valAddrs = append(valAddrs, sdk.ValAddress(accAddr))
}

return valAddrs
}

// FundAccount is a utility function that funds an account by minting and
// sending the coins to the address. This should be used for testing purposes
// only!
Expand Down
102 changes: 102 additions & 0 deletions x/oracle/ante/fee_discount.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package ante

import (
sdkerrors "cosmossdk.io/errors"
"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
errortypes "github.com/cosmos/cosmos-sdk/types/errors"
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"

"github.com/NibiruChain/nibiru/v2/x/oracle/types"
)

// VoteFeeDiscountDecorator checks if the Tx signer is a validator and
// has or hasn't voted in the current voting period. If it's their first time,
// we apply a discount on fees or gas. Otherwise, normal cost applies.
//
// In real code, you'll likely store more config, such as a reference to the
// staking keeper to fetch validator info, or track the current epoch.
type VoteFeeDiscountDecorator struct {
oracleKeeper OracleKeeperI
stakingKeeper StakingKeeperI
}

// NewVoteFeeDiscountDecorator is the constructor.
func NewVoteFeeDiscountDecorator(
oracleKeeper OracleKeeperI,
stakingKeeper StakingKeeperI,
) VoteFeeDiscountDecorator {
return VoteFeeDiscountDecorator{
oracleKeeper: oracleKeeper,
stakingKeeper: stakingKeeper,
}
}

// AnteHandle implements sdk.AnteDecorator. The discount logic is
// purely demonstrative; adapt to your own logic.
func (vfd VoteFeeDiscountDecorator) AnteHandle(
ctx sdk.Context,
tx sdk.Tx,
simulate bool,
next sdk.AnteHandler,
) (newCtx sdk.Context, err error) {
// 1. We check if there's exactly one signer (typical for Oracle votes).
// If your chain supports multiple signers, adapt accordingly.
sigTx, ok := tx.(authsigning.SigVerifiableTx)
if !ok {
return ctx, sdkerrors.Wrapf(errortypes.ErrTxDecode, "invalid tx type %T", tx)
}

sigs, err := sigTx.GetSignaturesV2()
if err != nil {
return next(ctx, tx, simulate)
}

Check warning on line 53 in x/oracle/ante/fee_discount.go

View check run for this annotation

Codecov / codecov/patch

x/oracle/ante/fee_discount.go#L52-L53

Added lines #L52 - L53 were not covered by tests
if len(sigs) != 1 {
// passthrough if not exactly one signer
return next(ctx, tx, simulate)
}

// ensure all messages are prevoting or voting messages
for _, msg := range tx.GetMsgs() {
if _, ok := msg.(*types.MsgAggregateExchangeRatePrevote); !ok {
if _, ok := msg.(*types.MsgAggregateExchangeRateVote); !ok {
return next(ctx, tx, simulate)
}
}
}

// 2. Check if the signer is a validator
valAddr := sdk.ValAddress(sigTx.GetSigners()[0])
validator, found := vfd.stakingKeeper.GetValidator(ctx, valAddr)
if !found {
return next(ctx, tx, simulate)
}

if validator.Jailed {
return next(ctx, tx, simulate)
}

// needs to have at least 0.5% of the supply bonded
totalBonded := vfd.stakingKeeper.TotalBondedTokens(ctx)
currentlyBonded := validator.Tokens

if currentlyBonded.LT(totalBonded.Mul(math.NewInt(50)).Quo(math.NewInt(10000))) {
return next(ctx, tx, simulate)
}
Comment on lines +79 to +85
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Adjust Bonded Tokens Threshold Calculation for Precision

The calculation for determining if a validator has at least 0.5% of the bonded tokens (line 83) uses integer arithmetic, which may lead to imprecise results due to truncation. Consider using precise decimal arithmetic with sdk.Dec to ensure accuracy in the threshold calculation.

Apply this diff to enhance precision:

-	if currentlyBonded.LT(totalBonded.Mul(math.NewInt(50)).Quo(math.NewInt(10000))) {
+	threshold := sdk.NewDecFromInt(totalBonded).Mul(sdk.NewDecWithPrec(5, 3)) // 0.5%
+	if sdk.NewDecFromInt(currentlyBonded).LT(threshold) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// needs to have at least 0.5% of the supply bonded
totalBonded := vfd.stakingKeeper.TotalBondedTokens(ctx)
currentlyBonded := validator.Tokens
if currentlyBonded.LT(totalBonded.Mul(math.NewInt(50)).Quo(math.NewInt(10000))) {
return next(ctx, tx, simulate)
}
// needs to have at least 0.5% of the supply bonded
totalBonded := vfd.stakingKeeper.TotalBondedTokens(ctx)
currentlyBonded := validator.Tokens
threshold := sdk.NewDecFromInt(totalBonded).Mul(sdk.NewDecWithPrec(5, 3)) // 0.5%
if sdk.NewDecFromInt(currentlyBonded).LT(threshold) {
return next(ctx, tx, simulate)
}


// 3. If validator, we check whether they've posted a vote this period
hasVoted := vfd.oracleKeeper.HasVotedInCurrentPeriod(ctx, valAddr)
if !hasVoted {
// 4. If first time, let's say we reduce gas cost.
minGasPrices := ctx.MinGasPrices()
var discounted []sdk.DecCoin
for _, mgp := range minGasPrices {
discounted = append(discounted, sdk.NewDecCoinFromDec(mgp.Denom, mgp.Amount.QuoInt64(69_420)))
}
// We'll create a new context with the updated MinGasPrices
ctx = ctx.WithMinGasPrices(discounted)
}
Comment on lines +87 to +98
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure Gas Discount Logic Correctly Applies Discounts

The gas price reduction divides the MinGasPrices by 69,420 (line 94), which seems arbitrary and may reduce fees below acceptable limits, potentially affecting network security.

Consider revising the discount factor to a more appropriate value or making it configurable.

Apply this diff to adjust the discount factor:

-			discounted = append(discounted, sdk.NewDecCoinFromDec(mgp.Denom, mgp.Amount.QuoInt64(69_420)))
+			discountFactor := sdk.NewDecWithPrec(5, 1) // 50% discount
+			discountedAmount := mgp.Amount.Mul(discountFactor)
+			discounted = append(discounted, sdk.NewDecCoinFromDec(mgp.Denom, discountedAmount))

Committable suggestion skipped: line range outside the PR's diff.


// 5. Keep going in the AnteHandler chain
return next(ctx, tx, simulate)
}
Loading
Loading