diff --git a/CHANGELOG.md b/CHANGELOG.md index 352362c6545c..5d70a8118662 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (client/tx) [#18472](https://github.com/cosmos/cosmos-sdk/pull/18472) Utilizes the correct Pubkey when simulating a transaction. * (baseapp) [#18486](https://github.com/cosmos/cosmos-sdk/pull/18486) Fixed FinalizeBlock calls not being passed to ABCIListeners * (baseapp) [#18383](https://github.com/cosmos/cosmos-sdk/pull/18383) Fixed a data race inside BaseApp.getContext, found by end-to-end (e2e) tests. * (client/server) [#18345](https://github.com/cosmos/cosmos-sdk/pull/18345) Consistently set viper prefix in client and server. It defaults for the binary name for both client and server. diff --git a/client/tx/aux_builder_test.go b/client/tx/aux_builder_test.go index 4758247f9e86..aa2bf7751d20 100644 --- a/client/tx/aux_builder_test.go +++ b/client/tx/aux_builder_test.go @@ -1,11 +1,10 @@ -package tx_test +package tx import ( "testing" "github.com/stretchr/testify/require" - "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" @@ -26,7 +25,7 @@ func TestAuxTxBuilder(t *testing.T) { // required for test case: "GetAuxSignerData works for DIRECT_AUX" counterModule.RegisterInterfaces(reg) - var b tx.AuxTxBuilder + var b AuxTxBuilder testcases := []struct { name string @@ -200,7 +199,7 @@ func TestAuxTxBuilder(t *testing.T) { for _, tc := range testcases { tc := tc t.Run(tc.name, func(t *testing.T) { - b = tx.NewAuxTxBuilder() + b = NewAuxTxBuilder() err := tc.malleate() if tc.expErr { diff --git a/client/tx/factory.go b/client/tx/factory.go index 02f6a41e159d..e3ca6041d9e0 100644 --- a/client/tx/factory.go +++ b/client/tx/factory.go @@ -15,6 +15,7 @@ import ( "github.com/cosmos/cosmos-sdk/client/flags" codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/crypto/keyring" + "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -33,6 +34,7 @@ type Factory struct { timeoutHeight uint64 gasAdjustment float64 chainID string + fromName string offline bool generateOnly bool memo string @@ -92,6 +94,7 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) (Factory, e accountRetriever: clientCtx.AccountRetriever, keybase: clientCtx.Keyring, chainID: clientCtx.ChainID, + fromName: clientCtx.FromName, offline: clientCtx.Offline, generateOnly: clientCtx.GenerateOnly, gas: gasSetting.Gas, @@ -406,10 +409,8 @@ func (f Factory) BuildSimTx(msgs ...sdk.Msg) ([]byte, error) { // Create an empty signature literal as the ante handler will populate with a // sentinel pubkey. sig := signing.SignatureV2{ - PubKey: pk, - Data: &signing.SingleSignatureData{ - SignMode: f.signMode, - }, + PubKey: pk, + Data: f.getSimSignatureData(pk), Sequence: f.Sequence(), } if err := txb.SetSignatures(sig); err != nil { @@ -435,16 +436,13 @@ func (f Factory) getSimPK() (cryptotypes.PubKey, error) { 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") + record, err := f.keybase.Key(f.fromName) + if err != nil { + return nil, err } - // take the first record just for simulation purposes - pk, ok = records[0].PubKey.GetCachedValue().(cryptotypes.PubKey) + pk, ok = record.PubKey.GetCachedValue().(cryptotypes.PubKey) if !ok { return nil, errors.New("cannot build signature for simulation, failed to convert proto Any to public key") } @@ -453,6 +451,26 @@ func (f Factory) getSimPK() (cryptotypes.PubKey, error) { return pk, nil } +// getSimSignatureData based on the pubKey type gets the correct SignatureData type +// to use for building a simulation tx. +func (f Factory) getSimSignatureData(pk cryptotypes.PubKey) signing.SignatureData { + multisigPubKey, ok := pk.(*multisig.LegacyAminoPubKey) + if !ok { + return &signing.SingleSignatureData{SignMode: f.signMode} + } + + multiSignatureData := make([]signing.SignatureData, 0, multisigPubKey.Threshold) + for i := uint32(0); i < multisigPubKey.Threshold; i++ { + multiSignatureData = append(multiSignatureData, &signing.SingleSignatureData{ + SignMode: f.SignMode(), + }) + } + + return &signing.MultiSignatureData{ + Signatures: multiSignatureData, + } +} + // 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. diff --git a/client/tx/factory_test.go b/client/tx/factory_test.go index 3b055781bd0b..402d178a98dc 100644 --- a/client/tx/factory_test.go +++ b/client/tx/factory_test.go @@ -1,4 +1,4 @@ -package tx_test +package tx import ( "testing" @@ -6,30 +6,114 @@ import ( "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/client" - "github.com/cosmos/cosmos-sdk/client/tx" + "github.com/cosmos/cosmos-sdk/codec" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" + "github.com/cosmos/cosmos-sdk/crypto/hd" + "github.com/cosmos/cosmos-sdk/crypto/keyring" + "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" + "github.com/cosmos/cosmos-sdk/crypto/types" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + "github.com/cosmos/cosmos-sdk/types/tx/signing" ) -func TestFactoryPrepate(t *testing.T) { +func TestFactoryPrepare(t *testing.T) { t.Parallel() - factory := tx.Factory{} + factory := Factory{} clientCtx := client.Context{} output, err := factory.Prepare(clientCtx.WithOffline(true)) require.NoError(t, err) require.Equal(t, output, factory) - factory = tx.Factory{}.WithAccountRetriever(client.MockAccountRetriever{ReturnAccNum: 10, ReturnAccSeq: 1}).WithAccountNumber(5) + factory = Factory{}.WithAccountRetriever(client.MockAccountRetriever{ReturnAccNum: 10, ReturnAccSeq: 1}).WithAccountNumber(5) output, err = factory.Prepare(clientCtx.WithFrom("foo")) require.NoError(t, err) require.NotEqual(t, output, factory) require.Equal(t, output.AccountNumber(), uint64(5)) require.Equal(t, output.Sequence(), uint64(1)) - factory = tx.Factory{}.WithAccountRetriever(client.MockAccountRetriever{ReturnAccNum: 10, ReturnAccSeq: 1}) + factory = Factory{}.WithAccountRetriever(client.MockAccountRetriever{ReturnAccNum: 10, ReturnAccSeq: 1}) output, err = factory.Prepare(clientCtx.WithFrom("foo")) require.NoError(t, err) require.NotEqual(t, output, factory) require.Equal(t, output.AccountNumber(), uint64(10)) require.Equal(t, output.Sequence(), uint64(1)) } + +func TestFactory_getSimPKType(t *testing.T) { + // setup keyring + registry := codectypes.NewInterfaceRegistry() + cryptocodec.RegisterInterfaces(registry) + k := keyring.NewInMemory(codec.NewProtoCodec(registry)) + + tests := []struct { + name string + fromName string + genKey func(fromName string, k keyring.Keyring) error + wantType types.PubKey + }{ + { + name: "simple key", + fromName: "testKey", + genKey: func(fromName string, k keyring.Keyring) error { + _, err := k.NewAccount(fromName, testdata.TestMnemonic, "", "", hd.Secp256k1) + return err + }, + wantType: (*secp256k1.PubKey)(nil), + }, + { + name: "multisig key", + fromName: "multiKey", + genKey: func(fromName string, k keyring.Keyring) error { + pk := multisig.NewLegacyAminoPubKey(1, []types.PubKey{&multisig.LegacyAminoPubKey{}}) + _, err := k.SaveMultisig(fromName, pk) + return err + }, + wantType: (*multisig.LegacyAminoPubKey)(nil), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.genKey(tt.fromName, k) + require.NoError(t, err) + f := Factory{ + keybase: k, + fromName: tt.fromName, + simulateAndExecute: true, + } + got, err := f.getSimPK() + require.NoError(t, err) + require.IsType(t, tt.wantType, got) + }) + } +} + +func TestFactory_getSimSignatureData(t *testing.T) { + tests := []struct { + name string + pk types.PubKey + wantType any + }{ + { + name: "simple pubkey", + pk: &secp256k1.PubKey{}, + wantType: (*signing.SingleSignatureData)(nil), + }, + { + name: "multisig pubkey", + pk: &multisig.LegacyAminoPubKey{}, + wantType: (*signing.MultiSignatureData)(nil), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := Factory{}.getSimSignatureData(tt.pk) + require.IsType(t, tt.wantType, got) + }) + } +} diff --git a/client/tx/legacy_test.go b/client/tx/legacy_test.go index 15aac031523e..cd7a64776fa4 100644 --- a/client/tx/legacy_test.go +++ b/client/tx/legacy_test.go @@ -1,4 +1,4 @@ -package tx_test +package tx import ( "github.com/cosmos/cosmos-sdk/testutil/testdata" diff --git a/client/tx/tx_test.go b/client/tx/tx_test.go index 340fbc804349..384c2177d650 100644 --- a/client/tx/tx_test.go +++ b/client/tx/tx_test.go @@ -1,4 +1,4 @@ -package tx_test +package tx import ( "context" @@ -9,12 +9,11 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/grpc" - ante "cosmossdk.io/x/auth/ante" + "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/auth/signing" authtx "cosmossdk.io/x/auth/tx" "github.com/cosmos/cosmos-sdk/client" - "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/crypto/hd" @@ -81,7 +80,7 @@ func TestCalculateGas(t *testing.T) { defaultSignMode, err := signing.APISignModeToInternal(txCfg.SignModeHandler().DefaultMode()) require.NoError(t, err) - txf := tx.Factory{}. + txf := Factory{}. WithChainID("test-chain"). WithTxConfig(txCfg).WithSignMode(defaultSignMode) @@ -90,7 +89,7 @@ func TestCalculateGas(t *testing.T) { gasUsed: tc.args.mockGasUsed, wantErr: tc.args.mockWantErr, } - simRes, gotAdjusted, err := tx.CalculateGas(mockClientCtx, txf.WithGasAdjustment(stc.args.adjustment)) + simRes, gotAdjusted, err := CalculateGas(mockClientCtx, txf.WithGasAdjustment(stc.args.adjustment)) if stc.expPass { require.NoError(t, err) require.Equal(t, simRes.GasInfo.GasUsed, stc.wantEstimate) @@ -104,8 +103,8 @@ func TestCalculateGas(t *testing.T) { } } -func mockTxFactory(txCfg client.TxConfig) tx.Factory { - return tx.Factory{}. +func mockTxFactory(txCfg client.TxConfig) Factory { + return Factory{}. WithTxConfig(txCfg). WithAccountNumber(50). WithSequence(23). @@ -196,7 +195,7 @@ func TestMnemonicInMemo(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - txf := tx.Factory{}. + txf := Factory{}. WithTxConfig(txConfig). WithAccountNumber(50). WithSequence(23). @@ -267,7 +266,7 @@ func TestSign(t *testing.T) { testCases := []struct { name string - txf tx.Factory + txf Factory txb client.TxBuilder from string overwrite bool @@ -354,7 +353,7 @@ func TestSign(t *testing.T) { var prevSigs []signingtypes.SignatureV2 for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - err = tx.Sign(context.TODO(), tc.txf, tc.from, tc.txb, tc.overwrite) + err = Sign(context.TODO(), tc.txf, tc.from, tc.txb, tc.overwrite) if len(tc.expectedPKs) == 0 { requireT.Error(err) } else { @@ -409,7 +408,7 @@ func TestPreprocessHook(t *testing.T) { txb, err := txfDirect.BuildUnsignedTx(msg1, msg2) requireT.NoError(err) - err = tx.Sign(context.TODO(), txfDirect, from, txb, false) + err = Sign(context.TODO(), txfDirect, from, txb, false) requireT.NoError(err) // Run preprocessing