Skip to content

Commit

Permalink
Merge branch 'master' into robert/bech32-pubkey-slashing
Browse files Browse the repository at this point in the history
  • Loading branch information
sahith-narahari authored Nov 19, 2020
2 parents fdac398 + 4be5cd3 commit 07cfed4
Show file tree
Hide file tree
Showing 5 changed files with 354 additions and 106 deletions.
1 change: 1 addition & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ jobs:
run: |
excludelist="$(find ./ -type f -name '*.go' | xargs grep -l 'DONTCOVER')"
excludelist+=" $(find ./ -type f -name '*.pb.go')"
excludelist+=" $(find ./ -type f -name '*.pb.gw.go')"
excludelist+=" $(find ./ -type f -path './tests/mocks/*.go')"
for filename in ${excludelist}; do
filename=$(echo $filename | sed 's/^./github.com\/cosmos\/cosmos-sdk/g')
Expand Down
195 changes: 96 additions & 99 deletions crypto/hd/hdpath_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,6 @@ func mnemonicToSeed(mnemonic string) []byte {
return bip39.NewSeed(mnemonic, defaultBIP39Passphrase)
}

func TestPathParamsString(t *testing.T) {
path := hd.NewParams(44, 0, 0, false, 0)
require.Equal(t, "m/44'/0'/0'/0/0", path.String())
path = hd.NewParams(44, 33, 7, true, 9)
require.Equal(t, "m/44'/33'/7'/1/9", path.String())
}

func TestStringifyFundraiserPathParams(t *testing.T) {
path := hd.NewFundraiserParams(4, types.CoinType, 22)
require.Equal(t, "m/44'/118'/4'/0/22", path.String())
Expand Down Expand Up @@ -99,98 +92,6 @@ func TestParamsFromPath(t *testing.T) {

}

func TestBIP32Vecs(t *testing.T) {

seed := mnemonicToSeed("barrel original fuel morning among eternal " +
"filter ball stove pluck matrix mechanic")
master, ch := hd.ComputeMastersFromSeed(seed)
fmt.Println("keys from fundraiser test-vector (cosmos, bitcoin, ether)")
fmt.Println()

// cosmos, absolute path
priv, err := hd.DerivePrivateKeyForPath(master, ch, types.FullFundraiserPath)
require.NoError(t, err)
require.NotEmpty(t, priv)
fmt.Println(hex.EncodeToString(priv[:]))

absPrivKey := hex.EncodeToString(priv[:])

// cosmos, relative path
priv, err = hd.DerivePrivateKeyForPath(master, ch, "44'/118'/0'/0/0")
require.NoError(t, err)
require.NotEmpty(t, priv)

relPrivKey := hex.EncodeToString(priv[:])

// check compatibility between relative and absolute HD paths
require.Equal(t, relPrivKey, absPrivKey)

// bitcoin
priv, err = hd.DerivePrivateKeyForPath(master, ch, "m/44'/0'/0'/0/0")
require.NoError(t, err)
require.NotEmpty(t, priv)
fmt.Println(hex.EncodeToString(priv[:]))

// ether
priv, err = hd.DerivePrivateKeyForPath(master, ch, "m/44'/60'/0'/0/0")
require.NoError(t, err)
require.NotEmpty(t, priv)
fmt.Println(hex.EncodeToString(priv[:]))

// INVALID
priv, err = hd.DerivePrivateKeyForPath(master, ch, "m/X/0'/0'/0/0")
require.Error(t, err)
require.Empty(t, priv)

priv, err = hd.DerivePrivateKeyForPath(master, ch, "m/-44/0'/0'/0/0")
require.Error(t, err)
require.Empty(t, priv)

fmt.Println()
fmt.Println("keys generated via https://coinomi.com/recovery-phrase-tool.html")
fmt.Println()

seed = mnemonicToSeed(
"advice process birth april short trust crater change bacon monkey medal garment " +
"gorilla ranch hour rival razor call lunar mention taste vacant woman sister")
master, ch = hd.ComputeMastersFromSeed(seed)
priv, _ = hd.DerivePrivateKeyForPath(master, ch, "m/44'/1'/1'/0/4")
fmt.Println(hex.EncodeToString(priv[:]))

seed = mnemonicToSeed("idea naive region square margin day captain habit " +
"gun second farm pact pulse someone armed")
master, ch = hd.ComputeMastersFromSeed(seed)
priv, err = hd.DerivePrivateKeyForPath(master, ch, "m/44'/0'/0'/0/420")
require.NoError(t, err)
fmt.Println(hex.EncodeToString(priv[:]))

fmt.Println()
fmt.Println("BIP 32 example")
fmt.Println()

// bip32 path: m/0/7
seed = mnemonicToSeed("monitor flock loyal sick object grunt duty ride develop assault harsh history")
master, ch = hd.ComputeMastersFromSeed(seed)
priv, err = hd.DerivePrivateKeyForPath(master, ch, "m/0/7")
require.NoError(t, err) // TODO: shouldn't this error?
fmt.Println(hex.EncodeToString(priv[:]))

// Output: keys from fundraiser test-vector (cosmos, bitcoin, ether)
//
// bfcb217c058d8bbafd5e186eae936106ca3e943889b0b4a093ae13822fd3170c
// e77c3de76965ad89997451de97b95bb65ede23a6bf185a55d80363d92ee37c3d
// 7fc4d8a8146dea344ba04c593517d3f377fa6cded36cd55aee0a0bb968e651bc
//
// keys generated via https://coinomi.com/recovery-phrase-tool.html
//
// a61f10c5fecf40c084c94fa54273b6f5d7989386be4a37669e6d6f7b0169c163
// 32c4599843de3ef161a629a461d12c60b009b676c35050be5f7ded3a3b23501f
//
// BIP 32 example
//
// c4c11d8c03625515905d7e89d25dfc66126fbc629ecca6db489a1a72fc4bda78
}

func TestCreateHDPath(t *testing.T) {
type args struct {
coinType uint32
Expand Down Expand Up @@ -284,3 +185,99 @@ func TestDeriveHDPathRange(t *testing.T) {
})
}
}

