[release-v2.1] main: Use backported wire updates.#3658
Merged
davecgh merged 16 commits intodecred:release-v2.1from Apr 6, 2026
Merged
[release-v2.1] main: Use backported wire updates.#3658davecgh merged 16 commits intodecred:release-v2.1from
davecgh merged 16 commits intodecred:release-v2.1from
Conversation
This allows WriteHeaderN to avoid two unnecessary allocations by using the same buffer to encode both the message header and the encoded message payload. The payload is appended to zeroed-out header bytes in the buffer, before calculating the payload checksum and serializing the header to the bytes in the beginning of the buffer. This change also results in one less Write to the network connection.
When ReadMessage/ReadMessageN error due to the message having an invalid
network magic or invalid encoding of the command string, error immediately
instead of reading the remaining byte count reported in the message header.
There is no good reason to do so, and the explanation given by the
discardInput comment is bogus.
Unlike message serialization for generic purposes (via the BtcEncode method or
the Message interface), ReadMessage{,N} is intended to only be used for
network connections which have already negotiated a protocol version. Upon
any obvious error, the socket should just be closed (as dcrd's peer and
dcrwallet's p2p package both do) without reading any more input.
The previous behavior of discarding this input to read the next wire protocol
message is not documented by ReadMessage{,N}. Changing this is considered to
be a bug fix rather than a major API break.
This special cases writes to bytes.Buffer, which is always the writer type written to by WriteMessageN. There are several optimizations that can be implemented by special casing this type: First, pulling temporary short buffers from binary freelist can be skipped entirely, and instead the binary encoding of integers can be appended directly to its existing capacity. This avoids the synchronization cost to add and remove buffers from the free list, and for applications which only ever write wire messages with WriteMessageN, the allocation and ongoing garbage collection scanning cost to for these buffers can be completely skipped. Second, special casing the buffer type in WriteVarString saves us from creating a temporary heap copy of a string, as the buffer's WriteString method can be used instead. Third, special casing the buffer allows WriteMessageN to calculate the serialize size and grow its buffer so all remaining appends for writing block and transactions will not have to reallocate the buffer's backing allocation. This same optimization can be applied to other messages in the future.
This adds two additional type cases to the optimized writes for BLAKE-256 (used by transactions and the original PoW algorithm) and BLAKE-3 (used by the current PoW algorithm). It also updates both the block header and transaction code to use these hashers during the calculation of block and transaction hashes.
A previous commit which intended to remove the no longer used *[4]byte special case (for message header checksums) from writeElement inadvertently removed this from readElement instead. Remove it from the intended place, and restore the readElement case to avoid hitting the slow path.
This adds a couple of tests to ensure a couple of the newer error codes produce the expected human-readable output that were missed when adding them. It also adds a new define to the enum to count how many there are along with a test to detect missing entries.
This modifies the code that deals with serializing and deserializing the version message int64s to be more restrictive and reject any values that result in times that require special handling for comparisons. This clamping behavior is not strictly required since code that deals with the timestamps later is careful to avoid bad timestamps in general, but it safer to just reject them at the protocol level so even if code elsewhere is not taking extra precautions there would still not be any potential issues.
This commit contains several performance and memory optimizations to improve message deserialization, both when generically called through the BtcDecode method of the Message interface and when reading wire protocol using ReadMessageN. Special cases have been added for the bytes.Buffer and bytes.Reader reader types, avoiding the allocation and synchronization costs associated with using temporary buffers from the binary freelist and avoiding the heap allocations required due to the reader interface leaking the slice parameter. ReadMessageN has reduced the number of allocations it must incur by avoiding the readElements helper function. Finally, a new internal buffer type is introduced that is used exclusively by ReadMessageN. Since the lifetime and memory of this buffer is under the control of the wire package, and the buffer is never reset or truncated, this allows transaction script deserialization to slice the internal bytes of this buffer rather than pulling temporary buffers from the scriptPool freelist and copying these into a new contiguous allocation.
The code that handles deserializing transaction scripts by way of the free list cleans up in the event of error by returning the non-nil scripts to the free list to avoid leaking them. This is making an implicit (and undocumented) assumption that it is only deserializing into empty instances and therefore any non-nil scripts in the inputs and outputs in the event of a failed deserialization came from the free list. The consequence of that is that it is possible that any slices that were set by the caller prior to a failed deserialization could incorrectly be returned to the free list and ultimately get clobbered later. While this is not an issue for dcrd since it never deserializes into non-empty instances, there is no guarantee that is true for all callers. In order to ensure safety for all callers, this nils the input and output slices prior to deserializing anything in order to ensure the aforementioned assumption is always satisfied.
This avoids needing the fragile ErrUnexpectedEOF handling and prevents read errors when the reader is buffered and next chunk of bytes is only partially available. Discovered by a failing wallet unit test which passed in a reader created by hex.NewDecoder.
This adds some additional tests for short reads. - Adds cases from some missing paths in the tests for readElement - Adds another scenario when testing readElement that reader that only provides at most one byte per Read - Adds a new test name TestShortReads that exercise each of the individual readX funcs with the various supported readers as well as a couple of readers for the default path including a one byte reader Together these tests help ensure all of the new short read paths are fully exercised.
This retracts that 1.7.3 wire release, updates the wire module copyright year in the files modified since the previous wire/v1.7.2 release and serves as a base for wire/v1.7.4.
This updates the tests which exercise various read message error paths to improve their accuracy, modernize the error detection, and make them more consistent with the newer formatting practices.
Currently, message decoding correctly reads exactly the amount of bytes that are needed and silently ignores any remaining bytes. This is correct and expected behavior, however, the entire raw buffer is also passed through to rest of the application code unaltered (aka with the extra trailing bytes) for use in some very specific cases. While there are no serious consequences to this behavior currently, it is not ideal and could potentially lead to unexpected consequences in the future. With that in mind, this adds an additional safety check to reject any messages that are not fully consumed while decoding to prevent them immediately at the protocol level rather than leaving it to code at higher layers to deal with.
This updates the 2.1 release branch to use the latest version of the wire module which includes various optimizations, test improvements, hardening against potential misuse, and a tightening of the protocol to reject messages with trailing bytes. In particular, the following updated module version is used: - github.com/decred/dcrd/wire@v1.7.5
davecgh
changed the title
davecgh
changed the title
jrick
approved these changes
Apr 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This updates the 2.1 release branch to use the latest version of the
wiremodule which includes various optimizations, test improvements, hardening against potential misuse, and a tightening of the protocol to reject messages with trailing bytes.In particular, the following updated module version is used:
Note that it also cherry picks all of the relevant commits included in updates to the
wiremodule to ensure they are also included in the release branch even though it is not strictly necessary sincego.modhas been updated to require the new release and thus will pull in the new code. However, from past experience, not having code backported to modules available in the release branch too leads to headaches for devs building from source in their local workspace with overrides such asthose in
go.work.The following PRs are included: