The Bitcoin wire protocol includes several fields with their lengths
encoded according to a variable length integer encoding scheme that does
not enforce a unique encoding for all numbers.
This can lead to a situation where deserializing and re-serializing the
same data can result in different bytes. There are no currently known
issues due to this, but it is safer to reject such subtle differences as
they could potentially lead to exploits.
Consequently, this commit modifies the varint decoding function to error
when the value is not canonically encoded which effectively means that
all messages with varints that are not canonically encoded will now be
rejected. This will not cause issues with old client versions in
regards to blocks and transactions since the data is deserialized into
memory and then reserialized before being relayed thereby effectively
erasing any non-canonical encodings.
Also, new tests have been added to ensure non-canonical encodings are
properly rejected and exercise the new code, and the default user agent
version for wire has been bumped to version 0.2.1 to differentiate the
new behavior.
The equivalent logic was implemented in Bitcoin Core by PR 2884.
This commit adds a new function which is similar to the DoubleSha256
function except it returns a ShaHash copy instead of a byte slice. It
also adds a new benchmark for it.
This can be a slight optimization in certain cases where the caller
ultimately wants a ShaHash since it can avoid a heap allocation and
additional copy to convert the result to a ShaHash (the function simply
performs a type cast against the returned array which is not possible
against a []byte).
existing: DoubleSha256 500000 3081 ns/op 32 B/op 1 allocs/op
new: DoubleSha256SH 500000 2939 ns/op 0 B/op 0 allocs/op
The hashing functions for blocks and transactions have also been updated
to make use of the new function since they directly return the ShaHash.
The transaction change in particular is quite useful since transactions
are frequently hashed and this change allows all of those hashes to avoid
an additional heap allocation.
This commit contains three classes of optimizations:
- Reducing the number of unnecessary hash copies
- Improve the performance of the DoubleSha256 function
- A couple of minor optimizations of the ShaHash functions
The first class is a result of the Bytes function on a ShaHash making a
copy of the bytes before returning them. It really should have been named
CloneBytes, but that would break the API now.
To address this, a comment has been added to the function which explicitly
calls out the copy behavior. In addition, all call sites of .Bytes on a
ShaHash in the code base have been updated to simply slice the array when
a copy is not needed. This saves a significant amount of data copying.
The second optimization modifies the DoubleSha256 function to directly use
fastsha256.Sum256 instead of the hasher interface. This reduces the
number of allocations needed. A benchmark for the function has been added
as well.
old: BenchmarkDoubleSha256 500000 3691 ns/op 192 B/op 3 allocs/op
new: BenchmarkDoubleSha256 500000 3081 ns/op 32 B/op 1 allocs/op
The final optimizations are for the ShaHash IsEqual and SetBytes functions
which have been modified to make use of the fact the type is an array and
remove an unneeded subslice.
This commit contains the entire btcwire repository along with several
changes needed to move all of the files into the wire directory in
order to prepare it for merging. This does NOT update btcd or any of the
other packages to use the new location as that will be done separately.
- All import paths in the old btcwire test files have been changed to the
new location
- All references to btcwire as the package name have been chagned to
wire
- The coveralls badge has been removed since it unfortunately doesn't
support coverage of sub-packages
This is ongoing work toward #214.
