proto: add proto files for ibc unblock by tac0turtle · Pull Request #4853 · tendermint/tendermint

tac0turtle · 2020-05-18T16:24:11Z

Description

these proto files are meant to help unblock ibc in their quest of migrating the ibc module to proto.

Closes: #XXX

auto-comment · 2020-05-18T16:24:15Z

👋 Thanks for creating a PR!

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.

Wrote tests
Updated CHANGELOG_PENDING.md
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Updated relevant documentation (docs/) and code comments
Re-reviewed Files changed in the Github PR explorer
Applied Appropriate Labels

Thank you for your contribution to Tendermint! 🚀

tac0turtle · 2020-05-19T09:22:15Z

@fedekunze @cwgoes does this look good for what is needed? I added all the types which are used in the light clients

codecov-commenter · 2020-05-19T09:39:59Z

Codecov Report

Merging #4853 into master will decrease coverage by 0.50%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4853      +/-   ##
==========================================
- Coverage   63.56%   63.06%   -0.51%     
==========================================
  Files         225      176      -49     
  Lines       21135    16018    -5117     
==========================================
- Hits        13435    10101    -3334     
+ Misses       6607     5091    -1516     
+ Partials     1093      826     -267

Impacted Files	Coverage Δ
libs/bits/bit_array.go	`81.14% <100.00%> (ø)`
blockchain/v0/pool.go	`78.66% <0.00%> (-0.32%)`	⬇️
node/id.go	`0.00% <0.00%> (ø)`
p2p/conn_set.go
consensus/ticker.go
crypto/secp256k1/secp256k1.go
p2p/codec.go
privval/retry_signer_client.go
consensus/codec.go
crypto/merkle/simple_proof.go
... and 58 more

fedekunze

LGTM in general. Thanks Marko. The only type missing is tmmath.Fraction. Can you add that too? 🙏

proto/blockchain/msgs.proto

proto/consensus/msgs.proto

proto/consensus/walmsgs.proto

proto/crypto/keys/types.proto

tac0turtle · 2020-05-20T08:21:43Z

The only type missing is tmmath.Fraction

I believe you will have to add this in the sdk. We do not encode tmmath.Fraction and I would want to avoid having prototypes for things that we do not encode.

erikgrinaker

LGTM

tac0turtle · 2020-05-22T10:23:02Z

found some bugs in this PR while thinking about proto yesterday. Writing some tests that help fix them

fedekunze · 2020-05-22T13:39:51Z

found some bugs in this PR while thinking

😲

types/evidence_test.go

types/evidence.go

types/block.go

melekes · 2020-05-25T07:05:51Z

types/block.go


