Conversation
also make read only function use const pointer Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for ENR (Ethereum Node Record) conversion to peer identity and multiaddress handling, enhances the API with safer method signatures using const pointers, and introduces new functionality for QUIC protocol addresses. The changes also improve code modularity by separating file I/O operations from generic reader/writer functions.
Key changes:
- Added peer ID and multiaddress conversion methods for all ENR types
- Introduced QUIC protocol support alongside existing UDP functionality
- Refactored method signatures to use const pointers for read-only operations
- Split file operations into generic reader/writer functions for better modularity
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/root.zig | Refactored file I/O operations into generic reader/writer functions and updated method calls to use new naming conventions |
| src/enr.zig | Added peer ID/multiaddress conversion methods, QUIC support, and changed method signatures to use const pointers for safety |
| build.zig.zon | Added peer-id dependency for peer identity functionality |
| build.zig | Integrated peer-id and multiformats dependencies into the build system |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| defer file.close(); | ||
| /// Generic function to read multiple ENRs from any reader | ||
| pub fn readMultipleENRs(reader: anytype, enr_list: *std.ArrayList(ENR), delimiter: []const u8) !void { | ||
| const content = try reader.readAllAlloc(enr_list.allocator, std.math.maxInt(usize)); |
There was a problem hiding this comment.
Using std.math.maxInt(usize) as the maximum read size could lead to memory exhaustion attacks. Consider setting a reasonable upper limit for file size to prevent potential DoS vulnerabilities.
| defer file.close(); | ||
| /// Generic function to read multiple EncodedENRs from any reader | ||
| pub fn readMultipleEncodedENRs(reader: anytype, enr_list: *std.ArrayList(EncodedENR), delimiter: []const u8) !void { | ||
| const content = try reader.readAllAlloc(enr_list.allocator, std.math.maxInt(usize)); |
There was a problem hiding this comment.
Using std.math.maxInt(usize) as the maximum read size could lead to memory exhaustion attacks. Consider setting a reasonable upper limit for file size to prevent potential DoS vulnerabilities.
Signed-off-by: Chen Kai <281165273grape@gmail.com>
| } | ||
|
|
||
| pub fn getQUIC(self: *const Self) !?u16 { | ||
| if (self.get("quic")) |udp_bytes| { |
There was a problem hiding this comment.
This method is reading from a "quic" key but the variable is named udp_bytes, is this desired?
… char Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
Signed-off-by: Chen Kai <281165273grape@gmail.com>
g11tech
left a comment
g11tech
left a comment
There was a problem hiding this comment.
lgtm but @GrapeBaBa we should join ENR, SignableENR and the RLP Enr into just one ENR structure as union over these three tags, so that we dont have to duplicate the functions again and again and do thing zig native way
Which duplicate functions you mean, in the If you mean in the |
Uh oh!
There was an error while loading. Please reload this page.