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

Implement Cosmos connection #1285

Merged
merged 33 commits into from
Dec 3, 2019
Merged

Implement Cosmos connection #1285

merged 33 commits into from
Dec 3, 2019

Conversation

willclarktech
Copy link
Contributor

@willclarktech willclarktech commented Oct 29, 2019

Part 2 of #1220
Merge after #1284

  • connection implementation
  • very basic REST client (RPC requires Tendermint v0.32)
  • Cosmos HD paths

Connection methods not implemented:

  • anything requiring streams
  • getToken/getAllTokens: no obvious (documented) way to query for these via the REST API

@willclarktech willclarktech changed the base branch from 1220-cosmos-codec to master October 29, 2019 11:30
@willclarktech willclarktech force-pushed the 1220-cosmos-connection branch 2 times, most recently from 4ef8e69 to 600ac92 Compare October 29, 2019 12:15
@willclarktech
Copy link
Contributor Author

willclarktech commented Oct 29, 2019

CI is currently failing in browsers because CORS is not enabled on the Cosmos Gaia REST server. See cosmos/cosmos-sdk#4622

Edit: Working with hack - 3c82059

@willclarktech willclarktech force-pushed the 1220-cosmos-connection branch 3 times, most recently from 0e92c2b to a2d1e01 Compare October 30, 2019 12:25
@willclarktech willclarktech force-pushed the 1220-cosmos-connection branch 2 times, most recently from b293643 to e40f964 Compare October 30, 2019 14:42
@willclarktech willclarktech force-pushed the 1220-cosmos-connection branch 4 times, most recently from fac84ab to 0af02ad Compare November 6, 2019 14:00
@willclarktech willclarktech force-pushed the 1220-cosmos-connection branch 3 times, most recently from 52c3c0b to e234967 Compare November 19, 2019 15:05
@webmaster128
Copy link
Contributor

@willclarktech can you please update this PR?

packages/iov-cosmos/src/cosmosconnection.ts Show resolved Hide resolved
@@ -94,6 +94,10 @@ export class HdPaths {
return HdPaths.bip44(HdPaths.coinTypes.eth, 0, 0, account);
}

public static cosmos(account: number): readonly Slip10RawIndex[] {
return HdPaths.bip44(HdPaths.coinTypes.atom, 0, 0, account);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a documentation link for this path? My usual source (https://github.com/trezor/trezor-firmware/blob/master/core/docs/misc/coins-bip44-paths.md) does not list Cosmos

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See https://github.com/cosmos/cosmos-sdk/blob/2c96bbb/crypto/keys/types.go#L34-L36

Copy link
Contributor

Choose a reason for hiding this comment

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

That cosmos SDK link is very helpful. There however, a two dimentional space is create (account and index but let's ignore the naming of the axes for now). In https://github.com/cosmos/cosmos-sdk/blob/v0.37.4/client/keys/add.go#L39-L259 it looks like both dimensions are used.

It would be interesting to find out what wallets typically do.

Copy link
Contributor Author

@willclarktech willclarktech Nov 28, 2019

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Finally a confirmation here! cosmos/cosmos-sdk#4278 (comment)

@webmaster128 webmaster128 merged commit c253246 into master Dec 3, 2019
@webmaster128 webmaster128 deleted the 1220-cosmos-connection branch December 3, 2019 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants