Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Commit

Permalink
imp(ante): refactor AnteHandler (#1479)
Browse files Browse the repository at this point in the history 
* imp(ante): refactor AnteHandler

* fix test

* test

* Adjust deprecated sdkerrors import (#1493)

* refactor test files

* Apply suggestions from code review

Co-authored-by: 4rgon4ut <59182467+4rgon4ut@users.noreply.github.com>

* lint

* prioritization comment

* fix test

Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com>
Co-authored-by: 4rgon4ut <59182467+4rgon4ut@users.noreply.github.com>
  • Loading branch information
@fedekunze @MalteHerrmann @4rgon4ut
3 people authored Nov 25, 2022
1 parent f4c7be2 commit 16fb2e1
Show file tree
Hide file tree
Showing 19 changed files with 597 additions and 508 deletions.
11 changes: 6 additions & 5 deletions app/ante/ante.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import (

tmlog "github.com/tendermint/tendermint/libs/log"

errorsmod "cosmossdk.io/errors"
"github.com/cosmos/cosmos-sdk/crypto/types/multisig"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
errortypes "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
authante "github.com/cosmos/cosmos-sdk/x/auth/ante"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
Expand Down Expand Up @@ -51,8 +52,8 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
// cosmos-sdk tx with dynamic fee extension
anteHandler = newCosmosAnteHandler(options)
default:
return ctx, sdkerrors.Wrapf(
sdkerrors.ErrUnknownExtensionOptions,
return ctx, errorsmod.Wrapf(
errortypes.ErrUnknownExtensionOptions,
"rejecting tx with unsupported extension option: %s", typeURL,
)
}
Expand All @@ -66,7 +67,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
case sdk.Tx:
anteHandler = newCosmosAnteHandler(options)
default:
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid transaction type: %T", tx)
return ctx, errorsmod.Wrapf(errortypes.ErrUnknownRequest, "invalid transaction type: %T", tx)
}

return anteHandler(ctx, tx, sim)
Expand All @@ -75,7 +76,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {

func Recover(logger tmlog.Logger, err *error) {
if r := recover(); r != nil {
*err = sdkerrors.Wrapf(sdkerrors.ErrPanic, "%v", r)
*err = errorsmod.Wrapf(errortypes.ErrPanic, "%v", r)

if e, ok := r.(error); ok {
logger.Error(
Expand Down
61 changes: 33 additions & 28 deletions app/ante/eip712.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ package ante
import (
"fmt"

errorsmod "cosmossdk.io/errors"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
errortypes "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
authante "github.com/cosmos/cosmos-sdk/x/auth/ante"
"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
Expand Down Expand Up @@ -63,12 +64,12 @@ func (svd Eip712SigVerificationDecorator) AnteHandle(ctx sdk.Context,

sigTx, ok := tx.(authsigning.SigVerifiableTx)
if !ok {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "tx %T doesn't implement authsigning.SigVerifiableTx", tx)
return ctx, errorsmod.Wrapf(errortypes.ErrInvalidType, "tx %T doesn't implement authsigning.SigVerifiableTx", tx)
}

authSignTx, ok := tx.(authsigning.Tx)
if !ok {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "tx %T doesn't implement the authsigning.Tx interface", tx)
return ctx, errorsmod.Wrapf(errortypes.ErrInvalidType, "tx %T doesn't implement the authsigning.Tx interface", tx)
}

// stdSigs contains the sequence number, account number, and signatures.
Expand All @@ -82,12 +83,16 @@ func (svd Eip712SigVerificationDecorator) AnteHandle(ctx sdk.Context,

// EIP712 allows just one signature
if len(sigs) != 1 {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "invalid number of signers (%d); EIP712 signatures allows just one signature", len(sigs))
return ctx, errorsmod.Wrapf(
errortypes.ErrTooManySignatures,
"invalid number of signers (%d); EIP712 signatures allows just one signature",
len(sigs),
)
}

// check that signer length and signature length are the same
if len(sigs) != len(signerAddrs) {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "invalid number of signer; expected: %d, got %d", len(signerAddrs), len(sigs))
return ctx, errorsmod.Wrapf(errortypes.ErrorInvalidSigner, "invalid number of signers; expected: %d, got %d", len(signerAddrs), len(sigs))
}

// EIP712 has just one signature, avoid looping here and only read index 0
Expand All @@ -102,13 +107,13 @@ func (svd Eip712SigVerificationDecorator) AnteHandle(ctx sdk.Context,
// retrieve pubkey
pubKey := acc.GetPubKey()
if !simulate && pubKey == nil {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set")
return ctx, errorsmod.Wrap(errortypes.ErrInvalidPubKey, "pubkey on account is not set")
}

// Check account sequence number.
if sig.Sequence != acc.GetSequence() {
return ctx, sdkerrors.Wrapf(
sdkerrors.ErrWrongSequence,
return ctx, errorsmod.Wrapf(
errortypes.ErrWrongSequence,
"account sequence mismatch, expected %d, got %d", acc.GetSequence(), sig.Sequence,
)
}
Expand All @@ -134,7 +139,7 @@ func (svd Eip712SigVerificationDecorator) AnteHandle(ctx sdk.Context,

if err := VerifySignature(pubKey, signerData, sig.Data, svd.signModeHandler, authSignTx); err != nil {
errMsg := fmt.Errorf("signature verification failed; please verify account number (%d) and chain-id (%s): %w", accNum, chainID, err)
return ctx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, errMsg.Error())
return ctx, errorsmod.Wrap(errortypes.ErrUnauthorized, errMsg.Error())
}

return next(ctx, tx, simulate)
Expand All @@ -152,20 +157,20 @@ func VerifySignature(
switch data := sigData.(type) {
case *signing.SingleSignatureData:
if data.SignMode != signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON {
return sdkerrors.Wrapf(sdkerrors.ErrNotSupported, "unexpected SignatureData %T: wrong SignMode", sigData)
return errorsmod.Wrapf(errortypes.ErrNotSupported, "unexpected SignatureData %T: wrong SignMode", sigData)
}

// Note: this prevents the user from sending thrash data in the signature field
// Note: this prevents the user from sending trash data in the signature field
if len(data.Signature) != 0 {
return sdkerrors.Wrap(sdkerrors.ErrTooManySignatures, "invalid signature value; EIP712 must have the cosmos transaction signature empty")
return errorsmod.Wrap(errortypes.ErrTooManySignatures, "invalid signature value; EIP712 must have the cosmos transaction signature empty")
}

// @contract: this code is reached only when Msg has Web3Tx extension (so this custom Ante handler flow),
// and the signature is SIGN_MODE_LEGACY_AMINO_JSON which is supported for EIP712 for now

msgs := tx.GetMsgs()
if len(msgs) == 0 {
return sdkerrors.Wrap(sdkerrors.ErrNoSignatures, "tx doesn't contain any msgs to verify signature")
return errorsmod.Wrap(errortypes.ErrNoSignatures, "tx doesn't contain any msgs to verify signature")
}

txBytes := legacytx.StdSignBytes(
Expand All @@ -182,33 +187,33 @@ func VerifySignature(

signerChainID, err := ethermint.ParseChainID(signerData.ChainID)
if err != nil {
return sdkerrors.Wrapf(err, "failed to parse chainID: %s", signerData.ChainID)
return errorsmod.Wrapf(err, "failed to parse chain-id: %s", signerData.ChainID)
}

txWithExtensions, ok := tx.(authante.HasExtensionOptionsTx)
if !ok {
return sdkerrors.Wrap(sdkerrors.ErrUnknownExtensionOptions, "tx doesnt contain any extensions")
return errorsmod.Wrap(errortypes.ErrUnknownExtensionOptions, "tx doesnt contain any extensions")
}
opts := txWithExtensions.GetExtensionOptions()
if len(opts) != 1 {
return sdkerrors.Wrap(sdkerrors.ErrUnknownExtensionOptions, "tx doesnt contain expected amount of extension options")
return errorsmod.Wrap(errortypes.ErrUnknownExtensionOptions, "tx doesnt contain expected amount of extension options")
}

extOpt, ok := opts[0].GetCachedValue().(*ethermint.ExtensionOptionsWeb3Tx)
if !ok {
return sdkerrors.Wrap(sdkerrors.ErrInvalidChainID, "unknown extension option")
return errorsmod.Wrap(errortypes.ErrUnknownExtensionOptions, "unknown extension option")
}

if extOpt.TypedDataChainID != signerChainID.Uint64() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidChainID, "invalid chainID")
return errorsmod.Wrap(errortypes.ErrInvalidChainID, "invalid chain-id")
}

if len(extOpt.FeePayer) == 0 {
return sdkerrors.Wrap(sdkerrors.ErrUnknownExtensionOptions, "no feePayer on ExtensionOptionsWeb3Tx")
return errorsmod.Wrap(errortypes.ErrUnknownExtensionOptions, "no feePayer on ExtensionOptionsWeb3Tx")
}
feePayer, err := sdk.AccAddressFromBech32(extOpt.FeePayer)
if err != nil {
return sdkerrors.Wrap(err, "failed to parse feePayer from ExtensionOptionsWeb3Tx")
return errorsmod.Wrap(err, "failed to parse feePayer from ExtensionOptionsWeb3Tx")
}

feeDelegation := &eip712.FeeDelegationOptions{
Expand All @@ -217,7 +222,7 @@ func VerifySignature(

typedData, err := eip712.WrapTxToTypedData(ethermintCodec, extOpt.TypedDataChainID, msgs[0], txBytes, feeDelegation)
if err != nil {
return sdkerrors.Wrap(err, "failed to pack tx data in EIP712 object")
return errorsmod.Wrap(err, "failed to create EIP-712 typed data from tx")
}

sigHash, _, err := apitypes.TypedDataAndHash(typedData)
Expand All @@ -227,7 +232,7 @@ func VerifySignature(

feePayerSig := extOpt.FeePayerSig
if len(feePayerSig) != ethcrypto.SignatureLength {
return sdkerrors.Wrap(sdkerrors.ErrorInvalidSigner, "signature length doesn't match typical [R||S||V] signature 65 bytes")
return errorsmod.Wrap(errortypes.ErrorInvalidSigner, "signature length doesn't match typical [R||S||V] signature 65 bytes")
}

// Remove the recovery offset if needed (ie. Metamask eip712 signature)
Expand All @@ -237,36 +242,36 @@ func VerifySignature(

feePayerPubkey, err := secp256k1.RecoverPubkey(sigHash, feePayerSig)
if err != nil {
return sdkerrors.Wrap(err, "failed to recover delegated fee payer from sig")
return errorsmod.Wrap(err, "failed to recover delegated fee payer from sig")
}

ecPubKey, err := ethcrypto.UnmarshalPubkey(feePayerPubkey)
if err != nil {
return sdkerrors.Wrap(err, "failed to unmarshal recovered fee payer pubkey")
return errorsmod.Wrap(err, "failed to unmarshal recovered fee payer pubkey")
}

pk := &ethsecp256k1.PubKey{
Key: ethcrypto.CompressPubkey(ecPubKey),
}

if !pubKey.Equals(pk) {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "feePayer pubkey %s is different from transaction pubkey %s", pubKey, pk)
return errorsmod.Wrapf(errortypes.ErrInvalidPubKey, "feePayer pubkey %s is different from transaction pubkey %s", pubKey, pk)
}

recoveredFeePayerAcc := sdk.AccAddress(pk.Address().Bytes())

if !recoveredFeePayerAcc.Equals(feePayer) {
return sdkerrors.Wrapf(sdkerrors.ErrorInvalidSigner, "failed to verify delegated fee payer %s signature", recoveredFeePayerAcc)
return errorsmod.Wrapf(errortypes.ErrorInvalidSigner, "failed to verify delegated fee payer %s signature", recoveredFeePayerAcc)
}

// VerifySignature of ethsecp256k1 accepts 64 byte signature [R||S]
// WARNING! Under NO CIRCUMSTANCES try to use pubKey.VerifySignature there
if !secp256k1.VerifySignature(pubKey.Bytes(), sigHash, feePayerSig[:len(feePayerSig)-1]) {
return sdkerrors.Wrap(sdkerrors.ErrorInvalidSigner, "unable to verify signer signature of EIP712 typed data")
return errorsmod.Wrap(errortypes.ErrorInvalidSigner, "unable to verify signer signature of EIP712 typed data")
}

return nil
default:
return sdkerrors.Wrapf(sdkerrors.ErrTooManySignatures, "unexpected SignatureData %T", sigData)
return errorsmod.Wrapf(errortypes.ErrTooManySignatures, "unexpected SignatureData %T", sigData)
}
}
Loading

0 comments on commit 16fb2e1

Please sign in to comment.