fix: support maximum block size in P2P sync - #3419
Conversation
📝 WalkthroughWalkthroughThe sync package now sets the messenger serialization limit to twice the default maximum blob size. A test verifies that a maximum-sized blob can be serialized in a header response. ChangesP2P message size support
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3419 +/- ##
==========================================
+ Coverage 62.37% 62.40% +0.02%
==========================================
Files 120 121 +1
Lines 13465 13467 +2
==========================================
+ Hits 8399 8404 +5
+ Misses 4125 4123 -2
+ Partials 941 940 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/sync/p2p_message_size_test.go`:
- Around line 30-33: Update the HeaderResponse fixture in the regression test to
set StatusCode to StatusCode_OK before calling response.Size() and
serde.Marshal, matching the successful go-header server serialization path.
In `@pkg/sync/p2p_message_size.go`:
- Around line 9-12: Remove the process-wide `serde.MaxMessageSize` increase from
init, keeping inbound HeaderRequest reads bounded to the request’s actual size.
Apply the larger limit only around HeaderResponse serialization, using the
relevant response-writing or serialization function and restoring the previous
limit afterward.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e4603f46-f05a-4eda-9afe-114743093fd6
📒 Files selected for processing (2)
pkg/sync/p2p_message_size.gopkg/sync/p2p_message_size_test.go
| response := &p2ppb.HeaderResponse{Body: body} | ||
| buf := make([]byte, response.Size()+binary.MaxVarintLen64) | ||
| _, err = serde.Marshal(response, buf) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Set StatusCode_OK in the regression fixture.
The successful go-header server path serializes HeaderResponse with StatusCode_OK. This fixture leaves StatusCode at zero, so response.Size() excludes a field present in production. Set the status before calculating the size and marshaling the response.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/sync/p2p_message_size_test.go` around lines 30 - 33, Update the
HeaderResponse fixture in the regression test to set StatusCode to StatusCode_OK
before calling response.Size() and serde.Marshal, matching the successful
go-header server serialization path.
| func init() { | ||
| // go-header wraps block data in another protobuf message, so leave room for | ||
| // framing overhead beyond the maximum block payload. | ||
| serde.MaxMessageSize = 2 * blobsize.DefaultMaxBlobSize |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'MaxMessageSize|serde\.(Read|Write|Marshal|Unmarshal)' --glob '*.go' .Repository: evstack/ev-node
Length of output: 1867
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Changed files/stat:\n'
git diff --stat || true
printf '\nRelevant files:\n'
fd -a 'p2p_message_size|blobsize|client\.(go|test\.go)|go.mod|go.sum' . | sed 's#^\./##'
printf '\ngo.mod dependency refs:\n'
rg -n 'go-libp2p-messenger|go-header|blobsize' go.mod go.sum || trueRepository: evstack/ev-node
Length of output: 2029
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'inspect p2p_message_size.go:\n'
cat -n pkg/sync/p2p_message_size.go 2>/dev/null || true
printf '\ninspect blobsize.go:\n'
fd -a 'blobsize.go' . | while read -r f; do
echo "--- ${f#"$PWD/"}"
cat -n "$f"
done
printf '\nprobe go-libp2p-messenger source if vendored/module cache available:\n'
go env GOMODCACHE GOPATH GOMOD 2>/dev/null | sed -n '1,20p'
go env GOMODCACHE 2>/dev/null | xargs -r find -path '*go-libp2p-messenger*' -maxdepth 4 -type d 2>/dev/null | head -20 || trueRepository: evstack/ev-node
Length of output: 1542
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gocache="$(go env GOMODCACHE 2>/dev/null)"
printf 'GOMODCACHE=%s\n' "$gocache"
printf '\npackage cache entries:\n'
find "${gocache:-/go/pkg/mod}" -type f \( -name 'serde.go' -o -name 'serdes.go' -o -name 'go.mod' \) \
| grep -E 'go-libp2p-messenger|go-header' | sort | head -50
printf '\nserde.go content if present:\n'
f="$(find "${gocache:-/go/pkg/mod}" -type f -path '*/go-libp2p-messenger@v0.2.2/serde/serde.go' | head -1 || true)"
if [ -n "${f:-}" ]; then
wc -l "$f"
sed -n '1,220p' "$f" | cat -n
fiRepository: evstack/ev-node
Length of output: 4349
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'go-header peer and messenger references:\n'
find "${GOMODCACHE:-/go/pkg/mod}/github.com/celestiaorg/go-header@v0.8.5" -maxdepth 5 -type f -name '*.go' -print0 \
| xargs -0 rg -n 'messenger|serde|NewLibP2P|HeaderRequest|HeaderResponse|Read\(|Write\(' || true
printf '\nSearch ev-node sync/p2p for go-header usage:\n'
rg -n -C 3 'go-header|HeaderRequest|HeaderResponse|Read\(|Write\(|p2p_message_size|MaxMessageSize' pkg go.mod --glob '*.go' || trueRepository: evstack/ev-node
Length of output: 36389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'go-libp2p-messenger serializer.go source:\n'
sed -n '1,220p' "${GOMODCACHE:-/go/pkg/mod}/github.com/celestiaorg/go-libp2p-messenger@v0.2.2/serde/serializer.go" | cat -n
printf '\ngo-libp2p-messenger version 0.2.2 serializer.go length-pooling evidence:\n'
python3 - <<'PY'
from pathlib import Path
p = Path.home() / 'go/pkg/mod/github.com/celestiaorg/go-libp2p-messenger@v0.2.2/serde/serializer.go'
if p.exists():
text = p.read_text()
print(f'exists={p}')
for name in ['func Read', 'func Write', 'func Marshal', 'MaxMessageSize', 'uvarintSize']:
print(f'{name}={name in text}')
# show key functions
for name in ['func Read', 'func Write', 'func Marshal']:
idx = text.find(f'func {name}')
if idx != -1:
start = text.rfind('\n', 0, idx-1)
end = text.find('\nfunc ', start+1)
print(f'\n-- {name} --')
print(text[start+1:end if end != -1 else len(text)])
PYRepository: evstack/ev-node
Length of output: 334
Avoid widening the process-wide serde limit before reading requests.
go-header reads inbound HeaderRequest with serde.Read, so this global setting also applies to requests. A peer can make go-libp2p-messenger allocate a buffer up to 2 * DefaultMaxBlobSize for the request length before HeaderRequest.Unmarshal rejects it. Keep serde.MaxMessageSize small enough for HeaderRequest, and widen the limit only where HeaderResponse serialization is constrained.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/sync/p2p_message_size.go` around lines 9 - 12, Remove the process-wide
`serde.MaxMessageSize` increase from init, keeping inbound HeaderRequest reads
bounded to the request’s actual size. Apply the larger limit only around
HeaderResponse serialization, using the relevant response-writing or
serialization function and restoring the previous limit afterward.
Source: MCP tools
2 participants
Overview
ev-node allows block payloads up to
DefaultMaxBlobSize(5 MiB by default), while the go-header exchange inherits go-libp2p-messenger's 1 MiB message limit. As a result, P2P historical sync resets the stream when it encounters an otherwise valid block larger than 1 MiB.This change configures the go-header framing limit to twice
DefaultMaxBlobSize, leaving room for the protobuf response envelope while keeping the limit derived from ev-node's existing block-size configuration.It also adds a regression test that serializes a maximum-size
P2PDatapayload through the sameHeaderResponseframing path used by go-header.Impact
Nodes can exchange and catch up through P2P when blocks exceed 1 MiB, up to ev-node's configured maximum block size.
Validation
go test ./... -count=1go vet ./pkg/sync ./pkg/p2pSummary by CodeRabbit