Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix(client/tx): simulate with correct pk #18472

Merged
merged 10 commits into from
Nov 17, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,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) [#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.
* (simulation) [#17911](https://github.com/cosmos/cosmos-sdk/pull/17911) Fix all problems with executing command `make test-sim-custom-genesis-fast` for simulation test.
Expand Down
39 changes: 28 additions & 11 deletions client/tx/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -33,6 +34,7 @@ type Factory struct {
timeoutHeight uint64
gasAdjustment float64
chainID string
fromName string
offline bool
generateOnly bool
memo string
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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")
}
Expand All @@ -453,6 +451,25 @@ 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 {
if multisigPubKey, ok := pk.(*multisig.LegacyAminoPubKey); ok {
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,
}
}
return &signing.SingleSignatureData{
SignMode: f.signMode,
}
JulianToledano marked this conversation as resolved.
Show resolved Hide resolved
}

// 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.
Expand Down
Loading