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

Enable simple CLI offline mode to disable querying node & request parameters #3720

Closed
cwgoes opened this issue Feb 23, 2019 · 11 comments
Closed
Assignees

Comments

@cwgoes
Copy link
Contributor

cwgoes commented Feb 23, 2019

It is quite difficult right now to use the CLI on an offline computer to sign transactions - the CLI generally expects to be able to query a node for account info, sequence number, etc. and which options need to be overridden are unclear (this is true even with --generate-only).

I propose we implement a --offline flag, which when specified causes gaiacli to prompt the user for all required values (account number, sequence number) and skip any validity checks requiring state queries (e.g. checking account balance).

cc @alessio thoughts?

AC

AC1

CLIContext.EnsureAccountExists{,FromAddr}() unconditionally returns true when --offline is passed in gaiacli

AC2

When --offline is passed in gaiacli no RPC queries are sent to the node.

@jackzampolin
Copy link
Member

I think this is a good idea. I think we should consider making --generate-only use offline mode and avoid adding more flags if possible.

@cwgoes
Copy link
Contributor Author

cwgoes commented Feb 25, 2019

Sure, whatever's easiest - I think we already have an --offline flag, it just doesn't always work.

@alessio
Copy link
Contributor

alessio commented Mar 4, 2019

--generate-only and --offline aren't the same thing. Whilst --offline prevents (or "should" maybe, as I can confirm it does not always work) the program from accessing the node, one could enable --generate-only in online mode so that the output produced is populated with data extracted from a query to the node.

ACTIONS:

  1. fix --offline as it does not always work - @cwgoes can you please list commands/cases you stumbled upon where offline didn't work as expected please? I got a few ideas on how to fix this.
  2. --offline must imply --generate-only - IIRC this should be the case already

@alessio
Copy link
Contributor

alessio commented Apr 16, 2019

I had second thoughts on --offline. With the recent changes introduced in v0.34.0 --generate-only now disables access to keybase - which may not necessarily imply lack of connectivity. Conversely, it might actually be inconsistent for --offline to disable keybase access too. Thoughts?

@alexanderbez
Copy link
Contributor

What exactly are you suggesting @alessio? Keybase has nothing to do with online connectivity; just a local encrypted embedded DB. What does --offline have to do with the Keybase?

@alessio
Copy link
Contributor

alessio commented Apr 16, 2019

I was taking back my --offline must imply --generate-only (#3720 (comment)). I meant: --offline have nothing to do with the Keybase.
To recap, --generate-only does 2 things:

  • avoid broadcasting the tx
  • disable keybase access (since v0.34.0)

And rightly so. --offline should just:

  • Cut the connection towards a node by preventing Context.Client from being initialised.

Context should be adapted to operate in absence of connectivity - e.g. EnsureAccountExists() should always return true. Other than multisign and sign which implement --offline, when parsing --from we query the node unconditionally.

In other words: this is a proper request for a new feature.

@alexanderbez
Copy link
Contributor

alexanderbez commented Apr 16, 2019

Hmmm, we still need to use CLIContext (not sure if that's what you meant). But yes, I suppose EnsureAccountExists should return true during offline mode so no lookups are actually made. Also, not sure what you mean by "when parsing --from we query the node unconditionally."? You don't query a node for an address/account -- that's a keybase lookup.

The actionable items (AC) I see are:

  • Return true for EnsureAccountExists when --offline is passed in the CLI
  • Ensure no RPC queries are mode when --offline is passed in the CLI

What do you think?

@alessio
Copy link
Contributor

alessio commented Apr 16, 2019

Also, not sure what you mean by "when parsing --from we query the node unconditionally."

All right, true that - sorry for causing confusion. A key name in --from needs to be expanded - unless --generate-only is on, in which case we should expect the from flag to carry an address.

Ensure no RPC queries are mode when --offline is passed in the CLI

We should open a dedicated issue and write relevant ACs as that is what this issue is all about.

[brain dump continues]

We should really get rid of the --from flag from anywhere it is not required. It is a misnomer in most cases and CLIContext should account for the signer key to be passed to GetFrom{Address,Name} from a positional argument instead.

But we're talking about quite a refactoring here...

[brain dump ends]

@alexanderbez
Copy link
Contributor

We should open a dedicated issue and write relevant ACs as that is what this issue is all about.

AFAIK, this is the very issue. Do you agree with the AC I've listed?

@alessio
Copy link
Contributor

alessio commented Apr 17, 2019

Yes, they look good to me.

@cwgoes
Copy link
Contributor Author

cwgoes commented Jul 4, 2019

I believe this has since been done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants