From ae1fc74f80fa4685e54023c73af02f6c7e2dd650 Mon Sep 17 00:00:00 2001 From: Thane Thomson Date: Thu, 11 Aug 2022 13:30:39 -0400 Subject: [PATCH] abci: Make ABCI and P2P wire-level length delimiters consistent (#9182) * abci: use protoio for length delimitation (#5818) Migrate ABCI to use protoio (uint64 length delimiters) instead of int64 length delimiters to be consistent with the approach used in the P2P layer. Closes: #5783 * Import ReadMsg interface change from #5868 Signed-off-by: Thane Thomson * Convert PR number to link in UPGRADING Signed-off-by: Thane Thomson * Update Tendermint Socket Protocol docs to reflect length prefix encoding change Signed-off-by: Thane Thomson * Clarify that length delimiters are varints Signed-off-by: Thane Thomson Signed-off-by: Thane Thomson Co-authored-by: Marko --- CHANGELOG_PENDING.md | 3 ++- UPGRADING.md | 15 ++++++++--- abci/types/messages.go | 53 +++++--------------------------------- spec/abci/client-server.md | 35 +++++++++++++------------ 4 files changed, 39 insertions(+), 67 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index f343c9b92..6ddb8f795 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -14,12 +14,13 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [abci/counter] \#6684 Delete counter example app - [txResults] \#9175 Remove `gas_used` & `gas_wanted` from being merkelized in the lastresulthash in the header + - [abci] \#5783 Make length delimiter encoding consistent (`uint64`) between ABCI and P2P wire-level protocols - P2P Protocol - Go API - - [all] [#9144] Change spelling from British English to American (@cmwaters) + - [all] \#9144 Change spelling from British English to American (@cmwaters) - Rename "Subscription.Cancelled()" to "Subscription.Canceled()" in libs/pubsub - Blockchain Protocol diff --git a/UPGRADING.md b/UPGRADING.md index 617cf5050..1010fae46 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,15 +1,22 @@ # Upgrading Tendermint Core -This guide provides instructions for upgrading to specific versions of Tendermint Core. - +This guide provides instructions for upgrading to specific versions of +Tendermint Core. ## v0.37 (Unreleased) -This version requires a coordinated network upgrade. It alters the elements in the predigest of the `LastResultsHash` and thus -all nodes must upgrade together (see #9175). +This version requires a coordinated network upgrade. It alters the elements in +the predigest of the `LastResultsHash` and thus all nodes must upgrade together +(see [\#9175](https://github.com/tendermint/tendermint/pull/9175)). NOTE: v0.35 was recalled and v0.36 was skipped +### ABCI Changes + +* In v0.34, messages on the wire used to be length-delimited with `int64` varint + values, which was inconsistent with the `uint64` varint length delimiters used + in the P2P layer. Both now consistently use `uint64` varint length delimiters. + ## v0.34.20 ### Feature: Priority Mempool diff --git a/abci/types/messages.go b/abci/types/messages.go index 531f75fed..0686b6b25 100644 --- a/abci/types/messages.go +++ b/abci/types/messages.go @@ -1,11 +1,10 @@ package types import ( - "bufio" - "encoding/binary" "io" "github.com/gogo/protobuf/proto" + "github.com/tendermint/tendermint/libs/protoio" ) const ( @@ -14,57 +13,19 @@ const ( // WriteMessage writes a varint length-delimited protobuf message. func WriteMessage(msg proto.Message, w io.Writer) error { - bz, err := proto.Marshal(msg) + protoWriter := protoio.NewDelimitedWriter(w) + _, err := protoWriter.WriteMsg(msg) if err != nil { return err } - return encodeByteSlice(w, bz) + + return nil } // ReadMessage reads a varint length-delimited protobuf message. func ReadMessage(r io.Reader, msg proto.Message) error { - return readProtoMsg(r, msg, maxMsgSize) -} - -func readProtoMsg(r io.Reader, msg proto.Message, maxSize int) error { - // binary.ReadVarint takes an io.ByteReader, eg. a bufio.Reader - reader, ok := r.(*bufio.Reader) - if !ok { - reader = bufio.NewReader(r) - } - length64, err := binary.ReadVarint(reader) - if err != nil { - return err - } - length := int(length64) - if length < 0 || length > maxSize { - return io.ErrShortBuffer - } - buf := make([]byte, length) - if _, err := io.ReadFull(reader, buf); err != nil { - return err - } - return proto.Unmarshal(buf, msg) -} - -//----------------------------------------------------------------------- -// NOTE: we copied wire.EncodeByteSlice from go-wire rather than keep -// go-wire as a dep - -func encodeByteSlice(w io.Writer, bz []byte) (err error) { - err = encodeVarint(w, int64(len(bz))) - if err != nil { - return - } - _, err = w.Write(bz) - return -} - -func encodeVarint(w io.Writer, i int64) (err error) { - var buf [10]byte - n := binary.PutVarint(buf[:], i) - _, err = w.Write(buf[0:n]) - return + _, err := protoio.NewDelimitedReader(r, maxMsgSize).ReadMsg(msg) + return err } //---------------------------------------- diff --git a/spec/abci/client-server.md b/spec/abci/client-server.md index 78c226743..1ffbe2e43 100644 --- a/spec/abci/client-server.md +++ b/spec/abci/client-server.md @@ -66,25 +66,28 @@ Note the length-prefixing used in the socket implementation (TSP) does not apply ### TSP -Tendermint Socket Protocol is an asynchronous, raw socket server which provides ordered message passing over unix or tcp. -Messages are serialized using Protobuf3 and length-prefixed with a [signed Varint](https://developers.google.com/protocol-buffers/docs/encoding?csw=1#signed-integers) +Tendermint Socket Protocol is an asynchronous, raw socket server which provides +ordered message passing over unix or tcp. Messages are serialized using +Protobuf3 and length-prefixed with an [unsigned +varint](https://developers.google.com/protocol-buffers/docs/encoding?csw=1#varints) -If GRPC is not available in your language, or you require higher -performance, or otherwise enjoy programming, you may implement your own -ABCI server using the Tendermint Socket Protocol. The first step is still to auto-generate the relevant data -types and codec in your language using `protoc`. In addition to being proto3 encoded, messages coming over -the socket are length-prefixed to facilitate use as a streaming protocol. proto3 doesn't have an -official length-prefix standard, so we use our own. The first byte in -the prefix represents the length of the Big Endian encoded length. The -remaining bytes in the prefix are the Big Endian encoded length. +If GRPC is not available in your language, or you require higher performance, or +otherwise enjoy programming, you may implement your own ABCI server using the +Tendermint Socket Protocol. The first step is still to auto-generate the +relevant data types and codec in your language using `protoc`. In addition to +being proto3 encoded, messages coming over the socket are length-prefixed to +facilitate use as a streaming protocol. proto3 doesn't have an official +length-prefix standard, so we use our own. The first byte in the prefix +represents the length of the Big Endian encoded length. The remaining bytes in +the prefix are the Big Endian encoded length. -For example, if the proto3 encoded ABCI message is 0xDEADBEEF (4 -bytes), the length-prefixed message is 0x0104DEADBEEF. If the proto3 -encoded ABCI message is 65535 bytes long, the length-prefixed message -would be like 0x02FFFF.... +For example, if the proto3 encoded ABCI message is 0xDEADBEEF (4 bytes), the +length-prefixed message is 0x0104DEADBEEF. If the proto3 encoded ABCI message is +65535 bytes long, the length-prefixed message would be like 0x02FFFF.... -The benefit of using this `varint` encoding over the old version (where integers were encoded as `` is that -it is the standard way to encode integers in Protobuf. It is also generally shorter. +The benefit of using this `varint` encoding over the old version (where integers +were encoded as `` is that it is the common way to +encode integers in Protobuf. It is also generally shorter. As noted above, this prefixing does not apply for GRPC.