From 59a49529ac010800080fab529cd90818ec050661 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Wed, 1 Sep 2021 23:58:05 +0200 Subject: [PATCH] chore: revert TxFactory changes (#10053) * Revert "refactor: Move some methods inside TX Factory (backport #9421) (#10039)" This reverts commit 0155244d2a104af8722e04320bf3972eecc7e9b1. * remove conflict marks from changelog * update changelog --- CHANGELOG.md | 8 +- client/tx/factory.go | 134 -------------------------------- client/tx/tx.go | 139 ++++++++++++++++++++++++++++++++-- client/tx/tx_test.go | 10 +-- x/auth/client/tx.go | 6 ++ x/genutil/client/cli/gentx.go | 4 +- 6 files changed, 150 insertions(+), 151 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c68b0d9bc540..da51f0a3bb65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,13 +53,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes -+ [\#9965](https://github.com/cosmos/cosmos-sdk/pull/9965) Fixed `simd version` command output to report the right release tag. -+ -### API Breaking Changes -* (client/tx) [\#9421](https://github.com/cosmos/cosmos-sdk/pull/9421/) `BuildUnsignedTx`, `BuildSimTx`, `PrintUnsignedStdTx` functions are moved to - the Tx Factory as methods. +* [\#9965](https://github.com/cosmos/cosmos-sdk/pull/9965) Fixed `simd version` command output to report the right release tag. ### Client Breaking Changes + * [\#10041](https://github.com/cosmos/cosmos-sdk/pull/10041) Remove broadcast & encode legacy REST endpoints. Please see the [REST Endpoints Migration guide](https://docs.cosmos.network/master/migrations/rest.html) to migrate to the new REST endpoints. ## [v0.43.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0) - 2021-08-10 @@ -150,6 +147,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/capability) [\#9836](https://github.com/cosmos/cosmos-sdk/pull/9836) Removed `InitializeAndSeal(ctx sdk.Context)` and replaced with `Seal()`. App must add x/capability module to the begin blockers which will assure that the x/capability keeper is properly initialized. The x/capability begin blocker must be run before any other module which uses x/capability. + ### State Machine Breaking * (x/{bank,distrib,gov,slashing,staking}) [\#8363](https://github.com/cosmos/cosmos-sdk/issues/8363) Store keys have been modified to allow for variable-length addresses. diff --git a/client/tx/factory.go b/client/tx/factory.go index 3dc994b3917d..2cb951922d0a 100644 --- a/client/tx/factory.go +++ b/client/tx/factory.go @@ -1,16 +1,11 @@ package tx import ( - "errors" - "fmt" - "os" - "github.com/spf13/pflag" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/crypto/keyring" - "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/tx/signing" ) @@ -194,132 +189,3 @@ func (f Factory) WithTimeoutHeight(height uint64) Factory { f.timeoutHeight = height return f } - -// BuildUnsignedTx builds a transaction to be signed given a set of messages. -// Once created, the fee, memo, and messages are set. -func (f Factory) BuildUnsignedTx(msgs ...sdk.Msg) (client.TxBuilder, error) { - if f.chainID == "" { - return nil, fmt.Errorf("chain ID required but not specified") - } - - fees := f.fees - - if !f.gasPrices.IsZero() { - if !fees.IsZero() { - return nil, errors.New("cannot provide both fees and gas prices") - } - - glDec := sdk.NewDec(int64(f.gas)) - - // Derive the fees based on the provided gas prices, where - // fee = ceil(gasPrice * gasLimit). - fees = make(sdk.Coins, len(f.gasPrices)) - - for i, gp := range f.gasPrices { - fee := gp.Amount.Mul(glDec) - fees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) - } - } - - tx := f.txConfig.NewTxBuilder() - - if err := tx.SetMsgs(msgs...); err != nil { - return nil, err - } - - tx.SetMemo(f.memo) - tx.SetFeeAmount(fees) - tx.SetGasLimit(f.gas) - tx.SetTimeoutHeight(f.TimeoutHeight()) - - return tx, nil -} - -// PrintUnsignedTx will generate an unsigned transaction and print it to the writer -// specified by ctx.Output. If simulation was requested, the gas will be -// simulated and also printed to the same writer before the transaction is -// printed. -func (f Factory) PrintUnsignedTx(clientCtx client.Context, msgs ...sdk.Msg) error { - if f.SimulateAndExecute() { - if clientCtx.Offline { - return errors.New("cannot estimate gas in offline mode") - } - - _, adjusted, err := CalculateGas(clientCtx, f, msgs...) - if err != nil { - return err - } - - f = f.WithGas(adjusted) - _, _ = fmt.Fprintf(os.Stderr, "%s\n", GasEstimateResponse{GasEstimate: f.Gas()}) - } - - tx, err := f.BuildUnsignedTx(msgs...) - if err != nil { - return err - } - - json, err := clientCtx.TxConfig.TxJSONEncoder()(tx.GetTx()) - if err != nil { - return err - } - - return clientCtx.PrintString(fmt.Sprintf("%s\n", json)) -} - -// BuildSimTx creates an unsigned tx with an empty single signature and returns -// the encoded transaction or an error if the unsigned transaction cannot be -// built. -func (f Factory) BuildSimTx(msgs ...sdk.Msg) ([]byte, error) { - txb, err := f.BuildUnsignedTx(msgs...) - if err != nil { - return nil, err - } - - // 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: f.signMode, - }, - Sequence: f.Sequence(), - } - if err := txb.SetSignatures(sig); err != nil { - return nil, err - } - - return f.txConfig.TxEncoder()(txb.GetTx()) -} - -// 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. A new Factory with -// the updated fields will be returned. -func (f Factory) Prepare(clientCtx client.Context) (Factory, error) { - fc := f - - from := clientCtx.GetFromAddress() - - if err := fc.accountRetriever.EnsureExists(clientCtx, from); err != nil { - return fc, err - } - - initNum, initSeq := fc.accountNumber, fc.sequence - if initNum == 0 || initSeq == 0 { - num, seq, err := fc.accountRetriever.GetAccountNumberSequence(clientCtx, from) - if err != nil { - return fc, err - } - - if initNum == 0 { - fc = fc.WithAccountNumber(num) - } - - if initSeq == 0 { - fc = fc.WithSequence(seq) - } - } - - return fc, nil -} diff --git a/client/tx/tx.go b/client/tx/tx.go index ee0c7553de83..8e20be369bfd 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -14,6 +14,7 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/client/input" + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -44,17 +45,49 @@ func GenerateOrBroadcastTxWithFactory(clientCtx client.Context, txf Factory, msg } if clientCtx.GenerateOnly { - return txf.PrintUnsignedTx(clientCtx, msgs...) + return GenerateTx(clientCtx, txf, msgs...) } return BroadcastTx(clientCtx, txf, msgs...) } +// GenerateTx will generate an unsigned transaction and print it to the writer +// specified by ctx.Output. If simulation was requested, the gas will be +// simulated and also printed to the same writer before the transaction is +// printed. +func GenerateTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error { + if txf.SimulateAndExecute() { + if clientCtx.Offline { + return errors.New("cannot estimate gas in offline mode") + } + + _, adjusted, err := CalculateGas(clientCtx, txf, msgs...) + if err != nil { + return err + } + + txf = txf.WithGas(adjusted) + _, _ = fmt.Fprintf(os.Stderr, "%s\n", GasEstimateResponse{GasEstimate: txf.Gas()}) + } + + tx, err := BuildUnsignedTx(txf, msgs...) + if err != nil { + return err + } + + json, err := clientCtx.TxConfig.TxJSONEncoder()(tx.GetTx()) + if err != nil { + return err + } + + return clientCtx.PrintString(fmt.Sprintf("%s\n", json)) +} + // BroadcastTx attempts to generate, sign and broadcast a transaction with the // given set of messages. It will also simulate gas requirements if necessary. // It will return an error upon failure. func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error { - txf, err := txf.Prepare(clientCtx) + txf, err := prepareFactory(clientCtx, txf) if err != nil { return err } @@ -73,7 +106,7 @@ func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error { return nil } - tx, err := txf.BuildUnsignedTx(msgs...) + tx, err := BuildUnsignedTx(txf, msgs...) if err != nil { return err } @@ -164,7 +197,7 @@ func WriteGeneratedTxResponse( } } - tx, err := txf.BuildUnsignedTx(msgs...) + tx, err := BuildUnsignedTx(txf, msgs...) if rest.CheckBadRequestError(w, err) { return } @@ -184,12 +217,78 @@ func WriteGeneratedTxResponse( _, _ = w.Write(output) } +// BuildUnsignedTx builds a transaction to be signed given a set of messages. The +// transaction is initially created via the provided factory's generator. Once +// created, the fee, memo, and messages are set. +func BuildUnsignedTx(txf Factory, msgs ...sdk.Msg) (client.TxBuilder, error) { + if txf.chainID == "" { + return nil, fmt.Errorf("chain ID required but not specified") + } + + fees := txf.fees + + if !txf.gasPrices.IsZero() { + if !fees.IsZero() { + return nil, errors.New("cannot provide both fees and gas prices") + } + + glDec := sdk.NewDec(int64(txf.gas)) + + // Derive the fees based on the provided gas prices, where + // fee = ceil(gasPrice * gasLimit). + fees = make(sdk.Coins, len(txf.gasPrices)) + + for i, gp := range txf.gasPrices { + fee := gp.Amount.Mul(glDec) + fees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) + } + } + + tx := txf.txConfig.NewTxBuilder() + + if err := tx.SetMsgs(msgs...); err != nil { + return nil, err + } + + tx.SetMemo(txf.memo) + tx.SetFeeAmount(fees) + tx.SetGasLimit(txf.gas) + tx.SetTimeoutHeight(txf.TimeoutHeight()) + + return tx, nil +} + +// BuildSimTx creates an unsigned tx with an empty single signature and returns +// the encoded transaction or an error if the unsigned transaction cannot be +// built. +func BuildSimTx(txf Factory, msgs ...sdk.Msg) ([]byte, error) { + txb, err := BuildUnsignedTx(txf, msgs...) + if err != nil { + return nil, err + } + + // 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, + }, + Sequence: txf.Sequence(), + } + if err := txb.SetSignatures(sig); err != nil { + return nil, err + } + + return txf.txConfig.TxEncoder()(txb.GetTx()) +} + // CalculateGas simulates the execution of a transaction and returns the // simulation response obtained by the query and the adjusted gas amount. func CalculateGas( clientCtx gogogrpc.ClientConn, txf Factory, msgs ...sdk.Msg, ) (*tx.SimulateResponse, uint64, error) { - txBytes, err := txf.BuildSimTx(msgs...) + txBytes, err := BuildSimTx(txf, msgs...) if err != nil { return nil, 0, err } @@ -205,6 +304,36 @@ func CalculateGas( return simRes, uint64(txf.GasAdjustment() * float64(simRes.GasInfo.GasUsed)), nil } +// prepareFactory 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. A new Factory with +// the updated fields will be returned. +func prepareFactory(clientCtx client.Context, txf Factory) (Factory, error) { + from := clientCtx.GetFromAddress() + + if err := txf.accountRetriever.EnsureExists(clientCtx, from); err != nil { + return txf, err + } + + initNum, initSeq := txf.accountNumber, txf.sequence + if initNum == 0 || initSeq == 0 { + num, seq, err := txf.accountRetriever.GetAccountNumberSequence(clientCtx, from) + if err != nil { + return txf, err + } + + if initNum == 0 { + txf = txf.WithAccountNumber(num) + } + + if initSeq == 0 { + txf = txf.WithSequence(seq) + } + } + + return txf, nil +} + // SignWithPrivKey signs a given tx with the given private key, and returns the // corresponding SignatureV2 if the signing is successful. func SignWithPrivKey( diff --git a/client/tx/tx_test.go b/client/tx/tx_test.go index 54bcc7ade649..b095d05b9e5d 100644 --- a/client/tx/tx_test.go +++ b/client/tx/tx_test.go @@ -107,7 +107,7 @@ func TestBuildSimTx(t *testing.T) { WithSignMode(txCfg.SignModeHandler().DefaultMode()) msg := banktypes.NewMsgSend(sdk.AccAddress("from"), sdk.AccAddress("to"), nil) - bz, err := txf.BuildSimTx(msg) + bz, err := tx.BuildSimTx(txf, msg) require.NoError(t, err) require.NotNil(t, bz) } @@ -122,7 +122,7 @@ func TestBuildUnsignedTx(t *testing.T) { WithChainID("test-chain") msg := banktypes.NewMsgSend(sdk.AccAddress("from"), sdk.AccAddress("to"), nil) - tx, err := txf.BuildUnsignedTx(msg) + tx, err := tx.BuildUnsignedTx(txf, msg) require.NoError(t, err) require.NotNil(t, tx) @@ -169,11 +169,11 @@ func TestSign(t *testing.T) { WithSignMode(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON) msg1 := banktypes.NewMsgSend(info1.GetAddress(), sdk.AccAddress("to"), nil) msg2 := banktypes.NewMsgSend(info2.GetAddress(), sdk.AccAddress("to"), nil) - txb, err := txfNoKeybase.BuildUnsignedTx(msg1, msg2) + txb, err := tx.BuildUnsignedTx(txfNoKeybase, msg1, msg2) requireT.NoError(err) - txb2, err := txfNoKeybase.BuildUnsignedTx(msg1, msg2) + txb2, err := tx.BuildUnsignedTx(txfNoKeybase, msg1, msg2) requireT.NoError(err) - txbSimple, err := txfNoKeybase.BuildUnsignedTx(msg2) + txbSimple, err := tx.BuildUnsignedTx(txfNoKeybase, msg2) requireT.NoError(err) testCases := []struct { diff --git a/x/auth/client/tx.go b/x/auth/client/tx.go index 47268e643902..80debcc1dedf 100644 --- a/x/auth/client/tx.go +++ b/x/auth/client/tx.go @@ -30,6 +30,12 @@ func (gr GasEstimateResponse) String() string { return fmt.Sprintf("gas estimate: %d", gr.GasEstimate) } +// PrintUnsignedStdTx builds an unsigned StdTx and prints it to os.Stdout. +func PrintUnsignedStdTx(txBldr tx.Factory, clientCtx client.Context, msgs []sdk.Msg) error { + err := tx.GenerateTx(clientCtx, txBldr, msgs...) + return err +} + // 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. diff --git a/x/genutil/client/cli/gentx.go b/x/genutil/client/cli/gentx.go index 94903b3d45f4..be347b745b05 100644 --- a/x/genutil/client/cli/gentx.go +++ b/x/genutil/client/cli/gentx.go @@ -155,14 +155,14 @@ $ %s gentx my-key-name 1000000stake --home=/path/to/home/dir --keyring-backend=o if key.GetType() == keyring.TypeOffline || key.GetType() == keyring.TypeMulti { cmd.PrintErrln("Offline key passed in. Use `tx sign` command to sign.") - return txBldr.PrintUnsignedTx(clientCtx, msg) + return authclient.PrintUnsignedStdTx(txBldr, clientCtx, []sdk.Msg{msg}) } // write the unsigned transaction to the buffer w := bytes.NewBuffer([]byte{}) clientCtx = clientCtx.WithOutput(w) - if err = txBldr.PrintUnsignedTx(clientCtx, msg); err != nil { + if err = authclient.PrintUnsignedStdTx(txBldr, clientCtx, []sdk.Msg{msg}); err != nil { return errors.Wrap(err, "failed to print unsigned std tx") }