Skip to content

Commit

Permalink
Relax Chain ID format requirement for EIP-712 verification
Browse files Browse the repository at this point in the history
The upstream Eip712SigVerificationDecorator requires the Cosmos Chain ID
to adhere to a format like "evmos_7001-1" which does not work for
Gravity's "gravity-bridge-3" format. This commit adds a EvmChainId value
to the AnteHandler HandlerOptions so that Gravity can override that
check and use its existing Chain ID format.

Chain IDs are difficult to change and cause a lot of issues with IBC
channels once changed. By relaxing this restriction the integration of
EIP-712 is made much simpler.
  • Loading branch information
ChristianBorst committed Apr 18, 2024
1 parent e1e27e1 commit abd1a5d
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 11 deletions.
36 changes: 26 additions & 10 deletions app/ante/eip712.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,23 @@ func init() {
type Eip712SigVerificationDecorator struct {
ak evmtypes.AccountKeeper
signModeHandler authsigning.SignModeHandler
evmChainId string
}

// NewEip712SigVerificationDecorator creates a new Eip712SigVerificationDecorator
func NewEip712SigVerificationDecorator(ak evmtypes.AccountKeeper, signModeHandler authsigning.SignModeHandler) Eip712SigVerificationDecorator {
// Allows an optional EVM Chain ID (e.g. 1 for Ethereum Mainnet). The upstream Evmos repo requires the Cosmos Chain ID
// to be parseable by a regex but evmChainId removes that restriction by allowing an override.
func NewEip712SigVerificationDecorator(ak evmtypes.AccountKeeper, signModeHandler authsigning.SignModeHandler, evmChainId string) Eip712SigVerificationDecorator {
return Eip712SigVerificationDecorator{
ak: ak,
signModeHandler: signModeHandler,
evmChainId: evmChainId,
}
}

// AnteHandle handles validation of EIP712 signed cosmos txs.
// it is not run on RecheckTx
// Note that this decorator differs from the upstream repo by removing the chain ID format restriction
func (svd Eip712SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
// no need to verify signatures on recheck tx
if ctx.IsReCheckTx() {
Expand Down Expand Up @@ -116,6 +121,18 @@ func (svd Eip712SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx,
accNum = acc.GetAccountNumber()
}

// There are two ChainIDs to verify, the Cosmos string and the EVM number. First try the EVM override value,
// then try to parse if no override provided
var evmChainID string = svd.evmChainId
if evmChainID == "" {
cId, err := ethermint.ParseChainID(chainID)

if err != nil {
return ctx, sdkerrors.Wrapf(err, "failed to parse chainID: %s", chainID)
}
evmChainID = cId.String()
}

signerData := authsigning.SignerData{
ChainID: chainID,
AccountNumber: accNum,
Expand All @@ -126,8 +143,8 @@ func (svd Eip712SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx,
return next(ctx, tx, simulate)
}

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

Expand All @@ -136,12 +153,15 @@ func (svd Eip712SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx,

// VerifySignature verifies a transaction signature contained in SignatureData abstracting over different signing modes
// and single vs multi-signatures.
// Note that this decorator differs from the upstream repo by removing the chain ID format restriction, so signerData.ChainID is not parsed.
// The evmChainID parameter is used instead of Cosmos ChainID parsing
func VerifySignature(
pubKey cryptotypes.PubKey,
signerData authsigning.SignerData,
sigData signing.SignatureData,
_ authsigning.SignModeHandler,
tx authsigning.Tx,
evmChainID string,
) error {
switch data := sigData.(type) {
case *signing.SingleSignatureData:
Expand Down Expand Up @@ -174,11 +194,6 @@ func VerifySignature(
msgs, tx.GetMemo(),
)

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

txWithExtensions, ok := tx.(authante.HasExtensionOptionsTx)
if !ok {
return sdkerrors.Wrap(sdkerrors.ErrUnknownExtensionOptions, "tx doesnt contain any extensions")
Expand All @@ -199,8 +214,9 @@ func VerifySignature(
return sdkerrors.Wrap(sdkerrors.ErrInvalidChainID, "unknown extension option")
}

if extOpt.TypedDataChainID != signerChainID.Uint64() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidChainID, "invalid chainID")
typedDataChainID := fmt.Sprint(extOpt.TypedDataChainID)
if typedDataChainID != evmChainID {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidChainID, "eip-712 domain chainID: (%s) != (%s)", typedDataChainID, evmChainID)
}

if len(extOpt.FeePayer) == 0 {
Expand Down
5 changes: 4 additions & 1 deletion app/ante/handler_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (

// HandlerOptions extend the SDK's AnteHandler options by requiring the IBC
// channel keeper, EVM Keeper and Fee Market Keeper.
// Note that this struct differs from the upstream repo by adding the optional EvmChainID override used in
// EIP-712 signature verification instead of parsing the Cosmos Chain ID
type HandlerOptions struct {
AccountKeeper evmtypes.AccountKeeper
BankKeeper evmtypes.BankKeeper
Expand All @@ -26,6 +28,7 @@ type HandlerOptions struct {
SignModeHandler authsigning.SignModeHandler
SigGasConsumer func(meter sdk.GasMeter, sig signing.SignatureV2, params authtypes.Params) error
MaxTxGasWanted uint64
EvmChainID string
}

func (options HandlerOptions) validate() error {
Expand Down Expand Up @@ -104,7 +107,7 @@ func newCosmosAnteHandlerEip712(options HandlerOptions) sdk.AnteHandler {
ante.NewValidateSigCountDecorator(options.AccountKeeper),
ante.NewSigGasConsumeDecorator(options.AccountKeeper, options.SigGasConsumer),
// Note: signature verification uses EIP instead of the cosmos signature validator
NewEip712SigVerificationDecorator(options.AccountKeeper, options.SignModeHandler),
NewEip712SigVerificationDecorator(options.AccountKeeper, options.SignModeHandler, options.EvmChainID),
ante.NewIncrementSequenceDecorator(options.AccountKeeper),
ibcante.NewAnteDecorator(options.IBCKeeper),
NewGasWantedDecorator(options.EvmKeeper, options.FeeMarketKeeper),
Expand Down

0 comments on commit abd1a5d

Please sign in to comment.