-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
refactor(x/auth): use transaction service #19967
Changes from all commits
78a4c7a
93f0734
d9806f9
682520b
2611c90
e589a20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -16,6 +16,7 @@ import ( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"cosmossdk.io/log" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
storetypes "cosmossdk.io/store/types" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"cosmossdk.io/x/auth" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"cosmossdk.io/x/auth/ante" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"cosmossdk.io/x/auth/ante/unorderedtx" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
authkeeper "cosmossdk.io/x/auth/keeper" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
authsims "cosmossdk.io/x/auth/simulation" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -292,13 +293,40 @@ func NewSimApp( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// set custom ante handlers | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
app.setCustomAnteHandler() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err := app.Load(loadLatest); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
panic(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return app | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// overwritte default ante handlers with custom ante handlers | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// set SkipAnteHandler to true in app config and set custom ante handler on baseapp | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (app *SimApp) setCustomAnteHandler() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
anteHandler, err := NewAnteHandler( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
HandlerOptions{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ante.HandlerOptions{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
AccountKeeper: app.AuthKeeper, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
BankKeeper: app.BankKeeper, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SignModeHandler: app.txConfig.SignModeHandler(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
FeegrantKeeper: app.FeeGrantKeeper, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SigGasConsumer: ante.DefaultSigVerificationGasConsumer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
&app.CircuitBreakerKeeper, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
app.UnorderedTxManager, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
panic(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Set the AnteHandler for the app | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
app.SetAnteHandler(anteHandler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+296
to
+328
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Review the implementation of The func (app *SimApp) setCustomAnteHandler() error {
- anteHandler, err := NewAnteHandler(
+ options := HandlerOptions{
+ ante.HandlerOptions{
+ AccountKeeper: app.AuthKeeper,
+ BankKeeper: app.BankKeeper,
+ SignModeHandler: app.txConfig.SignModeHandler(),
+ FeegrantKeeper: app.FeeGrantKeeper,
+ SigGasConsumer: ante.DefaultSigVerificationGasConsumer,
+ },
+ &app.CircuitBreakerKeeper,
+ app.UnorderedTxManager,
+ }
+ anteHandler, err := NewAnteHandler(options)
if err != nil {
- panic(err)
+ return fmt.Errorf("failed to set custom ante handler: %w", err)
}
app.SetAnteHandler(anteHandler)
+ return nil
}
- app.setCustomAnteHandler()
+ if err := app.setCustomAnteHandler(); err != nil {
+ panic(err)
+ } Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Close implements the Application interface and closes all necessary application | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// resources. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (app *SimApp) Close() error { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package ante | ||
|
||
import ( | ||
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
) | ||
|
||
var SimSecp256k1PubkeyInternal = simSecp256k1Pubkey | ||
|
||
func SetSVDPubKey(svd SigVerificationDecorator, ctx sdk.Context, acc sdk.AccountI, txPubKey cryptotypes.PubKey) error { | ||
return svd.setPubKey(ctx, acc, txPubKey) | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -10,6 +10,7 @@ import ( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"google.golang.org/protobuf/types/known/anypb" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"cosmossdk.io/core/transaction" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider grouping the Cosmos SDK imports together for better readability. import (
"context"
"encoding/base64"
"encoding/hex"
"errors"
"fmt"
secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4"
"google.golang.org/protobuf/types/known/anypb"
+ "cosmossdk.io/core/transaction"
errorsmod "cosmossdk.io/errors"
storetypes "cosmossdk.io/store/types"
aa_interface_v1 "cosmossdk.io/x/accounts/interfaces/account_abstraction/v1"
authsigning "cosmossdk.io/x/auth/signing"
"cosmossdk.io/x/auth/types"
txsigning "cosmossdk.io/x/tx/signing"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256r1"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
"github.com/cosmos/cosmos-sdk/types/tx"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
) Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
errorsmod "cosmossdk.io/errors" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
storetypes "cosmossdk.io/store/types" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
aa_interface_v1 "cosmossdk.io/x/accounts/interfaces/account_abstraction/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -215,7 +216,7 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ boo | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ctx.EventManager().EmitEvents(events) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return next(ctx, tx, ctx.ExecMode() == sdk.ExecModeSimulate) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return next(ctx, tx, false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// authenticate the authentication of the TX for a specific tx signer. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -280,7 +281,7 @@ func (svd SigVerificationDecorator) consumeSignatureGas( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pubKey cryptotypes.PubKey, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
signature signing.SignatureV2, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) error { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ctx.ExecMode() == sdk.ExecModeSimulate && pubKey == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if svd.ak.Environment().TransactionService.ExecMode(ctx) == transaction.ExecModeSimulate && pubKey == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace the direct comparison of - if svd.ak.Environment().TransactionService.ExecMode(ctx) == transaction.ExecModeSimulate && pubKey == nil {
+ if svd.ak.Environment().TransactionService.ExecMode(ctx) == transaction.ExecModeSimulate && !reflect.ValueOf(pubKey).IsValid() { This change ensures that the check for a Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pubKey = simSecp256k1Pubkey | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -310,7 +311,7 @@ func (svd SigVerificationDecorator) verifySig(ctx sdk.Context, tx sdk.Tx, acc sd | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// we're in simulation mode, or in ReCheckTx, or context is not | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// on sig verify tx, then we do not need to verify the signatures | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// in the tx. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ctx.ExecMode() == sdk.ExecModeSimulate || ctx.IsReCheckTx() || !ctx.IsSigverifyTx() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if svd.ak.Environment().TransactionService.ExecMode(ctx) == transaction.ExecModeSimulate || ctx.IsReCheckTx() || !ctx.IsSigverifyTx() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -384,7 +385,7 @@ func (svd SigVerificationDecorator) setPubKey(ctx sdk.Context, acc sdk.AccountI, | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if txPubKey == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// if we're not in simulation mode, and we do not have a valid pubkey | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// for this signer, then we simply error. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ctx.ExecMode() != sdk.ExecModeSimulate { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if svd.ak.Environment().TransactionService.ExecMode(ctx) != transaction.ExecModeSimulate { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider handling the error case when the execution mode is not - if svd.ak.Environment().TransactionService.ExecMode(ctx) != transaction.ExecModeSimulate {
+ if execMode := svd.ak.Environment().TransactionService.ExecMode(ctx); execMode != transaction.ExecModeSimulate {
+ return fmt.Errorf("the account %s is without a pubkey and did not provide a pubkey in the tx to set it; execution mode: %s", acc.GetAddress().String(), execMode)
+ } Including the execution mode in the error message provides more context for debugging and understanding why the operation failed, especially in environments where the execution mode might vary. Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return fmt.Errorf("the account %s is without a pubkey and did not provide a pubkey in the tx to set it", acc.GetAddress().String()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// if we're in simulation mode, then we can populate the pubkey with the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -479,7 +480,7 @@ func (vscd ValidateSigCountDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ b | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return next(ctx, tx, ctx.ExecMode() == sdk.ExecModeSimulate) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return next(ctx, tx, false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// DefaultSigVerificationGasConsumer is the default implementation of SignatureVerificationGasConsumer. It consumes gas | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip
Codebase Verification
The verification process has identified that
ExecModeReCheck
is not referenced in the codebase outside of its definition. This suggests that the integration of the new execution mode might not be complete, as there are no instances where it is being checked or set in the context of execution modes. It's advisable to review the implementation to ensure thatExecModeReCheck
is properly utilized where applicable.Analysis chain
Adding
ExecModeReCheck
enhances execution mode handling. Ensure that all references to execution modes are updated to include this new mode where applicable.Scripts Executed
The following scripts were executed for the analysis:
Script:
Length of output: 5035