Cap'n Proto example #5438
Cap'n Proto example #5438
Conversation
|
This pull request introduces 1 alert when merging a83e9c9 into 876beef - view on LGTM.com new alerts:
|
|
Update, I think I figured out how generics work... not super crazy just a learning curve and some boilerplate I think... Not intending to work on this PoC anymore unless there's clear will around pursuing this approach more... Which should probably be more related to fundamental properties of the encoding than how hard/easy it is to migrate. |
| struct StdTx(Msg, PubKey) { | ||
| msg @0 :Msg; | ||
| fee @1 :StdFee; | ||
| signatures @2 :List(StdSignature(PubKey)); |
There was a problem hiding this comment.
Wow, these generics are great for expressing data structures, so much more sophisticated than protobuf
| @@ -212,4 +212,11 @@ devdoc-clean: | |||
| devdoc-update: | |||
| docker pull tendermint/devdoc | |||
|
|
|||
| CAPNP_COMPILE = capnp compile -I$(HOME)/go/pkg/mod/github.com/capnproto/[email protected]+incompatible/std -I. -ogo | |||
There was a problem hiding this comment.
Where does capnp come from? Please add it to make tools
| @@ -28,6 +28,7 @@ require ( | |||
| github.com/tendermint/tendermint v0.32.8 | |||
| github.com/tendermint/tm-db v0.2.0 | |||
| gopkg.in/yaml.v2 v2.2.7 | |||
| zombiezen.com/go/capnproto2 v2.18.0+incompatible | |||
There was a problem hiding this comment.
What's up with the two capnproto libs and the +incompatible flag?
I looked it up, and see it is a way to deal with libs that predate modules.
https://github.com/capnproto/go-capnproto2 seems to have go.sum, but not go.mod. If we go with this, seems like a nice PR to clean up those dependenices.
|
|
||
| import "fmt" | ||
|
|
||
| func CoinFromCoinE(e CoinE) (coin Coin, err error) { |
There was a problem hiding this comment.
I like this approach.
We can
(1) define powerful, deterministic zero-copy serialization format with cap'n proto and
(2) define functions to convert these cap'n proto structs to any custom go struct we want (or any custom js struct, etc)
The overhead should be comparable to parsing the data with another codec and we can use language-specific idioms.
There was a problem hiding this comment.
The downside is that this requires a conversion function to be written for each struct that we want to use as a normal struct in code. This is not too much work, but is boilerplate that devs abhor.
This function could have been created by codegen and maybe enhancing that codegen would make this super-smooth.
We could use cap'n proto for serialization and canonical signing. Then write custom code to parse it into convenient structs to work with. Once we are happy with these hand-crafted conversion functions and the only complaint is the hand-coding of them, we could automate them with codegen - but only when we are all clear on what we want.
| @@ -0,0 +1,32 @@ | |||
| @0xb02b1ca217480de7; | |||
|
|
|||
| using Go = import "/go.capnp"; | |||
There was a problem hiding this comment.
This is a very nice syntax, but does this break compilation in other languages?
Will we need to add these little language-specific headers for each language we want to support?
Or copy/modify the .capnp files for each repo.
This is not horrible but does make it a bit harder for a script to ensure the codecs are identical
|
|
||
| struct Msg(Content) { | ||
| msgDeposit @0 :MsgDeposit; | ||
| msgSubmitProposal @1 :MsgSubmitProposal; |
There was a problem hiding this comment.
Shouldn't this be MsgSubmitProposal(Content)?
|
|
||
| using SDK = import "/types/codec.capnp"; | ||
|
|
||
| struct Msg(Content) { |
There was a problem hiding this comment.
Shouldn't this be a union type?
As written, it seems like it may contain a deposit, a proposal, and a vote.
Is that wanted?
| return sdk.ErrInvalidAddress(msg.Proposer.String()) | ||
| var proposer sdk.AccAddress | ||
| proposer, err = msg.Proposer() | ||
| if err != nil { |
There was a problem hiding this comment.
Huh, these err != nil checks are quite annoying. Otherwise, this use of a cap'n proto native struct doesn't seem so much worse than a normal struct (dev-ux-wise).
This is unavoidable, as Unmarshal usually returns an error for parse error, and we cannot actually validate all fields until we read them (as there is no parsing phase). I am wondering on how to make this simpler for the developer, short of the go 2 proposal for check keyword. Note that if cross-language support is a central concern, this issue is irrelevant in eg. rust or javascript. Javascript will throw and exception, and Rust will return a result, which can be short-circuited by simply appending ? (even easier than check).
If we do the example like CoinsEToCoins, then that conversion becomes a parse function, which may return an error (naturally), but the rest of the usage of the struct must not worry about any errors, as all elements have already been parsed. In the sdk use case, it is highly unlikely we will ignore fields and never parse them (unlike some streaming api use cases), so the "parse function" (also mentioned above) seems like a good way to handle the ergonomics.
@alexanderbez @jordansexton @marbar3778
Here is where I've gotten so far with cap'n proto. It's probably most interesting to look at the
.capnpfiles to see how their generics work compared to the way things need to be done with protobuf. There is something slightly more elegant about this approach for handling interfaces. But I think it's a relatively minor upgrade... and wouldn't be any to convince me of this approach vs protobuf.It also may be useful to check out the gov
msgs.goexample uses - currently their generated code includes an error on all the getters although the example code I saw doesn't... Not sure if I have the wrong version or something. It's not too big a deal but a little annoying to deal with everything maybe returning an error. Also, I haven't figured out quite how to use the generics from code yet because go doesn't support generics. If I figure it out, I'll update this.Overall, I think this approach is likely workable - a bit more of a learning curve but haven't seem any total blockers yet.