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

func CalculateGas not support multisigaddress ? #8821

Closed
4 tasks
tpkeeper opened this issue Mar 9, 2021 · 6 comments
Closed
4 tasks

func CalculateGas not support multisigaddress ? #8821

tpkeeper opened this issue Mar 9, 2021 · 6 comments
Assignees
Labels
S:needs more info This bug can't be addressed until more information is provided by the reporter. T: Dev UX UX for SDK developers (i.e. how to call our code)
Milestone

Comments

@tpkeeper
Copy link

tpkeeper commented Mar 9, 2021

Summary of Bug

func CalculateGas(
queryFunc func(string, []byte) ([]byte, int64, error), txf Factory, msgs ...sdk.Msg,
) (tx.SimulateResponse, uint64, error)

try send from multisig address with auto cal gas,got this err:

expected *signing.MultiSignatureData, got, *signing.SingleSignatureData: invalid request
github.com/cosmos/cosmos-sdk/client.Context.queryABCI
/Users/tpkeeper/go/pkg/mod/github.com/cosmos/[email protected]/client/query.go:82
github.com/cosmos/cosmos-sdk/client.Context.query
/Users/tpkeeper/go/pkg/mod/github.com/cosmos/[email protected]/client/query.go:97
github.com/cosmos/cosmos-sdk/client.Context.QueryWithData
/Users/tpkeeper/go/pkg/mod/github.com/cosmos/[email protected]/client/query.go:39
github.com/cosmos/cosmos-sdk/client/tx.CalculateGas
/Users/tpkeeper/go/pkg/mod/github.com/cosmos/[email protected]/client/tx/tx.go:291

Version

v0.41.4

Steps to Reproduce

send from multisigaddress with auto gas


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@aaronc aaronc added the T:Bug label Mar 9, 2021
@aaronc
Copy link
Member

aaronc commented Mar 9, 2021

Can you share more info about the parameters you have passed in? Specifically the tx.Factory and sdk.Msgs. The error message indicates that you have used a multisig pubkey without an actual multi-signature.

@aaronc aaronc added the S:needs more info This bug can't be addressed until more information is provided by the reporter. label Mar 9, 2021
@tpkeeper
Copy link
Author

Can you share more info about the parameters you have passed in? Specifically the tx.Factory and sdk.Msgs. The error message indicates that you have used a multisig pubkey without an actual multi-signature.

tx.Factory

        account, err := c.clientCtx.AccountRetriever.GetAccount(c.clientCtx, c.clientCtx.GetFromAddress())
	if err != nil {
		return nil, err
	}
	cmd := cobra.Command{}
	txf := clientTx.NewFactoryCLI(c.clientCtx, cmd.Flags())
	txf = txf.WithSequence(account.GetSequence()).
		WithAccountNumber(account.GetAccountNumber()).
		WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON). //multi sig need this mod
		WithGasAdjustment(1.5).
		WithGasPrices(c.gasPrice).
		WithGas(0).
		WithSimulateAndExecute(true)


	_, adjusted, err := clientTx.CalculateGas(c.clientCtx.QueryWithData, txf, msgs...)
	if err != nil {
		return nil, err
	}
	txf = txf.WithGas(adjusted)

	txBuilderRaw, err := clientTx.BuildUnsignedTx(txf, msgs...)

sdk.Msg

msg := xBankTypes.NewMsgSend(c.clientCtx.GetFromAddress(), toAddr, amount)

@clevinson clevinson added T: Dev UX UX for SDK developers (i.e. how to call our code) and removed T:Bug labels Mar 10, 2021
@clevinson clevinson added this to the v0.42.X milestone Mar 10, 2021
@amaury1093
Copy link
Contributor

The issue is that CalculateGas doesn't handle multisigs right now, it hardcodes SingleSignature:

cosmos-sdk/client/tx/tx.go

Lines 260 to 265 in 1fa2c22

// Create an empty signature literal as the ante handler will populate with a
// sentinel pubkey.
sig := signing.SignatureV2{
PubKey: &secp256k1.PubKey{},
Data: &signing.SingleSignatureData{
SignMode: txf.signMode,

To be honest, I think the tx.Factory is just not well-designed, it has confusing overlaps with client.Context and TxBuilder, so I prefer to mark this issue as wontfix

As an alternative, users should use the Simulate gRPC endpoint to get gas, and to create a multisig tx either 1/ use the CLI or 2/ manually add MultiSignatureData to a txBuilder. @technicallyty is creating a PR with 1/.

Multisig UX (CLI and programmatic) improvement can be tracked in #8141.

@tpkeeper are you okay with the altnernatvie I proposed for getting gas for multisigs?

@tpkeeper
Copy link
Author

tpkeeper commented Apr 2, 2021

The issue is that CalculateGas doesn't handle multisigs right now, it hardcodes SingleSignature:

cosmos-sdk/client/tx/tx.go

Lines 260 to 265 in 1fa2c22

// Create an empty signature literal as the ante handler will populate with a
// sentinel pubkey.
sig := signing.SignatureV2{
PubKey: &secp256k1.PubKey{},
Data: &signing.SingleSignatureData{
SignMode: txf.signMode,

To be honest, I think the tx.Factory is just not well-designed, it has confusing overlaps with client.Context and TxBuilder, so I prefer to mark this issue as wontfix

As an alternative, users should use the Simulate gRPC endpoint to get gas, and to create a multisig tx either 1/ use the CLI or 2/ manually add MultiSignatureData to a txBuilder. @technicallyty is creating a PR with 1/.

Multisig UX (CLI and programmatic) improvement can be tracked in #8141.

@tpkeeper are you okay with the altnernatvie I proposed for getting gas for multisigs?

just set gas limit very large like 50000000

@amaury1093
Copy link
Contributor

just set gas limit very large like 50000000

True, but there's also a risk of paying high fees.

@tpkeeper In #9038, we have an example of calculating gas for a multisig tx. Is that useful? If yes, can we close this issue and use that as an alternative?

@tpkeeper
Copy link
Author

tpkeeper commented Apr 2, 2021

thanks ,i will try it and give a feed back

just set gas limit very large like 50000000

True, but there's also a risk of paying high fees.

@tpkeeper In #9038, we have an example of calculating gas for a multisig tx. Is that useful? If yes, can we close this issue and use that as an alternative?

thanks ,i will try it and give you a feed back

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S:needs more info This bug can't be addressed until more information is provided by the reporter. T: Dev UX UX for SDK developers (i.e. how to call our code)
Projects
None yet
Development

No branches or pull requests

5 participants