Skip to content

Commit

Permalink
fix!: Add ValidateTx to ExtensionOptionChecker ante (#21403)
Browse files Browse the repository at this point in the history
  • Loading branch information
facundomedica authored Aug 26, 2024
1 parent 28c792b commit 19e0de5
Show file tree
Hide file tree
Showing 14 changed files with 30 additions and 16 deletions.
4 changes: 2 additions & 2 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ func NewSimApp(
app.ModuleManager = module.NewManager(
genutil.NewAppModule(appCodec, app.AuthKeeper, app.StakingKeeper, app, txConfig, genutiltypes.DefaultMessageValidator),
accounts.NewAppModule(appCodec, app.AccountsKeeper),
auth.NewAppModule(appCodec, app.AuthKeeper, app.AccountsKeeper, authsims.RandomGenesisAccounts),
auth.NewAppModule(appCodec, app.AuthKeeper, app.AccountsKeeper, authsims.RandomGenesisAccounts, nil),
vesting.NewAppModule(app.AuthKeeper, app.BankKeeper),
bank.NewAppModule(appCodec, app.BankKeeper, app.AuthKeeper),
feegrantmodule.NewAppModule(appCodec, app.AuthKeeper, app.BankKeeper, app.FeeGrantKeeper, app.interfaceRegistry),
Expand Down Expand Up @@ -555,7 +555,7 @@ func NewSimApp(
// NOTE: this is not required apps that don't use the simulator for fuzz testing
// transactions
overrideModules := map[string]module.AppModuleSimulation{
authtypes.ModuleName: auth.NewAppModule(app.appCodec, app.AuthKeeper, app.AccountsKeeper, authsims.RandomGenesisAccounts),
authtypes.ModuleName: auth.NewAppModule(app.appCodec, app.AuthKeeper, app.AccountsKeeper, authsims.RandomGenesisAccounts, nil),
}
app.sm = module.NewSimulationManagerFromAppModules(app.ModuleManager.Modules, overrideModules)

Expand Down
2 changes: 1 addition & 1 deletion simapp/app_di.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func NewSimApp(
// NOTE: this is not required apps that don't use the simulator for fuzz testing
// transactions
overrideModules := map[string]module.AppModuleSimulation{
authtypes.ModuleName: auth.NewAppModule(app.appCodec, app.AuthKeeper, &app.AccountsKeeper, authsims.RandomGenesisAccounts),
authtypes.ModuleName: auth.NewAppModule(app.appCodec, app.AuthKeeper, &app.AccountsKeeper, authsims.RandomGenesisAccounts, nil),
}
app.sm = module.NewSimulationManagerFromAppModules(app.ModuleManager.Modules, overrideModules)

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/auth/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func initFixture(t *testing.T) *fixture {
assert.NilError(t, bankKeeper.SetParams(newCtx, params))

accountsModule := accounts.NewAppModule(cdc, accountsKeeper)
authModule := auth.NewAppModule(cdc, authKeeper, accountsKeeper, authsims.RandomGenesisAccounts)
authModule := auth.NewAppModule(cdc, authKeeper, accountsKeeper, authsims.RandomGenesisAccounts, nil)
bankModule := bank.NewAppModule(cdc, bankKeeper, authKeeper)

integrationApp := integration.NewIntegrationApp(newCtx, logger, keys, cdc,
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/bank/keeper/deterministic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func initDeterministicFixture(t *testing.T) *deterministicFixture {

assert.NilError(t, bankKeeper.SetParams(newCtx, banktypes.DefaultParams()))

authModule := auth.NewAppModule(cdc, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts)
authModule := auth.NewAppModule(cdc, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts, nil)
bankModule := bank.NewAppModule(cdc, bankKeeper, accountKeeper)

integrationApp := integration.NewIntegrationApp(newCtx, logger, keys, cdc,
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/distribution/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func initFixture(t *testing.T) *fixture {
cdc, runtime.NewEnvironment(runtime.NewKVStoreService(keys[distrtypes.StoreKey]), logger), accountKeeper, bankKeeper, stakingKeeper, cometService, distrtypes.ModuleName, authority.String(),
)

authModule := auth.NewAppModule(cdc, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts)
authModule := auth.NewAppModule(cdc, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts, nil)
bankModule := bank.NewAppModule(cdc, bankKeeper, accountKeeper)
stakingModule := staking.NewAppModule(cdc, stakingKeeper, accountKeeper, bankKeeper)
distrModule := distribution.NewAppModule(cdc, distrKeeper, accountKeeper, bankKeeper, stakingKeeper)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/evidence/keeper/infraction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func initFixture(tb testing.TB) *fixture {
router = router.AddRoute(evidencetypes.RouteEquivocation, testEquivocationHandler(evidenceKeeper))
evidenceKeeper.SetRouter(router)

authModule := auth.NewAppModule(cdc, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts)
authModule := auth.NewAppModule(cdc, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts, nil)
bankModule := bank.NewAppModule(cdc, bankKeeper, accountKeeper)
stakingModule := staking.NewAppModule(cdc, stakingKeeper, accountKeeper, bankKeeper)
slashingModule := slashing.NewAppModule(cdc, slashingKeeper, accountKeeper, bankKeeper, stakingKeeper, cdc.InterfaceRegistry(), cometInfoService)
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/example/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func Example() {
)

// subspace is nil because we don't test params (which is legacy anyway)
authModule := auth.NewAppModule(encodingCfg.Codec, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts)
authModule := auth.NewAppModule(encodingCfg.Codec, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts, nil)

// here bankkeeper and staking keeper is nil because we are not testing them
// subspace is nil because we don't test params (which is legacy anyway)
Expand Down Expand Up @@ -174,7 +174,7 @@ func Example_oneModule() {
)

// subspace is nil because we don't test params (which is legacy anyway)
authModule := auth.NewAppModule(encodingCfg.Codec, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts)
authModule := auth.NewAppModule(encodingCfg.Codec, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts, nil)

// create the application and register all the modules from the previous step
integrationApp := integration.NewIntegrationApp(
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/gov/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func initFixture(tb testing.TB) *fixture {
err = govKeeper.Params.Set(newCtx, v1.DefaultParams())
assert.NilError(tb, err)

authModule := auth.NewAppModule(cdc, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts)
authModule := auth.NewAppModule(cdc, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts, nil)
bankModule := bank.NewAppModule(cdc, bankKeeper, accountKeeper)
stakingModule := staking.NewAppModule(cdc, stakingKeeper, accountKeeper, bankKeeper)
govModule := gov.NewAppModule(cdc, govKeeper, accountKeeper, bankKeeper, poolKeeper)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/staking/keeper/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func initFixture(tb testing.TB) *fixture {

stakingKeeper := stakingkeeper.NewKeeper(cdc, runtime.NewEnvironment(runtime.NewKVStoreService(keys[types.StoreKey]), log.NewNopLogger(), runtime.EnvWithQueryRouterService(queryRouter), runtime.EnvWithMsgRouterService(msgRouter)), accountKeeper, bankKeeper, authority.String(), addresscodec.NewBech32Codec(sdk.Bech32PrefixValAddr), addresscodec.NewBech32Codec(sdk.Bech32PrefixConsAddr), runtime.NewContextAwareCometInfoService())

authModule := auth.NewAppModule(cdc, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts)
authModule := auth.NewAppModule(cdc, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts, nil)
bankModule := bank.NewAppModule(cdc, bankKeeper, accountKeeper)
stakingModule := staking.NewAppModule(cdc, stakingKeeper, accountKeeper, bankKeeper)

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/staking/keeper/deterministic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func initDeterministicFixture(t *testing.T) *deterministicFixture {

stakingKeeper := stakingkeeper.NewKeeper(cdc, runtime.NewEnvironment(runtime.NewKVStoreService(keys[stakingtypes.StoreKey]), log.NewNopLogger()), accountKeeper, bankKeeper, authority.String(), addresscodec.NewBech32Codec(sdk.Bech32PrefixValAddr), addresscodec.NewBech32Codec(sdk.Bech32PrefixConsAddr), runtime.NewContextAwareCometInfoService())

authModule := auth.NewAppModule(cdc, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts)
authModule := auth.NewAppModule(cdc, accountKeeper, acctsModKeeper, authsims.RandomGenesisAccounts, nil)
bankModule := bank.NewAppModule(cdc, bankKeeper, accountKeeper)
stakingModule := staking.NewAppModule(cdc, stakingKeeper, accountKeeper, bankKeeper)

Expand Down
2 changes: 2 additions & 0 deletions x/auth/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [#19290](https://github.com/cosmos/cosmos-sdk/issues/19290) Pass `appmodule.Environment` to NewKeeper instead of passing individual services.
* [#19535](https://github.com/cosmos/cosmos-sdk/pull/19535) Remove vesting account creation when the chain is running. The accounts module is required for creating [#vesting accounts](../accounts/defaults/lockup/README.md) on a running chain.
* [#19600](https://github.com/cosmos/cosmos-sdk/pull/19600) add a consensus query method to the consensus module in order for modules to query consensus for the consensus params.
* [#19600](https://github.com/cosmos/cosmos-sdk/pull/21403) NewAppModule now takes in `ante.ExtensionOptionChecker`, but it's only used in server/v2 chains, so it's safe to pass in nil for the rest of the users.


### Consensus Breaking Changes

Expand Down
11 changes: 8 additions & 3 deletions x/auth/ante/ext.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package ante

import (
"context"

codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down Expand Up @@ -32,7 +34,7 @@ type RejectExtensionOptionsDecorator struct {
// options which can optionally be included in protobuf transactions that don't pass the checker.
// Users that need extension options should pass a custom checker that returns true for the
// needed extension options.
func NewExtensionOptionsDecorator(checker ExtensionOptionChecker) sdk.AnteDecorator {
func NewExtensionOptionsDecorator(checker ExtensionOptionChecker) RejectExtensionOptionsDecorator {
if checker == nil {
checker = rejectExtensionOption
}
Expand All @@ -42,10 +44,13 @@ func NewExtensionOptionsDecorator(checker ExtensionOptionChecker) sdk.AnteDecora

var _ sdk.AnteDecorator = RejectExtensionOptionsDecorator{}

func (r RejectExtensionOptionsDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error {
return checkExtOpts(tx, r.checker)
}

// AnteHandle implements the AnteDecorator.AnteHandle method
func (r RejectExtensionOptionsDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
err = checkExtOpts(tx, r.checker)
if err != nil {
if err := r.ValidateTx(ctx, tx); err != nil {
return ctx, err
}

Expand Down
5 changes: 4 additions & 1 deletion x/auth/depinject.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"cosmossdk.io/core/appmodule"
"cosmossdk.io/depinject"
"cosmossdk.io/depinject/appconfig"
"cosmossdk.io/x/auth/ante"
"cosmossdk.io/x/auth/keeper"
"cosmossdk.io/x/auth/simulation"
"cosmossdk.io/x/auth/types"
Expand Down Expand Up @@ -36,6 +37,8 @@ type ModuleInputs struct {
AddressCodec address.Codec
RandomGenesisAccountsFn types.RandomGenesisAccountsFn `optional:"true"`
AccountI func() sdk.AccountI `optional:"true"`

ExtensionOptionChecker ante.ExtensionOptionChecker `optional:"true"`
}

type ModuleOutputs struct {
Expand Down Expand Up @@ -71,7 +74,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {
}

k := keeper.NewAccountKeeper(in.Environment, in.Cdc, in.AccountI, in.AccountsModKeeper, maccPerms, in.AddressCodec, in.Config.Bech32Prefix, auth)
m := NewAppModule(in.Cdc, k, in.AccountsModKeeper, in.RandomGenesisAccountsFn)
m := NewAppModule(in.Cdc, k, in.AccountsModKeeper, in.RandomGenesisAccountsFn, in.ExtensionOptionChecker)

return ModuleOutputs{AccountKeeper: k, Module: m}
}
4 changes: 4 additions & 0 deletions x/auth/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type AppModule struct {
randGenAccountsFn types.RandomGenesisAccountsFn
accountsModKeeper types.AccountsModKeeper
cdc codec.Codec
extOptChecker ante.ExtensionOptionChecker
}

// IsAppModule implements the appmodule.AppModule interface.
Expand All @@ -56,12 +57,14 @@ func NewAppModule(
accountKeeper keeper.AccountKeeper,
ak types.AccountsModKeeper,
randGenAccountsFn types.RandomGenesisAccountsFn,
extOptChecker ante.ExtensionOptionChecker,
) AppModule {
return AppModule{
accountKeeper: accountKeeper,
randGenAccountsFn: randGenAccountsFn,
accountsModKeeper: ak,
cdc: cdc,
extOptChecker: extOptChecker,
}
}

Expand Down Expand Up @@ -159,6 +162,7 @@ func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error {
ante.NewValidateMemoDecorator(am.accountKeeper),
ante.NewConsumeGasForTxSizeDecorator(am.accountKeeper),
ante.NewValidateSigCountDecorator(am.accountKeeper),
ante.NewExtensionOptionsDecorator(am.extOptChecker),
}

sdkTx, ok := tx.(sdk.Tx)
Expand Down

0 comments on commit 19e0de5

Please sign in to comment.