Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(network): encoding and decoding of peersharing peer address #589

Merged

Conversation

lancevincentsalera
Copy link
Contributor

REASON FOR FIX

Context

  • The peer client start the peersharing mini-protocol by sending MsgShareRequest to the peer server.
  • The peer client receives a message via MsgSharePeers that contains PeerAddresses from the peer server.
    • PeerAddress enum contains Ipv4 and Ipv6
  • The CDDL specification for peer addresses requires:
    • IPv4: An array [0, word32, portNumber]
    • IPv6: An array [1, word32, word32, word32, word32, flowInfo, scopeId, portNumber]
  • The original implementation decodes the IPv4 address as raw bytes instead of u32.
  • The original implementation did not include the extra fields flowInfo and scopeId for IPv6.
  • The original implementation did not segment and encode IPv6 into four 32-bit words as per the CDDL.

Issue

  • IPv4 Issue: The address was being decoded as raw bytes rather than as a single 32‑bit integer causing TypeMismatch. Encoding should also be tweaked as such to match the word32 in the network spec CDDL.
  • IPv6 Issue: The IPv6 address was not split into four 32‑bit words, and the additional required fields—flow_info and scope_id—were missing.
  • These issues led to incorrect binary representations during encoding/decoding, causing protocol mismatches.

Solution

IPv4 Encoding/Decoding:

  • Encoding: Use Ipv4Addr::to_bits() to convert the IPv4 address to a 32‑bit unsigned integer (word32) instead of encoding raw bytes.
  • Decoding: Read the u32 value and reconstruct the IPv4 address with Ipv4Addr::from_bits().
    IPv6 Encoding/Decoding:
  • Encoding: Convert the Ipv6Addr into a 128‑bit integer using Ipv6Addr::to_bits().
  • Split this 128‑bit value into four 32‑bit words (by shifting and masking).
  • Append the additional fields: flow_info and scope_id (both as u32) and the port.
  • Decoding: Read four u32 values, combine them into a u128 (using bit shifts), and reconstruct the IPv6 address via Ipv6Addr::from_bits().
  • Read the additional u32 values for flow_info and scope_id, as well as the port.
  • These changes ensure the encoded format strictly follows the CDDL.

Results

  • IPv4 Addresses: Now encoded as [0, word32, portNumber] where the word32 is the IPv4 address converted via to_bits().
  • IPv6 Addresses: Now encoded as [1, word0, word1, word2, word3, flowInfo, scopeId, portNumber], matching the CDDL exactly.
  • The network layer now correctly interprets and reconstructs peer addresses.
  • Peer discovery and overall protocol operations run as expected without mismatches.

Testing

  • Unit Tests: Added tests that:
    • Encode an IPv4 address, decode it back, and verify that the resulting Ipv4Addr matches the original.
    • Encode an IPv6 address along with flow_info, scope_id, and port; then decode and verify that all fields are correctly restored.
  • Utilized local pallas changes for Boros dependency and tested peersharing capabilities inside. New peers were discovered.
  • Manual Debugging: Verified (via logging and hex dumps) that the encoded and decoded output now exactly matches the CDDL specification.

@scarmuega scarmuega merged commit 510dcb7 into txpipe:main Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants