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

move feegrant ante to auth ante #8682

Merged
merged 39 commits into from
Mar 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
d7b83ba
move feegrant ante to auth ante
atheeshp Feb 22, 2021
967dd28
Merge branch 'master' into atheesh/move_feegrant_ante
atheeshp Feb 24, 2021
d692f10
Merge branch 'master' into atheesh/move_feegrant_ante
atheeshp Feb 25, 2021
b229812
update ante builder
atheeshp Feb 26, 2021
7eb8897
Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/mo…
atheeshp Feb 26, 2021
f3874a7
Merge branch 'atheesh/move_feegrant_ante' of github.com:cosmos/cosmos…
atheeshp Feb 26, 2021
c8b38fa
remove commented code
atheeshp Feb 26, 2021
d1d74b8
Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/mo…
atheeshp Mar 3, 2021
8bd1cfe
review changes
atheeshp Mar 5, 2021
b282167
Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/mo…
atheeshp Mar 5, 2021
5e2c190
fix lint
atheeshp Mar 5, 2021
3c6c8a1
review changes
atheeshp Mar 5, 2021
3d633df
Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/mo…
atheeshp Mar 5, 2021
f29391f
review changes
atheeshp Mar 8, 2021
81bfce6
review changes
atheeshp Mar 8, 2021
a652ace
review changes
atheeshp Mar 8, 2021
6fa68e3
Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/mo…
atheeshp Mar 8, 2021
8fdf57b
fix ante builder
atheeshp Mar 8, 2021
a1e986c
Merge branch 'master' into atheesh/move_feegrant_ante
atheeshp Mar 9, 2021
87f2ecd
Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/mo…
atheeshp Mar 9, 2021
88b38e5
review changes
atheeshp Mar 9, 2021
6fd08a2
Merge branch 'master' into atheesh/move_feegrant_ante
atheeshp Mar 12, 2021
2355779
Merge branch 'master' into atheesh/move_feegrant_ante
atheeshp Mar 12, 2021
3d72ad3
Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/mo…
atheeshp Mar 18, 2021
b5404aa
update ante builder with options struct
atheeshp Mar 18, 2021
587b681
Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/mo…
atheeshp Mar 18, 2021
4343835
Merge branch 'atheesh/move_feegrant_ante' of github.com:cosmos/cosmos…
atheeshp Mar 18, 2021
576a93a
review changes
atheeshp Mar 18, 2021
0e47dcf
Merge branch 'master' into atheesh/move_feegrant_ante
atheeshp Mar 18, 2021
6ac8011
Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/mo…
atheeshp Mar 19, 2021
943190c
review changes
atheeshp Mar 19, 2021
3e4649c
Merge branch 'atheesh/move_feegrant_ante' of github.com:cosmos/cosmos…
atheeshp Mar 19, 2021
bb91e5b
Merge branch 'master' into atheesh/move_feegrant_ante
atheeshp Mar 20, 2021
7c08eba
Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/mo…
atheeshp Mar 22, 2021
e852d05
add changelog
atheeshp Mar 22, 2021
7ca23a6
Merge branch 'atheesh/move_feegrant_ante' of github.com:cosmos/cosmos…
atheeshp Mar 22, 2021
dcfdf08
Merge branch 'master' into atheesh/move_feegrant_ante
amaury1093 Mar 22, 2021
4a6199f
Merge branch 'master' into atheesh/move_feegrant_ante
aaronc Mar 22, 2021
148294a
Merge branch 'master' into atheesh/move_feegrant_ante
atheeshp Mar 23, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/bank) [\#8517](https://github.com/cosmos/cosmos-sdk/pull/8517) `SupplyI` interface and `Supply` are removed and uses `sdk.Coins` for supply tracking
* (x/upgrade) [\#8743](https://github.com/cosmos/cosmos-sdk/pull/8743) `UpgradeHandler` includes a new argument `VersionMap` which helps facilitate in-place migrations.
* (x/auth) [\#8129](https://github.com/cosmos/cosmos-sdk/pull/8828) Updated `SigVerifiableTx.GetPubKeys` method signature to return error.
* [\#8682](https://github.com/cosmos/cosmos-sdk/pull/8682) `ante.NewAnteHandler` updated to receive all positional params as `ante.HandlerOptions` struct. If required fields aren't set, throws error accordingly.


### State Machine Breaking
Expand Down
21 changes: 15 additions & 6 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ import (
evidencekeeper "github.com/cosmos/cosmos-sdk/x/evidence/keeper"
evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types"
feegrant "github.com/cosmos/cosmos-sdk/x/feegrant"
feegrantante "github.com/cosmos/cosmos-sdk/x/feegrant/ante"
feegrantkeeper "github.com/cosmos/cosmos-sdk/x/feegrant/keeper"
feegranttypes "github.com/cosmos/cosmos-sdk/x/feegrant/types"
"github.com/cosmos/cosmos-sdk/x/genutil"
Expand Down Expand Up @@ -375,12 +374,22 @@ func NewSimApp(
// initialize BaseApp
app.SetInitChainer(app.InitChainer)
app.SetBeginBlocker(app.BeginBlocker)
app.SetAnteHandler(
feegrantante.NewAnteHandler(
app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper, ante.DefaultSigVerificationGasConsumer,
encodingConfig.TxConfig.SignModeHandler(),
),

anteHandler, err := ante.NewAnteHandler(
ante.HandlerOptions{
AccountKeeper: app.AccountKeeper,
BankKeeper: app.BankKeeper,
SignModeHandler: encodingConfig.TxConfig.SignModeHandler(),
FeegrantKeeper: app.FeeGrantKeeper,
SigGasConsumer: ante.DefaultSigVerificationGasConsumer,
},
Comment on lines +378 to +385
Copy link
Member

Choose a reason for hiding this comment

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

@alessio @jgimeno can someone at your team take a look at this? Just want to make sure you're all aware and onboard with us moving positional parameters into an options struct here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Verbal ACK from @alexanderbez on our SDK call today

)

if err != nil {
panic(err)
}

app.SetAnteHandler(anteHandler)
app.SetEndBlocker(app.EndBlocker)

if loadLatest {
Expand Down
59 changes: 42 additions & 17 deletions x/auth/ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,57 @@ package ante

import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/signing"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
"github.com/cosmos/cosmos-sdk/x/auth/types"
)

// HandlerOptions are the options required for constructing a default SDK AnteHandler.
type HandlerOptions struct {
AccountKeeper AccountKeeper
BankKeeper types.BankKeeper
FeegrantKeeper FeegrantKeeper
SignModeHandler authsigning.SignModeHandler
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
SigGasConsumer func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error
}

// NewAnteHandler returns an AnteHandler that checks and increments sequence
// numbers, checks signatures & account numbers, and deducts fees from the first
// signer.
func NewAnteHandler(
ak AccountKeeper, bankKeeper types.BankKeeper,
sigGasConsumer SignatureVerificationGasConsumer,
signModeHandler signing.SignModeHandler,
) sdk.AnteHandler {
return sdk.ChainAnteDecorators(
func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
if options.AccountKeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "account keeper is required for ante builder")
}

if options.BankKeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "bank keeper is required for ante builder")
}

if options.SignModeHandler == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for ante builder")
}

var sigGasConsumer = options.SigGasConsumer
if sigGasConsumer == nil {
sigGasConsumer = DefaultSigVerificationGasConsumer
}

anteDecorators := []sdk.AnteDecorator{
NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first
NewRejectExtensionOptionsDecorator(),
NewMempoolFeeDecorator(),
NewValidateBasicDecorator(),
TxTimeoutHeightDecorator{},
NewValidateMemoDecorator(ak),
NewConsumeGasForTxSizeDecorator(ak),
NewRejectFeeGranterDecorator(),
NewSetPubKeyDecorator(ak), // SetPubKeyDecorator must be called before all signature verification decorators
NewValidateSigCountDecorator(ak),
NewDeductFeeDecorator(ak, bankKeeper),
NewSigGasConsumeDecorator(ak, sigGasConsumer),
NewSigVerificationDecorator(ak, signModeHandler),
NewIncrementSequenceDecorator(ak),
)
NewValidateMemoDecorator(options.AccountKeeper),
NewConsumeGasForTxSizeDecorator(options.AccountKeeper),
NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper),
NewSetPubKeyDecorator(options.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators
NewValidateSigCountDecorator(options.AccountKeeper),
NewSigGasConsumeDecorator(options.AccountKeeper, sigGasConsumer),
NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler),
NewIncrementSequenceDecorator(options.AccountKeeper),
}

return sdk.ChainAnteDecorators(anteDecorators...), nil
}
29 changes: 20 additions & 9 deletions x/auth/ante/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1010,15 +1010,26 @@ func (suite *AnteTestSuite) TestCustomSignatureVerificationGasConsumer() {
suite.SetupTest(false) // setup

// setup an ante handler that only accepts PubKeyEd25519
suite.anteHandler = ante.NewAnteHandler(suite.app.AccountKeeper, suite.app.BankKeeper, func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error {
switch pubkey := sig.PubKey.(type) {
case *ed25519.PubKey:
meter.ConsumeGas(params.SigVerifyCostED25519, "ante verify: ed25519")
return nil
default:
return sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey)
}
}, suite.clientCtx.TxConfig.SignModeHandler())
anteHandler, err := ante.NewAnteHandler(
ante.HandlerOptions{
AccountKeeper: suite.app.AccountKeeper,
BankKeeper: suite.app.BankKeeper,
FeegrantKeeper: suite.app.FeeGrantKeeper,
SignModeHandler: suite.clientCtx.TxConfig.SignModeHandler(),
SigGasConsumer: func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error {
switch pubkey := sig.PubKey.(type) {
case *ed25519.PubKey:
meter.ConsumeGas(params.SigVerifyCostED25519, "ante verify: ed25519")
return nil
default:
return sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey)
}
},
},
)

suite.Require().NoError(err)
suite.anteHandler = anteHandler

// Same data for every test cases
accounts := suite.CreateTestAccounts(1)
Expand Down
5 changes: 5 additions & 0 deletions x/auth/ante/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,8 @@ type AccountKeeper interface {
SetAccount(ctx sdk.Context, acc types.AccountI)
GetModuleAddress(moduleName string) sdk.AccAddress
}

// FeegrantKeeper defines the expected feegrant keeper.
type FeegrantKeeper interface {
UseGrantedFees(ctx sdk.Context, granter, grantee sdk.AccAddress, fee sdk.Coins) error
}
40 changes: 31 additions & 9 deletions x/auth/ante/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,16 @@ func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b
// Call next AnteHandler if fees successfully deducted
// CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator
type DeductFeeDecorator struct {
ak AccountKeeper
bankKeeper types.BankKeeper
ak AccountKeeper
bankKeeper types.BankKeeper
feegrantKeeper FeegrantKeeper
}

func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper) DeductFeeDecorator {
func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper) DeductFeeDecorator {
return DeductFeeDecorator{
ak: ak,
bankKeeper: bk,
ak: ak,
bankKeeper: bk,
feegrantKeeper: fk,
}
}

Expand All @@ -80,16 +82,36 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo
panic(fmt.Sprintf("%s module account has not been set", types.FeeCollectorName))
}

fee := feeTx.GetFee()
feePayer := feeTx.FeePayer()
feePayerAcc := dfd.ak.GetAccount(ctx, feePayer)
feeGranter := feeTx.FeeGranter()

if feePayerAcc == nil {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "fee payer address: %s does not exist", feePayer)
deductFeesFrom := feePayer

// if feegranter set deduct fee from feegranter account.
// this works with only when feegrant enabled.
if feeGranter != nil {
if dfd.feegrantKeeper == nil {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee grants are not enabled")
} else if !feeGranter.Equals(feePayer) {
err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee)

if err != nil {
return ctx, sdkerrors.Wrapf(err, "%s not allowed to pay fees from %s", feeGranter, feePayer)
}
}

deductFeesFrom = feeGranter
}

deductFeesFromAcc := dfd.ak.GetAccount(ctx, deductFeesFrom)
if deductFeesFromAcc == nil {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "fee payer address: %s does not exist", deductFeesFrom)
}

// deduct the fees
if !feeTx.GetFee().IsZero() {
err = DeductFees(dfd.bankKeeper, ctx, feePayerAcc, feeTx.GetFee())
err = DeductFees(dfd.bankKeeper, ctx, deductFeesFromAcc, feeTx.GetFee())
if err != nil {
return ctx, err
}
Expand Down
27 changes: 0 additions & 27 deletions x/auth/ante/fee_grant.go

This file was deleted.

32 changes: 0 additions & 32 deletions x/auth/ante/fee_grant_test.go

This file was deleted.

2 changes: 1 addition & 1 deletion x/auth/ante/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (suite *AnteTestSuite) TestDeductFees() {
err = simapp.FundAccount(suite.app, suite.ctx, addr1, coins)
suite.Require().NoError(err)

dfd := ante.NewDeductFeeDecorator(suite.app.AccountKeeper, suite.app.BankKeeper)
dfd := ante.NewDeductFeeDecorator(suite.app.AccountKeeper, suite.app.BankKeeper, nil)
antehandler := sdk.ChainAnteDecorators(dfd)

_, err = antehandler(suite.ctx, tx, false)
Expand Down
Loading