Remove the old message PublicKey proto oneof #7390
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7390 +/- ##
=======================================
Coverage 55.33% 55.33%
=======================================
Files 587 587
Lines 36502 36502
=======================================
Hits 20200 20200
Misses 14204 14204
Partials 2098 2098 |
blushi
requested review from
aaronc,
alessio and
alexanderbez
as code owners
blushi
requested review from
amaury1093,
anilcse,
atheeshp,
clevinson and
robert-zaremba
Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>
| @@ -2,33 +2,9 @@ syntax = "proto3"; | |||
| package cosmos.base.crypto.v1beta1; | |||
There was a problem hiding this comment.
@blushi would it actually make sense to remove this proto file, and put the 2 remaining messages inside cosmos.crypto.multisig?
There was a problem hiding this comment.
yes I believe that makes sense.
There was a problem hiding this comment.
moved the proto file to proto/crypto/multisig but kept the generated types in crypto/types because:
- putting it in
crypto/keys/multisigwouldn't make much sense ; - it would introduce some import cycles.
| @@ -0,0 +1,25 @@ | |||
| syntax = "proto3"; | |||
| package cosmos.crypto.multisig.v1beta1; | |||
There was a problem hiding this comment.
Should we remove v1beta1 here? I believe in cosmos.crypto.* we kind of decided not to put any v1beta1 anywhere
There was a problem hiding this comment.
It made sense to remove the versioning for PubKey types which are then used to generate addresses but it might be better to keep it for MultiSignature and CompactBitArray like it's the case for the rest of the proto types.
Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>
amaury1093
added
the
A:automerge
| // CompactBitArray is an implementation of a space efficient bit array. | ||
| // This is used to ensure that the encoded data takes up a minimal amount of | ||
| // space after proto encoding. | ||
| // This is not thread safe, and is not intended for concurrent usage. |
There was a problem hiding this comment.
Could you provide a link in the comment to the encoding spec?
Description
ref: #7357
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes