Skip to content

fix(protobuf): tolerate empty custom fields#2789

Open
richard-ramos wants to merge 1 commit into
masterfrom
protobuf-decoder
Open

fix(protobuf): tolerate empty custom fields#2789
richard-ramos wants to merge 1 commit into
masterfrom
protobuf-decoder

Conversation

@richard-ramos

@richard-ramos richard-ramos commented Jul 8, 2026

Copy link
Copy Markdown
Member

Summary

Treat present-but-empty protobuf payloads for single-value custom MultiAddress, PeerId, PublicKey, and Signature fields as an unsuccessful field decode instead of raising from the custom decoder.

Non-empty malformed payloads still raise ProtobufValueError, preserving fatal decode behavior for corrupt field contents.

Affected Areas

  • Protocol Logic
    • Custom protobuf field decoding for multiaddresses, peer IDs, public keys, and signatures.

Impact on Library Users

Optional fields encoded as empty bytes are now handled like absent field values at the field decoder level, so they do not abort the surrounding protobuf decode solely because the optional field is empty.

Risk Assessment

Low. Successful decoding is unchanged, and malformed non-empty values continue to fail as before.

@richard-ramos richard-ramos force-pushed the protobuf-decoder branch 2 times, most recently from 8de3dc9 to 4f25aaa Compare July 8, 2026 18:14
@richard-ramos richard-ramos changed the title fix(protobuf): make custom decoders not throw errors fix(protobuf): tolerate empty custom fields Jul 8, 2026
@richard-ramos richard-ramos marked this pull request as ready for review July 8, 2026 18:18
@richard-ramos richard-ramos requested review from a team, gmelodie, rlve and vladopajic July 8, 2026 18:18
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.29%. Comparing base (057dfb3) to head (ef4de71).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
libp2p/peerid.nim 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2789      +/-   ##
==========================================
- Coverage   81.32%   81.29%   -0.03%     
==========================================
  Files         171      171              
  Lines       31013    31023      +10     
  Branches       12       12              
==========================================
  Hits        25221    25221              
- Misses       5792     5802      +10     
Files with missing lines Coverage Δ
libp2p/crypto/crypto.nim 64.68% <100.00%> (+0.22%) ⬆️
libp2p/multiaddress.nim 85.58% <100.00%> (-0.17%) ⬇️
libp2p/peerid.nim 90.19% <50.00%> (-0.36%) ⬇️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@richard-ramos richard-ramos enabled auto-merge July 8, 2026 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: new

Development

Successfully merging this pull request may close these issues.

4 participants