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: change the behavior of offline mode correctly #15123

Merged
merged 11 commits into from
Feb 23, 2023
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,8 @@ extension interfaces. `module.Manager.Modules` is now of type `map[string]interf
* (x/staking) [#14590](https://github.com/cosmos/cosmos-sdk/pull/14590) `MsgUndelegateResponse` now includes undelegated amount. `x/staking` module's `keeper.Undelegate` now returns 3 values (completionTime,undelegateAmount,error) instead of 2.
* (x/feegrant) [14649](https://github.com/cosmos/cosmos-sdk/pull/14649) Extract Feegrant in its own go.mod and rename the package to `cosmossdk.io/x/feegrant`.
* (x/bank) [#14894](https://github.com/cosmos/cosmos-sdk/pull/14894) Return a human readable denomination for IBC vouchers when querying bank balances. Added a `ResolveDenom` parameter to `types.QueryAllBalancesRequest`.
* (crypto) [#15070](https://github.com/cosmos/cosmos-sdk/pull/15070) `GenerateFromPassword` and `Cost` from `bcrypt.go` now take a `uint32` instead of a `int` type.
* (crypto) [#15070](https://github.com/cosmos/cosmos-sdk/pull/15070) `GenerateFromPassword` and `Cost` from `bcrypt.go` now take a `uint32` instead of a `int` type.
* (client) [#15123](https://github.com/cosmos/cosmos-sdk/pull/15123) `NewFactoryCLI` now returns an error, in addition to the `Factory`.

### Client Breaking Changes

Expand Down Expand Up @@ -308,6 +309,7 @@ extension interfaces. `module.Manager.Modules` is now of type `map[string]interf
* (server) [#13778](https://github.com/cosmos/cosmos-sdk/pull/13778) Set Cosmos SDK default endpoints to localhost to avoid unknown exposure of endpoints.
* (x/auth) [#13877](https://github.com/cosmos/cosmos-sdk/pull/13877) Handle missing account numbers during `InitGenesis`.
* (cli) [#14509](https://github.com/cosmos/cosmos-sdk/pull/#14509) Added missing options to keyring-backend flag usage
* (cli) [#15123](https://github.com/cosmos/cosmos-sdk/pull/15123) Change the offline mode behavior that does not match the written in the description to behave as written

### Deprecated

Expand Down
27 changes: 15 additions & 12 deletions client/tx/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type Factory struct {
}

// NewFactoryCLI creates a new Factory.
func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) Factory {
func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) (Factory, error) {
tkxkd0159 marked this conversation as resolved.
Show resolved Hide resolved
signModeStr := clientCtx.SignModeStr

signMode := signing.SignMode_SIGN_MODE_UNSPECIFIED
Expand All @@ -64,8 +64,16 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) Factory {
signMode = signing.SignMode_SIGN_MODE_EIP_191
}

accNum, _ := flagSet.GetUint64(flags.FlagAccountNumber)
accSeq, _ := flagSet.GetUint64(flags.FlagSequence)
var accNum, accSeq uint64
if clientCtx.Offline {
if flagSet.Changed(flags.FlagAccountNumber) && flagSet.Changed(flags.FlagSequence) {
accNum, _ = flagSet.GetUint64(flags.FlagAccountNumber)
accSeq, _ = flagSet.GetUint64(flags.FlagSequence)
} else {
return Factory{}, errors.New("account-number and sequence must be set in offline mode")
}
}

gasAdj, _ := flagSet.GetFloat64(flags.FlagGasAdjustment)
memo, _ := flagSet.GetString(flags.FlagNote)
timeoutHeight, _ := flagSet.GetUint64(flags.FlagTimeoutHeight)
Expand Down Expand Up @@ -105,7 +113,7 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) Factory {

f = f.WithPreprocessTxHook(clientCtx.PreprocessTxHook)

return f
return f, nil
}

func (f Factory) AccountNumber() uint64 { return f.accountNumber }
Expand Down Expand Up @@ -435,19 +443,14 @@ func (f Factory) Prepare(clientCtx client.Context) (Factory, error) {
}

initNum, initSeq := fc.accountNumber, fc.sequence
if initNum == 0 || initSeq == 0 {
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)
}
fc = fc.WithAccountNumber(num)
fc = fc.WithSequence(seq)
}

