R4R: Gaia Lite Generate Only Support (w/o Keybase) #3396

alexanderbez · 2019-01-25T16:14:41Z

Minor CLI context and utils cleanup
REST now allows using an address OR key name (just like the CLI ⚡️)
- This does cause a breaking change (name => from, again just like the CLI ⚡️)
Update CompleteAndBroadcastTxREST
- Remove generate only logic (this now should be handled in each respective REST handler)
- Use prepareTxBuilder which will allow account number and nonce to be derived from state
Update swagger
- Remove any notion generate_only and simulate as query params (these were deprecated long agooooo)
- Update name to from
- Formatting fixes from @faboweb

closes: #3284

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (docs/)
Added entries in PENDING.md with issue #
rereviewed Files changed in the github PR explorer

For Admin Use:

Added appropriate labels to PR (ex. wip, ready-for-review, docs)
Reviewers Assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

- remove gen only logic - update base req to use from instead of name - use context to get the from address and name

client/utils/rest.go

x/bank/client/rest/sendtx.go

client/context/query.go

client/context/context.go

codecov · 2019-01-28T18:46:05Z

Codecov Report

Merging #3396 into develop will increase coverage by 0.07%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #3396      +/-   ##
===========================================
+ Coverage    59.35%   59.42%   +0.07%     
===========================================
  Files          131      131              
  Lines         9868     9891      +23     
===========================================
+ Hits          5857     5878      +21     
- Misses        3639     3641       +2     
  Partials       372      372

fedekunze

Can we add some tests for the Automatic account number and sequence population when fields are omitted and Generate only functionality no longer requires access to a local Keybase on lcd_test.go ?

client/utils/rest.go

fedekunze · 2019-01-28T19:08:25Z

docs/gaia/gaiacli.md

