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

R4R: Allow --from to be a name or an address #2073

Merged
merged 16 commits into from
Sep 25, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Updates from code review
  • Loading branch information
mslipper committed Aug 30, 2018
commit ac4bc2aaabf794f571113abad4628b22e00dc4c9
2 changes: 1 addition & 1 deletion client/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (ctx CLIContext) WithAccountStore(accountStore string) CLIContext {
return ctx
}

// WithFrom returns a copy of the context with an updated from address orname.
// WithFrom returns a copy of the context with an updated from address or name.
func (ctx CLIContext) WithFrom(from string) CLIContext {
ctx.From = from
return ctx
Expand Down
50 changes: 30 additions & 20 deletions client/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,11 @@ func SendTx(txCtx authctx.TxContext, cliCtx context.CLIContext, msgs []sdk.Msg)
return err
}

// TODO: (ref #1903) Allow for user supplied account number without
// automatically doing a manual lookup.
if txCtx.AccountNumber == 0 {
accNum, err := cliCtx.GetAccountNumber(from)
if err != nil {
return err
}

txCtx = txCtx.WithAccountNumber(accNum)
}

// TODO: (ref #1903) Allow for user supplied account sequence without
// automatically doing a manual lookup.
if txCtx.Sequence == 0 {
accSeq, err := cliCtx.GetAccountSequence(from)
if err != nil {
return err
}

txCtx = txCtx.WithSequence(accSeq)
txCtxPtr, err := lookupTxCtx(txCtx, cliCtx, from)
if err != nil {
return err
}
txCtx = *txCtxPtr

name, err := cliCtx.GetFromName()
if err != nil {
Expand All @@ -79,6 +63,32 @@ func SendTx(txCtx authctx.TxContext, cliCtx context.CLIContext, msgs []sdk.Msg)
return cliCtx.EnsureBroadcastTx(txBytes)
}

func lookupTxCtx(txCtx authctx.TxContext, cliCtx context.CLIContext, from []byte) (*authctx.TxContext, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did this come from? I don't think we should be returning pointers to contexts. Is this from the original simulation PR @alessio ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nvm, I don't see this on develop. I don't think lookupTxCtx is appropriate here. See prepareTxContext here. I think it should be analogous to that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was trying to reduce the cyclomatic complexity of the function above - I can revert.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohhh no, we should definitely reduce cyclomatic complexity! We just should:

  1. Not return a pointer
  2. Avoid a merge conflict cause @alessio's PR has something similar

That's all :-)

// TODO: (ref #1903) Allow for user supplied account number without
// automatically doing a manual lookup.
if txCtx.AccountNumber == 0 {
accNum, err := cliCtx.GetAccountNumber(from)
if err != nil {
return nil, err
}

txCtx = txCtx.WithAccountNumber(accNum)
}

// TODO: (ref #1903) Allow for user supplied account sequence without
// automatically doing a manual lookup.
if txCtx.Sequence == 0 {
accSeq, err := cliCtx.GetAccountSequence(from)
if err != nil {
return nil, err
}

txCtx = txCtx.WithSequence(accSeq)
}

return &txCtx, nil
}

// EnrichCtxWithGas calculates the gas estimate that would be consumed by the
// transaction and set the transaction's respective value accordingly.
func EnrichCtxWithGas(txCtx authctx.TxContext, cliCtx context.CLIContext, name, passphrase string, msgs []sdk.Msg) (authctx.TxContext, error) {
Expand Down