Skip to content

Commit

Permalink
refactor!: simplify signing packages (#23470)
Browse files Browse the repository at this point in the history
  • Loading branch information
julienrbrt authored Jan 22, 2025
1 parent 9f71028 commit e441778
Show file tree
Hide file tree
Showing 51 changed files with 389 additions and 395 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i

### Improvements

* [#23470](https://github.com/cosmos/cosmos-sdk/pull/23470) Converge to use of one single sign mode type and signer data:
* Use api's signmode throughout the SDK to align with `cosmossdk.io/tx`. This allows developer not to juggle between sign mode types
* Deprecate `authsigning.SignerData` in favor of txsigning.SignerData and replace its usage
* Move helpers to go from one sign mode enum to another to `types/signing` instead of `x/auth/signing` (less dependency on `x`)

### Bug Fixes

### API Breaking Changes
Expand Down
20 changes: 12 additions & 8 deletions baseapp/msg_service_router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,19 @@ import (

abci "github.com/cometbft/cometbft/api/cometbft/abci/v1"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/types/known/anypb"

coretesting "cosmossdk.io/core/testing"
"cosmossdk.io/depinject"
"cosmossdk.io/log"
txsigning "cosmossdk.io/x/tx/signing"

"github.com/cosmos/cosmos-sdk/client/tx"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
)

Expand Down Expand Up @@ -143,9 +144,6 @@ func TestMsgService(t *testing.T) {
// set the TxDecoder in the BaseApp for minimal tx simulations
app.SetTxDecoder(txConfig.TxDecoder())

defaultSignMode, err := authsigning.APISignModeToInternal(txConfig.SignModeHandler().DefaultMode())
require.NoError(t, err)

testdata.RegisterInterfaces(interfaceRegistry)
testdata.RegisterMsgServer(
app.MsgServiceRouter(),
Expand Down Expand Up @@ -173,7 +171,7 @@ func TestMsgService(t *testing.T) {
sigV2 := signing.SignatureV2{
PubKey: priv.PubKey(),
Data: &signing.SingleSignatureData{
SignMode: defaultSignMode,
SignMode: txConfig.SignModeHandler().DefaultMode(),
Signature: nil,
},
Sequence: 0,
Expand All @@ -183,14 +181,20 @@ func TestMsgService(t *testing.T) {
require.NoError(t, err)

// Second round: all signer infos are set, so each signer can sign.
signerData := authsigning.SignerData{
anyPk, err := codectypes.NewAnyWithValue(priv.PubKey())
require.NoError(t, err)

signerData := txsigning.SignerData{
ChainID: "test",
AccountNumber: 0,
Sequence: 0,
PubKey: priv.PubKey(),
PubKey: &anypb.Any{
TypeUrl: anyPk.TypeUrl,
Value: anyPk.Value,
},
}
sigV2, err = tx.SignWithPrivKey(
context.TODO(), defaultSignMode, signerData,
context.TODO(), txConfig.SignModeHandler().DefaultMode(), signerData,
txBuilder, priv, txConfig, 0)
require.NoError(t, err)
err = txBuilder.SetSignatures(sigV2)
Expand Down
9 changes: 5 additions & 4 deletions client/tx/aux_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"google.golang.org/protobuf/types/known/anypb"
"google.golang.org/protobuf/types/known/timestamppb"

apisigning "cosmossdk.io/api/cosmos/tx/signing/v1beta1"
txv1beta1 "cosmossdk.io/api/cosmos/tx/v1beta1"
txsigning "cosmossdk.io/x/tx/signing"
"cosmossdk.io/x/tx/signing/aminojson"
Expand Down Expand Up @@ -127,15 +128,15 @@ func (b *AuxTxBuilder) SetPubKey(pk cryptotypes.PubKey) error {

// SetSignMode sets the aux signer's sign mode. Allowed sign modes are
// DIRECT_AUX and LEGACY_AMINO_JSON.
func (b *AuxTxBuilder) SetSignMode(mode signing.SignMode) error {
func (b *AuxTxBuilder) SetSignMode(mode apisigning.SignMode) error {
switch mode {
case signing.SignMode_SIGN_MODE_DIRECT_AUX, signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON:
case apisigning.SignMode_SIGN_MODE_DIRECT_AUX, apisigning.SignMode_SIGN_MODE_LEGACY_AMINO_JSON:
default:
return sdkerrors.ErrInvalidRequest.Wrapf("AuxTxBuilder can only sign with %s or %s",
signing.SignMode_SIGN_MODE_DIRECT_AUX, signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
apisigning.SignMode_SIGN_MODE_DIRECT_AUX, apisigning.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
}

b.auxSignerData.Mode = mode
b.auxSignerData.Mode, _ = signing.APISignModeToInternal(mode)
return nil
}

Expand Down
25 changes: 14 additions & 11 deletions client/tx/aux_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (

"github.com/stretchr/testify/require"

apisigning "cosmossdk.io/api/cosmos/tx/signing/v1beta1"

"github.com/cosmos/cosmos-sdk/client/tx"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/testutil"
Expand Down Expand Up @@ -53,7 +55,7 @@ func TestAuxTxBuilder(t *testing.T) {
{
"cannot set SIGN_MODE_DIRECT",
func() error {
return b.SetSignMode(signing.SignMode_SIGN_MODE_DIRECT)
return b.SetSignMode(apisigning.SignMode_SIGN_MODE_DIRECT)
},
true, "AuxTxBuilder can only sign with SIGN_MODE_DIRECT_AUX or SIGN_MODE_LEGACY_AMINO_JSON",
},
Expand Down Expand Up @@ -105,7 +107,7 @@ func TestAuxTxBuilder(t *testing.T) {
func() error {
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
require.NoError(t, b.SetSignMode(signing.SignMode_SIGN_MODE_DIRECT_AUX))
require.NoError(t, b.SetSignMode(apisigning.SignMode_SIGN_MODE_DIRECT_AUX))

_, err := b.GetSignBytes()
return err
Expand All @@ -117,7 +119,7 @@ func TestAuxTxBuilder(t *testing.T) {
func() error {
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
require.NoError(t, b.SetSignMode(signing.SignMode_SIGN_MODE_DIRECT_AUX))
require.NoError(t, b.SetSignMode(apisigning.SignMode_SIGN_MODE_DIRECT_AUX))

_, err := b.GetSignBytes()
require.NoError(t, err)
Expand All @@ -133,7 +135,7 @@ func TestAuxTxBuilder(t *testing.T) {
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
b.SetAddress(addr1Str)
require.NoError(t, b.SetSignMode(signing.SignMode_SIGN_MODE_DIRECT_AUX))
require.NoError(t, b.SetSignMode(apisigning.SignMode_SIGN_MODE_DIRECT_AUX))

_, err := b.GetSignBytes()
require.NoError(t, err)
Expand All @@ -154,7 +156,7 @@ func TestAuxTxBuilder(t *testing.T) {
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
b.SetAddress(addr1Str)
err := b.SetSignMode(signing.SignMode_SIGN_MODE_DIRECT_AUX)
err := b.SetSignMode(apisigning.SignMode_SIGN_MODE_DIRECT_AUX)
require.NoError(t, err)

_, err = b.GetSignBytes()
Expand All @@ -164,7 +166,7 @@ func TestAuxTxBuilder(t *testing.T) {
auxSignerData, err := b.GetAuxSignerData()

// Make sure auxSignerData is correctly populated
checkCorrectData(t, cdc, auxSignerData, signing.SignMode_SIGN_MODE_DIRECT_AUX)
checkCorrectData(t, cdc, auxSignerData, apisigning.SignMode_SIGN_MODE_DIRECT_AUX)

return err
},
Expand All @@ -176,7 +178,7 @@ func TestAuxTxBuilder(t *testing.T) {
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
b.SetAddress(addr1Str)
err := b.SetSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
err := b.SetSignMode(apisigning.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
require.NoError(t, err)

_, err = b.GetSignBytes()
Expand All @@ -195,7 +197,7 @@ func TestAuxTxBuilder(t *testing.T) {
require.NoError(t, b.SetMsgs(msg1))
require.NoError(t, b.SetPubKey(pub1))
b.SetAddress(addr1Str)
err := b.SetSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
err := b.SetSignMode(apisigning.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
require.NoError(t, err)

_, err = b.GetSignBytes()
Expand All @@ -205,7 +207,7 @@ func TestAuxTxBuilder(t *testing.T) {
auxSignerData, err := b.GetAuxSignerData()

// Make sure auxSignerData is correctly populated
checkCorrectData(t, cdc, auxSignerData, signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
checkCorrectData(t, cdc, auxSignerData, apisigning.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)

return err
},
Expand All @@ -229,7 +231,7 @@ func TestAuxTxBuilder(t *testing.T) {
}

// checkCorrectData that the auxSignerData's content matches the inputs we gave.
func checkCorrectData(t *testing.T, cdc codec.Codec, auxSignerData typestx.AuxSignerData, signMode signing.SignMode) {
func checkCorrectData(t *testing.T, cdc codec.Codec, auxSignerData typestx.AuxSignerData, signMode apisigning.SignMode) {
t.Helper()
pkAny, err := codectypes.NewAnyWithValue(pub1)
require.NoError(t, err)
Expand All @@ -247,6 +249,7 @@ func checkCorrectData(t *testing.T, cdc codec.Codec, auxSignerData typestx.AuxSi
require.Equal(t, chainID, auxSignerData.SignDoc.ChainId)
require.Equal(t, msgAny, body.GetMessages()[0])
require.Equal(t, pkAny, auxSignerData.SignDoc.PublicKey)
require.Equal(t, signMode, auxSignerData.Mode)
m, _ := signing.InternalSignModeToAPI(auxSignerData.Mode)
require.Equal(t, signMode, m)
require.Equal(t, rawSig, auxSignerData.Sig)
}
19 changes: 10 additions & 9 deletions client/tx/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/cosmos/go-bip39"
"github.com/spf13/pflag"

apisigning "cosmossdk.io/api/cosmos/tx/signing/v1beta1"
"cosmossdk.io/math"

"github.com/cosmos/cosmos-sdk/client"
Expand Down Expand Up @@ -47,7 +48,7 @@ type Factory struct {
feePayer sdk.AccAddress
gasPrices sdk.DecCoins
extOptions []*codectypes.Any
signMode signing.SignMode
signMode apisigning.SignMode
simulateAndExecute bool
preprocessTxHook client.PreprocessTxFn
}
Expand All @@ -62,18 +63,18 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) (Factory, e
return Factory{}, fmt.Errorf("failed to bind flags to viper: %w", err)
}

signMode := signing.SignMode_SIGN_MODE_UNSPECIFIED
signMode := apisigning.SignMode_SIGN_MODE_UNSPECIFIED
switch clientCtx.SignModeStr {
case flags.SignModeDirect:
signMode = signing.SignMode_SIGN_MODE_DIRECT
signMode = apisigning.SignMode_SIGN_MODE_DIRECT
case flags.SignModeLegacyAminoJSON:
signMode = signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON
signMode = apisigning.SignMode_SIGN_MODE_LEGACY_AMINO_JSON
case flags.SignModeDirectAux:
signMode = signing.SignMode_SIGN_MODE_DIRECT_AUX
signMode = apisigning.SignMode_SIGN_MODE_DIRECT_AUX
case flags.SignModeTextual:
signMode = signing.SignMode_SIGN_MODE_TEXTUAL
signMode = apisigning.SignMode_SIGN_MODE_TEXTUAL
case flags.SignModeEIP191:
signMode = signing.SignMode_SIGN_MODE_EIP_191
signMode = apisigning.SignMode_SIGN_MODE_EIP_191 //nolint:staticcheck // We still need to check if it was called
}

var accNum, accSeq uint64
Expand Down Expand Up @@ -237,12 +238,12 @@ func (f Factory) WithSimulateAndExecute(sim bool) Factory {
}

// SignMode returns the sign mode configured in the Factory
func (f Factory) SignMode() signing.SignMode {
func (f Factory) SignMode() apisigning.SignMode {
return f.signMode
}

// WithSignMode returns a copy of the Factory with an updated sign mode value.
func (f Factory) WithSignMode(mode signing.SignMode) Factory {
func (f Factory) WithSignMode(mode apisigning.SignMode) Factory {
f.signMode = mode
return f
}
Expand Down
25 changes: 16 additions & 9 deletions client/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ import (

gogogrpc "github.com/cosmos/gogoproto/grpc"
"github.com/spf13/pflag"
"google.golang.org/protobuf/types/known/anypb"

apisigning "cosmossdk.io/api/cosmos/tx/signing/v1beta1"
txsigning "cosmossdk.io/x/tx/signing"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/input"
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"
Expand Down Expand Up @@ -168,7 +173,7 @@ func CalculateGas(
// corresponding SignatureV2 if the signing is successful.
func SignWithPrivKey(
ctx context.Context,
signMode signing.SignMode, signerData authsigning.SignerData,
signMode apisigning.SignMode, signerData txsigning.SignerData,
txBuilder client.TxBuilder, priv cryptotypes.PrivKey, txConfig client.TxConfig,
accSeq uint64,
) (signing.SignatureV2, error) {
Expand Down Expand Up @@ -206,7 +211,7 @@ func SignWithPrivKey(
func countDirectSigners(data signing.SignatureData) int {
switch data := data.(type) {
case *signing.SingleSignatureData:
if data.SignMode == signing.SignMode_SIGN_MODE_DIRECT {
if data.SignMode == apisigning.SignMode_SIGN_MODE_DIRECT {
return 1
}

Expand Down Expand Up @@ -254,12 +259,9 @@ func Sign(ctx client.Context, txf Factory, name string, txBuilder client.TxBuild

var err error
signMode := txf.signMode
if signMode == signing.SignMode_SIGN_MODE_UNSPECIFIED {
if signMode == apisigning.SignMode_SIGN_MODE_UNSPECIFIED {
// use the SignModeHandler's default mode if unspecified
signMode, err = authsigning.APISignModeToInternal(txf.txConfig.SignModeHandler().DefaultMode())
if err != nil {
return err
}
signMode = txf.txConfig.SignModeHandler().DefaultMode()
}

k, err := txf.keybase.Key(name)
Expand All @@ -277,12 +279,17 @@ func Sign(ctx client.Context, txf Factory, name string, txBuilder client.TxBuild
return err
}

signerData := authsigning.SignerData{
anyPk, err := codectypes.NewAnyWithValue(pubKey)
if err != nil {
return err
}

signerData := txsigning.SignerData{
ChainID: txf.chainID,
AccountNumber: txf.accountNumber,
Sequence: txf.sequence,
PubKey: pubKey,
Address: addressStr,
PubKey: &anypb.Any{TypeUrl: anyPk.TypeUrl, Value: anyPk.Value},
}

// For SIGN_MODE_DIRECT, calling SetSignatures calls setSignerInfos on
Expand Down
17 changes: 7 additions & 10 deletions client/tx/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"github.com/stretchr/testify/require"
"google.golang.org/grpc"

apisigning "cosmossdk.io/api/cosmos/tx/signing/v1beta1"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
Expand Down Expand Up @@ -82,12 +84,10 @@ func TestCalculateGas(t *testing.T) {

for _, tc := range testCases {
txCfg, _ := newTestTxConfig()
defaultSignMode, err := signing.APISignModeToInternal(txCfg.SignModeHandler().DefaultMode())
require.NoError(t, err)

txf := Factory{}.
WithChainID("test-chain").
WithTxConfig(txCfg).WithSignMode(defaultSignMode)
WithTxConfig(txCfg).WithSignMode(txCfg.SignModeHandler().DefaultMode())

t.Run(tc.name, func(t *testing.T) {
mockClientCtx := mockContext{
Expand Down Expand Up @@ -120,9 +120,6 @@ func mockTxFactory(txCfg client.TxConfig) Factory {

func TestBuildSimTx(t *testing.T) {
txCfg, cdc := newTestTxConfig()
defaultSignMode, err := signing.APISignModeToInternal(txCfg.SignModeHandler().DefaultMode())
require.NoError(t, err)

kb, err := keyring.New(t.Name(), "test", t.TempDir(), nil, cdc)
require.NoError(t, err)

Expand All @@ -133,7 +130,7 @@ func TestBuildSimTx(t *testing.T) {
fromAddr, err := ac.BytesToString(sdk.AccAddress("from"))
require.NoError(t, err)

txf := mockTxFactory(txCfg).WithSignMode(defaultSignMode).WithKeybase(kb)
txf := mockTxFactory(txCfg).WithSignMode(txCfg.SignModeHandler().DefaultMode()).WithKeybase(kb)
msg := &countertypes.MsgIncreaseCounter{Signer: fromAddr, Count: 1}
bz, err := txf.BuildSimTx(msg)
require.NoError(t, err)
Expand Down Expand Up @@ -263,9 +260,9 @@ func TestSign(t *testing.T) {
txfNoKeybase := mockTxFactory(txConfig)
txfDirect := txfNoKeybase.
WithKeybase(kb).
WithSignMode(signingtypes.SignMode_SIGN_MODE_DIRECT)
WithSignMode(apisigning.SignMode_SIGN_MODE_DIRECT)
txfAmino := txfDirect.
WithSignMode(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
WithSignMode(apisigning.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
addr1, err := k1.GetAddress()
requireT.NoError(err)
addr2, err := k2.GetAddress()
Expand Down Expand Up @@ -423,7 +420,7 @@ func TestPreprocessHook(t *testing.T) {

txfDirect := mockTxFactory(txConfig).
WithKeybase(kb).
WithSignMode(signingtypes.SignMode_SIGN_MODE_DIRECT).
WithSignMode(apisigning.SignMode_SIGN_MODE_DIRECT).
WithPreprocessTxHook(preprocessHook)

addr1, err := kr.GetAddress()
Expand Down
Loading

0 comments on commit e441778

Please sign in to comment.