-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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: --dry-run
not working when using tx command
#11558
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
00a62fc
cli: fix` --dry-run` not working when using tx command
julienrbrt 88fc525
add changelog
julienrbrt 0179f5c
implement feedback
julienrbrt b0b39a8
update flags description
julienrbrt 70c6681
improve description
julienrbrt ca53993
Implement feedback
julienrbrt 10bf7ca
spacing
julienrbrt 2d62887
Merge branch 'master' into 11149-dry-run-not-worked-when-use-tx-command
julienrbrt File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package client | |
|
||
import ( | ||
"bufio" | ||
"fmt" | ||
"io" | ||
"os" | ||
|
||
|
@@ -52,9 +53,9 @@ type Context struct { | |
FeePayer sdk.AccAddress | ||
FeeGranter sdk.AccAddress | ||
Viper *viper.Viper | ||
|
||
// IsAux is true when the signer is an auxiliary signer (e.g. the tipper). | ||
IsAux bool | ||
IsAux bool | ||
|
||
// TODO: Deprecated (remove). | ||
LegacyAmino *codec.LegacyAmino | ||
|
@@ -334,16 +335,31 @@ func (ctx Context) printOutput(out []byte) error { | |
return nil | ||
} | ||
|
||
// GetFromFields returns a from account address, account name and keyring type, given either | ||
// an address or key name. If genOnly is true, only a valid Bech32 cosmos | ||
// address is returned. | ||
func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddress, string, keyring.KeyType, error) { | ||
// GetFromFields returns a from account address, account name and keyring type, given either an address or key name. | ||
// If clientCtx.Simulate is true the keystore is not accessed and a valid address must be provided | ||
// If clientCtx.GenerateOnly is true the keystore is only accessed if a key name is provided | ||
func GetFromFields(clientCtx Context, kr keyring.Keyring, from string) (sdk.AccAddress, string, keyring.KeyType, error) { | ||
if from == "" { | ||
return nil, "", 0, nil | ||
} | ||
|
||
addr, err := sdk.AccAddressFromBech32(from) | ||
switch { | ||
case clientCtx.Simulate: | ||
if err != nil { | ||
return nil, "", 0, fmt.Errorf("a valid bech32 address must be provided in simulation mode: %w", err) | ||
} | ||
|
||
return addr, "", 0, nil | ||
|
||
case clientCtx.GenerateOnly: | ||
if err == nil { | ||
return addr, "", 0, nil | ||
} | ||
} | ||
|
||
var k *keyring.Record | ||
if addr, err := sdk.AccAddressFromBech32(from); err == nil { | ||
if err == nil { | ||
k, err = kr.KeyByAddress(addr) | ||
if err != nil { | ||
return nil, "", 0, err | ||
|
@@ -355,7 +371,7 @@ func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddres | |
} | ||
} | ||
|
||
addr, err := k.GetAddress() | ||
addr, err = k.GetAddress() | ||
if err != nil { | ||
return nil, "", 0, err | ||
} | ||
|
@@ -365,7 +381,7 @@ func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddres | |
|
||
// NewKeyringFromBackend gets a Keyring object from a backend | ||
func NewKeyringFromBackend(ctx Context, backend string) (keyring.Keyring, error) { | ||
if ctx.GenerateOnly || ctx.Simulate { | ||
if ctx.Simulate { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is removed because |
||
backend = keyring.BackendMemory | ||
} | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I dont think there's a need to pass both
clientCtx
andclientCtx.Keyring
, we might as well just passclientCtx
and get the keyring from that inGetFromFields
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, however the keyring used is not always took from the context ⇾ https://github.com/cosmos/cosmos-sdk/pull/11558/files#diff-eb0834c140c5c40cca7f39ab9d334536d3529642fce425754b8608c5e79b6430R110
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. You can always call
clientCtx.WithKeyring
prior to calling it, but that's a good point. I'm indifferent here.