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: clamp to consensus max gas #51

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
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
4 changes: 2 additions & 2 deletions proto/feemarket/feemarket/v1/params.proto
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ message Params {
(gogoproto.nullable) = false
];

// TargetBlockUtilization is the target block utilization.
// TargetBlockUtilization is the target block utilization (in units of gas).
uint64 target_block_utilization = 8;

// MaxBlockUtilization is the maximum block utilization.
// MaxBlockUtilization is the maximum block utilization (in units of gas).
uint64 max_block_utilization = 9;

// Window defines the window size for calculating an adaptive learning rate
Expand Down
16 changes: 5 additions & 11 deletions tests/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ import (
bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
"github.com/cosmos/cosmos-sdk/x/consensus"
consensuskeeper "github.com/cosmos/cosmos-sdk/x/consensus/keeper"
"github.com/cosmos/cosmos-sdk/x/crisis"
crisiskeeper "github.com/cosmos/cosmos-sdk/x/crisis/keeper"
distr "github.com/cosmos/cosmos-sdk/x/distribution"
distrkeeper "github.com/cosmos/cosmos-sdk/x/distribution/keeper"
"github.com/cosmos/cosmos-sdk/x/evidence"
Expand Down Expand Up @@ -96,7 +94,6 @@ var (
},
),
params.AppModuleBasic{},
crisis.AppModuleBasic{},
slashing.AppModuleBasic{},
feegrantmodule.AppModuleBasic{},
upgrade.AppModuleBasic{},
Expand Down Expand Up @@ -132,7 +129,6 @@ type TestApp struct {
MintKeeper mintkeeper.Keeper
DistrKeeper distrkeeper.Keeper
GovKeeper *govkeeper.Keeper
CrisisKeeper *crisiskeeper.Keeper
aljo242 marked this conversation as resolved.
Show resolved Hide resolved
UpgradeKeeper *upgradekeeper.Keeper
ParamsKeeper paramskeeper.Keeper
AuthzKeeper authzkeeper.Keeper
Expand Down Expand Up @@ -210,7 +206,6 @@ func New(
&app.MintKeeper,
&app.DistrKeeper,
&app.GovKeeper,
&app.CrisisKeeper,
&app.UpgradeKeeper,
&app.ParamsKeeper,
&app.AuthzKeeper,
Expand Down Expand Up @@ -272,10 +267,11 @@ func New(
}

postHandlerOptions := PostHandlerOptions{
AccountKeeper: app.AccountKeeper,
BankKeeper: app.BankKeeper,
FeeGrantKeeper: app.FeeGrantKeeper,
FeeMarketKeeper: &app.FeeMarketKeeper,
AccountKeeper: app.AccountKeeper,
BankKeeper: app.BankKeeper,
FeeGrantKeeper: app.FeeGrantKeeper,
FeeMarketKeeper: &app.FeeMarketKeeper,
ConsensusParamsKeeper: &app.ConsensusParamsKeeper,
}
postHandler, err := NewPostHandler(postHandlerOptions)
if err != nil {
Expand All @@ -292,8 +288,6 @@ func New(

/**** Module Options ****/

app.ModuleManager.RegisterInvariants(app.CrisisKeeper)

// RegisterUpgradeHandlers is used for registering any on-chain upgrades.
// app.RegisterUpgradeHandlers()

Expand Down
10 changes: 1 addition & 9 deletions tests/app/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
authzmodulev1 "cosmossdk.io/api/cosmos/authz/module/v1"
bankmodulev1 "cosmossdk.io/api/cosmos/bank/module/v1"
consensusmodulev1 "cosmossdk.io/api/cosmos/consensus/module/v1"
crisismodulev1 "cosmossdk.io/api/cosmos/crisis/module/v1"
distrmodulev1 "cosmossdk.io/api/cosmos/distribution/module/v1"
evidencemodulev1 "cosmossdk.io/api/cosmos/evidence/module/v1"
feegrantmodulev1 "cosmossdk.io/api/cosmos/feegrant/module/v1"
Expand All @@ -30,7 +29,6 @@ import (
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
consensustypes "github.com/cosmos/cosmos-sdk/x/consensus/types"
crisistypes "github.com/cosmos/cosmos-sdk/x/crisis/types"
distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types"
"github.com/cosmos/cosmos-sdk/x/feegrant"
Expand Down Expand Up @@ -59,7 +57,7 @@ var (
genesisModuleOrder = []string{
capabilitytypes.ModuleName, authtypes.ModuleName, banktypes.ModuleName, feemarkettypes.ModuleName,
distrtypes.ModuleName, stakingtypes.ModuleName, slashingtypes.ModuleName, govtypes.ModuleName,
minttypes.ModuleName, crisistypes.ModuleName, genutiltypes.ModuleName, evidencetypes.ModuleName, authz.ModuleName,
minttypes.ModuleName, genutiltypes.ModuleName, evidencetypes.ModuleName, authz.ModuleName,
feegrant.ModuleName, group.ModuleName, paramstypes.ModuleName, upgradetypes.ModuleName,
vestingtypes.ModuleName, consensustypes.ModuleName,
}
Expand Down Expand Up @@ -112,7 +110,6 @@ var (
authtypes.ModuleName,
banktypes.ModuleName,
govtypes.ModuleName,
crisistypes.ModuleName,
genutiltypes.ModuleName,
authz.ModuleName,
feegrant.ModuleName,
Expand All @@ -122,7 +119,6 @@ var (
consensustypes.ModuleName,
},
EndBlockers: []string{
crisistypes.ModuleName,
govtypes.ModuleName,
feemarkettypes.ModuleName,
stakingtypes.ModuleName,
Expand Down Expand Up @@ -231,10 +227,6 @@ var (
Name: govtypes.ModuleName,
Config: appconfig.WrapAny(&govmodulev1.Module{}),
},
{
Name: crisistypes.ModuleName,
Config: appconfig.WrapAny(&crisismodulev1.Module{}),
},
{
Name: consensustypes.ModuleName,
Config: appconfig.WrapAny(&consensusmodulev1.Module{}),
Expand Down
3 changes: 0 additions & 3 deletions tests/app/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ func (app *TestApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs [
allowedAddrsMap[addr] = true
}

/* Just to be safe, assert the invariants on current state. */
app.CrisisKeeper.AssertInvariants(ctx)

/* Handle fee distribution state. */

// withdraw all validator commission
Expand Down
7 changes: 4 additions & 3 deletions tests/app/feemarketd/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli"
"github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/cosmos/cosmos-sdk/x/crisis"
genutilcli "github.com/cosmos/cosmos-sdk/x/genutil/client/cli"
"github.com/spf13/cobra"
"github.com/spf13/viper"
Expand All @@ -31,6 +30,8 @@ import (
"github.com/skip-mev/feemarket/tests/app/params"
)

var flagCrisisDummy = "x-crisis-skip-assert-invariants" // dummy flag so that the crisis module does not need to be imported for security reasons

func NewRootCmd() *cobra.Command {
// we "pre"-instantiate the application for getting the injected/configured encoding configuration
simApp := app.New(log.NewNopLogger(), dbm.NewMemDB(), nil, true, simtestutil.NewAppOptionsWithFlagHome(app.DefaultNodeHome))
Expand Down Expand Up @@ -177,8 +178,8 @@ func initRootCmd(rootCmd *cobra.Command, encodingConfig params.EncodingConfig) {
)
}

func addModuleInitFlags(startCmd *cobra.Command) {
crisis.AddModuleInitFlags(startCmd)
func addModuleInitFlags(cmd *cobra.Command) {
cmd.Flags().Bool(flagCrisisDummy, true, "dummy flag for crisis module")
}

func genesisCommand(encodingConfig params.EncodingConfig, cmds ...*cobra.Command) *cobra.Command {
Expand Down
14 changes: 10 additions & 4 deletions tests/app/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import (

// PostHandlerOptions are the options required for constructing a FeeMarket PostHandler.
type PostHandlerOptions struct {
AccountKeeper feemarketpost.AccountKeeper
BankKeeper feemarketpost.BankKeeper
FeeMarketKeeper feemarketpost.FeeMarketKeeper
FeeGrantKeeper feemarketpost.FeeGrantKeeper
AccountKeeper feemarketpost.AccountKeeper
BankKeeper feemarketpost.BankKeeper
FeeMarketKeeper feemarketpost.FeeMarketKeeper
FeeGrantKeeper feemarketpost.FeeGrantKeeper
ConsensusParamsKeeper feemarketpost.ConsensusKeeper
}

// NewPostHandler returns a PostHandler chain with the fee deduct decorator.
Expand All @@ -30,12 +31,17 @@ func NewPostHandler(options PostHandlerOptions) (sdk.PostHandler, error) {
return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "feemarket keeper is required for post builder")
}

if options.ConsensusParamsKeeper == nil {
aljo242 marked this conversation as resolved.
Show resolved Hide resolved
return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "consensus params keeper is required for post builder")
}

postDecorators := []sdk.PostDecorator{
feemarketpost.NewFeeMarketDeductDecorator(
options.AccountKeeper,
options.BankKeeper,
options.FeeGrantKeeper,
options.FeeMarketKeeper,
options.ConsensusParamsKeeper,
),
}

Expand Down
35 changes: 35 additions & 0 deletions testutils/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ package keeper
import (
"testing"

consensustypes "github.com/cosmos/cosmos-sdk/x/consensus/types"

"github.com/cometbft/cometbft/libs/log"
tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
consensuskeeper "github.com/cosmos/cosmos-sdk/x/consensus/keeper"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
testkeeper "github.com/skip-mev/chaintestutil/keeper"
"github.com/stretchr/testify/require"
Expand All @@ -22,6 +25,7 @@ import (
type TestKeepers struct {
testkeeper.TestKeepers
FeeMarketKeeper *feemarketkeeper.Keeper
ConsensusKeeper *consensuskeeper.Keeper
}

// TestMsgServers holds all message servers used during keeper tests for all modules
Expand All @@ -35,6 +39,16 @@ var additionalMaccPerms = map[string][]string{
feemarkettypes.FeeCollectorName: {authtypes.Burner},
}

var ConsensusParams = &tmproto.ConsensusParams{
Block: &tmproto.BlockParams{
MaxBytes: 1_000_000,
MaxGas: 1_000_000,
},
Evidence: nil,
Validator: nil,
Version: nil,
}
aljo242 marked this conversation as resolved.
Show resolved Hide resolved

// NewTestSetup returns initialized instances of all the keepers and message servers of the modules
func NewTestSetup(t testing.TB, options ...testkeeper.SetupOption) (sdk.Context, TestKeepers, TestMsgServers) {
options = append(options, testkeeper.WithAdditionalModuleAccounts(additionalMaccPerms))
Expand All @@ -43,6 +57,7 @@ func NewTestSetup(t testing.TB, options ...testkeeper.SetupOption) (sdk.Context,

// initialize extra keeper
feeMarketKeeper := FeeMarket(tk.Initializer, tk.AccountKeeper)
consensusKeeper := Consensus(tk.Initializer)
require.NoError(t, tk.Initializer.LoadLatest())

// initialize msg servers
Expand All @@ -58,9 +73,13 @@ func NewTestSetup(t testing.TB, options ...testkeeper.SetupOption) (sdk.Context,
err = feeMarketKeeper.SetParams(ctx, feemarkettypes.DefaultParams())
require.NoError(t, err)

// init dummy consensus params
consensusKeeper.Set(ctx, ConsensusParams)
aljo242 marked this conversation as resolved.
Show resolved Hide resolved

testKeepers := TestKeepers{
TestKeepers: tk,
FeeMarketKeeper: feeMarketKeeper,
ConsensusKeeper: consensusKeeper,
}

testMsgServers := TestMsgServers{
Expand All @@ -71,6 +90,22 @@ func NewTestSetup(t testing.TB, options ...testkeeper.SetupOption) (sdk.Context,
return ctx, testKeepers, testMsgServers
}

// Consensus initializes the consensus params module using the testkeepers intializer.
func Consensus(
initializer *testkeeper.Initializer,
) *consensuskeeper.Keeper {
storeKey := sdk.NewKVStoreKey(consensustypes.StoreKey)
initializer.StateStore.MountStoreWithDB(storeKey, storetypes.StoreTypeIAVL, initializer.DB)

k := consensuskeeper.NewKeeper(
initializer.Codec,
storeKey,
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

return &k
}
aljo242 marked this conversation as resolved.
Show resolved Hide resolved

// FeeMarket initializes the fee market module using the testkeepers intializer.
func FeeMarket(
initializer *testkeeper.Initializer,
Expand Down
15 changes: 12 additions & 3 deletions x/feemarket/ante/suite/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
feemarketante "github.com/skip-mev/feemarket/x/feemarket/ante"
"github.com/skip-mev/feemarket/x/feemarket/ante/mocks"
feemarketpost "github.com/skip-mev/feemarket/x/feemarket/post"
postmocks "github.com/skip-mev/feemarket/x/feemarket/post/mocks"
)

type TestSuite struct {
Expand All @@ -34,12 +35,14 @@ type TestSuite struct {

AccountKeeper feemarketante.AccountKeeper
FeeMarketKeeper feemarketpost.FeeMarketKeeper
ConsensusKeeper feemarketpost.ConsensusKeeper
BankKeeper feemarketante.BankKeeper
FeeGrantKeeper feemarketante.FeeGrantKeeper

MockBankKeeper *mocks.BankKeeper
MockFeeGrantKeeper *mocks.FeeGrantKeeper
EncCfg encoding.TestEncodingConfig
MockBankKeeper *mocks.BankKeeper
MockFeeGrantKeeper *mocks.FeeGrantKeeper
MockConsensusParamsKeeper *postmocks.ConsensusKeeper
EncCfg encoding.TestEncodingConfig
}

// TestAccount represents an account used in the tests in x/auth/ante.
Expand Down Expand Up @@ -75,8 +78,10 @@ func SetupTestSuite(t *testing.T, mock bool) *TestSuite {

s.AccountKeeper = testKeepers.AccountKeeper
s.FeeMarketKeeper = testKeepers.FeeMarketKeeper
s.ConsensusKeeper = testKeepers.ConsensusKeeper
s.MockBankKeeper = mocks.NewBankKeeper(t)
s.MockFeeGrantKeeper = mocks.NewFeeGrantKeeper(t)
s.MockConsensusParamsKeeper = postmocks.NewConsensusKeeper(t)

s.ClientCtx = client.Context{}.WithTxConfig(s.EncCfg.TxConfig)
s.TxBuilder = s.ClientCtx.TxConfig.NewTxBuilder()
Expand All @@ -89,9 +94,12 @@ func SetupTestSuite(t *testing.T, mock bool) *TestSuite {
func (s *TestSuite) SetupHandlers(mock bool) {
bankKeeper := s.BankKeeper
feeGrantKeeper := s.FeeGrantKeeper
consensusKeeper := s.ConsensusKeeper

if mock {
bankKeeper = s.MockBankKeeper
feeGrantKeeper = s.MockFeeGrantKeeper
consensusKeeper = s.MockConsensusParamsKeeper
}

// create basic antehandler with the feemarket decorator
Expand All @@ -112,6 +120,7 @@ func (s *TestSuite) SetupHandlers(mock bool) {
bankKeeper,
feeGrantKeeper,
s.FeeMarketKeeper,
consensusKeeper,
),
}

Expand Down
4 changes: 2 additions & 2 deletions x/feemarket/fuzz/aimd_eip1559_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestAIMDLearningRate(t *testing.T) {
prevLearningRate := state.LearningRate

// Update the fee market.
if err := state.Update(blockUtilization, params); err != nil {
if err := state.Update(blockUtilization, params.MaxBlockUtilization); err != nil {
t.Fatalf("block update errors: %v", err)
}

Expand Down Expand Up @@ -80,7 +80,7 @@ func TestAIMDBaseFee(t *testing.T) {
blockUtilization := gasGen.Draw(t, "gas")
prevBaseFee := state.BaseFee

if err := state.Update(blockUtilization, params); err != nil {
if err := state.Update(blockUtilization, params.MaxBlockUtilization); err != nil {
t.Fatalf("block update errors: %v", err)
}

Expand Down
4 changes: 2 additions & 2 deletions x/feemarket/fuzz/eip1559_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestLearningRate(t *testing.T) {
blockUtilization := rapid.Uint64Range(0, params.MaxBlockUtilization).Draw(t, "gas")

// Update the fee market.
if err := state.Update(blockUtilization, params); err != nil {
if err := state.Update(blockUtilization, params.MaxBlockUtilization); err != nil {
t.Fatalf("block update errors: %v", err)
}

Expand All @@ -50,7 +50,7 @@ func TestBaseFee(t *testing.T) {
blockUtilization := rapid.Uint64Range(0, params.MaxBlockUtilization).Draw(t, "gas")

// Update the fee market.
if err := state.Update(blockUtilization, params); err != nil {
if err := state.Update(blockUtilization, params.MaxBlockUtilization); err != nil {
t.Fatalf("block update errors: %v", err)
}

Expand Down
Loading
Loading