Speed up TLV serialization length accounting#4642
Open
joostjager wants to merge 3 commits into
Open
Conversation
Add an environment-variable switch that lets stdin fuzz targets use the dev-null test logger. This keeps direct invocations verbose by default, while external runners can opt into quieter passing-case replays.
|
👋 Thanks for assigning @wpaulino as a reviewer! |
Collaborator
|
No issues found. I performed a thorough line-by-line review of all substantive changes: Serialization macros (
DevNull logger (
Fuzz targets (~60 files + template):
|
Add direct serialized length implementations for common serialization wrappers and use direct BigSize length accounting when computing TLV record lengths. This avoids routing length calculations back through in-memory writers for common nested serialization paths. Cover the BigSize boundary values used by TLV record length counting by comparing the direct calculation against actual serialized TLV records.
joostjager
force-pushed
the
ser-tlv-length-fastpath
branch
from
7afe504 to
084d0ac
Compare
wpaulino
reviewed
May 28, 2026
| } } | ||
| } | ||
|
|
||
| macro_rules! impl_writeable_tlv_fields { |
Contributor
There was a problem hiding this comment.
Since we already have existing macros that implement both Writeable and Readable with the name impl_writeable_*, we might want to rename this slightly
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
Add a write-only TLV helper macro that emits both write and serialized_length from the same field list. Reuse the shared TLV length helper from impl_writeable_tlv_based so the existing generated writer path and the new custom-read path stay aligned. Use the new helper for the hot channel funding and commitment transaction TLV writers while leaving their custom read implementations unchanged.
joostjager
force-pushed
the
ser-tlv-length-fastpath
branch
from
084d0ac to
b0e52bd
Compare
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
4 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.
Recent splice fuzzing coverage made it clear that some chanmon consistency inputs spend a surprising amount of time encoding snapshots. This PR tightens the serialization length path used by those snapshots.
Slow fuzz byte string used while checking this:
a1a0a3ffa2a1ffffa2a1ff00a2a1ffa2a1a0a3ffa2a1ff19ffffffffffffffacFor that byte string, median real time over 5 runs with target output suppressed: