-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Regen Network/Slashing protobuf #5627
Regen Network/Slashing protobuf #5627
Conversation
Add base for migrating slashing module to protobuf
…g_protobuf_migration
…twork/cosmos-sdk into regen-network/slashing_protobuf
…twork/cosmos-sdk into regen-network/slashing_protobuf
5395ce8
to
d90bb3e
Compare
…ork/slashing_protobuf # Conflicts: # x/slashing/app_test.go
simapp/codec.go
Outdated
@@ -15,6 +16,7 @@ type AppCodec struct { | |||
amino *codec.Codec | |||
|
|||
Params *params.Codec | |||
Slashing *slashing.Codec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think we should have something more generic when modules don't actually do anything special with their codec. What do you think @alexanderbez ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file should be removed as the new design does not contain this struct. you can see here: https://github.com/cosmos/cosmos-sdk/blob/08502d6d98bff3872916f8b69cde2d255351d3fb/simapp/codec/codec.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Bump types, keeper to root
…ork/slashing_protobuf # Conflicts: # CHANGELOG.md
Codecov Report
@@ Coverage Diff @@
## master #5627 +/- ##
==========================================
- Coverage 39.42% 38.79% -0.63%
==========================================
Files 326 327 +1
Lines 29014 29505 +491
==========================================
+ Hits 11438 11446 +8
- Misses 16386 16867 +481
- Partials 1190 1192 +2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Just a few small nit picks 👍
CHANGELOG.md
Outdated
for JSON encoding. | ||
* The `Keeper` constructor now takes a `codec.Marshaler` instead of a concrete Amino codec. This exact type | ||
provided is specified by `ModuleCdc`. | ||
* (distr) [\#5610](https://github.com/cosmos/cosmos-sdk/pull/5610) Migrate the `x/distribution` module to use Protocol Buffer for state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this line added?
x/slashing/types/codec.go
Outdated
cdc.RegisterConcrete(MsgUnjail{}, "cosmos-sdk/MsgUnjail", nil) | ||
} | ||
|
||
type Codec struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't actually need a concrete type here anymore. See https://github.com/cosmos/cosmos-sdk/blob/master/x/distribution/types/codec.go for an example. You can just copy and modify.
x/slashing/types/types.proto
Outdated
syntax = "proto3"; | ||
package cosmos_sdk.x.slashing.v1; | ||
|
||
option go_package = "types"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think some other modules do this wrong and need to be fixed. This should be github.com/cosmos/cosmos-sdk/x/slashing/types
.
…ork/slashing_protobuf # Conflicts: # x/slashing/keeper/test_common.go
…twork/cosmos-sdk into regen-network/slashing_protobuf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 🎉
…ork/slashing_protobuf
Closes: regen-network#19
Description
For contributor use:
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerFor admin use:
WIP
,R4R
,docs
, etc)