@@ -231,7 +233,12 @@ gaiacli tx send \
  --generate-only > unsignedSendTx.json
 ```

-You can now sign the transaction file generated through the `--generate-only` flag by providing your key to the following command:
+__Note__: Simulation cannot be used in conjunction with tx generation only


Can we have a custom container to make the note bigger ? (check https://vuepress.vuejs.org/guide/markdown.html#custom-containers)

And why is the simulation not working in this case?

@fedekunze yeah sure, updated!

@faboweb not sure what you mean? Is the comment/note not explanatory enough? You can't use simulate and generate_only together because simulate requires a pubkey and generate_only does not use a Keybase (as requested) and thus you cannot attach a pubkey.

Why does the simulation need a real pubkey?

Is this the same issue: #3423 ?

No, I cannot reproduce #3396 -- "auto" works for me. Simulation needs a public key because a large part of gas estimation is signature verification.

Also note: "gas": "auto" =/= "simulate"

ah ok, I missed that those are different things. But for signature verification you could use any pubkey, can't you?

Let's not pollute this PR with the discussion and move it over to #3430

faboweb · 2019-01-28T19:19:51Z

Use prepareTxBuilder which will allow account number and nonce to be derived from state

so we don't need to set those? that is probably the awesomest change ever!!!

alexanderbez · 2019-01-28T19:24:13Z

so we don't need to set those? that is probably the awesomest change ever!!!

@faboweb yup! And it was just a one-liner :)

Co-Authored-By: alexanderbez <alexanderbez@users.noreply.github.com>

alexanderbez · 2019-01-28T19:37:51Z

Can we add some tests for the Automatic account number and sequence population when fields are omitted and Generate only functionality no longer requires access to a local Keybase on lcd_test.go ?

@fedekunze yes, sure thing.

…into bez/3284-gaia-lite-gen-only-tx

jackzampolin

Great changes @alexanderbez LGTM

Aleksandr Bezobchuk added 2 commits January 25, 2019 11:11

update swagger from original pr

be414e8

cleanup CLIContext and expose from fields/helpers

2c1a548

alexanderbez added gaia-lite labels Jan 25, 2019

alexanderbez requested review from alessio, faboweb, jackzampolin, cwgoes and fedekunze January 25, 2019 16:14

alexanderbez requested review from ebuchman and rigelrozanski as code owners January 25, 2019 16:14

This was referenced Jan 25, 2019

WIP: tx generate only #3287

Closed

R4R: CLIContext Refactor #3321

Closed

Aleksandr Bezobchuk added 2 commits January 25, 2019 11:21

update utils and API

a495415

update rest utils

54c10c7

- remove gen only logic - update base req to use from instead of name - use context to get the from address and name

alexanderbez added the T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). label Jan 25, 2019

alexanderbez mentioned this pull request Jan 25, 2019

Generate_only should create txs without requiring a local key nor password #3284

Closed

4 tasks

Aleksandr Bezobchuk added 3 commits January 25, 2019 13:22

refactor WriteGenerateStdTxResponse and implement ToTxBuilder

8405680

update CompleteAndBroadcastTxREST godoc

64fa9f9

further refactor CompleteAndBroadcastTxREST

e2f3854

alexanderbez added the prelaunch label Jan 25, 2019

update bank transfer REST route to support gen only

b7c8e4b

alexanderbez commented Jan 25, 2019

View reviewed changes

client/utils/rest.go Outdated Show resolved Hide resolved

add gen only comment on address validation

7b2641c

jackzampolin reviewed Jan 25, 2019

View reviewed changes

x/bank/client/rest/sendtx.go Show resolved Hide resolved

jackzampolin reviewed Jan 25, 2019

View reviewed changes

client/context/query.go Show resolved Hide resolved

jackzampolin reviewed Jan 25, 2019

View reviewed changes

client/context/query.go Show resolved Hide resolved

jackzampolin reviewed Jan 25, 2019

View reviewed changes

client/context/context.go Show resolved Hide resolved

alessio removed the prelaunch label Jan 27, 2019

jackzampolin removed the gaia-lite label Jan 28, 2019

fix and update LCD tests to use gas estimation and generation only

69f488d

update docs

e31f60c

alexanderbez requested a review from zramsay as a code owner January 28, 2019 18:46

Aleksandr Bezobchuk added 2 commits January 28, 2019 13:50

update swagger docs

db52973

add pending log entry

ea854aa

alexanderbez changed the title ~~WIP: Gaia Lite Generate Only Support (w/o Keybase)~~ R4R: Gaia Lite Generate Only Support (w/o Keybase) Jan 28, 2019

alexanderbez added ready-for-review and removed wip labels Jan 28, 2019

Merge branch 'develop' into bez/3284-gaia-lite-gen-only-tx

c26e4bf

fedekunze reviewed Jan 28, 2019

View reviewed changes

fedekunze and others added 2 commits January 28, 2019 14:24

Update client/utils/rest.go

1328e0c

Co-Authored-By: alexanderbez <alexanderbez@users.noreply.github.com>

use Vue tip notes

01baa21

Aleksandr Bezobchuk added 3 commits January 28, 2019 15:01

Merge remote-tracking branch 'origin/bez/3284-gaia-lite-gen-only-tx' …

4b06321

…into bez/3284-gaia-lite-gen-only-tx

implement TestCoinSendGenerateOnly

9c892c9

implement TestCoinSendAccAuto

30aebc1

alessio approved these changes Jan 28, 2019

View reviewed changes

faboweb mentioned this pull request Jan 29, 2019

Remove account_number and sequence luniehq/lunie#1900

Closed

alexanderbez mentioned this pull request Jan 29, 2019

Automatic gas calculation doesn't work with generate_only #3423

Closed

4 tasks

alexanderbez modified the milestone: v0.31.0 (Launch RC) Jan 29, 2019

alexanderbez mentioned this pull request Jan 29, 2019

CLI: Do not require Keybase for tx generate only #3426

Closed

4 tasks

jackzampolin approved these changes Jan 29, 2019

View reviewed changes

alexanderbez mentioned this pull request Jan 29, 2019

Generate_only doesn't respect gas #3430

Closed

4 tasks

jackzampolin merged commit 90797f5 into develop Jan 29, 2019

alessio deleted the bez/3284-gaia-lite-gen-only-tx branch January 29, 2019 19:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R4R: Gaia Lite Generate Only Support (w/o Keybase) #3396

R4R: Gaia Lite Generate Only Support (w/o Keybase) #3396

alexanderbez commented Jan 25, 2019 •

edited

Loading

codecov bot commented Jan 28, 2019 •

edited

Loading

fedekunze left a comment

fedekunze Jan 28, 2019

faboweb Jan 28, 2019

alexanderbez Jan 28, 2019

faboweb Jan 29, 2019 •

edited

Loading

faboweb Jan 29, 2019

alexanderbez Jan 29, 2019 •

edited

Loading

faboweb Jan 29, 2019

faboweb Jan 29, 2019

faboweb commented Jan 28, 2019

alexanderbez commented Jan 28, 2019

alexanderbez commented Jan 28, 2019

jackzampolin left a comment

R4R: Gaia Lite Generate Only Support (w/o Keybase) #3396

R4R: Gaia Lite Generate Only Support (w/o Keybase) #3396

Conversation

alexanderbez commented Jan 25, 2019 • edited Loading

codecov bot commented Jan 28, 2019 • edited Loading

Codecov Report

fedekunze left a comment

Choose a reason for hiding this comment

fedekunze Jan 28, 2019

Choose a reason for hiding this comment

faboweb Jan 28, 2019

Choose a reason for hiding this comment

alexanderbez Jan 28, 2019

Choose a reason for hiding this comment

faboweb Jan 29, 2019 • edited Loading

Choose a reason for hiding this comment

faboweb Jan 29, 2019

Choose a reason for hiding this comment

alexanderbez Jan 29, 2019 • edited Loading

Choose a reason for hiding this comment

faboweb Jan 29, 2019

Choose a reason for hiding this comment

faboweb Jan 29, 2019

Choose a reason for hiding this comment

faboweb commented Jan 28, 2019

alexanderbez commented Jan 28, 2019

alexanderbez commented Jan 28, 2019

jackzampolin left a comment

Choose a reason for hiding this comment

alexanderbez commented Jan 25, 2019 •

edited

Loading

codecov bot commented Jan 28, 2019 •

edited

Loading

faboweb Jan 29, 2019 •

edited

Loading

alexanderbez Jan 29, 2019 •

edited

Loading