Skip to content

Commit

Permalink
refactor: Move some methods inside TX Factory (backport cosmos#9421) (c…
Browse files Browse the repository at this point in the history 
…osmos#10039)

* refactor: Move some methods inside TX Factory (cosmos#9421)

<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰
v    Before smashing the submit button please review the checkboxes.
v    If a checkbox is n/a - please still include it but + a little note why
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## Description

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review.
-->

Putting some things inside the factory (which was very anemic struct) has helped me to understand the flow. Feel free to merge if you see some benefit.

closes: #XXXX

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

- [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes

(cherry picked from commit e17be87)

# Conflicts:
#	CHANGELOG.md

* Update CHANGELOG.md

Co-authored-by: Jonathan Gimeno <[email protected]>
Co-authored-by: Robert Zaremba <[email protected]>
  • Loading branch information
@mergify @jgimeno
@robert-zaremba @JeancarloBarrios
3 people authored and JeancarloBarrios committed Sep 28, 2024
1 parent 3e5543a commit 5ebc2b8
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 230 deletions.
12 changes: 6 additions & 6 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ 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.

## [v0.43.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0) - 2021-08-10

### Features
Expand Down Expand Up @@ -148,11 +152,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.


### Deprecated

## [v0.52.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.52.0) - 2024-XX-XX

Every module contains its own CHANGELOG.md. Please refer to the module you are interested in.
### State Machine Breaking

### Features

Expand Down
198 changes: 19 additions & 179 deletions client/tx/factory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,8 @@ package tx
import (
"errors"
"fmt"
"math/big"
"os"
"strings"
"time"

"github.com/cosmos/go-bip39"
"github.com/spf13/pflag"

"cosmossdk.io/math"
Expand All @@ -17,9 +13,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"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
)
Expand Down Expand Up @@ -253,84 +247,11 @@ func (f Factory) WithTimeoutHeight(height uint64) Factory {
return f
}

// WithTimeoutTimestamp returns a copy of the Factory with an updated timeout timestamp.
func (f Factory) WithTimeoutTimestamp(timestamp time.Time) Factory {
f.timeoutTimestamp = timestamp
return f
}

// WithUnordered returns a copy of the Factory with an updated unordered field.
func (f Factory) WithUnordered(v bool) Factory {
f.unordered = v
return f
}

// WithFeeGranter returns a copy of the Factory with an updated fee granter.
func (f Factory) WithFeeGranter(fg sdk.AccAddress) Factory {
f.feeGranter = fg
return f
}

// WithFeePayer returns a copy of the Factory with an updated fee granter.
func (f Factory) WithFeePayer(fp sdk.AccAddress) Factory {
f.feePayer = fp
return f
}

// WithPreprocessTxHook returns a copy of the Factory with an updated preprocess tx function,
// allows for preprocessing of transaction data using the TxBuilder.
func (f Factory) WithPreprocessTxHook(preprocessFn client.PreprocessTxFn) Factory {
f.preprocessTxHook = preprocessFn
return f
}

// PreprocessTx calls the preprocessing hook with the factory parameters and
// returns the result.
func (f Factory) PreprocessTx(keyname string, builder client.TxBuilder) error {
if f.preprocessTxHook == nil {
// Allow pass-through
return nil
}

key, err := f.Keybase().Key(keyname)
if err != nil {
return fmt.Errorf("error retrieving key from keyring: %w", err)
}

return f.preprocessTxHook(f.chainID, key.GetType(), builder)
}

// WithExtensionOptions returns a Factory with given extension options added to the existing options,
// Example to add dynamic fee extension options:
//
// extOpt := ethermint.ExtensionOptionDynamicFeeTx{
// MaxPriorityPrice: math.NewInt(1000000),
// }
//
// extBytes, _ := extOpt.Marshal()
//
// extOpts := []*types.Any{
// {
// TypeUrl: "/ethermint.types.v1.ExtensionOptionDynamicFeeTx",
// Value: extBytes,
// },
// }
//
// txf.WithExtensionOptions(extOpts...)
func (f Factory) WithExtensionOptions(extOpts ...*codectypes.Any) Factory {
f.extOptions = extOpts
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.offline && f.generateOnly {
if f.chainID != "" {
return nil, errors.New("chain ID cannot be used when offline and generate-only flags are set")
}
} else if f.chainID == "" {
return nil, errors.New("chain ID required but not specified")
if f.chainID == "" {
return nil, fmt.Errorf("chain ID required but not specified")
}

fees := f.fees
Expand All @@ -340,9 +261,7 @@ func (f Factory) BuildUnsignedTx(msgs ...sdk.Msg) (client.TxBuilder, error) {
return nil, errors.New("cannot provide both fees and gas prices")
}

// f.gas is a uint64 and we should convert to LegacyDec
// without the risk of under/overflow via uint64->int64.
glDec := math.LegacyNewDecFromBigInt(new(big.Int).SetUint64(f.gas))
glDec := sdk.NewDec(int64(f.gas))

// Derive the fees based on the provided gas prices, where
// fee = ceil(gasPrice * gasLimit).
Expand All @@ -354,11 +273,6 @@ func (f Factory) BuildUnsignedTx(msgs ...sdk.Msg) (client.TxBuilder, error) {
}
}

// Prevent simple inclusion of a valid mnemonic in the memo field
if f.memo != "" && bip39.IsMnemonicValid(strings.ToLower(f.memo)) {
return nil, errors.New("cannot provide a valid mnemonic seed in the memo field")
}

tx := f.txConfig.NewTxBuilder()

if err := tx.SetMsgs(msgs...); err != nil {
Expand All @@ -368,15 +282,7 @@ func (f Factory) BuildUnsignedTx(msgs ...sdk.Msg) (client.TxBuilder, error) {
tx.SetMemo(f.memo)
tx.SetFeeAmount(fees)
tx.SetGasLimit(f.gas)
tx.SetFeeGranter(f.feeGranter)
tx.SetFeePayer(f.feePayer)
tx.SetTimeoutHeight(f.TimeoutHeight())
tx.SetTimeoutTimestamp(f.TimeoutTimestamp())
tx.SetUnordered(f.Unordered())

if etx, ok := tx.(client.ExtendedTxBuilder); ok {
etx.SetExtensionOptions(f.extOptions...)
}

return tx, nil
}
Expand All @@ -391,14 +297,7 @@ func (f Factory) PrintUnsignedTx(clientCtx client.Context, msgs ...sdk.Msg) erro
return errors.New("cannot estimate gas in offline mode")
}

// Prepare TxFactory with acc & seq numbers as CalculateGas requires
// account and sequence numbers to be set
preparedTxf, err := f.Prepare(clientCtx)
if err != nil {
return err
}

_, adjusted, err := CalculateGas(clientCtx, preparedTxf, msgs...)
_, adjusted, err := CalculateGas(clientCtx, f, msgs...)
if err != nil {
return err
}
Expand All @@ -407,17 +306,12 @@ func (f Factory) PrintUnsignedTx(clientCtx client.Context, msgs ...sdk.Msg) erro
_, _ = fmt.Fprintf(os.Stderr, "%s\n", GasEstimateResponse{GasEstimate: f.Gas()})
}

unsignedTx, err := f.BuildUnsignedTx(msgs...)
tx, err := f.BuildUnsignedTx(msgs...)
if err != nil {
return err
}

encoder := f.txConfig.TxJSONEncoder()
if encoder == nil {
return errors.New("cannot print unsigned tx: tx json encoder is nil")
}

json, err := encoder(unsignedTx.GetTx())
json, err := clientCtx.TxConfig.TxJSONEncoder()(tx.GetTx())
if err != nil {
return err
}
Expand All @@ -434,88 +328,34 @@ func (f Factory) BuildSimTx(msgs ...sdk.Msg) ([]byte, error) {
return nil, err
}

pk, err := f.getSimPK()
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: pk,
Data: f.getSimSignatureData(pk),
PubKey: &secp256k1.PubKey{},
Data: &signing.SingleSignatureData{
SignMode: f.signMode,
},
Sequence: f.Sequence(),
}
if err := txb.SetSignatures(sig); err != nil {
return nil, err
}

encoder := f.txConfig.TxEncoder()
if encoder == nil {
return nil, errors.New("cannot simulate tx: tx encoder is nil")
}

return encoder(txb.GetTx())
}

// getSimPK gets the public key to use for building a simulation tx.
// Note, we should only check for keys in the keybase if we are in simulate and execute mode,
// e.g. when using --gas=auto.
// When using --dry-run, we are in simulation mode only and should not check the keybase.
// Ref: https://github.com/cosmos/cosmos-sdk/issues/11283
func (f Factory) getSimPK() (cryptotypes.PubKey, error) {
var (
ok bool
pk cryptotypes.PubKey = &secp256k1.PubKey{} // use default public key type
)

if f.simulateAndExecute && f.keybase != nil {
record, err := f.keybase.Key(f.fromName)
if err != nil {
return nil, err
}

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")
}
}

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,
}
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.
// Note: When in offline mode, the Prepare does nothing and returns the original factory.
// 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) {
if clientCtx.Offline {
return f, nil
}

fc := f
from := clientCtx.FromAddress

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 {
Expand Down
Loading

0 comments on commit 5ebc2b8

Please sign in to comment.