+// ToProto converts SignedHeader to protobuf
+func (sh *SignedHeader) ToProto() *tmproto.SignedHeader {
+	if sh == nil {


is that even possible?

in usage, it may not be possible but while testing it was an issue so I decided to include it

melekes · 2020-05-25T07:06:02Z

types/block.go

+	}
+
+	psh := new(tmproto.SignedHeader)
+	if sh.Header != nil {


this is not possible imho (if Tendermint works correctly)

I do think having the case may be good. It may not be possible but to test this func and get the correct output I do need to check this

…rmint into marko/add_proto

melekes

👍

fedekunze · 2020-05-25T17:55:13Z

thanks @marbar3778! Will this PR be included in the next release?

tac0turtle · 2020-05-25T18:15:03Z

yes it will be. We will be working on creating a release in the coming days

these proto files are meant to help unblock ibc in their quest of migrating the ibc module to proto. Closes: #XXX

these proto files are meant to help unblock ibc in their quest of migrating the ibc module to proto.

egonspace · 2020-07-02T09:08:20Z

This PR is already done, but I have a question. ToProto() and FromProto() functions that convert between tmproto.state and state/State will be implemented later? I can't see them in this PR.

tac0turtle · 2020-07-02T09:12:24Z

yes they have been added. you can find them here:

tendermint/state/state.go

Line 133 in dedf0d2

func (state *State) ToProto() (*tmstate.State, error) {

egonspace · 2020-07-02T09:14:42Z

yes they have been added. you can find them here:

tendermint/state/state.go

Line 133 in dedf0d2

func (state *State) ToProto() (*tmstate.State, error) {

Oh, thank you. That's right.

add proto files

a5f07ed

tac0turtle self-assigned this May 18, 2020

tac0turtle added R:minor Release: Minor T:encoding Type: Amino, ProtoBuf labels May 18, 2020

tac0turtle added 4 commits May 19, 2020 10:49

add to/from proto funcs

e8f464d

add test for bits

eecf36b

fix lint

2cd0cdc

add to/from proto for val & valset

5abb3ff

tac0turtle marked this pull request as ready for review May 19, 2020 09:31

tac0turtle requested review from melekes and tessr as code owners May 19, 2020 09:31

tac0turtle requested review from erikgrinaker and fedekunze May 19, 2020 09:31

Merge branch 'master' into marko/add_proto

d2c270b

fedekunze suggested changes May 19, 2020

View reviewed changes

proto/blockchain/msgs.proto Show resolved Hide resolved

proto/consensus/msgs.proto Show resolved Hide resolved

proto/consensus/msgs.proto Show resolved Hide resolved

proto/consensus/walmsgs.proto Show resolved Hide resolved

proto/crypto/keys/types.proto Show resolved Hide resolved

tac0turtle added 2 commits May 20, 2020 11:38

add comments

d6466af

Merge branch 'master' into marko/add_proto

172449d

fedekunze approved these changes May 20, 2020

View reviewed changes

erikgrinaker approved these changes May 20, 2020

View reviewed changes

try bumping timeout

504accf

add tests for evideence to/from proto

4c0e1a8

Merge branch 'master' into marko/add_proto

1f56101

tac0turtle commented May 22, 2020

View reviewed changes

types/evidence_test.go Outdated Show resolved Hide resolved

melekes reviewed May 25, 2020

View reviewed changes

tac0turtle added 2 commits May 25, 2020 12:04

return error and add some more tests

cd19e94

Merge branch 'marko/add_proto' of https://github.com/tendermint/tende…

adbd30c

…rmint into marko/add_proto

melekes approved these changes May 25, 2020

View reviewed changes

Merge branch 'master' into marko/add_proto

76b95e3

tac0turtle added the S:automerge Automatically merge PR when requirements pass label May 25, 2020

Merge branch 'master' into marko/add_proto

626d835

mergify bot merged commit e03b61a into master May 25, 2020

mergify bot deleted the marko/add_proto branch May 25, 2020 15:52

tessr removed the R:minor Release: Minor label May 27, 2020

tac0turtle added a commit that referenced this pull request May 27, 2020

proto: add proto files for ibc unblock (#4853)

130800f

these proto files are meant to help unblock ibc in their quest of migrating the ibc module to proto. Closes: #XXX

tac0turtle added a commit that referenced this pull request May 27, 2020

proto: add proto files for ibc unblock (#4853)

50fcee9

these proto files are meant to help unblock ibc in their quest of migrating the ibc module to proto. Closes: #XXX

tac0turtle added a commit that referenced this pull request May 27, 2020

proto: add proto files for ibc unblock (#4853) (#4906)

8c35827

these proto files are meant to help unblock ibc in their quest of migrating the ibc module to proto.

tessr pushed a commit that referenced this pull request May 28, 2020

proto: add proto files for ibc unblock (#4853) (#4906)

d9c2f01

these proto files are meant to help unblock ibc in their quest of migrating the ibc module to proto.

zemyblue mentioned this pull request Jul 2, 2020

Apply Tendermint v0.33.5 Finschia/ostracon#99

Merged

5 tasks

Conversation

tac0turtle commented May 18, 2020

Description

Uh oh!

auto-comment bot commented May 18, 2020

Uh oh!

tac0turtle commented May 19, 2020

Uh oh!

codecov-commenter commented May 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fedekunze left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tac0turtle commented May 20, 2020

Uh oh!

erikgrinaker left a comment

Choose a reason for hiding this comment

Uh oh!

tac0turtle commented May 22, 2020

Uh oh!

fedekunze commented May 22, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

melekes May 25, 2020

Choose a reason for hiding this comment

Uh oh!

tac0turtle May 25, 2020

Choose a reason for hiding this comment

Uh oh!

melekes May 25, 2020

Choose a reason for hiding this comment

Uh oh!

tac0turtle May 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melekes left a comment

Choose a reason for hiding this comment

Uh oh!

fedekunze commented May 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tac0turtle commented May 25, 2020

Uh oh!

egonspace commented Jul 2, 2020

Uh oh!

tac0turtle commented Jul 2, 2020

Uh oh!

egonspace commented Jul 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov-commenter commented May 19, 2020 •

edited

Loading

tac0turtle May 25, 2020 •

edited

Loading

fedekunze commented May 25, 2020 •

edited

Loading