return fc, nil
Expand Down
9 changes: 8 additions & 1 deletion client/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ import (
// GenerateOrBroadcastTxCLI will either generate and print and unsigned transaction
// or sign it and broadcast it returning an error upon failure.
func GenerateOrBroadcastTxCLI(clientCtx client.Context, flagSet *pflag.FlagSet, msgs ...sdk.Msg) error {
txf := NewFactoryCLI(clientCtx, flagSet)
txf, err := NewFactoryCLI(clientCtx, flagSet)
if err != nil {
return err
}

return GenerateOrBroadcastTxWithFactory(clientCtx, txf, msgs...)
}
Expand Down Expand Up @@ -69,6 +72,10 @@ func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error {
}

if txf.SimulateAndExecute() || clientCtx.Simulate {
if clientCtx.Offline {
return errors.New("cannot estimate gas in offline mode")
}

_, adjusted, err := CalculateGas(clientCtx, txf, msgs...)
if err != nil {
return err
Expand Down
17 changes: 14 additions & 3 deletions tests/e2e/auth/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -1213,9 +1213,20 @@ func (s *E2ETestSuite) TestCLIMultisign() {
sign2File := testutil.WriteToNewTempFile(s.T(), account2Signature.String())
defer sign2File.Close()

// Does not work in offline mode.
_, err = authclitestutil.TxMultiSignExec(val1.ClientCtx, multisigRecord.Name, multiGeneratedTxFile.Name(), "--offline", sign1File.Name(), sign2File.Name())
s.Require().EqualError(err, fmt.Sprintf("couldn't verify signature for address %s", addr1))
// Work in offline mode.
multisigAccNum, multisigSeq, err := val1.ClientCtx.AccountRetriever.GetAccountNumberSequence(val1.ClientCtx, addr)
s.Require().NoError(err)
_, err = authclitestutil.TxMultiSignExec(
val1.ClientCtx,
multisigRecord.Name,
multiGeneratedTxFile.Name(),
fmt.Sprintf("--%s", flags.FlagOffline),
fmt.Sprintf("--%s=%d", flags.FlagAccountNumber, multisigAccNum),
fmt.Sprintf("--%s=%d", flags.FlagSequence, multisigSeq),
sign1File.Name(),
sign2File.Name(),
)
s.Require().NoError(err)

val1.ClientCtx.Offline = false
multiSigWith2Signatures, err := authclitestutil.TxMultiSignExec(val1.ClientCtx, multisigRecord.Name, multiGeneratedTxFile.Name(), sign1File.Name(), sign2File.Name())
Expand Down
3 changes: 3 additions & 0 deletions tests/e2e/tx/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/stretchr/testify/suite"

errorsmod "cosmossdk.io/errors"

"cosmossdk.io/simapp"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
Expand Down Expand Up @@ -90,6 +91,8 @@ func (s *E2ETestSuite) SetupSuite() {
sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(1)),
),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s", flags.FlagOffline),
fmt.Sprintf("--%s=0", flags.FlagAccountNumber),
fmt.Sprintf("--%s=2", flags.FlagSequence),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
Expand Down
5 changes: 4 additions & 1 deletion x/auth/client/cli/tips.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ func GetAuxToFeeCommand() *cobra.Command {
return fmt.Errorf("expected chain-id %s, got %s in aux signer data", clientCtx.ChainID, auxSignerData.SignDoc.ChainId)
}

f := clienttx.NewFactoryCLI(clientCtx, cmd.Flags())
f, err := clienttx.NewFactoryCLI(clientCtx, cmd.Flags())
if err != nil {
return err
}

txBuilder := clientCtx.TxConfig.NewTxBuilder()
err = txBuilder.AddAuxSignerData(auxSignerData)
Expand Down
10 changes: 8 additions & 2 deletions x/auth/client/cli/tx_multisign.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ func makeMultiSignCmd() func(cmd *cobra.Command, args []string) (err error) {
return
}

txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags())
txFactory, err := tx.NewFactoryCLI(clientCtx, cmd.Flags())
if err != nil {
return err
}
if txFactory.SignMode() == signingtypes.SignMode_SIGN_MODE_UNSPECIFIED {
txFactory = txFactory.WithSignMode(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
}
Expand Down Expand Up @@ -257,7 +260,10 @@ func makeBatchMultisignCmd() func(cmd *cobra.Command, args []string) error {
}

txCfg := clientCtx.TxConfig
txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags())
txFactory, err := tx.NewFactoryCLI(clientCtx, cmd.Flags())
if err != nil {
return err
}
if txFactory.SignMode() == signingtypes.SignMode_SIGN_MODE_UNSPECIFIED {
txFactory = txFactory.WithSignMode(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
}
Expand Down
5 changes: 4 additions & 1 deletion x/auth/client/cli/tx_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ func makeSignBatchCmd() func(cmd *cobra.Command, args []string) error {
if err != nil {
return err
}
txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags())
txFactory, err := tx.NewFactoryCLI(clientCtx, cmd.Flags())
if err != nil {
return err
}
txCfg := clientCtx.TxConfig
printSignatureOnly, _ := cmd.Flags().GetBool(flagSigOnly)

Expand Down
5 changes: 4 additions & 1 deletion x/auth/client/cli/validate_sigs.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,10 @@ func readTxAndInitContexts(clientCtx client.Context, cmd *cobra.Command, filenam
return clientCtx, tx.Factory{}, nil, err
}

txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags())
txFactory, err := tx.NewFactoryCLI(clientCtx, cmd.Flags())
if err != nil {
return clientCtx, tx.Factory{}, nil, err
}

return clientCtx, txFactory, stdTx, nil
}
5 changes: 4 additions & 1 deletion x/genutil/client/cli/gentx.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,10 @@ $ %s gentx my-key-name 1000000stake --home=/path/to/home/dir --keyring-backend=o
return errors.Wrap(err, "failed to validate account in genesis")
}

txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags())
txFactory, err := tx.NewFactoryCLI(clientCtx, cmd.Flags())
if err != nil {
return err
}

pub, err := key.GetAddress()
if err != nil {
Expand Down
7 changes: 5 additions & 2 deletions x/staking/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,16 @@ where we can get the pubkey using "%s tendermint show-validator"
return err
}

txf, err := tx.NewFactoryCLI(clientCtx, cmd.Flags())
tkxkd0159 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}

validator, err := parseAndValidateValidatorJSON(clientCtx.Codec, args[0])
if err != nil {
return err
}

txf := tx.NewFactoryCLI(clientCtx, cmd.Flags()).
WithTxConfig(clientCtx.TxConfig).WithAccountRetriever(clientCtx.AccountRetriever)
txf, msg, err := newBuildCreateValidatorMsg(clientCtx, txf, cmd.Flags(), validator)
if err != nil {
return err
Expand Down