Skip to content

brontide: use tiered buffer pool for encrypted message bodies#10949

Open
kpshukla wants to merge 2 commits into
lightningnetwork:masterfrom
kpshukla:perf/brontide-tiered-buffer-pool
Open

brontide: use tiered buffer pool for encrypted message bodies#10949
kpshukla wants to merge 2 commits into
lightningnetwork:masterfrom
kpshukla:perf/brontide-tiered-buffer-pool

Conversation

@kpshukla

@kpshukla kpshukla commented Jul 3, 2026

Copy link
Copy Markdown

Summary

Fixes #10246.

bodyBufferPool (added in #10183) always allocates a max-size (65,551
byte) buffer for every outgoing message body, to avoid reallocation.
In practice, most LN wire messages (gossip updates, HTLC messages,
etc.) are well under that size, so the max-size allocation is wasted
memory for the common case.

This PR replaces the single pool with a tieredBufferPool: five
sync.Pools bucketed at 256B / 1KB / 4KB / 16KB / max
(maxMessageSize). WriteMessage already knows the plaintext length
up front, so it now requests a buffer sized to len(p) + macSize
instead of always taking the largest tier. Buffers are returned to the
pool matching their exact capacity; a buffer whose capacity doesn't
match any tier (shouldn't happen in practice) is discarded rather than
risk polluting a tier with an oddly-sized buffer.

headerBufferPool is untouched — encrypted headers are a fixed 18
bytes (encHeaderSize), so there's no benefit to tiering it.

Test plan

  • go build ./... — clean, no downstream breakage in packages that
    depend on brontide.
  • go test ./brontide/... — full existing suite passes unchanged,
    including:
    • TestConnectionCorrectness, TestConcurrentHandshakes
    • TestMaxPayloadLength (boundary at math.MaxUint16)
    • TestWriteMessageChunking, TestFlush (partial-write / resume paths)
    • TestBolt0008TestVectors (BOLT#8 spec vectors)
    • All Fuzz* seed corpus cases (FuzzRandomActOne/Two/Three,
      FuzzStatic*, FuzzRandom*EncDec, etc.)
  • Added TestTieredBufferPool covering tier selection at
    boundaries (exact match on a tier, rounds up to next tier, falls
    back to largest tier when the request exceeds every tier) and the
    discard-on-capacity-mismatch path.
  • gofmt -l and go vet ./brontide/... — clean.

kpshukla added 2 commits July 3, 2026 12:28
bodyBufferPool previously always handed out a max-size (65,551 byte)
buffer for every outgoing message, even though the vast majority of
wire messages (gossip updates, HTLC messages, etc.) are well under
that size. Replace it with a tiered pool (256B / 1KB / 4KB / 16KB /
max) so WriteMessage requests a buffer sized to the actual message
instead of always paying for the worst case.

headerBufferPool is left as-is since encrypted headers are a fixed 18
bytes and gain nothing from tiering.
@github-actions github-actions Bot added the severity-critical Requires expert review - security/consensus critical label Jul 3, 2026
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

🔴 PR Severity: CRITICAL

file classification | 3 files | 164 lines changed

🔴 Critical (1 file)
  • brontide/noise.go - Modifies the Noise protocol / encrypted peer connection implementation (brontide is listed as a CRITICAL package: encrypted peer connections, Noise protocol)
🟢 Low (2 files)
  • brontide/noise_test.go - Test-only change, excluded from severity-driving classification
  • docs/release-notes/release-notes-0.22.0.md - Release notes documentation update

Analysis

The only substantive (non-test, non-doc) file touched is brontide/noise.go, which implements lnd's Noise_XK-based transport encryption for peer connections. Per the severity rules, brontide/* is classified as CRITICAL since it directly affects the security of encrypted peer-to-peer communication. No severity bump was needed: excluding the test file and release notes, this PR touches 1 file and ~86 lines (75 additions / 11 deletions) in noise.go, well under the >20 file / >500 line bump thresholds, and it doesn't span multiple distinct critical packages. The accompanying test additions in noise_test.go are a good sign but don't reduce the need for expert review of the underlying Noise protocol logic change.


To override, add a severity-override-{critical,high,medium,low} label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

severity-critical Requires expert review - security/consensus critical

Projects

None yet

Development

Successfully merging this pull request may close these issues.

brontide: optimize brontide sync.Pool with tiered buffer sizes

1 participant