Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add ledger/multisig detection in SignTx functions #9026

Merged
merged 6 commits into from
Mar 31, 2021
Merged

Add ledger/multisig detection in SignTx functions #9026

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b21597a
Add ledger/multisig detection in SignTx functions
amaury1093 Mar 30, 2021
6371572
Fix tests
amaury1093 Mar 30, 2021
b820f61
Update CHANGELOG.md
amaury1093 Mar 30, 2021
2c0b329
update cl
amaury1093 Mar 30, 2021
51e8df8
Merge branch 'master' into am/9023-gentx
Mar 30, 2021
41499bc
Merge branch 'master' into am/9023-gentx
amaury1093 Mar 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions x/auth/client/cli/tx_sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/client/tx"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
authclient "github.com/cosmos/cosmos-sdk/x/auth/client"
"github.com/cosmos/cosmos-sdk/x/auth/client/rest"
)
Expand Down Expand Up @@ -114,9 +113,6 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error {
return err
}
} else {
if txFactory.SignMode() == signing.SignMode_SIGN_MODE_UNSPECIFIED {
txFactory = txFactory.WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) //nolint:staticcheck
}
err = authclient.SignTxWithSignerAddress(
txFactory, clientCtx, multisigAddr, clientCtx.GetFromName(), txBuilder, clientCtx.Offline, true)
}
Expand Down Expand Up @@ -218,9 +214,7 @@ func makeSignCmd() func(cmd *cobra.Command, args []string) error {
if err != nil {
return err
}
if txF.SignMode() == signing.SignMode_SIGN_MODE_UNSPECIFIED {
txF = txF.WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) //nolint:staticcheck
}

txCfg := clientCtx.TxConfig
txBuilder, err := txCfg.WrapTxBuilder(newTx)
if err != nil {
Expand Down
21 changes: 18 additions & 3 deletions x/auth/client/tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ import (
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/tx"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
"github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx"
)

Expand All @@ -37,13 +39,20 @@ func PrintUnsignedStdTx(txBldr tx.Factory, clientCtx client.Context, msgs []sdk.
// SignTx signs a transaction managed by the TxBuilder using a `name` key stored in Keybase.
// The new signature is appended to the TxBuilder when overwrite=false or overwritten otherwise.
// Don't perform online validation or lookups if offline is true.
func SignTx(txFactory tx.Factory, clientCtx client.Context, name string, stdTx client.TxBuilder, offline, overwriteSig bool) error {
func SignTx(txFactory tx.Factory, clientCtx client.Context, name string, txBuilder client.TxBuilder, offline, overwriteSig bool) error {
info, err := txFactory.Keybase().Key(name)
if err != nil {
return err
}

// Ledger and Multisigs only support LEGACY_AMINO_JSON signing.
if txFactory.SignMode() == signing.SignMode_SIGN_MODE_UNSPECIFIED &&
(info.GetType() == keyring.TypeLedger || info.GetType() == keyring.TypeMulti) {
txFactory = txFactory.WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) //nolint:staticcheck
}

addr := sdk.AccAddress(info.GetPubKey().Address())
if !isTxSigner(addr, stdTx.GetTx().GetSigners()) {
if !isTxSigner(addr, txBuilder.GetTx().GetSigners()) {
return fmt.Errorf("%s: %s", sdkerrors.ErrorInvalidSigner, name)
}
if !offline {
Expand All @@ -53,14 +62,20 @@ func SignTx(txFactory tx.Factory, clientCtx client.Context, name string, stdTx c
}
}

return tx.Sign(txFactory, name, stdTx, overwriteSig)
return tx.Sign(txFactory, name, txBuilder, overwriteSig)
}

// SignTxWithSignerAddress attaches a signature to a transaction.
// Don't perform online validation or lookups if offline is true, else
// populate account and sequence numbers from a foreign account.
// This function should only be used when signing with a multisig. For
// normal keys, please use SignTx directly.
Comment on lines +71 to +72
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is called twice in the sdk, and both are with multisigs. As I understand, there's no use-case for calling this without a multisig, correct?

Copy link
Member

Choose a reason for hiding this comment

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

That's probably right

func SignTxWithSignerAddress(txFactory tx.Factory, clientCtx client.Context, addr sdk.AccAddress,
name string, txBuilder client.TxBuilder, offline, overwrite bool) (err error) {
// Multisigs only support LEGACY_AMINO_JSON signing.
if txFactory.SignMode() == signing.SignMode_SIGN_MODE_UNSPECIFIED {
txFactory = txFactory.WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) //nolint:staticcheck
}

// check whether the address is a signer
if !isTxSigner(addr, txBuilder.GetTx().GetSigners()) {
Expand Down