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

refactor(x/auth/tx): Implement UnorderedTxManager tx validator #21426

Merged
merged 13 commits into from
Aug 29, 2024
34 changes: 24 additions & 10 deletions x/auth/ante/unordered.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package ante

import (
"bytes"
"context"
"crypto/sha256"
"encoding/binary"
"fmt"
"sync"
"time"

Expand Down Expand Up @@ -69,29 +71,41 @@ func (d *UnorderedTxDecorator) AnteHandle(
_ bool,
next sdk.AnteHandler,
) (sdk.Context, error) {
if err := d.ValidateTx(ctx, tx); err != nil {
return ctx, err
}
return next(ctx, tx, false)
}

func (d *UnorderedTxDecorator) ValidateTx(ctx context.Context, tx transaction.Tx) error {
hieuvubk marked this conversation as resolved.
Show resolved Hide resolved
sdkTx, ok := tx.(sdk.Tx)
if !ok {
return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx)
}
Comment on lines +80 to +84
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve error message for invalid transaction type.

The error message can be more descriptive to provide better context.

Apply this diff to improve the error message:

-	return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx)
+	return fmt.Errorf("invalid transaction type %T, expected sdk.Tx", tx)
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
func (d *UnorderedTxDecorator) ValidateTx(ctx context.Context, tx transaction.Tx) error {
sdkTx, ok := tx.(sdk.Tx)
if !ok {
return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx)
}
func (d *UnorderedTxDecorator) ValidateTx(ctx context.Context, tx transaction.Tx) error {
sdkTx, ok := tx.(sdk.Tx)
if !ok {
return fmt.Errorf("invalid transaction type %T, expected sdk.Tx", tx)
}


unorderedTx, ok := tx.(sdk.TxWithUnordered)
if !ok || !unorderedTx.GetUnordered() {
// If the transaction does not implement unordered capabilities or has the
// unordered value as false, we bypass.
return next(ctx, tx, false)
return nil
}

headerInfo := d.env.HeaderService.HeaderInfo(ctx)
timeoutTimestamp := unorderedTx.GetTimeoutTimeStamp()
if timeoutTimestamp.IsZero() || timeoutTimestamp.Unix() == 0 {
return ctx, errorsmod.Wrap(
return errorsmod.Wrap(
sdkerrors.ErrInvalidRequest,
"unordered transaction must have timeout_timestamp set",
)
}
if timeoutTimestamp.Before(headerInfo.Time) {
return ctx, errorsmod.Wrap(
return errorsmod.Wrap(
sdkerrors.ErrInvalidRequest,
"unordered transaction has a timeout_timestamp that has already passed",
)
}
if timeoutTimestamp.After(headerInfo.Time.Add(d.maxTimeoutDuration)) {
return ctx, errorsmod.Wrapf(
return errorsmod.Wrapf(
sdkerrors.ErrInvalidRequest,
"unordered tx ttl exceeds %s",
d.maxTimeoutDuration.String(),
Expand All @@ -100,24 +114,24 @@ func (d *UnorderedTxDecorator) AnteHandle(

// consume gas in all exec modes to avoid gas estimation discrepancies
if err := d.env.GasService.GasMeter(ctx).Consume(d.sha256Cost, "consume gas for calculating tx hash"); err != nil {
return ctx, errorsmod.Wrap(sdkerrors.ErrOutOfGas, "out of gas")
return errorsmod.Wrap(sdkerrors.ErrOutOfGas, "out of gas")
}

// Avoid checking for duplicates and creating the identifier in simulation mode
// This is done to avoid sha256 computation in simulation mode
if d.env.TransactionService.ExecMode(ctx) == transaction.ExecModeSimulate {
return next(ctx, tx, false)
return nil
}

// calculate the tx hash
txHash, err := TxIdentifier(uint64(timeoutTimestamp.Unix()), tx)
txHash, err := TxIdentifier(uint64(timeoutTimestamp.Unix()), sdkTx)
if err != nil {
return ctx, err
return err
}

// check for duplicates
if d.txManager.Contains(txHash) {
return ctx, errorsmod.Wrap(
return errorsmod.Wrap(
sdkerrors.ErrInvalidRequest,
"tx %X is duplicated",
)
Expand All @@ -127,7 +141,7 @@ func (d *UnorderedTxDecorator) AnteHandle(
d.txManager.Add(txHash, timeoutTimestamp)
}

return next(ctx, tx, false)
return nil
}

// TxIdentifier returns a unique identifier for a transaction that is intended to be unordered.
Expand Down
11 changes: 8 additions & 3 deletions x/auth/tx/config/depinject.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,9 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {
)

var (
minGasPrices sdk.DecCoins
feeTxValidator *ante.DeductFeeDecorator
minGasPrices sdk.DecCoins
feeTxValidator *ante.DeductFeeDecorator
unorderedTxValidator *ante.UnorderedTxDecorator
)
if in.AccountKeeper != nil && in.BankKeeper != nil && in.Viper != nil {
minGasPricesStr := in.Viper.GetString(flagMinGasPricesV2)
Expand All @@ -135,8 +136,12 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {
feeTxValidator.SetMinGasPrices(minGasPrices) // set min gas price in deduct fee decorator
}

if in.UnorderedTxManager != nil {
unorderedTxValidator = ante.NewUnorderedTxDecorator(unorderedtx.DefaultMaxTimeoutDuration, in.UnorderedTxManager, in.Environment, ante.DefaultSha256Cost)
}

return ModuleOutputs{
Module: NewAppModule(svd, feeTxValidator, in.ExtraTxValidators...),
Module: NewAppModule(svd, feeTxValidator, unorderedTxValidator, in.ExtraTxValidators...),
BaseAppOption: newBaseAppOption(txConfig, in),
TxConfig: txConfig,
TxConfigOptions: txConfigOptions,
Expand Down
19 changes: 14 additions & 5 deletions x/auth/tx/config/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ var (
// Additionally, it registers tx validators that do not really have a place in other modules.
// This module is only useful for chains using server/v2. Ante/Post handlers are setup via baseapp options in depinject.
type AppModule struct {
sigVerification ante.SigVerificationDecorator
feeTxValidator *ante.DeductFeeDecorator
sigVerification ante.SigVerificationDecorator
feeTxValidator *ante.DeductFeeDecorator
unorderTxValidator *ante.UnorderedTxDecorator
// txValidators contains tx validator that can be injected into the module via depinject.
// tx validators should be module based, but it can happen that you do not want to create a new module
// and simply depinject-it.
Expand All @@ -30,12 +31,14 @@ type AppModule struct {
func NewAppModule(
sigVerification ante.SigVerificationDecorator,
feeTxValidator *ante.DeductFeeDecorator,
unorderTxValidator *ante.UnorderedTxDecorator,
txValidators ...appmodulev2.TxValidator[transaction.Tx],
) AppModule {
return AppModule{
sigVerification: sigVerification,
feeTxValidator: feeTxValidator,
txValidators: txValidators,
sigVerification: sigVerification,
feeTxValidator: feeTxValidator,
unorderTxValidator: unorderTxValidator,
txValidators: txValidators,
}
}

Expand All @@ -57,5 +60,11 @@ func (a AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error {
return err
}

if a.unorderTxValidator != nil {
if err := a.unorderTxValidator.ValidateTx(ctx, tx); err != nil {
return err
}
}

return a.sigVerification.ValidateTx(ctx, tx)
}
Loading