Conversation
Codecov Report
@@ Coverage Diff @@
## sdk2 #167 +/- ##
==========================================
- Coverage 10.68% 10.26% -0.42%
==========================================
Files 12 12
Lines 2172 2163 -9
==========================================
- Hits 232 222 -10
- Misses 1924 1925 +1
Partials 16 16
Continue to review full report at Codecov.
|
| Data: []byte("hello"), | ||
| Gas: 43, | ||
| Fee: 12, | ||
| Fee: KI64Pair{[]byte("pho"), 12}, |
odeke-em
left a comment
There was a problem hiding this comment.
LGTM but I don't understand how the protobuf-c null terminator string problem affects us in Go.
|
It doesn't affect us but it will affect anyone writing C applications, if we ever put a 0x00 byte in the KVPair "string". |
|
Gotcha gotcha, great point. |
|
Presumably the idea here is to put the coin denomination in as the key? I think part of the motivation for the previous KVPair structure was to enable DeliverTx tags to have values that are either strings or integers - this change would force them to be strings (well, bytes, but same same). Is that what we want ? Otherwise this looks good, except that we may actually want int128 ... |
|
The pubsub query could include casting of bytes into anything... so maybe for comparison you need to first cast it like "int64(value) < 123" or something. @melekes |
|
yeah, we'll need to cast before making a comparison https://github.com/tendermint/tmlibs/blob/master/pubsub/query/query.go#L228-L249 |
-> Unable to correctly deserialize strings containing null characters protobuf-c/protobuf-c#203
-> We already use KVPair in several places and it's always been {[]byte, []byte} (e.g. we should refactor to eventually just use tmlibs/common/KVPair. Using gogoproto we can do that now).
-> Protobuf3 strings are supposed to be only utf8 or 7-bit ansii but for something called "KVPair", we don't want to be restricted to unicode or ansii characters.
-> Perhaps using []byte makes us more vigilant in validating the input before accepting it, than be lulled into complacency w/ a string.
So this PR simplifies KVPair to just be ([]byte, []byte), and mayb we can do another PR to just use tmlibs/common.KVPair instead, or do it here.
It also introduces a more restrictive type, KI64Pair. We can later introduce KI32Pair, KU64Pair, etc.