func ExampleStringifyPathParams() {
path := hd.NewParams(44, 0, 0, false, 0)
fmt.Println(path.String())
path = hd.NewParams(44, 33, 7, true, 9)
fmt.Println(path.String())
// Output:
// m/44'/0'/0'/0/0
// m/44'/33'/7'/1/9
}

func ExampleSomeBIP32TestVecs() {
seed := mnemonicToSeed("barrel original fuel morning among eternal " +
"filter ball stove pluck matrix mechanic")
master, ch := hd.ComputeMastersFromSeed(seed)
fmt.Println("keys from fundraiser test-vector (cosmos, bitcoin, ether)")
fmt.Println()
// cosmos
priv, err := hd.DerivePrivateKeyForPath(master, ch, types.FullFundraiserPath)
if err != nil {
fmt.Println("INVALID")
} else {
fmt.Println(hex.EncodeToString(priv[:]))
}
// bitcoin
priv, err = hd.DerivePrivateKeyForPath(master, ch, "44'/0'/0'/0/0")
if err != nil {
fmt.Println("INVALID")
} else {
fmt.Println(hex.EncodeToString(priv[:]))
}
// ether
priv, err = hd.DerivePrivateKeyForPath(master, ch, "44'/60'/0'/0/0")
if err != nil {
fmt.Println("INVALID")
} else {
fmt.Println(hex.EncodeToString(priv[:]))
}
// INVALID
priv, err = hd.DerivePrivateKeyForPath(master, ch, "X/0'/0'/0/0")
if err != nil {
fmt.Println("INVALID")
} else {
fmt.Println(hex.EncodeToString(priv[:]))
}
priv, err = hd.DerivePrivateKeyForPath(master, ch, "-44/0'/0'/0/0")
if err != nil {
fmt.Println("INVALID")
} else {
fmt.Println(hex.EncodeToString(priv[:]))
}

fmt.Println()
fmt.Println("keys generated via https://coinomi.com/recovery-phrase-tool.html")
fmt.Println()

seed = mnemonicToSeed(
"advice process birth april short trust crater change bacon monkey medal garment " +
"gorilla ranch hour rival razor call lunar mention taste vacant woman sister")
master, ch = hd.ComputeMastersFromSeed(seed)
priv, _ = hd.DerivePrivateKeyForPath(master, ch, "44'/1'/1'/0/4")
fmt.Println(hex.EncodeToString(priv[:]))

seed = mnemonicToSeed("idea naive region square margin day captain habit " +
"gun second farm pact pulse someone armed")
master, ch = hd.ComputeMastersFromSeed(seed)
priv, _ = hd.DerivePrivateKeyForPath(master, ch, "44'/0'/0'/0/420")
fmt.Println(hex.EncodeToString(priv[:]))

fmt.Println()
fmt.Println("BIP 32 example")
fmt.Println()

// bip32 path: m/0/7
seed = mnemonicToSeed("monitor flock loyal sick object grunt duty ride develop assault harsh history")
master, ch = hd.ComputeMastersFromSeed(seed)
priv, _ = hd.DerivePrivateKeyForPath(master, ch, "0/7")
fmt.Println(hex.EncodeToString(priv[:]))

// Output: keys from fundraiser test-vector (cosmos, bitcoin, ether)
//
// bfcb217c058d8bbafd5e186eae936106ca3e943889b0b4a093ae13822fd3170c
// e77c3de76965ad89997451de97b95bb65ede23a6bf185a55d80363d92ee37c3d
// 7fc4d8a8146dea344ba04c593517d3f377fa6cded36cd55aee0a0bb968e651bc
// INVALID
// INVALID
//
// keys generated via https://coinomi.com/recovery-phrase-tool.html
//
// a61f10c5fecf40c084c94fa54273b6f5d7989386be4a37669e6d6f7b0169c163
// 32c4599843de3ef161a629a461d12c60b009b676c35050be5f7ded3a3b23501f
//
// BIP 32 example
//
// c4c11d8c03625515905d7e89d25dfc66126fbc629ecca6db489a1a72fc4bda78
}
22 changes: 15 additions & 7 deletions docs/architecture/adr-031-msg-service.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ This isn't necessarily bad, but it does add overhead to creating modules.
We decide to use protobuf `service` definitions for defining `Msg`s as well as
the code generated by them as a replacement for `Msg` handlers.

