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

Remove password/keybase from REST Client #3674

Merged
merged 12 commits into from
Feb 19, 2019

Conversation

alexanderbez
Copy link
Contributor

@alexanderbez alexanderbez commented Feb 18, 2019

  • Remove generate_only and password from BaseReq
  • Remove /keys/* routes
  • Remove /tx/sign route
  • Cleanup REST tests
  • Update Swagger

closes: #3641

/cc @cosmos/cosmos-ui


  • 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)

@alexanderbez alexanderbez added the T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). label Feb 19, 2019
@alexanderbez alexanderbez marked this pull request as ready for review February 19, 2019 14:43
@codecov
Copy link

codecov bot commented Feb 19, 2019

Codecov Report

Merging #3674 into develop will increase coverage by 0.61%.
The diff coverage is 88.88%.

@@             Coverage Diff             @@
##           develop    #3674      +/-   ##
===========================================
+ Coverage     60.6%   61.22%   +0.61%     
===========================================
  Files          189      189              
  Lines        14236    14039     -197     
===========================================
- Hits          8628     8595      -33     
+ Misses        5063     4902     -161     
+ Partials       545      542       -3

Copy link
Collaborator

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

utACK

@alexanderbez
Copy link
Contributor Author

alexanderbez commented Feb 19, 2019

will cherry-pick commit into release/v0.32.1 once merged into develop

@jackzampolin jackzampolin merged commit e39debd into develop Feb 19, 2019
@alexanderbez alexanderbez mentioned this pull request Feb 19, 2019
5 tasks
@cachitu
Copy link

cachitu commented Feb 27, 2019

Without a working light client on iOS and Android and no /keys api, how can we handle wallets management locally on mobiles (create, recover)?

@alexanderbez
Copy link
Contributor Author

@cachitu great question. To support mobile, client implementation needs to exist in order to be able to manage the keys locally on the device. You can then talk to a trusted REST client to broadcast the signed txs.

@cachitu
Copy link

cachitu commented Feb 27, 2019

I tried with gomobile to generate the iOS lcd.framework, but seems non-functional/incomplete. Will the cosmos team help to get the right working SDKs for mobile? Thank you.

@jackzampolin
Copy link
Member

jackzampolin commented Feb 27, 2019

@cachitu Our story for mobile right now is a bit incomplete. There are good key and wallet generation utilities for that platform (we use the bip 32/44 standard). There are also a number of community tools for working with cosmos based chains: https://github.com/binance-chain/javascript-sdk/

We have a bi-weekly developer call for wallet developers that I would love for you to join. Please shoot us an email to partners@tendermint.com and reference this GH post.

@Knight-H
Copy link

I'm not sure how helpful this is... but in my scenario, a hybrid application (using javascript) was developed for the mobile platforms and I used the code in wallet.js in Cosmos Voyager (Cosmos' UI and Wallet Project) to generateWallet() (returning mnemonic, private, public, cosmosAddress) and also sign() to sign the generate_only Tx (without signature) and broadcast them to one of the full Nodes.

However, I am also interested in implementing the Lite Client on the mobile (since this procedure should be the best practice as highlighted in Lite Client Overview?) to let the mobile clients be able to achieve some form of trust to the blockchain network as written here.

But as @jackzampolin mentioned above, do I take it that Lite Client on mobile is still incomplete and the way to implement it for now is for the Clients to use one of the Node's rest server API directly (without proofs)?

@mappum
Copy link
Contributor

mappum commented Mar 15, 2019

@Knight-H There is a JavaScript light client: https://github.com/nomic-io/js-tendermint

Note that this only implements the logic for the Tendermint level so you'll have to be able to encode/decode cosmos-sdk transactions outside of that.

@Yoge-Code
Copy link

Yoge-Code commented May 23, 2019

How to implement key manager locally? Are there some docs or references about the signature work ? Or are there some alternative ways to sign a transaction in a remote machine that running a python server instead of implementing a python SDK?

@alexanderbez
Copy link
Contributor Author

@YG-FLUSH Signatures work over the secp256k1 curve. Given a valid private key, you just have to generate a valid JSON object over which serialize and then sign: https://github.com/cosmos/cosmos-sdk/blob/v0.34.4/x/auth/stdtx.go#L178-L196

Of course, you'll need a codec for the fee and msgs or you can construct these JSON payloads manually. I know there are JS implementations of this, but idk about Python.

@Yoge-Code
Copy link

Yoge-Code commented May 24, 2019

@alexanderbez Thanks for your replay! Now I get the python secp256k1 lib, and it works. But It is hard to find the private key (I had found it by debugging the gaiacli source code and printed it). Any simple way here to export the private key?

@alexanderbez
Copy link
Contributor Author

No, I imagine for security reasons we don't expose the private key. This is what the mnemonic is for.

@alessio
Copy link
Contributor

alessio commented May 27, 2019

@YG-FLUSH Look at crypto/keys/mintkey.EncryptArmorPrivKey - that should export your private key in ASCII armored format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants