feat: implement snappy for gossip objects follow the spec#240
feat: implement snappy for gossip objects follow the spec#240g11tech merged 2 commits intoblockblaz:mainfrom
Conversation
Signed-off-by: Chen Kai <281165273grape@gmail.com>
GrapeBaBa
requested review from
RazorClient,
anshalshukla,
bhaskar1001101,
bomanaps,
chetanyb,
Copilot,
g11tech,
gballet and
spiral-ladder
and removed request for
Copilot and
g11tech
There was a problem hiding this comment.
Pull Request Overview
This PR introduces Snappy compression support for gossip objects in the ethlibp2p network module to follow the specification. The implementation adds compression for outgoing messages and decompression for incoming messages, improving network efficiency and robustness.
- Integrates the
zig-snappydependency for compression/decompression functionality - Updates message handling to compress outgoing gossip messages before publishing
- Adds decompression of incoming messages with proper error handling and debug logging
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkgs/network/src/ethlibp2p.zig | Implements Snappy compression/decompression for gossip message handling |
| build.zig.zon | Adds zig-snappy dependency configuration |
| build.zig | Integrates snappyz module into the build system |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| defer serialized.deinit(); | ||
| try ssz.serialize(types.SignedBeamBlock, data.block, &serialized); | ||
|
|
||
| break :blockbytes serialized.items; | ||
| break :blockbytes try serialized.toOwnedSlice(); | ||
| }, | ||
| .vote => votebytes: { | ||
| var serialized = std.ArrayList(u8).init(self.allocator); | ||
| defer serialized.deinit(); |
There was a problem hiding this comment.
Using defer serialized.deinit() followed by serialized.toOwnedSlice() is incorrect. The toOwnedSlice() method transfers ownership of the memory to the caller and clears the ArrayList, so the subsequent deinit() call will attempt to free already-transferred memory. Remove the defer serialized.deinit() line since toOwnedSlice() handles the cleanup.
There was a problem hiding this comment.
that is wrong, toOwnedSlice() allocated a new memory
There was a problem hiding this comment.
actually toOwnedSlice does clear the memory as well so deinit is unnecessary but not harmful either
There was a problem hiding this comment.
Yes, got it. unnecessary to call
| defer serialized.deinit(); | ||
| try ssz.serialize(types.SignedBeamBlock, data.block, &serialized); | ||
|
|
||
| break :blockbytes serialized.items; | ||
| break :blockbytes try serialized.toOwnedSlice(); | ||
| }, | ||
| .vote => votebytes: { | ||
| var serialized = std.ArrayList(u8).init(self.allocator); | ||
| defer serialized.deinit(); |
There was a problem hiding this comment.
Using defer serialized.deinit() followed by serialized.toOwnedSlice() is incorrect. The toOwnedSlice() method transfers ownership of the memory to the caller and clears the ArrayList, so the subsequent deinit() call will attempt to free already-transferred memory. Remove the defer serialized.deinit() line since toOwnedSlice() handles the cleanup.
There was a problem hiding this comment.
that is wrong, toOwnedSlice() allocated a new memory
This pull request introduces Snappy compression support for network message handling in the
ethlibp2pZig module. The main changes include integrating thezig-snappydependency, updating the build system to support it, and modifying message serialization and deserialization to use Snappy compression and decompression. This improves network efficiency and robustness by compressing outgoing messages and handling decompression errors gracefully.Fix #219