Below we define how this will look for the `SubmitProposal` message from `x/gov` module.
Below we define how this will look for the `SubmitProposal` message from `x/gov` module.
We start with a `Msg` `service` definition:

```proto
Expand Down Expand Up @@ -105,7 +105,7 @@ should use the more canonical `Msg...Request` names.
Currently, we are encoding `Msg`s as `Any` in `Tx`s which involves packing the
binary-encoded `Msg` with its type URL.

The type URL for `MsgSubmitProposal` based on the proto3 spec is `/cosmos.gov.MsgSubmitProposal`.
The type URL for `MsgSubmitProposal` based on the proto3 spec is `/cosmos.gov.MsgSubmitProposal`.

The fully-qualified name for the `SubmitProposal` service method above (also
based on the proto3 and gRPC specs) is `/cosmos.gov.Msg/SubmitProposal` which varies
Expand All @@ -117,15 +117,15 @@ In order to encode service methods in transactions, we encode them as `Any`s in
the same `TxBody.messages` field as other `Msg`s. We simply set `Any.type_url`
to the full-qualified method name (ex. `/cosmos.gov.Msg/SubmitProposal`) and
set `Any.value` to the protobuf encoding of the request message
(`MsgSubmitProposal` in this case).
(`MsgSubmitProposal` in this case).

### Decoding

When decoding, `TxBody.UnpackInterfaces` will need a special case
to detect if `Any` type URLs match the service method format (ex. `/cosmos.gov.Msg/SubmitProposal`)
by checking for two `/` characters. Messages that are method names plus request parameters
instead of a normal `Any` messages will get unpacked into the `ServiceMsg` struct:

```go
type ServiceMsg struct {
// MethodName is the fully-qualified service name
Expand All @@ -139,7 +139,7 @@ type ServiceMsg struct {

In the future, `service` definitions may become the primary method for defining
`Msg`s. As a starting point, we need to integrate with the SDK's existing routing
and `Msg` interface.
and `Msg` interface.

To do this, `ServiceMsg` implements the `sdk.Msg` interface and its handler does the
actual method routing, allowing this feature to be added incrementally on top of
Expand Down Expand Up @@ -218,17 +218,25 @@ Separate handler definition is no longer needed with this approach.

## Consequences

This design changes how a module functionality is exposed and accessed. It deprecates the existing `Handler` interface and `AppModule.Route` in favor of [Protocol Buffer Services](https://developers.google.com/protocol-buffers/docs/proto3#services) and Service Routing described above. This dramatically simplifies the code. We don't need to create handlers and keepers any more. Use of Protocol Buffer auto-generated clients clearly separates the communication interfaces between the module and a modules user. The control logic (aka handlers and keepers) is not exposed any more. A module interface can be seen as a black box accessible through a client API. It's worth to note that the client interfaces are also generated by Protocol Buffers.

This also allows us to change how we perform functional tests. Instead of mocking AppModules and Router, we will mock a client (server will stay hidden). More specifically: we will never mock `moduleA.MsgServer` in `moduleB`, but rather `moduleA.MsgClient`. One can think about it as working with external services (eg DBs, or online servers...). We assume that the transmission between clients and servers is correctly handled by generated Protocol Buffers.

Finally, closing a module to client API opens desirable OCAP patterns discussed in ADR-033. Since server implementation and interface is hidden, nobody can hold "keepers"/servers and will be forced to relay on the client interface, which will drive developers for correct encapsulation and software engineering patterns.

### Pros
- communicates return type clearly
- manual handler registration and return type marshaling is no longer needed, just implement the interface and register it
- some keeper code could be automatically generate, this would improve the UX of [\#7093](https://github.com/cosmos/cosmos-sdk/issues/7093) approach (1) if we chose to adopt that
- generated client code could be useful for clients
- communication interface is automatically generated, the developer can now focus only on the state transition methods - this would improve the UX of [\#7093](https://github.com/cosmos/cosmos-sdk/issues/7093) approach (1) if we chose to adopt that
- generated client code could be useful for clients and tests
- dramatically reduces and simplifies the code

### Cons
- supporting both this and the current concrete `Msg` type approach simultaneously could be confusing
(we could choose to deprecate the current approach)
- using `service` definitions outside the context of gRPC could be confusing (but doesn’t violate the proto3 spec)


## References

- [Initial Github Issue \#7122](https://github.com/cosmos/cosmos-sdk/issues/7122)
Expand Down
Loading

0 comments on commit 07cfed4

Please sign in to comment.