Skip to content

Commit

Permalink
feat: EIP191 sign mode (backport cosmos#11533) (cosmos#11545)
Browse files Browse the repository at this point in the history 
* feat: EIP191 sign mode (cosmos#11533)

Adds [EIP-191](https://eips.ethereum.org/EIPS/eip-191) as a supported sign mode.

Ref: cosmos#10553 (comment)

---

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)

(cherry picked from commit dc66ddd)

* fix conflicts

* fix conflicts++

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: marbar3778 <marbar3778@yahoo.com>
  • Loading branch information
@mergify @fedekunze
@tac0turtle @Eengineer1
3 people authored and Eengineer1 committed Aug 26, 2022
1 parent dc7b1b2 commit 04c8c0e
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 174 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

* (tx) [#\11533](https://github.com/cosmos/cosmos-sdk/pull/11533) Register [`EIP191`](https://eips.ethereum.org/EIPS/eip-191) as an available `SignMode` for chains to use.
* [\#11430](https://github.com/cosmos/cosmos-sdk/pull/11430) Introduce a new `grpc-only` flag, such that when enabled, will start the node in a query-only mode. Note, gRPC MUST be enabled with this flag.
* (x/bank) [\#11417](https://github.com/cosmos/cosmos-sdk/pull/11417) Introduce a new `SpendableBalances` gRPC query that retrieves an account's total (paginated) spendable balances.
* (x/bank) [\#10771](https://github.com/cosmos/cosmos-sdk/pull/10771) Add safety check on bank module perms to allow module-specific mint restrictions (e.g. only minting a certain denom).
Expand Down
38 changes: 3 additions & 35 deletions client/cmd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,21 +220,8 @@ func readTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, err
clientCtx = clientCtx.WithSignModeStr(signModeStr)
}

if clientCtx.FeePayer == nil || flagSet.Changed(flags.FlagFeePayer) {
payer, _ := flagSet.GetString(flags.FlagFeePayer)

if payer != "" {
payerAcc, err := sdk.AccAddressFromBech32(payer)
if err != nil {
return clientCtx, err
}

clientCtx = clientCtx.WithFeePayerAddress(payerAcc)
}
}

if clientCtx.FeeGranter == nil || flagSet.Changed(flags.FlagFeeGranter) {
granter, _ := flagSet.GetString(flags.FlagFeeGranter)
if clientCtx.FeeGranter == nil || flagSet.Changed(flags.FlagFeeAccount) {
granter, _ := flagSet.GetString(flags.FlagFeeAccount)

if granter != "" {
granterAcc, err := sdk.AccAddressFromBech32(granter)
Expand All @@ -248,7 +235,7 @@ func readTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, err

if clientCtx.From == "" || flagSet.Changed(flags.FlagFrom) {
from, _ := flagSet.GetString(flags.FlagFrom)
fromAddr, fromName, keyType, err := GetFromFields(clientCtx, clientCtx.Keyring, from)
fromAddr, fromName, keyType, err := GetFromFields(clientCtx.Keyring, from, clientCtx.GenerateOnly)
if err != nil {
return clientCtx, err
}
Expand All @@ -263,25 +250,6 @@ func readTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, err
clientCtx = clientCtx.WithSignModeStr(flags.SignModeLegacyAminoJSON)
}
}

if !clientCtx.IsAux || flagSet.Changed(flags.FlagAux) {
isAux, _ := flagSet.GetBool(flags.FlagAux)
clientCtx = clientCtx.WithAux(isAux)
if isAux {
// If the user didn't explicitly set an --output flag, use JSON by
// default.
if clientCtx.OutputFormat == "" || !flagSet.Changed(cli.OutputFlag) {
clientCtx = clientCtx.WithOutputFormat("json")
}

// If the user didn't explicitly set a --sign-mode flag, use
// DIRECT_AUX by default.
if clientCtx.SignModeStr == "" || !flagSet.Changed(flags.FlagSignMode) {
clientCtx = clientCtx.WithSignModeStr(flags.SignModeDirectAux)
}
}
}

return clientCtx, nil
}

Expand Down
2 changes: 2 additions & 0 deletions client/flags/flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ const (
SignModeDirect = "direct"
// SignModeLegacyAminoJSON is the value of the --sign-mode flag for SIGN_MODE_LEGACY_AMINO_JSON
SignModeLegacyAminoJSON = "amino-json"
// SignModeEIP191 is the value of the --sign-mode flag for SIGN_MODE_EIP_191
SignModeEIP191 = "eip-191"
)

// List of CLI flags
Expand Down
109 changes: 17 additions & 92 deletions client/tx/factory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"os"

"cosmossdk.io/math"
"github.com/spf13/pflag"

"github.com/cosmos/cosmos-sdk/client"
Expand All @@ -14,7 +13,6 @@ import (
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/tx"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
)

Expand All @@ -30,13 +28,8 @@ type Factory struct {
timeoutHeight uint64
gasAdjustment float64
chainID string
offline bool
generateOnly bool
memo string
fees sdk.Coins
tip *tx.Tip
feeGranter sdk.AccAddress
feePayer sdk.AccAddress
gasPrices sdk.DecCoins
signMode signing.SignMode
simulateAndExecute bool
Expand All @@ -52,8 +45,6 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) Factory {
signMode = signing.SignMode_SIGN_MODE_DIRECT
case flags.SignModeLegacyAminoJSON:
signMode = signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON
case flags.SignModeDirectAux:
signMode = signing.SignMode_SIGN_MODE_DIRECT_AUX
case flags.SignModeEIP191:
signMode = signing.SignMode_SIGN_MODE_EIP_191
}
Expand All @@ -72,8 +63,6 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) Factory {
accountRetriever: clientCtx.AccountRetriever,
keybase: clientCtx.Keyring,
chainID: clientCtx.ChainID,
offline: clientCtx.Offline,
generateOnly: clientCtx.GenerateOnly,
gas: gasSetting.Gas,
simulateAndExecute: gasSetting.Simulate,
accountNumber: accNum,
Expand All @@ -82,18 +71,11 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) Factory {
gasAdjustment: gasAdj,
memo: memo,
signMode: signMode,
feeGranter: clientCtx.FeeGranter,
feePayer: clientCtx.FeePayer,
}

feesStr, _ := flagSet.GetString(flags.FlagFees)
f = f.WithFees(feesStr)

tipsStr, _ := flagSet.GetString(flags.FlagTip)
// Add tips to factory. The tipper is necessarily the Msg signer, i.e.
// the from address.
f = f.WithTips(tipsStr, clientCtx.FromAddress.String())

gasPricesStr, _ := flagSet.GetString(flags.FlagGasPrices)
f = f.WithGasPrices(gasPricesStr)

Expand Down Expand Up @@ -151,20 +133,6 @@ func (f Factory) WithFees(fees string) Factory {
return f
}

// WithTips returns a copy of the Factory with an updated tip.
func (f Factory) WithTips(tip string, tipper string) Factory {
parsedTips, err := sdk.ParseCoinsNormalized(tip)
if err != nil {
panic(err)
}

f.tip = &tx.Tip{
Tipper: tipper,
Amount: parsedTips,
}
return f
}

// WithGasPrices returns a copy of the Factory with updated gas prices.
func (f Factory) WithGasPrices(gasPrices string) Factory {
parsedGasPrices, err := sdk.ParseDecCoins(gasPrices)
Expand Down Expand Up @@ -230,26 +198,10 @@ func (f Factory) WithTimeoutHeight(height uint64) Factory {
return f
}

// WithFeeGranter returns a copy of the Factory with an updated fee granter.
func (f Factory) WithFeeGranter(fg sdk.AccAddress) Factory {
f.feeGranter = fg
return f
}

// WithFeePayer returns a copy of the Factory with an updated fee granter.
func (f Factory) WithFeePayer(fp sdk.AccAddress) Factory {
f.feePayer = fp
return f
}

// BuildUnsignedTx builds a transaction to be signed given a set of messages.
// Once created, the fee, memo, and messages are set.
func (f Factory) BuildUnsignedTx(msgs ...sdk.Msg) (client.TxBuilder, error) {
if f.offline && f.generateOnly {
if f.chainID != "" {
return nil, fmt.Errorf("chain ID cannot be used when offline and generate-only flags are set")
}
} else if f.chainID == "" {
if f.chainID == "" {
return nil, fmt.Errorf("chain ID required but not specified")
}

Expand All @@ -260,7 +212,7 @@ func (f Factory) BuildUnsignedTx(msgs ...sdk.Msg) (client.TxBuilder, error) {
return nil, errors.New("cannot provide both fees and gas prices")
}

glDec := math.LegacyNewDec(int64(f.gas))
glDec := sdk.NewDec(int64(f.gas))

// Derive the fees based on the provided gas prices, where
// fee = ceil(gasPrice * gasLimit).
Expand All @@ -281,8 +233,6 @@ func (f Factory) BuildUnsignedTx(msgs ...sdk.Msg) (client.TxBuilder, error) {
tx.SetMemo(f.memo)
tx.SetFeeAmount(fees)
tx.SetGasLimit(f.gas)
tx.SetFeeGranter(f.feeGranter)
tx.SetFeePayer(f.feePayer)
tx.SetTimeoutHeight(f.TimeoutHeight())

return tx, nil
Expand All @@ -298,14 +248,7 @@ func (f Factory) PrintUnsignedTx(clientCtx client.Context, msgs ...sdk.Msg) erro
return errors.New("cannot estimate gas in offline mode")
}

// Prepare TxFactory with acc & seq numbers as CalculateGas requires
// account and sequence numbers to be set
preparedTxf, err := f.Prepare(clientCtx)
if err != nil {
return err
}

_, adjusted, err := CalculateGas(clientCtx, preparedTxf, msgs...)
_, adjusted, err := CalculateGas(clientCtx, f, msgs...)
if err != nil {
return err
}
Expand Down Expand Up @@ -336,9 +279,19 @@ func (f Factory) BuildSimTx(msgs ...sdk.Msg) ([]byte, error) {
return nil, err
}

pk, err := f.getSimPK()
if err != nil {
return nil, err
// use the first element from the list of keys in order to generate a valid
// pubkey that supports multiple algorithms

var pk cryptotypes.PubKey = &secp256k1.PubKey{} // use default public key type

if f.keybase != nil {
infos, _ := f.keybase.List()
if len(infos) == 0 {
return nil, errors.New("cannot build signature for simulation, key infos slice is empty")
}

// take the first info record just for simulation purposes
pk = infos[0].GetPubKey()
}

// Create an empty signature literal as the ante handler will populate with a
Expand All @@ -357,41 +310,13 @@ func (f Factory) BuildSimTx(msgs ...sdk.Msg) ([]byte, error) {
return f.txConfig.TxEncoder()(txb.GetTx())
}

// getSimPK gets the public key to use for building a simulation tx.
// Note, we should only check for keys in the keybase if we are in simulate and execute mode,
// e.g. when using --gas=auto.
// When using --dry-run, we are is simulation mode only and should not check the keybase.
// Ref: https://github.com/cosmos/cosmos-sdk/issues/11283
func (f Factory) getSimPK() (cryptotypes.PubKey, error) {
var (
ok bool
pk cryptotypes.PubKey = &secp256k1.PubKey{} // use default public key type
)

// Use the first element from the list of keys in order to generate a valid
// pubkey that supports multiple algorithms.
if f.simulateAndExecute && f.keybase != nil {
records, _ := f.keybase.List()
if len(records) == 0 {
return nil, errors.New("cannot build signature for simulation, key records slice is empty")
}

// take the first record just for simulation purposes
pk, ok = records[0].PubKey.GetCachedValue().(cryptotypes.PubKey)
if !ok {
return nil, errors.New("cannot build signature for simulation, failed to convert proto Any to public key")
}
}

return pk, nil
}

// Prepare ensures the account defined by ctx.GetFromAddress() exists and
// if the account number and/or the account sequence number are zero (not set),
// they will be queried for and set on the provided Factory. A new Factory with
// the updated fields will be returned.
func (f Factory) Prepare(clientCtx client.Context) (Factory, error) {
fc := f

from := clientCtx.GetFromAddress()

if err := fc.accountRetriever.EnsureExists(clientCtx, from); err != nil {
Expand Down
25 changes: 5 additions & 20 deletions proto/cosmos/tx/signing/v1beta1/signing.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,42 +7,27 @@ import "google/protobuf/any.proto";
option go_package = "github.com/cosmos/cosmos-sdk/types/tx/signing";

// SignMode represents a signing mode with its own security guarantees.
//
// This enum should be considered a registry of all known sign modes
// in the Cosmos ecosystem. Apps are not expected to support all known
// sign modes. Apps that would like to support custom sign modes are
// encouraged to open a small PR against this file to add a new case
// to this SignMode enum describing their sign mode so that different
// apps have a consistent version of this enum.
enum SignMode {
// SIGN_MODE_UNSPECIFIED specifies an unknown signing mode and will be
// rejected.
// rejected
SIGN_MODE_UNSPECIFIED = 0;

// SIGN_MODE_DIRECT specifies a signing mode which uses SignDoc and is
// verified with raw bytes from Tx.
// verified with raw bytes from Tx
SIGN_MODE_DIRECT = 1;

// SIGN_MODE_TEXTUAL is a future signing mode that will verify some
// human-readable textual representation on top of the binary representation
// from SIGN_MODE_DIRECT. It is currently not supported.
// from SIGN_MODE_DIRECT
SIGN_MODE_TEXTUAL = 2;

// SIGN_MODE_DIRECT_AUX specifies a signing mode which uses
// SignDocDirectAux. As opposed to SIGN_MODE_DIRECT, this sign mode does not
// require signers signing over other signers' `signer_info`. It also allows
// for adding Tips in transactions.
//
// Since: cosmos-sdk 0.46
SIGN_MODE_DIRECT_AUX = 3;

// SIGN_MODE_LEGACY_AMINO_JSON is a backwards compatibility mode which uses
// Amino JSON and will be removed in the future.
// Amino JSON and will be removed in the future
SIGN_MODE_LEGACY_AMINO_JSON = 127;

// SIGN_MODE_EIP_191 specifies the sign mode for EIP 191 signing on the Cosmos
// SDK. Ref: https://eips.ethereum.org/EIPS/eip-191
//
//
// Currently, SIGN_MODE_EIP_191 is registered as a SignMode enum variant,
// but is not implemented on the SDK by default. To enable EIP-191, you need
// to pass a custom `TxConfig` that has an implementation of
Expand Down

0 comments on commit 04c8c0e

Please sign in to comment.